Skip to content

core, eth, trie, light: clean up trie interface#26388

Merged
karalabe merged 2 commits intoethereum:masterfrom
rjl493456442:cleanup-trie-interface
Jan 3, 2023
Merged

core, eth, trie, light: clean up trie interface#26388
karalabe merged 2 commits intoethereum:masterfrom
rjl493456442:cleanup-trie-interface

Conversation

@rjl493456442
Copy link
Copy Markdown
Member

@rjl493456442 rjl493456442 commented Dec 28, 2022

This PR cleans up/improves some trie interfaces:

  • TryGetAccount, TryUpdateAccount, TryDeleteAccount accepts common.Address as the parameters, which is more intuitive to callers

Comment thread core/state/trie_prefetcher.go Outdated
@rjl493456442
Copy link
Copy Markdown
Member Author

Let's hold-on this PR for a while.

I am not sure the change for TryGetNode is really needed.

@rjl493456442 rjl493456442 force-pushed the cleanup-trie-interface branch from b5809b5 to 45743bc Compare December 29, 2022 07:49
Copy link
Copy Markdown
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM, only minor suggestion to name them ByHash instead of WithHash, since it refers to whether we look it up by hash.

Comment thread eth/protocols/snap/handler.go Outdated
// 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.TryGetAccountWithHash(common.BytesToHash(pathset[0]))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
account, err := accTrie.TryGetAccountWithHash(common.BytesToHash(pathset[0]))
account, err := accTrie.TryGetAccountByHash(common.BytesToHash(pathset[0]))

?

Comment thread eth/protocols/snap/handler.go Outdated
return nil, nil
}
acc, err := accTrie.TryGetAccountWithPreHashedKey(account[:])
acc, err := accTrie.TryGetAccountWithHash(account)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
acc, err := accTrie.TryGetAccountWithHash(account)
acc, err := accTrie.TryGetAccountByHash(account)

?

@holiman holiman added this to the 1.11.0 milestone Dec 30, 2022
@karalabe karalabe merged commit 6c149fd into ethereum:master Jan 3, 2023
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
* all: cleanup trie interface

* eth, trie: address comments
gzliudan pushed a commit to gzliudan/XDPoSChain that referenced this pull request Jun 20, 2025
* all: cleanup trie interface

* eth, trie: address comments
gzliudan pushed a commit to gzliudan/XDPoSChain that referenced this pull request Nov 17, 2025
* all: cleanup trie interface

* eth, trie: address comments
AnilChinchawale pushed a commit to XinFinOrg/XDPoSChain that referenced this pull request Nov 18, 2025
* all: cleanup trie interface

* eth, trie: address comments

Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants