feat: merge-train/barretenberg - #23902
Merged
Merged
Conversation
…th Constantine recoder (#23691) Isolates the scalar-multiplication-**independent** ECC changes from the Pippenger refactor (#23297) into their own reviewable layer, stacked on top of the Constantine recoder PR (#23562). ## Stack `merge-train/barretenberg` ← #23562 (Constantine recoder) ← **this PR** ← #23297 (Pippenger) This PR depends on #23562 because it consumes the shared signed-Booth primitives in `ecc/groups/booth_recode.hpp`. It must merge after #23562. ## What's here - **element**: add `straus_msm`; rewrite `batch_mul_with_endomorphism` / `operator*` onto carry-less signed-Booth window slices, replacing the old `EndomorphismWnaf` lookup-table path. - **field**: short-circuit `split_into_endomorphism_scalars` for small `k`. - **wnaf**: drop the runtime `fixed_wnaf` overload — orphaned once `EndomorphismWnaf` (its only caller) was removed. - **dedup vs booth_recode.hpp**: element_impl no longer carries its own copy of `BoothSliceParams` / `compute_booth_slice_params`; the generic branchless `booth_packed_digit` reader now lives in `booth_recode.hpp` as a shared primitive. element_impl keeps only its element-specific GLV-endo window schedule + EC math. The Pippenger MSM path keeps its own perf-tuned reader. - **tests**: `straus_msm` correctness/edge cases and `batch_mul` K2 bit-width coverage. ## Testing `ecc_tests` builds and `StrausMsm*` + `BatchMul*` pass on the endomorphism curves (bn254, grumpkin); secp curves skip the endo tests by design.
Ports [AztecProtocol/barretenberg-claude#3643](AztecProtocol/barretenberg-claude#3643). --------- Co-authored-by: sergei iakovenko <105737703+iakovenkos@users.noreply.github.com> Co-authored-by: iakovenkos <sergey.s.yakovenko@gmail.com>
iakovenkos
requested review from
IlyasRidhuan,
MirandaWood and
jeanmon
as code owners
June 5, 2026 12:22
iakovenkos
enabled auto-merge
June 5, 2026 13:45
iakovenkos
self-requested a review
June 5, 2026 13:45
…#23887) Stacks on #23297 (base = `lde/zacs-pippenger`). Adds edge-case unit tests, fuzzers, and one out-of-bounds fix for the new round-parallel Pippenger / MSM, focused on the areas the refactor is most fragile in: **arena sizing** and **parameter-dependent dispatch**. Test-only except for a one-line OOB fix in an existing test. Validated natively **and under AddressSanitizer + `_GLIBCXX_DEBUG`**. ## Bug found & fixed **Out-of-bounds scalar read in `test_batch_multi_scalar_mul_large_dense`** (existing in #23297). It indexed `scalars[k*8192 + i]` up to 32767 against the 31013-element fixture. Release builds did silent UB (both result and reference read the same out-of-bounds garbage, so the assertion still passed — the bug was invisible); the debug/ASAN allocator aborts. Wrapped with `% num_points`, matching the sibling ragged test. ## Tests added **Arena (the documented "drift" bug class — sizer vs live allocator):** - `ArenaLayoutFitsAcrossDispatchSpace` — sweeps ~14k combos of thread count (incl. odd/non-pow2), N at every dispatch boundary, **`dedup=true` and `ext_glv=true`** (neither was ever checked against the sizer), and bit budgets, plus 1600 deterministic pseudo-random N. Any miss is a guaranteed Zone overflow. - `EffectiveNumBitsBandSmallScalars` — drives real MSMs in the native non-GLV mid-band (2^13<n<2^17) where the sizer's defensive bit-budget sweep is skipped, so an under-count surfaces as an OOB write rather than passing silently. **Dispatch / GLV / dedup:** - `DispatchFuzz` — randomized differential fuzzer over (N, start_index, thread count, scalar distribution, dedup) vs naive. - `GlvExtremeMagnitudeScalars` — magnitude-maximizing scalars (r−1, (r−1)/2, λ±1, the k2-negative-fix region, bit-{126,127,128,129,253} boundaries) through the real `use_glv=true` path; checks the 2-limb (≤128-bit) split halves recombine to k·P, catching any silent >128-bit truncation or top-window carry drop. - `DedupLargeClusterCarryAndCaps` — equal-scalar clusters of distinct points past `DEDUP_MAX_CHUNK_MEMBERS` and the staged cap, exercising the multi-chunk tree-reduce carry and the partial-consolidation fallback (vs naive and vs dedup-off). - `ExternalGlvDoubledDirect` — the `external_glv_doubled` aliasing branch directly (was only reached transitively via the batch driver), incl. N above the native GLV threshold. **Batch driver (the production commit-batch path):** - `BatchDriverSharedPathRagged` — `batch_multi_scalar_mul(handle_edge_cases=false)` is the **only** entry into the shared `pippenger_round_parallel_batched` path (GLV-group assignment, `external_glv_doubled` aliasing, single shared arena sized to the largest member). Every existing batch test uses the default `handle_edge_cases=true` and routes around it. Ragged batch with empty MSMs interspersed (the `n==0 → infinity` skips), fully-zero MSMs, a mixed GLV / non-GLV group split, total nonzero past the >4096 REBALANCE threshold, and mixed per-MSM dedup hints. **Large-N multi-threaded edge cases (Jacobian path):** - `HandleEdgeCases{PointAtInfinity,InversePairs,AllInfinity}` — infinity / P+(−P) bucket collisions at large N with forced 8 threads, hitting the multi-threaded Jacobian split + cross-thread reduction. `scalar_multiplication_safe_mode.test.cpp` covers these only at ≤60 pts (single-threaded). ## Verification Built `ecc_tests` on native (x86_64) — all new tests pass for BN254 and Grumpkin; full `*ScalarMultiplication*`+`*VariableWindow*` suite green. Then rebuilt under the `asan-fast` preset (ASAN + `_GLIBCXX_DEBUG`) and ran the entire suite (123 tests): clean after the OOB fix — the **real** Zone P/W/S allocator validated across the whole swept dispatch space, not just the test-side layout mirror. clang-format-20 clean. Large/GLV-threshold-dependent cases are `__wasm__`-skipped per the file's convention. --------- Co-authored-by: iakovenkos <sergey.s.yakovenko@gmail.com>
ludamad
added
ci-full
iakovenkos
approved these changes
Jun 8, 2026
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
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
4 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.
BEGIN_COMMIT_OVERRIDE
refactor: Booth-slice element MSM (straus_msm + batch_mul), shared with Constantine recoder (#23691)
feat: Pippenger refactor full 11 may (#23297)
chore: pippenger edge-case unit tests, fuzzers, bench markers removal (#23887)
END_COMMIT_OVERRIDE