-
Notifications
You must be signed in to change notification settings - Fork 167
fix(tests): fixed test_push.py::test_stack_overflow
test by reducing contract size to below allowed max
#2002
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: main
Are you sure you want to change the base?
Conversation
Noo another coverage report issue |
tests/frontier/opcodes/test_push.py
Outdated
contract_code = push_opcode(excerpt) * stack_height + Op.SSTORE | ||
contract_code: Bytecode = Bytecode() | ||
for _ in range(stack_height - 2): | ||
contract_code += Op.PUSH0 # mostly use push0 to avoid contract size limit exceeded |
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.
We are currently using the Frontier
and Homestead
forks, which do not support the PUSH0
opcode. Since PUSH0
was introduced in the Shanghai
upgrade, you should use Op.PUSH1(0) instead.
Related EIP: https://eips.ethereum.org/EIPS/eip-3855
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.
True, ty! Coverage still fails tho :/
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.
I check the log, it seems all the cases are passing now. The error message is as follow, I do not know what happen, but maybe a second run will solve this XD
Run echo "=== Original BASE (main) test IDs ==="
=== Original BASE (main) test IDs ===
Traceback (most recent call last):
File "<string>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/work/execution-spec-tests/execution-spec-tests/evmtest_coverage/coverage/BASE_TESTS/.meta/index.json'
Error: Process completed with exit code 1.
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.
I fixed those issues by making the coverage script more robust, now we unlocked a new error :D but this is the line coverage error that i expected in the first place, the thing that happens whenever you change anything of an existing test
92a2f4d
to
18e3ab2
Compare
🗒️ Description
Issue is described in #1699, ty for reporting!
How to run
First run reth via:
reth node --dev --chain dev --http --authrpc.addr 127.0.0.1 --http.port 8545
then run:
uv run execute remote -m state_test --fork=Shanghai --rpc-endpoint=http://127.0.0.1:8545 --rpc-seed-key=2a871d0798f97d79848a013d4936a73bf4cc922c825d33c1cf7073dff6d409c6 --rpc-chain-id=1337 --tx-wait-timeout 60 -vv -s tests/frontier/opcodes/test_push.py::test_stack_overflow
Before this PR it failed from PUSH23 onwards due to max contract size (24576) exceeded. After this PR everything passes. I also tried filling and consume direct with geth's evm and it all passes. Ty for the push0 idea @marioevz !
🔗 Related Issues or PRs
N/A.
✅ Checklist
tox
checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
type(scope):
.mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_from
marker.