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

Implement Transparent and Sapling validation for transaction v5 #1981

Closed
9 tasks done
teor2345 opened this issue Apr 6, 2021 · 0 comments · Fixed by #2371 or #2437
Closed
9 tasks done

Implement Transparent and Sapling validation for transaction v5 #1981

teor2345 opened this issue Apr 6, 2021 · 0 comments · Fixed by #2371 or #2437
Assignees
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-5 Network Upgrade: NU5 specific tasks

Comments

@teor2345
Copy link
Contributor

teor2345 commented Apr 6, 2021

Is your feature request related to a problem? Please describe.

In #1829 and #1980, we parse and check transparent and sapling in transaction v5. But we need to call those functions in the transaction verifier.

Describe the solution you'd like

add Transaction::V5 to transaction::check (#1980 + PRs, #2070):

  • quote the relevant consensus rules in each function's docs
  • add orchard TODOs
  • refactor to remove duplicate code, and make the consensus rule logic clearer
  • implement the missing "no PrevOuts" coinbase check, added a few days ago in zcash/zips@10710d9

add Transaction::V5 to transaction::Verifier:

  • reject V5 transactions before the NU5 activation height on the configured network
  • make sure Transaction.network_upgrade matches the currently active network upgrade (Validate nConsensusBranchId #2100)
    • the current upgrade is based on the height and configured network, it won't always be NU5

Other transparent and sapling consensus rules:

  • quote the relevant consensus rules in each function's docs
  • add orchard TODOs
  • refactor to remove duplicate code, and make the consensus rule logic clearer

Describe alternatives you've considered

These consensus rules are required for NU5.

Additional context

We also need to implement Orchard validation for transaction v5 (#2379)

@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 NU-5 Network Upgrade: NU5 specific tasks P-Medium labels Apr 6, 2021
@teor2345 teor2345 added this to the 2021 Sprint 9 milestone Apr 6, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Apr 13, 2021
@mpguerra mpguerra mentioned this issue Apr 26, 2021
53 tasks
@teor2345 teor2345 modified the milestones: 2021 Sprint 9, 2021 Sprint 12 May 3, 2021
@jvff jvff self-assigned this Jun 7, 2021
@mpguerra mpguerra linked a pull request Jun 14, 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-5 Network Upgrade: NU5 specific tasks
Projects
None yet
4 participants