diff --git a/docs/docs-developers/docs/resources/migration_notes.md b/docs/docs-developers/docs/resources/migration_notes.md index 6d599bccc6b1..1a9fa8128419 100644 --- a/docs/docs-developers/docs/resources/migration_notes.md +++ b/docs/docs-developers/docs/resources/migration_notes.md @@ -9,6 +9,40 @@ Aztec is in active development. Each version may introduce breaking changes that ## TBD +### Two separate init nullifiers for private and public + +Contract initialization now emits two separate nullifiers instead of one: a **private init nullifier** and a **public init nullifier**. Each nullifier gates its respective execution domain: + +- Private external functions check the private init nullifier. +- Public external functions check the public init nullifier. + +**How initializers work:** + +- **Private initializers** emit the private init nullifier. If the contract has any external public functions, the protocol auto-enqueues a public call to emit the public init nullifier. +- **Public initializers** emit both nullifiers directly. +- Contracts with no public functions only emit the private init nullifier. + +**`only_self` functions no longer have init checks.** They behave as if marked `noinitcheck`. + +**External functions called during private initialization must be `#[only_self]`.** Init nullifiers are emitted at the end of the initializer, so any external functions called on the initializing contract (e.g. via `enqueue_self` or `call_self`) during initialization will fail the init check unless they skip it. + +**Breaking change for deployment:** If your contract has external public functions and a private initializer, the class must be registered onchain before initialization. You can no longer pass `skipClassPublication: true`, because the auto-enqueued public call requires the class to be available. + +```diff + const deployed = await MyContract.deploy(wallet, ...args).send({ +- skipClassPublication: true, + }).deployed(); +``` + +### [Aztec.nr] Made `compute_note_hash_for_nullification` unconstrained + +This function shouldn't have been constrained in the first place, as constrained computation of `HintedNote` nullifiers is dangerous (constrained computation of nullifiers can be performed only on the `ConfirmedNote` type). If you were calling this from a constrained function, consider using `compute_confirmed_note_hash_for_nullification` instead. Unconstrained usage is safe. + +### [Aztec.nr] Changes to standard note hash computation + +Note hashes used to be computed with the storage slot being the last value of the preimage, it is now the first. This is to make it easier to ensure all note hashes have proper domain separation. + +This change requires no input from your side unless you were testing or relying on hardcoded note hashes. ### [Aztec.js] `getPublicEvents` now returns an object instead of an array `getPublicEvents` now returns a `GetPublicEventsResult` object with `events` and `maxLogsHit` fields instead of a plain array. This enables pagination through large result sets using the new `afterLog` filter option. @@ -23,7 +57,6 @@ The `maxLogsHit` flag indicates whether the log limit was reached, meaning more ### [Aztec.nr] Removed `get_random_bytes` The `get_random_bytes` unconstrained function has been removed from `aztec::utils::random`. If you were using it, you can replace it with direct calls to the `random` oracle from `aztec::oracle::random` and convert to bytes yourself. - ### [Aztec.js] `simulate()`, `send()`, and deploy return types changed to always return objects All SDK interaction methods now return structured objects that include offchain output alongside the primary result. This affects `.simulate()`, `.send()`, deploy `.send()`, and `Wallet.sendTx()`. diff --git a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr index b7b27b9c3d16..a5a21a797b95 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr @@ -1,15 +1,18 @@ -use crate::macros::{ - calls_generation::{ - external_functions::{generate_external_function_calls, generate_external_function_self_calls_structs}, - internal_functions::generate_call_internal_struct, - }, - dispatch::generate_public_dispatch, - internals_functions_generation::{create_fn_abi_exports, process_functions}, - notes::NOTES, - storage::STORAGE_LAYOUT_NAME, - utils::{ - get_trait_impl_method, is_fn_contract_library_method, is_fn_external, is_fn_internal, is_fn_test, - module_has_storage, +use crate::{ + macros::{ + calls_generation::{ + external_functions::{generate_external_function_calls, generate_external_function_self_calls_structs}, + internal_functions::generate_call_internal_struct, + }, + dispatch::generate_public_dispatch, + emit_public_init_nullifier::generate_emit_public_init_nullifier, + internals_functions_generation::{create_fn_abi_exports, process_functions}, + notes::NOTES, + storage::STORAGE_LAYOUT_NAME, + utils::{ + get_trait_impl_method, is_fn_contract_library_method, is_fn_external, is_fn_internal, is_fn_test, + module_has_storage, + }, }, }; @@ -53,7 +56,8 @@ pub comptime fn aztec(m: Module) -> Quoted { } else { quote {} }; - let public_dispatch = generate_public_dispatch(m); + let (has_public_init_nullifier_fn, emit_public_init_nullifier_fn_body) = generate_emit_public_init_nullifier(m); + let public_dispatch = generate_public_dispatch(m, has_public_init_nullifier_fn); quote { $interface @@ -65,6 +69,7 @@ pub comptime fn aztec(m: Module) -> Quoted { $public_dispatch $sync_state_fn_and_abi_export $process_message_fn_and_abi_export + $emit_public_init_nullifier_fn_body } } diff --git a/noir-projects/aztec-nr/aztec/src/macros/calls_generation/external_functions_stubs.nr b/noir-projects/aztec-nr/aztec/src/macros/calls_generation/external_functions_stubs.nr index 068f6c8cbcbe..513ea3593ba8 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/calls_generation/external_functions_stubs.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/calls_generation/external_functions_stubs.nr @@ -38,7 +38,7 @@ comptime fn create_stub_base(f: FunctionDefinition) -> (Quoted, Quoted, Quoted, let fn_name_str = f"\"{fn_name}\"".quoted_contents(); let fn_name_len: u32 = unquote!(quote { $fn_name_str.as_bytes().len()}); - let fn_selector: Field = compute_fn_selector(f); + let fn_selector: Field = compute_fn_selector(f.name(), f.parameters()); ( fn_name, fn_parameters_list, serialized_args_array_construction, serialized_args_array_name, diff --git a/noir-projects/aztec-nr/aztec/src/macros/dispatch.nr b/noir-projects/aztec-nr/aztec/src/macros/dispatch.nr index 52175d808070..fe9dcd16d80d 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/dispatch.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/dispatch.nr @@ -1,23 +1,28 @@ use crate::macros::internals_functions_generation::external_functions_registry::get_public_functions; use crate::protocol::meta::utils::get_params_len_quote; use crate::utils::cmap::CHashMap; +use super::functions::initialization_utils::EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME; use super::utils::compute_fn_selector; use std::panic; -/// Returns an `fn public_dispatch(...)` function for the given module that's assumed to be an Aztec contract. -pub comptime fn generate_public_dispatch(m: Module) -> Quoted { +/// Generates a `public_dispatch` function for an Aztec contract module `m`. +/// +/// The generated function dispatches public calls based on selector to the appropriate contract function. If +/// `generate_emit_public_init_nullifier` is true, it also handles dispatch to the macro-generated +/// `__emit_public_init_nullifier` function. +pub comptime fn generate_public_dispatch(m: Module, generate_emit_public_init_nullifier: bool) -> Quoted { let functions = get_public_functions(m); let unit = get_type::<()>(); let seen_selectors = &mut CHashMap::::new(); - let ifs = functions.map(|function: FunctionDefinition| { + let mut ifs = functions.map(|function: FunctionDefinition| { let parameters = function.parameters(); let return_type = function.return_type(); - let selector: Field = compute_fn_selector(function); let fn_name = function.name(); + let selector: Field = compute_fn_selector(fn_name, parameters); // Since function selectors are computed as the first 4 bytes of the hash of the function signature, it's // possible to have collisions. With the following check, we ensure it doesn't happen within the same contract. @@ -96,6 +101,22 @@ pub comptime fn generate_public_dispatch(m: Module) -> Quoted { if_ }); + // If we injected the auto-generated public function to emit the public initialization nullifier, then + // we'll also need to handle its dispatch. + if generate_emit_public_init_nullifier { + let name = EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME; + let init_nullifier_selector: Field = compute_fn_selector(name, @[]); + + ifs = ifs.push_back( + quote { + if selector == $init_nullifier_selector { + $name(); + aztec::oracle::avm::avm_return([]); + } + }, + ); + } + if ifs.len() == 0 { // No dispatch function if there are no public functions quote {} diff --git a/noir-projects/aztec-nr/aztec/src/macros/emit_public_init_nullifier.nr b/noir-projects/aztec-nr/aztec/src/macros/emit_public_init_nullifier.nr new file mode 100644 index 000000000000..1e39bba588b2 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/macros/emit_public_init_nullifier.nr @@ -0,0 +1,34 @@ +use crate::macros::{ + functions::initialization_utils::{EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME, has_public_init_checked_functions}, + internals_functions_generation::external_functions_registry::get_private_functions, + utils::is_fn_initializer, +}; + +/// Returns `(has_public_init_nullifier_fn, fn_body)` for the auto-generated public init nullifier function. +/// +/// Contracts with a private initializer and public functions that check initialization need an auto-generated public +/// function to emit the public init nullifier. If these conditions are met, returns `(true, fn_body_quoted)`; +/// otherwise returns `(false, quote {})`. +pub(crate) comptime fn generate_emit_public_init_nullifier(m: Module) -> (bool, Quoted) { + let has_private_initializer = get_private_functions(m).any(|f: FunctionDefinition| is_fn_initializer(f)); + let has_public_fns_with_init_check = has_public_init_checked_functions(m); + + if has_private_initializer & has_public_fns_with_init_check { + let name = EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME; + let assertion_message = f"Function {name} can only be called by the same contract".as_quoted_str(); + let body = quote { + #[aztec::macros::internals_functions_generation::abi_attributes::abi_public] + unconstrained fn $name() { + let context = aztec::context::PublicContext::new(|| { 0 }); + assert( + context.maybe_msg_sender().unwrap() == context.this_address(), + $assertion_message, + ); + aztec::macros::functions::initialization_utils::mark_as_initialized_public(context); + } + }; + (true, body) + } else { + (false, quote {}) + } +} diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/initialization_utils.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/initialization_utils.nr index 351684905292..7c816adfda32 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/initialization_utils.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/initialization_utils.nr @@ -1,47 +1,117 @@ use crate::protocol::{ - abis::function_selector::FunctionSelector, address::AztecAddress, constants::DOM_SEP__INITIALIZER, - hash::poseidon2_hash_with_separator, traits::ToField, + abis::function_selector::FunctionSelector, + address::AztecAddress, + constants::{DOM_SEP__INITIALIZER, DOM_SEP__PUBLIC_INITIALIZATION_NULLIFIER}, + hash::poseidon2_hash_with_separator, + traits::ToField, }; +use std::meta::{ctstring::AsCtString, unquote}; use crate::{ context::{PrivateContext, PublicContext}, + macros::{ + internals_functions_generation::external_functions_registry::get_public_functions, utils::fn_has_noinitcheck, + }, nullifier::utils::compute_nullifier_existence_request, oracle::get_contract_instance::{ get_contract_instance, get_contract_instance_deployer_avm, get_contract_instance_initialization_hash_avm, }, }; -// Used by `create_mark_as_initialized` (you won't find it through searching) +/// The name of the auto-generated function that emits the public init nullifier. +/// +/// This function is injected into the public dispatch table for contracts with initializers. +pub(crate) comptime global EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME: Quoted = quote { __emit_public_init_nullifier }; + +/// Returns `true` if the module has any public functions that require initialization checks (i.e. that don't have +/// `#[noinitcheck]`). If all public functions skip init checks, there's no point emitting the public init nullifier +/// since nothing will check it. +pub(crate) comptime fn has_public_init_checked_functions(m: Module) -> bool { + get_public_functions(m).any(|f: FunctionDefinition| !fn_has_noinitcheck(f)) +} + +/// Selector for `EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME`, derived at comptime so it stays in sync. +global EMIT_PUBLIC_INIT_NULLIFIER_SELECTOR: FunctionSelector = comptime { + let name = EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME; + let sig = f"{name}()".as_ctstring(); + unquote!(quote { FunctionSelector::from_signature($sig) }) +}; + +/// Emits (only) the public initialization nullifier. +/// +/// This function is called by the aztec-nr auto-generated external public contract function (enqueued by private +/// [`initializer`](crate::macros::functions::initializer) functions), and also by +/// [`mark_as_initialized_from_public_initializer`] for public initializers. +/// +/// # Warning +/// +/// This should not be called manually. Incorrect use can leave the contract in a broken initialization state (e.g. +/// emitting the public nullifier without the private one). The macro-generated code handles this automatically. pub fn mark_as_initialized_public(context: PublicContext) { - let init_nullifier = compute_unsiloed_contract_initialization_nullifier((context).this_address()); + let init_nullifier = compute_public_init_nullifier(context.this_address()); context.push_nullifier(init_nullifier); } -// Used by `create_mark_as_initialized` (you won't find it through searching) -pub fn mark_as_initialized_private(context: &mut PrivateContext) { - let init_nullifier = compute_unsiloed_contract_initialization_nullifier((*context).this_address()); +fn mark_as_initialized_private(context: &mut PrivateContext) { + let init_nullifier = compute_private_init_nullifier((*context).this_address()); context.push_nullifier(init_nullifier); } -// Used by `create_init_check` (you won't find it through searching) +/// Emits the private initialization nullifier and, if relevant, enqueues the emission of the public one. +/// +/// If the contract has public functions that perform initialization checks (i.e. that don't have `#[noinitcheck]`), +/// this also enqueues a call to the auto-generated `__emit_public_init_nullifier` function so the public nullifier is +/// emitted in public. Called by private [`initializer`](crate::macros::functions::initializer) macros. +pub fn mark_as_initialized_from_private_initializer(context: &mut PrivateContext, emit_public_init_nullifier: bool) { + mark_as_initialized_private(context); + if emit_public_init_nullifier { + context.call_public_function((*context).this_address(), EMIT_PUBLIC_INIT_NULLIFIER_SELECTOR, [], false); + } +} + +/// Emits both initialization nullifiers (private and public). +/// +/// Called by public [`initializer`](crate::macros::functions::initializer) macros, since public initializers must set +/// both so that both private and public functions see the contract as initialized. +pub fn mark_as_initialized_from_public_initializer(context: PublicContext) { + let private_nullifier = compute_private_init_nullifier(context.this_address()); + context.push_nullifier(private_nullifier); + mark_as_initialized_public(context); +} + +/// Asserts that the contract has been initialized, from public's perspective. +/// +/// Checks that the public initialization nullifier exists. pub fn assert_is_initialized_public(context: PublicContext) { - let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context.this_address()); - // Safety: TODO(F-239) - this is currently unsafe, we cannot rely on the nullifier existing to determine that any - // public component of contract initialization has been complete. + let init_nullifier = compute_public_init_nullifier(context.this_address()); + // Safety: the public init nullifier is only ever emitted by public functions, and so the timing concerns from + // nullifier_exists_unsafe do not apply. Additionally, it is emitted after all initializer functions have run, + // so initialization is guaranteed to be complete by the time it exists. assert(context.nullifier_exists_unsafe(init_nullifier, context.this_address()), "Not initialized"); } -// Used by `create_init_check` (you won't find it through searching) +/// Asserts that the contract has been initialized, from private's perspective. +/// +/// Checks that the private initialization nullifier exists. pub fn assert_is_initialized_private(context: &mut PrivateContext) { - let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context.this_address()); + let init_nullifier = compute_private_init_nullifier(context.this_address()); let nullifier_existence_request = compute_nullifier_existence_request(init_nullifier, context.this_address()); context.assert_nullifier_exists(nullifier_existence_request); } -fn compute_unsiloed_contract_initialization_nullifier(address: AztecAddress) -> Field { +// TODO(F-194): This leaks whether a contract has been initialized, since anyone who knows the address can compute this +// nullifier and check for its existence. It is also not domain separated. +fn compute_private_init_nullifier(address: AztecAddress) -> Field { address.to_field() } +fn compute_public_init_nullifier(address: AztecAddress) -> Field { + poseidon2_hash_with_separator( + [address.to_field()], + DOM_SEP__PUBLIC_INITIALIZATION_NULLIFIER, + ) +} + // Used by `create_assert_correct_initializer_args` (you won't find it through searching) pub fn assert_initialization_matches_address_preimage_public(context: PublicContext) { let address = context.this_address(); diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr index 75ab13d794ee..b9db8529f058 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr @@ -9,6 +9,7 @@ use crate::macros::{ }; use super::utils::{fn_has_allow_phase_change, fn_has_noinitcheck, is_fn_initializer, is_fn_only_self, is_fn_view}; use auth_registry::AUTHORIZE_ONCE_REGISTRY; +use initialization_utils::EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME; // Functions can have multiple attributes applied to them, e.g. a single function can have #[external("public")], // #[view] and #[only_self]. However. the order in which this will be evaluated is unknown, which makes combining them @@ -29,7 +30,7 @@ use auth_registry::AUTHORIZE_ONCE_REGISTRY; /// /// Initializer functions are similar to constructors: /// - can only be called once -/// - [`external`] non-initializer functions cannot be called until one of the initializers has ben called +/// - [`external`] non-initializer functions cannot be called until one of the initializers has been called /// /// The only exception are [`noinitcheck`] functions, which can be called even if none of the initializers has been /// called. @@ -63,17 +64,26 @@ use auth_registry::AUTHORIZE_ONCE_REGISTRY; /// initialization by marking them with the `#[noinitcheck]` attribute - though any contract state initialization will /// of course not have taken place. /// -/// ## Cost +/// ## How It Works +/// +/// Initializers emit nullifiers to mark the contract as initialized. Two separate nullifiers are used (a private init +/// nullifier and a public init nullifier) because private nullifiers are committed before public execution +/// begins: if a single nullifier were used, public functions enqueued by the initializer would see it as +/// existing *before* the public initialization code had a chance to run. /// -/// The initialization process emits a nullifier which marks the contract as initialized. All other [`external`] -/// functions are automatically made to check that this nullifier exists, ensuring initialization. +/// - **Private initializers** emit the private init nullifier. For contracts that also have external public functions, +/// they auto-enqueue a call to an auto-generated public function that emits the public init nullifier during public +/// execution. This function name is reserved and cannot be used by contract developers. +/// - **Public initializers** emit both nullifiers directly. +/// - **Private external functions** check the private init nullifier. +/// - **Public external functions** check the public init nullifier. /// -/// For private non-initializer functions, the cost of this check is equivalent to that of a call to +/// For private non-initializer functions, the cost of this check is equivalent to a call to /// [`PrivateContext::assert_nullifier_exists`](crate::context::PrivateContext::assert_nullifier_exists). For public /// ones, it is equivalent to a call to /// [`PublicContext::nullifier_exists_unsafe`](crate::context::PublicContext::nullifier_exists_unsafe). /// -/// The [`noinitcheck`] attribute can be used to skip the initialization nullifer checks. +/// The [`noinitcheck`] attribute can be used to skip these checks. [`only_self`] functions also implicitly skip them. pub comptime fn initializer(f: FunctionDefinition) { // Marker attribute - see the comment at the top of this file @@ -185,6 +195,24 @@ pub comptime fn allow_phase_change(f: FunctionDefinition) { /// /// A classic example is a private mint function in a token, which would require an enqueued public call in which the /// total supply is incremented. +/// +/// ## Initialization Checks +/// +/// `only_self` functions implicitly skip initialization checks (as if they had [`noinitcheck`]). We want +/// `only_self` functions to be callable during initialization, so we can't have them check the init nullifier +/// since it would fail. +/// +/// This is safe because `only_self` functions can be called only by the contract the function is in, meaning +/// execution must start with another external function in the same contract. Eventually the call stack reaches the +/// `only_self` function, but let's focus on that external entry point: +/// - If it already performed an init check, then we are safe. +/// - If it skipped the init check (via [`noinitcheck`]), then the contract developer is explicitly choosing to not +/// check for initialization, and so will our `only_self` function. That's a design choice by the developer. If +/// we didn't skip the init check on `only_self`, the developer would just add `noinitcheck` to it anyway. +/// - If it was the initializer, note that init nullifiers are emitted at the end of initialization: the private +/// init nullifier after all private execution, and the public one after all public execution. So in terms of +/// init checking, everything behaves as if the contract hasn't been initialized yet, and the same two points +/// above still apply. pub comptime fn only_self(f: FunctionDefinition) { // Marker attribute - see the comment at the top of this file @@ -197,6 +225,8 @@ pub comptime fn only_self(f: FunctionDefinition) { f"The #[only_self] attribute can only be applied to #[external(\"private\")] or #[external(\"public\")] functions - {name} is neither", ); } + + f.add_attribute("noinitcheck"); } /// View functions cannot modify state in any way, including performing contract calls that would in turn modify state. @@ -379,6 +409,18 @@ comptime fn assert_valid_public(f: FunctionDefinition) { f"The #[allow_phase_change] attribute cannot be applied to #[external(\"public\")] functions - {name}", ); } + + if f.name() == EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME { + panic( + f"Function name '{EMIT_PUBLIC_INIT_NULLIFIER_FN_NAME}' is reserved for internal use", + ); + } + + if f.name() == quote { public_dispatch } { + panic( + f"Function name 'public_dispatch' is reserved for internal use", + ); + } } comptime fn assert_valid_utility(f: FunctionDefinition) { diff --git a/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/private.nr b/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/private.nr index 88c0f8590d4d..6f9d8d8592c1 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/private.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/private.nr @@ -1,4 +1,5 @@ use crate::macros::{ + functions::initialization_utils::has_public_init_checked_functions, internals_functions_generation::external::helpers::{create_authorize_once_check, get_abi_relevant_attributes}, utils::{ fn_has_allow_phase_change, fn_has_authorize_once, fn_has_noinitcheck, is_fn_initializer, is_fn_only_self, @@ -77,9 +78,10 @@ pub(crate) comptime fn generate_private_external(f: FunctionDefinition) -> Quote }; let (assert_initializer, mark_as_initialized) = if is_fn_initializer(f) { + let has_public_fns_with_init_check = has_public_init_checked_functions(f.module()); ( quote { aztec::macros::functions::initialization_utils::assert_initialization_matches_address_preimage_private(*self.context); }, - quote { aztec::macros::functions::initialization_utils::mark_as_initialized_private(self.context); }, + quote { aztec::macros::functions::initialization_utils::mark_as_initialized_from_private_initializer(self.context, $has_public_fns_with_init_check); }, ) } else { (quote {}, quote {}) @@ -177,6 +179,9 @@ pub(crate) comptime fn generate_private_external(f: FunctionDefinition) -> Quote let body_quote = body.map(|expr| expr.quoted()).join(quote { }); + // `mark_as_initialized` is placed after the user's function body. If it ran at the beginning, the contract + // would appear initialized while the initializer is still running, allowing contracts called by the initializer + // to re-enter into a half-initialized contract. let to_append = quote { $return_value $mark_as_initialized diff --git a/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/public.nr b/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/public.nr index d481fd0a9666..0eaf0adea0ea 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/public.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/internals_functions_generation/external/public.nr @@ -79,7 +79,7 @@ pub(crate) comptime fn generate_public_external(f: FunctionDefinition) -> Quoted let (assert_initializer, mark_as_initialized) = if is_fn_initializer(f) { ( quote { aztec::macros::functions::initialization_utils::assert_initialization_matches_address_preimage_public(self.context); }, - quote { aztec::macros::functions::initialization_utils::mark_as_initialized_public(self.context); }, + quote { aztec::macros::functions::initialization_utils::mark_as_initialized_from_public_initializer(self.context); }, ) } else { (quote {}, quote {}) @@ -108,6 +108,9 @@ pub(crate) comptime fn generate_public_external(f: FunctionDefinition) -> Quoted $authorize_once_check }; + // `mark_as_initialized` is placed after the user's function body. If it ran at the beginning, the contract + // would appear initialized while the initializer is still running, allowing contracts called by the initializer + // to re-enter into a half-initialized contract. let to_append = quote { $mark_as_initialized }; diff --git a/noir-projects/aztec-nr/aztec/src/macros/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/mod.nr index 50f17bc3250a..41f1e0a6904d 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/mod.nr @@ -3,6 +3,7 @@ pub mod aztec; pub mod dispatch; pub(crate) mod calls_generation; +pub(crate) mod emit_public_init_nullifier; pub mod internals_functions_generation; pub mod functions; pub mod utils; diff --git a/noir-projects/aztec-nr/aztec/src/macros/utils.nr b/noir-projects/aztec-nr/aztec/src/macros/utils.nr index 151079bb208b..40cdd20251c9 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/utils.nr @@ -85,15 +85,15 @@ comptime fn signature_of_type(typ: Type) -> CtString { } } -pub(crate) comptime fn compute_fn_selector(f: FunctionDefinition) -> Field { +/// Computes a function selector from a name and parameter list. +pub(crate) comptime fn compute_fn_selector(fn_name: Quoted, parameters: [(Quoted, Type)]) -> Field { // The function selector is computed from the function signature, which is made up of the function name and types // of parameters, but not including the return type. For example, given: // // fn foo(a: Field, b: AztecAddress) -> Field // // The signature will be "foo(Field,AztecAddress)". - let fn_name = f.name(); - let args_signatures = f.parameters().map(|(_, typ)| signature_of_type(typ)).join(",".as_ctstring()); + let args_signatures = parameters.map(|(_, typ)| signature_of_type(typ)).join(",".as_ctstring()); let signature_quote = f"{fn_name}({args_signatures})".as_ctstring(); let computation_quote = quote { diff --git a/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_emit_public_init_nullifier/Nargo.toml b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_emit_public_init_nullifier/Nargo.toml new file mode 100644 index 000000000000..1cfbb552131b --- /dev/null +++ b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_emit_public_init_nullifier/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "reserved_emit_public_init_nullifier" +type = "contract" +authors = [""] + +[dependencies] +aztec = { path = "../../../aztec" } diff --git a/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_emit_public_init_nullifier/src/main.nr b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_emit_public_init_nullifier/src/main.nr new file mode 100644 index 000000000000..8fefacdd7369 --- /dev/null +++ b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_emit_public_init_nullifier/src/main.nr @@ -0,0 +1,11 @@ +/// Public functions cannot use the reserved name __emit_public_init_nullifier. +/// +use aztec::macros::aztec; + +#[aztec] +contract ReservedEmitPublicInitNullifier { + use aztec::macros::functions::external; + + #[external("public")] + fn __emit_public_init_nullifier() {} +} diff --git a/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_public_dispatch/Nargo.toml b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_public_dispatch/Nargo.toml new file mode 100644 index 000000000000..e6c4a85c297f --- /dev/null +++ b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_public_dispatch/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "reserved_public_dispatch" +type = "contract" +authors = [""] + +[dependencies] +aztec = { path = "../../../aztec" } diff --git a/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_public_dispatch/src/main.nr b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_public_dispatch/src/main.nr new file mode 100644 index 000000000000..f3493f9ccfa3 --- /dev/null +++ b/noir-projects/aztec-nr/macro_compilation_failure_tests/failure_contracts/reserved_public_dispatch/src/main.nr @@ -0,0 +1,11 @@ +/// Public functions cannot use the reserved name public_dispatch. +/// +use aztec::macros::aztec; + +#[aztec] +contract ReservedPublicDispatch { + use aztec::macros::functions::external; + + #[external("public")] + fn public_dispatch() {} +} diff --git a/noir-projects/noir-contracts/Nargo.toml b/noir-projects/noir-contracts/Nargo.toml index 7b7c76bf8bad..c0afb35f7b34 100644 --- a/noir-projects/noir-contracts/Nargo.toml +++ b/noir-projects/noir-contracts/Nargo.toml @@ -46,6 +46,7 @@ members = [ "contracts/test/counter_contract", "contracts/test/event_only_contract", "contracts/test/import_test_contract", + "contracts/test/init_test_contract", "contracts/test/invalid_account_contract", "contracts/test/no_constructor_contract", "contracts/test/note_getter_contract", @@ -55,6 +56,7 @@ members = [ "contracts/test/oracle_version_check_contract", "contracts/test/parent_contract", "contracts/test/pending_note_hashes_contract", + "contracts/test/private_init_test_contract", "contracts/test/public_immutable_contract", "contracts/test/returning_tuple_contract", "contracts/test/scope_test_contract", diff --git a/noir-projects/noir-contracts/contracts/test/init_test_contract/Nargo.toml b/noir-projects/noir-contracts/contracts/test/init_test_contract/Nargo.toml new file mode 100644 index 000000000000..2d378fe1845a --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/init_test_contract/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "init_test_contract" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../../aztec-nr/aztec" } diff --git a/noir-projects/noir-contracts/contracts/test/init_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/init_test_contract/src/main.nr new file mode 100644 index 000000000000..2120f72e0fa5 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/init_test_contract/src/main.nr @@ -0,0 +1,77 @@ +// A contract with both private and public functions used to test initialization behavior, including init checks, +// self-calls during initialization, and public/private ordering. +use aztec::macros::aztec; + +#[aztec] +pub contract InitTest { + use aztec::macros::{ + functions::{external, initializer, noinitcheck, only_self, view}, + storage::storage, + }; + use aztec::protocol::address::AztecAddress; + use aztec::state_vars::{Map, PublicMutable}; + + #[storage] + struct Storage { + public_values: Map, Context>, + } + + #[external("private")] + #[initializer] + fn constructor(owner: AztecAddress, value: Field) {} + + #[external("private")] + #[initializer] + fn initializer_self_calling_private_not_init_checked(owner: AztecAddress, value: Field) { + self.call_self.priv_no_init_check(owner, value); + } + + #[external("private")] + #[initializer] + fn initializer_self_calling_private_init_checked(owner: AztecAddress, value: Field) { + self.call_self.priv_init_check(owner, value); + } + + #[external("private")] + #[initializer] + fn initializer_enqueuing_public_init_checked(owner: AztecAddress, value: Field) { + self.enqueue_self.pub_init_check(owner, value); + } + + #[external("private")] + #[initializer] + fn initializer_calling_only_self(owner: AztecAddress, value: Field) { + self.call_self.priv_only_self(owner, value); + } + + // Tests that a public initializer cannot self-call a public function that has init checks, since the contract is + // not yet initialized at that point. + #[external("public")] + #[initializer] + fn public_initializer_self_calling_init_checked(owner: AztecAddress, value: Field) { + self.call_self.pub_init_check(owner, value); + } + + #[external("private")] + #[only_self] + fn priv_only_self(owner: AztecAddress, value: Field) {} + + #[external("private")] + fn priv_init_check(owner: AztecAddress, value: Field) {} + + #[external("private")] + #[noinitcheck] + fn priv_no_init_check(owner: AztecAddress, value: Field) {} + + #[external("public")] + fn pub_init_check(owner: AztecAddress, value: Field) { + self.storage.public_values.at(owner).write(value); + } + + #[external("public")] + #[noinitcheck] + #[view] + fn pub_no_init_check(owner: AztecAddress) -> pub Field { + self.storage.public_values.at(owner).read() + } +} diff --git a/noir-projects/noir-contracts/contracts/test/private_init_test_contract/Nargo.toml b/noir-projects/noir-contracts/contracts/test/private_init_test_contract/Nargo.toml new file mode 100644 index 000000000000..3414de8c017f --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/private_init_test_contract/Nargo.toml @@ -0,0 +1,9 @@ +[package] +name = "private_init_test_contract" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../../aztec-nr/aztec" } +field_note = { path = "../../../../aztec-nr/field-note" } diff --git a/noir-projects/noir-contracts/contracts/test/private_init_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/private_init_test_contract/src/main.nr new file mode 100644 index 000000000000..f4eadfc6292f --- /dev/null +++ b/noir-projects/noir-contracts/contracts/test/private_init_test_contract/src/main.nr @@ -0,0 +1,47 @@ +// A private-only contract (no public functions) used to test initialization behavior. Since it has no public +// functions, it does not need to be published on-chain to be tested. +use aztec::macros::aztec; + +#[aztec] +pub contract PrivateInitTest { + use aztec::{ + macros::{functions::{external, initializer, noinitcheck}, storage::storage}, + messages::message_delivery::MessageDelivery, + protocol::address::AztecAddress, + state_vars::{Owned, PrivateMutable}, + }; + use field_note::FieldNote; + + #[storage] + struct Storage { + value: Owned, Context>, + } + + #[initializer] + #[external("private")] + fn initialize(initial_value: Field) { + let owner = self.msg_sender(); + self.storage.value.at(owner).initialize(FieldNote { value: initial_value }).deliver( + MessageDelivery.ONCHAIN_CONSTRAINED, + ); + } + + #[external("private")] + fn private_init_check_write_value(new_value: Field) { + let owner = self.msg_sender(); + self.storage.value.at(owner).replace(|_old| FieldNote { value: new_value }).deliver( + MessageDelivery.ONCHAIN_CONSTRAINED, + ); + } + + #[external("private")] + #[noinitcheck] + fn private_no_init_check_emit_nullifier(nullifier: Field) { + self.context.push_nullifier(nullifier); + } + + #[external("utility")] + unconstrained fn utility_read_value(owner: AztecAddress) -> Field { + self.storage.value.at(owner).view_note().value + } +} diff --git a/noir-projects/noir-contracts/contracts/test/stateful_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/stateful_test_contract/src/main.nr index 4baa4a76d448..6f0d7ba3a87e 100644 --- a/noir-projects/noir-contracts/contracts/test/stateful_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test/stateful_test_contract/src/main.nr @@ -3,13 +3,7 @@ use aztec::macros::aztec; #[aztec] pub contract StatefulTest { - use aztec::macros::{ - functions::{ - external, initialization_utils::assert_is_initialized_private, initializer, noinitcheck, - view, - }, - storage::storage, - }; + use aztec::macros::{functions::{external, initializer, noinitcheck, view}, storage::storage}; use aztec::{ messages::message_delivery::MessageDelivery, note::{note_getter_options::NoteGetterOptions, note_viewer_options::NoteViewerOptions}, @@ -60,18 +54,6 @@ pub contract StatefulTest { } } - #[external("private")] - fn destroy_and_create(recipient: AztecAddress) { - assert_is_initialized_private(self.context); - let sender = self.msg_sender(); - - let _ = self.storage.notes.at(sender).pop_notes(NoteGetterOptions::new().set_limit(2)); - - self.storage.notes.at(recipient).insert(FieldNote { value: 92 }).deliver( - MessageDelivery.ONCHAIN_CONSTRAINED, - ); - } - #[external("private")] #[noinitcheck] fn destroy_and_create_no_init_check(recipient: AztecAddress) { diff --git a/noir-projects/noir-contracts/contracts/test/updatable_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/updatable_contract/src/main.nr index 317e381aa137..08ed569b0d7a 100644 --- a/noir-projects/noir-contracts/contracts/test/updatable_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test/updatable_contract/src/main.nr @@ -1,10 +1,11 @@ use aztec::macros::aztec; -/// A contract that can be updated to a new contract class. Used to test contract updates in e2e_contract_updates.test.ts. +/// A contract that can be updated to a new contract class. Used to test contract updates in +/// e2e_contract_updates.test.ts. #[aztec] contract Updatable { use aztec::{ - macros::{functions::{external, initializer}, storage::storage}, + macros::{functions::{external, initializer, internal, only_self}, storage::storage}, messages::message_delivery::MessageDelivery, protocol::{ address::AztecAddress, constants::CONTRACT_INSTANCE_REGISTRY_CONTRACT_ADDRESS, @@ -29,12 +30,18 @@ contract Updatable { self.storage.private_value.at(owner).initialize(note).deliver( MessageDelivery.ONCHAIN_CONSTRAINED, ); - self.enqueue_self.set_public_value(initial_value); + self.enqueue_self._initialize_public_value(initial_value); } #[external("public")] fn set_public_value(new_value: Field) { - self.storage.public_value.write(new_value); + self.internal.write_public_value(new_value); + } + + #[external("public")] + #[only_self] + fn _initialize_public_value(new_value: Field) { + self.internal.write_public_value(new_value); } #[external("private")] @@ -55,6 +62,11 @@ contract Updatable { .get_update_delay()) } + #[internal("public")] + fn write_public_value(new_value: Field) { + self.storage.public_value.write(new_value); + } + #[external("utility")] unconstrained fn get_private_value(owner: AztecAddress) -> Field { self.storage.private_value.at(owner).view_note().value 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 6100302d21ee..dd7f7895a6a3 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -713,6 +713,9 @@ pub global DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT: u32 = 623934423; // TODO: consider moving these to aztec-nr pub global DOM_SEP__INITIALIZATION_NULLIFIER: u32 = 1653084894; +pub global DOM_SEP__PUBLIC_INITIALIZATION_NULLIFIER: u32 = 3342006647; + +/// Domain separator for L1 to L2 message secret hashes. pub global DOM_SEP__SECRET_HASH: u32 = 4199652938; pub global DOM_SEP__TX_NULLIFIER: u32 = 1025801951; pub global DOM_SEP__SIGNATURE_PAYLOAD: u32 = 463525807; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants_tests.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants_tests.nr index 6f7c9dee1cc1..6a5048b6926f 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants_tests.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants_tests.nr @@ -14,7 +14,8 @@ use crate::{ DOM_SEP__NOTE_NULLIFIER, DOM_SEP__OVSK_M, DOM_SEP__PARTIAL_ADDRESS, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT, DOM_SEP__PRIVATE_FUNCTION_LEAF, DOM_SEP__PRIVATE_LOG_FIRST_FIELD, DOM_SEP__PRIVATE_TX_HASH, DOM_SEP__PROTOCOL_CONTRACTS, - DOM_SEP__PUBLIC_BYTECODE, DOM_SEP__PUBLIC_CALLDATA, DOM_SEP__PUBLIC_KEYS_HASH, + DOM_SEP__PUBLIC_BYTECODE, DOM_SEP__PUBLIC_CALLDATA, + DOM_SEP__PUBLIC_INITIALIZATION_NULLIFIER, DOM_SEP__PUBLIC_KEYS_HASH, DOM_SEP__PUBLIC_LEAF_SLOT, DOM_SEP__PUBLIC_STORAGE_MAP_SLOT, DOM_SEP__PUBLIC_TX_HASH, DOM_SEP__SECRET_HASH, DOM_SEP__SIGNATURE_PAYLOAD, DOM_SEP__SILOED_NOTE_HASH, DOM_SEP__SILOED_NULLIFIER, DOM_SEP__SYMMETRIC_KEY, DOM_SEP__SYMMETRIC_KEY_2, DOM_SEP__TSK_M, @@ -129,7 +130,7 @@ impl HashedValueTester::new(); + let mut tester = HashedValueTester::<51, 44>::new(); // ----------------- // Domain separators @@ -183,6 +184,10 @@ fn hashed_values_match_derived() { DOM_SEP__INITIALIZATION_NULLIFIER, "initialization_nullifier", ); + tester.assert_dom_sep_matches_derived( + DOM_SEP__PUBLIC_INITIALIZATION_NULLIFIER, + "public_initialization_nullifier", + ); tester.assert_dom_sep_matches_derived(DOM_SEP__SECRET_HASH, "secret_hash"); tester.assert_dom_sep_matches_derived(DOM_SEP__TX_NULLIFIER, "tx_nullifier"); tester.assert_dom_sep_matches_derived(DOM_SEP__SIGNATURE_PAYLOAD, "signature_payload"); diff --git a/yarn-project/constants/src/constants.gen.ts b/yarn-project/constants/src/constants.gen.ts index 6675dfa2cca1..c192fb14654e 100644 --- a/yarn-project/constants/src/constants.gen.ts +++ b/yarn-project/constants/src/constants.gen.ts @@ -541,6 +541,7 @@ export enum DomainSeparator { CIPHERTEXT_FIELD_MASK = 1870492847, PARTIAL_NOTE_VALIDITY_COMMITMENT = 623934423, INITIALIZATION_NULLIFIER = 1653084894, + PUBLIC_INITIALIZATION_NULLIFIER = 3342006647, SECRET_HASH = 4199652938, TX_NULLIFIER = 1025801951, SIGNATURE_PAYLOAD = 463525807, diff --git a/yarn-project/end-to-end/src/composed/ha/e2e_ha_full.test.ts b/yarn-project/end-to-end/src/composed/ha/e2e_ha_full.test.ts index 279b42a40de6..9739b6f74bd8 100644 --- a/yarn-project/end-to-end/src/composed/ha/e2e_ha_full.test.ts +++ b/yarn-project/end-to-end/src/composed/ha/e2e_ha_full.test.ts @@ -282,8 +282,6 @@ describe('HA Full Setup', () => { const { receipt } = await deployer.deploy(ownerAddress, sender, 1).send({ from: ownerAddress, contractAddressSalt: new Fr(BigInt(1)), - skipClassPublication: true, - skipInstancePublication: true, wait: { returnReceipt: true }, }); @@ -519,8 +517,6 @@ describe('HA Full Setup', () => { const { receipt } = await deployer.deploy(ownerAddress, ownerAddress, i + 100).send({ from: ownerAddress, contractAddressSalt: new Fr(BigInt(i + 100)), - skipClassPublication: true, - skipInstancePublication: true, wait: { returnReceipt: true }, }); diff --git a/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts b/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts index ff0bf8f94e9b..c7b9a3be05c4 100644 --- a/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts +++ b/yarn-project/end-to-end/src/composed/web3signer/e2e_multi_validator_node_key_store.test.ts @@ -1,7 +1,7 @@ import type { AztecNodeConfig } from '@aztec/aztec-node'; import { AztecAddress, EthAddress } from '@aztec/aztec.js/addresses'; import { NO_WAIT, waitForProven } from '@aztec/aztec.js/contracts'; -import { ContractDeployer } from '@aztec/aztec.js/deployment'; +import { ContractDeployer, publishContractClass } from '@aztec/aztec.js/deployment'; import { Fr } from '@aztec/aztec.js/fields'; import { type AztecNode, waitForTx } from '@aztec/aztec.js/node'; import type { Wallet } from '@aztec/aztec.js/wallet'; @@ -314,6 +314,9 @@ describe('e2e_multi_validator_node', () => { config.ethereumSlotDuration * 3, 1, ); + + const publishClass = await publishContractClass(wallet, StatefulTestContractArtifact); + await publishClass.send({ from: ownerAddress }); }); afterEach(async () => { @@ -327,7 +330,6 @@ describe('e2e_multi_validator_node', () => { from: ownerAddress, contractAddressSalt, skipClassPublication: true, - skipInstancePublication: true, wait: NO_WAIT, }); }; diff --git a/yarn-project/end-to-end/src/e2e_block_building.test.ts b/yarn-project/end-to-end/src/e2e_block_building.test.ts index 23b17b5d98f6..6dddbf7ced61 100644 --- a/yarn-project/end-to-end/src/e2e_block_building.test.ts +++ b/yarn-project/end-to-end/src/e2e_block_building.test.ts @@ -138,8 +138,6 @@ describe('e2e_block_building', () => { const options: DeployOptions = { from: ownerAddress, contractAddressSalt: new Fr(BigInt(i + 1)), - skipClassPublication: true, - skipInstancePublication: true, }; const instance = await methods[i].getInstance(options); addresses.push(instance.address); diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract/deploy_method.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract/deploy_method.test.ts index ed7cfba703fb..cd28a3568e55 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract/deploy_method.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract/deploy_method.test.ts @@ -6,6 +6,7 @@ import { type AztecNode, createAztecNodeClient } from '@aztec/aztec.js/node'; import type { Wallet } from '@aztec/aztec.js/wallet'; import { TokenContract } from '@aztec/noir-contracts.js/Token'; import { CounterContract } from '@aztec/noir-test-contracts.js/Counter'; +import { InitTestContract } from '@aztec/noir-test-contracts.js/InitTest'; import { NoConstructorContract } from '@aztec/noir-test-contracts.js/NoConstructor'; import { StatefulTestContract } from '@aztec/noir-test-contracts.js/StatefulTest'; import { GasFees } from '@aztec/stdlib/gas'; @@ -99,6 +100,31 @@ describe('e2e_deploy_contract deploy method', () => { expect((await contract.methods.summed_values(owner).simulate({ from: defaultAccountAddress })).result).toEqual(30n); }); + // The public init nullifier is emitted at the end of the initializer. If it were emitted at the beginning, + // the contract would appear initialized while the initializer body is still running, allowing external callers + // to interact with a half-initialized contract. As a consequence, any public calls the initializer enqueues + // run before the nullifier exists and cannot pass init checks. + it('refuses to self-call an init-checked function during public initialization', async () => { + const owner = defaultAccountAddress; + await expect( + InitTestContract.deployWithOpts( + { wallet, method: 'public_initializer_self_calling_init_checked' }, + owner, + 42, + ).simulate({ from: defaultAccountAddress }), + ).rejects.toThrow(/Not initialized/); + }); + + // Private functions execute before public functions, so the init check in create_note fails + // because the public initializer hasn't emitted the private initialization nullifier yet. + it('refuses to call a private init-checked function in same tx as public initialization', async () => { + const owner = defaultAccountAddress; + const deployMethod = StatefulTestContract.deployWithOpts({ wallet, method: 'public_constructor' }, owner, 42); + const contract = await deployMethod.register(); + const batch = new BatchCall(wallet, [deployMethod, contract.methods.create_note(owner, 10)]); + await expect(batch.send({ from: defaultAccountAddress })).rejects.toThrow(/Cannot find the leaf for nullifier/); + }); + it('deploys a contract with a default initializer not named constructor', async () => { logger.debug(`Deploying contract with a default initializer named initialize`); const opts = { skipClassPublication: true, skipInstancePublication: true, from: defaultAccountAddress }; diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract/legacy.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract/legacy.test.ts index 9d16aeb08bb0..ae2c526cdbce 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract/legacy.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract/legacy.test.ts @@ -97,8 +97,6 @@ describe('e2e_deploy_contract legacy', () => { const badDeploy = new ContractDeployer(artifact, wallet).deploy(AztecAddress.ZERO, ...initArgs); const firstOpts: DeployOptions = { - skipClassPublication: true, - skipInstancePublication: true, from: defaultAccountAddress, }; const secondOpts: DeployOptions = { diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract/private_initialization.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract/private_initialization.test.ts index a6814602e130..5dbe52080cbd 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract/private_initialization.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract/private_initialization.test.ts @@ -1,16 +1,19 @@ import { AztecAddress } from '@aztec/aztec.js/addresses'; -import { BatchCall } from '@aztec/aztec.js/contracts'; +import { BatchCall, getContractInstanceFromInstantiationParams } from '@aztec/aztec.js/contracts'; +import { publishContractClass, publishInstance } from '@aztec/aztec.js/deployment'; import { Fr } from '@aztec/aztec.js/fields'; import type { Logger } from '@aztec/aztec.js/log'; import type { AztecNode } from '@aztec/aztec.js/node'; +import { InitTestContract } from '@aztec/noir-test-contracts.js/InitTest'; import { NoConstructorContract } from '@aztec/noir-test-contracts.js/NoConstructor'; -import { StatefulTestContract } from '@aztec/noir-test-contracts.js/StatefulTest'; -import { TestContract } from '@aztec/noir-test-contracts.js/Test'; +import { PrivateInitTestContract } from '@aztec/noir-test-contracts.js/PrivateInitTest'; import { siloNullifier } from '@aztec/stdlib/hash'; import { TX_ERROR_EXISTING_NULLIFIER } from '@aztec/stdlib/tx'; import type { TestWallet } from '../test-wallet/test_wallet.js'; -import { DeployTest, type StatefulContractCtorArgs } from './deploy_test.js'; +import { DeployTest } from './deploy_test.js'; + +type InitTestCtorArgs = Parameters; describe('e2e_deploy_contract private initialization', () => { const t = new DeployTest('private initialization'); @@ -22,6 +25,7 @@ describe('e2e_deploy_contract private initialization', () => { beforeAll(async () => { ({ logger, wallet, aztecNode, defaultAccountAddress } = await t.setup()); + await publishContractClass(wallet, InitTestContract.artifact).then(c => c.send({ from: defaultAccountAddress })); }); afterAll(() => t.teardown()); @@ -30,8 +34,13 @@ describe('e2e_deploy_contract private initialization', () => { // Requires registering the contract artifact and instance locally in the pxe. // The function has a noinitcheck flag so it can be called without initialization. it('executes a noinitcheck function in an uninitialized contract', async () => { - const contract = await t.registerContract(wallet, TestContract); - const { receipt } = await contract.methods.emit_nullifier(10).send({ from: defaultAccountAddress }); + const contract = await t.registerContract(wallet, PrivateInitTestContract, { + initArgs: [0], + constructorName: 'initialize', + }); + const { receipt } = await contract.methods + .private_no_init_check_emit_nullifier(10) + .send({ from: defaultAccountAddress }); const txEffects = await aztecNode.getTxEffect(receipt.txHash); const expected = await siloNullifier(contract.address, new Fr(10)); @@ -52,69 +61,153 @@ describe('e2e_deploy_contract private initialization', () => { ).resolves.toEqual(expect.objectContaining({ result: true })); }); - // Tests privately initializing an undeployed contract. Also requires pxe registration in advance. + // Tests privately initializing an undeployed contract, then calling init-checked functions. it('privately initializes an undeployed contract from an account contract', async () => { - const owner = (await wallet.createAccount()).address; - const initArgs: StatefulContractCtorArgs = [owner, 42]; - const contract = await t.registerContract(wallet, StatefulTestContract, { initArgs }); + const contract = await t.registerContract(wallet, PrivateInitTestContract, { + initArgs: [42], + constructorName: 'initialize', + }); logger.info(`Calling the constructor for ${contract.address}`); - await contract.methods.constructor(...initArgs).send({ from: defaultAccountAddress }); + await contract.methods.initialize(42).send({ from: defaultAccountAddress }); logger.info(`Checking if the constructor was run for ${contract.address}`); - expect((await contract.methods.summed_values(owner).simulate({ from: owner })).result).toEqual(42n); + expect( + (await contract.methods.utility_read_value(defaultAccountAddress).simulate({ from: defaultAccountAddress })) + .result, + ).toEqual(42n); logger.info(`Calling a private function that requires initialization on ${contract.address}`); - await contract.methods.create_note(owner, 10).send({ from: defaultAccountAddress }); - expect((await contract.methods.summed_values(owner).simulate({ from: owner })).result).toEqual(52n); + await contract.methods.private_init_check_write_value(43).send({ from: defaultAccountAddress }); + expect( + (await contract.methods.utility_read_value(defaultAccountAddress).simulate({ from: defaultAccountAddress })) + .result, + ).toEqual(43n); }); // Tests privately initializing multiple undeployed contracts on the same tx through an account contract. it('initializes multiple undeployed contracts in a single tx', async () => { - const owner = (await wallet.createAccount()).address; - const initArgs: StatefulContractCtorArgs[] = [42, 52].map(value => [owner, value]); const contracts = await Promise.all( - initArgs.map(initArgs => t.registerContract(wallet, StatefulTestContract, { initArgs })), + [42, 52].map(value => + t.registerContract(wallet, PrivateInitTestContract, { initArgs: [value], constructorName: 'initialize' }), + ), ); - const calls = contracts.map((c, i) => c.methods.constructor(...initArgs[i])); + const calls = [42, 52].map((value, i) => contracts[i].methods.initialize(value)); await new BatchCall(wallet, calls).send({ from: defaultAccountAddress }); - expect((await contracts[0].methods.summed_values(owner).simulate({ from: owner })).result).toEqual(42n); - expect((await contracts[1].methods.summed_values(owner).simulate({ from: owner })).result).toEqual(52n); + expect( + (await contracts[0].methods.utility_read_value(defaultAccountAddress).simulate({ from: defaultAccountAddress })) + .result, + ).toEqual(42n); + expect( + (await contracts[1].methods.utility_read_value(defaultAccountAddress).simulate({ from: defaultAccountAddress })) + .result, + ).toEqual(52n); }); it('initializes and calls a private function in a single tx', async () => { - const owner = (await wallet.createAccount()).address; - const initArgs: StatefulContractCtorArgs = [owner, 42]; - const contract = await t.registerContract(wallet, StatefulTestContract, { initArgs }); + const contract = await t.registerContract(wallet, PrivateInitTestContract, { + initArgs: [42], + constructorName: 'initialize', + }); const batch = new BatchCall(wallet, [ - contract.methods.constructor(...initArgs), - contract.methods.create_note(owner, 10), + contract.methods.initialize(42), + contract.methods.private_init_check_write_value(43), ]); logger.info(`Executing constructor and private function in batch at ${contract.address}`); await batch.send({ from: defaultAccountAddress }); - expect((await contract.methods.summed_values(owner).simulate({ from: owner })).result).toEqual(52n); + expect( + (await contract.methods.utility_read_value(defaultAccountAddress).simulate({ from: defaultAccountAddress })) + .result, + ).toEqual(43n); }); it('refuses to initialize a contract twice', async () => { - const owner = (await wallet.createAccount()).address; - const initArgs: StatefulContractCtorArgs = [owner, 42]; - const contract = await t.registerContract(wallet, StatefulTestContract, { initArgs }); - await contract.methods.constructor(...initArgs).send({ from: defaultAccountAddress }); - await expect(contract.methods.constructor(...initArgs).send({ from: defaultAccountAddress })).rejects.toThrow( + const contract = await t.registerContract(wallet, PrivateInitTestContract, { + initArgs: [42], + constructorName: 'initialize', + }); + await contract.methods.initialize(42).send({ from: defaultAccountAddress }); + await expect(contract.methods.initialize(42).send({ from: defaultAccountAddress })).rejects.toThrow( TX_ERROR_EXISTING_NULLIFIER, ); }); it('refuses to call a private function that requires initialization', async () => { const owner = (await wallet.createAccount()).address; - const initArgs: StatefulContractCtorArgs = [owner, 42]; - const contract = await t.registerContract(wallet, StatefulTestContract, { initArgs }); - // TODO(@spalladino): It'd be nicer to be able to fail the assert with a more descriptive message. - await expect(contract.methods.create_note(owner, 10).send({ from: defaultAccountAddress })).rejects.toThrow( + const initArgs: InitTestCtorArgs = [owner, 42]; + const contract = await t.registerContract(wallet, InitTestContract, { initArgs }); + // TODO(#14894): It'd be nicer to be able to fail the assert with a more descriptive message. + await expect(contract.methods.priv_init_check(owner, 10).send({ from: defaultAccountAddress })).rejects.toThrow( /Cannot find the leaf for nullifier/i, ); }); + // A public call enqueued before the private constructor should fail the init check, even though the + // private constructor emits the init nullifier in the same tx. + it('refuses to call a public function enqueued before private initialization in same tx', async () => { + const { contract, initArgs } = await deployUninitialized(); + const owner = defaultAccountAddress; + const batch = new BatchCall(wallet, [ + contract.methods.pub_init_check(owner, 84), + contract.methods.constructor(...initArgs), + ]); + await expect(batch.simulate({ from: defaultAccountAddress })).rejects.toThrow(/Not initialized/); + }); + + it('allows calling a public function enqueued after private initialization in same tx', async () => { + const { contract, initArgs } = await deployUninitialized(); + const owner = defaultAccountAddress; + const batch = new BatchCall(wallet, [ + contract.methods.constructor(...initArgs), + contract.methods.pub_init_check(owner, 84), + ]); + await batch.send({ from: defaultAccountAddress }); + expect((await contract.methods.pub_no_init_check(owner).simulate({ from: defaultAccountAddress })).result).toEqual( + 84n, + ); + }); + + it('allows self-calling a noinitcheck function from a private initializer', async () => { + const owner = (await wallet.createAccount()).address; + const initArgs: InitTestCtorArgs = [owner, 42]; + const contract = await registerAndPublishContract(initArgs, { + constructorName: 'initializer_self_calling_private_not_init_checked', + }); + await contract.methods + .initializer_self_calling_private_not_init_checked(...initArgs) + .send({ from: defaultAccountAddress }); + }); + + it('allows calling an only_self function from a private initializer', async () => { + const owner = (await wallet.createAccount()).address; + const initArgs: InitTestCtorArgs = [owner, 42]; + const contract = await registerAndPublishContract(initArgs, { + constructorName: 'initializer_calling_only_self', + }); + await contract.methods.initializer_calling_only_self(...initArgs).send({ from: defaultAccountAddress }); + }); + + it('refuses to self-call an init-checked function during private initialization', async () => { + const owner = defaultAccountAddress; + const initArgs: InitTestCtorArgs = [owner, 42]; + const contract = await t.registerContract(wallet, InitTestContract, { + initArgs, + constructorName: 'initializer_self_calling_private_init_checked', + }); + await expect( + contract.methods.initializer_self_calling_private_init_checked(...initArgs).send({ from: defaultAccountAddress }), + ).rejects.toThrow(/Cannot find the leaf for nullifier/); + }); + + it('refuses to run an enqueued public init-checked self-call from private initialization', async () => { + const { contract, initArgs } = await deployUninitialized({ + constructorName: 'initializer_enqueuing_public_init_checked', + }); + await expect( + contract.methods.initializer_enqueuing_public_init_checked(...initArgs).simulate({ from: defaultAccountAddress }), + ).rejects.toThrow(/Not initialized/); + }); + it('refuses to initialize a contract with incorrect args', async () => { const owner = (await wallet.createAccount()).address; - const contract = await t.registerContract(wallet, StatefulTestContract, { initArgs: [owner, 42] }); + const contract = await t.registerContract(wallet, InitTestContract, { initArgs: [owner, 42] }); await expect(contract.methods.constructor(owner, 43).simulate({ from: defaultAccountAddress })).rejects.toThrow( /Initialization hash does not match/, ); @@ -122,7 +215,7 @@ describe('e2e_deploy_contract private initialization', () => { it('refuses to initialize an instance from a different deployer', async () => { const owner = (await wallet.createAccount()).address; - const contract = await t.registerContract(wallet, StatefulTestContract, { + const contract = await t.registerContract(wallet, InitTestContract, { initArgs: [owner, 42], deployer: owner, }); @@ -130,4 +223,34 @@ describe('e2e_deploy_contract private initialization', () => { /Initializer address is not the contract deployer/i, ); }); + + /** Registers a contract instance locally and publishes it on-chain (so sequencers can find public function's bytecode). */ + async function registerAndPublishContract( + initArgs: InitTestCtorArgs, + opts: { constructorName?: string; deployer?: AztecAddress } = {}, + ) { + const salt = Fr.random(); + const instance = await getContractInstanceFromInstantiationParams(InitTestContract.artifact, { + constructorArgs: initArgs, + salt, + constructorArtifact: opts.constructorName, + deployer: opts.deployer, + }); + await publishInstance(wallet, instance).send({ from: defaultAccountAddress }); + return t.registerContract(wallet, InitTestContract, { + initArgs, + salt, + constructorName: opts.constructorName, + deployer: opts.deployer, + }); + } + + /** Publishes a contract instance on-chain without initializing it. */ + async function deployUninitialized(opts: { constructorName?: string } = {}) { + const owner = defaultAccountAddress; + const initArgs: InitTestCtorArgs = [owner, 42]; + const constructorName = opts.constructorName; + const contract = await registerAndPublishContract(initArgs, { constructorName }); + return { contract, initArgs }; + } }); diff --git a/yarn-project/end-to-end/src/e2e_multi_eoa.test.ts b/yarn-project/end-to-end/src/e2e_multi_eoa.test.ts index 02bad7926b51..3b776cb963c4 100644 --- a/yarn-project/end-to-end/src/e2e_multi_eoa.test.ts +++ b/yarn-project/end-to-end/src/e2e_multi_eoa.test.ts @@ -111,8 +111,6 @@ describe('e2e_multi_eoa', () => { const deployMethod = StatefulTestContract.deploy(wallet, defaultAccountAddress, 0); const deployMethodTx = await proveInteraction(wallet, deployMethod, { contractAddressSalt: Fr.random(), - skipClassPublication: true, - skipInstancePublication: true, from: defaultAccountAddress, }); diff --git a/yarn-project/end-to-end/src/e2e_multi_validator/e2e_multi_validator_node.test.ts b/yarn-project/end-to-end/src/e2e_multi_validator/e2e_multi_validator_node.test.ts index 47ec290401f2..53060168826d 100644 --- a/yarn-project/end-to-end/src/e2e_multi_validator/e2e_multi_validator_node.test.ts +++ b/yarn-project/end-to-end/src/e2e_multi_validator/e2e_multi_validator_node.test.ts @@ -117,8 +117,6 @@ describe('e2e_multi_validator_node', () => { const { receipt: tx } = await deployer.deploy(ownerAddress, sender, 1).send({ from: ownerAddress, contractAddressSalt: new Fr(BigInt(1)), - skipClassPublication: true, - skipInstancePublication: true, wait: { returnReceipt: true }, }); await waitForProven(aztecNode, tx, { @@ -180,8 +178,6 @@ describe('e2e_multi_validator_node', () => { const { receipt: tx } = await deployer.deploy(ownerAddress, sender, 1).send({ from: ownerAddress, contractAddressSalt: new Fr(BigInt(1)), - skipClassPublication: true, - skipInstancePublication: true, wait: { returnReceipt: true }, }); await waitForProven(aztecNode, tx, { diff --git a/yarn-project/end-to-end/src/e2e_sequencer/reload_keystore.test.ts b/yarn-project/end-to-end/src/e2e_sequencer/reload_keystore.test.ts index 667728c4056b..221c3086d924 100644 --- a/yarn-project/end-to-end/src/e2e_sequencer/reload_keystore.test.ts +++ b/yarn-project/end-to-end/src/e2e_sequencer/reload_keystore.test.ts @@ -133,8 +133,6 @@ describe('e2e_reload_keystore', () => { const { txHash: sentTx1 } = await deployer.deploy(ownerAddress, ownerAddress, 1).send({ from: ownerAddress, contractAddressSalt: new Fr(1), - skipClassPublication: true, - skipInstancePublication: true, wait: NO_WAIT, }); const receipt1 = await waitForTx(aztecNode, sentTx1); @@ -200,8 +198,6 @@ describe('e2e_reload_keystore', () => { const { txHash: sentTx2 } = await deployer.deploy(ownerAddress, ownerAddress, 2).send({ from: ownerAddress, contractAddressSalt: new Fr(2), - skipClassPublication: true, - skipInstancePublication: true, wait: NO_WAIT, }); const receipt2 = await waitForTx(aztecNode, sentTx2); diff --git a/yarn-project/end-to-end/src/e2e_sequencer_config.test.ts b/yarn-project/end-to-end/src/e2e_sequencer_config.test.ts index 9db69b105ae2..a0c91687d866 100644 --- a/yarn-project/end-to-end/src/e2e_sequencer_config.test.ts +++ b/yarn-project/end-to-end/src/e2e_sequencer_config.test.ts @@ -2,6 +2,7 @@ import { getInitialTestAccountsData } from '@aztec/accounts/testing'; import type { AztecNode } from '@aztec/aztec.js/node'; import { TxReceipt } from '@aztec/aztec.js/tx'; import { Bot, type BotConfig, BotStore, getBotDefaultConfig } from '@aztec/bot'; +import { DEFAULT_DA_GAS_LIMIT } from '@aztec/constants'; import type { Logger } from '@aztec/foundation/log'; import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; import type { SequencerClient } from '@aztec/sequencer-client'; @@ -82,7 +83,7 @@ describe('e2e_sequencer_config', () => { expect(totalManaUsed).toBeGreaterThan(0n); bot.updateConfig({ l2GasLimit: Number(totalManaUsed), - daGasLimit: Number(totalManaUsed), + daGasLimit: DEFAULT_DA_GAS_LIMIT, }); // Set the maxL2BlockGas to the total mana used diff --git a/yarn-project/end-to-end/src/e2e_simple.test.ts b/yarn-project/end-to-end/src/e2e_simple.test.ts index 1dbfe5861776..e38df666d579 100644 --- a/yarn-project/end-to-end/src/e2e_simple.test.ts +++ b/yarn-project/end-to-end/src/e2e_simple.test.ts @@ -75,8 +75,6 @@ describe('e2e_simple', () => { const { receipt: txReceipt } = await deployer.deploy(ownerAddress, sender, 1).send({ from: ownerAddress, contractAddressSalt: new Fr(BigInt(1)), - skipClassPublication: true, - skipInstancePublication: true, wait: { returnReceipt: true }, }); await waitForProven(aztecNode, txReceipt, {