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

post-Canyon receipt-root deposit tx hashing fix #152

Merged

Conversation

roberto-bayardo
Copy link
Contributor

@roberto-bayardo roberto-bayardo commented Oct 11, 2023

Description

Changes the post-Canyon deposit tx receipt encoding for the receipt-root calculation to include the deposit nonce, while preserving old behavior for pre-Canyon deposit txs with deposit nonces. Adds an optional version number to the deposit receipt consensus encoding that is specified only after Canyon hardfork.

Tests

Added test to confirm that legacy behavior of EncodeIndex for post-Regolith / pre-Canyon receipt hashing is maintained, while the post-Canyon behavior correctly includes the nonce and matches the behavior of MarshalBinary.

Existing receipt tests that accepted deposit txs all extended to test receipts both with & without deposit receipt version.

Metadata

@roberto-bayardo roberto-bayardo force-pushed the canyon/fix-receipt-hash branch 5 times, most recently from 0329e31 to 0810748 Compare October 11, 2023 17:55
@roberto-bayardo roberto-bayardo changed the title WIP: post-Canyon receipt-root deposit tx hashing fix post-Canyon receipt-root deposit tx hashing fix Oct 11, 2023
@roberto-bayardo roberto-bayardo force-pushed the canyon/fix-receipt-hash branch 2 times, most recently from 91e3800 to d7b47ba Compare October 11, 2023 18:23
@roberto-bayardo roberto-bayardo marked this pull request as ready for review October 11, 2023 18:28
@roberto-bayardo roberto-bayardo force-pushed the canyon/fix-receipt-hash branch 2 times, most recently from 7d3c419 to 91ec9fe Compare October 11, 2023 21:50
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM.

core/types/receipt_test.go Show resolved Hide resolved
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

LGTM

@zhwrd zhwrd merged commit ec46803 into ethereum-optimism:optimism Oct 13, 2023
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

op-geth receipt-hashing bug
4 participants