-
Notifications
You must be signed in to change notification settings - Fork 179
Multiple Enhancements #1
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
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.
Awesome work! Some small linting errors, but overall looks great.
pre = { | ||
"0x1000000000000000000000000000000000000000": Account( | ||
balance=0x0ba1a9ce0ba1a9ce, | ||
code=Yul(""" |
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.
so cool
no coverage on dup tests ? |
DUP tests were added in this first EEST PR #1 :) ...a long time before you added the automated coverage checker. |
…back Address review comments by optimizing loop efficiency: 1. Move function selector MSTORE outside loops (Comment ethereum#2) - BALANCEOF_SELECTOR and APPROVE_SELECTOR now stored once per contract - Saves 3 gas (G_VERY_LOW) per iteration - Total savings: ~6,471 gas for 50-50 ratio with 10M budget and 3 contracts 2. Remove unused return data from CALL operations (Comment ethereum#1) - Changed ret_offset=96/128, ret_size=32 to ret_offset=0, ret_size=0 - Eliminates unnecessary memory expansion - Minor gas savings, cleaner implementation Skipped Comment ethereum#3 (use Op.GAS for addresses): - Would lose determinism (GAS varies per iteration) - Adds complexity for minimal benefit - Counter still needed for loop control Changes applied to: - test_sload_empty_erc20_balanceof - test_sstore_erc20_approve - test_mixed_sload_sstore (both SLOAD and SSTORE loops)
…back Address review comments by optimizing loop efficiency: 1. Move function selector MSTORE outside loops (Comment ethereum#2) - BALANCEOF_SELECTOR and APPROVE_SELECTOR now stored once per contract - Saves 3 gas (G_VERY_LOW) per iteration - Total savings: ~6,471 gas for 50-50 ratio with 10M budget and 3 contracts 2. Remove unused return data from CALL operations (Comment ethereum#1) - Changed ret_offset=96/128, ret_size=32 to ret_offset=0, ret_size=0 - Eliminates unnecessary memory expansion - Minor gas savings, cleaner implementation Skipped Comment ethereum#3 (use Op.GAS for addresses): - Would lose determinism (GAS varies per iteration) - Adds complexity for minimal benefit - Counter still needed for loop control Changes applied to: - test_sload_empty_erc20_balanceof - test_sstore_erc20_approve - test_mixed_sload_sstore (both SLOAD and SSTORE loops)
Changes Included
Test Functions as Generators
Each test function should now be modeled as a generator in order to yield multiple test vectors which result in multiple similar test cases with minimal changes.
I think this could reduce the amount of code necessary in the fillers when multiple test cases are very similar to each other and, instead of rewriting the same filler function again, one can simply change the
pre
,post
ortxs
objects and yield a new test.Test Functions Now Take Fork as the Single Parameter
Some test cases require behavior changes depending on the fork they run. E.g. when a fork changes the opcode gas cost, the resulting balances might differ depending on the fork.
The test can now take a single parameter, which is the fork name, to modify its behavior accordingly.
Another potentially better option could be for the test functions to take
**kwargs
, and then read the fork from there.This could make the test cases a bit more future-proof, and even the decorators could include more keyword parameters to be read by the test functions to modify its behavior.
Yul Compiler
The
ethereum_test
package now includes aYul
class which is a compiler of Yul source code.This results in the filler now requiring the
solc
binary to be installed to properly fill all the tests, which I think is an acceptable tradeoff to make the tests more readable.Alloc and Result are now checked
The returned
alloc
mapping returned by theevm t8n
is now checked against thepost
object proposed by the test vector.result
mapping is also checked to verify that, if any transaction failed to execute, the failure is expected by the test vector.Expanded README
Added the section
Writing Tests
to the README document to include a small guide on how to write a test.I think this section needs a lot of cleanup and further info, but this could be only a start.
Also, we could host a more detailed guide somewhere else if necessary.
New Arguments for
ethereum_test_filler
Added new arguments
--test-module
and--test-case
to limit test filling of a single test module or test case.ToDos (Can be separate PRs)
Add more internal tests for each of the newly introduced objects.
Wanted to add a proof of concept of a decorator that takes a test case and adds multiple variations of it.
E.g. an EOF (EVM Object Format) decorator that could generate both combinations of a test case with legacy contracts or EOF contracts.
Add info to the generated fixture such as the
evm
orb11r
versions used to generate it.Add automatic fixture generation to this repo.