diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 90bb9a483d..8f3b79b401 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -1769,3 +1769,78 @@ func TestEIP3651(t *testing.T) { t.Fatalf("sender balance incorrect: expected %d, got %d", expected, actual) } } + +// 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) { + var ( + aa = common.HexToAddress("0x000000000000000000000000000000000000aaaa") + bb = common.HexToAddress("0x000000000000000000000000000000000000bbbb") + // Generate a canonical chain to act as the main dataset + engine = ethash.NewFaker() + db = rawdb.NewMemoryDatabase() + + // A sender who makes transactions, has some funds + key, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + address = crypto.PubkeyToAddress(key.PublicKey) + funds = big.NewInt(10000000000000000) + gspec = &Genesis{ + Config: params.TestChainConfig, + Alloc: GenesisAlloc{ + address: {Balance: funds}, + // The address 0xAAAAA selfdestructs if called + aa: { + // Code needs to just selfdestruct + Code: []byte{byte(vm.PC), 0xFF}, + Nonce: 1, + Balance: big.NewInt(0), + }, + // The address 0xBBBB send 1 wei to 0xAAAA, then reverts + bb: { + Code: []byte{ + byte(vm.PC), // [0] + byte(vm.DUP1), // [0,0] + byte(vm.DUP1), // [0,0,0] + byte(vm.DUP1), // [0,0,0,0] + byte(vm.PUSH1), 0x01, // [0,0,0,0,1] (value) + byte(vm.PUSH2), 0xaa, 0xaa, // [0,0,0,0,1, 0xaaaa] + byte(vm.GAS), + byte(vm.CALL), + byte(vm.REVERT), + }, + Balance: big.NewInt(1), + }, + }, + } + genesis = gspec.MustCommit(db) + ) + + blocks, _ := GenerateChain(params.TestChainConfig, genesis, engine, db, 1, func(i int, b *BlockGen) { + b.SetCoinbase(common.Address{1}) + // One transaction to AAAA + tx, _ := types.SignTx(types.NewTransaction(0, aa, + big.NewInt(0), 50000, b.header.BaseFee, nil), types.HomesteadSigner{}, key) + b.AddTx(tx) + // One transaction to BBBB + tx, _ = types.SignTx(types.NewTransaction(1, bb, + big.NewInt(0), 100000, b.header.BaseFee, nil), types.HomesteadSigner{}, key) + b.AddTx(tx) + }) + // Import the canonical chain + diskdb := rawdb.NewMemoryDatabase() + gspec.MustCommit(diskdb) + + chain, err := NewBlockChain(diskdb, nil, params.TestChainConfig, engine, vm.Config{}) + if err != nil { + t.Fatalf("failed to create tester chain: %v", err) + } + if n, err := chain.InsertChain(blocks); err != nil { + t.Fatalf("block %d: failed to insert into chain: %v", n, err) + } +} diff --git a/core/state/state_object.go b/core/state/state_object.go index 15492f4dfb..5c7dab8b4c 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -77,9 +77,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, reset for every transaction - 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 need to be flushed to disk + fakeStorage Storage // Fake storage which constructed by caller for debugging purpose. // Cache flags. dirtyCode bool // true if the code was updated @@ -123,14 +124,18 @@ func newObject(db *StateDB, address common.Address, data Account, onDirty func(a if data.CodeHash == nil { data.CodeHash = types.EmptyCodeHash.Bytes() } + if data.Root == (common.Hash{}) { + data.Root = types.EmptyRootHash + } return &stateObject{ - db: db, - address: address, - addrHash: crypto.Keccak256Hash(address[:]), - data: data, - originStorage: make(Storage), - dirtyStorage: make(Storage), - onDirty: onDirty, + db: db, + address: address, + addrHash: crypto.Keccak256Hash(address[:]), + data: data, + originStorage: make(Storage), + pendingStorage: make(Storage), + dirtyStorage: make(Storage), + onDirty: onDirty, } } @@ -199,9 +204,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 @@ -212,6 +219,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 { @@ -271,14 +279,27 @@ func (s *stateObject) setState(key, value common.Hash) { } } +// 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 storage trie defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now()) + // 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 @@ -293,6 +314,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 10b06717bb..a2781d8776 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -49,8 +49,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 @@ -109,14 +110,15 @@ 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), - accessList: newAccessList(), - transientStorage: newTransientStorage(), + 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), + accessList: newAccessList(), + transientStorage: newTransientStorage(), }, nil } @@ -140,6 +142,7 @@ 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 @@ -471,14 +474,14 @@ func (s *StateDB) GetTransientState(addr common.Address, key common.Hash) common // // 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 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)) } @@ -486,14 +489,12 @@ 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 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[:])) } @@ -501,17 +502,28 @@ func (s *StateDB) deleteStateObject(stateObject *stateObject) { func (s *StateDB) DeleteAddress(addr common.Address) { stateObject := s.getStateObject(addr) if stateObject != nil && !stateObject.deleted { + stateObject.deleted = true s.deleteStateObject(stateObject) } } -// Retrieve a state object given my the address. Returns nil if not found. -func (s *StateDB) getStateObject(addr common.Address) (stateObject *stateObject) { +// 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 @@ -541,7 +553,7 @@ func (s *StateDB) setStateObject(object *stateObject) { // Retrieve 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.deleted { + if stateObject == nil { stateObject, _ = s.createObject(addr) } return stateObject @@ -556,7 +568,8 @@ func (s *StateDB) MarkStateObjectDirty(addr common.Address) { // 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.getStateObject(addr) + prev = s.getDeletedStateObject(addr) // Note, prev might have been deleted, we need that! + newobj = newObject(s, addr, Account{}, s.MarkStateObjectDirty) newobj.setNonce(0) // sets the object to dirty if prev == nil { @@ -628,18 +641,29 @@ func (s *StateDB) Copy() *StateDB { // Copy all the basic fields, initialize the memory ones state := &StateDB{ - db: s.db, - trie: s.db.CopyTrie(s.trie), - stateObjects: make(map[common.Address]*stateObject, len(s.stateObjectsDirty)), - stateObjectsDirty: make(map[common.Address]struct{}, len(s.stateObjectsDirty)), - refund: s.refund, - logs: make(map[common.Hash][]*types.Log, len(s.logs)), - logSize: s.logSize, - preimages: make(map[common.Hash][]byte), + db: s.db, + trie: s.db.CopyTrie(s.trie), + stateObjects: make(map[common.Address]*stateObject, len(s.stateObjectsDirty)), + stateObjectsPending: make(map[common.Address]struct{}, len(s.stateObjectsPending)), + stateObjectsDirty: make(map[common.Address]struct{}, len(s.stateObjectsDirty)), + refund: s.refund, + logs: make(map[common.Hash][]*types.Log, len(s.logs)), + logSize: s.logSize, + preimages: make(map[common.Hash][]byte), + } + // Above, we don't copy the actual journal. This means that if the copy is copied, the + // loop above will be a no-op, since the copy's journal is empty. + // Thus, here we iterate over stateObjects, to enable copies of copies + for addr := range s.stateObjectsPending { + if _, exist := state.stateObjects[addr]; !exist { + state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state, state.MarkStateObjectDirty) + } + state.stateObjectsPending[addr] = struct{}{} } - // Copy the dirty states, logs, and preimages for addr := range s.stateObjectsDirty { - state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state, state.MarkStateObjectDirty) + if _, exist := state.stateObjects[addr]; !exist { + state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state, state.MarkStateObjectDirty) + } state.stateObjectsDirty[addr] = struct{}{} } @@ -702,18 +726,23 @@ func (s *StateDB) GetRefund() uint64 { return s.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.stateObjectsDirty { - stateObject := s.stateObjects[addr] - if stateObject.selfDestructed || (deleteEmptyObjects && stateObject.empty()) { - s.deleteStateObject(stateObject) - } else { - stateObject.updateRoot(s.db) - s.updateStateObject(stateObject) + obj, exist := s.stateObjects[addr] + if !exist { + continue } - stateObject.created = false + if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { + obj.deleted = true + } else { + obj.finalise() + } + obj.created = false + s.stateObjectsPending[addr] = struct{}{} + s.stateObjectsDirty[addr] = struct{}{} } // Invalidate journal because reverting across transactions is not allowed. s.clearJournalAndRefund() @@ -723,8 +752,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 defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) @@ -762,24 +804,19 @@ func (s *StateDB) DeleteSuicides() { func (s *StateDB) clearJournalAndRefund() { s.journal = nil - s.validRevisions = s.validRevisions[:0] s.refund = 0 + s.validRevisions = s.validRevisions[:0] // Snapshots can be created without journal entires } // Commit writes the state to the underlying in-memory trie database. -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) // Commit objects to the trie, measuring the elapsed time codeWriter := s.db.TrieDB().DiskDB().NewBatch() - for addr, obj := range s.stateObjects { - _, isDirty := s.stateObjectsDirty[addr] - switch { - case obj.selfDestructed || (isDirty && deleteEmptyObjects && obj.empty()): - // If the object has been removed, don't bother syncing it - // and just mark it for deletion in the trie. - s.deleteStateObject(obj) - case isDirty: + for addr := range s.stateObjectsDirty { + if obj := s.stateObjects[addr]; !obj.deleted { // Write any contract code associated with the state object if obj.code != nil && obj.dirtyCode { rawdb.WriteCode(codeWriter, common.BytesToHash(obj.CodeHash()), obj.code) @@ -789,10 +826,10 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) if err := obj.CommitTrie(s.db); err != nil { return common.Hash{}, err } - // Update the object in the main account trie. - s.updateStateObject(obj) } - delete(s.stateObjectsDirty, addr) + } + if len(s.stateObjectsDirty) > 0 { + s.stateObjectsDirty = make(map[common.Address]struct{}) } if codeWriter.ValueSize() > 0 { if err := codeWriter.Write(); err != nil { @@ -802,7 +839,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) // Write the account trie changes, measuing the amount of wasted time 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 @@ -812,7 +849,6 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) } return nil }) - return root, err } // Prepare handles the preparatory steps for executing a state transition with. diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index 75a5844501..89ed1187a6 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -657,3 +657,38 @@ func TestStateDBTransientStorage(t *testing.T) { t.Fatalf("transient storage mismatch: have %x, want %x", got, value) } } + +// 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.SelfDestruct(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") + } +}