feat!: refactor NoteGetterOptions::select API (#8504)

Author: nventuro

docs/docs/migration_notes.md

@@ -6,11 +6,25 @@ keywords: [sandbox, aztec, notes, migration, updating, upgrading]
 
 Aztec is in full-speed development. Literally every version breaks compatibility with the previous ones. This page attempts to target errors and difficulties you might encounter when upgrading, and how to resolve them.
 
+## TBD
+
+### [Aztec.nr] Rework `NoteGetterOptions::select`
+
+The `select` function in both `NoteGetterOptions` and `NoteViewerOptions` no longer takes an `Option` of a comparator, but instead requires an explicit comparator to be passed. Additionally, the order of the parameters has been changed so that they are `(lhs, operator, rhs)`. These two changes should make invocations of the function easier to read:
+
+```diff
+- options.select(ValueNote::properties().value, amount, Option::none())
++ options.select(ValueNote::properties().value, Comparator.EQ, amount)
+```
+
 ## 0.53.0
+
 ### [Aztec.nr] Remove `OwnedNote` and create `UintNote`
+
 `OwnedNote` allowed having a U128 `value` in the custom note while `ValueNote` restricted to just a Field.
 
 We have removed `OwnedNote` but are introducing a more genric `UintNote` within aztec.nr
+
 ```
 #[aztec(note)]
 struct UintNote {
@@ -24,9 +38,11 @@ struct UintNote {
 ```
 
 ### [TXE] logging
+
 You can now use `debug_log()` within your contract to print logs when using the TXE
 
 Remember to set the following environment variables to activate debug logging:
+
 ```bash
 export DEBUG="aztec:*"
 export LOG_LEVEL="debug"
@@ -36,7 +52,7 @@ export LOG_LEVEL="debug"
 
 `is_valid_impl` method in account contract asserted if signature was true. Instead now we will return the verification to give flexibility to developers to handle it as they please.
 
-```diff
+````diff
 - let verification = std::ecdsa_secp256k1::verify_signature(public_key.x, public_key.y, signature, hashed_message);
 - assert(verification == true);
 - true
@@ -57,7 +73,7 @@ Public keys (ivpk, ovpk, npk, tpk) should no longer be fetched using the old `ge
 +let owner_keys = get_current_public_keys(&mut context, owner);
 +let owner_ivpk_m = owner_keys.ivpk_m;
 +let owner_ovpk_m = owner_keys.ovpk_m;
-```
+````
 
 If using more than one key per account, this will result in very large circuit gate count reductions.
 

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

@@ -228,7 +228,6 @@ An example of such options is using the [filter_notes_min_sum (GitHub link)](htt
 
 This function has the same behavior as `pop_notes` above but it does not delete the notes.
 
-
 ### `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.
@@ -255,7 +254,7 @@ You can view the implementation [here (GitHub link)](https://github.com/AztecPro
 
 ### `selects: BoundedVec<Option<Select>, N>`
 
-`selects` is a collection of filtering criteria, specified by `Select { property_selector: PropertySelector, value: Field, comparator: u3 }` structs. It instructs the data oracle to find notes whose serialized field (as specified by the PropertySelector) matches the provided `value`, according to the `comparator`. The PropertySelector is in turn specified as having an `index` (nth position of the selected field in the serialized note), an `offset` (byte offset inside the selected serialized field) and `length` (bytes to read of the field from the offset)
+`selects` is a collection of filtering criteria, specified by `Select { property_selector: PropertySelector, comparator: u8, value: Field }` structs. It instructs the data oracle to find notes whose serialized field (as specified by the `PropertySelector`) matches the provided `value`, according to the `comparator`. The PropertySelector is in turn specified as having an `index` (nth position of the selected field in the serialized note), an `offset` (byte offset inside the selected serialized field) and `length` (bytes to read of the field from the offset). These values are not expected to be manually computed, but instead specified by passing functions autogenerated from the note definition.
 
 ### `sorts: BoundedVec<Option<Sort>, N>`
 
@@ -329,17 +328,17 @@ This method sets the status of notes to retrieve (active or nullified).
 
 #### Example 1
 
-The following code snippet creates an instance of `NoteGetterOptions`, which has been configured to find the cards that belong to `account_address`. The returned cards are sorted by their points in descending order, and the first `offset` cards with the highest points are skipped.
+The following code snippet creates an instance of `NoteGetterOptions`, which has been configured to find the cards that belong to an account with nullifying key hash equal to `account_npk_m_hash`. The returned cards are sorted by their points in descending order, and the first `offset` cards with the highest points are skipped.
 
 #include_code state_vars-NoteGetterOptionsSelectSortOffset /noir-projects/noir-contracts/contracts/docs_example_contract/src/options.nr rust
 
-The first value of `.select` and `.sort` is the index of a field in a note type. For the note type `CardNote` that has the following fields:
+The first value of `.select` and `.sort` indicates the property of the note we're looking for. For this we use helper functions that are autogenerated from the note definition. `CardNote` that has the following fields:
 
 #include_code state_vars-CardNote /noir-projects/noir-contracts/contracts/docs_example_contract/src/types/card_note.nr rust
 
-The indices are: 0 for `points`, 1 for `randomness`, and 2 for `npk_m_hash`.
+`CardNote::properties()` will return a struct with the values to pass for each field, which are related to their indices inside the `CardNote` struct, internal offset and length.
 
-In the example, `.select(2, account_address, Option::none())` matches the 2nd field of `CardNote`, which is `npk_m_hash`, and returns the cards whose `npk_m_hash` field equals `account_npk_m_hash`, equality is the comparator used because because including no comparator (the third argument) defaults to using the equality comparator. The current possible values of Comparator are specified in the Note Getter Options implementation linked above.
+In the example, `.select(CardNote::properties().npk_m_hash, Comparator.EQ, account_npk_m_hash)` matches notes which have the `npk_m_hash` field set to `account_npk_m_hash`. In this case we're using the equality comparator, but other operations exist in the `Comparator` utility struct.
 
 `.sort(0, SortOrder.DESC)` sorts the 0th field of `CardNote`, which is `points`, in descending order.
 

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

@@ -196,8 +196,8 @@ fn rejects_mismatched_selector() {
     let mut options = NoteGetterOptions::new();
     options = options.select(
             PropertySelector { index: 0, offset: 0, length: 32 },
-            value + 1,
-            Option::some(Comparator.EQ)
+            Comparator.EQ,
+            value + 1
         );
 
     let _ = constrain_get_notes_internal(&mut context, storage_slot, opt_notes, options);

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

@@ -1,7 +1,6 @@
 use std::option::Option;
 use dep::protocol_types::{constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, traits::ToField};
 use crate::note::note_interface::NoteInterface;
-use crate::utils::comparison::Comparator;
 
 struct PropertySelector {
     index: u8,
@@ -11,13 +10,15 @@ struct PropertySelector {
 
 struct Select {
     property_selector: PropertySelector,
-    value: Field,
     comparator: u8,
+    value: Field,
 }
 
 impl Select {
-    pub fn new(property_selector: PropertySelector, value: Field, comparator: u8) -> Self {
-        Select { property_selector, value, comparator }
+    // The selected property will be the left hand side and value the right hand side of the operation, so e.g. the
+    // object created by new(property, Comparator.GT, value) represents 'property > value'.
+    pub fn new(property_selector: PropertySelector, comparator: u8, value: Field) -> Self {
+        Select { property_selector, comparator, value }
     }
 }
 
@@ -90,18 +91,10 @@ impl<Note, let N: u32, let M: u32, PREPROCESSOR_ARGS, FILTER_ARGS> NoteGetterOpt
     pub fn select<T>(
         &mut self,
         property_selector: PropertySelector,
-        value: T,
-        comparator: Option<u8>
+        comparator: u8,
+        value: T
     ) -> Self where T: ToField {
-        self.selects.push(
-            Option::some(
-                Select::new(
-                    property_selector,
-                    value.to_field(),
-                    comparator.unwrap_or(Comparator.EQ)
-                )
-            )
-        );
+        self.selects.push(Option::some(Select::new(property_selector, comparator, value.to_field())));
         *self
     }
 

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

@@ -1,5 +1,5 @@
 use std::option::Option;
-use crate::note::note_getter_options::{PropertySelector, Select, Sort, Comparator, NoteStatus};
+use crate::note::note_getter_options::{PropertySelector, Select, Sort, NoteStatus};
 use dep::protocol_types::traits::ToField;
 use crate::note::note_interface::NoteInterface;
 use crate::note::constants::MAX_NOTES_PER_PAGE;
@@ -32,18 +32,10 @@ impl<Note, let N: u32, let M: u32> NoteViewerOptions<Note, N, M> {
     pub fn select<T>(
         &mut self,
         property_selector: PropertySelector,
-        value: T,
-        comparator: Option<u8>
+        comparator: u8,
+        value: T
     ) -> Self where T: ToField {
-        self.selects.push(
-            Option::some(
-                Select::new(
-                    property_selector,
-                    value.to_field(),
-                    comparator.unwrap_or(Comparator.EQ)
-                )
-            )
-        );
+        self.selects.push(Option::some(Select::new(property_selector, comparator, value.to_field())));
         *self
     }
 

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

@@ -6,7 +6,7 @@ contract Child {
         context::gas::GasOpts, protocol_types::{abis::call_context::CallContext},
         note::{note_getter_options::NoteGetterOptions, note_header::NoteHeader},
         encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
-        keys::getters::get_current_public_keys
+        keys::getters::get_current_public_keys, utils::comparison::Comparator
     };
     use dep::value_note::value_note::ValueNote;
 
@@ -21,7 +21,7 @@ contract Child {
     fn value(input: Field) -> Field {
         input + context.chain_id() + context.version()
     }
-    // Returns a sum of the input and the chain id and version of the contract in private circuit public input's return_values. 
+    // Returns a sum of the input and the chain id and version of the contract in private circuit public input's return_values.
     // Can only be called from this contract.
     #[aztec(private)]
     #[aztec(internal)]
@@ -62,7 +62,7 @@ contract Child {
     #[aztec(private)]
     fn private_get_value(amount: Field, owner: AztecAddress) -> Field {
         let mut options = NoteGetterOptions::new();
-        options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1);
+        options = options.select(ValueNote::properties().value, Comparator.EQ, amount).set_limit(1);
         let notes = storage.a_map_with_private_values.at(owner).get_notes(options);
         notes.get(0).value
     }
@@ -78,7 +78,7 @@ contract Child {
     }
 
     // Increments `current_value` by `new_value`. Can only be called from this contract.
-    #[aztec(public)] 
+    #[aztec(public)]
     #[aztec(internal)]
     fn pub_inc_value_internal(new_value: Field) -> Field {
         let old_value = storage.current_value.read();

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

@@ -6,7 +6,7 @@ contract DelegatedOn {
     };
     use dep::aztec::{
         encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
-        keys::getters::get_current_public_keys
+        keys::getters::get_current_public_keys, utils::comparison::Comparator
     };
     use dep::value_note::value_note::ValueNote;
 
@@ -34,7 +34,7 @@ contract DelegatedOn {
     #[aztec(private)]
     fn get_private_value(amount: Field, owner: AztecAddress) -> pub Field {
         let mut options = NoteGetterOptions::new();
-        options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1);
+        options = options.select(ValueNote::properties().value, Comparator.EQ, amount).set_limit(1);
         let notes = storage.a_map_with_private_values.at(owner).get_notes(options);
         notes.get(0).value
     }
@@ -43,4 +43,3 @@ contract DelegatedOn {
         storage.current_value.read()
     }
 }
-

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

@@ -3,6 +3,7 @@ contract Delegator {
     use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PublicMutable, PrivateSet, Deserialize, Map};
     use dep::value_note::value_note::ValueNote;
     use dep::delegated_on::DelegatedOn;
+    use dep::aztec::utils::comparison::Comparator;
 
     #[aztec(storage)]
     struct Storage {
@@ -33,7 +34,7 @@ contract Delegator {
     #[aztec(private)]
     fn get_private_value(amount: Field, owner: AztecAddress) -> pub Field {
         let mut options = NoteGetterOptions::new();
-        options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1);
+        options = options.select(ValueNote::properties().value, Comparator.EQ, amount).set_limit(1);
         let notes = storage.a_map_with_private_values.at(owner).get_notes(options);
         notes.get_unchecked(0).value
     }

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

@@ -6,7 +6,7 @@ mod types;
 // (tests ordering in the circuit)
 
 // you have a card (PrivateMutable). Anyone can create a bigger card. Whoever is bigger will be the leader.
-// it also has dummy methods and other examples used for documentation e.g. 
+// it also has dummy methods and other examples used for documentation e.g.
 // how to create custom notes, a custom struct for public state, a custom note that may be unencrypted
 // also has `options.nr` which shows various ways of using `NoteGetterOptions` to query notes
 // it also shows what our macros do behind the scenes!
@@ -19,7 +19,6 @@ contract DocsExample {
         PrivateSet, SharedImmutable, Deserialize
     };
     use dep::aztec::encrypted_logs::encrypted_note_emission::{encode_and_encrypt_note, encode_and_encrypt_note_with_keys};
-    use dep::aztec::note::note_getter_options::Comparator;
     use dep::aztec::keys::getters::get_current_public_keys;
     // how to import methods from other files/folders within your workspace
     use crate::types::{card_note::{CardNote, CARD_NOTE_LEN}, leader::Leader};
@@ -33,7 +32,7 @@ contract DocsExample {
         // docs:start:storage-private-mutable-declaration
         legendary_card: PrivateMutable<CardNote>,
         // docs:end:storage-private-mutable-declaration
-        // just used for docs example to show how to create a private mutable map. 
+        // just used for docs example to show how to create a private mutable map.
         profiles: Map<AztecAddress, PrivateMutable<CardNote>>,
         // docs:start:storage-set-declaration
         set: PrivateSet<CardNote>,
@@ -112,7 +111,7 @@ contract DocsExample {
     fn get_shared_immutable_constrained_private_indirect() -> pub Leader {
         // This is a private function that calls another private function
         // and returns the response.
-        // Used to test that we can retrieve values through calls and 
+        // Used to test that we can retrieve values through calls and
         // correctly return them in the simulation
         let mut leader = DocsExample::at(context.this_address()).get_shared_immutable_constrained_private().view(&mut context);
         leader.points += 1;
@@ -123,7 +122,7 @@ contract DocsExample {
     fn get_shared_immutable_constrained_public_indirect() -> pub Leader {
         // This is a public function that calls another public function
         // and returns the response.
-        // Used to test that we can retrieve values through calls and 
+        // Used to test that we can retrieve values through calls and
         // correctly return them in the simulation
         let mut leader = DocsExample::at(context.this_address()).get_shared_immutable_constrained_public().view(&mut context);
         leader.points += 1;
@@ -213,15 +212,9 @@ contract DocsExample {
     }
 
     // docs:start:state_vars-NoteGetterOptionsComparatorExampleNoir
-    unconstrained fn read_note(amount: Field, comparator: u8) -> pub BoundedVec<CardNote, 10> {
+    unconstrained fn read_note(comparator: u8, amount: Field) -> pub BoundedVec<CardNote, 10> {
         let mut options = NoteViewerOptions::new();
-        storage.set.view_notes(
-            options.select(
-                CardNote::properties().points,
-                amount,
-                Option::some(comparator)
-            )
-        )
+        storage.set.view_notes(options.select(CardNote::properties().points, comparator, amount))
     }
     // docs:end:state_vars-NoteGetterOptionsComparatorExampleNoir
 
@@ -327,13 +320,13 @@ contract DocsExample {
 
         // Our original inputs!
         a: Field,
-        b: Field // The actual return type of our circuit is the PrivateCircuitPublicInputs struct, this will be the 
-        // input to our kernel! 
+        b: Field // The actual return type of our circuit is the PrivateCircuitPublicInputs struct, this will be the
+        // input to our kernel!
         // docs:start:context-example-return
     ) -> pub PrivateCircuitPublicInputs {
         // docs:end:context-example-return
         // ************************************************************
-        // The hasher is a structure used to generate a hash of the circuits inputs. 
+        // The hasher is a structure used to generate a hash of the circuits inputs.
         // docs:start:context-example-hasher
         let mut args_hasher = dep::aztec::hash::ArgsHasher::new();
         args_hasher.add(a);

noir-projects/noir-contracts/contracts/docs_example_contract/src/options.nr

@@ -2,17 +2,21 @@ use crate::types::card_note::{CardNote, CARD_NOTE_LEN, CARD_NOTE_BYTES_LEN};
 use dep::aztec::prelude::{AztecAddress, NoteGetterOptions};
 
 use dep::aztec::protocol_types::constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL;
-use dep::aztec::note::note_getter_options::{Sort, SortOrder};
+use dep::aztec::{note::note_getter_options::{Sort, SortOrder}, utils::comparison::Comparator};
 
 // Shows how to use NoteGetterOptions and query for notes.
 
 // docs:start:state_vars-NoteGetterOptionsSelectSortOffset
-pub fn create_points_card_getter_options(
-    points: Field,
+pub fn create_npk_card_getter_options(
+    account_npk_m_hash: Field,
     offset: u32
 ) -> NoteGetterOptions<CardNote, CARD_NOTE_LEN, CARD_NOTE_BYTES_LEN, Field, Field> {
     let mut options = NoteGetterOptions::new();
-    options.select(CardNote::properties().points, points, Option::none()).sort(CardNote::properties().points, SortOrder.DESC).set_offset(offset)
+    options.select(
+        CardNote::properties().npk_m_hash,
+        Comparator.EQ,
+        account_npk_m_hash
+    ).sort(CardNote::properties().points, SortOrder.DESC).set_offset(offset)
 }
 // docs:end:state_vars-NoteGetterOptionsSelectSortOffset
 
@@ -23,10 +27,10 @@ pub fn create_exact_card_getter_options(
     account_npk_m_hash: Field
 ) -> NoteGetterOptions<CardNote, CARD_NOTE_LEN, CARD_NOTE_BYTES_LEN, Field, Field> {
     let mut options = NoteGetterOptions::new();
-    options.select(CardNote::properties().points, points as Field, Option::none()).select(CardNote::properties().randomness, secret, Option::none()).select(
+    options.select(CardNote::properties().points, Comparator.EQ, points as Field).select(CardNote::properties().randomness, Comparator.EQ, secret).select(
         CardNote::properties().npk_m_hash,
-        account_npk_m_hash,
-        Option::none()
+        Comparator.EQ,
+        account_npk_m_hash
     )
 }
 // docs:end:state_vars-NoteGetterOptionsMultiSelects