trie: improve node typings and class architecture #3708
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
packages/trie/src/types.ts
Outdated
| @@ -8,9 +8,17 @@ export type TrieNode = BranchNode | ExtensionNode | LeafNode | |||
|
|
|||
| export type Nibbles = number[] | |||
|
|
|||
| // [ encodedPath, key ] | |||
There was a problem hiding this comment.
Hmm, I find this documentation somewhat misleading. The key in this context would be the pointer to whatever hash this ExtensionNode points to. I think this is implied to by "key" but this might be confused with the MPT-keys (so the ones you use as get argument). Not sure what a better term here would be though. referencedHash? Or maybe pointerHash?
There was a problem hiding this comment.
I've copied what is in the ethereum.org documentation. I agree that it is not immediately obvious what this represents. I will remove these two comments and instead add a link to the docs.
There was a problem hiding this comment.
I dunno if I like the updated comment any better. The docs do explain what the extension node is, in that the key is the DB key for the next node.
Can we get rid of the "raw node" terminology and just have extension nodes, which are simply the encoded path to the node and then the DB key for the next lookup, and then lead nodes, which are the encoded path and then the leaf value (which is either an rlp encoded account or null, right)?
There was a problem hiding this comment.
Yeah that's fair.
I'm not sure I get the idea behind getting rid of the "raw node" terminology. We could do a renaming, but then we would have to rework the classes since these are already called "ExensionNode" and "LeafNode". Additionally, in comments and various places (e.g. methods like decodeRawNode), we have been referring to "raw nodes" quite often, and I would tend not to make changes that are too intrusive.
I've updated the comment, let me know if that seems clear enough
This PR resolves #3704 by:
ExtensionAndLeafNodeBaseclass and making it abstract.encodeKeymethod from the extension and leaf node classes as they were not usedNodeReferenceOrRawNodetypeBranchNodeBranchValuetype that fits the types of the values of the _branches of the branch node class.