Merged
Conversation
## Summary Forward-port of the v4 fix (#21975). The native `ChonkVerifier::reduce_to_ipa_claim` was missing `kernel_io.pairing_inputs.check()` — the accumulated pairing points from inner recursive verifications. The recursive verifier already aggregates these points; the native verifier must explicitly check them. ## Details - Added `kernel_io.pairing_inputs.check()` after `reconstruct_from_public` in the native `reduce_to_ipa_claim` - This completes parity between native and recursive verification paths - `chonk_tests` builds successfully ClaudeBox log: https://claudebox.work/s/fe964a7f5fb2b43c?run=1
## Summary Enables libstdc++ debug mode (`_GLIBCXX_DEBUG`) in the `debug` and `debug-fast` CMake presets. ## Motivation From the BB audit: `_GLIBCXX_DEBUG` catches logical container UB that ASAN misses. For example, `operator[]` on a `reserve()`d-but-empty vector is UB that ASAN cannot detect (because `reserve()` allocates the backing memory — the UB is logical, not a heap overflow). With `_GLIBCXX_DEBUG`, libstdc++ immediately aborts with a clear diagnostic: > attempt to subscript container with out-of-bounds index 0, but container only holds 0 elements ## Changes - Added `-D_GLIBCXX_DEBUG` to `CXXFLAGS` in the `debug` preset - Added `-D_GLIBCXX_DEBUG` to `CXXFLAGS` in the `debug-fast` preset - ASAN presets inherit from debug, so they also benefit ClaudeBox log: https://claudebox.work/s/638c6b632d5cd8af?run=2
## Summary Adds `--memory_profile_out <file>` flag to `bb prove` that outputs a JSON array of RSS checkpoints at key proving stages for each circuit. Each checkpoint records the stage name (alloc, trace, oink, sumcheck, accumulate), circuit index, circuit name, and peak RSS in MB. In CI, `extract_memory_benchmarks.py` converts these into dashboard entries (one overlaid line per circuit stage, tracked across commits). - RSS checkpoints at 5 stages per circuit: after_alloc, after_trace, after_oink, after_sumcheck, after_accumulate - Circuit names threaded from ChonkAccumulate - Reuses `peak_rss_bytes()` from logstr.cpp (no duplicate getrusage) - JSON via msgpack serialization - Native only (getrusage unavailable in wasm) Dashboard visualization: AztecProtocol/benchmark-page-data#1 Refs: AztecProtocol/barretenberg#1641 ## Example JSON output (62 checkpoints for 13 circuits) ```json [ {"stage": "after_alloc", "circuit_index": 0, "circuit_name": "MultiCallEntrypoint:entrypoint", "rss_mb": 120}, {"stage": "after_trace", "circuit_index": 0, "circuit_name": "MultiCallEntrypoint:entrypoint", "rss_mb": 129}, {"stage": "after_oink", "circuit_index": 0, "circuit_name": "MultiCallEntrypoint:entrypoint", "rss_mb": 135}, {"stage": "after_sumcheck", "circuit_index": 0, "circuit_name": "MultiCallEntrypoint:entrypoint", "rss_mb": 137}, {"stage": "after_accumulate", "circuit_index": 0, "circuit_name": "MultiCallEntrypoint:entrypoint", "rss_mb": 137}, {"stage": "after_alloc", "circuit_index": 1, "circuit_name": "private_kernel_init", "rss_mb": 147}, ... {"stage": "after_trace", "circuit_index": 6, "circuit_name": "EcdsaRAccount:entrypoint", "rss_mb": 227}, ... {"stage": "after_trace", "circuit_index": 12, "circuit_name": "hiding_kernel", "rss_mb": 263} ] ``` ## Test plan - [x] Builds cleanly - [x] Tested with deploy_ecdsar1+sponsored_fpc flow: 62 checkpoints, correct circuit names - [x] Extraction script produces correct dashboard entries - [ ] CI build passes
…ments (#22215) - Max chonk capacity test is very heavy, and running it in nightly debug is not really needed - Noticed that we're still using log_n independent challenges for eccvm, translator, and batched honk-translator, switched to using dyadic power of a single challenge, as it's done elsewhere - Noticed that previously introduced a redundnt gate separator in the batched flow - MegaZK sumcheck path can re-use translator's gate separator
## Summary Fixes WASM build failure introduced by #22145 (--memory_profile_out flag for Chonk memory profiling). Two issues in `memory_profile.cpp`: 1. **Narrowing conversion**: `peak_rss_bytes() / (1024ULL * 1024ULL)` produces `unsigned long long` which can't narrow to `size_t` (32-bit `unsigned long` on WASM) in an initializer list. Fixed with `static_cast<size_t>(...)`. 2. **Undefined symbol**: `peak_rss_bytes()` lives in the `env` module which is intentionally excluded from the WASM link target. Guarded the call with `#ifndef __wasm__`, returning 0 for RSS on WASM (memory profiling is only meaningful on native builds). ## Test plan - [x] `./bootstrap.sh ci` — all 1761 tests pass, both WASM builds succeed ClaudeBox log: https://claudebox.work/s/b5d31ce0b9e0fa7f?run=1
johnathan79717
added
ci-full
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
Fixes/updates addressing findings from translator audit.
…EBUG compat (#22239) ## Summary Fixes ASAN and debug build failures introduced by PR #22218 which added `_GLIBCXX_DEBUG` to debug/ASAN build presets. Under `_GLIBCXX_DEBUG`, `std::vector` is not a literal type (the debug wrapper lacks constexpr constructors), causing compilation errors in any `constexpr` function that uses `std::vector`. Two functions affected: - `affine_element::hash_to_curve()` — uses `std::vector<uint8_t>` internally - `ECCVMHardcodedVKAndHash::get_all()` — returns `std::vector<Commitment>` Neither function is ever evaluated at compile time, so removing `constexpr` is safe. ## Test plan - [x] ASAN preset (`asan-fast`) builds clean from scratch (1103/1103 targets, 0 failures) - [x] Debug preset builds clean from scratch (1111/1111 targets, 0 failures) - [x] Format check passes
Fixing a rare edge case caused by a bug in radix sort recursive calls
…pp (#22261) ## Summary Two `write()` overloads in `serialize.hpp` used `&*buf.end()` to obtain a raw pointer into the newly-resized region. Under `_GLIBCXX_DEBUG` (enabled in asan-fast and debug presets since #22218), dereferencing `end()` is a debug assertion abort — even though the subtracted pointer was in-bounds. Replaced with `buf.data() + buf.size()` which yields the same pointer without touching any iterator. ## Failure `ChonkTests.Basic` in the asan-fast build aborted with: ``` Error: attempt to dereference a past-the-end iterator. ``` ## Fix - `serialize.hpp:166`: `&*buf.end() - sizeof(value)` → `buf.data() + buf.size() - sizeof(value)` - `serialize.hpp:251`: `&*buf.end() - N` → `buf.data() + buf.size() - N` ## Test plan - [x] `ChonkTests.Basic` passes with debug-fast preset (`_GLIBCXX_DEBUG` enabled) ClaudeBox log: https://claudebox.work/s/4aef0cbe07a366e4?run=1
### Audit Context Addresses findings from the "Aztec - Cryptographic Primitives" external audit. This is response 0, covering the findings that have straightforward fixes. ### Changes Made **Finding 1: Off-Curve Proof Commitment Crashes WASM Verifier** Replace `BB_ASSERT(val.on_curve())` with explicit `throw_or_abort` in both FrCodec and U256Codec deserialization paths (`field_conversion.hpp`). This routes the error through the standard error path that is catchable by bbapi's try-catch in native builds, rather than going through `assert_failure`. **Finding 2: WASM Process DOS via Oversized Polynomial in Prover Commit Path** No changes in this PR. Requires a WASM-compatible recovery boundary (setjmp/longjmp or extending try_catch_shim.hpp). Will be addressed in a follow-up. **Finding 3: SRS Downloaded Using HTTP** No changes in this PR. Already mitigated by SHA-256 chunk hash verification (PR #21113). Switching to HTTPS requires resolving the OpenSSL cross-compilation dependency. Deferred. **Finding 4: bbapi Unix Socket Accepts Unauthenticated SRS Replacement** - Add `chmod(socket_path, 0600)` after `bind()` on both macOS and Linux socket paths, matching the 0600 mode already used for the SHM transport. - Add null-guard to `init_bn254_mem_crs_factory()` to prevent replacing an already-initialized SRS, matching the existing guards on `init_bn254_net_crs_factory` and `init_bn254_file_crs_factory`. **Finding 5: Latent Shift-UB in get_scalar_slice** Add `static_assert(MAX_SLICE_BITS < 64, ...)` to encode the invariant that the shift in `get_scalar_slice` remains well-defined. **Finding 6: batch_commit() Subspan Constructed Before Bounds Check** Move the SRS bounds check before the `subspan()` call in `batch_commit()`. `std::span::subspan()` has UB when offset > size(). This brings `batch_commit` in line with `commit()` which already validates first. **Finding 7: Witness Polynomial Coefficients Vulnerable to Leakage** No changes. Threat model does not support this being a real vector: PXE in an extension runs in a separate origin, and for embedded wallets there is no trust boundary. Not prioritized. **Finding 8: BitVector::set() Non-Atomic RMW Has No Thread-Safety Guard** Add NOT THREAD-SAFE documentation to `BitVector` class noting that concurrent `set()` calls on indices in the same 64-bit word will race. Current usage is safe due to per-thread `BucketAccumulators` ownership. **Finding 9: batch_mul Mutates Scalars Through const Interface** Change `batch_mul`'s public interface from `std::span<const Fr>` to `std::span<Fr>`, making the mutation contract explicit. The MSM internally converts scalars from/to Montgomery form, so callers must provide mutable scalars. Updated HyperNova prover/verifier wrappers (drop const) and IPA `reduce_batch_opening_claim` (mutable copy). ### Checklist - [x] Confirmed and documented security issues found - [x] Verified that tests cover all critical paths - [x] Verified build passes (`ninja` clean build) - [x] Ran ecc_tests (830 passed), srs_tests (29 passed), commitment_schemes_tests (88 passed), hypernova_tests (9 passed)
## Summary Replaces #22229 (which was accidentally merged into the wrong branch and reverted). Switches `--memory_profile_out` from peak RSS (getrusage, monotonically increasing) to live heap usage (mallinfo2, goes up and down). This matches what Tracy's memory view showed and reveals when memory is actually freed. - Uses `mallinfo2().uordblks` on Linux, returns 0 on WASM, falls back to peak RSS on other platforms - Renamed `RssCheckpoint` to `MemoryCheckpoint`, `rss_mb` to `heap_mb` - Renamed `add_rss_checkpoint` to `add_checkpoint` Refs: AztecProtocol/barretenberg#1641 ## Example output ``` 0 MultiCallEntrypoint:entrypoint after_alloc 76 MiB 0 MultiCallEntrypoint:entrypoint after_oink 60 MiB <-- freed during commitments 6 EcdsaRAccount:entrypoint after_trace 202 MiB <-- peak 6 EcdsaRAccount:entrypoint after_oink 157 MiB <-- 45 MiB freed 8 SponsoredFPC:sponsor after_alloc 42 MiB <-- small circuit ``` ## Test plan - [x] Builds cleanly (native + asan-fast locally) - [x] Tested with deploy_ecdsar1+sponsored_fpc: heap values go up and down as expected - [ ] CI build passes --------- Co-authored-by: AztecBot <tech@aztec-labs.com>
## Summary Two `write()` overloads in `serialize.hpp` used `&*buf.end()` to get a pointer to newly appended space. Dereferencing `end()` is UB, now caught by `_GLIBCXX_DEBUG` (enabled in asan-fast builds via #22218). This caused `ChonkTests.Basic` to abort in CI. ## Fix Replace `&*buf.end() - offset` with `buf.data() + buf.size() - offset` (well-defined pointer arithmetic). ## Verification - `ChonkTests.Basic` under asan-fast: PASSED - Full `barretenberg/cpp/bootstrap.sh ci`: All 6148 tests passed Detailed analysis: https://gist.github.com/AztecBot/76c4c49c772843199db95099062ffeb3" ClaudeBox log: https://claudebox.work/s/1aa27334b5d24bba?run=1
## Summary PR #22263 replaced `BB_ASSERT(val.on_curve())` with `throw_or_abort()` in `field_conversion.hpp` for the external audit response. This bypasses `BB_DISABLE_ASSERTS()` and causes `ChonkBatchVerifierTests.RandomMixedBatches` to crash when deserializing corrupted IPA proof points during `batch_check()`. Added try-catch in `batch_check()` so exceptions from corrupted proof data return `false` (triggering bisection) instead of terminating the process. This is consistent with the existing try-catch in `parallel_reduce()`. All 29 chonk tests pass. Full analysis: https://gist.github.com/AztecBot/71089def650112d95757a7d3af6c0d67 ClaudeBox log: https://claudebox.work/s/fd385651d6fa6262?run=1
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
5 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
fix: verify accumulated pairing points in native ChonkVerifier (#22224)
chore: enable _GLIBCXX_DEBUG in debug build presets (#22218)
feat: add --memory_profile_out flag for Chonk memory profiling (#22145)
fix: disable max capacity test in debug + tiny gate separator improvements (#22215)
fix: WASM build for memory_profile.cpp (#22231)
fix: translator audit fixes (#22242)
fix: remove constexpr from functions using std::vector for _GLIBCXX_DEBUG compat (#22239)
fix: pippenger edge case (#22256)
fix: avoid dereferencing past-the-end vector iterators in serialize.hpp (#22261)
chore: crypto primitives external audit response 0 (#22263)
feat: switch memory profiling from peak RSS to live heap usage (#22266)
fix: replace UB end-iterator dereference in serialize.hpp (#22262)
fix: catch exceptions in ChonkBatchVerifier::batch_check (#22270)
END_COMMIT_OVERRIDE