Merge branch 'next' into ad/chore/ci-release-pr-canary

Author: AztecBot

barretenberg/.claude/skills/stdlib-point-at-infinity/SKILL.md

@@ -0,0 +1,68 @@
+---
+name: stdlib-point-at-infinity
+description: Guidelines for handling point-at-infinity in stdlib circuit types. Use when working on serialization, public inputs, or cycle_group/biggroup code.
+---
+
+# Stdlib Point-at-Infinity Handling
+
+## Two stdlib element types for bn254
+
+- **`element_default::element`** (Ultra arithmetization): Separate `_is_infinity` bool_ct flag. Coordinates can be arbitrary when infinity is set. Has `is_point_at_infinity()` returning `bool_ct`.
+- **`goblin_element`** (Mega arithmetization): Represents infinity as `(0,0)` by construction. No separate infinity flag. No `is_point_at_infinity()` method on the circuit type. ECCVM enforces the `(0,0)` convention.
+- The alias `element<Builder, Fq, Fr, G>` resolves to one or the other via `IsGoblinBigGroup`.
+- Both types have `get_value()` returning a native `affine_element` with `is_point_at_infinity()`.
+
+## cycle_group (Grumpkin) infinity handling
+
+`cycle_group` has `_is_infinity` bool computed from `x^2 + 5*y^2 == 0` (which has no non-trivial solutions in the field, since `p == 2 mod 5`). For `(0,0)` this gives `0 == 0`, so infinity is auto-detected.
+
+**Non-canonical infinity**: `cycle_group` operations (`operator+`, `operator-`, `dbl`, `batch_mul`) can produce points at infinity with non-canonical coordinates (`_is_infinity=true` but `x,y != 0,0`). This happens because the arithmetic uses `conditional_assign` to avoid division by zero -- the "real" coordinates are garbage when the infinity flag is set.
+
+**Observation boundaries**: Canonicalization to `(0,0)` is deferred to these boundaries:
+- `StdlibCodec::serialize_to_fields` -- canonicalizes `grumpkin_commitment` via `conditional_assign`
+- `cycle_group::set_public` -- canonicalizes before exposing as public inputs
+- `cycle_group::operator==` and `assert_equal` -- handles infinity comparison
+
+## StdlibCodec::serialize_to_fields (field_conversion.hpp)
+
+- **grumpkin_commitment**: Canonicalizes infinity to `(0,0)` via `conditional_assign`. This IS needed because the `IPA::accumulate` -> `full_verify_recursive` path computes `G_zero_1 + G_zero_2 * alpha` using `cycle_group` arithmetic, which can produce non-canonical infinity when a malicious prover sends both `G_zero` values as `(0,0)`.
+- **bn254_commitment**: Allows canonical `(0,0)` infinity; asserts (`BB_ASSERT`) that infinity points have zero coordinates. All existing code paths (public inputs, transcript) produce canonical `(0,0)` infinity, so the assert is a safety guard against misuse. No canonicalization is performed (unlike `grumpkin_commitment`), since there are no available code paths that produce non-canonical bn254 infinity.
+
+## Analyzing whether canonicalization is needed
+
+Trace whether the value comes from:
+
+1. **Deserialization** (from public inputs via `reconstruct_from_public`, or from transcript via `receive_from_prover`): Coordinates are already canonical `(0,0)` for infinity. Canonicalization is a no-op.
+2. **cycle_group arithmetic** (`operator+`, `operator-`, `dbl`, `batch_mul`): Coordinates may be non-canonical when `_is_infinity` is true. Canonicalization IS needed.
+
+Key production paths for grumpkin commitments through `serialize_to_fields`:
+- **IPA `add_claim_to_hash_buffer`** (verifier side): Commitment is deserialized from public inputs -> already canonical.
+- **IPA `accumulate` hashing of `G_zero`**: `G_zero` is deserialized from transcript -> already canonical.
+- **IPA `full_verify_recursive`**: Accumulated commitment is the result of cycle_group arithmetic (`G_zero_1 + G_zero_2 * alpha`) -> may be non-canonical if infinity -> canonicalization needed.
+- **VK hashing** (`flavor.hpp` `to_field_elements`/`hash_with_origin_tagging`): Commitments are deserialized from fields -> already canonical.
+
+## Recursive verification and malicious provers
+
+For recursive verifier circuits, the circuit must be **constructible** even with malicious witness values (it just won't be satisfiable). This means:
+- Do NOT use `BB_ASSERT` on values a malicious prover can control -- it would crash circuit construction.
+- Use `conditional_assign` canonicalization instead, which produces correct circuit constraints regardless of witness values.
+- `BB_ASSERT` is appropriate for invariants that hold across all existing code paths (e.g., `bn254_commitment` in `serialize_to_fields` asserts canonical `(0,0)` form for infinity, since all paths that can reach it produce canonical infinity).
+
+## Common bug patterns to watch for
+
+These patterns have caused repeated bugs across biggroup, cycle_group, ECCVM, AVM, and ECDSA:
+
+### 1. Representation mismatch: internal sentinel vs `(0,0)`
+BB's native `affine_element` uses an internal sentinel for infinity (MSB set on the x coordinate's raw representation, not a valid field element). Noir, AVM, and the transcript convention use `(0,0)`. Any code that reads raw coordinates without checking `is_point_at_infinity()` (or without calling `get_standard_form()` on stdlib types) will see sentinel values, not `(0,0)`. Always use the standard-form convention `(0,0)` at component boundaries.
+
+### 2. Forgetting to propagate the infinity flag through conditional operations
+When doing `conditional_assign` or `conditional_select` on points, both the coordinates AND the `_is_infinity` flag must be selected. Multiple ECDSA and biggroup bugs came from selecting x/y but leaving `_is_infinity` unchanged.
+
+### 3. Incomplete addition formulas crash on infinity
+Performance-optimized ECC (chain_add, Montgomery ladder) assumes inputs are never infinity and never equal. When infinity appears as an intermediate value, these formulas divide by zero or produce wrong results. If a code path can encounter infinity mid-computation, use complete addition (`operator+`) instead of `chain_add_start`/`chain_add`/`chain_add_end`. This costs ~2% more gates but is correct.
+
+### 4. Constructor and validation bypasses
+Constructors that accept a direct `is_infinity` flag can bypass on-curve validation (a point with `is_infinity=true` but `x,y != 0` passes `validate_on_curve` because the check is skipped for infinity). The 4-argument biggroup constructor with explicit infinity flag is now private. Prefer the 2-argument `(x, y)` constructor which auto-detects infinity from `x == 0 && y == 0`.
+
+### 5. Forgetting to canonicalize before comparison or hashing
+`cycle_group::operator==` and `assert_equal` handle infinity correctly, but raw coordinate comparison does not. Before comparing or hashing point coordinates, ensure infinity points have canonical `(0,0)` coordinates. The observation boundary pattern (serialize_to_fields, set_public, operator==) exists for this reason.

