fix: graceful failures in verifier code paths + other fixes#22311
Merged
iakovenkos merged 10 commits intomerge-train/barretenbergfrom Apr 6, 2026
Merged
fix: graceful failures in verifier code paths + other fixes#22311iakovenkos merged 10 commits intomerge-train/barretenbergfrom
iakovenkos merged 10 commits intomerge-train/barretenbergfrom
Conversation
AztecBot
added
ci-draft
Convert BB_ASSERT/BB_ASSERT_EQ/BB_ASSERT_LTE calls in verifier code paths to return false or throw_or_abort instead of panicking. Malformed proofs should cause verification to fail gracefully, not crash the process via SIGABRT. Changes by area: - IPA (ipa.hpp): 5 assertions in reduce_verify_internal_native, batch_reduce_verify, and reduce_verify_internal_recursive now return false on invalid input instead of asserting - Translator verifier: 4 size-check assertions now return ReductionResult with reduction_succeeded=false - KZG: MSM size assertion converted to info() warning - Shplonk: constructor assertion converted to explicit throw_or_abort - Shplemini: 2 assertions converted to graceful handling - Proof compression: 11 assertions in decompress_chonk_proof converted to explicit throw_or_abort with descriptive messages - ChonkProof: from_field_elements assertion converted to throw_or_abort - Transcript: 2 bounds-check assertions in receive_from_prover and deserialize_from_buffer converted to explicit throw_or_abort
AztecBot
force-pushed
the
claudebox/6a1ba966035b66be-3
branch
from
816473c to
d026957
Compare
- IPA recursive: move SRS check before circuit construction, remove redundant witness-value G_zero comparison (assert_equal suffices) - KZG: throw_or_abort for MSM size mismatch (structural invariant) - Shplemini: throw_or_abort for commitment mismatch and ZK requirement - Translator: throw_or_abort for PCS size mismatch (compile-time invariant)
msgpack_unpack now checks that the raw value is < modulus before converting to Montgomery form, closing a proof-boundary inconsistency where MsgPack paths accepted non-canonical encodings that stricter paths (concatenate_proof, proof compression) would reject.
Move cleanup of loaded_circuit_constraints, loaded_circuit_vk, and loaded_circuit_name to immediately after extracting their values, before any step that can throw. Previously, a failure in VK validation, circuit creation, or accumulation would leave the request with a moved-from optional that still reports has_value()==true, allowing a subsequent ChonkAccumulate to bypass the ChonkLoad guard and operate on poisoned state.
ChonkStart now resets loaded_circuit_name, loaded_circuit_constraints, and loaded_circuit_vk when beginning a new session. Previously, a circuit loaded in a prior session would survive across ChonkStart calls, allowing ChonkAccumulate to silently reuse stale state without requiring a fresh ChonkLoad.
Use the offset-tracking msgpack::unpack overload and verify that the entire buffer was consumed, consistent with the hardened pattern already used in PrivateExecutionStepRaw loaders.
Add a size check before subtracting LENGTH_WITHOUT_PUB_INPUTS from the MegaZK proof size, matching the existing guard in UltraVerifier. Without this, a truncated proof would cause size_t underflow and an obscure crash deep in OinkVerifier rather than a clean early error.
The has_last_app_been_accumulated flag used num_circuits - 4 but the last app is at position num_circuits - 3 (pre-increment). This caused the native verifier accumulator hash to never be updated for the last app, producing misleading debug output. Debug-only code, no production impact.
…NativeFieldRelation Add negative test that applies the (acc += q, quotient -= 1) mutation at RESULT_ROW and verifies the higher carry check (subrelation 1) catches it. Also populate evaluation_input_x and batching_challenge_v in the existing accumulator transfer test helper so it supports NonNativeField checks.
Random bytes can now trigger multiple exception types: msgpack type_error, runtime_error from trailing data check, or runtime_error from non-canonical field encoding. Use EXPECT_ANY_THROW instead of checking for a specific exception type.
iakovenkos
changed the title
suyash67
reviewed
Apr 6, 2026
| static bool full_verify_recursive(const VK& vk, const OpeningClaim<Curve>& opening_claim, auto& transcript) | ||
| requires Curve::is_stdlib_type | ||
| { | ||
| // Check SRS size up front before any circuit construction |
suyash67
reviewed
Apr 6, 2026
| * Replacing current_acc with current_acc + p and quotient with quotient - 1 preserves this | ||
| * equation, plus its mod-2^272 and mod-r projections. However, the limb-level carry witnesses | ||
| * (relation_wide_limbs) become stale: the new intermediate sums at limbs 2-3 no longer match | ||
| * the old high carry value, and subrelation 1 (higher mod 2^136 check) detects the mismatch. |
suyash67
approved these changes
Apr 6, 2026
Contributor
suyash67
left a comment
suyash67
left a comment
There was a problem hiding this comment.
looks good, thanks for adding the failure test case in translator!
AztecBot
added a commit
that referenced
this pull request
Apr 6, 2026
…ormat Fr::from_u64 was storing values in little-endian, but the C++ field::msgpack_unpack expects big-endian (network byte order). This was masked before because the C++ side silently reduced non-canonical values via to_montgomery_form_reduced(). After #22311 added strict non-canonical field validation (throwing for values >= modulus), values like Fr::from_u64(123) produce 0x7B<<248 which exceeds the BN254 Fr modulus (0x30...) and cause a C++ exception that propagates through the FFI boundary, aborting the Rust test process.
3 tasks
ludamad
pushed a commit
that referenced
this pull request
Apr 6, 2026
…2233) ## Summary Fixes bb-rs test failure introduced by #22311 (graceful failures in verifier code paths). `Fr::from_u64` was storing values in **little-endian**, but the C++ `field::msgpack_unpack` expects **big-endian** (network byte order). This was masked before because the C++ side silently reduced non-canonical values via `to_montgomery_form_reduced()`. After #22311 added strict non-canonical field validation (throwing for values >= modulus), values like `Fr::from_u64(123)` produce `0x7B<<248` which exceeds the BN254 Fr modulus (`0x30644e72...`) and cause a C++ exception that propagates through the FFI boundary, aborting the Rust test process with "Rust cannot catch foreign exceptions". ## Fix One-line change: store the u64 value in bytes 24..32 as big-endian instead of bytes 0..8 as little-endian. ## Test plan - [x] Reproduced failure locally — `test_pedersen_commit_deterministic` crashes with SIGABRT - [x] Applied fix — all 70 bb-rs tests pass (3 perf tests ignored) - [ ] `./bootstrap.sh ci` running
2 tasks
ludamad
pushed a commit
that referenced
this pull request
Apr 6, 2026
…nical field encoding (#22333) ## Summary Fixes CI failure in `merge-train/barretenberg` caused by the interaction of #22233 (Fr::from_u64 big-endian encoding) and #22311 (strict non-canonical field validation). The `corruptProofFields()` test helper was flipping high-order bytes (indices 0,1) of field elements, creating values exceeding the BN254 Fr modulus. The strict validation now rejects these at deserialization time with `non-canonical encoding (value >= modulus)` before they reach the verifier. **Fix**: Flip low-order bytes (indices 30,31) instead — still corrupts the proof but keeps field values canonical. ## Test plan - [x] `batch_verifier.test.ts` — all 4 tests pass locally (previously 1 failing) ClaudeBox log: https://claudebox.work/s/e5146dc92fea29dd?run=1
github-merge-queue bot
pushed a commit
that referenced
this pull request
Apr 6, 2026
BEGIN_COMMIT_OVERRIDE fix: reject VK with log_circuit_size=0 in UltraKeccak verifier (#22319) chore: merkle tree audit 2 (#21475) fix: graceful failures in verifier code paths + other fixes (#22311) fix: Fr::from_u64 big-endian encoding to match C++ msgpack format (#22233) fix: corrupt low-order bytes in batch verifier test to avoid non-canonical field encoding (#22333) fix: skip MsgpackRejectsNonCanonical test in WASM builds (#22335) END_COMMIT_OVERRIDE
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.
Summary
Hardens barretenberg verifier code paths and API boundaries based on security review findings.
Verifier robustness
BB_ASSERTwith context-appropriate failures in verifier code paths —throw_or_abortfor parsing/structural invariants, return false for native verification failures, circuit constraints for recursive verifiersfull_verify_recursive(fail early on misconfiguration)assert_equalis the real enforcement)Input validation
MsgPackdeserialization (value >= modulusnow throws instead of silently reducing)ChonkProofMsgPackdeserialization (consistent with hardenedPrivateExecutionStepRawloaders)API state machine fixes
ChonkStart: clear staleloaded_circuit_*state when starting a new session, preventing silent reuse of circuits from a prior sessionChonkAccumulate: clear loaded state immediately after move, preventing use-after-move on retry if a later step throwsDebug
has_last_app_been_accumulatedflag (num_circuits - 4→num_circuits - 3), debug-onlyTests
MsgPackfield encoding rejection (all 6 field types)NonNativeFieldRelationaccumulator alias attack