diff --git a/core/state/state_object.go b/core/state/state_object.go index 9e8fe8f869..15492f4dfb 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -77,7 +77,7 @@ 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 + 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. @@ -128,7 +128,7 @@ func newObject(db *StateDB, address common.Address, data Account, onDirty func(a address: address, addrHash: crypto.Keccak256Hash(address[:]), data: data, - cachedStorage: make(Storage), + originStorage: make(Storage), dirtyStorage: make(Storage), onDirty: onDirty, } @@ -179,40 +179,34 @@ func (s *stateObject) getTrie(db Database) Trie { return s.trie } -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] - } - // 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) - } - return value -} - +// 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] } - value, exists := s.cachedStorage[key] - if exists { + // If we have a dirty value for this state entry, return it + value, dirty := s.dirtyStorage[key] + if dirty { return value } - // Load from DB in case it is missing. + // Otherwise return the entry's original value + return s.GetCommittedState(db, key) +} + +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] + } + // If we have the original value cached, return that + value, cached := s.originStorage[key] + if 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) @@ -225,9 +219,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 +263,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 { @@ -287,6 +278,13 @@ func (s *stateObject) updateTrie(db Database) Trie { tr := s.getTrie(db) for key, value := range s.dirtyStorage { delete(s.dirtyStorage, key) + + // 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 @@ -372,7 +370,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 cdc8ccf8e6..59dd8bba16 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)) } - if so0.deleted != so1.deleted { - t.Fatalf("Deleted mismatch: have %v, want %v", so0.deleted, so1.deleted) + 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) + } } } diff --git a/core/state/statedb.go b/core/state/statedb.go index 91f5b5e771..445da17edc 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -102,23 +102,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) @@ -209,11 +192,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 +308,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, bhash) + 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.GetCommittedState(s.db, hash) } return common.Hash{} } @@ -591,19 +596,14 @@ 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 { + cb(key, value) + continue } + cb(key, common.BytesToHash(it.Value)) } return nil } diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index 0634fa646c..75a5844501 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 {