From 18ca6723be0ef1f88c10950be34263ca9382d2a9 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Sun, 12 May 2024 20:29:57 +0000 Subject: [PATCH 1/3] Check for public args in aztec functions --- .../app_subscription_contract/src/main.nr | 2 +- .../ecdsa_account_contract/src/main.nr | 4 +-- .../contracts/escrow_contract/src/main.nr | 2 +- .../src/main.nr | 2 +- .../schnorr_account_contract/src/main.nr | 2 +- .../src/main.nr | 2 +- .../src/main.nr | 2 +- noir/noir-repo/aztec_macros/src/lib.rs | 26 +++++++++++++++---- .../aztec_macros/src/transforms/functions.rs | 18 +++++++++++++ .../aztec_macros/src/utils/errors.rs | 6 +++++ 10 files changed, 53 insertions(+), 13 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr index 74cc3d1861b..03cd9da4ba7 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr @@ -33,7 +33,7 @@ contract AppSubscription { global SUBSCRIPTION_TXS = 5; #[aztec(private)] - fn entrypoint(payload: pub DAppPayload, user_address: pub AztecAddress) { + fn entrypoint(payload: DAppPayload, user_address: AztecAddress) { assert(context.msg_sender().to_field() == 0); assert_current_call_valid_authwit(&mut context, user_address); diff --git a/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr index d992251604d..89da4adddb8 100644 --- a/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr @@ -26,7 +26,7 @@ contract EcdsaAccount { // Creates a new account out of an ECDSA public key to use for signature verification #[aztec(private)] #[aztec(initializer)] - fn constructor(signing_pub_key_x: pub [u8; 32], signing_pub_key_y: pub [u8; 32]) { + fn constructor(signing_pub_key_x: [u8; 32], signing_pub_key_y: [u8; 32]) { let this = context.this_address(); let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this); storage.public_key.initialize(&mut pub_key_note, true); @@ -34,7 +34,7 @@ contract EcdsaAccount { // Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts #[aztec(private)] - fn entrypoint(app_payload: pub AppPayload, fee_payload: pub FeePayload) { + fn entrypoint(app_payload: AppPayload, fee_payload: FeePayload) { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); actions.entrypoint(app_payload, fee_payload); } diff --git a/noir-projects/noir-contracts/contracts/escrow_contract/src/main.nr b/noir-projects/noir-contracts/contracts/escrow_contract/src/main.nr index bc17776d83d..ae45b80b6e5 100644 --- a/noir-projects/noir-contracts/contracts/escrow_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/escrow_contract/src/main.nr @@ -16,7 +16,7 @@ contract Escrow { // Creates a new instance #[aztec(private)] #[aztec(initializer)] - fn constructor(owner: pub AztecAddress) { + fn constructor(owner: AztecAddress) { let mut note = AddressNote::new(owner, owner); storage.owner.initialize(&mut note, true); } diff --git a/noir-projects/noir-contracts/contracts/multi_call_entrypoint_contract/src/main.nr b/noir-projects/noir-contracts/contracts/multi_call_entrypoint_contract/src/main.nr index 22d0d6ea39f..d5c405c6c61 100644 --- a/noir-projects/noir-contracts/contracts/multi_call_entrypoint_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/multi_call_entrypoint_contract/src/main.nr @@ -7,7 +7,7 @@ contract MultiCallEntrypoint { use dep::authwit::entrypoint::app::AppPayload; #[aztec(private)] - fn entrypoint(app_payload: pub AppPayload) { + fn entrypoint(app_payload: AppPayload) { app_payload.execute_calls(&mut context); } } diff --git a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr index 39cc3384b3b..b7e0f890eb0 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr @@ -28,7 +28,7 @@ contract SchnorrAccount { // Constructs the contract #[aztec(private)] #[aztec(initializer)] - fn constructor(signing_pub_key_x: pub Field, signing_pub_key_y: pub Field) { + fn constructor(signing_pub_key_x: Field, signing_pub_key_y: Field) { let this = context.this_address(); // docs:start:initialize let mut pub_key_note = PublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this); diff --git a/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr index a660670a2a5..c16e7adfd62 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr @@ -16,7 +16,7 @@ contract SchnorrHardcodedAccount { // Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts #[aztec(private)] - fn entrypoint(app_payload: pub AppPayload, fee_payload: pub FeePayload) { + fn entrypoint(app_payload: AppPayload, fee_payload: FeePayload) { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); actions.entrypoint(app_payload, fee_payload); } diff --git a/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr index 5c75c095d2f..471a255f639 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr @@ -13,7 +13,7 @@ contract SchnorrSingleKeyAccount { // Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts #[aztec(private)] - fn entrypoint(app_payload: pub AppPayload, fee_payload: pub FeePayload) { + fn entrypoint(app_payload: AppPayload, fee_payload: FeePayload) { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); actions.entrypoint(app_payload, fee_payload); } diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index 17ae999fb8f..bd4e2db700f 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -7,7 +7,7 @@ use transforms::{ generate_contract_interface, stub_function, update_fn_signatures_in_contract_interface, }, events::{generate_selector_impl, transform_events}, - functions::{export_fn_abi, transform_function, transform_unconstrained}, + functions::{check_for_public_args, export_fn_abi, transform_function, transform_unconstrained}, note_interface::{generate_note_interface_impl, inject_note_exports}, storage::{ assign_storage_slots, check_for_storage_definition, check_for_storage_implementation, @@ -180,7 +180,7 @@ fn transform_module( if has_transformed_module { // We only want to run these checks if the macro processor has found the module to be an Aztec contract. - let private_functions_count = module + let private_functions: Vec<_> = module .functions .iter() .filter(|func| { @@ -189,10 +189,26 @@ fn transform_module( .secondary .iter() .any(|attr| is_custom_attribute(attr, "aztec(private)")) - }) - .count(); + }).collect(); - if private_functions_count > MAX_CONTRACT_PRIVATE_FUNCTIONS { + let public_functions: Vec<_> = module + .functions + .iter() + .filter(|func| { + func.def + .attributes + .secondary + .iter() + .any(|attr| is_custom_attribute(attr, "aztec(public)")) + }).collect(); + + let private_function_count = private_functions.len(); + + check_for_public_args(&private_functions)?; + + check_for_public_args(&public_functions)?; + + if private_function_count > MAX_CONTRACT_PRIVATE_FUNCTIONS { return Err(AztecMacroError::ContractHasTooManyPrivateFunctions { span: Span::default(), }); diff --git a/noir/noir-repo/aztec_macros/src/transforms/functions.rs b/noir/noir-repo/aztec_macros/src/transforms/functions.rs index 90563c6085c..6909c91a670 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/functions.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/functions.rs @@ -792,3 +792,21 @@ fn add_cast_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement { vec![cast_operation], // args ))) } + + +/** + * Takes a vector of functions and checks for the presence of arguments with Public visibility + * Returns AztecMAcroError::PublicArgsDisallowed if found + */ +pub fn check_for_public_args(functions: &Vec<&NoirFunction>) -> Result<(), AztecMacroError> { + for func in functions { + for param in &func.def.parameters { + if param.visibility == Visibility::Public { + return Err(AztecMacroError::PublicArgsDisallowed { + span: func.span(), + }); + } + } + } + Ok(()) +} diff --git a/noir/noir-repo/aztec_macros/src/utils/errors.rs b/noir/noir-repo/aztec_macros/src/utils/errors.rs index db86012a007..8d1a19bfea9 100644 --- a/noir/noir-repo/aztec_macros/src/utils/errors.rs +++ b/noir/noir-repo/aztec_macros/src/utils/errors.rs @@ -20,6 +20,7 @@ pub enum AztecMacroError { CouldNotGenerateContractInterface { secondary_message: Option }, EventError { span: Span, message: String }, UnsupportedAttributes { span: Span, secondary_message: Option }, + PublicArgsDisallowed { span: Span }, } impl From for MacroError { @@ -95,6 +96,11 @@ impl From for MacroError { secondary_message, span: Some(span), }, + AztecMacroError::PublicArgsDisallowed { span } => MacroError { + primary_message: "Aztec functions can't have public arguments".to_string(), + secondary_message: None, + span: Some(span), + }, } } } From 634b6f312088271c61e4933ec72c3f0e738f515b Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Sun, 12 May 2024 20:45:59 +0000 Subject: [PATCH 2/3] Formatting --- noir/noir-repo/aztec_macros/src/lib.rs | 28 +++++++++++-------- .../aztec_macros/src/transforms/functions.rs | 5 +--- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index bd4e2db700f..5f0326bea5a 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -7,7 +7,9 @@ use transforms::{ generate_contract_interface, stub_function, update_fn_signatures_in_contract_interface, }, events::{generate_selector_impl, transform_events}, - functions::{check_for_public_args, export_fn_abi, transform_function, transform_unconstrained}, + functions::{ + check_for_public_args, export_fn_abi, transform_function, transform_unconstrained, + }, note_interface::{generate_note_interface_impl, inject_note_exports}, storage::{ assign_storage_slots, check_for_storage_definition, check_for_storage_implementation, @@ -189,22 +191,24 @@ fn transform_module( .secondary .iter() .any(|attr| is_custom_attribute(attr, "aztec(private)")) - }).collect(); + }) + .collect(); let public_functions: Vec<_> = module - .functions - .iter() - .filter(|func| { - func.def - .attributes - .secondary - .iter() - .any(|attr| is_custom_attribute(attr, "aztec(public)")) - }).collect(); + .functions + .iter() + .filter(|func| { + func.def + .attributes + .secondary + .iter() + .any(|attr| is_custom_attribute(attr, "aztec(public)")) + }) + .collect(); let private_function_count = private_functions.len(); - check_for_public_args(&private_functions)?; + check_for_public_args(&private_functions)?; check_for_public_args(&public_functions)?; diff --git a/noir/noir-repo/aztec_macros/src/transforms/functions.rs b/noir/noir-repo/aztec_macros/src/transforms/functions.rs index 6909c91a670..a8514242763 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/functions.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/functions.rs @@ -793,7 +793,6 @@ fn add_cast_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement { ))) } - /** * Takes a vector of functions and checks for the presence of arguments with Public visibility * Returns AztecMAcroError::PublicArgsDisallowed if found @@ -802,9 +801,7 @@ pub fn check_for_public_args(functions: &Vec<&NoirFunction>) -> Result<(), Aztec for func in functions { for param in &func.def.parameters { if param.visibility == Visibility::Public { - return Err(AztecMacroError::PublicArgsDisallowed { - span: func.span(), - }); + return Err(AztecMacroError::PublicArgsDisallowed { span: func.span() }); } } } From 5b9b1fab51e35c2112c0136785b5b69cd8529a92 Mon Sep 17 00:00:00 2001 From: PhilWindle <60546371+PhilWindle@users.noreply.github.com> Date: Mon, 13 May 2024 11:00:34 +0100 Subject: [PATCH 3/3] Update noir/noir-repo/aztec_macros/src/transforms/functions.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Álvaro Rodríguez --- noir/noir-repo/aztec_macros/src/transforms/functions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir/noir-repo/aztec_macros/src/transforms/functions.rs b/noir/noir-repo/aztec_macros/src/transforms/functions.rs index a8514242763..487d15ceabe 100644 --- a/noir/noir-repo/aztec_macros/src/transforms/functions.rs +++ b/noir/noir-repo/aztec_macros/src/transforms/functions.rs @@ -797,7 +797,7 @@ fn add_cast_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement { * Takes a vector of functions and checks for the presence of arguments with Public visibility * Returns AztecMAcroError::PublicArgsDisallowed if found */ -pub fn check_for_public_args(functions: &Vec<&NoirFunction>) -> Result<(), AztecMacroError> { +pub fn check_for_public_args(functions: &[&NoirFunction]) -> Result<(), AztecMacroError> { for func in functions { for param in &func.def.parameters { if param.visibility == Visibility::Public {