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

The tx verifier does not enforce using the correct consensus branch ID in the SIGHASH precomputation #9089

Open
upbqdn opened this issue Dec 16, 2024 · 0 comments · May be fixed by #9139
Open
Assignees
Labels
C-bug Category: This is a bug I-consensus Zebra breaks a Zcash consensus rule
Milestone

Comments

@upbqdn
Copy link
Member

upbqdn commented Dec 16, 2024

When precomputing the SIGHASH of V5 txs, we pick the consensus branch ID from the nConsensusBranchId field if the tx has it. If the field is set to a consensus branch ID corresponding to NU5 and NU6 is active, the SIGHASH precomputation doesn't follow this consensus rule:

[NU6 only] All transactions MUST use the NU6 consensus branch ID 0xC8E71055 as defined in ZIP-253.

This bug was mitigated in #9063 by checking the nConsensusBranchId field early in the tx verifier, so it won't accept NU5 transactions after NU6 activation anymore. However, we should fix the precomputation to adhere to the above consensus rule since it takes place in zebra-chain and not zebra-consensus, where the verifier is, and can be used by other crates.

The bug originates in this fn: https://github.com/zcash/librustzcash/blob/443faf9e855e0acf7aca509885ac2ede8090c5bd/zcash_primitives/src/transaction/mod.rs#L604-L616, which considers the consensus_branch_id param for pre-V5 txs but doesn't do so for V5 ones.

@upbqdn upbqdn added C-bug Category: This is a bug I-consensus Zebra breaks a Zcash consensus rule labels Dec 16, 2024
@github-project-automation github-project-automation bot moved this to New in Zebra Dec 16, 2024
@mpguerra mpguerra moved this from New to In progress in Zebra Dec 17, 2024
@upbqdn upbqdn self-assigned this Dec 21, 2024
@mpguerra mpguerra added this to the NU6 milestone Jan 8, 2025
@mpguerra mpguerra linked a pull request Jan 20, 2025 that will close this issue
7 tasks
@upbqdn upbqdn changed the title The tx verifier does not enforce using the correct consensus branch ID in the SIGHASH computation The tx verifier does not enforce using the correct consensus branch ID in the SIGHASH precomputation Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug I-consensus Zebra breaks a Zcash consensus rule
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

2 participants