Skip to content

Commit 8ee7069

Browse files
fix(bb): clamp BatchMergeProver degree-check loop to fix nightly debug SIGABRT (#23077)
Fix BatchMergeTests/*.TooManySubtablesFails by making the code use the TweakableBatchMergeProver. ## Summary Unblocks the [Nightly Debug Build](https://github.com/AztecProtocol/aztec-packages/actions/runs/25539203050) on `next`. With `_GLIBCXX_DEBUG` enabled, `BatchMergeTests/2.TooManySubtablesFails` aborts (exit 134) with: ``` Error: attempt to subscript container with out-of-bounds index 40, but container only holds 40 elements. ``` ## Root cause `TooManySubtablesFails` uses `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`) bounds-checks `std::vector::operator[]` and traps. `BB_DISABLE_ASSERTS()` cannot suppress this — the bounds check is libstdc++'s, not bb's. ## Why the previously-merged fix didn't fix anything PR #23019 (already merged into `merge-train/barretenberg` via the open #23025) was *titled* the same as this PR but only modified `TweakableBatchMergeProver::construct_proof`. The actual `TooManySubtablesFails` test path uses `prove_and_verify` with the default `fault_mode = FaultMode::NONE`, which routes through `BatchMergeProver` (the base class), not `TweakableBatchMergeProver`. I confirmed this in this container by building `goblin_tests` against `origin/merge-train/barretenberg` HEAD with the debug preset — the same OOB still aborts. The merge train cannot deliver this fix without a real prover-side change. ## Fix Clamp the loop to `min(flattened_columns.size(), degree_check_challenges.size())` inside `BatchMergeProver::compute_degree_check_polynomial`. 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. This is the same one-file change as the open draft #22976; opening this fresh PR against current `next` so it doesn't sit silently in draft. ## Verification Reproduced inside the ClaudeBox container on `f23aa82c52` (current `next` HEAD) with the same flags as the nightly: ```bash cd barretenberg/cpp HOME=/tmp cmake --preset debug -DAVM_TRANSPILER_LIB= 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). Confirmed identical abort on `origin/merge-train/barretenberg` HEAD. - Post-fix: 2 passed, 2 skipped (the native-curve cases self-skip). Full `goblin_tests` post-fix: **68 passed / 7 skipped / 0 failed** (342s). ## Related PRs - #22937 (merged) — fixed the prior abort site (`gemini_masking_poly` virtual size). - #23019 (merged into merge-train/barretenberg via open #23025) — same title, but the diff only modified `TweakableBatchMergeProver`, which is unused on this test path; it is a no-op for the nightly. - #22976 (open draft) — same one-file prover clamp as this PR, against an older base. Detailed analysis: https://gist.github.com/AztecBot/7be72c96a1d3d18458dce92a828116a2 ClaudeBox log: https://claudebox.work/s/8ad866e315acbe92?run=1 ClaudeBox log: https://claudebox.work/s/8ad866e315acbe92?run=1 --------- Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
1 parent 3134e84 commit 8ee7069

1 file changed

Lines changed: 11 additions & 6 deletions

File tree

barretenberg/cpp/src/barretenberg/goblin/batch_merge.test.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ enum class FaultMode : uint8_t {
4141
PADDING_NOT_INFINITY, // padded slot sends non-zero shift size and non-zero commitment/eval
4242
SHIFT_SIZE_MINUS_ONE, // send k-1 as shift size for a subtable polynomial of size k
4343
ZK_TABLE_DEGREE_TOO_HIGH, // zk table has degree above verifier hard-coded ZK shift
44-
ZERO_SUBTABLES_CLAIM // send 0 as number of subtables
44+
ZERO_SUBTABLES_CLAIM, // send 0 as number of subtables,
45+
TOO_MANY_SUBTABLES, // send a number of subtables above the max that the verifier is configured for
4546
};
4647

4748
void populate_subtable(const std::shared_ptr<ECCOpQueue>& op_queue, size_t num_ops)
@@ -238,17 +239,20 @@ class TweakableBatchMergeProver : public BatchMergeProver {
238239
}
239240

240241
// Step 6: degree-check poly
241-
bool is_too_many_subtables = flattened_cols.size() > num_degree_check_challenges;
242-
if (is_too_many_subtables) {
242+
if (fault_mode == FaultMode::TOO_MANY_SUBTABLES) {
243243
// This is the case in which we test that if the prover sends more columns than the max number of tables
244244
// then the verifier rejects
245-
degree_check_challenges.push_back(degree_check_challenges.back() * degree_check_challenge);
245+
size_t diff = flattened_cols.size() - num_degree_check_challenges;
246+
for (size_t idx = 0; idx < diff * NUM_WIRES; ++idx) {
247+
// Add challenges for the extra columns sent by the prover
248+
degree_check_challenges.push_back(degree_check_challenges.back() * degree_check_challenge);
249+
}
246250
}
247251

248252
Polynomial degree_check_poly =
249253
compute_degree_check_polynomial(flattened_cols, degree_check_challenges, max_shift_size);
250254

251-
if (is_too_many_subtables) {
255+
if (fault_mode == FaultMode::TOO_MANY_SUBTABLES) {
252256
// Remove the extra challenge added above to keep the degree check poly consistent with the rest of the
253257
// proof
254258
degree_check_challenges.pop_back();
@@ -257,6 +261,7 @@ class TweakableBatchMergeProver : public BatchMergeProver {
257261
if (fault_mode == FaultMode::BAD_DEGREE_CHECK_POLY && !degree_check_poly.is_empty()) {
258262
degree_check_poly.at(0) += FF(1);
259263
}
264+
260265
transcript->send_to_verifier("DEGREE_CHECK_POLY", pcs_commitment_key.commit(degree_check_poly));
261266

262267
// Step 7
@@ -487,7 +492,7 @@ TYPED_TEST(BatchMergeTests, TooManySubtablesFails)
487492
} else {
488493
BB_DISABLE_ASSERTS();
489494
auto op_queue = make_op_queue_with_n_subtables(TestFixture::NumSubtables + 1);
490-
auto res = TestFixture::prove_and_verify(op_queue);
495+
auto res = TestFixture::prove_and_verify(op_queue, FaultMode::TOO_MANY_SUBTABLES);
491496
EXPECT_FALSE(res.reduction_ok); // Caught by product check
492497
EXPECT_FALSE(res.pairing_ok); // Verifier uses fewer commitments than the one sent
493498
if constexpr (TestFixture::IsRecursive) {

0 commit comments

Comments
 (0)