fix(vault): compute share<>asset conversions in a 256-bit intermediate

[alejoamiras]

Closes the last open item from the security audit: F-004, the share/asset conversion overflow.

The bug

_convert_to_shares / _convert_to_assets compute a * b / denominator. Done natively in u128, the intermediate product overflows for large-but-legitimate inputs. Noir range-checks u128, so the transaction reverts rather than wrapping — a vault whose totals reach that range can no longer be deposited to or withdrawn from, permanently locking every participant's funds. Both sites carried TODOs.

The fix

A new conversion.nr module with a mul_div primitive that widens both operands to noir-bignum's U256 (the same library the escrow's key derivation already uses), multiplies and divides there, then narrows the quotient back to u128, asserting it fits.

U256 arithmetic is modulo 2^256, and the largest possible product (2^128-1)^2 is 2^129-1 short of that modulus — so the product is always exact, with no modular wraparound.

Rounding is unchanged. Both old and new return floor(p/d) + (round_up && p%d != 0). For every input the old code accepted, the results are identical; widening only extends the domain that succeeds. This matters because the vault's economic safety depends on rounding always favouring the vault — a shift in either direction would leak value between the vault and its depositors.

mul_div also rejects a zero denominator explicitly. noir-bignum's constrained udiv_mod fails on it, but its unconstrained path assumes non-zero and would return a meaningless witness. Unreachable from the vault, but the helper is now safe in isolation.

Pulling mul_div into its own module is what makes the overflow boundary testable at all: a real vault cannot be driven to a 2^128 supply in a test, but the primitive can be called directly at its edges.

Validation

• 195 vault_contract Noir tests. 188 pre-existing all still green — the strongest evidence rounding did not shift — plus 7 new ones covering limb round-trips, rounding direction both ways, products that previously overflowed, max operands, and the two revert guards.
• Codex adversarial review: correct, rounding invariance proven algebraically, no value-leak path. Its zero-denominator hardening is applied.

Second commit: CI block geometry

While validating this, the JS suite failed to publish the Vault contract class. That turned out not to be a problem with this change:

aztec start --local-network defaults to 3s blocks. With 72s slots that is 21 blocks per checkpoint, and the per-tx DA admission limit is ceil(daBudget / blocks * 1.5) = 55,882 DA gas. Mainnet runs 6s blocks (10 blocks per checkpoint) and admits 117,668.

The protocol picks that 1.5 multiplier deliberately, and says so in aztec stdlib gas/tx_gas_limits.ts — it is set "so the largest tx we want to support — a maximal contract class registration (~97k DA gas) — fits a single block under v5 mainnet geometry (72s slots, 6s blocks → 10 blocks per checkpoint)". The default local geometry therefore advertises a limit below the ~97k the protocol guarantees for exactly this kind of transaction, and rejects contracts that are valid on the network we ship to.

Publishing the Vault class costs ~64k DA gas: fine on mainnet, inside the protocol's stated envelope, over the local cap. This is not specific to this PR — main's Vault already sits at ~54k, i.e. 97% of the local ceiling, so essentially any growth in that contract trips it.

So the JS job now pins SEQ_BLOCK_DURATION_MS=6000, making CI reproduce mainnet geometry rather than a stricter one. Reusable workflows do not inherit the caller's env and run-tests.yml exposes no knob for it, so the JS job is inlined (run-js-tests: false on the reusable call) purely to own the environment. It still calls the same pinned setup-aztec and js-tests composite actions.

Important

This renames the check from checks / JS Tests to JS Tests. Any branch protection rule naming the old check needs updating, or it will block merges waiting on a check that no longer runs.

The better long-term fix is upstream: either the local network should default to mainnet geometry, or run-tests.yml should expose the knob. Worth raising with the Aztec CI folks.

Note

The vault README still describes the overflow as a known issue. That warning block is rewritten in #24 (unmerged); leaving it avoids a three-way conflict.