-
Notifications
You must be signed in to change notification settings - Fork 413
feat(test-benchmark): add terminating opcode benchmark #2027
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: forks/amsterdam
Are you sure you want to change the base?
feat(test-benchmark): add terminating opcode benchmark #2027
Conversation
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2027 +/- ##
================================================
Coverage 86.33% 86.33%
================================================
Files 538 538
Lines 34557 34557
Branches 3222 3222
================================================
Hits 29835 29835
Misses 4148 4148
Partials 574 574
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Sentry. π New features to boost your workflow:
|
marioevz
left a comment
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.
Some comments, thanks!
|
|
||
|
|
||
| @pytest.mark.repricing | ||
| def test_pop(benchmark_test: BenchmarkTestFiller) -> None: |
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.
Would it be worth it here to create a "from max stack height" version here?
One where we first push fork.max_stack_height() elements, then pop them all and repeat?
| elif opcode.popped_stack_items > 0: | ||
| attack_block = opcode(Op.PUSH0, return_size) |
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.
| elif opcode.popped_stack_items > 0: | |
| attack_block = opcode(Op.PUSH0, return_size) | |
| elif opcode.kwargs == ["offset", "size"]: | |
| attack_block = opcode(offset=Op.PUSH0, size=return_size) |
It's a tiny bit more explicit.
|
|
||
| attack_block = Bytecode() | ||
| if opcode == Op.SELFDESTRUCT: | ||
| attack_block = Op.SELFDESTRUCT(Op.COINBASE) |
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.
In this case we are testing moving funds to a warm account, but would it be worth it to send to a cold and new account in another version of this test?
We can make the ExtCallGenerator send 1 wei with each call so that the selfdestruct actually has an effect too.
Perhaps self-destruct deserves its own test function.
ποΈ Description
Add new benchmark test for repricing analysis:
POPINVALID,RETURN,REVERT,STOPπ Related Issues or PRs
None
β Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture