Trie - async walk generator #2904

ScottyPoi · 2023-07-20T07:55:03Z

Trie -- `walkTrieIterable`

QuickStart:

from: packages/trie
run: npx vitest test/util/asyncWalk.spec.ts

observe console logs to see details of walk.
test includes walk of "Sparse" Trie and comparison to walk controller API.

Intention:

To establish more user friendly API for walking a Trie
- And do so without altering the walk controller
To enable users to walk a Sparse* Trie
- *Sparse meaning a valid merkle trie built from a proof
- "Missing" nodes in a sparse trie cause the walk controller to throw an Error, and end the walk.
- So users cannot use the walk controller to navigate sparse Tries.

Feature

This PR introduces a new approach to Trie walking, via AsyncIterable

This is a new method, independent from the existing WalkController class.

**It does not presume to replace the WalkController in any active context at this time. **
details on why in comments

Main differences from WalkController:

Iterator API is more straightforward and intuitive. User create the async iterator object, and then proceed via for...await
Sparse Trie walking
"CurrentKey" is yielded by the iterator along with each node, instead of just _nibbles
Inner and Outer OnFound functions, combined with a NodeFilter parameter enable very customizable walks.

Async Generator / Async Iterable / Async Iterator

Uses the iteration protocols described in the MDN docs here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

Function

the _walkTrie function is exported form trie/src/util/asyncWalk.ts, but also internalized as a bound method for the Trie class: trie.walkTrieIterablle

Do not fear the bind

This is not bound to an interface, just to an exported function. This is so that the new code will not add length to the already very long trie.ts file. Hyperlinks is VSCode, etc. will link to the function implementation iteself with one jump. You will not have to search for it. The class method should inherit the typedoc comments as well.

binding the function context to this allows the external function to access private class methods.
For example:

trie.hash() is a private method, that can only be used inside the class itself.
by binding the _walkTrie function to a Trie instance, _walkTrie can access this.hash()
This avoids patterns like (trie as any).hash() and other hacks

_walkTrie

_walkTrie generates the Async Iterable:

export async function* _walkTrie(
  this: Trie,
  nodeHash: Uint8Array = this.root(),
  currentKey: number[] = [],
  onFound: OnFound = async (_trieNode: TrieNode, _key: number[]) => {},
  filter: NodeFilter = async (_trieNode: TrieNode, _key: number[]) => true,
  visited: Set<string> = new Set<string>()
): AsyncIterable<{ node: TrieNode; currentKey: number[] }>

OnFound

This is a callback that will be called on every node encountered on the walk ("pre-filter").
Useful when NodeFilter is defined, as it will be called on all nodes, not only the filtered nodes that are yielded.

NodeFilter

Pass a filter function that will return true for desired nodes. The walk will actually yield only nodes that pass the filter.

NodeHash / CurrentKey / Visited

With these params left undefined, the walk will begin at the root, and later use these parameters to call itself recursively during the walk. If defined from the start, they can be used to begin the walk from another place.

Use

trie: Trie

const walk = trie.walkTrieIterable(trie.root())

for await ( const { node, currentKey } of walk )  {
  // ... do something i.e. console.log( { node, currentKey } )
}

Helpers

Included are 2 examples of helper functions that setup and execute the walk iterator for specific common uses. These are simply two of many possible wrapper functions that could be designed for common uses. I have listed a few more ideas below.

walkAllNodes
Walk the Trie from the root, and execute onFound on every node in the Trie.
This will run through the whole walk with default parameters and no filter.

trie.walkAllNodes(onFound: OnFound) => Promise<void>

walkAllValueNodes
Walk the Trie from the root, and execute onFound on all Trie nodes that store a VALUE
This will run through the whole walk from the root with a pre-set filter for VALUE_NODES ( LeafNodes and BranchNodes with non-null values )

trie.walkAllValueNodes(onFound: OnFound) => Promise<void>

additional filter-based helpers could include:

allLeafNodes
allBranchNodes
allExtensionNodes
getSubTree

possible OnFound-based helpers could include:

