-
Notifications
You must be signed in to change notification settings - Fork 775
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
Verkle Implementation: Build out Trie Processing #3430
Changes from 20 commits
0b03444
f468ce2
c1ce5eb
a317698
1670155
55d644f
974d1ae
570f8c4
430d801
fa34dde
b2806cf
f9e019e
74676b0
a267be1
e3ab994
6e7e91e
4a716a7
a2ed90e
1902ebc
541240c
76da96c
52d50a8
86dd9e9
189b112
60219ae
bbd0e96
2301d2c
dcdd778
cf33476
b9285e2
1dcc64d
f42ac23
e4e257a
1b1706d
96f99aa
e4bcb66
27144c0
7f4ead6
abc442d
eeb2ec8
e7e81bf
991f4d2
58e5298
06350d0
df22e18
69267f7
911c5b1
a38cc80
ce343ae
9c7b214
ae09bdb
05bfebd
0b4904c
cd65a9a
1ca879e
3ab0700
720ab09
b47fa49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,23 +1,23 @@ | ||||||
import { equalsBytes } from '@ethereumjs/util' | ||||||
import { BIGINT_0, bytesToBigInt, equalsBytes } from '@ethereumjs/util' | ||||||
|
||||||
import { POINT_IDENTITY } from '../util/crypto.js' | ||||||
|
||||||
import { BaseVerkleNode } from './baseVerkleNode.js' | ||||||
import { LeafNode } from './leafNode.js' | ||||||
import { NODE_WIDTH, VerkleNodeType } from './types.js' | ||||||
|
||||||
import type { VerkleNode, VerkleNodeOptions } from './types.js' | ||||||
import type { VerkleCrypto } from '../types.js' | ||||||
import type { VerkleNodeOptions } from './types.js' | ||||||
|
||||||
export class InternalNode extends BaseVerkleNode<VerkleNodeType.Internal> { | ||||||
// Array of references to children nodes | ||||||
public children: Array<VerkleNode | null> | ||||||
// Array of commitments to child nodes | ||||||
public children: Uint8Array[] | ||||||
public copyOnWrite: Record<string, Uint8Array> | ||||||
public type = VerkleNodeType.Internal | ||||||
|
||||||
/* TODO: options.children is not actually used here */ | ||||||
constructor(options: VerkleNodeOptions[VerkleNodeType.Internal]) { | ||||||
super(options) | ||||||
this.children = options.children ?? new Array(NODE_WIDTH).fill(null) | ||||||
this.children = options.children ?? new Array(NODE_WIDTH).fill(new Uint8Array()) | ||||||
this.copyOnWrite = options.copyOnWrite ?? {} | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
|
@@ -29,11 +29,17 @@ export class InternalNode extends BaseVerkleNode<VerkleNodeType.Internal> { | |||||
// Not implemented yet | ||||||
} | ||||||
|
||||||
setChild(index: number, child: VerkleNode) { | ||||||
// Updates the commitment value for a child node at the corresponding index | ||||||
setChild(index: number, child: Uint8Array) { | ||||||
this.children[index] = child | ||||||
// TODO: Update commitment as well | ||||||
} | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
static fromRawNode(rawNode: Uint8Array[], depth: number): InternalNode { | ||||||
static fromRawNode( | ||||||
rawNode: Uint8Array[], | ||||||
depth: number, | ||||||
verkleCrypto: VerkleCrypto | ||||||
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. Can we remove depth from this API? |
||||||
): InternalNode { | ||||||
const nodeType = rawNode[0][0] | ||||||
if (nodeType !== VerkleNodeType.Internal) { | ||||||
throw new Error('Invalid node type') | ||||||
|
@@ -47,35 +53,47 @@ export class InternalNode extends BaseVerkleNode<VerkleNodeType.Internal> { | |||||
// TODO: Generate Point from rawNode value | ||||||
const commitment = rawNode[rawNode.length - 1] | ||||||
|
||||||
return new InternalNode({ commitment, depth }) | ||||||
return new InternalNode({ commitment, depth, verkleCrypto }) | ||||||
} | ||||||
|
||||||
static create(depth: number): InternalNode { | ||||||
static create(depth: number, verkleCrypto: VerkleCrypto): InternalNode { | ||||||
const node = new InternalNode({ | ||||||
commitment: POINT_IDENTITY, | ||||||
depth, | ||||||
verkleCrypto, | ||||||
}) | ||||||
|
||||||
return node | ||||||
} | ||||||
|
||||||
getChildren(index: number): VerkleNode | null { | ||||||
getChildren(index: number): Uint8Array | null { | ||||||
return this.children?.[index] ?? null | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
insert(key: Uint8Array, value: Uint8Array, resolver: () => void): void { | ||||||
insert( | ||||||
key: Uint8Array, | ||||||
value: Uint8Array, | ||||||
resolver: () => void, | ||||||
verkleCrypto?: VerkleCrypto | ||||||
): void { | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
const values = new Array<Uint8Array>(NODE_WIDTH) | ||||||
values[key[31]] = value | ||||||
this.insertStem(key.slice(0, 31), values, resolver) | ||||||
this.insertStem(key.slice(0, 31), values, resolver, verkleCrypto!) | ||||||
} | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
insertStem(stem: Uint8Array, values: Uint8Array[], resolver: () => void): void { | ||||||
insertStem( | ||||||
stem: Uint8Array, | ||||||
values: Uint8Array[], | ||||||
resolver: () => void, | ||||||
verkleCrypto: VerkleCrypto | ||||||
): void { | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
// 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 | ||||||
this.cowChild(childIndex) | ||||||
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. 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.
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (equalsBytes(child.stem, stem)) { | ||||||
return child.insertMultiple(stem, values) | ||||||
|
@@ -85,35 +103,64 @@ export class InternalNode extends BaseVerkleNode<VerkleNodeType.Internal> { | |||||
// on the next byte in both keys, a recursion into | ||||||
// the moved leaf node can occur. | ||||||
const nextByteInExistingKey = child.stem[this.depth + 1] | ||||||
const newBranch = InternalNode.create(this.depth + 1) | ||||||
const newBranch = InternalNode.create(this.depth + 1, this.verkleCrypto) | ||||||
newBranch.cowChild(nextByteInExistingKey) | ||||||
this.children[childIndex] = newBranch | ||||||
this.children[childIndex] = newBranch.commitment | ||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
// Next word differs, so this was the last level. | ||||||
// Insert it directly into its final slot. | ||||||
const leafNode = LeafNode.create(stem, values) | ||||||
// TODO: Fix this following `trie.put` logic | ||||||
let leafCommitment = verkleCrypto.zeroCommitment | ||||||
let c1 = verkleCrypto.zeroCommitment | ||||||
let c2 = verkleCrypto.zeroCommitment | ||||||
for (const [idx, value] of values.entries()) { | ||||||
if (bytesToBigInt(value) > BIGINT_0) { | ||||||
leafCommitment = verkleCrypto.updateCommitment( | ||||||
leafCommitment, | ||||||
idx, | ||||||
new Uint8Array(32), | ||||||
value | ||||||
) | ||||||
if (idx < 128) { | ||||||
// We multiply the commitment index by 2 here because each 32 byte value in the leaf node is represented as two 16 byte arrays | ||||||
c1 = verkleCrypto.updateCommitment(c1, idx * 2, new Uint8Array(32), value) | ||||||
} else { | ||||||
c2 = verkleCrypto.updateCommitment(c2, (idx - 128) * 2, new Uint8Array(32), value) | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
const leafNode = LeafNode.create( | ||||||
stem, | ||||||
values, | ||||||
this.depth + 1, | ||||||
leafCommitment, | ||||||
c1, | ||||||
c2, | ||||||
this.verkleCrypto | ||||||
) | ||||||
|
||||||
// TODO - Why is the leaf node set at depth + 2 instead of + 1)? | ||||||
leafNode.setDepth(this.depth + 2) | ||||||
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. |
||||||
newBranch.cowChild(nextByteInInsertedKey) | ||||||
newBranch.children[nextByteInInsertedKey] = leafNode | ||||||
} else if (child instanceof InternalNode) { | ||||||
this.cowChild(childIndex) | ||||||
return child.insertStem(stem, values, resolver) | ||||||
return child.insertStem(stem, values, resolver, verkleCrypto) | ||||||
} else { | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
throw new Error('Invalid node type') | ||||||
} | ||||||
} | ||||||
|
||||||
// TODO: go-verkle also adds the bitlist to the raw format. | ||||||
raw(): Uint8Array[] { | ||||||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
throw new Error('not implemented yet') | ||||||
// return [new Uint8Array([VerkleNodeType.Internal]), ...this.children, this.commitment] | ||||||
return [new Uint8Array([VerkleNodeType.Internal]), ...this.children, this.commitment] | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,14 @@ | ||
/* eslint-disable @typescript-eslint/no-unused-vars */ | ||
import { BaseVerkleNode } from './baseVerkleNode.js' | ||
import { NODE_WIDTH, VerkleNodeType } from './types.js' | ||
|
||
import type { Point } from '../types.js' | ||
import type { VerkleCrypto } from '../types.js' | ||
import type { VerkleNodeOptions } from './types.js' | ||
|
||
export class LeafNode extends BaseVerkleNode<VerkleNodeType.Leaf> { | ||
public stem: Uint8Array | ||
public values: Uint8Array[] | ||
public c1: Point | ||
public c2: Point | ||
public values: Uint8Array[] // Array of 256 possible values represented as 32 byte Uint8Arrays | ||
public c1?: Uint8Array | ||
public c2?: Uint8Array | ||
public type = VerkleNodeType.Leaf | ||
|
||
constructor(options: VerkleNodeOptions[VerkleNodeType.Leaf]) { | ||
|
@@ -21,11 +20,27 @@ export class LeafNode extends BaseVerkleNode<VerkleNodeType.Leaf> { | |
this.c2 = options.c2 | ||
} | ||
|
||
static create(stem: Uint8Array, values: Uint8Array[]): LeafNode { | ||
throw new Error('Not implemented') | ||
static create( | ||
stem: Uint8Array, | ||
values: Uint8Array[], | ||
depth: number, | ||
commitment: Uint8Array, | ||
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. Can we remove depth here too? |
||
c1: Uint8Array, | ||
c2: Uint8Array, | ||
verkleCrypto: VerkleCrypto | ||
): LeafNode { | ||
return new LeafNode({ | ||
stem, | ||
values, | ||
depth, | ||
commitment, | ||
c1, | ||
c2, | ||
verkleCrypto, | ||
}) | ||
} | ||
|
||
static fromRawNode(rawNode: Uint8Array[], depth: number): LeafNode { | ||
static fromRawNode(rawNode: Uint8Array[], depth: number, verkleCrypto: VerkleCrypto): LeafNode { | ||
const nodeType = rawNode[0][0] | ||
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. Remove depth here? |
||
if (nodeType !== VerkleNodeType.Leaf) { | ||
throw new Error('Invalid node type') | ||
|
@@ -39,11 +54,11 @@ export class LeafNode extends BaseVerkleNode<VerkleNodeType.Leaf> { | |
const stem = rawNode[1] | ||
// TODO: Convert the rawNode commitments to points | ||
const commitment = rawNode[2] | ||
acolytec3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const c1 = rawNode[3] as unknown as Point | ||
const c2 = rawNode[4] as unknown as Point | ||
const c1 = rawNode[3] | ||
const c2 = rawNode[4] | ||
const values = rawNode.slice(5, rawNode.length) | ||
|
||
return new LeafNode({ depth, stem, values, c1, c2, commitment }) | ||
return new LeafNode({ depth, stem, values, c1, c2, commitment, verkleCrypto }) | ||
} | ||
commit(): Uint8Array { | ||
throw new Error('Not implemented') | ||
|
@@ -73,8 +88,8 @@ export class LeafNode extends BaseVerkleNode<VerkleNodeType.Leaf> { | |
new Uint8Array([VerkleNodeType.Leaf]), | ||
this.stem, | ||
this.commitment, | ||
this.c1.bytes(), | ||
this.c2.bytes(), | ||
this.c1 ?? new Uint8Array(), | ||
this.c2 ?? new Uint8Array(), | ||
...this.values, | ||
] | ||
} | ||
|
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 should now use the import from
verkle
inUtil
, just opened a related issue here ethereumjs/verkle-cryptography-wasm#54I would go as far and argue that we should also remove the
verkle-cryptography-wasm
dependency from this package as well and dependency inject, and so to have this wider strategic goal over the head that this should be replaceable and also here optimally a JS implementation being used.We then also automatically continue to evolve a generic interface around this and keep thinking about what's needed and what not (I e.g. still like the idea that we take the more hashing related parts out directly and move over to the pedersen/poseidon hash usage from Pauls libraries https://github.com/paulmillr/scure-starknet (if applicable) and with this slowly shrink down the surface of what we need to compile over into the WASM stuff.
Feels to me that there is still too much in.
Practical note: if depency injection feels a bit impractical for now for testing and the like we could also decide to do at a later stage.
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.
Ah, but the main argument for direct dependency injection (so, this would mean: having a mandatory
crypto
constructor argument) is actually that we avoid having the same bundle problems again once/if we have Verkle added somewhere as a dependency.