From d754f363e5e960443d2d4c216246e318a5c5cb42 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Tue, 24 Feb 2026 16:47:27 +0800 Subject: [PATCH] core/state: remove account reset operation v2 #29520 (#1934) --- core/state/journal.go | 151 +++++++++++---- core/state/state_object.go | 51 ++--- core/state/state_test.go | 105 +--------- core/state/statedb.go | 329 +++++++++++++++----------------- core/state/statedb_fuzz_test.go | 28 +-- core/state/statedb_test.go | 201 +++++++++++++------ core/vm/evm.go | 17 +- core/vm/interface.go | 1 + trie/triestate/state.go | 8 + 9 files changed, 471 insertions(+), 420 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index ec7ea43d74..ff97e63fd4 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -17,6 +17,7 @@ package state import ( + "maps" "math/big" "github.com/XinFinOrg/XDPoSChain/common" @@ -31,6 +32,9 @@ type journalEntry interface { // dirtied returns the Ethereum address modified by this journal entry. dirtied() *common.Address + + // copy returns a deep-copied journal entry. + copy() journalEntry } // journal contains the list of state modifications applied since the last state @@ -85,6 +89,18 @@ func (j *journal) length() int { return len(j.entries) } +// copy returns a deep-copied journal. +func (j *journal) copy() *journal { + entries := make([]journalEntry, 0, j.length()) + for i := 0; i < j.length(); i++ { + entries = append(entries, j.entries[i].copy()) + } + return &journal{ + entries: entries, + dirties: maps.Clone(j.dirties), + } +} + func (j *journal) createContract(addr common.Address) { j.append(createContractChange{account: addr}) } @@ -107,17 +123,6 @@ type ( createContractChange struct { account common.Address } - resetObjectChange struct { - account common.Address - prev *stateObject - prevdestruct bool - prevAccount []byte - prevStorage map[common.Hash][]byte - - prevAccountOriginExist bool - prevAccountOrigin []byte - prevStorageOrigin map[common.Hash][]byte - } selfDestructChange struct { account common.Address prev bool // whether account had already self-destructed @@ -155,6 +160,7 @@ type ( touchChange struct { account common.Address } + // Changes to the access list accessListAddAccountChange struct { address common.Address @@ -164,6 +170,7 @@ type ( slot common.Hash } + // Changes to transient storage transientStorageChange struct { account common.Address key, prevalue common.Hash @@ -172,50 +179,30 @@ type ( func (ch createObjectChange) revert(s *StateDB) { delete(s.stateObjects, ch.account) - delete(s.stateObjectsDirty, ch.account) } func (ch createObjectChange) dirtied() *common.Address { return &ch.account } +func (ch createObjectChange) copy() journalEntry { + return createObjectChange{ + account: ch.account, + } +} + func (ch createContractChange) revert(s *StateDB) { - s.getStateObject(ch.account).created = false + s.getStateObject(ch.account).newContract = false } func (ch createContractChange) dirtied() *common.Address { return nil } -func (ch resetObjectChange) revert(s *StateDB) { - s.setStateObject(ch.prev) - if !ch.prevdestruct { - delete(s.stateObjectsDestruct, ch.prev.address) +func (ch createContractChange) copy() journalEntry { + return createContractChange{ + account: ch.account, } - if ch.prevAccount != nil { - s.accounts[ch.prev.addrHash] = ch.prevAccount - } else { - delete(s.accounts, ch.prev.addrHash) - } - if ch.prevStorage != nil { - s.storages[ch.prev.addrHash] = ch.prevStorage - } else { - delete(s.storages, ch.prev.addrHash) - } - if ch.prevAccountOriginExist { - s.accountsOrigin[ch.prev.address] = ch.prevAccountOrigin - } else { - delete(s.accountsOrigin, ch.prev.address) - } - if ch.prevStorageOrigin != nil { - s.storagesOrigin[ch.prev.address] = ch.prevStorageOrigin - } else { - delete(s.storagesOrigin, ch.prev.address) - } -} - -func (ch resetObjectChange) dirtied() *common.Address { - return &ch.account } func (ch selfDestructChange) revert(s *StateDB) { @@ -230,6 +217,14 @@ func (ch selfDestructChange) dirtied() *common.Address { return &ch.account } +func (ch selfDestructChange) copy() journalEntry { + return selfDestructChange{ + account: ch.account, + prev: ch.prev, + prevbalance: new(big.Int).Set(ch.prevbalance), + } +} + var ripemd = common.HexToAddress("0000000000000000000000000000000000000003") func (ch touchChange) revert(s *StateDB) { @@ -239,6 +234,12 @@ func (ch touchChange) dirtied() *common.Address { return &ch.account } +func (ch touchChange) copy() journalEntry { + return touchChange{ + account: ch.account, + } +} + func (ch balanceChange) revert(s *StateDB) { s.getStateObject(ch.account).setBalance(ch.prev) } @@ -247,6 +248,13 @@ func (ch balanceChange) dirtied() *common.Address { return &ch.account } +func (ch balanceChange) copy() journalEntry { + return balanceChange{ + account: ch.account, + prev: new(big.Int).Set(ch.prev), + } +} + func (ch nonceChange) revert(s *StateDB) { s.getStateObject(ch.account).setNonce(ch.prev) } @@ -255,6 +263,13 @@ func (ch nonceChange) dirtied() *common.Address { return &ch.account } +func (ch nonceChange) copy() journalEntry { + return nonceChange{ + account: ch.account, + prev: ch.prev, + } +} + func (ch codeChange) revert(s *StateDB) { s.getStateObject(ch.account).setCode(crypto.Keccak256Hash(ch.prevCode), ch.prevCode) } @@ -263,6 +278,13 @@ func (ch codeChange) dirtied() *common.Address { return &ch.account } +func (ch codeChange) copy() journalEntry { + return codeChange{ + account: ch.account, + prevCode: common.CopyBytes(ch.prevCode), + } +} + func (ch storageChange) revert(s *StateDB) { s.getStateObject(ch.account).setState(ch.key, ch.prevalue) } @@ -271,6 +293,14 @@ func (ch storageChange) dirtied() *common.Address { return &ch.account } +func (ch storageChange) copy() journalEntry { + return storageChange{ + account: ch.account, + key: ch.key, + prevalue: ch.prevalue, + } +} + func (ch transientStorageChange) revert(s *StateDB) { s.setTransientState(ch.account, ch.key, ch.prevalue) } @@ -279,6 +309,14 @@ func (ch transientStorageChange) dirtied() *common.Address { return nil } +func (ch transientStorageChange) copy() journalEntry { + return transientStorageChange{ + account: ch.account, + key: ch.key, + prevalue: ch.prevalue, + } +} + func (ch refundChange) revert(s *StateDB) { s.refund = ch.prev } @@ -287,6 +325,12 @@ func (ch refundChange) dirtied() *common.Address { return nil } +func (ch refundChange) copy() journalEntry { + return refundChange{ + prev: ch.prev, + } +} + func (ch addLogChange) revert(s *StateDB) { logs := s.logs[ch.txhash] if len(logs) == 1 { @@ -301,6 +345,12 @@ func (ch addLogChange) dirtied() *common.Address { return nil } +func (ch addLogChange) copy() journalEntry { + return addLogChange{ + txhash: ch.txhash, + } +} + func (ch addPreimageChange) revert(s *StateDB) { delete(s.preimages, ch.hash) } @@ -309,6 +359,12 @@ func (ch addPreimageChange) dirtied() *common.Address { return nil } +func (ch addPreimageChange) copy() journalEntry { + return addPreimageChange{ + hash: ch.hash, + } +} + func (ch accessListAddAccountChange) revert(s *StateDB) { /* One important invariant here, is that whenever a (addr, slot) is added, if the @@ -326,6 +382,12 @@ func (ch accessListAddAccountChange) dirtied() *common.Address { return nil } +func (ch accessListAddAccountChange) copy() journalEntry { + return accessListAddAccountChange{ + address: ch.address, + } +} + func (ch accessListAddSlotChange) revert(s *StateDB) { s.accessList.DeleteSlot(ch.address, ch.slot) } @@ -333,3 +395,10 @@ func (ch accessListAddSlotChange) revert(s *StateDB) { func (ch accessListAddSlotChange) dirtied() *common.Address { return nil } + +func (ch accessListAddSlotChange) copy() journalEntry { + return accessListAddSlotChange{ + address: ch.address, + slot: ch.slot, + } +} diff --git a/core/state/state_object.go b/core/state/state_object.go index f2cdd71247..baaf93f507 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -32,22 +32,8 @@ import ( "github.com/XinFinOrg/XDPoSChain/trie/trienode" ) -type Code []byte - -func (c Code) String() string { - return string(c) //strings.Join(Disassemble(c), " ") -} - type Storage map[common.Hash]common.Hash -func (s Storage) String() (str string) { - for key, value := range s { - str += fmt.Sprintf("%X : %X\n", key, value) - } - - return -} - func (s Storage) Copy() Storage { return maps.Clone(s) } @@ -66,8 +52,8 @@ type stateObject struct { data types.StateAccount // Account data with all mutations applied in the scope of block // Write caches. - trie Trie // storage trie, which becomes non-nil on first access - code Code // contract bytecode, which gets set when code is loaded + trie Trie // storage trie, which becomes non-nil on first access + code []byte // contract bytecode, which gets set when code is loaded originStorage Storage // Storage cache of original entries to dedup rewrites pendingStorage Storage // Storage entries that need to be flushed to disk, at the end of an entire block @@ -80,13 +66,12 @@ type stateObject struct { // account is still accessible in the scope of same transaction. selfDestructed bool - // Flag whether the account was marked as deleted. A self-destructed account - // or an account that is considered as empty will be marked as deleted at - // the end of transaction and no longer accessible anymore. - deleted bool - - // Flag whether the object was created in the current transaction - created bool + // This is an EIP-6780 flag indicating whether the object is eligible for + // self-destruct according to EIP-6780. The flag could be set either when + // the contract is just created within the current transaction, or when the + // object was previously existent and is being deployed as a contract within + // the current transaction. + newContract bool } // empty returns whether the account is considered empty. @@ -96,10 +81,7 @@ func (s *stateObject) empty() bool { // newObject creates a state object. func newObject(db *StateDB, address common.Address, acct *types.StateAccount) *stateObject { - var ( - origin = acct - created = acct == nil // true if the account was not existent - ) + origin := acct if acct == nil { acct = types.NewEmptyStateAccount() } @@ -112,7 +94,6 @@ func newObject(db *StateDB, address common.Address, acct *types.StateAccount) *s originStorage: make(Storage), pendingStorage: make(Storage), dirtyStorage: make(Storage), - created: created, } } @@ -229,6 +210,10 @@ func (s *stateObject) finalise() { if len(s.dirtyStorage) > 0 { s.dirtyStorage = make(Storage) } + // Revoke the flag at the end of the transaction. It finalizes the status + // of the newly-created object as it's no longer eligible for self-destruct + // by EIP-6780. For non-newly-created objects, it's a no-op. + s.newContract = false } // updateTrie writes cached storage modifications into the object's storage trie. @@ -396,12 +381,12 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject { obj.trie = db.db.CopyTrie(s.trie) } obj.code = s.code - obj.dirtyStorage = s.dirtyStorage.Copy() obj.originStorage = s.originStorage.Copy() obj.pendingStorage = s.pendingStorage.Copy() - obj.selfDestructed = s.selfDestructed + obj.dirtyStorage = s.dirtyStorage.Copy() obj.dirtyCode = s.dirtyCode - obj.deleted = s.deleted + obj.selfDestructed = s.selfDestructed + obj.newContract = s.newContract return obj } @@ -416,7 +401,7 @@ func (s *stateObject) Address() common.Address { // Code returns the contract code associated with this object, if any. func (s *stateObject) Code() []byte { - if s.code != nil { + if len(s.code) != 0 { return s.code } if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) { @@ -434,7 +419,7 @@ func (s *stateObject) Code() []byte { // or zero if none. This method is an almost mirror of Code, but uses a cache // inside the database to avoid loading codes seen recently. func (s *stateObject) CodeSize() int { - if s.code != nil { + if len(s.code) != 0 { return len(s.code) } if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) { diff --git a/core/state/state_test.go b/core/state/state_test.go index 2a63b6732e..012a7f0690 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -190,107 +190,20 @@ func TestSnapshotEmpty(t *testing.T) { s.state.RevertToSnapshot(s.state.Snapshot()) } -func TestSnapshot2(t *testing.T) { - db := rawdb.NewMemoryDatabase() - state, _ := New(types.EmptyRootHash, NewDatabase(db)) +func TestCreateObjectRevert(t *testing.T) { + state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase())) + addr := common.BytesToAddress([]byte("so0")) + snap := state.Snapshot() - stateobjaddr0 := common.BytesToAddress([]byte("so0")) - stateobjaddr1 := common.BytesToAddress([]byte("so1")) - var storageaddr common.Hash - - data0 := common.BytesToHash([]byte{17}) - data1 := common.BytesToHash([]byte{18}) - - state.SetState(stateobjaddr0, storageaddr, data0) - state.SetState(stateobjaddr1, storageaddr, data1) - - // db, trie are already non-empty values - so0 := state.getStateObject(stateobjaddr0) + state.CreateAccount(addr) + so0 := state.getStateObject(addr) so0.SetBalance(big.NewInt(42)) so0.SetNonce(43) so0.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e'}), []byte{'c', 'a', 'f', 'e'}) - so0.selfDestructed = false - so0.deleted = false state.setStateObject(so0) - root, _ := state.Commit(0, false) - state.Reset(root) - - // and one with deleted == true - so1 := state.getStateObject(stateobjaddr1) - so1.SetBalance(big.NewInt(52)) - so1.SetNonce(53) - so1.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e', '2'}), []byte{'c', 'a', 'f', 'e', '2'}) - so1.selfDestructed = true - so1.deleted = true - state.setStateObject(so1) - - so1 = state.getStateObject(stateobjaddr1) - if so1 != nil { - t.Fatalf("deleted object not nil when getting") - } - - snapshot := state.Snapshot() - state.RevertToSnapshot(snapshot) - - so0Restored := state.getStateObject(stateobjaddr0) - // Update lazily-loaded values before comparing. - so0Restored.GetState(storageaddr) - so0Restored.Code() - // non-deleted is equal (restored) - compareStateObjects(so0Restored, so0, t) - - // deleted should be nil, both before and after restore of state copy - so1Restored := state.getStateObject(stateobjaddr1) - if so1Restored != nil { - t.Fatalf("deleted object not nil after restoring snapshot: %+v", so1Restored) - } -} - -func compareStateObjects(so0, so1 *stateObject, t *testing.T) { - if so0.Address() != so1.Address() { - t.Fatalf("Address mismatch: have %v, want %v", so0.address, so1.address) - } - if so0.Balance().Cmp(so1.Balance()) != 0 { - t.Fatalf("Balance mismatch: have %v, want %v", so0.Balance(), so1.Balance()) - } - if so0.Nonce() != so1.Nonce() { - t.Fatalf("Nonce mismatch: have %v, want %v", so0.Nonce(), so1.Nonce()) - } - if so0.data.Root != so1.data.Root { - t.Errorf("Root mismatch: have %x, want %x", so0.data.Root[:], so1.data.Root[:]) - } - if !bytes.Equal(so0.CodeHash(), so1.CodeHash()) { - t.Fatalf("CodeHash mismatch: have %v, want %v", so0.CodeHash(), so1.CodeHash()) - } - if !bytes.Equal(so0.code, so1.code) { - t.Fatalf("Code mismatch: have %v, want %v", so0.code, so1.code) - } - - if len(so1.dirtyStorage) != len(so0.dirtyStorage) { - t.Errorf("Dirty storage size mismatch: have %d, want %d", len(so1.dirtyStorage), len(so0.dirtyStorage)) - } - for k, v := range so1.dirtyStorage { - if so0.dirtyStorage[k] != v { - t.Errorf("Dirty storage key %x mismatch: have %v, want %v", k, so0.dirtyStorage[k], v) - } - } - for k, v := range so0.dirtyStorage { - if so1.dirtyStorage[k] != v { - t.Errorf("Dirty storage key %x mismatch: have %v, want none.", k, v) - } - } - if len(so1.originStorage) != len(so0.originStorage) { - t.Errorf("Origin storage size mismatch: have %d, want %d", len(so1.originStorage), len(so0.originStorage)) - } - for k, v := range so1.originStorage { - if so0.originStorage[k] != v { - t.Errorf("Origin storage key %x mismatch: have %v, want %v", k, so0.originStorage[k], v) - } - } - for k, v := range so0.originStorage { - if so1.originStorage[k] != v { - t.Errorf("Origin storage key %x mismatch: have %v, want none.", k, v) - } + state.RevertToSnapshot(snap) + if state.Exist(addr) { + t.Error("Unexpected account after revert") } } diff --git a/core/state/statedb.go b/core/state/statedb.go index 0e787ae252..b1b282def9 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -43,6 +43,26 @@ type revision struct { journalIndex int } +type mutationType int + +const ( + update mutationType = iota + deletion +) + +type mutation struct { + typ mutationType + applied bool +} + +func (m *mutation) copy() *mutation { + return &mutation{typ: m.typ, applied: m.applied} +} + +func (m *mutation) isDelete() bool { + return m.typ == deletion +} + // StateDB structs within the ethereum protocol are used to store anything // within the merkle trie. StateDBs take care of caching and storing // nested states. It's the general query interface to retrieve: @@ -70,12 +90,22 @@ type StateDB struct { accountsOrigin map[common.Address][]byte // The original value of mutated accounts in 'slim RLP' encoding storagesOrigin map[common.Address]map[common.Hash][]byte // The original value of mutated slots in prefix-zero trimmed rlp format - // This map holds 'live' objects, which will get modified while processing - // a state transition. - stateObjects map[common.Address]*stateObject - stateObjectsPending map[common.Address]struct{} // State objects finalized but not yet written to the trie - stateObjectsDirty map[common.Address]struct{} // State objects modified in the current execution - stateObjectsDestruct map[common.Address]*types.StateAccount // State objects destructed in the block along with its previous value + // This map holds 'live' objects, which will get modified while + // processing a state transition. + stateObjects map[common.Address]*stateObject + + // This map holds 'deleted' objects. An object with the same address + // might also occur in the 'stateObjects' map due to account + // resurrection. The account value is tracked as the original value + // before the transition. This map is populated at the transaction + // boundaries. + stateObjectsDestruct map[common.Address]*types.StateAccount + + // This map tracks the account mutations that occurred during the + // transition. Uncommitted mutations belonging to the same account + // can be merged into a single one which is equivalent from database's + // perspective. This map is populated at the transaction boundaries. + mutations map[common.Address]*mutation // DB error. // State objects are used by the consensus core and VM which are @@ -150,9 +180,8 @@ func New(root common.Hash, db Database) (*StateDB, error) { accountsOrigin: make(map[common.Address][]byte), storagesOrigin: make(map[common.Address]map[common.Hash][]byte), stateObjects: make(map[common.Address]*stateObject), - stateObjectsPending: make(map[common.Address]struct{}), - stateObjectsDirty: make(map[common.Address]struct{}), stateObjectsDestruct: make(map[common.Address]*types.StateAccount), + mutations: make(map[common.Address]*mutation), logs: make(map[common.Hash][]*types.Log), preimages: make(map[common.Hash][]byte), journal: newJournal(), @@ -184,8 +213,6 @@ func (s *StateDB) Reset(root common.Hash) error { } s.trie = tr s.stateObjects = make(map[common.Address]*stateObject) - s.stateObjectsPending = make(map[common.Address]struct{}) - s.stateObjectsDirty = make(map[common.Address]struct{}) s.thash = common.Hash{} s.txIndex = 0 s.logs = make(map[common.Hash][]*types.Log) @@ -520,7 +547,7 @@ func (s *StateDB) SelfDestruct6780(addr common.Address) (*big.Int, bool) { if stateObject == nil { return new(big.Int), false } - if stateObject.created { + if stateObject.newContract { return s.SelfDestruct(addr), true } return new(big.Int).Set(stateObject.Balance()), false @@ -586,12 +613,11 @@ func (s *StateDB) updateStateObject(obj *stateObject) { } // deleteStateObject removes the given object from the state trie. -func (s *StateDB) deleteStateObject(obj *stateObject) { +func (s *StateDB) deleteStateObject(addr common.Address) { // Track the amount of time wasted on deleting the account from the trie defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) // Delete the account from the trie - addr := obj.Address() if err := s.trie.DeleteAccount(addr); err != nil { s.setError(fmt.Errorf("deleteStateObject (%x) error: %v", addr[:], err)) } @@ -600,31 +626,22 @@ func (s *StateDB) deleteStateObject(obj *stateObject) { // DeleteAddress removes the address from the state trie. func (s *StateDB) DeleteAddress(addr common.Address) { stateObject := s.getStateObject(addr) - if stateObject != nil && !stateObject.deleted { - stateObject.deleted = true - s.deleteStateObject(stateObject) + if stateObject != nil { + s.deleteStateObject(stateObject.address) } } // getStateObject retrieves a state object given by the address, returning nil if -// the object is not found or was deleted in this execution context. If you need -// to differentiate between non-existent/just-deleted, use getDeletedStateObject. +// the object is not found or was deleted in this execution context. func (s *StateDB) getStateObject(addr common.Address) *stateObject { - if obj := s.getDeletedStateObject(addr); obj != nil && !obj.deleted { - return obj - } - return nil -} - -// getDeletedStateObject is similar to getStateObject, but instead of returning -// nil for a deleted state object, it returns the actual object with the deleted -// flag set. This is needed by the state journal to revert to the correct s- -// destructed object instead of wiping all knowledge about the state object. -func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { // Prefer live objects if any is available if obj := s.stateObjects[addr]; obj != nil { return obj } + // Short circuit if the account is already destructed in this block. + if _, ok := s.stateObjectsDestruct[addr]; ok { + return nil + } // Load the object from the database start := time.Now() data, err := s.trie.GetAccount(addr) @@ -648,71 +665,28 @@ func (s *StateDB) setStateObject(object *stateObject) { // GetOrNewStateObject retrieves a state object or create a new state object if nil. func (s *StateDB) GetOrNewStateObject(addr common.Address) *stateObject { - stateObject := s.getStateObject(addr) - if stateObject == nil { - stateObject, _ = s.createObject(addr) + obj := s.getStateObject(addr) + if obj == nil { + obj, _ = s.createObject(addr) } - return stateObject + return obj } -// 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, nil) - if prev == nil { - s.journal.append(createObjectChange{account: addr}) - } else { - // The original account should be marked as destructed and all cached - // account and storage data should be cleared as well. Note, it must - // be done here, otherwise the destruction event of "original account" - // will be lost. - _, prevdestruct := s.stateObjectsDestruct[prev.address] - if !prevdestruct { - s.stateObjectsDestruct[prev.address] = prev.origin - } - // There may be some cached account/storage data already since IntermediateRoot - // will be called for each transaction before byzantium fork which will always - // cache the latest account/storage data. - prevAccount, ok := s.accountsOrigin[prev.address] - s.journal.append(resetObjectChange{ - account: addr, - prev: prev, - prevdestruct: prevdestruct, - prevAccount: s.accounts[prev.addrHash], - prevStorage: s.storages[prev.addrHash], - prevAccountOriginExist: ok, - prevAccountOrigin: prevAccount, - prevStorageOrigin: s.storagesOrigin[prev.address], - }) - delete(s.accounts, prev.addrHash) - delete(s.storages, prev.addrHash) - delete(s.accountsOrigin, prev.address) - delete(s.storagesOrigin, prev.address) - } - - s.setStateObject(newobj) - if prev != nil && !prev.deleted { - return newobj, prev - } - return newobj, nil +// 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) createObject(addr common.Address) (obj, prev *stateObject) { + obj = newObject(s, addr, nil) + s.journal.append(createObjectChange{account: addr}) + s.setStateObject(obj) + return obj, nil } -// CreateAccount explicitly creates a state object. If a state object with the address -// already exists the balance is carried over to the new account. -// -// CreateAccount is called during the EVM CREATE operation. The situation might arise that -// a contract does the following: -// -// 1. sends funds to sha(account ++ (nonce + 1)) -// 2. tx_create(sha(account ++ nonce)) (note that this gets the address of 1) -// -// Carrying over the balance ensures that Ether doesn't disappear. +// 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 +// consensus bug eventually. func (s *StateDB) CreateAccount(addr common.Address) { - newObj, prev := s.createObject(addr) - if prev != nil { - newObj.setBalance(prev.data.Balance) - } + s.createObject(addr) } // CreateContract is used whenever a contract is created. This may be preceded @@ -722,8 +696,8 @@ func (s *StateDB) CreateAccount(addr common.Address) { // correctly handle EIP-6780 'delete-in-same-transaction' logic. func (s *StateDB) CreateContract(addr common.Address) { obj := s.getStateObject(addr) - if obj != nil && !obj.created { - obj.created = true + if obj != nil && !obj.newContract { + obj.newContract = true s.journal.createContract(addr) } } @@ -772,71 +746,34 @@ func (s *StateDB) Copy() *StateDB { state := &StateDB{ db: s.db, trie: s.db.CopyTrie(s.trie), + hasher: crypto.NewKeccakState(), originalRoot: s.originalRoot, - accounts: make(map[common.Hash][]byte), - storages: make(map[common.Hash]map[common.Hash][]byte), - accountsOrigin: make(map[common.Address][]byte), - storagesOrigin: make(map[common.Address]map[common.Hash][]byte), - 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.journal.dirties)), - stateObjectsDestruct: make(map[common.Address]*types.StateAccount, len(s.stateObjectsDestruct)), + accounts: copySet(s.accounts), + storages: copy2DSet(s.storages), + accountsOrigin: copySet(s.accountsOrigin), + storagesOrigin: copy2DSet(s.storagesOrigin), + stateObjects: make(map[common.Address]*stateObject, len(s.stateObjects)), + stateObjectsDestruct: maps.Clone(s.stateObjectsDestruct), + mutations: make(map[common.Address]*mutation, len(s.mutations)), + dbErr: s.dbErr, refund: s.refund, + thash: s.thash, + txIndex: s.txIndex, logs: make(map[common.Hash][]*types.Log, len(s.logs)), logSize: s.logSize, preimages: maps.Clone(s.preimages), - journal: newJournal(), - hasher: crypto.NewKeccakState(), + journal: s.journal.copy(), + validRevisions: slices.Clone(s.validRevisions), + nextRevisionId: s.nextRevisionId, } - // Copy the dirty states, logs, and preimages - for addr := range s.journal.dirties { - // As documented [here](https://github.com/XinFinOrg/XDPoSChain/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 - } + // Deep copy cached state objects. + for addr, obj := range s.stateObjects { + state.stateObjects[addr] = obj.deepCopy(state) } - // 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.stateObjectsPending[addr] = struct{}{} + // Deep copy the object state markers. + for addr, op := range s.mutations { + state.mutations[addr] = op.copy() } - for addr := range s.stateObjectsDirty { - if _, exist := state.stateObjects[addr]; !exist { - state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state) - } - state.stateObjectsDirty[addr] = struct{}{} - } - // Deep copy the destruction markers. - for addr, value := range s.stateObjectsDestruct { - if value == nil { - state.stateObjectsDestruct[addr] = nil - continue - } - cpy := new(types.StateAccount) - *cpy = *value - state.stateObjectsDestruct[addr] = cpy - } - // Deep copy the state changes made in the scope of block - // along with their original values. - state.accounts = copySet(s.accounts) - state.storages = copy2DSet(s.storages) - state.accountsOrigin = copySet(s.accountsOrigin) - state.storagesOrigin = copy2DSet(s.storagesOrigin) - // Deep copy the logs occurred in the scope of block for hash, logs := range s.logs { cpy := make([]*types.Log, len(logs)) @@ -898,7 +835,8 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { } if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { - obj.deleted = true + delete(s.stateObjects, obj.address) + s.markDelete(addr) // We need to maintain account deletions explicitly (will remain // set indefinitely). Note only the first occurred self-destruct @@ -914,11 +852,9 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { delete(s.accountsOrigin, obj.address) // Clear out any previously updated account data (may be recreated via a resurrect) delete(s.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect) } else { - obj.finalise() // Prefetch slots in the background + obj.finalise() + s.markUpdate(addr) } - obj.created = false - s.stateObjectsPending[addr] = struct{}{} - s.stateObjectsDirty[addr] = struct{}{} } // Invalidate journal because reverting across transactions is not allowed. s.clearJournalAndRefund() @@ -936,19 +872,46 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { // the account prefetcher. Instead, let's process all the storage updates // first, giving the account prefetches just a few more milliseconds of time // to pull useful data from disk. - for addr := range s.stateObjectsPending { - obj := s.stateObjects[addr] - if obj.deleted { - s.deleteStateObject(obj) - s.AccountDeleted += 1 - } else { + for addr, op := range s.mutations { + if op.applied { + continue + } + if op.isDelete() { + continue + } + if obj, ok := s.stateObjects[addr]; ok && obj != nil { obj.updateRoot() - s.updateStateObject(obj) + } + } + // Perform updates before deletions. This prevents resolution of unnecessary trie nodes + // in circumstances similar to the following: + // + // Consider nodes `A` and `B` who share the same full node parent `P` and have no other siblings. + // During the execution of a block: + // - `A` self-destructs, + // - `C` is created, and also shares the parent `P`. + // If the self-destruct is handled first, then `P` would be left with only one child, thus collapsed + // into a shortnode. This requires `B` to be resolved from disk. + // Whereas if the created node is handled first, then the collapse is avoided, and `B` is not resolved. + var ( + deletedAddrs []common.Address + ) + for addr, op := range s.mutations { + if op.applied { + continue + } + op.applied = true + + if op.isDelete() { + deletedAddrs = append(deletedAddrs, addr) + } else { + s.updateStateObject(s.stateObjects[addr]) s.AccountUpdated += 1 } } - if len(s.stateObjectsPending) > 0 { - s.stateObjectsPending = make(map[common.Address]struct{}) + for _, deletedAddr := range deletedAddrs { + s.deleteStateObject(deletedAddr) + s.AccountDeleted += 1 } // Track the amount of time wasted on hashing the account trie defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) @@ -1111,9 +1074,6 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er // 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 var ( accountTrieNodesUpdated int @@ -1124,20 +1084,23 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er codeWriter = s.db.DiskDB().NewBatch() ) // Handle all state deletions first - incomplete, err := s.handleDestruction(nodes) - if err != nil { + if _, err := s.handleDestruction(nodes); err != nil { return common.Hash{}, err } // Handle all state updates afterwards - for addr := range s.stateObjectsDirty { - obj := s.stateObjects[addr] - if obj.deleted { + for addr, op := range s.mutations { + if op.isDelete() { continue } + obj, ok := s.stateObjects[addr] + if !ok || obj == nil { + log.Error("State object missing for mutation during commit", "address", addr) + continue + } + // Write any contract code associated with the state object - if obj.code != nil && obj.dirtyCode { + if len(obj.code) != 0 && obj.dirtyCode { rawdb.WriteCode(codeWriter, common.BytesToHash(obj.CodeHash()), obj.code) - s.trie.UpdateContractCode(obj.Address(), common.BytesToHash(obj.CodeHash()), obj.code) obj.dirtyCode = false } // Write any storage changes in the state object to its storage trie @@ -1198,11 +1161,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er } if root != origin { start := time.Now() - set := &triestate.Set{ - Accounts: s.accountsOrigin, - Storages: s.storagesOrigin, - Incomplete: incomplete, - } + set := triestate.New(s.accountsOrigin, s.storagesOrigin) if err := s.db.TrieDB().Update(root, origin, block, nodes, set); err != nil { return common.Hash{}, err } @@ -1214,7 +1173,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er s.storages = make(map[common.Hash]map[common.Hash][]byte) s.accountsOrigin = make(map[common.Address][]byte) s.storagesOrigin = make(map[common.Address]map[common.Hash][]byte) - s.stateObjectsDirty = make(map[common.Address]struct{}) + s.mutations = make(map[common.Address]*mutation) s.stateObjectsDestruct = make(map[common.Address]*types.StateAccount) return root, nil } @@ -1323,3 +1282,19 @@ func copy2DSet[k comparable](set map[k]map[common.Hash][]byte) map[k]map[common. } return copied } + +func (s *StateDB) markDelete(addr common.Address) { + if _, ok := s.mutations[addr]; !ok { + s.mutations[addr] = &mutation{} + } + s.mutations[addr].applied = false + s.mutations[addr].typ = deletion +} + +func (s *StateDB) markUpdate(addr common.Address) { + if _, ok := s.mutations[addr]; !ok { + s.mutations[addr] = &mutation{} + } + s.mutations[addr].applied = false + s.mutations[addr].typ = update +} diff --git a/core/state/statedb_fuzz_test.go b/core/state/statedb_fuzz_test.go index e3dea0cfc1..1f35d6f942 100644 --- a/core/state/statedb_fuzz_test.go +++ b/core/state/statedb_fuzz_test.go @@ -47,10 +47,11 @@ import ( // A list of states are created by applying actions. The state changes between // each state instance are tracked and be verified. type stateTest struct { - addrs []common.Address // all account addresses - actions [][]testAction // modifications to the state, grouped by block - chunk int // The number of actions per chunk - err error // failure details are reported through this field + addrs []common.Address // all account addresses + actions [][]testAction // modifications to the state, grouped by block + chunk int // The number of actions per chunk + byzantium bool // Whether to use Byzantium finalization rules + err error // failure details are reported through this field } // newStateTestAction creates a random action that changes state. @@ -93,7 +94,9 @@ func newStateTestAction(addr common.Address, r *rand.Rand, index int) testAction { name: "CreateAccount", fn: func(a testAction, s *StateDB) { - s.CreateAccount(addr) + if !s.Exist(addr) { + s.CreateAccount(addr) + } }, }, { @@ -114,9 +117,9 @@ func newStateTestAction(addr common.Address, r *rand.Rand, index int) testAction } for i := range action.args { if nonRandom { - action.args[i] = rand.Int63n(10000) + 1 // set balance to non-zero + action.args[i] = r.Int63n(10000) + 1 // set balance to non-zero } else { - action.args[i] = rand.Int63n(10000) + action.args[i] = r.Int63n(10000) } names = append(names, fmt.Sprint(action.args[i])) } @@ -131,7 +134,7 @@ func (*stateTest) Generate(r *rand.Rand, size int) reflect.Value { for i := range addrs { addrs[i][0] = byte(i) } - actions := make([][]testAction, rand.Intn(5)+1) + actions := make([][]testAction, r.Intn(5)+1) for i := 0; i < len(actions); i++ { actions[i] = make([]testAction, size) @@ -150,9 +153,10 @@ func (*stateTest) Generate(r *rand.Rand, size int) reflect.Value { chunk = 1 } return reflect.ValueOf(&stateTest{ - addrs: addrs, - actions: actions, - chunk: chunk, + addrs: addrs, + actions: actions, + chunk: chunk, + byzantium: r.Intn(2) == 0, }) } @@ -182,7 +186,7 @@ func (test *stateTest) run() bool { disk = rawdb.NewMemoryDatabase() tdb = trie.NewDatabaseWithConfig(disk, &trie.Config{OnCommit: onCommit}) sdb = NewDatabaseWithNodeDB(disk, tdb) - byzantium = rand.Intn(2) == 0 + byzantium = test.byzantium ) for i, actions := range test.actions { root := types.EmptyRootHash diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index 5f3ab62cb7..f77bbdaa61 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -19,7 +19,6 @@ package state import ( "bytes" "encoding/binary" - "errors" "fmt" "math" "math/big" @@ -34,7 +33,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/core/rawdb" "github.com/XinFinOrg/XDPoSChain/core/tracing" "github.com/XinFinOrg/XDPoSChain/core/types" - "github.com/XinFinOrg/XDPoSChain/trie" ) // Tests that updating a state trie does not leak any database writes prior to @@ -208,6 +206,80 @@ func TestCopy(t *testing.T) { } } +// TestCopyWithDirtyJournal tests if Copy can correctly create an equal copied +// stateDB with dirty journal present. +func TestCopyWithDirtyJournal(t *testing.T) { + db := NewDatabase(rawdb.NewMemoryDatabase()) + orig, _ := New(types.EmptyRootHash, db) + + // Fill up the initial states + for i := byte(0); i < 255; i++ { + obj := orig.GetOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(big.NewInt(int64(i))) + obj.data.Root = common.HexToHash("0xdeadbeef") + orig.updateStateObject(obj) + } + root, _ := orig.Commit(0, true) + orig, _ = New(root, db) + + // modify all in memory without finalizing + for i := byte(0); i < 255; i++ { + obj := orig.GetOrNewStateObject(common.BytesToAddress([]byte{i})) + amount := big.NewInt(int64(i)) + obj.SetBalance(new(big.Int).Sub(obj.Balance(), amount)) + + orig.updateStateObject(obj) + } + cpy := orig.Copy() + + orig.Finalise(true) + for i := byte(0); i < 255; i++ { + root := orig.GetStorageRoot(common.BytesToAddress([]byte{i})) + if root != (common.Hash{}) { + t.Errorf("Unexpected storage root %x", root) + } + } + cpy.Finalise(true) + for i := byte(0); i < 255; i++ { + root := cpy.GetStorageRoot(common.BytesToAddress([]byte{i})) + if root != (common.Hash{}) { + t.Errorf("Unexpected storage root %x", root) + } + } + if cpy.IntermediateRoot(true) != orig.IntermediateRoot(true) { + t.Error("State is not equal after copy") + } +} + +// TestCopyObjectState creates an original state, S1, and makes a copy S2. +// It then proceeds to make changes to S1. Those changes are _not_ supposed +// to affect S2. This test checks that the copy properly deep-copies the objectstate +func TestCopyObjectState(t *testing.T) { + db := NewDatabase(rawdb.NewMemoryDatabase()) + orig, _ := New(types.EmptyRootHash, db) + + // Fill up the initial states + for i := byte(0); i < 5; i++ { + obj := orig.GetOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(big.NewInt(int64(i))) + obj.data.Root = common.HexToHash("0xdeadbeef") + orig.updateStateObject(obj) + } + orig.Finalise(true) + cpy := orig.Copy() + for _, op := range cpy.mutations { + if have, want := op.applied, false; have != want { + t.Fatalf("Error in test itself, the 'done' flag should not be set before Commit, have %v want %v", have, want) + } + } + orig.Commit(0, true) + for _, op := range cpy.mutations { + if have, want := op.applied, false; have != want { + t.Fatalf("Error: original state affected copy, have %v want %v", have, want) + } + } +} + func TestSnapshotRandom(t *testing.T) { config := &quick.Config{MaxCount: 1000} err := quick.Check((*snapshotTest).run, config) @@ -291,7 +363,9 @@ func newTestAction(addr common.Address, r *rand.Rand) testAction { { name: "CreateAccount", fn: func(a testAction, s *StateDB) { - s.CreateAccount(addr) + if !s.Exist(addr) { + s.CreateAccount(addr) + } }, }, { @@ -338,6 +412,15 @@ func newTestAction(addr common.Address, r *rand.Rand) testAction { }, args: make([]int64, 1), }, + { + name: "AddPreimage", + fn: func(a testAction, s *StateDB) { + preimage := []byte{1} + hash := common.BytesToHash(preimage) + s.AddPreimage(hash, preimage) + }, + args: make([]int64, 1), + }, { name: "AddAddressToAccessList", fn: func(a testAction, s *StateDB) { @@ -362,18 +445,9 @@ func newTestAction(addr common.Address, r *rand.Rand) testAction { }, args: make([]int64, 2), }, - { - name: "AddPreimage", - fn: func(a testAction, s *StateDB) { - preimage := []byte{1} - hash := common.BytesToHash(preimage) - s.AddPreimage(hash, preimage) - }, - args: make([]int64, 1), - }, } action := actions[r.Intn(len(actions))] - nameargs := make([]string, 0, 1+len(action.args)) + var nameargs []string if !action.noAddr { nameargs = append(nameargs, addr.Hex()) } @@ -516,51 +590,6 @@ func TestTouchDelete(t *testing.T) { } } -// TestCommitCopy tests the copy from a committed state is not functional. -func TestCommitCopy(t *testing.T) { - state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase())) - - // Create an account and check if the retrieved balance is correct - addr := common.HexToAddress("0xaffeaffeaffeaffeaffeaffeaffeaffeaffeaffe") - skey := common.HexToHash("aaa") - sval := common.HexToHash("bbb") - - state.SetBalance(addr, big.NewInt(42), tracing.BalanceChangeUnspecified) // Change the account trie - state.SetCode(addr, []byte("hello")) // Change an external metadata - state.SetState(addr, skey, sval) // Change the storage trie - - if balance := state.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { - t.Fatalf("initial balance mismatch: have %v, want %v", balance, 42) - } - if code := state.GetCode(addr); !bytes.Equal(code, []byte("hello")) { - t.Fatalf("initial code mismatch: have %x, want %x", code, []byte("hello")) - } - if val := state.GetState(addr, skey); val != sval { - t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval) - } - if val := state.GetCommittedState(addr, skey); val != (common.Hash{}) { - t.Fatalf("initial committed storage slot mismatch: have %x, want %x", val, common.Hash{}) - } - // Copy the committed state database, the copied one is not functional. - state.Commit(0, true) - copied := state.Copy() - if balance := copied.GetBalance(addr); balance.Cmp(big.NewInt(0)) != 0 { - t.Fatalf("unexpected balance: have %v", balance) - } - if code := copied.GetCode(addr); code != nil { - t.Fatalf("unexpected code: have %x", code) - } - if val := copied.GetState(addr, skey); val != (common.Hash{}) { - t.Fatalf("unexpected storage slot: have %x", val) - } - if val := copied.GetCommittedState(addr, skey); val != (common.Hash{}) { - t.Fatalf("unexpected storage slot: have %x", val) - } - if !errors.Is(copied.Error(), trie.ErrCommitted) { - t.Fatalf("unexpected state error, %v", copied.Error()) - } -} - func TestStateDBAccessList(t *testing.T) { // Some helpers addr := func(a string) common.Address { @@ -965,6 +994,62 @@ func TestCopyCopyCommitCopy(t *testing.T) { } } +// TestCommitCopy tests the copy from a committed state is not fully functional. +func TestCommitCopy(t *testing.T) { + db := NewDatabase(rawdb.NewMemoryDatabase()) + state, _ := New(types.EmptyRootHash, db) + + // Create an account and check if the retrieved balance is correct + addr := common.HexToAddress("0xaffeaffeaffeaffeaffeaffeaffeaffeaffeaffe") + skey1, skey2 := common.HexToHash("a1"), common.HexToHash("a2") + sval1, sval2 := common.HexToHash("b1"), common.HexToHash("b2") + + state.SetBalance(addr, big.NewInt(42), tracing.BalanceChangeUnspecified) // Change the account trie + state.SetCode(addr, []byte("hello")) // Change an external metadata + state.SetState(addr, skey1, sval1) // Change the storage trie + + if balance := state.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("initial balance mismatch: have %v, want %v", balance, 42) + } + if code := state.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("initial code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := state.GetState(addr, skey1); val != sval1 { + t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval1) + } + if val := state.GetCommittedState(addr, skey1); val != (common.Hash{}) { + t.Fatalf("initial committed storage slot mismatch: have %x, want %x", val, common.Hash{}) + } + root, _ := state.Commit(0, true) + + state, _ = New(root, db) + state.SetState(addr, skey2, sval2) + state.Commit(0, true) + + // Copy the committed state database, the copied one is not fully functional. + copied := state.Copy() + if balance := copied.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("unexpected balance: have %v", balance) + } + if code := copied.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("unexpected code: have %x", code) + } + // Miss slots because of non-functional trie after commit + if val := copied.GetState(addr, skey1); val != (common.Hash{}) { + t.Fatalf("unexpected storage slot: have %x", val) + } + if val := copied.GetCommittedState(addr, skey1); val != (common.Hash{}) { + t.Fatalf("unexpected storage slot: have %x", val) + } + // Slots cached in the stateDB, available after commit + if val := copied.GetState(addr, skey2); val != sval2 { + t.Fatalf("unexpected storage slot: have %x", val) + } + if val := copied.GetCommittedState(addr, skey2); val != sval2 { + t.Fatalf("unexpected storage slot: have %x, want %x", val, sval2) + } +} + // Tests that account and storage tries are flushed in the correct order and that // no data loss occurs. func TestFlushOrderDataLoss(t *testing.T) { diff --git a/core/vm/evm.go b/core/vm/evm.go index 2be72c4a86..0a1d21e923 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -469,6 +469,7 @@ func (evm *EVM) create(caller common.Address, code []byte, gas uint64, value *ui return nil, common.Address{}, gas, ErrNonceUintOverflow } evm.StateDB.SetNonce(caller, nonce+1) + // We add this to the access list _before_ taking a snapshot. Even if the creation fails, // the access-list change should not be rolled back if evm.chainRules.IsEIP1559 { @@ -476,7 +477,7 @@ func (evm *EVM) create(caller common.Address, code []byte, gas uint64, value *ui } // Ensure there's no existing contract already at the designated address. // Account is regarded as existent if any of these three conditions is met: - // - the nonce is nonzero + // - the nonce is non-zero // - the code is non-empty // - the storage is non-empty contractHash := evm.StateDB.GetCodeHash(address) @@ -489,9 +490,19 @@ func (evm *EVM) create(caller common.Address, code []byte, gas uint64, value *ui } return nil, common.Address{}, 0, ErrContractAddressCollision } - // Create a new account on the state + // Create a new account on the state only if the object was not present. + // It might be possible the contract code is deployed to a pre-existent + // account with non-zero balance. snapshot := evm.StateDB.Snapshot() - evm.StateDB.CreateAccount(address) + if !evm.StateDB.Exist(address) { + evm.StateDB.CreateAccount(address) + } + // CreateContract means that regardless of whether the account previously existed + // in the state trie or not, it _now_ becomes created as a _contract_ account. + // This is performed _prior_ to executing the initcode, since the initcode + // acts inside that account. + evm.StateDB.CreateContract(address) + if evm.chainRules.IsEIP158 { evm.StateDB.SetNonce(address, 1) } diff --git a/core/vm/interface.go b/core/vm/interface.go index bdb55efe02..8559bb8d47 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -28,6 +28,7 @@ import ( // StateDB is an EVM database for full state querying. type StateDB interface { CreateAccount(common.Address) + CreateContract(common.Address) SubBalance(common.Address, *big.Int, tracing.BalanceChangeReason) *big.Int AddBalance(common.Address, *big.Int, tracing.BalanceChangeReason) *big.Int diff --git a/trie/triestate/state.go b/trie/triestate/state.go index 5a13138942..0b59280f95 100644 --- a/trie/triestate/state.go +++ b/trie/triestate/state.go @@ -26,3 +26,11 @@ type Set struct { Storages map[common.Address]map[common.Hash][]byte // Mutated storage set, nil means the slot was not present Incomplete map[common.Address]struct{} // Indicator whether the storage slot is incomplete due to large deletion } + +// New constructs the state set with provided data. +func New(accounts map[common.Address][]byte, storages map[common.Address]map[common.Hash][]byte) *Set { + return &Set{ + Accounts: accounts, + Storages: storages, + } +}