fix: BB_DISABLE_ASSERTS in HonkRecursionConstraintTestWithoutPredicate.GenerateVKFromConstraints (debug build)#22811
Closed
AztecBot wants to merge 1 commit into
Closed
fix: BB_DISABLE_ASSERTS in HonkRecursionConstraintTestWithoutPredicate.GenerateVKFromConstraints (debug build)#22811AztecBot wants to merge 1 commit into
AztecBot wants to merge 1 commit into
Conversation
75a1125 to
a0f603a
Compare
…e.GenerateVKFromConstraints (debug build)
a0f603a to
5a1a557
Compare
This was referenced May 6, 2026
federicobarbacovi
added a commit
that referenced
this pull request
May 7, 2026
…g SIGABRT (#23019) ## Summary Unblocks the `Nightly Debug Build` workflow, which has been failing again on `next` after #22937 landed. With the `gemini_masking_poly` fix in #22937 the dsl_tests abort site is gone; the next abort site is in `BatchMergeProver`, hit by `BatchMergeTests/{2,3}.TooManySubtablesFails`. Most recent failing run: https://github.com/AztecProtocol/aztec-packages/actions/runs/25478845904 (b30fe8f, exit 134, 7m46s). ## Root cause `TooManySubtablesFails` deliberately calls `BB_DISABLE_ASSERTS()` so it can drive the verifier with `N = max_subtables + 1` subtables. With BB asserts demoted to warnings, control flow falls past the `BB_ASSERT_LTE(N, M, ...)` guard at the top of `BatchMergeProver::construct_proof` and reaches `compute_degree_check_polynomial`, which iterates over `flattened_columns.size()` (sized for `N`) but indexes `degree_check_challenges` (sized for `(M+1) * NUM_WIRES`). In release the over-read returns garbage that the verifier rejects — exactly what the test wants. In debug, libstdc++'s `_GLIBCXX_DEBUG` (set by the `debug` preset's `CXXFLAGS`) makes `std::vector::operator[]` bounds-checked and the OOB at `idx == 40` traps: ``` Error: attempt to subscript container with out-of-bounds index 40, but container only holds 40 elements. timeout: the monitored command dumped core ``` `BB_DISABLE_ASSERTS()` can't suppress this — the bounds check is libstdc++'s, not bb's. ## Fix Clamp the loop to `min(flattened_columns.size(), degree_check_challenges.size())`. Normal paths are unaffected because both sizes equal `(M+1) * NUM_WIRES` whenever `N <= M` (the assert holds). The misuse path now produces a partial degree-check polynomial that the verifier still rejects in both Debug and Release — same observable behaviour, no UB. ## Verification Reproduced inside the ClaudeBox container on `b30fe8f401` with the same flags as the nightly: ```bash cd barretenberg/cpp rm -rf build-debug CXXFLAGS="-gdwarf-4 -D_GLIBCXX_DEBUG" CC=clang-20 CXX=clang++-20 \ cmake -S . -B build-debug -G Ninja \ -DCMAKE_BUILD_TYPE=Debug -DENABLE_ASAN=OFF -DDISABLE_ASM=OFF -DENABLE_STACKTRACES=ON cd build-debug && ninja goblin_tests ./bin/goblin_tests --gtest_filter='*TooManySubtablesFails*' ``` - Pre-fix: aborts on `BatchMergeTests/2.TooManySubtablesFails` with the OOB shown above (exit 134). - Post-fix: 2 passed, 2 skipped (the native-curve cases self-skip). Full `goblin_tests`: 68 passed / 7 skipped / 0 failed. ## Relationship to other PRs - #22937 (merged) fixed the prior abort site (`gemini_masking_poly` virtual size). Without it the build never reached `BatchMergeProver`; with it, this is the next abort site. - #22976 is a draft from an earlier ClaudeBox session carrying the same one-file change against an older base. This PR re-opens the fix on top of current `next` (`b30fe8f401`) and is ready for review so it doesn't sit silently in draft. Once this merges, #22976 can be closed. - #22811 (BB_DISABLE_ASSERTS workaround for `HonkRecursionConstraintTestWithoutPredicate.GenerateVKFromConstraints`) is obsoleted by #22937 and intentionally not included. Detailed analysis: https://gist.github.com/AztecBot/3ea741b9545337f7a3adcfb60b088378 ClaudeBox log: https://claudebox.work/s/6f2f86264b1de77e?run=1 ClaudeBox log: https://claudebox.work/s/6f2f86264b1de77e?run=1 --------- Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Collaborator
Author
|
Automatically closing this stale claudebox draft PR (no updates for 5+ days). Re-open if still needed. |
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
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.
Summary
Unblock the nightly
barretenberg-debugbuild, which has been red every night since 2026-04-22 (14 consecutive nights as of 2026-05-06). The job aborts indsl_tests HonkRecursionConstraintTestWithoutPredicate/1.GenerateVKFromConstraints(UltraZK) with:
in
SharedShiftedVirtualZeroesArray::getduring the first sumcheck proverround.
Root cause
OinkProver::commit_to_masking_polyallocatesgemini_masking_polywithvirtual_size = polynomials.max_end_index(), which is4135(odd) for thiscircuit.
SumcheckProverRound::compute_effective_round_sizerounds the roundsize up to the next even, so
extend_edgesreadsgemini_masking_poly[edge_idx + 1]atindex == virtual_size. That hits thedebug-only
BB_ASSERT_DEBUG. In release the sameget()falls through to thereturn zerobranch (index == end_), the read is well-defined, and proofsare unaffected; only the debug nightly catches it.
Introduced by #22334 (
chore!: masking at the top of the trace).Fix
Apply the same surgical
BB_DISABLE_ASSERTS()workaround already used for thesibling test
HonkRecursionConstraintTestWithPredicate.GenerateVKFromConstraints(PR #21542). Debug-only, scoped to the test entry — production proving paths
keep the assert live.
The proper fix (rounding
virtual_sizeup to even in the polynomial allocator)is being pursued separately on draft PRs #22793 / #22789 / #22754. None have
landed in 14+ days; this workaround unblocks the nightly while that work
continues.
Verification
Local
debugpreset (clang-20):PR-branch CI is green on 5a1a557 (run 25304528960, 2026-05-04).
Recurring failures
Branch is on top of
cf1a239d27; the diff is the 8-line workaround only.Latest investigation: https://gist.github.com/AztecBot/9d47670a3d742f725c5fc370cd774080
Latest ClaudeBox log: https://claudebox.work/s/e6a43c73e5c30256?run=1
Original investigation gist: https://gist.github.com/AztecBot/50eeb6c9a88e27e020a95e626ceed9a0
@ludamad / honk-team — please mark this ready for review and merge so the
nightly stops paging us until the proper fix in #22754 / #22793 lands.