From 32600fcc0adb09e19f44905da90e2f83dfc40195 Mon Sep 17 00:00:00 2001 From: Alejo Amiras Date: Wed, 19 Aug 2026 14:20:09 +0000 Subject: [PATCH 1/3] docs: surface known trust boundaries in Vault, MultiToken, Dripper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documentation follow-ups from the 2026-08 security audit. No contract logic changes. - Vault (main.nr): a prominent block comment at the top records that the ~15 `Order matters: … reentrancy` orderings are necessary but NOT sufficient — they do not hold when the asset or shares token has an ARC-403 hook, because the hook runs inside the transfer before the balance moves (audit F-001/F-002). The recurring inline phrase "to neutralize ARC-403 reentrancy" asserted a guarantee the code does not provide; corrected to point at that note. - MultiToken (README): add a status warning. It was the only token contract with no per-file caveat, yet it carries the commitment trust-model issue (a commitment binds neither id nor amount, so it is not a payment guarantee). Kept severity-accurate: it references the repo-wide unaudited status rather than implying it is as unfinished as the Vault. - Dripper (README): the existing "dev/testing only" note did not name the mechanism. Sharpened to state it is an uncapped, permissionless minter whose sole safety boundary is never being a valuable token's minter (audit F-006). Validated: aztec-nargo fmt --check clean, aztec compile OK. --- src/dripper/README.md | 3 +- src/multitoken_contract/README.md | 3 ++ src/vault_contract/src/main.nr | 53 +++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/dripper/README.md b/src/dripper/README.md index a686ebca..afed852b 100644 --- a/src/dripper/README.md +++ b/src/dripper/README.md @@ -2,7 +2,8 @@ The `Dripper` contract provides a convenient faucet mechanism for minting tokens into private or public balances. Anyone can easily invoke the functions below to request tokens for testing or development purposes. -> **Note**: This contract is designed for development and testing environments only. Do not use in production. As a dev utility rather than a standard, it is intentionally outside the repository's automated test scope. +> [!WARNING] +> The Dripper is an **uncapped, permissionless minter**: `drip_to_public` / `drip_to_private` let *anyone* mint *any* amount (up to `u64::MAX` per call, repeatable) of any token for which the Dripper is the configured `minter`. Its only safety boundary is external — it must **never be granted `minter` on a token that holds real value**, on any network. It is a development/testing faucet only, and as a dev utility rather than a standard it is intentionally outside the repository's automated test scope. ## Public Functions diff --git a/src/multitoken_contract/README.md b/src/multitoken_contract/README.md index 5b94c080..6036d073 100644 --- a/src/multitoken_contract/README.md +++ b/src/multitoken_contract/README.md @@ -4,6 +4,9 @@ The `MultiToken` contract implements an ERC-1155-like multi-token with Aztec-spe Compared to the single-asset [`Token`](../token_contract/README.md), every balance-changing function takes an extra `id: Field` selecting the token, there is no `decimals` and no `total_supply`, and the on-chain event is `TransferSingle` (ERC-1155 naming) instead of `Transfer`. +> [!WARNING] +> Like everything in this repository, `MultiToken` is **experimental, unaudited software** (see the repo-level [Security Status](../../README.md#️-security-status-unaudited)). One behaviour in particular is easy to misuse: a transfer commitment does **not** bind the token id or amount — the completer chooses both. This is intentional, but it means a commitment is **not a payment guarantee**. Read the [Commitment trust model](#commitment-trust-model) before using one in an escrow or marketplace flow. + ## ARC-403: Authorization Hook Like `Token`, this contract implements the optional ARC-403 authorization hook: when an `auth_contract` is configured, every transfer and burn calls it before mutating balances, and the operation reverts if the hook reverts. If `auth_contract` is the zero address, the hook is disabled and the token behaves as a plain multi-token. The interface is **id-bearing** — the hook receives the token id so policies can differ per id: diff --git a/src/vault_contract/src/main.nr b/src/vault_contract/src/main.nr index c5ea4695..0db56f24 100644 --- a/src/vault_contract/src/main.nr +++ b/src/vault_contract/src/main.nr @@ -2,6 +2,27 @@ pub mod test; use aztec::macros::aztec; +// ============================================================================ +// ⚠️ ARC-403 reentrancy limitation — READ BEFORE RELYING ON THE ORDERING BELOW +// ---------------------------------------------------------------------------- +// Throughout this contract, `// Order matters:` comments arrange asset transfers +// and share mint/burns so that any callback would observe either fully-pre- or +// fully-post-operation state. That reasoning is ONLY sound if a token transfer +// is indivisible. It is NOT when the asset or shares token has an ARC-403 +// authorization hook configured: the hook runs *inside* the token transfer, +// BEFORE the balance actually moves (see token_contract `_call_auth_*`, invoked +// ahead of the balance write). A hooked token therefore hands control to the +// authorization contract while this vault is mid-operation — the exact +// intermediate state the ordering is written to prevent — and a reentrant vault +// call can read a share price no completed operation would produce. +// +// The ordering below is thus necessary but NOT sufficient: it does not protect +// a vault whose asset or shares token carries a non-zero `auth_contract`. +// Only wrap tokens with no hook, or a fully trusted one. This is a known, +// unresolved exposure (security audit 2026-08, findings F-001/F-002); see the +// Vault README warning. The per-site comments below are kept for their ordering +// intent but should be read against this limitation. +// ============================================================================ #[aztec] pub contract Vault { use aztec::{ @@ -149,7 +170,7 @@ pub contract Vault { _convert_to_shares(assets, total_assets, total_supply, vault_offset, ROUND_DOWN); assert(shares > 0, "Zero shares, insufficient assets"); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_public_to_public( from, @@ -209,7 +230,7 @@ pub contract Vault { let shares_token = self.storage.shares.read(); _validate_from_private::<5>(self.context, from); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_private_to_public( from, @@ -235,7 +256,7 @@ pub contract Vault { let asset_token = self.storage.asset.read(); _validate_from_private::<4>(self.context, from); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_private_to_public( from, @@ -312,7 +333,7 @@ pub contract Vault { let partial_note = self.call(Token::at(shares_token).initialize_transfer_commitment(to, self.address)); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_private_to_public( from, @@ -361,7 +382,7 @@ pub contract Vault { let vault_offset = self.storage.vault_offset.read(); let assets = _convert_to_assets(shares, total_assets, total_supply, vault_offset, ROUND_UP); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_public_to_public( from, @@ -441,7 +462,7 @@ pub contract Vault { let asset_commitment = self.call(Token::at(asset_token).initialize_transfer_commitment(from, self.address)); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take max_assets from the sender self.call(Token::at(asset_token).transfer_private_to_public( from, @@ -482,7 +503,7 @@ pub contract Vault { let asset_commitment = self.call(Token::at(asset_token).initialize_transfer_commitment(from, self.address)); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take max_assets from the sender self.call(Token::at(asset_token).transfer_private_to_public( from, @@ -524,7 +545,7 @@ pub contract Vault { let vault_offset = self.storage.vault_offset.read(); let shares = _convert_to_shares(assets, total_assets, total_supply, vault_offset, ROUND_UP); - // Order matters: burn before transferring to neutralize ARC-403 reentrancy. + // Order matters: burn before transferring to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Burn the sender's shares on the shares token self.call(Token::at(shares_token).burn_public(from, shares, nonce)); @@ -548,7 +569,7 @@ pub contract Vault { let asset_token = self.storage.asset.read(); _validate_from_private::<4>(self.context, from); - // Order matters: burn after calculating shares in public to neutralize ARC-403 reentrancy. + // Order matters: burn after calculating shares in public to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Calculate and burn the sender's shares self.enqueue_self.settle_withdraw_public_to_private_internal(from, assets, nonce); @@ -579,7 +600,7 @@ pub contract Vault { // Burn shares from the sender's private balance on the shares token self.call(Token::at(shares_token).burn_private(from, shares, nonce)); - // Order matters: transfer after burning to neutralize ARC-403 reentrancy. + // Order matters: transfer after burning to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Validate that the shares-assets ratio is correct self.enqueue_self.settle_withdraw_private_to_private_internal(assets, shares); @@ -653,7 +674,7 @@ pub contract Vault { // Burn max_shares from the sender's private balance on the shares token self.call(Token::at(shares_token).burn_private(from, max_shares, nonce)); - // Order matters: transfer after burning to neutralize ARC-403 reentrancy. + // Order matters: transfer after burning to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Burn the correct amount of shares // Any excess amount of shares is sent back to the sender via commitment // Reverts if the amount of shares required is greater than max_shares @@ -690,7 +711,7 @@ pub contract Vault { let assets = _convert_to_assets(shares, total_assets, total_supply, vault_offset, ROUND_DOWN); - // Order matters: burn before transferring to neutralize ARC-403 reentrancy. + // Order matters: burn before transferring to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Burn the sender's shares on the shares token self.call(Token::at(shares_token).burn_public(from, shares, nonce)); @@ -744,7 +765,7 @@ pub contract Vault { // Burn shares from the sender's private balance on the shares token self.call(Token::at(shares_token).burn_private(from, shares, nonce)); - // Order matters: transfer after burning to neutralize ARC-403 reentrancy. + // Order matters: transfer after burning to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Transfers any outstanding assets without revealing the recipient // Reverts if min_assets is greater than allowed self.enqueue_self.settle_redeem_private_to_private_exact_internal( @@ -781,7 +802,7 @@ pub contract Vault { let asset_commitment = self.call(Token::at(asset_token).initialize_transfer_commitment(to, self.address)); - // Order matters: transfer after burning to neutralize ARC-403 reentrancy. + // Order matters: transfer after burning to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Burns shares and transfers any outstanding assets without revealing the recipient // Reverts if min_assets is greater than allowed self.enqueue_self.settle_redeem_public_to_private_exact_internal( @@ -1081,7 +1102,7 @@ pub contract Vault { assert(shares <= max_shares, "Too many shares requested"); - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_public_to_public( from, @@ -1222,7 +1243,7 @@ pub contract Vault { _convert_to_shares(assets, total_assets, total_supply, vault_offset, ROUND_DOWN); let outstanding_shares = max_shares - min_shares; // Reverts with underflow if invalid - // Order matters: transfer before minting to neutralize ARC-403 reentrancy. + // Order matters: transfer before minting to order effects safely (necessary but NOT sufficient against a hooked token — see the ARC-403 reentrancy note at the top of this contract). // Take the assets from the sender self.call(Token::at(asset_token).transfer_public_to_public( from, From 0090873539dc07183143a5cafcb9db81c67c5a84 Mon Sep 17 00:00:00 2001 From: Alejo Amiras Date: Wed, 19 Aug 2026 14:48:02 +0000 Subject: [PATCH 2/3] feat: add cancel_authwit to Token, NFT and MultiToken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream-parity function present in every upstream reference token contract but absent from all three of ours. Without it, a private authwit that has been granted but not yet consumed cannot be revoked. Ported verbatim from the upstream Token/NFT contracts (aztec-packages v5.1.0): #[external("private")] fn cancel_authwit(inner_hash: Field) { let on_behalf_of = self.msg_sender(); let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash); self.context.push_nullifier_unsafe(nullifier); } Cancellation pre-emits the same authwit nullifier the consume path emits, so a later authwit-gated call fails with a duplicate nullifier. Three tests per contract: the negative case (cancel then attempt to consume -> duplicate nullifiers), a positive control (same flow without the cancel succeeds, attributing the failure to the cancel), and caller isolation (a foreign account cancelling with the owner's exact inner hash does NOT revoke the owner's authwit, since the nullifier is bound to msg_sender). READMEs list the new function. Additive: a new selector, no existing ABI touched. Validated: token 84 / nft 64 / multitoken 80 Noir tests, aztec compile OK. Codex adversarial review: safe and upstream-equivalent; its two suggestions (tighten the negative assertion, add the caller-isolation test) are applied — note its proposed error string 'already present' was empirically wrong, the matchable revert is 'duplicate nullifiers'. Co-Authored-By: Claude Fable 5 --- src/multitoken_contract/README.md | 1 + src/multitoken_contract/src/main.nr | 12 ++ src/multitoken_contract/src/test.nr | 1 + .../src/test/cancel_authwit.nr | 127 ++++++++++++++++++ src/nft_contract/README.md | 10 ++ src/nft_contract/src/main.nr | 12 ++ src/nft_contract/src/test.nr | 1 + src/nft_contract/src/test/cancel_authwit.nr | 94 +++++++++++++ src/token_contract/README.md | 10 ++ src/token_contract/src/main.nr | 12 ++ src/token_contract/src/test.nr | 1 + src/token_contract/src/test/cancel_authwit.nr | 111 +++++++++++++++ 12 files changed, 392 insertions(+) create mode 100644 src/multitoken_contract/src/test/cancel_authwit.nr create mode 100644 src/nft_contract/src/test/cancel_authwit.nr create mode 100644 src/token_contract/src/test/cancel_authwit.nr diff --git a/src/multitoken_contract/README.md b/src/multitoken_contract/README.md index 6036d073..1a8385cf 100644 --- a/src/multitoken_contract/README.md +++ b/src/multitoken_contract/README.md @@ -82,6 +82,7 @@ All addresses are `AztecAddress`; `id` is a `Field`, `amount` is a `u128`, and ` - `initialize_transfer_commitment(to, completer) -> Field` — Creates a partial note (privacy entrance) to be completed by later transfers/mints. Id-agnostic: the completer binds `id` and `amount`. See [Commitment trust model](#commitment-trust-model) before using a commitment as a payment guarantee. - `mint_to_private(to, id, amount)` — Minter mints `id` into a private balance. Fully private. - `burn_private(from, id, amount, nonce)` — Burns `id` from a private balance. Fully private. +- `cancel_authwit(inner_hash)` — Cancels a private authwit the caller previously granted, by emitting its `(msg_sender, inner_hash)` nullifier so it can no longer be consumed. ### Public Functions diff --git a/src/multitoken_contract/src/main.nr b/src/multitoken_contract/src/main.nr index 72554317..2567c663 100644 --- a/src/multitoken_contract/src/main.nr +++ b/src/multitoken_contract/src/main.nr @@ -7,6 +7,7 @@ use aztec::macros::aztec; pub contract MultiToken { // aztec library use aztec::{ + authwit::auth::compute_authwit_nullifier, macros::{ events::event, functions::{authorize_once, external, initializer, internal, only_self, view}, @@ -450,6 +451,17 @@ pub contract MultiToken { self.emit(TransferSingle { from, to: AztecAddress::zero(), id, amount }); } + /// @notice Cancels a private authentication witness the caller previously granted + /// @dev Emits the authwit nullifier for `(msg_sender, inner_hash)`, so an authwit that has been + /// granted but not yet consumed can no longer be used. Matches the upstream token contracts. + /// @param inner_hash The inner hash of the authwit to cancel + #[external("private")] + fn cancel_authwit(inner_hash: Field) { + let on_behalf_of = self.msg_sender(); + let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash); + self.context.push_nullifier_unsafe(nullifier); + } + /** ========================================================== * ================= TOKEN LIBRARIES ========================= * ======================================================== */ diff --git a/src/multitoken_contract/src/test.nr b/src/multitoken_contract/src/test.nr index db99ce37..0c5313cb 100644 --- a/src/multitoken_contract/src/test.nr +++ b/src/multitoken_contract/src/test.nr @@ -13,4 +13,5 @@ mod burn_public; mod authorization; mod balance_of; mod initialize_transfer_commitment; +mod cancel_authwit; pub mod utils; diff --git a/src/multitoken_contract/src/test/cancel_authwit.nr b/src/multitoken_contract/src/test/cancel_authwit.nr new file mode 100644 index 00000000..374a579a --- /dev/null +++ b/src/multitoken_contract/src/test/cancel_authwit.nr @@ -0,0 +1,127 @@ +use crate::MultiToken; +use crate::test::utils; +use aztec::authwit::auth::compute_inner_authwit_hash; +use aztec::hash::hash_args; +use aztec::protocol::traits::ToField; +use aztec::test::helpers::authwit as authwit_cheatcodes; +use generic_proxy::GenericProxy; + +// Proves that once `owner` cancels a private authwit, a caller holding it can no longer consume it: +// the cancellation pre-emits the authwit nullifier, so the later authwit-gated transfer fails when +// it tries to emit the same nullifier again. +#[test(should_fail_with = "duplicate nullifiers")] +unconstrained fn cancelled_authwit_cannot_be_consumed() { + let id: Field = 1; + let (mut env, multitoken_contract_address, owner, recipient, _minter, proxy) = + utils::setup_and_mint_to_private_with_proxy(id); + + let transfer_amount = (1_000 as u128); + let transfer_call = MultiToken::at(multitoken_contract_address).transfer_private_to_private( + owner, + recipient, + id, + transfer_amount, + 1, + ); + + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + let inner_hash = compute_inner_authwit_hash([ + proxy.to_field(), + transfer_call.selector.to_field(), + hash_args(transfer_call.args), + ]); + env.call_private(owner, MultiToken::at(multitoken_contract_address).cancel_authwit(inner_hash)); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_5( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); +} + +// Positive control: the same flow WITHOUT the cancel succeeds, attributing the failure above to the +// cancellation. +#[test] +unconstrained fn uncancelled_authwit_is_consumed() { + let id: Field = 1; + let (mut env, multitoken_contract_address, owner, recipient, _minter, proxy) = + utils::setup_and_mint_to_private_with_proxy(id); + + let transfer_amount = (1_000 as u128); + let transfer_call = MultiToken::at(multitoken_contract_address).transfer_private_to_private( + owner, + recipient, + id, + transfer_amount, + 1, + ); + + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_5( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); + + utils::check_private_balance( + env, + multitoken_contract_address, + recipient, + id, + transfer_amount, + ); +} + +// Caller isolation: a foreign account cancelling with the owner's exact inner hash does NOT revoke +// the owner's authwit — the nullifier is bound to `msg_sender`. +#[test] +unconstrained fn foreign_cancel_does_not_revoke_owner_authwit() { + let id: Field = 1; + let (mut env, multitoken_contract_address, owner, recipient, _minter, proxy) = + utils::setup_and_mint_to_private_with_proxy(id); + + let transfer_amount = (1_000 as u128); + let transfer_call = MultiToken::at(multitoken_contract_address).transfer_private_to_private( + owner, + recipient, + id, + transfer_amount, + 1, + ); + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + let inner_hash = compute_inner_authwit_hash([ + proxy.to_field(), + transfer_call.selector.to_field(), + hash_args(transfer_call.args), + ]); + // `recipient` (not the granter) attempts to cancel the owner's authwit + env.call_private( + recipient, + MultiToken::at(multitoken_contract_address).cancel_authwit(inner_hash), + ); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_5( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); + utils::check_private_balance( + env, + multitoken_contract_address, + recipient, + id, + transfer_amount, + ); +} diff --git a/src/nft_contract/README.md b/src/nft_contract/README.md index b6613871..97b43a96 100644 --- a/src/nft_contract/README.md +++ b/src/nft_contract/README.md @@ -169,6 +169,16 @@ fn mint_to_private(to: AztecAddress, token_id: Field) { /* ... */ } fn burn_private(from: AztecAddress, token_id: Field, _nonce: Field) { /* ... */ } ``` +### cancel_authwit +```rust +/// @notice Cancels a private authentication witness the caller previously granted +/// @dev Emits the authwit nullifier for `(msg_sender, inner_hash)`, so an authwit that has been +/// granted but not yet consumed can no longer be used +/// @param inner_hash The inner hash of the authwit to cancel +#[private] +fn cancel_authwit(inner_hash: Field) { /* ... */ } +``` + ## Public Functions ### transfer_public_to_public diff --git a/src/nft_contract/src/main.nr b/src/nft_contract/src/main.nr index 9a8db51a..12320e55 100644 --- a/src/nft_contract/src/main.nr +++ b/src/nft_contract/src/main.nr @@ -7,6 +7,7 @@ use aztec::macros::aztec; pub contract NFT { // aztec library use aztec::{ + authwit::auth::compute_authwit_nullifier, macros::{ events::event, functions::{authorize_once, external, initializer, internal, only_self, view}, @@ -408,6 +409,17 @@ pub contract NFT { self.emit(Transfer { from, to: AztecAddress::zero(), token_id }); } + /// @notice Cancels a private authentication witness the caller previously granted + /// @dev Emits the authwit nullifier for `(msg_sender, inner_hash)`, so an authwit that has been + /// granted but not yet consumed can no longer be used. Matches the upstream NFT contract. + /// @param inner_hash The inner hash of the authwit to cancel + #[external("private")] + fn cancel_authwit(inner_hash: Field) { + let on_behalf_of = self.msg_sender(); + let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash); + self.context.push_nullifier_unsafe(nullifier); + } + /** ========================================================== * ================= TOKEN LIBRARIES ========================= * ======================================================== */ diff --git a/src/nft_contract/src/test.nr b/src/nft_contract/src/test.nr index 6528cd53..b4b8d0b9 100644 --- a/src/nft_contract/src/test.nr +++ b/src/nft_contract/src/test.nr @@ -10,5 +10,6 @@ mod transfer_private_to_public_with_commitment; mod transfer_private_to_public; mod transfer_public_to_private; mod transfer_public_to_public; +mod cancel_authwit; pub mod utils; mod view; diff --git a/src/nft_contract/src/test/cancel_authwit.nr b/src/nft_contract/src/test/cancel_authwit.nr new file mode 100644 index 00000000..17a35660 --- /dev/null +++ b/src/nft_contract/src/test/cancel_authwit.nr @@ -0,0 +1,94 @@ +use crate::NFT; +use crate::test::utils; +use aztec::authwit::auth::compute_inner_authwit_hash; +use aztec::hash::hash_args; +use aztec::protocol::traits::ToField; +use aztec::test::helpers::authwit as authwit_cheatcodes; +use generic_proxy::GenericProxy; + +// Proves that once `owner` cancels a private authwit, a caller holding it can no longer consume it: +// the cancellation pre-emits the authwit nullifier, so the later authwit-gated transfer fails when +// it tries to emit the same nullifier again. +#[test(should_fail_with = "duplicate nullifiers")] +unconstrained fn cancelled_authwit_cannot_be_consumed() { + let token_id = 1; + let (mut env, nft_contract_address, owner, _minter, recipient, proxy) = + utils::setup_and_mint_to_private_with_proxy(token_id); + + let transfer_call = + NFT::at(nft_contract_address).transfer_private_to_private(owner, recipient, token_id, 1); + + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + let inner_hash = compute_inner_authwit_hash([ + proxy.to_field(), + transfer_call.selector.to_field(), + hash_args(transfer_call.args), + ]); + env.call_private(owner, NFT::at(nft_contract_address).cancel_authwit(inner_hash)); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_4( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); +} + +// Positive control: the same flow WITHOUT the cancel succeeds, attributing the failure above to the +// cancellation. +#[test] +unconstrained fn uncancelled_authwit_is_consumed() { + let token_id = 1; + let (mut env, nft_contract_address, owner, _minter, recipient, proxy) = + utils::setup_and_mint_to_private_with_proxy(token_id); + + let transfer_call = + NFT::at(nft_contract_address).transfer_private_to_private(owner, recipient, token_id, 1); + + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_4( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); + + utils::assert_owns_private_nft(env, nft_contract_address, recipient, token_id); +} + +// Caller isolation: a foreign account cancelling with the owner's exact inner hash does NOT revoke +// the owner's authwit — the nullifier is bound to `msg_sender`. +#[test] +unconstrained fn foreign_cancel_does_not_revoke_owner_authwit() { + let token_id = 1; + let (mut env, nft_contract_address, owner, _minter, recipient, proxy) = + utils::setup_and_mint_to_private_with_proxy(token_id); + + let transfer_call = + NFT::at(nft_contract_address).transfer_private_to_private(owner, recipient, token_id, 1); + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + let inner_hash = compute_inner_authwit_hash([ + proxy.to_field(), + transfer_call.selector.to_field(), + hash_args(transfer_call.args), + ]); + // `recipient` (not the granter) attempts to cancel the owner's authwit + env.call_private(recipient, NFT::at(nft_contract_address).cancel_authwit(inner_hash)); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_4( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); + utils::assert_owns_private_nft(env, nft_contract_address, recipient, token_id); +} diff --git a/src/token_contract/README.md b/src/token_contract/README.md index 2cf938f1..3ea4c8ca 100644 --- a/src/token_contract/README.md +++ b/src/token_contract/README.md @@ -357,6 +357,16 @@ fn mint_to_private(to: AztecAddress, amount: u128) { /* ... */ } fn burn_private(from: AztecAddress, amount: u128, nonce: Field) { /* ... */ } ``` +### cancel_authwit +```rust +/// @notice Cancels a private authentication witness the caller previously granted +/// @dev Emits the authwit nullifier for `(msg_sender, inner_hash)`, so an authwit that has been +/// granted but not yet consumed can no longer be used +/// @param inner_hash The inner hash of the authwit to cancel +#[private] +fn cancel_authwit(inner_hash: Field) { /* ... */ } +``` + ## ARC-403 Authorization Hook The authorization contract address is set at construction via the `auth_contract` parameter on both constructors and stored as an immutable field. A zero address disables the hook. When set, each hooked function calls either `authorize_private(from, amount, selector)` or `authorize_public(from, amount, selector)` on the authorization contract after authwit validation and before any balance mutation. The token operation reverts if the authorization call reverts. The hook variant matches the calling function's context — private functions call `authorize_private`, public functions call `authorize_public` — and the `selector` passed is the calling function's own selector. diff --git a/src/token_contract/src/main.nr b/src/token_contract/src/main.nr index 3201ef61..fd57d2ae 100644 --- a/src/token_contract/src/main.nr +++ b/src/token_contract/src/main.nr @@ -6,6 +6,7 @@ use aztec::macros::aztec; pub contract Token { // aztec library use aztec::{ + authwit::auth::compute_authwit_nullifier, macros::{ events::event, functions::{authorize_once, external, initializer, internal, only_self, view}, @@ -491,6 +492,17 @@ pub contract Token { self.internal._burn_public(from, amount); } + /// @notice Cancels a private authentication witness the caller previously granted + /// @dev Emits the authwit nullifier for `(msg_sender, inner_hash)`, so an authwit that has been + /// granted but not yet consumed can no longer be used. Matches the upstream Token contract. + /// @param inner_hash The inner hash of the authwit to cancel + #[external("private")] + fn cancel_authwit(inner_hash: Field) { + let on_behalf_of = self.msg_sender(); + let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash); + self.context.push_nullifier_unsafe(nullifier); + } + /// @notice Decreases the total supply by `amount` /// @param amount The amount of tokens to decrease the total supply by #[external("public")] diff --git a/src/token_contract/src/test.nr b/src/token_contract/src/test.nr index e6a695d3..a4f256f3 100644 --- a/src/token_contract/src/test.nr +++ b/src/token_contract/src/test.nr @@ -12,5 +12,6 @@ mod transfer_public_to_public; mod transfer_public_to_commitment; mod transfer_public_to_private; mod authorization; +mod cancel_authwit; mod view; pub mod utils; diff --git a/src/token_contract/src/test/cancel_authwit.nr b/src/token_contract/src/test/cancel_authwit.nr new file mode 100644 index 00000000..9ddb0d5c --- /dev/null +++ b/src/token_contract/src/test/cancel_authwit.nr @@ -0,0 +1,111 @@ +use crate::test::utils; +use crate::Token; +use aztec::authwit::auth::compute_inner_authwit_hash; +use aztec::hash::hash_args; +use aztec::protocol::traits::ToField; +use aztec::test::helpers::authwit as authwit_cheatcodes; +use generic_proxy::GenericProxy; + +// Proves that once `owner` cancels a private authwit, a caller holding that authwit can no longer +// consume it: the cancellation pre-emits the authwit nullifier, so the later authwit-gated transfer +// fails when it tries to emit the same nullifier again. +#[test(should_fail_with = "duplicate nullifiers")] +unconstrained fn cancelled_authwit_cannot_be_consumed() { + let (mut env, token_contract_address, owner, recipient, proxy) = + utils::setup_and_mint_to_private_with_proxy(); + + let transfer_amount = (1000 as u128); + let transfer_call = Token::at(token_contract_address).transfer_private_to_private( + owner, + recipient, + transfer_amount, + 1, + ); + + // owner grants the authwit to the proxy + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + // owner cancels it, using the same inner hash the authwit machinery derives + let inner_hash = compute_inner_authwit_hash([ + proxy.to_field(), + transfer_call.selector.to_field(), + hash_args(transfer_call.args), + ]); + env.call_private(owner, Token::at(token_contract_address).cancel_authwit(inner_hash)); + + // proxy now attempts the transfer with the cancelled authwit — must fail + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_4( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); +} + +// Sanity: the SAME flow WITHOUT the cancel succeeds, so the failure above is attributable to the +// cancellation and not to some unrelated setup problem. +#[test] +unconstrained fn uncancelled_authwit_is_consumed() { + let (mut env, token_contract_address, owner, recipient, proxy) = + utils::setup_and_mint_to_private_with_proxy(); + + let transfer_amount = (1000 as u128); + let transfer_call = Token::at(token_contract_address).transfer_private_to_private( + owner, + recipient, + transfer_amount, + 1, + ); + + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_4( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); + + utils::check_private_balance(env, token_contract_address, recipient, transfer_amount); +} + +// Caller isolation: a foreign account cancelling with the owner's exact inner hash does NOT revoke +// the owner's authwit — the nullifier is bound to `msg_sender`, so the foreign cancel nullifies a +// different (attacker-bound) value and the owner's grant to the proxy still consumes successfully. +#[test] +unconstrained fn foreign_cancel_does_not_revoke_owner_authwit() { + let (mut env, token_contract_address, owner, recipient, proxy) = + utils::setup_and_mint_to_private_with_proxy(); + + let transfer_amount = (1000 as u128); + let transfer_call = Token::at(token_contract_address).transfer_private_to_private( + owner, + recipient, + transfer_amount, + 1, + ); + authwit_cheatcodes::add_private_authwit_from_call(env, owner, proxy, transfer_call); + + let inner_hash = compute_inner_authwit_hash([ + proxy.to_field(), + transfer_call.selector.to_field(), + hash_args(transfer_call.args), + ]); + // `recipient` (not the granter) attempts to cancel the owner's authwit + env.call_private(recipient, Token::at(token_contract_address).cancel_authwit(inner_hash)); + + // The owner's authwit is unaffected: the proxy transfer still succeeds + env.call_private( + owner, + GenericProxy::at(proxy).forward_private_4( + transfer_call.target_contract, + transfer_call.selector, + transfer_call.args, + ), + ); + utils::check_private_balance(env, token_contract_address, recipient, transfer_amount); +} From 252c352406dbdf206fae6f38976d9a99e325eb9c Mon Sep 17 00:00:00 2001 From: Alejo Amiras Date: Wed, 19 Aug 2026 15:00:53 +0000 Subject: [PATCH 3/3] fix(nft)!: align note-hashing with upstream aztec-nr scheme BREAKING: changes NFTNote hashes, so a fixed contract deploys under a new class id. Existing on-chain NFT notes were created under the old scheme and will not match; in-flight partial notes must be drained or abandoned before upgrading code at an existing address, and PXE/wallet note databases must use the artifact matching each deployed class. nft_note.nr used a pre-refactor hashing scheme that diverged from both upstream and this repo's own multitoken_note.nr: - compute_partial_commitment used the generic DOM_SEP__NOTE_HASH; now uses the dedicated DOM_SEP__PARTIAL_NOTE_COMMITMENT. - compute_complete_note_hash hand-rolled the preimage with the storage slot in the middle ([commitment, storage_slot, token_id]); now calls aztec-nr's compute_note_hash(storage_slot, [commitment, token_id]), which fixes the slot first (preventing cross-slot collisions). Both the direct note-hash path and the partial-note completion path route through the same two functions, so directly-created and completed-partial notes remain indistinguishable. Ported upstream's note_hash_matches_completed_partial_note_hash test to lock that invariant, and corrected two comments that wrongly claimed the partial commitment includes the storage slot. Validated: nft_contract 65 Noir tests, NFT TS integration suite 9/9 against a local sandbox (proving PXE note discovery works with the new scheme), aztec compile OK. Codex adversarial review: matches upstream, correct, internally consistent. Co-Authored-By: Claude Fable 5 --- src/nft_contract/src/types/nft_note.nr | 59 ++++++++++++++++++++------ 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/src/nft_contract/src/types/nft_note.nr b/src/nft_contract/src/types/nft_note.nr index ebfb9e5d..40d3cdcc 100644 --- a/src/nft_contract/src/types/nft_note.nr +++ b/src/nft_contract/src/types/nft_note.nr @@ -6,12 +6,15 @@ use aztec::{ delivery::{do_private_message_delivery, MessageDelivery}, logs::partial_note::encode_partial_note_private_message, }, - note::{note_interface::{NoteHash, NoteType}, utils::compute_note_nullifier}, + note::{ + note_interface::{NoteHash, NoteType}, + utils::{compute_note_hash, compute_note_nullifier}, + }, oracle::random::random, protocol::{ address::AztecAddress, constants::{ - DOM_SEP__NOTE_COMPLETION_LOG_TAG, DOM_SEP__NOTE_HASH, + DOM_SEP__NOTE_COMPLETION_LOG_TAG, DOM_SEP__PARTIAL_NOTE_COMMITMENT, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT, }, hash::{compute_log_tag, poseidon2_hash_with_separator}, @@ -49,7 +52,7 @@ impl NoteHash for NFTNote { // values, so that notes all behave the same way regardless of how they were created. To achieve this, we // perform both steps of the partial note computation. - // First we create the partial note from a commitment to the private content (including storage slot). + // First we create the partial note from a commitment to the private content. let partial_note = PartialNFTNote { commitment: compute_partial_commitment(owner, randomness) }; @@ -151,7 +154,10 @@ impl NFTNote { /// Computes a commitment to the private content of a partial NFTNote, i.e. the fields that will remain private. All /// other note fields will be made public. fn compute_partial_commitment(owner: AztecAddress, randomness: Field) -> Field { - poseidon2_hash_with_separator([owner.to_field(), randomness], DOM_SEP__NOTE_HASH) + poseidon2_hash_with_separator( + [owner.to_field(), randomness], + DOM_SEP__PARTIAL_NOTE_COMMITMENT, + ) } #[derive(Packable)] @@ -165,8 +171,9 @@ impl NoteType for NFTPartialNotePrivateLogContent { } } -/// A partial instance of a NFTNote. This value represents a private commitment to the owner, randomness and storage -/// slot, but the token id field has not yet been set. A partial note can be completed in public with the `complete` +/// A partial instance of a NFTNote. This value represents a private commitment to the owner and randomness (the storage +/// slot is folded into the completed note hash, not the commitment), but the token id field has not yet been set. A +/// partial note can be completed in public with the `complete` /// function (revealing the token id to the public), resulting in a NFTNote that can be used like any other one (except /// of course that its token id is known). #[derive(Packable, Serialize, Deserialize)] @@ -224,12 +231,38 @@ impl PartialNFTNote { } fn compute_complete_note_hash(self, storage_slot: Field, token_id: Field) -> Field { - // Here we finalize the note hash by including the (public) storage slot and token id into the partial note - // commitment. Note that we use the same separator as we used for the first round of poseidon - this is not - // an issue. - poseidon2_hash_with_separator( - [self.commitment, storage_slot, token_id], - DOM_SEP__NOTE_HASH, - ) + // Finalize the note hash by folding the (public) storage slot and token id into the partial note commitment. + // `compute_note_hash` fixes the storage slot at the first position of the preimage (preventing cross-slot + // collisions) and appends the remaining fields in order. + compute_note_hash(storage_slot, [self.commitment, token_id]) + } +} + +mod test { + use super::{compute_partial_commitment, NFTNote, PartialNFTNote}; + use aztec::{ + note::note_interface::NoteHash, + protocol::{address::AztecAddress, traits::FromField}, + }; + + global token_id: Field = 17; + global randomness: Field = 42; + global owner: AztecAddress = AztecAddress::from_field(50); + global storage_slot: Field = 13; + + #[test] + fn note_hash_matches_completed_partial_note_hash() { + // A NFTNote must have the same note hash as a PartialNFTNote created and then completed with the same private + // values. This requires the same hash function in both flows, with the fields in the same order — the invariant + // that lets a directly-created note and a completed partial note be used interchangeably. + let note = NFTNote { token_id }; + let note_hash = note.compute_note_hash(owner, storage_slot, randomness); + + let partial_note = + PartialNFTNote { commitment: compute_partial_commitment(owner, randomness) }; + let completed_partial_note_hash = + partial_note.compute_complete_note_hash(storage_slot, token_id); + + assert_eq(note_hash, completed_partial_note_hash); } }