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

Add final sapling root test vectors #2243

Merged
merged 5 commits into from
Jun 4, 2021
Merged

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jun 3, 2021

Motivation

In #2227, we need final sapling roots to test the chain history integration.

API Specifications

This PR contains data from zcashd's z_gettreestate RPC:
https://zcash-rpc.github.io/z_gettreestate.html

Solution

Add test vectors:

  • Add final sapling root test vectors for mainnet and testnet
    • zcashd RPC returns the final sapling roots in big-endian order, but we want them in block order
  • Add a test to make sure each block test vector has a final sapling root test vector
  • Add tests to catch common mistakes
  • Tidy some formatting and imports

Fix and test block header commitments:

  • Ignore pre-sapling block commitments
    • Previously Zebra expected this variant to be all-zeroes
  • Add structural and semantic validation tests for block commitment parsing

Review

@conradoplg is waiting on these test vectors for #2227.

Reviewer Checklist

Follow Up Work

Implement and test final sapling roots in Zebra #1287

Also tidy some formatting and imports
@teor2345 teor2345 added NU-1 Sapling Network Upgrade: Sapling specific tasks A-rust Area: Updates to Rust code NU-3 Heartwood Network Upgrade: Heartwood specific tasks C-enhancement Category: This is an improvement NU-5 Network Upgrade: NU5 specific tasks P-Medium labels Jun 3, 2021
@teor2345 teor2345 added this to the 2021 Sprint 11 - Zcon2 milestone Jun 3, 2021
@teor2345 teor2345 requested a review from conradoplg June 3, 2021 04:55
@teor2345 teor2345 self-assigned this Jun 3, 2021
conradoplg added a commit that referenced this pull request Jun 3, 2021
dconnolly
dconnolly previously approved these changes Jun 4, 2021
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@teor2345 teor2345 marked this pull request as draft June 4, 2021 01:51
@teor2345

This comment has been minimized.

teor2345 added 3 commits June 4, 2021 16:12
This makes the test vectors match the byte order in the block header,
rather than the zcashd RPC responses.
Previously, Zebra expected this reserved field to be all zeroes,
but some mainnet and testnet blocks had other values.
History roots are excluded from these tests, because they require
contextual validation.
@teor2345 teor2345 marked this pull request as ready for review June 4, 2021 06:17
@teor2345
Copy link
Contributor Author

teor2345 commented Jun 4, 2021

I've made the following changes to this PR:

  • Reverse final sapling root byte order
    • zcashd RPC returns the final sapling roots in big-endian order, but we want them in block order
  • Ignore pre-sapling block commitments
    • Previously Zebra expected this variant to be all-zeroes
  • Add structural and semantic validation tests for block commitment parsing

@conradoplg feel free to approve and merge this PR so you can keep on going with #2227.

Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works with the code from #2227 😁

@conradoplg conradoplg merged commit c453fbf into main Jun 4, 2021
@conradoplg conradoplg deleted the sapling-final-root-vectors branch June 4, 2021 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-1 Sapling Network Upgrade: Sapling specific tasks NU-3 Heartwood Network Upgrade: Heartwood specific tasks NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants