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

fix(types): ensure Environment fields have even byte length #1268

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

danceratopz
Copy link
Member

🗒️ Description

These changes ensure that the fields from Environment passed to t8n have even byte length by setting their type to ZeroPaddedHexNumber. This is required by evmone-t8n.

This fix is taken verbatim from @winsvega's fix from #1263 for a quick merge to unblock #1244.

🔗 Related Issues

This became an issue after:

Should fix the issues with the "Evmone Coverage Script" workflow in:

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.

@danceratopz danceratopz added type:bug Something isn't working scope:fill Scope: fill command scope:types Scope: Changes to ethereum_test_types, ethereum_test_base_types labels Feb 26, 2025
@danceratopz danceratopz requested a review from marioevz February 26, 2025 14:08
@chfast
Copy link
Member

chfast commented Feb 26, 2025

This is problem probably for difficulty because evmone don't treat the difficulty as a number. And almost all tests use value 0x20000. If it used value 0x2000 it would had been fine.

Anyway, we can correct it in evmone-t8n.

However, if you plan to depend on t8n tools from multiple projects long term you should create a t8n conformance test suite.

@danceratopz
Copy link
Member Author

This is problem probably for difficulty because evmone don't treat the difficulty as a number. And almost all tests use value 0x20000. If it used value 0x2000 it would had been fine.

Yup, realised that :)

Anyway, we can correct it in evmone-t8n.

Thanks @chfast, if we can address it here, there's no need to change it here in evmone-t8n.

However, if you plan to depend on t8n tools from multiple projects long term you should create a t8n conformance test suite.

Yup, we're going to add this.

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@marioevz marioevz merged commit f57a956 into main Feb 27, 2025
21 checks passed
@marioevz marioevz deleted the fix-fill-with-evmone branch February 27, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:fill Scope: fill command scope:types Scope: Changes to ethereum_test_types, ethereum_test_base_types type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants