feat: panicking on direct calls to external funcs by benesjan · Pull Request #18126 · AztecProtocol/aztec-packages

benesjan · 2025-10-31T20:53:19Z

Fixes F-27

Given #[private] fn foo() we want for foo() to issue a compilation error. To achieve that we need to replace the function's body with a static_assert(false).

Given that the #[external(...)] macro is being processed before the #[aztec] macro we can do the following:

In the #[external(...)] macro invocation I register the function into 3 global variables based on the external type (private, public or utility),
then in the #[aztec] macros for each of these functions I generate a new function with __aztecnr_internals__ prefix by taking the original function's args and body, doing the same transformation on it as we do now for external funcs and then I inject these new functions into the contract,
then I replace the body of the original function's static_assert(false, "yo, go check the docs how to actually call a function");
for the functions that are currently injected by the #[aztec] macro (e.g. sync_private_state) I would not apply the #[external(...)] macro attribute as is currently done but instead I would just directly call the "transformation function" on it that is also used in step 3 and then inject the function.

Note that this PR is currently not mergeable as I needed to disable caching in BB to get a fresh AVM transpiler + there is a broken test. Both of these are (most likely) unrelated to this PR so I think this can be already reviewed.

benesjan · 2025-10-31T20:53:37Z

benesjan · 2025-11-05T16:34:49Z

avm-transpiler/src/transpile_contract.rs


        for function in contract.functions {
-            if function.custom_attributes.contains(&"public".to_string()) {
+            if function.custom_attributes.contains(&"abi_public".to_string()) {


Renamed the attribute from public to abi_public because now the dev never sees it and abi_public is used only to inform the artifact generation regarding what type of function it is.

abi_public is a much better name as it's way more searchable and it makes it clearer that now it's just a low-level thing.

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

benesjan · 2025-11-05T17:31:27Z

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

+
+pub(crate) comptime fn transform_private(f: FunctionDefinition) -> Quoted {
+    // TODO(benesjan): This function should only generate the new function's quote. Move the following to a different
+    // place.


In a followup PR I would rename these functions and make them solely new function quote-generation focused.

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

benesjan · 2025-11-05T17:40:00Z

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

 ///
-/// # Important
-/// This function needs to be run before transformations modifying the signature of the target function `f` are applied
-/// (e.g. transform_private).


No longer relevant as the original is now not getting transformed.

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

benesjan · 2025-11-05T18:08:07Z

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

@@ -249,36 +264,45 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -
 }

 comptime fn generate_sync_private_state() -> Quoted {


Here is quite a big change as now I could not apply the external utility macro to the functions because the transformation functionality would not get triggered as that is handled in the aztec macro which is currently being processed here.

So I needed to do the transformation manually. The ugliest here is the manual creation of the funciton abi exports but I would say it's ok.

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

yarn-project/simulator/src/public/public_tx_simulator/apps_tests/bench.test.ts

barretenberg/cpp/bootstrap.sh

nventuro

Thanks for swimming through the macro mud!

nventuro · 2025-11-07T22:45:35Z

docs/examples/bootstrap.sh

 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_INTERNAL_PREFIX=${STRIP_INTERNAL_PREFIX:-"$REPO_ROOT/noir-projects/noir-contracts/scripts/strip_internal_prefix.sh"}


'internal' is maybe a complicated term given that #[internal] is a thing no?

You have a point. Renamed it to strip_aztec_nr_prefix.sh.

nventuro · 2025-11-07T22:48:39Z

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

+    // #[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


We could check this in the #[aztec] macro no? Go through all utility fns and check they're correct etc. Essentially first collect all atrtributes, put them in the registry, and then do all the work in one pass.

You are right that now we could. Now it's possible because we have the external macro add the function definitions to the corresponding to the corresponding external function registry by type. So we can move these checks there.

That will be cleaner.

Will take a note to do it.

nventuro · 2025-11-07T22:50:43Z

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

+    // TODO(benesjan): This function should only generate the new function's quote. Move the following to a different
+    // place.


The following what?

Meant the following line where is the call to register_public_fn_stubs. I've already did the change in a PR up the stack so will not clarify this here to avoid unnecessary conflicts.

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

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

nventuro · 2025-11-07T23:59:20Z

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

+//! The functionality in this module is triggered by the `#[aztec]` macro. It generates new functions, prefixed with
+//! `__aztec_nr_internals___`, from the ones marked with `#[external(...)]` attributes. The original functions are then
+//! modified to be uncallable. This prevents developers from inadvertently calling a function directly, instead of
+//! performing a proper contract call.


When did you start doing these module comments?

When I learnt from you that they exist. Do you like it?

nventuro · 2025-11-08T00:07:46Z

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

+        // 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();


Is this right? Don't we want to keep the original name?

At this point we've made the original external functions uncallable by injecting the static assert and hence here in the public dispatch we need to use the modified naming that invokes the modified generated functions.

.../noir-contracts-comp-failures/contracts/panic_on_direct_private_external_fn_call/src/main.nr

AztecBot · 2025-11-08T23:46:32Z

Flakey Tests

🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033[38;2;188;109;208mFLAKED\033[0m (\033[38;2;250;217;121m8;;http://ci.aztec-labs.com/75fa720da8e80ab0�75fa720da8e80ab08;;�\033[0m): BOX=react BROWSER=firefox run_compose_test react-firefox box boxes (76s) (code: 1) (\033[38;2;188;109;208mJan Beneš\033[0m: feat: panicking on direct calls to external funcs (#18126))

benesjan force-pushed the 10-31-feat_panicking_on_direct_calls_to_external_funcs branch 3 times, most recently from b4943a1 to b4a5454 Compare November 4, 2025 14:55

benesjan changed the base branch from next to graphite-base/18126 November 4, 2025 20:05

benesjan force-pushed the 10-31-feat_panicking_on_direct_calls_to_external_funcs branch from 728ea91 to 9c49f01 Compare November 4, 2025 20:05

benesjan changed the base branch from graphite-base/18126 to 10-17-refactor_moving_atec-nargo_init_to_aztec November 4, 2025 20:05

benesjan mentioned this pull request Nov 4, 2025

refactor!: new aztec command #17792

Merged

benesjan marked this pull request as ready for review November 4, 2025 23:15

benesjan requested review from a team, charlielye, dbanks12, fcarreiro and sirasistant as code owners November 4, 2025 23:15

benesjan marked this pull request as draft November 5, 2025 00:33

benesjan force-pushed the 10-17-refactor_moving_atec-nargo_init_to_aztec branch from b0bd5bd to 7ff9178 Compare November 5, 2025 16:10

benesjan force-pushed the 10-31-feat_panicking_on_direct_calls_to_external_funcs branch from 201dd56 to 4b8cc14 Compare November 5, 2025 16:10