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(noir): better NoteGetterOptions. #1695

Merged
merged 9 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ export class ClientTxExecutionContext {
*
* @param contractAddress - The contract address.
* @param storageSlot - The storage slot.
* @param numSelects - The number of valid selects in selectBy and selectValues.
* @param selectBy - An array of indices of the fields to selects.
* @param selectValues - The values to match.
* @param sortBy - An array of indices of the fields to sort.
* @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.
Expand All @@ -118,6 +121,9 @@ export class ClientTxExecutionContext {
public async getNotes(
contractAddress: AztecAddress,
storageSlot: ACVMField,
numSelects: number,
selectBy: ACVMField[],
selectValues: ACVMField[],
sortBy: ACVMField[],
sortOrder: ACVMField[],
limit: number,
Expand All @@ -136,6 +142,8 @@ export class ClientTxExecutionContext {

// Nullified pending notes are already removed from the list.
const notes = pickNotes([...dbNotesFiltered, ...pendingNotes], {
selectBy: selectBy.slice(0, numSelects).map(field => +field),
selectValues: selectValues.slice(0, numSelects).map(field => fromACVMField(field)),
sortBy: sortBy.map(field => +field),
sortOrder: sortOrder.map(field => +field),
limit,
Expand Down
94 changes: 84 additions & 10 deletions yarn-project/acir-simulator/src/client/pick_notes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Fr } from '@aztec/foundation/fields';
import { SortOrder, pickNotes } from './pick_notes.js';

describe('getNotes', () => {
const expectSortedNotes = (notes: { preimage: Fr[] }[], ...expected: [number, bigint[]][]) => {
const expectNotesFields = (notes: { preimage: Fr[] }[], ...expected: [number, bigint[]][]) => {
expect(notes.length).toBe(expected[0][1].length);
expected.forEach(([fieldIndex, fields]) => {
for (let i = 0; i < notes.length; ++i) {
Expand All @@ -12,6 +12,13 @@ describe('getNotes', () => {
});
};

const expectNotes = (notes: { preimage: Fr[] }[], expected: bigint[][]) => {
expect(notes.length).toBe(expected.length);
notes.forEach((note, i) => {
expect(note.preimage.map(p => p.value)).toEqual(expected[i]);
});
};

const createNote = (preimage: bigint[]) => ({
preimage: preimage.map(f => new Fr(f)),
});
Expand All @@ -30,29 +37,29 @@ describe('getNotes', () => {
{
const options = { sortBy: [1], sortOrder: [SortOrder.ASC] };
const result = pickNotes(notes, options);
expectSortedNotes(result, [1, [0n, 1n, 5n, 5n, 5n, 6n]]);
expectNotesFields(result, [1, [0n, 1n, 5n, 5n, 5n, 6n]]);
}

// Sort 1st field in descending order.
{
const options = { sortBy: [1] };
const result = pickNotes(notes, options);
expectSortedNotes(result, [1, [6n, 5n, 5n, 5n, 1n, 0n]], [0, [7n, 4n, 6n, 6n, 2n, 0n]]);
expectNotesFields(result, [1, [6n, 5n, 5n, 5n, 1n, 0n]], [0, [7n, 4n, 6n, 6n, 2n, 0n]]);
}

// Sort 1st and 0th fields in descending order.
{
const options = { sortBy: [1, 0] };
const result = pickNotes(notes, options);
expectSortedNotes(result, [1, [6n, 5n, 5n, 5n, 1n, 0n]], [0, [7n, 6n, 6n, 4n, 2n, 0n]]);
expectNotesFields(result, [1, [6n, 5n, 5n, 5n, 1n, 0n]], [0, [7n, 6n, 6n, 4n, 2n, 0n]]);
}

// Sort 1st field in descending order
// Then 0th field in ascending order
{
const options = { sortBy: [1, 0], sortOrder: [SortOrder.DESC, SortOrder.ASC] };
const result = pickNotes(notes, options);
expectSortedNotes(
expectNotesFields(
result,
[1, [6n, 5n, 5n, 5n, 1n, 0n]],
[0, [7n, 4n, 6n, 6n, 2n, 0n]],
Expand All @@ -66,7 +73,7 @@ describe('getNotes', () => {
{
const options = { sortBy: [1, 0, 2], sortOrder: [SortOrder.DESC, SortOrder.ASC, SortOrder.DESC] };
const result = pickNotes(notes, options);
expectSortedNotes(
expectNotesFields(
result,
[1, [6n, 5n, 5n, 5n, 1n, 0n]],
[0, [7n, 4n, 6n, 6n, 2n, 0n]],
Expand All @@ -84,26 +91,93 @@ describe('getNotes', () => {
{
const options = { sortBy, limit: 3 };
const result = pickNotes(notes, options);
expectSortedNotes(result, [0, [8n, 6n, 5n]]);
expectNotesFields(result, [0, [8n, 6n, 5n]]);
}

{
const options = { sortBy, limit: 3, offset: 1 };
const result = pickNotes(notes, options);
expectSortedNotes(result, [0, [6n, 5n, 2n]]);
expectNotesFields(result, [0, [6n, 5n, 2n]]);
}

{
const options = { sortBy, limit: 3, offset: 4 };
const result = pickNotes(notes, options);
expectSortedNotes(result, [0, [0n]]);
expectNotesFields(result, [0, [0n]]);
}
});

it('should not change order if sortOrder is NADA', () => {
const notes = [createNote([2n]), createNote([8n]), createNote([6n]), createNote([5n]), createNote([0n])];
const options = { sortBy: [0], sortOrder: [SortOrder.NADA] };
const result = pickNotes(notes, options);
expectSortedNotes(result, [0, [2n, 8n, 6n, 5n, 0n]]);
expectNotesFields(result, [0, [2n, 8n, 6n, 5n, 0n]]);
});

it('should get notes that have the required fields', () => {
const notes = [
createNote([2n, 1n, 3n]),
createNote([1n, 2n, 3n]),
createNote([3n, 2n, 0n]),
createNote([2n, 2n, 0n]),
createNote([2n, 3n, 3n]),
];

{
const options = { selectBy: [0], selectValues: [new Fr(2n)] };
const result = pickNotes(notes, options);
expectNotes(result, [
[2n, 1n, 3n],
[2n, 2n, 0n],
[2n, 3n, 3n],
]);
}

{
const options = { selectBy: [0, 2], selectValues: [new Fr(2n), new Fr(3n)] };
const result = pickNotes(notes, options);
expectNotes(result, [
[2n, 1n, 3n],
[2n, 3n, 3n],
]);
}

{
const options = { selectBy: [1, 2], selectValues: [new Fr(2n), new Fr(3n)] };
const result = pickNotes(notes, options);
expectNotes(result, [[1n, 2n, 3n]]);
}

{
const options = { selectBy: [1], selectValues: [new Fr(5n)] };
const result = pickNotes(notes, options);
expectNotes(result, []);
}

{
const options = { selectBy: [0, 1], selectValues: [new Fr(2), new Fr(5n)] };
const result = pickNotes(notes, options);
expectNotes(result, []);
}
});

it('should get sorted matching notes', () => {
const notes = [
createNote([2n, 1n, 3n]),
createNote([4n, 5n, 8n]),
createNote([7n, 6n, 8n]),
createNote([6n, 5n, 2n]),
createNote([0n, 0n, 8n]),
createNote([6n, 5n, 8n]),
];

const options = { selectBy: [2], selectValues: [new Fr(8n)], sortBy: [1], sortOrder: [SortOrder.ASC] };
const result = pickNotes(notes, options);
expectNotes(result, [
[0n, 0n, 8n],
[4n, 5n, 8n],
[6n, 5n, 8n],
[7n, 6n, 8n],
]);
});
});
17 changes: 15 additions & 2 deletions yarn-project/acir-simulator/src/client/pick_notes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ export enum SortOrder {
* Options for selecting items from the database.
*/
interface GetOptions {
/**
* An array of indices of the fields to select.
* Default: empty array.
*/
selectBy?: number[];
LeilaWang marked this conversation as resolved.
Show resolved Hide resolved
/**
* An array of values of the corresponding fields to select and match.
* Default: empty array.
*/
selectValues?: Fr[];
/**
* An array of indices of the fields to sort.
* Default: empty array.
Expand Down Expand Up @@ -45,6 +55,9 @@ interface BasicNoteData {
preimage: Fr[];
}

const selectNotes = <T extends BasicNoteData>(notes: T[], selectBy: number[], selectValues: Fr[]): T[] =>
notes.filter(note => selectBy.every((fieldIndex, i) => note.preimage[fieldIndex]?.equals(selectValues[i])));

const sortNotes = (a: Fr[], b: Fr[], sortBy: number[], sortOrder: number[], level = 0): number => {
const index = sortBy[level];
if (sortBy[level] === undefined) return 0;
Expand All @@ -65,9 +78,9 @@ const sortNotes = (a: Fr[], b: Fr[], sortBy: number[], sortOrder: number[], leve
*/
export function pickNotes<T extends BasicNoteData>(
notes: T[],
{ sortBy = [], sortOrder = [], limit = 0, offset = 0 }: GetOptions,
{ selectBy = [], selectValues = [], sortBy = [], sortOrder = [], limit = 0, offset = 0 }: GetOptions,
) {
return notes
return selectNotes(notes, selectBy, selectValues)
.sort((a, b) => sortNotes(a.preimage, b.preimage, sortBy, sortOrder))
.slice(offset, limit ? offset + limit : undefined);
}
15 changes: 13 additions & 2 deletions yarn-project/acir-simulator/src/client/private_execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,19 @@ export class PrivateFunctionExecution {
const { publicKey, partialAddress } = await this.context.db.getCompleteAddress(address);
return [publicKey.x, publicKey.y, partialAddress].map(toACVMField);
},
getNotes: ([slot], sortBy, sortOrder, [limit], [offset], [returnSize]) =>
this.context.getNotes(this.contractAddress, slot, sortBy, sortOrder, +limit, +offset, +returnSize),
getNotes: ([slot], [numSelects], selectBy, selectValues, sortBy, sortOrder, [limit], [offset], [returnSize]) =>
this.context.getNotes(
this.contractAddress,
slot,
+numSelects,
selectBy,
selectValues,
sortBy,
sortOrder,
+limit,
+offset,
+returnSize,
),
getRandomField: () => Promise.resolve(toACVMField(Fr.random())),
notifyCreatedNote: ([storageSlot], preimage, [innerNoteHash]) => {
this.context.pushNewNote(
Expand Down
15 changes: 13 additions & 2 deletions yarn-project/acir-simulator/src/client/unconstrained_execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,19 @@ export class UnconstrainedFunctionExecution {
const { publicKey, partialAddress } = await this.context.db.getCompleteAddress(address);
return [publicKey.x, publicKey.y, partialAddress].map(toACVMField);
},
getNotes: ([slot], sortBy, sortOrder, [limit], [offset], [returnSize]) =>
this.context.getNotes(this.contractAddress, slot, sortBy, sortOrder, +limit, +offset, +returnSize),
getNotes: ([slot], [numSelects], selectBy, selectValues, sortBy, sortOrder, [limit], [offset], [returnSize]) =>
this.context.getNotes(
this.contractAddress,
slot,
+numSelects,
selectBy,
selectValues,
sortBy,
sortOrder,
+limit,
+offset,
+returnSize,
),
getRandomField: () => Promise.resolve(toACVMField(Fr.random())),
debugLog: (...params) => {
this.log(oracleDebugCallToFormattedStr(params));
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec.js/src/abis/ecdsa_account_contract.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ contract EasyPrivateToken {
let balances = storage.balances;

// Return the sum of all notes in the set.
balance_utils::get_balance(balances.at(owner).storage_slot)
balance_utils::get_balance(balances.at(owner).set)
}

// Computes note hash and nullifier.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
mod filter;

use dep::std::hash::pedersen;
use dep::aztec::note::note_interface::NoteInterface;
use dep::aztec::note::note_header::NoteHeader;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ contract Escrow {
AddressNote,
AddressNoteMethods,
ADDRESS_NOTE_LEN,
filter::filter_by_owner_and_address,
};

use crate::storage::Storage;
Expand Down Expand Up @@ -62,7 +61,7 @@ contract Escrow {
let storage = Storage::init();

// We don't remove note from the owners set. If a note exists, the owner and recipient are legit.
let options = NoteGetterOptions::with_filter(filter_by_owner_and_address, [sender, this]);
let options = NoteGetterOptions::new().select(0, sender).select(1, this).set_limit(1);
let notes = storage.owners.get_notes(&mut context, options);
let note = notes[0].unwrap_unchecked();
// Filter is not constrained. We still need to check if the note is what we expected.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ contract NonNativeToken {
let storage = Storage::init();
let owner_balance = storage.balances.at(owner);

balance_utils::get_balance(owner_balance.storage_slot)
balance_utils::get_balance(owner_balance)
}

// Computes note hash and nullifier.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ contract PendingCommitments {
// Libs
use dep::value_note::{
balance_utils,
filter::{filter_notes_min_sum, get_1_note, get_max_notes},
filter::filter_notes_min_sum,
value_note::{VALUE_NOTE_LEN, ValueNote, ValueNoteMethods},
};

Expand Down Expand Up @@ -173,7 +173,7 @@ contract PendingCommitments {

let owner_balance = storage.balances.at(owner);

let options = NoteGetterOptions::with_filter(get_1_note, 0);
let options = NoteGetterOptions::new().set_limit(1);
let note = owner_balance.get_notes(&mut context, options)[0].unwrap();

assert(expected_value == note.value);
Expand All @@ -198,7 +198,7 @@ contract PendingCommitments {

let owner_balance = storage.balances.at(owner);

let options = NoteGetterOptions::with_filter(get_max_notes, 0);
let options = NoteGetterOptions::new();
let maybe_notes = owner_balance.get_notes(&mut context, options);

assert(maybe_notes[0].is_none());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ contract PokeableToken {
balance_utils,
utils::{send_note, spend_notes},
value_note::{VALUE_NOTE_LEN, ValueNoteMethods, ValueNote},
filter::get_max_notes,
};
use dep::aztec::abi;
use dep::aztec::abi::PrivateContextInputs;
Expand Down Expand Up @@ -71,7 +70,7 @@ contract PokeableToken {
// Pick from the set of sender's notes.
let sender_balance = storage.balances.at(sender);

let options = NoteGetterOptions::with_filter(get_max_notes, 0);
let options = NoteGetterOptions::new();
let maybe_notes = sender_balance.get_notes(&mut context, options);
let mut note_sum = 0;
for i in 0..maybe_notes.len() {
Expand Down Expand Up @@ -107,7 +106,7 @@ contract PokeableToken {
let sender_balance = storage.balances.at(sender);

// Return the sum of all notes in the set.
balance_utils::get_balance(sender_balance.storage_slot)
balance_utils::get_balance(sender_balance)
}

// Computes note hash and nullifier.
Expand Down
Loading