From 6c149fd4ad063f7c24d726a73bc0546badd1bc73 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Tue, 3 Jan 2023 21:41:40 +0800 Subject: [PATCH] core, eth, trie, light: clean up trie interface (#26388) * all: cleanup trie interface * eth, trie: address comments --- core/state/database.go | 17 ++++++++++++----- core/state/statedb.go | 6 +++--- core/state/trie_prefetcher.go | 6 +----- eth/protocols/snap/handler.go | 4 ++-- eth/protocols/snap/sync.go | 2 +- light/trie.go | 12 ++++++------ trie/secure_trie.go | 26 +++++++++++++------------- 7 files changed, 38 insertions(+), 35 deletions(-) diff --git a/core/state/database.go b/core/state/database.go index fbd6d2883..7a719f0d9 100644 --- a/core/state/database.go +++ b/core/state/database.go @@ -73,8 +73,13 @@ type Trie interface { // trie.MissingNodeError is returned. TryGet(key []byte) ([]byte, error) - // TryGetAccount abstract an account read from the trie. - TryGetAccount(key []byte) (*types.StateAccount, error) + // TryGetAccount 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) // TryUpdate 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 @@ -82,15 +87,17 @@ type Trie interface { // database, a trie.MissingNodeError is returned. TryUpdate(key, value []byte) error - // TryUpdateAccount abstract an account write to the trie. - TryUpdateAccount(key []byte, account *types.StateAccount) error + // TryUpdateAccount 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 // TryDelete removes any existing value for key from the trie. If a node was not // found in the database, a trie.MissingNodeError is returned. TryDelete(key []byte) error // TryDeleteAccount abstracts an account deletion from the trie. - TryDeleteAccount(key []byte) error + TryDeleteAccount(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/statedb.go b/core/state/statedb.go index 8d0aacf58..fb2459ffc 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -510,7 +510,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.TryUpdateAccount(addr, &obj.data); err != nil { s.setError(fmt.Errorf("updateStateObject (%x) error: %v", addr[:], err)) } @@ -531,7 +531,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.TryDeleteAccount(addr); err != nil { s.setError(fmt.Errorf("deleteStateObject (%x) error: %v", addr[:], err)) } } @@ -585,7 +585,7 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { if data == nil { start := time.Now() var err error - data, err = s.trie.TryGetAccount(addr.Bytes()) + data, err = s.trie.TryGetAccount(addr) if metrics.EnabledExpensive { s.AccountReads += time.Since(start) } diff --git a/core/state/trie_prefetcher.go b/core/state/trie_prefetcher.go index 2e16f587c..f142c86bb 100644 --- a/core/state/trie_prefetcher.go +++ b/core/state/trie_prefetcher.go @@ -336,11 +336,7 @@ func (sf *subfetcher) loop() { if _, ok := sf.seen[string(task)]; ok { sf.dups++ } else { - if len(task) == len(common.Address{}) { - sf.trie.TryGetAccount(task) - } else { - sf.trie.TryGet(task) - } + sf.trie.TryGet(task) sf.seen[string(task)] = struct{}{} } } diff --git a/eth/protocols/snap/handler.go b/eth/protocols/snap/handler.go index 60f9898f4..6941d522a 100644 --- a/eth/protocols/snap/handler.go +++ b/eth/protocols/snap/handler.go @@ -417,7 +417,7 @@ func ServiceGetStorageRangesQuery(chain *core.BlockChain, req *GetStorageRangesP if err != nil { return nil, nil } - acc, err := accTrie.TryGetAccountWithPreHashedKey(account[:]) + acc, err := accTrie.TryGetAccountByHash(account) if err != nil || acc == nil { return nil, nil } @@ -523,7 +523,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.TryGetAccountWithPreHashedKey(pathset[0]) + account, err := accTrie.TryGetAccountByHash(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 diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index a9e35f971..1193af676 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -3083,7 +3083,7 @@ func (t *healRequestSort) Swap(i, j int) { func (t *healRequestSort) Merge() []TrieNodePathSet { var result []TrieNodePathSet for _, path := range t.syncPaths { - pathset := TrieNodePathSet([][]byte(path)) + pathset := TrieNodePathSet(path) if len(path) == 1 { // It's an account reference. result = append(result, pathset) diff --git a/light/trie.go b/light/trie.go index 0092eee13..c7c08331c 100644 --- a/light/trie.go +++ b/light/trie.go @@ -115,9 +115,9 @@ func (t *odrTrie) TryGet(key []byte) ([]byte, error) { return res, err } -func (t *odrTrie) TryGetAccount(key []byte) (*types.StateAccount, error) { - key = crypto.Keccak256(key) +func (t *odrTrie) TryGetAccount(address common.Address) (*types.StateAccount, error) { var res types.StateAccount + key := crypto.Keccak256(address.Bytes()) err := t.do(key, func() (err error) { value, err := t.trie.TryGet(key) if err != nil { @@ -131,8 +131,8 @@ func (t *odrTrie) TryGetAccount(key []byte) (*types.StateAccount, error) { return &res, err } -func (t *odrTrie) TryUpdateAccount(key []byte, acc *types.StateAccount) error { - key = crypto.Keccak256(key) +func (t *odrTrie) TryUpdateAccount(address common.Address, acc *types.StateAccount) error { + key := crypto.Keccak256(address.Bytes()) value, err := rlp.EncodeToBytes(acc) if err != nil { return fmt.Errorf("decoding error in account update: %w", err) @@ -157,8 +157,8 @@ func (t *odrTrie) TryDelete(key []byte) error { } // TryDeleteAccount abstracts an account deletion from the trie. -func (t *odrTrie) TryDeleteAccount(key []byte) error { - key = crypto.Keccak256(key) +func (t *odrTrie) TryDeleteAccount(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 96faab158..158a69cc2 100644 --- a/trie/secure_trie.go +++ b/trie/secure_trie.go @@ -90,11 +90,11 @@ func (t *StateTrie) TryGet(key []byte) ([]byte, error) { return t.trie.TryGet(t.hashKey(key)) } -// TryGetAccount attempts to retrieve an account with provided trie path. +// TryGetAccount 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(key []byte) (*types.StateAccount, error) { - res, err := t.trie.TryGet(t.hashKey(key)) +func (t *StateTrie) TryGetAccount(address common.Address) (*types.StateAccount, error) { + res, err := t.trie.TryGet(t.hashKey(address.Bytes())) if res == nil || err != nil { return nil, err } @@ -103,11 +103,11 @@ func (t *StateTrie) TryGetAccount(key []byte) (*types.StateAccount, error) { return ret, err } -// TryGetAccountWithPreHashedKey does the same thing as TryGetAccount, however -// it expects a key that is already hashed. This constitutes an abstraction leak, -// since the client code needs to know the key format. -func (t *StateTrie) TryGetAccountWithPreHashedKey(key []byte) (*types.StateAccount, error) { - res, err := t.trie.TryGet(key) +// 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) { + res, err := t.trie.TryGet(addrHash.Bytes()) if res == nil || err != nil { return nil, err } @@ -156,8 +156,8 @@ func (t *StateTrie) TryUpdate(key, value []byte) error { // TryUpdateAccount account will abstract the write of an account to the // secure trie. -func (t *StateTrie) TryUpdateAccount(key []byte, acc *types.StateAccount) error { - hk := t.hashKey(key) +func (t *StateTrie) TryUpdateAccount(address common.Address, acc *types.StateAccount) error { + hk := t.hashKey(address.Bytes()) data, err := rlp.EncodeToBytes(acc) if err != nil { return err @@ -165,7 +165,7 @@ func (t *StateTrie) TryUpdateAccount(key []byte, acc *types.StateAccount) error if err := t.trie.TryUpdate(hk, data); err != nil { return err } - t.getSecKeyCache()[string(hk)] = common.CopyBytes(key) + t.getSecKeyCache()[string(hk)] = address.Bytes() return nil } @@ -186,8 +186,8 @@ func (t *StateTrie) TryDelete(key []byte) error { } // TryDeleteAccount abstracts an account deletion from the trie. -func (t *StateTrie) TryDeleteAccount(key []byte) error { - hk := t.hashKey(key) +func (t *StateTrie) TryDeleteAccount(address common.Address) error { + hk := t.hashKey(address.Bytes()) delete(t.getSecKeyCache(), string(hk)) return t.trie.TryDelete(hk) }