Skip to content

Conversation

@benesjan
Copy link
Contributor

@benesjan benesjan commented Jan 8, 2026

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.

Copy link
Contributor Author

benesjan commented Jan 8, 2026

@benesjan benesjan force-pushed the 01-08-refactor_dropping_notehashleafindexmap branch from 69f815a to 8692d8d Compare January 8, 2026 20:14
@benesjan benesjan marked this pull request as ready for review January 8, 2026 20:16
@AztecBot AztecBot force-pushed the 01-08-refactor_dropping_notehashleafindexmap branch from 8692d8d to fddec79 Compare January 8, 2026 20:34
@AztecBot AztecBot enabled auto-merge January 8, 2026 20:34
@benesjan benesjan requested a review from nventuro January 8, 2026 20:38
@benesjan benesjan force-pushed the 01-08-refactor_dropping_notehashleafindexmap branch from fddec79 to 45066ab Compare January 9, 2026 03:27
Copy link
Contributor

@nventuro nventuro left a 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.

Comment on lines -73 to -74
async getNoteHashMembershipWitness(leafIndex: bigint): Promise<MembershipWitness<typeof NOTE_HASH_TREE_HEIGHT>> {
const path = await this.node.getNoteHashSiblingPath(this.blockNumber, leafIndex);
Copy link
Contributor

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.

@AztecBot
Copy link
Collaborator

AztecBot commented Jan 12, 2026

Flakey Tests

🤖 says: This CI run detected 3 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033FLAKED\033 (8;;http://ci.aztec-labs.com/79b059ae6e8a32c7�79b059ae6e8a32c78;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_epochs/epochs_invalidate_block.parallel.test.ts "committee member invalidates a block if proposer does not come through" (101s) (code: 1) group:e2e-p2p-epoch-flakes (\033Aztec Bot\033: refactor: dropping noteHashLeafIndexMap (#19443))
\033FLAKED\033 (8;;http://ci.aztec-labs.com/20488951acb74139�20488951acb741398;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_l1_publisher/e2e_l1_publisher.test.ts (98s) (code: 1) (\033Aztec Bot\033: refactor: dropping noteHashLeafIndexMap (#19443))
\033FLAKED\033 (8;;http://ci.aztec-labs.com/4a8bb95d1925448e�4a8bb95d1925448e8;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_p2p/gossip_network.test.ts (451s) (code: 1) group:e2e-p2p-epoch-flakes (\033Aztec Bot\033: refactor: dropping noteHashLeafIndexMap (#19443))

@AztecBot AztecBot force-pushed the 01-08-refactor_dropping_notehashleafindexmap branch from 7ecd252 to 83a369a Compare January 12, 2026 13:38
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).
@AztecBot AztecBot force-pushed the 01-08-refactor_dropping_notehashleafindexmap branch from 83a369a to 1e1c8b8 Compare January 12, 2026 13:41
@AztecBot AztecBot added this pull request to the merge queue Jan 12, 2026
Merged via the queue into next with commit d7ca562 Jan 12, 2026
17 checks passed
@AztecBot AztecBot deleted the 01-08-refactor_dropping_notehashleafindexmap branch January 12, 2026 14:15
github-actions bot pushed a commit that referenced this pull request Jan 12, 2026
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).
AztecBot pushed a commit that referenced this pull request Jan 12, 2026
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.
AztecBot pushed a commit that referenced this pull request Jan 12, 2026
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.
github-merge-queue bot pushed a commit that referenced this pull request Jan 12, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants