Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/main-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,34 @@ jobs:
uses: AztecProtocol/aztec-ci-actions/.github/workflows/run-tests.yml@431859e477234b8690eb1f80d1305d2e34f10f1b # v0.1.1
with:
runner: ubuntu-latest
# JS tests run in the `js-tests` job below instead, which needs a job-level env var
# that a reusable workflow cannot inherit from its caller.
run-js-tests: false
secrets: inherit

# Kept in lockstep with the `js-tests` job in pr-checks.yml — see the rationale for
# SEQ_BLOCK_DURATION_MS there.
js-tests:
name: JS Tests
runs-on: ubuntu-latest
timeout-minutes: 60
permissions:
contents: read
env:
# 6s blocks = mainnet geometry (10 blocks/checkpoint, 117,668 DA gas per tx). The local
# default of 3s blocks caps a tx at 55,882, below the ~97k the protocol guarantees for a
# maximal contract class registration, which the Vault class publication needs.
SEQ_BLOCK_DURATION_MS: '6000'
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
fetch-depth: 0

- name: Setup Aztec environment
uses: AztecProtocol/aztec-ci-actions/actions/setup-aztec@431859e477234b8690eb1f80d1305d2e34f10f1b # v0.1.1
with:
start-pxe: 'true'
run-codegen: 'true'

- name: Run JS tests
uses: AztecProtocol/aztec-ci-actions/actions/js-tests@431859e477234b8690eb1f80d1305d2e34f10f1b # v0.1.1
70 changes: 27 additions & 43 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
@@ -1,64 +1,48 @@
name: PR Checks

# SCRATCH, do not merge: live e2e of aztec-benchmark's `block-duration-ms` input
# (AztecProtocol/aztec-benchmark@d623993, branch worktree-aztec-v6-update).
# J1 "6000" should pass; J2 (omitted) should fail at the local DA cap; J3 "6s" should fail in Configure.

on:
pull_request:

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
cancel-in-progress: true

permissions:
contents: read

jobs:
# Job-level gating (not workflow-level `paths:`) so a skipped benchmark still
# reports as passing for branch protection.
changes:
name: Changes
runs-on: ubuntu-latest
j1-bench-6000:
uses: AztecProtocol/aztec-benchmark/.github/workflows/pr-benchmark.yml@d623993df314206cf1fd08443d10631976bdcf1d
with:
runner: ubuntu-latest
block-duration-ms: "6000"
permissions:
pull-requests: read
outputs:
benchmark: ${{ steps.filter.outputs.benchmark }}
steps:
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: filter
with:
filters: |
benchmark:
- 'src/**/*.nr'
- '**/Nargo.toml'
- 'benchmarks/**'
# every benchmark imports its setup helpers from this file
- 'src/ts/test/utils.ts'
- 'tsconfig.json'
- 'package.json'
- 'yarn.lock'
- '.github/workflows/pr-checks.yml'

format:
name: Format
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
with:
node-version: '22'
cache: 'yarn'
- run: yarn install --frozen-lockfile
- run: yarn format:check
contents: read
pull-requests: write
issues: write
actions: read
secrets: inherit

checks:
uses: AztecProtocol/aztec-ci-actions/.github/workflows/run-tests.yml@431859e477234b8690eb1f80d1305d2e34f10f1b # v0.1.1
j2-bench-default:
uses: AztecProtocol/aztec-benchmark/.github/workflows/pr-benchmark.yml@d623993df314206cf1fd08443d10631976bdcf1d
with:
runner: ubuntu-latest
permissions:
contents: read
pull-requests: write
issues: write
actions: read
secrets: inherit

benchmark:
# Skipping also skips this branch's baseline-artifact upload; a PR stacked on
# top of a skipped branch falls back to `if_no_artifact_found: warn` downstream.
needs: changes
if: needs.changes.outputs.benchmark == 'true'
uses: AztecProtocol/aztec-benchmark/.github/workflows/pr-benchmark.yml@39244c9306f7b460aec570297100f3513f7ef936 # v5.0.1
j3-bench-invalid:
uses: AztecProtocol/aztec-benchmark/.github/workflows/pr-benchmark.yml@d623993df314206cf1fd08443d10631976bdcf1d
with:
runner: ubuntu-latest
block-duration-ms: "6s"
permissions:
contents: read
pull-requests: write
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/update-baseline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ concurrency:

