search

ethereum / execution-specs

Specification for the Execution Layer. Tracking network upgrades.
Creative Commons Zero v1.0 Universal
808 stars 222 forks source link

create transient storage snapshots for `process_create_message` #918

Closed gurukamath closed 2 months ago

gurukamath commented 3 months ago

(closes #917 )

What was wrong?

process_create_message did not create the right snapshots for rollback in case something went wrong.

Related to Issue #917

How was it fixed?

Add transient_storage to Environment and create appropriate snapshots.

Cute Animal Picture

Cute Animals - 1 of 1

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.72%. Comparing base (bf47143) to head (d663e77). Report is 65 commits behind head on forks/cancun.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## forks/cancun #918 +/- ## ================================================ + Coverage 69.96% 74.72% +4.76% ================================================ Files 610 611 +1 Lines 34295 34897 +602 ================================================ + Hits 23993 26076 +2083 + Misses 10302 8821 -1481 ``` | [Flag](https://app.codecov.io/gh/ethereum/execution-specs/pull/918/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/ethereum/execution-specs/pull/918/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum) | `74.72% <100.00%> (+4.76%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ethereum#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

holiman commented 2 months ago

@SamWilsn once you rebase the statetest branch to include this, I can fuzz it again and see if it holds up

petertdavies commented 2 months ago

LGTM!

holiman commented 2 months ago

@gurukamath could you explain a bit about the circumstances here? The original testcase was pretty messy, building a very deeply nested calltree. I'm trying to make a canonical testcase for this (https://github.com/holiman/goevmlab/pull/140) , but would appreciate some guidance.

gurukamath commented 2 months ago

@gurukamath could you explain a bit about the circumstances here? The original testcase was pretty messy, building a very deeply nested calltree. I'm trying to make a canonical testcase for this (holiman/goevmlab#140) , but would appreciate some guidance.

I believe the case is of the following

  1. A does a tstore a value and and tries to create B using CREATE/CREATE2
  2. The creation of B fails because len(contract_code) > MAX_CODE_SIZE
  3. A then tries to tload the same value as in step 1.
  4. sstore then stores this value from tload