feat: panicking on direct calls to external funcs

avm-transpiler/src/transpile_contract.rs

@@ -92,7 +92,7 @@ impl From<CompiledAcirContractArtifact> for TranspiledContractArtifact {
         let mut has_public_dispatch = false;
 
         for function in contract.functions {
-            if function.custom_attributes.contains(&"public".to_string()) {
+            if function.custom_attributes.contains(&"abi_public".to_string()) {
                 if function.name == "public_dispatch" {
                     has_public_dispatch = true;
                 }
@@ -192,7 +192,7 @@ fn create_revert_dispatch_fn() -> AvmOrAcirContractFunctionArtifact {
     let empty_dispatch_fn = AvmContractFunctionArtifact {
         name: "public_dispatch".to_string(),
         is_unconstrained: true,
-        custom_attributes: vec!["public".to_string()],
+        custom_attributes: vec!["abi_public".to_string()],
         abi: Abi {
             parameters: vec![AbiParameter {
                 name: "selector".to_string(),

aztec-up/bin/aztec

@@ -249,6 +249,15 @@ EOF
       --entrypoint=/usr/src/barretenberg/cpp/build/bin/bb-avm \
       $DOCKER_REPO:$VERSION aztec_process
 
+    # Strip internal prefixes from all compiled contract JSONs in target directory
+    docker run ${TTY_FLAGS:-} \
+      --user $(id -u):$(id -g) \
+      -v $HOME:$HOME \
+      -e HOME=$HOME \
+      --workdir="$PWD" \
+      --entrypoint=/bin/bash \
+      $DOCKER_REPO:$VERSION -c 'for json in target/*.json; do [ -f "$json" ] && /usr/src/noir-projects/noir-contracts/scripts/strip_aztec_nr_prefix.sh "$json"; done'
+
     echo "Compilation complete!"
     ;;
   test)

cspell.json

@@ -63,10 +63,10 @@
     "checkpointed",
     "checksummed",
     "chiplet",
+    "chonk",
     "cimg",
     "ciphertext",
     "ciphertexts",
-    "chonk",
     "clonedeep",
     "clonedeepwith",
     "cmd",
@@ -342,6 +342,7 @@
     "typecheck",
     "typegen",
     "typeparam",
+    "uncallable",
     "undeployed",
     "undici",
     "unexclude",

docs/examples/bootstrap.sh

@@ -10,6 +10,7 @@ REPO_ROOT=$(git rev-parse --show-toplevel)
 export BB=${BB:-"$REPO_ROOT/barretenberg/cpp/build/bin/bb"}
 export NARGO=${NARGO:-"$REPO_ROOT/noir/noir-repo/target/release/nargo"}
 export TRANSPILER=${TRANSPILER:-"$REPO_ROOT/avm-transpiler/target/release/avm-transpiler"}
+export STRIP_AZTEC_NR_PREFIX=${STRIP_AZTEC_NR_PREFIX:-"$REPO_ROOT/noir-projects/noir-contracts/scripts/strip_aztec_nr_prefix.sh"}
 export BB_HASH=${BB_HASH:-$("$REPO_ROOT/barretenberg/cpp/bootstrap.sh" hash)}
 export NOIR_HASH=${NOIR_HASH:-$("$REPO_ROOT/noir/bootstrap.sh" hash)}
 

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

@@ -1,6 +1,7 @@
 use crate::macros::{
     dispatch::generate_public_dispatch,
     functions::{self_call_registry, stub_registry, utils::check_each_fn_macroified},
+    internals_functions_generation::{create_fn_abi_exports, process_functions},
     notes::NOTES,
     storage::STORAGE_LAYOUT_NAME,
     utils::{get_trait_impl_method, module_has_storage},
@@ -10,11 +11,18 @@ use crate::macros::{
 /// the `sync_private_state` utility function.
 /// Note: This is a module annotation, so the returned quote gets injected inside the module (contract) itself.
 pub comptime fn aztec(m: Module) -> Quoted {
+    // Functions that don't have #[external(...)], #[contract_library_method], or #[test] are not allowed in contracts.
+    check_each_fn_macroified(m);
+
+    // We generate new functions prefixed with `__aztec_nr_internals__` and we replace the original functions' bodies
+    // with `static_assert(false, ...)` to prevent them from being called directly from within the contract.
+    let functions = process_functions(m);
+
     let interface = generate_contract_interface(m);
     let self_call_structs = generate_self_call_structs(m);
 
-    // Functions that don't have #[external(...)], #[contract_library_method], or #[test] are not allowed in contracts.
-    check_each_fn_macroified(m);
+    // We generate ABI exports for all the external functions in the contract.
+    let fn_abi_exports = create_fn_abi_exports(m);
 
     // We generate `_compute_note_hash_and_nullifier`, `sync_private_state` and `process_message`
     // functions only if they are not already implemented. If they are implemented we just insert empty
@@ -26,12 +34,17 @@ pub comptime fn aztec(m: Module) -> Quoted {
     } else {
         quote {}
     };
-    let sync_private_state = if !m.functions().any(|f| f.name() == quote { sync_private_state }) {
+    let sync_private_state_fn_and_abi_export = if !m.functions().any(|f| {
+        f.name() == quote { sync_private_state }
+    }) {
         generate_sync_private_state()
     } else {
         quote {}
     };
-    let process_message = if !m.functions().any(|f| f.name() == quote { process_message }) {
+
+    let process_message_fn_and_abi_export = if !m.functions().any(|f| {
+        f.name() == quote { process_message }
+    }) {
         generate_process_message()
     } else {
         quote {}
@@ -41,10 +54,12 @@ pub comptime fn aztec(m: Module) -> Quoted {
     quote {
         $interface
         $self_call_structs
+        $functions
+        $fn_abi_exports
         $contract_library_method_compute_note_hash_and_nullifier
         $public_dispatch
-        $sync_private_state
-        $process_message
+        $sync_private_state_fn_and_abi_export
+        $process_message_fn_and_abi_export
     }
 }
 
@@ -249,36 +264,45 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -
 }
 
 comptime fn generate_sync_private_state() -> Quoted {
-    // We obtain the `external` function on the next line instead of directly doing
-    // `#[aztec::macros::functions::external("utility")]` in the returned quote because the latter would result in
-    // the function attribute having the full path in the ABI. This is undesirable because we use the information in
-    // the ABI only to determine whether a function is `external("private")`, `external("public")`, or `external("utility")`.
-    let external = crate::macros::functions::external;
-
-    // All we need to do here is trigger message discovery, but this is already done by the #[external("utility")] macro - we don't
-    // need to do anything extra.
     quote {
-        #[$external("utility")]
+        pub struct sync_private_state_parameters {}
+
+        #[abi(functions)]
+        pub struct sync_private_state_abi {
+            parameters: sync_private_state_parameters,
+        }
+
+        #[aztec::macros::internals_functions_generation::abi_attributes::abi_utility]
         unconstrained fn sync_private_state() {
+            let address = aztec::context::utility_context::UtilityContext::new().this_address();
+
+            aztec::messages::discovery::discover_new_messages(address, _compute_note_hash_and_nullifier);
         }
     }
 }
 
 comptime fn generate_process_message() -> Quoted {
-    // We obtain the `external` function on the next line instead of directly doing
-    // `#[aztec::macros::functions::external("utility")]` in the returned quote because the latter would result in
-    // the function attribute having the full path in the ABI. This is undesirable because we use the information in
-    // the ABI only to determine whether a function is `external("private")`, `external("public")`, or `external("utility")`.
-    let external = crate::macros::functions::external;
-
     quote {
-        #[$external("utility")]
+        pub struct process_message_parameters {
+            pub message_ciphertext: BoundedVec<Field, aztec::messages::encoding::MESSAGE_CIPHERTEXT_LEN>,
+            pub message_context: aztec::messages::processing::message_context::MessageContext,
+        }
+
+        #[abi(functions)]
+        pub struct process_message_abi {
+            parameters: process_message_parameters,
+        }
+
+        #[aztec::macros::internals_functions_generation::abi_attributes::abi_utility]
         unconstrained fn process_message(
             message_ciphertext: BoundedVec<Field, aztec::messages::encoding::MESSAGE_CIPHERTEXT_LEN>,
             message_context: aztec::messages::processing::message_context::MessageContext,
         ) {
+            let address = aztec::context::utility_context::UtilityContext::new().this_address();
+
+            aztec::messages::discovery::discover_new_messages(address, _compute_note_hash_and_nullifier);     
             aztec::messages::discovery::process_message::process_message_ciphertext(
-                self.address,
+                address,
                 _compute_note_hash_and_nullifier,
                 message_ciphertext,
                 message_context,

noir-projects/aztec-nr/aztec/src/macros/dispatch.nr

@@ -1,13 +1,12 @@
+use crate::macros::internals_functions_generation::external_functions_registry::get_public_functions;
 use super::utils::compute_fn_selector;
 use poseidon::poseidon2::Poseidon2Hasher;
 use protocol_types::meta::utils::get_params_len_quote;
 use std::{collections::umap::UHashMap, hash::BuildHasherDefault, panic};
 
 /// Returns an `fn public_dispatch(...)` function for the given module that's assumed to be an Aztec contract.
 pub comptime fn generate_public_dispatch(m: Module) -> Quoted {
-    let functions = m.functions();
-    let functions =
-        functions.filter(|function: FunctionDefinition| function.has_named_attribute("public"));
+    let functions = get_public_functions(m);
 
     let unit = get_type::<()>();
 
@@ -68,9 +67,12 @@ pub comptime fn generate_public_dispatch(m: Module) -> Quoted {
             args = args.push_back(quote { $param_name });
         }
 
+        // We call a function whose name is prefixed with `__aztec_nr_internals__`. This is necessary because the
+        // original function is intentionally made uncallable, preventing direct invocation within the contract.
+        // Instead, a new function with the same name, but prefixed by `__aztec_nr_internals__`, has been generated to
+        // be called here. For more details see the `process_functions` function.
+        let name = f"__aztec_nr_internals__{fn_name}".quoted_contents();
         let args = args.join(quote { , });
-        // name of the function is assigned just before the call so debug metadata doesn't span most of this macro when figuring out where the call comes from.
-        let name = function.name();
         let call = quote { $name($args) };
 
         let return_code = if return_type == unit {
@@ -104,10 +106,8 @@ pub comptime fn generate_public_dispatch(m: Module) -> Quoted {
         let dispatch = ifs.join(quote {  });
 
         let body = quote {
-            // We mark this as public because our whole system depends on public
-            // functions having this attribute. However, the public MACRO will
-            // handle the public_dispatch function specially and do nothing.
-            #[external("public")]
+            // We mark this as public because our whole system depends on public functions having this attribute.
+            #[aztec::macros::internals_functions_generation::abi_attributes::abi_public]
             pub unconstrained fn public_dispatch(selector: Field) {
                 $dispatch
             }

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

@@ -1,4 +1,3 @@
-pub(crate) mod abi_export;
 pub(crate) mod call_interface_stubs;
 // TODO: Move initialization_utils out of this crate
 // See https://github.com/AztecProtocol/aztec-packages/pull/15856#discussion_r2229134689 for more details
@@ -9,10 +8,7 @@ pub(crate) mod auth_registry;
 pub(crate) mod utils;
 
 use crate::macros::{
-    functions::{
-        abi_export::create_fn_abi_export,
-        utils::{transform_private, transform_public, transform_utility},
-    },
+    internals_functions_generation::external_functions_registry,
     utils::{is_fn_external, module_has_initializer},
 };
 use super::utils::{fn_has_noinitcheck, is_fn_initializer, is_fn_only_self, is_fn_view};
@@ -134,30 +130,36 @@ pub comptime fn authorize_once(
     AUTHORIZE_ONCE_REGISTRY.insert(f, (from_arg_name, nonce_arg_name));
 }
 
-/// Same as in Solidity external functions are functions that our callable from outside the contract. External
-/// functions can be either private, public, or utility.
-pub comptime fn external(f: FunctionDefinition, f_type: CtString) -> Quoted {
+/// Same as in Solidity external functions are functions that our callable from outside the contract.
+///
+/// There are 3 types of external functions: private, public, and utility.
+/// - private functions are executed client-side and preserve privacy.
+/// - public functions are executed sequencer-side and do not preserve privacy, similar to the EVM.
+/// - utility functions are standalone unconstrained functions that cannot be called from another function in a
+///   contract. They are typically used either to obtain some information from the contract (e.g. token balance of a
+///   user) or to modify internal contract-related state of PXE (e.g. processing logs in Aztec.nr during sync).
+///
+/// In this function we perform basic checks on the function to ensure it is valid and then we add the function to the
+/// external functions registry for later processing by the `#[aztec]` macro.
+pub comptime fn external(f: FunctionDefinition, f_type: CtString) {
     if f_type.eq("private") {
-        private(f)
+        assert_valid_private(f);
+        external_functions_registry::add_private(f);
     } else if f_type.eq("public") {
-        public(f)
+        assert_valid_public(f);
+        external_functions_registry::add_public(f);
     } else if f_type.eq("utility") {
-        utility(f)
+        assert_valid_utility(f);
+        external_functions_registry::add_utility(f);
     } else {
         let function_name = f.name();
         panic(
             f"Function '{function_name}' is marked as #[external(\"{f_type}\")], but '{f_type}' is not a valid external function type. External functions must be one of 'private', 'public' or 'utility'",
         );
-        quote {}
     }
 }
 
-/// Private functions are executed client-side and preserve privacy.
-comptime fn private(f: FunctionDefinition) -> Quoted {
-    // We need to add this attribute to be able to identify if the function is external private when constructing
-    // the contract artifact.
-    f.add_attribute("private");
-
+comptime fn assert_valid_private(f: FunctionDefinition) {
     let visibility = f.visibility();
     if visibility != quote {} {
         let name = f.name();
@@ -172,57 +174,26 @@ comptime fn private(f: FunctionDefinition) -> Quoted {
             f"#[external(\"private\")] functions must not be unconstrained - {name} is",
         );
     }
-
-    // The abi export function is expected to be executed before the function is transformed.
-    let fn_abi_export = create_fn_abi_export(f);
-
-    transform_private(f);
-
-    fn_abi_export
 }
 
-/// Public functions are executed sequencer-side and do not preserve privacy, similar to the EVM.
-comptime fn public(f: FunctionDefinition) -> Quoted {
-    // We need to add this attribute to be able to identify if the function is external public when constructing
-    // the contract artifact.
-    f.add_attribute("public");
-
-    // We don't want to transform the public_dispatch function.
-    if f.name() == quote { public_dispatch } {
-        quote {}
-    } else {
-        let visibility = f.visibility();
-        if visibility != quote {} {
-            let name = f.name();
-            panic(
-                f"A function marked as #[external(\"public\")] must not have public Noir visibility - {name}'s visibility is '{visibility}'",
-            );
-        }
-
-        if f.is_unconstrained() {
-            let name = f.name();
-            panic(
-                f"#[external(\"public\")] functions must not be unconstrained - {name} is",
-            );
-        }
-
-        // The abi export function is expected to be executed before the function is transformed.
-        let fn_abi_export = create_fn_abi_export(f);
-
-        transform_public(f);
+comptime fn assert_valid_public(f: FunctionDefinition) {
+    let visibility = f.visibility();
+    if visibility != quote {} {
+        let name = f.name();
+        panic(
+            f"A function marked as #[external(\"public\")] must not have public Noir visibility - {name}'s visibility is '{visibility}'",
+        );
+    }
 
-        fn_abi_export
+    if f.is_unconstrained() {
+        let name = f.name();
+        panic(
+            f"#[external(\"public\")] functions must not be unconstrained - {name} is",
+        );
     }
 }
 
-/// Utility functions are standalone unconstrained functions that cannot be called from another function in a contract.
-/// They are typically used either to obtain some information from the contract (e.g. token balance of a user) or to
-/// modify internal contract-related state of PXE (e.g. processing logs in Aztec.nr during sync).
-comptime fn utility(f: FunctionDefinition) -> Quoted {
-    // We need to add this attribute to be able to identify if the function is external utility when constructing
-    // the contract artifact.
-    f.add_attribute("utility");
-
+comptime fn assert_valid_utility(f: FunctionDefinition) {
     let visibility = f.visibility();
     if visibility != quote {} {
         let name = f.name();
@@ -238,22 +209,11 @@ comptime fn utility(f: FunctionDefinition) -> Quoted {
         );
     }
 
-    post_external_utility_checks(f);
-
-    // The abi export function is expected to be executed before the function is transformed.
-    let fn_abi_export = create_fn_abi_export(f);
-
-    transform_utility(f);
-
-    fn_abi_export
-}
-
-/// Utility functions cannot be used with the following modifiers: #[authorize_once], #[only_self], #[view],
-/// #[initializer], and #[noinitcheck]. Since we cannot enforce a specific ordering between these modifiers and
-/// #[external(...)], and we cannot access the #[external(...)] argument from within these modifiers' implementations
-/// (as accessing EXTERNAL_REGISTRY would require enforcing ordering), we perform these compatibility checks here in
-/// the utility macro.
-comptime fn post_external_utility_checks(f: FunctionDefinition) {
+    // Utility functions cannot be used with the following modifiers: #[authorize_once], #[only_self], #[view],
+    // #[initializer], and #[noinitcheck]. Since we cannot enforce a specific ordering between these modifiers and
+    // #[external(...)], and we cannot access the #[external(...)] argument from within these modifiers' implementations
+    // (as accessing EXTERNAL_REGISTRY would require enforcing ordering), we perform these compatibility checks here in
+    // the utility macro.
     if AUTHORIZE_ONCE_REGISTRY.get(f).is_some() {
         let name = f.name();
         panic(