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..1a8385cf 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: @@ -79,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/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); } } 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); +} 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,