fix(test): Don't include shielded data in genesis blocks #7302
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
upbqdn
changed the title
upbqdn
changed the title
github-actions
bot
added
the
C-trivial
upbqdn
removed
the
C-trivial
github-actions
bot
added
the
C-trivial
upbqdn
removed
the
C-trivial
arya2
approved these changes
Aug 8, 2023
There was a problem hiding this comment.
This looks great! Let's merge this into dedup-tree-insertion-fin-state to fix those tests.
The disk format upgrade panicked here and possibly elsewhere:
can't upgrade a database that has already been upgraded, or is newer
https://github.com/ZcashFoundation/zebra/actions/runs/5798021847/job/15717359090?pr=7302#step:8:215
|
Thanks for looking at the tests. I forgot to mention that the bug that this PR addresses is gone—an example of the bug in #7266: https://github.com/ZcashFoundation/zebra/actions/runs/5798018815/job/15717022677. This PR fails because of an unrelated bug that I'll fix in another PR. |
upbqdn
merged commit 8ec4631
into
dedup-tree-insertion-fin-state
274 of 285 checks passed
mergify bot
pushed a commit
that referenced
this pull request
Aug 9, 2023
… trees into finalized state (#7266) * Pass ZebraDB to batch preparation * Dedup the insertion of Sapling trees into database * Dedup the insertion of Orchard trees into database * Update snapshots * Rename batch preparation of trees * Simplify the naming of note commitment trees * Correctly retrieve Sapling trees from fin state * Correctly retrieve Orchard trees from fin state * Simplify the naming of methods for Sprout trees * Simplify the naming of methods for Sapling trees * Simplify the naming of methods for Orchard trees * Reduce disk reads by caching trees. (#7276) * Bump the state minor version * Reset the state patch version * Simplify the preparation of genesis trees * Store the roots of the trees of the genesis block * Add the genesis roots to snapshots * fix(test): Don't include shielded data in genesis blocks (#7302) * fix(state): Fix marking format upgrades (#7304) --------- Co-authored-by: Arya <aryasolhi@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Our proptests were generating genesis blocks containing transactions with joinsplits and shielded data. PR #7266 asserts that the note commitment trees associated with the genesis block are equal to the empty trees. These assertions were failing in that PR because the note commitment trees for genesis blocks weren't empty.
Solution
Reviewer Checklist