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.
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
core: implement EIP-4788 BeaconRoot precompile #27289
core: implement EIP-4788 BeaconRoot precompile #27289
Changes from 8 commits
e851002
6f11378
d6d0d6d
10390cf
3d4ecc3
f115bf6
2bd0102
bd221a9
4a08450
564bfee
8cf4db1
9a11599
8a030c9
8057030
7ac6529
2ce4c01
b544bd6
ba8ea61
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding it like this delegates the responsibility of "when to start storing data for EIP-4788" entirely to the CL layer. When/of they start including a beaconroot, we start storing it into the state.
IMO we should
As for RLP, similarly, we need to be careful, and strict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the order between BeaconRoot and DataGasUsed specified? Both are activated in the same fork...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think its specified yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be at the end of all the 4844 stuff; I was trying to specify this abstractly to avoid thrash from other EIPs but I can see how this is unclear so I made it explicit:
ethereum/EIPs#7297
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this instead of in Cancun? Now we have cancun with KZG but no beaconnroot, and we have this with beaconroot but no KZG