barretenberg/cpp/cmake/module.cmake

@@ -111,6 +111,12 @@ function(barretenberg_module_with_sources MODULE_NAME)
         endif()
         list(APPEND lib_targets ${MODULE_NAME})
 
+        set(MODULE_LINK_NAME ${MODULE_NAME})
+    elseif(MODULE_DEPENDENCIES AND NOT BENCH_SOURCE_FILES AND NOT FUZZERS_SOURCE_FILES)
+        # Header-only module with dependencies: create an INTERFACE library
+        # so dependents can still reference this module by name.
+        add_library(${MODULE_NAME} INTERFACE)
+        target_link_libraries(${MODULE_NAME} INTERFACE ${MODULE_DEPENDENCIES})
         set(MODULE_LINK_NAME ${MODULE_NAME})
     endif()
 

barretenberg/cpp/cmake/threading.cmake

@@ -30,6 +30,7 @@ if(ENABLE_PAR_ALGOS)
     find_package(TBB QUIET OPTIONAL_COMPONENTS tbb)
     if(${TBB_FOUND})
         message(STATUS "std::execution parallel algorithms are enabled.")
+        link_libraries(TBB::tbb)
     else()
         message(STATUS "Could not locate Intel TBB, disabling std::execution parallel algorithms.")
         add_definitions(-DNO_PAR_ALGOS)

