emit nullifiers in set.get_notes

docs/docs/developers/contracts/references/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.

docs/docs/developers/contracts/writing_contracts/historical_data/archive_tree/how_to_prove_history.md

@@ -55,6 +55,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:
@@ -63,7 +67,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
 

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

@@ -2,7 +2,7 @@ use dep::protocol_types::{constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, abis:
 use crate::context::{PrivateContext, PublicContext, Context};
 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_consumption
 };
@@ -44,35 +44,41 @@ impl<Note> PrivateSet<Note> {
     // 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>(self, note: Note) where Note: NoteInterface<N> {
-        let context = self.context.private.unwrap();
-        let note_hash = compute_note_hash_for_consumption(note);
-        let has_been_read = 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(context, note);
-    }
-    // docs:end:remove
-
     // docs:start:get_notes
     pub fn get_notes<N, FILTER_ARGS>(
         self,
         options: NoteGetterOptions<Note, N, FILTER_ARGS>
     ) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N> {
+        let context = self.context.private.unwrap();
+
         let storage_slot = self.storage_slot;
-        let opt_notes = get_notes(self.context.private.unwrap(), storage_slot, options);
+        let opt_notes = get_notes(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(context, note);
+            }
+        }
+
         opt_notes
     }
     // docs:end:get_notes

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

@@ -49,11 +49,6 @@ impl EasyPrivateUint {
                 // spending someone else's notes).
                 assert(note.owner.eq(owner));
 
-                // 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

@@ -45,7 +45,12 @@ pub fn decrement_by_at_most(
     let mut decremented = 0;
     for i in 0..opt_notes.len() {
         if opt_notes[i].is_some() {
-            decremented += destroy_note(balance, owner, opt_notes[i].unwrap_unchecked());
+            let note = opt_notes[i].unwrap_unchecked();
+
+            // Ensure the note is actually owned by the owner (to prevent user from generating a valid proof while
+            // spending someone else's notes).
+            assert(note.owner.eq(owner));
+            decremented += note.value;
         }
     }
 
@@ -59,15 +64,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>, 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).
-    assert(note.owner.eq(owner));
-
-    balance.remove(note);
-
-    note.value
-}

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

@@ -32,7 +32,6 @@ contract Benchmarking {
         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);
         increment(owner_notes, note.value, owner);
     }
 

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

@@ -1,12 +1,9 @@
 use dep::aztec::prelude::{AztecAddress, FunctionSelector, PrivateContext, NoteHeader, NoteGetterOptions, NoteViewerOptions};
 
 use dep::aztec::{
-    protocol_types::{
-    traits::{ToField, Serialize, FromField},
-    constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL,
-},
+    protocol_types::{traits::{ToField, Serialize, FromField}, constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL},
     context::{PublicContext, Context}, note::note_getter::view_notes, state_vars::PrivateSet,
-    note::constants::MAX_NOTES_PER_PAGE,
+    note::constants::MAX_NOTES_PER_PAGE
 };
 use dep::std;
 use dep::std::{option::Option};
@@ -138,19 +135,12 @@ impl Deck {
 
         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);
-        }
-    }
-
     unconstrained pub fn view_cards(self, offset: u32) -> [Option<Card>; MAX_NOTES_PER_PAGE] {
         let mut options = NoteViewerOptions::new();
         let opt_notes = self.set.view_notes(options.set_offset(offset));

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,14 @@
 // 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
+    },
+        context::Context, note::note_getter_options::NoteStatus
     };
 
     use dep::aztec::protocol_types::{grumpkin_point::GrumpkinPoint, contract_class_id::ContractClassId};
-    use dep::aztec::{context::Context, note::note_getter_options::NoteStatus};
     // docs:start:imports
     use dep::aztec::history::{
         contract_inclusion::{
@@ -59,7 +61,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().owner,
             owner.to_field(),
@@ -68,17 +70,22 @@ 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 = private_values.view_notes(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.owner == owner);
+
         // docs:end:get_note_from_pxe
 
         // 2) Prove the note inclusion
         if (use_block_number) {
             // docs:start:prove_note_inclusion
-            prove_note_inclusion_at(maybe_note.unwrap_unchecked(), block_number, context);
+            prove_note_inclusion_at(note, block_number, context);
             // docs:end:prove_note_inclusion
         } else {
-            prove_note_inclusion(maybe_note.unwrap_unchecked(), context);
+            prove_note_inclusion(note, context);
         }
     }
 
@@ -110,7 +117,7 @@ contract InclusionProofs {
     ) {
         // 2) 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().owner,
             owner.to_field(),
@@ -119,15 +126,19 @@ 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 = private_values.view_notes(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.owner == owner);
 
         // 3) Compute the nullifier from the note
         // docs:start:prove_note_not_nullified
         if (use_block_number) {
-            prove_note_not_nullified_at(maybe_note.unwrap_unchecked(), block_number, &mut context);
+            prove_note_not_nullified_at(note, block_number, &mut context);
         } else {
-            prove_note_not_nullified(maybe_note.unwrap_unchecked(), &mut context);
+            prove_note_not_nullified(note, &mut context);
         }
         // docs:end:prove_note_not_nullified
     }
@@ -141,7 +152,7 @@ contract InclusionProofs {
     ) {
         // 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().owner,
             owner.to_field(),
@@ -150,8 +161,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 = private_values.view_notes(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.owner == owner);
 
         // 2) Prove the note validity
         if (use_block_number) {
@@ -173,10 +188,7 @@ contract InclusionProofs {
             owner.to_field(),
             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
 

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

@@ -4,9 +4,7 @@
 // be read/nullified before their creation etc.
 contract PendingNoteHashes {
     // Libs
-    use dep::aztec::prelude::{
-        AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PrivateContext, Map, PrivateSet
-    };
+    use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PrivateContext, Map, PrivateSet};
     use dep::value_note::{balance_utils, filter::filter_notes_min_sum, value_note::{VALUE_NOTE_LEN, ValueNote}};
     use dep::aztec::context::{PublicContext, Context};
 
@@ -30,15 +28,13 @@ contract PendingNoteHashes {
         owner_balance.insert(&mut note, true);
 
         let options = NoteGetterOptions::with_filter(filter_notes_min_sum, amount);
-        // get note inserted above
+        // get and delete note inserted above
         let maybe_notes = owner_balance.get_notes(options);
 
         let note0 = maybe_notes[0].unwrap();
         assert(note.value == note0.value);
         assert(maybe_notes[1].is_none());
 
-        owner_balance.remove(note0);
-
         note0.value
     }
 
@@ -86,8 +82,6 @@ contract PendingNoteHashes {
 
         assert(expected_value == note.value);
 
-        owner_balance.remove(note);
-
         expected_value
     }