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

feature: Add block header field metadata #217

Merged
merged 6 commits into from
Jul 26, 2023

Conversation

marioevz
Copy link
Member

Changes Included

  • Replaces the fragile dictionary mixing method to collect the header fields with a method based on field metadata, and now each header field contains metadata with the following information:
    • Method used to determine requirement based on block number, timestamp and fork configuration
    • Field source from: Current test environment, or transition tool (t8n) result dictionary
    • Default value if field is required but no value was provided by either the test environment or the transition tool result

Open questions:
Should we throw an exception if the t8n tool did not provide us the sufficient information to create the block?
At the moment we provide defaults for the 4844 fields, but for 4788 the beacon block root contained in the header will actually modify the state root, and therefore we should throw an exception in this case if the field was not provided or was ignored when calculating the state root.

Fixes #215

@winsvega

@winsvega
Copy link
Collaborator

Ideally yes it should response all the fields indicating t8n knows what it's doing

Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

src/ethereum_test_tools/common/types.py Outdated Show resolved Hide resolved
Co-authored-by: Spencer Taylor-Brown <spencer@spencertaylorbrown.uk>
@marioevz marioevz merged commit d5ec094 into ethereum:main Jul 26, 2023
@marioevz marioevz deleted the check-required-fields-from-t8n branch July 26, 2023 18:41
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.

Cancun fixture generated with blocks that are not Cancun
3 participants