jobs:
baseline:
uses: AztecProtocol/aztec-benchmark/.github/workflows/update-baseline.yml@39244c9306f7b460aec570297100f3513f7ef936 # v5.0.1
uses: AztecProtocol/aztec-benchmark/.github/workflows/update-baseline.yml@d623993df314206cf1fd08443d10631976bdcf1d
with:
runner: ubuntu-latest
block-duration-ms: "6000"
permissions:
contents: read
actions: write
Expand Down
15 changes: 14 additions & 1 deletion src/ts/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { type Wallet, AccountManager } from '@aztec-labs/aztec.js/wallet';
import { Fr } from '@aztec-labs/aztec.js/fields';
import { createAztecNodeClient, waitForNode, waitForTx } from '@aztec-labs/aztec.js/node';
import { type ContractInstanceWithAddress } from '@aztec-labs/aztec.js/contracts';
import { publishContractClass } from '@aztec-labs/aztec.js/deployment';
import { TxHash } from '@aztec-labs/aztec.js/tx';
import { EmbeddedWallet } from '@aztec-labs/wallets/embedded';
import { registerInitialLocalNetworkAccountsInWallet } from '@aztec-labs/wallets/testing';
Expand Down Expand Up @@ -244,7 +245,19 @@ export async function deployNFTWithMinter(wallet: EmbeddedWallet, deployer: Azte
* Each vault pool deploys its own VaultDeployer instance via initializers; there is no shared factory.
*/
export async function ensureVaultContractClassPublished(wallet: Wallet, deployer: AztecAddress): Promise<void> {
await VaultContract.deploy(wallet, deployer, 1).send({ from: deployer });
// Publish ONLY the contract class, which is all callers need. Deploying a throwaway Vault would
// publish the class *and* the instance *and* run the constructor in one transaction, and the
// Vault's public dispatch bytecode is big enough that the combined DA gas leaves little headroom
// under the node's per-tx admission limit. That limit is derived from block geometry, so it is not
// a constant: see SEQ_BLOCK_DURATION_MS in .github/workflows/pr-checks.yml for why CI pins the
// local network to mainnet's.
//
// Publication emits a nullifier keyed on the class id, so a second publish of the same class is
// rejected with "Existing nullifier". Guard on registration state the same way DeployMethod does.
const contractClass = await getContractClassFromArtifact(VaultContractArtifact);
const { isContractClassPubliclyRegistered } = await wallet.getContractClassMetadata(contractClass.id);
if (isContractClassPubliclyRegistered) return;
await (await publishContractClass(wallet, VaultContractArtifact)).send({ from: deployer });
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/vault_contract/Nargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ type = "contract"
aztec = { git = "https://github.com/aztec-labs-eng/aztec-nr", tag = "v6.0.0-rc.1", directory = "aztec" }
token_contract = { path = "../token_contract" }
generic_proxy = { path = "../generic_proxy" }
bignum = { git = "https://github.com/noir-lang/noir-bignum", tag = "v0.10.0" }
5 changes: 2 additions & 3 deletions src/vault_contract/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ 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. Two known issues, neither currently scheduled for a fix:
> The AIP-4626 functionality of this contract is not yet production-ready. Use it at your own risk. One known issue, not 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.
> **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.

## Architecture

Expand Down
137 changes: 137 additions & 0 deletions src/vault_contract/src/conversion.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
//! Overflow-safe fixed-point conversion math for the vault.
//!
//! The share<>asset conversions compute `a * b / denominator`. Done natively in `u128`, the
//! intermediate product `a * b` overflows for large-but-legitimate inputs (Noir range-checks u128,
//! so the transaction reverts rather than wrapping) — this is the availability bug flagged by the
//! 2026-08 security audit (F-004). We compute the product and the division in a 256-bit intermediate
//! (`noir-bignum`'s `U256` — three 120-bit limbs, modulus 2^256) so nothing overflows until
//! the final result, which is narrowed back to `u128` and only fails if the true result genuinely
//! exceeds `u128` — an impossibility for real balances.

use bignum::bignum::BigNum;
use bignum::U256;

/// 2^120, the width of a single `U256` limb. A `u128` splits across the low two limbs: limb 0 holds
/// the low 120 bits, limb 1 the high 8 bits.
global TWO_POW_120: u128 = 0x1000000000000000000000000000000;

/// Widens a `u128` into a `U256` (little-endian, 120-bit limbs).
fn u256_from_u128(v: u128) -> U256 {
U256::from_limbs([v % TWO_POW_120, v / TWO_POW_120, 0])
}

/// Narrows a `U256` back to `u128`, asserting it fits. Reverts if the value exceeds `u128::MAX`.
fn u256_to_u128(v: U256) -> u128 {
let hi = v.get_limb(1);
// A u128 occupies limb 0 (120 bits) plus at most the low 8 bits of limb 1; anything above that
// does not fit. `hi < 256` keeps `hi * TWO_POW_120` below 2^128 so the reconstruction cannot
// itself overflow.
assert((v.get_limb(2) == 0) & (hi < 256), "conversion result exceeds u128");
// hi < 256 (asserted above), so hi * TWO_POW_120 < 2^128 and the sum fits u128.
v.get_limb(0) + hi * TWO_POW_120
}

/// True iff `v` is zero.
fn u256_is_zero(v: U256) -> bool {
(v.get_limb(0) == 0) & (v.get_limb(1) == 0) & (v.get_limb(2) == 0)
}

/// Computes `a * b / denominator` without overflowing on the intermediate product.
///
/// Rounds down by default; when `round_up` is true and the division leaves a remainder, rounds up.
/// This matches the previous native-`u128` implementation exactly for every input that did not
/// overflow — the widening only extends the range of inputs that succeed, it does not change any
/// result. `denominator` must be non-zero (it always is at the call sites: `total_assets + 1` and
/// `total_supply + vault_offset` with `vault_offset >= 1`).
pub fn mul_div(a: u128, b: u128, denominator: u128, round_up: bool) -> u128 {
// noir-bignum's constrained `udiv_mod` enforces `remainder < divisor` (which fails for a zero
// divisor), but its unconstrained path assumes a non-zero divisor and would otherwise return a
// meaningless witness. Guard explicitly so the helper is safe in isolation, not only at the
// vault's (always non-zero) call sites.
assert(denominator > 0, "division by zero");
let numerator = u256_from_u128(a) * u256_from_u128(b);
let (quotient, remainder) = numerator.udiv_mod(u256_from_u128(denominator));

let mut result = u256_to_u128(quotient);
if round_up & !u256_is_zero(remainder) {
result = result + 1;
}
result
}

mod test {
use super::{mul_div, TWO_POW_120, u256_from_u128, u256_to_u128};

global ROUND_DOWN: bool = false;
global ROUND_UP: bool = true;

#[test]
fn round_trip_u128_boundaries() {
// The widen/narrow pair is identity across the u128 range.
for v in [0, 1, TWO_POW_120 - 1, TWO_POW_120, 0xffffffffffffffffffffffffffffffff] {
assert_eq(u256_to_u128(u256_from_u128(v)), v);
}
}

#[test]
fn matches_native_for_small_values() {
// Exact division: no rounding either way.
assert_eq(mul_div(1000, 3, 3, ROUND_DOWN), 1000);
assert_eq(mul_div(1000, 3, 3, ROUND_UP), 1000);
// 1:1 ratio, the vault's initial state.
assert_eq(mul_div(1000, 1, 1, ROUND_DOWN), 1000);
// Zero numerator.
assert_eq(mul_div(0, 12345, 7, ROUND_UP), 0);
}

#[test]
fn rounding_direction_is_preserved() {
// 7 / 2 = 3 remainder 1: down floors, up ceils. This is the invariant the vault's economic
// safety depends on — rounding must never shift, or value leaks to or from depositors.
assert_eq(mul_div(7, 1, 2, ROUND_DOWN), 3);
assert_eq(mul_div(7, 1, 2, ROUND_UP), 4);
// 10 / 3 = 3 remainder 1.
assert_eq(mul_div(5, 2, 3, ROUND_DOWN), 3);
assert_eq(mul_div(5, 2, 3, ROUND_UP), 4);
// No remainder => up does NOT add one.
assert_eq(mul_div(9, 1, 3, ROUND_UP), 3);
}

#[test]
fn product_that_overflowed_u128_now_converts() {
// a * b here is 2^200, far beyond u128::MAX (2^128 - 1); the native `a * b` reverted.
// 2^100 * 2^100 / 2^100 = 2^100, which fits u128.
let two_pow_100: u128 = 0x10000000000000000000000000;
assert_eq(mul_div(two_pow_100, two_pow_100, two_pow_100, ROUND_DOWN), two_pow_100);

// A non-trivial ratio at the same scale: (2^100 * (3 * 2^100)) / (2 * 2^100) = 3 * 2^100 / 2
// = 1.5 * 2^100 => floor is 2^100 + 2^99, ceil the same (exact, no remainder).
let expected: u128 = two_pow_100 + (two_pow_100 / 2);
assert_eq(mul_div(two_pow_100, 3 * two_pow_100, 2 * two_pow_100, ROUND_DOWN), expected);
assert_eq(mul_div(two_pow_100, 3 * two_pow_100, 2 * two_pow_100, ROUND_UP), expected);
}

#[test]
fn max_operands_do_not_overflow_the_intermediate() {
// Both operands at u128::MAX: native `a * b` = (2^128-1)^2 reverts; here it divides cleanly.
let max: u128 = 0xffffffffffffffffffffffffffffffff;
// max * max / max = max.
assert_eq(mul_div(max, max, max, ROUND_DOWN), max);
}

#[test(should_fail_with = "division by zero")]
fn zero_denominator_reverts() {
// The helper guards a zero denominator explicitly (unreachable from the vault, whose
// denominators are total_assets+1 / total_supply+vault_offset with vault_offset >= 1).
let _ = mul_div(100, 5, 0, ROUND_DOWN);
}

#[test(should_fail_with = "conversion result exceeds u128")]
fn result_exceeding_u128_reverts_cleanly() {
// max * max / 1 = (2^128-1)^2, which genuinely does not fit u128. Reverts with our message
// rather than a raw range-check failure. Unreachable for real balances (would require a
// denominator far below the operands), but proves the narrowing guard is present.
let max: u128 = 0xffffffffffffffffffffffffffffffff;
let _ = mul_div(max, max, 1, ROUND_DOWN);
}
}
33 changes: 17 additions & 16 deletions src/vault_contract/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod conversion;
pub mod test;

use aztec::macros::aztec;
Expand Down Expand Up @@ -1451,7 +1452,6 @@ pub contract Vault {
/// @param vault_offset The vault offset
/// @param rounding The rounding direction (ROUND_UP or ROUND_DOWN)
/// @return The equivalent amount of shares
// TODO: The multiplication `assets * (total_supply + vault_offset)` can overflow u128 for large values.
#[contract_library_method]
fn _convert_to_shares(
assets: u128,
Expand All @@ -1460,13 +1460,14 @@ pub contract Vault {
vault_offset: u128,
rounding: bool,
) -> u128 {
let mul_term = assets * (total_supply + vault_offset);
let denominator = total_assets + 1;
let mut shares = mul_term / denominator;
if (rounding == ROUND_UP) & (mul_term % denominator > 0) {
shares = shares + 1;
}
shares
// `assets * (total_supply + vault_offset)` overflows u128 for large inputs; compute it in a
// 512-bit intermediate. Rounding is unchanged (see `conversion::mul_div`).
crate::conversion::mul_div(
assets,
total_supply + vault_offset,
total_assets + 1,
rounding == ROUND_UP,
)
}

/// @notice Converts an amount of shares to assets using the given exchange rate parameters
Expand All @@ -1476,7 +1477,6 @@ pub contract Vault {
/// @param vault_offset The vault offset
/// @param rounding The rounding direction (ROUND_UP or ROUND_DOWN)
/// @return The equivalent amount of assets
// TODO: The multiplication `shares * (total_assets + 1)` can overflow u128 for large values.
#[contract_library_method]
fn _convert_to_assets(
shares: u128,
Expand All @@ -1485,13 +1485,14 @@ pub contract Vault {
vault_offset: u128,
rounding: bool,
) -> u128 {
let mul_term = shares * (total_assets + 1);
let denominator = total_supply + vault_offset;
let mut assets = mul_term / denominator;
if (rounding == ROUND_UP) & (mul_term % denominator > 0) {
assets = assets + 1;
}
assets
// `shares * (total_assets + 1)` overflows u128 for large inputs; compute it in a 512-bit
// intermediate. Rounding is unchanged (see `conversion::mul_div`).
crate::conversion::mul_div(
shares,
total_assets + 1,
total_supply + vault_offset,
rounding == ROUND_UP,
)
}

/** ==========================================================
Expand Down
Loading