From 3eb198b1a5c0861425b4ec89d3732179b2b860b2 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Fri, 6 Mar 2026 13:45:44 +0800 Subject: [PATCH] refactor(core): storage journal entry should revert dirtyness too #29641 (#2078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- core/state/journal.go | 18 ++++++++++++------ core/state/state_object.go | 37 ++++++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index ff97e63fd4..76c2ee93e7 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -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, } } diff --git a/core/state/state_object.go b/core/state/state_object.go index baaf93f507..07c285e89b 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -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