From 8140b3de8ca06804bfd8c97c2e429e2b4dbc4b63 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 5 Aug 2024 10:54:14 -0500 Subject: [PATCH 1/5] Add a notion of zombies to preserve empty account behavior --- core/state/journal.go | 22 +++++++++++++++++++++ core/state/journal_arbitrum.go | 30 +++++++++++++++++++++++++++++ core/state/journal_arbitrum_test.go | 14 ++++++++++++++ core/state/statedb.go | 19 ++++++++++++++++-- 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 core/state/journal_arbitrum_test.go diff --git a/core/state/journal.go b/core/state/journal.go index c0f5615c98..37775c602b 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -40,6 +40,8 @@ type journalEntry interface { // commit. These are tracked to be able to be reverted in the case of an execution // exception or request for reversal. type journal struct { + zombieEntries map[common.Address]int // Arbitrum: number of createZombieChange entries for each address + entries []journalEntry // Current changes tracked by the journal dirties map[common.Address]int // Dirty accounts and the number of changes } @@ -47,6 +49,8 @@ type journal struct { // newJournal creates a new initialized journal. func newJournal() *journal { return &journal{ + zombieEntries: make(map[common.Address]int), + dirties: make(map[common.Address]int), } } @@ -56,6 +60,10 @@ func (j *journal) append(entry journalEntry) { j.entries = append(j.entries, entry) if addr := entry.dirtied(); addr != nil { j.dirties[*addr]++ + // Arbitrum: also track the number of zombie changes + if isZombie(entry) { + j.zombieEntries[*addr]++ + } } } @@ -70,6 +78,13 @@ func (j *journal) revert(statedb *StateDB, snapshot int) { if addr := j.entries[i].dirtied(); addr != nil { if j.dirties[*addr]--; j.dirties[*addr] == 0 { delete(j.dirties, *addr) + + // Revert zombieEntries tracking + if isZombie(j.entries[i]) { + if j.zombieEntries[*addr]--; j.zombieEntries[*addr] == 0 { + delete(j.zombieEntries, *addr) + } + } } } } @@ -95,6 +110,8 @@ func (j *journal) copy() *journal { entries = append(entries, j.entries[i].copy()) } return &journal{ + zombieEntries: maps.Clone(j.zombieEntries), + entries: entries, dirties: maps.Clone(j.dirties), } @@ -106,6 +123,11 @@ type ( account *common.Address } + // Changes to the account trie without being marked as dirty. + createZombieChange struct { + account *common.Address + } + // createContractChange represents an account becoming a contract-account. // This event happens prior to executing initcode. The journal-event simply // manages the created-flag, in order to allow same-tx destruction. diff --git a/core/state/journal_arbitrum.go b/core/state/journal_arbitrum.go index b286303150..59767edae7 100644 --- a/core/state/journal_arbitrum.go +++ b/core/state/journal_arbitrum.go @@ -77,3 +77,33 @@ func (ch EvictWasm) copy() journalEntry { Debug: ch.Debug, } } + +// Arbitrum: only implemented by createZombieChange +type possibleZombie interface { + // Arbitrum: return true if this change should, on its own, create an empty account. + // If combined with another non-zombie change the empty account will be cleaned up. + isZombie() bool +} + +func isZombie(entry journalEntry) bool { + possiblyZombie, isPossiblyZombie := entry.(possibleZombie) + return isPossiblyZombie && possiblyZombie.isZombie() +} + +func (ch createZombieChange) revert(s *StateDB) { + delete(s.stateObjects, *ch.account) +} + +func (ch createZombieChange) dirtied() *common.Address { + return ch.account +} + +func (ch createZombieChange) copy() journalEntry { + return createZombieChange{ + account: ch.account, + } +} + +func (ch createZombieChange) isZombie() bool { + return true +} diff --git a/core/state/journal_arbitrum_test.go b/core/state/journal_arbitrum_test.go new file mode 100644 index 0000000000..9ef6d7539c --- /dev/null +++ b/core/state/journal_arbitrum_test.go @@ -0,0 +1,14 @@ +package state + +import "testing" + +func TestIsZombie(t *testing.T) { + var nonZombie journalEntry = createObjectChange{} + if isZombie(nonZombie) { + t.Error("createObjectChange should not be a zombie") + } + var zombie journalEntry = createZombieChange{} + if !isZombie(zombie) { + t.Error("createZombieChange should be a zombie") + } +} diff --git a/core/state/statedb.go b/core/state/statedb.go index 5cf8e066dc..3908771de6 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -436,6 +436,11 @@ func (s *StateDB) AddBalance(addr common.Address, amount *uint256.Int, reason tr // SubBalance subtracts amount from the account associated with addr. func (s *StateDB) SubBalance(addr common.Address, amount *uint256.Int, reason tracing.BalanceChangeReason) { + // Arbitrum: this behavior created empty accounts in old geth versions. + if amount.IsZero() && s.getStateObject(addr) == nil { + s.createZombie(addr) + return + } stateObject := s.getOrNewStateObject(addr) if stateObject != nil { s.arbExtraData.unexpectedBalanceDelta.Sub(s.arbExtraData.unexpectedBalanceDelta, amount.ToBig()) @@ -695,6 +700,15 @@ func (s *StateDB) createObject(addr common.Address) *stateObject { return obj } +// createObject creates a new state object. The assumption is held there is no +// existing account with the given address, otherwise it will be silently overwritten. +func (s *StateDB) createZombie(addr common.Address) *stateObject { + obj := newObject(s, addr, nil) + s.journal.append(createZombieChange{account: &addr}) + s.setStateObject(obj) + return obj +} + // CreateAccount explicitly creates a new state object, assuming that the // account did not previously exist in the state. If the account already // exists, this function will silently overwrite it which might lead to a @@ -842,7 +856,8 @@ func (s *StateDB) GetRefund() uint64 { // into the tries just yet. Only IntermediateRoot or Commit will do that. func (s *StateDB) Finalise(deleteEmptyObjects bool) { addressesToPrefetch := make([][]byte, 0, len(s.journal.dirties)) - for addr := range s.journal.dirties { + for addr, dirtyCount := range s.journal.dirties { + isZombie := s.journal.zombieEntries[addr] == dirtyCount obj, exist := s.stateObjects[addr] if !exist { // ripeMD is 'touched' at block 1714175, in tx 0x1237f737031e40bcde4a8b7e717b2d15e3ecadfe49bb1bbc71ee9deb09c6fcf2 @@ -853,7 +868,7 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { // Thus, we can safely ignore it here continue } - if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { + if obj.selfDestructed || (deleteEmptyObjects && obj.empty() && !isZombie) { delete(s.stateObjects, obj.address) s.markDelete(addr) From cbe2004902414123cd85bf1343f1012fd667a633 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 6 Aug 2024 09:22:50 -0500 Subject: [PATCH 2/5] Fix zombie creation to only happen for destructed objects --- core/state/statedb.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 3908771de6..a4bc16de80 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -438,8 +438,10 @@ func (s *StateDB) AddBalance(addr common.Address, amount *uint256.Int, reason tr func (s *StateDB) SubBalance(addr common.Address, amount *uint256.Int, reason tracing.BalanceChangeReason) { // Arbitrum: this behavior created empty accounts in old geth versions. if amount.IsZero() && s.getStateObject(addr) == nil { - s.createZombie(addr) - return + if _, destructed := s.stateObjectsDestruct[addr]; destructed { + s.createZombie(addr) + return + } } stateObject := s.getOrNewStateObject(addr) if stateObject != nil { From ee114edd89a52d98f1148cc55592a131bca8e230 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 6 Aug 2024 09:40:37 -0500 Subject: [PATCH 3/5] Fix tracing of subtracting zero balance --- core/state/statedb.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index a4bc16de80..e61ad16bc0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -440,7 +440,6 @@ func (s *StateDB) SubBalance(addr common.Address, amount *uint256.Int, reason tr if amount.IsZero() && s.getStateObject(addr) == nil { if _, destructed := s.stateObjectsDestruct[addr]; destructed { s.createZombie(addr) - return } } stateObject := s.getOrNewStateObject(addr) From 0adf3d1a797e12f43b6d257827bd75a9d08e4ac5 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 9 Oct 2024 22:42:36 -0500 Subject: [PATCH 4/5] Make zombie creation explicit --- core/state/statedb.go | 6 ------ core/state/statedb_arbitrum.go | 9 +++++++++ core/vm/interface.go | 3 +++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index e61ad16bc0..273cee9b4e 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -436,12 +436,6 @@ func (s *StateDB) AddBalance(addr common.Address, amount *uint256.Int, reason tr // SubBalance subtracts amount from the account associated with addr. func (s *StateDB) SubBalance(addr common.Address, amount *uint256.Int, reason tracing.BalanceChangeReason) { - // Arbitrum: this behavior created empty accounts in old geth versions. - if amount.IsZero() && s.getStateObject(addr) == nil { - if _, destructed := s.stateObjectsDestruct[addr]; destructed { - s.createZombie(addr) - } - } stateObject := s.getOrNewStateObject(addr) if stateObject != nil { s.arbExtraData.unexpectedBalanceDelta.Sub(s.arbExtraData.unexpectedBalanceDelta, amount.ToBig()) diff --git a/core/state/statedb_arbitrum.go b/core/state/statedb_arbitrum.go index e459ad4570..73bb5c77df 100644 --- a/core/state/statedb_arbitrum.go +++ b/core/state/statedb_arbitrum.go @@ -134,6 +134,15 @@ func (s *StateDB) AddStylusPagesEver(new uint16) { s.arbExtraData.everWasmPages = common.SaturatingUAdd(s.arbExtraData.everWasmPages, new) } +// Arbitrum: certain behavior created empty accounts in old geth versions. +func (s *StateDB) CreateZombieIfDeleted(addr common.Address) { + if s.getStateObject(addr) == nil { + if _, destructed := s.stateObjectsDestruct[addr]; destructed { + s.createZombie(addr) + } + } +} + func NewDeterministic(root common.Hash, db Database) (*StateDB, error) { sdb, err := New(root, db, nil) if err != nil { diff --git a/core/vm/interface.go b/core/vm/interface.go index 0818f75b32..49ca835fba 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -44,6 +44,9 @@ type StateDB interface { AddStylusPages(new uint16) (uint16, uint16) AddStylusPagesEver(new uint16) + // Arbitrum: preserve old empty account behavior + CreateZombieIfDeleted(common.Address) + Deterministic() bool Database() state.Database From 5cbfbafeea2e13345236508154a34cadd66492e6 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 9 Oct 2024 22:46:00 -0500 Subject: [PATCH 5/5] Clarify CreateZombieIfDeleted comment --- core/state/statedb_arbitrum.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state/statedb_arbitrum.go b/core/state/statedb_arbitrum.go index 73bb5c77df..de223605f4 100644 --- a/core/state/statedb_arbitrum.go +++ b/core/state/statedb_arbitrum.go @@ -134,7 +134,7 @@ func (s *StateDB) AddStylusPagesEver(new uint16) { s.arbExtraData.everWasmPages = common.SaturatingUAdd(s.arbExtraData.everWasmPages, new) } -// Arbitrum: certain behavior created empty accounts in old geth versions. +// Arbitrum: preserve empty account behavior from old geth and ArbOS versions. func (s *StateDB) CreateZombieIfDeleted(addr common.Address) { if s.getStateObject(addr) == nil { if _, destructed := s.stateObjectsDestruct[addr]; destructed {