Skip to content

Commit

Permalink
fix: Check for public args in aztec functions (#6355)
Browse files Browse the repository at this point in the history
This PR modifies the aztec macros to error if arguments with public
visibility are specified on private or public functions. Closes #6285

---------

Co-authored-by: Álvaro Rodríguez <sirasistant@gmail.com>
  • Loading branch information
PhilWindle and sirasistant committed May 14, 2024
1 parent c36f0fa commit 219efd6
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ 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);
}

// 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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);
}
Expand Down
28 changes: 24 additions & 4 deletions noir/noir-repo/aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ 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,
Expand Down Expand Up @@ -180,7 +182,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| {
Expand All @@ -190,9 +192,27 @@ fn transform_module(
.iter()
.any(|attr| is_custom_attribute(attr, "aztec(private)"))
})
.count();
.collect();

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_functions_count > MAX_CONTRACT_PRIVATE_FUNCTIONS {
if private_function_count > MAX_CONTRACT_PRIVATE_FUNCTIONS {
return Err(AztecMacroError::ContractHasTooManyPrivateFunctions {
span: Span::default(),
});
Expand Down
15 changes: 15 additions & 0 deletions noir/noir-repo/aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,3 +792,18 @@ 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: &[&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(())
}
6 changes: 6 additions & 0 deletions noir/noir-repo/aztec_macros/src/utils/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub enum AztecMacroError {
CouldNotGenerateContractInterface { secondary_message: Option<String> },
EventError { span: Span, message: String },
UnsupportedAttributes { span: Span, secondary_message: Option<String> },
PublicArgsDisallowed { span: Span },
}

impl From<AztecMacroError> for MacroError {
Expand Down Expand Up @@ -95,6 +96,11 @@ impl From<AztecMacroError> 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),
},
}
}
}

0 comments on commit 219efd6

Please sign in to comment.