diff --git a/consensus/tests/engine_v1_tests/helper.go b/consensus/tests/engine_v1_tests/helper.go index 1898421264a2..daaf79c00e91 100644 --- a/consensus/tests/engine_v1_tests/helper.go +++ b/consensus/tests/engine_v1_tests/helper.go @@ -25,7 +25,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/crypto" "github.com/XinFinOrg/XDPoSChain/log" "github.com/XinFinOrg/XDPoSChain/params" - "github.com/XinFinOrg/XDPoSChain/rlp" "github.com/XinFinOrg/XDPoSChain/trie" ) @@ -128,14 +127,7 @@ func getCommonBackend(t *testing.T, chainConfig *params.ChainConfig) *backends.S code, _ := contractBackendForSC.CodeAt(ctx, validatorSCAddr, nil) storage := make(map[common.Hash]common.Hash) f := func(key, val common.Hash) bool { - decode := []byte{} - trim := bytes.TrimLeft(val.Bytes(), "\x00") - err := rlp.DecodeBytes(trim, &decode) - if err != nil { - t.Fatalf("Failed while decode byte") - } - storage[key] = common.BytesToHash(decode) - log.Info("DecodeBytes", "value", val.String(), "decode", storage[key].String()) + storage[key] = val return true } err = contractBackendForSC.ForEachStorageAt(ctx, validatorSCAddr, nil, f) diff --git a/consensus/tests/engine_v2_tests/helper.go b/consensus/tests/engine_v2_tests/helper.go index bd9a512a7734..0702c3f768c5 100644 --- a/consensus/tests/engine_v2_tests/helper.go +++ b/consensus/tests/engine_v2_tests/helper.go @@ -28,7 +28,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/crypto" "github.com/XinFinOrg/XDPoSChain/log" "github.com/XinFinOrg/XDPoSChain/params" - "github.com/XinFinOrg/XDPoSChain/rlp" "github.com/XinFinOrg/XDPoSChain/trie" "github.com/stretchr/testify/assert" ) @@ -178,14 +177,7 @@ func getCommonBackend(t *testing.T, chainConfig *params.ChainConfig) *backends.S code, _ := contractBackendForSC.CodeAt(ctx, validatorSCAddr, nil) storage := make(map[common.Hash]common.Hash) f := func(key, val common.Hash) bool { - decode := []byte{} - trim := bytes.TrimLeft(val.Bytes(), "\x00") - err := rlp.DecodeBytes(trim, &decode) - if err != nil { - t.Fatalf("Failed while decode byte") - } - storage[key] = common.BytesToHash(decode) - log.Info("DecodeBytes", "value", val.String(), "decode", storage[key].String()) + storage[key] = val return true } err = contractBackendForSC.ForEachStorageAt(ctx, validatorSCAddr, nil, f) @@ -262,14 +254,7 @@ func getMultiCandidatesBackend(t *testing.T, chainConfig *params.ChainConfig, n code, _ := contractBackendForSC.CodeAt(ctx, validatorSCAddr, nil) storage := make(map[common.Hash]common.Hash) f := func(key, val common.Hash) bool { - decode := []byte{} - trim := bytes.TrimLeft(val.Bytes(), "\x00") - err := rlp.DecodeBytes(trim, &decode) - if err != nil { - t.Fatalf("Failed while decode byte") - } - storage[key] = common.BytesToHash(decode) - log.Info("DecodeBytes", "value", val.String(), "decode", storage[key].String()) + storage[key] = val return true } err = contractBackendForSC.ForEachStorageAt(ctx, validatorSCAddr, nil, f) @@ -347,14 +332,7 @@ func getProtectorObserverBackend(t *testing.T, chainConfig *params.ChainConfig) code, _ := contractBackendForSC.CodeAt(ctx, validatorSCAddr, nil) storage := make(map[common.Hash]common.Hash) f := func(key, val common.Hash) bool { - decode := []byte{} - trim := bytes.TrimLeft(val.Bytes(), "\x00") - err := rlp.DecodeBytes(trim, &decode) - if err != nil { - t.Fatalf("Failed while decode byte") - } - storage[key] = common.BytesToHash(decode) - log.Info("DecodeBytes", "value", val.String(), "decode", storage[key].String()) + storage[key] = val return true } err = contractBackendForSC.ForEachStorageAt(ctx, validatorSCAddr, nil, f) diff --git a/contracts/validator/validator_test.go b/contracts/validator/validator_test.go index 00ed5836503f..f710f8bb14b7 100644 --- a/contracts/validator/validator_test.go +++ b/contracts/validator/validator_test.go @@ -16,7 +16,6 @@ package validator import ( - "bytes" "context" "encoding/json" "fmt" @@ -36,7 +35,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/crypto" "github.com/XinFinOrg/XDPoSChain/log" "github.com/XinFinOrg/XDPoSChain/params" - "github.com/XinFinOrg/XDPoSChain/rlp" ) var ( @@ -300,12 +298,7 @@ func TestStatedbUtils(t *testing.T) { code, _ := contractBackend.CodeAt(ctx, validatorAddress, nil) storage := make(map[common.Hash]common.Hash) f := func(key, val common.Hash) bool { - decode := []byte{} - trim := bytes.TrimLeft(val.Bytes(), "\x00") - rlp.DecodeBytes(trim, &decode) - storage[key] = common.BytesToHash(decode) - // t.Log("DecodeBytes", "value", val.String(), "decode", storage[key].String()) - // t.Log("key", key.String(), "value", storage[key].String()) + storage[key] = val return true } contractBackend.ForEachStorageAt(ctx, validatorAddress, nil, f) diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 90bb9a483df5..8f3b79b401b5 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 9e8fe8f86912..f24702e4c229 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 - cachedStorage Storage // Storage entry cache to avoid duplicate reads - 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, - cachedStorage: 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, } } @@ -179,45 +184,42 @@ func (s *stateObject) getTrie(db Database) Trie { return s.trie } -func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Hash { +// GetState retrieves a value from the account storage trie. +func (s *stateObject) GetState(db Database, key common.Hash) common.Hash { // If the fake storage is set, only lookup the state here(in the debugging mode) if s.fakeStorage != nil { return s.fakeStorage[key] } - // Track the amount of time wasted on reading the storage trie - defer func(start time.Time) { s.db.StorageReads += time.Since(start) }(time.Now()) - value := common.Hash{} - // Load from DB in case it is missing. - enc, err := s.getTrie(db).TryGet(key[:]) - if err != nil { - s.setError(err) - return common.Hash{} - } - if len(enc) > 0 { - _, content, _, err := rlp.Split(enc) - if err != nil { - s.setError(err) - } - value.SetBytes(content) + // If we have a dirty value for this state entry, return it + value, dirty := s.dirtyStorage[key] + if dirty { + return value } - return value + // Otherwise return the entry's original value + return s.GetCommittedState(db, key) } -func (s *stateObject) GetState(db Database, key common.Hash) common.Hash { +func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Hash { // If the fake storage is set, only lookup the state here(in the debugging mode) if s.fakeStorage != nil { return s.fakeStorage[key] } - value, exists := s.cachedStorage[key] - if exists { + // If we have a pending write or clean cached, return that + if value, pending := s.pendingStorage[key]; pending { return value } - // Load from DB in case it is missing. + if value, cached := s.originStorage[key]; cached { + return value + } + // Track the amount of time wasted on reading the storage trie + defer func(start time.Time) { s.db.StorageReads += time.Since(start) }(time.Now()) + // Otherwise load the value from the database enc, err := s.getTrie(db).TryGet(key[:]) if err != nil { s.setError(err) return common.Hash{} } + var value common.Hash if len(enc) > 0 { _, content, _, err := rlp.Split(enc) if err != nil { @@ -225,9 +227,7 @@ func (s *stateObject) GetState(db Database, key common.Hash) common.Hash { } value.SetBytes(content) } - if (value != common.Hash{}) { - s.cachedStorage[key] = value - } + s.originStorage[key] = value return value } @@ -271,7 +271,6 @@ func (s *stateObject) SetStorage(storage map[common.Hash]common.Hash) { } func (s *stateObject) setState(key, value common.Hash) { - s.cachedStorage[key] = value s.dirtyStorage[key] = value if s.onDirty != nil { @@ -280,13 +279,36 @@ 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. +// It will return nil if the trie has not been loaded and no changes have been made func (s *stateObject) updateTrie(db Database) Trie { + // Make sure all dirty slots are finalized into the pending storage area + s.finalise() + if len(s.pendingStorage) == 0 { + return s.trie + } // 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 + } + s.originStorage[key] = value + if (value == common.Hash{}) { s.setError(tr.TryDelete(key[:])) continue @@ -295,13 +317,18 @@ 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 } // UpdateRoot sets the trie root to the current root hash of func (s *stateObject) updateRoot(db Database) { - s.updateTrie(db) - + // If nothing changed, don't bother with hashing anything + if s.updateTrie(db) == nil { + return + } // Track the amount of time wasted on hashing the storage trie defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now()) @@ -311,7 +338,10 @@ func (s *stateObject) updateRoot(db Database) { // CommitTrie the storage trie of the object to dwb. // This updates the trie root. func (s *stateObject) CommitTrie(db Database) error { - s.updateTrie(db) + // If nothing changed, don't bother with hashing anything + if s.updateTrie(db) == nil { + return nil + } if s.dbErr != nil { return s.dbErr } @@ -372,7 +402,7 @@ func (s *stateObject) deepCopy(db *StateDB, onDirty func(addr common.Address)) * } stateObject.code = s.code stateObject.dirtyStorage = s.dirtyStorage.Copy() - stateObject.cachedStorage = s.dirtyStorage.Copy() + stateObject.originStorage = s.originStorage.Copy() stateObject.selfDestructed = s.selfDestructed stateObject.dirtyCode = s.dirtyCode stateObject.deleted = s.deleted diff --git a/core/state/state_test.go b/core/state/state_test.go index cdc8ccf8e602..59dd8bba1632 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -98,11 +98,15 @@ func (s *StateSuite) TestNull(c *checker.C) { s.state.CreateAccount(address) //value := common.FromHex("0x823140710bf13990e4500136726d8b55") var value common.Hash + s.state.SetState(address, common.Hash{}, value) s.state.Commit(false) - value = s.state.GetState(address, common.Hash{}) - if !value.IsZero() { - c.Errorf("expected empty hash. got %x", value) + + if value := s.state.GetState(address, common.Hash{}); value != (common.Hash{}) { + c.Errorf("expected empty current value, got %x", value) + } + if value := s.state.GetCommittedState(address, common.Hash{}); value != (common.Hash{}) { + c.Errorf("expected empty committed value, got %x", value) } } @@ -112,20 +116,24 @@ func (s *StateSuite) TestSnapshot(c *checker.C) { data1 := common.BytesToHash([]byte{42}) data2 := common.BytesToHash([]byte{43}) + // snapshot the genesis state + genesis := s.state.Snapshot() + // set initial state object value s.state.SetState(stateobjaddr, storageaddr, data1) - // get snapshot of current state snapshot := s.state.Snapshot() - // set new state object value + // set a new state object value, revert it and ensure correct content s.state.SetState(stateobjaddr, storageaddr, data2) - // restore snapshot s.state.RevertToSnapshot(snapshot) - // get state storage value - res := s.state.GetState(stateobjaddr, storageaddr) + c.Assert(s.state.GetState(stateobjaddr, storageaddr), checker.DeepEquals, data1) + c.Assert(s.state.GetCommittedState(stateobjaddr, storageaddr), checker.DeepEquals, common.Hash{}) - c.Assert(data1, checker.DeepEquals, res) + // revert up to the genesis state and ensure correct content + s.state.RevertToSnapshot(genesis) + c.Assert(s.state.GetState(stateobjaddr, storageaddr), checker.DeepEquals, common.Hash{}) + c.Assert(s.state.GetCommittedState(stateobjaddr, storageaddr), checker.DeepEquals, common.Hash{}) } func (s *StateSuite) TestSnapshotEmpty(c *checker.C) { @@ -211,24 +219,30 @@ func compareStateObjects(so0, so1 *stateObject, t *testing.T) { t.Fatalf("Code mismatch: have %v, want %v", so0.code, so1.code) } - if len(so1.cachedStorage) != len(so0.cachedStorage) { - t.Errorf("Storage size mismatch: have %d, want %d", len(so1.cachedStorage), len(so0.cachedStorage)) + 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.cachedStorage { - if so0.cachedStorage[k] != v { - t.Errorf("Storage key %x mismatch: have %v, want %v", k, so0.cachedStorage[k], v) + 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.cachedStorage { - if so1.cachedStorage[k] != v { - t.Errorf("Storage key %x mismatch: have %v, want none.", 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 so0.selfDestructed != so1.selfDestructed { - t.Fatalf("self-destructed mismatch: have %v, want %v", so0.selfDestructed, so1.selfDestructed) + 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) + } } - if so0.deleted != so1.deleted { - t.Fatalf("Deleted mismatch: have %v, want %v", so0.deleted, so1.deleted) + for k, v := range so0.originStorage { + if so1.originStorage[k] != v { + t.Errorf("Origin storage key %x mismatch: have %v, want none.", k, v) + } } } diff --git a/core/state/statedb.go b/core/state/statedb.go index 91f5b5e7710a..8661982cb3de 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 @@ -102,23 +103,6 @@ type AccountInfo struct { StorageHash common.Hash } -func (s *StateDB) SubRefund(gas uint64) { - s.journal = append(s.journal, refundChange{ - prev: s.refund}) - if gas > s.refund { - panic(fmt.Sprintf("Refund counter below zero (gas: %d > refund: %d)", gas, s.refund)) - } - s.refund -= gas -} - -func (s *StateDB) GetCommittedState(addr common.Address, hash common.Hash) common.Hash { - stateObject := s.getStateObject(addr) - if stateObject != nil { - return stateObject.GetCommittedState(s.db, hash) - } - return common.Hash{} -} - // Create a new state from a given trie. func New(root common.Hash, db Database) (*StateDB, error) { tr, err := db.OpenTrie(root) @@ -126,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 } @@ -157,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 @@ -209,11 +195,23 @@ func (s *StateDB) Preimages() map[common.Hash][]byte { return s.preimages } +// AddRefund adds gas to the refund counter func (s *StateDB) AddRefund(gas uint64) { s.journal = append(s.journal, refundChange{prev: s.refund}) s.refund += gas } +// SubRefund removes gas from the refund counter. +// This method will panic if the refund counter goes below zero +func (s *StateDB) SubRefund(gas uint64) { + s.journal = append(s.journal, refundChange{ + prev: s.refund}) + if gas > s.refund { + panic(fmt.Sprintf("Refund counter below zero (gas: %d > refund: %d)", gas, s.refund)) + } + s.refund -= gas +} + // Exist reports whether the given account address exists in the state. // Notably this also returns true for self-destructed accounts. func (s *StateDB) Exist(addr common.Address) bool { @@ -313,10 +311,20 @@ func (s *StateDB) GetAccountInfo(addr common.Address) *AccountInfo { return &result } -func (s *StateDB) GetState(addr common.Address, bhash common.Hash) common.Hash { +// GetState retrieves a value from the given account's storage trie. +func (s *StateDB) GetState(addr common.Address, hash common.Hash) common.Hash { + stateObject := s.getStateObject(addr) + if stateObject != nil { + return stateObject.GetState(s.db, hash) + } + return common.Hash{} +} + +// GetCommittedState retrieves a value from the given account's committed storage trie. +func (s *StateDB) GetCommittedState(addr common.Address, hash common.Hash) common.Hash { stateObject := s.getStateObject(addr) if stateObject != nil { - return stateObject.GetState(s.db, bhash) + return stateObject.GetCommittedState(s.db, hash) } return common.Hash{} } @@ -334,7 +342,8 @@ func (s *StateDB) StorageTrie(addr common.Address) Trie { return nil } cpy := stateObject.deepCopy(s, nil) - return cpy.updateTrie(s.db) + cpy.updateTrie(s.db) + return cpy.getTrie(s.db) } func (s *StateDB) HasSelfDestructed(addr common.Address) bool { @@ -466,14 +475,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)) } @@ -481,14 +490,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[:])) } @@ -496,17 +503,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 @@ -536,7 +554,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 @@ -551,7 +569,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 { @@ -591,18 +610,25 @@ func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common if so == nil { return nil } - - // When iterating over the storage check the cache first - for h, value := range so.cachedStorage { - cb(h, value) - } - it := trie.NewIterator(so.getTrie(db.db).NodeIterator(nil)) + for it.Next() { - // ignore cached values key := common.BytesToHash(db.trie.GetKey(it.Key)) - if _, ok := so.cachedStorage[key]; !ok { - cb(key, common.BytesToHash(it.Value)) + if value, dirty := so.dirtyStorage[key]; dirty { + if !cb(key, value) { + return nil + } + continue + } + + if len(it.Value) > 0 { + _, content, _, err := rlp.Split(it.Value) + if err != nil { + return err + } + if !cb(key, common.BytesToHash(content)) { + return nil + } } } return nil @@ -616,18 +642,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), - } - // Copy the dirty states, logs, and preimages + 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{}{} + } 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{}{} } @@ -690,18 +727,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) + obj, exist := s.stateObjects[addr] + if !exist { + continue + } + if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { + obj.deleted = true } else { - stateObject.updateRoot(s.db) - s.updateStateObject(stateObject) + obj.finalise() } - stateObject.created = false + obj.created = false + s.stateObjectsPending[addr] = struct{}{} + s.stateObjectsDirty[addr] = struct{}{} } // Invalidate journal because reverting across transactions is not allowed. s.clearJournalAndRefund() @@ -711,8 +753,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()) @@ -750,24 +805,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) @@ -777,10 +827,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 { @@ -790,7 +840,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 @@ -800,7 +850,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 0634fa646ca9..89ed1187a619 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -411,11 +411,11 @@ func (test *snapshotTest) checkEqual(state, checkstate *StateDB) error { checkeq("GetCodeSize", state.GetCodeSize(addr), checkstate.GetCodeSize(addr)) // Check storage. if obj := state.getStateObject(addr); obj != nil { - state.ForEachStorage(addr, func(key, val common.Hash) bool { - return checkeq("GetState("+key.Hex()+")", val, checkstate.GetState(addr, key)) + state.ForEachStorage(addr, func(key, value common.Hash) bool { + return checkeq("GetState("+key.Hex()+")", checkstate.GetState(addr, key), value) }) - checkstate.ForEachStorage(addr, func(key, checkval common.Hash) bool { - return checkeq("GetState("+key.Hex()+")", state.GetState(addr, key), checkval) + checkstate.ForEachStorage(addr, func(key, value common.Hash) bool { + return checkeq("GetState("+key.Hex()+")", checkstate.GetState(addr, key), value) }) } if err != nil { @@ -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") + } +}