Skip to content
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

feat: nullified note retrieval in get_notes and view_notes #4208

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions yarn-project/acir-simulator/src/acvm/oracle/oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export class Oracle {
sortOrder: ACVMField[],
[limit]: ACVMField[],
[offset]: ACVMField[],
[includeNullified]: ACVMField[],
[returnSize]: ACVMField[],
): Promise<ACVMField[]> {
const noteDatas = await this.typedOracle.getNotes(
Expand All @@ -178,6 +179,7 @@ export class Oracle {
sortOrder.map(s => +s),
+limit,
+offset,
!!+includeNullified,
);

const noteLength = noteDatas?.[0]?.note.items.length ?? 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export abstract class TypedOracle {
_sortOrder: number[],
_limit: number,
_offset: number,
_includeNullified: boolean,
): Promise<NoteData[]> {
throw new Error('Not available.');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export class ClientExecutionContext extends ViewDataOracle {
* @param sortOrder - The order of the corresponding index in sortBy. (1: DESC, 2: ASC, 0: Do nothing)
* @param limit - The number of notes to retrieve per query.
* @param offset - The starting index for pagination.
* @param includeNullified - Whether to include nullified notes.
* @returns Array of note data.
*/
public async getNotes(
Expand All @@ -200,12 +201,13 @@ export class ClientExecutionContext extends ViewDataOracle {
sortOrder: number[],
limit: number,
offset: number,
includeNullified: boolean,
): Promise<NoteData[]> {
// Nullified pending notes are already removed from the list.
const pendingNotes = this.noteCache.getNotes(this.contractAddress, storageSlot);

const pendingNullifiers = this.noteCache.getNullifiers(this.contractAddress);
const dbNotes = await this.db.getNotes(this.contractAddress, storageSlot);
const dbNotes = await this.db.getNotes(this.contractAddress, storageSlot, includeNullified);
const dbNotesFiltered = dbNotes.filter(n => !pendingNullifiers.has((n.siloedNullifier as Fr).value));

const notes = pickNotes<NoteData>([...dbNotesFiltered, ...pendingNotes], {
Expand Down
3 changes: 2 additions & 1 deletion yarn-project/acir-simulator/src/client/db_oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ export interface DBOracle extends CommitmentsDB {
*
* @param contractAddress - The AztecAddress instance representing the contract address.
* @param storageSlot - The Fr instance representing the storage slot of the notes.
* @param includeNullified - Whether to include nullified notes.
* @returns A Promise that resolves to an array of note data.
*/
getNotes(contractAddress: AztecAddress, storageSlot: Fr): Promise<NoteData[]>;
getNotes(contractAddress: AztecAddress, storageSlot: Fr, includeNullified: boolean): Promise<NoteData[]>;

/**
* Retrieve the artifact information of a specific function within a contract.
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/acir-simulator/src/client/view_data_oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ export class ViewDataOracle extends TypedOracle {
* @param sortOrder - The order of the corresponding index in sortBy. (1: DESC, 2: ASC, 0: Do nothing)
* @param limit - The number of notes to retrieve per query.
* @param offset - The starting index for pagination.
* @param includeNullified - Whether to include nullified notes.
* @returns Array of note data.
*/
public async getNotes(
Expand All @@ -213,8 +214,9 @@ export class ViewDataOracle extends TypedOracle {
sortOrder: number[],
limit: number,
offset: number,
includeNullified: boolean,
): Promise<NoteData[]> {
const dbNotes = await this.db.getNotes(this.contractAddress, storageSlot);
const dbNotes = await this.db.getNotes(this.contractAddress, storageSlot, includeNullified);
return pickNotes<NoteData>(dbNotes, {
selects: selectBy.slice(0, numSelects).map((index, i) => ({ index, value: selectValues[i] })),
sorts: sortBy.map((index, i) => ({ index, order: sortOrder[i] })),
Expand Down
3 changes: 3 additions & 0 deletions yarn-project/aztec-nr/aztec/src/note/note_getter.nr
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ unconstrained fn get_note_internal<Note, N>(storage_slot: Field, note_interface:
[],
1, // limit
0, // offset
false, // include_nullified - TODO: add a way for get_note to retrieve nullified notes?
nventuro marked this conversation as resolved.
Show resolved Hide resolved
placeholder_note,
placeholder_fields
)[0].unwrap() // Notice: we don't allow dummies to be returned from get_note (singular).
Expand All @@ -140,6 +141,7 @@ unconstrained fn get_notes_internal<Note, N, FILTER_ARGS>(
sort_order,
options.limit,
options.offset,
options.include_nullified,
placeholder_opt_notes,
placeholder_fields
);
Expand Down Expand Up @@ -167,6 +169,7 @@ unconstrained pub fn view_notes<Note, N>(
sort_order,
options.limit,
options.offset,
options.include_nullified,
placeholder_opt_notes,
placeholder_fields
)
Expand Down
13 changes: 11 additions & 2 deletions yarn-project/aztec-nr/aztec/src/note/note_getter_options.nr
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct NoteGetterOptions<Note, N, FILTER_ARGS> {
offset: u32,
filter: fn ([Option<Note>; MAX_READ_REQUESTS_PER_CALL], FILTER_ARGS) -> [Option<Note>; MAX_READ_REQUESTS_PER_CALL],
filter_args: FILTER_ARGS,
include_nullified: bool,
}
// docs:end:NoteGetterOptions

Expand All @@ -66,6 +67,7 @@ impl<Note, N, FILTER_ARGS> NoteGetterOptions<Note, N, FILTER_ARGS> {
offset: 0,
filter: return_all_notes,
filter_args: 0,
include_nullified: false,
}
}

Expand All @@ -82,6 +84,7 @@ impl<Note, N, FILTER_ARGS> NoteGetterOptions<Note, N, FILTER_ARGS> {
offset: 0,
filter,
filter_args,
include_nullified: false,
}
}

Expand All @@ -99,16 +102,22 @@ impl<Note, N, FILTER_ARGS> NoteGetterOptions<Note, N, FILTER_ARGS> {
*self
}

// This method lets you set a limit for the maximum number of notes to be retrieved in a single query result.
// This method lets you set a limit for the maximum number of notes to be retrieved in a single query result.
pub fn set_limit(&mut self, limit: u32) -> Self {
assert(limit <= MAX_READ_REQUESTS_PER_CALL as u32);
self.limit = limit;
*self
}

// This method sets the offset value, which determines where to start retrieving notes in the query results.
// This method sets the offset value, which determines where to start retrieving notes in the query results.
pub fn set_offset(&mut self, offset: u32) -> Self {
self.offset = offset;
*self
}

// This method sets the include_nullified flag, which indicates that nullified notes should also be retrieved.
pub fn include_nullified(&mut self) -> Self {
nventuro marked this conversation as resolved.
Show resolved Hide resolved
self.include_nullified = true;
*self
}
}
7 changes: 7 additions & 0 deletions yarn-project/aztec-nr/aztec/src/note/note_viewer_options.nr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ struct NoteViewerOptions<Note, N> {
sorts: BoundedVec<Option<Sort>, N>,
limit: u32,
offset: u32,
include_nullified: bool,
}
// docs:end:NoteViewerOptions

Expand All @@ -19,6 +20,7 @@ impl<Note, N> NoteViewerOptions<Note, N> {
sorts: BoundedVec::new(Option::none()),
limit: MAX_NOTES_PER_PAGE as u32,
offset: 0,
include_nullified: false,
}
}

Expand All @@ -42,4 +44,9 @@ impl<Note, N> NoteViewerOptions<Note, N> {
self.offset = offset;
*self
}

pub fn include_nullified(&mut self) -> Self {
nventuro marked this conversation as resolved.
Show resolved Hide resolved
self.include_nullified = true;
*self
}
}
5 changes: 5 additions & 0 deletions yarn-project/aztec-nr/aztec/src/oracle/notes.nr
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ fn get_notes_oracle<N, S>(
_sort_order: [u2; N],
_limit: u32,
_offset: u32,
_include_nullified: bool,
_return_size: u32,
_placeholder_fields: [Field; S]
) -> [Field; S] {}
Expand All @@ -44,6 +45,7 @@ unconstrained fn get_notes_oracle_wrapper<N, S>(
sort_order: [u2; N],
limit: u32,
offset: u32,
include_nullified: bool,
mut placeholder_fields: [Field; S]
) -> [Field; S] {
let return_size = placeholder_fields.len() as u32;
Expand All @@ -56,6 +58,7 @@ unconstrained fn get_notes_oracle_wrapper<N, S>(
sort_order,
limit,
offset,
include_nullified,
return_size,
placeholder_fields
)
Expand All @@ -71,6 +74,7 @@ unconstrained pub fn get_notes<Note, N, M, S, NS>(
sort_order: [u2; M],
limit: u32,
offset: u32,
include_nullified: bool,
mut placeholder_opt_notes: [Option<Note>; S], // TODO: Remove it and use `limit` to initialize the note array.
placeholder_fields: [Field; NS] // TODO: Remove it and use `limit` to initialize the note array.
) -> [Option<Note>; S] {
Expand All @@ -83,6 +87,7 @@ unconstrained pub fn get_notes<Note, N, M, S, NS>(
sort_order,
limit,
offset,
include_nullified,
placeholder_fields
);
let num_notes = fields[0] as u32;
Expand Down
2 changes: 2 additions & 0 deletions yarn-project/circuit-types/src/notes/note_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ export type NoteFilter = {
storageSlot?: Fr;
/** The owner of the note (whose public key was used to encrypt the note). */
owner?: AztecAddress;
/** The status of the note. Defaults to 'active_only'. */
status?: 'active_only' | 'include_nullified'; // TODO: add 'nullified_only'
};
86 changes: 86 additions & 0 deletions yarn-project/end-to-end/src/e2e_get_notes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { AztecAddress, Wallet, toBigInt } from '@aztec/aztec.js';
import { TestContract } from '@aztec/noir-contracts';

import { setup } from './fixtures/utils.js';

describe('e2e_get_notes', () => {
// export DEBUG=aztec:e2e_get_notes
nventuro marked this conversation as resolved.
Show resolved Hide resolved
let wallet: Wallet;
let teardown: () => Promise<void>;

let contract: TestContract;
let owner: AztecAddress;

beforeAll(async () => {
({ teardown, wallet } = await setup());

contract = await TestContract.deploy(wallet).send().deployed();
owner = wallet.getCompleteAddress().address;
}, 100_000);

afterAll(() => teardown());

const VALUE = 5;

// To prevent tests from interacting with one another, we'll have each use a different storage slot.
let storageSlot: number = 2;

beforeEach(() => {
storageSlot += 1;
});

async function assertNoteIsReturned(storageSlot: number, expectedValue: number, includeNullified: boolean) {
const viewNotesResult = await contract.methods.call_view_notes(storageSlot, includeNullified).view();

// call_get_notes exposes the return value via an event since we cannot use view() with it.
const tx = contract.methods.call_get_notes(storageSlot, includeNullified).send();
await tx.wait();

const logs = (await tx.getUnencryptedLogs()).logs;
expect(logs.length).toBe(1);

const getNotesResult = toBigInt(logs[0].log.data);

expect(viewNotesResult).toEqual(getNotesResult);
expect(viewNotesResult).toEqual(BigInt(expectedValue));
}

async function assertNoReturnValue(storageSlot: number, includeNullified: boolean) {
await expect(contract.methods.call_view_notes(storageSlot, includeNullified).view()).rejects.toThrow('is_some');
await expect(contract.methods.call_get_notes(storageSlot, includeNullified).send().wait()).rejects.toThrow(
'is_some',
);
}

describe('active note only', () => {
const includeNullified = false;

it('returns active notes', async () => {
await contract.methods.call_create_note(VALUE, owner, storageSlot).send().wait();
await assertNoteIsReturned(storageSlot, VALUE, includeNullified);
});

it('does not return nullified notes', async () => {
await contract.methods.call_create_note(VALUE, owner, storageSlot).send().wait();
await contract.methods.call_destroy_note(storageSlot).send().wait();

await assertNoReturnValue(storageSlot, includeNullified);
});
});

describe('active and nullified notes', () => {
nventuro marked this conversation as resolved.
Show resolved Hide resolved
const includeNullified = true;

it('returns active notes', async () => {
await contract.methods.call_create_note(VALUE, owner, storageSlot).send().wait();
await assertNoteIsReturned(storageSlot, VALUE, includeNullified);
});

it('returns nullified notes', async () => {
await contract.methods.call_create_note(VALUE, owner, storageSlot).send().wait();
await contract.methods.call_destroy_note(storageSlot).send().wait();

await assertNoteIsReturned(storageSlot, VALUE, includeNullified);
}, 30_000);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ type = "contract"
[dependencies]
aztec = { path = "../../../aztec-nr/aztec" }
field_note = { path = "../../../aztec-nr/field-note" }
value_note = { path = "../../../aztec-nr/value-note" }
token_portal_content_hash_lib = { path = "../token_portal_content_hash_lib" }
Loading
Loading