diff --git a/cspell.json b/cspell.json index dc2183f8f52e..0c64bcaf57a8 100644 --- a/cspell.json +++ b/cspell.json @@ -235,7 +235,6 @@ "noirup", "allow_phase_change", "nophasecheck", - "nullifer", "Nullifiable", "offchain", "onchain", @@ -367,6 +366,7 @@ "unfinalized", "uniquified", "uniquify", + "unlinkability", "unkonstrained", "unnullify", "unpadded", diff --git a/docs/developer_versioned_docs/version-v3.0.0-devnet.6-patch.1/docs/resources/migration_notes.md b/docs/developer_versioned_docs/version-v3.0.0-devnet.6-patch.1/docs/resources/migration_notes.md index 1e3368250ecd..8a7933dfcebe 100644 --- a/docs/developer_versioned_docs/version-v3.0.0-devnet.6-patch.1/docs/resources/migration_notes.md +++ b/docs/developer_versioned_docs/version-v3.0.0-devnet.6-patch.1/docs/resources/migration_notes.md @@ -1987,7 +1987,7 @@ Doing the changes is as straightforward as: `UintNote` has also been updated to use the native `u128` type. -### [aztec-nr] Removed `compute_note_hash_and_optionally_a_nullifer` +### [aztec-nr] Removed `compute_note_hash_and_optionally_a_nullifier` This function is no longer mandatory for contracts, and the `#[aztec]` macro no longer injects it. diff --git a/docs/developer_versioned_docs/version-v4.0.0-nightly.20260217/docs/resources/migration_notes.md b/docs/developer_versioned_docs/version-v4.0.0-nightly.20260217/docs/resources/migration_notes.md index 474d7dc366e3..866c3970fa2f 100644 --- a/docs/developer_versioned_docs/version-v4.0.0-nightly.20260217/docs/resources/migration_notes.md +++ b/docs/developer_versioned_docs/version-v4.0.0-nightly.20260217/docs/resources/migration_notes.md @@ -2684,7 +2684,7 @@ Doing the changes is as straightforward as: `UintNote` has also been updated to use the native `u128` type. -### [aztec-nr] Removed `compute_note_hash_and_optionally_a_nullifer` +### [aztec-nr] Removed `compute_note_hash_and_optionally_a_nullifier` This function is no longer mandatory for contracts, and the `#[aztec]` macro no longer injects it. diff --git a/docs/docs-developers/docs/resources/migration_notes.md b/docs/docs-developers/docs/resources/migration_notes.md index b7d1acf80b68..53043f308558 100644 --- a/docs/docs-developers/docs/resources/migration_notes.md +++ b/docs/docs-developers/docs/resources/migration_notes.md @@ -9,6 +9,75 @@ 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. + +Note that `Wallet.getContractMetadata` now returns `isContractInitialized: undefined` when the wallet does not have the contract instance registered, since `init_hash` is needed to compute the nullifier and initialization status cannot be determined. Previously, this check worked for any address. Callers should check for `undefined` before branching on the boolean value. + +If you use `assert_contract_was_initialized_by` or `assert_contract_was_not_initialized_by` from `aztec::history::deployment`, these now require an additional `init_hash: Field` parameter: + +```diff ++ let instance = get_contract_instance(contract_address); + assert_contract_was_initialized_by( + block_header, + contract_address, ++ instance.initialization_hash, + ); +``` + +### [Aztec.js] `TxReceipt` now includes `epochNumber` + +`TxReceipt` now includes an `epochNumber` field that indicates which epoch the transaction was included in. + +### [Aztec.js] `computeL2ToL1MembershipWitness` signature changed + +The function signature has changed to resolve the epoch internally from a transaction hash, rather than requiring the caller to pass the epoch number. + +**Migration:** + +```diff +- const witness = await computeL2ToL1MembershipWitness(aztecNode, epochNumber, messageHash); +- // epoch was passed in by the caller ++ const witness = await computeL2ToL1MembershipWitness(aztecNode, messageHash, txHash); ++ // epoch is now available on the returned witness ++ const epoch = witness.epochNumber; +``` + +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: @@ -56,6 +125,16 @@ The return type `L2ToL1MembershipWitness` now includes `epochNumber`. An optiona **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. +### [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. @@ -122,9 +201,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, + ); } ``` @@ -179,6 +263,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`. @@ -238,6 +323,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`. @@ -263,7 +349,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`)** @@ -277,7 +363,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** @@ -324,22 +410,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 @@ -348,12 +438,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: @@ -2984,7 +3075,7 @@ Doing the changes is as straightforward as: `UintNote` has also been updated to use the native `u128` type. -### [aztec-nr] Removed `compute_note_hash_and_optionally_a_nullifer` +### [aztec-nr] Removed `compute_note_hash_and_optionally_a_nullifier` This function is no longer mandatory for contracts, and the `#[aztec]` macro no longer injects it. diff --git a/noir-projects/aztec-nr/aztec/src/capsules/mod.nr b/noir-projects/aztec-nr/aztec/src/capsules/mod.nr index 6fa92e11f23a..ec8af10da3b2 100644 --- a/noir-projects/aztec-nr/aztec/src/capsules/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/capsules/mod.nr @@ -76,8 +76,12 @@ impl CapsuleArray { capsules::store(self.contract_address, self.base_slot, current_length - 1); } - /// Iterates over the entire array, calling the callback with all values and their array index. The order in which - /// values are processed is arbitrary. + /// Calls a function on each element of the array. + /// + /// The function `f` is called once with each array value and its corresponding index. The order in which values + /// are processed is arbitrary. + /// + /// ## Array Mutation /// /// It is safe to delete the current element (and only the current element) from inside the callback via `remove`: /// ```noir diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 4f278b5fa62c..0232e747f925 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -349,71 +349,56 @@ impl PrivateContext { self.note_hashes.push(Counted::new(note_hash, self.next_counter())); } - /// Pushes a new nullifier to the Aztec blockchain's global Nullifier Tree (a state tree). + /// Creates a new [nullifier](crate::nullifier). /// - /// See also: `push_nullifier_for_note_hash`. + /// ## Safety /// - /// Low-level function: Ordinarily, smart contract developers will not need to manually call this. Aztec-nr's state - /// variables (see `../state_vars/`) are designed to understand when to create and push new nullifiers. - /// - /// A nullifier can only be emitted once. Duplicate nullifier insertions are rejected by the protocol. - /// - /// Generally, a nullifier is emitted to prevent an action from happening more than once, in such a way that the - /// action cannot be linked (by an observer of the blockchain) to any earlier transactions. - /// - /// I.e. a nullifier is a random-looking, but deterministic record of a private, one-time action, which does not - /// leak what action has been taken, and which preserves the property of "tx unlinkability". + /// This is a low-level function that must be used with great care to avoid subtle corruption of contract state. + /// Instead of calling this function, consider using the higher-level [`crate::state_vars::SingleUseClaim`]. /// - /// Usually, a nullifier will be emitted to "spend" a note (a piece of private state), without revealing which - /// specific note is being spent. + /// In particular, callers must ensure all nullifiers created by a contract are properly domain-separated, so that + /// unrelated components don't interfere with one another (e.g. a transaction nullifier accidentally marking a + /// variable as initialized). Only [`PrivateContext::push_nullifier_for_note_hash`] should be used for note + /// nullifiers, never this one. /// - /// (Important: in such cases, use the below `push_nullifier_for_note_hash`). - /// - /// Sometimes, a nullifier might be emitted completely unrelated to any notes. Examples include initialization of a - /// new contract; initialization of a PrivateMutable, or signalling in Semaphore-like applications. This - /// `push_nullifier` function serves such use cases. - /// - /// # Arguments - /// * `nullifier` - /// - /// # Advanced - /// From here, the protocol's kernel circuits will take over and insert the nullifier into the protocol's - /// "nullifier tree" (in the Base Rollup circuit). Before insertion, the protocol will: - /// - "Silo" the `nullifier` with the contract address of this function, to yield a `siloed_nullifier`. This - /// prevents state collisions between different smart contracts. - /// - Ensure the `siloed_nullifier` is unique (the nullifier tree is an indexed merkle tree which supports - /// efficient non-membership proofs). + /// ## Advanced /// + /// The raw `nullifier` is not what is inserted into the Aztec state tree: it will be first siloed by contract + /// address via [`crate::protocol::hash::compute_siloed_nullifier`] in order to prevent accidental or malicious + /// interference of nullifiers from different contracts. pub fn push_nullifier(&mut self, nullifier: Field) { notify_created_nullifier(nullifier); self.nullifiers.push(Nullifier { value: nullifier, note_hash: 0 }.count(self.next_counter())); } - /// Pushes a nullifier that corresponds to a specific note hash. + /// Creates a new [nullifier](crate::nullifier) associated with a note. /// - /// Low-level function: Ordinarily, smart contract developers will not need to manually call this. Aztec-nr's state - /// variables (see `../state_vars/`) are designed to understand when to create and push new nullifiers. + /// This is a variant of [`PrivateContext::push_nullifier`] that is used for note nullifiers, i.e. nullifiers that + /// correspond to a note. If a note and its nullifier are created in the same transaction, then the private kernels + /// will 'squash' these values, deleting them both as if they never existed and reducing transaction fees. /// - /// This is a specialized version of `push_nullifier` that links a nullifier to the specific note hash it's - /// nullifying. This is the most common usage pattern for nullifiers. See `push_nullifier` for more explanation on - /// nullifiers. + /// The `nullification_note_hash` must be the result of calling + /// [`crate::note::utils::compute_confirmed_note_hash_for_nullification`] for pending notes, and `0` for settled + /// notes (which cannot be squashed). /// - /// # Arguments - /// * `nullifier` - /// * `nullified_note_hash` - The note hash of the note being nullified + /// ## Safety /// - /// # Advanced - ///Important: usage of this function doesn't mean that the world will _see_ that this nullifier relates to the - /// given nullified_note_hash (as that would violate "tx unlinkability"); it simply informs the user's PXE about - /// the relationship (via `notify_nullified_note`). The PXE can then use this information to feed hints to the - /// kernel circuits for "squashing" purposes: If a note is nullified during the same tx which created it, we can - /// "squash" (delete) the note and nullifier (and any private logs associated with the note), to save on data - /// emission costs. - /// - pub fn push_nullifier_for_note_hash(&mut self, nullifier: Field, nullified_note_hash: Field) { + /// This is a low-level function that must be used with great care to avoid subtle corruption of contract state. + /// Instead of calling this function, consider using the higher-level [`crate::note::lifecycle::destroy_note`]. + /// + /// The precautions listed for [`PrivateContext::push_nullifier`] apply here as well, and callers should + /// additionally ensure `nullification_note_hash` corresponds to a note emitted by this contract, with its hash + /// computed in the same transaction execution phase as the call to this function. Finally, only this function + /// should be used for note nullifiers, never [`PrivateContext::push_nullifier`]. + /// + /// Failure to do these things can result in unprovable contexts, accidental deletion of notes, or double-spend + /// attacks. + pub fn push_nullifier_for_note_hash(&mut self, nullifier: Field, nullification_note_hash: Field) { let nullifier_counter = self.next_counter(); - notify_nullified_note(nullifier, nullified_note_hash, nullifier_counter); - self.nullifiers.push(Nullifier { value: nullifier, note_hash: nullified_note_hash }.count(nullifier_counter)); + notify_nullified_note(nullifier, nullification_note_hash, nullifier_counter); + self.nullifiers.push(Nullifier { value: nullifier, note_hash: nullification_note_hash }.count( + nullifier_counter, + )); } /// Returns the anchor block header - the historical block header that this private function is reading from. @@ -607,7 +592,7 @@ impl PrivateContext { /// network as a whole. For example, if a contract interaction sets include-by to some publicly-known value (e.g. /// the time when a contract upgrades), then the wallet might wish to set an even lower one to avoid revealing that /// this tx is interacting with said contract. Ideally, all wallets should standardize on an approach in order to - /// provide users with a large anonymity set -- although the exact approach + /// provide users with a large privacy set -- although the exact approach /// will need to be discussed. Wallets that deviate from a standard might accidentally reveal which wallet each /// transaction originates from. /// diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index a084674ba3df..ad9e91c0ada4 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -366,22 +366,26 @@ impl PublicContext { unsafe { avm::emit_note_hash(note_hash) }; } - /// Adds a new nullifier to the Aztec blockchain's global Nullifier Tree. + /// Creates a new [nullifier](crate::nullifier). /// - /// Whilst nullifiers are primarily intended as a _privacy-preserving_ record of a one-time action, they can also - /// be used to efficiently record _public_ one-time actions too. Hence why you're seeing this function within the - /// PublicContext. An example is to check whether a contract has been published: we emit a nullifier that is - /// deterministic, but whose preimage is _not_ private. + /// While nullifiers are primarily intended as a _privacy-preserving_ record of a one-time action, they can also + /// be used to efficiently record _public_ one-time actions. This function allows creating nullifiers from public + /// contract functions, which behave just like those created from private functions. /// - /// # Arguments - /// * `nullifier` - A unique field element that represents the consumed state + /// ## Safety /// - /// # Advanced - /// * Nullifier is immediately added to the global nullifier tree - /// * Emitted nullifiers are immediately visible to all subsequent transactions in the same block - /// * Automatically siloed with the contract address by the protocol - /// * Used for preventing double-spending and ensuring one-time actions + /// This is a low-level function that must be used with great care to avoid subtle corruption of contract state. + /// + /// In particular, callers must ensure all nullifiers created by a contract are properly domain-separated, so that + /// unrelated components don't interfere with one another (e.g. a transaction nullifier accidentally marking a + /// variable as initialized). Note nullifiers should only be created via + /// [`crate::context::PrivateContext::push_nullifier_for_note_hash`]. + /// + /// ## Advanced /// + /// The raw `nullifier` is not what is inserted into the Aztec state tree: it will be first siloed by contract + /// address via [`crate::protocol::hash::compute_siloed_nullifier`] in order to prevent accidental or malicious + /// interference of nullifiers from different contracts. pub fn push_nullifier(_self: Self, nullifier: Field) { // Safety: AVM opcodes are constrained by the AVM itself unsafe { avm::emit_nullifier(nullifier) }; diff --git a/noir-projects/aztec-nr/aztec/src/history/nullifier.nr b/noir-projects/aztec-nr/aztec/src/history/nullifier.nr index 866e4fe9a1cb..697e82d926dd 100644 --- a/noir-projects/aztec-nr/aztec/src/history/nullifier.nr +++ b/noir-projects/aztec-nr/aztec/src/history/nullifier.nr @@ -26,7 +26,7 @@ mod test; /// /// ## Cost /// -/// This function performs a full merkle tree inclusion proof, which is in the order of 4k gates. +/// This function performs a single merkle tree inclusion proof, which is ~4k gates. /// /// If you don't need to assert existence at a _specific_ past block, consider using /// [`PrivateContext::assert_nullifier_exists`](crate::context::PrivateContext::assert_nullifier_exists) instead, which @@ -82,7 +82,7 @@ pub fn assert_nullifier_existed_by(block_header: BlockHeader, siloed_nullifier: /// /// ## Cost /// -/// This function performs a full merkle tree inclusion proof, which is in the order of 4k gates. +/// This function performs a single merkle tree inclusion proof, which is ~4k gates. pub fn assert_nullifier_did_not_exist_by(block_header: BlockHeader, siloed_nullifier: Field) { // 1) Get the membership witness of a low nullifier of the nullifier. // Safety: The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe. diff --git a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr index ba866fc2fc9d..d37ccf1114d9 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/aztec.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/aztec.nr @@ -1,19 +1,21 @@ -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, +mod compute_note_hash_and_nullifier; + +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}, + storage::STORAGE_LAYOUT_NAME, + utils::{is_fn_contract_library_method, is_fn_external, is_fn_internal, is_fn_test, module_has_storage}, }, }; +use compute_note_hash_and_nullifier::generate_contract_library_methods_compute_note_hash_and_nullifier; + /// Marks a contract as an Aztec contract, generating the interfaces for its functions and notes, as well as injecting /// the `sync_state` utility function. /// @@ -34,12 +36,16 @@ pub comptime fn aztec(m: Module) -> 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 {} }; @@ -143,157 +149,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() -> Quoted { quote { @@ -309,7 +164,9 @@ comptime fn generate_sync_state() -> Quoted { 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, + Option::none(), Option::some(aztec::messages::processing::offchain::sync_inbox), ); } @@ -337,7 +194,9 @@ comptime fn generate_process_message() -> Quoted { aztec::messages::discovery::process_message::process_message_ciphertext( address, - _compute_note_hash_and_nullifier, + _compute_note_hash, + _compute_note_nullifier, + Option::none(), 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/macros/events.nr b/noir-projects/aztec-nr/aztec/src/macros/events.nr index b7a5ccd96ea6..62c71634d32c 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/events.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/events.nr @@ -10,6 +10,9 @@ pub comptime mut global EVENT_SELECTORS: CHashMap = CHashMap::new comptime fn generate_event_interface_and_get_selector(s: TypeDefinition) -> (Quoted, Field) { let name = s.name(); + let typ = s.as_type(); + let event_type_name: str<_> = f"{name}".as_quoted_str!(); + let max_event_serialized_len = crate::messages::logs::event::MAX_EVENT_SERIALIZED_LEN; let event_selector = compute_struct_selector(s, quote { crate::event::EventSelector::from_signature }); @@ -23,6 +26,18 @@ comptime fn generate_event_interface_and_get_selector(s: TypeDefinition) -> (Quo quote { impl aztec::event::event_interface::EventInterface for $name { fn get_event_type_id() -> aztec::event::EventSelector { + // This static assertion ensures the event's serialized length doesn't exceed the maximum + // allowed size. While this check would ideally live in the Serialize trait implementation, we + // place it here since this function is always generated by our macros and the Serialize trait + // implementation is not. + // + // Note: We set the event type name and max serialized length as local variables because + // injecting them directly into the error message doesn't work. + let event_type_name = $event_type_name; + let max_event_serialized_len: u32 = $max_event_serialized_len; + let event_serialized_len = <$typ as aztec::protocol::traits::Serialize>::N; + std::static_assert(event_serialized_len <= $max_event_serialized_len, f"{event_type_name} has a serialized length of {event_serialized_len} fields, which exceeds the maximum allowed length of {max_event_serialized_len} fields. See https://docs.aztec.network/errors/5"); + $from_field($event_selector) } } @@ -42,6 +57,14 @@ comptime fn register_event_selector(event_selector: Field, event_name: Quoted) { EVENT_SELECTORS.insert(event_selector, event_name); } +/// Generates the core event functionality for a struct, including the +/// [`EventInterface`](crate::event::event_interface::EventInterface) implementation (which provides the event type +/// id) and a [`Serialize`](crate::protocol::traits::Serialize) implementation if one is not already provided. +/// +/// ## Requirements +/// +/// The event struct must not exceed +/// [`MAX_EVENT_SERIALIZED_LEN`](crate::messages::logs::event::MAX_EVENT_SERIALIZED_LEN) when serialized. pub comptime fn event(s: TypeDefinition) -> Quoted { let (event_interface_impl, event_selector) = generate_event_interface_and_get_selector(s); register_event_selector(event_selector, s.name()); diff --git a/noir-projects/aztec-nr/aztec/src/macros/notes.nr b/noir-projects/aztec-nr/aztec/src/macros/notes.nr index 7c51db3a046d..2836182df312 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/notes.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/notes.nr @@ -48,8 +48,8 @@ comptime fn generate_note_type_impl(s: TypeDefinition, note_type_id: Field) -> Q // directly into the error message doesn't work. let note_type_name = $note_type_name; let max_note_packed_len: u32 = $max_note_packed_len; // Casting to u32 to avoid the value to be printed in hex. - let note_packed_len = <$typ as Packable>::N; - std::static_assert(note_packed_len <= $max_note_packed_len, f"{note_type_name} has a packed length of {note_packed_len} fields, which exceeds the maximum allowed length of {max_note_packed_len} fields"); + let note_packed_len = <$typ as aztec::protocol::traits::Packable>::N; + std::static_assert(note_packed_len <= $max_note_packed_len, f"{note_type_name} has a packed length of {note_packed_len} fields, which exceeds the maximum allowed length of {max_note_packed_len} fields. See https://docs.aztec.network/errors/4"); $note_type_id } @@ -78,8 +78,8 @@ comptime fn generate_note_hash_trait_impl(s: TypeDefinition) -> Quoted { quote { impl aztec::note::note_interface::NoteHash for $name { fn compute_note_hash(self, owner: aztec::protocol::address::AztecAddress, storage_slot: Field, randomness: Field) -> Field { - let inputs = aztec::protocol::traits::Packable::pack(self).concat( [aztec::protocol::traits::ToField::to_field(owner), storage_slot, randomness]); - aztec::protocol::hash::poseidon2_hash_with_separator(inputs, aztec::protocol::constants::DOM_SEP__NOTE_HASH) + let data = aztec::protocol::traits::Packable::pack(self).concat([aztec::protocol::traits::ToField::to_field(owner), randomness]); + aztec::note::utils::compute_note_hash(storage_slot, data) } fn compute_nullifier( @@ -93,10 +93,7 @@ comptime fn generate_note_hash_trait_impl(s: TypeDefinition) -> Quoted { // in the quote to avoid "trait not in scope" compiler warnings. let owner_npk_m_hash = aztec::protocol::traits::Hash::hash(owner_npk_m); let secret = context.request_nhk_app(owner_npk_m_hash); - aztec::protocol::hash::poseidon2_hash_with_separator( - [note_hash_for_nullification, secret], - aztec::protocol::constants::DOM_SEP__NOTE_NULLIFIER as Field, - ) + aztec::note::utils::compute_note_nullifier(note_hash_for_nullification, [secret]) } unconstrained fn compute_nullifier_unconstrained( @@ -113,10 +110,7 @@ comptime fn generate_note_hash_trait_impl(s: TypeDefinition) -> Quoted { // in the quote to avoid "trait not in scope" compiler warnings. let owner_npk_m_hash = aztec::protocol::traits::Hash::hash(owner_npk_m); let secret = aztec::keys::getters::get_nhk_app(owner_npk_m_hash); - aztec::protocol::hash::poseidon2_hash_with_separator( - [note_hash_for_nullification, secret], - aztec::protocol::constants::DOM_SEP__NOTE_NULLIFIER as Field, - ) + aztec::note::utils::compute_note_nullifier(note_hash_for_nullification, [secret]) }) } } @@ -280,7 +274,7 @@ pub comptime fn custom_note(s: TypeDefinition) -> Quoted { } } -/// Asserts that the given note implements the `Packable` trait. +/// Asserts that the given note implements the [`Packable`](crate::protocol::traits::Packable) trait. /// /// We require that notes have the `Packable` trait implemented because it is used when emitting a note in a log or as /// an offchain message. 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 d22099f30705..83f7f65a91cf 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr @@ -1,9 +1,9 @@ use crate::logging::{aztecnr_debug_log, aztecnr_debug_log_format, aztecnr_warn_log_format}; use crate::protocol::address::AztecAddress; -pub mod nonce_discovery; -pub mod partial_notes; -pub mod private_events; +pub(crate) mod nonce_discovery; +pub(crate) mod partial_notes; +pub(crate) mod private_events; pub mod private_notes; pub mod process_message; @@ -11,9 +11,10 @@ use crate::{ capsules::CapsuleArray, messages::{ discovery::process_message::process_message_ciphertext, + encoding::MAX_MESSAGE_CONTENT_LEN, logs::note::MAX_NOTE_PACKED_LEN, processing::{ - get_private_logs, offchain::OffchainInboxSync, OffchainMessageWithContext, + get_private_logs, MessageContext, offchain::OffchainInboxSync, OffchainMessageWithContext, pending_tagged_log::PendingTaggedLog, validate_and_store_enqueued_notes_and_events, }, }, @@ -21,44 +22,44 @@ use crate::{ }; pub struct NoteHashAndNullifier { - /// The result of NoteHash::compute_note_hash + /// The result of [`crate::note::note_interface::NoteHash::compute_note_hash`]. pub note_hash: Field, - /// The result of NoteHash::compute_nullifier_unconstrained (since all of message discovery is unconstrained). - /// This is `None` if the nullifier cannot be computed (e.g., because the nullifier hiding key is not available). + /// The result of [`crate::note::note_interface::NoteHash::compute_nullifier_unconstrained`]. + /// + /// This value is unconstrained, as all of message discovery is unconstrained. It is `None` if the nullifier + /// cannot be computed (e.g. because the nullifier hiding key is not available). pub inner_nullifier: Option, } -/// A function which takes a note's packed content, address of the emitting contract, note nonce, storage slot and note -/// type ID and attempts to compute its note hash (not hashed by note nonce nor siloed by address) and inner nullifier -/// (not siloed by address). +/// A contract's way of computing note hashes. /// -/// This function must be user-provided as its implementation requires knowledge of how note type IDs are allocated in -/// a contract. The `#[aztec]` macro automatically creates such a contract library method called -/// `_compute_note_hash_and_nullifier`, which looks something like this: +/// 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. /// -/// ``` -/// |packed_note, owner, storage_slot, note_type_id, contract_address, randomness, note_nonce| { +/// 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 +/// +/// This function is meant to always be used on **settled** notes, i.e. those that have been inserted into the trees +/// and for which the nonce is known. It is never invoked in the context of a transient note, as those are not involved +/// in message processing. +/// +/// ## Automatic Implementation +/// +/// 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 it looks something like this: +/// +/// ```noir +/// |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 @@ -67,23 +68,56 @@ 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; -/// Performs the state synchronization process, in which private logs are downloaded and inspected to find new private -/// notes, partial notes and events, etc., and pending partial notes are processed to search for their completion logs. -/// This is the mechanism via which a contract updates its knowledge of its private state. +/// 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. /// -/// Note that the state is synchronized up to the latest block synchronized by PXE (referred to as "anchor block"). -/// That should be close to the chain tip as block synchronization is performed before contract function simulation is -/// done. +/// Contracts that emit custom messages (i.e. any with a message type that is not in [`crate::messages::msg_type`]) +/// need to use [`crate::macros::AztecConfig::custom_message_handler`] with a function of this type in order to +/// process them. They will otherwise be **silently ignored**. +pub type CustomMessageHandler = unconstrained fn[Env]( +/* contract_address */AztecAddress, +/* msg_type_id */ u64, +/* msg_metadata */ u64, +/* msg_content */ BoundedVec, +/* message_context */ MessageContext); + +/// Synchronizes the contract's private state with the network. /// -/// Receives the address of the contract on which discovery is performed along with its -/// `compute_note_hash_and_nullifier` function. -pub unconstrained fn do_sync_state( +/// As blocks are mined, it is possible for a contract's private state to change (e.g. with new notes being created), +/// but because these changes are private they will be invisible to most actors. This is the function that processes +/// new transactions in order to discover new notes, events, and other kinds of private state changes. +/// +/// 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( 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>, ) { aztecnr_debug_log!("Performing state synchronization"); @@ -102,7 +136,9 @@ pub unconstrained fn do_sync_state( process_message_ciphertext( contract_address, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, + process_custom_message, message_ciphertext, pending_tagged_log.context, ); @@ -121,7 +157,9 @@ pub unconstrained fn do_sync_state( msgs.for_each(|i, msg| { process_message_ciphertext( contract_address, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, + process_custom_message, msg.message_ciphertext, msg.message_context, ); @@ -133,7 +171,11 @@ pub unconstrained fn do_sync_state( // Then we process all pending partial notes, regardless of whether they were found in the current or previous // executions. - partial_notes::fetch_and_process_partial_note_completion_logs(contract_address, compute_note_hash_and_nullifier); + partial_notes::fetch_and_process_partial_note_completion_logs( + contract_address, + compute_note_hash, + compute_note_nullifier, + ); // Finally we validate all notes and events that were found as part of the previous processes, resulting in them // being added to PXE's database and retrievable via oracles (get_notes) and our TS API (PXE::getPrivateEvents). @@ -144,7 +186,7 @@ mod test { use crate::{ capsules::CapsuleArray, messages::{ - discovery::{do_sync_state, NoteHashAndNullifier}, + discovery::{CustomMessageHandler, do_sync_state}, logs::note::MAX_NOTE_PACKED_LEN, processing::{ offchain::OffchainInboxSync, pending_tagged_log::PendingTaggedLog, PENDING_TAGGED_LOG_ARRAY_BASE_SLOT, @@ -168,21 +210,39 @@ mod test { assert_eq(logs.len(), 1); let no_inbox_sync: Option> = Option::none(); - do_sync_state(contract_address, dummy_compute_nhnn, no_inbox_sync); + let no_handler: Option> = Option::none(); + do_sync_state( + contract_address, + dummy_compute_note_hash, + dummy_compute_note_nullifier, + no_handler, + no_inbox_sync, + ); assert_eq(logs.len(), 0); }); } - 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 7de03c3ebb45..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, @@ -10,10 +10,10 @@ use crate::protocol::{ /// A struct with the discovered information of a complete note, required for delivery to PXE. Note that this is *not* /// the complete note information, since it does not include content, storage slot, etc. -pub struct DiscoveredNoteInfo { - pub note_nonce: Field, - pub note_hash: Field, - pub inner_nullifier: Field, +pub(crate) struct DiscoveredNoteInfo { + pub(crate) note_nonce: Field, + pub(crate) note_hash: Field, + pub(crate) inner_nullifier: Field, } /// Searches for note nonces that will result in a note that was emitted in a transaction. While rare, it is possible @@ -23,10 +23,11 @@ pub 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 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 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 aa0b9fd2468a..e57c82caa0d0 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::{ @@ -17,7 +17,7 @@ use crate::logging::aztecnr_debug_log_format; use crate::protocol::{address::AztecAddress, hash::sha256_to_field, traits::{Deserialize, Serialize}}; /// The slot in the PXE capsules where we store a `CapsuleArray` of `DeliveredPendingPartialNote`. -pub global DELIVERED_PENDING_PARTIAL_NOTE_ARRAY_LENGTH_CAPSULES_SLOT: Field = sha256_to_field( +pub(crate) global DELIVERED_PENDING_PARTIAL_NOTE_ARRAY_LENGTH_CAPSULES_SLOT: Field = sha256_to_field( "AZTEC_NR::DELIVERED_PENDING_PARTIAL_NOTE_ARRAY_LENGTH_CAPSULES_SLOT".as_bytes(), ); @@ -33,7 +33,7 @@ pub(crate) struct DeliveredPendingPartialNote { pub(crate) recipient: AztecAddress, } -pub unconstrained fn process_partial_note_private_msg( +pub(crate) unconstrained fn process_partial_note_private_msg( contract_address: AztecAddress, recipient: AztecAddress, msg_metadata: u64, @@ -72,9 +72,10 @@ pub 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 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 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_events.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr index 4ead09f40119..5276b0191f7f 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/private_events.nr @@ -7,7 +7,7 @@ use crate::{ }; use crate::protocol::{address::AztecAddress, logging::debug_log_format, traits::ToField}; -pub unconstrained fn process_private_event_msg( +pub(crate) unconstrained fn process_private_event_msg( contract_address: AztecAddress, recipient: AztecAddress, msg_metadata: u64, 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 3bc092b09894..5df56c27cbd9 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; 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 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 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 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 33defbb50dbb..f3bab5481139 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, 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}, @@ -16,17 +16,19 @@ 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, ) { @@ -35,7 +37,9 @@ pub unconstrained fn process_message_ciphertext( if message_plaintext_option.is_some() { process_message_plaintext( contract_address, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, + process_custom_message, message_plaintext_option.unwrap(), message_context, ); @@ -44,9 +48,11 @@ pub unconstrained fn process_message_ciphertext( } } -pub unconstrained fn process_message_plaintext( +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, ) { @@ -68,7 +74,8 @@ pub unconstrained fn process_message_plaintext( message_context.unique_note_hashes_in_tx, message_context.first_nullifier_in_tx, message_context.recipient, - compute_note_hash_and_nullifier, + compute_note_hash, + compute_note_nullifier, msg_metadata, msg_content, ); diff --git a/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr b/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr index 888d208b7bb7..46b6a7a361d9 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/encryption/aes128.nr @@ -4,10 +4,12 @@ use crate::protocol::{ hash::poseidon2_hash_with_separator, point::Point, public_keys::AddressPoint, + traits::ToField, }; use crate::{ keys::{ecdh_shared_secret::derive_ecdh_shared_secret, ephemeral::generate_positive_ephemeral_key_pair}, + logging::aztecnr_warn_log_format, messages::{ encoding::{ EPH_PK_X_SIZE_IN_FIELDS, HEADER_CIPHERTEXT_SIZE_IN_BYTES, MESSAGE_CIPHERTEXT_LEN, MESSAGE_PLAINTEXT_LEN, @@ -255,7 +257,13 @@ impl MessageEncryption for AES128 { eph_sk, recipient .to_address_point() - .unwrap_or( + .unwrap_or_else(|| { + aztecnr_warn_log_format!( + "Attempted to encrypt message for an invalid recipient ({0})", + )( + [recipient.to_field()], + ); + // Safety: if the recipient is an invalid address, then it is not possible to encrypt a message for // them because we cannot establish a shared secret. This is never expected to occur during normal // operation. However, it is technically possible for us to receive an invalid address, and we must @@ -266,8 +274,10 @@ impl MessageEncryption for AES128 { // random valid address. The sender is free to choose this address and hence shared secret, but // this has no security implications as they already know not only the full plaintext but also the // ephemeral private key anyway. - unsafe { random_address_point() }, - ) + unsafe { + random_address_point() + } + }) .inner, ); diff --git a/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr b/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr index 0a135970cb89..8bd6c2bc36a7 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_metadata.nr @@ -82,8 +82,8 @@ impl NoteMetadata { self.stage == NoteStage.PENDING_PREVIOUS_PHASE } - /// Returns `true` if the note is settled, i.e. if it's been created in a prior transaction and is therefore - /// already in the note hash tree. + /// Returns `true` if the note is settled, i.e. if it's been created in a prior transaction and is therefore already + /// in the note hash tree. pub fn is_settled(self) -> bool { self.stage == NoteStage.SETTLED } diff --git a/noir-projects/aztec-nr/aztec/src/note/utils.nr b/noir-projects/aztec-nr/aztec/src/note/utils.nr index c649b2fb0455..0e8008d64651 100644 --- a/noir-projects/aztec-nr/aztec/src/note/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/note/utils.nr @@ -1,6 +1,40 @@ use crate::{context::NoteExistenceRequest, note::{ConfirmedNote, HintedNote, note_interface::NoteHash}}; -use crate::protocol::hash::{compute_siloed_note_hash, compute_unique_note_hash}; +use crate::protocol::{ + constants::{DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER}, + hash::{compute_siloed_note_hash, compute_unique_note_hash, poseidon2_hash_with_separator}, +}; + +/// Computes a domain-separated note hash. +/// +/// Receives the `storage_slot` of the [`crate::state_vars::StateVariable`] that holds the note, plus any arbitrary +/// note `data`. This typically includes randomness, owner, and domain specific values (e.g. numeric amount, address, +/// id, etc.). +/// +/// Usage of this function guarantees that different state variables will never produce colliding note hashes, even if +/// their underlying notes have different implementations. +pub fn compute_note_hash(storage_slot: Field, data: [Field; N]) -> Field { + // All state variables have different storage slots, so by placing this at a fixed first position in the preimage + // we prevent collisions. + poseidon2_hash_with_separator([storage_slot].concat(data), DOM_SEP__NOTE_HASH) +} + +/// Computes a domain-separated note nullifier. +/// +/// Receives the `note_hash_for_nullification` of the note (usually returned by +/// [`compute_confirmed_note_hash_for_nullification`]), plus any arbitrary note `data`. This typically includes +/// secrets, such as the app-siloed nullifier hiding key of the note's owner. +/// +/// Usage of this function guarantees that different state variables will never produce colliding note nullifiers, even +/// if their underlying notes have different implementations. +pub fn compute_note_nullifier(note_hash_for_nullification: Field, data: [Field; N]) -> Field { + // All notes have different note hashes for nullification (i.e. transient or settled), so by placing this at a + // fixed first position in the preimage we prevent collisions. + poseidon2_hash_with_separator( + [note_hash_for_nullification].concat(data), + DOM_SEP__NOTE_NULLIFIER, + ) +} /// Returns the [`NoteExistenceRequest`] used to prove a note exists. pub fn compute_note_existence_request(hinted_note: HintedNote) -> NoteExistenceRequest @@ -26,21 +60,27 @@ where } } -/// Returns the note hash that must be used to compute a note's nullifier when calling `NoteHash::compute_nullifier` or -/// `NoteHash::compute_nullifier_unconstrained`. -pub fn compute_note_hash_for_nullification(hinted_note: HintedNote) -> Field +/// Unconstrained variant of [`compute_confirmed_note_hash_for_nullification`]. +pub unconstrained fn compute_note_hash_for_nullification(hinted_note: HintedNote) -> Field where Note: NoteHash, { + // Creating a ConfirmedNote like we do here is typically unsafe, as we've not confirmed existence. We can do it + // here because this is an unconstrained function, so the returned value should not make its way to a constrained + // function. This lets us reuse the `compute_confirmed_note_hash_for_nullification` implementation. compute_confirmed_note_hash_for_nullification(ConfirmedNote::new( hinted_note, compute_note_existence_request(hinted_note).note_hash(), )) } -/// Same as `compute_note_hash_for_nullification`, except it takes the note hash used in a read request (i.e. what -/// `compute_note_existence_request` would return). This is useful in scenarios where that hash has already been -/// computed to reduce constraints by reusing this value. +/// Returns the note hash to use when computing its nullifier. +/// +/// The `note_hash_for_nullification` parameter [`NoteHash::compute_nullifier`] takes depends on the note's stage, e.g. +/// settled notes use the unique note hash, but pending notes cannot as they have no nonce. This function returns the +/// correct note hash to use. +/// +/// Use [`compute_note_hash_for_nullification`] when computing this value in unconstrained functions. pub fn compute_confirmed_note_hash_for_nullification(confirmed_note: ConfirmedNote) -> Field { // There is just one instance in which the note hash for nullification does not match the note hash used for a read // request, which is when dealing with pending previous phase notes. These had their existence proven using their diff --git a/noir-projects/aztec-nr/aztec/src/nullifier/mod.nr b/noir-projects/aztec-nr/aztec/src/nullifier/mod.nr index ba98535fe8e6..68b1453f5260 100644 --- a/noir-projects/aztec-nr/aztec/src/nullifier/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/nullifier/mod.nr @@ -1,3 +1,50 @@ //! Nullifier-related utilities. - +//! +//! Nullifiers are one of the key primitives of private state. A nullifier is a `Field` value that is stored in one of +//! the Aztec state trees: the nullifier tree. Only unique values can be inserted into this tree: attempting to create +//! an +//! already existing nullifier (a duplicate nullifier) will result in either the transaction being unprovable, invalid, +//! or reverting, depending on exactly when the duplicate is created. +//! +//! Generally, nullifiers are used to prevent an action from happening more than once, or to more generally 'consume' a +//! resource. This can include preventing re-initialization of contracts, replay attacks of signatures, repeated claims +//! of a deposit, double-spends of received funds, etc. To achieve this, nullifiers must be computed +//! **deterministically** from the resource they're consuming. For example a contract initialization nullifier might +//! use +//! its address, or a signature replay protection could use the signature hash. +//! +//! One of the key properties of nullifiers is that they can be created by private functions, resulting in transactions +//! that do not reveal which actions they've performed. Their computation often involves a **secret parameter**, often +//! derived from a nullifier hiding key (`nhk`) which prevents linking of the resource that was consumed from the +//! nullifier. For example, it is not possible to determine which nullifier corresponds to a given note hash without +//! knowledge of the `nhk`, and so the transactions that created the note and nullifier remain unlinked. +//! +//! In other words, a nullifier is (in most cases) a random-looking but deterministic record of a private, one-time +//! action, which does not leak what action has been taken, and which preserves the property of transaction +//! unlinkability. +//! +//! In some cases, nullifiers cannot be secret as knowledge of them **must** be public information. For example, +//! contracts used by multiple people (like tokens) cannot have secrets in their initialization nullifiers: for users +//! to +//! use the contract they must prove that it has been initialized, and this requires them being able to compute the +//! initialization nullifier. +//! +//! ## Nullifier Creation +//! +//! The low-level mechanisms to create new nullifiers are [`crate::context::PrivateContext::push_nullifier`] and +//! [`crate::context::PublicContext::push_nullifier`], but these require care and can be hard to use correctly. +//! Higher-level abstractions exist which safely create nullifiers, such as [`crate::note::lifecycle::destroy_note`] +//! and +//! [`crate::state_vars::SingleUseClaim`]. +//! +//! ## Reading Nullifiers +//! +//! Private functions can prove that nullifiers have been created via +//! [`crate::context::PrivateContext::assert_nullifier_exists`] and +//! [`crate::history::nullifier::assert_nullifier_existed_by`], but the only general mechanism to privately prove that +//! a +//! nullifier _does not_ exist is to create it - which can only be done once. +//! +//! Public functions on the other hand can prove both nullifier existence and non-existence via +//! [`crate::context::PublicContext::nullifier_exists_unsafe`]. pub mod utils; diff --git a/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr b/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr index 323028dac4db..7c8e64a85574 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/nullifiers.nr @@ -14,7 +14,7 @@ pub fn notify_created_nullifier(inner_nullifier: Field) { #[oracle(aztec_prv_notifyCreatedNullifier)] unconstrained fn notify_created_nullifier_oracle(_inner_nullifier: Field) {} -/// Returns true if the nullifier has been emitted in the same transaction, i.e. if [`notify_created_nullifier`] has +/// Returns `true` if the nullifier has been emitted in the same transaction, i.e. if [`notify_created_nullifier`] has /// been /// called for this inner nullifier from the contract with the specified address. /// @@ -29,7 +29,7 @@ pub unconstrained fn is_nullifier_pending(inner_nullifier: Field, contract_addre #[oracle(aztec_prv_isNullifierPending)] unconstrained fn is_nullifier_pending_oracle(_inner_nullifier: Field, _contract_address: AztecAddress) -> bool {} -/// Returns true if the nullifier exists. Note that a `true` value can be constrained by proving existence of the +/// Returns `true` if the nullifier exists. Note that a `true` value can be constrained by proving existence of the /// nullifier, but a `false` value should not be relied upon since other transactions may emit this nullifier before /// the current transaction is included in a block. While this might seem of little use at first, certain design /// patterns benefit from this abstraction (see e.g. `PrivateMutable`). diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr index 78cd64900a7b..6c4ee7cb35fb 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/delayed_public_mutable.nr @@ -12,26 +12,120 @@ use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars mod test; -/// Public mutable values with private read access. +/// Mutable public values with private read access. +/// +/// This is an advanced public state variable, with no native Solidity equivalent. +/// +/// Like [`PublicMutable`](crate::state_vars::PublicMutable) it represents a public value of type `T` that can be +/// written to repeatedly, but with a key improvement: the current value can also be **read from a private contract +/// function**. +/// +/// This comes at the cost of extra restrictions on the state variable: writes do not come into effect immediately, +/// they must be **scheduled** to take place after some minimum delay. Reading from the state variable will therefore +/// return the previous value until some time passes, which is why this is a _delayed_ mutable variable. +/// +/// It is these delays that enable the capacity for reads from private contract functions, as they provide guarantees +/// regarding how long can some historical state observed at the anchor block be known to not change. +/// +/// Delays can be modified during the lifetime of the contract. +/// +/// ## Access Patterns +/// +/// The current value stored in a `DelayedPublicMutable` can be read from public contract functions, and writes can be +/// scheduled to happen in the future. +/// +/// Public contract functions can also schedule changes to the write delay, as well as inspect any already scheduled +/// value or delay changes. +/// +/// Private contract functions can read the **current** value of the state variable, but not past or scheduled values. +/// They cannot read the current delay, and they cannot schedule any kind of value change. +/// +/// ## Privacy +/// +/// The value stored in `DelayedPublicMutable` is fully public, as are all scheduled value and delay changes. +/// +/// Reads from a private contract function are almost fully private: the only observable effect is that they set the +/// transaction's `expiration_timestamp` property, possibly reducing the privacy set. See +/// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp). +/// +/// ## Use Cases +/// +/// These are mostly an extension of [`PublicMutable`](crate::state_vars::PublicMutable)'s, given that what this state +/// variable essentially achieves is to provide private reads to it. For example, it can be used for global contract +/// configuration (such as fees, access control, etc.) that users will need to access during private interactions. +/// +/// The key consideration is whether the enforced minimum delay on writes prevents using this state variable. In some +/// scenarios this restriction is incompatible with requirements (such as a token's total supply, which must always be +/// up to date), while in others the enhanced privacy might make the tradeoff acceptable (such as when dealing with +/// contract pauses or access control revocation, where a delay of some hours could be acceptable). +/// +/// Note that, just like in [`PublicMutable`](crate::state_vars::PublicMutable), the fact that the values are public +/// does not necessarily mean the actions that update these values must themselves be wholly public. To learn more, +/// see the notes there regarding usage of [`only_self`](crate::macros::functions::only_self). +/// +/// ## Choosing Delays +/// +/// A short delay reduces the most obvious downside of `DelayedPublicMutable`, and so it is natural to wish to make it +/// be as low as possible. It is therefore important to understand the tradeoffs involved in delay selection. +/// +/// A shorter delay will result in a lower `expiration_timestamp` property of transactions that privately read the +/// state variable, reducing its privacy set. If the delay is smaller than that of any other contract, then this +/// privacy leak might be large enough to uniquely identify those transactions that interact with the contract - fully +/// defeating the purpose of `DelayedPublicMutable`. +/// +/// Additionally, a lower `expiration_timestamp` obviously causes transactions to expire earlier, resulting in +/// multiple issues. Among others, this can make large transactions that take long to prove be unfeasible, restrict +/// users with slow proving devices, and force large transaction fees to guarantee fast inclusion. +/// +/// In practice, a delay of at least a couple hours is recommended. From a privacy point of view the optimal delay is +/// [`crate::protocol::constants::MAX_TX_LIFETIME`], which puts contracts in the same privacy set as those that do not +/// use `DelayedPublicMutable` at all. +/// +/// ## Examples +/// +/// Declaring a `DelayedPublicMutable` in the contract's [`storage`](crate::macros::storage::storage) struct +/// requires specifying the type `T` that is stored in the variable, along with the initial delay used when scheduling +/// value changes: +/// +/// ```noir +/// global PAUSE_CONTRACT_INITIAL_DELAY_S: u64 = 6 * 60 * 60; // 6 hours +/// global CHANGE_AUTHORIZATION_INITIAL_DELAY_S: u64 = 24 * 60 * 60; // 24 hours +/// +/// #[storage] +/// struct Storage { +/// paused: DelayedPublicMutable, +/// user_authorization: Map, C>, +/// } +/// ``` +/// +/// Note that this initial delay can be altered during the contract's lifetime via +/// [`DelayedPublicMutable::schedule_delay_change`]. +/// +/// ## Requirements +/// +/// The type `T` stored in the `DelayedPublicMutable` must implement the `Eq` and +/// [`Packable`](crate::protocol::traits::Packable) traits. +/// +/// ## Implementation Details +/// +/// This state variable stores more information in public storage than +/// [`PublicMutable`](crate::state_vars::PublicMutable), as it needs to keep track of the current and scheduled change +/// information for both the value and the delay - see +/// [`crate::protocol::delayed_public_mutable::DelayedPublicMutableValues`]. +/// +/// It also stores a hash of this entire configuration so that private reads can be performed in a single historical +/// public storage read - see [`crate::utils::WithHash`]. +/// +/// This results in a total of `N * 2 + 2` storage slots used, where `N` is the packing length of the stored type `T`. +/// This makes it quite important to ensure `T`'s implementation of [`Packable`](crate::protocol::traits::Packable) is +/// space-efficient. pub struct DelayedPublicMutable { context: Context, storage_slot: Field, } -// DelayedPublicMutable stores a value of type T that is: -// - publicly known (i.e. unencrypted) -// - mutable in public -// - readable in private with no contention (i.e. multiple parties can all read the same value without blocking one -// another nor needing to coordinate) -// This is famously a hard problem to solve. DelayedPublicMutable makes it work by introducing a delay to public -// mutation: the value is not changed immediately but rather a value change is scheduled to happen in the future after -// some delay measured in seconds. Reads in private are only valid as long as they are included in a block with a -// timestamp not too far into the future, so that they can guarantee the value will not have possibly changed by then -// (because of the delay). The delay for changing a value is initially equal to InitialDelay, but can be changed by -// calling `schedule_delay_change`. -// -// This implementation requires that T implements the Packable and Eq traits, and allocates `M + 1` storage slots to -// this state variable. +// We allocate `M + 1` slots because we're going to store a `WithHash>`, +// and `WithHash` increases the packing length by one. impl StateVariable for DelayedPublicMutable where DelayedPublicMutableValues: Packable, @@ -50,15 +144,47 @@ impl DelayedPublicMutable + /// t1`). The result is that the current value continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// This also means that it is possible to **cancel** a scheduled change by calling `schedule_value_change` with + /// the current value. + /// + /// ## Examples + /// + /// A public setter that authorizes a user: + /// ```noir + /// #[external("public")] + /// fn authorize_user(user: AztecAddress) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.user_authorization.at(user).schedule_value_change(true); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_value_change(self, new_value: T) where T: Packable, { - let _value_change = self.schedule_and_return_value_change(new_value); + let _ = self.schedule_and_get_value_change(new_value); } - pub fn schedule_and_return_value_change(self, new_value: T) -> ScheduledValueChange + /// Schedules a write to the current value, returning the scheduled entry. + pub fn schedule_and_get_value_change(self, new_value: T) -> ScheduledValueChange where T: Packable, { @@ -77,6 +203,57 @@ where value_change } + /// Schedules a write to the current delay. + /// + /// This works just like [`schedule_value_change`](DelayedPublicMutable::schedule_value_change), except instead of + /// changing the value in the state variable, it changes the delay that will govern future invocations of that + /// function. + /// + /// The current delay does not immediately change. Once the current delay passes, + /// [`get_current_delay`](DelayedPublicMutable::get_current_delay) automatically begins to return `new_delay`, and + /// [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) begins using it. + /// + /// ## Multiple Scheduled Changes + /// + /// Only a **single** delay can be scheduled to become the new delay at a given point in time. Any prior scheduled + /// changes which have not yet become current are **replaced** with the new one and discarded. + /// + /// To illustrate this, consider a scenario at `t0` with a current delay `A`. A delay change to `B` is scheduled to + /// occur at `t1`. At some point _before_ `t1`, a second delay change to `C` is scheduled to occur at `t2` (`t2 > + /// t1`). The result is that the current delay continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// ## Delays When Changing Delays + /// + /// A delay change cannot always be immediate: if it were, then it'd be possible to break `DelayedPublicMutable`'s + /// invariants by setting the delay to a very low or zero value and then scheduling a value change, resulting in a + /// new value becoming the current one earlier than was predictable based on the prior delay. This would prohibit + /// private reads, which is the reason for existence of this state variable. + /// + /// Instead, delay changes are themselves scheduled and delay so that the property mentioned above is preserved. + /// This results in delay increases and decreases being asymmetrical. + /// + /// If the delay is being decreased, then this requires a delay equal to the difference between the current and new + /// delay, so that a scheduled value change that occurred as the new delay came into effect would be scheduled for + /// the same timestamp as if no delay change had occurred. + /// + /// If the delay is being increased, then the new delay becomes effective immediately, as new value changes would + /// be scheduled for a timestamp that is further than the current delay. + /// + /// ## Examples + /// + /// A public setter that sets the pause delay: + /// ```noir + /// #[public] + /// fn set_pause_delay(delay: u64) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.paused.schedule_delay_change(delay); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_delay_change(self, new_delay: u64) where T: Packable, @@ -87,9 +264,39 @@ where delay_change.schedule_change(new_delay, current_timestamp); + // We can't just update the `ScheduledDelayChange`, we need to update the entire storage because we need to + // also recompute and write the hash. + // We _could_ just read everything, update the hash and `ScheduledDelayChange` but not overwrite the + // `ScheduledValueChange`, resulting in fewer storage writes, but that would require careful handling of + // storage slots and `WithHash`'s internal layout, which is not worth doing at this point. self.write(self.read_value_change(), delay_change); } + /// Returns the current value. + /// + /// If [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) has never been called, then this + /// returns the default empty public storage value, which is all zeroes - equivalent to `let t = + /// T::unpack(std::mem::zeroed());`. + /// + /// It is not possible to detect if a `DelayedPublicMutable` has ever been initialized or not other than by testing + /// for the zero sentinel value. For a more robust solution, store an `Option` in the `DelayedPublicMutable`. + /// + /// Use [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) to instead get the last value that was + /// scheduled to become the current one (which will equal the current value if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns a user's authorization status: + /// ```noir + /// #[external("public")] + /// fn is_authorized(user: AztecAddress) -> bool { + /// self.storage.user_authorization.at(user).get_current_value() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked `2 * N + 1` times, where `N` is `T`'s packed length. pub fn get_current_value(self) -> T where T: Packable, @@ -100,6 +307,30 @@ where value_change.get_current_at(current_timestamp) } + /// Returns the current delay. + /// + /// This is the delay that would be used by [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) + /// if it were called in the current transaction. + /// + /// If [`schedule_delay_change`](DelayedPublicMutable::schedule_delay_change) has never been called, then this + /// returns the `InitialDelay` used in the [`storage`](crate::macros::storage::storage) struct. + /// + /// Use [`get_scheduled_delay`](DelayedPublicMutable::get_scheduled_delay) to instead get the last delay that was + /// scheduled to become the current one (which will equal the current delay if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns the pause delay: + /// ```noir + /// #[external("public")] + /// fn get_pause_delay() -> u64 { + /// self.storage.paused.get_current_delay() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked a single time, regardless of `T`. pub fn get_current_delay(self) -> u64 where T: Packable, @@ -108,6 +339,7 @@ where self.read_delay_change().get_current(current_timestamp) } + /// Returns the last scheduled value and timestamp of change. pub fn get_scheduled_value(self) -> (T, u64) where T: Packable, @@ -115,6 +347,7 @@ where self.read_value_change().get_scheduled() } + /// Returns the last scheduled delay and timestamp of change. pub fn get_scheduled_delay(self) -> (u64, u64) where T: Packable, @@ -166,7 +399,80 @@ impl DelayedPublicMutable` in the `DelayedPublicMutable`. + /// + /// ## Privacy + /// + /// This function leaks information via the transaction's expiration timestamp. The degree of leakage depends on + /// the delay that is set, and on whether a value change is currently scheduled. + /// + /// Private reads are based on a historical public storage read at the anchor block (i.e. + /// [`crate::history::storage::public_storage_historical_read`]). `DelayedPublicMutable` is able to provide + /// guarantees that values read in the past remain the state variable's current value into the future due to + /// the existence of delays when scheduling writes. It then sets the `expiration_timestamp` property of the current + /// transaction to ensure that the transaction can only be included in a block **prior** to the state variable's + /// value changing. In other words, it knows some facts about the near future up until some time horizon, and then + /// makes sure that it doesn't act on this knowledge past said moment. + /// + /// Because the `expiration_timestamp` property is part of the transaction's public information, any mutation to + /// this value could result in transaction fingerprinting. Note that multiple contracts may set this value during a + /// transaction: it is the smallest (most restrictive) timestamp that will be used. + /// + /// If the state variable **does not** have any value changes scheduled, then the timestamp will be set to that of + /// the anchor block plus the current delay. If multiple contracts use the same delay for their + /// `DelayedPublicMutable` state variables, then these will all be in the same privacy set. + /// + /// If the state variable **does** have a value change scheduled, then the timestamp will be set to equal the time + /// at which the current value will change, i.e. the one + /// [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) returns - which is public information. This + /// results in an unavoidable privacy leak of any transactions in which a contract privately reads a + /// `DelayedPublicMutable` that will change soon. + /// + /// Transactions that do not read from a `DelayedPublicMutable` are part of a privacy set in which the + /// `expiration_timestamp` is set to their anchor block plus [`crate::protocol::constants::MAX_TX_LIFETIME`], + /// making this the most privacy-preserving delay. The less frequent said value changes are, the more private the + /// contract is. Wallets can also then choose to further lower this timestamp to make it less obvious that their + /// transactions are interacting with this soon-to-change variable. + /// + /// ## Examples + /// + /// A private action that requires authorization: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert( + /// self.storage.user_authorization.at(self.msg_sender()).get_current_value(), + /// "caller is not authorized" + /// ); + /// + /// // do the action + /// } + /// ``` + /// + /// A private action that can be paused: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert(!self.storage.paused.get_current_value(), "contract is paused"); + /// + /// // do the action + /// } + /// ``` + /// + /// ## Cost + /// + /// This function performs a historical public storage read (which is ~4k gates), **regardless of /// `T`'s packed length**. This is because [`DelayedPublicMutable::schedule_value_change`] stores not just the /// value but also its hash: this function obtains the preimage from an oracle and proves that it matches the hash /// from public storage. diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr index 8e3acdf200ba..ef84849f3ebb 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_immutable.nr @@ -143,7 +143,7 @@ where self.compute_initialization_nullifier(secret) } - /// Returns whether this PrivateImmutable has been initialized. + /// Returns `true` if this PrivateImmutable has been initialized. pub unconstrained fn is_initialized(self) -> bool { let nullifier = self.get_initialization_nullifier(); check_nullifier_exists(nullifier) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr index 2db7c2f325e4..5a34b89806e2 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr @@ -28,10 +28,9 @@ mod test; /// A value stored in a `PublicImmutable` can be read and initialized from public contract functions. /// /// Unlike [`PublicMutable`](crate::state_vars::PublicMutable) it is **also** possible to read a `PublicImmutable` from -/// a -/// private contract function, though it is not possible to initialize one. A common pattern is to have these functions -/// [enqueue a public self calls](crate::contract_self::ContractSelfPrivate::enqueue) in which the initialization -/// operation is performed. +/// a private contract function, though it is not possible to initialize one. A common pattern is to have these +/// functions [enqueue a public self calls](crate::contract_self::ContractSelf::enqueue_self) in which the +/// initialization operation is performed. /// /// For a mutable (with restrictions) variant which also can be read from private functions see /// [`DelayedPublicMutable`](crate::state_vars::DelayedPublicMutable). @@ -54,26 +53,26 @@ mod test; /// /// `PublicImmutable`'s main limitation is the immutability, which in many cases leads to /// [`DelayedPublicMutable`](crate::state_vars::DelayedPublicMutable) being used instead. But in those cases where -/// fixed -/// values are not a problem, this is a fine choice for storage. +/// fixed values are not a problem, this is a fine choice for storage. /// /// ## Examples /// -/// Declaring a `PublicImmutable` in the the contract's [`storage`](crate::macros::storage::storage) struct requires +/// Declaring a `PublicImmutable` in the contract's [`storage`](crate::macros::storage::storage) struct requires /// specifying the type `T` that is stored in the variable: /// /// ```noir /// #[storage] -/// struct Storage { -/// decimals: PublicImmutable, +/// struct Storage { +/// decimals: PublicImmutable, /// -/// account_types: Map, Context>, +/// account_types: Map, C>, /// } /// ``` /// /// ## Requirements /// -/// The type `T` stored in the `PublicImmutable` must implement the `Packable` trait. +/// The type `T` stored in the `PublicImmutable` must implement the `Eq` and +/// [`Packable`](crate::protocol::traits::Packable) traits. /// /// ## Implementation Details /// @@ -130,7 +129,7 @@ impl PublicImmutable { /// #[external("public")] /// #[initializer] /// fn initialize(decimals: u8) { - /// self.storage.decimals.iniitalize(decimals); + /// self.storage.decimals.initialize(decimals); /// } /// ``` /// @@ -139,7 +138,7 @@ impl PublicImmutable { /// // Can only be called once per account /// #[external("public")] /// fn set_account_type(account_type: AccountType) { - /// self.storage.account_types.at(self.msg_sender()).iniitalize(account_type); + /// self.storage.account_types.at(self.msg_sender()).initialize(account_type); /// } /// ``` /// @@ -214,7 +213,7 @@ impl PublicImmutable { WithHash::public_storage_read(self.context, self.storage_slot) } - /// Returns true if the `PublicImmutable` has been initialized. + /// Returns `true` if the `PublicImmutable` has been initialized. /// /// ## Examples: /// @@ -223,7 +222,7 @@ impl PublicImmutable { /// #[external("public")] /// fn set_account_type_if_not_set(account_type: AccountType) { /// if !self.storage.account_types.at(self.msg_sender()).is_initialized() { - /// self.storage.account_types.at(self.msg_sender()).iniitalize(account_type); + /// self.storage.account_types.at(self.msg_sender()).initialize(account_type); /// } /// } /// ``` @@ -263,7 +262,7 @@ impl PublicImmutable { WithHash::utility_public_storage_read(self.context, self.storage_slot) } - /// Returns true if the `PublicImmutable` has been initialized. + /// Returns `true` if the `PublicImmutable` has been initialized. pub unconstrained fn is_initialized(self) -> bool { let nullifier = self.compute_initialization_inner_nullifier(); check_nullifier_exists(nullifier) @@ -300,7 +299,7 @@ impl PublicImmutable { /// #[storage] /// struct Storage { /// decimals: PublicImmutable, - /// symbol: PubicImmutable, + /// symbol: PublicImmutable, /// } /// /// // Good: both `decimals` and `symbol` are retrieved in a single historical public storage read diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr index c7f53d505d9d..a1ea5468c434 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable.nr @@ -2,6 +2,8 @@ use crate::context::{PublicContext, UtilityContext}; use crate::protocol::traits::Packable; use crate::state_vars::StateVariable; +mod test; + /// Mutable public values. /// /// This is one of the most basic public state variables. It is equivalent to a non-`immutable` non-`constant` Solidity @@ -35,7 +37,7 @@ use crate::state_vars::StateVariable; /// This is suitable for any kind of global state that needs to be accessible by everyone. For example, a token may /// have a public total supply, or a voting contract may have public vote tallies. /// -/// Note that contracts having public values does not necessarily mean the the actions that update these values must +/// Note that contracts having public values does not necessarily mean the actions that update these values must /// themselves be wholly public. For example, the token could allow for private minting and burning, and casting a vote /// could be kept private: these private functions would enqueue a public function that writes to the `PublicMutable`. /// @@ -50,22 +52,23 @@ use crate::state_vars::StateVariable; /// /// ## Examples /// -/// Declaring a `PublicMutable` in the the contract's [`storage`](crate::macros::storage::storage) struct requires +/// Declaring a `PublicMutable` in the contract's [`storage`](crate::macros::storage::storage) struct requires /// specifying the type `T` that is stored in the variable: /// /// ```noir /// #[storage] -/// struct Storage { -/// total_supply: PublicMutable, -/// public_balances: Map, Context>, +/// struct Storage { +/// total_supply: PublicMutable, +/// public_balances: Map, C>, /// -/// vote_tallies: Map, Context>, +/// vote_tallies: Map, C>, /// } /// ``` /// /// ## Requirements /// -/// The type `T` stored in the `PublicMutable` must implement the `Packable` trait. +/// The type `T` stored in the `PublicMutable` must implement the [`Packable`](crate::protocol::traits::Packable) +/// trait. /// /// ## Implementation Details /// @@ -162,7 +165,7 @@ impl PublicMutable { /// } /// ``` /// - /// An [`only_self`](crate::macros::functions::only_self) helper that updates public state trigered by a private + /// An [`only_self`](crate::macros::functions::only_self) helper that updates public state triggered by a private /// function: /// ```noir /// #[external("private")] @@ -175,8 +178,8 @@ impl PublicMutable { /// #[external("public")] /// #[only_self] /// fn _tally_vote(election_id: ElectionId, votes: u128) { - /// let current = self.storage.vote_tallies.read(); - /// self.storage.vote_tallies.write(current + votes); + /// let current = self.storage.vote_tallies.at(election_id).read(); + /// self.storage.vote_tallies.at(election_id).write(current + votes); /// } /// ``` /// diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable/test.nr index 29154c42d466..3c07e5bf1fbc 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable/test.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/public_mutable/test.nr @@ -1,4 +1,4 @@ -use crate::{context::{PublicContext, UtilityContext}, state_vars::PublicMutable}; +use crate::{context::{PublicContext, UtilityContext}, state_vars::{PublicMutable, StateVariable}}; use crate::test::{helpers::test_environment::TestEnvironment, mocks::MockStruct}; use std::mem::zeroed; diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr index 8a83efad2e79..326a4843110d 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/single_private_immutable.nr @@ -158,7 +158,7 @@ where self.compute_initialization_nullifier(secret) } - /// Returns whether this SinglePrivateImmutable has been initialized. + /// Returns `true` if this SinglePrivateImmutable has been initialized. pub unconstrained fn is_initialized(self) -> bool { let nullifier = self.get_initialization_nullifier(); check_nullifier_exists(nullifier) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr index e2c0e05d7625..d286aa346b78 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/single_private_mutable.nr @@ -249,7 +249,7 @@ where self.compute_initialization_nullifier(secret) } - /// Returns whether this SinglePrivateImmutable has been initialized. + /// Returns `true` if this SinglePrivateMutable has been initialized. pub unconstrained fn is_initialized(self) -> bool { let nullifier = self.get_initialization_nullifier(); check_nullifier_exists(nullifier) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr b/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr index 2edf233a6f9c..28f0ac6f860d 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr @@ -1,5 +1,6 @@ use crate::protocol::{ - address::AztecAddress, constants::DOM_SEP__NOTE_HASH, hash::poseidon2_hash_with_separator, traits::Hash, + address::AztecAddress, constants::DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER, hash::poseidon2_hash_with_separator, + traits::Hash, }; use crate::{ @@ -62,7 +63,6 @@ mod test; /// /// Public effects you emit alongside a claim (e.g. a public function call to update a tally) may still let observers /// infer who likely exercised the claim, so consider that when designing flows. -/// ``` pub struct SingleUseClaim { context: Context, storage_slot: Field, @@ -82,8 +82,10 @@ impl SingleUseClaim { /// This function is primarily used internally by functions [`SingleUseClaim::claim`], /// [`SingleUseClaim::assert_claimed`] and [`SingleUseClaim::has_claimed`] to coherently write and read state. fn compute_nullifier(self, owner_nhk_app: Field) -> Field { - // TODO(F-180): make sure we follow the nullifier convention - poseidon2_hash_with_separator([owner_nhk_app, self.storage_slot], DOM_SEP__NOTE_HASH) + poseidon2_hash_with_separator( + [owner_nhk_app, self.storage_slot], + DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER, + ) } } @@ -132,7 +134,7 @@ impl SingleUseClaim<&mut PrivateContext> { } impl SingleUseClaim { - /// Returns whether an owner has claimed this single use claim. + /// Returns `true` if an owner has claimed this single use claim. pub unconstrained fn has_claimed(self) -> bool { let owner_nhk_app = get_nhk_app(get_public_keys(self.owner).npk_m.hash()); check_nullifier_exists(self.compute_nullifier(owner_nhk_app)) diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index 7cc47bf41813..77985955ed71 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -12,18 +12,14 @@ use crate::{ event::{event_interface::EventInterface, EventMessage}, hash::hash_args, messages::{ - discovery::{ComputeNoteHashAndNullifier, NoteHashAndNullifier, process_message::process_message_plaintext}, + discovery::{ + ComputeNoteHash, ComputeNoteNullifier, CustomMessageHandler, process_message::process_message_plaintext, + }, encoding::MESSAGE_PLAINTEXT_LEN, logs::{event::encode_private_event_message, note::encode_private_note_message}, processing::{MessageContext, validate_and_store_enqueued_notes_and_events}, }, - note::{ - HintedNote, - note_interface::{NoteHash, NoteType}, - note_metadata::SettledNoteMetadata, - NoteMessage, - utils::compute_note_hash_for_nullification, - }, + note::{note_interface::{NoteHash, NoteType}, NoteMessage}, oracle::version::assert_compatible_oracle_version, test::helpers::{txe_oracles, utils::ContractDeployment}, utils::array::subarray, @@ -780,37 +776,35 @@ impl TestEnvironment { note_message.new_note.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 one - // specialized for `Note`. - 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 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(); self.discover_data_in_message_plaintext( 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, ); } @@ -902,29 +896,40 @@ 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", ) }; + let process_custom_message: Option> = Option::none(); self.discover_data_in_message_plaintext( 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 // on a message plaintext. @@ -952,7 +957,9 @@ 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/aztec-nr/aztec/src/test/mocks/mock_note.nr b/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr index fabceb778c14..bee037c40061 100644 --- a/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr +++ b/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr @@ -5,11 +5,9 @@ use crate::{ note::{HintedNote, note_interface::{NoteHash, NoteType}, note_metadata::NoteMetadata}, }; -use crate::protocol::{ - address::AztecAddress, - constants::{DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER}, - hash::poseidon2_hash_with_separator, - traits::{Packable, ToField}, +use crate::{ + note::utils::{compute_note_hash, compute_note_nullifier}, + protocol::{address::AztecAddress, traits::{Packable, ToField}}, }; #[derive(Eq, Packable)] @@ -26,8 +24,8 @@ impl NoteType for MockNote { impl NoteHash for MockNote { fn compute_note_hash(self, owner: AztecAddress, storage_slot: Field, randomness: Field) -> Field { - let input = self.pack().concat([owner.to_field(), storage_slot, randomness]); - poseidon2_hash_with_separator(input, DOM_SEP__NOTE_HASH) + let data = self.pack().concat([owner.to_field(), randomness]); + compute_note_hash(storage_slot, data) } fn compute_nullifier( @@ -38,10 +36,7 @@ impl NoteHash for MockNote { ) -> Field { // We don't use any kind of secret here since this is only a mock note and having it here would make tests more // cumbersome - poseidon2_hash_with_separator( - [note_hash_for_nullification], - DOM_SEP__NOTE_NULLIFIER as Field, - ) + compute_note_nullifier(note_hash_for_nullification, []) } unconstrained fn compute_nullifier_unconstrained( @@ -51,12 +46,7 @@ impl NoteHash for MockNote { ) -> Option { // We don't use any kind of secret here since this is only a mock note and having it here would make tests more // cumbersome - Option::some( - poseidon2_hash_with_separator( - [note_hash_for_nullification], - DOM_SEP__NOTE_NULLIFIER as Field, - ), - ) + Option::some(compute_note_nullifier(note_hash_for_nullification, [])) } } diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index e5c34f95423f..0c9ea6fcd8e8 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -4,14 +4,11 @@ use aztec::{ keys::getters::{get_nhk_app, get_public_keys, try_get_public_keys}, macros::notes::custom_note, messages::logs::partial_note::compute_partial_note_private_content_log, - note::note_interface::{NoteHash, NoteType}, + note::{note_interface::{NoteHash, NoteType}, utils::compute_note_nullifier}, oracle::random::random, protocol::{ address::AztecAddress, - constants::{ - DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT, - PRIVATE_LOG_SIZE_IN_FIELDS, - }, + constants::{DOM_SEP__NOTE_HASH, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT, PRIVATE_LOG_SIZE_IN_FIELDS}, hash::{compute_siloed_nullifier, poseidon2_hash_with_separator}, traits::{Deserialize, FromField, Hash, Packable, Serialize, ToField}, }, @@ -65,10 +62,7 @@ impl NoteHash for UintNote { let owner_npk_m = get_public_keys(owner).npk_m; let owner_npk_m_hash = owner_npk_m.hash(); let secret = context.request_nhk_app(owner_npk_m_hash); - poseidon2_hash_with_separator( - [note_hash_for_nullification, secret], - DOM_SEP__NOTE_NULLIFIER, - ) + compute_note_nullifier(note_hash_for_nullification, [secret]) } unconstrained fn compute_nullifier_unconstrained( @@ -80,10 +74,7 @@ impl NoteHash for UintNote { let owner_npk_m = public_keys.npk_m; let owner_npk_m_hash = owner_npk_m.hash(); let secret = get_nhk_app(owner_npk_m_hash); - poseidon2_hash_with_separator( - [note_hash_for_nullification, secret], - DOM_SEP__NOTE_NULLIFIER, - ) + compute_note_nullifier(note_hash_for_nullification, [secret]) }) } } diff --git a/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/Nargo.toml b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/Nargo.toml new file mode 100644 index 000000000000..fd5c446ab345 --- /dev/null +++ b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "invalid_event" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../aztec-nr/aztec" } diff --git a/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/expected_error b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/expected_error new file mode 100644 index 000000000000..4f092dbf4803 --- /dev/null +++ b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/expected_error @@ -0,0 +1 @@ +InvalidEvent has a serialized length of 11 fields, which exceeds the maximum allowed length of 10 fields. See https://docs.aztec.network/errors/5 diff --git a/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/src/invalid_event.nr b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/src/invalid_event.nr new file mode 100644 index 000000000000..a38ba6855727 --- /dev/null +++ b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/src/invalid_event.nr @@ -0,0 +1,18 @@ +use aztec::{ + macros::events::event, + messages::logs::event::MAX_EVENT_SERIALIZED_LEN, + protocol::{traits::Serialize, utils::writer::Writer}, +}; + +#[event] +pub struct InvalidEvent {} + +impl Serialize for InvalidEvent { + let N: u32 = MAX_EVENT_SERIALIZED_LEN + 1; + + fn serialize(self) -> [Field; Self::N] { + std::mem::zeroed() + } + + fn stream_serialize(self, _writer: &mut Writer) {} +} diff --git a/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/src/main.nr b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/src/main.nr new file mode 100644 index 000000000000..03fdfb49b062 --- /dev/null +++ b/noir-projects/noir-contracts-comp-failures/contracts/invalid_event/src/main.nr @@ -0,0 +1,15 @@ +use aztec::macros::aztec; + +mod invalid_event; + +#[aztec] +pub contract InvalidEventContract { + use crate::invalid_event::InvalidEvent; + use aztec::{event::event_interface::EventInterface, macros::functions::external}; + + // We have here this function in order for the static_assert in `get_event_type_id` to get triggered. + #[external("private")] + fn trigger_event_check() { + let _ = InvalidEvent::get_event_type_id(); + } +} diff --git a/noir-projects/noir-contracts-comp-failures/contracts/invalid_note/expected_error b/noir-projects/noir-contracts-comp-failures/contracts/invalid_note/expected_error index ee90fdf8adb9..02c046ffd7ab 100644 --- a/noir-projects/noir-contracts-comp-failures/contracts/invalid_note/expected_error +++ b/noir-projects/noir-contracts-comp-failures/contracts/invalid_note/expected_error @@ -1 +1 @@ -InvalidNote has a packed length of 9 fields, which exceeds the maximum allowed length of 8 fields +InvalidNote has a packed length of 9 fields, which exceeds the maximum allowed length of 8 fields. See https://docs.aztec.network/errors/4 diff --git a/noir-projects/noir-contracts/contracts/app/auth_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/auth_contract/src/main.nr index 836d8b4af942..4f2b7dbeeb68 100644 --- a/noir-projects/noir-contracts/contracts/app/auth_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/auth_contract/src/main.nr @@ -13,8 +13,8 @@ pub contract Auth { }; // docs:start:delayed_public_mutable_storage - // Authorizing a new address has a certain delay before it goes into effect. Set to 360 seconds which is 5 slots. - pub(crate) global CHANGE_AUTHORIZED_DELAY: u64 = 360; + // Authorizing a new address has a certain delay before it goes into effect. Set to 180 seconds. + pub(crate) global CHANGE_AUTHORIZED_DELAY: u64 = 180; #[storage] struct Storage { diff --git a/noir-projects/noir-contracts/contracts/app/claim_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/claim_contract/src/main.nr index 3d022fc8b76f..4a54aac54faa 100644 --- a/noir-projects/noir-contracts/contracts/app/claim_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/claim_contract/src/main.nr @@ -7,69 +7,77 @@ pub contract Claim { // docs:end:history_import use aztec::{ macros::{functions::{external, initializer}, storage::storage}, - note::{HintedNote, note_interface::NoteHash, utils::compute_note_hash_for_nullification}, - protocol::address::AztecAddress, - state_vars::PublicImmutable, + note::{HintedNote, utils::compute_confirmed_note_hash_for_nullification}, + protocol::{address::AztecAddress, traits::Packable}, + state_vars::{Map, Owned, PublicImmutable, SingleUseClaim}, }; use token::Token; use uint_note::UintNote; - // TODO: This can be optimized by storing the addresses in Config struct in 1 PublicImmutable (less merkle proofs). - #[storage] - struct Storage { + #[derive(Eq, Packable)] + struct ClaimConfig { // Address of a contract based on whose notes we distribute the rewards - target_contract: PublicImmutable, + target_contract: AztecAddress, // Token to be distributed as a reward when claiming - reward_token: PublicImmutable, + reward_token: AztecAddress, + } + + #[storage] + struct Storage { + config: PublicImmutable, + // Maps a note hash to owner-specific single-use claims, preventing double-claiming of rewards. + note_hash_claims: Map, Context>, Context>, } #[external("public")] #[initializer] fn constructor(target_contract: AztecAddress, reward_token: AztecAddress) { - self.storage.target_contract.initialize(target_contract); - self.storage.reward_token.initialize(reward_token); + self.storage.config.initialize(ClaimConfig { target_contract, reward_token }); } #[external("private")] fn claim(hinted_note: HintedNote, recipient: AztecAddress) { - // 1) Check that the note corresponds to the target contract - let target_address = self.storage.target_contract.read(); + // 1) Prove that the note exists + // docs:start:prove_note_inclusion + let header = self.context.get_anchor_block_header(); + let confirmed_note = assert_note_existed_by(header, hinted_note); + // docs:end:prove_note_inclusion + + let config = self.storage.config.read(); + + // 2) Check that the note corresponds to the target contract assert( - target_address == hinted_note.contract_address, + config.target_contract == confirmed_note.contract_address, "Note does not correspond to the target contract", ); - // 2) Check that the note is owned by the msg_sender - assert(hinted_note.owner == self.msg_sender(), "Note is not owned by the msg_sender"); + // 3) Check that the note is owned by the msg_sender + assert(confirmed_note.owner == self.msg_sender(), "Note is not owned by the msg_sender"); // Given that there is only 1 state variable in the Crowdfunding contract we don't need to constrain the storage // slot of the note as there is no risk of claiming with a note that is not a donation note. - // 3) Prove that the note hash exists in the note hash tree - // docs:start:prove_note_inclusion - let header = self.context.get_anchor_block_header(); - let _ = assert_note_existed_by(header, hinted_note); - // docs:end:prove_note_inclusion - - // 4) Compute and emit a nullifier which is unique to the note and this contract to ensure the reward can be - // claimed only once with the given note. - // Note: Only the owner of the npk_m will be able to produce the nhk_app and compute this nullifier. - // The nullifier is unique to the note and THIS contract because the protocol siloes all nullifiers with - // the address of a contract it was emitted from. - // TODO(#7775): manually computing the hash and passing it to compute_nullifier func is not great as note could - // handle it on its own or we could make assert_note_existed_by return note_hash_for_nullification. - let note_hash_for_nullification = compute_note_hash_for_nullification(hinted_note); - let nullifier = hinted_note.note.compute_nullifier( - self.context, - hinted_note.owner, - note_hash_for_nullification, - ); - self.context.push_nullifier(nullifier); + // 4) Consume the claim of this note, ensuring the reward can be claimed only once with the given note. Each + // claim being tied to its owner results in unlinkability of the claim's nullifier and the underlying note + // hash via the owner's nullifier hiding key (`nhk`). + // + // Note: we're using the note's hash as the claim identifier, as it serves as a unique identifier (because we + // know the note is settled, and hence its note hash unique). This is not the same thing as nullifying the note + // however, which can only be done from the contract that emitted the note. From the point of view of the + // Crowdfunding contract, the note is still active. + let note_hash_for_nullification = + compute_confirmed_note_hash_for_nullification(confirmed_note); + self + .storage + .note_hash_claims + .at(note_hash_for_nullification) + .at(confirmed_note.owner) + .claim(); - // 5) Finally we mint the reward token to the sender of the transaction - self.enqueue(Token::at(self.storage.reward_token.read()).mint_to_public( + // 5) Finally we mint the reward token to the requested recipient + self.enqueue(Token::at(config.reward_token).mint_to_public( recipient, - hinted_note.note.value, + confirmed_note.note.value, )); } } diff --git a/noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr b/noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr index 96b0c9ffff2b..a2dad7d1e64d 100644 --- a/noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr +++ b/noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr @@ -3,13 +3,11 @@ use aztec::{ keys::getters::{get_nhk_app, get_public_keys, try_get_public_keys}, macros::notes::custom_note, messages::logs::partial_note::compute_partial_note_private_content_log, - note::note_interface::{NoteHash, NoteType}, + note::{note_interface::{NoteHash, NoteType}, utils::compute_note_nullifier}, oracle::random::random, protocol::{ address::AztecAddress, - constants::{ - DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT, - }, + constants::{DOM_SEP__NOTE_HASH, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT}, hash::poseidon2_hash_with_separator, traits::{Deserialize, Hash, Packable, Serialize, ToField}, }, @@ -65,10 +63,7 @@ impl NoteHash for NFTNote { let owner_npk_m = get_public_keys(owner).npk_m; let owner_npk_m_hash = owner_npk_m.hash(); let secret = context.request_nhk_app(owner_npk_m_hash); - poseidon2_hash_with_separator( - [note_hash_for_nullification, secret], - DOM_SEP__NOTE_NULLIFIER, - ) + compute_note_nullifier(note_hash_for_nullification, [secret]) } unconstrained fn compute_nullifier_unconstrained( @@ -80,10 +75,7 @@ impl NoteHash for NFTNote { let owner_npk_m = public_keys.npk_m; let owner_npk_m_hash = owner_npk_m.hash(); let secret = get_nhk_app(owner_npk_m_hash); - poseidon2_hash_with_separator( - [note_hash_for_nullification, secret], - DOM_SEP__NOTE_NULLIFIER, - ) + compute_note_nullifier(note_hash_for_nullification, [secret]) }) } } diff --git a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr index 55757d6289d0..201954b1c4af 100644 --- a/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr @@ -145,9 +145,9 @@ pub contract Orderbook { // Nullify the order such that it cannot be fulfilled again. We emit a nullifier instead of deleting the order // from public storage because we get no refund for resetting public storage to zero and just emitting - // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order). We use the `order_id` - // itself as the nullifier because this contract does not work with notes and hence there is no risk of - // colliding with a real note nullifier. + // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order). + // TODO(F-399): pushing a raw nullifier with no domain separator like we do here is unsafe: we should instead + // use something like a singleton `SingleUseClaim`. self.context.push_nullifier(order_id); // Enqueue the fulfillment to finalize both partial notes 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/app/token_blacklist_contract/src/types/transparent_note.nr b/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/types/transparent_note.nr index 805e63a72ccd..85f941e9fcdb 100644 --- a/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/types/transparent_note.nr +++ b/noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/types/transparent_note.nr @@ -1,13 +1,8 @@ use aztec::{ context::PrivateContext, macros::notes::custom_note, - note::note_interface::NoteHash, - protocol::{ - address::AztecAddress, - constants::{DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER}, - hash::poseidon2_hash_with_separator, - traits::Packable, - }, + note::{note_interface::NoteHash, utils::{compute_note_hash, compute_note_nullifier}}, + protocol::{address::AztecAddress, traits::Packable}, }; use std::mem::zeroed; @@ -29,8 +24,8 @@ impl NoteHash for TransparentNote { storage_slot: Field, randomness: Field, ) -> Field { - let inputs = self.pack().concat([storage_slot, randomness]); - poseidon2_hash_with_separator(inputs, DOM_SEP__NOTE_HASH) + let data = self.pack().concat([randomness]); + compute_note_hash(storage_slot, data) } // Computing a nullifier in a transparent note is not guarded by making secret a part of the nullifier preimage (as @@ -47,10 +42,7 @@ impl NoteHash for TransparentNote { _owner: AztecAddress, note_hash_for_nullification: Field, ) -> Field { - poseidon2_hash_with_separator( - [note_hash_for_nullification], - DOM_SEP__NOTE_NULLIFIER as Field, - ) + compute_note_nullifier(note_hash_for_nullification, []) } unconstrained fn compute_nullifier_unconstrained( diff --git a/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr b/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr index f12ae2e959ec..a72ccc13fefb 100644 --- a/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr +++ b/noir-projects/noir-contracts/contracts/protocol/aztec_sublib/src/state_vars/delayed_public_mutable.nr @@ -8,22 +8,122 @@ use crate::protocol::{ traits::Packable, }; -use crate::{ - context::{PrivateContext, PublicContext, UtilityContext}, - state_vars::StateVariable, - utils::WithHash, -}; - -/// Public mutable values with private read access. +use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars::StateVariable, utils::WithHash}; + +/// Mutable public values with private read access. +/// +/// This is an advanced public state variable, with no native Solidity equivalent. +/// +/// Like [`PublicMutable`](crate::state_vars::PublicMutable) it represents a public value of type `T` that can be +/// written to repeatedly, but with a key improvement: the current value can also be **read from a private contract +/// function**. +/// +/// This comes at the cost of extra restrictions on the state variable: writes do not come into effect immediately, +/// they must be **scheduled** to take place after some minimum delay. Reading from the state variable will therefore +/// return the previous value until some time passes, which is why this is a _delayed_ mutable variable. +/// +/// It is these delays that enable the capacity for reads from private contract functions, as they provide guarantees +/// regarding how long can some historical state observed at the anchor block be known to not change. +/// +/// Delays can be modified during the lifetime of the contract. +/// +/// ## Access Patterns +/// +/// The current value stored in a `DelayedPublicMutable` can be read from public contract functions, and writes can be +/// scheduled to happen in the future. +/// +/// Public contract functions can also schedule changes to the write delay, as well as inspect any already scheduled +/// value or delay changes. +/// +/// Private contract functions can read the **current** value of the state variable, but not past or scheduled values. +/// They cannot read the current delay, and they cannot schedule any kind of value change. +/// +/// ## Privacy +/// +/// The value stored in `DelayedPublicMutable` is fully public, as are all scheduled value and delay changes. +/// +/// Reads from a private contract function are almost fully private: the only observable effect is that they set the +/// transaction's `expiration_timestamp` property, possibly reducing the privacy set. See +/// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp). +/// +/// ## Use Cases +/// +/// These are mostly an extension of [`PublicMutable`](crate::state_vars::PublicMutable)'s, given that what this state +/// variable essentially achieves is to provide private reads to it. For example, it can be used for global contract +/// configuration (such as fees, access control, etc.) that users will need to access during private interactions. +/// +/// The key consideration is whether the enforced minimum delay on writes prevents using this state variable. In some +/// scenarios this restriction is incompatible with requirements (such as a token's total supply, which must always be +/// up to date), while in others the enhanced privacy might make the tradeoff acceptable (such as when dealing with +/// contract pauses or access control revocation, where a delay of some hours could be acceptable). +/// +/// Note that, just like in [`PublicMutable`](crate::state_vars::PublicMutable), the fact that the values are public +/// does not necessarily mean the actions that update these values must themselves be wholly public. To learn more, +/// see the notes there regarding usage of [`only_self`](crate::macros::functions::only_self). +/// +/// ## Choosing Delays +/// +/// A short delay reduces the most obvious downside of `DelayedPublicMutable`, and so it is natural to wish to make it +/// be as low as possible. It is therefore important to understand the tradeoffs involved in delay selection. +/// +/// A shorter delay will result in a lower `expiration_timestamp` property of transactions that privately read the +/// state variable, reducing its privacy set. If the delay is smaller than that of any other contract, then this +/// privacy leak might be large enough to uniquely identify those transactions that interact with the contract - fully +/// defeating the purpose of `DelayedPublicMutable`. +/// +/// Additionally, a lower `expiration_timestamp` obviously causes transactions to expire earlier, resulting in +/// multiple issues. Among others, this can make large transactions that take long to prove be unfeasible, restrict +/// users with slow proving devices, and force large transaction fees to guarantee fast inclusion. +/// +/// In practice, a delay of at least a couple hours is recommended. From a privacy point of view the optimal delay is +/// [`crate::protocol::constants::MAX_TX_LIFETIME`], which puts contracts in the same privacy set as those that do not +/// use `DelayedPublicMutable` at all. +/// +/// ## Examples +/// +/// Declaring a `DelayedPublicMutable` in the contract's [`storage`](crate::macros::storage::storage) struct +/// requires specifying the type `T` that is stored in the variable, along with the initial delay used when scheduling +/// value changes: +/// +/// ```noir +/// global PAUSE_CONTRACT_INITIAL_DELAY_S: u64 = 6 * 60 * 60; // 6 hours +/// global CHANGE_AUTHORIZATION_INITIAL_DELAY_S: u64 = 24 * 60 * 60; // 24 hours +/// +/// #[storage] +/// struct Storage { +/// paused: DelayedPublicMutable, +/// user_authorization: Map, C>, +/// } +/// ``` +/// +/// Note that this initial delay can be altered during the contract's lifetime via +/// [`DelayedPublicMutable::schedule_delay_change`]. +/// +/// ## Requirements +/// +/// The type `T` stored in the `DelayedPublicMutable` must implement the `Eq` and +/// [`Packable`](crate::protocol::traits::Packable) traits. +/// +/// ## Implementation Details +/// +/// This state variable stores more information in public storage than +/// [`PublicMutable`](crate::state_vars::PublicMutable), as it needs to keep track of the current and scheduled change +/// information for both the value and the delay - see +/// [`crate::protocol::delayed_public_mutable::DelayedPublicMutableValues`]. +/// +/// It also stores a hash of this entire configuration so that private reads can be performed in a single historical +/// public storage read - see [`crate::utils::WithHash`]. +/// +/// This results in a total of `N * 2 + 2` storage slots used, where `N` is the packing length of the stored type `T`. +/// This makes it quite important to ensure `T`'s implementation of [`Packable`](crate::protocol::traits::Packable) is +/// space-efficient. pub struct DelayedPublicMutable { context: Context, storage_slot: Field, } -// DelayedPublicMutable stores a value of type T that is: -// - publicly known (i.e. unencrypted) -// - mutable in public -// - readable in private with no contention +// We allocate `M + 1` slots because we're going to store a `WithHash>`, +// and `WithHash` increases the packing length by one. impl StateVariable for DelayedPublicMutable where DelayedPublicMutableValues: Packable, @@ -42,15 +142,47 @@ impl DelayedPublicMutable + /// t1`). The result is that the current value continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// This also means that it is possible to **cancel** a scheduled change by calling `schedule_value_change` with + /// the current value. + /// + /// ## Examples + /// + /// A public setter that authorizes a user: + /// ```noir + /// #[external("public")] + /// fn authorize_user(user: AztecAddress) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.user_authorization.at(user).schedule_value_change(true); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_value_change(self, new_value: T) where T: Packable, { - let _value_change = self.schedule_and_return_value_change(new_value); + let _ = self.schedule_and_get_value_change(new_value); } - pub fn schedule_and_return_value_change(self, new_value: T) -> ScheduledValueChange + /// Schedules a write to the current value, returning the scheduled entry. + pub fn schedule_and_get_value_change(self, new_value: T) -> ScheduledValueChange where T: Packable, { @@ -62,18 +194,64 @@ where // TODO: make this configurable https://github.com/AztecProtocol/aztec-packages/issues/5501 let timestamp_of_change = current_timestamp + current_delay; - value_change.schedule_change( - new_value, - current_timestamp, - current_delay, - timestamp_of_change, - ); + value_change.schedule_change(new_value, current_timestamp, current_delay, timestamp_of_change); self.write(value_change, delay_change); value_change } + /// Schedules a write to the current delay. + /// + /// This works just like [`schedule_value_change`](DelayedPublicMutable::schedule_value_change), except instead of + /// changing the value in the state variable, it changes the delay that will govern future invocations of that + /// function. + /// + /// The current delay does not immediately change. Once the current delay passes, + /// [`get_current_delay`](DelayedPublicMutable::get_current_delay) automatically begins to return `new_delay`, and + /// [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) begins using it. + /// + /// ## Multiple Scheduled Changes + /// + /// Only a **single** delay can be scheduled to become the new delay at a given point in time. Any prior scheduled + /// changes which have not yet become current are **replaced** with the new one and discarded. + /// + /// To illustrate this, consider a scenario at `t0` with a current delay `A`. A delay change to `B` is scheduled to + /// occur at `t1`. At some point _before_ `t1`, a second delay change to `C` is scheduled to occur at `t2` (`t2 > + /// t1`). The result is that the current delay continues to be `A` all the way until `t2`, at which point it + /// changes to `C`. + /// + /// ## Delays When Changing Delays + /// + /// A delay change cannot always be immediate: if it were, then it'd be possible to break `DelayedPublicMutable`'s + /// invariants by setting the delay to a very low or zero value and then scheduling a value change, resulting in a + /// new value becoming the current one earlier than was predictable based on the prior delay. This would prohibit + /// private reads, which is the reason for existence of this state variable. + /// + /// Instead, delay changes are themselves scheduled and delay so that the property mentioned above is preserved. + /// This results in delay increases and decreases being asymmetrical. + /// + /// If the delay is being decreased, then this requires a delay equal to the difference between the current and new + /// delay, so that a scheduled value change that occurred as the new delay came into effect would be scheduled for + /// the same timestamp as if no delay change had occurred. + /// + /// If the delay is being increased, then the new delay becomes effective immediately, as new value changes would + /// be scheduled for a timestamp that is further than the current delay. + /// + /// ## Examples + /// + /// A public setter that sets the pause delay: + /// ```noir + /// #[public] + /// fn set_pause_delay(delay: u64) { + /// assert_eq(self.storage.admin.read(), self.msg_sender(), "caller is not admin"); + /// self.storage.paused.schedule_delay_change(delay); + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SSTORE` AVM opcode is invoked `2 * N + 2` times, where `N` is `T`'s packed length. pub fn schedule_delay_change(self, new_delay: u64) where T: Packable, @@ -84,9 +262,39 @@ where delay_change.schedule_change(new_delay, current_timestamp); + // We can't just update the `ScheduledDelayChange`, we need to update the entire storage because we need to + // also recompute and write the hash. + // We _could_ just read everything, update the hash and `ScheduledDelayChange` but not overwrite the + // `ScheduledValueChange`, resulting in fewer storage writes, but that would require careful handling of + // storage slots and `WithHash`'s internal layout, which is not worth doing at this point. self.write(self.read_value_change(), delay_change); } + /// Returns the current value. + /// + /// If [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) has never been called, then this + /// returns the default empty public storage value, which is all zeroes - equivalent to `let t = + /// T::unpack(std::mem::zeroed());`. + /// + /// It is not possible to detect if a `DelayedPublicMutable` has ever been initialized or not other than by testing + /// for the zero sentinel value. For a more robust solution, store an `Option` in the `DelayedPublicMutable`. + /// + /// Use [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) to instead get the last value that was + /// scheduled to become the current one (which will equal the current value if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns a user's authorization status: + /// ```noir + /// #[external("public")] + /// fn is_authorized(user: AztecAddress) -> bool { + /// self.storage.user_authorization.at(user).get_current_value() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked `2 * N + 1` times, where `N` is `T`'s packed length. pub fn get_current_value(self) -> T where T: Packable, @@ -97,6 +305,30 @@ where value_change.get_current_at(current_timestamp) } + /// Returns the current delay. + /// + /// This is the delay that would be used by [`schedule_value_change`](DelayedPublicMutable::schedule_value_change) + /// if it were called in the current transaction. + /// + /// If [`schedule_delay_change`](DelayedPublicMutable::schedule_delay_change) has never been called, then this + /// returns the `InitialDelay` used in the [`storage`](crate::macros::storage::storage) struct. + /// + /// Use [`get_scheduled_delay`](DelayedPublicMutable::get_scheduled_delay) to instead get the last delay that was + /// scheduled to become the current one (which will equal the current delay if the delay has already passed). + /// + /// ## Examples + /// + /// A public getter that returns the pause delay: + /// ```noir + /// #[external("public")] + /// fn get_pause_delay() -> u64 { + /// self.storage.paused.get_current_delay() + /// } + /// ``` + /// + /// ## Cost + /// + /// The `SLOAD` AVM opcode is invoked a single time, regardless of `T`. pub fn get_current_delay(self) -> u64 where T: Packable, @@ -105,6 +337,7 @@ where self.read_delay_change().get_current(current_timestamp) } + /// Returns the last scheduled value and timestamp of change. pub fn get_scheduled_value(self) -> (T, u64) where T: Packable, @@ -112,6 +345,7 @@ where self.read_value_change().get_scheduled() } + /// Returns the last scheduled delay and timestamp of change. pub fn get_scheduled_delay(self) -> (u64, u64) where T: Packable, @@ -123,6 +357,8 @@ where where T: Packable, { + // We don't read ScheduledValueChange directly by having it implement Packable because ScheduledValueChange and + // ScheduledDelayChange are packed together (sdc and svc.timestamp_of_change are stored in the same slot). let packed = self.context.storage_read(self.storage_slot); unpack_value_change::::N>(packed) } @@ -131,18 +367,26 @@ where where T: Packable, { + // Since all ScheduledDelayChange member are packed into a single field, we can read a single storage slot here + // and skip the ones that correspond to ScheduledValueChange members. We are abusing the fact that the field + // containing the ScheduledDelayChange data is the first one in the storage layout - otherwise we'd need to + // offset the storage slot to get the position where it'd land. We don't read ScheduledDelayChange directly by + // having it implement Packable because ScheduledValueChange and ScheduledDelayChange are packed together (sdc + // and svc.timestamp_of_change are stored in the same slot). let packed = self.context.storage_read(self.storage_slot); unpack_delay_change::(packed) } - fn write( - self, - value_change: ScheduledValueChange, - delay_change: ScheduledDelayChange, - ) + fn write(self, value_change: ScheduledValueChange, delay_change: ScheduledDelayChange) where T: Packable, { + // Whenever we write to public storage, we write both the value change and delay change to storage at once. We + // do so by wrapping them in a single struct (`DelayedPublicMutableValues`). Then we wrap the resulting struct + // in `WithHash`. Wrapping in `WithHash` makes for more costly writes but it also makes private proofs much + // simpler because they only need to produce a historical proof for the hash, which results in a single + // inclusion proof (as opposed to 4 in the best case scenario in which T is a single field). Private delayed + // public mutable reads are assumed to be much more frequent than public writes, so this tradeoff makes sense. let values = WithHash::new(DelayedPublicMutableValues::new(value_change, delay_change)); self.context.storage_write(self.storage_slot, values); @@ -153,25 +397,107 @@ impl DelayedPublicMutable` in the `DelayedPublicMutable`. + /// + /// ## Privacy + /// + /// This function does leak some privacy, though in a subtle way. Understanding this is key to understanding how to + /// use `DelayedPublicMutable` in a privacy-preserving way. + /// + /// Private reads are based on a historical public storage read at the anchor block (i.e. + /// [`crate::history::storage::public_storage_historical_read`]). `DelayedPublicMutable` is able to provide + /// guarantees about values read in the past remaining the state variable's current value into the future due to + /// the existence of delays when scheduling writes. It then sets the `expiration_timestamp` property of the current + /// transaction (see + /// [`PrivateContext::set_expiration_timestamp`](crate::context::PrivateContext::set_expiration_timestamp)) to + /// ensure that the transaction can only be included in a block **prior** to the state variable's value changing. + /// In other words, it knows some facts about the near future up until some time horizon, and then makes sure that + /// it doesn't act on this knowledge past said moment. + /// + /// Because the `expiration_timestamp` property is part of the transaction's public information, any mutation to + /// this value could result in transaction fingerprinting. Note that multiple contracts may set this value during a + /// transaction: it is the smallest (most restrictive) timestamp that will be used. + /// + /// If the state variable **does not** have any value changes scheduled, then the timestamp will be set to that of + /// the anchor block plus the current delay. If multiple contracts use the same delay for their + /// `DelayedPublicMutable` state variables, then these will all be in the same privacy set. + /// + /// If the state variable **does** have a value change scheduled, then the timestamp will be set to equal the time + /// at which the current value will change, i.e. the one + /// [`get_scheduled_value`](DelayedPublicMutable::get_scheduled_value) returns - which is public information. This + /// results in an unavoidable privacy leak of any transactions in which a contract privately reads a + /// `DelayedPublicMutable` that will change soon. + /// + /// Transactions that do not read from a `DelayedPublicMutable` are part of a privacy set in which the + /// `expiration_timestamp` is set to their anchor block plus [`crate::protocol::constants::MAX_TX_LIFETIME`], + /// making this the most privacy-preserving delay. The less frequent said value changes are, the more private the + /// contract is. Wallets can also then choose to further lower this timestamp to make it less obvious that their + /// transactions are interacting with this soon-to-change variable. + /// + /// ## Examples + /// + /// A private action that requires authorization: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert( + /// self.storage.user_authorization.at(self.msg_sender()).get_current_value(), + /// "caller is not authorized" + /// ); + /// + /// // do the action + /// } + /// ``` + /// + /// A private action that can be paused: + /// ```noir + /// #[external("private")] + /// fn do_action() { + /// assert(!self.storage.paused.get_current_value(), "contract is paused"); + /// + /// // do the action + /// } + /// ``` + /// + /// ## Cost + /// + /// This function performs a single merkle tree inclusion proof, which is in the order of 4k gates. pub fn get_current_value(self) -> T where T: Packable, { // When reading the current value in private we construct a historical state proof for the public value. + // However, since this value might change, we must constrain the maximum transaction timestamp as this proof + // will only be valid for the time we can ensure the value will not change, which will depend on the current + // delay and any scheduled delay changes. let (value_change, delay_change, anchor_timestamp) = self.anchor_read_from_public_storage(); + // We use the effective minimum delay as opposed to the current delay at the anchor block's timestamp as this + // one also takes into consideration any scheduled delay changes. For example, consider a scenario in which at + // timestamp `x` the current delay was 86400 seconds (1 day). We may naively think that the earliest we could + // change the value would be at timestamp `x + 86400` by scheduling immediately after the anchor block's + // timestamp, i.e. at timestamp `x + 1`. But if there was a delay change scheduled for timestamp `y` to reduce + // the delay to 43200 seconds (12 hours), then if a value change was scheduled at timestamp `y` it would go + // into effect at timestamp `y + 43200`, which is earlier than what we'd expect if we only considered the + // current delay. let effective_minimum_delay = delay_change.get_effective_minimum_delay_at(anchor_timestamp); let time_horizon = value_change.get_time_horizon(anchor_timestamp, effective_minimum_delay); - // We prevent this transaction from being included in any timestamp after the time horizon. + // We prevent this transaction from being included in any timestamp after the time horizon, ensuring that the + // historical public value matches the current one, since it can only change after the horizon. self.context.set_expiration_timestamp(time_horizon); value_change.get_current_at(anchor_timestamp) } - fn anchor_read_from_public_storage( - self, - ) -> (ScheduledValueChange, ScheduledDelayChange, u64) + fn anchor_read_from_public_storage(self) -> (ScheduledValueChange, ScheduledDelayChange, u64) where T: Packable, { diff --git a/noir-projects/noir-contracts/contracts/protocol/contract_class_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/protocol/contract_class_registry_contract/src/main.nr index 5668b3dc255c..2a9286394448 100644 --- a/noir-projects/noir-contracts/contracts/protocol/contract_class_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/protocol/contract_class_registry_contract/src/main.nr @@ -93,6 +93,7 @@ pub contract ContractClassRegistry { // Emit the contract class id as a nullifier: // - to demonstrate that this contract class hasn't been published before // - to enable apps to read that this contract class has been published. + // We use no domain separators because these are the only nullifiers this contract uses. context.push_nullifier(contract_class_id.to_field()); // Broadcast class info including public bytecode diff --git a/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr index 75de07726984..bc9a5eed0b94 100644 --- a/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr @@ -139,6 +139,7 @@ pub contract ContractInstanceRegistry { let address = AztecAddress::compute(public_keys, partial_address); // Emit address as nullifier: prevents duplicate deployment and proves publication. + // We use no domain separators because these are the only nullifiers this contract uses. context.push_nullifier(address.to_field()); // Broadcast deployment event. Uses non-standard serialization (2 fields per point, @@ -210,7 +211,7 @@ pub contract ContractInstanceRegistry { let scheduled_value_update = storage .updated_class_ids .at(address) - .schedule_and_return_value_change(new_contract_class_id); + .schedule_and_get_value_change(new_contract_class_id); let (prev_contract_class_id, timestamp_of_change) = scheduled_value_update.get_previous(); let event = ContractInstanceUpdated { 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/noir-projects/noir-contracts/contracts/test/spam_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test/spam_contract/src/main.nr index 9d26399f290e..7519a67fd2f2 100644 --- a/noir-projects/noir-contracts/contracts/test/spam_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test/spam_contract/src/main.nr @@ -6,12 +6,10 @@ pub contract Spam { use aztec::{ macros::{functions::{external, only_self}, storage::storage}, messages::message_delivery::MessageDelivery, - protocol::{ - constants::{ - DOM_SEP__NOTE_NULLIFIER, MAX_NOTE_HASHES_PER_CALL, MAX_NULLIFIERS_PER_CALL, - MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, - }, - hash::poseidon2_hash_with_separator, + note::utils::compute_note_nullifier, + protocol::constants::{ + MAX_NOTE_HASHES_PER_CALL, MAX_NULLIFIERS_PER_CALL, + MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, }, state_vars::{Map, Owned, PublicMutable}, }; @@ -34,10 +32,7 @@ pub contract Spam { for i in 0..MAX_NULLIFIERS_PER_CALL { if (i < nullifier_count) { - self.context.push_nullifier(poseidon2_hash_with_separator( - [nullifier_seed, i as Field], - DOM_SEP__NOTE_NULLIFIER as Field, - )); + self.context.push_nullifier(compute_note_nullifier(nullifier_seed, [i as Field])); } } diff --git a/noir-projects/noir-contracts/contracts/test/test_contract/src/test_note.nr b/noir-projects/noir-contracts/contracts/test/test_contract/src/test_note.nr index a0d4d6b00dad..a7a7a6260547 100644 --- a/noir-projects/noir-contracts/contracts/test/test_contract/src/test_note.nr +++ b/noir-projects/noir-contracts/contracts/test/test_contract/src/test_note.nr @@ -1,11 +1,8 @@ use aztec::{ context::PrivateContext, macros::notes::custom_note, - note::note_interface::NoteHash, - protocol::{ - address::AztecAddress, constants::DOM_SEP__NOTE_HASH, hash::poseidon2_hash_with_separator, - traits::Packable, - }, + note::{note_interface::NoteHash, utils::compute_note_hash}, + protocol::{address::AztecAddress, traits::Packable}, }; /// A note used only for testing purposes. @@ -24,8 +21,8 @@ impl NoteHash for TestNote { ) -> Field { // The note is inserted into the state in the Test contract so we provide a real compute_note_hash // implementation. - let inputs = self.pack().concat([storage_slot, randomness]); - poseidon2_hash_with_separator(inputs, DOM_SEP__NOTE_HASH) + let data = self.pack().concat([randomness]); + compute_note_hash(storage_slot, data) } fn compute_nullifier( diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr b/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr index 3c1ac469c9af..38e063a62be9 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr @@ -49,19 +49,20 @@ impl AztecAddress { Self { inner: 0 } } - /// Returns an address's `AddressPoint`, which can be used to create shared secrets with the owner - /// of the address. If the address is invalid (i.e. it is not a properly derived Aztec address), then this - /// returns `Option::none()`, and no shared secrets can be created. - pub fn to_address_point(self) -> Option { - // We compute the address point by taking our address as x, and then solving for y in the - // equation which defines the grumpkin curve: - // y^2 = x^3 - 17; x = address - let x = self.inner; - let y_squared = x * x * x - 17; + /// Returns `true` if the address is valid. + /// + /// An invalid address is one that can be proven to not be correctly derived, meaning it contains no contract code, + /// public keys, etc., and can therefore not receive messages nor execute calls. + pub fn is_valid(self) -> bool { + self.get_y().is_some() + } - // An invalid AztecAddress is one for which no y coordinate satisfies the curve equation, which we'll - // identify by proving that the square root of y_squared does not exist. - sqrt(y_squared).map(|y| { + /// Returns an address's [`AddressPoint`]. + /// + /// This can be used to create shared secrets with the owner of the address. If the address is invalid (see + /// [`AztecAddress::is_valid`]) then this returns `Option::none()`, and no shared secrets can be created. + pub fn to_address_point(self) -> Option { + self.get_y().map(|y| { // If we get a negative y coordinate (y > (r - 1) / 2), we swap it to the // positive one (where y <= (r - 1) / 2) by negating it. let final_y = if Self::is_positive(y) { y } else { -y }; @@ -83,6 +84,24 @@ impl AztecAddress { y.lt(MID_PLUS_1) } + /// Returns one of the two possible y-coordinates. + /// + /// Not all `AztecAddresses` are valid, in which case there is no corresponding y-coordinate. This returns + /// `Option::none()` for invalid addresses. + /// + /// An `AztecAddress` is defined by an x-coordinate, for which two y-coordinates exist as solutions to the curve + /// equation. This function returns either of them. Note that an [`AddressPoint`] must **always** have a positive + /// y-coordinate - if trying to obtain the underlying point use [`AztecAddress::to_address_point`] instead. + fn get_y(self) -> Option { + // We compute the address point by taking our address as x, and then solving for y in the + // equation which defines the grumpkin curve: + // y^2 = x^3 - 17; x = address + let x = self.inner; + let y_squared = x * x * x - 17; + + sqrt(y_squared) + } + pub fn compute(public_keys: PublicKeys, partial_address: PartialAddress) -> AztecAddress { // // address = address_point.x @@ -265,6 +284,10 @@ fn serde() { fn to_address_point_valid() { // x = 8 where x^3 - 17 = 512 - 17 = 495, which is a residue in this field let address = AztecAddress { inner: 8 }; + + assert(address.get_y().is_some()); // We don't bother checking the result of get_y as it is only used internally + assert(address.is_valid()); + let maybe_point = address.to_address_point(); assert(maybe_point.is_some()); @@ -279,7 +302,10 @@ fn to_address_point_valid() { #[test] unconstrained fn to_address_point_invalid() { // x = 3 where x^3 - 17 = 27 - 17 = 10, which is a non-residue in this field - let address = AztecAddress { inner: 3 }; // - let maybe_point = address.to_address_point(); - assert(maybe_point.is_none()); + let address = AztecAddress { inner: 3 }; + + assert(address.get_y().is_none()); + assert(!address.is_valid()); + + assert(address.to_address_point().is_none()); } 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 116fb2002d64..fb1a31032f5e 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -634,16 +634,49 @@ pub global MAX_PUBLIC_CALLS_TO_UNIQUE_CONTRACT_CLASS_IDS: u32 = 21; // See ./constants_tests.nr for how the values are generated. // --------------------------------------------------------------- -// Note hash generator index which can be used by custom implementations of NoteHash::compute_note_hash +/// Domain separator for note hashes. +/// +/// This is not technically a protocol constant as note hashes are computed by each contract. pub global DOM_SEP__NOTE_HASH: u32 = 116501019; -pub global DOM_SEP__NOTE_HASH_NONCE: u32 = 1721808740; -pub global DOM_SEP__UNIQUE_NOTE_HASH: u32 = 226850429; + +/// Domain separator for siloed note hashes. +/// +/// Used by [`crate::hash::compute_siloed_note_hash`]. pub global DOM_SEP__SILOED_NOTE_HASH: u32 = 3361878420; +/// Domain separator for unique note hashes. +/// +/// Used by [`crate::hash::compute_unique_note_hash`]. +pub global DOM_SEP__UNIQUE_NOTE_HASH: u32 = 226850429; + +/// Domain separator for nonces. +/// +/// Used by [`crate::hash::compute_note_hash_nonce`]. +pub global DOM_SEP__NOTE_HASH_NONCE: u32 = 1721808740; + +/// Domain separator for `SingleUseClaim` nullifiers. +/// +/// This is not technically a protocol constant as these nullifiers are computed by each contract. +pub global DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER: u32 = 1465998995; + +/// Domain separator for note nullifiers. +/// +/// This is not technically a protocol constant as note nullifiers are computed by each contract. pub global DOM_SEP__NOTE_NULLIFIER: u32 = 50789342; -pub global DOM_SEP__MESSAGE_NULLIFIER: u32 = 3754509616; + +/// Domain separator for siloed nullifiers. +/// +/// Used by [`crate::hash::compute_siloed_nullifier`]. pub global DOM_SEP__SILOED_NULLIFIER: u32 = 57496191; +/// Domain separator for L1 to L2 message nullifiers. +/// +/// This is not technically a protocol constant as message nullifiers are computed by each contract. +pub global DOM_SEP__MESSAGE_NULLIFIER: u32 = 3754509616; + +/// Domain separator for private log tags. +/// +/// Used by [`crate::hash::compute_siloed_private_log_first_field`]. pub global DOM_SEP__PRIVATE_LOG_FIRST_FIELD: u32 = 2769976252; pub global DOM_SEP__PUBLIC_LEAF_SLOT: u32 = 1247650290; @@ -710,13 +743,30 @@ pub global DOM_SEP__CIPHERTEXT_FIELD_MASK: u32 = 1870492847; pub global DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT: u32 = 623934423; // --------------------------------------------------------------- -// TODO: consider moving these to aztec-nr +// TODO(F-397): move these to aztec-nr, along with note hash, note nullifier, message nullifier and single use claim +// nullifier. +/// Domain separator for state variable initialization. +/// +/// Should not be reused for a given storage slot. 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; + +/// Domain separator for transaction nullifiers. +/// +/// Used to produce cancellable (replaceable) transactions. +/// +/// This is not technically a protocol constant as cancellable transactions are an account contract feature. pub global DOM_SEP__TX_NULLIFIER: u32 = 1025801951; + +/// Domain separator for account contract payloads. +/// +/// Used to check for authorization to execute a payload. +/// +/// This is not technically a protocol constant as payload authorization is an account contract feature. 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 6a5048b6926f..af72df93e106 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 @@ -18,9 +18,10 @@ use crate::{ 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, - DOM_SEP__TX_NULLIFIER, DOM_SEP__TX_REQUEST, DOM_SEP__UNIQUE_NOTE_HASH, - NULL_MSG_SENDER_CONTRACT_ADDRESS, SIDE_EFFECT_MASKING_ADDRESS, TX_START_PREFIX, + DOM_SEP__SILOED_NULLIFIER, DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER, DOM_SEP__SYMMETRIC_KEY, + DOM_SEP__SYMMETRIC_KEY_2, DOM_SEP__TSK_M, DOM_SEP__TX_NULLIFIER, DOM_SEP__TX_REQUEST, + DOM_SEP__UNIQUE_NOTE_HASH, NULL_MSG_SENDER_CONTRACT_ADDRESS, SIDE_EFFECT_MASKING_ADDRESS, + TX_START_PREFIX, }, hash::poseidon2_hash_bytes, traits::{FromField, ToField}, @@ -140,6 +141,10 @@ fn hashed_values_match_derived() { tester.assert_dom_sep_matches_derived(DOM_SEP__UNIQUE_NOTE_HASH, "unique_note_hash"); tester.assert_dom_sep_matches_derived(DOM_SEP__SILOED_NOTE_HASH, "siloed_note_hash"); tester.assert_dom_sep_matches_derived(DOM_SEP__NOTE_NULLIFIER, "note_nullifier"); + tester.assert_dom_sep_matches_derived( + DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER, + "single_use_claim_nullifier", + ); tester.assert_dom_sep_matches_derived(DOM_SEP__SILOED_NULLIFIER, "siloed_nullifier"); tester.assert_dom_sep_matches_derived( DOM_SEP__PRIVATE_LOG_FIRST_FIELD, diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr b/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr index 33fb026d8ae8..d29c3d5f4299 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr @@ -1,5 +1,4 @@ -// Traits need to be first for the #[derive(...)] macro to work. -pub mod traits; +//! Types and constants related to the Aztec protocol. // The modules below are in a specific order that makes derivation of serde traits work. pub mod address; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr b/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr index 95929e3f8389..4ca42ab7ed99 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/meta/mod.nr @@ -97,7 +97,7 @@ comptime fn get_where_trait_clause(s: TypeDefinition, trait_name: Quoted) -> Quo } } -/// Generates a `Packable` trait implementation for a given struct `s`. +/// Generates a [`Packable`](crate::traits::Packable) trait implementation for a given struct `s`. /// /// # Arguments /// * `s` - The struct type definition to generate the implementation for diff --git a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts index 1b99f2b3657e..68711711b23b 100644 --- a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts +++ b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts @@ -208,7 +208,7 @@ describe('e2e_crowdfunding_and_claim', () => { // docs:start:local-tx-fails await expect( claimContract.methods.claim(anotherDonationNote, donorAddress).send({ from: unrelatedAddress }), - ).rejects.toThrow('hinted_note.owner == self.msg_sender()'); + ).rejects.toThrow('confirmed_note.owner == self.msg_sender()'); // docs:end:local-tx-fails }); diff --git a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts index edf5e87ea101..e82ca7a4003f 100644 --- a/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts +++ b/yarn-project/pxe/src/contract_function_simulator/oracle/private_execution.test.ts @@ -277,7 +277,7 @@ describe('Private Execution test suite', () => { const computeNoteHash = (note: Note, owner: AztecAddress, storageSlot: Fr, randomness: Fr) => { // We're assuming here that the note hash function is the default one injected by the #[note] macro. return poseidon2HashWithSeparator( - [...note.items, owner.toField(), storageSlot, randomness], + [storageSlot, ...note.items, owner.toField(), randomness], DomainSeparator.NOTE_HASH, ); };