barretenberg/cpp/src/CMakeLists.txt

@@ -165,13 +165,10 @@ set(BARRETENBERG_TARGET_OBJECTS
     $<TARGET_OBJECTS:common_objects>
     $<TARGET_OBJECTS:crypto_aes128_objects>
     $<TARGET_OBJECTS:crypto_blake2s_objects>
-    $<TARGET_OBJECTS:crypto_blake3s_objects>
-    $<TARGET_OBJECTS:crypto_ecdsa_objects>
     $<TARGET_OBJECTS:crypto_keccak_objects>
     $<TARGET_OBJECTS:crypto_pedersen_commitment_objects>
     $<TARGET_OBJECTS:crypto_pedersen_hash_objects>
     $<TARGET_OBJECTS:crypto_poseidon2_objects>
-    $<TARGET_OBJECTS:crypto_schnorr_objects>
     $<TARGET_OBJECTS:crypto_sha256_objects>
     $<TARGET_OBJECTS:dsl_objects>
     $<TARGET_OBJECTS:ecc_objects>

barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description.test.cpp

@@ -130,7 +130,7 @@ TEST(boomerang_ultra_circuit_constructor, test_graph_for_elliptic_add_gate)
     uint32_t x3 = circuit_constructor.add_variable(p3.x);
     uint32_t y3 = circuit_constructor.add_variable(p3.y);
 
-    circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });
+    circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/true });
 
     StaticAnalyzer graph = StaticAnalyzer(circuit_constructor);
     auto connected_components = graph.find_connected_components();
@@ -197,7 +197,7 @@ TEST(boomerang_ultra_circuit_constructor, test_graph_for_elliptic_together)
     uint32_t x3 = circuit_constructor.add_variable(p3.x);
     uint32_t y3 = circuit_constructor.add_variable(p3.y);
 
-    circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });
+    circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/true });
     affine_element p4(element(p3).dbl());
     uint32_t x4 = circuit_constructor.add_variable(p4.x);
     uint32_t y4 = circuit_constructor.add_variable(p4.y);
@@ -214,7 +214,7 @@ TEST(boomerang_ultra_circuit_constructor, test_graph_for_elliptic_together)
     uint32_t x7 = circuit_constructor.add_variable(p7.x);
     uint32_t y7 = circuit_constructor.add_variable(p7.y);
 
-    circuit_constructor.create_ecc_add_gate({ x5, y5, x6, y6, x7, y7, 1 });
+    circuit_constructor.create_ecc_add_gate({ x5, y5, x6, y6, x7, y7, /*is_addition=*/true });
     affine_element p8(element(p7).dbl());
     uint32_t x8 = circuit_constructor.add_variable(p8.x);
     uint32_t y8 = circuit_constructor.add_variable(p8.y);
@@ -572,7 +572,7 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_ecc_ad
     uint32_t x3 = circuit_constructor.add_variable(p3.x);
     uint32_t y3 = circuit_constructor.add_variable(p3.y);
 
-    circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });
+    circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/true });
 
     StaticAnalyzer graph = StaticAnalyzer(circuit_constructor);
     auto variables_gate_counts = graph.get_variables_gate_counts();

barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_merge_recursive_verifier.test.cpp

