Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate lock times in the transaction verifier #2389

Closed
teor2345 opened this issue Jun 24, 2021 · 0 comments · Fixed by #3060
Closed

Validate lock times in the transaction verifier #2389

teor2345 opened this issue Jun 24, 2021 · 0 comments · Fixed by #3060
Assignees
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter) S-needs-spec-update Status: Not in the Zcash spec, but it should be

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jun 24, 2021

Motivation

Zebra's transaction verifier needs to check the Transaction lock_time field.

Specifications

The lock_time (or nLockTime) and tx_in.sequence fields are defined in the transaction encoding specification.

https://zips.z.cash/protocol/protocol.pdf#consensusfrombitcoin

The transaction must be finalized: either its locktime must be in the past (or less than or equal to the current block height), or all of its sequence numbers must be 0xffffffff.

https://developer.bitcoin.org/devguide/transactions.html#non-standard-transactions

Zcash-specific clarification:

Lock times are only enabled if there is at least one sequence number that is not u32::MAX. So transactions with no transparent inputs are always accepted, regardless of the value of the lock time field.

zcash/zips#539

Interaction with Input Sequence Numbers

[There] are four-byte sequence numbers in every input. Sequence numbers were meant to allow multiple signers to agree to update a transaction; when they finished updating the transaction, they could agree to set every input’s sequence number to the four-byte unsigned maximum (0xffffffff), allowing the transaction to be added to a block even if its time lock had not expired.

Even today, setting all sequence numbers to 0xffffffff (the default in Bitcoin Core) can still disable the time lock, so if you want to use locktime, at least one input must have a sequence number below the maximum. Since sequence numbers are not used by the network for any other purpose, setting any sequence number to zero is sufficient to enable locktime.

Locktime itself is an unsigned 4-byte integer which can be parsed two ways:

If less than 500 million, locktime is parsed as a block height. The transaction can be added to any block which has this height or higher.

If greater than or equal to 500 million, locktime is parsed using the Unix epoch time format (the number of seconds elapsed since 1970-01-01T00:00 UTC—currently over 1.395 billion). The transaction can be added to any block whose block time is greater than the locktime.

https://developer.bitcoin.org/devguide/transactions.html#locktime-and-sequence-number

Clarification:

Coinbase inputs have a sequence number, and coinbase transaction lock times are checked just like any other transaction.

zcash/zips#539

Interaction with Expiry Height

This rule is in ZIP 203, but we don't think it was ever implemented in zcashd:
If used in combination with nLockTime, both nLockTime and nExpiryHeight must be block heights.

Designs

Zebra already parses the lock time:

/// Get this transaction's lock time.
pub fn lock_time(&self) -> LockTime {

pub enum LockTime {
/// Unlock at a particular block height.
Height(block::Height),
/// Unlock at a particular time.
Time(DateTime<Utc>),
}

And input sequence numbers:

pub enum Input {
/// A reference to an output of a previous transaction.
PrevOut {
/// The previous output transaction reference.
outpoint: OutPoint,
/// The script that authorizes spending `outpoint`.
unlock_script: Script,
/// The sequence number for the output.
sequence: u32,
},
/// New coins created by the block reward.
Coinbase {
/// The height of this block.
height: block::Height,
/// Free data inserted by miners after the block height.
data: CoinbaseData,
/// The sequence number for the output.
sequence: u32,
},
}

But lock_time returns zero lock times as Height(0), rather than None.

Similarly, Zebra does not check sequence numbers. If there are no sequence numbers, or they are all u32::MAX, lock_time should return None.

Missing Information in Transaction Verifier Request

The block time is not currently sent as part of the transaction verifier request.

@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Medium labels Jun 24, 2021
@teor2345 teor2345 added this to the 2021 Sprint 15 milestone Jun 24, 2021
@mpguerra mpguerra added the NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter) label Jun 24, 2021
@teor2345 teor2345 added the S-needs-spec-update Status: Not in the Zcash spec, but it should be label Jun 24, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Jun 28, 2021
@teor2345 teor2345 added the NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) label Jul 13, 2021
@mpguerra mpguerra assigned jvff and unassigned jvff Nov 9, 2021
@jvff jvff self-assigned this Nov 10, 2021
@mpguerra mpguerra linked a pull request Nov 18, 2021 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter) S-needs-spec-update Status: Not in the Zcash spec, but it should be
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants