feat!: emit nullifiers in set.get_notes

docs/docs/guides/smart_contracts/writing_contracts/how_to_prove_history.md

@@ -56,6 +56,10 @@ Retrieve the note from the user's PXE.
 
 In this example, the user's notes are stored in a map called `private_values`. We retrieve this map, then select 1 note from it with the value of `1`.
 
+:::note 
+We use `view_notes` since we just need the note data - we'll construct the inclusion proofs ourselves. We could have also received the note data via e.g. function parameters. Had we used `get_notes`, that would've created a second redundant inclusion proof for the note, as well as emitted a nullifier to ensure the note is active. However, using `view_notes` means we must constrain the retrieved note manually.
+:::
+
 ## 4. Prove that a note was included in a specified block
 
 To prove that a note existed in a specified block, call `prove_note_inclusion_at` as shown in this example:
@@ -64,7 +68,7 @@ To prove that a note existed in a specified block, call `prove_note_inclusion_at
 
 This function takes in 3 arguments:
 
-1. The note (`maybe_note.unwrap_unchecked()`). Here, `unwrap_unchecked()` returns the inner value without asserting `self.is_some()`
+1. The note
 2. The block number
 3. Private context
 

docs/docs/migration_notes.md

@@ -152,6 +152,22 @@ The function `debug_log_array_with_prefix` has been removed. Use `debug_log_form
 + debug_log_format("Prefix {}", my_array);
 ```
 
+### [Aztec.nr] `PrivateSet.get_notes()` now also deletes them, `remove()` has been removed
+
+To prevent accidental errors in which notes are read from a set but not validated to be current (by emitting a nullifier), `get_notes()` now also nullifies the read notes. Most applications deleted notes immediately by calling `remove()`, which no longer exists - this is now taken care of automatically.
+
+```diff
+let maybe_notes = storage.set.get_notes(options);
+- for i in 0..maybe_notes.len() {
+-     if maybe_notes[i].is_some() {
+-         let note = maybe_notes[i].unwrap_unchecked();
+-         storage.set.remove(note);
+-     }
+}
+```
+
+`get_notes()` will soon be renamed to `pop_notes()` to make this new behavior more obvious to the reader.
+
 ## 0.39.0
 
 ### [Aztec.nr] Mutable delays in `SharedMutable`
@@ -173,7 +189,7 @@ What used to be called encryption public key is now master incoming viewing publ
 
 ## 0.38.0
 
-### [Aztec.nr] Emitting encrypted logs
+### [Aztec.nr] Emmiting encrypted logs
 
 The `emit_encrypted_log` function is now a context method.
 

docs/docs/reference/smart_contract_reference/storage/private_state.md

@@ -61,7 +61,7 @@ A `PrivateSet` may point to _multiple_ notes at a time. The "current value" of a
 :::note
 The term "some accumulation" is intentionally vague. The interpretation of the "current value" of a `PrivateSet` must be expressed by the smart contract developer. A common use case for a `PrivateSet` is to represent the sum of a collection of values (in which case 'accumulation' is 'summation').
 
-Think of a ZCash balance (or even a Bitcoin balance). The "current value" of a user's ZCash balance is the sum of all unspent (not-yet nullified) notes belonging to that user. To modify the "current value" of a `PrivateSet` state variable, is to [`insert`](#insert) new notes into the `PrivateSet`, or [`remove`](#remove) notes from that set.
+Think of a ZCash balance (or even a Bitcoin balance). The "current value" of a user's ZCash balance is the sum of all unspent (not-yet nullified) notes belonging to that user. To modify the "current value" of a `PrivateSet` state variable, is to [`insert`](#insert) new notes into the `PrivateSet`, or to remove (via [`get_notes`](#get_notes)) notes from that set.
 :::
 
 Interestingly, if a developer requires a private state to be modifiable by users who _aren't_ privy to the value of that state, a `PrivateSet` is a very useful type. The `insert` method allows new notes to be added to the `PrivateSet` without knowing any of the other notes in the set! (Like posting an envelope into a post box, you don't know what else is in there!).
@@ -212,16 +212,6 @@ The usage is similar to using the `insert` method with the difference that this
 
 #include_code insert_from_public /noir-projects/noir-contracts/contracts/token_contract/src/main.nr rust
 
-### `remove`
-
-Will remove a note from the `PrivateSet` if it previously has been read from storage, e.g. you have fetched it through a `get_notes` call. This is useful when you want to remove a note that you have previously read from storage and do not have to read it again.
-
-Nullifiers are emitted when reading values to make sure that they are up to date.
-
-An example of how to use this operation is visible in the `easy_private_state`:
-
-#include_code remove /noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr rust
-
 ### `get_notes`
 
 This function returns the notes the account has access to.
@@ -234,6 +224,8 @@ An example of such options is using the [filter_notes_min_sum](https://github.co
 
 #include_code get_notes /noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr rust
 
+To ensure that the notes being read are current and have not yet been nullified, `get_notes` automatically emits nullifiers for them. This means that reading notes from a set also destroys said notes! While this might seem strange at first, it is actually consistent with most use cases: for example, token notes are consumed when used in order to perform a transfer.
+
 ### `view_notes`
 
 Functionally similar to [`get_notes`](#get_notes), but executed unconstrained and can be used by the wallet to fetch notes for use by front-ends etc.

noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr

@@ -5,7 +5,7 @@ use dep::protocol_types::{
 use crate::context::{PrivateContext, PublicContext};
 use crate::note::{
     constants::MAX_NOTES_PER_PAGE, lifecycle::{create_note, create_note_hash_from_public, destroy_note},
-    note_getter::{get_notes, view_notes}, note_getter_options::NoteGetterOptions,
+    note_getter::{get_notes, view_notes}, note_getter_options::{NoteGetterOptions, NoteStatus},
     note_header::NoteHeader, note_interface::NoteInterface, note_viewer_options::NoteViewerOptions,
     utils::compute_note_hash_for_read_request
 };
@@ -52,34 +52,39 @@ impl<Note> PrivateSet<Note, &mut PrivateContext> {
     // DEPRECATED
     fn assert_contains_and_remove(_self: Self, _note: &mut Note, _nonce: Field) {
         assert(
-            false, "`assert_contains_and_remove` has been deprecated. Please call PXE.addNote() to add a note to the database. Then use PrivateSet.get_notes() and PrivateSet.remove() in your contract to verify and remove a note."
+            false, "`assert_contains_and_remove` has been deprecated. Please call PXE.addNote() to add a note to the database. Then use PrivateSet.get_notes() in your contract to verify and remove a note."
         );
     }
 
     // DEPRECATED
     fn assert_contains_and_remove_publicly_created(_self: Self, _note: &mut Note) {
         assert(
-            false, "`assert_contains_and_remove_publicly_created` has been deprecated. Please call PXE.addNote() to add a note to the database. Then use PrivateSet.get_notes() and PrivateSet.remove() in your contract to verify and remove a note."
+            false, "`assert_contains_and_remove_publicly_created` has been deprecated. Please call PXE.addNote() to add a note to the database. Then use PrivateSet.get_notes() in your contract to verify and remove a note."
         );
     }
 
-    // docs:start:remove
-    pub fn remove<N, M>(self, note: Note) where Note: NoteInterface<N, M> {
-        let note_hash = compute_note_hash_for_read_request(note);
-        let has_been_read = self.context.note_hash_read_requests.any(|r: ReadRequest| r.value == note_hash);
-        assert(has_been_read, "Can only remove a note that has been read from the set.");
-
-        destroy_note(self.context, note);
-    }
-    // docs:end:remove
-
     // docs:start:get_notes
     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);
+
+        assert(options.status == NoteStatus.ACTIVE, "PrivateSet can only retrieve active notes");
+
+        // We immediately emit nullifiers for all notes read. An advanced user might wish to only use and nullify some
+        // of them, but we don't allow for this use case to avoid scenarios in which they mistakenly don't nullify all
+        // used notes.
+        // These kinds of optimizations can be approximated regardless via usage of `NoteGetterOptions` by providing
+        // appropriate filters, etc.
+        for i in 0..opt_notes.len() {
+            if opt_notes[i].is_some() {
+                let note = opt_notes[i].unwrap_unchecked();
+                destroy_note(self.context, note);
+            }
+        }
+
         opt_notes
     }
     // docs:end:get_notes

noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr

@@ -54,11 +54,6 @@ impl<Context> EasyPrivateUint<&mut PrivateContext> {
                 // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this assert after rotating key
                 assert(note.npk_m_hash.eq(owner_npk_m_hash));
 
-                // Removes the note from the owner's set of notes.
-                // docs:start:remove
-                self.set.remove(note);
-                // docs:end:remove
-
                 minuend += note.value as u64;
             }
         }

noir-projects/aztec-nr/value-note/src/utils.nr

@@ -70,8 +70,6 @@ pub fn decrement_by_at_most(
             // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this after rotating keys.
             assert(note.npk_m_hash.eq(owner_npk_m_hash));
             decremented += note.value;
-
-            balance.remove(note);
         }
     }
 
@@ -85,22 +83,3 @@ pub fn decrement_by_at_most(
 
     decremented
 }
-
-// Removes the note from the owner's set of notes.
-// Returns the value of the destroyed note.
-pub fn destroy_note(
-    balance: PrivateSet<ValueNote, &mut PrivateContext>,
-    owner: AztecAddress,
-    note: ValueNote
-) -> Field {
-    // Ensure the note is actually owned by the owner (to prevent user from generating a valid proof while
-    // spending someone else's notes).
-    let owner_npk_m_hash = get_npk_m_hash(balance.context, owner);
-
-    // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this after rotating keys.
-    assert(note.npk_m_hash.eq(owner_npk_m_hash));
-
-    balance.remove(note);
-
-    note.value
-}

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

@@ -31,8 +31,9 @@ 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();
-        owner_notes.remove(note);
+
+        let note = notes[0].unwrap();
+
         increment(owner_notes, note.value, owner);
     }
 

noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr

@@ -144,18 +144,11 @@ impl Deck<&mut PrivateContext> {
 
         found_cards.map(
             |card_note: Option<CardNote>| {
-            assert(card_note.is_some(), "Card not found");
-            card_note.unwrap_unchecked()
+                assert(card_note.is_some(), "Card not found");
+                card_note.unwrap_unchecked()
         }
         )
     }
-
-    pub fn remove_cards<N>(&mut self, cards: [Card; N], owner: AztecAddress) {
-        let card_notes = self.get_cards(cards, owner);
-        for card_note in card_notes {
-            self.set.remove(card_note.note);
-        }
-    }
 }
 
 impl Deck<()> {

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

@@ -36,7 +36,9 @@ contract CardGame {
         let player = context.msg_sender();
 
         let mut collection = storage.collections.at(player);
-        collection.remove_cards(cards, player);
+        // Note that by reading the cards we're also removing them from the set.
+        let _ = collection.get_cards(cards, player);
+
         let mut game_deck = storage.game_decks.at(game as Field).at(player);
         let _added_to_game_deck = game_deck.add_cards(cards, player);
         let strength = compute_deck_strength(cards);
@@ -68,7 +70,8 @@ contract CardGame {
         let player = context.msg_sender();
 
         let mut game_deck = storage.game_decks.at(game as Field).at(player);
-        game_deck.remove_cards([card], player);
+        // Note that by reading the cards we're also removing them from the set.
+        let _ = game_deck.get_cards([card], player);
 
         // docs:start:call_public_function
         CardGame::at(context.this_address()).on_card_played(game, player, card.to_field()).enqueue(&mut context);

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

@@ -1,12 +1,16 @@
 // A demonstration of inclusion and non-inclusion proofs.
 contract InclusionProofs {
-    use dep::aztec::prelude::{
-        AztecAddress, EthAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PrivateContext, Map,
-        PrivateSet, PublicMutable
+    use dep::aztec::{
+        prelude::{
+        AztecAddress, EthAddress, FunctionSelector, NoteHeader, NoteGetterOptions, NoteViewerOptions,
+        PrivateContext, Map, PrivateSet, PublicMutable
+    },
+        note::{constants::MAX_NOTES_PER_PAGE, note_getter::view_notes, note_getter_options::NoteStatus},
+        keys::getters::{get_npk_m_hash, get_ivpk_m}
     };
 
     use dep::aztec::protocol_types::{grumpkin_point::GrumpkinPoint, contract_class_id::ContractClassId, header::Header};
-    use dep::aztec::{note::note_getter_options::NoteStatus, keys::getters::{get_npk_m_hash, get_ivpk_m}};
+
     // docs:start:imports
     // Imports are not needed as inclusion / non_inclusion proofs are accessible on the header.
     // docs:end:imports
@@ -52,7 +56,7 @@ contract InclusionProofs {
         // docs:start:get_note_from_pxe
         // 1) Get the note from PXE.
         let private_values = storage.private_values.at(owner);
-        let mut options = NoteGetterOptions::new();
+        let mut options = NoteViewerOptions::new();
         options = options.select(
             ValueNote::properties().npk_m_hash,
             owner_npk_m_hash,
@@ -61,8 +65,13 @@ 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 maybe_notes: [Option<ValueNote>; MAX_NOTES_PER_PAGE] = view_notes(private_values.storage_slot, options);
+        let note = maybe_notes[0].unwrap();
+
+        assert(note.get_header().contract_address == context.this_address());
+        assert(note.get_header().storage_slot == private_values.storage_slot);
+        assert(note.npk_m_hash == owner_npk_m_hash);
+
         // docs:end:get_note_from_pxe
 
         // 2) Prove the note inclusion
@@ -72,7 +81,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
     }
 
@@ -110,7 +119,7 @@ contract InclusionProofs {
         // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key.
         // 1) Get the note from PXE
         let private_values = storage.private_values.at(owner);
-        let mut options = NoteGetterOptions::new();
+        let mut options = NoteViewerOptions::new();
         options = options.select(
             ValueNote::properties().npk_m_hash,
             owner_npk_m_hash,
@@ -119,16 +128,20 @@ 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 maybe_notes: [Option<ValueNote>; MAX_NOTES_PER_PAGE] = view_notes(private_values.storage_slot, options);
+        let note = maybe_notes[0].unwrap();
+
+        assert(note.get_header().contract_address == context.this_address());
+        assert(note.get_header().storage_slot == private_values.storage_slot);
+        assert(note.npk_m_hash == owner_npk_m_hash);
 
         let header = if (use_block_number) {
             context.get_header_at(block_number)
         } else {
             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
     }
 
@@ -144,7 +157,7 @@ contract InclusionProofs {
         // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key.
         // 1) Get the note from PXE.
         let private_values = storage.private_values.at(owner);
-        let mut options = NoteGetterOptions::new();
+        let mut options = NoteViewerOptions::new();
         options = options.select(
             ValueNote::properties().npk_m_hash,
             owner_npk_m_hash,
@@ -153,8 +166,12 @@ 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 maybe_notes: [Option<ValueNote>; MAX_NOTES_PER_PAGE] = view_notes(private_values.storage_slot, options);
+        let note = maybe_notes[0].unwrap();
+
+        assert(note.get_header().contract_address == context.this_address());
+        assert(note.get_header().storage_slot == private_values.storage_slot);
+        assert(note.npk_m_hash == owner_npk_m_hash);
 
         // 2) Prove the note validity
         let header = if (use_block_number) {
@@ -180,10 +197,7 @@ contract InclusionProofs {
             owner_npk_m_hash,
             Option::none()
         ).set_limit(1);
-        let notes = private_values.get_notes(options);
-        let note = notes[0].unwrap();
-
-        private_values.remove(note);
+        let _ = private_values.get_notes(options);
     }
     // docs:end:nullify_note