zkEVM general refactoring

[jsign]

This zkevm folder has received new tests quickly in the last weeks. As we tried to parallelize work between many people, we've intentionally kept PRs from overlapping to avoid conflicts.

This means that there are a couple of tasks that we should revisit when the dust settles. I'll leave a list here, and please feel free to add more so we can address them later.

• Review test names and how they're organized in files
• Many tests have the usual code_prefix + iter_loop + code_suffix construction. See if we can extract this logic and make most of them reuse a more generic code.
• Revisit any TODO comments and see if they still apply. If that's the case, at a minimum, create a separate issue to keep track of it.
• Figure out a way to verify that the benchmark tests behave as expected: most (all?) use a while loop, we should verify either at the test level OR at runtime level (with a post-state check) that the test behaves as expected. Note that runtime level implicitly means we have to add some kind of verification logic in the test, which will cost gas and will also add to the counted zk cycles thus impacting the outcome. It would be very nice to have some kind of assertion in the test itself to verify that (for context also see attempts like here 3e2cf75#r2127226177):
  • Loop runs three or more times
  • Loop is balanced (no stack under/overflows)
• Check if the tests contain duplicate tests (which either run the same code or a small variation but with the same goal in the same setting) (Nice to have)

cc @kevaundray @jochem-brouwer @marioevz @chfast

[jochem-brouwer]

I have added some items mainly to cleanup duplicates and also to add (or at least think about) verification of the tests, because it seems we have a faulty test (context: #1693 (comment))

[jsign]

Leaving here some notes about a talk we had yesterday with @jochem-brouwer and @danceratopz.
The topic was related to detecting that a benchmark test is broken. The reason why this happens is that if a tx was executed until an OOG happened, it is not possible to distinguish easily:
a. The tx behaved as expected and executed whatever opcode/precompile using all available gas.
b. Or, for whatever reason, the test is broken and maybe there was a stack (under/overflow) or the precompile was called with the wrong parameters.

The latter case will also result in the tx consuming all gas despite not run what we wanted.

The idea that we want to try is the following:

• Add a job in the CI that fills the tests using evmone or some other backend that can generate block traces.
• After filling the tests, the CI analyzes this trace and checks that a new state_test parameter called min_opcode_count = { Opcode : min_count, ... } is satisfied.

That is: as a test write,r I can add a new type of assertion min_opcode_count where I can ask the testing framework to be sure there are at least N executed opcodes when the test is filled.

Doing this would allow us to distinguish cases a. or b. described above.

This strategy sounds much simpler for the test writer than asking to precisely calculate gas costs and stop the test just before running out of gas or similar. That is, we can keep our current style of tests, which tries to execute as many opcodes as possible and call it a day (which avoids a lot of complexity).

The min_opcode_count is just an example of naming, but we probably need something more powerful to detect "min number of precompile calls" or similar.

@marioevz, how do you feel about this? I mainly wanted to add you to the loop of this idea to get your thoughts and see if you can think of a better idea or if you already sense that this strategy can have some drawbacks.

[LouisTsai-Csie]

Based on the discussion in #1737 (comment), we could further optimize the implementation of While loop

Summary: optimizing the STATICCALL benchmark loop for better gas efficiency.

• Current Approach: Uses a While loop with POP(STATICCALL(...)).
• Proposed Optimization: Replace the loop with a flat sequence of many STATICCALLs (with optional POPs), followed by a JUMP back to the start. This reduces the cost of JUMP by spreading it over many calls.
• Use of DUP Instead of PUSH: Using DUP1 instead of PUSH20 saves gas and bytecode size, PUSH20 costs 3 gas and 20 bytes, DUP1 costs only 2 gas and no immediate data.
• Stack Layout Consideration: To reuse the address with DUP, the stack must be carefully arranged (e.g., DUP5 if the address is in the correct position).
• Questions Raised:
  • Does STATICCALL use GAS by default or need to be manually set?
  • Does EEST auto-use PUSH0 when pushing zero, or does it default to PUSH1(0)?
• Conclusion: There’s agreement that using DUP and a jump loop is more efficient, but the exact stack setup needs verification. The implementation might be further optimized.