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!: make note_getter return BoundedVec instead of an Option array #7050

Merged
merged 14 commits into from
Jun 19, 2024
28 changes: 28 additions & 0 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,34 @@ Aztec is in full-speed development. Literally every version breaks compatibility

## TBD

### [Aztec.nr] `note_getter` returns `BoundedVec`

The `get_notes` and `view_notes` function no longer return an array of options (i.e. `[Option<Note>, N_NOTES]`) but instead a `BoundedVec<Note, N_NOTES>`. This better conveys the useful property the old array had of having all notes collapsed at the beginning of the array, which allows for powerful optimizations and gate count reduction when setting the `options.limit` value.

A `BoundedVec` has a `max_len()`, which equals the number of elements it can hold, and a `len()`, which equals the number of elements it currently holds. Since `len()` is typically not knwon at compile time, iterating over a `BoundedVec` looks slightly different than iterating over an array of options:

```diff
- let option_notes = get_notes(options);
- for i in 0..option_notes.len() {
- if option_notes[i].is_some() {
- let note = option_notes[i].unwrap_unchecked();
- }
- }
+ let notes = get_notes(options);
+ for i in 0..notes.max_len() {
+ if i < notes.len() {
+ let note = notes.get_unchecked(i);
+ }
+ }
```

To further reduce gate count, you can iterate over `options.limit` instead of `max_len()`, since `options.limit` is guaranteed to be larger than `len()` and smaller or equal to `max_len()`:
nventuro marked this conversation as resolved.
Show resolved Hide resolved

```diff
- for i in 0..notes.max_len() {
+ for i in 0..options.limit {
```

### [Aztec.nr] `options.limit` has to be constant

