refactor: Make get notes return all notes at beginning of array #4991

noir-projects/aztec-nr/Nargo.toml

@@ -8,4 +8,5 @@ members = [
   "field-note",
   "slow-updates-tree",
   "value-note",
+  "tests",
 ]

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

@@ -101,6 +101,18 @@ pub fn get_notes<Note, N, FILTER_ARGS>(
     options: NoteGetterOptions<Note, N, FILTER_ARGS>
 ) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N> {
     let opt_notes = get_notes_internal(storage_slot, options);
+
+    _get_notes_constrain_get_notes_internal(context, storage_slot, opt_notes, options)
+}
+
+pub fn _get_notes_constrain_get_notes_internal<Note, N, FILTER_ARGS>(
+    context: &mut PrivateContext,
+    storage_slot: Field,
+    opt_notes: [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
+    options: NoteGetterOptions<Note, N, FILTER_ARGS>
+) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N> {
+    let mut returned_notes = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+
     let mut num_notes = 0;
     let mut prev_fields = [0; N];
     for i in 0..opt_notes.len() {
@@ -120,6 +132,9 @@ pub fn get_notes<Note, N, FILTER_ARGS>(
             // failure if malicious oracle injects 0 nonce here for a "pre-existing" note.
             context.push_note_hash_read_request(note_hash_for_read_request);
 
+            // The below code is used to collapse a sparse array into one where the values are guaranteed to be at the front of the array
+            // We write at returned_notes[num_notes] because num_notes is only advanced when we have a value in opt_notes
+            returned_notes[num_notes] = Option::some(note);
             num_notes += 1;
         };
     }
@@ -129,7 +144,7 @@ pub fn get_notes<Note, N, FILTER_ARGS>(
 
     assert(num_notes != 0, "Cannot return zero notes");
 
-    opt_notes
+    returned_notes
 }
 
 unconstrained fn get_note_internal<Note, N>(storage_slot: Field) -> Note where Note: NoteInterface<N> {

noir-projects/aztec-nr/tests/Nargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "tests"
+authors = ["aztec-labs"]
+compiler_version = ">=0.18.0"
+type = "lib"
+
+[dependencies]
+aztec = { path = "../aztec" }
+protocol_types = { path = "../../noir-protocol-circuits/crates/types" }

noir-projects/aztec-nr/tests/src/lib.nr

@@ -0,0 +1,2 @@
+mod note_getter_test;
+mod mock;

noir-projects/aztec-nr/tests/src/mock.nr

@@ -0,0 +1,3 @@
+mod test_note;
+
+use test_note::TestNote;

noir-projects/aztec-nr/tests/src/mock/test_note.nr

@@ -0,0 +1,63 @@
+use dep::aztec::context::PrivateContext;
+use dep::aztec::note::{
+    note_header::NoteHeader,
+    note_getter_options::{NoteGetterOptions, Select, Sort, SortOrder, Comparator, NoteStatus, PropertySelector},
+    note_interface::NoteInterface, note_viewer_options::NoteViewerOptions,
+    utils::compute_note_hash_for_consumption, note_getter::_get_notes_constrain_get_notes_internal
+};
+
+global TEST_NOTE_LENGTH = 1;
+
+struct TestNote {
+  header: NoteHeader,
+  value: Field,
+}
+
+impl NoteInterface<TEST_NOTE_LENGTH> for TestNote {
+    fn serialize_content(self) -> [Field; TEST_NOTE_LENGTH] {
+      [self.value]
+    }
+
+    fn deserialize_content(fields: [Field; TEST_NOTE_LENGTH]) -> Self {
+      Self {
+        value: fields[0],
+        header: NoteHeader::empty(),
+      }
+    }
+
+    fn compute_note_content_hash(self) -> Field {
+      0
+    }
+
+    fn get_header(self) -> NoteHeader {
+      self.header
+    }
+
+    fn set_header(&mut self, header: NoteHeader) -> () {
+      self.header = header;
+    }
+
+    fn get_note_type_id() -> Field {
+      0
+    }
+
+    fn compute_nullifier(self, _context: &mut PrivateContext) -> Field {
+      0
+    }
+
+    fn compute_nullifier_without_context(self) -> Field {
+      0
+    }
+
+    fn broadcast(self, context: &mut PrivateContext, slot: Field) {
+      assert(
+        false, "TestNote does not support broadcast."
+      );
+    }
+}
+
+impl TestNote {
+    pub fn new(value: Field) -> Self {
+        TestNote { value, header: NoteHeader::empty() }
+    }
+}

noir-projects/aztec-nr/tests/src/note_getter_test.nr

@@ -0,0 +1,131 @@
+use dep::protocol_types::constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL;
+use dep::aztec::context::PrivateContext;
+use dep::aztec::note::{
+    note_header::NoteHeader,
+    note_getter_options::{NoteGetterOptions, Sort, SortOrder, Comparator, PropertySelector},
+    note_getter::_get_notes_constrain_get_notes_internal
+};
+use dep::aztec::protocol_types::address::AztecAddress;
+use crate::mock::test_note::TestNote;
+
+#[test]
+fn sets_note_manually_and_fetches_it() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+    context.inputs.call_context.storage_contract_address = AztecAddress::from_field(69);
+
+    let mut test_note = TestNote::new(1337);
+    test_note.header = NoteHeader::new(AztecAddress::from_field(69), 0, 42);
+
+    let mut opt_notes = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[0] = Option::some(test_note);
+
+    let storage_slot: Field = 42;
+
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[0] = Option::some(test_note);
+
+    let mut options = NoteGetterOptions::new();
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+    assert_eq(returned[0].unwrap().value, 1337);
+}
+
+#[test(should_fail)]
+fn cannot_return_zero_notes() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+    let storage_slot: Field = 0;
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+
+    let mut options = NoteGetterOptions::new();
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+}
+
+#[test(should_fail)]
+fn mismatched_address() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+    context.inputs.call_context.storage_contract_address = AztecAddress::from_field(1);
+
+    let storage_slot: Field = 0;
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[0] = Option::some(TestNote::new(1));
+
+    let mut options = NoteGetterOptions::new();
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+}
+
+#[test(should_fail)]
+fn mismatched_storage_slot() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+    context.inputs.call_context.storage_contract_address = AztecAddress::from_field(1);
+
+    let mut test_note = TestNote::new(1);
+    test_note.header = NoteHeader::new(AztecAddress::from_field(1), 0, 1);
+
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[0] = Option::some(test_note);
+
+    let storage_slot: Field = 0;
+
+    let mut options = NoteGetterOptions::new();
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+}
+
+#[test(should_fail)]
+fn invalid_selector() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+    context.inputs.call_context.storage_contract_address = AztecAddress::from_field(1);
+
+    let mut test_note = TestNote::new(1);
+    test_note.header = NoteHeader::new(AztecAddress::from_field(1), 0, 0);
+
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[0] = Option::some(test_note);
+
+    let storage_slot: Field = 0;
+
+    let mut options = NoteGetterOptions::new().select(
+        PropertySelector { index: 0, offset: 0, length: 32 },
+        10,
+        Option::some(Comparator.EQ)
+    );
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+}
+
+#[test(should_fail)]
+fn invalid_note_order() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[0] = Option::some(TestNote::new(1));
+    opt_notes[1] = Option::some(TestNote::new(2));
+
+    let storage_slot: Field = 0;
+
+    let mut options = NoteGetterOptions::new().sort(
+        PropertySelector { index: 0, offset: 0, length: 32 },
+        SortOrder.DESC
+    );
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+}
+
+#[test]
+fn sparse_notes_array() {
+    let mut context: PrivateContext = dep::std::unsafe::zeroed();
+
+    let mut opt_notes: [Option<TestNote>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
+    opt_notes[1] = Option::some(TestNote::new(0));
+    opt_notes[2] = Option::some(TestNote::new(1));
+    opt_notes[3] = Option::some(TestNote::new(2));
+    opt_notes[5] = Option::some(TestNote::new(3));
+    opt_notes[8] = Option::some(TestNote::new(4));
+    opt_notes[11] = Option::some(TestNote::new(5));
+    opt_notes[19] = Option::some(TestNote::new(6));
+
+    let storage_slot: Field = 0;
+
+    let mut options = NoteGetterOptions::new();
+    let returned = _get_notes_constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);
+
+    for i in 0..7 {
+        assert(returned[i].unwrap().value == i as Field);
+    }
+}