refactor: staged writes on note store #19519
Conversation
mverzilli
added
the
ci-no-fail-fast
…ged-writes-in-private-events
AztecBot
force-pushed
the
martin/staged-writes-in-private-events
branch
from
ee88b93 to
08fb4a0
Compare
AztecBot
force-pushed
the
martin/staged-writes-in-private-events
branch
2 times, most recently
from
8dddf6e to
96dfd77
Compare
mverzilli
force-pushed
the
martin/staged-writes-in-private-events
branch
from
e6f491b to
ef81e22
Compare
AztecBot
force-pushed
the
martin/staged-writes-in-private-events
branch
from
39f2ca9 to
0cf3d5c
Compare
…on an unknown note
benesjan
left a comment
benesjan
left a comment
There was a problem hiding this comment.
Didn't yet go through it all but the naming in note_store needs polishing
| } | ||
| } | ||
|
|
||
| async #readNotesByStorageSlotAndScope(jobId: string, scope: string, slot: Fr): Promise<Set<string>> { |
There was a problem hiding this comment.
Under a note I imagine a NoteDao but here and in many other places it refers to note id/note nullifier.
Also why have the args order swapped in func name and in the actual args?
There was a problem hiding this comment.
I agree with you but the names are already quite long as they are. That comment for example refers to:
#readNotesByStorageSlotAndScope
perhaps the most accurate name would be:
#readNoteNullifiersByStorageSlotAndScope
which feels like too much (although this is clearly subjective)
A different kind of issue that makes naming hard in this store is that there's a lot of conflicts in the role of nullifiers.
A nullifier is:
- something we compute from a note
- something we use as the note id
- something that is proven to be emitted
- based on whether we saw it emitted, we decide to put notes on one collection (#notes) or another (#nullifiedNotes)
So I'm a bit on the fence about mentioning "nullifier" in those helpers because they are about filtering active notes
Maybe the whole store should be simplified, with full notes being stored in a single index by nullifier and some sort of status flag to specify whether they are active or nullified
But that would be out of scope for this PR
| contractAddress: AztecAddress, | ||
| ): Promise<Set<string>> { | ||
| const nullifiedNotesForJob = this.#getNullifiedNotesForJob(jobId); | ||
| const persistedNotesByContractAndScope = this.#notesByContractAndScope.get(scope); |
Fifth and last part of the series started with #19445.
This makes the NoteStore work based on staged writes. With this, notes (and nullifier metadata) aren't written to persistent storage until PXE decides to commit the job.
Closes F-163