chore: add some macro checks

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

@@ -7,8 +7,14 @@ pub(crate) mod stub_registry;
 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},
+    },
+    utils::{is_fn_private, is_fn_public, is_fn_utility, module_has_initializer},
+};
 use auth_registry::AUTHORIZE_ONCE_REGISTRY;
-use utils::{transform_private, transform_public, transform_utility};
 
 // Functions can have multiple attributes applied to them, e.g. a single function can have #[public], #[view] and
 // #[internal]. However. the order in which this will be evaluated is unknown, which makes combining them tricky.
@@ -26,23 +32,57 @@ use utils::{transform_private, transform_public, transform_utility};
 ///  - it can only be called once
 ///  - if there are multiple initializer functions, only one of them can be called
 ///  - no non-initializer functions can be called until an initializer has ben called (except `noinitcheck` functions)
-pub comptime fn initializer(_f: FunctionDefinition) {
-    // Marker attribute
+pub comptime fn initializer(f: FunctionDefinition) {
+    // Marker attribute - see the comment above
+
+    if !is_fn_private(f) & !is_fn_public(f) {
+        let name = f.name();
+        panic(
+            f"The #[initializer] attribute can only be applied to #[private] or #[public] functions - {name} is neither",
+        );
+    }
 }
 
 /// Functions with noinitcheck can be called before contract initialization.
-pub comptime fn noinitcheck(_f: FunctionDefinition) {
-    // Marker attribute
+pub comptime fn noinitcheck(f: FunctionDefinition) {
+    // Marker attribute - see the comment above
+
+    if !is_fn_private(f) & !is_fn_public(f) {
+        let name = f.name();
+        panic(
+            f"The #[noinitcheck] attribute can only be applied to #[private] or #[public] functions - {name} is neither",
+        );
+    }
+
+    if !module_has_initializer(f.module()) {
+        panic(
+            f"The #[noinitcheck] attribute is unnecessary for contracts with no #[initializer] functions",
+        );
+    }
 }
 
 /// Internal functions can only be called by the contract itself, typically from private into public.
-pub comptime fn internal(_f: FunctionDefinition) {
-    // Marker attribute
+pub comptime fn internal(f: FunctionDefinition) {
+    // Marker attribute - see the comment above
+
+    if !is_fn_private(f) & !is_fn_public(f) {
+        let name = f.name();
+        panic(
+            f"The #[internal] attribute can only be applied to #[private] or #[public] functions - {name} is neither",
+        );
+    }
 }
 
 /// View functions can only be called in a static execution context.
-pub comptime fn view(_f: FunctionDefinition) {
-    // Marker attribute
+pub comptime fn view(f: FunctionDefinition) {
+    // Marker attribute - see the comment above
+
+    if !is_fn_private(f) & !is_fn_public(f) {
+        let name = f.name();
+        panic(
+            f"The #[view] attribute can only be applied to #[private] or #[public] functions - {name} is neither",
+        );
+    }
 }
 
 /// Private and public functions can require an authorization check to be performed before execution. This
@@ -58,12 +98,38 @@ pub comptime fn authorize_once(
     from_arg_name: CtString,
     nonce_arg_name: CtString,
 ) {
+    if !is_fn_private(f) & !is_fn_public(f) {
+        let name = f.name();
+        panic(
+            f"The #[authorize_once] attribute can only be applied to #[private] or #[public] functions - {name} is neither",
+        );
+    }
+
     AUTHORIZE_ONCE_REGISTRY.insert(f, (from_arg_name, nonce_arg_name));
 }
 
 /// Private functions are executed client-side and preserve privacy.
 pub comptime fn private(f: FunctionDefinition) -> Quoted {
-    transform_private(f)
+    if is_fn_public(f) | is_fn_utility(f) {
+        let name = f.name();
+        panic(
+            f"A function marked as #[private] cannot also be #[public] or #[utility] - {name} is more than one of these",
+        );
+    }
+
+    if f.is_unconstrained() {
+        let name = f.name();
+        panic(
+            f"#[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.
@@ -72,13 +138,47 @@ pub comptime fn public(f: FunctionDefinition) -> Quoted {
     if f.name() == quote { public_dispatch } {
         quote {}
     } else {
-        transform_public(f)
+        if is_fn_private(f) | is_fn_utility(f) {
+            let name = f.name();
+            panic(
+                f"A function marked as #[public] cannot also be #[private] or #[utility] - {name} is more than one of these",
+            );
+        }
+
+        if f.is_unconstrained() {
+            let name = f.name();
+            panic(f"#[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);
+
+        fn_abi_export
     }
 }
 
 /// 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).
 pub comptime fn utility(f: FunctionDefinition) -> Quoted {
-    transform_utility(f)
+    if is_fn_private(f) | is_fn_public(f) {
+        let name = f.name();
+        panic(
+            f"A function marked as #[utility] cannot also be #[private] or #[public] - {name} is more than one of these",
+        );
+    }
+
+    if !f.is_unconstrained() {
+        let name = f.name();
+        panic(f"#[utility] must be unconstrained - {name} isn't");
+    }
+
+    // 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
 }

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

@@ -1,7 +1,6 @@
 use crate::macros::{
     functions::{
-        abi_export::create_fn_abi_export, auth_registry::AUTHORIZE_ONCE_REGISTRY,
-        call_interface_stubs::stub_fn, stub_registry,
+        auth_registry::AUTHORIZE_ONCE_REGISTRY, call_interface_stubs::stub_fn, stub_registry,
     },
     notes::NOTES,
     utils::{
@@ -13,19 +12,10 @@ use crate::macros::{
 };
 use std::meta::{ctstring::AsCtString, type_of};
 
-pub(crate) comptime fn transform_private(f: FunctionDefinition) -> Quoted {
-    let fn_abi = create_fn_abi_export(f);
+pub(crate) comptime fn transform_private(f: FunctionDefinition) {
     let fn_stub = stub_fn(f);
     stub_registry::register(f.module(), fn_stub);
 
-    // If a function is further modified as unconstrained, we throw an error
-    if f.is_unconstrained() {
-        let name = f.name();
-        panic(
-            f"Function {name} is annotated with #[private] but marked as unconstrained, remove unconstrained keyword",
-        );
-    }
-
     let module_has_initializer = module_has_initializer(f.module());
     let module_has_storage = module_has_storage(f.module());
 
@@ -185,23 +175,12 @@ pub(crate) comptime fn transform_private(f: FunctionDefinition) -> Quoted {
             .as_type(),
     );
     f.set_return_data();
-
-    fn_abi
 }
 
-pub(crate) comptime fn transform_public(f: FunctionDefinition) -> Quoted {
-    let fn_abi = create_fn_abi_export(f);
+pub(crate) comptime fn transform_public(f: FunctionDefinition) {
     let fn_stub = stub_fn(f);
     stub_registry::register(f.module(), fn_stub);
 
-    // If a function is further modified as unconstrained, we throw an error
-    if f.is_unconstrained() {
-        let name = f.name();
-        panic(
-            f"Function {name} is annotated with #[public] but marked as unconstrained, remove unconstrained keyword",
-        );
-    }
-
     let module_has_initializer = module_has_initializer(f.module());
     let module_has_storage = module_has_storage(f.module());
 
@@ -298,23 +277,12 @@ pub(crate) comptime fn transform_public(f: FunctionDefinition) -> Quoted {
     // bytecode.
     f.set_unconstrained(true);
     f.set_return_public(true);
-
-    fn_abi
 }
 
-pub(crate) comptime fn transform_utility(f: FunctionDefinition) -> Quoted {
-    let fn_abi = create_fn_abi_export(f);
+pub(crate) comptime fn transform_utility(f: FunctionDefinition) {
     let fn_stub = stub_fn(f);
     stub_registry::register(f.module(), fn_stub);
 
-    // Check if function is marked as unconstrained
-    if !f.is_unconstrained() {
-        let name = f.name();
-        panic(
-            f"Function {name} is annotated with #[utility] but not marked as unconstrained, add unconstrained keyword",
-        );
-    }
-
     // Create utility context
     let context_creation =
         quote { let mut context = dep::aztec::context::utility_context::UtilityContext::new(); };
@@ -349,8 +317,6 @@ pub(crate) comptime fn transform_utility(f: FunctionDefinition) -> Quoted {
     f.set_body(modified_body);
 
     f.set_return_public(true);
-
-    fn_abi
 }
 
 comptime fn create_internal_check(f: FunctionDefinition) -> Quoted {
@@ -416,15 +382,12 @@ pub(crate) comptime fn create_message_discovery_call() -> Quoted {
 /// This check is injected by the `#[authorize_once("from_arg_name", "nonce_arg_name")]`, which allows the user to define
 /// which parameters to use.
 pub(crate) comptime fn create_authorize_once_check(f: FunctionDefinition) -> Quoted {
-    if !is_fn_private(f) & !is_fn_public(f) {
-        panic(
-            f"Functions marked with #[authorize_once] must either be #[private] or #[public]",
-        );
-    }
     let maybe_authorize_once_args = AUTHORIZE_ONCE_REGISTRY.get(f);
     let authorize_once_args = if maybe_authorize_once_args.is_some() {
         maybe_authorize_once_args.unwrap()
     } else {
+        // We need to for authorize_once to have already executed so that we can retrieve its params - this depends on
+        // the order in which the attributes are applied.
         panic(
             f"Functions marked with #[authorize_once] must have the #[private] or #[public] attribute placed last",
         )

noir-projects/noir-contracts/contracts/app/simple_token_contract/src/main.nr

@@ -89,7 +89,7 @@ pub contract SimpleToken {
     }
 
     #[utility]
-    pub(crate) unconstrained fn private_balance_of(owner: AztecAddress) -> u128 {
+    unconstrained fn private_balance_of(owner: AztecAddress) -> u128 {
         storage.balances.at(owner).balance_of()
     }
 

noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr

@@ -653,7 +653,7 @@ pub contract Token {
 
     // docs:start:balance_of_private
     #[utility]
-    pub(crate) unconstrained fn balance_of_private(owner: AztecAddress) -> u128 {
+    unconstrained fn balance_of_private(owner: AztecAddress) -> u128 {
         storage.balances.at(owner).balance_of()
     }
     // docs:end:balance_of_private