core/state: storage journal entry should revert dirtyness too #29641
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Interesting. I think this "error" should ideally have been detected by the post-state checker in |
Nevermind this. The error here is unrelated to what I was talking about |
holiman
approved these changes
Apr 24, 2024
No wait, it is related. Rolling back a journal-entry which dirtied a slot would results in Ah, instead of iterating and checking key by key, we should compare the whole |
|
Indeed, that's what happens |
|
This commit should detect it: 9b3104d So this PR should fix the error I thus introduce into that PR |
rjl493456442
approved these changes
Apr 24, 2024
jorgemmsilva
pushed a commit
to iotaledger/go-ethereum
that referenced
this pull request
Jun 17, 2024
…um#29641) Currently our state journal tracks each storage update to a contract, having the ability to revert those changes to the previously set value. For the very first modification however, it behaves a bit wonky. Reverting the update doesn't actually remove the dirty-ness of the slot, rather leaves it as "change this slot to it's original value". This can cause issues down the line with for example write witnesses needing to gather an unneeded proof. This PR modifies the storageChange journal entry to not only track the previous value of a slot, but also whether there was any previous value at all set in the current execution context. In essence, the PR changes the semantic of storageChange so it does not simply track storage changes, rather it tracks dirty storage changes, an important distinction for being able to cleanly revert the journal item.
stwiname
pushed a commit
to subquery/data-node-go-ethereum
that referenced
this pull request
Sep 9, 2024
…um#29641) Currently our state journal tracks each storage update to a contract, having the ability to revert those changes to the previously set value. For the very first modification however, it behaves a bit wonky. Reverting the update doesn't actually remove the dirty-ness of the slot, rather leaves it as "change this slot to it's original value". This can cause issues down the line with for example write witnesses needing to gather an unneeded proof. This PR modifies the storageChange journal entry to not only track the previous value of a slot, but also whether there was any previous value at all set in the current execution context. In essence, the PR changes the semantic of storageChange so it does not simply track storage changes, rather it tracks dirty storage changes, an important distinction for being able to cleanly revert the journal item.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently our state journal tracks each storage update to a contract, having the ability to revert those changes to the previously set value.
For the very first modification however, it behaves a bit wonky. Reverting the update doesn't actually remove the dirty-ness of the slot, rather leaves it as "change this slot to it's original value". This can cause issues down the line with for example write witnesses needing to gather an unneeded proof.
This PR modifies the
storageChangejournal entry to not only track the previous value of a slot, but also whether there was any previous value at all set in the current execution context. In essence, the PR changes the semantic ofstorageChangeso it does not simply track storage changes, rather it tracks dirty storage changes, an important distinction for being able to cleanly revert the journal item.I do not expect any performance impact whatsoever, even if we load a few slots fewer. The PR is more about correctness to handle a corner-case that would bubble up in later code.