From 484b1330482378d0f85cd4a86ba56f3efeec8c63 Mon Sep 17 00:00:00 2001 From: Aztec Bot <49558828+AztecBot@users.noreply.github.com> Date: Thu, 9 Apr 2026 12:16:25 -0400 Subject: [PATCH 1/9] fix: skip heavy recursion tests in debug builds (#22446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Instead of bumping timeouts for slow tests in debug mode, skip them entirely. `HonkRecursionConstraintTest` and `ChonkRecursionConstraintTest` take 400-600s in debug builds, hitting the 600s default timeout. These are heavy recursion constraint tests that build full recursive circuits — the same code paths are already exercised (with debug assertions) by faster tests in the suite. This replaces the timeout-bump approach from PR #22347 with Adam's suggestion to be selective about what runs in debug mode. Supersedes https://github.com/AztecProtocol/aztec-packages/pull/22427 ClaudeBox log: https://claudebox.work/s/c3fd56d2f55c879c?run=2 --- barretenberg/cpp/bootstrap.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/bootstrap.sh b/barretenberg/cpp/bootstrap.sh index d9c1735eee83..feaf939f5dd7 100755 --- a/barretenberg/cpp/bootstrap.sh +++ b/barretenberg/cpp/bootstrap.sh @@ -245,16 +245,17 @@ function test_cmds_native { awk '/^[a-zA-Z]/ {suite=$1} /^[ ]/ {print suite$1}' | \ grep -v 'DISABLED_' | \ while read -r test; do + # Skip heavy recursion tests in debug builds — they take 400-600s and the same + # code paths are already exercised (with assertions) by faster tests in the suite. + if [[ "$native_preset" == *debug* ]] && [[ "$test" =~ ^(HonkRecursionConstraintTest|ChonkRecursionConstraintTest) ]]; then + continue + fi local prefix=$hash # A little extra resource for these tests. # IPARecursiveTests fails with 2 threads. if [[ "$test" =~ ^(AcirAvmRecursionConstraint|ChonkKernelCapacity|AvmRecursiveTests|IPARecursiveTests|HonkRecursionConstraintTest|ChonkRecursionConstraintTest) ]]; then prefix="$prefix:CPUS=4:MEM=8g" fi - # These tests routinely take 400-600s in debug builds; bump from the 600s default. - if [[ "$test" =~ ^(HonkRecursionConstraintTest|ChonkRecursionConstraintTest) ]]; then - prefix="$prefix:TIMEOUT=900s" - fi echo -e "$prefix barretenberg/cpp/scripts/run_test.sh $bin_name $test" done || (echo "Failed to list tests in $bin" && exit 1) done From 993d3694e07236ce7e01974d4560f2ef4caf4b37 Mon Sep 17 00:00:00 2001 From: Aztec Bot <49558828+AztecBot@users.noreply.github.com> Date: Fri, 10 Apr 2026 08:29:30 -0400 Subject: [PATCH 2/9] fix: add clear error for unsatisfiable ACIR AssertZero opcode (#22417) ## Summary When an ACIR `AssertZero` opcode has no variables but a non-zero constant, the circuit is fundamentally unsatisfiable (`nonzero == 0` can never hold). Previously this hit a generic assertion about empty gates with no indication of the real problem. Added an early check in `assert_zero_to_quad_constraints` that detects this case and produces a clear error: "circuit is unsatisfiable. An AssertZero opcode contains no variables but has a non-zero constant, which can never equal zero." ## Test plan - All 60 existing quad/big-quad constraint tests pass - Built and ran `dsl_tests` successfully ClaudeBox log: https://claudebox.work/s/bcd66a8253a37704?run=1 --- .../dsl/acir_format/acir_format.test.cpp | 3 +-- .../acir_format/acir_to_constraint_buf.cpp | 11 +++++++++ .../arithmetic_constraints.test.cpp | 23 +++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp index b01c0c7043c6..6ddff0bdf83c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp @@ -36,8 +36,7 @@ TYPED_TEST(AcirFormatTests, ExpressionWithOnlyConstantTermFails) .return_values = {}, }; - EXPECT_THROW_WITH_MESSAGE(circuit_serde_to_acir_format(circuit), - "split_into_mul_quad_gates: resulted in zero gates."); + EXPECT_THROW_WITH_MESSAGE(circuit_serde_to_acir_format(circuit), "circuit is unsatisfiable"); } TYPED_TEST(AcirFormatTests, ExpressionWithCancellingCoefficientsFails) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp index 519d3f991255..c8450c7ff09f 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp @@ -571,6 +571,17 @@ void assert_zero_to_quad_constraints(Acir::Opcode::AssertZero const& arg, AcirFo }; auto linear_terms = process_linear_terms(arg.value); + + // Check for unsatisfiable constraint: no variables but a non-zero constant means the circuit requires + // `constant == 0` which can never be satisfied. + if (arg.value.mul_terms.empty() && linear_terms.empty()) { + fr constant = from_buffer_with_bound_checks(arg.value.q_c); + BB_ASSERT_EQ(constant, + fr::zero(), + "circuit is unsatisfiable. An AssertZero opcode contains no variables but has a non-zero " + "constant, which can never equal zero."); + } + bool is_single_gate = is_single_arithmetic_gate(arg.value, linear_terms); std::vector> mul_quads = split_into_mul_quad_gates(arg.value, linear_terms); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/arithmetic_constraints.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/arithmetic_constraints.test.cpp index a35451b373ea..703452ac7f40 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/arithmetic_constraints.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/arithmetic_constraints.test.cpp @@ -385,3 +385,26 @@ TYPED_TEST(BigQuadOpcodeGateCountTest, OpcodeGateCount) EXPECT_EQ(program.constraints.gates_per_opcode, std::vector({ BIG_QUAD })); } + +TEST(AssertZeroConstraintTest, UnsatisfiableCircuitWithNonZeroConstantOnly) +{ + // An AssertZero opcode with no variables and a non-zero constant is fundamentally unsatisfiable: + // it asserts `nonzero_constant == 0` which can never hold. + Acir::Expression expr{ .q_c = bb::fr::one().to_buffer() }; + Acir::Opcode::AssertZero assert_zero{ .value = expr }; + AcirFormat af; + EXPECT_THROW_WITH_MESSAGE( + assert_zero_to_quad_constraints(assert_zero, af, 0), + "circuit is unsatisfiable. An AssertZero opcode contains no variables but has a non-zero constant"); +} + +TEST(AssertZeroConstraintTest, SatisfiableCircuitWithZeroConstantOnly) +{ + // An AssertZero opcode with no variables and a zero constant is trivially satisfiable (0 == 0), + // but should still be rejected since it produces a zero gate. + Acir::Expression expr{ .q_c = bb::fr::zero().to_buffer() }; + Acir::Opcode::AssertZero assert_zero{ .value = expr }; + AcirFormat af; + EXPECT_THROW_WITH_MESSAGE(assert_zero_to_quad_constraints(assert_zero, af, 0), + "split_into_mul_quad_gates: resulted in zero gates"); +} From a91a851d34396acb64b3c573a820f053ed8b8b3f Mon Sep 17 00:00:00 2001 From: Raju Krishnamoorthy Date: Fri, 10 Apr 2026 15:54:23 +0200 Subject: [PATCH 3/9] feat: enforce accumulator_not_empty = 0 at ECCVM lagrange_first row (#22461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add `lagrange_first * transcript_accumulator_not_empty = 0` subrelation to `ECCVMTranscriptRelation`. This is a prerequisite for #22334 (masking at top of ECCVM circuit). The audit in #22442 identified that when `lagrange_first` moves to row k (away from the PCS-enforced zero row), `transcript_accumulator_not_empty` is the only shiftable column where a malicious prover can potentially set a non-zero value at the `lagrange_first` row without any existing relation catching it. Setting it to 1 disables `INFINITY_ACC_X/Y`, allowing arbitrary accumulator coordinates to be injected. ## Changes - New subrelation `ACCUMULATOR_NOT_EMPTY_INIT` in `ecc_transcript_relation.hpp` (degree 2) - Gate count updates (+174 gates from the new subrelation): - `ECCVM_RECURSIVE_VERIFIER_GATE_COUNT`: 224336 → 224510 - `CHONK_RECURSION_GATES`: 1491593 → 1491767 ## Test plan - [x] `eccvm_tests` — all 44 tests pass - [x] `stdlib_eccvm_verifier_tests` — `SingleRecursiveVerification` passes with updated gate count - [x] `dsl_tests` — `GateCountChonkRecursion` passes with updated gate count Co-authored-by: notnotraju --- .../dsl/acir_format/gate_count_constants.hpp | 4 ++-- .../relations/ecc_vm/ecc_transcript_relation.hpp | 6 ++++-- .../relations/ecc_vm/ecc_transcript_relation_impl.hpp | 9 +++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/gate_count_constants.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/gate_count_constants.hpp index faad18e4cda0..6950442ec33b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/gate_count_constants.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/gate_count_constants.hpp @@ -113,7 +113,7 @@ constexpr std::tuple HONK_RECURSION_CONSTANTS( // ======================================== // Gate count for Chonk recursive verification (Ultra with RollupIO) -inline constexpr size_t CHONK_RECURSION_GATES = 1491593; +inline constexpr size_t CHONK_RECURSION_GATES = 1491767; // ======================================== // Hypernova Recursion Constants @@ -147,7 +147,7 @@ inline constexpr size_t HIDING_KERNEL_ULTRA_OPS = 127; // ======================================== // Gate count for ECCVM recursive verifier (Ultra-arithmetized) -inline constexpr size_t ECCVM_RECURSIVE_VERIFIER_GATE_COUNT = 224336; +inline constexpr size_t ECCVM_RECURSIVE_VERIFIER_GATE_COUNT = 224510; // ======================================== // Goblin AVM Recursive Verifier Constants diff --git a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation.hpp index 3c40113c16cc..cd2da009e911 100644 --- a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation.hpp @@ -99,11 +99,13 @@ template class ECCVMTranscriptRelationImpl { INFINITY_ACC_X = 29, // Infinity flag consistency: acc_y = 0 when accumulator empty INFINITY_ACC_Y = 30, + // Boundary: accumulator_not_empty must be 0 at lagrange_first row + ACCUMULATOR_NOT_EMPTY_INIT = 31, NUM_SUBRELATIONS, }; - static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ - 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, + static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ + 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, }; static_assert(NUM_SUBRELATIONS == SUBRELATION_PARTIAL_LENGTHS.size()); diff --git a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation_impl.hpp b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation_impl.hpp index 17302c0ff69b..e11c92142bc3 100644 --- a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_transcript_relation_impl.hpp @@ -566,5 +566,14 @@ void ECCVMTranscriptRelationImpl::accumulate(ContainerOverSubrelations& accu is_accumulator_empty * transcript_accumulator_x * scaling_factor; // degree 2 std::get(accumulator) += is_accumulator_empty * transcript_accumulator_y * scaling_factor; // degree 2 + + /** + * @brief Enforce accumulator_not_empty = 0 at the lagrange_first row. + * Without this, a malicious prover can set accumulator_not_empty = 1 at the lagrange_first row, + * which disables INFINITY_ACC_X/Y above, allowing arbitrary accumulator coordinates to be injected + * without any relation catching it. + */ + std::get(accumulator) += + lagrange_first * View(in.transcript_accumulator_not_empty) * scaling_factor; // degree 2 } } // namespace bb From d0385aa5473c20d3a19c9df157f08f53a0593c95 Mon Sep 17 00:00:00 2001 From: Aztec Bot <49558828+AztecBot@users.noreply.github.com> Date: Fri, 10 Apr 2026 09:54:57 -0400 Subject: [PATCH 4/9] fix: skip heavy recursion tests in debug builds, keep one for assertion coverage (#22389) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes nightly barretenberg debug build failure (CI run https://github.com/AztecProtocol/aztec-packages/actions/runs/24228497408). **Root cause:** `AvmRecursionInnerCircuitTests.Tampering` from `vm2_tests` timed out at 600s. CI log: http://ci.aztec-labs.com/ee4d306df4cbfd3c **Fix:** 1. Add `AvmRecursionInnerCircuitTests` to the debug build skip list (matching PR #22446 pattern) 2. Reinstate `HonkRecursionConstraintTestWithoutPredicate/1.GenerateVKFromConstraints` (241s, well within timeout) so the debug-only `native_verification_debug` code path in `honk_recursion_constraint.cpp:126` is still exercised **Debug assertion audit of all skipped suites:** - `HonkRecursionConstraintTest`: The only unique debug code is `native_verification_debug` (`#ifndef NDEBUG`, line 126) — a native side-verification sanity check. All tampering tests call `BB_DISABLE_ASSERTS()` so no debug assertions fire there. We keep `/1.GenerateVKFromConstraints` to cover this. - `ChonkRecursionConstraintTest`: All `BB_ASSERT_DEBUG` in `chonk.cpp` are disabled by `BB_DISABLE_ASSERTS()` in the test. `ChonkTests` tampering (separate binary, not skipped) still runs. - `AvmRecursionInnerCircuitTests`: Only `BB_ASSERT_LTE` (circuit size bounds) which is hit on the happy path by `AvmRecursiveTests`. ClaudeBox log: https://claudebox.work/s/7d8cbad767a56122?run=5 --- barretenberg/cpp/bootstrap.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/bootstrap.sh b/barretenberg/cpp/bootstrap.sh index feaf939f5dd7..060f41086dba 100755 --- a/barretenberg/cpp/bootstrap.sh +++ b/barretenberg/cpp/bootstrap.sh @@ -247,8 +247,12 @@ function test_cmds_native { while read -r test; do # Skip heavy recursion tests in debug builds — they take 400-600s and the same # code paths are already exercised (with assertions) by faster tests in the suite. - if [[ "$native_preset" == *debug* ]] && [[ "$test" =~ ^(HonkRecursionConstraintTest|ChonkRecursionConstraintTest) ]]; then - continue + # Keep WithoutPredicate/1.GenerateVKFromConstraints (241s) so that the debug-only + # native_verification_debug path in honk_recursion_constraint.cpp is still exercised. + if [[ "$native_preset" == *debug* ]] && [[ "$test" =~ ^(HonkRecursionConstraintTest|ChonkRecursionConstraintTest|AvmRecursionInnerCircuitTests) ]]; then + if [[ "$test" != "HonkRecursionConstraintTestWithoutPredicate/1.GenerateVKFromConstraints" ]]; then + continue + fi fi local prefix=$hash # A little extra resource for these tests. From 3178ffc101c77e109ec943db0e1d3d4eed953cab Mon Sep 17 00:00:00 2001 From: nishatkoti Date: Sat, 11 Apr 2026 01:20:00 +0530 Subject: [PATCH 5/9] fix: external audit fixes for Pedersen (#22434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **finding 19: guard in `hash` for empty inputs** --- fixed 29e5bbbe9671a2d7948d7d8ffd81aac46cd085ce The guarantee documented in `pedersen.hpp` “hash output is never point at infinity” applies when the input size is greater than 0. When called on an empty input, `hash` was returning the x co-ordinate of point at infinity. This is addressed now by adding a `throw_or_abort` guard in `hash`to catch empty inputs. Tests are also included which verify that hashing an empty input throws an exception. **finding 2: guard to prevent integer underflow in `convert_buffer` on empty input** --- fixed fc402569a691b979a557e6209b2ae2039b5a08fc `convert_buffer` had an integer underflow when it was passed an empty input. This is addressed by including a `throw_or_abort` guard in `hash_buffer` before the call to `convert_buffer` is made so that an empty input cannot be sent to `convert_buffer`. Tests are also included which verify that hashing an empty input throws an exception. **finding 18: capping `hash_index` `bbapi` Pedersen handlers** --- not fixed `hash_index` is currently not capped. However, as pointed out, it is hardcoded to 0 in the current code and is not reachable. Hence, not fixing this. --- .../crypto/pedersen_hash/pedersen.cpp | 8 +++++++ .../crypto/pedersen_hash/pedersen.test.cpp | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.cpp b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.cpp index 895dc501ab66..3c5829072818 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.cpp @@ -77,6 +77,10 @@ std::vector pedersen_hash_base::convert_buffer template typename Curve::BaseField pedersen_hash_base::hash(const std::vector& inputs, const GeneratorContext context) { + if (inputs.empty()) { + throw_or_abort("pedersen hash: empty input"); + } + Element result = length_generator * Fr(inputs.size()); return (result + pedersen_commitment_base::commit_native(inputs, context)).normalize().x; } @@ -88,6 +92,10 @@ template typename Curve::BaseField pedersen_hash_base::hash_buffer(const std::vector& input, const GeneratorContext context) { + if (input.empty()) { + throw_or_abort("pedersen hash_buffer: empty input"); + } + std::vector converted = convert_buffer(input); if (converted.size() < 2) { diff --git a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp index a3d55868755d..f10a4fb92206 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp @@ -64,5 +64,26 @@ TEST(Pedersen, Hash32Bytes) EXPECT_EQ(got, expected); } +// Verifies that hashing an empty input throws an exception +TEST(Pedersen, HashRejectsEmptyInput) +{ + EXPECT_THROW( + { + auto result = pedersen_hash::hash({}); + static_cast(result); + }, + std::runtime_error); +} + +// Verifies that hashing an empty input throws an exception +TEST(Pedersen, HashBufferRejectsEmptyInput) +{ + EXPECT_THROW( + { + auto result = pedersen_hash::hash_buffer({}); + static_cast(result); + }, + std::runtime_error); +} } // namespace bb::crypto From 855ca1627d5051817243059b1b5cbc62b53c8e4d Mon Sep 17 00:00:00 2001 From: nishatkoti Date: Sat, 11 Apr 2026 01:23:35 +0530 Subject: [PATCH 6/9] chore!: fix BASE off-by-one in create_small_range_constraint in theta step of keccak (#22404) `create_small_range_constraint(var, range)` constrains `var` to `[0-range]`. To constrain `lo` and `hi`, which are base-11 digits, it was invoked with `range=BASE` where `BASE=11`. This constrained `lo` and `hi` incorrectly to `[0-11]` instead of `[0-10]`. This PR fixes this issue by calling `create_small_range_constraint` with `range=BASE-1` which correctly constrains `lo` and `hi` to `[0-10]`. --------- Co-authored-by: ledwards2225 --- .../cpp/scripts/test_chonk_standalone_vks_havent_changed.sh | 4 ++-- .../cpp/src/barretenberg/stdlib/hash/keccak/keccak.cpp | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh b/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh index 8797f2a56079..f0ae20dbfdbd 100755 --- a/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh +++ b/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_changed.sh @@ -21,7 +21,7 @@ script_path="$root/barretenberg/cpp/scripts/test_chonk_standalone_vks_havent_cha # - Generate a hash for versioning: sha256sum bb-chonk-inputs.tar.gz # - Upload the compressed results: aws s3 cp bb-chonk-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-chonk-inputs-[hash(0:8)].tar.gz # Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0 -pinned_short_hash="50947760" +pinned_short_hash="d519f639" pinned_chonk_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-chonk-inputs-${pinned_short_hash}.tar.gz" function update_pinned_hash_in_script { @@ -66,7 +66,7 @@ function check_circuit_vks { bb_check_args+=(--disable_asserts) fi - output=$($bb "${bb_check_args[@]}") || exit_code=$? + output=$($bb "${bb_check_args[@]}" 2>&1) || exit_code=$? if [[ $exit_code -ne 0 ]]; then # Check if this is actually a VK change diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.cpp index 269ea823828e..50c09ee849fc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.cpp @@ -312,8 +312,10 @@ template void keccak::theta(keccak_state& internal) // assert equal should cost 1 gate (multipliers are all constants) D[i].assert_equal((hi * multiplicand).add_two(mid * 11, lo)); - internal.context->create_small_range_constraint(hi.get_witness_index(), static_cast(BASE)); - internal.context->create_small_range_constraint(lo.get_witness_index(), static_cast(BASE)); + // Range-constrain hi and lo to valid base-11 digits [0, 10]. Using BASE - 1 because + // create_small_range_constraint(var, N) constrains var to [0, N] inclusive. + internal.context->create_small_range_constraint(hi.get_witness_index(), static_cast(BASE - 1)); + internal.context->create_small_range_constraint(lo.get_witness_index(), static_cast(BASE - 1)); // If number of bits in KECCAK_THETA_OUTPUT table does NOT cleanly divide KECCAK_LANE_SIZE=64, // we need an additional range constraint to ensure that mid < 11^64 From bc37900f1e2eaeb403e7e7190db3220da4af2c6b Mon Sep 17 00:00:00 2001 From: nishatkoti Date: Sat, 11 Apr 2026 01:27:29 +0530 Subject: [PATCH 7/9] fix: external audit fixes for Keccak (#22436) **finding 7: inverted assertion in keccak fuzzer** --- fixed 4da6a6545264003dcf60c15402fe3fb0b295b589 The keccak fuzzer had an inverted assertion in the circuit checker. This has been fixed. **finding 10: delete dead code pertaining to `hash_field_element`** --- fixed 940f6e437a2784fc96680c9c96b315bc2e6c649d `hash_field_element` and `hash_field_elements` constitute dead code, which was missed to be removed in the internal audit. These have been deleted now. --- .../src/barretenberg/crypto/keccak/keccak.cpp | 27 ------------------- .../src/barretenberg/crypto/keccak/keccak.hpp | 4 --- .../stdlib/hash/keccak/keccak.fuzzer.cpp | 2 +- 3 files changed, 1 insertion(+), 32 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.cpp b/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.cpp index 2fefbecc97b5..44915a0e79a4 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.cpp +++ b/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.cpp @@ -110,30 +110,3 @@ struct keccak256 ethash_keccak256(const uint8_t* data, size_t size) NOEXCEPT keccak(hash.word64s, 256, data, size); return hash; } - -struct keccak256 hash_field_elements(const uint64_t* limbs, size_t num_elements) -{ - std::vector input_buffer(num_elements * KECCAK256_OUTPUT_BYTES); - - for (size_t i = 0; i < num_elements; ++i) { - for (size_t j = 0; j < KECCAK256_OUTPUT_WORDS; ++j) { - uint64_t word = (limbs[i * KECCAK256_OUTPUT_WORDS + j]); - size_t idx = i * 32 + j * 8; - input_buffer[idx] = (uint8_t)((word >> 56) & 0xff); - input_buffer[idx + 1] = (uint8_t)((word >> 48) & 0xff); - input_buffer[idx + 2] = (uint8_t)((word >> 40) & 0xff); - input_buffer[idx + 3] = (uint8_t)((word >> 32) & 0xff); - input_buffer[idx + 4] = (uint8_t)((word >> 24) & 0xff); - input_buffer[idx + 5] = (uint8_t)((word >> 16) & 0xff); - input_buffer[idx + 6] = (uint8_t)((word >> 8) & 0xff); - input_buffer[idx + 7] = (uint8_t)(word & 0xff); - } - } - - return ethash_keccak256(input_buffer.data(), num_elements * KECCAK256_OUTPUT_BYTES); -} - -struct keccak256 hash_field_element(const uint64_t* limb) -{ - return hash_field_elements(limb, 1); -} diff --git a/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.hpp b/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.hpp index 9bf2eaed8db9..e8eba7252fd0 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/keccak/keccak.hpp @@ -40,10 +40,6 @@ void ethash_keccakf1600(uint64_t state[KECCAKF1600_LANES]) NOEXCEPT; struct keccak256 ethash_keccak256(const uint8_t* data, size_t size) NOEXCEPT; -struct keccak256 hash_field_elements(const uint64_t* limbs, size_t num_elements); - -struct keccak256 hash_field_element(const uint64_t* limb); - namespace bb::crypto { /** * @brief A wrapper class used to construct `KeccakTranscript`. diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.fuzzer.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.fuzzer.cpp index 28ac0a7694d9..c39f9f41642a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.fuzzer.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.fuzzer.cpp @@ -81,6 +81,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) assert(circuit_output_u == expected_state); // Verify circuit correctness - assert(!CircuitChecker::check(builder)); + assert(CircuitChecker::check(builder)); return 0; } From ae42cd189b0e2910f12520eebe0d2bfeb7b20040 Mon Sep 17 00:00:00 2001 From: nishatkoti Date: Sat, 11 Apr 2026 01:41:14 +0530 Subject: [PATCH 8/9] fix: external audit fixes for BLAKE (#22443) **finding 11: change `key_array` to use correct element count** --- fixed dfc08b51dc0a20e32cac6c05b2a83b2b3f8e0548 `key_array` over-allocated bytes and zero-initialized the additional bytes that are never used. This has been addressed by allocating only as many bytes as needed. **finding 13: input validation consistency between `blake3s` and `blake3s_constexpr`** --- fixed 97ec044d3e330d250d59d8120327f92803870ad5 `barretenberg` does not support Blake3 with input lengths greater than 1024 bytes. This is enforced in `blake3s` via `BB_ASSERT_LTE(input.size, 1024)` and such a check is missing in `blake3s_constexpr`. The check was probably omitted because `BB_ASSERT_LTE` is runtime and not compile-time. This has been addressed via introducing a compile-time check. **finding 15: performance optimisation in `blake3_constraint.cpp`** --- fixed f9b13ab7e27b9ee026cd7e26743fe92c51042723 `barretenberg` does not support Blake3 with input lengths greater than 1024 bytes. The input size validation check happens after memory is allocated to the input, even for an invalid input. This has been addressed to first check that the input size is less than or equal to 1024 bytes, and then followed by the memory allocation for the input. **finding 22: blake2s increment_counter logic** --- fixed 25eccda8019fc292ee419d806eaa5e2514b6f32e `increment_counter` relied on `ranged_less_than<32>` over an untruncated field value to detect 32-bit carry and implicitly assumed the counter words were constants, making the logic fragile and potentially incorrect (for impractically large messages). This has been fixed by explicitly asserting that `t[0]` and `t[1]` are constants, and restoring the earlier version where the carry is computed using native 32-bit truncation, ensuring correct wrap-around semantics. Thanks for noting this. --- .../src/barretenberg/crypto/blake3s/blake3s.hpp | 2 +- .../src/barretenberg/crypto/blake3s/blake3s.tcc | 9 +++++++-- .../dsl/acir_format/blake3_constraint.cpp | 4 +++- .../barretenberg/stdlib/hash/blake2s/blake2s.cpp | 14 ++++++++------ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.hpp b/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.hpp index 8ccdf341cd06..b00c8624df67 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.hpp @@ -61,7 +61,7 @@ enum blake3s_constant { BLAKE3_MAX_DEPTH = 54 }; -using key_array = std::array; +using key_array = std::array; using block_array = std::array; using state_array = std::array; using out_array = std::array; diff --git a/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.tcc b/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.tcc index d8f354c919ab..3604b541ee0c 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.tcc +++ b/barretenberg/cpp/src/barretenberg/crypto/blake3s/blake3s.tcc @@ -180,7 +180,7 @@ struct output_t { constexpr output_t make_output(const key_array& input_cv, const uint8_t* block, uint8_t block_len, uint8_t flags) { output_t ret; - for (size_t i = 0; i < (BLAKE3_OUT_LEN >> 2); ++i) { + for (size_t i = 0; i < ret.input_cv.size(); ++i) { ret.input_cv[i] = input_cv[i]; } for (size_t i = 0; i < BLAKE3_BLOCK_LEN; i++) { @@ -193,7 +193,7 @@ constexpr output_t make_output(const key_array& input_cv, const uint8_t* block, constexpr void blake3_hasher_init(blake3_hasher* self) { - for (size_t i = 0; i < (BLAKE3_KEY_LEN >> 2); ++i) { + for (size_t i = 0; i < IV.size(); ++i) { self->key[i] = IV[i]; self->cv[i] = IV[i]; } @@ -260,6 +260,11 @@ std::vector blake3s(std::vector const& input) constexpr std::array blake3s_constexpr(const uint8_t* input, const size_t input_size) { + if (std::is_constant_evaluated()) { + if (input_size > 1024U) { + __builtin_trap(); + } + } blake3_hasher hasher; blake3_hasher_init(&hasher); blake3_hasher_update(&hasher, input, input_size); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/blake3_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/blake3_constraint.cpp index 0ae533ecde9e..8d8f9e939024 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/blake3_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/blake3_constraint.cpp @@ -16,6 +16,9 @@ template void create_blake3_constraints(Builder& builder, con using byte_array_ct = bb::stdlib::byte_array; using field_ct = bb::stdlib::field_t; + BB_ASSERT_LTE( + constraint.inputs.size(), 1024U, "Barretenberg does not support blake3 inputs with more than 1024 bytes"); + // Build input byte array by appending constrained byte_arrays byte_array_ct arr = byte_array_ct::constant_padding(&builder, 0); // Start with empty array @@ -30,7 +33,6 @@ template void create_blake3_constraints(Builder& builder, con // Safe write: both arr and element_bytes are constrained arr.write(element_bytes); } - BB_ASSERT_LTE(arr.size(), 1024U, "Barretenberg does not support blake3 inputs with more than 1024 bytes"); byte_array_ct output_bytes = bb::stdlib::Blake3s::hash(arr); for (const auto& [output_byte, result_byte_idx] : zip_view(output_bytes.bytes(), constraint.result)) { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.cpp index 8122b9b4d11c..67530473ed56 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.cpp @@ -39,15 +39,17 @@ namespace bb::stdlib { template void Blake2s::increment_counter(blake2s_state& S, const uint32_t inc) { + // Note that the initial blake2s_state values are circuit constants. + BB_ASSERT(S.t[0].is_constant()); + BB_ASSERT(S.t[1].is_constant()); + field_ct inc_scalar(static_cast(inc)); - // Note that the initial blake2s_state values are circuit constants. S.t[0] = S.t[0] + inc_scalar; - - // Note that although blake2s_state is a circuit constant, we use designated functions such as - // `ranged_less_than` to enforce constraints as appropriate. - bool_ct to_inc = S.t[0].template ranged_less_than<32>(inc_scalar); - S.t[1] = S.t[1] + field_ct(to_inc); + // Enforced constant state (t[0], t[1]) allows computing the carry out-of-circuit (with correct 32-bit wrap) without + // adding constraints. + const bool to_inc = uint32_t(uint256_t(S.t[0].get_value())) < inc; + S.t[1] = S.t[1] + (to_inc ? field_ct(1) : field_ct(0)); } template void Blake2s::compress(blake2s_state& S, byte_array_ct const& in) From b3e284cec019b3b19f8c93b200852391c50a61a3 Mon Sep 17 00:00:00 2001 From: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Date: Fri, 10 Apr 2026 13:56:56 -0700 Subject: [PATCH 9/9] chore: misc hash gadget updates (#22452) | # | Finding | Resolution | |---|---------|------------| | 3, 12 | Msgpack NIL produces null `shared_ptr`, dereferenced in ACIR parsers | **Patched.** NIL rejection in deserializer helpers, plus tests | | 6 | `result_infinite` unconstrained in EC gadgets; prover could forge it | **Already fixed** in PR #21865 | | 9 | `assert_valid_variables` debug-only, heap-allocated a vector per call | **Patched.** Updated to use `initializer_list`/`span` to make cheap enough, always-on, (check in `get_variable` too expensive) | | 16 | Sparse witness map amplifies ~10 bytes into ~137 GB allocation | **Accepted risk.** No cap works without potentially rejecting valid inputs | | 20 | UB shift-by-64 in `get_wnaf_bits`; hash tests Ultra-only | **Patched.** Fixed UB, added Mega coverage to 4 test suites | | 21 | Clang under-aligns NTTP objects, UB in `barrett_reduction` | **Patched.** Root cause different than claimed in issue (see `static_assert` guards), Pointer NTTP refactor resolves UB per UBsan | --- .../dsl/acir_format/acir_null_deref.test.cpp | 185 ++++++++++++++++++ .../dsl/acir_format/serde/acir.hpp | 6 + .../cpp/src/barretenberg/ecc/groups/wnaf.hpp | 12 +- .../src/barretenberg/numeric/uintx/uintx.hpp | 12 +- .../barretenberg/numeric/uintx/uintx.test.cpp | 23 +-- .../barretenberg/numeric/uintx/uintx_impl.hpp | 39 ++-- .../stdlib/encryption/aes128/aes128.test.cpp | 109 ++++++----- .../stdlib/hash/blake2s/blake2s.test.cpp | 41 ++-- .../stdlib/hash/blake3s/blake3s.test.cpp | 73 ++++--- .../stdlib/hash/keccak/keccak.test.cpp | 49 +++-- .../circuit_builder_base.hpp | 4 +- .../circuit_builder_base_impl.hpp | 11 +- 12 files changed, 432 insertions(+), 132 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_null_deref.test.cpp diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_null_deref.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_null_deref.test.cpp new file mode 100644 index 000000000000..b86539fadf0c --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_null_deref.test.cpp @@ -0,0 +1,185 @@ +/** + * @file acir_null_deref.test.cpp + * @brief Exploit and fix tests for null shared_ptr dereference in ACIR deserialization. + * + * Demonstrates that crafted ACIR bytecode containing msgpack NIL values for + * shared_ptr> fields would produce a null pointer dereference in + * acir_to_constraint_buf.cpp, and that the fix (rejecting NIL in conv_fld_from_kvmap + * and conv_fld_from_array) prevents the crash. + * + * Attack vector: An attacker crafts raw ACIR bytecode (bypassing the Noir compiler) + * containing a BlackBoxFuncCall opcode where a fixed-size array field is encoded as + * msgpack NIL (0xc0). Without the fix, the AztecProtocol/msgpack-c fork silently + * converts NIL to a null shared_ptr, which is then dereferenced unconditionally. + * With the fix, deserialization rejects NIL for required fields and throws. + */ + +#include +#include +#include +#include + +#include "acir_to_constraint_buf.hpp" +#include "barretenberg/common/assert.hpp" +#include "barretenberg/serialize/msgpack_impl.hpp" +#include "serde/acir.hpp" + +using namespace acir_format; + +class AcirNullDerefTest : public ::testing::Test {}; + +/** + * Helper: build a ProgramWithoutBrillig wire format by manually packing with msgpack. + * We can't use ProgramWithoutBrillig::msgpack_pack because std::monostate lacks a pack adaptor. + * Instead, we pack the structure by hand: ARRAY[ARRAY[circuit], NIL]. + */ +static std::vector serialize_program_with_circuit(const Acir::Circuit& circuit) +{ + msgpack::sbuffer sbuf; + msgpack::packer packer(sbuf); + + // ProgramWithoutBrillig = ARRAY[functions, unconstrained_functions] + packer.pack_array(2); + + // functions = ARRAY[circuit] + packer.pack_array(1); + packer.pack(circuit); + + // unconstrained_functions = NIL (std::monostate — ignored by ProgramWithoutBrillig::msgpack_unpack) + packer.pack_nil(); + + // Prepend format marker 0x03 (FORMAT_MSGPACK_COMPACT) + std::vector buf; + buf.reserve(1 + sbuf.size()); + buf.push_back(0x03); + buf.insert(buf.end(), sbuf.data(), sbuf.data() + sbuf.size()); + return buf; +} + +// ============================================================================ +// Exploit tests: These construct null shared_ptr directly in the struct, +// bypassing deserialization. They prove the dereference sites are unguarded. +// ============================================================================ + +TEST_F(AcirNullDerefTest, AES128Encrypt_NullIV_DirectCircuit_Crashes) +{ + Acir::BlackBoxFuncCall::AES128Encrypt aes_op{ + .inputs = {}, + .iv = nullptr, + .key = nullptr, + .outputs = {}, + }; + + Acir::BlackBoxFuncCall bbfc; + bbfc.value = aes_op; + + Acir::Circuit circuit{ + .opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } }, + .public_parameters = {}, + .return_values = {}, + }; + + // Dereferences nullptr at acir_to_constraint_buf.cpp:636: *arg.iv + EXPECT_DEATH(circuit_serde_to_acir_format(circuit), ""); +} + +TEST_F(AcirNullDerefTest, Keccakf1600_NullInputs_DirectCircuit_Crashes) +{ + Acir::BlackBoxFuncCall::Keccakf1600 keccak_op{ + .inputs = nullptr, + .outputs = nullptr, + }; + + Acir::BlackBoxFuncCall bbfc; + bbfc.value = keccak_op; + + Acir::Circuit circuit{ + .opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } }, + .public_parameters = {}, + .return_values = {}, + }; + + // Dereferences nullptr at acir_to_constraint_buf.cpp:716: *arg.inputs + EXPECT_DEATH(circuit_serde_to_acir_format(circuit), ""); +} + +TEST_F(AcirNullDerefTest, Sha256Compression_NullInputs_DirectCircuit_Crashes) +{ + Acir::BlackBoxFuncCall::Sha256Compression sha_op{ + .inputs = nullptr, + .hash_values = nullptr, + .outputs = nullptr, + }; + + Acir::BlackBoxFuncCall bbfc; + bbfc.value = sha_op; + + Acir::Circuit circuit{ + .opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } }, + .public_parameters = {}, + .return_values = {}, + }; + + // Dereferences nullptr at acir_to_constraint_buf.cpp:644: *arg.inputs + EXPECT_DEATH(circuit_serde_to_acir_format(circuit), ""); +} + +// ============================================================================ +// Fix tests: These go through the deserialization path (raw bytes or msgpack +// roundtrip). The fix in conv_fld_from_array rejects NIL before it can become +// a null shared_ptr, throwing instead of crashing. +// ============================================================================ + +TEST_F(AcirNullDerefTest, AES128Encrypt_NullIV_FromBytes_ThrowsAfterFix) +{ + Acir::BlackBoxFuncCall::AES128Encrypt aes_op{ + .inputs = {}, + .iv = nullptr, // Serialized as NIL (0xc0) + .key = nullptr, + .outputs = {}, + }; + + Acir::BlackBoxFuncCall bbfc; + bbfc.value = aes_op; + + Acir::Circuit circuit{ + .opcodes = { Acir::Opcode{ Acir::Opcode::BlackBoxFuncCall{ .value = bbfc } } }, + .public_parameters = {}, + .return_values = {}, + }; + + auto buf = serialize_program_with_circuit(circuit); + + // Verify the buffer contains NIL byte (0xc0) from the null shared_ptr + size_t nil_count = 0; + for (uint8_t b : buf) { + if (b == 0xc0) { + nil_count++; + } + } + ASSERT_GE(nil_count, 2U) << "Buffer must contain at least 2 NIL (0xc0) bytes for null iv and key"; + + // After fix: deserialization rejects NIL for required fields → throws instead of SIGSEGV + EXPECT_THROW_WITH_MESSAGE(circuit_buf_to_acir_format(std::move(buf)), "nil value for required field"); +} + +TEST_F(AcirNullDerefTest, NullSharedPtr_RejectedByMsgpackRoundtrip) +{ + // After the fix, a null shared_ptr encoded as NIL is rejected during deserialization. + // conv_fld_from_array detects NIL at the array slot and throws before converting. + Acir::BlackBoxFuncCall::AES128Encrypt original{ + .inputs = {}, + .iv = nullptr, + .key = nullptr, + .outputs = {}, + }; + + // Serialize (null shared_ptr → NIL byte 0xc0) + msgpack::sbuffer sbuf; + msgpack::pack(sbuf, original); + + // Deserialize — should throw because iv slot is NIL + auto oh = msgpack::unpack(sbuf.data(), sbuf.size()); + Acir::BlackBoxFuncCall::AES128Encrypt deserialized; + EXPECT_THROW_WITH_MESSAGE(oh.get().convert(deserialized), "nil value for required field"); +} diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp index f4300460c0d9..cb20bb49b70b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -32,6 +32,9 @@ struct Helpers { { auto it = kvmap.find(field_name); if (it != kvmap.end()) { + if (!is_optional && it->second->type == msgpack::type::NIL) { + throw_or_abort("nil value for required field: " + struct_name + "::" + field_name); + } try { it->second->convert(field); } catch (const msgpack::type_error&) { @@ -54,6 +57,9 @@ struct Helpers { throw_or_abort("index out of bounds: " + struct_name + "::" + field_name + " at " + std::to_string(index)); } auto element = array.ptr[index]; + if (element.type == msgpack::type::NIL) { + throw_or_abort("nil value for required field: " + struct_name + "::" + field_name); + } try { element.convert(field); } catch (const msgpack::type_error&) { diff --git a/barretenberg/cpp/src/barretenberg/ecc/groups/wnaf.hpp b/barretenberg/cpp/src/barretenberg/ecc/groups/wnaf.hpp index 9ec8606b9788..84db855f53ea 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/groups/wnaf.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/groups/wnaf.hpp @@ -87,18 +87,18 @@ template inline uint64_t get_wnaf_bits_const( */ inline uint64_t get_wnaf_bits(const uint64_t* scalar, const uint64_t bits, const uint64_t bit_position) noexcept { - const auto lo_limb_idx = static_cast(bit_position >> 6); const auto hi_limb_idx = static_cast((bit_position + bits - 1) >> 6); const uint64_t lo_shift = bit_position & 63UL; const uint64_t bit_mask = (1UL << static_cast(bits)) - 1UL; + const bool spans_limbs = (lo_limb_idx != hi_limb_idx); - const uint64_t lo = (scalar[lo_limb_idx] >> lo_shift); - const uint64_t hi_shift = bit_position ? 64UL - (bit_position & 63UL) : 0; - const uint64_t hi = ((scalar[hi_limb_idx] << (hi_shift))); - const uint64_t hi_mask = bit_mask & (0ULL - (lo_limb_idx != hi_limb_idx)); + const uint64_t lo = scalar[lo_limb_idx] >> lo_shift; + // spans_limbs is true only when bit_position is not aligned to a limb boundary, so lo_shift > 0 + // and 64 - lo_shift ∈ [1, 63]. (If lo_shift were 0, 64 - lo_shift = 64 would be UB on uint64_t.) + const uint64_t hi = spans_limbs ? (scalar[hi_limb_idx] << (64UL - lo_shift)) : 0UL; - return (lo & bit_mask) | (hi & hi_mask); + return (lo | hi) & bit_mask; } /** diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp index 5a42fb866fb8..1c4c4a776075 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.hpp @@ -271,7 +271,13 @@ template class uintx { base_uint lo; base_uint hi; - template std::pair barrett_reduction() const; + // `modulus` is passed as a pointer NTTP rather than a class-type value NTTP. The latter + // would cause Clang to materialize a template parameter object in `.rodata` at the section's + // natural alignment (16 bytes), which under-aligns an `alignas(32)` type and is UB under + // UBSan's alignment check. A pointer NTTP has no synthesized storage; it just names an + // existing static object, whose `alignas(32)` is honored normally. We therefore pass `&X` where `X` is a named + // `static constexpr base_uint` (see uses in `divmod`). + template std::pair barrett_reduction() const; // This only works (and is only used) for uint256_t std::pair divmod(const uintx& b) const; @@ -307,6 +313,10 @@ using uint512_t = uintx; extern template class uintx; using uint1024_t = uintx; +// Pin the [class.mem]/19 alignment-propagation rule: uintx must inherit alignas(32). +static_assert(alignof(uint512_t) >= 32); +static_assert(alignof(uint1024_t) >= 32); + } // namespace bb::numeric using bb::numeric::uint1024_t; // NOLINT diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp index bbbb85696b8d..3e049c7b9242 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx.test.cpp @@ -8,9 +8,10 @@ using namespace bb; // Explicit instantiation of barrett_reduction for the test-only 1024-bit modulus. namespace bb::numeric { -constexpr uint512_t TEST_MODULUS(uint256_t{ "0x04689e957a1242c84a50189c6d96cadca602072d09eac1013b5458a2275d69b1" }, - uint256_t{ "0x0925c4b8763cbf9c599a6f7c0348d21cb00b85511637560626edfa5c34c6b38d" }); -template std::pair uintx::barrett_reduction() const; +static constexpr uint512_t TEST_MODULUS( + uint256_t{ "0x04689e957a1242c84a50189c6d96cadca602072d09eac1013b5458a2275d69b1" }, + uint256_t{ "0x0925c4b8763cbf9c599a6f7c0348d21cb00b85511637560626edfa5c34c6b38d" }); +template std::pair uintx::barrett_reduction<&TEST_MODULUS>() const; } // namespace bb::numeric namespace { @@ -25,9 +26,9 @@ TEST(uintx, BarrettReduction512) static constexpr uint64_t modulus_1 = 0x97816a916871ca8dUL; static constexpr uint64_t modulus_2 = 0xb85045b68181585dUL; static constexpr uint64_t modulus_3 = 0x30644e72e131a029UL; - constexpr uint256_t modulus(modulus_0, modulus_1, modulus_2, modulus_3); + static constexpr uint256_t modulus(modulus_0, modulus_1, modulus_2, modulus_3); - const auto [quotient_result, remainder_result] = x.barrett_reduction(); + const auto [quotient_result, remainder_result] = x.barrett_reduction<&modulus>(); const auto [quotient_expected, remainder_expected] = x.divmod_base(uint512_t(modulus)); EXPECT_EQ(quotient_result, quotient_expected); EXPECT_EQ(remainder_result, remainder_expected); @@ -37,11 +38,11 @@ TEST(uintx, BarrettReduction1024) { uint1024_t x = engine.get_random_uint1024(); - constexpr uint256_t modulus_lo{ "0x04689e957a1242c84a50189c6d96cadca602072d09eac1013b5458a2275d69b1" }; - constexpr uint256_t modulus_hi{ "0x0925c4b8763cbf9c599a6f7c0348d21cb00b85511637560626edfa5c34c6b38d" }; - constexpr uint512_t modulus{ modulus_lo, modulus_hi }; + static constexpr uint256_t modulus_lo{ "0x04689e957a1242c84a50189c6d96cadca602072d09eac1013b5458a2275d69b1" }; + static constexpr uint256_t modulus_hi{ "0x0925c4b8763cbf9c599a6f7c0348d21cb00b85511637560626edfa5c34c6b38d" }; + static constexpr uint512_t modulus{ modulus_lo, modulus_hi }; - const auto [quotient_result, remainder_result] = x.barrett_reduction(); + const auto [quotient_result, remainder_result] = x.barrett_reduction<&modulus>(); const auto [quotient_expected, remainder_expected] = x.divmod_base(uint1024_t(modulus)); EXPECT_EQ(quotient_result, quotient_expected); EXPECT_EQ(remainder_result, remainder_expected); @@ -336,14 +337,14 @@ TEST(uintx, Slice) TEST(uintx, BarrettReductionRegression) { // Test specific modulus and self values that may cause issues - constexpr uint256_t modulus{ "0xffffffff00000001000000000000000000000000ffffffffffffffffffffffff" }; + static constexpr uint256_t modulus{ "0xffffffff00000001000000000000000000000000ffffffffffffffffffffffff" }; // Test case 1: self = 0xffffffff0000000000000000000000000000000000000000003a000000000000 // This is a 256-bit value, so we need to construct it as a single uint256_t constexpr uint256_t self_value{ "0xffffffff0000000000000000000000000000000000000000003a000000000000" }; uint512_t self(self_value); self = self << 256; - const auto [quotient_result, remainder_result] = self.barrett_reduction(); + const auto [quotient_result, remainder_result] = self.barrett_reduction<&modulus>(); const auto [quotient_expected, remainder_expected] = self.divmod_base(uint512_t(modulus)); EXPECT_EQ(quotient_result, quotient_expected); EXPECT_EQ(remainder_result, remainder_expected); diff --git a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp index b4faf8a927c2..92664fd81524 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uintx/uintx_impl.hpp @@ -236,21 +236,26 @@ template bool uintx::operator<=(const uintx& other) template std::pair, uintx> uintx::divmod(const uintx& b) const { - constexpr uint256_t BN254FQMODULUS256 = - uint256_t(0x3C208C16D87CFD47UL, 0x97816a916871ca8dUL, 0xb85045b68181585dUL, 0x30644e72e131a029UL); - constexpr uint256_t SECP256K1FQMODULUS256 = - uint256_t(0xFFFFFFFEFFFFFC2FULL, 0xFFFFFFFFFFFFFFFFULL, 0xFFFFFFFFFFFFFFFFULL, 0xFFFFFFFFFFFFFFFFULL); - constexpr uint256_t SECP256R1FQMODULUS256 = - uint256_t(0xFFFFFFFFFFFFFFFFULL, 0x00000000FFFFFFFFULL, 0x0000000000000000ULL, 0xFFFFFFFF00000001ULL); - - if (b == uintx(BN254FQMODULUS256)) { - return (*this).template barrett_reduction(); - } - if (b == uintx(SECP256K1FQMODULUS256)) { - return (*this).template barrett_reduction(); - } - if (b == uintx(SECP256R1FQMODULUS256)) { - return (*this).template barrett_reduction(); + // Barrett-reduction fast paths apply only when base_uint == uint256_t; for larger uintx + // instantiations a 256-bit `b` can never equal a 1024-bit-or-wider dividend's modulus. + if constexpr (std::is_same_v) { + // `static` is required so that `&X` below is a valid pointer NTTP for barrett_reduction. + static constexpr uint256_t BN254FQMODULUS256 = + uint256_t(0x3C208C16D87CFD47UL, 0x97816a916871ca8dUL, 0xb85045b68181585dUL, 0x30644e72e131a029UL); + static constexpr uint256_t SECP256K1FQMODULUS256 = + uint256_t(0xFFFFFFFEFFFFFC2FULL, 0xFFFFFFFFFFFFFFFFULL, 0xFFFFFFFFFFFFFFFFULL, 0xFFFFFFFFFFFFFFFFULL); + static constexpr uint256_t SECP256R1FQMODULUS256 = + uint256_t(0xFFFFFFFFFFFFFFFFULL, 0x00000000FFFFFFFFULL, 0x0000000000000000ULL, 0xFFFFFFFF00000001ULL); + + if (b == uintx(BN254FQMODULUS256)) { + return (*this).template barrett_reduction<&BN254FQMODULUS256>(); + } + if (b == uintx(SECP256K1FQMODULUS256)) { + return (*this).template barrett_reduction<&SECP256K1FQMODULUS256>(); + } + if (b == uintx(SECP256R1FQMODULUS256)) { + return (*this).template barrett_reduction<&SECP256R1FQMODULUS256>(); + } } return divmod_base(b); @@ -268,9 +273,11 @@ template std::pair, uintx> uintx, uintx> */ template -template +template std::pair, uintx> uintx::barrett_reduction() const { + const base_uint& modulus = *modulus_ptr; + // N.B. k could be modulus.get_msb() + 1 if we have strong bounds on the max value of (*self) // (a smaller k would allow us to fit `redc_parameter` into `base_uint` and not `uintx`) constexpr size_t k = base_uint::length() - 1; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/encryption/aes128/aes128.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/encryption/aes128/aes128.test.cpp index 78a680cb6041..dcd095a23f34 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/encryption/aes128/aes128.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/encryption/aes128/aes128.test.cpp @@ -3,26 +3,34 @@ #include "barretenberg/crypto/aes128/aes128.hpp" #include "barretenberg/numeric/bitop/sparse_form.hpp" #include "barretenberg/stdlib/primitives/plookup/plookup.hpp" +#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp" #include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" #include using namespace bb; +// ============================================================================ +// Gadget tests — exercised against both UltraCircuitBuilder and MegaCircuitBuilder +// ============================================================================ + +template class StdlibAes128 : public ::testing::Test {}; + +using BuilderTypes = ::testing::Types; +TYPED_TEST_SUITE(StdlibAes128, BuilderTypes); + // Helper function to create field element as either constant or witness -stdlib::field_t create_field_element(UltraCircuitBuilder& builder, - const uint256_t& value, - bool as_witness) +template +stdlib::field_t create_field_element(Builder& builder, const uint256_t& value, bool as_witness) { if (as_witness) { - return stdlib::field_t(stdlib::witness_t(&builder, fr(value))); - } else { - return stdlib::field_t(value); + return stdlib::field_t(stdlib::witness_t(&builder, fr(value))); } + return stdlib::field_t(value); } // Helper function to convert byte array to uint256_t -uint256_t convert_bytes_to_uint256(const uint8_t* data) +static uint256_t convert_bytes_to_uint256(const uint8_t* data) { uint256_t converted(0); for (uint64_t i = 0; i < 16; ++i) { @@ -33,9 +41,9 @@ uint256_t convert_bytes_to_uint256(const uint8_t* data) } // Test function for a specific combination of witness/constant inputs -void test_aes128_combination(bool key_as_witness, bool iv_as_witness, bool input_as_witness) +template void test_aes128_combination(bool key_as_witness, bool iv_as_witness, bool input_as_witness) { - typedef stdlib::field_t field_pt; + using field_pt = stdlib::field_t; uint8_t key[16]{ 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c }; uint8_t out[64]{ 0x76, 0x49, 0xab, 0xac, 0x81, 0x19, 0xb2, 0x46, 0xce, 0xe9, 0x8e, 0x9b, 0x12, 0xe9, 0x19, 0x7d, @@ -48,7 +56,7 @@ void test_aes128_combination(bool key_as_witness, bool iv_as_witness, bool input 0x30, 0xc8, 0x1c, 0x46, 0xa3, 0x5c, 0xe4, 0x11, 0xe5, 0xfb, 0xc1, 0x19, 0x1a, 0x0a, 0x52, 0xef, 0xf6, 0x9f, 0x24, 0x45, 0xdf, 0x4f, 0x9b, 0x17, 0xad, 0x2b, 0x41, 0x7b, 0xe6, 0x6c, 0x37, 0x10 }; - auto builder = UltraCircuitBuilder(); + auto builder = Builder(); // Create input blocks with specified witness/constant configuration std::vector in_field; @@ -88,9 +96,10 @@ void test_aes128_combination(bool key_as_witness, bool iv_as_witness, bool input } // Test function for mixed input blocks (some constant, some witness) +template void test_aes128_mixed_input(bool key_as_witness, bool iv_as_witness, const std::vector& input_block_config) { - typedef stdlib::field_t field_pt; + using field_pt = stdlib::field_t; uint8_t key[16]{ 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c }; uint8_t out[64]{ 0x76, 0x49, 0xab, 0xac, 0x81, 0x19, 0xb2, 0x46, 0xce, 0xe9, 0x8e, 0x9b, 0x12, 0xe9, 0x19, 0x7d, @@ -103,7 +112,7 @@ void test_aes128_mixed_input(bool key_as_witness, bool iv_as_witness, const std: 0x30, 0xc8, 0x1c, 0x46, 0xa3, 0x5c, 0xe4, 0x11, 0xe5, 0xfb, 0xc1, 0x19, 0x1a, 0x0a, 0x52, 0xef, 0xf6, 0x9f, 0x24, 0x45, 0xdf, 0x4f, 0x9b, 0x17, 0xad, 0x2b, 0x41, 0x7b, 0xe6, 0x6c, 0x37, 0x10 }; - auto builder = UltraCircuitBuilder(); + auto builder = Builder(); // Create input blocks with mixed witness/constant configuration std::vector in_field; @@ -144,51 +153,52 @@ void test_aes128_mixed_input(bool key_as_witness, bool iv_as_witness, const std: } } -TEST(stdlib_aes128, encrypt_64_bytes_all_witness) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_all_witness) { - test_aes128_combination(true, true, true); + test_aes128_combination(true, true, true); } -TEST(stdlib_aes128, encrypt_64_bytes_all_constant) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_all_constant) { - test_aes128_combination(false, false, false); + test_aes128_combination(false, false, false); } -TEST(stdlib_aes128, encrypt_64_bytes_key_witness_iv_constant_input_constant) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_key_witness_iv_constant_input_constant) { - test_aes128_combination(true, false, false); + test_aes128_combination(true, false, false); } -TEST(stdlib_aes128, encrypt_64_bytes_key_constant_iv_witness_input_constant) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_key_constant_iv_witness_input_constant) { - test_aes128_combination(false, true, false); + test_aes128_combination(false, true, false); } -TEST(stdlib_aes128, encrypt_64_bytes_key_constant_iv_constant_input_witness) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_key_constant_iv_constant_input_witness) { - test_aes128_combination(false, false, true); + test_aes128_combination(false, false, true); } -TEST(stdlib_aes128, encrypt_64_bytes_key_witness_iv_witness_input_constant) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_key_witness_iv_witness_input_constant) { - test_aes128_combination(true, true, false); + test_aes128_combination(true, true, false); } -TEST(stdlib_aes128, encrypt_64_bytes_key_witness_iv_constant_input_witness) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_key_witness_iv_constant_input_witness) { - test_aes128_combination(true, false, true); + test_aes128_combination(true, false, true); } -TEST(stdlib_aes128, encrypt_64_bytes_key_constant_iv_witness_input_witness) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_key_constant_iv_witness_input_witness) { - test_aes128_combination(false, true, true); + test_aes128_combination(false, true, true); } // Original test for backward compatibility -TEST(stdlib_aes128, encrypt_64_bytes_original) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_original) { - typedef stdlib::field_t field_pt; - typedef stdlib::witness_t witness_pt; + using Builder = TypeParam; + using field_pt = stdlib::field_t; + using witness_pt = stdlib::witness_t; uint8_t key[16]{ 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c }; uint8_t out[64]{ 0x76, 0x49, 0xab, 0xac, 0x81, 0x19, 0xb2, 0x46, 0xce, 0xe9, 0x8e, 0x9b, 0x12, 0xe9, 0x19, 0x7d, @@ -210,7 +220,7 @@ TEST(stdlib_aes128, encrypt_64_bytes_original) return converted; }; - auto builder = UltraCircuitBuilder(); + auto builder = Builder(); std::vector in_field{ witness_pt(&builder, fr(convert_bytes(in))), @@ -239,44 +249,44 @@ TEST(stdlib_aes128, encrypt_64_bytes_original) } // Mixed input tests - different combinations of constant/witness blocks -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_first_witness_rest_constant) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_first_witness_rest_constant) { - test_aes128_mixed_input(false, false, { true, false, false, false }); + test_aes128_mixed_input(false, false, { true, false, false, false }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_alternating_witness_constant) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_alternating_witness_constant) { - test_aes128_mixed_input(false, false, { true, false, true, false }); + test_aes128_mixed_input(false, false, { true, false, true, false }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_first_constant_rest_witness) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_first_constant_rest_witness) { - test_aes128_mixed_input(false, false, { false, true, true, true }); + test_aes128_mixed_input(false, false, { false, true, true, true }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_key_witness_mixed_blocks) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_key_witness_mixed_blocks) { - test_aes128_mixed_input(true, false, { true, false, true, false }); + test_aes128_mixed_input(true, false, { true, false, true, false }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_iv_witness_mixed_blocks) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_iv_witness_mixed_blocks) { - test_aes128_mixed_input(false, true, { false, true, false, true }); + test_aes128_mixed_input(false, true, { false, true, false, true }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_key_iv_witness_mixed_blocks) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_key_iv_witness_mixed_blocks) { - test_aes128_mixed_input(true, true, { true, false, true, false }); + test_aes128_mixed_input(true, true, { true, false, true, false }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_all_witness_blocks) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_all_witness_blocks) { - test_aes128_mixed_input(false, false, { true, true, true, true }); + test_aes128_mixed_input(false, false, { true, true, true, true }); } -TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_all_constant_blocks) +TYPED_TEST(StdlibAes128, encrypt_64_bytes_mixed_input_all_constant_blocks) { - test_aes128_mixed_input(false, false, { false, false, false, false }); + test_aes128_mixed_input(false, false, { false, false, false, false }); } // ============================================================================ @@ -288,6 +298,9 @@ TEST(stdlib_aes128, encrypt_64_bytes_mixed_input_all_constant_blocks) // - Normalization maps: even digits → 0, odd digits → 1 (exactly XOR semantics) // // This allows XOR to be computed as: sparse(a) + sparse(b), then normalize +// +// These tests exercise the plookup lookup tables and `numeric::map_*` helpers +// rather than the aes128 gadget template itself, so they are Ultra-only. // ============================================================================ constexpr uint64_t AES_SPARSE_BASE = 9; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.test.cpp index 57c24f2f51ce..793659f909e8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/blake2s/blake2s.test.cpp @@ -2,6 +2,8 @@ #include "barretenberg/circuit_checker/circuit_checker.hpp" #include "barretenberg/flavor/ultra_flavor.hpp" #include "barretenberg/numeric/uint256/uint256.hpp" +#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp" +#include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" #include "barretenberg/ultra_honk/prover_instance.hpp" #include "blake2s.hpp" #include @@ -9,13 +11,12 @@ using namespace bb; using namespace bb::stdlib; -using Builder = UltraCircuitBuilder; +template class StdlibBlake2s : public ::testing::Test {}; -using field_ct = field_t; -using witness_ct = witness_t; -using byte_array_ct = byte_array; -using public_witness_t = public_witness_t; +using BuilderTypes = ::testing::Types; +TYPED_TEST_SUITE(StdlibBlake2s, BuilderTypes); +namespace { std::vector test_vectors = { "", "a", "ab", @@ -30,9 +31,13 @@ std::vector test_vectors = { "", "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz01", "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012", "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789" }; +} // namespace -TEST(stdlib_blake2s, test_single_block) +TYPED_TEST(StdlibBlake2s, test_single_block) { + using Builder = TypeParam; + using byte_array_ct = byte_array; + Builder builder; std::string input = "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz01"; std::vector input_v(input.begin(), input.end()); @@ -50,8 +55,11 @@ TEST(stdlib_blake2s, test_single_block) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake2s, test_double_block) +TYPED_TEST(StdlibBlake2s, test_double_block) { + using Builder = TypeParam; + using byte_array_ct = byte_array; + Builder builder; std::string input = "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789"; std::vector input_v(input.begin(), input.end()); @@ -69,8 +77,11 @@ TEST(stdlib_blake2s, test_double_block) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake2s, test_witness_and_constant) +TYPED_TEST(StdlibBlake2s, test_witness_and_constant) { + using Builder = TypeParam; + using byte_array_ct = byte_array; + Builder builder; // create a byte array that is a circuit witness @@ -108,8 +119,11 @@ TEST(stdlib_blake2s, test_witness_and_constant) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake2s, test_constant_only) +TYPED_TEST(StdlibBlake2s, test_constant_only) { + using Builder = TypeParam; + using byte_array_ct = byte_array; + Builder builder; size_t len = 65; @@ -136,8 +150,10 @@ TEST(stdlib_blake2s, test_constant_only) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake2s, test_multiple_sized_blocks) +TYPED_TEST(StdlibBlake2s, test_multiple_sized_blocks) { + using Builder = TypeParam; + using byte_array_ct = byte_array; int i = 0; @@ -165,8 +181,11 @@ TEST(stdlib_blake2s, test_multiple_sized_blocks) // second half of every call to `g` to ensure that the overflow doesn't go beyond 3 bits. The edge case that caused // addition overflow issues in Blake is tested here. See https://hackmd.io/@aztec-network/SyTHLkAWZx for a detailed // description of the addition overflow issue. -TEST(stdlib_blake2s, test_edge_case_addition_overflow) +TYPED_TEST(StdlibBlake2s, test_edge_case_addition_overflow) { + using Builder = TypeParam; + using byte_array_ct = byte_array; + std::array v = { 0x0E, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xF6, 0xFF, diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/blake3s/blake3s.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/blake3s/blake3s.test.cpp index 557fe3dbd8ec..1db65ad19129 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/blake3s/blake3s.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/blake3s/blake3s.test.cpp @@ -2,14 +2,19 @@ #include "barretenberg/circuit_checker/circuit_checker.hpp" #include "barretenberg/common/assert.hpp" #include "barretenberg/common/streams.hpp" +#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp" +#include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" #include "blake3s.hpp" #include using namespace bb; -using byte_array_ct = stdlib::byte_array; -using UltraBuilder = UltraCircuitBuilder; +template class StdlibBlake3s : public ::testing::Test {}; +using BuilderTypes = ::testing::Types; +TYPED_TEST_SUITE(StdlibBlake3s, BuilderTypes); + +namespace { std::vector test_vectors = { std::string{}, "a", "ab", @@ -24,15 +29,19 @@ std::vector test_vectors = { std::string{}, "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz01", "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012", "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789" }; +} // namespace -TEST(stdlib_blake3s, test_single_block) +TYPED_TEST(StdlibBlake3s, test_single_block) { - auto builder = UltraBuilder(); + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + + Builder builder; std::string input = "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz01"; std::vector input_v(input.begin(), input.end()); byte_array_ct input_arr(&builder, input_v); - byte_array_ct output = stdlib::Blake3s::hash(input_arr); + byte_array_ct output = stdlib::Blake3s::hash(input_arr); std::vector expected = blake3::blake3s(input_v); @@ -44,14 +53,17 @@ TEST(stdlib_blake3s, test_single_block) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake3s, test_double_block) +TYPED_TEST(StdlibBlake3s, test_double_block) { - auto builder = UltraBuilder(); + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + + Builder builder; std::string input = "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789"; std::vector input_v(input.begin(), input.end()); byte_array_ct input_arr(&builder, input_v); - byte_array_ct output = stdlib::Blake3s::hash(input_arr); + byte_array_ct output = stdlib::Blake3s::hash(input_arr); std::vector expected = blake3::blake3s(input_v); @@ -63,20 +75,26 @@ TEST(stdlib_blake3s, test_double_block) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake3s, test_too_large_input) +TYPED_TEST(StdlibBlake3s, test_too_large_input) { - auto builder = UltraBuilder(); + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + + Builder builder; std::vector input_v(1025, 0); byte_array_ct input_arr(&builder, input_v); - EXPECT_THROW_WITH_MESSAGE(stdlib::Blake3s::hash(input_arr), + EXPECT_THROW_WITH_MESSAGE(stdlib::Blake3s::hash(input_arr), "Barretenberg does not support blake3s with input lengths greater than 1024 bytes."); } -TEST(stdlib_blake3s, test_witness_and_constant) +TYPED_TEST(StdlibBlake3s, test_witness_and_constant) { - UltraBuilder builder; + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + + Builder builder; // create a byte array that is a circuit witness std::string witness_str = "abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz"; @@ -100,7 +118,7 @@ TEST(stdlib_blake3s, test_witness_and_constant) EXPECT_EQ(input_arr.get_value(), input_v); // hash the combined byte array - byte_array_ct output = stdlib::Blake3s::hash(input_arr); + byte_array_ct output = stdlib::Blake3s::hash(input_arr); // compute expected hash auto expected = blake3::blake3s(input_v); @@ -113,9 +131,12 @@ TEST(stdlib_blake3s, test_witness_and_constant) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake3s, test_constant_only) +TYPED_TEST(StdlibBlake3s, test_constant_only) { - UltraBuilder builder; + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + + Builder builder; size_t len = 65; // create a byte array that is a circuit constant @@ -128,7 +149,7 @@ TEST(stdlib_blake3s, test_constant_only) EXPECT_EQ(input_arr.get_value(), input_v); // hash the byte array - byte_array_ct output = stdlib::Blake3s::hash(input_arr); + byte_array_ct output = stdlib::Blake3s::hash(input_arr); // compute expected hash auto expected = blake3::blake3s(input_v); @@ -141,17 +162,20 @@ TEST(stdlib_blake3s, test_constant_only) EXPECT_EQ(proof_result, true); } -TEST(stdlib_blake3s, test_multiple_sized_blocks) +TYPED_TEST(StdlibBlake3s, test_multiple_sized_blocks) { + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + int i = 0; for (auto v : test_vectors) { - UltraBuilder builder; + Builder builder; std::vector input_v(v.begin(), v.end()); byte_array_ct input_arr(&builder, input_v); - byte_array_ct output = stdlib::Blake3s::hash(input_arr); + byte_array_ct output = stdlib::Blake3s::hash(input_arr); auto expected = blake3::blake3s(input_v); @@ -169,18 +193,21 @@ TEST(stdlib_blake3s, test_multiple_sized_blocks) // second half of every call to `g` to ensure that the overflow doesn't go beyond 3 bits. The edge case that caused // addition overflow issues in Blake is tested here. See https://hackmd.io/@aztec-network/SyTHLkAWZx for a detailed // description of the addition overflow issue. -TEST(stdlib_blake3s, test_edge_case_addition_overflow) +TYPED_TEST(StdlibBlake3s, test_edge_case_addition_overflow) { + using Builder = TypeParam; + using byte_array_ct = stdlib::byte_array; + std::array v = { 0xC3, 0x2B, 0xC3, 0x91, 0x23, 0xFF, 0xFF, 0xFF, 0xFF, 0xC3, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xC3, 0x03, 0x83, 0x83, 0x83, 0x40 }; - UltraBuilder builder; + Builder builder; std::vector input_v(v.begin(), v.end()); byte_array_ct input_arr(&builder, input_v); - byte_array_ct output = stdlib::Blake3s::hash(input_arr); + byte_array_ct output = stdlib::Blake3s::hash(input_arr); auto expected = blake3::blake3s(input_v); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.test.cpp index 4135a5a6f98b..4400c8bfd2ed 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/hash/keccak/keccak.test.cpp @@ -2,23 +2,28 @@ #include "../../primitives/plookup/plookup.hpp" #include "barretenberg/circuit_checker/circuit_checker.hpp" #include "barretenberg/numeric/random/engine.hpp" +#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp" +#include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" #include "keccak.hpp" #include using namespace bb; -using Builder = UltraCircuitBuilder; -using byte_array = stdlib::byte_array; -using public_witness_t = stdlib::public_witness_t; -using field_ct = stdlib::field_t; -using witness_ct = stdlib::witness_t; +template class StdlibKeccak : public ::testing::Test {}; + +using BuilderTypes = ::testing::Types; +TYPED_TEST_SUITE(StdlibKeccak, BuilderTypes); namespace { auto& engine = numeric::get_debug_randomness(); } -TEST(stdlib_keccak, keccak_format_input_table) +TYPED_TEST(StdlibKeccak, keccak_format_input_table) { + using Builder = TypeParam; + using field_ct = stdlib::field_t; + using witness_ct = stdlib::witness_t; + Builder builder = Builder(); for (size_t i = 0; i < 25; ++i) { @@ -41,8 +46,12 @@ TEST(stdlib_keccak, keccak_format_input_table) EXPECT_EQ(proof_result, true); } -TEST(stdlib_keccak, keccak_format_output_table) +TYPED_TEST(StdlibKeccak, keccak_format_output_table) { + using Builder = TypeParam; + using field_ct = stdlib::field_t; + using witness_ct = stdlib::witness_t; + Builder builder = Builder(); for (size_t i = 0; i < 25; ++i) { @@ -58,8 +67,12 @@ TEST(stdlib_keccak, keccak_format_output_table) EXPECT_EQ(proof_result, true); } -TEST(stdlib_keccak, keccak_theta_output_table) +TYPED_TEST(StdlibKeccak, keccak_theta_output_table) { + using Builder = TypeParam; + using field_ct = stdlib::field_t; + using witness_ct = stdlib::witness_t; + Builder builder = Builder(); for (size_t i = 0; i < 25; ++i) { @@ -84,8 +97,12 @@ TEST(stdlib_keccak, keccak_theta_output_table) EXPECT_EQ(proof_result, true); } -TEST(stdlib_keccak, keccak_rho_output_table) +TYPED_TEST(StdlibKeccak, keccak_rho_output_table) { + using Builder = TypeParam; + using field_ct = stdlib::field_t; + using witness_ct = stdlib::witness_t; + Builder builder = Builder(); constexpr_for<0, 25, 1>([&] { @@ -109,7 +126,7 @@ TEST(stdlib_keccak, keccak_rho_output_table) const uint256_t expected_msb = (binary_native >> 63); field_ct limb(witness_ct(&builder, extended_native)); field_ct result_msb; - field_ct result_limb = stdlib::keccak::normalize_and_rotate(limb, result_msb); + field_ct result_limb = stdlib::keccak::template normalize_and_rotate(limb, result_msb); EXPECT_EQ(static_cast(result_limb.get_value()), expected_limb); EXPECT_EQ(static_cast(result_msb.get_value()), expected_msb); }); @@ -119,8 +136,12 @@ TEST(stdlib_keccak, keccak_rho_output_table) EXPECT_EQ(proof_result, true); } -TEST(stdlib_keccak, keccak_chi_output_table) +TYPED_TEST(StdlibKeccak, keccak_chi_output_table) { + using Builder = TypeParam; + using field_ct = stdlib::field_t; + using witness_ct = stdlib::witness_t; + static constexpr uint64_t chi_normalization_table[5]{ 0, // 1 + 2a - b + c => a xor (~b & c) 0, 1, 1, 0, @@ -156,8 +177,12 @@ TEST(stdlib_keccak, keccak_chi_output_table) } // Matches the fuzzer logic -TEST(stdlib_keccak, permutation_opcode) +TYPED_TEST(StdlibKeccak, permutation_opcode) { + using Builder = TypeParam; + using field_ct = stdlib::field_t; + using witness_ct = stdlib::witness_t; + Builder builder = Builder(); // Create a random state (25 lanes of 64 bits) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp index 3a556e5a6309..4e7a87e629fe 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp @@ -16,6 +16,7 @@ #include #include +#include #include namespace bb { @@ -76,7 +77,8 @@ template class CircuitBuilderBase { * variables * @param variable_indices The indices to validate */ - void assert_valid_variables(const std::vector& variable_indices); + void assert_valid_variables(std::initializer_list variable_indices); + void assert_valid_variables(std::span variable_indices); /** * @brief The permutation on variable tags, as a constituent of the generalized permutation argument. diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base_impl.hpp index 2571bf23179a..98298b3f883a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/circuit_builder_base_impl.hpp @@ -145,13 +145,18 @@ void CircuitBuilderBase::assert_equal(const uint32_t a_variable_idx, } template -void CircuitBuilderBase::assert_valid_variables([[maybe_unused]] const std::vector& variable_indices) +void CircuitBuilderBase::assert_valid_variables(std::initializer_list variable_indices) +{ + for (const auto& variable_index : variable_indices) { + BB_ASSERT_LT(variable_index, variables.size()); + } +} + +template void CircuitBuilderBase::assert_valid_variables(std::span variable_indices) { -#ifndef NDEBUG for (const auto& variable_index : variable_indices) { BB_ASSERT_LT(variable_index, variables.size()); } -#endif } template bool CircuitBuilderBase::failed() const