Skip to content
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

Fix EOF initcode handling before EOF is enabled #893

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

chfast
Copy link
Member

@chfast chfast commented May 20, 2024

The special rule of handling EOF initcode in CREATE/CREATE2 instructions should not be activated before EOF is activated.

@chfast chfast added bug Something isn't working EOF labels May 20, 2024
@chfast chfast requested review from gumb0 and pdobacz May 20, 2024 09:41
Copy link

codecov bot commented May 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.50%. Comparing base (7ad882c) to head (b449afc).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #893   +/-   ##
=======================================
  Coverage   98.50%   98.50%           
=======================================
  Files         130      130           
  Lines       15625    15655   +30     
=======================================
+ Hits        15391    15421   +30     
  Misses        234      234           
Flag Coverage Δ
ethereum-tests 27.90% <3.12%> (-0.06%) ⬇️
ethereum-tests-silkpre 19.66% <3.12%> (-0.05%) ⬇️
execution-spec-tests 19.02% <2.94%> (-0.04%) ⬇️
unittests 94.36% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/evmone/instructions_calls.cpp 98.71% <100.00%> (+<0.01%) ⬆️
...est/unittests/state_transition_eof_create_test.cpp 100.00% <100.00%> (ø)

tx.gas_limit = block.gas_limit;
pre.get(tx.sender).balance = tx.gas_limit * tx.max_gas_price + tx.value + 1;

const bytecode init_container = eof_bytecode(ret(0, 1));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we usually use eof_bytecode(OP_INVALID) as the smallest valid eof container. Also, eof_bytecode(ret(0, 1)) fails validation due to wrong max stack height. However irrelevant to the logic being tested, I think it's safer to use a valid EOF code here.

Ah, I see the same is used above. I think this should be fixed there as well when we're at it.

The special rule of handling EOF initcode in CREATE/CREATE2 instructions
should not be activated before EOF is activated.
Copy link

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chfast chfast merged commit 864492f into master May 22, 2024
24 checks passed
@chfast chfast deleted the eof/fix_eof_initcode branch May 22, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working EOF
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants