fix: check a note is read before nullifying it. (#2076)

Closes #1899

docs/docs/dev_docs/contracts/state_variables.md

@@ -225,7 +225,7 @@ We can also remove a note from a set:
 
 #include_code state_vars-SetRemove /yarn-project/noir-contracts/src/contracts/docs_example_contract/src/actions.nr rust
 
-Note that the transaction won't fail if the note we are removing does not exist in the set. As a best practice, we should fetch the notes by calling [`get_notes`](#get_notes), which does a membership check under the hood to make sure the notes exist, and then feed the returned notes to the `remove` function.
+Note that the proof will fail if the note we are removing does not exist. To avoid this, it's advisable to first retrieve the notes using [`get_notes`](#get_notes), which does a membership check under the hood to make sure the notes exist, and then we can safely provide these notes as input to the `remove` function.
 
 ### `.get_notes`
 

yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr

@@ -1,15 +1,15 @@
+use dep::std::option::Option;
 use crate::abi::PublicContextInputs;
 use crate::constants_gen::{MAX_NOTES_PER_PAGE, MAX_READ_REQUESTS_PER_CALL};
 use crate::context::{PrivateContext, PublicContext};
-use crate::note::lifecycle::{create_note, create_note_hash_from_public, destroy_note};
 use crate::note::{
+    lifecycle::{create_note, create_note_hash_from_public, destroy_note},
     note_getter::{ensure_note_exists, ensure_note_hash_exists, get_notes, view_notes},
     note_getter_options::NoteGetterOptions,
     note_interface::NoteInterface,
     note_viewer_options::NoteViewerOptions,
-    utils::compute_inner_note_hash,
+    utils::compute_note_hash_for_read_or_nullify,
 };
-use dep::std::option::Option;
 
 struct Set<Note, N> {
     private_context: Option<&mut PrivateContext>,
@@ -62,7 +62,12 @@ impl<Note, N> Set<Note, N> {
             self.note_interface,
             &mut note_with_header,
         );
-        self.remove(note_with_header);
+        destroy_note(
+            self.private_context.unwrap(),
+            self.storage_slot,
+            note_with_header,
+            self.note_interface,
+        );
     }
 
     // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1386):
@@ -88,11 +93,20 @@ impl<Note, N> Set<Note, N> {
         // this hack once public kernel injects nonces.
         header.nonce = 1;
         set_header(&mut note_with_header, header);
-
-        self.remove(note_with_header);
+        destroy_note(
+            self.private_context.unwrap(),
+            self.storage_slot,
+            note_with_header,
+            self.note_interface,
+        );
     }
 
     fn remove(self, note: Note) {
+        let note_hash = compute_note_hash_for_read_or_nullify(self.note_interface, note);
+        let read_requests = self.private_context.unwrap_unchecked().read_requests;
+        let has_been_read = read_requests.any(|r| r == note_hash);
+        assert(has_been_read, "Can only remove a note that has been read from the set.");
+
         destroy_note(
             self.private_context.unwrap(),
             self.storage_slot,

yarn-project/noir-libs/noir-aztec/src/types/vec.nr

@@ -37,18 +37,87 @@ impl<T, MaxLen> BoundedVec<T, MaxLen> {
         self.len -= 1;
         elem
     }
+
+    fn any<Env>(self, predicate: fn[Env](T) -> bool) -> bool {
+        let mut ret = false;
+        let mut exceeded_len = false;
+        for i in 0..MaxLen {
+            exceeded_len |= i == self.len;
+            if (!exceeded_len) {
+                ret |= predicate(self.storage[i]);
+            }
+        }
+        ret
+    }
+}
+
+#[test]
+fn test_vec_push_pop() {
+    let mut vec: BoundedVec<Field, 3> = BoundedVec::new(0);
+    assert(vec.len == 0);
+    vec.push(2);
+    assert(vec.len == 1);
+    vec.push(4);
+    assert(vec.len == 2);
+    vec.push(6);
+    assert(vec.len == 3);
+    let x = vec.pop();
+    assert(x == 6);
+    assert(vec.len == 2);
+    assert(vec.get(0) == 2);
+    assert(vec.get(1) == 4);
+}
+
+#[test]
+fn test_vec_push_array() {
+    let mut vec: BoundedVec<Field, 3> = BoundedVec::new(0);
+    vec.push_array([2, 4]);
+    assert(vec.len == 2);
+    assert(vec.get(0) == 2);
+    assert(vec.get(1) == 4);
+}
+
+#[test(should_fail)]
+fn test_vec_get_out_of_bound() {
+    let mut vec: BoundedVec<Field, 2> = BoundedVec::new(0);
+    vec.push_array([2, 4]);
+    let _x = vec.get(2);
+}
+
+#[test(should_fail)]
+fn test_vec_get_not_declared() {
+    let mut vec: BoundedVec<Field, 2> = BoundedVec::new(0);
+    vec.push_array([2]);
+    let _x = vec.get(1);
+}
+
+#[test(should_fail)]
+fn test_vec_get_uninitialised() {
+    let mut vec: BoundedVec<Field, 2> = BoundedVec::new(0);
+    let _x = vec.get(0);
+}
+
+#[test(should_fail)]
+fn test_vec_push_overflow() {
+    let mut vec: BoundedVec<Field, 1> = BoundedVec::new(0);
+    vec.push(1);
+    vec.push(2);
+}
+
+#[test]
+fn test_vec_any() {
+    let mut vec: BoundedVec<Field, 3> = BoundedVec::new(0);
+    vec.push_array([2, 4, 6]);
+    assert(vec.any(|v| v == 2) == true);
+    assert(vec.any(|v| v == 4) == true);
+    assert(vec.any(|v| v == 6) == true);
+    assert(vec.any(|v| v == 3) == false);
 }
 
-// #[test]
-// fn test_vec() {
-//     let vec: BoundedVec<Field, 2> = BoundedVec::new(0);
-//     assert(vec.len == 0);
-//     let vec1 = vec.push(1);
-//     assert(vec1.len == 1);
-//     let vec2 = vec1.push(1);
-//     assert(vec2.len == 2);
-//     let vec3 = vec2.push(1);
-//     assert(vec3.len == 3);
-//     let x = vec3.pop();
-//     assert(x == 1);
-// }
+#[test]
+fn test_vec_any_not_default() {
+    let default_value = 1;
+    let mut vec: BoundedVec<Field, 3> = BoundedVec::new(default_value);
+    vec.push_array([2, 4]);
+    assert(vec.any(|v| v == default_value) == false);
+}