-
Notifications
You must be signed in to change notification settings - Fork 167
feat(tests): test all opcodes by fork #748
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
33095ff
to
9af2f28
Compare
9fab3b1
to
046d998
Compare
I put def test_cover_revert(state_test: StateTestFiller, pre: Alloc): here the coverage is red because in original test there were some failing transactions due to invalid stack for opcodes, and create opcodes didn't work as intended, also transaction was running as creatte instead I use call contracts here. |
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 - nice test!
Added a few minor suggestions below, mainly just style and clearer docstrings.
Also added a framwork test to check that we cover the full possible opcode range: bfc84a5
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.
One single comment, but I think it could be left for a follow up PR, and we could merge this one as is (after Dan's comments). Thanks!
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.
ποΈ Description
define lists of opcodes by fork and verify that they are actually supported only at the allowed forks
π Related Issues
PR ethereum/tests#1399
β Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.