@@ -41,26 +41,10 @@ template <class RecursiveBuilder> class BoomerangRecursiveMergeVerifierTest : pu
 
     static void analyze_circuit(RecursiveBuilder& outer_circuit)
     {
-        // AUDITTODO: The 8 under-constrained variables are the _is_infinity boolean flags from the 8
-        // commitments created via goblin_element::from_witness (4 t_commitments + 4 T_prev_commitments).
-        // Each boolean is only constrained by a single bool gate (x * (x - 1) = 0) and is not
-        // connected to the point coordinates. This may be a security issue if the infinity flag is not
-        // properly bound to the coordinates via Fiat-Shamir - a malicious prover could potentially
-        // set the flag independently of the actual point value.
-        constexpr size_t EXPECTED_UNCONSTRAINED_INFINITY_FLAGS = 4;
-
-        if constexpr (IsMegaBuilder<RecursiveBuilder>) {
-            MegaStaticAnalyzer tool = MegaStaticAnalyzer(outer_circuit);
-            auto result = tool.analyze_circuit();
-            EXPECT_EQ(result.first.size(), 1);
-            EXPECT_EQ(result.second.size(), EXPECTED_UNCONSTRAINED_INFINITY_FLAGS);
-        }
-        if constexpr (IsUltraBuilder<RecursiveBuilder>) {
-            StaticAnalyzer tool = StaticAnalyzer(outer_circuit);
-            auto result = tool.analyze_circuit();
-            EXPECT_EQ(result.first.size(), 1);
-            EXPECT_EQ(result.second.size(), EXPECTED_UNCONSTRAINED_INFINITY_FLAGS);
-        }
+        auto tool = StaticAnalyzer_<bb::fr, RecursiveBuilder>(outer_circuit);
+        auto result = tool.analyze_circuit();
+        EXPECT_EQ(result.first.size(), 1);
+        EXPECT_EQ(result.second.size(), 0);
     }
 
     static void prove_and_verify_merge(const std::shared_ptr<ECCOpQueue>& op_queue,
@@ -96,9 +80,15 @@ template <class RecursiveBuilder> class BoomerangRecursiveMergeVerifierTest : pu
         auto merge_transcript = std::make_shared<StdlibTranscript<RecursiveBuilder>>();
         RecursiveMergeVerifier verifier{ settings, merge_transcript };
         const stdlib::Proof<RecursiveBuilder> stdlib_merge_proof(outer_circuit, merge_proof);
-        [[maybe_unused]] auto [pairing_points, merged_commitments, reduction_succeeded] =
+        auto [pairing_points, merged_commitments, reduction_succeeded] =
             verifier.reduce_to_pairing_check(stdlib_merge_proof, recursive_merge_commitments);
 
+        // The pairing points are public outputs from the recursive verifier that will be verified externally via a
+        // pairing check. Their output coordinate limbs (from goblin batch_mul's queue_ecc_eq) may only appear in a
+        // single ECC op gate. Calling fix_witness() adds explicit constraints on these values so the StaticAnalyzer
+        // does not flag them as under-constrained.
+        pairing_points.fix_witness();
+
         // Check for a failure flag in the recursive verifier circuit
         EXPECT_FALSE(outer_circuit.failed());
         if (run_analyzer) {

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder_elliptic.test.cpp

@@ -65,7 +65,7 @@ TEST_F(UltraCircuitBuilderElliptic, Addition)
     auto points = create_add_points(1, 2, true);
 
     auto [x1, y1, x2, y2, x3, y3] = add_add_gate_variables(builder, points);
-    builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });
+    builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/true });
 
     EXPECT_TRUE(CircuitChecker::check(builder));
 }
@@ -78,7 +78,7 @@ TEST_F(UltraCircuitBuilderElliptic, AdditionFailure)
         auto points = create_add_points(1, 2, true);
         modify_points(points);
         auto [x1, y1, x2, y2, x3, y3] = add_add_gate_variables(builder, points);
