-
Notifications
You must be signed in to change notification settings - Fork 718
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
Build out verkle trie processing #3430
base: master
Are you sure you want to change the base?
Conversation
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.
Had a little bit of time and quickly glanced through, nice work! left some comments on some of your questions/todos
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem like that should belong to this PR?
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.
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.
Choose a reason for hiding this comment
The 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.
|
||
// TODO - Why is the leaf node set at depth + 2 instead of + 1)? |
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.
WIP