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

Retroactively enable ZIP 216 before NU5 activation #6399

Merged
merged 1 commit into from Feb 11, 2023

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Feb 2, 2023

This completes the work started in #6000.

Closes #6396.

@str4d str4d added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-consensus Area: Consensus rules NU5 Network upgrade: NU5-specific tasks A-sapling Area: Sapling protocol labels Feb 2, 2023
@str4d str4d added this to the Release 5.5.0 milestone Feb 2, 2023
daira
daira previously approved these changes Feb 2, 2023
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK with non-blocking suggestions (modulo test failures).

This completes the work started in zcash#6000.

Closes zcash#6396.
@str4d
Copy link
Contributor Author

str4d commented Feb 2, 2023

Rebased on master to include v5.4.0-rc4 changes and fix tests, and then force-pushed to address @daira's comments.

@nuttycom nuttycom added the safe-to-build Used to send PR to prod CI environment label Feb 2, 2023
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK

@ECC-CI ECC-CI removed the safe-to-build Used to send PR to prod CI environment label Feb 2, 2023
@nuttycom nuttycom added safe-to-build Used to send PR to prod CI environment and removed safe-to-build Used to send PR to prod CI environment labels Feb 2, 2023
@str4d str4d marked this pull request as draft February 3, 2023 00:04
@ECC-CI ECC-CI removed the safe-to-build Used to send PR to prod CI environment label Feb 3, 2023
@str4d str4d marked this pull request as ready for review February 3, 2023 20:28
@str4d
Copy link
Contributor Author

str4d commented Feb 3, 2023

I ran a full reindex with this branch, and was able to reindex up to and past NU5 without errors, confirming that ZIP 216 is compatible with the revealed coinbase outputs before NU5.

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK 9ce6753

@daira
Copy link
Contributor

daira commented Feb 5, 2023

The changes in this PR are correct, but I want to do another review pass over the resulting code to make sure we are completely consistent with zcash/zips#672.

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

This can be merged now.

@nuttycom nuttycom merged commit cb22267 into zcash:master Feb 11, 2023
str4d added a commit to zcash/librustzcash that referenced this pull request Nov 30, 2023
As a side-effect, we remove the ability to verify individual
transactions with pre-ZIP 216 rules (which we already removed from
`zcashd` consensus nodes in zcash/zcash#6000 and zcash/zcash#6399, as
all pre-ZIP 216 transactions on mainnet are also valid under ZIP 216).
str4d added a commit to zcash/librustzcash that referenced this pull request Nov 30, 2023
As a side-effect, we remove the ability to verify individual
transactions with pre-ZIP 216 rules (which we already removed from
`zcashd` consensus nodes in zcash/zcash#6000 and zcash/zcash#6399, as
all pre-ZIP 216 transactions on mainnet are also valid under ZIP 216).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rules A-sapling Area: Sapling protocol C-cleanup Category: PRs that clean code up or issues documenting cleanup. NU5 Network upgrade: NU5-specific tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Finish retroactively enabling ZIP 216 before NU5
4 participants