ZIP-203: Validate Transaction Expiry Height in the transaction verifier

[teor2345]

Motivation

The transaction verifier needs to check the ZIP-203: Transaction Expiry consensus rules.

Specifications

Zcash Spec

[Overwinter to Canopy inclusive, pre-NU5] nExpiryHeight MUST be less than or equal to 499999999.

[NU5 onward] nExpiryHeight MUST be less than or equal to 499999999 for non-coinbase transactions.

[Overwinter onward] If a transaction is not a coinbase transaction and its nExpiryHeight field is nonzero, then it MUST NOT be mined at a block height greater than its nExpiryHeight.

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

ZIP-203

nExpiryHeight will never be a UNIX timestamp, unlike nLockTime values

Coinbase: nExpiryHeight on coinbase transactions is ignored [before NU5 activation]

https://zips.z.cash/zip-0203#specification

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.

NU5 Changes

from NU5 activation, the nExpiryHeight field of a coinbase transaction MUST be set equal to the block height.

The motivation is to ensure that transaction IDs remain unique

https://zips.z.cash/zip-0203#changes-for-nu5

Note: this rule applies to both v4 and v5 transactions after NU5 activation.

Designs

Zebra parses the expiry height, and validates it in the mempool storage, but not in the transaction verifier:

zebra/zebra-chain/src/transaction.rs

Lines 184 to 185 in 22f2f06

	/// Get this transaction's expiry height, if any.
	pub fn expiry_height(&self) -> Option<block::Height> {

Related Work

Validate the lock time field #2389

Support the large block heights required by the spec #1113 - out of scope for NU5 testnet activation

[teor2345]

I've updated this ticket to include https://zips.z.cash/zip-0203#changes-for-nu5

[oxarbitrage]

In this ticket several of the rules are hard to know if we need to implement them and if so how to do it in Zebra.

[Overwinter to Canopy inclusive, pre-NU5] nExpiryHeight MUST be less than or equal to 499999999.

In Zebra nExpiryHeight is of type Height(u32). Each Height has a max value of 499_999_999: https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/block/height.rs#L44

[NU5 onward] nExpiryHeight MUST be less than or equal to 499999999 for non-coinbase transactions.

This is always the case in zebra for coinbase and non coinbase transactions by the use of Height. I can't see the reason to remove this constraint after nu5 when nExpiryHeight need to match the block height (which can't be above 499_999_999 anyway). If we need to make changes to support heights above 499_999_999 to use in this field i don't know what will be the best approach to do so.

[Overwinter onward] If a transaction is not a coinbase transaction and its nExpiryHeight field is nonzero, then it MUST NOT be mined at a block height greater than its nExpiryHeight.

I think we already did this in the mempool work (need to double check).

nExpiryHeight will never be a UNIX timestamp, unlike nLockTime values

We use Height(u32) for this field.

Coinbase: nExpiryHeight on coinbase transactions is ignored [before NU5 activation]

This is what we do now.

from NU5 activation, the nExpiryHeight field of a coinbase transaction MUST be set equal to the block height.

The motivation is to ensure that transaction IDs remain unique

This is clear, i have a PR for this that will push soon.

[conradoplg]

In this ticket several of the rules are hard to know if we need to implement them and if so how to do it in Zebra.

[Overwinter to Canopy inclusive, pre-NU5] nExpiryHeight MUST be less than or equal to 499999999.

In Zebra nExpiryHeight is of type Height(u32). Each Height has a max value of 499_999_999: https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/block/height.rs#L44

This is true, but that Height limit is supposed to be higher, see #1113. It's also not always being enforced correctly, see #3068

So it's best to explicitly add the 499999999 limit to nExpiryHeight, which will be redundant now but it won't be anymore when we increase the Height limit.

[NU5 onward] nExpiryHeight MUST be less than or equal to 499999999 for non-coinbase transactions.

This is always the case in zebra for coinbase and non coinbase transactions by the use of Height. I can't see the reason to remove this constraint after nu5 when nExpiryHeight need to match the block height (which can't be above 499_999_999 anyway). If we need to make changes to support heights above 499_999_999 to use in this field i don't know what will be the best approach to do so.

The same reasoning as the previous one applies.

nExpiryHeight will never be a UNIX timestamp, unlike nLockTime values

We use Height(u32) for this field.

The reason for that comment is that nLockTime may be either a block height or a timestamp, depending on its value. But this doesn't apply for nExpiryHeight, it's always a height.

[oxarbitrage]

Thanks for the replies @conradoplg . Much clear now. I was able to mock a PR with that help.

#3082

It seems this ticket involves 3 rules where the PR is implementing 2 of them. We have pending at least one rule to complete the ticket (more details in the PR).

[mpguerra]

How does this issue relate to #1113? do we need to do #1113 for NU5 mainnet activation? It seems like we might and if so we need to schedule it in for next sprint

[teor2345]

How does this issue relate to #1113? do we need to do #1113 for NU5 mainnet activation? It seems like we might and if so we need to schedule it in for next sprint

No, we don't need to do #1113 any time soon:

• the coinbase script height won't reach block height 500 million until the year 3000, so we can wait until then to remove the limit
• the expiry height is not checked against the limit, this is already implemented correctly
• the consensus rules for parsing the lock time require this limit, this is already implemented correctly

I'll update that ticket.