chore: Fix some instances of missing unsafe blocks

noir-projects/aztec-nr/aztec/src/history/note_inclusion.nr

@@ -12,13 +12,15 @@ trait ProveNoteInclusion {
 
 impl ProveNoteInclusion for Header {
     fn prove_note_inclusion<Note, N, M>(self, note: Note) where Note: NoteInterface<N, M> {
-        // 1) Compute note_hash
         let note_hash = compute_note_hash_for_nullify(note);
 
-        // 2) Get the membership witness of the note in the note hash tree
-        let witness = get_note_hash_membership_witness(self.global_variables.block_number as u32, note_hash);
+        let witness = unsafe {
+            get_note_hash_membership_witness(self.global_variables.block_number as u32, note_hash)
+        };
 
-        // 3) Prove that the commitment is in the note hash tree
+        // Note inclusion is fairly straightforward, since all we need to prove is that a note exists in the note tree -
+        // we don't even care _where_ in the tree it is stored. This is because entries in the note hash tree are
+        // unique.
         assert_eq(
             self.state.partial.note_hash_tree.root, root_from_sibling_path(note_hash, witness.index, witness.path), "Proving note inclusion failed"
         );

noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion.nr

@@ -13,21 +13,21 @@ trait ProveNullifierInclusion {
 impl ProveNullifierInclusion for Header {
     fn prove_nullifier_inclusion(self, nullifier: Field) {
         // 1) Get the membership witness of the nullifier
-        let witness = get_nullifier_membership_witness(self.global_variables.block_number as u32, nullifier);
+        let witness = unsafe {
+            get_nullifier_membership_witness(self.global_variables.block_number as u32, nullifier)
+        };
 
-        // 2) Check that the witness we obtained matches the nullifier
-        assert(witness.leaf_preimage.nullifier == nullifier, "Nullifier does not match value in witness");
-
-        // 3) Compute the nullifier tree leaf
-        let nullifier_leaf = witness.leaf_preimage.hash();
-
-        // 4) Prove that the nullifier is in the nullifier tree
+        // 2) First we prove that the tree leaf in the witness is present in the nullifier tree. This is expected to be
+        // the leaf that contains the nullifier we're proving inclusion for.
         assert(
             self.state.partial.nullifier_tree.root
-            == root_from_sibling_path(nullifier_leaf, witness.index, witness.path), "Proving nullifier inclusion failed"
+            == root_from_sibling_path(witness.leaf_preimage.hash(), witness.index, witness.path), "Proving nullifier inclusion failed"
         );
-        // --> Now we have traversed the trees all the way up to archive root and verified that the nullifier
-        //     was included in the nullifier tree.
+
+        // 3) Then we simply check that the value in the leaf is the expected one. Note that we don't need to perform
+        // any checks on the rest of the values in the leaf preimage (the next index or next nullifier), since all we
+        // care about is showing that the tree contains an entry with the expected nullifier.
+        assert(witness.leaf_preimage.nullifier == nullifier, "Nullifier does not match value in witness");
     }
 }
 

noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion.nr

@@ -12,30 +12,31 @@ trait ProveNullifierNonInclusion {
 impl ProveNullifierNonInclusion for Header {
     fn prove_nullifier_non_inclusion(self, nullifier: Field) {
         // 1) Get the membership witness of a low nullifier of the nullifier
-        let witness = get_low_nullifier_membership_witness(self.global_variables.block_number as u32, nullifier);
-
-        // 2) Prove that the nullifier is not included in the nullifier tree
-
-        // 2.a) Compute the low nullifier leaf and prove that it is in the nullifier tree
-        let low_nullifier_leaf = witness.leaf_preimage.hash();
+        let witness = unsafe {
+            get_low_nullifier_membership_witness(self.global_variables.block_number as u32, nullifier)
+        };
+
+        // 2) First we prove that the tree leaf in the witness is present in the nullifier tree. This is expected to be
+        // the 'low leaf', i.e. the leaf that would come immediately before the nullifier's leaf, if the nullifier were
+        // to be in the tree.
+        let low_nullifier_leaf = witness.leaf_preimage;
         assert(
             self.state.partial.nullifier_tree.root
-            == root_from_sibling_path(low_nullifier_leaf, witness.index, witness.path), "Proving nullifier non-inclusion failed: Could not prove low nullifier inclusion"
+            == root_from_sibling_path(low_nullifier_leaf.hash(), witness.index, witness.path), "Proving nullifier non-inclusion failed: Could not prove low nullifier inclusion"
         );
 
-        // 2.b) Prove that the low nullifier is smaller than the nullifier
+        // 3) Prove that the low leaf is indeed smaller than the nullifier
         assert(
-            full_field_less_than(witness.leaf_preimage.nullifier, nullifier), "Proving nullifier non-inclusion failed: low_nullifier.value < nullifier.value check failed"
+            full_field_less_than(low_nullifier_leaf.nullifier, nullifier), "Proving nullifier non-inclusion failed: low_nullifier.value < nullifier.value check failed"
         );
 
-        // 2.c) Prove that the low nullifier is pointing "over" the nullifier to prove that the nullifier is not
-        // included in the nullifier tree (or to 0 if the to-be-inserted nullifier is the largest of all)
+        // 4) Prove that the low leaf is pointing "over" the nullifier, which means that the nullifier is not included
+        // in the nullifier tree, since if it were it'd need to be between the low leaf and the next leaf. Note the
+        // special case in which the low leaf is the largest of all entries, in which case there's no 'next' entry.
         assert(
-            full_field_greater_than(witness.leaf_preimage.next_nullifier, nullifier)
-            | (witness.leaf_preimage.next_index == 0), "Proving nullifier non-inclusion failed: low_nullifier.next_value > nullifier.value check failed"
+            full_field_greater_than(low_nullifier_leaf.next_nullifier, nullifier)
+            | (low_nullifier_leaf.next_index == 0), "Proving nullifier non-inclusion failed: low_nullifier.next_value > nullifier.value check failed"
         );
-        // --> Now we have traversed the trees all the way up to archive root and verified that the nullifier
-        //     was not yet included in the nullifier tree.
     }
 }
 

noir-projects/aztec-nr/aztec/src/history/public_storage.nr

@@ -12,24 +12,36 @@ trait PublicStorageHistoricalRead {
 
 impl PublicStorageHistoricalRead for Header {
     fn public_storage_historical_read(self, storage_slot: Field, contract_address: AztecAddress) -> Field {
-        // 1) Compute the leaf slot by siloing the storage slot with the contract address
+        // 1) Compute the leaf index by siloing the storage slot with the contract address
         let public_data_tree_index = poseidon2_hash_with_separator(
             [contract_address.to_field(), storage_slot],
             GENERATOR_INDEX__PUBLIC_LEAF_INDEX
         );
 
-        // 2) Get the membership witness of the slot
-        let witness = get_public_data_witness(
-            self.global_variables.block_number as u32,
-            public_data_tree_index
+        // 2) Get the membership witness for the tree index.
+        let witness = unsafe {
+            get_public_data_witness(
+                self.global_variables.block_number as u32,
+                public_data_tree_index
+            )
+        };
+
+        // 3) The witness is made up of two parts: the preimage of the leaf and the proof that it exists in the tree.
+        // We first prove that the witness is indeed valid for the public data tree, i.e. that the preimage is of a
+        // value present in the tree. Note that `hash` returns not just the hash of the value but also the metadata
+        // (slot, next index and next slot).
+        assert(
+            self.state.partial.public_data_tree.root
+            == root_from_sibling_path(witness.leaf_preimage.hash(), witness.index, witness.path), "Proving public value inclusion failed"
         );
 
-        // 3) Extract the value from the witness leaf and check that the storage slot is correct
+        // 4) Now that we know the preimage is valid, we determine the value that's represented by this tree entry. Here
+        // we have two scenarios:
+        // 1. The tree entry is initialized, and the value is the same as the one in the witness
+        // 2. The entry was never initialized, and the value is default zero (the default)
+        // The code below is based on the same checks in `validate_public_data_reads` in `base_rollup_inputs`.
         let preimage = witness.leaf_preimage;
 
-        // Here we have two cases. Code based on same checks in `validate_public_data_reads` in `base_rollup_inputs`
-        // 1. The value is the same as the one in the witness
-        // 2. The value was never initialized and is zero
         let is_less_than_slot = full_field_less_than(preimage.slot, public_data_tree_index);
         let is_next_greater_than = full_field_less_than(public_data_tree_index, preimage.next_slot);
         let is_max = ((preimage.next_index == 0) & (preimage.next_slot == 0));
@@ -42,13 +54,6 @@ impl PublicStorageHistoricalRead for Header {
             preimage.value
         };
 
-        // 4) Prove that the leaf exists in the public data tree. Note that `hash` returns not just the hash of the value
-        // but also the metadata (slot, next index and next slot).
-        assert(
-            self.state.partial.public_data_tree.root
-            == root_from_sibling_path(preimage.hash(), witness.index, witness.path), "Proving public value inclusion failed"
-        );
-
         value
     }
 }

noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr

@@ -24,9 +24,7 @@ global KEY_REGISTRY_UPDATE_BLOCKS = 5;
 
 global KEY_REGISTRY_STORAGE_SLOT = 1;
 
-// A helper function since requesting nsk_app is very common
-// TODO(#6543)
-pub fn get_nsk_app(npk_m_hash: Field) -> Field {
+unconstrained pub fn get_nsk_app(npk_m_hash: Field) -> Field {
     get_key_validation_request(npk_m_hash, NULLIFIER_INDEX).sk_app
 }
 

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

@@ -80,13 +80,17 @@ pub fn get_note<Note, let N: u32, let M: u32>(
     context: &mut PrivateContext,
     storage_slot: Field
 ) -> (Note, Field) where Note: NoteInterface<N, M> {
-    let note = get_note_internal(storage_slot);
+    let note = unsafe {
+        get_note_internal(storage_slot)
+    };
 
+    // Constraining that we got a valid note from the oracle is fairly straightforward: all we need to do is check that
+    // the metadata is correct, and that the note exists.
     check_note_header(*context, storage_slot, note);
 
     let note_hash_for_read_request = compute_note_hash_for_read_request(note);
-
     context.push_note_hash_read_request(note_hash_for_read_request);
+
     (note, note_hash_for_read_request)
 }
 
@@ -95,8 +99,12 @@ pub fn get_notes<Note, let N: u32, let M: u32, PREPROCESSOR_ARGS, FILTER_ARGS>(
     storage_slot: Field,
     options: NoteGetterOptions<Note, N, M, PREPROCESSOR_ARGS, FILTER_ARGS>
 ) -> (BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL>, BoundedVec<Field, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL>) where Note: NoteInterface<N, M> + Eq {
-    let opt_notes = get_notes_internal(storage_slot, options);
+    let opt_notes = unsafe {
+        get_notes_internal(storage_slot, options)
+    };
 
+    // We apply the constraints in a separate function instead of inlining them here to make it easier to test that
+    // these checks correctly reject bad notes.
     constrain_get_notes_internal(context, storage_slot, opt_notes, options)
 }
 

noir-projects/aztec-nr/aztec/src/oracle/get_contract_instance.nr

@@ -19,8 +19,13 @@ unconstrained pub fn get_contract_instance_internal_avm(address: AztecAddress) -
 }
 
 pub fn get_contract_instance(address: AztecAddress) -> ContractInstance {
-    let instance = ContractInstance::deserialize(get_contract_instance_internal(address));
-    assert(instance.to_address().eq(address));
+    let instance = unsafe {
+        ContractInstance::deserialize(get_contract_instance_internal(address))
+    };
+    // The to_address function combines all values in the instance object to produce an address, so by checking that we
+    // get the expected address we validate the entire struct.
+    assert_eq(instance.to_address(), address);
+
     instance
 }
 

noir-projects/aztec-nr/aztec/src/oracle/header.nr

@@ -34,7 +34,9 @@ pub fn get_header_at(block_number: u32, context: PrivateContext) -> Header {
         );
 
         // 3) Get the header hint of a given block from an oracle
-        let historical = get_header_at_internal(block_number);
+        let historical = unsafe {
+            get_header_at_internal(block_number)
+        };
 
         // 4) We make sure that the header hint we received from the oracle exists in the state tree and is the actual header
         // at the desired block number
@@ -60,7 +62,9 @@ fn constrain_get_header_at_internal(
     let block_hash = header_hint.hash();
 
     // 2) Get the membership witness of the block in the archive tree
-    let witness = get_archive_membership_witness(last_archive_block_number, block_hash);
+    let witness = unsafe {
+        get_archive_membership_witness(last_archive_block_number, block_hash)
+    };
 
     // 3) Check that the block is in the archive (i.e. the witness is valid)
     assert(

noir-projects/aztec-nr/aztec/src/oracle/key_validation_request.nr

@@ -14,7 +14,9 @@ unconstrained fn get_key_validation_request_internal(
     KeyValidationRequest::deserialize(result)
 }
 
-pub fn get_key_validation_request(pk_m_hash: Field, key_index: Field) -> KeyValidationRequest {
+unconstrained pub fn get_key_validation_request(
+    pk_m_hash: Field,
+    key_index: Field
+) -> KeyValidationRequest {
     get_key_validation_request_internal(pk_m_hash, key_index)
 }
-

noir-projects/aztec-nr/aztec/src/oracle/keys.nr

@@ -4,12 +4,8 @@ use dep::protocol_types::{address::{AztecAddress, PartialAddress}, point::Point}
 #[oracle(getPublicKeysAndPartialAddress)]
 unconstrained fn get_public_keys_and_partial_address_oracle(_address: AztecAddress) -> [Field; 13] {}
 
-unconstrained fn get_public_keys_and_partial_address_oracle_wrapper(address: AztecAddress) -> [Field; 13] {
-    get_public_keys_and_partial_address_oracle(address)
-}
-
-pub fn get_public_keys_and_partial_address(address: AztecAddress) -> (PublicKeys, PartialAddress) {
-    let result = get_public_keys_and_partial_address_oracle_wrapper(address);
+unconstrained pub fn get_public_keys_and_partial_address(address: AztecAddress) -> (PublicKeys, PartialAddress) {
+    let result = get_public_keys_and_partial_address_oracle(address);
 
     let keys = PublicKeys {
         npk_m: NpkM { inner: Point { x: result[0], y: result[1], is_infinite: result[2] as bool } },

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

@@ -70,13 +70,15 @@ impl<Note, N, M> PrivateMutable<Note, &mut PrivateContext> where Note: NoteInter
     // docs:end:replace
 
     pub fn initialize_or_replace(self, note: &mut Note) -> NoteEmission<Note> {
-        let is_initialized = check_nullifier_exists(self.compute_initialization_nullifier());
+        let is_initialized = unsafe {
+            check_nullifier_exists(self.compute_initialization_nullifier())
+        };
 
-        // check_nullifier_exists() is an unconstrained function - we can constrain a true value by providing an 
-        // inclusion proof of the nullifier, but cannot constrain a false value since a non-inclusion proof would only 
+        // check_nullifier_exists() is an unconstrained function - we can constrain a true value by providing an
+        // inclusion proof of the nullifier, but cannot constrain a false value since a non-inclusion proof would only
         // be valid if done in public.
         // Ultimately, this is not an issue ginen that we'll either:
-        //  - initialize the state variable, which would fail if it was already initialized due to the duplicate 
+        //  - initialize the state variable, which would fail if it was already initialized due to the duplicate
         //    nullifier, or
         //  - replace the current value, which would fail if it was not initialized since we wouldn't be able to produce
         //    an inclusion proof for the current note

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

@@ -97,27 +97,28 @@ impl<T, let INITIAL_DELAY: u32, Context> SharedMutable<T, INITIAL_DELAY, Context
         // Instead, we get an oracle to provide us the correct values for both the value and delay changes, and instead
         // prove inclusion of their hash, which is both a much smaller proof (a single slot), and also independent of
         // the size of T.
-        let (value_change_hint, delay_change_hint) = get_public_storage_hints(address, self.storage_slot, historical_block_number);
+        let (value_change_hint, delay_change_hint) = unsafe {
+            get_public_storage_hints(address, self.storage_slot, historical_block_number)
+        };
 
         // Ideally the following would be simply public_storage::read_historical, but we can't implement that yet.
         let hash = header.public_storage_historical_read(self.get_hash_storage_slot(), address);
 
-        // @todo This is written strangely to bypass a formatting issue with the if that is breaking ci.
-        let (a, b, c) = if hash != 0 {
-            let a = SharedMutable::hash_scheduled_data(value_change_hint, delay_change_hint);
-            (a, value_change_hint, delay_change_hint)
+        if hash != 0 {
+            assert_eq(
+                hash, SharedMutable::hash_scheduled_data(value_change_hint, delay_change_hint), "Hint values do not match hash"
+            );
         } else {
             // The hash slot can only hold a zero if it is uninitialized, meaning no value or delay change was ever
             // scheduled. Therefore, the hints must then correspond to uninitialized scheduled changes.
-            let b = ScheduledValueChange::deserialize(zeroed());
-            let c = ScheduledDelayChange::deserialize(zeroed());
-            (hash, b, c)
+            assert_eq(
+                value_change_hint, ScheduledValueChange::deserialize(zeroed()), "Non-zero value change for zero hash"
+            );
+            assert_eq(
+                delay_change_hint, ScheduledDelayChange::deserialize(zeroed()), "Non-zero delay change for zero hash"
+            );
         };
 
-        assert_eq(hash, a, "Hint values do not match hash");
-        assert_eq(value_change_hint, b, "Non-zero value change for zero hash");
-        assert_eq(delay_change_hint, c, "Non-zero delay change for zero hash");
-
         (value_change_hint, delay_change_hint, historical_block_number)
     }
 }

noir-projects/aztec-nr/aztec/src/utils/collapse_array.nr

@@ -6,14 +6,19 @@ use dep::protocol_types::traits::Eq;
 // this returns
 //   collapsed: [3, 1]
 pub fn collapse_array<T, let N: u32>(input: [Option<T>; N]) -> BoundedVec<T, N> where T: Eq {
+    // Computing the collpased BoundedVec would result in a very large number of constraints, since we'd need to loop
+    // over the input array and conditionally write to a dynamic vec index, which is a very unfriendly pattern to the
+    // proving backend.
+    // Instead, we use an unconstrained function to produce the final collapsed array, along with some hints, and then
+    // verify that the input and collapsed arrays are equivalent.
     let (collapsed, collapsed_to_input_index_mapping) = unsafe {
         get_collapse_hints(input)
     };
     verify_collapse_hints(input, collapsed, collapsed_to_input_index_mapping);
     collapsed
 }
 
-pub fn verify_collapse_hints<T, let N: u32>(
+fn verify_collapse_hints<T, let N: u32>(
     input: [Option<T>; N],
     collapsed: BoundedVec<T, N>,
     collapsed_to_input_index_mapping: BoundedVec<u32, N>

noir-projects/aztec-nr/aztec/src/utils/mod.nr

@@ -3,4 +3,4 @@ mod comparison;
 mod point;
 mod test;
 
-use crate::utils::collapse_array::{collapse_array, verify_collapse_hints};
+use crate::utils::collapse_array::collapse_array;