Open
Conversation
Minor fixes in groups: - `serialize_from_buffer` accepted off-curve bytes without checking the curve equation. Now throws if the deserialized point doesn't satisfy y² = x³ + ax + b. - `from_compressed_unsafe` passed x₂ = r + n to `Fq()` without a bounds check. Since r > p − n for most ECDSA signatures, Fq would silently reduce x₂ modulo p and return a spurious candidate. Now guards both candidates with `x < Fq::modulus` before evaluating the curve equation. - `batch_mul_with_endomorphism` skew corrections used add_chunked, which has no edge-case handling and aborts when the accumulator equals ±P. Switched to `affine_element::operator+` (Jacobian) which handles all cases correctly. - `batch_invert` used `reserve()` then indexed with operator[], which is UB (size remains 0). Changed to `resize()`.
## Summary Fixes nightly barretenberg debug build failures. ### 1. `CBind.CatchesExceptionAndReturnsErrorResponse` test (bbapi_srs.cpp) The test relied on undefined behavior (buffer overread in `from_buffer`) to trigger an exception. In debug builds, the UB silently reads garbage and returns success instead of throwing. Fix: add proper bounds validation to `SrsInitSrs::execute` and `SrsInitGrumpkinSrs::execute`. ### 2. `ChonkRecursionConstraintTest.GenerateRecursiveChonkVerifierVKFromConstraints` (pairing_points.hpp) In VK generation mode (`write_vk`), witnesses are zero-initialized and the recursive verifier runs on structurally-correct but numerically-meaningless dummy data. The stdlib curve operations produce intermediate/final pairing points whose **native values** (extracted via `.get_value()`) are garbage — they don't satisfy y² = x³ + 3. The stdlib `PairingPoints` constructor has a debug-only (`#ifndef NDEBUG`) sanity check that calls `native_pp.check()` → `reduced_ate_pairing_batch_precomputed()` → `throw_or_abort()` on these off-curve points. In release builds this check is compiled out entirely. Fix: wrap the debug-only native pairing checks in try-catch so they log the failure without crashing. ## Test Plan - All 15 bbapi tests pass in debug mode - `ChonkRecursionConstraintTest.GenerateRecursiveChonkVerifierVKFromConstraints` passes in debug mode - `ChonkTests.Basic` passes in debug mode" --------- Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>
Refactor chonk.cpp: - Split `accumulate` into two code paths: hiding kernel and everything else - Split recursive verification function into two functions: folding verification and public inputs + consistency checks - Some renaming
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
chore: fixes in ecc/group internal audit (#21920)
fix: add bounds checking to SrsInitSrs to fix debug build test (#21985)
chore: Refactor chonk.cpp (#22055)
fix: a bunch of minor fixes (#21993)
END_COMMIT_OVERRIDE