logNodes
copyNodes
sortNodes
garbageCollect
getAllPaths
getAllKeys

codecov · 2023-07-20T08:01:14Z

Codecov Report

Merging #2904 (4be9931) into master (191faf5) will increase coverage by 0.01%.
The diff coverage is 81.72%.

Additional details and impacted files

Flag	Coverage Δ
block	`88.66% <ø> (ø)`
blockchain	`92.58% <ø> (ø)`
client	`88.40% <ø> (+0.12%)`	⬆️
common	`98.64% <ø> (ø)`
ethash	`∅ <ø> (∅)`
evm	`69.95% <ø> (ø)`
rlp	`∅ <ø> (∅)`
statemanager	`85.16% <ø> (ø)`
trie	`89.88% <81.72%> (-0.69%)`	⬇️
tx	`95.96% <ø> (ø)`
util	`86.77% <ø> (ø)`
vm	`79.21% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

acolytec3 · 2023-07-20T20:41:33Z

Do not fear the bind

It's not fear but great dislike. If this behaves as described, I fear no longer.

acolytec3 · 2023-07-21T02:17:05Z

Just to make sure I understand how this works, this isn't changing the walk strategy, which is depth first, left to right, correct? It's more or less the same as the current walk controller, except that it's an async generator that allows the library consumer to trigger the next step, right?

acolytec3

Looks good so far. One error message I think needs fixing (see comment)

ScottyPoi · 2023-07-21T03:12:35Z

Just to make sure I understand how this works, this isn't changing the walk strategy, which is depth first, left to right, correct? It's more or less the same as the current walk controller, except that it's an async generator that allows the library consumer to trigger the next step, right?

yes exactly. same walk pattern, different API.
the walk controller API can be hard to grasp, and this should be slightly more intuitive to use.

The walk controller method is designed to throw Errors, and certain parts of the Trie and other libraries that use the Trie rely on those Errors. Those expectations run deep, so the walk controller needs to stay as it is.

This way, we can build up a more user friendly Trie walking API without touching the walk controller

ScottyPoi · 2023-07-21T03:23:44Z

Looks good so far. One error message I think needs fixing (see comment)

where is comment? it's not showing up for me

ScottyPoi · 2023-07-21T05:02:02Z

packages/trie/src/util/asyncWalk.ts

+    }
+    if (node instanceof BranchNode) {
+      for (const [nibble, childNode] of node._branches.entries()) {
+        if (childNode === null) {


- reported as untouched by coverage // if (childNode === null) { // continue // }

~~TODO:~~
conditional removed
see comment below

empty branches seem to be empty Uint8Array, not null.

why does union type for node._branches include null?

a new branch node's ._branches array is initiallly all null values:

const branch = new BranchNode() branch._branches = [null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null]

when a branch node is serialized, the empty branches are included as a single byte each, basically the RLP encoding of [].

deserialization does not convert that byte back to null, of course. it deserializes to the empty array ([]).

so while null is a possible type for the _branches array of a branch node, in practice, the walk function should not encounter the null type, and this conditional should be unneccesary.

acolytec3 · 2023-07-21T10:05:56Z

Looks good so far. One error message I think needs fixing (see comment)

where is comment? it's not showing up for me

Sorry, was commenting on the wrong PR 🤦

acolytec3

This looks fantastic as an addition. The one thing I think would be helpful would be to add some of the commentary from the PR to the docs for trie so people are aware of this option. I know we have library consumers who use trie as is and this will definitely be helpful for them and we need to surface it so they know it exists.

holgerd77 · 2023-07-21T14:42:31Z

Yeah, cool, also a big fan of such API additions to the Trie library! 🤩 Would be great if the originator of this feature request could also have a look before we merge, so this would be @TimDaub and this #2856 issue, right?

holgerd77 · 2023-07-21T17:49:18Z

Updated this via UI

ScottyPoi · 2023-07-22T08:03:26Z

Yeah, cool, also a big fan of such API additions to the Trie library! star_struck Would be great if the originator of this feature request could also have a look before we merge, so this would be @TimDaub and this #2856 issue, right?

Yep, @TimDaub -- This does not explicitly offer the feature that you suggested, but may offer a simpler approach to your use case from #2856

TimDaub · 2023-07-24T12:02:19Z

I‘ll try to take a look in an hour or so

TimDaub · 2023-07-24T13:13:48Z

packages/trie/src/util/asyncWalk.ts

+      const nextKey = [...currentKey, ...node._nibbles]
+      yield* _walkTrie.bind(this)(childNode, nextKey, onFound, filter, visited)
+    }
+  } catch (e) {


Why apply this try catch on the entire function and not log the error. Don't want to evolve myself into coding standards of ethereumjs too much but IMO a good practice is to do 1 statement per try catch and then match the statement's respective errors or allow errors to bubble up further.

Really the only potential error here is something like
"DB: Node Missing". We want the walk to ignore these errors and keep going, and I would prefer it happened silently.

I don't want the error to throw, because that stops the walk from happening. I want it to simply continue on.

The reason here is that we want to walk through a sparse trie -- a Trie that has been built from a proof.
A walk through a trie like this will find a hash on the branch of a proof node for a node that is not in the database,

said node was not part of the proof, but it's hash was part of the RLP encoding of one of the proof nodes. So the walk will look for that hash in the DB, and the DB will throw an error. Which in turn would stop the walk prematurely, even though it is a valid Trie with nodes remaining.

TimDaub · 2023-07-24T13:15:42Z

packages/trie/src/trie.ts

+   */
+  async walkAllValueNodes(onFound: OnFound): Promise<void> {
+    for await (const { node, currentKey } of this.walkTrieIterable(
+      this.root(),


Is it possible to make this user-definable so that e.g. we could start pass in a branch root on a lower level to skip a bunch of upper-level walking?

Yes -- this is one example of a method that can be built using walkTrieIterable

The walk iterator can be set up to start from any node. However note that the currentKey value returned for each node will be rooted from the starting node. so to string together a node's "full key", you would still need the nibbles from the path to the starting node somehow.

TimDaub · 2023-07-24T13:18:55Z

packages/trie/src/util/asyncWalk.ts

+    if (node === undefined || visited.has(toHex(this.hash(node!.serialize())))) {
+      return
+    }
+    visited.add(toHex(this.hash(node!.serialize())))


From working with trie.ts, I think it'd be awesome if there were a canonical representation of the trie's and node's hash values and if trie.ts would provide this serialization of values directly instead of having "users" implement their own thing. (not necessarily actionable in this PR specifically)

TimDaub · 2023-07-24T13:25:13Z

Hey, so I left some high-level feedback that may not be super actionable right now. It's more what came to mind when I was reading the code. Thank you so much for improving the library!

This module is interesting to me, especially providing a reliable way to get back keys per node and walkAllValueNodes. I could imagine replacing (and hence) simplifying the current logic of my reconciliation algorithm with it!

I won't be able to integrate it e.g. this week, and I'm expecting to raise more issues once I'm implementing it as I learn more about it, but if this finds its way into a release, I'd be more than happy to integrate it and report back how it went!

ScottyPoi · 2023-07-26T01:56:15Z

@TimDaub

Thanks for the feedback. I responded to a couple comments, but would love to continue the conversation.

Re:

From working with trie.ts, I think it'd be awesome if there were a canonical representation of the trie's and node's hash values and if trie.ts would provide this serialization of values directly instead of having "users" implement their own thing. (not necessarily actionable in this PR specifically)

Can you be more specific? This is certainly for a different PR, but if you can help me understand what you mean, I can look into it.

acolytec3

Lgtm!

ScottyPoi added 4 commits July 20, 2023 01:33

Trie: async iterator _walkTrie function

4438660

write test/demo script

4412639

Trie: internalize walkTrieIterable into Trie class

426f99b

Trie: include helper methods for all nodes / value nodes

1097366

Merge branch 'master' into trie-walk

db3e278

Merge branch 'master' into trie-walk

c7e15cd

acolytec3 requested changes Jul 21, 2023

View reviewed changes

ScottyPoi commented Jul 21, 2023

View reviewed changes

ScottyPoi added 2 commits July 20, 2023 23:23

remove null conditional

708f4ed

update test with sparse trie example

3e1185c

ScottyPoi added type: feature PR state: needs review prio: P4 nice-to-have package: mpt target: master Work to be done towards master branch labels Jul 21, 2023

acolytec3 requested changes Jul 21, 2023

View reviewed changes

Merge branch 'master' into trie-walk

cf6d3c1

Merge remote-tracking branch 'origin/master' into trie-walk

947ba9f

TimDaub reviewed Jul 24, 2023

View reviewed changes

Merge branch 'master' into trie-walk

4be9931

acolytec3 approved these changes Jul 26, 2023

View reviewed changes

ScottyPoi merged commit e984704 into master Jul 26, 2023

holgerd77 deleted the trie-walk branch July 31, 2023 10:18

holgerd77 mentioned this pull request Aug 15, 2023

Trie: Verbose WalkController Test Output #2964

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trie - async walk generator #2904

Trie - async walk generator #2904

ScottyPoi commented Jul 20, 2023 •

edited

Loading

codecov bot commented Jul 20, 2023 •

edited

Loading

acolytec3 commented Jul 20, 2023

acolytec3 commented Jul 21, 2023

acolytec3 left a comment

ScottyPoi commented Jul 21, 2023 •

edited

Loading

ScottyPoi commented Jul 21, 2023

ScottyPoi Jul 21, 2023 •

edited

Loading

acolytec3 commented Jul 21, 2023

acolytec3 left a comment

holgerd77 commented Jul 21, 2023

holgerd77 commented Jul 21, 2023

ScottyPoi commented Jul 22, 2023

TimDaub commented Jul 24, 2023

TimDaub Jul 24, 2023

ScottyPoi Jul 26, 2023

TimDaub Jul 24, 2023

ScottyPoi Jul 26, 2023

TimDaub Jul 24, 2023 •

edited

Loading

TimDaub commented Jul 24, 2023

ScottyPoi commented Jul 26, 2023

acolytec3 left a comment

Trie - async walk generator #2904

Trie - async walk generator #2904

Conversation

ScottyPoi commented Jul 20, 2023 • edited Loading

Trie -- walkTrieIterable

Intention:

Feature

Async Generator / Async Iterable / Async Iterator

Function

_walkTrie

OnFound

NodeFilter

NodeHash / CurrentKey / Visited

Use

Helpers

codecov bot commented Jul 20, 2023 • edited Loading

Codecov Report

acolytec3 commented Jul 20, 2023

acolytec3 commented Jul 21, 2023

acolytec3 left a comment

Choose a reason for hiding this comment

ScottyPoi commented Jul 21, 2023 • edited Loading

ScottyPoi commented Jul 21, 2023

ScottyPoi Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

acolytec3 commented Jul 21, 2023

acolytec3 left a comment

Choose a reason for hiding this comment

holgerd77 commented Jul 21, 2023

holgerd77 commented Jul 21, 2023

ScottyPoi commented Jul 22, 2023

TimDaub commented Jul 24, 2023

TimDaub Jul 24, 2023

Choose a reason for hiding this comment

ScottyPoi Jul 26, 2023

Choose a reason for hiding this comment

TimDaub Jul 24, 2023

Choose a reason for hiding this comment

ScottyPoi Jul 26, 2023

Choose a reason for hiding this comment

TimDaub Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

TimDaub commented Jul 24, 2023

ScottyPoi commented Jul 26, 2023

acolytec3 left a comment

Choose a reason for hiding this comment

ScottyPoi commented Jul 20, 2023 •

edited

Loading

Trie -- `walkTrieIterable`

codecov bot commented Jul 20, 2023 •

edited

Loading

ScottyPoi commented Jul 21, 2023 •

edited

Loading

ScottyPoi Jul 21, 2023 •

edited

Loading

TimDaub Jul 24, 2023 •

edited

Loading