From 0d4c38865e9cda492e71221c4c429d9b1bec8ac5 Mon Sep 17 00:00:00 2001 From: Martin HS Date: Wed, 24 Apr 2024 11:59:06 +0200 Subject: [PATCH] core/state: remove account reset operation v2 (#29520) * core/state, tests: remove account reset operation * core/state, core/vm: implement createcontract journal event * core/state: make createcontract not emit dirtied account, unskip tests * core/state: add createcontract to journal fuzzing * core/state: fix journal * core/state: address comments * core/state: remove useless code --------- Co-authored-by: Gary Rong --- core/state/journal.go | 151 ++++++++++++++---- core/state/state_object.go | 54 +++---- core/state/state_test.go | 102 +----------- core/state/statedb.go | 270 ++++++++++++++++---------------- core/state/statedb_fuzz_test.go | 4 +- core/state/statedb_test.go | 173 ++++++++++++++------ core/vm/evm.go | 21 ++- core/vm/interface.go | 1 + tests/block_test.go | 8 - 9 files changed, 426 insertions(+), 358 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index 6cdc1fc868..cfd3782eb0 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -17,6 +17,8 @@ package state import ( + "maps" + "github.com/ethereum/go-ethereum/common" "github.com/holiman/uint256" ) @@ -29,6 +31,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 @@ -83,22 +88,31 @@ 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), + } +} + type ( // Changes to the account trie. createObjectChange 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 + // 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. + createContractChange struct { + account common.Address } + selfDestructChange struct { account *common.Address prev bool // whether account had already self-destructed @@ -136,6 +150,7 @@ type ( touchChange struct { account *common.Address } + // Changes to the access list accessListAddAccountChange struct { address *common.Address @@ -145,6 +160,7 @@ type ( slot *common.Hash } + // Changes to transient storage transientStorageChange struct { account *common.Address key, prevalue common.Hash @@ -153,34 +169,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 resetObjectChange) revert(s *StateDB) { - s.setStateObject(ch.prev) - if !ch.prevdestruct { - delete(s.stateObjectsDestruct, ch.prev.address) - } - if ch.prevAccount != nil { - s.accounts[ch.prev.addrHash] = ch.prevAccount - } - if ch.prevStorage != nil { - s.storages[ch.prev.addrHash] = ch.prevStorage - } - if ch.prevAccountOriginExist { - s.accountsOrigin[ch.prev.address] = ch.prevAccountOrigin - } - if ch.prevStorageOrigin != nil { - s.storagesOrigin[ch.prev.address] = ch.prevStorageOrigin +func (ch createObjectChange) copy() journalEntry { + return createObjectChange{ + account: ch.account, } } -func (ch resetObjectChange) dirtied() *common.Address { - return ch.account +func (ch createContractChange) revert(s *StateDB) { + s.getStateObject(ch.account).newContract = false +} + +func (ch createContractChange) dirtied() *common.Address { + return nil +} + +func (ch createContractChange) copy() journalEntry { + return createContractChange{ + account: ch.account, + } } func (ch selfDestructChange) revert(s *StateDB) { @@ -195,6 +207,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(uint256.Int).Set(ch.prevbalance), + } +} + var ripemd = common.HexToAddress("0000000000000000000000000000000000000003") func (ch touchChange) revert(s *StateDB) { @@ -204,6 +224,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) } @@ -212,6 +238,13 @@ func (ch balanceChange) dirtied() *common.Address { return ch.account } +func (ch balanceChange) copy() journalEntry { + return balanceChange{ + account: ch.account, + prev: new(uint256.Int).Set(ch.prev), + } +} + func (ch nonceChange) revert(s *StateDB) { s.getStateObject(*ch.account).setNonce(ch.prev) } @@ -220,6 +253,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(common.BytesToHash(ch.prevhash), ch.prevcode) } @@ -228,6 +268,14 @@ func (ch codeChange) dirtied() *common.Address { return ch.account } +func (ch codeChange) copy() journalEntry { + return codeChange{ + account: ch.account, + prevhash: common.CopyBytes(ch.prevhash), + prevcode: common.CopyBytes(ch.prevcode), + } +} + func (ch storageChange) revert(s *StateDB) { s.getStateObject(*ch.account).setState(ch.key, ch.prevalue) } @@ -236,6 +284,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) } @@ -244,6 +300,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 } @@ -252,6 +316,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 { @@ -266,6 +336,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) } @@ -274,6 +350,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 @@ -291,6 +373,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) } @@ -298,3 +386,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 db3c32f2f2..8978b30542 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -32,21 +32,8 @@ import ( "github.com/holiman/uint256" ) -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) } @@ -65,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 @@ -75,17 +62,16 @@ type stateObject struct { // Cache flags. dirtyCode bool // true if the code was updated - // Flag whether the account was marked as self-destructed. The self-destructed account - // is still accessible in the scope of same transaction. + // Flag whether the account was marked as self-destructed. The self-destructed + // 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. @@ -95,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() } @@ -111,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, } } @@ -264,6 +246,10 @@ func (s *stateObject) finalise(prefetch bool) { 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 is responsible for persisting cached storage changes into the @@ -463,12 +449,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 } @@ -483,7 +469,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()) { @@ -501,7 +487,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 c6e6db906e..9200e4abe9 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -194,106 +194,20 @@ func TestSnapshotEmpty(t *testing.T) { s.state.RevertToSnapshot(s.state.Snapshot()) } -func TestSnapshot2(t *testing.T) { +func TestCreateObjectRevert(t *testing.T) { state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil) + 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(uint256.NewInt(42), tracing.BalanceChangeUnspecified) 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, _ = New(root, state.db, state.snaps) - - // and one with deleted == true - so1 := state.getStateObject(stateobjaddr1) - so1.SetBalance(uint256.NewInt(52), tracing.BalanceChangeUnspecified) - 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 ab152dd18d..4a934fe82c 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -44,6 +44,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: @@ -75,12 +95,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 @@ -154,9 +184,8 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*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(), @@ -472,8 +501,7 @@ func (s *StateDB) Selfdestruct6780(addr common.Address) { if stateObject == nil { return } - - if stateObject.created { + if stateObject.newContract { s.SelfDestruct(addr) } } @@ -552,24 +580,16 @@ func (s *StateDB) deleteStateObject(addr common.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 + } // If no live objects are available, attempt to use snapshots var data *types.StateAccount if s.snap != nil { @@ -622,69 +642,40 @@ 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) *stateObject { + obj := newObject(s, addr, nil) + s.journal.append(createObjectChange{account: &addr}) + s.setStateObject(obj) + return obj } -// 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 +// by CreateAccount, but that is not required if it already existed in the +// state due to funds sent beforehand. +// This operation sets the 'newContract'-flag, which is required in order to +// correctly handle EIP-6780 'delete-in-same-transaction' logic. +func (s *StateDB) CreateContract(addr common.Address) { + obj := s.getStateObject(addr) + if !obj.newContract { + obj.newContract = true + s.journal.append(createContractChange{account: addr}) } } @@ -695,21 +686,25 @@ func (s *StateDB) Copy() *StateDB { state := &StateDB{ db: s.db, trie: s.db.CopyTrie(s.trie), + hasher: crypto.NewKeccakState(), originalRoot: s.originalRoot, accounts: copySet(s.accounts), storages: copy2DSet(s.storages), accountsOrigin: copySet(s.accountsOrigin), storagesOrigin: copy2DSet(s.storagesOrigin), - 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)), + 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, // In order for the block producer to be able to use and make additions // to the snapshot tree, we need to copy that as well. Otherwise, any @@ -718,39 +713,14 @@ func (s *StateDB) Copy() *StateDB { snaps: s.snaps, snap: s.snap, } - // 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 - } + // 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 logs occurred in the scope of block for hash, logs := range s.logs { cpy := make([]*types.Log, len(logs)) @@ -760,7 +730,6 @@ func (s *StateDB) Copy() *StateDB { } state.logs[hash] = cpy } - // Do we need to copy the access list and transient storage? // In practice: No. At the start of a transaction, these two lists are empty. // In practice, we only ever copy state _between_ transactions/blocks, never @@ -825,7 +794,8 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { continue } if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { - obj.deleted = true + delete(s.stateObjects, obj.address) + s.markDelete(addr) // If ether was sent to account post-selfdestruct it is burnt. if bal := obj.Balance(); s.logger != nil && s.logger.OnBalanceChange != nil && obj.selfDestructed && bal.Sign() != 0 { @@ -846,11 +816,8 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { delete(s.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect) } else { obj.finalise(true) // Prefetch slots in the background + s.markUpdate(addr) } - obj.created = false - s.stateObjectsPending[addr] = struct{}{} - s.stateObjectsDirty[addr] = struct{}{} - // At this point, also ship the address off to the precacher. The precacher // will start loading tries, and when the change is eventually committed, // the commit-phase will be a lot faster @@ -889,10 +856,14 @@ 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 { - if obj := s.stateObjects[addr]; !obj.deleted { - obj.updateRoot() + for addr, op := range s.mutations { + if op.applied { + continue } + if op.isDelete() { + continue + } + s.stateObjects[addr].updateRoot() } // Now we're about to start to write changes to the trie. The trie is so far // _untouched_. We can check with the prefetcher, if it can give us a trie @@ -902,7 +873,6 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { s.trie = trie } } - usedAddrs := make([][]byte, 0, len(s.stateObjectsPending)) // Perform updates before deletions. This prevents resolution of unnecessary trie nodes // in circumstances similar to the following: // @@ -913,13 +883,21 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { // 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 := range s.stateObjectsPending { - if obj := s.stateObjects[addr]; !obj.deleted { - s.updateStateObject(obj) - s.AccountUpdated += 1 + var ( + usedAddrs [][]byte + deletedAddrs []common.Address + ) + for addr, op := range s.mutations { + if op.applied { + continue + } + op.applied = true + + if op.isDelete() { + deletedAddrs = append(deletedAddrs, addr) } else { - deletedAddrs = append(deletedAddrs, obj.address) + s.updateStateObject(s.stateObjects[addr]) + s.AccountUpdated += 1 } usedAddrs = append(usedAddrs, common.CopyBytes(addr[:])) // Copy needed for closure } @@ -930,9 +908,6 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { if prefetcher != nil { prefetcher.used(common.Hash{}, s.originalRoot, usedAddrs) } - if len(s.stateObjectsPending) > 0 { - s.stateObjectsPending = make(map[common.Address]struct{}) - } // Track the amount of time wasted on hashing the account trie defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) @@ -1176,11 +1151,12 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er 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 := s.stateObjects[addr] + // Write any contract code associated with the state object if obj.code != nil && obj.dirtyCode { rawdb.WriteCode(codeWriter, common.BytesToHash(obj.CodeHash()), obj.code) @@ -1280,7 +1256,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 } @@ -1395,3 +1371,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 65cf278108..6317681a7f 100644 --- a/core/state/statedb_fuzz_test.go +++ b/core/state/statedb_fuzz_test.go @@ -96,7 +96,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) + } }, }, { diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index edde6e8254..1a3eccfe10 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -225,6 +225,78 @@ func TestCopy(t *testing.T) { } } +// TestCopyWithDirtyJournal tests if Copy can correct create a equal copied +// stateDB with dirty journal present. +func TestCopyWithDirtyJournal(t *testing.T) { + db := NewDatabase(rawdb.NewMemoryDatabase()) + orig, _ := New(types.EmptyRootHash, db, nil) + + // Fill up the initial states + for i := byte(0); i < 255; i++ { + obj := orig.getOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(uint256.NewInt(uint64(i)), tracing.BalanceChangeUnspecified) + obj.data.Root = common.HexToHash("0xdeadbeef") + orig.updateStateObject(obj) + } + root, _ := orig.Commit(0, true) + orig, _ = New(root, db, nil) + + // modify all in memory without finalizing + for i := byte(0); i < 255; i++ { + obj := orig.getOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.SubBalance(uint256.NewInt(uint64(i)), tracing.BalanceChangeUnspecified) + 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, nil) + + // Fill up the initial states + for i := byte(0); i < 5; i++ { + obj := orig.getOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(uint256.NewInt(uint64(i)), tracing.BalanceChangeUnspecified) + 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) @@ -308,7 +380,30 @@ 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) + } + }, + }, + { + name: "CreateContract", + fn: func(a testAction, s *StateDB) { + if !s.Exist(addr) { + s.CreateAccount(addr) + } + contractHash := s.GetCodeHash(addr) + emptyCode := contractHash == (common.Hash{}) || contractHash == types.EmptyCodeHash + storageRoot := s.GetStorageRoot(addr) + emptyStorage := storageRoot == (common.Hash{}) || storageRoot == types.EmptyRootHash + if s.GetNonce(addr) == 0 && emptyCode && emptyStorage { + s.CreateContract(addr) + // We also set some code here, to prevent the + // CreateContract action from being performed twice in a row, + // which would cause a difference in state when unrolling + // the journal. (CreateContact assumes created was false prior to + // invocation, and the journal rollback sets it to false). + s.SetCode(addr, []byte{1}) + } }, }, { @@ -709,18 +804,19 @@ func TestCopyCopyCommitCopy(t *testing.T) { } } -// TestCommitCopy tests the copy from a committed state is not functional. +// TestCommitCopy tests the copy from a committed state is not fully functional. func TestCommitCopy(t *testing.T) { - state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil) + db := NewDatabase(rawdb.NewMemoryDatabase()) + state, _ := New(types.EmptyRootHash, db, nil) // Create an account and check if the retrieved balance is correct addr := common.HexToAddress("0xaffeaffeaffeaffeaffeaffeaffeaffeaffeaffe") - skey := common.HexToHash("aaa") - sval := common.HexToHash("bbb") + skey1, skey2 := common.HexToHash("a1"), common.HexToHash("a2") + sval1, sval2 := common.HexToHash("b1"), common.HexToHash("b2") state.SetBalance(addr, uint256.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 + state.SetState(addr, skey1, sval1) // Change the storage trie if balance := state.GetBalance(addr); balance.Cmp(uint256.NewInt(42)) != 0 { t.Fatalf("initial balance mismatch: have %v, want %v", balance, 42) @@ -728,25 +824,38 @@ func TestCommitCopy(t *testing.T) { 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.GetState(addr, skey1); val != sval1 { + t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval1) } - if val := state.GetCommittedState(addr, skey); val != (common.Hash{}) { + if val := state.GetCommittedState(addr, skey1); 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) + root, _ := state.Commit(0, true) + + state, _ = New(root, db, nil) + state.SetState(addr, skey2, sval2) + state.Commit(1, true) + + // Copy the committed state database, the copied one is not fully functional. copied := state.Copy() - if balance := copied.GetBalance(addr); balance.Cmp(uint256.NewInt(0)) != 0 { + if balance := copied.GetBalance(addr); balance.Cmp(uint256.NewInt(42)) != 0 { t.Fatalf("unexpected balance: have %v", balance) } - if code := copied.GetCode(addr); code != nil { + if code := copied.GetCode(addr); !bytes.Equal(code, []byte("hello")) { t.Fatalf("unexpected code: have %x", code) } - if val := copied.GetState(addr, skey); val != (common.Hash{}) { + // 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", sval1) + } + if val := copied.GetCommittedState(addr, skey1); val != (common.Hash{}) { t.Fatalf("unexpected storage slot: have %x", val) } - if val := copied.GetCommittedState(addr, skey); val != (common.Hash{}) { + // Slots cached in the stateDB, available after commit + if val := copied.GetState(addr, skey2); val != sval2 { + t.Fatalf("unexpected storage slot: have %x", sval1) + } + if val := copied.GetCommittedState(addr, skey2); val != sval2 { t.Fatalf("unexpected storage slot: have %x", val) } if !errors.Is(copied.Error(), trie.ErrCommitted) { @@ -1103,40 +1212,6 @@ func TestStateDBTransientStorage(t *testing.T) { } } -func TestResetObject(t *testing.T) { - var ( - disk = rawdb.NewMemoryDatabase() - tdb = triedb.NewDatabase(disk, nil) - db = NewDatabaseWithNodeDB(disk, tdb) - snaps, _ = snapshot.New(snapshot.Config{CacheSize: 10}, disk, tdb, types.EmptyRootHash) - state, _ = New(types.EmptyRootHash, db, snaps) - addr = common.HexToAddress("0x1") - slotA = common.HexToHash("0x1") - slotB = common.HexToHash("0x2") - ) - // Initialize account with balance and storage in first transaction. - state.SetBalance(addr, uint256.NewInt(1), tracing.BalanceChangeUnspecified) - state.SetState(addr, slotA, common.BytesToHash([]byte{0x1})) - state.IntermediateRoot(true) - - // Reset account and mutate balance and storages - state.CreateAccount(addr) - state.SetBalance(addr, uint256.NewInt(2), tracing.BalanceChangeUnspecified) - state.SetState(addr, slotB, common.BytesToHash([]byte{0x2})) - root, _ := state.Commit(0, true) - - // Ensure the original account is wiped properly - snap := snaps.Snapshot(root) - slot, _ := snap.Storage(crypto.Keccak256Hash(addr.Bytes()), crypto.Keccak256Hash(slotA.Bytes())) - if len(slot) != 0 { - t.Fatalf("Unexpected storage slot") - } - slot, _ = snap.Storage(crypto.Keccak256Hash(addr.Bytes()), crypto.Keccak256Hash(slotB.Bytes())) - if !bytes.Equal(slot, []byte{0x2}) { - t.Fatalf("Unexpected storage slot value %v", slot) - } -} - func TestDeleteStorage(t *testing.T) { var ( disk = rawdb.NewMemoryDatabase() diff --git a/core/vm/evm.go b/core/vm/evm.go index 045506a1fd..c18353a973 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -436,14 +436,15 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, return nil, common.Address{}, gas, ErrNonceUintOverflow } evm.StateDB.SetNonce(caller.Address(), 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 + + // 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.IsBerlin { evm.StateDB.AddAddressToAccessList(address) } // 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) @@ -456,9 +457,19 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, } 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 30742e96de..774360a08e 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -29,6 +29,7 @@ import ( // StateDB is an EVM database for full state querying. type StateDB interface { CreateAccount(common.Address) + CreateContract(common.Address) SubBalance(common.Address, *uint256.Int, tracing.BalanceChangeReason) AddBalance(common.Address, *uint256.Int, tracing.BalanceChangeReason) diff --git a/tests/block_test.go b/tests/block_test.go index 43e3d99b3e..1ba84f5f24 100644 --- a/tests/block_test.go +++ b/tests/block_test.go @@ -64,14 +64,6 @@ func TestExecutionSpecBlocktests(t *testing.T) { } bt := new(testMatcher) - // These tests fail as of https://github.com/ethereum/go-ethereum/pull/28666, since we - // no longer delete "leftover storage" when deploying a contract. - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/self_destructing_initcode_create_tx.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/self_destructing_initcode.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/recreate_self_destructed_contract_different_txs.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/delegatecall_from_new_contract_to_pre_existing_contract.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/create_selfdestruct_same_tx.json`) - bt.walk(t, executionSpecBlockchainTestDir, func(t *testing.T, name string, test *BlockTest) { execBlockTest(t, bt, test) })