From 41f89ca94423fad523a4427c9af2c327fe344fe2 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Mon, 27 Mar 2023 10:48:46 +0200 Subject: [PATCH] core/state, trie: remove Try prefix in Trie accessors (#26975) This change renames StateTrie methods to remove the Try* prefix. We added the Trie methods with prefix 'Try' a long time ago, working around the problem that most existing methods of Trie did not return the database error. This weird naming convention has persisted until now. Co-authored-by: Gary Rong --- core/state/database.go | 24 +++++++++--------- core/state/state_object.go | 6 ++--- core/state/statedb.go | 6 ++--- core/state/trie_prefetcher.go | 4 +-- eth/protocols/snap/handler.go | 8 +++--- light/trie.go | 12 ++++----- trie/secure_trie.go | 47 +++++++++++++++++------------------ trie/sync_test.go | 2 +- 8 files changed, 54 insertions(+), 55 deletions(-) diff --git a/core/state/database.go b/core/state/database.go index a67c414d9..82f620b46 100644 --- a/core/state/database.go +++ b/core/state/database.go @@ -68,36 +68,36 @@ type Trie interface { // TODO(fjl): remove this when StateTrie is removed GetKey([]byte) []byte - // TryGetStorage returns the value for key stored in the trie. The value bytes + // GetStorage returns the value for key stored in the trie. The value bytes // must not be modified by the caller. If a node was not found in the database, // a trie.MissingNodeError is returned. - TryGetStorage(addr common.Address, key []byte) ([]byte, error) + GetStorage(addr common.Address, key []byte) ([]byte, error) - // TryGetAccount abstracts an account read from the trie. It retrieves the + // GetAccount abstracts an account read from the trie. It retrieves the // account blob from the trie with provided account address and decodes it // with associated decoding algorithm. If the specified account is not in // the trie, nil will be returned. If the trie is corrupted(e.g. some nodes // are missing or the account blob is incorrect for decoding), an error will // be returned. - TryGetAccount(address common.Address) (*types.StateAccount, error) + GetAccount(address common.Address) (*types.StateAccount, error) - // TryUpdateStorage associates key with value in the trie. If value has length zero, + // UpdateStorage associates key with value in the trie. If value has length zero, // any existing value is deleted from the trie. The value bytes must not be modified // by the caller while they are stored in the trie. If a node was not found in the // database, a trie.MissingNodeError is returned. - TryUpdateStorage(addr common.Address, key, value []byte) error + UpdateStorage(addr common.Address, key, value []byte) error - // TryUpdateAccount abstracts an account write to the trie. It encodes the + // UpdateAccount abstracts an account write to the trie. It encodes the // provided account object with associated algorithm and then updates it // in the trie with provided address. - TryUpdateAccount(address common.Address, account *types.StateAccount) error + UpdateAccount(address common.Address, account *types.StateAccount) error - // TryDeleteStorage removes any existing value for key from the trie. If a node + // DeleteStorage removes any existing value for key from the trie. If a node // was not found in the database, a trie.MissingNodeError is returned. - TryDeleteStorage(addr common.Address, key []byte) error + DeleteStorage(addr common.Address, key []byte) error - // TryDeleteAccount abstracts an account deletion from the trie. - TryDeleteAccount(address common.Address) error + // DeleteAccount abstracts an account deletion from the trie. + DeleteAccount(address common.Address) error // Hash returns the root hash of the trie. It does not write to the database and // can be used even if the trie doesn't have one. diff --git a/core/state/state_object.go b/core/state/state_object.go index 936f6dae2..95fe6f52f 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -201,7 +201,7 @@ func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Has s.db.setError(err) return common.Hash{} } - enc, err = tr.TryGetStorage(s.address, key.Bytes()) + enc, err = tr.GetStorage(s.address, key.Bytes()) if metrics.EnabledExpensive { s.db.StorageReads += time.Since(start) } @@ -294,7 +294,7 @@ func (s *stateObject) updateTrie(db Database) (Trie, error) { var v []byte if (value == common.Hash{}) { - if err := tr.TryDeleteStorage(s.address, key[:]); err != nil { + if err := tr.DeleteStorage(s.address, key[:]); err != nil { s.db.setError(err) return nil, err } @@ -302,7 +302,7 @@ func (s *stateObject) updateTrie(db Database) (Trie, error) { } else { // Encoding []byte cannot fail, ok to ignore the error. v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:])) - if err := tr.TryUpdateStorage(s.address, key[:], v); err != nil { + if err := tr.UpdateStorage(s.address, key[:], v); err != nil { s.db.setError(err) return nil, err } diff --git a/core/state/statedb.go b/core/state/statedb.go index 38583f51d..b89f4bfd8 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -512,7 +512,7 @@ func (s *StateDB) updateStateObject(obj *stateObject) { } // Encode the account and update the account trie addr := obj.Address() - if err := s.trie.TryUpdateAccount(addr, &obj.data); err != nil { + if err := s.trie.UpdateAccount(addr, &obj.data); err != nil { s.setError(fmt.Errorf("updateStateObject (%x) error: %v", addr[:], err)) } @@ -533,7 +533,7 @@ func (s *StateDB) deleteStateObject(obj *stateObject) { } // Delete the account from the trie addr := obj.Address() - if err := s.trie.TryDeleteAccount(addr); err != nil { + if err := s.trie.DeleteAccount(addr); err != nil { s.setError(fmt.Errorf("deleteStateObject (%x) error: %v", addr[:], err)) } } @@ -587,7 +587,7 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { if data == nil { start := time.Now() var err error - data, err = s.trie.TryGetAccount(addr) + data, err = s.trie.GetAccount(addr) if metrics.EnabledExpensive { s.AccountReads += time.Since(start) } diff --git a/core/state/trie_prefetcher.go b/core/state/trie_prefetcher.go index edd370fbd..844f72fc1 100644 --- a/core/state/trie_prefetcher.go +++ b/core/state/trie_prefetcher.go @@ -339,9 +339,9 @@ func (sf *subfetcher) loop() { sf.dups++ } else { if len(task) == common.AddressLength { - sf.trie.TryGetAccount(common.BytesToAddress(task)) + sf.trie.GetAccount(common.BytesToAddress(task)) } else { - sf.trie.TryGetStorage(sf.addr, task) + sf.trie.GetStorage(sf.addr, task) } sf.seen[string(task)] = struct{}{} } diff --git a/eth/protocols/snap/handler.go b/eth/protocols/snap/handler.go index d7c940044..55781ac54 100644 --- a/eth/protocols/snap/handler.go +++ b/eth/protocols/snap/handler.go @@ -418,7 +418,7 @@ func ServiceGetStorageRangesQuery(chain *core.BlockChain, req *GetStorageRangesP if err != nil { return nil, nil } - acc, err := accTrie.TryGetAccountByHash(account) + acc, err := accTrie.GetAccountByHash(account) if err != nil || acc == nil { return nil, nil } @@ -510,7 +510,7 @@ func ServiceGetTrieNodesQuery(chain *core.BlockChain, req *GetTrieNodesPacket, s case 1: // If we're only retrieving an account trie node, fetch it directly - blob, resolved, err := accTrie.TryGetNode(pathset[0]) + blob, resolved, err := accTrie.GetNode(pathset[0]) loads += resolved // always account database reads, even for failures if err != nil { break @@ -524,7 +524,7 @@ func ServiceGetTrieNodesQuery(chain *core.BlockChain, req *GetTrieNodesPacket, s if snap == nil { // We don't have the requested state snapshotted yet (or it is stale), // but can look up the account via the trie instead. - account, err := accTrie.TryGetAccountByHash(common.BytesToHash(pathset[0])) + account, err := accTrie.GetAccountByHash(common.BytesToHash(pathset[0])) loads += 8 // We don't know the exact cost of lookup, this is an estimate if err != nil || account == nil { break @@ -545,7 +545,7 @@ func ServiceGetTrieNodesQuery(chain *core.BlockChain, req *GetTrieNodesPacket, s break } for _, path := range pathset[1:] { - blob, resolved, err := stTrie.TryGetNode(path) + blob, resolved, err := stTrie.GetNode(path) loads += resolved // always account database reads, even for failures if err != nil { break diff --git a/light/trie.go b/light/trie.go index 0f05a8000..2f8f71c61 100644 --- a/light/trie.go +++ b/light/trie.go @@ -105,7 +105,7 @@ type odrTrie struct { trie *trie.Trie } -func (t *odrTrie) TryGetStorage(_ common.Address, key []byte) ([]byte, error) { +func (t *odrTrie) GetStorage(_ common.Address, key []byte) ([]byte, error) { key = crypto.Keccak256(key) var res []byte err := t.do(key, func() (err error) { @@ -115,7 +115,7 @@ func (t *odrTrie) TryGetStorage(_ common.Address, key []byte) ([]byte, error) { return res, err } -func (t *odrTrie) TryGetAccount(address common.Address) (*types.StateAccount, error) { +func (t *odrTrie) GetAccount(address common.Address) (*types.StateAccount, error) { var res types.StateAccount key := crypto.Keccak256(address.Bytes()) err := t.do(key, func() (err error) { @@ -131,7 +131,7 @@ func (t *odrTrie) TryGetAccount(address common.Address) (*types.StateAccount, er return &res, err } -func (t *odrTrie) TryUpdateAccount(address common.Address, acc *types.StateAccount) error { +func (t *odrTrie) UpdateAccount(address common.Address, acc *types.StateAccount) error { key := crypto.Keccak256(address.Bytes()) value, err := rlp.EncodeToBytes(acc) if err != nil { @@ -142,14 +142,14 @@ func (t *odrTrie) TryUpdateAccount(address common.Address, acc *types.StateAccou }) } -func (t *odrTrie) TryUpdateStorage(_ common.Address, key, value []byte) error { +func (t *odrTrie) UpdateStorage(_ common.Address, key, value []byte) error { key = crypto.Keccak256(key) return t.do(key, func() error { return t.trie.TryUpdate(key, value) }) } -func (t *odrTrie) TryDeleteStorage(_ common.Address, key []byte) error { +func (t *odrTrie) DeleteStorage(_ common.Address, key []byte) error { key = crypto.Keccak256(key) return t.do(key, func() error { return t.trie.TryDelete(key) @@ -157,7 +157,7 @@ func (t *odrTrie) TryDeleteStorage(_ common.Address, key []byte) error { } // TryDeleteAccount abstracts an account deletion from the trie. -func (t *odrTrie) TryDeleteAccount(address common.Address) error { +func (t *odrTrie) DeleteAccount(address common.Address) error { key := crypto.Keccak256(address.Bytes()) return t.do(key, func() error { return t.trie.TryDelete(key) diff --git a/trie/secure_trie.go b/trie/secure_trie.go index 01e004d02..b25bf758c 100644 --- a/trie/secure_trie.go +++ b/trie/secure_trie.go @@ -75,25 +75,25 @@ func NewStateTrie(id *ID, db *Database) (*StateTrie, error) { // Get returns the value for key stored in the trie. // The value bytes must not be modified by the caller. func (t *StateTrie) Get(key []byte) []byte { - res, err := t.TryGetStorage(common.Address{}, key) + res, err := t.GetStorage(common.Address{}, key) if err != nil { log.Error("Unhandled trie error in StateTrie.Get", "err", err) } return res } -// TryGet returns the value for key stored in the trie. -// The value bytes must not be modified by the caller. -// If the specified node is not in the trie, nil will be returned. +// GetStorage attempts to retrieve a storage slot with provided account address +// and slot key. The value bytes must not be modified by the caller. +// If the specified storage slot is not in the trie, nil will be returned. // If a trie node is not found in the database, a MissingNodeError is returned. -func (t *StateTrie) TryGetStorage(_ common.Address, key []byte) ([]byte, error) { +func (t *StateTrie) GetStorage(_ common.Address, key []byte) ([]byte, error) { return t.trie.TryGet(t.hashKey(key)) } -// TryGetAccount attempts to retrieve an account with provided account address. +// GetAccount attempts to retrieve an account with provided account address. // If the specified account is not in the trie, nil will be returned. // If a trie node is not found in the database, a MissingNodeError is returned. -func (t *StateTrie) TryGetAccount(address common.Address) (*types.StateAccount, error) { +func (t *StateTrie) GetAccount(address common.Address) (*types.StateAccount, error) { res, err := t.trie.TryGet(t.hashKey(address.Bytes())) if res == nil || err != nil { return nil, err @@ -103,10 +103,10 @@ func (t *StateTrie) TryGetAccount(address common.Address) (*types.StateAccount, return ret, err } -// TryGetAccountByHash does the same thing as TryGetAccount, however -// it expects an account hash that is the hash of address. This constitutes an -// abstraction leak, since the client code needs to know the key format. -func (t *StateTrie) TryGetAccountByHash(addrHash common.Hash) (*types.StateAccount, error) { +// GetAccountByHash does the same thing as GetAccount, however it expects an +// account hash that is the hash of address. This constitutes an abstraction +// leak, since the client code needs to know the key format. +func (t *StateTrie) GetAccountByHash(addrHash common.Hash) (*types.StateAccount, error) { res, err := t.trie.TryGet(addrHash.Bytes()) if res == nil || err != nil { return nil, err @@ -116,11 +116,11 @@ func (t *StateTrie) TryGetAccountByHash(addrHash common.Hash) (*types.StateAccou return ret, err } -// TryGetNode attempts to retrieve a trie node by compact-encoded path. It is not +// GetNode attempts to retrieve a trie node by compact-encoded path. It is not // possible to use keybyte-encoding as the path might contain odd nibbles. // If the specified trie node is not in the trie, nil will be returned. // If a trie node is not found in the database, a MissingNodeError is returned. -func (t *StateTrie) TryGetNode(path []byte) ([]byte, int, error) { +func (t *StateTrie) GetNode(path []byte) ([]byte, int, error) { return t.trie.TryGetNode(path) } @@ -131,12 +131,12 @@ func (t *StateTrie) TryGetNode(path []byte) ([]byte, int, error) { // The value bytes must not be modified by the caller while they are // stored in the trie. func (t *StateTrie) Update(key, value []byte) { - if err := t.TryUpdateStorage(common.Address{}, key, value); err != nil { + if err := t.UpdateStorage(common.Address{}, key, value); err != nil { log.Error("Unhandled trie error in StateTrie.Update", "err", err) } } -// TryUpdate associates key with value in the trie. Subsequent calls to +// UpdateStorage associates key with value in the trie. Subsequent calls to // Get will return value. If value has length zero, any existing value // is deleted from the trie and calls to Get will return nil. // @@ -144,7 +144,7 @@ func (t *StateTrie) Update(key, value []byte) { // stored in the trie. // // If a node is not found in the database, a MissingNodeError is returned. -func (t *StateTrie) TryUpdateStorage(_ common.Address, key, value []byte) error { +func (t *StateTrie) UpdateStorage(_ common.Address, key, value []byte) error { hk := t.hashKey(key) err := t.trie.TryUpdate(hk, value) if err != nil { @@ -154,9 +154,8 @@ func (t *StateTrie) TryUpdateStorage(_ common.Address, key, value []byte) error return nil } -// TryUpdateAccount account will abstract the write of an account to the -// secure trie. -func (t *StateTrie) TryUpdateAccount(address common.Address, acc *types.StateAccount) error { +// UpdateAccount will abstract the write of an account to the secure trie. +func (t *StateTrie) UpdateAccount(address common.Address, acc *types.StateAccount) error { hk := t.hashKey(address.Bytes()) data, err := rlp.EncodeToBytes(acc) if err != nil { @@ -171,22 +170,22 @@ func (t *StateTrie) TryUpdateAccount(address common.Address, acc *types.StateAcc // Delete removes any existing value for key from the trie. func (t *StateTrie) Delete(key []byte) { - if err := t.TryDeleteStorage(common.Address{}, key); err != nil { + if err := t.DeleteStorage(common.Address{}, key); err != nil { log.Error("Unhandled trie error in StateTrie.Delete", "err", err) } } -// TryDelete removes any existing value for key from the trie. +// DeleteStorage removes any existing storage slot from the trie. // If the specified trie node is not in the trie, nothing will be changed. // If a node is not found in the database, a MissingNodeError is returned. -func (t *StateTrie) TryDeleteStorage(_ common.Address, key []byte) error { +func (t *StateTrie) DeleteStorage(_ common.Address, key []byte) error { hk := t.hashKey(key) delete(t.getSecKeyCache(), string(hk)) return t.trie.TryDelete(hk) } -// TryDeleteAccount abstracts an account deletion from the trie. -func (t *StateTrie) TryDeleteAccount(address common.Address) error { +// DeleteAccount abstracts an account deletion from the trie. +func (t *StateTrie) DeleteAccount(address common.Address) error { hk := t.hashKey(address.Bytes()) delete(t.getSecKeyCache(), string(hk)) return t.trie.TryDelete(hk) diff --git a/trie/sync_test.go b/trie/sync_test.go index 8fec37833..f404dc1cd 100644 --- a/trie/sync_test.go +++ b/trie/sync_test.go @@ -154,7 +154,7 @@ func testIterativeSync(t *testing.T, count int, bypath bool) { } } else { for i, element := range elements { - data, _, err := srcTrie.TryGetNode(element.syncPath[len(element.syncPath)-1]) + data, _, err := srcTrie.GetNode(element.syncPath[len(element.syncPath)-1]) if err != nil { t.Fatalf("failed to retrieve node data for path %x: %v", element.path, err) }