-
Notifications
You must be signed in to change notification settings - Fork 581
refactor: dropping noteHashLeafIndexMap #19443
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
69f815a to
8692d8d
Compare
8692d8d to
fddec79
Compare
fddec79 to
45066ab
Compare
nventuro
left a comment
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.
Lovely. I'm checking with the alpha team if the new method is slower.
| async getNoteHashMembershipWitness(leafIndex: bigint): Promise<MembershipWitness<typeof NOTE_HASH_TREE_HEIGHT>> { | ||
| const path = await this.node.getNoteHashSiblingPath(this.blockNumber, leafIndex); |
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.
Wow, this naming mismatch was quite confusing.
Flakey Tests🤖 says: This CI run detected 3 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
7ecd252 to
83a369a
Compare
Tracking the note hash leaf indexes during execution is unnecessary because when building the kernel hints we are able to obtain the note hash membership witness directly by value instead of by index. I guess the API obtaining the witness by value might be a bit slower since we first need to find where in the tree the it is while index directly points into a tree. But even if this is the case it doesn't seem to be worth the complexity (if the slowness turns out to be a problem we can always store a map from note hash to index in the node given that the note hashes are unique). Decided to do this when trying to tackle [F-218](https://linear.app/aztec-labs/issue/F-218/remove-note-and-nullif-indexes-from-note-and-event-storage).
83a369a to
1e1c8b8
Compare
Tracking the note hash leaf indexes during execution is unnecessary because when building the kernel hints we are able to obtain the note hash membership witness directly by value instead of by index. I guess the API obtaining the witness by value might be a bit slower since we first need to find where in the tree the it is while index directly points into a tree. But even if this is the case it doesn't seem to be worth the complexity (if the slowness turns out to be a problem we can always store a map from note hash to index in the node given that the note hashes are unique). Decided to do this when trying to tackle [F-218](https://linear.app/aztec-labs/issue/F-218/remove-note-and-nullif-indexes-from-note-and-event-storage).
As Martin mentioned [here](#19327 (comment)) makes sense to have the reorg handling be atomic. In this PR I tackle that. Note that this PR doesn't need to be in stack with #19443 but #19451 depends on both so putting this one into stack was the only way to unblock myself working on #19451.
As Martin mentioned [here](#19327 (comment)) makes sense to have the reorg handling be atomic. In this PR I tackle that. Note that this PR doesn't need to be in stack with #19443 but #19451 depends on both so putting this one into stack was the only way to unblock myself working on #19451.
As Martin mentioned [here](#19327 (comment)) makes sense to have the reorg handling be atomic. In this PR I tackle that. Note that this PR doesn't need to be in stack with #19443 but #19451 depends on both so putting this one into stack was the only way to unblock myself working on #19451.

Tracking the note hash leaf indexes during execution is unnecessary because when building the kernel hints we are able to obtain the note hash membership witness directly by value instead of by index. I guess the API obtaining the witness by value might be a bit slower since we first need to find where in the tree the it is while index directly points into a tree. But even if this is the case it doesn't seem to be worth the complexity (if the slowness turns out to be a problem we can always store a map from note hash to index in the node given that the note hashes are unique).
Decided to do this when trying to tackle F-218.