From 730bc937a11ff0c2bbe89dc6555734541dd756e2 Mon Sep 17 00:00:00 2001 From: Leila Wang Date: Wed, 6 Sep 2023 18:49:32 +0000 Subject: [PATCH 1/3] Check note is read before removing from a set. --- .../noir-aztec/src/state_vars/set.nr | 26 ++++++--- .../noir-libs/noir-aztec/src/types/vec.nr | 54 ++++++++++++++----- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr b/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr index b8813c989a3..5012ab86e7e 100644 --- a/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr +++ b/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 { private_context: Option<&mut PrivateContext>, @@ -62,7 +62,12 @@ impl Set { 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 Set { // 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, diff --git a/yarn-project/noir-libs/noir-aztec/src/types/vec.nr b/yarn-project/noir-libs/noir-aztec/src/types/vec.nr index 4f0e1c95701..9c2961b91b6 100644 --- a/yarn-project/noir-libs/noir-aztec/src/types/vec.nr +++ b/yarn-project/noir-libs/noir-aztec/src/types/vec.nr @@ -37,18 +37,46 @@ impl BoundedVec { self.len -= 1; elem } + + fn any(self, predicate: fn[Env](T) -> bool) -> bool { + let mut ret = false; + for i in 0..MaxLen { + if (i < self.len as u64) { + ret |= predicate(self.storage[i]); + } + } + ret + } +} + +#[test] +fn test_vec_push_pop() { + let mut vec: BoundedVec = 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(vec.len == 2); + assert(x == 6); +} + +#[test(should_fail)] +fn test_vec_push_overflow() { + let mut vec: BoundedVec = BoundedVec::new(0); + vec.push(1); + vec.push(2); } -// #[test] -// fn test_vec() { -// let vec: BoundedVec = 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); -// } \ No newline at end of file +#[test] +fn test_vec_any() { + let mut vec: BoundedVec = BoundedVec::new(0); + vec.push(0); + vec.push(1); + vec.push(2); + assert(vec.any(|v| v == 2) == true); + assert(vec.any(|v| v == 3) == false); +} \ No newline at end of file From 3b375880c14786c012a5691bdcb03896d39a9f3f Mon Sep 17 00:00:00 2001 From: Leila Wang Date: Wed, 6 Sep 2023 19:10:00 +0000 Subject: [PATCH 2/3] Update docs. --- docs/docs/dev_docs/contracts/state_variables.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/dev_docs/contracts/state_variables.md b/docs/docs/dev_docs/contracts/state_variables.md index e368c30ab42..e2e7242f793 100644 --- a/docs/docs/dev_docs/contracts/state_variables.md +++ b/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` From 33f7dbfd5c5fa518ac45213ff4f971fad6b1e2a3 Mon Sep 17 00:00:00 2001 From: Leila Wang Date: Thu, 7 Sep 2023 16:26:27 +0000 Subject: [PATCH 3/3] Add more tests for BoundedVec. --- .../noir-libs/noir-aztec/src/types/vec.nr | 51 +++++++++++++++++-- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/yarn-project/noir-libs/noir-aztec/src/types/vec.nr b/yarn-project/noir-libs/noir-aztec/src/types/vec.nr index 9c2961b91b6..9442f977439 100644 --- a/yarn-project/noir-libs/noir-aztec/src/types/vec.nr +++ b/yarn-project/noir-libs/noir-aztec/src/types/vec.nr @@ -40,8 +40,10 @@ impl BoundedVec { fn any(self, predicate: fn[Env](T) -> bool) -> bool { let mut ret = false; + let mut exceeded_len = false; for i in 0..MaxLen { - if (i < self.len as u64) { + exceeded_len |= i == self.len; + if (!exceeded_len) { ret |= predicate(self.storage[i]); } } @@ -60,8 +62,39 @@ fn test_vec_push_pop() { vec.push(6); assert(vec.len == 3); let x = vec.pop(); - assert(vec.len == 2); 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 = 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 = 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 = BoundedVec::new(0); + vec.push_array([2]); + let _x = vec.get(1); +} + +#[test(should_fail)] +fn test_vec_get_uninitialised() { + let mut vec: BoundedVec = BoundedVec::new(0); + let _x = vec.get(0); } #[test(should_fail)] @@ -74,9 +107,17 @@ fn test_vec_push_overflow() { #[test] fn test_vec_any() { let mut vec: BoundedVec = BoundedVec::new(0); - vec.push(0); - vec.push(1); - vec.push(2); + 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_any_not_default() { + let default_value = 1; + let mut vec: BoundedVec = BoundedVec::new(default_value); + vec.push_array([2, 4]); + assert(vec.any(|v| v == default_value) == false); } \ No newline at end of file