From 01808421e20ba9d19c029b64fcda841df77c9aff Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Wed, 21 Dec 2022 17:21:21 +0800 Subject: [PATCH] core/state: return error when storage trie can't be opened (#26350) This changes the StorageTrie method to return an error when the trie is not available. It used to return an 'empty trie' in this case, but that's not possible anymore under PBSS. --- core/state/dump.go | 7 +++- core/state/state_object.go | 77 ++++++++++++++++++++++++++------------ core/state/statedb.go | 37 ++++++++++++------ eth/api.go | 5 ++- eth/api_test.go | 6 ++- internal/ethapi/api.go | 6 ++- 6 files changed, 97 insertions(+), 41 deletions(-) diff --git a/core/state/dump.go b/core/state/dump.go index d97520f08..d834cbad3 100644 --- a/core/state/dump.go +++ b/core/state/dump.go @@ -168,7 +168,12 @@ func (s *StateDB) DumpToCollector(c DumpCollector, conf *DumpConfig) (nextKey [] } if !conf.SkipStorage { account.Storage = make(map[common.Hash]string) - storageIt := trie.NewIterator(obj.getTrie(s.db).NodeIterator(nil)) + tr, err := obj.getTrie(s.db) + if err != nil { + log.Error("Failed to load storage trie", "err", err) + continue + } + storageIt := trie.NewIterator(tr.NodeIterator(nil)) for storageIt.Next() { _, content, _, err := rlp.Split(storageIt.Value) if err != nil { diff --git a/core/state/state_object.go b/core/state/state_object.go index d2693b0c0..7e1709ee0 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -148,7 +148,10 @@ func (s *stateObject) touch() { } } -func (s *stateObject) getTrie(db Database) Trie { +// getTrie returns the associated storage trie. The trie will be opened +// if it's not loaded previously. An error will be returned if trie can't +// be loaded. +func (s *stateObject) getTrie(db Database) (Trie, error) { if s.trie == nil { // Try fetching from prefetcher first // We don't prefetch empty tries @@ -158,15 +161,14 @@ func (s *stateObject) getTrie(db Database) Trie { s.trie = s.db.prefetcher.trie(s.addrHash, s.data.Root) } if s.trie == nil { - var err error - s.trie, err = db.OpenStorageTrie(s.db.originalRoot, s.addrHash, s.data.Root) + tr, err := db.OpenStorageTrie(s.db.originalRoot, s.addrHash, s.data.Root) if err != nil { - s.trie, _ = db.OpenStorageTrie(s.db.originalRoot, s.addrHash, common.Hash{}) - s.setError(fmt.Errorf("can't create storage trie: %v", err)) + return nil, err } + s.trie = tr } } - return s.trie + return s.trie, nil } // GetState retrieves a value from the account storage trie. @@ -221,7 +223,12 @@ func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Has // If the snapshot is unavailable or reading from it fails, load from the database. if s.db.snap == nil || err != nil { start := time.Now() - enc, err = s.getTrie(db).TryGet(key.Bytes()) + tr, err := s.getTrie(db) + if err != nil { + s.setError(err) + return common.Hash{} + } + enc, err = tr.TryGet(key.Bytes()) if metrics.EnabledExpensive { s.db.StorageReads += time.Since(start) } @@ -304,23 +311,29 @@ func (s *stateObject) finalise(prefetch bool) { } // 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 { +// It will return nil if the trie has not been loaded and no changes have been +// made. An error will be returned if the trie can't be loaded/updated correctly. +func (s *stateObject) updateTrie(db Database) (Trie, error) { // Make sure all dirty slots are finalized into the pending storage area s.finalise(false) // Don't prefetch anymore, pull directly if need be if len(s.pendingStorage) == 0 { - return s.trie + return s.trie, nil } // Track the amount of time wasted on updating the storage trie if metrics.EnabledExpensive { defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now()) } // The snapshot storage map for the object - var storage map[common.Hash][]byte + var ( + storage map[common.Hash][]byte + hasher = s.db.hasher + ) + tr, err := s.getTrie(db) + if err != nil { + s.setError(err) + return nil, err + } // Insert all the pending updates into the trie - tr := s.getTrie(db) - hasher := s.db.hasher - usedStorage := make([][]byte, 0, len(s.pendingStorage)) for key, value := range s.pendingStorage { // Skip noop changes, persist actual changes @@ -331,12 +344,18 @@ func (s *stateObject) updateTrie(db Database) Trie { var v []byte if (value == common.Hash{}) { - s.setError(tr.TryDelete(key[:])) + if err := tr.TryDelete(key[:]); err != nil { + s.setError(err) + return nil, err + } s.db.StorageDeleted += 1 } else { // Encoding []byte cannot fail, ok to ignore the error. v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:])) - s.setError(tr.TryUpdate(key[:], v)) + if err := tr.TryUpdate(key[:], v); err != nil { + s.setError(err) + return nil, err + } s.db.StorageUpdated += 1 } // If state snapshotting is active, cache the data til commit @@ -358,37 +377,47 @@ func (s *stateObject) updateTrie(db Database) Trie { if len(s.pendingStorage) > 0 { s.pendingStorage = make(Storage) } - return tr + return tr, nil } -// UpdateRoot sets the trie root to the current root hash of +// UpdateRoot sets the trie root to the current root hash of. An error +// will be returned if trie root hash is not computed correctly. func (s *stateObject) updateRoot(db Database) { + tr, err := s.updateTrie(db) + if err != nil { + s.setError(fmt.Errorf("updateRoot (%x) error: %w", s.address, err)) + return + } // If nothing changed, don't bother with hashing anything - if s.updateTrie(db) == nil { + if tr == nil { return } // Track the amount of time wasted on hashing the storage trie if metrics.EnabledExpensive { defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now()) } - s.data.Root = s.trie.Hash() + s.data.Root = tr.Hash() } // commitTrie submits the storage changes into the storage trie and re-computes // the root. Besides, all trie changes will be collected in a nodeset and returned. func (s *stateObject) commitTrie(db Database) (*trie.NodeSet, error) { - // If nothing changed, don't bother with hashing anything - if s.updateTrie(db) == nil { - return nil, nil + tr, err := s.updateTrie(db) + if err != nil { + return nil, err } if s.dbErr != nil { return nil, s.dbErr } + // If nothing changed, don't bother with committing anything + if tr == nil { + return nil, nil + } // Track the amount of time wasted on committing the storage trie if metrics.EnabledExpensive { defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now()) } - root, nodes, err := s.trie.Commit(false) + root, nodes, err := tr.Commit(false) if err == nil { s.data.Root = root } diff --git a/core/state/statedb.go b/core/state/statedb.go index 91eb07a22..993bda3e3 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -339,13 +339,19 @@ func (s *StateDB) GetProofByHash(addrHash common.Hash) ([][]byte, error) { // GetStorageProof returns the Merkle proof for given storage slot. func (s *StateDB) GetStorageProof(a common.Address, key common.Hash) ([][]byte, error) { - var proof proofList - trie := s.StorageTrie(a) - if trie == nil { - return proof, errors.New("storage trie for requested address does not exist") + trie, err := s.StorageTrie(a) + if err != nil { + return nil, err } - err := trie.Prove(crypto.Keccak256(key.Bytes()), 0, &proof) - return proof, err + if trie == nil { + return nil, errors.New("storage trie for requested address does not exist") + } + var proof proofList + err = trie.Prove(crypto.Keccak256(key.Bytes()), 0, &proof) + if err != nil { + return nil, err + } + return proof, nil } // GetCommittedState retrieves a value from the given account's committed storage trie. @@ -362,15 +368,18 @@ func (s *StateDB) Database() Database { return s.db } -// StorageTrie returns the storage trie of an account. -// The return value is a copy and is nil for non-existent accounts. -func (s *StateDB) StorageTrie(addr common.Address) Trie { +// StorageTrie returns the storage trie of an account. The return value is a copy +// and is nil for non-existent accounts. An error will be returned if storage trie +// is existent but can't be loaded correctly. +func (s *StateDB) StorageTrie(addr common.Address) (Trie, error) { stateObject := s.getStateObject(addr) if stateObject == nil { - return nil + return nil, nil } cpy := stateObject.deepCopy(s) - cpy.updateTrie(s.db) + if _, err := cpy.updateTrie(s.db); err != nil { + return nil, err + } return cpy.getTrie(s.db) } @@ -654,7 +663,11 @@ func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common if so == nil { return nil } - it := trie.NewIterator(so.getTrie(db.db).NodeIterator(nil)) + tr, err := so.getTrie(db.db) + if err != nil { + return err + } + it := trie.NewIterator(tr.NodeIterator(nil)) for it.Next() { key := common.BytesToHash(db.trie.GetKey(it.Key)) diff --git a/eth/api.go b/eth/api.go index 0591b98de..ceed85ef5 100644 --- a/eth/api.go +++ b/eth/api.go @@ -417,7 +417,10 @@ func (api *DebugAPI) StorageRangeAt(ctx context.Context, blockHash common.Hash, } defer release() - st := statedb.StorageTrie(contractAddress) + st, err := statedb.StorageTrie(contractAddress) + if err != nil { + return StorageRangeResult{}, err + } if st == nil { return StorageRangeResult{}, fmt.Errorf("account %x doesn't exist", contractAddress) } diff --git a/eth/api_test.go b/eth/api_test.go index 250591c10..fca17f121 100644 --- a/eth/api_test.go +++ b/eth/api_test.go @@ -209,7 +209,11 @@ func TestStorageRangeAt(t *testing.T) { }, } for _, test := range tests { - result, err := storageRangeAt(state.StorageTrie(addr), test.start, test.limit) + tr, err := state.StorageTrie(addr) + if err != nil { + t.Error(err) + } + result, err := storageRangeAt(tr, test.start, test.limit) if err != nil { t.Error(err) } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index ea0cbfe86..49b65559d 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -660,8 +660,10 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st if state == nil || err != nil { return nil, err } - - storageTrie := state.StorageTrie(address) + storageTrie, err := state.StorageTrie(address) + if err != nil { + return nil, err + } storageHash := types.EmptyRootHash codeHash := state.GetCodeHash(address) storageProof := make([]StorageResult, len(storageKeys))