Skip to content

refactor(core/rawdb): improve db APIs for accessing trie nodes #29362 #2313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gzliudan wants to merge 1 commit into
base: dev-upgrade
Choose a base branch
from
+55 −84
Open

refactor(core/rawdb): improve db APIs for accessing trie nodes #29362 #2313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 55 additions & 74 deletions core/rawdb/accessors_trie.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/XinFinOrg/XDPoSChain/common"
"github.com/XinFinOrg/XDPoSChain/crypto"
"github.com/XinFinOrg/XDPoSChain/crypto/keccak"
"github.com/XinFinOrg/XDPoSChain/ethdb"
"github.com/XinFinOrg/XDPoSChain/log"
)
Expand All @@ -46,45 +45,39 @@ const HashScheme = "hashScheme"
// on extra state diffs to survive deep reorg.
const PathScheme = "pathScheme"

// nodeHasher used to derive the hash of trie node.
type nodeHasher struct{ sha crypto.KeccakState }
// hasher is used to compute the sha256 hash of the provided data.
type hasher struct{ sha crypto.KeccakState }

var hasherPool = sync.Pool{
New: func() interface{} { return &nodeHasher{sha: keccak.NewLegacyKeccak256().(crypto.KeccakState)} },
New: func() interface{} { return &hasher{sha: crypto.NewKeccakState()} },
Comment on lines +48 to +52
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says this helper computes a sha256 hash, but the implementation uses Keccak (via crypto.NewKeccakState/crypto.HashData). Please update the comment (and any related naming) to avoid misleading future maintainers about the hashing algorithm used here.

Copilot uses AI. Check for mistakes.
}

func newNodeHasher() *nodeHasher { return hasherPool.Get().(*nodeHasher) }
func returnHasherToPool(h *nodeHasher) { hasherPool.Put(h) }
func newHasher() *hasher {
return hasherPool.Get().(*hasher)
}

