From 4c5d1bd95e8ad10dc63ba7dd8daec1c7d667f40e Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Wed, 2 Jul 2025 14:07:21 +0800 Subject: [PATCH] core: change handling of dirty objects in state #15225 #16485 #16491 (#1171) * core: change handling of dirty objects in state #15225 * core/state: fix bug in copy of copy State #16485 * core/state: fix ripemd-cornercase in Copy #16491 --- core/blockchain_test.go | 111 +++++++++++++++++++++++++++ core/state/dump.go | 2 +- core/state/journal.go | 151 +++++++++++++++++++++++++++++++------ core/state/state_object.go | 54 ++++--------- core/state/statedb.go | 92 ++++++++++++---------- core/state/statedb_test.go | 38 +++++++--- tests/state_test.go | 6 -- 7 files changed, 329 insertions(+), 125 deletions(-) diff --git a/core/blockchain_test.go b/core/blockchain_test.go index a5fd586a91..319394124a 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -1379,6 +1379,117 @@ func TestLargeReorgTrieGC(t *testing.T) { } } +// Benchmarks large blocks with value transfers to non-existing accounts +func benchmarkLargeNumberOfValueToNonexisting(b *testing.B, numTxs, numBlocks int, recipientFn func(uint64) common.Address, dataFn func(uint64) []byte) { + var ( + signer = types.HomesteadSigner{} + testBankKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + testBankAddress = crypto.PubkeyToAddress(testBankKey.PublicKey) + bankFunds = big.NewInt(100000000000000000) + gspec = Genesis{ + Config: params.TestChainConfig, + Alloc: GenesisAlloc{ + testBankAddress: {Balance: bankFunds}, + common.HexToAddress("0xc0de"): { + Code: []byte{0x60, 0x01, 0x50}, + Balance: big.NewInt(0), + }, // push 1, pop + }, + GasLimit: 100e6, // 100 M + } + ) + // Generate the original common chain segment and the two competing forks + engine := ethash.NewFaker() + db := rawdb.NewMemoryDatabase() + genesis := gspec.MustCommit(db) + + blockGenerator := func(i int, block *BlockGen) { + block.SetCoinbase(common.Address{1}) + for txi := 0; txi < numTxs; txi++ { + uniq := uint64(i*numTxs + txi) + recipient := recipientFn(uniq) + //recipient := common.BigToAddress(big.NewInt(0).SetUint64(1337 + uniq)) + tx, err := types.SignTx(types.NewTransaction(uniq, recipient, big.NewInt(1), params.TxGas, big.NewInt(1), nil), signer, testBankKey) + if err != nil { + b.Error(err) + } + block.AddTx(tx) + } + } + + shared, _ := GenerateChain(params.TestChainConfig, genesis, engine, db, numBlocks, blockGenerator) + b.StopTimer() + b.ResetTimer() + for i := 0; i < b.N; i++ { + // Import the shared chain and the original canonical one + diskdb := rawdb.NewMemoryDatabase() + gspec.MustCommit(diskdb) + + chain, err := NewBlockChain(diskdb, nil, params.TestChainConfig, engine, vm.Config{}) + if err != nil { + b.Fatalf("failed to create tester chain: %v", err) + } + b.StartTimer() + if _, err := chain.InsertChain(shared); err != nil { + b.Fatalf("failed to insert shared chain: %v", err) + } + b.StopTimer() + if got := chain.CurrentBlock().Transactions().Len(); got != numTxs*numBlocks { + b.Fatalf("Transactions were not included, expected %d, got %d", (numTxs * numBlocks), got) + + } + } +} +func BenchmarkBlockChain_1x1000ValueTransferToNonexisting(b *testing.B) { + var ( + numTxs = 1000 + numBlocks = 1 + ) + + recipientFn := func(nonce uint64) common.Address { + return common.BigToAddress(big.NewInt(0).SetUint64(1337 + nonce)) + } + dataFn := func(nonce uint64) []byte { + return nil + } + + benchmarkLargeNumberOfValueToNonexisting(b, numTxs, numBlocks, recipientFn, dataFn) +} +func BenchmarkBlockChain_1x1000ValueTransferToExisting(b *testing.B) { + var ( + numTxs = 1000 + numBlocks = 1 + ) + b.StopTimer() + b.ResetTimer() + + recipientFn := func(nonce uint64) common.Address { + return common.BigToAddress(big.NewInt(0).SetUint64(1337)) + } + dataFn := func(nonce uint64) []byte { + return nil + } + + benchmarkLargeNumberOfValueToNonexisting(b, numTxs, numBlocks, recipientFn, dataFn) +} +func BenchmarkBlockChain_1x1000Executions(b *testing.B) { + var ( + numTxs = 1000 + numBlocks = 1 + ) + b.StopTimer() + b.ResetTimer() + + recipientFn := func(nonce uint64) common.Address { + return common.BigToAddress(big.NewInt(0).SetUint64(0xc0de)) + } + dataFn := func(nonce uint64) []byte { + return nil + } + + benchmarkLargeNumberOfValueToNonexisting(b, numTxs, numBlocks, recipientFn, dataFn) +} + /* Collection test for BlocksHashCache cases diff --git a/core/state/dump.go b/core/state/dump.go index ba4bfee0e1..f7f4c4506d 100644 --- a/core/state/dump.go +++ b/core/state/dump.go @@ -53,7 +53,7 @@ func (s *StateDB) RawDump() Dump { panic(err) } - obj := newObject(nil, common.BytesToAddress(addr), data, nil) + obj := newObject(nil, common.BytesToAddress(addr), data) account := DumpAccount{ Balance: data.Balance.String(), Nonce: data.Nonce, diff --git a/core/state/journal.go b/core/state/journal.go index 2be83be064..128cbb7fb2 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -22,15 +22,66 @@ import ( "github.com/XinFinOrg/XDPoSChain/common" ) +// journalEntry is a modification entry in the state change journal that can be +// reverted on demand. type journalEntry interface { - undo(*StateDB) + // revert undoes the changes introduced by this journal entry. + revert(*StateDB) + + // dirtied returns the Ethereum address modified by this journal entry. + dirtied() *common.Address } -type journal []journalEntry +// journal contains the list of state modifications applied since the last state +// commit. These are tracked to be able to be reverted in case of an execution +// exception or revertal request. +type journal struct { + entries []journalEntry // Current changes tracked by the journal + dirties map[common.Address]int // Dirty accounts and the number of changes +} + +// newJournal create a new initialized journal. +func newJournal() *journal { + return &journal{ + dirties: make(map[common.Address]int), + } +} + +// append inserts a new modification entry to the end of the change journal. +func (j *journal) append(entry journalEntry) { + j.entries = append(j.entries, entry) + if addr := entry.dirtied(); addr != nil { + j.dirties[*addr]++ + } +} + +// revert undoes a batch of journalled modifications along with any reverted +// dirty handling too. +func (j *journal) revert(statedb *StateDB, snapshot int) { + for i := len(j.entries) - 1; i >= snapshot; i-- { + // Undo the changes made by the operation + j.entries[i].revert(statedb) + + // Drop any dirty tracking induced by the change + if addr := j.entries[i].dirtied(); addr != nil { + if j.dirties[*addr]--; j.dirties[*addr] == 0 { + delete(j.dirties, *addr) + } + } + } + j.entries = j.entries[:snapshot] +} + +// dirty explicitly sets an address to dirty, even if the change entries would +// otherwise suggest it as clean. This method is an ugly hack to handle the RIPEMD +// precompile consensus exception. +func (j *journal) dirty(addr common.Address) { + j.dirties[addr]++ +} // length returns the current number of entries in the journal. func (j *journal) length() int { - return len(*j) + return len(j.entries) } type ( @@ -95,16 +146,24 @@ type ( } ) -func (ch createObjectChange) undo(s *StateDB) { +func (ch createObjectChange) revert(s *StateDB) { delete(s.stateObjects, *ch.account) delete(s.stateObjectsDirty, *ch.account) } -func (ch resetObjectChange) undo(s *StateDB) { +func (ch createObjectChange) dirtied() *common.Address { + return ch.account +} + +func (ch resetObjectChange) revert(s *StateDB) { s.setStateObject(ch.prev) } -func (ch selfDestructChange) undo(s *StateDB) { +func (ch resetObjectChange) dirtied() *common.Address { + return nil +} + +func (ch selfDestructChange) revert(s *StateDB) { obj := s.getStateObject(*ch.account) if obj != nil { obj.selfDestructed = ch.prev @@ -112,42 +171,68 @@ func (ch selfDestructChange) undo(s *StateDB) { } } -var ripemd = common.HexToAddress("0000000000000000000000000000000000000003") - -func (ch touchChange) undo(s *StateDB) { - if !ch.prev && *ch.account != ripemd { - s.getStateObject(*ch.account).touched = ch.prev - if !ch.prevDirty { - delete(s.stateObjectsDirty, *ch.account) - } - } +func (ch selfDestructChange) dirtied() *common.Address { + return ch.account } -func (ch balanceChange) undo(s *StateDB) { +var ripemd = common.HexToAddress("0000000000000000000000000000000000000003") + +func (ch touchChange) revert(s *StateDB) { +} + +func (ch touchChange) dirtied() *common.Address { + return ch.account +} + +func (ch balanceChange) revert(s *StateDB) { s.getStateObject(*ch.account).setBalance(ch.prev) } -func (ch nonceChange) undo(s *StateDB) { +func (ch balanceChange) dirtied() *common.Address { + return ch.account +} + +func (ch nonceChange) revert(s *StateDB) { s.getStateObject(*ch.account).setNonce(ch.prev) } -func (ch codeChange) undo(s *StateDB) { +func (ch nonceChange) dirtied() *common.Address { + return ch.account +} + +func (ch codeChange) revert(s *StateDB) { s.getStateObject(*ch.account).setCode(common.BytesToHash(ch.prevhash), ch.prevcode) } -func (ch storageChange) undo(s *StateDB) { +func (ch codeChange) dirtied() *common.Address { + return ch.account +} + +func (ch storageChange) revert(s *StateDB) { s.getStateObject(*ch.account).setState(ch.key, ch.prevalue) } -func (ch transientStorageChange) undo(s *StateDB) { +func (ch storageChange) dirtied() *common.Address { + return ch.account +} + +func (ch transientStorageChange) revert(s *StateDB) { s.setTransientState(*ch.account, ch.key, ch.prevalue) } -func (ch refundChange) undo(s *StateDB) { +func (ch transientStorageChange) dirtied() *common.Address { + return nil +} + +func (ch refundChange) revert(s *StateDB) { s.refund = ch.prev } -func (ch addLogChange) undo(s *StateDB) { +func (ch refundChange) dirtied() *common.Address { + return nil +} + +func (ch addLogChange) revert(s *StateDB) { logs := s.logs[ch.txhash] if len(logs) == 1 { delete(s.logs, ch.txhash) @@ -157,11 +242,19 @@ func (ch addLogChange) undo(s *StateDB) { s.logSize-- } -func (ch addPreimageChange) undo(s *StateDB) { +func (ch addLogChange) dirtied() *common.Address { + return nil +} + +func (ch addPreimageChange) revert(s *StateDB) { delete(s.preimages, ch.hash) } -func (ch accessListAddAccountChange) undo(s *StateDB) { +func (ch addPreimageChange) dirtied() *common.Address { + return nil +} + +func (ch accessListAddAccountChange) revert(s *StateDB) { /* One important invariant here, is that whenever a (addr, slot) is added, if the addr is not already present, the add causes two journal entries: @@ -174,6 +267,14 @@ func (ch accessListAddAccountChange) undo(s *StateDB) { s.accessList.DeleteAddress(*ch.address) } -func (ch accessListAddSlotChange) undo(s *StateDB) { +func (ch accessListAddAccountChange) dirtied() *common.Address { + return nil +} + +func (ch accessListAddSlotChange) revert(s *StateDB) { s.accessList.DeleteSlot(*ch.address, *ch.slot) } + +func (ch accessListAddSlotChange) dirtied() *common.Address { + return nil +} diff --git a/core/state/state_object.go b/core/state/state_object.go index f24702e4c2..700ef6cef8 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -96,10 +96,6 @@ type stateObject struct { // Flag whether the object was created in the current transaction created bool - - touched bool - - onDirty func(addr common.Address) // Callback method to mark a state object newly dirty } // empty returns whether the account is considered empty. @@ -117,7 +113,7 @@ type Account struct { } // newObject creates a state object. -func newObject(db *StateDB, address common.Address, data Account, onDirty func(addr common.Address)) *stateObject { +func newObject(db *StateDB, address common.Address, data Account) *stateObject { if data.Balance == nil { data.Balance = new(big.Int) } @@ -135,7 +131,6 @@ func newObject(db *StateDB, address common.Address, data Account, onDirty func(a originStorage: make(Storage), pendingStorage: make(Storage), dirtyStorage: make(Storage), - onDirty: onDirty, } } @@ -153,23 +148,17 @@ func (s *stateObject) setError(err error) { func (s *stateObject) markSelfdestructed() { s.selfDestructed = true - if s.onDirty != nil { - s.onDirty(s.Address()) - s.onDirty = nil - } } func (s *stateObject) touch() { - s.db.journal = append(s.db.journal, touchChange{ - account: &s.address, - prev: s.touched, - prevDirty: s.onDirty == nil, + s.db.journal.append(touchChange{ + account: &s.address, }) - if s.onDirty != nil { - s.onDirty(s.Address()) - s.onDirty = nil + if s.address == ripemd { + // Explicitly put it in the dirty-cache, which is otherwise generated from + // flattened journals. + s.db.journal.dirty(s.address) } - s.touched = true } func (s *stateObject) getTrie(db Database) Trie { @@ -244,7 +233,7 @@ func (s *stateObject) SetState(db Database, key, value common.Hash) { return } // New value is different, update and journal the change - s.db.journal = append(s.db.journal, storageChange{ + s.db.journal.append(storageChange{ account: &s.address, key: key, prevalue: prev, @@ -272,11 +261,6 @@ func (s *stateObject) SetStorage(storage map[common.Hash]common.Hash) { func (s *stateObject) setState(key, value common.Hash) { s.dirtyStorage[key] = value - - if s.onDirty != nil { - s.onDirty(s.Address()) - s.onDirty = nil - } } // finalise moves all dirty storage slots into the pending area to be hashed or @@ -380,7 +364,7 @@ func (s *stateObject) SubBalance(amount *big.Int) { } func (s *stateObject) SetBalance(amount *big.Int) { - s.db.journal = append(s.db.journal, balanceChange{ + s.db.journal.append(balanceChange{ account: &s.address, prev: new(big.Int).Set(s.data.Balance), }) @@ -389,14 +373,10 @@ func (s *stateObject) SetBalance(amount *big.Int) { func (s *stateObject) setBalance(amount *big.Int) { s.data.Balance = amount - if s.onDirty != nil { - s.onDirty(s.Address()) - s.onDirty = nil - } } -func (s *stateObject) deepCopy(db *StateDB, onDirty func(addr common.Address)) *stateObject { - stateObject := newObject(db, s.address, s.data, onDirty) +func (s *stateObject) deepCopy(db *StateDB) *stateObject { + stateObject := newObject(db, s.address, s.data) if s.trie != nil { stateObject.trie = db.db.CopyTrie(s.trie) } @@ -436,7 +416,7 @@ func (s *stateObject) Code(db Database) []byte { func (s *stateObject) SetCode(codeHash common.Hash, code []byte) { prevcode := s.Code(s.db.db) - s.db.journal = append(s.db.journal, codeChange{ + s.db.journal.append(codeChange{ account: &s.address, prevhash: s.CodeHash(), prevcode: prevcode, @@ -448,14 +428,10 @@ func (s *stateObject) setCode(codeHash common.Hash, code []byte) { s.code = code s.data.CodeHash = codeHash[:] s.dirtyCode = true - if s.onDirty != nil { - s.onDirty(s.Address()) - s.onDirty = nil - } } func (s *stateObject) SetNonce(nonce uint64) { - s.db.journal = append(s.db.journal, nonceChange{ + s.db.journal.append(nonceChange{ account: &s.address, prev: s.data.Nonce, }) @@ -464,10 +440,6 @@ func (s *stateObject) SetNonce(nonce uint64) { func (s *stateObject) setNonce(nonce uint64) { s.data.Nonce = nonce - if s.onDirty != nil { - s.onDirty(s.Address()) - s.onDirty = nil - } } func (s *stateObject) CodeHash() []byte { diff --git a/core/state/statedb.go b/core/state/statedb.go index 35be068de9..fe393ff74b 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -78,7 +78,7 @@ type StateDB struct { // Journal of state modifications. This is the backbone of // Snapshot and RevertToSnapshot. - journal journal + journal *journal validRevisions []revision nextRevisionId int @@ -117,6 +117,7 @@ func New(root common.Hash, db Database) (*StateDB, error) { stateObjectsDirty: make(map[common.Address]struct{}), logs: make(map[common.Hash][]*types.Log), preimages: make(map[common.Hash][]byte), + journal: newJournal(), accessList: newAccessList(), transientStorage: newTransientStorage(), }, nil @@ -155,7 +156,7 @@ func (s *StateDB) Reset(root common.Hash) error { } func (s *StateDB) AddLog(log *types.Log) { - s.journal = append(s.journal, addLogChange{txhash: s.thash}) + s.journal.append(addLogChange{txhash: s.thash}) log.TxHash = s.thash log.TxIndex = uint(s.txIndex) @@ -183,7 +184,7 @@ func (s *StateDB) Logs() []*types.Log { // AddPreimage records a SHA3 preimage seen by the VM. func (s *StateDB) AddPreimage(hash common.Hash, preimage []byte) { if _, ok := s.preimages[hash]; !ok { - s.journal = append(s.journal, addPreimageChange{hash: hash}) + s.journal.append(addPreimageChange{hash: hash}) pi := make([]byte, len(preimage)) copy(pi, preimage) s.preimages[hash] = pi @@ -197,15 +198,14 @@ func (s *StateDB) Preimages() map[common.Hash][]byte { // AddRefund adds gas to the refund counter func (s *StateDB) AddRefund(gas uint64) { - s.journal = append(s.journal, refundChange{prev: s.refund}) + s.journal.append(refundChange{prev: s.refund}) s.refund += gas } // SubRefund removes gas from the refund counter. // This method will panic if the refund counter goes below zero func (s *StateDB) SubRefund(gas uint64) { - s.journal = append(s.journal, refundChange{ - prev: s.refund}) + s.journal.append(refundChange{prev: s.refund}) if gas > s.refund { panic(fmt.Sprintf("Refund counter below zero (gas: %d > refund: %d)", gas, s.refund)) } @@ -341,7 +341,7 @@ func (s *StateDB) StorageTrie(addr common.Address) Trie { if stateObject == nil { return nil } - cpy := stateObject.deepCopy(s, nil) + cpy := stateObject.deepCopy(s) cpy.updateTrie(s.db) return cpy.getTrie(s.db) } @@ -421,7 +421,7 @@ func (s *StateDB) SelfDestruct(addr common.Address) { if stateObject == nil { return } - s.journal = append(s.journal, selfDestructChange{ + s.journal.append(selfDestructChange{ account: &addr, prev: stateObject.selfDestructed, prevbalance: new(big.Int).Set(stateObject.Balance()), @@ -450,7 +450,7 @@ func (s *StateDB) SetTransientState(addr common.Address, key, value common.Hash) return } - s.journal = append(s.journal, transientStorageChange{ + s.journal.append(transientStorageChange{ account: &addr, key: key, prevalue: prev, @@ -542,7 +542,7 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { return nil } // Insert into the live set - obj := newObject(s, addr, data, s.MarkStateObjectDirty) + obj := newObject(s, addr, data) s.setStateObject(obj) return obj } @@ -560,23 +560,16 @@ func (s *StateDB) GetOrNewStateObject(addr common.Address) *stateObject { return stateObject } -// MarkStateObjectDirty adds the specified object to the dirty map to avoid costly -// state object cache iteration to find a handful of modified ones. -func (s *StateDB) MarkStateObjectDirty(addr common.Address) { - s.stateObjectsDirty[addr] = struct{}{} -} - // createObject creates a new state object. If there is an existing account with // the given address, it is overwritten and returned as the second return value. func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject) { prev = s.getDeletedStateObject(addr) // Note, prev might have been deleted, we need that! - - newobj = newObject(s, addr, Account{}, s.MarkStateObjectDirty) + newobj = newObject(s, addr, Account{}) newobj.setNonce(0) // sets the object to dirty if prev == nil { - s.journal = append(s.journal, createObjectChange{account: &addr}) + s.journal.append(createObjectChange{account: &addr}) } else { - s.journal = append(s.journal, resetObjectChange{prev: prev}) + s.journal.append(resetObjectChange{prev: prev}) } newobj.created = true @@ -644,26 +637,44 @@ func (s *StateDB) Copy() *StateDB { state := &StateDB{ db: s.db, trie: s.db.CopyTrie(s.trie), - stateObjects: make(map[common.Address]*stateObject, len(s.stateObjectsDirty)), + stateObjects: make(map[common.Address]*stateObject, len(s.journal.dirties)), stateObjectsPending: make(map[common.Address]struct{}, len(s.stateObjectsPending)), - stateObjectsDirty: make(map[common.Address]struct{}, len(s.stateObjectsDirty)), + stateObjectsDirty: make(map[common.Address]struct{}, len(s.journal.dirties)), refund: s.refund, logs: make(map[common.Hash][]*types.Log, len(s.logs)), logSize: s.logSize, preimages: make(map[common.Hash][]byte), + journal: newJournal(), } - // Above, we don't copy the actual journal. This means that if the copy is copied, the - // loop above will be a no-op, since the copy's journal is empty. - // Thus, here we iterate over stateObjects, to enable copies of copies + // Copy the dirty states, logs, and preimages + for addr := range s.journal.dirties { + // As documented [here](https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527), + // and in the Finalise-method, there is a case where an object is in the journal but not + // in the stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we need to check for + // nil + if object, exist := s.stateObjects[addr]; exist { + // Even though the original object is dirty, we are not copying the journal, + // so we need to make sure that any side-effect the journal would have caused + // during a commit (or similar op) is already applied to the copy. + state.stateObjects[addr] = object.deepCopy(state) + + state.stateObjectsDirty[addr] = struct{}{} // Mark the copy dirty to force internal (code/state) commits + state.stateObjectsPending[addr] = struct{}{} // Mark the copy pending to force external (account) commits + } + } + // Above, we don't copy the actual journal. This means that if the copy + // is copied, the loop above will be a no-op, since the copy's journal + // is empty. Thus, here we iterate over stateObjects, to enable copies + // of copies. for addr := range s.stateObjectsPending { if _, exist := state.stateObjects[addr]; !exist { - state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state, state.MarkStateObjectDirty) + state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state) } state.stateObjectsPending[addr] = struct{}{} } for addr := range s.stateObjectsDirty { if _, exist := state.stateObjects[addr]; !exist { - state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state, state.MarkStateObjectDirty) + state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state) } state.stateObjectsDirty[addr] = struct{}{} } @@ -697,7 +708,7 @@ func (s *StateDB) Copy() *StateDB { func (s *StateDB) Snapshot() int { id := s.nextRevisionId s.nextRevisionId++ - s.validRevisions = append(s.validRevisions, revision{id, len(s.journal)}) + s.validRevisions = append(s.validRevisions, revision{id, s.journal.length()}) return id } @@ -712,13 +723,8 @@ func (s *StateDB) RevertToSnapshot(revid int) { } snapshot := s.validRevisions[idx].journalIndex - // Replay the journal to undo changes. - for i := len(s.journal) - 1; i >= snapshot; i-- { - s.journal[i].undo(s) - } - s.journal = s.journal[:snapshot] - - // Remove invalidated snapshots from the stack. + // Replay the journal to undo changes and remove invalidated snapshots + s.journal.revert(s, snapshot) s.validRevisions = s.validRevisions[:idx] } @@ -731,11 +737,12 @@ func (s *StateDB) GetRefund() uint64 { // the journal as well as the refunds. Finalise, however, will not push any updates // into the tries just yet. Only IntermediateRoot or Commit will do that. func (s *StateDB) Finalise(deleteEmptyObjects bool) { - for addr := range s.stateObjectsDirty { + for addr := range s.journal.dirties { obj, exist := s.stateObjects[addr] if !exist { continue } + if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { obj.deleted = true } else { @@ -804,9 +811,9 @@ func (s *StateDB) DeleteSuicides() { } func (s *StateDB) clearJournalAndRefund() { - s.journal = nil + s.journal = newJournal() + s.validRevisions = s.validRevisions[:0] s.refund = 0 - s.validRevisions = s.validRevisions[:0] // Snapshots can be created without journal entires } // Commit writes the state to the underlying in-memory trie database. @@ -814,6 +821,9 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) { // Finalize any pending changes and merge everything into the tries s.IntermediateRoot(deleteEmptyObjects) + for addr := range s.journal.dirties { + s.stateObjectsDirty[addr] = struct{}{} + } // Commit objects to the trie, measuring the elapsed time codeWriter := s.db.TrieDB().DiskDB().NewBatch() for addr := range s.stateObjectsDirty { @@ -895,7 +905,7 @@ func (s *StateDB) Prepare(rules params.Rules, sender, coinbase common.Address, d // AddAddressToAccessList adds the given address to the access list func (s *StateDB) AddAddressToAccessList(addr common.Address) { if s.accessList.AddAddress(addr) { - s.journal = append(s.journal, accessListAddAccountChange{&addr}) + s.journal.append(accessListAddAccountChange{&addr}) } } @@ -907,10 +917,10 @@ func (s *StateDB) AddSlotToAccessList(addr common.Address, slot common.Hash) { // scope of 'address' without having the 'address' become already added // to the access list (via call-variant, create, etc). // Better safe than sorry, though - s.journal = append(s.journal, accessListAddAccountChange{&addr}) + s.journal.append(accessListAddAccountChange{&addr}) } if slotMod { - s.journal = append(s.journal, accessListAddSlotChange{ + s.journal.append(accessListAddSlotChange{ address: &addr, slot: &slot, }) diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index 49a86ec857..e7d54fc3d7 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -439,11 +439,12 @@ func (s *StateSuite) TestTouchDelete(c *check.C) { snapshot := s.state.Snapshot() s.state.AddBalance(common.Address{}, new(big.Int)) - if len(s.state.stateObjectsDirty) != 1 { + + if len(s.state.journal.dirties) != 1 { c.Fatal("expected one dirty state object") } s.state.RevertToSnapshot(snapshot) - if len(s.state.stateObjectsDirty) != 0 { + if len(s.state.journal.dirties) != 0 { c.Fatal("expected no dirty state object") } } @@ -550,7 +551,7 @@ func TestStateDBAccessList(t *testing.T) { verifySlots("cc", "01") // now start rolling back changes - state.journal[7].undo(state) + state.journal.revert(state, 7) if _, ok := state.SlotInAccessList(addr("cc"), slot("01")); ok { t.Fatalf("slot present, expected missing") } @@ -558,7 +559,7 @@ func TestStateDBAccessList(t *testing.T) { verifySlots("aa", "01") verifySlots("bb", "01", "02", "03") - state.journal[6].undo(state) + state.journal.revert(state, 6) if state.AddressInAccessList(addr("cc")) { t.Fatalf("addr present, expected missing") } @@ -566,40 +567,40 @@ func TestStateDBAccessList(t *testing.T) { verifySlots("aa", "01") verifySlots("bb", "01", "02", "03") - state.journal[5].undo(state) + state.journal.revert(state, 5) if _, ok := state.SlotInAccessList(addr("aa"), slot("01")); ok { t.Fatalf("slot present, expected missing") } verifyAddrs("aa", "bb") verifySlots("bb", "01", "02", "03") - state.journal[4].undo(state) + state.journal.revert(state, 4) if _, ok := state.SlotInAccessList(addr("bb"), slot("03")); ok { t.Fatalf("slot present, expected missing") } verifyAddrs("aa", "bb") verifySlots("bb", "01", "02") - state.journal[3].undo(state) + state.journal.revert(state, 3) if _, ok := state.SlotInAccessList(addr("bb"), slot("02")); ok { t.Fatalf("slot present, expected missing") } verifyAddrs("aa", "bb") verifySlots("bb", "01") - state.journal[2].undo(state) + state.journal.revert(state, 2) if _, ok := state.SlotInAccessList(addr("bb"), slot("01")); ok { t.Fatalf("slot present, expected missing") } verifyAddrs("aa", "bb") - state.journal[1].undo(state) + state.journal.revert(state, 1) if state.AddressInAccessList(addr("bb")) { t.Fatalf("addr present, expected missing") } verifyAddrs("aa") - state.journal[0].undo(state) + state.journal.revert(state, 0) if state.AddressInAccessList(addr("aa")) { t.Fatalf("addr present, expected missing") } @@ -642,7 +643,7 @@ func TestStateDBTransientStorage(t *testing.T) { // revert the transient state being set and then check that the // value is now the empty hash - state.journal[0].undo(state) + state.journal.revert(state, 0) if got, exp := state.GetTransientState(addr, key), (common.Hash{}); exp != got { t.Fatalf("transient storage mismatch: have %x, want %x", got, exp) } @@ -690,3 +691,18 @@ func TestDeleteCreateRevert(t *testing.T) { t.Fatalf("self-destructed contract came alive") } } + +// TestCopyOfCopy tests that modified objects are carried over to the copy, and the copy of the copy. +// See https://github.com/ethereum/go-ethereum/pull/15225#issuecomment-380191512 +func TestCopyOfCopy(t *testing.T) { + state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase())) + addr := common.HexToAddress("aaaa") + state.SetBalance(addr, big.NewInt(42)) + + if got := state.Copy().GetBalance(addr).Uint64(); got != 42 { + t.Fatalf("1st copy fail, expected 42, got %v", got) + } + if got := state.Copy().Copy().GetBalance(addr).Uint64(); got != 42 { + t.Fatalf("2nd copy fail, expected 42, got %v", got) + } +} diff --git a/tests/state_test.go b/tests/state_test.go index 871c1f347a..9395d514ff 100644 --- a/tests/state_test.go +++ b/tests/state_test.go @@ -43,14 +43,8 @@ func TestState(t *testing.T) { // Expected failures: st.fails(`^stRevertTest/RevertPrecompiledTouch\.json/EIP158`, "bug in test") - st.fails(`^stRevertTest/RevertPrefoundEmptyOOG\.json/EIP158`, "bug in test") st.fails(`^stRevertTest/RevertPrecompiledTouch\.json/Byzantium`, "bug in test") - st.fails(`^stRevertTest/RevertPrefoundEmptyOOG\.json/Byzantium`, "bug in test") st.fails(`^stRandom2/randomStatetest64[45]\.json/(EIP150|Frontier|Homestead)/.*`, "known bug #15119") - st.fails(`^stCreateTest/TransactionCollisionToEmpty\.json/EIP158/2`, "known bug ") - st.fails(`^stCreateTest/TransactionCollisionToEmpty\.json/EIP158/3`, "known bug ") - st.fails(`^stCreateTest/TransactionCollisionToEmpty\.json/Byzantium/2`, "known bug ") - st.fails(`^stCreateTest/TransactionCollisionToEmpty\.json/Byzantium/3`, "known bug ") st.walk(t, stateTestDir, func(t *testing.T, name string, test *StateTest) { for _, subtest := range test.Subtests() {