Validate transaction lock times

[jvff]

Motivation

Transactions can have lock times configured so that they are only included in a block after a certain block height or date and time. This is a consensus rule inherited from Bitcoin and that Zebra must validate.

Specifications

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

Solution

• Validate if the sequence numbers enable the transaction's lock time in Transaction::lock_time getter
  • If the lock time is disabled (or zero), None is returned
• Validate the lock time in a new function in the zebra_consensus::transaction::check module
  • That function is called by the transaction verifier on all transactions

Review

@teor2345 and/or @oxarbitrage can review this. This is still in draft because I have only outlined the tests I want to add, but I haven't finished writing them yet.

Reviewer Checklist

• Code implements Specs and Designs
• Tests for Expected Behaviour
• Tests for Errors

Follow Up Work

[teor2345]

I think the Bitcoin docs are wrong about the height check.

Also, we need to be careful to use the correct time field and comparison operators.

[jvff]

Updated. Difference since last review can be seen here.

I still haven't finished the tests, I'll try to do that on Monday.

I'm not sure how to handle the mempool transactions. For the block height, I think it's currently using the current tip height plus one:

zebra/zebrad/src/components/mempool/downloads.rs

Line 296 in 0960e4f

let height = (height + 1).expect("must have next height");

Should we add a tolerance of a few extra blocks?

As for the block time, I think the options are:

• only unlocked transactions are accepted in the mempool,
• locked transactions are accepted and stay in the mempool until they are unlocked (however long that may take),
• only transactions that will be unlocked in at most a future time are accepted in the mempool.

The current implementation uses the third option using the local node time plus a two hours margin of error between the local node time and the network's block time, plus a tolerance of an hour. Should I use the state service in the mempool downloader to get the block time and pass it in the request instead of using the local time in the verifier itself?

[teor2345]

Let's try to limit the scope of this PR to essential NU5 tasks.
We can schedule extra tests or mempool changes in future sprints.

I'm not sure how to handle the mempool transactions. For the block height, I think it's currently using the current tip height plus one:

zebra/zebrad/src/components/mempool/downloads.rs

Line 296 in 0960e4f

let height = (height + 1).expect("must have next height");

Can we split mempool validation into a separate PR?
It's not required for NU5.

The mempool and lock time rules are completely undocumented for Zcash. The mempool is not consensus-critical, but we want to avoid usability bugs where Zebra rejects transactions that zcashd accepts. So it is better to skip mempool rules.

In that PR, we should consider:

• What's the goal here?
• What rules are we actually implementing?

I think zcashd uses the block height and median time past of the current chain tip. But we need to check. I really want to avoid using the local node's time, because expecting users to have an accurate clock is a usability issue.

[teor2345]

I'm concerned that the scope of this PR has become a lot larger.

Can we just do the NU5 changes and basic test vectors in the initial PR?
Then we can schedule other work in future sprints.

[jvff]

Updated. Since I removed the incomplete property tests, I think this PR is no longer blocked by it so can have its draft status removed.

Changes since last review can be seen here.

[teor2345]

Thanks, looks great!

I'm just about to fix up the unrelated coverage job failure.

[teor2345]

I merged the new errors from two different PRs.