feat!: commence call interface unification (#15894)

You can just do things. In this case, delete ~700 lines and end up with
a net improvement.

This deletes all of the `Void` variants of the call interfaces, in turn
deleting the `call_<>_void` variants, nicely unifying the testing
experience - we now just `call_public` or `call_private` without
worrying about return types. I also simplified `ReturnsHash` a bit since
it had three ways of achieving the same simple tasks, and added some
tests and clarifying comments about how the `()` case is handled.

There's lots more to improve on the `CallInterfaces`, and many more
removals to be done, but this initial pass is worth doing separately
because it directly improves `TestEnvironment`'s API.

Author: nventuro

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

@@ -10,9 +10,8 @@ pub mod call_interfaces;
 pub mod gas;
 
 pub use call_interfaces::{
-    PrivateCallInterface, PrivateStaticCallInterface, PrivateStaticVoidCallInterface,
-    PrivateVoidCallInterface, PublicCallInterface, PublicStaticCallInterface,
-    PublicStaticVoidCallInterface, PublicVoidCallInterface, UtilityCallInterface,
+    PrivateCallInterface, PrivateStaticCallInterface, PublicCallInterface,
+    PublicStaticCallInterface, UtilityCallInterface,
 };
 pub use private_context::PrivateContext;
 pub use public_context::PublicContext;

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

@@ -1,6 +1,13 @@
 use crate::{hash::hash_args_array, oracle::execution_cache};
 use dep::protocol_types::traits::Deserialize;
 
+/// A hash that represents a private contract function call's return value. Call `get_preimage` to get the underlying
+/// value.
+///
+/// The kernels don't process the actual return values but instead their hashes, so it is up to contracts to populate
+/// oracles with the preimages of these hashes on return to make them available to their callers.
+///
+/// Public calls don't utilize this mechanism since the AVM does process the full return values.
 pub struct ReturnsHash {
     hash: Field,
 }
@@ -10,36 +17,83 @@ impl ReturnsHash {
         ReturnsHash { hash }
     }
 
-    pub fn assert_empty(self) {
-        assert_eq(self.hash, 0);
-    }
-
-    pub fn raw(self) -> Field {
-        self.hash
-    }
-
-    /// This is only used during private execution, since in public it is the VM itself that keeps track of return
-    /// values.
+    /// Fetches the underlying return value from an oracle, constraining that it corresponds to the return data hash.
     pub fn get_preimage<T>(self) -> T
     where
         T: Deserialize,
     {
-        // Safety: We verify that the value returned by `load` is the preimage of `hash`, fully constraining it.
+        // Safety: We verify that the value returned by `load` is the preimage of `hash`, fully constraining it. If `T`
+        // is `()`, then `preimage` must be an array of length 0 (since that is `()`'s deserialization length).
+        // `hash_args_array` handles empty arrays following the protocol rules (i.e. an empty args array is signaled
+        // with a zero hash), correctly constraining `self.hash`.
         let preimage = unsafe { execution_cache::load(self.hash) };
-        assert_eq(self.hash, hash_args_array(preimage));
+        assert_eq(self.hash, hash_args_array(preimage), "Preimage mismatch");
 
         Deserialize::deserialize(preimage)
     }
+}
 
