diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index 4beb7e9fc14..ea8ee634133 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -12,6 +12,34 @@ Aztec is in full-speed development. Literally every version breaks compatibility Earlier we had just one function - `transfer()` which used authwits to handle the case where a contract/user wants to transfer funds on behalf of another user. To reduce circuit sizes and proof times, we are breaking up `transfer` and introducing a dedicated `transferFrom()` function like in the ERC20 standard. +### [Aztec.nr] `note_getter` returns `BoundedVec` + +The `get_notes` and `view_notes` function no longer return an array of options (i.e. `[Option, N_NOTES]`) but instead a `BoundedVec`. 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 or equal to `len()`, and smaller or equal to `max_len()`: + +```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. diff --git a/noir-projects/aztec-nr/aztec/src/note/note_getter.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter.nr index cef9498d6c8..53648875124 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter.nr @@ -104,7 +104,7 @@ pub fn get_notes( context: &mut PrivateContext, storage_slot: Field, options: NoteGetterOptions -) -> [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface { +) -> BoundedVec where Note: NoteInterface { let opt_notes = get_notes_internal(storage_slot, options); constrain_get_notes_internal(context, storage_slot, opt_notes, options) @@ -115,8 +115,8 @@ fn constrain_get_notes_internal( storage_slot: Field, opt_notes: [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL], options: NoteGetterOptions -) -> [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface { - let mut returned_notes = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL]; +) -> BoundedVec where Note: NoteInterface { + 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), @@ -126,7 +126,6 @@ fn constrain_get_notes_internal( 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]; @@ -151,14 +150,12 @@ fn constrain_get_notes_internal( // 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 } @@ -223,12 +220,13 @@ unconstrained fn get_notes_internal( unconstrained pub fn view_notes( storage_slot: Field, options: NoteViewerOptions -) -> [Option; MAX_NOTES_PER_PAGE] where Note: NoteInterface { +) -> BoundedVec where Note: NoteInterface { 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, @@ -246,7 +244,16 @@ unconstrained pub fn view_notes( 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( diff --git a/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr index 9ee0738312e..42d28e25a05 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr @@ -21,6 +21,19 @@ fn build_valid_note(value: Field) -> MockNote { MockNote::new(value).contract_address(cheatcodes::get_contract_address()).storage_slot(storage_slot).build() } +fn assert_equivalent_vec_and_array(vec: BoundedVec, arr: [Option; 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 env = setup(); @@ -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); } @@ -48,7 +61,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); } @@ -78,7 +91,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")] @@ -137,7 +150,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); } diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr index e6c01bd3212..0c310180181 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr @@ -76,7 +76,7 @@ impl PrivateImmutable { // docs:start:view_note unconstrained pub fn view_note(self) -> Note where Note: NoteInterface { 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 } diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr index 1a66e07692c..56b752b6cef 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr @@ -118,7 +118,7 @@ impl PrivateMutable { // docs:start:view_note unconstrained pub fn view_note(self) -> Note where Note: NoteInterface { 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 } diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr index 07a2552be26..1e9125d9bb7 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr @@ -58,10 +58,8 @@ impl PrivateSet { pub fn get_notes( self, options: NoteGetterOptions - ) -> [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface { - let storage_slot = self.storage_slot; - let opt_notes = get_notes(self.context, storage_slot, options); - opt_notes + ) -> BoundedVec where Note: NoteInterface { + get_notes(self.context, self.storage_slot, options) } // docs:end:get_notes } @@ -71,7 +69,7 @@ impl PrivateSet { unconstrained pub fn view_notes( self, options: NoteViewerOptions - ) -> [Option; MAX_NOTES_PER_PAGE] where Note: NoteInterface { + ) -> BoundedVec where Note: NoteInterface { view_notes(self.storage_slot, options) } // docs:end:view_notes diff --git a/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr b/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr index 0f7103ac9ff..ff23dd0e159 100644 --- a/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr +++ b/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr @@ -41,13 +41,13 @@ impl 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 diff --git a/noir-projects/aztec-nr/value-note/src/balance_utils.nr b/noir-projects/aztec-nr/value-note/src/balance_utils.nr index 35b35d7e2dd..0f4a6715f18 100644 --- a/noir-projects/aztec-nr/value-note/src/balance_utils.nr +++ b/noir-projects/aztec-nr/value-note/src/balance_utils.nr @@ -12,16 +12,16 @@ unconstrained pub fn get_balance_with_offset(set: PrivateSet 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); } diff --git a/noir-projects/noir-contracts/contracts/benchmarking_contract/src/main.nr b/noir-projects/noir-contracts/contracts/benchmarking_contract/src/main.nr index a5a27fa3f57..7e2dde51083 100644 --- a/noir-projects/noir-contracts/contracts/benchmarking_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/benchmarking_contract/src/main.nr @@ -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); } diff --git a/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr b/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr index 01d06077fb7..e8ae03dd953 100644 --- a/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr +++ b/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr @@ -123,12 +123,17 @@ impl Deck<&mut PrivateContext> { pub fn get_cards(&mut self, cards: [Card; N]) -> [CardNote; N] { let options = NoteGetterOptions::with_filter(filter_cards, cards); - let maybe_notes = self.set.get_notes(options); + let notes = self.set.get_notes(options); + + // This array will hold the notes that correspond to each of the requested cards. It begins empty (with all the + // options being none) and we gradually fill it up as we find the matching notes. let mut found_cards = [Option::none(); N]; + for i in 0..options.limit { - if maybe_notes[i].is_some() { - let card_note = CardNote::from_note(maybe_notes[i].unwrap_unchecked()); + if i < notes.len() { + let card_note = CardNote::from_note(notes.get_unchecked(i)); + // For each note that we read, we search for a matching card that we have not already marked as found. for j in 0..cards.len() { if found_cards[j].is_none() & (cards[j].strength == card_note.card.strength) @@ -139,6 +144,7 @@ impl Deck<&mut PrivateContext> { } } + // And then we assert that we did indeed find all cards, since found_cards and cards have the same length. found_cards.map( |card_note: Option| { assert(card_note.is_some(), "Card not found"); @@ -156,16 +162,19 @@ impl Deck<&mut PrivateContext> { } impl Deck { - unconstrained pub fn view_cards(self, offset: u32) -> [Option; MAX_NOTES_PER_PAGE] { + unconstrained pub fn view_cards(self, offset: u32) -> BoundedVec { let mut options = NoteViewerOptions::new(); - let opt_notes = self.set.view_notes(options.set_offset(offset)); - let mut opt_cards = [Option::none(); MAX_NOTES_PER_PAGE]; - - for i in 0..opt_notes.len() { - opt_cards[i] = opt_notes[i].map(|note: ValueNote| Card::from_field(note.value)); + let notes = self.set.view_notes(options.set_offset(offset)); + + // TODO: ideally we'd do let cards = notes.map(|note| Cards::from_field(note.value)); + // see https://github.com/noir-lang/noir/pull/5250 + let mut cards = BoundedVec::new(); + cards.len = notes.len(); + for i in 0..notes.len() { + cards.storage[i] = Card::from_field(notes.get_unchecked(i).value); } - opt_cards + cards } } diff --git a/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr b/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr index bfb982eef57..df8318028a1 100644 --- a/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr @@ -117,13 +117,12 @@ contract CardGame { game_storage.write(game_data); } - unconstrained fn view_collection_cards(owner: AztecAddress, offset: u32) -> pub [Option; MAX_NOTES_PER_PAGE] { + unconstrained fn view_collection_cards(owner: AztecAddress, offset: u32) -> pub BoundedVec { let collection = storage.collections.at(owner); - collection.view_cards(offset) } - unconstrained fn view_game_cards(game: u32, player: AztecAddress, offset: u32) -> pub [Option; MAX_NOTES_PER_PAGE] { + unconstrained fn view_game_cards(game: u32, player: AztecAddress, offset: u32) -> pub BoundedVec { let game_deck = storage.game_decks.at(game as Field).at(player); game_deck.view_cards(offset) diff --git a/noir-projects/noir-contracts/contracts/child_contract/src/main.nr b/noir-projects/noir-contracts/contracts/child_contract/src/main.nr index 6a34f458cce..ab9483cce2a 100644 --- a/noir-projects/noir-contracts/contracts/child_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/child_contract/src/main.nr @@ -65,7 +65,7 @@ contract Child { let mut options = NoteGetterOptions::new(); options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1); let notes = storage.a_map_with_private_values.at(owner).get_notes(options); - notes[0].unwrap_unchecked().value + notes.get_unchecked(0).value } // Increments `current_value` by `new_value` diff --git a/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr b/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr index e296d211aab..b843313be4b 100644 --- a/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr @@ -59,7 +59,7 @@ contract Counter { let counter_slot = Counter::storage().counters.slot; let owner_slot = derive_storage_slot_in_map(counter_slot, owner); let mut options = NoteViewerOptions::new(); - let opt_notes: [Option; MAX_NOTES_PER_PAGE] = view_notes(owner_slot, options); - assert(opt_notes[0].unwrap().value == 5); + let notes: BoundedVec = view_notes(owner_slot, options); + assert(notes.get(0).value == 5); } } diff --git a/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr b/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr index 7b7a30fb200..da7a24ce09b 100644 --- a/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr @@ -35,7 +35,7 @@ contract DelegatedOn { let mut options = NoteGetterOptions::new(); options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1); let notes = storage.a_map_with_private_values.at(owner).get_notes(options); - notes[0].unwrap_unchecked().value + notes.get_unchecked(0).value } unconstrained fn view_public_value() -> pub Field { diff --git a/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr b/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr index b2ee7e2f556..8cb753c23e9 100644 --- a/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr @@ -35,7 +35,7 @@ contract Delegator { let mut options = NoteGetterOptions::new(); options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1); let notes = storage.a_map_with_private_values.at(owner).get_notes(options); - notes[0].unwrap_unchecked().value + notes.get_unchecked(0).value } unconstrained fn view_public_value() -> pub Field { diff --git a/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr b/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr index b56307349b3..9653946054c 100644 --- a/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr @@ -211,17 +211,15 @@ contract DocsExample { } // docs:start:state_vars-NoteGetterOptionsComparatorExampleNoir - unconstrained fn read_note(amount: Field, comparator: u8) -> pub [Option; 10] { + unconstrained fn read_note(amount: Field, comparator: u8) -> pub BoundedVec { let mut options = NoteViewerOptions::new(); - let notes = storage.set.view_notes( + storage.set.view_notes( options.select( CardNote::properties().points, amount, Option::some(comparator) ) - ); - - notes + ) } // docs:end:state_vars-NoteGetterOptionsComparatorExampleNoir diff --git a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr index 0e0fe7e2ddd..372b11fbd5b 100644 --- a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr @@ -55,8 +55,7 @@ contract InclusionProofs { if (nullified) { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let notes = private_values.get_notes(options); - let maybe_note = notes[0]; + let note = private_values.get_notes(options).get_unchecked(0); // docs:end:get_note_from_pxe // 2) Prove the note inclusion @@ -66,7 +65,7 @@ contract InclusionProofs { context.get_header() }; // docs:start:prove_note_inclusion - header.prove_note_inclusion(maybe_note.unwrap_unchecked()); + header.prove_note_inclusion(note); // docs:end:prove_note_inclusion } @@ -107,8 +106,7 @@ contract InclusionProofs { if (fail_case) { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let notes = private_values.get_notes(options); - let maybe_note = notes[0]; + let note = private_values.get_notes(options).get_unchecked(0); let header = if (use_block_number) { context.get_header_at(block_number) @@ -116,7 +114,7 @@ contract InclusionProofs { context.get_header() }; // docs:start:prove_note_not_nullified - header.prove_note_not_nullified(maybe_note.unwrap_unchecked(), &mut context); + header.prove_note_not_nullified(note, &mut context); // docs:end:prove_note_not_nullified } @@ -134,8 +132,7 @@ contract InclusionProofs { if (nullified) { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let notes = private_values.get_notes(options); - let note = notes[0].unwrap(); + let note = private_values.get_notes(options).get(0); // 2) Prove the note validity let header = if (use_block_number) { @@ -155,7 +152,7 @@ contract InclusionProofs { let mut options = NoteGetterOptions::new(); options = options.set_limit(1); let notes = private_values.get_notes(options); - let note = notes[0].unwrap(); + let note = notes.get(0); private_values.remove(note); } diff --git a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr index e6901b1a319..b8789b55e6f 100644 --- a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr @@ -274,8 +274,8 @@ contract Parent { let counter_slot = Child::storage().a_map_with_private_values.slot; let owner_slot = derive_storage_slot_in_map(counter_slot, owner); let mut options = NoteViewerOptions::new(); - let opt_notes: [Option; MAX_NOTES_PER_PAGE] = view_notes(owner_slot, options); - let note_value = opt_notes[0].unwrap().value; + let notes: BoundedVec = view_notes(owner_slot, options); + let note_value = notes.get(0).value; assert(note_value == value_to_set); assert(note_value == result); // Get value from child through parent diff --git a/noir-projects/noir-contracts/contracts/pending_note_hashes_contract/src/main.nr b/noir-projects/noir-contracts/contracts/pending_note_hashes_contract/src/main.nr index 0fb30e0d728..6f5076d4ddf 100644 --- a/noir-projects/noir-contracts/contracts/pending_note_hashes_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/pending_note_hashes_contract/src/main.nr @@ -40,11 +40,11 @@ contract PendingNoteHashes { let options = NoteGetterOptions::with_filter(filter_notes_min_sum, amount); // get note inserted above - let maybe_notes = owner_balance.get_notes(options); + let notes = owner_balance.get_notes(options); - let note0 = maybe_notes[0].unwrap(); + let note0 = notes.get(0); assert(note.value == note0.value); - assert(maybe_notes[1].is_none()); + assert(notes.len() == 1); owner_balance.remove(note0); @@ -58,10 +58,13 @@ contract PendingNoteHashes { let options = NoteGetterOptions::with_filter(filter_notes_min_sum, amount); // get note (note inserted at bottom of function shouldn't exist yet) - let maybe_notes = owner_balance.get_notes(options); + let notes = owner_balance.get_notes(options); - assert(maybe_notes[0].is_none()); - assert(maybe_notes[1].is_none()); + // TODO https://github.com/AztecProtocol/aztec-packages/issues/7059: clean this test up - we won't actually hit + // this assert because the same one exists in get_notes (we always return at least one note), so all lines after + // this one are effectively dead code. We should find a different way to test what this function attempts to + // test. + assert(notes.len() == 0); let header = context.get_header(); let owner_npk_m_hash = header.get_npk_m_hash(&mut context, owner); @@ -139,7 +142,7 @@ contract PendingNoteHashes { let mut options = NoteGetterOptions::new(); options = options.set_limit(1); - let note = owner_balance.get_notes(options)[0].unwrap(); + let note = owner_balance.get_notes(options).get(0); assert(expected_value == note.value); @@ -154,10 +157,11 @@ contract PendingNoteHashes { let owner_balance = storage.balances.at(owner); let options = NoteGetterOptions::new(); - let maybe_notes = owner_balance.get_notes(options); + let notes = owner_balance.get_notes(options); - assert(maybe_notes[0].is_none()); - assert(maybe_notes[1].is_none()); + // TODO https://github.com/AztecProtocol/aztec-packages/issues/7059: review what this test aimed to test, as + // it's now superfluous: get_notes never returns 0 notes. + assert(notes.len() == 0); } // Test pending note hashes with note insertion done in a nested call @@ -384,7 +388,7 @@ contract PendingNoteHashes { let notes = owner_balance.get_notes(NoteGetterOptions::new()); for i in 0..max_notes_per_call() { - let note = notes[i].unwrap(); + let note = notes.get(i); owner_balance.remove(note); } } diff --git a/noir-projects/noir-contracts/contracts/static_child_contract/src/main.nr b/noir-projects/noir-contracts/contracts/static_child_contract/src/main.nr index 89c31eb8482..9ed71ce290e 100644 --- a/noir-projects/noir-contracts/contracts/static_child_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/static_child_contract/src/main.nr @@ -73,7 +73,7 @@ contract StaticChild { Option::none() ).set_limit(1); let notes = storage.a_private_value.get_notes(options); - notes[0].unwrap_unchecked().value + notes.get_unchecked(0).value } // Increments `current_value` by `new_value` diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr index 79046b74aa0..0ec38297670 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -111,9 +111,9 @@ contract Test { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let opt_notes: [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = get_notes(&mut context, storage_slot, options); + let notes: BoundedVec = get_notes(&mut context, storage_slot, options); - opt_notes[0].unwrap().value + notes.get(0).value } #[aztec(private)] @@ -127,9 +127,9 @@ contract Test { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let opt_notes: [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = get_notes(&mut context, storage_slot, options); + let notes: BoundedVec = get_notes(&mut context, storage_slot, options); - [opt_notes[0].unwrap().value, opt_notes[1].unwrap().value] + [notes.get(0).value, notes.get(1).value] } unconstrained fn call_view_notes(storage_slot: Field, active_or_nullified: bool) -> pub Field { @@ -142,9 +142,9 @@ contract Test { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let opt_notes: [Option; MAX_NOTES_PER_PAGE] = view_notes(storage_slot, options); + let notes: BoundedVec = view_notes(storage_slot, options); - opt_notes[0].unwrap().value + notes.get(0).value } unconstrained fn call_view_notes_many(storage_slot: Field, active_or_nullified: bool) -> pub [Field; 2] { @@ -157,9 +157,9 @@ contract Test { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } - let opt_notes: [Option; MAX_NOTES_PER_PAGE] = view_notes(storage_slot, options); + let notes: BoundedVec = view_notes(storage_slot, options); - [opt_notes[0].unwrap().value, opt_notes[1].unwrap().value] + [notes.get(0).value, notes.get(1).value] } #[aztec(private)] @@ -169,9 +169,9 @@ contract Test { ); let options = NoteGetterOptions::new(); - let opt_notes: [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = get_notes(&mut context, storage_slot, options); + let notes: BoundedVec = get_notes(&mut context, storage_slot, options); - let note = opt_notes[0].unwrap(); + let note = notes.get(0); destroy_note(&mut context, note); } @@ -475,7 +475,7 @@ contract Test { let mut options = NoteGetterOptions::new(); options = options.select(TestNote::properties().value, secret_hash, Option::none()).set_limit(1); let notes = notes_set.get_notes(options); - let note = notes[0].unwrap_unchecked(); + let note = notes.get_unchecked(0); notes_set.remove(note); } diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr index fd4590e5b6a..defab65772e 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr @@ -173,7 +173,7 @@ contract TokenBlacklist { Option::none() ).set_limit(1); let notes = pending_shields.get_notes(options); - let note = notes[0].unwrap_unchecked(); + let note = notes.get_unchecked(0); // Remove the note from the pending shields set pending_shields.remove(note); diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr index 56b63bc7590..a5436a93d95 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr @@ -39,16 +39,15 @@ impl BalancesMap { let mut balance = U128::from_integer(0); // docs:start:view_notes let mut options = NoteViewerOptions::new(); - let opt_notes = self.map.at(owner).view_notes(options.set_offset(offset)); + let notes = self.map.at(owner).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 = balance + opt_notes[i].unwrap_unchecked().get_amount(); + for i in 0..options.limit { + if i < notes.len() { + balance = balance + notes.get_unchecked(i).get_amount(); } } - if (opt_notes[len - 1].is_some()) { - balance = balance + self.balance_of_with_offset(owner, offset + opt_notes.len() as u32); + if (notes.len() == options.limit) { + balance = balance + self.balance_of_with_offset(owner, offset + options.limit); } balance @@ -84,13 +83,13 @@ impl BalancesMap { ) -> OuterNoteEmission where T: NoteInterface + OwnedNote { // docs:start:get_notes let options = NoteGetterOptions::with_filter(filter_notes_min_sum, subtrahend); - let maybe_notes = self.map.at(owner).get_notes(options); + let notes = self.map.at(owner).get_notes(options); // docs:end:get_notes let mut minuend: U128 = U128::from_integer(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. // This will call the the `compute_nullifer` function of the `token_note` diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr index fca6ed10b8f..b263abeff01 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -281,7 +281,7 @@ contract Token { Option::none() ).set_limit(1); let notes = pending_shields.get_notes(options); - let note = notes[0].unwrap_unchecked(); + let note = notes.get_unchecked(0); // Remove the note from the pending shields set pending_shields.remove(note); @@ -457,13 +457,13 @@ contract Token { let balances_slot = Token::storage().balances.slot; let recipient_slot = derive_storage_slot_in_map(balances_slot, recipient); let mut options = NoteViewerOptions::new(); - let opt_notes: [Option; MAX_NOTES_PER_PAGE] = view_notes(recipient_slot, options); - assert(opt_notes[0].unwrap().amount.to_field() == transfer_amount); + let notes: BoundedVec = view_notes(recipient_slot, options); + assert(notes.get(0).amount.to_field() == transfer_amount); let owner_slot = derive_storage_slot_in_map(balances_slot, owner); let mut options = NoteViewerOptions::new(); - let opt_notes: [Option; MAX_NOTES_PER_PAGE] = view_notes(owner_slot, options); - assert(opt_notes[0].unwrap().amount.to_field() == mint_amount - transfer_amount); + let notes: BoundedVec = view_notes(owner_slot, options); + assert(notes.get(0).amount.to_field() == mint_amount - transfer_amount); } } // docs:end:token_all \ No newline at end of file diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr b/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr index cada57a7497..a13206360c3 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr @@ -42,16 +42,15 @@ impl BalancesMap { let mut balance = U128::from_integer(0); // docs:start:view_notes let mut options = NoteViewerOptions::new(); - let opt_notes = self.map.at(owner).view_notes(options.set_offset(offset)); + let notes = self.map.at(owner).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 = balance + opt_notes[i].unwrap_unchecked().get_amount(); + for i in 0..options.limit { + if i < notes.len() { + balance = balance + notes.get_unchecked(i).get_amount(); } } - if (opt_notes[len - 1].is_some()) { - balance = balance + self.balance_of_with_offset(owner, offset + opt_notes.len() as u32); + if (notes.len() == options.limit) { + balance = balance + self.balance_of_with_offset(owner, offset + options.limit); } balance @@ -87,13 +86,13 @@ impl BalancesMap { ) -> OuterNoteEmission where T: NoteInterface + OwnedNote { // docs:start:get_notes let options = NoteGetterOptions::with_filter(filter_notes_min_sum, subtrahend); - let maybe_notes = self.map.at(owner).get_notes(options); + let notes = self.map.at(owner).get_notes(options); // docs:end:get_notes let mut minuend: U128 = U128::from_integer(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. // This will call the the `compute_nullifer` function of the `token_note` diff --git a/yarn-project/end-to-end/src/e2e_card_game.test.ts b/yarn-project/end-to-end/src/e2e_card_game.test.ts index ef9fa465666..24629f1e8f7 100644 --- a/yarn-project/end-to-end/src/e2e_card_game.test.ts +++ b/yarn-project/end-to-end/src/e2e_card_game.test.ts @@ -44,13 +44,13 @@ interface Game { current_round: bigint; } -interface NoirOption { - _is_some: boolean; - _value: T; +interface NoirBoundedVec { + storage: T[]; + len: bigint; } -function unwrapOptions(options: NoirOption[]): T[] { - return options.filter((option: any) => option._is_some).map((option: any) => option._value); +function boundedVecToArray(boundedVec: NoirBoundedVec): T[] { + return boundedVec.storage.slice(0, Number(boundedVec.len)); } // Game settings. @@ -149,7 +149,7 @@ describe('e2e_card_game', () => { // docs:end:send_tx const collection = await contract.methods.view_collection_cards(firstPlayer, 0).simulate({ from: firstPlayer }); const expected = getPackedCards(0, seed); - expect(unwrapOptions(collection)).toMatchObject(expected); + expect(boundedVecToArray(collection)).toMatchObject(expected); }); describe('game join', () => { @@ -161,7 +161,7 @@ describe('e2e_card_game', () => { contract.methods.buy_pack(seed).send().wait(), contractAsSecondPlayer.methods.buy_pack(seed).send().wait(), ]); - firstPlayerCollection = unwrapOptions( + firstPlayerCollection = boundedVecToArray( await contract.methods.view_collection_cards(firstPlayer, 0).simulate({ from: firstPlayer }), ); }); @@ -180,8 +180,8 @@ describe('e2e_card_game', () => { ).rejects.toThrow(`Assertion failed: Cannot return zero notes`); const collection = await contract.methods.view_collection_cards(firstPlayer, 0).simulate({ from: firstPlayer }); - expect(unwrapOptions(collection)).toHaveLength(1); - expect(unwrapOptions(collection)).toMatchObject([firstPlayerCollection[1]]); + expect(boundedVecToArray(collection)).toHaveLength(1); + expect(boundedVecToArray(collection)).toMatchObject([firstPlayerCollection[1]]); expect((await contract.methods.view_game(GAME_ID).simulate({ from: firstPlayer })) as Game).toMatchObject({ players: [ @@ -204,10 +204,10 @@ describe('e2e_card_game', () => { }); it('should start games', async () => { - const secondPlayerCollection = unwrapOptions( + const secondPlayerCollection = boundedVecToArray( (await contract.methods .view_collection_cards(secondPlayer, 0) - .simulate({ from: secondPlayer })) as NoirOption[], + .simulate({ from: secondPlayer })) as NoirBoundedVec, ); await Promise.all([ @@ -257,15 +257,15 @@ describe('e2e_card_game', () => { contractAsThirdPlayer.methods.buy_pack(seed).send().wait(), ]); - firstPlayerCollection = unwrapOptions( + firstPlayerCollection = boundedVecToArray( await contract.methods.view_collection_cards(firstPlayer, 0).simulate({ from: firstPlayer }), ); - secondPlayerCollection = unwrapOptions( + secondPlayerCollection = boundedVecToArray( await contract.methods.view_collection_cards(secondPlayer, 0).simulate({ from: secondPlayer }), ); - thirdPlayerCOllection = unwrapOptions( + thirdPlayerCOllection = boundedVecToArray( await contract.methods.view_collection_cards(thirdPlayer, 0).simulate({ from: thirdPlayer }), ); }); @@ -319,8 +319,8 @@ describe('e2e_card_game', () => { await contractFor(winner).methods.claim_cards(GAME_ID, game.rounds_cards.map(cardToField)).send().wait(); - const winnerCollection = unwrapOptions( - (await contract.methods.view_collection_cards(winner, 0).simulate({ from: winner })) as NoirOption[], + const winnerCollection = boundedVecToArray( + (await contract.methods.view_collection_cards(winner, 0).simulate({ from: winner })) as NoirBoundedVec, ); const winnerGameDeck = [winnerCollection[0], winnerCollection[3]]; diff --git a/yarn-project/end-to-end/src/e2e_note_getter.test.ts b/yarn-project/end-to-end/src/e2e_note_getter.test.ts index c26ecffe30d..be87db5bd48 100644 --- a/yarn-project/end-to-end/src/e2e_note_getter.test.ts +++ b/yarn-project/end-to-end/src/e2e_note_getter.test.ts @@ -3,18 +3,18 @@ import { DocsExampleContract, TestContract } from '@aztec/noir-contracts.js'; import { setup } from './fixtures/utils.js'; -interface NoirOption { - _is_some: boolean; - _value: T; +interface NoirBoundedVec { + storage: T[]; + len: bigint; +} + +function boundedVecToArray(boundedVec: NoirBoundedVec): T[] { + return boundedVec.storage.slice(0, Number(boundedVec.len)); } const sortFunc = (a: any, b: any) => a.points > b.points ? 1 : a.points < b.points ? -1 : a.randomness > b.randomness ? 1 : -1; -function unwrapOptions(options: NoirOption[]): T[] { - return options.filter((option: any) => option._is_some).map((option: any) => option._value); -} - describe('e2e_note_getter', () => { let wallet: Wallet; let teardown: () => Promise; @@ -60,7 +60,7 @@ describe('e2e_note_getter', () => { ]); expect( - unwrapOptions(returnEq) + boundedVecToArray(returnEq) .map(({ points, randomness }: any) => ({ points, randomness })) .sort(sortFunc), ).toStrictEqual( @@ -71,7 +71,7 @@ describe('e2e_note_getter', () => { ); expect( - unwrapOptions(returnNeq) + boundedVecToArray(returnNeq) .map(({ points, randomness }: any) => ({ points, randomness })) .sort(sortFunc), ).toStrictEqual( @@ -89,7 +89,7 @@ describe('e2e_note_getter', () => { ); expect( - unwrapOptions(returnLt) + boundedVecToArray(returnLt) .map(({ points, randomness }: any) => ({ points, randomness })) .sort(sortFunc), ).toStrictEqual( @@ -103,7 +103,7 @@ describe('e2e_note_getter', () => { ); expect( - unwrapOptions(returnGt) + boundedVecToArray(returnGt) .map(({ points, randomness }: any) => ({ points, randomness })) .sort(sortFunc), ).toStrictEqual( @@ -116,7 +116,7 @@ describe('e2e_note_getter', () => { ); expect( - unwrapOptions(returnLte) + boundedVecToArray(returnLte) .map(({ points, randomness }: any) => ({ points, randomness })) .sort(sortFunc), ).toStrictEqual( @@ -132,7 +132,7 @@ describe('e2e_note_getter', () => { ); expect( - unwrapOptions(returnGte) + boundedVecToArray(returnGte) .map(({ points, randomness }: any) => ({ points, randomness })) .sort(sortFunc), ).toStrictEqual( @@ -179,7 +179,7 @@ describe('e2e_note_getter', () => { async function assertNoReturnValue(storageSlot: number, activeOrNullified: boolean) { await expect(contract.methods.call_view_notes(storageSlot, activeOrNullified).simulate()).rejects.toThrow( - 'is_some', + 'index < self.len', // from BoundedVec::get ); await expect(contract.methods.call_get_notes(storageSlot, activeOrNullified).prove()).rejects.toThrow( `Assertion failed: Cannot return zero notes`,