chore: misc unsafe improvements (#8803)

Continuation of the safety efforts: I removed the `unconstrained`
property from some functions that are actually always safe to call (e.g.
logs), improved documentation a bit, and expanded on some `unsafe`
callsites explaining why what we're doing is correct.

This was originally much larger, covering `unsafe_rand` and
`get_key_validation_request`, but I left those out as things got a bit
out of hand.

noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr

@@ -53,10 +53,7 @@ pub struct PrivateCallInterface<let N: u32, T, Env> {
 
 impl<let N: u32, T, Env> PrivateCallInterface<N, T, Env> {
     pub fn call<let M: u32>(self, context: &mut PrivateContext) -> T where T: Deserialize<M> {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         let returns = context.call_private_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -69,19 +66,13 @@ impl<let N: u32, T, Env> PrivateCallInterface<N, T, Env> {
     }
 
     pub fn view<let M: u32>(self, context: &mut PrivateContext) -> T where T: Deserialize<M> {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false);
         returns.unpack_into()
     }
 
     pub fn delegate_call<let M: u32>(self, context: &mut PrivateContext) -> T where T: Deserialize<M> {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, false, true);
         returns.unpack_into()
     }
@@ -105,10 +96,7 @@ pub struct PrivateVoidCallInterface<let N: u32, Env> {
 
 impl<let N: u32, Env> PrivateVoidCallInterface<N, Env> {
     pub fn call(self, context: &mut PrivateContext) {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_private_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -119,18 +107,12 @@ impl<let N: u32, Env> PrivateVoidCallInterface<N, Env> {
     }
 
     pub fn view(self, context: &mut PrivateContext) {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false).assert_empty();
     }
 
     pub fn delegate_call(self, context: &mut PrivateContext) {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, false, true).assert_empty();
     }
 }
@@ -153,10 +135,7 @@ pub struct PrivateStaticCallInterface<let N: u32, T, Env> {
 
 impl<let N: u32, T, Env> PrivateStaticCallInterface<N, T, Env> {
     pub fn view<let M: u32>(self, context: &mut PrivateContext) -> T where T: Deserialize<M> {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false);
         returns.unpack_into()
     }
@@ -180,10 +159,7 @@ pub struct PrivateStaticVoidCallInterface<let N: u32, Env> {
 
 impl<let N: u32, Env> PrivateStaticVoidCallInterface<N, Env> {
     pub fn view(self, context: &mut PrivateContext) {
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(self.args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false).assert_empty();
     }
 }
@@ -227,10 +203,7 @@ impl<let N: u32, T, Env> PublicCallInterface<N, T, Env> {
 
     pub fn enqueue(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -242,10 +215,7 @@ impl<let N: u32, T, Env> PublicCallInterface<N, T, Env> {
 
     pub fn enqueue_view(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -257,10 +227,7 @@ impl<let N: u32, T, Env> PublicCallInterface<N, T, Env> {
 
     pub fn delegate_enqueue(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -310,10 +277,7 @@ impl<let N: u32, Env> PublicVoidCallInterface<N, Env> {
 
     pub fn enqueue(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -325,10 +289,7 @@ impl<let N: u32, Env> PublicVoidCallInterface<N, Env> {
 
     pub fn enqueue_view(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -340,10 +301,7 @@ impl<let N: u32, Env> PublicVoidCallInterface<N, Env> {
 
     pub fn delegate_enqueue(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -384,10 +342,7 @@ impl<let N: u32, T, Env> PublicStaticCallInterface<N, T, Env> {
 
     pub fn enqueue_view(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,
@@ -427,10 +382,7 @@ impl<let N: u32, Env> PublicStaticVoidCallInterface<N, Env> {
 
     pub fn enqueue_view(self, context: &mut PrivateContext) {
         let args_hash = hash_args(self.args);
-        let packed_args_hash = unsafe {
-            pack_arguments(self.args)
-        };
-        assert(args_hash == packed_args_hash);
+        pack_arguments(self.args);
         context.call_public_function_with_packed_args(
             self.target_contract,
             self.selector,

noir-projects/aztec-nr/aztec/src/context/packed_returns.nr

@@ -19,7 +19,11 @@ impl PackedReturns {
     }
 
     pub fn unpack<let N: u32>(self) -> [Field; N] {
-        let unpacked: [Field; N] = unpack_returns(self.packed_returns);
+        // We verify that the value returned by `unpack_returns` is the preimage of `packed_returns`, fully constraining
+        // it.
+        let unpacked: [Field; N] = unsafe {
+            unpack_returns(self.packed_returns)
+        };
         assert_eq(self.packed_returns, hash_args_array(unpacked));
         unpacked
     }

noir-projects/aztec-nr/aztec/src/context/private_context.nr

@@ -295,7 +295,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) -> PackedReturns {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.call_private_function_with_packed_args(contract_address, function_selector, args_hash, false, false)
     }
 
@@ -306,7 +306,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) -> PackedReturns {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.call_private_function_with_packed_args(contract_address, function_selector, args_hash, true, false)
     }
 
@@ -317,7 +317,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) -> PackedReturns {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.call_private_function_with_packed_args(contract_address, function_selector, args_hash, false, true)
     }
 
@@ -407,7 +407,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.call_public_function_with_packed_args(contract_address, function_selector, args_hash, false, false)
     }
 
@@ -418,7 +418,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.call_public_function_with_packed_args(contract_address, function_selector, args_hash, true, false)
     }
 
@@ -429,7 +429,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.call_public_function_with_packed_args(contract_address, function_selector, args_hash, false, true)
     }
 
@@ -495,7 +495,7 @@ impl PrivateContext {
         args: [Field; ARGS_COUNT]
     ) {
         let args_hash = hash_args_array(args);
-        assert(args_hash == arguments::pack_arguments_array(args));
+        arguments::pack_arguments_array(args);
         self.set_public_teardown_function_with_packed_args(contract_address, function_selector, args_hash, false, false)
     }
 

noir-projects/aztec-nr/aztec/src/messaging.nr

@@ -3,8 +3,7 @@ use crate::{
     oracle::get_l1_to_l2_membership_witness::get_l1_to_l2_membership_witness
 };
 
-use dep::protocol_types::merkle_tree::root::root_from_sibling_path;
-use dep::protocol_types::{constants::L1_TO_L2_MSG_TREE_HEIGHT, address::{AztecAddress, EthAddress}, utils::arr_copy_slice};
+use dep::protocol_types::{address::{AztecAddress, EthAddress}, merkle_tree::root::root_from_sibling_path};
 
 pub fn process_l1_to_l2_message(
     l1_to_l2_root: Field,
@@ -25,12 +24,12 @@ pub fn process_l1_to_l2_message(
         secret_hash
     );
 
-    let returned_message = get_l1_to_l2_membership_witness(storage_contract_address, message_hash, secret);
-    let leaf_index = returned_message[0];
-    let sibling_path = arr_copy_slice(returned_message, [0; L1_TO_L2_MSG_TREE_HEIGHT], 1);
+    // We prove that `message_hash` is in the tree by showing the derivation of the tree root, using a merkle path we
+    // get from an oracle.
+    let (leaf_index, sibling_path) = unsafe {
+        get_l1_to_l2_membership_witness(storage_contract_address, message_hash, secret)
+    };
 
-    // Check that the message is in the tree
-    // This is implicitly checking that the values of the message are correct
     let root = root_from_sibling_path(message_hash, leaf_index, sibling_path);
     assert(root == l1_to_l2_root, "Message not in state");
 

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

@@ -19,15 +19,12 @@ pub fn create_note<Note, let N: u32>(
     let note_hash = note.compute_note_hash();
 
     let serialized_note = Note::serialize_content(*note);
-    assert(
-        notify_created_note(
+    notify_created_note(
         storage_slot,
         Note::get_note_type_id(),
         serialized_note,
         note_hash,
         note_hash_counter
-    )
-        == 0
     );
 
     context.push_note_hash(note_hash);
@@ -81,8 +78,7 @@ pub fn destroy_note_unsafe<Note, let N: u32>(
     };
 
     let nullifier_counter = context.side_effect_counter;
-    assert(notify_nullified_note(nullifier, notification_note_hash, nullifier_counter) == 0);
+    notify_nullified_note(nullifier, notification_note_hash, nullifier_counter);
 
     context.push_nullifier_for_note_hash(nullifier, notification_note_hash)
 }
-

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

@@ -105,7 +105,7 @@ pub fn compute_note_hash_for_nullify<Note, let N: u32>(note: Note) -> Field wher
     compute_note_hash_for_nullify_internal(note, note_hash_for_read_request)
 }
 
-pub fn compute_note_hash_and_optionally_a_nullifier<T, let N: u32, let S: u32>(
+unconstrained pub fn compute_note_hash_and_optionally_a_nullifier<T, let N: u32, let S: u32>(
     deserialize_content: fn([Field; N]) -> T,
     note_header: NoteHeader,
     compute_nullifier: bool,

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

@@ -1,24 +1,34 @@
-#[oracle(packArgumentsArray)]
-unconstrained fn pack_arguments_array_oracle<let N: u32>(_args: [Field; N]) -> Field {}
+/// Notifies the simulator that `args` will later be used at some point during execution, referenced by their hash. This
+/// allows the simulator to know how to respond to this future request.
+///
+/// This is only used during private execution, since in public it is the VM itself that keeps track of arguments.
+pub fn pack_arguments(args: [Field]) {
+    // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. When
+    // unpacking however the caller must check that the returned value is indeed the preimage.
+    unsafe {
+        pack_arguments_oracle_wrapper(args)
+    };
+}
 
-#[oracle(packArguments)]
-unconstrained fn pack_arguments_oracle(_args: [Field]) -> Field {}
+/// Same as `pack_arguments`, but using arrays instead of slices.
+pub fn pack_arguments_array<let N: u32>(args: [Field; N]) {
+    // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. When
+    // unpacking however the caller must check that the returned value is indeed the preimage.
+    unsafe {
+        pack_arguments_array_oracle_wrapper(args)
+    };
+}
 
-/// - Pack arguments (array version) will notify the simulator that these arguments will be used later at
-///   some point in the call.
-/// - When the external call is made later, the simulator will know what the values unpack to.
-/// - This oracle will not be required in public vm functions, as the vm will keep track of arguments
-///   itself.
-unconstrained pub fn pack_arguments_array<let N: u32>(args: [Field; N]) -> Field {
-    pack_arguments_array_oracle(args)
+unconstrained fn pack_arguments_oracle_wrapper(args: [Field]) {
+    let _ = pack_arguments_oracle(args);
 }
 
-/// - Pack arguments (slice version) will notify the simulator that these arguments will be used later at
-///   some point in the call.
-/// - When the external call is made later, the simulator will know what the values unpack to.
-/// - This oracle will not be required in public vm functions, as the vm will keep track of arguments
-///   itself.
-unconstrained pub fn pack_arguments(args: [Field]) -> Field {
-    pack_arguments_oracle(args)
+unconstrained fn pack_arguments_array_oracle_wrapper<let N: u32>(args: [Field; N]) {
+    let _ = pack_arguments_array_oracle(args);
 }
 
+#[oracle(packArguments)]
+unconstrained fn pack_arguments_oracle(_args: [Field]) -> Field {}
+
+#[oracle(packArgumentsArray)]
+unconstrained fn pack_arguments_array_oracle<let N: u32>(_args: [Field; N]) -> Field {}

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

@@ -56,9 +56,15 @@ unconstrained pub fn set_public_teardown_function_call_internal(
     );
 }
 
-#[oracle(notifySetMinRevertibleSideEffectCounter)]
-unconstrained fn notify_set_min_revertible_side_effect_counter_oracle(_counter: u32) {}
+pub fn notify_set_min_revertible_side_effect_counter(counter: u32) {
+    unsafe {
+        notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter)
+    };
+}
 
-unconstrained pub fn notify_set_min_revertible_side_effect_counter(counter: u32) {
+unconstrained pub fn notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter: u32) {
     notify_set_min_revertible_side_effect_counter_oracle(counter);
 }
+
+#[oracle(notifySetMinRevertibleSideEffectCounter)]
+unconstrained fn notify_set_min_revertible_side_effect_counter_oracle(_counter: u32) {}