The `limit` parameter in `NoteGetterOptions` and `NoteViewerOptions` is now required to be a compile-time constant. This allows performing loops over this value, which leads to reduced circuit gate counts when setting a `limit` value.
Expand Down
31 changes: 19 additions & 12 deletions noir-projects/aztec-nr/aztec/src/note/note_getter.nr
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub fn get_notes<Note, N, M, FILTER_ARGS>(
context: &mut PrivateContext,
storage_slot: Field,
options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N, M> {
) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> {
let opt_notes = get_notes_internal(storage_slot, options);

constrain_get_notes_internal(context, storage_slot, opt_notes, options)
Expand All @@ -115,8 +115,8 @@ fn constrain_get_notes_internal<Note, N, M, FILTER_ARGS>(
storage_slot: Field,
opt_notes: [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N, M> {
let mut returned_notes = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> {
let mut returned_notes = BoundedVec::new();

// The filter is applied first to avoid pushing note read requests for notes we're not interested in. Note that
// while the filter function can technically mutate the contents of the notes (as opposed to simply removing some),
Expand All @@ -126,7 +126,6 @@ fn constrain_get_notes_internal<Note, N, M, FILTER_ARGS>(
let filter_args = options.filter_args;
let filtered_notes = filter_fn(opt_notes, filter_args);

let mut num_notes = 0;
let mut prev_fields = [0; N];
for i in 0..filtered_notes.len() {
let opt_note = filtered_notes[i];
Expand All @@ -151,14 +150,12 @@ fn constrain_get_notes_internal<Note, N, M, FILTER_ARGS>(
// resulting in a smaller circuit and faster proving times.
// We write at returned_notes[num_notes] because num_notes is only advanced when we have a value in
// filtered_notes.
returned_notes[num_notes] = Option::some(note);
num_notes += 1;
returned_notes.push(note);
};
}

assert(num_notes <= options.limit, "Got more notes than limit.");

assert(num_notes != 0, "Cannot return zero notes");
assert(returned_notes.len() <= options.limit, "Got more notes than limit.");
assert(returned_notes.len() != 0, "Cannot return zero notes");

returned_notes
}
Expand Down Expand Up @@ -223,12 +220,13 @@ unconstrained fn get_notes_internal<Note, N, M, FILTER_ARGS>(
unconstrained pub fn view_notes<Note, N, M>(
storage_slot: Field,
options: NoteViewerOptions<Note, N, M>
) -> [Option<Note>; MAX_NOTES_PER_PAGE] where Note: NoteInterface<N, M> {
) -> BoundedVec<Note, MAX_NOTES_PER_PAGE> where Note: NoteInterface<N, M> {
let (num_selects, select_by_indexes, select_by_offsets, select_by_lengths, select_values, select_comparators, sort_by_indexes, sort_by_offsets, sort_by_lengths, sort_order) = flatten_options(options.selects, options.sorts);
let placeholder_opt_notes = [Option::none(); MAX_NOTES_PER_PAGE];
let placeholder_fields = [0; VIEW_NOTE_ORACLE_RETURN_LENGTH];
let placeholder_note_length = [0; N];
oracle::notes::get_notes(

let notes_array = oracle::notes::get_notes(
storage_slot,
num_selects,
select_by_indexes,
Expand All @@ -246,7 +244,16 @@ unconstrained pub fn view_notes<Note, N, M>(
placeholder_opt_notes,
placeholder_fields,
placeholder_note_length
)
);

let mut notes = BoundedVec::new();
for i in 0..notes_array.len() {
if notes_array[i].is_some() {
notes.push(notes_array[i].unwrap_unchecked());
}
}

notes
}

unconstrained fn flatten_options<Note, N>(
Expand Down
21 changes: 17 additions & 4 deletions noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ fn build_valid_note(value: Field) -> MockNote {
MockNote::new(value).contract_address(contract_address).storage_slot(storage_slot).build()
}

fn assert_equivalent_vec_and_array<T, N>(vec: BoundedVec<T, N>, arr: [Option<T>; N]) where T: Eq {
let mut count = 0;

for i in 0..N {
if arr[i].is_some() {
assert_eq(arr[i].unwrap(), vec.get(count));
count += 1;
}
}

assert_eq(count, vec.len());
}

#[test]
fn processes_single_note() {
let mut context = setup();
Expand All @@ -32,7 +45,7 @@ fn processes_single_note() {
let options = NoteGetterOptions::new();
let returned = constrain_get_notes_internal(&mut context, storage_slot, notes_to_constrain, options);

assert_eq(returned, notes_to_constrain);
assert_equivalent_vec_and_array(returned, notes_to_constrain);
assert_eq(context.note_hash_read_requests.len(), 1);
}

Expand All @@ -47,7 +60,7 @@ fn processes_many_notes() {
let options = NoteGetterOptions::new();
let returned = constrain_get_notes_internal(&mut context, storage_slot, notes_to_constrain, options);

assert_eq(returned, notes_to_constrain);
assert_equivalent_vec_and_array(returned, notes_to_constrain);
assert_eq(context.note_hash_read_requests.len(), 2);
}

Expand Down Expand Up @@ -76,7 +89,7 @@ fn collapses_notes_at_the_beginning_of_the_array() {
expected[5] = Option::some(build_valid_note(5));
expected[6] = Option::some(build_valid_note(6));

assert_eq(returned, expected);
assert_equivalent_vec_and_array(returned, expected);
}

#[test(should_fail_with="Cannot return zero notes")]
Expand Down Expand Up @@ -132,7 +145,7 @@ fn applies_filter_before_constraining() {
let mut expected = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
expected[0] = Option::some(build_valid_note(42));

assert_eq(returned, expected);
assert_equivalent_vec_and_array(returned, expected);
assert_eq(context.note_hash_read_requests.len(), 1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl<Note> PrivateImmutable<Note, UnconstrainedContext> {
// docs:start:view_note
unconstrained pub fn view_note<N, M>(self) -> Note where Note: NoteInterface<N, M> {
let mut options = NoteViewerOptions::new();
view_notes(self.storage_slot, options.set_limit(1))[0].unwrap()
view_notes(self.storage_slot, options.set_limit(1)).get(0)
}
// docs:end:view_note
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<Note> PrivateMutable<Note, UnconstrainedContext> {
// docs:start:view_note
unconstrained pub fn view_note<N, M>(self) -> Note where Note: NoteInterface<N, M> {
let mut options = NoteViewerOptions::new();
view_notes(self.storage_slot, options.set_limit(1))[0].unwrap()
view_notes(self.storage_slot, options.set_limit(1)).get(0)
}
// docs:end:view_note
}
8 changes: 3 additions & 5 deletions noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,8 @@ impl<Note> PrivateSet<Note, &mut PrivateContext> {
pub fn get_notes<N, M, FILTER_ARGS>(
self,
options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N, M> {
let storage_slot = self.storage_slot;
let opt_notes = get_notes(self.context, storage_slot, options);
opt_notes
) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> {
get_notes(self.context, self.storage_slot, options)
}
// docs:end:get_notes
}
Expand All @@ -71,7 +69,7 @@ impl<Note> PrivateSet<Note, UnconstrainedContext> {
unconstrained pub fn view_notes<N, M>(
self,
options: NoteViewerOptions<Note, N, M>
) -> [Option<Note>; MAX_NOTES_PER_PAGE] where Note: NoteInterface<N, M> {
) -> BoundedVec<Note, MAX_NOTES_PER_PAGE> where Note: NoteInterface<N, M> {
view_notes(self.storage_slot, options)
}
// docs:end:view_notes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ impl<Context> EasyPrivateUint<&mut PrivateContext> {

// docs:start:get_notes
let options = NoteGetterOptions::with_filter(filter_notes_min_sum, subtrahend as Field);
let maybe_notes = self.set.get_notes(options);
let notes = self.set.get_notes(options);
// docs:end:get_notes

let mut minuend: u64 = 0;
for i in 0..options.limit {
if maybe_notes[i].is_some() {
let note = maybe_notes[i].unwrap_unchecked();
if i < notes.len() {
let note = notes.get_unchecked(i);

// Removes the note from the owner's set of notes.
// docs:start:remove
Expand Down
14 changes: 7 additions & 7 deletions noir-projects/aztec-nr/value-note/src/balance_utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ unconstrained pub fn get_balance_with_offset(set: PrivateSet<ValueNote, Unconstr
let mut balance = 0;
// docs:start:view_notes
let mut options = NoteViewerOptions::new();
let opt_notes = set.view_notes(options.set_offset(offset));
let notes = set.view_notes(options.set_offset(offset));
// docs:end:view_notes
let len = opt_notes.len();
for i in 0..len {
if opt_notes[i].is_some() {
balance += opt_notes[i].unwrap_unchecked().value;
for i in 0..options.limit {
if i < notes.len() {
balance += notes.get_unchecked(i).value;
}
}
if (opt_notes[len - 1].is_some()) {
balance += get_balance_with_offset(set, offset + opt_notes.len() as u32);

if (notes.len() == options.limit) {
balance += get_balance_with_offset(set, offset + options.limit);
}

balance
Expand Down
6 changes: 3 additions & 3 deletions noir-projects/aztec-nr/value-note/src/utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ pub fn decrement_by_at_most(
outgoing_viewer: AztecAddress
) -> Field {
let options = create_note_getter_options_for_decreasing_balance(max_amount);
let opt_notes = balance.get_notes(options);
let notes = balance.get_notes(options);

let mut decremented = 0;
for i in 0..options.limit {
if opt_notes[i].is_some() {
let note = opt_notes[i].unwrap_unchecked();
if i < notes.len() {
let note = notes.get_unchecked(i);

decremented += destroy_note(balance, note);
}
Expand Down
82 changes: 82 additions & 0 deletions noir-projects/noir-contracts/compile.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
echo "app_subscription_contract"
nventuro marked this conversation as resolved.
Show resolved Hide resolved
nargo compile --silence-warnings --package app_subscription_contract
echo "auth_contract"
nargo compile --silence-warnings --package auth_contract
echo "avm_initializer_test_contract"
nargo compile --silence-warnings --package avm_initializer_test_contract
echo "avm_test_contract"
nargo compile --silence-warnings --package avm_test_contract
echo "benchmarking_contract"
nargo compile --silence-warnings --package benchmarking_contract
echo "card_game_contract"
nargo compile --silence-warnings --package card_game_contract
echo "child_contract"
nargo compile --silence-warnings --package child_contract
echo "claim_contract"
nargo compile --silence-warnings --package claim_contract
echo "contract_class_registerer_contract"
nargo compile --silence-warnings --package contract_class_registerer_contract
echo "contract_instance_deployer_contract"
nargo compile --silence-warnings --package contract_instance_deployer_contract
echo "counter_contract"
nargo compile --silence-warnings --package counter_contract
echo "crowdfunding_contract"
nargo compile --silence-warnings --package crowdfunding_contract
echo "delegated_on_contract"
nargo compile --silence-warnings --package delegated_on_contract
echo "delegator_contract"
nargo compile --silence-warnings --package delegator_contract
echo "docs_example_contract"
nargo compile --silence-warnings --package docs_example_contract
echo "easy_private_token_contract"
nargo compile --silence-warnings --package easy_private_token_contract
echo "easy_private_voting_contract"
nargo compile --silence-warnings --package easy_private_voting_contract
echo "ecdsa_account_contract"
nargo compile --silence-warnings --package ecdsa_account_contract
echo "escrow_contract"
nargo compile --silence-warnings --package escrow_contract
echo "fpc_contract"
nargo compile --silence-warnings --package fpc_contract
echo "gas_token_contract"
nargo compile --silence-warnings --package gas_token_contract
echo "import_test_contract"
nargo compile --silence-warnings --package import_test_contract
echo "inclusion_proofs_contract"
nargo compile --silence-warnings --package inclusion_proofs_contract
echo "key_registry_contract"
nargo compile --silence-warnings --package key_registry_contract
echo "lending_contract"
nargo compile --silence-warnings --package lending_contract
echo "multi_call_entrypoint_contract"
nargo compile --silence-warnings --package multi_call_entrypoint_contract
echo "parent_contract"
nargo compile --silence-warnings --package parent_contract
echo "pending_note_hashes_contract"
nargo compile --silence-warnings --package pending_note_hashes_contract
echo "price_feed_contract"
nargo compile --silence-warnings --package price_feed_contract
echo "schnorr_account_contract"
nargo compile --silence-warnings --package schnorr_account_contract
echo "schnorr_hardcoded_account_contract"
nargo compile --silence-warnings --package schnorr_hardcoded_account_contract
echo "schnorr_single_key_account_contract"
nargo compile --silence-warnings --package schnorr_single_key_account_contract
echo "stateful_test_contract"
nargo compile --silence-warnings --package stateful_test_contract
echo "static_child_contract"
nargo compile --silence-warnings --package static_child_contract
echo "static_parent_contract"
nargo compile --silence-warnings --package static_parent_contract
echo "test_contract"
nargo compile --silence-warnings --package test_contract
echo "test_log_contract"
nargo compile --silence-warnings --package test_log_contract
echo "token_blacklist_contract"
nargo compile --silence-warnings --package token_blacklist_contract
echo "token_bridge_contract"
nargo compile --silence-warnings --package token_bridge_contract
echo "token_contract"
nargo compile --silence-warnings --package token_contract
echo "uniswap_contract"
nargo compile --silence-warnings --package uniswap_contract
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract Benchmarking {
let owner_notes = storage.notes.at(owner);
let mut getter_options = NoteGetterOptions::new();
let notes = owner_notes.get_notes(getter_options.set_limit(1).set_offset(index));
let note = notes[0].unwrap_unchecked();
let note = notes.get_unchecked(0);
owner_notes.remove(note);
increment(owner_notes, note.value, owner, outgoing_viewer);
}
Expand Down
Loading
Loading