-        builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });
+        builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/true });
         EXPECT_FALSE(CircuitChecker::check(builder));
     };
 
@@ -96,7 +96,7 @@ TEST_F(UltraCircuitBuilderElliptic, Subtraction)
     UltraCircuitBuilder builder;
     auto points = create_add_points(1, 2, false); // false = subtraction
     auto [x1, y1, x2, y2, x3, y3] = add_add_gate_variables(builder, points);
-    builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, -1 });
+    builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/false });
     EXPECT_TRUE(CircuitChecker::check(builder));
 }
 
@@ -108,7 +108,7 @@ TEST_F(UltraCircuitBuilderElliptic, SubtractionFailure)
         auto points = create_add_points(1, 2, /*is_addition=*/false);
         modify_points(points);
         auto [x1, y1, x2, y2, x3, y3] = add_add_gate_variables(builder, points);
-        builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*sign_coefficient=*/-1 });
+        builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, /*is_addition=*/false });
         EXPECT_FALSE(CircuitChecker::check(builder));
     };
 
@@ -165,8 +165,8 @@ TEST_F(UltraCircuitBuilderElliptic, MultipleOperationsUnchained)
     auto [sub_x1, sub_y1, sub_x2, sub_y2, sub_x3, sub_y3] = add_add_gate_variables(builder, sub_points);
     auto [dbl_x1, dbl_y1, dbl_x3, dbl_y3] = add_dbl_gate_variables(builder, dbl_points);
 
-    builder.create_ecc_add_gate({ add_x1, add_y1, add_x2, add_y2, add_x3, add_y3, /*sign_coefficient=*/1 });
-    builder.create_ecc_add_gate({ sub_x1, sub_y1, sub_x2, sub_y2, sub_x3, sub_y3, /*sign_coefficient=*/-1 });
+    builder.create_ecc_add_gate({ add_x1, add_y1, add_x2, add_y2, add_x3, add_y3, /*is_addition=*/true });
+    builder.create_ecc_add_gate({ sub_x1, sub_y1, sub_x2, sub_y2, sub_x3, sub_y3, /*is_addition=*/false });
     builder.create_ecc_dbl_gate({ dbl_x1, dbl_y1, dbl_x3, dbl_y3 });
 
     EXPECT_EQ(builder.blocks.elliptic.size(), 6UL); // 3 unchained operations, 2 gates each
@@ -194,8 +194,8 @@ TEST_F(UltraCircuitBuilderElliptic, ChainedOperations)
     uint32_t x_result = builder.add_variable(result.x);
     uint32_t y_result = builder.add_variable(result.y);
 
-    builder.create_ecc_add_gate({ x1, y1, x2, y2, x_temp, y_temp, /*sign_coefficient=*/1 });
-    builder.create_ecc_add_gate({ x_temp, y_temp, x3, y3, x_result, y_result, /*sign_coefficient=*/1 });
+    builder.create_ecc_add_gate({ x1, y1, x2, y2, x_temp, y_temp, /*is_addition=*/true });
+    builder.create_ecc_add_gate({ x_temp, y_temp, x3, y3, x_result, y_result, /*is_addition=*/true });
 
     EXPECT_EQ(builder.blocks.elliptic.size(), 3UL); // 2 chained operations = 2 + (2 - 1) gates
     EXPECT_TRUE(CircuitChecker::check(builder));
@@ -230,9 +230,9 @@ TEST_F(UltraCircuitBuilderElliptic, ChainedOperationsWithDouble)
     uint32_t x_result = builder.add_variable(result.x);
     uint32_t y_result = builder.add_variable(result.y);
 
-    builder.create_ecc_add_gate({ x1, y1, x2, y2, x_temp1, y_temp1, /*sign_coefficient=*/1 });
+    builder.create_ecc_add_gate({ x1, y1, x2, y2, x_temp1, y_temp1, /*is_addition=*/true });
     builder.create_ecc_dbl_gate({ x_temp1, y_temp1, x_temp2, y_temp2 });
