From 9265a267cdc6f4e545f3a4320c3072f4ec3ee29d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 12 Aug 2019 10:15:54 +0300 Subject: [PATCH] Revert "core/state: be smarter as to where we read state from" This reverts commit aef093b73f1660acff6e1735165f51c7a7088df5. --- core/state/journal.go | 10 ++++----- core/state/state_object.go | 46 +++++++------------------------------- core/state/statedb.go | 38 +++++++++++++++---------------- core/state_prefetcher.go | 2 ++ 4 files changed, 33 insertions(+), 63 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index 075b3a7a57..a03ca57dbc 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -151,7 +151,7 @@ func (ch resetObjectChange) dirtied() *common.Address { } func (ch suicideChange) revert(s *StateDB) { - obj := s.getStateObject(*ch.account, true) + obj := s.getStateObject(*ch.account) if obj != nil { obj.suicided = ch.prev obj.setBalance(ch.prevbalance) @@ -172,7 +172,7 @@ func (ch touchChange) dirtied() *common.Address { } func (ch balanceChange) revert(s *StateDB) { - s.getStateObject(*ch.account, true).setBalance(ch.prev) + s.getStateObject(*ch.account).setBalance(ch.prev) } func (ch balanceChange) dirtied() *common.Address { @@ -180,7 +180,7 @@ func (ch balanceChange) dirtied() *common.Address { } func (ch nonceChange) revert(s *StateDB) { - s.getStateObject(*ch.account, true).setNonce(ch.prev) + s.getStateObject(*ch.account).setNonce(ch.prev) } func (ch nonceChange) dirtied() *common.Address { @@ -188,7 +188,7 @@ func (ch nonceChange) dirtied() *common.Address { } func (ch codeChange) revert(s *StateDB) { - s.getStateObject(*ch.account, true).setCode(common.BytesToHash(ch.prevhash), ch.prevcode) + s.getStateObject(*ch.account).setCode(common.BytesToHash(ch.prevhash), ch.prevcode) } func (ch codeChange) dirtied() *common.Address { @@ -196,7 +196,7 @@ func (ch codeChange) dirtied() *common.Address { } func (ch storageChange) revert(s *StateDB) { - s.getStateObject(*ch.account, true).setState(ch.key, ch.prevalue) + s.getStateObject(*ch.account).setState(ch.key, ch.prevalue) } func (ch storageChange) dirtied() *common.Address { diff --git a/core/state/state_object.go b/core/state/state_object.go index dcc4bbcdbe..9706797c95 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -162,56 +162,27 @@ func (s *stateObject) getTrie(db Database) Trie { return s.trie } -// GetState retrieves a value from the account storage. This method will use the -// state snapshot so retrieve the value (opposed to the trie directly) since the -// read doesn't need to pull in log(n) trie nodes in addition from disk. +// 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 (debugging mode) + // If the fake storage is set, only lookup the state here(in the debugging mode) if s.fakeStorage != nil { return s.fakeStorage[key] } - // No fake storage, retrieve a real object from the storage trie. We're cheating - // a bit here since we know that this method is only using during simple reads. - // As such, we possibly will not write this key, so might as well avoid touching - // trie nodes and pull it directly from the state snapshot. - return s.getState(db, key, true) -} - -// getState retrieves a value from the account's storage, but the caller gets to -// control whether to use the state snapshot or the state trie as the source. For -// simple reads, the snapshot should be used as it's faster. For writes however, -// using the trie will be a bit slower, but will pre-cache nodes needed during -// commit anyway. -func (s *stateObject) getState(db Database, key common.Hash, snapshot bool) common.Hash { // If we have a dirty value for this state entry, return it value, dirty := s.dirtyStorage[key] if dirty { return value } // Otherwise return the entry's original value - return s.getCommittedState(db, key, snapshot) + return s.GetCommittedState(db, key) } -// GetCommittedState retrieves a value from the committed account storage. This -// method will use the slow trie (opposed to state snapshots) since the committed -// value is only ever used to avoid writes, so we can pre-load trie nodes. +// GetCommittedState retrieves a value from the committed account storage trie. func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Hash { - // If the fake storage is set, only lookup the state here (debugging mode) + // If the fake storage is set, only lookup the state here(in the debugging mode) if s.fakeStorage != nil { return s.fakeStorage[key] } - // No fake storage, retrieve a real object from the storage trie. We're cheating - // a bit here since we know that this method is only using during net sstore gas - // metering. As such, we probably not only read, but also write this key in the - // same transaction, so might as well pre-cache trie nodes on the write path. - return s.getCommittedState(db, key, false) -} - -// getCommittedState retrieves a value from the account's storage, but the caller -// gets to control whether to use the state snapshot or the state trie. For simple -// reads, the snapshot should be used as it's faster. For pre-writes however, using -// the trie will be a bit slower, but will pre-cache nodes needed during commit. -func (s *stateObject) getCommittedState(db Database, key common.Hash, snapshot bool) common.Hash { // If we have the original value cached, return that value, cached := s.originStorage[key] if cached { @@ -222,7 +193,7 @@ func (s *stateObject) getCommittedState(db Database, key common.Hash, snapshot b enc []byte err error ) - if snapshot && s.db.snap != nil { + if s.db.snap != nil { if metrics.EnabledExpensive { defer func(start time.Time) { s.db.SnapshotStorageReads += time.Since(start) }(time.Now()) } @@ -256,9 +227,8 @@ func (s *stateObject) SetState(db Database, key, value common.Hash) { s.fakeStorage[key] = value return } - // If the new value is the same as old, don't set (use the trie to cache any - // nodes if we decide to write) - prev := s.getState(db, key, false) + // If the new value is the same as old, don't set + prev := s.GetState(db, key) if prev == value { return } diff --git a/core/state/statedb.go b/core/state/statedb.go index defe61d114..e817c14dc7 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -234,19 +234,19 @@ func (self *StateDB) SubRefund(gas uint64) { // Exist reports whether the given account address exists in the state. // Notably this also returns true for suicided accounts. func (self *StateDB) Exist(addr common.Address) bool { - return self.getStateObject(addr, true) != nil + return self.getStateObject(addr) != nil } // Empty returns whether the state object is either non-existent // or empty according to the EIP161 specification (balance = nonce = code = 0) func (self *StateDB) Empty(addr common.Address) bool { - so := self.getStateObject(addr, true) + so := self.getStateObject(addr) return so == nil || so.empty() } // Retrieve the balance from the given address or 0 if object not found func (self *StateDB) GetBalance(addr common.Address) *big.Int { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject != nil { return stateObject.Balance() } @@ -254,7 +254,7 @@ func (self *StateDB) GetBalance(addr common.Address) *big.Int { } func (self *StateDB) GetNonce(addr common.Address) uint64 { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject != nil { return stateObject.Nonce() } @@ -273,7 +273,7 @@ func (self *StateDB) BlockHash() common.Hash { } func (self *StateDB) GetCode(addr common.Address) []byte { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject != nil { return stateObject.Code(self.db) } @@ -281,7 +281,7 @@ func (self *StateDB) GetCode(addr common.Address) []byte { } func (self *StateDB) GetCodeSize(addr common.Address) int { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject == nil { return 0 } @@ -296,7 +296,7 @@ func (self *StateDB) GetCodeSize(addr common.Address) int { } func (self *StateDB) GetCodeHash(addr common.Address) common.Hash { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject == nil { return common.Hash{} } @@ -305,7 +305,7 @@ func (self *StateDB) GetCodeHash(addr common.Address) common.Hash { // GetState retrieves a value from the given account's storage trie. func (self *StateDB) GetState(addr common.Address, hash common.Hash) common.Hash { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject != nil { return stateObject.GetState(self.db, hash) } @@ -330,11 +330,9 @@ func (self *StateDB) GetStorageProof(a common.Address, key common.Hash) ([][]byt return [][]byte(proof), err } -// GetCommittedState retrieves a value from the given account's committed storage. -// This method will use the slow trie (opposed to state snapshots) since the value -// committed is only ever used to avoid writes, so we can pre-load trie nodes. +// GetCommittedState retrieves a value from the given account's committed storage trie. func (self *StateDB) GetCommittedState(addr common.Address, hash common.Hash) common.Hash { - stateObject := self.getStateObject(addr, false) + stateObject := self.getStateObject(addr) if stateObject != nil { return stateObject.GetCommittedState(self.db, hash) } @@ -349,7 +347,7 @@ func (self *StateDB) Database() Database { // StorageTrie returns the storage trie of an account. // The return value is a copy and is nil for non-existent accounts. func (self *StateDB) StorageTrie(addr common.Address) Trie { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject == nil { return nil } @@ -358,7 +356,7 @@ func (self *StateDB) StorageTrie(addr common.Address) Trie { } func (self *StateDB) HasSuicided(addr common.Address) bool { - stateObject := self.getStateObject(addr, true) + stateObject := self.getStateObject(addr) if stateObject != nil { return stateObject.suicided } @@ -428,7 +426,7 @@ func (self *StateDB) SetStorage(addr common.Address, storage map[common.Hash]com // The account's state object is still available until the state is committed, // getStateObject will return a non-nil account after Suicide. func (self *StateDB) Suicide(addr common.Address) bool { - stateObject := self.getStateObject(addr, false) + stateObject := self.getStateObject(addr) if stateObject == nil { return false } @@ -487,7 +485,7 @@ func (s *StateDB) deleteStateObject(stateObject *stateObject) { } // Retrieve a state object given by the address. Returns nil if not found. -func (s *StateDB) getStateObject(addr common.Address, snapshot bool) (stateObject *stateObject) { +func (s *StateDB) getStateObject(addr common.Address) (stateObject *stateObject) { // Prefer live objects if obj := s.stateObjects[addr]; obj != nil { if obj.deleted { @@ -497,7 +495,7 @@ func (s *StateDB) getStateObject(addr common.Address, snapshot bool) (stateObjec } // If no live objects are available, attempt to use snapshots var data Account - if snapshot && s.snap != nil { + if s.snap != nil { if metrics.EnabledExpensive { defer func(start time.Time) { s.SnapshotAccountReads += time.Since(start) }(time.Now()) } @@ -540,7 +538,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, true) + stateObject := self.getStateObject(addr) if stateObject == nil || stateObject.deleted { stateObject, _ = self.createObject(addr) } @@ -550,7 +548,7 @@ 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, false) + prev = self.getStateObject(addr) newobj = newObject(self, addr, Account{}) newobj.setNonce(0) // sets the object to dirty if prev == nil { @@ -580,7 +578,7 @@ func (self *StateDB) CreateAccount(addr common.Address) { } func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common.Hash) bool) error { - so := db.getStateObject(addr, true) + so := db.getStateObject(addr) if so == nil { return nil } diff --git a/core/state_prefetcher.go b/core/state_prefetcher.go index cb85a05b57..bb5db4ced1 100644 --- a/core/state_prefetcher.go +++ b/core/state_prefetcher.go @@ -65,6 +65,8 @@ func (p *statePrefetcher) Prefetch(block *types.Block, statedb *state.StateDB, c return // Ugh, something went horribly wrong, bail out } } + // All transactions processed, finalize the block to force loading written-only trie paths + statedb.Finalise(true) // TODO(karalabe): should we run this on interrupt too? } // precacheTransaction attempts to apply a transaction to the given state database