chore: merge v5 into v5-next - #24701
Closed
AztecBot wants to merge 889 commits into
Closed
Conversation
fix(bb): keep lookup trace block early to fix ultra_honk +350MiB regression
* feat(bb): fake-GLV ECDSA verification for secp256r1
Implement secp256r1-specific ECDSA scalar multiplication using the
fake-GLV idea: instead of a single 256-bit 2-MSM, the verifier asks
the prover for half-size scalar decompositions (α_i, β_i) such that
α_i · P_i ≡ β_i · T_i (mod n) with |α_i|, |β_i| ≈ √n, then checks
each pair as an independent 2-MSM at 129 bits and sums the hints.
Soundness:
- T₁ and T₂ are computed natively from prover-supplied (α, β) and
verified by asserting each 2-MSM equals the point at infinity.
- β_i ≠ 0 is asserted (rules out the trivial (0, 0) decomposition).
- u_i ∈ {0, 1, n−1} is substituted to 2 in-circuit before native
multiplication to avoid degenerate point inputs to from_witness.
- Off-curve public keys are substituted to 2·G upstream before the
native T₂ = u₂·Q multiplication, so from_witness only sees valid
curve points. The original is_point_on_curve bit still flows into
the validity output.
The two-2-MSM structure was chosen over a single 4-MSM because the
4-MSM variant admits a forgery: with T₁, T₂ as free witnesses and
only a single combined infinity assertion, an adversary can solve
a linear system for T₁ to make the equation hold without knowing a
discrete log. Splitting into two assertions binds each hint to its
own base independently.
Gate count is currently ~72k vs the ~68.5k baseline; the non-native
doubling cost dominates so two 129-bit chains roughly match one
256-bit chain. Further optimization (custom Montgomery ladder, joint
lattice decomposition) will follow.
* feat(bb): Pedersen-style fixed-base mul u·G for secp256r1 via plookup
Add `element::secp256r1_fixed_base_mul(u)` which computes u·G without any
in-circuit doublings of a non-native accumulator. The 256-bit scalar is
sliced into 32 little-endian 8-bit windows; for each window position w,
the precomputed entry k holds the point `k · 2^(8w) · G + 2^w · H`, where
H is the "biggroup table offset generator". Five basic tables per window
cover the non-native limb axes (xlo, xhi, ylo, yhi, xyprime), giving
32 × 5 = 160 BasicTables total. Each window contributes 5 plookup gates
plus one chain-add into a running 32-point sum; the constant aggregate
offset (2^32 − 1) · H is subtracted at the end.
Table backend lives in plookup_tables/secp256r1_fixed_base.{hpp,cpp}:
the 32 × 256 native entries are materialised once per process under
`std::call_once`, and each BasicTable is the preprocessed lookup-poly
slice for one (axis, window) pair. Five contiguous BasicTableId ranges
(SECP256R1_FIXED_BASE_{XLO,XHI,YLO,YHI,XYPRIME}_0, 32 IDs each) are
reserved in types.hpp and dispatched in plookup_tables.cpp.
Cost for a single u·G call: 8,296 gates (trace) on UltraCircuitBuilder.
Table contents (≈41k rows × 3 columns) land in the preprocessed lookup
polynomial — shared across every call in the circuit, so batch ECDSA
verification does not pay them per signature.
Soundness:
- Each byte witness is range-constrained to 8 bits, and the plookup
membership argument implicitly constrains it to column 1 of the table
(= 0..255).
- The 32 bytes are reconstructed and asserted equal to u in Fr.
- Table entries are circuit constants, so the looked-up point is pinned
by the range-checked byte witness.
Caller responsibility: u = 0 produces the all-offsets result so the final
subtraction lands at (0, 0) (off-curve). The ECDSA path already substitutes
u_i ∈ {0, ±1} → 2 upstream, so this is not a real concern in practice.
* feat(bb): secp256r1 ECDSA hybrid mul (fixed-base u₁·G + 4-bit wNAF ladder for u₂·Q)
Rewrite the body of `element::secp256r1_ecdsa_mul` so that the two scalar
multiplications use specialised paths:
- u₁·G uses the Pedersen-style fixed-base plookup mul (no in-circuit
doublings). T₁ is pinned to u₁·G by construction, so no fake-GLV
identity is needed on the u₁ side.
- u₂·Q keeps the fake-GLV structure (half-GCD → α₂, β₂ with |α|,|β| < √n,
identity α₂·Q − β₂·T₂ = O) but verifies it via a custom 2-MSM
Montgomery ladder rather than the generic batch_mul. The scalars use
4-bit signed wNAFs (32 entries each, α₂ stagger=1, β₂ stagger=0); odd
multiples of Q and T₂_msm live in 16-entry signed-wNAF ROM tables
(reusing `four_bit_table_plookup`). The loop is 32 iters × (2 explicit
dbl + 2-input Montgomery ladder), then α₂ stagger fragment + skew
corrections; offset-generator subtraction at the end flips the
accumulator's is_infinity flag iff the identity holds.
Wider 8-bit ROM tables (256 entries) were measured at 148k gates — the
≈127 in-circuit point-additions needed to build each 256-entry table
dominate. 4-bit reduces table-build cost ≈30× while still cutting wNAF
loop iterations by half versus the 1-bit NAF used by the previous
generic 2-MSM.
Soundness (u₂ side, unchanged from prior sound fake-GLV):
- β₂ range-constrained to 129 bits and asserted nonzero.
- β₂_signed · u₂ ≡ α₂ (mod n) enforced.
- α₂, |β₂| wNAFs reconstructed in-circuit and asserted equal to the
bigfield witnesses.
- ROM table indices implicitly range-checked by the lookup.
- T₂ pinned to u₂·Q by the 2-MSM identity (given β₂ ≠ 0).
Substitutions: u₁ → 2 iff u₁ = 0 (avoids the fixed-base (0, 0) output);
u₂ → 2 iff u₂ ∈ {0, ±1} (avoids degenerate fake-GLV decomposition and
ROM-table x-coordinate collisions between Q and ±Q).
Measured gate count for a single secp256r1 ECDSA verify:
72,082 (prior sound fake-GLV baseline) → 44,933 (-38%)
All 63 stdlib ECDSA tests pass.
* refactor(bb): use generic batch_mul for the u₂·Q identity check in secp256r1 ECDSA
Replace the custom 4-bit-wNAF Montgomery ladder for the fake-GLV identity
α₂·Q − β₂·T₂ = O with a single `batch_mul({pubkey, T₂_msm}, {α₂, |β₂|},
129, false)` call. `batch_mul` runs a 4-NAF-columns-per-iteration Strauss
MSM over the standard 1-bit signed NAF with a 4-entry combined batch
lookup table over {±Q ± T₂_msm}; the terminating
`subtract_internal(offset_generator_end)` flips the is_infinity flag iff
the identity holds.
Gate-count comparison for a single secp256r1 ECDSA verify (u₁·G is the
fixed-base plookup mul in all hybrid variants):
prior sound fake-GLV baseline (no fixed-base): 72,082
hybrid + 8-bit custom wNAF ladder (256-entry ROM): 148,211
hybrid + 4-bit custom wNAF ladder (16-entry ROM): 44,933
hybrid + 1-bit NAF via batch_mul 44,385 ← this commit
The custom 8-bit ladder is dominated by in-circuit ROM table
construction (~127 chain-adds per 256-entry table × 2 tables). 4-bit and
1-bit collapse the table cost almost completely (~7 chain-adds for the
16-entry tables; ~3 for the combined 4-entry batch table) and end up
within ~600 gates of each other. The fixed-base path for u₁·G is what
unlocks the ~38% saving versus the prior baseline; the choice of u₂·Q
strategy moves the total by < 1k gates around 44–45k.
batch_mul is strictly simpler code-wise (no custom wNAF helper, no
manual ROM table construction, no offset-generator math, no template
hack of passing a uint256_t through a secp256k1::fr container) and is
marginally cheaper, so it's the preferred variant going forward.
All 63 stdlib ECDSA tests pass.
* perf(bb): tighten secp256r1 fake-GLV scalars to 128 bits
The half-GCD bound gives |α₂|, |β₂_abs| strictly < √n < 2¹²⁸ for secp256r1,
so drop the 129-bit max_num_bits in batch_mul and the 129-bit range checks to
128. EcdsaMulGateCount: 44,385 → 44,131 gates.
* refactor(bb): drop redundant u₁=0 substitution in secp256r1 ECDSA mul
`secp256r1_fixed_base_mul(0)` returns canonical infinity (raw_result equals
total_offset, and `operator-` detects the P − P case and sets is_infinity).
`T₁ + T₂` with T₁ = ∞ correctly returns T₂, so the upstream verifier sees
the right answer when u₁ = 0. Drop the `u1_safe` conditional_assign and
add an EcdsaMulU1Zero test pinning the behavior.
EcdsaMulGateCount: 44,131 → 44,063 gates.
* refactor(bb): bundle secp256r1 fixed-base plookup tables into MultiTables
Group the 160 BasicTables (32 windows × 5 axes) into 10 MultiTables — one
LO/HI pair per axis — and drive each with the bigfield Fr's u_low / u_high
limb-pair via `plookup_read::get_lookup_accumulators`. C1 step = 256 slices
the input scalar into 8-bit windows; C2/C3 step = 0 yields per-window
(limb_a, limb_b) values without accumulation. Saves ~116 gates on the full
ECDSA mul (44,063 → 43,947) by retiring the manual 32-byte witness +
explicit per-window lookup-gate emission.
* perf(bb): drop secp256r1 fixed-base XYPRIME plookup tables
Remove the per-window XYPRIME basic/multi tables that held the precomputed
prime-basis limbs of each table-entry's (x, y). Recompute prime_basis_limb
in-circuit from the four binary-basis limbs via the 4-limb
`unsafe_construct_from_limbs`, adding ~2 add gates per coordinate per window
(+96 gates per ECDSA verification). This saves 32 BasicTables × 256 entries =
8192 preprocessed rows from the cumulative lookup-table footprint, which
dominates the circuit size in the ECDSA-r1 + SHA256 use case (tables_size
76952 → 68760, ~10.6% off table rows).
Also expand the gate-count diagnostic in the secp256r1 biggroup tests to
report `tables_size` and `max(num_gates, tables_size)` alongside `num_gates`,
since the real circuit size is the max of the two.
* perf(bb): switch secp256r1 fixed-base windows to 7-bit + short tails
Replace the uniform 8-bit window layout with a 7-bit-dominant layout that
keeps the bigfield's natural lo/hi split (136 / 120 bits):
- lo half: 19 × 7-bit "big" windows + 1 × 3-bit tail (LO_TAIL_WINDOW)
- hi half: 17 × 7-bit "big" windows + 1 × 1-bit tail (HI_TAIL_WINDOW)
Total: 36 × 7-bit windows (table size 128 each) + 2 small tail tables
(size 8 for the lo tail, size 2 for the hi tail). All inter-window steps
are 7-bit aligned so the MultiTable column-1 step stays uniform at 128;
only the per-window slice_sizes vector encodes the tails' bit-widths.
The change targets cumulative lookup-table footprint for the ECDSA-r1 +
SHA256 verification flow, where SHA256's tables (~36k rows) plus the old
8-bit secp256r1 tables (32,768) pushed total tables_size to 68,760 — over
the 2^16 row cap. With this layout the secp256r1 contribution drops to
18,472 rows and cumulative tables_size becomes 54,464 — comfortably under
2^16 with ~11k rows of margin.
Per-call gate count grows by ~900 (8 more windows than before, ~110 gates
each for the extra lookup + chain_add + prime_limb compute). On the dsl
ECDSA-r1 opcode the old gate-count constant (72611) was already stale
from the earlier XYPRIME drop; this commit also brings it in line with
the actual count (45942) and adjusts the test pin accordingly.
Verified against stdlib_primitives_tests, stdlib_ecdsa_tests (full suite
including Wycherproof + infinity regressions), stdlib_circuit_builders_tests,
dsl OpcodeGateCountTests + EcdsaConstraintsTest, and the chonk-standalone
VK invariance script.
* fix(bb.js): bump WASM initial memory pages 35 → 37
The secp256r1 fixed-base plookup tables added in this branch include a
static std::array<std::array<AffineElement, 128>, 38> native_table that
contributes ~304 KiB to the WASM data segment, pushing the linker-declared
initial memory size from 35 to 37 pages (64 KiB each). The hardcoded
default in BarretenbergWasmMain.init must match, otherwise
WebAssembly.instantiate fails with:
LinkError: memory import has 35 pages which is smaller than the
declared initial of 37
Caught by barretenberg_wasm/index.test.js on CI.
* declare constant instead of hardcoded 38.
* add a comment.
* test(bb): add boomerang static-analyzer tests for secp256r1
Covers `secp256r1_fixed_base_mul` and `secp256r1_ecdsa_mul`:
- `fixed_base_mul`: random witness scalar, asserts 0 flagged variables
in one gate (after `finalize_circuit()`).
- `fixed_base_mul_u_zero`: exercises the offset-subtract-to-infinity
edge case from the PR description.
- `ecdsa_mul`: random witness inputs; the 10 expected flagged variables
are attributed in the source to the `T2_neg.conditional_select(T2,
beta2_neg)` call (biggroup conditional_select expands to two bigfield
conditional_assigns × 5 limbs each = 10), each constrained by a
single arithmetic gate that fully pins its output.
- `two_ecdsa_muls_in_same_builder`: count must scale linearly; catches
shared-state contamination across invocations.
- `ecdsa_mul_edge_cases`: u₂ ∈ {0, ±1} and u₁ = 0 substitution paths
all produce the same 10, confirming the circuit's wire structure is
input-independent.
* fix u2 bug: signature verification must fail when u2 is degen.
* refactor(bb): return u2_is_acceptable from secp256r1_ecdsa_mul
`secp256r1_ecdsa_mul` now returns `Secp256r1EcdsaMulResult { element result;
bool_ct u2_is_acceptable; }`. The caller in `ecdsa_verify_signature` consumes
the flag and ANDs it into the validity bit, dedup'ing the three bigfield
equality checks against {0, ±1} that previously lived in both functions.
Test call sites updated to access `.result`. ECDSA_SECP256R1 gate count
adjusted to 45945 (net +1 gate vs pre-bug-fix).
* test(bb): secp256r1 fixed-base layout and u2_is_acceptable wiring regressions
Adds two regression tests for the secp256r1 ECDSA path:
- PlookupTests.Secp256r1FixedBaseSliceSizeBound pins the soundness contract
of the eight SECP256R1_FIXED_BASE multitables: the slicer rejects keys past
the per-half bit budget, and each slot's slice_size matches its backing
basic-table size. Constants are derived from Secp256r1FixedBaseParams so the
test follows any future layout change.
- EcdsaTests.Secp256r1HonestU2EqualsOneRejected pins the is_u2_acceptable
AND-term on the validity flag in ecdsa_impl.hpp. Constructs an honest
signature with s = r so u2 = r * s^{-1} === 1 (mod n), confirms native ECDSA
accepts, and asserts the in-circuit verifier rejects (regression for the
fix in 3222906fd8).
* test(bb): add secp256r1 fuzzers
* chore(bb): refresh pinned Chonk IVC inputs to 44f45cc9b9f040ba
Generated by ci-refresh-chonk.
Only the pinned Chonk input hash is committed here; the immediate follow-up CI run is skipped intentionally.
--ci-skip
---------
Co-authored-by: AztecBot <tech@aztecprotocol.com>
feat: merge-train/barretenberg
Generated by ci-refresh-chonk. Only the pinned Chonk input hash is committed here; the immediate follow-up CI run is skipped intentionally. --ci-skip
fix: next conflicts
fix(kernel): add fixed gas cost for updating fee payer balance to private-only txs (finding #1082)
fix(kernel): assert bit size when packing delayed public mutable values (finding #986)
… (finding #1073) Address audit finding [#1073](https://app.audithub.dev/app/organizations/272/projects/821/project-viewer?issueId=1073&version=1726) ### The bug `read_current_value_of_delayed_public_mutable_with_hash` returns an empty value when the stored hash is zero, but it never constrained the `delayed_public_mutable` hint in that branch, only the non-zero branch checks the hint against the hash. The same hint is also consumed by `get_expiration_timestamp_for_contract_updates` to compute the contract-update time horizon. So when the hash is zero, a malicious prover could pass an arbitrary (non-empty) `delayed_public_mutable` to manipulate the time horizon while the read itself still returned empty. ### The fix In the zero-hash branch, assert that the packed hint is all zeros (`assert_eq(packed, std::mem::zeroed(), ...)`), guaranteeing the hint is empty before it can be used elsewhere. The function now always returns `delayed_public_mutable.svc.get_current_at(...)`, since the emptiness check makes that equivalent to the zeroed value in the uninitialized case. Added a comment in `get_expiration_timestamp_for_contract_updates` pointing to where the hint is constrained. ### Tests Added `read_uninitialized_value_with_non_empty_hint_fails`, which feeds a zero hash with a non-empty hint and expects the new assertion to fire. Updated the existing `read_uninitialized_value` test to pass an empty hint, matching the now-enforced precondition.
fix(avm): ECC - enforce canonical infinity inputs cannot fail with `!is_on_curve`
…_public_mutable fix(kernel): Constrain empty delayed public mutable when hash is zero (finding #1073)
Address audit finding [#1071](https://app.audithub.dev/app/organizations/272/projects/821/project-viewer?issueId=1071&version=1726) ### The bug `assert_sorted_padded_transformed_array` only checked `original_array_length <= CappedSize`, but the claimed output length is `result_array_length = original_array_length + num_padded_items_hint`, and that hint was unconstrained against `CappedSize`. A malicious prover could inflate the hint so `result_array_length > CappedSize`. Items in the range `[CappedSize, result_array_length)` only get the `counter == 0` check (from the `i >= CappedSize` branch). Their values are never validated, yet they sit within the claimed length of the output array, letting an attacker smuggle arbitrary values into the array under a `counter == 0` disguise. ### The fix Replaced the `original_array_length <= CappedSize` assert with `result_array_length <= CappedSize` (which subsumes the `original check since result_array_length >= original_array_length`). ### Tests Added `inflate_padded_items_hint_to_add_random_value_within_claimed_length`, which reproduces the attack (verified to pass on the buggy code before the fix, then flipped to `should_fail_with` after). Updated four existing tests that expected the old assertion message to the new "...all items in result_array" message.
fix(kernel): Constrain CappedSize against result length (finding #1071)
…handshake changes Resolve the public-v5-next -> v5-next conflict in the three generated standard-contract fixtures by regenerating them against the merged tree: - re-pinned standard contracts (pin-standard-build) with the merged handshake_registry source -> pinned-standard-contracts.tar.gz - regenerated standard_contract_data.ts and standard_addresses.nr via yarn generate:data (drift generator converged, no residual drift)
…next merge The public-v5-next merge adds the interactive-handshake support and an enlarged HandshakeRegistry contract chunk, pushing the bundled TXE size to ~14.01 MiB and tripping the total-size guard (14 MiB). No individual chunk exceeded its cap; only the aggregate limit needed bumping.
The previous regen ran a single pin/generate pass, leaving the pinned artifacts compiled against the pre-merge standard addresses: their bytecode still called the handshake registry at its old address (0x1954c59d...), which is never registered at runtime. The generator's drift check cannot catch this since it only verifies that the committed values match what is derived from the artifacts, not that the artifacts were compiled against those same values. Iterated pin-standard-build + generate to the fixpoint (converged in 2 passes: HandshakeRegistry embeds no standard addresses, the other three embed only the registry's). Verified the repinned bytecode embeds the live registry address and that automine/delivery/handshake_reuse passes locally (6/6).
fix: resolve public-v5-next → v5-next merge conflicts (regenerate standard-contract fixtures)
chore: merge public-v5-next into v5-next (raw, conflict markers)
chore: manual sync of v5-next
This commit records the raw output of `git merge v5` into v5-next with conflict markers left in place, so the conflicts are reviewable. The next commit resolves them.
Conflicts resolved: - yarn-project/txe/esbuild/plugins/size_guard.mjs (hand-maintained): merged both bump-log entries chronologically and kept the higher 15 MiB total cap from v5 (v5's interactive-handshake content is included in the merge, so it needs 15). - Generated stamp artifacts (standard_addresses.nr, standard_contract_data.ts, pinned-standard-contracts.tar.gz): resolved to the v5-next side as a consistent placeholder. These MUST be regenerated (./bootstrap.sh) on the merged tree: neither branch's values are correct because the merged aztec-nr library combines v5-next's security fixes (handshake, note validation, ephemeral keys) with v5's toolchain/library, so every standard contract recompiles to bytecode matching neither side. CI's drift-detecting generator will produce the correct values.
AztecBot
added
ci-draft
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
|
Duplicate of #24700 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges public
v5into publicv5-next, per the thread.v5was ahead ofv5-next(887 commits vs the merge-base;v5-nexthad 31 of its own), so this brings the release line's history back into the development line.As requested, this is split into two reviewable commits:
chore: merge v5 into v5-next (raw merge with conflict markers)— the raw output ofgit merge v5with conflict markers left in place, so the exact conflicts are visible.chore: resolve v5 -> v5-next merge conflicts— my best attempt at resolving them.Conflicts (4 files)
Hand-maintained — resolved with confidence:
yarn-project/txe/esbuild/plugins/size_guard.mjs— both sides bumped the TXE bundle size cap independently (v5 → 15 MiB for the handshake content on 07‑08; v5-next → 14.5 MiB on 07‑13). Kept both bump-log entries in chronological order and took the higher 15 MiB cap, since v5's handshake content is included in the merge and needs it.Generated stamp artifacts — resolved to the⚠️ need regeneration:
v5-nextside as a consistent placeholder,noir-projects/aztec-nr/aztec/src/standard_addresses.nryarn-project/standard-contracts/src/standard_contract_data.tsnoir-projects/noir-contracts/pinned-standard-contracts.tar.gzThese are auto-generated stamps (contract addresses, class IDs, bytecode commitments, VK hashes). Neither branch's values are correct after the merge, and they can't be hand-resolved:
handshake_registrycontract source isv5-next's (it carries the fairies security fixes — e.g. handshake-forging fix fix(aztec-nr): prevent recipient forging a colliding handshake #24403).aztec-nrlibrary combinesv5-next's security fixes (handshake, note validation, ephemeral keys) withv5's toolchain/library additions.I took the
v5-nextvalues so the committed stamps stay consistent with the security-fixed source that must ship, but the authoritative values must come from a./bootstrap.shregen of the merged tree (I couldn't run it here — no prebuiltnargo/bband no build cache; it's a full toolchain build). The drift-detecting generator (yarn workspace @aztec/standard-contracts run generate) will emit the correct values, and CI's drift check is expected to flag these three files until that regen is committed.Please review
size_guard.mjsresolution (straightforward).cc @nicolás Venturo @gregorio Juliana Quirós
Created by claudebox · group:
slackbot