-    builder.create_ecc_add_gate({ x_temp2, y_temp2, x3, y3, x_result, y_result, /*sign_coefficient=*/1 });
+    builder.create_ecc_add_gate({ x_temp2, y_temp2, x3, y3, x_result, y_result, /*is_addition=*/true });
 
     EXPECT_EQ(builder.blocks.elliptic.size(), 4UL); // 3 chained operations, 2 + (2 - 1) + (2 - 1) gates
     EXPECT_TRUE(CircuitChecker::check(builder));
@@ -268,9 +268,9 @@ TEST_F(UltraCircuitBuilderElliptic, ChainedOperationsDoubleFailure)
     uint32_t x_result = builder.add_variable(result.x);
     uint32_t y_result = builder.add_variable(result.y);
 
-    builder.create_ecc_add_gate({ x1, y1, x2, y2, x_temp1, y_temp1, /*sign_coefficient=*/1 });
+    builder.create_ecc_add_gate({ x1, y1, x2, y2, x_temp1, y_temp1, /*is_addition=*/true });
     builder.create_ecc_dbl_gate({ x_temp1, y_temp1, x_temp2, y_temp2 });
-    builder.create_ecc_add_gate({ x_temp2, y_temp2, x3, y3, x_result, y_result, /*sign_coefficient=*/1 });
+    builder.create_ecc_add_gate({ x_temp2, y_temp2, x3, y3, x_result, y_result, /*is_addition=*/true });
 
     EXPECT_EQ(builder.blocks.elliptic.size(), 4UL); // 3 chained operations, 2 + (2 - 1) + (2 - 1) gates
     // Should fail because the middle operation (doubling) has an invalid result

barretenberg/cpp/src/barretenberg/commitment_schemes/ipa/ipa.hpp

@@ -529,6 +529,14 @@ template <typename Curve_, size_t log_poly_length = CONST_ECCVM_LOG_N> class IPA
         // Receive a_zero from the prover
         const auto a_zero = transcript->template receive_from_prover<Fr>("IPA:a_0");
 
+        // OriginTag false positive: G_zero and a_zero are fully determined once all round challenges are fixed - the
+        // prover must send the correct values or the final relation check fails.
+        if constexpr (Curve::is_stdlib_type) {
+            const auto last_round_tag = round_challenges.back().get_origin_tag();
+            G_zero.set_origin_tag(last_round_tag);
+            const_cast<Fr&>(a_zero).set_origin_tag(last_round_tag);
+        }
+
         // Step 7.
         // Compute R = C' + ∑_{j ∈ [k]} u_j^{-1}L_j + ∑_{j ∈ [k]} u_jR_j - G₀ * a₀ - (f(\beta) - a₀ * b₀) ⋅ U
         // If everything is correct, then R == -C, as C':= C + f(\beta) ⋅ U

barretenberg/cpp/src/barretenberg/commitment_schemes/kzg/kzg.hpp

@@ -136,6 +136,13 @@ template <typename Curve_> class KZG {
         // This challenge is used to compute offset generators in the batch_mul call below
         const Fr masking_challenge = transcript->template get_challenge<Fr>("KZG:masking_challenge");
 
+        // OriginTag false positive: The quotient commitment is PCS-bound - the prover cannot
+        // choose it freely because it must satisfy the batched opening equation.
+        if constexpr (Curve::is_stdlib_type) {
+            const auto challenge_tag = masking_challenge.get_origin_tag();
+            quotient_commitment.set_origin_tag(challenge_tag);
+        }
+
         // The pairing check can be expressed as
         // e(C + [W]₁ ⋅ z, [1]₂) * e(−[W]₁, [X]₂) = 1, where C = ∑ commitmentsᵢ ⋅ scalarsᵢ.
         GroupElement P_0;