-    pub fn get_preimage_and_assert_empty<T>(self) -> T
-    where
-        T: Deserialize,
-    {
-        // Safety: We verify that the value returned by `load` is the preimage of `hash`, fully constraining it.
-        let preimage = unsafe { execution_cache::load(self.hash) };
-        assert_eq(self.hash, hash_args_array(preimage));
-        assert_eq(self.hash, 0);
+mod test {
+    use crate::{
+        hash::hash_args_array,
+        oracle::execution_cache,
+        test::{helpers::test_environment::TestEnvironment, mocks::mock_struct::MockStruct},
+    };
+    use super::ReturnsHash;
+    use protocol_types::traits::Serialize;
+    use std::test::OracleMock;
 
-        Deserialize::deserialize(preimage)
+    #[test]
+    unconstrained fn retrieves_preimage() {
+        let mut env = TestEnvironment::_new();
+        env.private_context(|_| {
+            let value = MockStruct::new(4, 7);
+            let serialized = value.serialize();
+
+            let hash = hash_args_array(serialized);
+            execution_cache::store(serialized, hash);
+
+            assert_eq(ReturnsHash::new(hash).get_preimage(), value);
+        });
     }
+
+    #[test]
+    unconstrained fn retrieves_empty_preimage() {
+        let mut env = TestEnvironment::_new();
+        env.private_context(|_| {
+            let value = ();
+            let serialized = [];
+
+            let hash = hash_args_array(serialized);
+            execution_cache::store(serialized, hash);
+
+            assert_eq(ReturnsHash::new(hash).get_preimage(), value);
+        });
+    }
+
+    #[test(should_fail_with = "Preimage mismatch")]
+    unconstrained fn rejects_bad_preimage() {
+        let value = MockStruct::new(4, 7);
+        let serialized = value.serialize();
+
+        let mut bad_serialized = serialized;
+        bad_serialized[0] += 1;
+
+        let hash = hash_args_array(serialized);
+
+        let _ = OracleMock::mock("loadFromExecutionCache").returns(bad_serialized);
+        assert_eq(ReturnsHash::new(hash).get_preimage(), value);
+    }
+
+    // This test passes due to a Noir bug.
+    // #[test(should_fail_with="Preimage mismatch")]
+    // unconstrained fn rejects_bad_empty_preimage() {
+    //     let value = ();
+    //     let serialized = [];
+
+    //     let hash = hash_args_array(serialized);
+
+    //     let _ = OracleMock::mock("loadFromExecutionCache").returns([1]);
+    //     assert_eq(ReturnsHash::new(hash).get_preimage(), value);
+    // }
 }

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

@@ -1,7 +1,7 @@
 use crate::macros::utils::{
     add_to_field_array, AsStrQuote, compute_fn_selector, is_fn_private, is_fn_public, is_fn_view,
 };
-use std::meta::{type_of, unquote};
+use std::meta::unquote;
 
 comptime global FROM_FIELD: TypedExpr = {
     let from_field_trait = quote { protocol_types::traits::FromField }.as_trait_constraint();
@@ -17,42 +17,21 @@ comptime global SERIALIZED_ARGS_NAME: Quoted = quote { serialized_args };
 
 pub comptime fn stub_fn(f: FunctionDefinition) -> Quoted {
     let is_static_call = is_fn_view(f);
-    let is_void = f.return_type() == type_of(());
 
     if is_fn_private(f) {
         if is_static_call {
-            if is_void {
-                create_private_static_void_stub(f)
-            } else {
-                create_private_static_stub(f)
-            }
+            create_private_static_stub(f)
         } else {
-            if is_void {
-                create_private_void_stub(f)
-            } else {
-                create_private_stub(f)
-            }
+            create_private_stub(f)
         }
     } else if is_fn_public(f) {
         if is_static_call {
-            if is_void {
-                create_public_static_void_stub(f)
-            } else {
-                create_public_static_stub(f)
-            }
+            create_public_static_stub(f)
         } else {
-            if is_void {
-                create_public_void_stub(f)
-            } else {
-                create_public_stub(f)
-            }
+            create_public_stub(f)
         }
     } else {
-        if is_void {
-            create_utility_void_stub(f)
-        } else {
-            create_utility_stub(f)
-        }
+        create_utility_stub(f)
     }
 }
 
@@ -146,43 +125,6 @@ comptime fn create_private_static_stub(f: FunctionDefinition) -> Quoted {
     }
 }
 
-comptime fn create_private_void_stub(f: FunctionDefinition) -> Quoted {
-    let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
-        create_stub_base(f);
-
-    quote {
-        pub fn $fn_name(self, $fn_parameters_list) -> dep::aztec::context::call_interfaces::PrivateVoidCallInterface<$fn_name_len, ()> {
-            $serialized_args_slice_construction
-            let selector = $FROM_FIELD($fn_selector);
-            dep::aztec::context::call_interfaces::PrivateVoidCallInterface::new(
-                self.target_contract,
-                selector,
-                $fn_name_str,
-                $SERIALIZED_ARGS_NAME,
-                false
-            )
-        }
-    }
-}
-
-comptime fn create_private_static_void_stub(f: FunctionDefinition) -> Quoted {
-    let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
-        create_stub_base(f);
-
-    quote {
-        pub fn $fn_name(self, $fn_parameters_list) -> dep::aztec::context::call_interfaces::PrivateStaticVoidCallInterface<$fn_name_len, ()> {
-            $serialized_args_slice_construction
-            let selector = $FROM_FIELD($fn_selector);
-            dep::aztec::context::call_interfaces::PrivateStaticVoidCallInterface::new(
-                self.target_contract,
-                selector,
-                $fn_name_str,
-                serialized_args
-            )
-        }
-    }
-}
-
 comptime fn create_public_stub(f: FunctionDefinition) -> Quoted {
     let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
         create_stub_base(f);
@@ -222,43 +164,6 @@ comptime fn create_public_static_stub(f: FunctionDefinition) -> Quoted {
     }
 }
 
-comptime fn create_public_void_stub(f: FunctionDefinition) -> Quoted {
-    let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
-        create_stub_base(f);
-
-    quote {
-        pub fn $fn_name(self, $fn_parameters_list) -> dep::aztec::context::call_interfaces::PublicVoidCallInterface<$fn_name_len, ()> {
-            $serialized_args_slice_construction
-            let selector = $FROM_FIELD($fn_selector);
-            dep::aztec::context::call_interfaces::PublicVoidCallInterface::new(
-                self.target_contract,
-                selector,
-                $fn_name_str,
-                $SERIALIZED_ARGS_NAME,
-                false
-            )
-        }
-    }
-}
-
-comptime fn create_public_static_void_stub(f: FunctionDefinition) -> Quoted {
-    let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
-        create_stub_base(f);
-
-    quote {
-        pub fn $fn_name(self, $fn_parameters_list) -> dep::aztec::context::call_interfaces::PublicStaticVoidCallInterface<$fn_name_len, ()> {
-            $serialized_args_slice_construction
-            let selector = $FROM_FIELD($fn_selector);
-            dep::aztec::context::call_interfaces::PublicStaticVoidCallInterface::new(
-                self.target_contract,
-                selector,
-                $fn_name_str,
-                serialized_args
-            )
-        }
-    }
-}
-
 comptime fn create_utility_stub(f: FunctionDefinition) -> Quoted {
     let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
         create_stub_base(f);
@@ -280,24 +185,3 @@ comptime fn create_utility_stub(f: FunctionDefinition) -> Quoted {
         }
     }
 }
