-
Notifications
You must be signed in to change notification settings - Fork 131
feat(tests): port intrinsic transaction gas tests from Ori #1535
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
base: main
Are you sure you want to change the base?
Conversation
1049a46
to
2713f01
Compare
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.
Looks good overall, just a couple of comments, thanks!
with this PR it can measure the coverage diff of blockchain converted tests as well |
coverage state revert is lost, but that's not the point of the test. |
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.
LGTM. I rebased, removed some unnecessary lines, and changed the parametrization to test both type-1 and type-2 tx instead on only type-2 after London.
Some more bc tests can be romved now. |
Interestingly, with #1554, the base blockchain tests from ethereum/tests are failing for evmone: https://github.com/ethereum/execution-spec-tests/actions/runs/14888483032/job/41814347440?pr=1535 |
@winsvega should it be ok to merge this with the failing coverage? |
Yes, I can trigger coverage rerun once evmone implementation is ready |
🗒️ Description
This combination tests are failing in Prague, I port it now as it can not be adjusted in .json tests. (the filler was removed in ethereum/tests)
So now we can easily switch formula calculation in python.
TODO
🔗 Related Issues
#1199
ethereum/tests#1488
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.