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

[protocol spec] [ZIP 203] Clarify how transaction lock times are handled #539

Open
jvff opened this issue Jul 14, 2021 · 7 comments
Open
Assignees
Milestone

Comments

@jvff
Copy link

jvff commented Jul 14, 2021

The current protocol specification doesn't seem to specify how transaction lock times are handled. The only references I could find were:

In the transaction encoding section: https://zips.z.cash/protocol/protocol.pdf#txnencoding

Unix-epoch UTC time or block height, encoded as in Bitcoin.

and in ZIP-203:

If used in combination with nLockTime, both nLockTime and nExpiryHeight must be block heights.

Zebra is starting to design how to implement the lock time validation (see ZcashFoundation/zebra#2389 for more info), and some questions were raised about what should be the correct behavior.

  1. Transparent inputs' sequence numbers seem to determine if a lock time is enabled or disabled. What happens to transactions that have no transparent inputs?
  2. Is there any special behavior for coinbase transactions?
  3. Does ZIP-203 mean that nLockTime can only be a timestamp if nExpiryHeight == 0?
  4. Would it be possible to make it explicit to the cases that happen based on the combination of:
    a. if all input sequence numbers are MAX_UINT or if at least one is not
    b. if the lock is a height or a Unix timestamp
    c. if the expiry height is zero on non-zero
    d. if the transaction is a coinbase transaction or not
@daira
Copy link
Collaborator

daira commented Jul 15, 2021

The behaviour of nLockTime is exactly as it was in the version of Bitcoin core we forked from (0.11.2). Sorry for the lack of documentation. nLockTime is orthogonal to nExpiryHeight; there is no interaction between the associated consensus rules.

@teor2345
Copy link
Contributor

teor2345 commented Nov 8, 2021

Here are our mempool-related lock time questions, which are not as urgent:

  • What block time should we use for mempool transactions?
  • Should Zebra reject mempool transactions that are a long way ahead of the current block height or local clock time?

@teor2345
Copy link
Contributor

teor2345 commented Nov 10, 2021

Here's what zcashd does, but these are all standard rules, so Zebra is free to accept additional transactions.

Here are our mempool-related lock time questions, which are not as urgent:

  • What block time should we use for mempool transactions?

zcashd uses the median time past of the best chain.

  • Should Zebra reject mempool transactions that are a long way ahead of the current block height or local clock time?

zcashd marks some of these as orphan transactions. (Zebra doesn't implement orphan transactions yet.)

@teor2345
Copy link
Contributor

teor2345 commented Nov 10, 2021

Here's what we found in the zcashd code:

  • Transparent inputs' sequence numbers seem to determine if a lock time is enabled or disabled. What happens to transactions that have no transparent inputs?

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. (This could be surprising behaviour for shielded transaction users.)

Note that coinbase inputs are treated as a transparent input for the purposes of these checks.

https://github.com/zcash/zcash/blob/1a7c2a3b04bcad6549be6d571bfdff8af9a2c814/src/main.cpp#L718

  • Is there any special behavior for coinbase transactions?

Coinbase inputs have a sequence number.

Coinbase transactions and coinbase inputs are checked just like any other transactions:
https://github.com/zcash/zcash/blob/1a7c2a3b04bcad6549be6d571bfdff8af9a2c814/src/main.cpp#L4728

  • Does ZIP-203 mean that nLockTime can only be a timestamp if nExpiryHeight == 0?

We couldn't find where this rule is implemented in zcashd, so we suspect that ZIP 203 is incorrect here.
(And Daira's comment above is correct.)

  • Would it be possible to make it explicit to the cases that happen based on the combination of:
    a. if all input sequence numbers are MAX_UINT or if at least one is not
    b. if the lock time is a height or a Unix timestamp
    c. if the expiry height is zero or non-zero
    d. if the transaction is a coinbase transaction or not

There isn't really any need for a table, because the rules are:

  1. Lock times are only enforced 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.
  2. Coinbase inputs have a sequence number, and coinbase transaction lock times are checked just like any other transaction.
  3. The block time and block height must be strictly less than the lock time/height. (The Bitcoin docs get this wrong.)
  4. A zero lock time is always allowed. (Otherwise, the genesis transaction would be invalid.)

@conradoplg
Copy link
Contributor

Lock times are only enabled if there is at least one sequence number that is u32::MAX.

There isn't really any need for a table, because the rules are:

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

I think the rule is that lock times are only enabled if there is at least one sequence number that is not u32::MAX. (It's so confusing!)

@teor2345
Copy link
Contributor

I think the rule is that lock times are only enabled if there is at least one sequence number that is not u32::MAX. (It's so confusing!)

Thanks, I've edited that rule.

I've also added two extra rules we've discovered while implementing.

@daira
Copy link
Collaborator

daira commented Dec 20, 2023

Merging #670 into this issue:

There isn't any specification for how mempools should check the lock time. Since mempools are used to create the block template, this can result in mined blocks being invalid.

The background info and an analysis of the zcashd implementation are in this Zebra ticket and PR:
ZcashFoundation/zebra#5984
ZcashFoundation/zebra#6027

Note that zcashd is slightly stricter than necessary. Checking lock-time < median-time-past + 1 is also valid under the consensus rules, but zcashd currently checks lock-time < median-time-past. (Since median-time-past < block-time, and the consensus rule is lock-time < block-time, that implies lock-time < median-time-past + 1 because they are integers.)

@daira daira changed the title Clarify how transaction lock times are handled [protocol spec] [ZIP 203] Clarify how transaction lock times are handled Dec 20, 2023
@daira daira added this to the Documentation backlog milestone Dec 20, 2023
@daira daira self-assigned this Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants