Build out verkle trie processing #3430
Conversation
electra.yaml
Outdated
| - el_type: ethereumjs | ||
| el_image: ethereumjs:local | ||
| cl_type: grandine | ||
| cl_image: ethpandaops/grandine:feature-electra |
There was a problem hiding this comment.
Doesn't seem like that should belong to this PR?
There was a problem hiding this comment.
It doesn't. Accidentally included and will remove.
| // Index of the child pointed by the next byte in the key | ||
| const childIndex = stem[this.depth] | ||
|
|
||
| const child = this.children[childIndex] | ||
|
|
||
| if (child instanceof LeafNode) { | ||
| // TODO: Understand the intent of what cowChild is suppoded to do |
There was a problem hiding this comment.
cow stands for "copy-on-write". As far as I recall cowChild is used as a record to mark the children that have been modified, in order to stack the commitment updates at the end since that is way more efficient.
|
|
||
| // TODO - Why is the leaf node set at depth + 2 instead of + 1)? |
There was a problem hiding this comment.
I think that this is because the leaf is inserted in the position of the suffix level commitment. So we have:
depth n : internal node:
depth n+1: the extension level commitment
depth n+2: the suffix level commitment
See this picture for how that corresponds to the structure:
| newBranch.children[nextByteInExistingKey] = child | ||
| child.depth += 1 | ||
|
|
||
| const nextByteInInsertedKey = stem[this.depth + 1] | ||
| if (nextByteInInsertedKey === nextByteInExistingKey) { | ||
| return newBranch.insertStem(stem, values, resolver) | ||
| return newBranch.insertStem(stem, values, resolver, verkleCrypto) |
There was a problem hiding this comment.
| return newBranch.insertStem(stem, values, resolver, verkleCrypto) | |
| return newBranch.insertStem(stem, values, resolver) |
packages/verkle/src/types.ts
Outdated
| @@ -8,6 +8,7 @@ import type { VerkleCrypto as VerkleFFI } from 'verkle-cryptography-wasm' | |||
| // Field representation of a commitment | |||
| export interface Fr {} | |||
|
|
|||
| // TODO: Decide if we still need this interface. Commitments are now all bytes from the JS perspective | |||
There was a problem hiding this comment.
I think we can do without this. We are not interacting with elliptic curve point representations directly, and always interfacing through verklecrypto
| @@ -142,3 +143,6 @@ export const HEADER_STORAGE_OFFSET = 64 | |||
| export const CODE_OFFSET = 128 | |||
| export const VERKLE_NODE_WIDTH = 256 | |||
| export const MAIN_STORAGE_OFFSET = BigInt(256) ** BigInt(31) | |||
|
|
|||
| export const zeroValues = new Array(256).fill(new Uint8Array()) | |||
| export const zeroCValues = new Array(128).fill(new Uint8Array()) | |||
There was a problem hiding this comment.
To me "CValue" is a bit unclear. Do you think it would be clearer if we named this "zeroLeafValues"?
There was a problem hiding this comment.
Not really sure. It's not precisely zeroLeafValues but more like the zeroHalfLeafValues. What do you think?
| @@ -80,7 +80,7 @@ export class WalkController { | |||
| for (const child of children) { | |||
| if (child.nodeRef !== null) { | |||
| const childKey = new Uint8Array([...key, child.keyExtension]) | |||
| this.pushNodeToQueue(child.nodeRef.hash(), childKey) | |||
| this.pushNodeToQueue(child.nodeRef.hash(undefined as any), childKey) | |||
There was a problem hiding this comment.
| const rawNode = RLP.decode(nodeRLP) as Uint8Array[] | ||
| const node = | ||
| rawNode[0][0] === VerkleNodeType.Leaf | ||
| ? // TODO: Figure out how to determine depth from key |
There was a problem hiding this comment.
I don't think Depth can be directly inferred from the key, as the same key would have different depths depending on which verkle trie structure they are a part of. Shouldn't that information be contained in the serialized node that is stored in the db?
I just checked the go code and they don't store that in their (legacy) serialization format.. maybe I'm missing something.
There was a problem hiding this comment.
Okay, I think that makes sense to me. We can ensure that's included in the serialized node.
| c2, | ||
| this.verkleCrypto | ||
| ) | ||
| await this._db.put(leafNode.hash(), leafNode.serialize()) | ||
| } | ||
|
|
There was a problem hiding this comment.
Maybe I'm not following this, but where is the part of the code where if the leafNode exists, we update it? I'm just seeing us create a new leaf if it does not exist in the trie.
There was a problem hiding this comment.
Yup, this is still very incomplete. This is where I'm currently focused. We need to actually pull parent nodes out of the DB and update the child commitment value in their values array and then update their commitment.
packages/verkle/src/verkleTree.ts
Outdated
| parentNode.children[parentIndex] = currentNode | ||
| // Parent key should be at least one shorter than the current key length | ||
| const parentKey = currentKey.slice(0, currentKey.length - 1) | ||
| const rawNode = await this.get(parentKey) |
There was a problem hiding this comment.
I'm not sure that this is a proper use of this.get? Shouldn't we use findPath instead for e.g.?
There was a problem hiding this comment.
Oh, you know, I think i meant to do trie._db.get here. Maybe that's the difference I was missing before
| // TODO: Determine how to decide if we need to go further up the tree or create a new internal node here | ||
| // Currently, this code would insert a new internal node at every step up the trie (which we don't want) |
There was a problem hiding this comment.
The way I'm thinking of this, we could find the nearest existing path to the leaf node that we want to insert, while keeping track of all the tries that have led to that, and update them sequentially, rather than coming from the child and looking up each parent. Does that make sense?
WIP