refactor(pkg/trie/triedb): introduce HashDB interface and integrate into TrieDB with added iterators
#4315
+1,731
−518
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timwu20
changed the title
HashDB interface and integrate into TrieDB, with added iteratorsHashDB interface and integrate into TrieDB and added iterators
timwu20
changed the title
HashDB interface and integrate into TrieDB and added iteratorsHashDB interface and integrate into TrieDB with added iterators
timwu20
force-pushed
the
tim/triedb-iterators-hashdb
branch
3 times, most recently
from
183d6d7
to
a81ff1b
Compare
timwu20
requested review from
dimartiro,
EclesioMeloJunior,
jimjbrettj and
P1sar
as code owners
timwu20
force-pushed
the
tim/triedb-iterators-hashdb
branch
from
9877caa
to
a81ff1b
Compare
timwu20
changed the title
HashDB interface and integrate into TrieDB with added iteratorsHashDB interface and integrate into TrieDB with added iterators
timwu20
force-pushed
the
tim/triedb-iterators-hashdb
branch
from
e2449c1
to
95f5c35
Compare
timwu20
changed the base branch from
refactor/client-db
to
tim/refactor-triedb-caching-base
timwu20
changed the base branch from
tim/refactor-triedb-caching-base
to
tim/refactor-client-db-base
timwu20
changed the base branch from
tim/refactor-client-db-base
to
refactor/client-db
…B, update tests, add required iterators, bug fixes.
timwu20
force-pushed
the
tim/triedb-iterators-hashdb
branch
from
95f5c35
to
a907f8a
Compare
haikoschol
approved these changes
Nov 8, 2024
| // are referenced by the key to look them up in the trie. As multiple different tries can have | ||
| // different values under the same key, it is up to the cache implementation to ensure that the | ||
| // different values under the same key, it up to the cache implementation to ensure that the |
There was a problem hiding this comment.
Wasn't the "is" here correct?
| @@ -358,15 +359,16 @@ type inspectResult struct { | |||
| changed bool | |||
| } | |||
|
|
|||
| // inspect inspects the given node `stored` and calls the `inspector` function | |||
| // inspect inspects the given node stored and calls the inspector function | |||
| // then returns the new node and a boolean indicating if the node has changed | |||
| func (t *TrieDB[H, Hasher]) inspect( | |||
| stored StoredNode, | |||
| key *nibbles.Nibbles, | |||
| inspector func(Node, *nibbles.Nibbles) (action, error), | |||
| ) (*inspectResult, error) { | |||
| // shallow copy since key will change offset through inspector | |||
There was a problem hiding this comment.
Suggested change
| // shallow copy since key will change offset through inspector |
assuming key.Clone() does not return a shallow copy
dimartiro
reviewed
Nov 9, 2024
| @@ -108,9 +109,9 @@ func (vr newValueRef[H]) equal(other nodeValue) bool { | |||
| } | |||
|
|
|||
| func NewValue[H hash.Hash](data []byte, threshold int) nodeValue { | |||
| if len(data) >= threshold { | |||
| if len(data) > threshold { | |||
There was a problem hiding this comment.
good catch, do we have a test for this?
Co-authored-by: Haiko Schol <[email protected]>
Co-authored-by: Haiko Schol <[email protected]>
Co-authored-by: Haiko Schol <[email protected]>
Co-authored-by: Haiko Schol <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
hashdb.HashDBhashdb.HashDBis an interface introduced around a persistent store where the hash of the inserted value is used as the key. This interface is found prominently in substrate, and is used as the underlying storage oftriedb.TrieDB. The integration of this interface as the underlying storage is included with the changes.hashdb.Prefixis introduced to represent prefix partials where there could be an existence of a padded byte. This is useful to represent nibble offset for a partial key.memorydb.MemoryDBmemorydb.MemoryDBis an implementation ofHashDBwith generic type parameters for theHash,Hasher,Key, andKeyFunction. Variations ofmemorydb.MemoryDBusing different key functions where the prefix is actually used vs not used are found in the substrate primitives and are used in PR #4318. One key feature that is of note is that values are actually reference counted, and not actually removed unless an equal amount of removes to the amount of gets.triedb.TrieDBIteratorandtriedb.TrieDBKeyIteratorThese two iterator types are introduced and used in PR #4318. They are simply exported iterators that will iterate through the pairs (key, values) or just the keys in lexographic order.
triedb.TrieDB.LookupFirstDescendantThis method is required to for
TrieBackendwork in PR #4318. It will look up the first descendant node from a given key.Tests
go test -tags integration github.com/ChainSafe/gossamerIssues
contributes to #3901