Skip to content

Commit cc60215

Browse files
AztecBotAztecBot
authored
fix(bb): keep padded batch-merge test columns within their virtual size (#25687)
Fixes the [Nightly Debug Build failure](https://github.com/AztecProtocol/aztec-packages/actions/runs/40273515013) on `next` @ c1b606f, plus two `ci-barretenberg` CI failures that this PR exposed. Both of those show up when the bb build misses the cache. ## 1. Debug-only assertion in `BatchMergeTests` The nightly debug run ([CI log](http://ci.aztec-labs.com/1792991923783727)) ran the whole bb suite. Exactly four tests failed, and they are the same test under each parameterization: - `goblin_tests BatchMergeTests/{0,1,2,3}.PaddedSupportPreservesOriginalCommitments` ``` Assertion failed: (index < virtual_size_ + virtual_padding) ``` **Cause.** `TweakableBatchMergeProver` (test-only) pads subtable columns with `Polynomial(col, col.size() + n)`. That copy constructor extends the memory-backed range (`end_`) but keeps the original `virtual_size_`. The padded rows therefore sit past the virtual size, and the next `get()` on them trips `BB_ASSERT_DEBUG(index < virtual_size_ + virtual_padding)`. Release builds compile that assert out, so only the nightly debug build catches it. **Fix.** A test helper, `pad_column`, appends the zero rows and raises the virtual size to cover them. Both padding sites in the tweakable prover use it: the `PADDED_SUPPORT` / `ODD_PADDED_SUPPORT` path and the `CANCELLED_SEPARATOR*` path. ## 2. `ci-barretenberg` lints bb.js before `ipc-runtime` is built [CI log](http://ci.aztec-labs.com/1793005339838367): ``` Unable to resolve path to module '@aztec-foundation/ipc-runtime' import-x/no-unresolved ``` bb.js resolves `@aztec-foundation/ipc-runtime` through a portal into `ipc-runtime/ts`, whose entry point is the built `dest/`. The `ci-barretenberg` entry point in the root `bootstrap.sh` ran the bb.js lint before anything built that. **Fix:** run `ipc-runtime/bootstrap.sh build` before the formatting step. ## 3. Concurrent presets race on `bb_schema_embed.hpp` Once lint passed, both wasm builds failed ([wasm-threads](http://ci.aztec-labs.com/7c4894142e713f18), [wasm](http://ci.aztec-labs.com/b223bf5ddc632957)): ``` CMake Error at src/barretenberg/bbapi/CMakeLists.txt:45 (configure_file): No such file or directory bb_schema_embed.hpp:6:54: fatal error: raw string missing terminating delimiter )bbschema" ``` `bbapi/CMakeLists.txt` ran `configure_file` into the source tree (`src/barretenberg/bbapi/generated/bb_schema_embed.hpp`). Every preset therefore wrote the same file, and CI configures native, wasm and wasm-threads at the same time. One configure saw the file mid-rewrite (ENOENT), and another compiled a truncated copy. **Fix:** generate the header into `${CMAKE_CURRENT_BINARY_DIR}`, add that directory as a private include on `bbapi_objects`, and include it as `"bb_schema_embed.hpp"`. The ipc-codegen outputs in `generated/` are untouched. ## Testing (local) - Batch merge, `debug` preset. Red: all 4 `PaddedSupportPreservesOriginalCommitments` parameterizations fail with the CI assertion. Green: all of `goblin_tests` passes (89 passed, 7 skipped on purpose, 0 failed). - CI ordering. Red: on a clean checkout, `CI=1 barretenberg/ts/bb.js/bootstrap.sh formatting` reproduces the 4 `no-unresolved` errors. Green: after `ipc-runtime/bootstrap.sh build`, it exits 0. - Schema embed: I could not trigger the race locally. With the change, `debug` configures and builds `bbapi_objects`, and `wasm-threads` configures and compiles `bbapi_schema.cpp.obj`. The header is generated under each build directory and no longer in the source tree. Everything else is left to CI. --- *Created by [claudebox](https://claudebox.work/v2/sessions/be2aead164432985/jobs/1) · group: `slackbot` · requested by Sergei · [Slack thread](https://aztecfoundation.slack.com/archives/C04ALP0UQHH/p1792992711137169?thread_ts=1792992711.137169&cid=C04ALP0UQHH)*
1 parent c1b606f commit cc60215

4 files changed

Lines changed: 20 additions & 4 deletions

File tree

‎barretenberg/cpp/src/barretenberg/bbapi/CMakeLists.txt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@ endif()
4040

4141
# Embed the schema so `bb msgpack schema` reports the exact contract this
4242
# binary was built against. Configure-time: re-runs when the schema changes.
43+
# Written into the build directory: presets configured concurrently (native,
44+
# wasm, wasm-threads) would otherwise race on one file in the source tree.
4345
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${BB_SCHEMA})
4446
file(READ ${BB_SCHEMA} BB_SCHEMA_CONTENT)
45-
configure_file(bb_schema_embed.hpp.in ${BB_GEN_DIR}/bb_schema_embed.hpp @ONLY)
47+
configure_file(bb_schema_embed.hpp.in ${CMAKE_CURRENT_BINARY_DIR}/bb_schema_embed.hpp @ONLY)
4648

4749
barretenberg_module(bbapi common chonk dsl crypto_poseidon2 crypto_pedersen_commitment crypto_pedersen_hash crypto_blake2s crypto_aes128 crypto_schnorr crypto_ecdsa ecc srs)
50+
target_include_directories(bbapi_objects PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
4851

4952
if(NOT FUZZING)
5053
add_dependencies(bbapi_objects bb_ipc_generated)

‎barretenberg/cpp/src/barretenberg/bbapi/bbapi_schema.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#include "barretenberg/bbapi/bbapi_schema.hpp"
22

3-
#include "barretenberg/bbapi/generated/bb_schema_embed.hpp"
3+
#include "bb_schema_embed.hpp"
44

55
namespace bb::bbapi {
66

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,17 @@ bb::fr compute_running_hash(const std::vector<bb::fr>& proof, size_t N)
142142
return previous_challenge;
143143
}
144144

145+
/**
146+
* Copy of `col` with `extra_rows` zero rows appended to its memory-backed support. The virtual size grows with it so
147+
* the appended rows stay addressable.
148+
*/
149+
template <typename Polynomial> Polynomial pad_column(const Polynomial& col, size_t extra_rows)
150+
{
151+
Polynomial padded(col, col.size() + extra_rows);
152+
padded.increase_virtual_size(std::max(padded.virtual_size(), padded.end_index()));
153+
return padded;
154+
}
155+
145156
/**
146157
* Local prover copy used only in tests, with controlled fault injection points.
147158
* Important: faults are applied before data is sent to transcript, so Fiat–Shamir remains consistent.
@@ -175,7 +186,7 @@ class TweakableBatchMergeProver : public BatchMergeProver {
175186
: 0;
176187
if (padding_rows != 0) {
177188
for (auto& col : subtable_cols[0]) {
178-
col = Polynomial(col, col.size() + padding_rows);
189+
col = pad_column(col, padding_rows);
179190
}
180191
}
181192

@@ -190,7 +201,7 @@ class TweakableBatchMergeProver : public BatchMergeProver {
190201
fault_mode == FaultMode::CANCELLED_SEPARATOR_WITH_EXTENDED_BOUND) {
191202
// An app must not be allowed to extend its support into the separator's two rows.
192203
for (auto& col : subtable_cols[0]) {
193-
col = Polynomial(col, col.size() + 2);
204+
col = pad_column(col, 2);
194205
}
195206
subtable_cols[0][0].at(shift_sizes[0]) = -FF(3);
196207
max_shift_size = std::max(max_shift_size, shift_sizes[0] + 2);

‎bootstrap.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,6 +1032,8 @@ case "$cmd" in
10321032
export USE_TEST_CACHE=1
10331033
export AVM=0
10341034
export AVM_TRANSPILER=0
1035+
# bb.js lint resolves @aztec-foundation/ipc-runtime through its built dest/.
1036+
ipc-runtime/bootstrap.sh build
10351037
barretenberg/ts/bb.js/bootstrap.sh formatting
10361038
barretenberg/crs/bootstrap.sh
10371039
barretenberg/cpp/bootstrap.sh ci

0 commit comments

Comments
 (0)