-
Notifications
You must be signed in to change notification settings - Fork 311
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
Sort and slice db and pending notes. #1112
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.
LGTM! Very nice work!
* The format that noir contracts use to get notes. | ||
* Information about a note needed during execution. | ||
*/ | ||
export interface NoteLoadOracleInputs { | ||
/** | ||
* The nonce of the note. | ||
*/ | ||
export interface NoteData { | ||
/** The contract address of the note. */ | ||
contractAddress: AztecAddress; | ||
/** The storage slot of the note. */ | ||
storageSlot: Fr; | ||
/** The nonce of the note. */ | ||
nonce: Fr; | ||
/** | ||
* The preimage of the note. | ||
*/ | ||
/** The preimage of the note */ | ||
preimage: Fr[]; | ||
/** | ||
* The note's leaf index in the private data tree. | ||
*/ | ||
index: bigint; | ||
/** The note's leaf index in the private data tree. Undefined for pending notes. */ | ||
index?: bigint; |
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.
nice cleanup merging these two structs
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/920): don't 'get' notes nullified in pendingNullifiers | ||
const pendingNotes = this.pendingNotes.filter( | ||
n => n.contractAddress.equals(contractAddress) && n.storageSlot.equals(storageSlotField), | ||
); |
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.
Very clean, nice.
|
||
import { SortOrder, pickNotes } from './pick_notes.js'; | ||
|
||
describe('getNotes', () => { |
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.
Nice tests!
); | ||
|
||
// TODO: Sort again. | ||
const notes = [...pendingNotes, ...dbNotes].slice(offset, limit ? offset + limit : undefined); | ||
const dbNotes = await this.db.getNotes(contractAddress, storageSlotField); |
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.
So will this just get ALL notes from the DB that match contractAddress & slot?
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.
Yeah, we have to do that anyway :( Because the preimage is stored as a single field in the db. So we always need to get all notes matching the contractAddress and slot, to parse it, sort it, and slice it.
Description
Closes #1030
Checklist: