diff --git a/core/state/state_object.go b/core/state/state_object.go index 4967c7046..f9d7559d8 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -79,9 +79,10 @@ type stateObject struct { trie Trie // storage trie, which becomes non-nil on first access code Code // contract bytecode, which gets set when code is loaded - originStorage Storage // Storage cache of original entries to dedup rewrites - dirtyStorage Storage // Storage entries that need to be flushed to disk - fakeStorage Storage // Fake storage which constructed by caller for debugging purpose. + originStorage Storage // Storage cache of original entries to dedup rewrites, reset for every transaction + pendingStorage Storage // Storage entries that need to be flushed to disk, at the end of an entire block + dirtyStorage Storage // Storage entries that have been modified in the current transaction execution + fakeStorage Storage // Fake storage which constructed by caller for debugging purpose. // Cache flags. // When an object is marked suicided it will be delete from the trie @@ -113,13 +114,17 @@ func newObject(db *StateDB, address common.Address, data Account) *stateObject { if data.CodeHash == nil { data.CodeHash = emptyCodeHash } + if data.Root == (common.Hash{}) { + data.Root = emptyRoot + } return &stateObject{ - db: db, - address: address, - addrHash: crypto.Keccak256Hash(address[:]), - data: data, - originStorage: make(Storage), - dirtyStorage: make(Storage), + db: db, + address: address, + addrHash: crypto.Keccak256Hash(address[:]), + data: data, + originStorage: make(Storage), + pendingStorage: make(Storage), + dirtyStorage: make(Storage), } } @@ -183,9 +188,11 @@ func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Has if s.fakeStorage != nil { return s.fakeStorage[key] } - // If we have the original value cached, return that - value, cached := s.originStorage[key] - if cached { + // If we have a pending write or clean cached, return that + if value, pending := s.pendingStorage[key]; pending { + return value + } + if value, cached := s.originStorage[key]; cached { return value } // Track the amount of time wasted on reading the storage trie @@ -198,6 +205,7 @@ func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Has s.setError(err) return common.Hash{} } + var value common.Hash if len(enc) > 0 { _, content, _, err := rlp.Split(enc) if err != nil { @@ -252,17 +260,29 @@ func (s *stateObject) setState(key, value common.Hash) { s.dirtyStorage[key] = value } +// finalise moves all dirty storage slots into the pending area to be hashed or +// committed later. It is invoked at the end of every transaction. +func (s *stateObject) finalise() { + for key, value := range s.dirtyStorage { + s.pendingStorage[key] = value + } + if len(s.dirtyStorage) > 0 { + s.dirtyStorage = make(Storage) + } +} + // updateTrie writes cached storage modifications into the object's storage trie. func (s *stateObject) updateTrie(db Database) Trie { + // Make sure all dirty slots are finalized into the pending storage area + s.finalise() + // Track the amount of time wasted on updating the storge trie if metrics.EnabledExpensive { defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now()) } - // Update all the dirty slots in the trie + // Insert all the pending updates into the trie tr := s.getTrie(db) - for key, value := range s.dirtyStorage { - delete(s.dirtyStorage, key) - + for key, value := range s.pendingStorage { // Skip noop changes, persist actual changes if value == s.originStorage[key] { continue @@ -277,6 +297,9 @@ func (s *stateObject) updateTrie(db Database) Trie { v, _ := rlp.EncodeToBytes(common.TrimLeftZeroes(value[:])) s.setError(tr.TryUpdate(key[:], v)) } + if len(s.pendingStorage) > 0 { + s.pendingStorage = make(Storage) + } return tr } diff --git a/core/state/statedb.go b/core/state/statedb.go index b07f08fd2..9034d5533 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -67,8 +67,9 @@ type StateDB struct { trie Trie // This map holds 'live' objects, which will get modified while processing a state transition. - stateObjects map[common.Address]*stateObject - stateObjectsDirty map[common.Address]struct{} + 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 // DB error. // State objects are used by the consensus core and VM which are @@ -111,13 +112,14 @@ func New(root common.Hash, db Database) (*StateDB, error) { return nil, err } return &StateDB{ - db: db, - trie: tr, - stateObjects: make(map[common.Address]*stateObject), - stateObjectsDirty: make(map[common.Address]struct{}), - logs: make(map[common.Hash][]*types.Log), - preimages: make(map[common.Hash][]byte), - journal: newJournal(), + db: db, + trie: tr, + stateObjects: make(map[common.Address]*stateObject), + stateObjectsPending: make(map[common.Address]struct{}), + stateObjectsDirty: make(map[common.Address]struct{}), + logs: make(map[common.Hash][]*types.Log), + preimages: make(map[common.Hash][]byte), + journal: newJournal(), }, nil } @@ -141,6 +143,7 @@ func (self *StateDB) Reset(root common.Hash) error { } self.trie = tr self.stateObjects = make(map[common.Address]*stateObject) + self.stateObjectsPending = make(map[common.Address]struct{}) self.stateObjectsDirty = make(map[common.Address]struct{}) self.thash = common.Hash{} self.bhash = common.Hash{} @@ -421,15 +424,15 @@ func (self *StateDB) Suicide(addr common.Address) bool { // // updateStateObject writes the given object to the trie. -func (s *StateDB) updateStateObject(stateObject *stateObject) { +func (s *StateDB) updateStateObject(obj *stateObject) { // Track the amount of time wasted on updating the account from the trie if metrics.EnabledExpensive { defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) } // Encode the account and update the account trie - addr := stateObject.Address() + addr := obj.Address() - data, err := rlp.EncodeToBytes(stateObject) + data, err := rlp.EncodeToBytes(obj) if err != nil { panic(fmt.Errorf("can't encode object at %x: %v", addr[:], err)) } @@ -437,25 +440,33 @@ func (s *StateDB) updateStateObject(stateObject *stateObject) { } // deleteStateObject removes the given object from the state trie. -func (s *StateDB) deleteStateObject(stateObject *stateObject) { +func (s *StateDB) deleteStateObject(obj *stateObject) { // Track the amount of time wasted on deleting the account from the trie if metrics.EnabledExpensive { defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) } // Delete the account from the trie - stateObject.deleted = true - - addr := stateObject.Address() + addr := obj.Address() s.setError(s.trie.TryDelete(addr[:])) } -// Retrieve a state object given by the address. Returns nil if not found. -func (s *StateDB) getStateObject(addr common.Address) (stateObject *stateObject) { - // Prefer live objects +// 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. +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 self- +// 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 { - if obj.deleted { - return nil - } return obj } // Track the amount of time wasted on loading the object from the database @@ -486,7 +497,7 @@ func (self *StateDB) setStateObject(object *stateObject) { // Retrieve a state object or create a new state object if nil. func (self *StateDB) GetOrNewStateObject(addr common.Address) *stateObject { stateObject := self.getStateObject(addr) - if stateObject == nil || stateObject.deleted { + if stateObject == nil { stateObject, _ = self.createObject(addr) } return stateObject @@ -495,7 +506,8 @@ func (self *StateDB) GetOrNewStateObject(addr common.Address) *stateObject { // 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 (self *StateDB) createObject(addr common.Address) (newobj, prev *stateObject) { - prev = self.getStateObject(addr) + prev = self.getDeletedStateObject(addr) // Note, prev might have been deleted, we need that! + newobj = newObject(self, addr, Account{}) newobj.setNonce(0) // sets the object to dirty if prev == nil { @@ -558,15 +570,16 @@ func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common func (self *StateDB) Copy() *StateDB { // Copy all the basic fields, initialize the memory ones state := &StateDB{ - db: self.db, - trie: self.db.CopyTrie(self.trie), - stateObjects: make(map[common.Address]*stateObject, len(self.journal.dirties)), - stateObjectsDirty: make(map[common.Address]struct{}, len(self.journal.dirties)), - refund: self.refund, - logs: make(map[common.Hash][]*types.Log, len(self.logs)), - logSize: self.logSize, - preimages: make(map[common.Hash][]byte, len(self.preimages)), - journal: newJournal(), + db: self.db, + trie: self.db.CopyTrie(self.trie), + stateObjects: make(map[common.Address]*stateObject, len(self.journal.dirties)), + stateObjectsPending: make(map[common.Address]struct{}, len(self.stateObjectsPending)), + stateObjectsDirty: make(map[common.Address]struct{}, len(self.journal.dirties)), + refund: self.refund, + logs: make(map[common.Hash][]*types.Log, len(self.logs)), + logSize: self.logSize, + preimages: make(map[common.Hash][]byte, len(self.preimages)), + journal: newJournal(), } // Copy the dirty states, logs, and preimages for addr := range self.journal.dirties { @@ -582,11 +595,17 @@ func (self *StateDB) Copy() *StateDB { // 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 self.stateObjectsPending { + if _, exist := state.stateObjects[addr]; !exist { + state.stateObjects[addr] = self.stateObjects[addr].deepCopy(state) + } + state.stateObjectsPending[addr] = struct{}{} + } for addr := range self.stateObjectsDirty { if _, exist := state.stateObjects[addr]; !exist { state.stateObjects[addr] = self.stateObjects[addr].deepCopy(state) - state.stateObjectsDirty[addr] = struct{}{} } + state.stateObjectsDirty[addr] = struct{}{} } for hash, logs := range self.logs { cpy := make([]*types.Log, len(logs)) @@ -631,11 +650,12 @@ func (self *StateDB) GetRefund() uint64 { return self.refund } -// Finalise finalises the state by removing the self destructed objects -// and clears the journal as well as the refunds. +// Finalise finalises the state by removing the self destructed objects and clears +// the journal as well as the refunds. Finalise, however, will not push any updates +// into the tries just yet. Only IntermediateRoot or Commit will do that. func (s *StateDB) Finalise(deleteEmptyObjects bool) { for addr := range s.journal.dirties { - stateObject, exist := s.stateObjects[addr] + obj, exist := s.stateObjects[addr] if !exist { // ripeMD is 'touched' at block 1714175, in tx 0x1237f737031e40bcde4a8b7e717b2d15e3ecadfe49bb1bbc71ee9deb09c6fcf2 // That tx goes out of gas, and although the notion of 'touched' does not exist there, the @@ -645,13 +665,12 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { // Thus, we can safely ignore it here continue } - - if stateObject.suicided || (deleteEmptyObjects && stateObject.empty()) { - s.deleteStateObject(stateObject) + if obj.suicided || (deleteEmptyObjects && obj.empty()) { + obj.deleted = true } else { - stateObject.updateRoot(s.db) - s.updateStateObject(stateObject) + obj.finalise() } + s.stateObjectsPending[addr] = struct{}{} s.stateObjectsDirty[addr] = struct{}{} } // Invalidate journal because reverting across transactions is not allowed. @@ -662,8 +681,21 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { // It is called in between transactions to get the root hash that // goes into transaction receipts. func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { + // Finalise all the dirty storage states and write them into the tries s.Finalise(deleteEmptyObjects) + for addr := range s.stateObjectsPending { + obj := s.stateObjects[addr] + if obj.deleted { + s.deleteStateObject(obj) + } else { + obj.updateRoot(s.db) + s.updateStateObject(obj) + } + } + if len(s.stateObjectsPending) > 0 { + s.stateObjectsPending = make(map[common.Address]struct{}) + } // Track the amount of time wasted on hashing the account trie if metrics.EnabledExpensive { defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) @@ -680,46 +712,40 @@ func (self *StateDB) Prepare(thash, bhash common.Hash, ti int) { } func (s *StateDB) clearJournalAndRefund() { - s.journal = newJournal() - s.validRevisions = s.validRevisions[:0] - s.refund = 0 + if len(s.journal.entries) > 0 { + s.journal = newJournal() + s.refund = 0 + } + s.validRevisions = s.validRevisions[:0] // Snapshots can be created without journal entires } // Commit writes the state to the underlying in-memory trie database. -func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) { - defer s.clearJournalAndRefund() +func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) { + // Finalize any pending changes and merge everything into the tries + s.IntermediateRoot(deleteEmptyObjects) - for addr := range s.journal.dirties { - s.stateObjectsDirty[addr] = struct{}{} - } // Commit objects to the trie, measuring the elapsed time - for addr, stateObject := range s.stateObjects { - _, isDirty := s.stateObjectsDirty[addr] - switch { - case stateObject.suicided || (isDirty && deleteEmptyObjects && stateObject.empty()): - // If the object has been removed, don't bother syncing it - // and just mark it for deletion in the trie. - s.deleteStateObject(stateObject) - case isDirty: + for addr := range s.stateObjectsDirty { + if obj := s.stateObjects[addr]; !obj.deleted { // Write any contract code associated with the state object - if stateObject.code != nil && stateObject.dirtyCode { - s.db.TrieDB().InsertBlob(common.BytesToHash(stateObject.CodeHash()), stateObject.code) - stateObject.dirtyCode = false + if obj.code != nil && obj.dirtyCode { + s.db.TrieDB().InsertBlob(common.BytesToHash(obj.CodeHash()), obj.code) + obj.dirtyCode = false } - // Write any storage changes in the state object to its storage trie. - if err := stateObject.CommitTrie(s.db); err != nil { + // Write any storage changes in the state object to its storage trie + if err := obj.CommitTrie(s.db); err != nil { return common.Hash{}, err } - // Update the object in the main account trie. - s.updateStateObject(stateObject) } - delete(s.stateObjectsDirty, addr) + } + if len(s.stateObjectsDirty) > 0 { + s.stateObjectsDirty = make(map[common.Address]struct{}) } // Write the account trie changes, measuing the amount of wasted time if metrics.EnabledExpensive { defer func(start time.Time) { s.AccountCommits += time.Since(start) }(time.Now()) } - root, err = s.trie.Commit(func(leaf []byte, parent common.Hash) error { + return s.trie.Commit(func(leaf []byte, parent common.Hash) error { var account Account if err := rlp.DecodeBytes(leaf, &account); err != nil { return nil @@ -733,5 +759,4 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) } return nil }) - return root, err } diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index bf073bc94..bb3b87ead 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -449,3 +449,38 @@ func TestCopyOfCopy(t *testing.T) { t.Fatalf("2nd copy fail, expected 42, got %v", got) } } + +// TestDeleteCreateRevert tests a weird state transition corner case that we hit +// while changing the internals of statedb. The workflow is that a contract is +// self destructed, then in a followup transaction (but same block) it's created +// again and the transaction reverted. +// +// The original statedb implementation flushed dirty objects to the tries after +// each transaction, so this works ok. The rework accumulated writes in memory +// first, but the journal wiped the entire state object on create-revert. +func TestDeleteCreateRevert(t *testing.T) { + // Create an initial state with a single contract + state, _ := New(common.Hash{}, NewDatabase(rawdb.NewMemoryDatabase())) + + addr := toAddr([]byte("so")) + state.SetBalance(addr, big.NewInt(1)) + + root, _ := state.Commit(false) + state.Reset(root) + + // Simulate self-destructing in one transaction, then create-reverting in another + state.Suicide(addr) + state.Finalise(true) + + id := state.Snapshot() + state.SetBalance(addr, big.NewInt(2)) + state.RevertToSnapshot(id) + + // Commit the entire state and make sure we don't crash and have the correct state + root, _ = state.Commit(true) + state.Reset(root) + + if state.getStateObject(addr) != nil { + t.Fatalf("self-destructed contract came alive") + } +}