feat: allow get_notes to return zero notes (#7621)

`get_notes` used to fail if no notes were found ever since
#4988 was merged.
The argument there was that since no constraints are applied, and note
non-existence cannot be proved, `get_notes` returning no notes is always
a mistake.

However, what that mistake means depends on the caller. Since we don't
want for the caller to have to pass an error message to `get_notes`, I
think it's better to return no notes and leave it up to the caller to
decide what to do. `BoundedVec` is a safe container: `get` will result
in errors if used with indices out of bounds.

I changed the callsites where we did `get_unchecked(0)` to `get(0)` -
this means that the errors now change from 'got 0 notes' to 'out of
bounds access'. Neither error message is great, so I didn't bother
improving this. Maybe we could have a `get` variant that received an
error message (like `Option::expect`), but we don't need to worry about
this for now.

Closes #7059
(indirectly due to how we need to change certain tests to address this)

noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr

@@ -133,7 +133,6 @@ fn constrain_get_notes_internal<Note, let N: u32, let M: u32, FILTER_ARGS>(
     // results in reduced gate counts when setting a limit value, since we guarantee that the limit is an upper bound
     // for the runtime length, and can therefore have fewer loop iterations.
     assert(notes.len() <= options.limit, "Got more notes than limit.");
-    assert(notes.len() != 0, "Cannot return zero notes");
 
     let mut prev_fields = [0; N];
     for i in 0..options.limit {

noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr

@@ -96,15 +96,15 @@ fn collapses_notes_at_the_beginning_of_the_array() {
     assert_equivalent_vec_and_array(returned, expected);
 }
 
-#[test(should_fail_with="Cannot return zero notes")]
-fn rejects_zero_notes() {
+fn can_return_zero_notes() {
     let mut env = setup();
     let mut context = env.private();
 
     let opt_notes: [Option<MockNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
 
     let options = NoteGetterOptions::new();
-    let _ = constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+    let returned = constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+    assert_eq(returned.len(), 0);
 }
 
 #[test(should_fail_with="Got more notes than limit.")]

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.get_unchecked(0);
+        let note = notes.get(0);
         owner_notes.remove(note);
         increment(owner_notes, note.value, owner, outgoing_viewer);
     }

noir-projects/noir-contracts/contracts/child_contract/src/main.nr

@@ -64,7 +64,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.get_unchecked(0).value
+        notes.get(0).value
     }
 
     // Increments `current_value` by `new_value`

noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr

@@ -34,7 +34,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.get_unchecked(0).value
+        notes.get(0).value
     }
 
     unconstrained fn view_public_value() -> pub Field {

noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr

@@ -55,7 +55,7 @@ contract InclusionProofs {
         if (nullified) {
             options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED);
         }
-        let note = private_values.get_notes(options).get_unchecked(0);
+        let note = private_values.get_notes(options).get(0);
         // docs:end:get_note_from_pxe
 
         // 2) Prove the note inclusion
@@ -106,7 +106,7 @@ contract InclusionProofs {
         if (fail_case) {
             options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED);
         }
-        let note = private_values.get_notes(options).get_unchecked(0);
+        let note = private_values.get_notes(options).get(0);
 
         let header = if (use_block_number) {
             context.get_header_at(block_number)

noir-projects/noir-contracts/contracts/pending_note_hashes_contract/src/main.nr

@@ -59,18 +59,14 @@ contract PendingNoteHashes {
         // get note (note inserted at bottom of function shouldn't exist yet)
         let notes = owner_balance.get_notes(options);
 
-        // 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);
 
         // Insert note
         let mut note = ValueNote::new(amount, owner_npk_m_hash);
-        owner_balance.insert(&mut note).emit(encode_and_encrypt_note(&mut context, context.msg_sender(), owner));
+        owner_balance.insert(&mut note).discard();
 
         0
     }
@@ -158,8 +154,6 @@ contract PendingNoteHashes {
         let options = NoteGetterOptions::new();
         let notes = owner_balance.get_notes(options);
 
-        // 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);
     }
 

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.get_unchecked(0).value
+        notes.get(0).value
     }
 
     // Increments `current_value` by `new_value`

noir-projects/noir-contracts/contracts/test_contract/src/main.nr

@@ -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.get_unchecked(0);
+        let note = notes.get(0);
         notes_set.remove(note);
     }
 

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.get_unchecked(0);
+        let note = notes.get(0);
         // Remove the note from the pending shields set
         pending_shields.remove(note);
 

noir-projects/noir-contracts/contracts/token_contract/src/main.nr

@@ -295,7 +295,7 @@ contract Token {
             Option::none()
         ).set_limit(1);
         let notes = pending_shields.get_notes(options);
-        let note = notes.get_unchecked(0);
+        let note = notes.get(0);
         // Remove the note from the pending shields set
         pending_shields.remove(note);
 

noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr

@@ -88,7 +88,7 @@ unconstrained fn mint_private_success() {
     utils::check_private_balance(token_contract_address, owner, mint_amount);
 }
 
-#[test(should_fail_with="Cannot return zero notes")]
+#[test(should_fail_with="Attempted to read past end of BoundedVec")]
 unconstrained fn mint_private_failure_double_spend() {
     // Setup without account contracts. We are not using authwits here, so dummy accounts are enough
     let (env, token_contract_address, owner, recipient) = utils::setup(/* with_account_contracts */ false);

yarn-project/end-to-end/src/e2e_blacklist_token_contract/minting.test.ts

@@ -111,7 +111,7 @@ describe('e2e_blacklist_token_contract mint', () => {
           'The note has been destroyed.',
         );
         await expect(asset.methods.redeem_shield(wallets[0].getAddress(), amount, secret).prove()).rejects.toThrow(
-          `Assertion failed: Cannot return zero notes`,
+          `Assertion failed: Attempted to read past end of BoundedVec`,
         );
       });
 

yarn-project/end-to-end/src/e2e_card_game.test.ts

@@ -183,7 +183,7 @@ describe('e2e_card_game', () => {
           .join_game(GAME_ID, [cardToField(firstPlayerCollection[0]), cardToField(firstPlayerCollection[1])])
           .send()
           .wait(),
-      ).rejects.toThrow(`Assertion failed: Cannot return zero notes`);
+      ).rejects.toThrow(`Card not found`);
 
       const collection = await contract.methods.view_collection_cards(firstPlayer, 0).simulate({ from: firstPlayer });
       expect(boundedVecToArray(collection)).toHaveLength(1);

yarn-project/end-to-end/src/e2e_delegate_calls/delegate.test.ts

@@ -30,7 +30,7 @@ describe.skip('e2e_delegate_calls', () => {
 
       await expect(
         delegatedOnContract.methods.get_private_value(sentValue, wallet.getCompleteAddress().address).simulate(),
-      ).rejects.toThrow(`Assertion failed: Cannot return zero notes 'num_notes != 0'`);
+      ).rejects.toThrow(`Assertion failed: Attempted to read past end of BoundedVec 'num_notes != 0'`);
 
       expect(delegatorValue).toEqual(sentValue);
     });

yarn-project/end-to-end/src/e2e_note_getter.test.ts

@@ -182,7 +182,7 @@ describe('e2e_note_getter', () => {
         'index < self.len', // from BoundedVec::get
       );
       await expect(contract.methods.call_get_notes(storageSlot, activeOrNullified).prove()).rejects.toThrow(
-        `Assertion failed: Cannot return zero notes`,
+        `Assertion failed: Attempted to read past end of BoundedVec`,
       );
     }
 

yarn-project/end-to-end/src/e2e_pending_note_hashes_contract.test.ts

@@ -108,9 +108,7 @@ describe('e2e_pending_note_hashes_contract', () => {
       )
       .send()
       .wait();
-    await expect(deployedContract.methods.get_note_zero_balance(owner).prove()).rejects.toThrow(
-      `Assertion failed: Cannot return zero notes`,
-    );
+    await deployedContract.methods.get_note_zero_balance(owner).send().wait();
 
     await expectNoteHashesSquashedExcept(0);
     await expectNullifiersSquashedExcept(0);
@@ -244,9 +242,8 @@ describe('e2e_pending_note_hashes_contract', () => {
       )
       .send()
       .wait();
-    await expect(deployedContract.methods.get_note_zero_balance(owner).prove()).rejects.toThrow(
-      `Assertion failed: Cannot return zero notes`,
-    );
+
+    await deployedContract.methods.get_note_zero_balance(owner).send().wait();
 
     // second TX creates 1 note, but it is squashed!
     await expectNoteHashesSquashedExcept(0);

yarn-project/end-to-end/src/e2e_token_contract/minting.test.ts

@@ -101,7 +101,7 @@ describe('e2e_token_contract minting', () => {
           'The note has been destroyed.',
         );
         await expect(asset.methods.redeem_shield(accounts[0].address, amount, secret).simulate()).rejects.toThrow(
-          `Assertion failed: Cannot return zero notes`,
+          `Assertion failed: Attempted to read past end of BoundedVec`,
         );
       });
 

yarn-project/simulator/src/client/private_execution.test.ts

@@ -1092,13 +1092,12 @@ describe('Private Execution test suite', () => {
       const artifact = getFunctionArtifact(PendingNoteHashesContractArtifact, 'test_bad_get_then_insert_flat');
 
       const args = [amountToTransfer, owner];
-      await expect(() =>
-        runSimulator({
-          args: args,
-          artifact: artifact,
-          contractAddress,
-        }),
-      ).rejects.toThrow(`Assertion failed: Cannot return zero notes`);
+      // This will throw if we read the note before it was inserted
+      await runSimulator({
+        args: args,
+        artifact: artifact,
+        contractAddress,
+      });
     });
   });
 
@@ -1126,7 +1125,7 @@ describe('Private Execution test suite', () => {
       oracle.getNotes.mockResolvedValue([]);
 
       await expect(() => runSimulator({ artifact, args })).rejects.toThrow(
-        `Assertion failed: Cannot return zero notes`,
+        `Assertion failed: Attempted to read past end of BoundedVec`,
       );
     });
   });