feat: Goblin recursive verifier

circuits/cpp/barretenberg/cpp/src/barretenberg/honk/flavor/ultra_recursive.hpp

@@ -41,7 +41,6 @@ class UltraRecursive {
   public:
     using CircuitBuilder = UltraCircuitBuilder;
     using Curve = plonk::stdlib::bn254<CircuitBuilder>;
-    using PCS = pcs::kzg::KZG<Curve>;
     using GroupElement = Curve::Element;
     using Commitment = Curve::Element;
     using CommitmentHandle = Curve::Element;

circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp

@@ -111,7 +111,7 @@ template <typename Curve> class GeminiProver_ {
                                                                    const Fr& r_challenge);
 }; // namespace proof_system::honk::pcs::gemini
 
-template <typename Curve> class GeminiVerifier_ {
+template <typename Curve, bool goblin_flag = false> class GeminiVerifier_ {
     using Fr = typename Curve::ScalarField;
     using GroupElement = typename Curve::Element;
     using Commitment = typename Curve::AffineElement;
@@ -232,21 +232,31 @@ template <typename Curve> class GeminiVerifier_ {
                                                                                Fr r)
     {
         // C₀ᵣ₊ = [F] + r⁻¹⋅[G]
-        GroupElement C0_r_pos = batched_f;
+        GroupElement C0_r_pos;
         // C₀ᵣ₋ = [F] - r⁻¹⋅[G]
-        GroupElement C0_r_neg = batched_f;
-        Fr r_inv = r.invert();
+        GroupElement C0_r_neg;
+        Fr r_inv = r.invert(); // r⁻¹
 
-        // TODO(luke): reinstate some kind of !batched_g.is_point_at_infinity() check for stdlib types? This is mostly
-        // relevant for Gemini unit tests since in practice batched_g != zero (i.e. we will always have shifted polys).
-        bool batched_g_is_point_at_infinity = false;
-        if constexpr (!Curve::is_stdlib_type) { // Note: required for Gemini tests with no shifts
-            batched_g_is_point_at_infinity = batched_g.is_point_at_infinity();
-        }
-        if (!batched_g_is_point_at_infinity) {
-            batched_g = batched_g * r_inv;
-            C0_r_pos += batched_g;
-            C0_r_neg -= batched_g;
+        // If in a recursive setting, perform a batch mul. Otherwise, accumulate directly.
+        // TODO(#673): The following if-else represents the stldib/native code paths. Once the "native" verifier is
+        // achieved through a builder Simulator, the stdlib codepath should become the only codepath.
+        if constexpr (Curve::is_stdlib_type) {
+            std::vector<GroupElement> commitments = { batched_f, batched_g };
+            auto builder = r.get_context();
+            auto one = Fr(builder, 1);
+            // TODO(#707): these batch muls include the use of 1 as a scalar. This is handled appropriately as a non-mul
+            // (add-accumulate) in the goblin batch_mul but is done inefficiently as a scalar mul in the conventional
+            // emulated batch mul.
+            C0_r_pos = GroupElement::template batch_mul<goblin_flag>(commitments, { one, r_inv });
+            C0_r_neg = GroupElement::template batch_mul<goblin_flag>(commitments, { one, -r_inv });
+        } else {
+            C0_r_pos = batched_f;
+            C0_r_neg = batched_f;
+            if (!batched_g.is_point_at_infinity()) {
+                batched_g = batched_g * r_inv;
+                C0_r_pos += batched_g;
+                C0_r_neg -= batched_g;
+            }
         }
 
         return { C0_r_pos, C0_r_neg };

circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp

@@ -11,7 +11,7 @@
 
 namespace proof_system::honk::pcs::kzg {
 
-template <typename Curve> class KZG {
+template <typename Curve, bool goblin_flag = false> class KZG {
     using CK = CommitmentKey<Curve>;
     using VK = VerifierCommitmentKey<Curve>;
     using Fr = typename Curve::ScalarField;
@@ -72,31 +72,34 @@ template <typename Curve> class KZG {
      *
      * @param claim OpeningClaim ({r, v}, C)
      * @return  {P₀, P₁} where
-     *      - P₀ = C − v⋅[1]₁ + r⋅[x]₁
-     *      - P₁ = [Q(x)]₁
+     *      - P₀ = C − v⋅[1]₁ + r⋅[W(x)]₁
+     *      - P₁ = [W(x)]₁
      */
     static std::array<GroupElement, 2> compute_pairing_points(const OpeningClaim<Curve>& claim,
                                                               auto& verifier_transcript)
     {
         auto quotient_commitment = verifier_transcript.template receive_from_prover<Commitment>("KZG:W");
 
-        auto lhs = claim.commitment + (quotient_commitment * claim.opening_pair.challenge);
-        // Add the evaluation point contribution v⋅[1]₁.
+        GroupElement P_0;
         // Note: In the recursive setting, we only add the contribution if it is not the point at infinity (i.e. if the
         // evaluation is not equal to zero).
-        // TODO(luke): What is the proper way to handle this? Contraints to show scalar (evaluation) is zero?
         if constexpr (Curve::is_stdlib_type) {
-            if (!claim.opening_pair.evaluation.get_value().is_zero()) {
-                auto ctx = verifier_transcript.builder;
-                lhs -= GroupElement::one(ctx) * claim.opening_pair.evaluation;
-            }
+            auto builder = verifier_transcript.builder;
+            auto one = Fr(builder, 1);
+            std::vector<GroupElement> commitments = { claim.commitment, quotient_commitment };
+            std::vector<Fr> scalars = { one, claim.opening_pair.challenge };
+            P_0 = GroupElement::template batch_mul<goblin_flag>(commitments, scalars);
+            // Note: This implementation assumes the evaluation is zero (as is the case for shplonk).
+            ASSERT(claim.opening_pair.evaluation.get_value() == 0);
         } else {
-            lhs -= GroupElement::one() * claim.opening_pair.evaluation;
+            P_0 = claim.commitment;
+            P_0 += quotient_commitment * claim.opening_pair.challenge;
+            P_0 -= GroupElement::one() * claim.opening_pair.evaluation;
         }
 
-        auto rhs = -quotient_commitment;
+        auto P_1 = -quotient_commitment;
 
-        return { lhs, rhs };
+        return { P_0, P_1 };
     };
 };
 } // namespace proof_system::honk::pcs::kzg

circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp

@@ -145,7 +145,7 @@ template <typename Curve> class ShplonkProver_ {
  * @brief Shplonk Verifier
  *
  */
-template <typename Curve> class ShplonkVerifier_ {
+template <typename Curve, bool goblin_flag = false> class ShplonkVerifier_ {
     using Fr = typename Curve::ScalarField;
     using GroupElement = typename Curve::Element;
     using Commitment = typename Curve::AffineElement;
@@ -174,84 +174,101 @@ template <typename Curve> class ShplonkVerifier_ {
 
         const Fr z_challenge = transcript.get_challenge("Shplonk:z");
 
+        // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1]
+        //     = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )]
+        GroupElement G_commitment;
+
         // compute simulated commitment to [G] as a linear combination of
         // [Q], { [fⱼ] }, [1]:
         //  [G] = [Q] - ∑ⱼ (1/zⱼ(r))[Bⱼ]  + ( ∑ⱼ (1/zⱼ(r)) Tⱼ(r) )[1]
         //      = [Q] - ∑ⱼ (1/zⱼ(r))[Bⱼ]  +                    G₀ [1]
-
         // G₀ = ∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ )
         auto G_commitment_constant = Fr(0);
 
-        // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1]
-        //     = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )]
-        GroupElement G_commitment = Q_commitment;
-
-        // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ)
-        std::vector<Fr> vanishing_evals;
-        vanishing_evals.reserve(num_claims);
-        for (const auto& claim : claims) {
-            vanishing_evals.emplace_back(z_challenge - claim.opening_pair.challenge);
-        }
-        // If recursion, invert elements individually, otherwise batch invert. (Inversion is cheap in circuits since we
-        // need only prove the correctness of a known inverse, we do not emulate its computation. Hence no need for
-        // batch inversion).
-        std::vector<Fr> inverse_vanishing_evals;
+        // TODO(#673): The recursive and non-recursive (native) logic is completely separated via the following
+        // conditional. Much of the logic could be shared, but I've chosen to do it this way since soon the "else"
+        // branch should be removed in its entirety, and "native" verification will utilize the recursive code paths
+        // using a builder Simulator.
         if constexpr (Curve::is_stdlib_type) {
-            for (const auto& val : vanishing_evals) {
-                inverse_vanishing_evals.emplace_back(val.invert());
+            auto builder = nu.get_context();
+
+            // Containers for the inputs to the final batch mul
+            std::vector<Commitment> commitments;
+            std::vector<Fr> scalars;
+
+            // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1]
+            //     = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )]
+            commitments.emplace_back(Q_commitment);
+            scalars.emplace_back(Fr(builder, 1)); // Fr(1)
+
+            // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ)
+            std::vector<Fr> inverse_vanishing_evals;
+            inverse_vanishing_evals.reserve(num_claims);
+            for (const auto& claim : claims) {
+                // Note: no need for batch inversion; emulated inverison is cheap. (just show known inverse is valid)
+                inverse_vanishing_evals.emplace_back((z_challenge - claim.opening_pair.challenge).invert());
             }
-        } else {
-            Fr::batch_invert(vanishing_evals);
-            inverse_vanishing_evals = vanishing_evals;
-        }
 
-        auto current_nu = Fr(1);
-        // Note: commitments and scalars vectors used only in recursion setting for batch mul
-        std::vector<Commitment> commitments;
-        std::vector<Fr> scalars;
-        for (size_t j = 0; j < num_claims; ++j) {
-            // (Cⱼ, xⱼ, vⱼ)
-            const auto& [opening_pair, commitment] = claims[j];
+            auto current_nu = Fr(1);
+            // Note: commitments and scalars vectors used only in recursion setting for batch mul
+            for (size_t j = 0; j < num_claims; ++j) {
+                // (Cⱼ, xⱼ, vⱼ)
+                const auto& [opening_pair, commitment] = claims[j];
 
-            Fr scaling_factor = current_nu * inverse_vanishing_evals[j]; // = ρʲ / ( r − xⱼ )
+                Fr scaling_factor = current_nu * inverse_vanishing_evals[j]; // = ρʲ / ( r − xⱼ )
+
+                // G₀ += ρʲ / ( r − xⱼ ) ⋅ vⱼ
+                G_commitment_constant += scaling_factor * opening_pair.evaluation;
 
-            // G₀ += ρʲ / ( r − xⱼ ) ⋅ vⱼ
-            G_commitment_constant += scaling_factor * opening_pair.evaluation;
+                current_nu *= nu;
 
-            // If recursion, store MSM inputs for batch mul, otherwise accumulate directly
-            if constexpr (Curve::is_stdlib_type) {
+                // Store MSM inputs for batch mul
                 commitments.emplace_back(commitment);
-                scalars.emplace_back(scaling_factor);
-            } else {
-                // [G] -= ρʲ / ( r − xⱼ )⋅[fⱼ]
-                G_commitment -= commitment * scaling_factor;
+                scalars.emplace_back(-scaling_factor);
             }
 
-            current_nu *= nu;
-        }
+            commitments.emplace_back(GroupElement::one(builder));
+            scalars.emplace_back(G_commitment_constant);
 
-        // If recursion, do batch mul to compute [G] -= ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ]
-        if constexpr (Curve::is_stdlib_type) {
-            G_commitment -= GroupElement::batch_mul(commitments, scalars);
-        }
+            // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1]
+            G_commitment = GroupElement::template batch_mul<goblin_flag>(commitments, scalars);
 
-        // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1]
-        Fr evaluation_zero;     // 0 \in Fr
-        GroupElement group_one; // [1]
-        if constexpr (Curve::is_stdlib_type) {
-            auto ctx = transcript.builder;
-            evaluation_zero = Fr::from_witness(ctx, 0);
-            group_one = GroupElement::one(ctx);
         } else {
-            //  GroupElement sort_of_one{ x, y };
-            evaluation_zero = Fr(0);
-            group_one = vk->srs->get_first_g1();
-        }
+            // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1]
+            //     = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )]
+            G_commitment = Q_commitment;
+
+            // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ)
+            std::vector<Fr> inverse_vanishing_evals;
+            inverse_vanishing_evals.reserve(num_claims);
+            for (const auto& claim : claims) {
+                inverse_vanishing_evals.emplace_back(z_challenge - claim.opening_pair.challenge);
+            }
+            Fr::batch_invert(inverse_vanishing_evals);
+
+            auto current_nu = Fr(1);
+            // Note: commitments and scalars vectors used only in recursion setting for batch mul
+            for (size_t j = 0; j < num_claims; ++j) {
+                // (Cⱼ, xⱼ, vⱼ)
+                const auto& [opening_pair, commitment] = claims[j];
 
-        G_commitment += group_one * G_commitment_constant;
+                Fr scaling_factor = current_nu * inverse_vanishing_evals[j]; // = ρʲ / ( r − xⱼ )
+
+                // G₀ += ρʲ / ( r − xⱼ ) ⋅ vⱼ
+                G_commitment_constant += scaling_factor * opening_pair.evaluation;
+
+                // [G] -= ρʲ / ( r − xⱼ )⋅[fⱼ]
+                G_commitment -= commitment * scaling_factor;
+
+                current_nu *= nu;
+            }
+
+            // [G] += G₀⋅[1] = [G] + (∑ⱼ ρʲ ⋅ vⱼ / ( r − xⱼ ))⋅[1]
+            G_commitment += vk->srs->get_first_g1() * G_commitment_constant;
+        }
 
         // Return opening pair (z, 0) and commitment [G]
-        return { { z_challenge, evaluation_zero }, G_commitment };
+        return { { z_challenge, Fr(0) }, G_commitment };
     };
 };
 } // namespace proof_system::honk::pcs::shplonk

circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp

@@ -67,8 +67,8 @@ struct ecc_op_tuple {
     uint32_t x_hi;
     uint32_t y_lo;
     uint32_t y_hi;
-    uint32_t z_lo;
-    uint32_t z_hi;
+    uint32_t z_1;
+    uint32_t z_2;
 };
 
 template <typename B, typename FF> inline void read(B& buf, poly_triple_<FF>& constraint)

circuits/cpp/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.test.cpp

@@ -18,6 +18,8 @@ namespace proof_system {
  */
 TEST(UltraCircuitBuilder, GoblinSimple)
 {
+    const size_t CHUNK_SIZE = plonk::NUM_LIMB_BITS_IN_FIELD_SIMULATION * 2;
+
     auto builder = UltraCircuitBuilder();
 
     // Compute a simple point accumulation natively
@@ -31,10 +33,17 @@ TEST(UltraCircuitBuilder, GoblinSimple)
     builder.queue_ecc_mul_accum(P2, z);
 
     // Add equality op gates based on the internal accumulator
-    auto P_result = builder.queue_ecc_eq();
-
-    // Check that value returned from internal accumulator is correct
-    EXPECT_EQ(P_result, P_expected);
+    auto eq_op_tuple = builder.queue_ecc_eq();
+
+    // Check that we can reconstruct the coordinates of P_expected from the data in variables
+    auto P_result_x_lo = uint256_t(builder.variables[eq_op_tuple.x_lo]);
+    auto P_result_x_hi = uint256_t(builder.variables[eq_op_tuple.x_hi]);
+    auto P_result_x = P_result_x_lo + (P_result_x_hi << CHUNK_SIZE);
+    auto P_result_y_lo = uint256_t(builder.variables[eq_op_tuple.y_lo]);
+    auto P_result_y_hi = uint256_t(builder.variables[eq_op_tuple.y_hi]);
+    auto P_result_y = P_result_y_lo + (P_result_y_hi << CHUNK_SIZE);
+    EXPECT_EQ(P_result_x, uint256_t(P_expected.x));
+    EXPECT_EQ(P_result_y, uint256_t(P_expected.y));
 
     // Check that the accumulator in the op queue has been reset to 0
     auto accumulator = builder.op_queue.get_accumulator();
@@ -49,64 +58,23 @@ TEST(UltraCircuitBuilder, GoblinSimple)
     EXPECT_EQ(builder.ecc_op_wire_1[4], EccOpCode::EQUALITY);
 
     // Check that we can reconstruct the coordinates of P1 from the op_wires
-    auto chunk_size = plonk::NUM_LIMB_BITS_IN_FIELD_SIMULATION * 2;
     auto P1_x_lo = uint256_t(builder.variables[builder.ecc_op_wire_2[0]]);
     auto P1_x_hi = uint256_t(builder.variables[builder.ecc_op_wire_3[0]]);
-    auto P1_x = P1_x_lo + (P1_x_hi << chunk_size);
+    auto P1_x = P1_x_lo + (P1_x_hi << CHUNK_SIZE);
     EXPECT_EQ(P1_x, uint256_t(P1.x));
     auto P1_y_lo = uint256_t(builder.variables[builder.ecc_op_wire_4[0]]);
     auto P1_y_hi = uint256_t(builder.variables[builder.ecc_op_wire_2[1]]);
-    auto P1_y = P1_y_lo + (P1_y_hi << chunk_size);
+    auto P1_y = P1_y_lo + (P1_y_hi << CHUNK_SIZE);
     EXPECT_EQ(P1_y, uint256_t(P1.y));
 
     // Check that we can reconstruct the coordinates of P2 from the op_wires
     auto P2_x_lo = uint256_t(builder.variables[builder.ecc_op_wire_2[2]]);
     auto P2_x_hi = uint256_t(builder.variables[builder.ecc_op_wire_3[2]]);
-    auto P2_x = P2_x_lo + (P2_x_hi << chunk_size);
+    auto P2_x = P2_x_lo + (P2_x_hi << CHUNK_SIZE);
     EXPECT_EQ(P2_x, uint256_t(P2.x));
     auto P2_y_lo = uint256_t(builder.variables[builder.ecc_op_wire_4[2]]);
     auto P2_y_hi = uint256_t(builder.variables[builder.ecc_op_wire_2[3]]);
-    auto P2_y = P2_y_lo + (P2_y_hi << chunk_size);
+    auto P2_y = P2_y_lo + (P2_y_hi << CHUNK_SIZE);
     EXPECT_EQ(P2_y, uint256_t(P2.y));
-
-    // Check that we can reconstruct the coordinates of P_result from the op_wires
-    auto P_expected_x_lo = uint256_t(builder.variables[builder.ecc_op_wire_2[4]]);
-    auto P_expected_x_hi = uint256_t(builder.variables[builder.ecc_op_wire_3[4]]);
-    auto P_expected_x = P_expected_x_lo + (P_expected_x_hi << chunk_size);
-    EXPECT_EQ(P_expected_x, uint256_t(P_expected.x));
-    auto P_expected_y_lo = uint256_t(builder.variables[builder.ecc_op_wire_4[4]]);
-    auto P_expected_y_hi = uint256_t(builder.variables[builder.ecc_op_wire_2[5]]);
-    auto P_expected_y = P_expected_y_lo + (P_expected_y_hi << chunk_size);
-    EXPECT_EQ(P_expected_y, uint256_t(P_expected.y));
 }
-
-/**
- * @brief Test correctness of native ecc batch mul performed behind the scenes when adding ecc op gates for a batch mul
- *
- */
-TEST(UltraCircuitBuilder, GoblinBatchMul)
-{
-    using Point = g1::affine_element;
-    using Scalar = fr;
-
-    auto builder = UltraCircuitBuilder();
-    const size_t num_muls = 3;
-
-    // Compute some random points and scalars to batch multiply
-    std::vector<Point> points;
-    std::vector<Scalar> scalars;
-    auto batched_expected = Point::infinity();
-    for (size_t i = 0; i < num_muls; ++i) {
-        points.emplace_back(Point::random_element());
-        scalars.emplace_back(Scalar::random_element());
-        batched_expected = batched_expected + points[i] * scalars[i];
-    }
-
-    // Populate the batch mul operands in the op wires and natively compute the result
-    auto batched_result = builder.batch_mul(points, scalars);
-
-    // Extract current accumulator point from the op queue and check the result
-    EXPECT_EQ(batched_result, batched_expected);
-}
-
 } // namespace proof_system