added computation of pairing points and pairing check in test

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

@@ -33,44 +33,14 @@ class UltraRecursive {
     using CircuitBuilder = UltraCircuitBuilder;
     using Curve = plonk::stdlib::bn254<CircuitBuilder>;
     using FF = Curve::ScalarField;
-    using G1 = Curve::Group;
-    // WORKTODO: is there a notion of element/affine_element here?
-    using GroupElement = G1;
-    using Commitment = G1;
-    using CommitmentHandle = G1;
+    using GroupElement = Curve::Element;
+    using Commitment = Curve::Element;
+    using CommitmentHandle = Curve::Element;
 
-    // WORKTODO: these.
+    // WORKTODO: these. do we need them?
     using CommitmentKey = pcs::CommitmentKey<Curve>;
     using VerifierCommitmentKey = pcs::VerifierCommitmentKey<Curve>;
-
-    // using CircuitBuilder = UltraCircuitBuilder;
-    // using PCSParams = pcs::kzg::Params;
-    // using PCS = pcs::kzg::KZG<PCSParams>;
-    // using Curve = PCSParams::Curve;
-    // using GroupElement = Curve::Element;
-    // using Commitment = Curve::AffineElement;
-    // using CommitmentHandle = Curve::AffineElement;
-    // using FF = Curve::ScalarField;
-    // using Polynomial = barretenberg::Polynomial<FF>;
-    // using PolynomialHandle = std::span<FF>;
-
-    // WORKTODO: I've deleted Polynomial and ProvingKey etc. but if this causes problems for Sumcheck/Gemini etc that
-    // need to be instantiated with prover functionality, it's an option to still define these things with native types
-    // even tho the main FF type is fr_ct. This might help avoid needing to split things out into separate
-    // prover/verifier classes e.g. SumcheckProver/SumcheckVerifier. The benefits of the latter should be considered
-    // though; is it a better abstraction to have different classes for these things just like we have a Prover and
-    // Verifier for the proof system itself? We dont for example have UltraProvingSystem::do_prover_stuff and
-    // UltraProvingSystem::do_verifier_stuff. That IS what we do for Sumcheck/Gemini/Shplonk. Could be worth doing a
-    // spike on just this concept. If this were the case, then instead of instantiating things like:
-    // using Gemini = pcs::gemini::MultilinearReductionScheme<PCSParams>;
-    // we would have
-    // Gemini = pcs::gemini::MultilinearReductionSchemeVerifier<StdlibParams>;
-    // and the class would not try to define things like Polynomial<fr_ct> which might cause problems.
-
-    // Question: How do we do the equivalent thing in Plonk, i.e. when we instantiate widgets with stdlib
-    // types, is there no issue with defining polynomials of fr_cts etc.?
-    // Note: may be useful at some point to just keep these as native to get PCS working in the recursive verfier (kind
-    // of like how the transcript is just doing native hashing).
+
     using PCS = pcs::kzg::KZG<Curve>;
 
     static constexpr size_t NUM_WIRES = CircuitBuilder::NUM_WIRES;
@@ -85,10 +55,6 @@ class UltraRecursive {
     static constexpr size_t NUM_WITNESS_ENTITIES = 11;
 
     // define the tuple of Relations that comprise the Sumcheck relation
-    // WORKTODO: Ideally this "just works", i.e. we can instantiate Relations with FF = fr_ct. This should be made to
-    // work even if it does not right now. That was certainly the goal from the getgo. Just need to define corresponding
-    // accumulator types. Actually, I dont see why the existing ValueAccumulatorTypes shouldnt work with fr_ct instead
-    // of fr. That would be sweet.
     using Relations = std::tuple<sumcheck::UltraArithmeticRelation<FF>,
                                  sumcheck::UltraPermutationRelation<FF>,
                                  sumcheck::LookupRelation<FF>,

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

@@ -1,10 +1,10 @@
 #pragma once
 
 #include "../claim.hpp"
-#include "barretenberg/honk/transcript/transcript.hpp"
-#include "barretenberg/polynomials/polynomial.hpp"
 #include "barretenberg/honk/pcs/commitment_key.hpp"
 #include "barretenberg/honk/pcs/verification_key.hpp"
+#include "barretenberg/honk/transcript/transcript.hpp"
+#include "barretenberg/polynomials/polynomial.hpp"
 
 #include <memory>
 #include <utility>
@@ -46,7 +46,6 @@ template <typename Curve> class KZG {
 
     /**
      * @brief Computes the KZG verification for an opening claim of a single polynomial commitment
-     * This reduction is non-interactive and always succeeds.
      *
      * @param vk is the verification key which has a pairing check function
      * @param claim OpeningClaim ({r, v}, C)
@@ -65,5 +64,39 @@ template <typename Curve> class KZG {
 
         return vk->pairing_check(lhs, rhs);
     };
+
+    /**
+     * @brief Computes the input points for the pairing check needed to verify a KZG opening claim of a single
+     * polynomial commitment. This reduction is non-interactive and always succeeds.
+     * @details This is used in the recursive setting where we want to "aggregate" proofs, not verify them.
+     *
+     * @param claim OpeningClaim ({r, v}, C)
+     * @return  {P₀, P₁} where
+     *      - P₀ = C − v⋅[1]₁ + r⋅[x]₁
+     *      - P₁ = [Q(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]₁.
+        // 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;
+            }
+        } else {
+            lhs -= GroupElement::one() * claim.opening_pair.evaluation;
+        }
+
+        auto rhs = -quotient_commitment;
+
+        return { lhs, rhs };
+    };
 };
 } // namespace proof_system::honk::pcs::kzg

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

@@ -253,8 +253,16 @@ template <typename Curve> class ShplonkVerifier_ {
             G_commitment += vk->srs->get_first_g1() * G_commitment_constant;
         }
 
+        Fr zero_evaluation;
+        if constexpr (Curve::is_stdlib_type) {
+            auto ctx = transcript.builder;
+            zero_evaluation = Fr::from_witness(ctx, 0);
+        } else {
+            zero_evaluation = Fr(0);
+        }
+
         // Return opening pair (z, 0) and commitment [G]
-        return { { z_challenge, Fr(0) }, G_commitment };
+        return { { z_challenge, zero_evaluation }, G_commitment };
     };
 };
 } // namespace proof_system::honk::pcs::shplonk

circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp

@@ -37,15 +37,15 @@ UltraRecursiveVerifier_<Flavor>& UltraRecursiveVerifier_<Flavor>::operator=(Ultr
  * @brief This function verifies an Ultra Honk proof for given program settings.
  *
  */
-template <typename Flavor> bool UltraRecursiveVerifier_<Flavor>::verify_proof(const plonk::proof& proof)
+template <typename Flavor> std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::verify_proof(const plonk::proof& proof)
 {
     using FF = typename Flavor::FF;
     using GroupElement = typename Flavor::GroupElement;
     using Commitment = typename Flavor::Commitment;
     using Curve = typename Flavor::Curve;
     using Gemini = ::proof_system::honk::pcs::gemini::GeminiVerifier_<Curve>;
     using Shplonk = ::proof_system::honk::pcs::shplonk::ShplonkVerifier_<Curve>;
-    // using PCS = typename Flavor::PCS;
+    using PCS = typename Flavor::PCS; // note: This can only be KZG
     using VerifierCommitments = typename Flavor::VerifierCommitments;
     using CommitmentLabels = typename Flavor::CommitmentLabels;
 
@@ -68,12 +68,9 @@ template <typename Flavor> bool UltraRecursiveVerifier_<Flavor>::verify_proof(co
     auto public_input_size_native = static_cast<size_t>(public_input_size.get_value());
     auto pub_inputs_offset_native = static_cast<size_t>(pub_inputs_offset.get_value());
 
-    if (circuit_size_native != key->circuit_size) {
-        return false;
-    }
-    if (public_input_size_native != key->num_public_inputs) {
-        return false;
-    }
+    // For debugging purposes only
+    ASSERT(circuit_size_native == key->circuit_size);
+    ASSERT(public_input_size_native == key->num_public_inputs);
 
     std::vector<FF> public_inputs;
     for (size_t i = 0; i < public_input_size_native; ++i) {
@@ -131,13 +128,8 @@ template <typename Flavor> bool UltraRecursiveVerifier_<Flavor>::verify_proof(co
     info("Sumcheck: num gates = ", builder->get_num_gates() - prev_num_gates);
     prev_num_gates = builder->get_num_gates();
 
-    // // Note(luke): Temporary. Done only to complete manifest through sumcheck. Delete once we proceed to Gemini.
-    // [[maybe_unused]] FF rho = transcript.get_challenge("rho");
-
     // If Sumcheck does not return an output, sumcheck verification has failed
-    if (!sumcheck_output.has_value()) {
-        return false;
-    }
+    ASSERT(sumcheck_output.has_value()); // TODO(luke): Appropriate way to handle this in circuit?
 
     // Extract multivariate opening point u = (u_0, ..., u_{d-1}) and purported multivariate evaluations at u
     auto [multivariate_challenge, purported_evaluations] = *sumcheck_output;
@@ -169,16 +161,19 @@ template <typename Flavor> bool UltraRecursiveVerifier_<Flavor>::verify_proof(co
     for (size_t i = 0; i < NUM_TO_BE_SHIFTED; ++i) {
         scalars_to_be_shifted.emplace_back(rhos[idx++]);
     }
-    // WORKTODO: The powers_of_rho fctn does not set the context of rhos[0] = FF(1) so we do it explicitly here. Can we
+    // TODO(luke): The powers_of_rho fctn does not set the context of rhos[0] = FF(1) so we do it explicitly here. Can we
     // do something silly like set it to rho.pow(0) in the fctn to make it work both native and stdlib?
     scalars_unshifted[0] = FF::from_witness(builder, 1);
 
     // Batch the commitments to the unshifted and to-be-shifted polynomials using powers of rho
     auto batched_commitment_unshifted = GroupElement::batch_mul(commitments.get_unshifted(), scalars_unshifted);
+
     info("Batch mul (unshifted): num gates = ", builder->get_num_gates() - prev_num_gates);
     prev_num_gates = builder->get_num_gates();
+
     auto batched_commitment_to_be_shifted =
         GroupElement::batch_mul(commitments.get_to_be_shifted(), scalars_to_be_shifted);
+
     info("Batch mul (to-be-shited): num gates = ", builder->get_num_gates() - prev_num_gates);
     prev_num_gates = builder->get_num_gates();
 
@@ -193,21 +188,19 @@ template <typename Flavor> bool UltraRecursiveVerifier_<Flavor>::verify_proof(co
 
     info("Gemini: num gates = ", builder->get_num_gates() - prev_num_gates);
     prev_num_gates = builder->get_num_gates();
-    // // Note(luke): Temporary. Done only to complete manifest through Gemini. Delete once we proceed to Shplonk.
-    // [[maybe_unused]] FF nu = transcript.get_challenge("Shplonk:nu");
 
     // Produce a Shplonk claim: commitment [Q] - [Q_z], evaluation zero (at random challenge z)
     auto shplonk_claim = Shplonk::reduce_verification(pcs_verification_key, gemini_claim, transcript);
-    (void)shplonk_claim;
+
     info("Shplonk: num gates = ", builder->get_num_gates() - prev_num_gates);
     prev_num_gates = builder->get_num_gates();
 
     info("Total: num gates = ", builder->get_num_gates());
-    // DEBUG!
-    return true;
 
-    // // // Verify the Shplonk claim with KZG or IPA
-    // return PCS::verify(pcs_verification_key, shplonk_claim, transcript);
+    // Constuct the inputs to the final KZG pairing check
+    auto pairing_points = PCS::compute_pairing_points(shplonk_claim, transcript);
+
+    return pairing_points;
 }
 
 template class UltraRecursiveVerifier_<proof_system::honk::flavor::UltraRecursive>;

circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.hpp

@@ -14,6 +14,7 @@ template <typename Flavor> class UltraRecursiveVerifier_ {
     using VerificationKey = typename Flavor::VerificationKey;
     using VerifierCommitmentKey = typename Flavor::VerifierCommitmentKey;
     using Builder = typename Flavor::CircuitBuilder;
+    using PairingPoints = std::array<typename Flavor::GroupElement, 2>;
 
   public:
     explicit UltraRecursiveVerifier_(Builder* builder, std::shared_ptr<VerificationKey> verifier_key = nullptr);
@@ -22,7 +23,7 @@ template <typename Flavor> class UltraRecursiveVerifier_ {
     UltraRecursiveVerifier_& operator=(const UltraRecursiveVerifier_& other) = delete;
     UltraRecursiveVerifier_& operator=(UltraRecursiveVerifier_&& other) noexcept;
 
-    bool verify_proof(const plonk::proof& proof);
+    PairingPoints verify_proof(const plonk::proof& proof);
 
     std::shared_ptr<VerificationKey> key;
     std::map<std::string, Commitment> commitments;

circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp

@@ -1,5 +1,5 @@
-#include "barretenberg/honk/proof_system/ultra_verifier.hpp"
 #include "barretenberg/honk/flavor/ultra_recursive.hpp"
+#include "barretenberg/honk/proof_system/ultra_verifier.hpp"
 
 #include "barretenberg/common/test.hpp"
 #include "barretenberg/ecc/curves/bn254/fq12.hpp"
@@ -61,9 +61,10 @@ template <typename OuterComposer> class RecursiveVerifierTest : public testing::
                                     inner_scalar_field_ct(witness_ct(&builder, 0)));
 
         big_a* big_b;
-
-        // WORKTODO: this provides a way to set the circuit size of the proof to be recursively verified. Formalize this a bit
-        const size_t num_gates = 1 << 10;
+
+        // WORKTODO: this provides a way to set the circuit size of the proof to be recursively verified. Formalize this
+        // a bit
+        const size_t num_gates = 1 << 4;
         for (size_t i = 0; i < num_gates; ++i) {
             fr a = fr::random_element();
             uint32_t a_idx = builder.add_variable(a);
@@ -93,21 +94,26 @@ template <typename OuterComposer> class RecursiveVerifierTest : public testing::
 
         // Instantiate the recursive verification key from the native verification key
         auto verification_key = std::make_shared<VerificationKey>(&outer_builder, native_verification_key);
-
+
+        // Perform native verification
+        auto native_verifier = inner_composer.create_verifier(inner_circuit);
+        ;
+        auto native_result = native_verifier.verify_proof(proof_to_recursively_verify);
+
         // Instantiate the recursive verifier and construct the recusive verification circuit
         RecursiveVerifier verifier(&outer_builder, verification_key);
-        auto result = verifier.verify_proof(proof_to_recursively_verify);
-        EXPECT_EQ(result, true);
+        auto pairing_points = verifier.verify_proof(proof_to_recursively_verify);
 
-        // Perform native verification and check that the result matches
-        auto native_verifier = inner_composer.create_verifier(inner_circuit);;
-        auto native_result = native_verifier.verify_proof(proof_to_recursively_verify);
-        EXPECT_EQ(result, native_result);
+        // Extract the pairing points and using the native verification key to perform the pairing. The result should
+        // match that of native verification.
+        auto lhs = pairing_points[0].get_value();
+        auto rhs = pairing_points[1].get_value();
+        auto recursive_result = native_verifier.pcs_verification_key->pairing_check(lhs, rhs);
+        EXPECT_EQ(recursive_result, native_result);
 
         // Confirm that the manifests produced by the recursive and native verifiers agree
         auto recursive_manifest = verifier.transcript.get_manifest();
         auto native_manifest = native_verifier.transcript.get_manifest();
-        // Note: Recursive manifest currently goes only though sumcheck
         // recursive_manifest.print();
         // native_manifest.print();
         for (size_t i = 0; i < recursive_manifest.size(); ++i) {
@@ -146,6 +152,9 @@ template <typename OuterComposer> class RecursiveVerifierTest : public testing::
         // Create a recursive verification circuit for the proof of the inner circuit
         create_outer_circuit(inner_circuit, outer_circuit);
 
+        if (outer_circuit.failed()) {
+            info(outer_circuit.err());
+        }
         EXPECT_EQ(outer_circuit.failed(), false);
         EXPECT_TRUE(outer_circuit.check_circuit());
     }
@@ -181,7 +190,7 @@ template <typename OuterComposer> class RecursiveVerifierTest : public testing::
     }
 };
 
-using OuterCircuitTypes = testing::Types< ::proof_system::honk::UltraComposer>;
+using OuterCircuitTypes = testing::Types<::proof_system::honk::UltraComposer>;
 
 TYPED_TEST_SUITE(RecursiveVerifierTest, OuterCircuitTypes);