diff --git a/README.md b/README.md index 325668de..ca2f6d31 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ The `Vault` contract is a standalone yield-bearing vault that holds an underlyin We published the [AIP-4626: Tokenized Vault Standard](https://forum.aztec.network/t/request-for-comments-aip-4626-tokenized-vault/8079) to the forum. Feel free to review and discuss the specification there. > [!WARNING] -> The Vault is **not production-ready**, even by this repository's unaudited standards: it has a [known overflow issue in the asset↔share conversion logic](src/vault_contract/README.md) and known privacy limitations documented in its README. No fix is currently scheduled. +> The Vault is **not production-ready**, even by this repository's unaudited standards. It is exposed to [reentrancy when the underlying asset token has an ARC-403 authorization hook configured](src/vault_contract/README.md) (its ordering-based protection does not hold, because the hook runs *during* a transfer, before balances move), and it has a known overflow issue in the asset↔share conversion logic plus known privacy limitations. No fix is currently scheduled — see its README before wrapping any hooked token. 📖 **[View detailed Vault documentation](src/vault_contract/README.md)** diff --git a/src/multitoken_contract/README.md b/src/multitoken_contract/README.md index 407ae00a..5b94c080 100644 --- a/src/multitoken_contract/README.md +++ b/src/multitoken_contract/README.md @@ -22,7 +22,18 @@ Like `Token`, this contract implements the optional ARC-403 authorization hook: | `burn_private` | `authorize_private` | | `burn_public` | `authorize_public` | -Mints (`mint_to_private`, `mint_to_public`, `mint_to_commitment`) are **not** hooked — minting is already gated by the `minter` address set at construction. +Mints (`mint_to_private`, `mint_to_public`, `mint_to_commitment`) are **not** hooked — minting is already gated by the `minter` address set at construction. As with [`Token`](../token_contract/README.md#considerations), this means an authorization contract is **not a universal kill switch**: pausing it halts transfers and burns while the `minter` can still issue new supply of any id. + +## Commitment trust model + +`initialize_transfer_commitment(to, completer)` binds the recipient, the note randomness, and the address permitted to complete the commitment. It deliberately does **not** bind `token_id` or `amount` — both are supplied by the completer at completion time. This is what makes the mechanism useful when the id or value cannot be known in private (for example when they depend on public state), and it is the documented intent. + +The consequence is that **a commitment is not a payment guarantee**. Whoever you nominate as `completer` chooses both which token id and how much to deposit, so a commitment is only as trustworthy as that party. Concretely: + +- **Safe**: you are the completer, or the completer is a party you already trust for the amount (e.g. the token's own minter, or a contract whose logic fixes the value in public execution). +- **Unsafe**: escrow, marketplace, or handoff patterns where a counterparty completes your commitment in exchange for something you release first. A malicious completer can satisfy the commitment with a negligible amount of an arbitrary id — including one they created — and the completion will succeed. + +Note this is a wider surface than the single-asset [`Token`](../token_contract/README.md), where the token is implicit and only the amount is completer-chosen. If you need a commitment whose contents are guaranteed, the value must be enforced outside this primitive. ## TransferSingle Events @@ -65,7 +76,7 @@ All addresses are `AztecAddress`; `id` is a `Field`, `amount` is a `u128`, and ` - `transfer_private_to_public(from, to, id, amount, nonce)` — Spends private notes and enqueues a public credit to `to`. - `transfer_private_to_commitment(from, id, commitment, amount, nonce)` — Spends private notes and completes an already-initialized commitment with `(id, amount)`. - `transfer_public_to_private(from, to, id, amount, nonce)` — Enqueues a public debit of `from` and emits a private note to `to`. -- `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`. +- `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. diff --git a/src/token_contract/README.md b/src/token_contract/README.md index 2e3882e9..2cf938f1 100644 --- a/src/token_contract/README.md +++ b/src/token_contract/README.md @@ -375,7 +375,7 @@ The authorization contract address is set at construction via the `auth_contract ### Considerations -- **Mints are not hooked.** `mint_to_public`, `mint_to_private`, and `mint_to_commitment` do not call the hook — minting is already gated by the `minter` address set at construction. +- **Mints are not hooked.** `mint_to_public`, `mint_to_private`, and `mint_to_commitment` do not call the hook — minting is already gated by the `minter` address set at construction. **An authorization contract is therefore not a universal kill switch**: pausing it halts transfers and burns, but the `minter` can still issue new supply. Deployments that treat the hook as an emergency stop should note that a compromised `minter` key cannot be contained by the authorization contract, since `minter` is a single immutable address while the hook can encode richer governance. Whether mints should invoke the hook (with `from` as the zero address, which would enable pause and supply-cap policies but still not recipient screening, since `to` is not forwarded) is an open question for the [ARC-403 draft](https://forum.aztec.network/t/arc-403-authtoken/7887). - **`to` is not forwarded to the hook.** The recipient cannot be provided consistently across all transfer flows (commitment-based transfers seal the recipient inside a hash preimage the sender never sees), so it is omitted entirely rather than passed inconsistently. As a result, a blocked sender can still receive funds, but might not be able to spend them. - **`transfer_public_to_private` is not fully private.** It calls `authorize_private`, but spending a public balance inherently reveals `from` and `amount` on-chain regardless of any privacy the authorization contract provides. Authorization contracts can use the `selector` argument to distinguish this case. diff --git a/src/token_contract/src/main.nr b/src/token_contract/src/main.nr index 4bfe8b57..3201ef61 100644 --- a/src/token_contract/src/main.nr +++ b/src/token_contract/src/main.nr @@ -506,8 +506,9 @@ pub contract Token { * ======================================================== */ /// @notice ARC-403: calls the public authorization hook when one is configured - /// @dev Invoked from external public functions; no-ops when auth_contract is zero - /// @param from The address tokens are moved from (zero address for mints) + /// @dev Invoked from external public functions; no-ops when auth_contract is zero. + /// Mints deliberately do not call this hook; see the ARC-403 section of the README. + /// @param from The address tokens are moved from /// @param amount The amount of tokens being moved #[internal("public")] fn _call_auth_public(from: AztecAddress, amount: u128) { @@ -522,7 +523,7 @@ pub contract Token { /// @dev Invoked from external private functions; no-ops when auth_contract is zero. /// Runs in private context, but the authorization contract may still create public /// side effects (e.g., via enqueued public calls or public events). - /// @param from The address tokens are moved from (zero address for mints) + /// @param from The address tokens are moved from /// @param amount The amount of tokens being moved #[internal("private")] fn _call_auth_private(from: AztecAddress, amount: u128) { diff --git a/src/vault_contract/README.md b/src/vault_contract/README.md index 437bbf28..19af351f 100644 --- a/src/vault_contract/README.md +++ b/src/vault_contract/README.md @@ -16,7 +16,10 @@ This contract follows the [AIP-4626: Tokenized Vault Standard](https://forum.azt > **WARNING — Experimental Feature** > -> The AIP-4626 functionality of this contract is not yet production-ready. Use it at your own risk. In particular there is a known overflow issue in the asset<>share conversion logic used on deposits and withdrawals. This can corrupt balances for sufficiently large inputs. +> The AIP-4626 functionality of this contract is not yet production-ready. Use it at your own risk. Two known issues, neither currently scheduled for a fix: +> +> 1. **Reentrancy via a hooked asset token.** This contract's protection against reentrancy is the *order* of its operations (assets are taken in before shares are minted; shares are burned before assets are paid out), on the assumption that a token transfer is indivisible. It is not: when the asset token has an ARC-403 `auth_contract` configured, that contract is invoked *during* the transfer, before balances move, and can observe the vault mid-operation — the exact intermediate state the ordering is meant to exclude. Reading the share price at that point yields a value no completed operation would produce, which can be used to extract value belonging to other shareholders. **Only wrap an asset token whose `get_auth_contract()` is the zero address, or one whose authorization contract you fully trust.** Vaults over tokens with no hook configured are not affected. +> 2. **Overflow in the asset↔share conversion** logic used on deposits and withdrawals, for sufficiently large inputs. ## Architecture