From 441e309c5deb6b5e56bf455e61d779655f7a698f Mon Sep 17 00:00:00 2001 From: Alejo Amiras Date: Wed, 19 Aug 2026 14:20:09 +0000 Subject: [PATCH] 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,