From dad62bc5ab868f8aeded16aa4da1df00d0f63f07 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 29 Jan 2024 10:57:03 +0100 Subject: [PATCH] core/state: modify how self-destruct journalling works The self-destruct journalling is a bit strange: we allow the 'selfdestruct' operation to be journalled several times. This makes it so that we also are forced to store whether the account was already destructed. What we can do instead, is to only journal the first destruction, and after that only journal balance-changes, but not journal the selfdestruct itself. This simplifies the journalling, so that internals about state management does not leak into the journal-API. --- core/state/journal.go | 19 +++++-------------- core/state/statedb.go | 24 +++++++++++++----------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index 5fdf59c88b..927c5f07cc 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -168,12 +168,8 @@ func (j *journal) JournalCreate(addr common.Address) { j.append(createObjectChange{account: &addr}) } -func (j *journal) JournalDestruct(addr common.Address, previouslyDestructed bool, prevBalance *uint256.Int) { - j.append(selfDestructChange{ - account: &addr, - prev: previouslyDestructed, - prevbalance: prevBalance.Clone(), - }) +func (j *journal) JournalDestruct(addr common.Address) { + j.append(selfDestructChange{account: &addr}) } func (j *journal) JournalSetState(addr common.Address, key, prev, origin common.Hash) { @@ -240,9 +236,7 @@ type ( } selfDestructChange struct { - account *common.Address - prev bool // whether account had already self-destructed - prevbalance *uint256.Int + account *common.Address } // Changes to individual accounts. @@ -325,8 +319,7 @@ func (ch createContractChange) copy() journalEntry { func (ch selfDestructChange) revert(s *StateDB) { obj := s.getStateObject(*ch.account) if obj != nil { - obj.selfDestructed = ch.prev - obj.setBalance(ch.prevbalance) + obj.selfDestructed = false } } @@ -336,9 +329,7 @@ func (ch selfDestructChange) dirtied() *common.Address { func (ch selfDestructChange) copy() journalEntry { return selfDestructChange{ - account: ch.account, - prev: ch.prev, - prevbalance: new(uint256.Int).Set(ch.prevbalance), + account: ch.account, } } diff --git a/core/state/statedb.go b/core/state/statedb.go index c553418770..fedf740f42 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -498,18 +498,20 @@ func (s *StateDB) SelfDestruct(addr common.Address) { if stateObject == nil { return } - var ( - prev = new(uint256.Int).Set(stateObject.Balance()) - n = new(uint256.Int) - ) - s.journal.JournalDestruct(addr, stateObject.selfDestructed, prev) - - if s.logger != nil && s.logger.OnBalanceChange != nil && prev.Sign() > 0 { - s.logger.OnBalanceChange(addr, prev.ToBig(), n.ToBig(), tracing.BalanceDecreaseSelfdestruct) + // Regardless of whether it is already destructed or not, we do have to + // journal the balance-change, if we set it to zero here. + if !stateObject.Balance().IsZero() { + stateObject.SetBalance(new(uint256.Int), tracing.BalanceDecreaseSelfdestruct) + if s.logger != nil && s.logger.OnBalanceChange != nil { + s.logger.OnBalanceChange(addr, stateObject.Balance().ToBig(), new(big.Int), tracing.BalanceDecreaseSelfdestruct) + } + } + // If it is already marked as self-destructed, we do not need to add it + // for journalling a second time. + if !stateObject.selfDestructed { + s.journal.JournalDestruct(addr) + stateObject.markSelfdestructed() } - - stateObject.markSelfdestructed() - stateObject.data.Balance = n } func (s *StateDB) Selfdestruct6780(addr common.Address) {