From 3f79c30c60a21264badb3d36108ed42c965d6150 Mon Sep 17 00:00:00 2001 From: thunkar Date: Thu, 18 Apr 2024 10:54:22 +0000 Subject: [PATCH 1/9] initial approach --- .../compute_note_hash_and_nullifier.rs | 58 ++++++++++++++----- .../aztec_macros/src/transforms/storage.rs | 46 +++------------ .../aztec_macros/src/utils/hir_utils.rs | 48 ++++++++++++++- 3 files changed, 100 insertions(+), 52 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs index 4ff97a5dcae..e01febecb40 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs +++ b/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_serialized_length, inject_fn, + }, }; // Check if "compute_note_hash_and_nullifier(AztecAddress,Field,Field,Field,[Field; N]) -> [Field; 4]" is defined @@ -59,13 +62,32 @@ pub fn inject_compute_note_hash_and_nullifier( return Ok(()); } + let traits: Vec<_> = collect_traits(context); + // 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)]. - let note_types = - fetch_notes(context).iter().map(|(path, _)| path.to_string()).collect::>(); + let notes = fetch_notes(context) + .iter() + .map(|(path, typ)| { + let serialized_len = get_serialized_length( + &traits, + "NoteInterface", + &Type::Struct(typ.clone(), vec![]), + &context.def_interner, + ) + .expect(&format!( + "Failed to get serialized length for note type {}", + path.to_string() + )); + (path.to_string(), serialized_len) + }) + .collect::>(); + + let note_types = notes.iter().map(|(path, _)| path.clone()).collect::>(); + let max_note_length = notes.iter().map(|(_, len)| len).max().unwrap_or(&0).clone(); // 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(¬e_types); + let func = generate_compute_note_hash_and_nullifier(¬e_types, max_note_length); // And inject the newly created function into the contract. @@ -85,8 +107,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: u64, +) -> 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 +124,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: u64, +) -> 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 +172,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 ) } diff --git a/noir/noir-repo/aztec_macros/src/transforms/storage.rs b/noir/noir-repo/aztec_macros/src/transforms/storage.rs index 9135be32443..d2e0f19fe5f 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/storage.rs +++ b/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, @@ -225,37 +225,9 @@ fn get_serialized_length( 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 +408,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| { diff --git a/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs b/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs index ae895d2075c..5e956b0841d 100644 --- a/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs +++ b/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs @@ -7,8 +7,8 @@ use noirc_frontend::{ resolution::{path_resolver::StandardPathResolver, resolver::Resolver}, type_check::type_check_func, }, - macros_api::{FileId, HirContext, MacroError, ModuleDefId, StructId}, - node_interner::{FuncId, TraitId}, + macros_api::{FileId, HirContext, MacroError, ModuleDefId, NodeInterner, StructId}, + node_interner::{FuncId, TraitId, TraitImplKind}, ItemVisibility, LetStatement, NoirFunction, Shared, Signedness, StructType, Type, }; @@ -309,3 +309,47 @@ fn find_non_contract_dependencies_bfs( }) }) } + +pub fn get_serialized_length( + traits: &[TraitId], + trait_name: &str, + typ: &Type, + interner: &NodeInterner, +) -> Result { + let serialized_trait_impl_kind = traits + .iter() + .find_map(|&trait_id| { + let r#trait = interner.get_trait(trait_id); + if r#trait.name.0.contents == trait_name && r#trait.generics.len() == 1 { + interner.lookup_all_trait_implementations(typ, trait_id).into_iter().next() + } else { + None + } + }) + .ok_or(MacroError { + primary_message: format!("Type {} must implement {} trait", typ, trait_name), + secondary_message: None, + span: None, + })?; + + let serialized_trait_impl_id = match serialized_trait_impl_kind { + TraitImplKind::Normal(trait_impl_id) => Ok(trait_impl_id), + _ => Err(MacroError { + primary_message: format!("{} trait impl for {} must not be assumed", trait_name, typ), + secondary_message: None, + span: 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(MacroError { + primary_message: format!("{} length for {} must be a constant", trait_name, typ), + secondary_message: None, + span: None, + }), + } +} From 3f8cbb30a5c7b1b00ac1905d8e5d5d1d651cc5db Mon Sep 17 00:00:00 2001 From: thunkar Date: Thu, 18 Apr 2024 11:06:52 +0000 Subject: [PATCH 2/9] improvements --- .../compute_note_hash_and_nullifier.rs | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs index e01febecb40..97469be1640 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs @@ -66,25 +66,32 @@ pub fn inject_compute_note_hash_and_nullifier( // 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)]. - let notes = fetch_notes(context) - .iter() - .map(|(path, typ)| { - let serialized_len = get_serialized_length( - &traits, - "NoteInterface", - &Type::Struct(typ.clone(), vec![]), - &context.def_interner, + let mut note_types = vec![]; + let mut note_lengths = vec![]; + + for (path, typ) in fetch_notes(context) { + let serialized_len = 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, ) - .expect(&format!( - "Failed to get serialized length for note type {}", - path.to_string() - )); - (path.to_string(), serialized_len) - }) - .collect::>(); - - let note_types = notes.iter().map(|(path, _)| path.clone()).collect::>(); - let max_note_length = notes.iter().map(|(_, len)| len).max().unwrap_or(&0).clone(); + })?; + note_types.push(path.to_string()); + note_lengths.push(serialized_len); + } + + let max_note_length = *note_lengths.iter().max().unwrap_or(&0); // 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(¬e_types, max_note_length); From 88ffb3035429ff36a7da1049fdcc94d8c4ca95ff Mon Sep 17 00:00:00 2001 From: thunkar Date: Thu, 18 Apr 2024 12:46:42 +0000 Subject: [PATCH 3/9] updated constantas! --- l1-contracts/src/core/libraries/ConstantsGen.sol | 2 +- .../noir-protocol-circuits/crates/types/src/constants.nr | 2 +- yarn-project/circuits.js/src/constants.gen.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index ea4ec9cca1c..2b263297a91 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -88,7 +88,7 @@ library Constants { uint256 internal constant DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631; uint256 internal constant DEPLOYER_CONTRACT_ADDRESS = - 0x1b5ecf3d26907648cf737f4304759b8c5850478e839e72f8ce1f5791b286e8f2; + 0x071de5b1e9065c428b021e27e1ba320c07b25ce057f6baeae8e00316aa21a6c9; uint256 internal constant L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 17; uint256 internal constant MAX_NOTE_FIELDS_LENGTH = 20; uint256 internal constant GET_NOTE_ORACLE_RETURN_LENGTH = 23; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 4d040a0db25..d1790b0fa72 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -129,7 +129,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 = 0x1b5ecf3d26907648cf737f4304759b8c5850478e839e72f8ce1f5791b286e8f2; +global DEPLOYER_CONTRACT_ADDRESS = 0x071de5b1e9065c428b021e27e1ba320c07b25ce057f6baeae8e00316aa21a6c9; // NOIR CONSTANTS - constants used only in yarn-packages/noir-contracts // Some are defined here because Noir doesn't yet support globals referencing other globals yet. diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index 9ac707568fd..07e33631bc1 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -73,7 +73,7 @@ export const REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af816635466f128568edb04c9fa024f6c87fb9010fdbffa68b3d99n; export const DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631n; -export const DEPLOYER_CONTRACT_ADDRESS = 0x1b5ecf3d26907648cf737f4304759b8c5850478e839e72f8ce1f5791b286e8f2n; +export const DEPLOYER_CONTRACT_ADDRESS = 0x071de5b1e9065c428b021e27e1ba320c07b25ce057f6baeae8e00316aa21a6c9n; export const L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 17; export const MAX_NOTE_FIELDS_LENGTH = 20; export const GET_NOTE_ORACLE_RETURN_LENGTH = 23; From f3ec0d1df772da8070b301a38094116a8bd7975b Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 19 Apr 2024 07:15:34 +0000 Subject: [PATCH 4/9] added compile check for note lengths and switched note processor to output errors --- .../compute_note_hash_and_nullifier.rs | 59 +++++++++++++++++-- .../pxe/src/note_processor/note_processor.ts | 2 +- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs index 97469be1640..0cbfad0581b 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs @@ -1,7 +1,7 @@ use noirc_errors::{Location, Span}; use noirc_frontend::{ graph::CrateId, - macros_api::{FileId, HirContext}, + macros_api::{FileId, HirContext, HirExpression, HirLiteral}, parse_program, FunctionReturnType, NoirFunction, Type, UnresolvedTypeData, }; @@ -69,8 +69,39 @@ pub fn inject_compute_note_hash_and_nullifier( let mut note_types = vec![]; let mut note_lengths = vec![]; + let max_note_length_const = context + .def_interner + .get_all_globals() + .iter() + .find_map(|global_info| { + if global_info.ident.0.contents == "MAX_NOTE_FIELDS_LENGTH" { + let stmt = context.def_interner.get_global_let_statement(global_info.id); + if let Some(let_stmt) = stmt { + let expression = context.def_interner.expression(&let_stmt.expression); + match expression { + HirExpression::Literal(HirLiteral::Integer(value, _)) => { + Some(value.to_u128()) + } + _ => None, + } + } else { + None + } + } else { + None + } + }) + .ok_or(( + AztecMacroError::CouldNotImplementComputeNoteHashAndNullifier { + secondary_message: Some( + "Failed to find MAX_NOTE_FIELDS_LENGTH constant".to_string(), + ), + }, + file_id, + ))?; + for (path, typ) in fetch_notes(context) { - let serialized_len = get_serialized_length( + let serialized_len: u128 = get_serialized_length( &traits, "NoteInterface", &Type::Struct(typ.clone(), vec![]), @@ -86,12 +117,28 @@ pub fn inject_compute_note_hash_and_nullifier( }, 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, + )); + } + note_types.push(path.to_string()); note_lengths.push(serialized_len); } - let max_note_length = *note_lengths.iter().max().unwrap_or(&0); + let max_note_length: u128 = *note_lengths.iter().max().unwrap_or(&0); // 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(¬e_types, max_note_length); @@ -116,7 +163,7 @@ pub fn inject_compute_note_hash_and_nullifier( fn generate_compute_note_hash_and_nullifier( note_types: &[String], - max_note_length: u64, + max_note_length: u128, ) -> NoirFunction { let function_source = generate_compute_note_hash_and_nullifier_source(note_types, max_note_length); @@ -133,7 +180,7 @@ fn generate_compute_note_hash_and_nullifier( fn generate_compute_note_hash_and_nullifier_source( note_types: &[String], - max_note_length: u64, + 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. diff --git a/yarn-project/pxe/src/note_processor/note_processor.ts b/yarn-project/pxe/src/note_processor/note_processor.ts index e8955b5e243..36a797db241 100644 --- a/yarn-project/pxe/src/note_processor/note_processor.ts +++ b/yarn-project/pxe/src/note_processor/note_processor.ts @@ -164,7 +164,7 @@ export class NoteProcessor { deferredNoteDaos.push(deferredNoteDao); } else { this.stats.failed++; - this.log.warn(`Could not process note because of "${e}". Discarding note...`); + this.log.error(`Could not process note because of "${e}". Discarding note...`); } } } From c62d29e404b3f06d8b8e4c8a64abd8df4cb682be Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 19 Apr 2024 09:31:32 +0000 Subject: [PATCH 5/9] stronger note and storage struct detection --- noir/noir-repo/aztec_macros/src/lib.rs | 9 +++++---- .../aztec_macros/src/transforms/functions.rs | 19 +++++++++++-------- .../aztec_macros/src/transforms/storage.rs | 14 ++++++++------ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index dff3193a327..048ccd866bc 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -93,9 +93,10 @@ 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 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)?; } @@ -103,7 +104,7 @@ fn transform_module( // 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; } } diff --git a/noir/noir-repo/aztec_macros/src/transforms/functions.rs b/noir/noir-repo/aztec_macros/src/transforms/functions.rs index 534d24289b7..40734e3574c 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/functions.rs +++ b/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, 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 storage_struct_name.is_some() { + let storage_def = abstract_storage(storage_struct_name.unwrap(), &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>, /// 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 ), ) } diff --git a/noir/noir-repo/aztec_macros/src/transforms/storage.rs b/noir/noir-repo/aztec_macros/src/transforms/storage.rs index d2e0f19fe5f..66057108517 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/storage.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/storage.rs @@ -214,11 +214,13 @@ pub fn get_storage_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") { @@ -476,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 }); From dd020accb99d7329cbc477e1a9db917dbb687b94 Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 19 Apr 2024 09:42:24 +0000 Subject: [PATCH 6/9] updated constants --- l1-contracts/src/core/libraries/ConstantsGen.sol | 2 +- .../noir-protocol-circuits/crates/types/src/constants.nr | 2 +- yarn-project/circuits.js/src/constants.gen.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index 586ce6be731..0b4c8838d45 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/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 = - 0x161b653b72ac5aa2982ce485b242b5c1e09afcbf27b89696f5a4e3151be37245; + 0x1c88164102b0c0849780323b87373398b4b14956a574eeb3c9c081f49eb6030c; uint256 internal constant AZTEC_ADDRESS_LENGTH = 1; uint256 internal constant DIMENSION_GAS_SETTINGS_LENGTH = 3; uint256 internal constant GAS_FEES_LENGTH = 3; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index cd982b40f1f..4d8d760b752 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/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 = 0x161b653b72ac5aa2982ce485b242b5c1e09afcbf27b89696f5a4e3151be37245; +global DEPLOYER_CONTRACT_ADDRESS = 0x1c88164102b0c0849780323b87373398b4b14956a574eeb3c9c081f49eb6030c; // LENGTH OF STRUCTS SERIALIZED TO FIELDS global AZTEC_ADDRESS_LENGTH = 1; diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index de9c6e4cc0e..ae967c1bf0b 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -74,7 +74,7 @@ export const REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af816635466f128568edb04c9fa024f6c87fb9010fdbffa68b3d99n; export const DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631n; -export const DEPLOYER_CONTRACT_ADDRESS = 0x161b653b72ac5aa2982ce485b242b5c1e09afcbf27b89696f5a4e3151be37245n; +export const DEPLOYER_CONTRACT_ADDRESS = 0x1c88164102b0c0849780323b87373398b4b14956a574eeb3c9c081f49eb6030cn; export const AZTEC_ADDRESS_LENGTH = 1; export const DIMENSION_GAS_SETTINGS_LENGTH = 3; export const GAS_FEES_LENGTH = 3; From 0c55df11d520a5da52d8504aee00bea8b4d4f2d7 Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 19 Apr 2024 09:50:44 +0000 Subject: [PATCH 7/9] clippy pass --- noir/noir-repo/aztec_macros/src/lib.rs | 4 ++-- noir/noir-repo/aztec_macros/src/transforms/functions.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index 048ccd866bc..17ae999fb8f 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -97,8 +97,8 @@ fn transform_module( let storage_defined = maybe_storage_struct_name.is_some(); 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)?; + 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 diff --git a/noir/noir-repo/aztec_macros/src/transforms/functions.rs b/noir/noir-repo/aztec_macros/src/transforms/functions.rs index 40734e3574c..8e90d40aaec 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/functions.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/functions.rs @@ -54,8 +54,8 @@ pub fn transform_function( } // Add access to the storage struct - if storage_struct_name.is_some() { - let storage_def = abstract_storage(storage_struct_name.unwrap(), &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); } From 5afbd53a1c698c1ba814f32a84a3541249e7cfe0 Mon Sep 17 00:00:00 2001 From: thunkar Date: Mon, 22 Apr 2024 06:00:11 +0000 Subject: [PATCH 8/9] PR comments --- .../compute_note_hash_and_nullifier.rs | 60 +++++++------------ .../aztec_macros/src/utils/hir_utils.rs | 38 +++++++++++- 2 files changed, 58 insertions(+), 40 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs index 0cbfad0581b..c8e7e807d87 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/compute_note_hash_and_nullifier.rs @@ -1,7 +1,7 @@ use noirc_errors::{Location, Span}; use noirc_frontend::{ graph::CrateId, - macros_api::{FileId, HirContext, HirExpression, HirLiteral}, + macros_api::{FileId, HirContext}, parse_program, FunctionReturnType, NoirFunction, Type, UnresolvedTypeData, }; @@ -9,7 +9,7 @@ use crate::utils::{ errors::AztecMacroError, hir_utils::{ collect_crate_functions, collect_traits, fetch_notes, get_contract_module_data, - get_serialized_length, inject_fn, + get_global_numberic_const, get_serialized_length, inject_fn, }, }; @@ -64,41 +64,20 @@ pub fn inject_compute_note_hash_and_nullifier( 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)]. - let mut note_types = vec![]; - let mut note_lengths = vec![]; - - let max_note_length_const = context - .def_interner - .get_all_globals() - .iter() - .find_map(|global_info| { - if global_info.ident.0.contents == "MAX_NOTE_FIELDS_LENGTH" { - let stmt = context.def_interner.get_global_let_statement(global_info.id); - if let Some(let_stmt) = stmt { - let expression = context.def_interner.expression(&let_stmt.expression); - match expression { - HirExpression::Literal(HirLiteral::Integer(value, _)) => { - Some(value.to_u128()) - } - _ => None, - } - } else { - None - } - } else { - None - } - }) - .ok_or(( - AztecMacroError::CouldNotImplementComputeNoteHashAndNullifier { - secondary_message: Some( - "Failed to find MAX_NOTE_FIELDS_LENGTH constant".to_string(), - ), - }, - file_id, - ))?; + // 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( @@ -134,11 +113,14 @@ pub fn inject_compute_note_hash_and_nullifier( )); } - note_types.push(path.to_string()); - note_lengths.push(serialized_len); + notes_and_lengths.push((path.to_string(), serialized_len)); } - let max_note_length: u128 = *note_lengths.iter().max().unwrap_or(&0); + let max_note_length: u128 = + *notes_and_lengths.iter().map(|(_, serialized_len)| serialized_len).max().unwrap_or(&0); + + let note_types = + notes_and_lengths.iter().map(|(note_type, _)| note_type.clone()).collect::>(); // 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(¬e_types, max_note_length); diff --git a/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs b/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs index 5e956b0841d..3b2f14fd87e 100644 --- a/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs +++ b/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs @@ -7,7 +7,10 @@ use noirc_frontend::{ resolution::{path_resolver::StandardPathResolver, resolver::Resolver}, type_check::type_check_func, }, - macros_api::{FileId, HirContext, MacroError, ModuleDefId, NodeInterner, StructId}, + macros_api::{ + FileId, HirContext, HirExpression, HirLiteral, MacroError, ModuleDefId, NodeInterner, + StructId, + }, node_interner::{FuncId, TraitId, TraitImplKind}, ItemVisibility, LetStatement, NoirFunction, Shared, Signedness, StructType, Type, }; @@ -353,3 +356,36 @@ pub fn get_serialized_length( }), } } + +pub fn get_global_numberic_const( + context: &HirContext, + const_name: &str, +) -> Result { + context + .def_interner + .get_all_globals() + .iter() + .find_map(|global_info| { + if global_info.ident.0.contents == const_name { + let stmt = context.def_interner.get_global_let_statement(global_info.id); + if let Some(let_stmt) = stmt { + let expression = context.def_interner.expression(&let_stmt.expression); + match expression { + HirExpression::Literal(HirLiteral::Integer(value, _)) => { + Some(value.to_u128()) + } + _ => None, + } + } else { + None + } + } else { + None + } + }) + .ok_or(MacroError { + primary_message: format!("Could not find {} global constant", const_name), + secondary_message: None, + span: None, + }) +} From 14680945f8cbd55e2db9e1166ec13ec718fdeae8 Mon Sep 17 00:00:00 2001 From: thunkar Date: Mon, 22 Apr 2024 06:08:19 +0000 Subject: [PATCH 9/9] updated contract addresses --- l1-contracts/src/core/libraries/ConstantsGen.sol | 2 +- .../noir-protocol-circuits/crates/types/src/constants.nr | 2 +- yarn-project/circuits.js/src/constants.gen.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index 27a50262e36..18770c1be97 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/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; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 7004102819e..7faf3c22c0e 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/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; diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index c3b1341b83b..4a8b78294c1 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -74,7 +74,7 @@ export const REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af816635466f128568edb04c9fa024f6c87fb9010fdbffa68b3d99n; export const DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631n; -export const DEPLOYER_CONTRACT_ADDRESS = 0x1f47133752dfcd9604f2d89c631797a84ed207c1c51d08533226dafcc8bd8548n; +export const DEPLOYER_CONTRACT_ADDRESS = 0x0cde95a10e1160d0ff69ac8212cb5902fa5add38d8596595631fcf3a667798e0n; export const DEFAULT_GAS_LIMIT = 1_000_000_000; export const DEFAULT_TEARDOWN_GAS_LIMIT = 100_000_000; export const DEFAULT_MAX_FEE_PER_GAS = 10;