diff --git a/docs/docs-developers/docs/resources/migration_notes.md b/docs/docs-developers/docs/resources/migration_notes.md index 54dbd96391fb..130d4fe115b4 100644 --- a/docs/docs-developers/docs/resources/migration_notes.md +++ b/docs/docs-developers/docs/resources/migration_notes.md @@ -9,6 +9,36 @@ Aztec is in active development. Each version may introduce breaking changes that ## TBD +### [Aztec.nr] `attempt_note_discovery` now takes two separate functions instead of one + +The `attempt_note_discovery` function (and related discovery functions like `do_sync_state`, `process_message_ciphertext`) now takes separate `compute_note_hash` and `compute_note_nullifier` arguments instead of a single combined `compute_note_hash_and_nullifier`. The corresponding type aliases are now `ComputeNoteHash` and `ComputeNoteNullifier` (instead of `ComputeNoteHashAndNullifier`). + +This split improves performance during nonce discovery: the note hash only needs to be computed once, while the old combined function recomputed it for every candidate nonce. + +Most contracts are not affected, as the macro-generated `sync_state` and `process_message` functions handle this automatically. Only contracts that call `attempt_note_discovery` directly need to update. + +**Migration:** + +```diff + attempt_note_discovery( + contract_address, + tx_hash, + unique_note_hashes_in_tx, + first_nullifier_in_tx, + recipient, +- _compute_note_hash_and_nullifier, ++ _compute_note_hash, ++ _compute_note_nullifier, + owner, + storage_slot, + randomness, + note_type_id, + packed_note, + ); +``` + +**Impact**: Contracts that call `attempt_note_discovery` or related discovery functions directly with a custom `_compute_note_hash_and_nullifier` argument. The old combined function is still generated (deprecated) but is no longer used by the framework. Additionally, if you had a custom `_compute_note_hash_and_nullifier` function then compilation will now fail as you'll need to also produce the corresponding `_compute_note_hash` and `_compute_note_nullifier` functions. + ### Private initialization nullifier now includes `init_hash` The private initialization nullifier is no longer derived from just the contract address. It is now computed as a Poseidon2 hash of `[address, init_hash]` using a dedicated domain separator. This prevents observers from determining whether a fully private contract has been initialized by simply knowing its address. @@ -47,6 +77,7 @@ The function signature has changed to resolve the epoch internally from a transa The return type `L2ToL1MembershipWitness` now includes `epochNumber`. An optional `messageIndexInTx` parameter can be passed as the fourth argument to disambiguate when a transaction emits multiple identical L2-to-L1 messages. **Impact**: All call sites that compute L2-to-L1 membership witnesses must update to the new argument order and extract `epochNumber` from the result instead of passing it in. + ### 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: @@ -149,9 +180,14 @@ When using `NO_WAIT`, returns `{ txHash, offchainEffects, offchainMessages }` in Offchain messages emitted by the transaction are available on the result: ```typescript -const { receipt, offchainMessages } = await contract.methods.foo(args).send({ from: sender }); +const { receipt, offchainMessages } = await contract.methods + .foo(args) + .send({ from: sender }); for (const msg of offchainMessages) { - console.log(`Message for ${msg.recipient} from contract ${msg.contractAddress}:`, msg.payload); + console.log( + `Message for ${msg.recipient} from contract ${msg.contractAddress}:`, + msg.payload, + ); } ``` @@ -206,6 +242,7 @@ counter/ This enables adding multiple contracts to a single workspace. Running `aztec new ` inside an existing workspace (a directory with a `Nargo.toml` containing `[workspace]`) now adds a new `_contract` and `_test` crate pair to the workspace instead of creating a new directory. **What changed:** + - Crate directories are now `_contract/` and `_test/` instead of `contract/` and `test/`. - Contract code is now at `_contract/src/main.nr` instead of `contract/src/main.nr`. - Contract dependencies go in `_contract/Nargo.toml` instead of `contract/Nargo.toml`. @@ -265,6 +302,7 @@ my_project/ ``` **What changed:** + - The `--contract` and `--lib` flags have been removed from `aztec new` and `aztec init`. These commands now always create a contract workspace. - Contract code is now at `contract/src/main.nr` instead of `src/main.nr`. - The `Nargo.toml` in the project root is now a workspace file. Contract dependencies go in `contract/Nargo.toml`. @@ -290,7 +328,7 @@ The wallet now passes scopes to PXE, and only the `from` address is in scope by 2. **Operations that access another contract's private state** (e.g., withdrawing from an escrow contract that nullifies the contract's own token notes). -``` +```` **Example: deploying a contract with private storage (e.g., `PrivateToken`)** @@ -304,7 +342,7 @@ The wallet now passes scopes to PXE, and only the `from` address is in scope by from: sender, + additionalScopes: [tokenInstance.address], }); -``` +```` **Example: withdrawing from an escrow contract** @@ -353,22 +391,26 @@ The `include_by_timestamp` field has been renamed to `expiration_timestamp` acro The Aztec CLI is now installed without Docker. The installation command has changed: **Old installation (deprecated):** + ```bash bash -i <(curl -sL https://install.aztec.network) aztec-up ``` **New installation:** + ```bash VERSION= bash -i <(curl -sL https://install.aztec.network/) ``` For example, to install version `#include_version_without_prefix`: + ```bash VERSION=#include_version_without_prefix bash -i <(curl -sL https://install.aztec.network/#include_version_without_prefix) ``` **Key changes:** + - Docker is no longer required to run the Aztec CLI tools - The `VERSION` environment variable must be set in the installation command - The version must also be included in the URL path @@ -377,12 +419,13 @@ VERSION=#include_version_without_prefix bash -i <(curl -sL https://install.aztec After installation, `aztec-up` functions as a version manager with the following commands: -| Command | Description | -|---------|-------------| +| Command | Description | +| ---------------------------- | ------------------------------------------- | | `aztec-up install ` | Install a specific version and switch to it | -| `aztec-up use ` | Switch to an already installed version | -| `aztec-up list` | List all installed versions | -| `aztec-up self-update` | Update aztec-up itself | +| `aztec-up use ` | Switch to an already installed version | +| `aztec-up list` | List all installed versions | +| `aztec-up self-update` | Update aztec-up itself | + ### `@aztec/test-wallet` replaced by `@aztec/wallets` The `@aztec/test-wallet` package has been removed. Use `@aztec/wallets` instead, which provides `EmbeddedWallet` with a `static create()` factory: diff --git a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr index ed1792b93bc2..7b1d85286faf 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr @@ -1,3 +1,5 @@ +mod compute_note_hash_and_nullifier; + use crate::{ macros::{ calls_generation::{ @@ -7,16 +9,14 @@ use crate::{ 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, - }, + utils::{is_fn_contract_library_method, is_fn_external, is_fn_internal, is_fn_test, module_has_storage}, }, messages::discovery::CustomMessageHandler, }; +use compute_note_hash_and_nullifier::generate_contract_library_methods_compute_note_hash_and_nullifier; + /// Configuration for the [`aztec`] macro. /// /// This type lets users override different parts of the default aztec-nr contract behavior, such @@ -100,12 +100,16 @@ pub comptime fn aztec(m: Module, args: [AztecConfig]) -> Quoted { // We generate ABI exports for all the external functions in the contract. let fn_abi_exports = create_fn_abi_exports(m); - // We generate `_compute_note_hash_and_nullifier`, `sync_state` and `process_message` functions only if they are - // not already implemented. If they are implemented we just insert empty quotes. + // We generate `_compute_note_hash`, `_compute_note_nullifier` (and the deprecated + // `_compute_note_hash_and_nullifier` wrapper), `sync_state` and `process_message` functions only if they are not + // already implemented. If they are implemented we just insert empty quotes. let contract_library_method_compute_note_hash_and_nullifier = if !m.functions().any(|f| { + // Note that we don't test for `_compute_note_hash` or `_compute_note_nullifier` in order to make this simpler + // - users must either implement all three or none. + // Down the line we'll remove this check and use `AztecConfig`. f.name() == quote { _compute_note_hash_and_nullifier } }) { - generate_contract_library_method_compute_note_hash_and_nullifier() + generate_contract_library_methods_compute_note_hash_and_nullifier() } else { quote {} }; @@ -220,157 +224,6 @@ comptime fn generate_contract_interface(m: Module) -> Quoted { } } -/// Generates a contract library method called `_compute_note_hash_and_nullifier` which is used for note discovery (to -/// create the `aztec::messages::discovery::ComputeNoteHashAndNullifier` function) and to implement the -/// `compute_note_hash_and_nullifier` unconstrained contract function. -comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -> Quoted { - if NOTES.len() > 0 { - // Contracts that do define notes produce an if-else chain where `note_type_id` is matched against the - // `get_note_type_id()` function of each note type that we know of, in order to identify the note type. Once we - // know it we call we correct `unpack` method from the `Packable` trait to obtain the underlying note type, and - // compute the note hash (non-siloed) and inner nullifier (also non-siloed). - - let mut if_note_type_id_match_statements_list = @[]; - for i in 0..NOTES.len() { - let typ = NOTES.get(i); - - let get_note_type_id = get_trait_impl_method( - typ, - quote { crate::note::note_interface::NoteType }, - quote { get_id }, - ); - let unpack = get_trait_impl_method( - typ, - quote { crate::protocol::traits::Packable }, - quote { unpack }, - ); - - let compute_note_hash = get_trait_impl_method( - typ, - quote { crate::note::note_interface::NoteHash }, - quote { compute_note_hash }, - ); - - let compute_nullifier_unconstrained = get_trait_impl_method( - typ, - quote { crate::note::note_interface::NoteHash }, - quote { compute_nullifier_unconstrained }, - ); - - let if_or_else_if = if i == 0 { - quote { if } - } else { - quote { else if } - }; - - if_note_type_id_match_statements_list = if_note_type_id_match_statements_list.push_back( - quote { - $if_or_else_if note_type_id == $get_note_type_id() { - // As an extra safety check we make sure that the packed_note BoundedVec has the expected - // length, since we're about to interpret its raw storage as a fixed-size array by calling the - // unpack function on it. - let expected_len = <$typ as $crate::protocol::traits::Packable>::N; - let actual_len = packed_note.len(); - if actual_len != expected_len { - aztec::protocol::logging::warn_log_format( - "[aztec-nr] Packed note length mismatch for note type id {2}: expected {0} fields, got {1}. Skipping note.", - [expected_len as Field, actual_len as Field, note_type_id], - ); - Option::none() - } else { - let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); - - let note_hash = $compute_note_hash(note, owner, storage_slot, randomness); - - // The message discovery process finds settled notes, that is, notes that were created in - // prior transactions and are therefore already part of the note hash tree. We therefore - // compute the nullification note hash by treating the note as a settled note with the - // provided note nonce. - let note_hash_for_nullification = - aztec::note::utils::compute_note_hash_for_nullification( - aztec::note::HintedNote { - note, - contract_address, - owner, - randomness, - storage_slot, - metadata: - aztec::note::note_metadata::SettledNoteMetadata::new( - note_nonce, - ) - .into(), - }, - ); - - let inner_nullifier = $compute_nullifier_unconstrained( - note, - owner, - note_hash_for_nullification, - ); - - Option::some( - aztec::messages::discovery::NoteHashAndNullifier { - note_hash, - inner_nullifier, - }, - ) - } - } - }, - ); - } - - let if_note_type_id_match_statements = if_note_type_id_match_statements_list.join(quote {}); - - quote { - /// Unpacks an array into a note corresponding to `note_type_id` and then computes its note hash (non-siloed) and inner nullifier (non-siloed) assuming the note has been inserted into the note hash tree with `note_nonce`. - /// - /// The signature of this function notably matches the `aztec::messages::discovery::ComputeNoteHashAndNullifier` type, and so it can be used to call functions from that module such as `do_sync_state` and `attempt_note_discovery`. - /// - /// This function is automatically injected by the `#[aztec]` macro. - #[contract_library_method] - unconstrained fn _compute_note_hash_and_nullifier( - packed_note: BoundedVec, - owner: aztec::protocol::address::AztecAddress, - storage_slot: Field, - note_type_id: Field, - contract_address: aztec::protocol::address::AztecAddress, - randomness: Field, - note_nonce: Field, - ) -> Option { - $if_note_type_id_match_statements - else { - aztec::protocol::logging::warn_log_format( - "[aztec-nr] Unknown note type id {0}. Skipping note.", - [note_type_id], - ); - Option::none() - } - } - } - } else { - // Contracts with no notes still implement this function to avoid having special-casing, the implementation - // simply throws immediately. - quote { - /// This contract does not use private notes, so this function should never be called as it will unconditionally fail. - /// - /// This function is automatically injected by the `#[aztec]` macro. - #[contract_library_method] - unconstrained fn _compute_note_hash_and_nullifier( - _packed_note: BoundedVec, - _owner: aztec::protocol::address::AztecAddress, - _storage_slot: Field, - _note_type_id: Field, - _contract_address: aztec::protocol::address::AztecAddress, - _randomness: Field, - _nonce: Field, - ) -> Option { - panic(f"This contract does not use private notes") - } - } - } -} - /// Generates the `sync_state` utility function that performs message discovery. comptime fn generate_sync_state(process_custom_message_option: Quoted, offchain_inbox_sync_option: Quoted) -> Quoted { quote { @@ -386,7 +239,8 @@ comptime fn generate_sync_state(process_custom_message_option: Quoted, offchain_ let address = aztec::context::UtilityContext::new().this_address(); aztec::messages::discovery::do_sync_state( address, - _compute_note_hash_and_nullifier, + _compute_note_hash, + _compute_note_nullifier, $process_custom_message_option, $offchain_inbox_sync_option, ); @@ -416,7 +270,8 @@ comptime fn generate_process_message(process_custom_message_option: Quoted) -> Q aztec::messages::discovery::process_message::process_message_ciphertext( address, - _compute_note_hash_and_nullifier, + _compute_note_hash, + _compute_note_nullifier, $process_custom_message_option, message_ciphertext, message_context, diff --git a/noir-projects/aztec-nr/aztec/src/macros/aztec/compute_note_hash_and_nullifier.nr b/noir-projects/aztec-nr/aztec/src/macros/aztec/compute_note_hash_and_nullifier.nr new file mode 100644 index 000000000000..7f8887b04315 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/macros/aztec/compute_note_hash_and_nullifier.nr @@ -0,0 +1,261 @@ +use crate::macros::{notes::NOTES, utils::get_trait_impl_method}; + +/// Generates two contract library methods called `_compute_note_hash` and `_compute_note_nullifier`, plus a +/// (deprecated) wrapper called `_compute_note_hash_and_nullifier`, which are used for note discovery (i.e. these are +/// of the `aztec::messages::discovery::ComputeNoteHash` and `aztec::messages::discovery::ComputeNoteNullifier` types). +pub(crate) comptime fn generate_contract_library_methods_compute_note_hash_and_nullifier() -> Quoted { + let compute_note_hash = generate_contract_library_method_compute_note_hash(); + let compute_note_nullifier = generate_contract_library_method_compute_note_nullifier(); + + quote { + $compute_note_hash + $compute_note_nullifier + + /// Unpacks an array into a note corresponding to `note_type_id` and then computes its note hash (non-siloed) and inner nullifier (non-siloed) assuming the note has been inserted into the note hash tree with `note_nonce`. + /// + /// This function is automatically injected by the `#[aztec]` macro. + #[contract_library_method] + #[allow(dead_code)] + unconstrained fn _compute_note_hash_and_nullifier( + packed_note: BoundedVec, + owner: aztec::protocol::address::AztecAddress, + storage_slot: Field, + note_type_id: Field, + contract_address: aztec::protocol::address::AztecAddress, + randomness: Field, + note_nonce: Field, + ) -> Option { + _compute_note_hash(packed_note, owner, storage_slot, note_type_id, contract_address, randomness).map(|note_hash| { + + let siloed_note_hash = aztec::protocol::hash::compute_siloed_note_hash(contract_address, note_hash); + let unique_note_hash = aztec::protocol::hash::compute_unique_note_hash(note_nonce, siloed_note_hash); + + let inner_nullifier = _compute_note_nullifier(unique_note_hash, packed_note, owner, storage_slot, note_type_id, contract_address, randomness); + + aztec::messages::discovery::NoteHashAndNullifier { + note_hash, + inner_nullifier, + } + }) + } + } +} + +comptime fn generate_contract_library_method_compute_note_hash() -> Quoted { + if NOTES.len() == 0 { + // Contracts with no notes still implement this function to avoid having special-casing, the implementation + // simply throws immediately. + quote { + /// This contract does not use private notes, so this function should never be called as it will unconditionally fail. + /// + /// This function is automatically injected by the `#[aztec]` macro. + #[contract_library_method] + unconstrained fn _compute_note_hash( + _packed_note: BoundedVec, + _owner: aztec::protocol::address::AztecAddress, + _storage_slot: Field, + _note_type_id: Field, + _contract_address: aztec::protocol::address::AztecAddress, + _randomness: Field, + ) -> Option { + panic(f"This contract does not use private notes") + } + } + } else { + // Contracts that do define notes produce an if-else chain where `note_type_id` is matched against the + // `get_note_type_id()` function of each note type that we know of, in order to identify the note type. Once we + // know it we call we correct `unpack` method from the `Packable` trait to obtain the underlying note type, and + // compute the note hash (non-siloed). + + let mut if_note_type_id_match_statements_list = @[]; + for i in 0..NOTES.len() { + let typ = NOTES.get(i); + + let get_note_type_id = get_trait_impl_method( + typ, + quote { crate::note::note_interface::NoteType }, + quote { get_id }, + ); + let unpack = get_trait_impl_method( + typ, + quote { crate::protocol::traits::Packable }, + quote { unpack }, + ); + + let compute_note_hash = get_trait_impl_method( + typ, + quote { crate::note::note_interface::NoteHash }, + quote { compute_note_hash }, + ); + + let if_or_else_if = if i == 0 { + quote { if } + } else { + quote { else if } + }; + + if_note_type_id_match_statements_list = if_note_type_id_match_statements_list.push_back( + quote { + $if_or_else_if note_type_id == $get_note_type_id() { + // As an extra safety check we make sure that the packed_note BoundedVec has the expected + // length, since we're about to interpret its raw storage as a fixed-size array by calling the + // unpack function on it. + let expected_len = <$typ as $crate::protocol::traits::Packable>::N; + let actual_len = packed_note.len(); + if actual_len != expected_len { + aztec::protocol::logging::warn_log_format( + "[aztec-nr] Packed note length mismatch for note type id {2}: expected {0} fields, got {1}. Skipping note.", + [expected_len as Field, actual_len as Field, note_type_id], + ); + Option::none() + } else { + let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); + + Option::some($compute_note_hash(note, owner, storage_slot, randomness)) + } + } + }, + ); + } + + let if_note_type_id_match_statements = if_note_type_id_match_statements_list.join(quote {}); + + quote { + /// Unpacks an array into a note corresponding to `note_type_id` and then computes its note hash (non-siloed). + /// + /// The signature of this function notably matches the `aztec::messages::discovery::ComputeNoteHash` type, and so it can be used to call functions from that module such as `do_sync_state` and `attempt_note_discovery`. + /// + /// This function is automatically injected by the `#[aztec]` macro. + #[contract_library_method] + unconstrained fn _compute_note_hash( + packed_note: BoundedVec, + owner: aztec::protocol::address::AztecAddress, + storage_slot: Field, + note_type_id: Field, + _contract_address: aztec::protocol::address::AztecAddress, + randomness: Field, + ) -> Option { + $if_note_type_id_match_statements + else { + aztec::protocol::logging::warn_log_format( + "[aztec-nr] Unknown note type id {0}. Skipping note.", + [note_type_id], + ); + Option::none() + } + } + } + } +} + +comptime fn generate_contract_library_method_compute_note_nullifier() -> Quoted { + if NOTES.len() == 0 { + // Contracts with no notes still implement this function to avoid having special-casing, the implementation + // simply throws immediately. + quote { + /// This contract does not use private notes, so this function should never be called as it will unconditionally fail. + /// + /// This function is automatically injected by the `#[aztec]` macro. + #[contract_library_method] + unconstrained fn _compute_note_nullifier( + _unique_note_hash: Field, + _packed_note: BoundedVec, + _owner: aztec::protocol::address::AztecAddress, + _storage_slot: Field, + _note_type_id: Field, + _contract_address: aztec::protocol::address::AztecAddress, + _randomness: Field, + ) -> Option { + panic(f"This contract does not use private notes") + } + } + } else { + // Contracts that do define notes produce an if-else chain where `note_type_id` is matched against the + // `get_note_type_id()` function of each note type that we know of, in order to identify the note type. Once we + // know it we call we correct `unpack` method from the `Packable` trait to obtain the underlying note type, and + // compute the inner nullifier (non-siloed). + + let mut if_note_type_id_match_statements_list = @[]; + for i in 0..NOTES.len() { + let typ = NOTES.get(i); + + let get_note_type_id = get_trait_impl_method( + typ, + quote { crate::note::note_interface::NoteType }, + quote { get_id }, + ); + let unpack = get_trait_impl_method( + typ, + quote { crate::protocol::traits::Packable }, + quote { unpack }, + ); + + let compute_nullifier_unconstrained = get_trait_impl_method( + typ, + quote { crate::note::note_interface::NoteHash }, + quote { compute_nullifier_unconstrained }, + ); + + let if_or_else_if = if i == 0 { + quote { if } + } else { + quote { else if } + }; + + if_note_type_id_match_statements_list = if_note_type_id_match_statements_list.push_back( + quote { + $if_or_else_if note_type_id == $get_note_type_id() { + // As an extra safety check we make sure that the packed_note BoundedVec has the expected + // length, since we're about to interpret its raw storage as a fixed-size array by calling the + // unpack function on it. + let expected_len = <$typ as $crate::protocol::traits::Packable>::N; + let actual_len = packed_note.len(); + if actual_len != expected_len { + aztec::protocol::logging::warn_log_format( + "[aztec-nr] Packed note length mismatch for note type id {2}: expected {0} fields, got {1}. Skipping note.", + [expected_len as Field, actual_len as Field, note_type_id], + ); + Option::none() + } else { + let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); + + // The message discovery process finds settled notes, that is, notes that were created in + // prior transactions and are therefore already part of the note hash tree. The note hash + // for nullification is hence the unique note hash. + $compute_nullifier_unconstrained(note, owner, unique_note_hash) + } + } + }, + ); + } + + let if_note_type_id_match_statements = if_note_type_id_match_statements_list.join(quote {}); + + quote { + /// Computes a note's inner nullifier (non-siloed) given its unique note hash, preimage and extra data. + /// + /// The signature of this function notably matches the `aztec::messages::discovery::ComputeNoteNullifier` type, and so it can be used to call functions from that module such as `do_sync_state` and `attempt_note_discovery`. + /// + /// This function is automatically injected by the `#[aztec]` macro. + #[contract_library_method] + unconstrained fn _compute_note_nullifier( + unique_note_hash: Field, + packed_note: BoundedVec, + owner: aztec::protocol::address::AztecAddress, + _storage_slot: Field, + note_type_id: Field, + _contract_address: aztec::protocol::address::AztecAddress, + _randomness: Field, + ) -> Option { + $if_note_type_id_match_statements + else { + aztec::protocol::logging::warn_log_format( + "[aztec-nr] Unknown note type id {0}. Skipping note.", + [note_type_id], + ); + Option::none() + } + } + } + } +} diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr index 0e716f344024..eca4d338ed91 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr @@ -31,15 +31,14 @@ pub struct NoteHashAndNullifier { pub inner_nullifier: Option, } -/// A contract's way of computing note hashes and nullifiers. +/// A contract's way of computing note hashes. /// -/// Each contract in the network is free to compute their note's hash and nullifiers as they see fit - the hash -/// function itself and the nullifier derivation are not enshrined or standardized. Some aztec-nr functions however do -/// need to know the details of this computation (e.g. when finding new notes), which is what this type represents. +/// Each contract in the network is free to compute their note's hash as they see fit - the hash function itself is not +/// enshrined or standardized. Some aztec-nr functions however do need to know the details of this computation (e.g. +/// when finding new notes), which is what this type represents. /// -/// This function takes a note's packed content, storage slot, note type ID, address of the emitting contract, -/// randomness and note nonce, and attempts to compute its inner note hash (not siloed by address nor uniqued by nonce) -/// and inner nullifier (not siloed by address). +/// This function takes a note's packed content, storage slot, note type ID, address of the emitting contract and +/// randomness, and attempts to compute its inner note hash (not siloed by address nor uniqued by nonce). /// /// ## Transient Notes /// @@ -51,31 +50,16 @@ pub struct NoteHashAndNullifier { /// /// The [`[#aztec]`](crate::macros::aztec::aztec) macro automatically creates a correct implementation of this function /// for each contract by inspecting all note types in use and the storage layout. This injected function is a -/// `#[contract_library_method]` called `_compute_note_hash_and_nullifier`, and it looks something like this: +/// `#[contract_library_method]` called `_compute_note_hash`, and it looks something like this: /// /// ```noir -/// |packed_note, owner, storage_slot, note_type_id, contract_address, randomness, note_nonce| { +/// |packed_note, owner, storage_slot, note_type_id, _contract_address, randomness| { /// if note_type_id == MyNoteType::get_id() { /// if packed_note.len() != MY_NOTE_TYPE_SERIALIZATION_LENGTH { /// Option::none() /// } else { /// let note = MyNoteType::unpack(aztec::utils::array::subarray(packed_note.storage(), 0)); -/// -/// let note_hash = note.compute_note_hash(owner, storage_slot, randomness); -/// let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification( -/// HintedNote { -/// note, contract_address, owner, randomness, storage_slot, -/// metadata: SettledNoteMetadata::new(note_nonce).into(), -/// }, -/// ); -/// -/// let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification); -/// -/// Option::some( -/// aztec::messages::discovery::NoteHashAndNullifier { -/// note_hash, inner_nullifier -/// } -/// ) +/// Option::some(note.compute_note_hash(owner, storage_slot, randomness)) /// } /// } else if note_type_id == MyOtherNoteType::get_id() { /// ... // Similar to above but calling MyOtherNoteType::unpack @@ -84,9 +68,30 @@ pub struct NoteHashAndNullifier { /// }; /// } /// ``` -pub type ComputeNoteHashAndNullifier = unconstrained fn[Env](/* packed_note */BoundedVec, /* +pub type ComputeNoteHash = unconstrained fn(/* packed_note */BoundedVec, /* owner */ AztecAddress, /* storage_slot */ Field, /* note_type_id */ Field, /* contract_address */ AztecAddress, /* -randomness */ Field, /* note nonce */ Field) -> Option; +randomness */ Field) -> Option; + +/// A contract's way of computing note nullifiers. +/// +/// Like [`ComputeNoteHash`], each contract is free to derive nullifiers as they see fit. This function takes the +/// unique note hash (used as the note hash for nullification for settled notes), plus the note's packed content and +/// metadata, and attempts to compute the inner nullifier (not siloed by address). +/// +/// ## Automatic Implementation +/// +/// The [`[#aztec]`](crate::macros::aztec::aztec) macro automatically creates a correct implementation of this function +/// for each contract called `_compute_note_nullifier`. It dispatches on `note_type_id` similarly to +/// [`ComputeNoteHash`], then calls the note's +/// [`compute_nullifier_unconstrained`](crate::note::note_interface::NoteHash::compute_nullifier_unconstrained) method. +pub type ComputeNoteNullifier = unconstrained fn(/* unique_note_hash */Field, /* packed_note */ BoundedVec, +/* owner */ AztecAddress, /* storage_slot */ Field, /* note_type_id */ Field, /* contract_address */ AztecAddress, +/* randomness */ Field) -> Option; + +/// Deprecated: use [`ComputeNoteHash`] and [`ComputeNoteNullifier`] instead. +pub type ComputeNoteHashAndNullifier = unconstrained fn[Env](/* packed_note */BoundedVec, +/* owner */ AztecAddress, /* storage_slot */ Field, /* note_type_id */ Field, /* contract_address */ AztecAddress, +/*randomness */ Field, /* note nonce */ Field) -> Option; /// A handler for custom messages. /// @@ -108,9 +113,10 @@ pub type CustomMessageHandler = unconstrained fn[Env]( /// /// The private state will be synchronized up to the block that will be used for private transactions (i.e. the anchor /// block. This will typically be close to the tip of the chain. -pub unconstrained fn do_sync_state( +pub unconstrained fn do_sync_state( contract_address: AztecAddress, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, process_custom_message: Option>, offchain_inbox_sync: Option>, ) { @@ -130,7 +136,8 @@ pub unconstrained fn do_sync_state> = Option::none(); do_sync_state( contract_address, - dummy_compute_nhnn, + dummy_compute_note_hash, + dummy_compute_note_nullifier, no_handler, no_inbox_sync, ); @@ -209,15 +222,26 @@ mod test { }); } - unconstrained fn dummy_compute_nhnn( + unconstrained fn dummy_compute_note_hash( + _packed_note: BoundedVec, + _owner: AztecAddress, + _storage_slot: Field, + _note_type_id: Field, + _contract_address: AztecAddress, + _randomness: Field, + ) -> Option { + Option::none() + } + + unconstrained fn dummy_compute_note_nullifier( + _unique_note_hash: Field, _packed_note: BoundedVec, _owner: AztecAddress, _storage_slot: Field, _note_type_id: Field, _contract_address: AztecAddress, _randomness: Field, - _note_nonce: Field, - ) -> Option { + ) -> Option { Option::none() } } diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index a9aaa6c84928..eab5007b66bb 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -1,6 +1,6 @@ -use crate::messages::{discovery::ComputeNoteHashAndNullifier, logs::note::MAX_NOTE_PACKED_LEN}; +use crate::messages::{discovery::{ComputeNoteHash, ComputeNoteNullifier}, logs::note::MAX_NOTE_PACKED_LEN}; -use crate::logging::aztecnr_debug_log_format; +use crate::logging::{aztecnr_debug_log_format, aztecnr_warn_log_format}; use crate::protocol::{ address::AztecAddress, constants::MAX_NOTE_HASHES_PER_TX, @@ -23,10 +23,11 @@ pub(crate) struct DiscoveredNoteInfo { /// /// Due to how nonces are computed, this function requires knowledge of the transaction in which the note was created, /// more specifically the list of all unique note hashes in it plus the value of its first nullifier. -pub(crate) unconstrained fn attempt_note_nonce_discovery( +pub(crate) unconstrained fn attempt_note_nonce_discovery( unique_note_hashes_in_tx: BoundedVec, first_nullifier_in_tx: Field, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, contract_address: AztecAddress, owner: AztecAddress, storage_slot: Field, @@ -42,67 +43,88 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( [unique_note_hashes_in_tx.len() as Field, contract_address.to_field(), storage_slot], ); - // We need to find nonces (typically just one) that result in a note hash that, once siloed into a unique note - // hash, is one of the note hashes created by the transaction. - // The nonce is meant to be derived from the index of the note hash in the transaction effects array. However, due - // to an issue in the kernels the nonce might actually use any of the possible note hash indices - not necessarily - // the one that corresponds to the note hash. Hence, we need to try them all. - for i in 0..MAX_NOTE_HASHES_PER_TX { - let nonce_for_i = compute_note_hash_nonce(first_nullifier_in_tx, i); - - // Given note nonce, note content and metadata, we can compute the note hash and silo it to check if - // the resulting unique note matches any in the transaction. - // TODO(#11157): handle failed note_hash_and_nullifier computation - let hashes = compute_note_hash_and_nullifier( - packed_note, - owner, - storage_slot, - note_type_id, - contract_address, - randomness, - nonce_for_i, - ) - .expect(f"Failed to compute a note hash for note type {note_type_id}"); - - let siloed_note_hash_for_i = compute_siloed_note_hash(contract_address, hashes.note_hash); - let unique_note_hash_for_i = compute_unique_note_hash(nonce_for_i, siloed_note_hash_for_i); - - let matching_notes = bvec_filter( - unique_note_hashes_in_tx, - |unique_note_hash_in_tx| unique_note_hash_in_tx == unique_note_hash_for_i, + let maybe_note_hash = compute_note_hash( + packed_note, + owner, + storage_slot, + note_type_id, + contract_address, + randomness, + ); + + if maybe_note_hash.is_none() { + aztecnr_warn_log_format!( + "Unable to compute note hash for note of id {0} with packed length {1}, skipping nonce discovery", + )( + [note_type_id, packed_note.len() as Field], ); - if matching_notes.len() > 1 { - let identical_note_hashes = matching_notes.len(); - // Note that we don't actually check that the note hashes array contains unique values, only that the note - // we found is unique. We don't expect for this to ever happen (it'd indicate a malicious node or PXE, - // which - // are both assumed to be cooperative) so testing for it just in case is unnecessary, but we _do_ need to - // handle it if we find a duplicate. - panic( - f"Received {identical_note_hashes} identical note hashes for a transaction - these should all be unique", - ) - } else if matching_notes.len() == 1 { - // Note that while we did check that the note hash is the preimage of a unique note hash, we perform no - // validations on the nullifier - we fundamentally cannot, since only the application knows how to compute - // nullifiers. We simply trust it to have provided the correct one: if it hasn't, then PXE may fail to - // realize that a given note has been nullified already, and calls to the application could result in - // invalid transactions (with duplicate nullifiers). This is not a concern because an application already - // has more direct means of making a call to it fail the transaction. - discovered_notes.push( - DiscoveredNoteInfo { - note_nonce: nonce_for_i, - note_hash: hashes.note_hash, - // TODO: The None case will be handled in a followup PR. - // https://linear.app/aztec-labs/issue/F-265/store-external-notes - inner_nullifier: hashes.inner_nullifier.expect( - f"Failed to compute nullifier for note type {note_type_id}", - ), - }, + } else { + let note_hash = maybe_note_hash.unwrap(); + let siloed_note_hash = compute_siloed_note_hash(contract_address, note_hash); + + // We need to find nonces (typically just one) that result in the siloed note hash that being uniqued into one + // of the transaction's effects. + // The nonce is meant to be derived from the index of the note hash in the transaction effects array. However, + // due to an issue in the kernels the nonce might actually use any of the possible note hash indices - not + // necessarily the one that corresponds to the note hash. Hence, we need to try them all. + for i in 0..MAX_NOTE_HASHES_PER_TX { + let nonce_for_i = compute_note_hash_nonce(first_nullifier_in_tx, i); + let unique_note_hash_for_i = compute_unique_note_hash(nonce_for_i, siloed_note_hash); + + let matching_notes = bvec_filter( + unique_note_hashes_in_tx, + |unique_note_hash_in_tx| unique_note_hash_in_tx == unique_note_hash_for_i, ); + if matching_notes.len() > 1 { + let identical_note_hashes = matching_notes.len(); + // Note that we don't actually check that the note hashes array contains unique values, only that the + // note we found is unique. We don't expect for this to ever happen (it'd indicate a malicious node or + // PXE, which are both assumed to be cooperative) so testing for it just in case is unnecessary, but we + // _do_ need to handle it if we find a duplicate. + panic( + f"Received {identical_note_hashes} identical note hashes for a transaction - these should all be unique", + ) + } else if matching_notes.len() == 1 { + let maybe_inner_nullifier_for_i = compute_note_nullifier( + unique_note_hash_for_i, + packed_note, + owner, + storage_slot, + note_type_id, + contract_address, + randomness, + ); - // We don't exit the loop - it is possible (though rare) for the exact same note content to be present - // multiple times in the same transaction with different nonces. This typically doesn't happen due to notes - // containing random values in order to hide their contents. + if maybe_inner_nullifier_for_i.is_none() { + // TODO: down the line we want to be able to store notes for which we don't know their nullifier, + // e.g. notes that belong to someone that is not us (and for which we therefore don't know their + // associated app-siloed nullifer hiding secret key). + // https://linear.app/aztec-labs/issue/F-265/store-external-notes + aztecnr_warn_log_format!( + "Unable to compute nullifier of unique note {0} with note type id {1} and owner {2}, skipping PXE insertion", + )( + [unique_note_hash_for_i, note_type_id, owner.to_field()], + ); + } else { + // Note that while we did check that the note hash is the preimage of a unique note hash, we + // perform no validations on the nullifier - we fundamentally cannot, since only the application + // knows how to compute nullifiers. We simply trust it to have provided the correct one: if it + // hasn't, then PXE may fail to realize that a given note has been nullified already, and calls to + // the application could result in invalid transactions (with duplicate nullifiers). This is not a + // concern because an application already has more direct means of making a call to it fail the + // transaction. + discovered_notes.push( + DiscoveredNoteInfo { + note_nonce: nonce_for_i, + note_hash, + inner_nullifier: maybe_inner_nullifier_for_i.unwrap(), + }, + ); + } + // We don't exit the loop - it is possible (though rare) for the exact same note content to be present + // multiple times in the same transaction with different nonces. This typically doesn't happen due to + // notes containing random values in order to hide their contents. + } } } @@ -129,9 +151,8 @@ unconstrained fn bvec_filter( mod test { use crate::{ - messages::{discovery::NoteHashAndNullifier, logs::note::MAX_NOTE_PACKED_LEN}, + messages::logs::note::MAX_NOTE_PACKED_LEN, note::{ - HintedNote, note_interface::{NoteHash, NoteType}, note_metadata::SettledNoteMetadata, utils::compute_note_hash_for_nullification, @@ -151,33 +172,35 @@ mod test { // This implementation could be simpler, but this serves as a nice example of the expected flow in a real // implementation, and as a sanity check that the interface is sufficient. - unconstrained fn compute_note_hash_and_nullifier( + + unconstrained fn compute_note_hash( packed_note: BoundedVec, owner: AztecAddress, storage_slot: Field, note_type_id: Field, - contract_address: AztecAddress, + _contract_address: AztecAddress, randomness: Field, - note_nonce: Field, - ) -> Option { - if note_type_id == MockNote::get_id() { + ) -> Option { + if (note_type_id == MockNote::get_id()) & (packed_note.len() == ::N) { let note = MockNote::unpack(array::subarray(packed_note.storage(), 0)); - let note_hash = note.compute_note_hash(owner, storage_slot, randomness); - - let note_hash_for_nullification = compute_note_hash_for_nullification( - HintedNote { - note, - contract_address, - owner, - randomness, - storage_slot, - metadata: SettledNoteMetadata::new(note_nonce).into(), - }, - ); - - let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification); + Option::some(note.compute_note_hash(owner, storage_slot, randomness)) + } else { + Option::none() + } + } - Option::some(NoteHashAndNullifier { note_hash, inner_nullifier }) + unconstrained fn compute_note_nullifier( + unique_note_hash: Field, + packed_note: BoundedVec, + owner: AztecAddress, + _storage_slot: Field, + note_type_id: Field, + _contract_address: AztecAddress, + _randomness: Field, + ) -> Option { + if (note_type_id == MockNote::get_id()) & (packed_note.len() == ::N) { + let note = MockNote::unpack(array::subarray(packed_note.storage(), 0)); + note.compute_nullifier_unconstrained(owner, unique_note_hash) } else { Option::none() } @@ -198,7 +221,8 @@ mod test { let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, @@ -210,22 +234,47 @@ mod test { assert_eq(discovered_notes.len(), 0); } - #[test(should_fail_with = "Failed to compute a note hash")] - unconstrained fn failed_hash_computation() { + #[test] + unconstrained fn failed_hash_computation_is_ignored() { let unique_note_hashes_in_tx = BoundedVec::from_array([random()]); - let packed_note = BoundedVec::new(); - let note_type_id = 0; // This note type id is unknown to compute_note_hash_and_nullifier let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + |_, _, _, _, _, _| Option::none(), + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, RANDOMNESS, - note_type_id, - packed_note, + MockNote::get_id(), + BoundedVec::new(), + ); + + assert_eq(discovered_notes.len(), 0); + } + + #[test] + unconstrained fn failed_nullifier_computation_is_ignored() { + let note_index_in_tx = 2; + let note_and_data = construct_note(VALUE, note_index_in_tx); + + let mut unique_note_hashes_in_tx = BoundedVec::from_array([ + random(), random(), random(), random(), random(), random(), random(), + ]); + unique_note_hashes_in_tx.set(note_index_in_tx, note_and_data.unique_note_hash); + + let discovered_notes = attempt_note_nonce_discovery( + unique_note_hashes_in_tx, + FIRST_NULLIFIER_IN_TX, + compute_note_hash, + |_, _, _, _, _, _, _| Option::none(), + CONTRACT_ADDRESS, + OWNER, + STORAGE_SLOT, + RANDOMNESS, + MockNote::get_id(), + BoundedVec::from_array(note_and_data.note.pack()), ); assert_eq(discovered_notes.len(), 0); @@ -276,7 +325,8 @@ mod test { let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, @@ -315,7 +365,8 @@ mod test { let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, @@ -354,7 +405,8 @@ mod test { let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, @@ -387,7 +439,8 @@ mod test { let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, @@ -421,7 +474,8 @@ mod test { let _ = attempt_note_nonce_discovery( unique_note_hashes_in_tx, FIRST_NULLIFIER_IN_TX, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, CONTRACT_ADDRESS, OWNER, STORAGE_SLOT, diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr index bf31e253f1d9..37d20096f871 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/partial_notes.nr @@ -1,7 +1,7 @@ use crate::{ capsules::CapsuleArray, messages::{ - discovery::{ComputeNoteHashAndNullifier, nonce_discovery::attempt_note_nonce_discovery}, + discovery::{ComputeNoteHash, ComputeNoteNullifier, nonce_discovery::attempt_note_nonce_discovery}, encoding::MAX_MESSAGE_CONTENT_LEN, logs::partial_note::{decode_partial_note_private_message, MAX_PARTIAL_NOTE_PRIVATE_PACKED_LEN}, processing::{ @@ -72,9 +72,10 @@ pub(crate) unconstrained fn process_partial_note_private_msg( /// Searches for logs that would result in the completion of pending partial notes, ultimately resulting in the notes /// being delivered to PXE if completed. -pub(crate) unconstrained fn fetch_and_process_partial_note_completion_logs( +pub(crate) unconstrained fn fetch_and_process_partial_note_completion_logs( contract_address: AztecAddress, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, ) { let pending_partial_notes = CapsuleArray::at( contract_address, @@ -130,7 +131,8 @@ pub(crate) unconstrained fn fetch_and_process_partial_note_completion_logs( let discovered_notes = attempt_note_nonce_discovery( log.unique_note_hashes_in_tx, log.first_nullifier_in_tx, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, contract_address, pending_partial_note.owner, storage_slot, diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr index c7aef71b9688..1070ba4856bf 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_notes.nr @@ -1,19 +1,20 @@ use crate::logging::{aztecnr_debug_log_format, aztecnr_warn_log_format}; use crate::messages::{ - discovery::{ComputeNoteHashAndNullifier, nonce_discovery::attempt_note_nonce_discovery}, + discovery::{ComputeNoteHash, ComputeNoteNullifier, nonce_discovery::attempt_note_nonce_discovery}, encoding::MAX_MESSAGE_CONTENT_LEN, logs::note::{decode_private_note_message, MAX_NOTE_PACKED_LEN}, processing::enqueue_note_for_validation, }; use crate::protocol::{address::AztecAddress, constants::MAX_NOTE_HASHES_PER_TX}; -pub(crate) unconstrained fn process_private_note_msg( +pub(crate) unconstrained fn process_private_note_msg( contract_address: AztecAddress, tx_hash: Field, unique_note_hashes_in_tx: BoundedVec, first_nullifier_in_tx: Field, recipient: AztecAddress, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, msg_metadata: u64, msg_content: BoundedVec, ) { @@ -28,7 +29,8 @@ pub(crate) unconstrained fn process_private_note_msg( unique_note_hashes_in_tx, first_nullifier_in_tx, recipient, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, owner, storage_slot, randomness, @@ -46,13 +48,14 @@ pub(crate) unconstrained fn process_private_note_msg( /// Attempts discovery of a note given information about its contents and the transaction in which it is suspected the /// note was created. -pub unconstrained fn attempt_note_discovery( +pub unconstrained fn attempt_note_discovery( contract_address: AztecAddress, tx_hash: Field, unique_note_hashes_in_tx: BoundedVec, first_nullifier_in_tx: Field, recipient: AztecAddress, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, owner: AztecAddress, storage_slot: Field, randomness: Field, @@ -62,7 +65,8 @@ pub unconstrained fn attempt_note_discovery( let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, first_nullifier_in_tx, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, contract_address, owner, storage_slot, diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr index db54d39ecd65..1becbd92cecd 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/process_message.nr @@ -1,6 +1,6 @@ use crate::messages::{ discovery::{ - ComputeNoteHashAndNullifier, CustomMessageHandler, partial_notes::process_partial_note_private_msg, + ComputeNoteHash, ComputeNoteNullifier, CustomMessageHandler, partial_notes::process_partial_note_private_msg, private_events::process_private_event_msg, private_notes::process_private_note_msg, }, encoding::{decode_message, MESSAGE_CIPHERTEXT_LEN, MESSAGE_PLAINTEXT_LEN}, @@ -18,17 +18,18 @@ use crate::protocol::address::AztecAddress; /// /// Notes result in nonce discovery being performed prior to delivery, which requires knowledge of the transaction hash /// in which the notes would've been created (typically the same transaction in which the log was emitted), along with -/// the list of unique note hashes in said transaction and the `compute_note_hash_and_nullifier` function. Once -/// discovered, the notes are enqueued for validation. +/// the list of unique note hashes in said transaction and the `compute_note_hash` and `compute_note_nullifier` +/// functions. Once discovered, the notes are enqueued for validation. /// /// Partial notes result in a pending partial note entry being stored in a PXE capsule, which will later be retrieved /// to search for the note's completion public log. /// /// Events are processed by computing an event commitment from the serialized event data and its randomness field, then /// enqueueing the event data and commitment for validation. -pub unconstrained fn process_message_ciphertext( +pub unconstrained fn process_message_ciphertext( contract_address: AztecAddress, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, process_custom_message: Option>, message_ciphertext: BoundedVec, message_context: MessageContext, @@ -38,7 +39,8 @@ pub unconstrained fn process_message_ciphertext( +pub(crate) unconstrained fn process_message_plaintext( contract_address: AztecAddress, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, process_custom_message: Option>, message_plaintext: BoundedVec, message_context: MessageContext, @@ -73,7 +76,8 @@ pub(crate) unconstrained fn process_message_plaintext = |packed_note, owner, storage_slot, note_type_id, contract_address, randomness, note_nonce| { + // We also need to provide an implementation for the `compute_note_hash` and `compute_note_nullifier` + // functions, which would typically be created by the macros for the different notes in a given contract. Here + // we build variants specialized for `Note`. + let compute_note_hash: ComputeNoteHash = |packed_note, owner, storage_slot, note_type_id, _contract_address, randomness| { assert_eq(note_type_id, Note::get_id()); assert_eq(packed_note.len(), ::N); let note = Note::unpack(subarray(packed_note.storage(), 0)); - let note_hash = note.compute_note_hash(owner, storage_slot, randomness); - let note_hash_for_nullification = compute_note_hash_for_nullification( - HintedNote { - note, - contract_address, - owner, - randomness, - storage_slot, - metadata: SettledNoteMetadata::new(note_nonce).into(), - }, - ); + Option::some(note.compute_note_hash(owner, storage_slot, randomness)) + }; - let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification); + let compute_note_nullifier: ComputeNoteNullifier = |unique_note_hash, packed_note, owner, _storage_slot, note_type_id, _contract_address, _randomness| { + assert_eq(note_type_id, Note::get_id()); + assert_eq(packed_note.len(), ::N); - Option::some(NoteHashAndNullifier { note_hash, inner_nullifier }) + let note = Note::unpack(subarray(packed_note.storage(), 0)); + + note.compute_nullifier_unconstrained(owner, unique_note_hash) }; let process_custom_message: Option> = Option::none(); @@ -814,7 +802,8 @@ impl TestEnvironment { message_plaintext, opts.contract_address, Option::some(note_message.new_note.owner), - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, process_custom_message, ); } @@ -907,12 +896,18 @@ impl TestEnvironment { event_message.new_event.randomness, )); - // We also need to provide an implementation for the `compute_note_hash_and_nullifier` function, which would - // typically be created by the macros for the different notes in a given contract. Here we build an empty one, - // since it will never be invoked as this is an event and not a note message. - let compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier<_> = |_packed_note, _owner, _storage_slot, _note_type_id, _contract_address, _randomness, _note_nonce| { + // We also need to provide an implementation for the `compute_note_hash` and `compute_note_nullifier` + // functions, which would typically be created by the macros for the different notes in a given contract. Here + // we build empty ones, since they will never be invoked as this is an event and not a note message. + let compute_note_hash: ComputeNoteHash = |_packed_note, _owner, _storage_slot, _note_type_id, _contract_address, _randomness| { + panic( + f"Unexpected compute_note_hash invocation in TestEnvironment::discover_event", + ) + }; + + let compute_note_nullifier: ComputeNoteNullifier = |_unique_note_hash, _packed_note, _owner, _storage_slot, _note_type_id, _contract_address, _randomness| { panic( - f"Unexpected compute_note_hash_and_nullifier invocation in TestEnvironment::discover_event", + f"Unexpected compute_note_nullifier invocation in TestEnvironment::discover_event", ) }; @@ -921,17 +916,19 @@ impl TestEnvironment { message_plaintext, opts.contract_address, Option::some(recipient), - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, process_custom_message, ); } - unconstrained fn discover_data_in_message_plaintext( + unconstrained fn discover_data_in_message_plaintext( self, message_plaintext: BoundedVec, contract_address: Option, recipient: Option, - compute_note_hash_and_nullifier: ComputeNoteHashAndNullifier, + compute_note_hash: ComputeNoteHash, + compute_note_nullifier: ComputeNoteNullifier, process_custom_message: Option>, ) { // This function will emulate the message discovery and processing that would happen in a real contract, based @@ -960,7 +957,8 @@ impl TestEnvironment { self.utility_context_opts(UtilityContextOptions { contract_address }, |context| { process_message_plaintext( context.this_address(), - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, process_custom_message, message_plaintext, message_context, diff --git a/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/main.nr index e2b5cd42dfe9..15d680e0909c 100644 --- a/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/main.nr @@ -292,7 +292,8 @@ pub contract TokenBlacklist { unique_note_hashes_in_tx, first_nullifier_in_tx, recipient, - _compute_note_hash_and_nullifier, + _compute_note_hash, + _compute_note_nullifier, AztecAddress::zero(), storage_slot, TRANSPARENT_NOTE_RANDOMNESS, diff --git a/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr index f077b11497ae..abb31baf8d26 100644 --- a/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr @@ -3,7 +3,8 @@ mod test; use aztec::macros::aztec; -/// A minimal contract used to test the macro-generated `_compute_note_hash_and_nullifier` function. +/// A minimal contract used to test the macro-generated `_compute_note_hash`, `_compute_note_nullifier` and +/// (deprecated) `_compute_note_hash_and_nullifier` functions. #[aztec] pub contract NoteHashAndNullifier { use aztec::{ diff --git a/yarn-project/aztec.js/src/authorization/call_authorization_request.ts b/yarn-project/aztec.js/src/authorization/call_authorization_request.ts index 89e6f06b7bf8..d90881a580a7 100644 --- a/yarn-project/aztec.js/src/authorization/call_authorization_request.ts +++ b/yarn-project/aztec.js/src/authorization/call_authorization_request.ts @@ -1,14 +1,16 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; import { FieldReader } from '@aztec/foundation/serialize'; import { AuthorizationSelector, FunctionSelector } from '@aztec/stdlib/abi'; +import { computeInnerAuthWitHash } from '@aztec/stdlib/auth-witness'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; +import { computeVarArgsHash } from '@aztec/stdlib/hash'; /** * An authwit request for a function call. Includes the preimage of the data * to be signed, as opposed of just the inner hash. */ export class CallAuthorizationRequest { - constructor( + private constructor( /** * The selector of the authwit type, used to identify it * when emitted from `emit_offchain_effect`oracle. @@ -38,6 +40,26 @@ export class CallAuthorizationRequest { public args: Fr[], ) {} + /** Validates that innerHash and argsHash are consistent with the provided preimage fields. */ + private async validate(): Promise { + const expectedArgsHash = await computeVarArgsHash(this.args); + if (!expectedArgsHash.equals(this.argsHash)) { + throw new Error( + `CallAuthorizationRequest argsHash mismatch: expected ${expectedArgsHash.toString()}, got ${this.argsHash.toString()}`, + ); + } + const expectedInnerHash = await computeInnerAuthWitHash([ + this.msgSender.toField(), + this.functionSelector.toField(), + this.argsHash, + ]); + if (!expectedInnerHash.equals(this.innerHash)) { + throw new Error( + `CallAuthorizationRequest innerHash mismatch: expected ${expectedInnerHash.toString()}, got ${this.innerHash.toString()}`, + ); + } + } + static getSelector(): Promise { return AuthorizationSelector.fromSignature('CallAuthorization((Field),(u32),Field)'); } @@ -51,7 +73,7 @@ export class CallAuthorizationRequest { `Invalid authorization selector for CallAuthwit: expected ${expectedSelector.toString()}, got ${selector.toString()}`, ); } - return new CallAuthorizationRequest( + const request = new CallAuthorizationRequest( selector, reader.readField(), AztecAddress.fromField(reader.readField()), @@ -59,5 +81,7 @@ export class CallAuthorizationRequest { reader.readField(), reader.readFieldArray(reader.remainingFields()), ); + await request.validate(); + return request; } } diff --git a/yarn-project/cli-wallet/src/utils/wallet.ts b/yarn-project/cli-wallet/src/utils/wallet.ts index a1493d0cc13c..3cdb6f9fead1 100644 --- a/yarn-project/cli-wallet/src/utils/wallet.ts +++ b/yarn-project/cli-wallet/src/utils/wallet.ts @@ -13,16 +13,16 @@ import { AccountManager, type Aliased, type SimulateOptions } from '@aztec/aztec import type { DefaultAccountEntrypointOptions } from '@aztec/entrypoints/account'; import { Fr } from '@aztec/foundation/curves/bn254'; import type { LogFn } from '@aztec/foundation/log'; -import type { AccessScopes, NotesFilter } from '@aztec/pxe/client/lazy'; +import type { NotesFilter } from '@aztec/pxe/client/lazy'; import type { PXEConfig } from '@aztec/pxe/config'; import type { PXE } from '@aztec/pxe/server'; import { createPXE, getPXEConfig } from '@aztec/pxe/server'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; import { deriveSigningKey } from '@aztec/stdlib/keys'; import { NoteDao } from '@aztec/stdlib/note'; -import type { TxProvingResult, TxSimulationResult } from '@aztec/stdlib/tx'; +import type { SimulationOverrides, TxProvingResult, TxSimulationResult } from '@aztec/stdlib/tx'; import { ExecutionPayload, mergeExecutionPayloads } from '@aztec/stdlib/tx'; -import { BaseWallet, type FeeOptions } from '@aztec/wallet-sdk/base-wallet'; +import { BaseWallet, type SimulateViaEntrypointOptions } from '@aztec/wallet-sdk/base-wallet'; import type { WalletDB } from '../storage/wallet_db.js'; import type { AccountType } from './constants.js'; @@ -224,21 +224,19 @@ export class CLIWallet extends BaseWallet { */ protected override async simulateViaEntrypoint( executionPayload: ExecutionPayload, - from: AztecAddress, - feeOptions: FeeOptions, - scopes: AccessScopes, - skipTxValidation?: boolean, - skipFeeEnforcement?: boolean, + opts: SimulateViaEntrypointOptions, ): Promise { - if (from.equals(AztecAddress.ZERO)) { - return super.simulateViaEntrypoint( - executionPayload, - from, - feeOptions, - scopes, - skipTxValidation, - skipFeeEnforcement, - ); + const { from, feeOptions, scopes } = opts; + let overrides: SimulationOverrides | undefined; + let fromAccount: Account; + if (!from.equals(AztecAddress.ZERO)) { + const { account, instance, artifact } = await this.getFakeAccountDataFor(from); + fromAccount = account; + overrides = { + contracts: { [from.toString()]: { instance, artifact } }, + }; + } else { + fromAccount = await this.getAccountFromAddress(from); } const feeExecutionPayload = await feeOptions.walletFeePaymentMethod?.getExecutionPayload(); @@ -251,7 +249,6 @@ export class CLIWallet extends BaseWallet { ? mergeExecutionPayloads([feeExecutionPayload, executionPayload]) : executionPayload; - const { account: fromAccount, instance, artifact } = await this.getFakeAccountDataFor(from); const chainInfo = await this.getChainInfo(); const txRequest = await fromAccount.createTxExecutionRequest( finalExecutionPayload, @@ -263,9 +260,7 @@ export class CLIWallet extends BaseWallet { simulatePublic: true, skipFeeEnforcement: true, skipTxValidation: true, - overrides: { - contracts: { [from.toString()]: { instance, artifact } }, - }, + overrides, scopes, }); } diff --git a/yarn-project/end-to-end/src/e2e_kernelless_simulation.test.ts b/yarn-project/end-to-end/src/e2e_kernelless_simulation.test.ts index 4cdd919dacad..683bcbb3fb32 100644 --- a/yarn-project/end-to-end/src/e2e_kernelless_simulation.test.ts +++ b/yarn-project/end-to-end/src/e2e_kernelless_simulation.test.ts @@ -9,6 +9,8 @@ import { PendingNoteHashesContract } from '@aztec/noir-test-contracts.js/Pending import { type AbiDecoded, decodeFromAbi, getFunctionArtifact } from '@aztec/stdlib/abi'; import { computeOuterAuthWitHash } from '@aztec/stdlib/auth-witness'; +import { jest } from '@jest/globals'; + import { deployToken, mintTokensToPrivate } from './fixtures/token_utils.js'; import { setup } from './fixtures/utils.js'; import type { TestWallet } from './test-wallet/test_wallet.js'; @@ -108,7 +110,7 @@ describe('Kernelless simulation', () => { nonceForAuthwits, ); - wallet.enableSimulatedSimulations(); + wallet.setSimulationMode('kernelless-override'); const { offchainEffects } = await addLiquidityInteraction.simulate({ from: liquidityProviderAddress, @@ -216,7 +218,7 @@ describe('Kernelless simulation', () => { ).resolves.toBeDefined(); }); - it('produces matching gas estimates between kernelless and with-kernels simulation', async () => { + it('produces matching gas estimates and fee payer between kernelless and with-kernels simulation', async () => { const swapperBalancesBefore = await getWalletBalances(swapperAddress); const ammBalancesBefore = await getAmmBalances(); @@ -236,27 +238,27 @@ describe('Kernelless simulation', () => { nonceForAuthwits, ); - wallet.enableSimulatedSimulations(); - const swapKernellessGas = ( - await swapExactTokensInteraction.simulate({ - from: swapperAddress, - includeMetadata: true, - }) - ).estimatedGas!; + const simulateTxSpy = jest.spyOn(wallet, 'simulateTx'); + + wallet.setSimulationMode('kernelless-override'); + const kernellessResult = await swapExactTokensInteraction.simulate({ + from: swapperAddress, + includeMetadata: true, + }); + const swapKernellessGas = kernellessResult.estimatedGas!; const swapAuthwit = await wallet.createAuthWit(swapperAddress, { caller: amm.address, action: token0.methods.transfer_to_public(swapperAddress, amm.address, amountIn, nonceForAuthwits), }); - wallet.disableSimulatedSimulations(); - const swapWithKernelsGas = ( - await swapExactTokensInteraction.simulate({ - from: swapperAddress, - includeMetadata: true, - authWitnesses: [swapAuthwit], - }) - ).estimatedGas!; + wallet.setSimulationMode('full'); + const withKernelsResult = await swapExactTokensInteraction.simulate({ + from: swapperAddress, + includeMetadata: true, + authWitnesses: [swapAuthwit], + }); + const swapWithKernelsGas = withKernelsResult.estimatedGas!; logger.info(`Kernelless gas: L2=${swapKernellessGas.gasLimits.l2Gas} DA=${swapKernellessGas.gasLimits.daGas}`); logger.info( @@ -265,6 +267,16 @@ describe('Kernelless simulation', () => { expect(swapKernellessGas.gasLimits.daGas).toEqual(swapWithKernelsGas.gasLimits.daGas); expect(swapKernellessGas.gasLimits.l2Gas).toEqual(swapWithKernelsGas.gasLimits.l2Gas); + + expect(simulateTxSpy).toHaveBeenCalledTimes(2); + const kernellessTxResult = await (simulateTxSpy.mock.results[0].value as ReturnType); + const withKernelsTxResult = await (simulateTxSpy.mock.results[1].value as ReturnType); + const kernellessFeePayer = kernellessTxResult.publicInputs.feePayer; + const withKernelsFeePayer = withKernelsTxResult.publicInputs.feePayer; + expect(kernellessFeePayer).toEqual(withKernelsFeePayer); + expect(kernellessFeePayer).toEqual(swapperAddress); + + simulateTxSpy.mockRestore(); }); }); @@ -288,7 +300,7 @@ describe('Kernelless simulation', () => { await pendingNoteHashesContract.methods.get_then_nullify_note.selector(), ); - wallet.enableSimulatedSimulations(); + wallet.setSimulationMode('kernelless-override'); const kernellessGas = ( await interaction.simulate({ from: adminAddress, @@ -296,7 +308,7 @@ describe('Kernelless simulation', () => { }) ).estimatedGas!; - wallet.disableSimulatedSimulations(); + wallet.setSimulationMode('full'); const withKernelsGas = ( await interaction.simulate({ from: adminAddress, @@ -325,14 +337,14 @@ describe('Kernelless simulation', () => { const mintAmount = 100n; // Insert a note with real kernels so it lands on-chain - wallet.disableSimulatedSimulations(); + wallet.setSimulationMode('full'); await pendingNoteHashesContract.methods.insert_note(mintAmount, adminAddress, adminAddress).send({ from: adminAddress, }); // Kernelless simulation of reading + nullifying that settled note produces a settled // read request that gets verified against the note hash tree at the anchor block - wallet.enableSimulatedSimulations(); + wallet.setSimulationMode('kernelless-override'); await expect( pendingNoteHashesContract.methods.get_then_nullify_note(mintAmount, adminAddress).simulate({ from: adminAddress, diff --git a/yarn-project/end-to-end/src/test-wallet/test_wallet.ts b/yarn-project/end-to-end/src/test-wallet/test_wallet.ts index 24dc0adb6c85..e1082cdc1a14 100644 --- a/yarn-project/end-to-end/src/test-wallet/test_wallet.ts +++ b/yarn-project/end-to-end/src/test-wallet/test_wallet.ts @@ -16,7 +16,7 @@ import { AccountManager, type SendOptions } from '@aztec/aztec.js/wallet'; import type { DefaultAccountEntrypointOptions } from '@aztec/entrypoints/account'; import { Fq, Fr } from '@aztec/foundation/curves/bn254'; import { GrumpkinScalar } from '@aztec/foundation/curves/grumpkin'; -import type { AccessScopes, NotesFilter } from '@aztec/pxe/client/lazy'; +import type { NotesFilter } from '@aztec/pxe/client/lazy'; import { type PXEConfig, getPXEConfig } from '@aztec/pxe/config'; import { PXE, type PXECreationOptions, createPXE } from '@aztec/pxe/server'; import { AuthWitness } from '@aztec/stdlib/auth-witness'; @@ -24,9 +24,9 @@ import { AztecAddress } from '@aztec/stdlib/aztec-address'; import { getContractInstanceFromInstantiationParams } from '@aztec/stdlib/contract'; import { deriveSigningKey } from '@aztec/stdlib/keys'; import type { NoteDao } from '@aztec/stdlib/note'; -import type { BlockHeader, TxHash, TxReceipt, TxSimulationResult } from '@aztec/stdlib/tx'; +import type { BlockHeader, SimulationOverrides, TxHash, TxReceipt, TxSimulationResult } from '@aztec/stdlib/tx'; import { ExecutionPayload, mergeExecutionPayloads } from '@aztec/stdlib/tx'; -import { BaseWallet, type FeeOptions } from '@aztec/wallet-sdk/base-wallet'; +import { BaseWallet, type SimulateViaEntrypointOptions } from '@aztec/wallet-sdk/base-wallet'; import { AztecNodeProxy, ProvenTx } from './utils.js'; @@ -125,21 +125,15 @@ export class TestWallet extends BaseWallet { protected accounts: Map = new Map(); /** - * Toggle for running "simulated simulations" when calling simulateTx. - * - * When this flag is true, simulateViaEntrypoint constructs a request using a fake account - * (and accepts contract overrides on the input) and the PXE emulates kernel effects without - * generating kernel witnesses. When false, simulateViaEntrypoint defers to the standard - * simulation path via the real account entrypoint. + * Controls how the test wallet simulates transactions: + * - `kernelless`: Skips kernel circuits but uses the real account contract. Default. + * - `kernelless-override`: Skips kernels and replaces the account with a stub that doesn't do authwit validation. + * - `full`: Uses real kernel circuits and real account contracts. Slow! */ - private simulatedSimulations = false; + private simulationMode: 'kernelless' | 'kernelless-override' | 'full' = 'kernelless'; - enableSimulatedSimulations() { - this.simulatedSimulations = true; - } - - disableSimulatedSimulations() { - this.simulatedSimulations = false; + setSimulationMode(mode: 'kernelless' | 'kernelless-override' | 'full') { + this.simulationMode = mode; } setMinFeePadding(value?: number) { @@ -220,27 +214,24 @@ export class TestWallet extends BaseWallet { return account.createAuthWit(intentInnerHash, chainInfo); } - /** - * Override simulateViaEntrypoint to use fake accounts for kernelless simulation - * when simulatedSimulations is enabled. Otherwise falls through to the real entrypoint path. - */ protected override async simulateViaEntrypoint( executionPayload: ExecutionPayload, - from: AztecAddress, - feeOptions: FeeOptions, - scopes: AccessScopes, - skipTxValidation?: boolean, - skipFeeEnforcement?: boolean, + opts: SimulateViaEntrypointOptions, ): Promise { - if (!this.simulatedSimulations) { - return super.simulateViaEntrypoint( - executionPayload, - from, - feeOptions, - scopes, - skipTxValidation, - skipFeeEnforcement, - ); + const { from, feeOptions, scopes, skipTxValidation, skipFeeEnforcement } = opts; + const skipKernels = this.simulationMode !== 'full'; + const useOverride = this.simulationMode === 'kernelless-override' && !from.equals(AztecAddress.ZERO); + + let overrides: SimulationOverrides | undefined; + let fromAccount: Account; + if (useOverride) { + const { account, instance, artifact } = await this.getFakeAccountDataFor(from); + fromAccount = account; + overrides = { + contracts: { [from.toString()]: { instance, artifact } }, + }; + } else { + fromAccount = await this.getAccountFromAddress(from); } const feeExecutionPayload = await feeOptions.walletFeePaymentMethod?.getExecutionPayload(); @@ -252,7 +243,6 @@ export class TestWallet extends BaseWallet { const finalExecutionPayload = feeExecutionPayload ? mergeExecutionPayloads([feeExecutionPayload, executionPayload]) : executionPayload; - const { account: fromAccount, instance, artifact } = await this.getFakeAccountDataFor(from); const chainInfo = await this.getChainInfo(); const txRequest = await fromAccount.createTxExecutionRequest( finalExecutionPayload, @@ -260,14 +250,12 @@ export class TestWallet extends BaseWallet { chainInfo, executionOptions, ); - const contractOverrides = { - [from.toString()]: { instance, artifact }, - }; return this.pxe.simulateTx(txRequest, { simulatePublic: true, - skipFeeEnforcement: true, - skipTxValidation: true, - overrides: { contracts: contractOverrides }, + skipKernels, + skipFeeEnforcement, + skipTxValidation, + overrides, scopes, }); } diff --git a/yarn-project/pxe/src/contract_function_simulator/contract_function_simulator.ts b/yarn-project/pxe/src/contract_function_simulator/contract_function_simulator.ts index 6b1bd80e6baf..046a2ccb9f30 100644 --- a/yarn-project/pxe/src/contract_function_simulator/contract_function_simulator.ts +++ b/yarn-project/pxe/src/contract_function_simulator/contract_function_simulator.ts @@ -448,6 +448,8 @@ export async function generateSimulatedProvingResult( privateExecutionResult.entrypoint.publicInputs.anchorBlockHeader.globalVariables.timestamp + BigInt(MAX_TX_LIFETIME); + let feePayer = AztecAddress.zero(); + const executions = [privateExecutionResult.entrypoint]; while (executions.length !== 0) { @@ -462,6 +464,13 @@ export async function generateSimulatedProvingResult( const { contractAddress } = execution.publicInputs.callContext; + if (execution.publicInputs.isFeePayer) { + if (!feePayer.isZero()) { + throw new Error('Multiple fee payers found in private execution result'); + } + feePayer = contractAddress; + } + scopedNoteHashes.push( ...execution.publicInputs.noteHashes .getActiveItems() @@ -682,7 +691,7 @@ export async function generateSimulatedProvingResult( daGas: TX_DA_GAS_OVERHEAD, }), ), - /*feePayer=*/ AztecAddress.zero(), + /*feePayer=*/ feePayer, /*expirationTimestamp=*/ expirationTimestamp, hasPublicCalls ? inputsForPublic : undefined, !hasPublicCalls ? inputsForRollup : undefined, diff --git a/yarn-project/wallet-sdk/src/base-wallet/base_wallet.ts b/yarn-project/wallet-sdk/src/base-wallet/base_wallet.ts index 9c2a74273705..762d520c156e 100644 --- a/yarn-project/wallet-sdk/src/base-wallet/base_wallet.ts +++ b/yarn-project/wallet-sdk/src/base-wallet/base_wallet.ts @@ -80,6 +80,16 @@ export type FeeOptions = { gasSettings: GasSettings; }; +/** Options for `simulateViaEntrypoint`. */ +export type SimulateViaEntrypointOptions = Pick< + SimulateOptions, + 'from' | 'additionalScopes' | 'skipTxValidation' | 'skipFeeEnforcement' +> & { + /** Fee options for the entrypoint */ + feeOptions: FeeOptions; + /** Scopes to use for the simulation */ + scopes: AccessScopes; +}; /** * A base class for Wallet implementations */ @@ -300,22 +310,20 @@ export abstract class BaseWallet implements Wallet { /** * Simulates calls through the standard PXE path (account entrypoint). * @param executionPayload - The execution payload to simulate. - * @param from - The sender address. - * @param feeOptions - Fee options for the transaction. - * @param skipTxValidation - Whether to skip tx validation. - * @param skipFeeEnforcement - Whether to skip fee enforcement. - * @param scopes - The scopes to use for the simulation. + * @param opts - Simulation options. */ - protected async simulateViaEntrypoint( - executionPayload: ExecutionPayload, - from: AztecAddress, - feeOptions: FeeOptions, - scopes: AccessScopes, - skipTxValidation?: boolean, - skipFeeEnforcement?: boolean, - ) { - const txRequest = await this.createTxExecutionRequestFromPayloadAndFee(executionPayload, from, feeOptions); - return this.pxe.simulateTx(txRequest, { simulatePublic: true, skipTxValidation, skipFeeEnforcement, scopes }); + protected async simulateViaEntrypoint(executionPayload: ExecutionPayload, opts: SimulateViaEntrypointOptions) { + const txRequest = await this.createTxExecutionRequestFromPayloadAndFee( + executionPayload, + opts.from, + opts.feeOptions, + ); + return this.pxe.simulateTx(txRequest, { + simulatePublic: true, + skipTxValidation: opts.skipTxValidation, + skipFeeEnforcement: opts.skipFeeEnforcement, + scopes: opts.scopes, + }); } /** @@ -357,14 +365,13 @@ export abstract class BaseWallet implements Wallet { ) : Promise.resolve([]), remainingCalls.length > 0 - ? this.simulateViaEntrypoint( - remainingPayload, - opts.from, + ? this.simulateViaEntrypoint(remainingPayload, { + from: opts.from, feeOptions, - this.scopesFrom(opts.from, opts.additionalScopes), - opts.skipTxValidation, - opts.skipFeeEnforcement ?? true, - ) + scopes: this.scopesFrom(opts.from, opts.additionalScopes), + skipTxValidation: opts.skipTxValidation, + skipFeeEnforcement: opts.skipFeeEnforcement ?? true, + }) : Promise.resolve(null), ]); diff --git a/yarn-project/wallet-sdk/src/base-wallet/index.ts b/yarn-project/wallet-sdk/src/base-wallet/index.ts index 4b6718cae7ae..224088457a7c 100644 --- a/yarn-project/wallet-sdk/src/base-wallet/index.ts +++ b/yarn-project/wallet-sdk/src/base-wallet/index.ts @@ -1,2 +1,2 @@ -export { BaseWallet, type FeeOptions } from './base_wallet.js'; +export { BaseWallet, type FeeOptions, type SimulateViaEntrypointOptions } from './base_wallet.js'; export { simulateViaNode, buildMergedSimulationResult, extractOptimizablePublicStaticCalls } from './utils.js'; diff --git a/yarn-project/wallets/src/embedded/embedded_wallet.ts b/yarn-project/wallets/src/embedded/embedded_wallet.ts index df2a73b9ecf1..dbc63b3deef8 100644 --- a/yarn-project/wallets/src/embedded/embedded_wallet.ts +++ b/yarn-project/wallets/src/embedded/embedded_wallet.ts @@ -1,22 +1,26 @@ import { type Account, SignerlessAccount } from '@aztec/aztec.js/account'; -import type { Aliased } from '@aztec/aztec.js/wallet'; +import { CallAuthorizationRequest } from '@aztec/aztec.js/authorization'; +import { type InteractionWaitOptions, type SendReturn, getGasLimits } from '@aztec/aztec.js/contracts'; +import type { Aliased, SendOptions } from '@aztec/aztec.js/wallet'; import { AccountManager } from '@aztec/aztec.js/wallet'; import type { DefaultAccountEntrypointOptions } from '@aztec/entrypoints/account'; import { Fq, Fr } from '@aztec/foundation/curves/bn254'; import type { Logger } from '@aztec/foundation/log'; -import type { AccessScopes, PXEConfig, PXECreationOptions } from '@aztec/pxe/client/lazy'; +import type { PXEConfig, PXECreationOptions } from '@aztec/pxe/client/lazy'; import type { PXE } from '@aztec/pxe/server'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; import { getContractInstanceFromInstantiationParams } from '@aztec/stdlib/contract'; +import { GasSettings } from '@aztec/stdlib/gas'; import type { AztecNode } from '@aztec/stdlib/interfaces/client'; import { deriveSigningKey } from '@aztec/stdlib/keys'; import { ExecutionPayload, SimulationOverrides, type TxSimulationResult, + collectOffchainEffects, mergeExecutionPayloads, } from '@aztec/stdlib/tx'; -import { BaseWallet, type FeeOptions } from '@aztec/wallet-sdk/base-wallet'; +import { BaseWallet, type SimulateViaEntrypointOptions } from '@aztec/wallet-sdk/base-wallet'; import type { AccountContractsProvider } from './account-contract-providers/types.js'; import { type AccountType, WalletDB } from './wallet_db.js'; @@ -32,7 +36,11 @@ export type EmbeddedWalletOptions = { pxeOptions?: PXECreationOptions; }; +const DEFAULT_ESTIMATED_GAS_PADDING = 0.1; + export class EmbeddedWallet extends BaseWallet { + protected estimatedGasPadding = DEFAULT_ESTIMATED_GAS_PADDING; + constructor( pxe: PXE, aztecNode: AztecNode, @@ -79,6 +87,66 @@ export class EmbeddedWallet extends BaseWallet { return storedSenders; } + /** + * Overrides the base sendTx to add a pre-simulation step before the actual send. The simulation + * estimates actual gas usage and captures call authorization requests to generate + * the necessary authwitnesses. + */ + public override async sendTx( + executionPayload: ExecutionPayload, + opts: SendOptions, + ): Promise> { + const feeOptions = await this.completeFeeOptionsForEstimation( + opts.from, + executionPayload.feePayer, + opts.fee?.gasSettings, + ); + + // Simulate the transaction first to estimate gas and capture required + // private authwitnesses based on offchain effects. + const simulationResult = await this.simulateViaEntrypoint(executionPayload, { + from: opts.from, + feeOptions, + scopes: this.scopesFrom(opts.from, opts.additionalScopes), + skipTxValidation: true, + }); + + const offchainEffects = collectOffchainEffects(simulationResult.privateExecutionResult); + const authWitnesses = await Promise.all( + offchainEffects.map(async effect => { + try { + const authRequest = await CallAuthorizationRequest.fromFields(effect.data); + return this.createAuthWit(opts.from, { + consumer: effect.contractAddress, + innerHash: authRequest.innerHash, + }); + } catch { + return undefined; + } + }), + ); + for (const authwit of authWitnesses) { + if (authwit) { + executionPayload.authWitnesses.push(authwit); + } + } + const estimated = getGasLimits(simulationResult, this.estimatedGasPadding); + this.log.verbose( + `Estimated gas limits for tx: DA=${estimated.gasLimits.daGas} L2=${estimated.gasLimits.l2Gas} teardownDA=${estimated.teardownGasLimits.daGas} teardownL2=${estimated.teardownGasLimits.l2Gas}`, + ); + const gasSettings = GasSettings.from({ + ...opts.fee?.gasSettings, + maxFeesPerGas: feeOptions.gasSettings.maxFeesPerGas, + maxPriorityFeesPerGas: feeOptions.gasSettings.maxPriorityFeesPerGas, + gasLimits: opts.fee?.gasSettings?.gasLimits ?? estimated.gasLimits, + teardownGasLimits: opts.fee?.gasSettings?.teardownGasLimits ?? estimated.teardownGasLimits, + }); + return super.sendTx(executionPayload, { + ...opts, + fee: { ...opts.fee, gasSettings }, + }); + } + /** * Simulates calls via a stub account entrypoint, bypassing real account authorization. * This allows kernelless simulation with contract overrides, skipping expensive @@ -86,12 +154,10 @@ export class EmbeddedWallet extends BaseWallet { */ protected override async simulateViaEntrypoint( executionPayload: ExecutionPayload, - from: AztecAddress, - feeOptions: FeeOptions, - scopes: AccessScopes, - skipTxValidation?: boolean, - skipFeeEnforcement?: boolean, + opts: SimulateViaEntrypointOptions, ): Promise { + const { from, feeOptions, scopes, skipTxValidation, skipFeeEnforcement } = opts; + let overrides: SimulationOverrides | undefined; let fromAccount: Account; if (!from.equals(AztecAddress.ZERO)) { @@ -220,6 +286,10 @@ export class EmbeddedWallet extends BaseWallet { this.minFeePadding = value ?? 0.5; } + setEstimatedGasPadding(value?: number) { + this.estimatedGasPadding = value ?? DEFAULT_ESTIMATED_GAS_PADDING; + } + stop() { return this.pxe.stop(); }