-
-comptime fn create_utility_void_stub(f: FunctionDefinition) -> Quoted {
-    let (fn_name, fn_parameters_list, serialized_args_slice_construction, fn_name_str, fn_name_len, fn_selector) =
-        create_stub_base(f);
-
-    // This is here because utility function call interfaces can only be used within TXe tests.
-    let modified_fn_name = f"_experimental_{fn_name}".quoted_contents();
-
-    quote {
-        pub fn $modified_fn_name(self, $fn_parameters_list) -> dep::aztec::context::call_interfaces::UtilityVoidCallInterface<$fn_name_len, ()> {
-            $serialized_args_slice_construction
-            let selector = $FROM_FIELD($fn_selector);
-            dep::aztec::context::call_interfaces::UtilityVoidCallInterface::new(
-                self.target_contract,
-                selector,
-                $fn_name_str,
-                $SERIALIZED_ARGS_NAME,
-            )
-        }
-    }
-}

noir-projects/aztec-nr/aztec/src/macros/functions/call_interface_stubs.nr

@@ -1,6 +1,6 @@
 use crate::{
     authwit::auth::{compute_authwit_message_hash, compute_inner_authwit_hash},
-    context::call_interfaces::{CallInterface, PublicVoidCallInterface},
+    context::call_interfaces::{CallInterface, PublicCallInterface},
     hash::hash_args,
     oracle::execution::{get_chain_id, get_version},
     test::helpers::{test_environment::TestEnvironment, txe_oracles},
@@ -48,9 +48,9 @@ where
         compute_inner_authwit_hash([caller.to_field(), selector.to_field(), args_hash]);
     let message_hash = compute_authwit_message_hash(target, chain_id, version, inner_hash);
 
-    let _ = env.call_public_void(
+    let _ = env.call_public(
         on_behalf_of,
-        PublicVoidCallInterface::<_, ()>::new(
+        PublicCallInterface::<_, ()>::new(
             CANONICAL_AUTH_REGISTRY_ADDRESS,
             comptime { FunctionSelector::from_signature("set_authorized(Field,bool)") },
             "set_authorized",