chore: Improve compute_note_hash_and_nullifier autogeneration and `…

…NoteProcessor` warnings (#5838)

Closes #5669,
#5670,
#4649

Correctly determines the signature of the autogenerated
`compute_note_hash_and_nullifier` by looking up the serialized content
size of notes used in the contract. Furthermore, that langth is compared
against MAX_NOTE_FIELDS_LEN and a compile time error is emited if the
user attempts to use a note that is bigger than currently supported.
Finally changed the `NoteProcessor` warns to be errors (even thought
this particular one shouldn't get that far anymore!)

l1-contracts/src/core/libraries/ConstantsGen.sol

@@ -89,7 +89,7 @@ library Constants {
   uint256 internal constant DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE =
     0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631;
   uint256 internal constant DEPLOYER_CONTRACT_ADDRESS =
-    0x1f47133752dfcd9604f2d89c631797a84ed207c1c51d08533226dafcc8bd8548;
+    0x0cde95a10e1160d0ff69ac8212cb5902fa5add38d8596595631fcf3a667798e0;
   uint256 internal constant DEFAULT_GAS_LIMIT = 1_000_000_000;
   uint256 internal constant DEFAULT_TEARDOWN_GAS_LIMIT = 100_000_000;
   uint256 internal constant DEFAULT_MAX_FEE_PER_GAS = 10;

noir-projects/noir-protocol-circuits/crates/types/src/constants.nr

@@ -126,7 +126,7 @@ global REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af8166354
 // CONTRACT INSTANCE CONSTANTS
 // sha224sum 'struct ContractInstanceDeployed'
 global DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631;
-global DEPLOYER_CONTRACT_ADDRESS = 0x1f47133752dfcd9604f2d89c631797a84ed207c1c51d08533226dafcc8bd8548;
+global DEPLOYER_CONTRACT_ADDRESS = 0x0cde95a10e1160d0ff69ac8212cb5902fa5add38d8596595631fcf3a667798e0;
 
 // GAS DEFAULTS
 global DEFAULT_GAS_LIMIT: u32 = 1_000_000_000;

noir/noir-repo/aztec_macros/src/lib.rs

@@ -93,17 +93,18 @@ fn transform_module(
     // Check for a user defined storage struct
 
     let maybe_storage_struct_name = check_for_storage_definition(module)?;
+
     let storage_defined = maybe_storage_struct_name.is_some();
 
-    if let Some(storage_struct_name) = maybe_storage_struct_name {
-        if !check_for_storage_implementation(module, &storage_struct_name) {
-            generate_storage_implementation(module, &storage_struct_name)?;
+    if let Some(ref storage_struct_name) = maybe_storage_struct_name {
+        if !check_for_storage_implementation(module, storage_struct_name) {
+            generate_storage_implementation(module, storage_struct_name)?;
         }
         // Make sure we're only generating the storage layout for the root crate
         // In case we got a contract importing other contracts for their interface, we
         // don't want to generate the storage layout for them
         if crate_id == context.root_crate_id() {
-            generate_storage_layout(module, storage_struct_name)?;
+            generate_storage_layout(module, storage_struct_name.clone())?;
         }
     }
 
@@ -164,14 +165,14 @@ fn transform_module(
             transform_function(
                 fn_type,
                 func,
-                storage_defined,
+                maybe_storage_struct_name.clone(),
                 is_initializer,
                 insert_init_check,
                 is_internal,
             )?;
             has_transformed_module = true;
         } else if storage_defined && func.def.is_unconstrained {
-            transform_unconstrained(func);
+            transform_unconstrained(func, maybe_storage_struct_name.clone().unwrap());
             has_transformed_module = true;
         }
     }

noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs

@@ -7,7 +7,10 @@ use noirc_frontend::{
 
 use crate::utils::{
     errors::AztecMacroError,
-    hir_utils::{collect_crate_functions, fetch_notes, get_contract_module_data, inject_fn},
+    hir_utils::{
+        collect_crate_functions, collect_traits, fetch_notes, get_contract_module_data,
+        get_global_numberic_const, get_serialized_length, inject_fn,
+    },
 };
 
 // Check if "compute_note_hash_and_nullifier(AztecAddress,Field,Field,Field,[Field; N]) -> [Field; 4]" is defined
@@ -59,13 +62,68 @@ pub fn inject_compute_note_hash_and_nullifier(
             return Ok(());
         }
 
+        let traits: Vec<_> = collect_traits(context);
+
+        // Get MAX_NOTE_FIELDS_LENGTH global to check if the notes in our contract are too long.
+        let max_note_length_const = get_global_numberic_const(context, "MAX_NOTE_FIELDS_LENGTH")
+            .map_err(|err| {
+                (
+                    AztecMacroError::CouldNotImplementComputeNoteHashAndNullifier {
+                        secondary_message: Some(err.primary_message),
+                    },
+                    file_id,
+                )
+            })?;
+
         // In order to implement compute_note_hash_and_nullifier, we need to know all of the different note types the
-        // contract might use. These are the types that are marked as #[aztec(note)].
+        // contract might use and their serialized lengths. These are the types that are marked as #[aztec(note)].
+        let mut notes_and_lengths = vec![];
+
+        for (path, typ) in fetch_notes(context) {
+            let serialized_len: u128 = get_serialized_length(
+                &traits,
+                "NoteInterface",
+                &Type::Struct(typ.clone(), vec![]),
+                &context.def_interner,
+            )
+            .map_err(|_err| {
+                (
+                    AztecMacroError::CouldNotImplementComputeNoteHashAndNullifier {
+                        secondary_message: Some(format!(
+                            "Failed to get serialized length for note type {}",
+                            path
+                        )),
+                    },
+                    file_id,
+                )
+            })?
+            .into();
+
+            if serialized_len > max_note_length_const {
+                return Err((
+                   AztecMacroError::CouldNotImplementComputeNoteHashAndNullifier {
+                        secondary_message: Some(format!(
+                            "Note type {} as {} fields, which is more than the maximum allowed length of {}.",
+                            path,
+                            serialized_len,
+                            max_note_length_const
+                        )),
+                    },
+                    file_id,
+                ));
+            }
+
+            notes_and_lengths.push((path.to_string(), serialized_len));
+        }
+
+        let max_note_length: u128 =
+            *notes_and_lengths.iter().map(|(_, serialized_len)| serialized_len).max().unwrap_or(&0);
+
         let note_types =
-            fetch_notes(context).iter().map(|(path, _)| path.to_string()).collect::<Vec<_>>();
+            notes_and_lengths.iter().map(|(note_type, _)| note_type.clone()).collect::<Vec<_>>();
 
         // We can now generate a version of compute_note_hash_and_nullifier tailored for the contract in this crate.
-        let func = generate_compute_note_hash_and_nullifier(&note_types);
+        let func = generate_compute_note_hash_and_nullifier(&note_types, max_note_length);
 
         // And inject the newly created function into the contract.
 
@@ -85,8 +143,12 @@ pub fn inject_compute_note_hash_and_nullifier(
     Ok(())
 }
 
-fn generate_compute_note_hash_and_nullifier(note_types: &[String]) -> NoirFunction {
-    let function_source = generate_compute_note_hash_and_nullifier_source(note_types);
+fn generate_compute_note_hash_and_nullifier(
+    note_types: &[String],
+    max_note_length: u128,
+) -> NoirFunction {
+    let function_source =
+        generate_compute_note_hash_and_nullifier_source(note_types, max_note_length);
 
     let (function_ast, errors) = parse_program(&function_source);
     if !errors.is_empty() {
@@ -98,25 +160,30 @@ fn generate_compute_note_hash_and_nullifier(note_types: &[String]) -> NoirFuncti
     function_ast.functions.remove(0)
 }
 
-fn generate_compute_note_hash_and_nullifier_source(note_types: &[String]) -> String {
+fn generate_compute_note_hash_and_nullifier_source(
+    note_types: &[String],
+    max_note_length: u128,
+) -> String {
     // TODO(#4649): The serialized_note parameter is a fixed-size array, but we don't know what length it should have.
     // For now we hardcode it to 20, which is the same as MAX_NOTE_FIELDS_LENGTH.
 
     if note_types.is_empty() {
         // Even if the contract does not include any notes, other parts of the stack expect for this function to exist,
         // so we include a dummy version.
-        "
+        format!(
+            "
         unconstrained fn compute_note_hash_and_nullifier(
             contract_address: dep::aztec::protocol_types::address::AztecAddress,
             nonce: Field,
             storage_slot: Field,
             note_type_id: Field,
-            serialized_note: [Field; 20]
-        ) -> pub [Field; 4] {
+            serialized_note: [Field; {}]
+        ) -> pub [Field; 4] {{
             assert(false, \"This contract does not use private notes\");
             [0, 0, 0, 0]
-        }"
-        .to_string()
+        }}",
+            max_note_length
+        )
     } else {
         // For contracts that include notes we do a simple if-else chain comparing note_type_id with the different
         // get_note_type_id of each of the note types.
@@ -141,12 +208,13 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &[String]) -> Str
                 nonce: Field,
                 storage_slot: Field,
                 note_type_id: Field,
-                serialized_note: [Field; 20]
+                serialized_note: [Field; {}]
             ) -> pub [Field; 4] {{
                 let note_header = dep::aztec::prelude::NoteHeader::new(contract_address, nonce, storage_slot);
 
                 {}
             }}",
+            max_note_length,
             full_if_statement
         )
     }

noir/noir-repo/aztec_macros/src/transforms/functions.rs

@@ -26,7 +26,7 @@ use crate::{
 pub fn transform_function(
     ty: &str,
     func: &mut NoirFunction,
-    storage_defined: bool,
+    storage_struct_name: Option<String>,
     is_initializer: bool,
     insert_init_check: bool,
     is_internal: bool,
@@ -54,8 +54,8 @@ pub fn transform_function(
     }
 
     // Add access to the storage struct
-    if storage_defined {
-        let storage_def = abstract_storage(&ty.to_lowercase(), false);
+    if let Some(storage_struct_name) = storage_struct_name {
+        let storage_def = abstract_storage(storage_struct_name, &ty.to_lowercase(), false);
         func.def.body.statements.insert(0, storage_def);
     }
 
@@ -206,8 +206,11 @@ pub fn export_fn_abi(
 /// ```
 ///
 /// This will allow developers to access their contract' storage struct in unconstrained functions
-pub fn transform_unconstrained(func: &mut NoirFunction) {
-    func.def.body.statements.insert(0, abstract_storage("Unconstrained", true));
+pub fn transform_unconstrained(func: &mut NoirFunction, storage_struct_name: String) {
+    func.def
+        .body
+        .statements
+        .insert(0, abstract_storage(storage_struct_name, "Unconstrained", true));
 }
 
 /// Helper function that returns what the private context would look like in the ast
@@ -572,7 +575,7 @@ fn abstract_return_values(func: &NoirFunction) -> Result<Option<Vec<Statement>>,
 /// unconstrained fn lol() {
 ///   let storage = Storage::init(Context::none());
 /// }
-fn abstract_storage(typ: &str, unconstrained: bool) -> Statement {
+fn abstract_storage(storage_struct_name: String, typ: &str, unconstrained: bool) -> Statement {
     let init_context_call = if unconstrained {
         call(
             variable_path(chained_dep!("aztec", "context", "Context", "none")), // Path
@@ -588,8 +591,8 @@ fn abstract_storage(typ: &str, unconstrained: bool) -> Statement {
     assignment(
         "storage", // Assigned to
         call(
-            variable_path(chained_path!("Storage", "init")), // Path
-            vec![init_context_call],                         // args
+            variable_path(chained_path!(storage_struct_name.as_str(), "init")), // Path
+            vec![init_context_call],                                            // args
         ),
     )
 }

noir/noir-repo/aztec_macros/src/transforms/storage.rs

@@ -1,12 +1,10 @@
-use std::borrow::Borrow;
-
 use noirc_errors::Span;
 use noirc_frontend::{
     graph::CrateId,
     macros_api::{
         FieldElement, FileId, HirContext, HirExpression, HirLiteral, HirStatement, NodeInterner,
     },
-    node_interner::{TraitId, TraitImplKind},
+    node_interner::TraitId,
     parse_program,
     parser::SortedModule,
     token::SecondaryAttribute,
@@ -23,7 +21,9 @@ use crate::{
             make_type, pattern, return_type, variable, variable_path,
         },
         errors::AztecMacroError,
-        hir_utils::{collect_crate_structs, collect_traits, get_contract_module_data},
+        hir_utils::{
+            collect_crate_structs, collect_traits, get_contract_module_data, get_serialized_length,
+        },
     },
 };
 
@@ -196,7 +196,7 @@ pub fn generate_storage_implementation(
 }
 
 /// Obtains the serialized length of a type that implements the Serialize trait.
-fn get_serialized_length(
+pub fn get_storage_serialized_length(
     traits: &[TraitId],
     typ: &Type,
     interner: &NodeInterner,
@@ -214,48 +214,22 @@ fn get_serialized_length(
             secondary_message: Some("State storage variable must be generic".to_string()),
         })?;
 
-    let is_note = traits.iter().any(|&trait_id| {
-        let r#trait = interner.get_trait(trait_id);
-        r#trait.name.0.contents == "NoteInterface"
-            && !interner.lookup_all_trait_implementations(stored_in_state, trait_id).is_empty()
-    });
+    let is_note = match stored_in_state {
+        Type::Struct(typ, _) => interner
+            .struct_attributes(&typ.borrow().id)
+            .iter()
+            .any(|attr| is_custom_attribute(attr, "aztec(note)")),
+        _ => false,
+    };
 
     // Maps and (private) Notes always occupy a single slot. Someone could store a Note in PublicMutable for whatever reason though.
     if struct_name == "Map" || (is_note && struct_name != "PublicMutable") {
         return Ok(1);
     }
 
-    let serialized_trait_impl_kind = traits
-        .iter()
-        .find_map(|&trait_id| {
-            let r#trait = interner.get_trait(trait_id);
-            if r#trait.borrow().name.0.contents == "Serialize"
-                && r#trait.borrow().generics.len() == 1
-            {
-                interner
-                    .lookup_all_trait_implementations(stored_in_state, trait_id)
-                    .into_iter()
-                    .next()
-            } else {
-                None
-            }
-        })
-        .ok_or(AztecMacroError::CouldNotAssignStorageSlots {
-            secondary_message: Some("Stored data must implement Serialize trait".to_string()),
-        })?;
-
-    let serialized_trait_impl_id = match serialized_trait_impl_kind {
-        TraitImplKind::Normal(trait_impl_id) => Ok(trait_impl_id),
-        _ => Err(AztecMacroError::CouldNotAssignStorageSlots { secondary_message: None }),
-    }?;
-
-    let serialized_trait_impl_shared = interner.get_trait_implementation(*serialized_trait_impl_id);
-    let serialized_trait_impl = serialized_trait_impl_shared.borrow();
-
-    match serialized_trait_impl.trait_generics.first().unwrap() {
-        Type::Constant(value) => Ok(*value),
-        _ => Err(AztecMacroError::CouldNotAssignStorageSlots { secondary_message: None }),
-    }
+    get_serialized_length(traits, "Serialize", stored_in_state, interner).map_err(|err| {
+        AztecMacroError::CouldNotAssignStorageSlots { secondary_message: Some(err.primary_message) }
+    })
 }
 
 /// Assigns storage slots to the storage struct fields based on the serialized length of the types. This automatic assignment
@@ -436,7 +410,7 @@ pub fn assign_storage_slots(
                 };
 
                 let type_serialized_len =
-                    get_serialized_length(&traits, field_type, &context.def_interner)
+                    get_storage_serialized_length(&traits, field_type, &context.def_interner)
                         .map_err(|err| (err, file_id))?;
 
                 context.def_interner.update_expression(new_call_expression.arguments[1], |expr| {
@@ -504,7 +478,7 @@ pub fn generate_storage_layout(
     let (struct_ast, errors) = parse_program(&storage_fields_source);
     if !errors.is_empty() {
         dbg!(errors);
-        return Err(AztecMacroError::CouldNotImplementNoteInterface {
+        return Err(AztecMacroError::CouldNotExportStorageLayout {
             secondary_message: Some("Failed to parse Noir macro code (struct StorageLayout). This is either a bug in the compiler or the Noir macro code".to_string()),
             span: None
         });