-
Notifications
You must be signed in to change notification settings - Fork 129
β¨ feat(tests): zkEVM - Add parametrized opcodes and gas limits #1483
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
β¨ feat(tests): zkEVM - Add parametrized opcodes and gas limits #1483
Conversation
4f6adcf
to
858af44
Compare
858af44
to
9668583
Compare
A series of stress tests that assess the ability of a specific | ||
opcode or precompile to process large proofs in a block. |
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.
opcodes/precompiles wouldn't process proofs.
These tests to be added should be seen as regular blockchain tests that so happen to be edge cases that we want to test in a zkEVM. In that sense, they should not be any different than other blockchain tests
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.
Agreed. These tests are no different. By "process" I mean making these opcode / precompile to take the bytecode as input, nothing more.
I think we want to just deploy N distinct contracts over as many blocks as needed, where N is the amount of EXTCODEHASH opcodes that I can fit into a block. Then in the subsequent blocks, we want to call 1 EXTCODEHASH. Then in the next block, we want to call 2 EXTCODEHASH, all the way upto N EXTCODEHASH.
We would be ignoring those blocks that are used to deploy contracts, because we only care about the blocks that are calling the EXTCODEHASH. But from the perspective of the execution spec tests, its just a blockchain test where we deploy N contracts and then call EXTCODEHASH/SIZE in subsequent blocks. ie nothing to do with proofs should be mentioned here and these tests can be used in a normal client too |
EXTCODEHASH requires a "setup" phase. This may not be the case for all opcodes/precompiles we want to test. For the Add opcode for example, we can have block 1, do as many Adds as possible to fill up the block. |
Makes sense. The code can be tweaked slightly to skip setup phase. |
c114004
to
afc7f13
Compare
Uhm, my take is that we should:
My argument on separating tests in the second and third bullets is exactly because the second requires some complicated setup, compared to opcode/precompile repetition being mostly stateless. While you could make that conditional, feels a bit complex and can be confusing (i.e., technically you can create a single test with all the tests in the repo and make it conditional :P ). To generalize the setup to avoid big pre-alloc @marioevz suggested and idea here. |
Thanks @jsign :) I misunderstood bullets (1) and (2) to be the same test. |
55853d2
to
a672724
Compare
ποΈ Description
The PR extends #1456 to add dynamic gas_limits and opcodes.
π Related Issues
#1453
How it works
execution-spec-tests/tests/zkevm/test_proof_size.py
Lines 2 to 9 in 9668583
Configuring gas limits
execution-spec-tests/tests/zkevm/test_proof_size.py
Line 37 in 858af44
Configuring opcodes
execution-spec-tests/tests/zkevm/test_proof_size.py
Lines 80 to 91 in 9668583
What's pending
As described in #1456, the pre state size is huge. To solve that, I experimented deploying contracts in the same block, it had the following issues:
I was wondering if there is a way to persist data between blocks in the test.
If this approach sounds okay, then I can extend this to include precompiles.
Even after deploying contracts in a separate blocks, communicating the deployed addresses to be consumed would need more work.
cc: @jsign
Output
β Checklist