diff --git a/src/vault_contract/src/main.nr b/src/vault_contract/src/main.nr index 0773a009..1c2c9c29 100644 --- a/src/vault_contract/src/main.nr +++ b/src/vault_contract/src/main.nr @@ -1478,6 +1478,17 @@ pub contract Vault { * ======================================================== */ /// @notice Validates that the caller possesses authwit from the `from` address or the caller is the `from` address + /// @dev Deliberately NOT `#[authorize_once("from", "nonce")]`: that macro asserts `nonce == 0` + /// whenever `from == msg_sender`, which is sound for the token contracts (their nonce exists + /// only for the token-call authwit) but wrong here. The vault's `nonce` is dual-purpose — it + /// is forwarded as the authwit nonce of the inner token operations the vault performs on the + /// user's behalf (asset `transfer_public_to_public`, shares `burn_private`/`burn_public`). + /// A self-caller needs no VAULT authwit but still grants token authwits to the vault, and + /// distinct nonces are what let otherwise-identical operations repeat: private authwit + /// nullifiers commit to the inner call hash, which includes the nonce, so an identical + /// private token call at nonce 0 can never be consumed twice (public authwits live in + /// registry state and could be re-authorized, but the private-path regression is decisive). + /// Forcing `nonce == 0` on self-calls would break e.g. two identical private self-deposits. #[contract_library_method] fn _validate_from_private(context: &mut PrivateContext, from: AztecAddress) { if (!from.eq(context.maybe_msg_sender().unwrap())) { @@ -1486,6 +1497,7 @@ pub contract Vault { } /// @notice Validates that the caller possesses authwit from the `from` address or the caller is the `from` address + /// @dev See `_validate_from_private` for why this is not `#[authorize_once]` #[internal("public")] fn _validate_from_public(from: AztecAddress) { if (!from.eq(self.msg_sender())) { diff --git a/src/vault_contract/src/test/deposit_private_to_private.nr b/src/vault_contract/src/test/deposit_private_to_private.nr index 030cd09c..8fba2373 100644 --- a/src/vault_contract/src/test/deposit_private_to_private.nr +++ b/src/vault_contract/src/test/deposit_private_to_private.nr @@ -326,3 +326,55 @@ unconstrained fn deposit_private_to_private_on_behalf_of_other_success() { // Check vault has assets utils::check_public_balance(env, asset_address, vault_address, deposit_amount); } + +#[test] +unconstrained fn deposit_private_to_private_repeated_identical_self_deposits_distinct_nonces() { + // Regression guard for the vault's dual-use `nonce`: it is forwarded as the inner asset + // transfer's authwit nonce, so a SELF-caller must be able to pass a nonzero nonce — distinct + // nonces are what let two otherwise-identical private asset authwits both be consumed. + // This is why the vault does not use #[authorize_once("from", "nonce")], which asserts + // nonce == 0 whenever from == msg_sender and would revert both deposits below. + let (mut env, vault_address, owner, recipient, asset_address, shares_address) = + utils::setup_with_vault(true); + + let deposit_amount: u128 = mint_amount; + + // First self-deposit: identical asset authwit shape, nonce 1 + env.call_private(owner, Token::at(asset_address).mint_to_private(owner, deposit_amount)); + let transfer_1 = + Token::at(asset_address).transfer_private_to_public(owner, vault_address, deposit_amount, 1); + authwit_cheatcodes::add_private_authwit_from_call(env, owner, vault_address, transfer_1); + let shares_1 = deposit_amount; + env.call_private( + owner, + Vault::at(vault_address).deposit_private_to_private( + owner, + recipient, + deposit_amount, + shares_1, + 1, + ), + ); + + // Second self-deposit: same from/to/amount (identical authwit preimage except the nonce) + env.call_private(owner, Token::at(asset_address).mint_to_private(owner, deposit_amount)); + let transfer_2 = + Token::at(asset_address).transfer_private_to_public(owner, vault_address, deposit_amount, 2); + authwit_cheatcodes::add_private_authwit_from_call(env, owner, vault_address, transfer_2); + // Request slightly fewer shares than the 1:1 first deposit: the +1/offset conversion rounds down + let shares_2 = deposit_amount / 2; + env.call_private( + owner, + Vault::at(vault_address).deposit_private_to_private( + owner, + recipient, + deposit_amount, + shares_2, + 2, + ), + ); + + // Both deposits landed: recipient holds both share issuances, vault holds both asset amounts + utils::check_private_balance(env, shares_address, recipient, shares_1 + shares_2); + utils::check_public_balance(env, asset_address, vault_address, deposit_amount + deposit_amount); +}