func (h *nodeHasher) hashData(data []byte) (n common.Hash) {
h.sha.Reset()
h.sha.Write(data)
h.sha.Read(n[:])
return n
func (h *hasher) hash(data []byte) common.Hash {
return crypto.HashData(h.sha, data)
}

// ReadAccountTrieNode retrieves the account trie node and the associated node
// hash with the specified node path.
func ReadAccountTrieNode(db ethdb.KeyValueReader, path []byte) ([]byte, common.Hash) {
data, err := db.Get(accountTrieNodeKey(path))
if err != nil {
return nil, common.Hash{}
}
hasher := newNodeHasher()
defer returnHasherToPool(hasher)
return data, hasher.hashData(data)
func (h *hasher) release() {
hasherPool.Put(h)
}

// ReadAccountTrieNode retrieves the account trie node with the specified node path.
func ReadAccountTrieNode(db ethdb.KeyValueReader, path []byte) []byte {
data, _ := db.Get(accountTrieNodeKey(path))
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadAccountTrieNode discards the db.Get error and returns the raw data unconditionally. Downstream code uses len(data)==0 to infer absence, which changes semantics if the key exists with an empty value and also hides non-NotFound read errors. Consider returning (data, error)/(data, ok) or ensuring callers check the Get error rather than using len(data) as the existence check.

Suggested change
data, _ := db.Get(accountTrieNodeKey(path))
key := accountTrieNodeKey(path)
has, err := db.Has(key)
if err != nil {
log.Crit("Failed to check account trie node", "err", err)
}
if !has {
return nil
}
data, err := db.Get(key)
if err != nil {
log.Crit("Failed to read account trie node", "err", err)
}

Copilot uses AI. Check for mistakes.
return data
}

// HasAccountTrieNode checks the account trie node presence with the specified
// node path and the associated node hash.
func HasAccountTrieNode(db ethdb.KeyValueReader, path []byte, hash common.Hash) bool {
data, err := db.Get(accountTrieNodeKey(path))
// HasAccountTrieNode checks the presence of the account trie node with the
// specified node path, regardless of the node hash.
func HasAccountTrieNode(db ethdb.KeyValueReader, path []byte) bool {
has, err := db.Has(accountTrieNodeKey(path))
if err != nil {
return false
}
hasher := newNodeHasher()
defer returnHasherToPool(hasher)
return hasher.hashData(data) == hash
return has
}

// WriteAccountTrieNode writes the provided account trie node into database.
Expand All @@ -101,28 +94,20 @@ func DeleteAccountTrieNode(db ethdb.KeyValueWriter, path []byte) {
}
}

// ReadStorageTrieNode retrieves the storage trie node and the associated node
// hash with the specified node path.
func ReadStorageTrieNode(db ethdb.KeyValueReader, accountHash common.Hash, path []byte) ([]byte, common.Hash) {
data, err := db.Get(storageTrieNodeKey(accountHash, path))
if err != nil {
return nil, common.Hash{}
}
hasher := newNodeHasher()
defer returnHasherToPool(hasher)
return data, hasher.hashData(data)
// ReadStorageTrieNode retrieves the storage trie node with the specified node path.
func ReadStorageTrieNode(db ethdb.KeyValueReader, accountHash common.Hash, path []byte) []byte {
data, _ := db.Get(storageTrieNodeKey(accountHash, path))
return data
Comment on lines +97 to +100
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadStorageTrieNode discards the db.Get error and returns the raw data unconditionally. Callers later treat len(data)==0 as "missing", which is not equivalent to "key not found" and can also mask non-NotFound DB read failures. Consider returning (data, error)/(data, ok) or ensure callers check the Get error.

Copilot uses AI. Check for mistakes.
}

// HasStorageTrieNode checks the storage trie node presence with the provided
// node path and the associated node hash.
func HasStorageTrieNode(db ethdb.KeyValueReader, accountHash common.Hash, path []byte, hash common.Hash) bool {
data, err := db.Get(storageTrieNodeKey(accountHash, path))
// HasStorageTrieNode checks the presence of the storage trie node with the
// specified account hash and node path, regardless of the node hash.
func HasStorageTrieNode(db ethdb.KeyValueReader, accountHash common.Hash, path []byte) bool {
has, err := db.Has(storageTrieNodeKey(accountHash, path))
if err != nil {
return false
}
hasher := newNodeHasher()
defer returnHasherToPool(hasher)
return hasher.hashData(data) == hash
return has
}

// WriteStorageTrieNode writes the provided storage trie node into database.
Expand Down Expand Up @@ -176,54 +161,54 @@ func HasTrieNode(db ethdb.KeyValueReader, owner common.Hash, path []byte, hash c
case HashScheme:
return HasLegacyTrieNode(db, hash)
case PathScheme:
var blob []byte
if owner == (common.Hash{}) {
return HasAccountTrieNode(db, path, hash)
blob = ReadAccountTrieNode(db, path)
} else {
blob = ReadStorageTrieNode(db, owner, path)
}
return HasStorageTrieNode(db, owner, path, hash)
if len(blob) == 0 {
return false
}
h := newHasher()
defer h.release()
return h.hash(blob) == hash // exists but not match
Comment on lines +164 to +175
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PathScheme branch, existence is inferred via len(blob)==0 after calling Read*TrieNode (which currently ignores db.Get errors). This makes "missing" depend on the stored value length (and treats a present-but-empty value as absent) and also prevents distinguishing genuine DB read errors from NotFound. Prefer checking the db.Get error (or a boolean ok) to decide existence, then hash the returned bytes (even if empty).

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This inline comment is misleading: the function returns whether the computed hash matches the provided hash; it doesn't specifically indicate the "exists but not match" case. Please reword the comment (or remove it) so it describes the boolean result accurately.

Suggested change
return h.hash(blob) == hash // exists but not match
return h.hash(blob) == hash // whether the stored node matches the provided hash

Copilot uses AI. Check for mistakes.
default:
panic(fmt.Sprintf("Unknown scheme %v", scheme))
}
}

// ReadTrieNode retrieves the trie node from database with the provided node info
// and associated node hash.
// hashScheme-based lookup requires the following:
// - hash
//
// pathScheme-based lookup requires the following:
// - owner
// - path
func ReadTrieNode(db ethdb.KeyValueReader, owner common.Hash, path []byte, hash common.Hash, scheme string) []byte {
switch scheme {
case HashScheme:
return ReadLegacyTrieNode(db, hash)
case PathScheme:
var (
blob []byte
nHash common.Hash
)
var blob []byte
if owner == (common.Hash{}) {
blob, nHash = ReadAccountTrieNode(db, path)
blob = ReadAccountTrieNode(db, path)
} else {
blob, nHash = ReadStorageTrieNode(db, owner, path)
blob = ReadStorageTrieNode(db, owner, path)
}
if nHash != hash {
if len(blob) == 0 {
return nil
}
h := newHasher()
defer h.release()
if h.hash(blob) != hash {
return nil // exists but not match
}
return blob
default:
panic(fmt.Sprintf("Unknown scheme %v", scheme))
}
}

// WriteTrieNode writes the trie node into database with the provided node info
// and associated node hash.
// hashScheme-based lookup requires the following:
// - hash
// WriteTrieNode writes the trie node into database with the provided node info.
//
// pathScheme-based lookup requires the following:
// - owner
// - path
// hash-scheme requires the node hash as the identifier.
// path-scheme requires the node owner and path as the identifier.
func WriteTrieNode(db ethdb.KeyValueWriter, owner common.Hash, path []byte, hash common.Hash, node []byte, scheme string) {
switch scheme {
case HashScheme:
Expand All @@ -239,14 +224,10 @@ func WriteTrieNode(db ethdb.KeyValueWriter, owner common.Hash, path []byte, hash
}
}

// DeleteTrieNode deletes the trie node from database with the provided node info
// and associated node hash.
// hashScheme-based lookup requires the following:
// - hash
// DeleteTrieNode deletes the trie node from database with the provided node info.
//
// pathScheme-based lookup requires the following:
// - owner
// - path
// hash-scheme requires the node hash as the identifier.
// path-scheme requires the node owner and path as the identifier.
func DeleteTrieNode(db ethdb.KeyValueWriter, owner common.Hash, path []byte, hash common.Hash, scheme string) {
switch scheme {
case HashScheme:
Expand Down
10 changes: 0 additions & 10 deletions trie/trienode/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,6 @@ func (set *NodeSet) Size() (int, int) {
return set.updates, set.deletes
}

// Hashes returns the hashes of all updated nodes. TODO(rjl493456442) how can
// we get rid of it?
func (set *NodeSet) Hashes() []common.Hash {
var ret []common.Hash
for _, node := range set.Nodes {
ret = append(ret, node.Hash)
}
return ret
}

// Summary returns a string-representation of the NodeSet.
func (set *NodeSet) Summary() string {
var out = new(strings.Builder)
Expand Down
Loading