refactor(core): storage journal entry should revert dirtyness too #29641 (#2078)

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.

Co-authored-by: Péter Szilágyi <peterke@gmail.com>
This commit is contained in:
Daniel Liu 2026-03-06 13:45:44 +08:00 committed by GitHub
parent 69ab9b8e30
commit 3eb198b1a5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 40 additions and 15 deletions

View file

@ -139,8 +139,9 @@ type (
prev uint64
}
storageChange struct {
account common.Address
key, prevalue common.Hash
account common.Address
key common.Hash
prevvalue *common.Hash
}
codeChange struct {
account common.Address
@ -286,7 +287,7 @@ func (ch codeChange) copy() journalEntry {
}
func (ch storageChange) revert(s *StateDB) {
s.getStateObject(ch.account).setState(ch.key, ch.prevalue)
s.getStateObject(ch.account).setState(ch.key, ch.prevvalue)
}
func (ch storageChange) dirtied() *common.Address {
@ -294,10 +295,15 @@ func (ch storageChange) dirtied() *common.Address {
}
func (ch storageChange) copy() journalEntry {
var prevvalue *common.Hash
if ch.prevvalue != nil {
copied := *ch.prevvalue
prevvalue = &copied
}
return storageChange{
account: ch.account,
key: ch.key,
prevalue: ch.prevalue,
account: ch.account,
key: ch.key,
prevvalue: prevvalue,
}
}

View file

@ -133,13 +133,20 @@ func (s *stateObject) getTrie() (Trie, error) {
// GetState retrieves a value from the account storage trie.
func (s *stateObject) GetState(key common.Hash) common.Hash {
value, _ := s.getState(key)
return value
}
// getState retrieves a value from the account storage trie and also returns if
// the slot is already dirty or not.
func (s *stateObject) getState(key common.Hash) (common.Hash, bool) {
// If we have a dirty value for this state entry, return it
value, dirty := s.dirtyStorage[key]
if dirty {
return value
return value, true
}
// Otherwise return the entry's original value
return s.GetCommittedState(key)
return s.GetCommittedState(key), false
}
func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
@ -183,22 +190,34 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
func (s *stateObject) SetState(key, value common.Hash) common.Hash {
// If the new value is the same as old, don't set. Otherwise, track only the
// dirty changes, supporting reverting all of it back to no change.
prev := s.GetState(key)
prev, dirty := s.getState(key)
if prev == value {
return prev
}
var prevvalue *common.Hash
if dirty {
prevvalue = &prev
}
// New value is different, update and journal the change
s.db.journal.append(storageChange{
account: s.address,
key: key,
prevalue: prev,
account: s.address,
key: key,
prevvalue: prevvalue,
})
s.setState(key, value)
s.setState(key, &value)
return prev
}
func (s *stateObject) setState(key, value common.Hash) {
s.dirtyStorage[key] = value
// setState updates a value in account dirty storage. If the value being set is
// nil (assuming journal revert), the dirtiness is removed.
func (s *stateObject) setState(key common.Hash, value *common.Hash) {
// If the first set is being reverted, undo the dirty marker
if value == nil {
delete(s.dirtyStorage, key)
return
}
// Otherwise set/update the dirty slot value (or restore it when invoked from a revert)
s.dirtyStorage[key] = *value
}
// finalise moves all dirty storage slots into the pending area to be hashed or