-
Notifications
You must be signed in to change notification settings - Fork 768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement trie.put
#3473
Implement trie.put
#3473
Changes from 7 commits
fdb94c8
1cc7910
591c4c3
adc60f1
8d5c9f2
c130d5a
ea2d9dd
0d572ba
5320929
fc484bb
9b00803
049e8f6
c9b4844
6607f74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import { | |
ValueEncoding, | ||
bytesToHex, | ||
equalsBytes, | ||
intToHex, | ||
zeros, | ||
} from '@ethereumjs/util' | ||
import debug from 'debug' | ||
|
@@ -13,7 +14,7 @@ import { loadVerkleCrypto } from 'verkle-cryptography-wasm' | |
import { CheckpointDB } from './db/checkpoint.js' | ||
import { InternalNode } from './node/internalNode.js' | ||
import { LeafNode } from './node/leafNode.js' | ||
import { type VerkleNode } from './node/types.js' | ||
import { type VerkleNode, VerkleNodeType } from './node/types.js' | ||
import { decodeNode } from './node/util.js' | ||
import { | ||
type Proof, | ||
|
@@ -133,7 +134,9 @@ export class VerkleTree { | |
} | ||
} | ||
|
||
return new VerkleTree(opts) | ||
const trie = new VerkleTree(opts) | ||
await trie._createRootNode() | ||
return trie | ||
} | ||
|
||
database(db?: DB<Uint8Array, Uint8Array>) { | ||
|
@@ -194,7 +197,6 @@ export class VerkleTree { | |
const suffix = key[key.length - 1] | ||
this.DEBUG && this.debug(`Stem: ${bytesToHex(stem)}; Suffix: ${suffix}`, ['GET']) | ||
const res = await this.findPath(stem) | ||
|
||
if (res.node instanceof LeafNode) { | ||
// The retrieved leaf node contains an array of 256 possible values. | ||
// The index of the value we want is at the key's last byte | ||
|
@@ -213,9 +215,149 @@ export class VerkleTree { | |
* @param value - the value to store | ||
* @returns A Promise that resolves once value is stored. | ||
*/ | ||
// TODO: Rewrite following logic in verkle.spec.ts "findPath validation" test | ||
async put(_key: Uint8Array, _value: Uint8Array): Promise<void> { | ||
throw new Error('not implemented') | ||
async put(key: Uint8Array, value: Uint8Array): Promise<void> { | ||
if (key.length !== 32) throw new Error(`expected key with length 32; got ${key.length}`) | ||
const stem = key.slice(0, 31) | ||
const suffix = key[key.length - 1] | ||
this.DEBUG && this.debug(`Stem: ${bytesToHex(stem)}; Suffix: ${suffix}`, ['PUT']) | ||
|
||
const putStack: [Uint8Array, VerkleNode][] = [] | ||
// Find path to nearest node | ||
const foundPath = await this.findPath(stem) | ||
|
||
// Sanity check - we should at least get the root node back | ||
if (foundPath.stack.length === 0) { | ||
throw new Error(`Root node not found in trie`) | ||
} | ||
|
||
// Step 1) Create or update the leaf node | ||
let leafNode: LeafNode | ||
// First see if leaf node already exists | ||
if (foundPath.node !== null) { | ||
// Sanity check to verify we have the right node type | ||
if (foundPath.node.type !== VerkleNodeType.Leaf) { | ||
throw new Error( | ||
`expected leaf node found at ${bytesToHex(stem)}. Got internal node instead` | ||
) | ||
} | ||
leafNode = foundPath.node as LeafNode | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can create some type guards for those, I think this will be useful in qute a few places. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just did: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good! |
||
// Sanity check to verify we have the right leaf node | ||
if (!equalsBytes(leafNode.stem, stem)) { | ||
throw new Error( | ||
`invalid leaf node found. Expected stem: ${bytesToHex(stem)}; got ${bytesToHex( | ||
(foundPath.node as LeafNode).stem | ||
)}` | ||
) | ||
} | ||
} else { | ||
// Leaf node doesn't exist, create a new one | ||
leafNode = await LeafNode.create( | ||
stem, | ||
new Array(256).fill(new Uint8Array(32)), | ||
this.verkleCrypto | ||
) | ||
this.DEBUG && this.debug(`Creating new leaf node at stem: ${bytesToHex(stem)}`, ['PUT']) | ||
} | ||
// Update value in leaf node and push to putStack | ||
leafNode.setValue(suffix, value) | ||
this.DEBUG && | ||
this.debug( | ||
`Updating value for suffix: ${suffix} at leaf node with stem: ${bytesToHex(stem)}`, | ||
['PUT'] | ||
) | ||
putStack.push([leafNode.hash(), leafNode]) | ||
|
||
// `path` is the path to the last node pushed to the `putStack` | ||
let path = leafNode.stem | ||
|
||
// Step 2) Determine if a new internal node is needed | ||
if (foundPath.stack.length > 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to make sure I am getting this correctly, could you confirm that this scenario is correct:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should be how it works. Not guaranteeing I got it exactly right but the tests seem to indicate so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, I didn't infer the above from how the function/tests were structured, but wrote it based on my understanding of verkle trees, good to hear it matches There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at your comment again, let me confirm something. What you call "index 2" here would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes that's what I meant, sorry for the confusing indexing |
||
// Only insert new internal node if we have more than 1 node in the path | ||
// since a single node indicates only the root node is in the path | ||
const nearestNodeTuple = foundPath.stack.pop()! | ||
let nearestNode = nearestNodeTuple[0] | ||
path = nearestNodeTuple[1] | ||
// Compute the portion of leafNode.stem and nearestNode.path that match (i.e. the partial path closest to leafNode.stem) | ||
const partialMatchingStemIndex = matchingBytesLength(leafNode.stem, path) | ||
if (nearestNode.type === VerkleNodeType.Leaf) { | ||
// We need to create a new internal node and set nearestNode and leafNode as child nodes of it | ||
nearestNode = nearestNode as LeafNode | ||
// Create new internal node | ||
const internalNode = InternalNode.create(this.verkleCrypto) | ||
// Set leafNode and nextNode as children of the new internal node | ||
internalNode.setChild(leafNode.stem[partialMatchingStemIndex], { | ||
commitment: leafNode.commitment, | ||
path: leafNode.stem, | ||
}) | ||
internalNode.setChild(nearestNode.stem[partialMatchingStemIndex], { | ||
commitment: nearestNode.commitment, | ||
path: nearestNode.stem, | ||
}) | ||
// Find the path to the new internal node (the matching portion of the leaf node and next node's stems) | ||
path = leafNode.stem.slice(0, partialMatchingStemIndex) | ||
this.DEBUG && this.debug(`Creating new internal node at path ${bytesToHex(path)}`, ['PUT']) | ||
// Push new internal node to putStack | ||
putStack.push([internalNode.hash(), internalNode]) | ||
} else { | ||
// Nearest node is an internal node. We need to update the appropriate child reference | ||
// to the new leaf node | ||
nearestNode = nearestNode as InternalNode | ||
nearestNode.setChild(leafNode.stem[partialMatchingStemIndex], { | ||
commitment: leafNode.commitment, | ||
path: leafNode.stem, | ||
}) | ||
this.DEBUG && | ||
this.debug( | ||
`Updating child reference for leaf node with stem: ${bytesToHex(stem)} at index ${ | ||
leafNode.stem[partialMatchingStemIndex] | ||
} in internal node at path ${bytesToHex( | ||
leafNode.stem.slice(0, partialMatchingStemIndex) | ||
)}`, | ||
['PUT'] | ||
) | ||
putStack.push([nearestNode.hash(), nearestNode]) | ||
} | ||
|
||
// Step 3) Walk up trie and update child references in parent internal nodes | ||
while (foundPath.stack.length > 1) { | ||
const [nextNode, nextPath] = foundPath.stack.pop()! as [InternalNode, Uint8Array] | ||
// Compute the child index to be updated on `nextNode` | ||
const childIndex = path[matchingBytesLength(path, nextPath)] | ||
// Update child reference | ||
nextNode.setChild(childIndex, { | ||
commitment: putStack[putStack.length - 1][1].commitment, | ||
path, | ||
}) | ||
this.DEBUG && | ||
this.debug( | ||
`Updating child reference for node with path: ${bytesToHex( | ||
path | ||
)} at index ${childIndex} in internal node at path ${bytesToHex(nextPath)}`, | ||
['PUT'] | ||
) | ||
// Hold onto `path` to current node for updating next parent node child index | ||
path = nextPath | ||
putStack.push([nextNode.hash(), nextNode]) | ||
} | ||
} | ||
|
||
// Step 4) Update root node | ||
const rootNode = foundPath.stack.pop()![0] as InternalNode | ||
rootNode.setChild(stem[0], { | ||
commitment: putStack[putStack.length - 1][1].commitment, | ||
path, | ||
}) | ||
this.root(this.verkleCrypto.serializeCommitment(rootNode.commitment)) | ||
this.DEBUG && | ||
this.debug( | ||
`Updating child reference for node with path: ${bytesToHex(path)} at index ${ | ||
path[0] | ||
} in root node`, | ||
['PUT'] | ||
) | ||
this.DEBUG && this.debug(`Updating root node hash to ${bytesToHex(this._root)}`, ['PUT']) | ||
putStack.push([this._root, rootNode]) | ||
await this.saveStack(putStack) | ||
} | ||
|
||
/** | ||
|
@@ -231,12 +373,13 @@ export class VerkleTree { | |
stack: [], | ||
remaining: key, | ||
} | ||
if (equalsBytes(this.root(), this.EMPTY_TREE_ROOT)) return result | ||
|
||
// TODO: Decide if findPath should return an empty stack if we have an empty trie or a path with just the empty root node | ||
// if (equalsBytes(this.root(), this.EMPTY_TREE_ROOT)) return result | ||
|
||
// Get root node | ||
let rawNode = await this._db.get(this.root()) | ||
if (rawNode === undefined) | ||
throw new Error('root node should exist when root not empty tree root') | ||
if (rawNode === undefined) throw new Error('root node should exist') | ||
|
||
const rootNode = decodeNode(rawNode, this.verkleCrypto) as InternalNode | ||
|
||
|
@@ -246,7 +389,7 @@ export class VerkleTree { | |
|
||
// Root node doesn't contain a child node's commitment on the first byte of the path so we're done | ||
if (equalsBytes(child.commitment, this.verkleCrypto.zeroCommitment)) { | ||
this.DEBUG && this.debug(`Partial Path ${key[0]} - found no child.`, ['FIND_PATH']) | ||
this.DEBUG && this.debug(`Partial Path ${intToHex(key[0])} - found no child.`, ['FIND_PATH']) | ||
return result | ||
} | ||
let finished = false | ||
|
@@ -282,16 +425,16 @@ export class VerkleTree { | |
// We found a different node than the one specified by `key` | ||
// so the sought node doesn't exist | ||
result.remaining = key.slice(matchingKeyLength) | ||
const pathToNearestNode = | ||
decodedNode.type === VerkleNodeType.Leaf ? (decodedNode as LeafNode).stem : child.path | ||
this.DEBUG && | ||
this.debug( | ||
`Path ${bytesToHex( | ||
key.slice(0, matchingKeyLength) | ||
)} - found path to nearest node ${bytesToHex( | ||
`Path ${bytesToHex(pathToNearestNode)} - found path to nearest node ${bytesToHex( | ||
decodedNode.hash() | ||
)} but target node not found.`, | ||
['FIND_PATH'] | ||
) | ||
result.stack.push([decodedNode, key.slice(0, matchingKeyLength)]) | ||
result.stack.push([decodedNode, pathToNearestNode]) | ||
return result | ||
} | ||
// Push internal node to path stack | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is very long. I find functions more difficult to read and debug when I have to scroll in order to see different parts of it.
I appreciate the way you use the comments to outline how things are happening step by step. I wonder if these different steps could be externalized into separate functions, so that the
put
function itself can fit on a screen?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a 100% valid criticism. Lemme look at the logic and see if there's a way to break it down. I think you're right that it can be done in smaller chunks.