Skip to content

fix(stages): clear pre-eip161 accounts#1747

Merged
gakonst merged 3 commits intomainfrom
fix/eip161-state
Mar 14, 2023
Merged

fix(stages): clear pre-eip161 accounts#1747
gakonst merged 3 commits intomainfrom
fix/eip161-state

Conversation

@joshieDo
Copy link
Copy Markdown
Collaborator

This makes sure that empty accounts created pre-eip161 are cleared from state accordingly.

This was causing state root mismatch at 2675000 / spurious dragon hardfork

@joshieDo joshieDo added C-bug An unexpected or incorrect behavior A-staged-sync Related to staged sync (pipelines and stages) A-execution-reth Related to the Execution and EVM labels Mar 14, 2023
@joshieDo joshieDo requested a review from rakita as a code owner March 14, 2023 05:32
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 14, 2023

Codecov Report

Merging #1747 (85e6c75) into main (5b90cbc) will increase coverage by 0.01%.
The diff coverage is 80.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1747      +/-   ##
==========================================
+ Coverage   72.64%   72.65%   +0.01%     
==========================================
  Files         392      392              
  Lines       47167    47168       +1     
==========================================
+ Hits        34265    34272       +7     
+ Misses      12902    12896       -6     
Flag Coverage Δ
integration-tests 20.13% <0.00%> (-0.03%) ⬇️
unit-tests 67.35% <80.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
crates/storage/provider/src/execution_result.rs 97.53% <75.00%> (-1.22%) ⬇️
crates/executor/src/executor.rs 93.08% <100.00%> (ø)

... and 6 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@joshieDo joshieDo marked this pull request as draft March 14, 2023 05:47
@joshieDo joshieDo marked this pull request as ready for review March 14, 2023 05:59
Comment on lines +101 to +103
if has_state_clear_eip && is_empty {
tx.delete::<tables::PlainAccountState>(address, None)?;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Incredible find

@gakonst gakonst merged commit ee3a8af into main Mar 14, 2023
@gakonst gakonst deleted the fix/eip161-state branch March 14, 2023 06:41
onbjerg added a commit that referenced this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-execution-reth Related to the Execution and EVM A-staged-sync Related to staged sync (pipelines and stages) C-bug An unexpected or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants