chore: Recursion todos (#2516)

Handle some TODOs including:

- (Issue
#[726](AztecProtocol/barretenberg#726))
Reconfigure Sumcheck Verifier to always return a `SumcheckOutput` rather
than a `std::optional<SumcheckOutput>`. This was fine for native
verification but doesn't work in the recursive setting (and the same
SumcheckVerifier code is shared between the two). For native
verification / debugging purposes, the verifier now returns a `verified`
flag as part of `SumcheckOutput`. In the recursive setting, the verifier
does not "abort early" if sumcheck fails, the proof will simply fail to
verify.
- Add failure tests for the Honk recursive verifiers. (There was no
issue associated with this).
- There were several instances of `TODO(luke)` (without a corresponding
issue) that were outdated, easily fixed, or more appropriate as
`Note:`'s

barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_prover.cpp

@@ -261,7 +261,7 @@ template <ECCVMFlavor Flavor> void ECCVMProver_<Flavor>::execute_univariatizatio
 
     // Compute d-1 polynomials Fold^(i), i = 1, ..., d-1.
     gemini_polynomials = Gemini::compute_gemini_polynomials(
-        sumcheck_output.challenge_point, std::move(batched_poly_unshifted), std::move(batched_poly_to_be_shifted));
+        sumcheck_output.challenge, std::move(batched_poly_unshifted), std::move(batched_poly_to_be_shifted));
 
     // Compute and add to trasnscript the commitments [Fold^(i)], i = 1, ..., d-1
     for (size_t l = 0; l < key->log_circuit_size - 1; ++l) {
@@ -279,7 +279,7 @@ template <ECCVMFlavor Flavor> void ECCVMProver_<Flavor>::execute_pcs_evaluation_
 {
     const FF r_challenge = transcript.get_challenge("Gemini:r");
     gemini_output = Gemini::compute_fold_polynomial_evaluations(
-        sumcheck_output.challenge_point, std::move(gemini_polynomials), r_challenge);
+        sumcheck_output.challenge, std::move(gemini_polynomials), r_challenge);
 
     for (size_t l = 0; l < key->log_circuit_size; ++l) {
         std::string label = "Gemini:a_" + std::to_string(l);

barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp

@@ -186,15 +186,14 @@ template <typename Flavor> bool ECCVMVerifier_<Flavor>::verify_proof(const plonk
     // Execute Sumcheck Verifier
     auto sumcheck = SumcheckVerifier<Flavor>(circuit_size);
 
-    std::optional sumcheck_output = sumcheck.verify(relation_parameters, transcript);
+    auto [multivariate_challenge, purported_evaluations, sumcheck_verified] =
+        sumcheck.verify(relation_parameters, transcript);
 
-    // If Sumcheck does not return an output, sumcheck verification has failed
-    if (!sumcheck_output.has_value()) {
+    // If Sumcheck did not verify, return false
+    if (sumcheck_verified.has_value() && !sumcheck_verified.value()) {
         return false;
     }
 
-    auto [multivariate_challenge, purported_evaluations] = *sumcheck_output;
-
     // Execute Gemini/Shplonk verification:
 
     // Construct inputs for Gemini verifier:
@@ -254,8 +253,10 @@ template <typename Flavor> bool ECCVMVerifier_<Flavor>::verify_proof(const plonk
     // 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);
 
-    // // Verify the Shplonk claim with KZG or IPA
-    return PCS::verify(pcs_verification_key, shplonk_claim, transcript);
+    // Verify the Shplonk claim with KZG or IPA
+    auto verified = PCS::verify(pcs_verification_key, shplonk_claim, transcript);
+
+    return sumcheck_verified.value() && verified;
 }
 
 template class ECCVMVerifier_<honk::flavor::ECCVM>;

barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_prover.cpp

@@ -139,7 +139,7 @@ template <UltraFlavor Flavor> void UltraProver_<Flavor>::execute_univariatizatio
 
     // Compute d-1 polynomials Fold^(i), i = 1, ..., d-1.
     gemini_polynomials = Gemini::compute_gemini_polynomials(
-        sumcheck_output.challenge_point, std::move(batched_poly_unshifted), std::move(batched_poly_to_be_shifted));
+        sumcheck_output.challenge, std::move(batched_poly_unshifted), std::move(batched_poly_to_be_shifted));
 
     // Compute and add to trasnscript the commitments [Fold^(i)], i = 1, ..., d-1
     for (size_t l = 0; l < instance->proving_key->log_circuit_size - 1; ++l) {
@@ -157,7 +157,7 @@ template <UltraFlavor Flavor> void UltraProver_<Flavor>::execute_pcs_evaluation_
 {
     const FF r_challenge = transcript.get_challenge("Gemini:r");
     univariate_openings = Gemini::compute_fold_polynomial_evaluations(
-        sumcheck_output.challenge_point, std::move(gemini_polynomials), r_challenge);
+        sumcheck_output.challenge, std::move(gemini_polynomials), r_challenge);
 
     for (size_t l = 0; l < instance->proving_key->log_circuit_size; ++l) {
         std::string label = "Gemini:a_" + std::to_string(l);

barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp

@@ -114,15 +114,14 @@ template <typename Flavor> bool UltraVerifier_<Flavor>::verify_proof(const plonk
     // Execute Sumcheck Verifier
     auto sumcheck = SumcheckVerifier<Flavor>(circuit_size);
 
-    std::optional sumcheck_output = sumcheck.verify(relation_parameters, transcript);
+    auto [multivariate_challenge, purported_evaluations, sumcheck_verified] =
+        sumcheck.verify(relation_parameters, transcript);
 
-    // If Sumcheck does not return an output, sumcheck verification has failed
-    if (!sumcheck_output.has_value()) {
+    // If Sumcheck did not verify, return false
+    if (sumcheck_verified.has_value() && !sumcheck_verified.value()) {
         return false;
     }
 
-    auto [multivariate_challenge, purported_evaluations] = *sumcheck_output;
-
     // Execute Gemini/Shplonk verification:
 
     // Construct inputs for Gemini verifier:
@@ -218,7 +217,9 @@ template <typename Flavor> bool UltraVerifier_<Flavor>::verify_proof(const plonk
     auto shplonk_claim = Shplonk::reduce_verification(pcs_verification_key, univariate_opening_claims, transcript);
 
     // Verify the Shplonk claim with KZG or IPA
-    return PCS::verify(pcs_verification_key, shplonk_claim, transcript);
+    auto verified = PCS::verify(pcs_verification_key, shplonk_claim, transcript);
+
+    return sumcheck_verified.value() && verified;
 }
 
 template class UltraVerifier_<honk::flavor::Ultra>;

barretenberg/cpp/src/barretenberg/honk/proof_system/work_queue.hpp

@@ -27,7 +27,6 @@ template <typename Curve> class work_queue {
     };
 
   private:
-    // TODO(luke): Consider handling all transcript interactions in the prover rather than embedding them in the queue.
     proof_system::honk::ProverTranscript<FF>& transcript;
     std::shared_ptr<CommitmentKey> commitment_key;
     std::vector<work_item> work_item_queue;

barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.hpp

@@ -172,8 +172,7 @@ template <typename Flavor> class SumcheckVerifier {
      * @param relation_parameters
      * @param transcript
      */
-    std::optional<SumcheckOutput<Flavor>> verify(const proof_system::RelationParameters<FF>& relation_parameters,
-                                                 auto& transcript)
+    SumcheckOutput<Flavor> verify(const proof_system::RelationParameters<FF>& relation_parameters, auto& transcript)
     {
         bool verified(true);
 
@@ -204,11 +203,6 @@ template <typename Flavor> class SumcheckVerifier {
 
             round.compute_next_target_sum(round_univariate, round_challenge);
             pow_univariate.partially_evaluate(round_challenge);
-
-            // TODO(#726): Properly handle this in the recursive setting.
-            if (!verified) {
-                return std::nullopt;
-            }
         }
 
         // Final round
@@ -225,11 +219,8 @@ template <typename Flavor> class SumcheckVerifier {
             checked = (full_honk_relation_purported_value == round.target_total_sum);
         }
         verified = verified && checked;
-        if (!verified) {
-            return std::nullopt;
-        }
 
-        return SumcheckOutput<Flavor>{ multivariate_challenge, purported_evaluations };
+        return SumcheckOutput<Flavor>{ multivariate_challenge, purported_evaluations, verified };
     };
 };
 } // namespace proof_system::honk::sumcheck

barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp

@@ -107,11 +107,11 @@ TEST_F(SumcheckTests, PolynomialNormalization)
 
     auto sumcheck = SumcheckProver<Flavor>(multivariate_n, transcript);
 
-    auto [multivariate_challenge, evaluations] = sumcheck.prove(full_polynomials, {});
+    auto output = sumcheck.prove(full_polynomials, {});
 
-    FF u_0 = multivariate_challenge[0];
-    FF u_1 = multivariate_challenge[1];
-    FF u_2 = multivariate_challenge[2];
+    FF u_0 = output.challenge[0];
+    FF u_1 = output.challenge[1];
+    FF u_2 = output.challenge[2];
 
     /* sumcheck.prove() terminates with sumcheck.multivariates.folded_polynoimals as an array such that
      * sumcheck.multivariates.folded_polynoimals[i][0] is the evaluatioin of the i'th multivariate at the vector of
@@ -162,9 +162,9 @@ TEST_F(SumcheckTests, Prover)
 
     auto sumcheck = SumcheckProver<Flavor>(multivariate_n, transcript);
 
-    auto [multivariate_challenge, evaluations] = sumcheck.prove(full_polynomials, {});
-    FF u_0 = multivariate_challenge[0];
-    FF u_1 = multivariate_challenge[1];
+    auto output = sumcheck.prove(full_polynomials, {});
+    FF u_0 = output.challenge[0];
+    FF u_1 = output.challenge[1];
     std::vector<FF> expected_values;
     for (auto& polynomial : full_polynomials) {
         // using knowledge of inputs here to derive the evaluation
@@ -176,7 +176,7 @@ TEST_F(SumcheckTests, Prover)
     }
 
     for (size_t poly_idx = 0; poly_idx < NUM_POLYNOMIALS; poly_idx++) {
-        EXPECT_EQ(evaluations[poly_idx], expected_values[poly_idx]);
+        EXPECT_EQ(output.claimed_evaluations[poly_idx], expected_values[poly_idx]);
     }
 }
 
@@ -242,9 +242,11 @@ TEST_F(SumcheckTests, ProverAndVerifierSimple)
 
         auto sumcheck_verifier = SumcheckVerifier<Flavor>(multivariate_n);
 
-        std::optional verifier_output = sumcheck_verifier.verify(relation_parameters, verifier_transcript);
+        auto verifier_output = sumcheck_verifier.verify(relation_parameters, verifier_transcript);
 
-        EXPECT_EQ(verifier_output.has_value(), expect_verified);
+        auto verified = verifier_output.verified.value();
+
+        EXPECT_EQ(verified, expect_verified);
     };
 
     run_test(/* expect_verified=*/true);
@@ -395,9 +397,11 @@ TEST_F(SumcheckTests, RealCircuitUltra)
 
     auto sumcheck_verifier = SumcheckVerifier<Flavor>(circuit_size);
 
-    std::optional verifier_output = sumcheck_verifier.verify(instance->relation_parameters, verifier_transcript);
+    auto verifier_output = sumcheck_verifier.verify(instance->relation_parameters, verifier_transcript);
+
+    auto verified = verifier_output.verified.value();
 
-    ASSERT_TRUE(verifier_output.has_value());
+    ASSERT_TRUE(verified);
 }
 
 } // namespace test_sumcheck_round

barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <array>
+#include <optional>
 #include <vector>
 namespace proof_system::honk::sumcheck {
 
@@ -12,34 +13,10 @@ template <typename Flavor> struct SumcheckOutput {
     using FF = typename Flavor::FF;
     using ClaimedEvaluations = typename Flavor::ClaimedEvaluations;
     // u = (u_0, ..., u_{d-1})
-    std::vector<FF> challenge_point;
+    std::vector<FF> challenge;
     // Evaluations in `u` of the polynomials used in Sumcheck
-    ClaimedEvaluations purported_evaluations;
-
-    SumcheckOutput()
-        : purported_evaluations(std::array<FF, Flavor::NUM_ALL_ENTITIES>()){};
-
-    SumcheckOutput(const std::vector<FF>& _challenge_point, const ClaimedEvaluations& _purported_evaluations)
-        : challenge_point(_challenge_point)
-        , purported_evaluations(_purported_evaluations){};
-
-    SumcheckOutput& operator=(SumcheckOutput&& other)
-    {
-        challenge_point = other.challenge_point;
-        purported_evaluations = other.purported_evaluations;
-        return *this;
-    };
-
-    SumcheckOutput(const SumcheckOutput& other)
-        : challenge_point(other.challenge_point)
-        , purported_evaluations(other.purported_evaluations){};
-
-    bool operator==(const SumcheckOutput& other) const
-    {
-        bool result{ false };
-        result = challenge_point == other.challenge_point;
-        result = purported_evaluations._data == other.purported_evaluations._data;
-        return result;
-    };
+    ClaimedEvaluations claimed_evaluations;
+    // Whether or not the claimed multilinear evaluations and final sumcheck evaluation have been confirmed
+    std::optional<bool> verified = false; // optional b/c this struct is shared by the Prover/Verifier
 };
 } // namespace proof_system::honk::sumcheck

barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_round.hpp

@@ -424,8 +424,7 @@ template <typename Flavor> class SumcheckVerifierRound {
         // with a simulated builder.
         bool sumcheck_round_failed(false);
         if constexpr (IsRecursiveFlavor<Flavor>) {
-            // TODO(#726): Need to constrain this equality and update the native optional return value mechanism for the
-            // recursive setting.
+            target_total_sum.assert_equal(total_sum);
             sumcheck_round_failed = (target_total_sum != total_sum).get_value();
         } else {
             sumcheck_round_failed = (target_total_sum != total_sum);

barretenberg/cpp/src/barretenberg/plonk/composer/composer_lib.cpp

@@ -30,8 +30,8 @@ void compute_monomial_and_coset_selector_forms(plonk::proving_key* circuit_provi
         barretenberg::polynomial selector_poly_fft(selector_poly, circuit_proving_key->circuit_size * 4 + 4);
         selector_poly_fft.coset_fft(circuit_proving_key->large_domain);
 
-        // TODO(luke): For Standard, the lagrange polynomials can be removed from the store at this point but this
-        // is not the case for Ultra. Implement?
+        // Note: For Standard, the lagrange polynomials could be removed from the store at this point but this
+        // is not the case for Ultra.
         circuit_proving_key->polynomial_store.put(selector_properties[i].name, std::move(selector_poly));
         circuit_proving_key->polynomial_store.put(selector_properties[i].name + "_fft", std::move(selector_poly_fft));
     }

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp

@@ -58,7 +58,6 @@ template <typename FF> void UltraCircuitBuilder_<FF>::finalize_circuit()
 // TODO(#423): This function adds valid (but arbitrary) gates to ensure that the circuit which includes
 // them will not result in any zero-polynomials. It also ensures that the first coefficient of the wire
 // polynomials is zero, which is required for them to be shiftable.
-// TODO(luke): Add ECC op gate to ensure op wires are non-zero?
 template <typename FF> void UltraCircuitBuilder_<FF>::add_gates_to_ensure_all_polys_are_non_zero()
 {
     // First add a gate to simultaneously ensure first entries of all wires is zero and to add a non

barretenberg/cpp/src/barretenberg/proof_system/composer/composer_lib.hpp

@@ -25,7 +25,6 @@ void construct_selector_polynomials(const typename Flavor::CircuitBuilder& circu
     // and (2) construct ecc op gate selector polynomial.
     // Note 1: All other selectors will be automatically and correctly initialized to 0 on this domain.
     // Note 2: If applicable, the ecc op gates are shifted down by 1 to account for a zero row.
-    // TODO(luke): Move this out of this function and directly into compute_proving_key?
     if constexpr (IsGoblinFlavor<Flavor>) {
         const size_t num_ecc_op_gates = circuit_constructor.num_ecc_op_gates;
         gate_offset += num_ecc_op_gates;

barretenberg/cpp/src/barretenberg/proof_system/relations/elliptic_relation.hpp

@@ -33,7 +33,7 @@ template <typename FF_> class EllipticRelationImpl {
                            const FF& scaling_factor){
         // OPTIMIZATION?: Karatsuba in general, at least for some degrees?
         //       See https://hackmd.io/xGLuj6biSsCjzQnYN-pEiA?both
-        // TODO(luke): Formatter doesnt properly handle explicit scoping below so turning off. Whats up?
+        // Note: Formatter turned off since it doesnt properly handle the explicit scoping below.
         // clang-format off
         // Contribution (1)
         {

barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/transcript/transcript.hpp

@@ -12,7 +12,7 @@
 #include "barretenberg/stdlib/primitives/field/field.hpp"
 #include "barretenberg/stdlib/utility/utility.hpp"
 
-// TODO(luke): this namespace will be sensible once stdlib is moved out of the plonk namespace
+// Note: this namespace will be sensible once stdlib is moved out of the plonk namespace
 namespace proof_system::plonk::stdlib::recursion::honk {
 template <typename Builder> class Transcript {
   public:

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

@@ -196,6 +196,37 @@ template <typename BuilderType> class GoblinRecursiveVerifierTest : public testi
             EXPECT_EQ(recursive_manifest[i], native_manifest[i]);
         }
     }
+
+    /**
+     * @brief Construct a verifier circuit for a proof whose data has been tampered with. Expect failure
+     * TODO(bberg #656): For now we get a "bad" proof by arbitrarily tampering with bits in a valid proof. It would be
+     * much nicer to explicitly change meaningful components, e.g. such that one of the multilinear evaluations is
+     * wrong. This is difficult now but should be straightforward if the proof is a struct.
+     */
+    static void test_recursive_verification_fails()
+    {
+        // Create an arbitrary inner circuit
+        auto inner_circuit = create_inner_circuit();
+
+        // Generate a proof over the inner circuit
+        InnerComposer inner_composer;
+        auto instance = inner_composer.create_instance(inner_circuit);
+        auto inner_prover = inner_composer.create_prover(instance);
+        auto inner_proof = inner_prover.construct_proof();
+        const auto native_verification_key = instance->compute_verification_key();
+
+        // Arbitrarily tamper with the proof to be verified
+        inner_proof.proof_data[10] = 25;
+
+        // Create a recursive verification circuit for the proof of the inner circuit
+        OuterBuilder outer_circuit;
+        auto verification_key = std::make_shared<VerificationKey>(&outer_circuit, native_verification_key);
+        RecursiveVerifier verifier(&outer_circuit, verification_key);
+        verifier.verify_proof(inner_proof);
+
+        // We expect the circuit check to fail due to the bad proof
+        EXPECT_FALSE(outer_circuit.check_circuit());
+    }
 };
 
 // Run the recursive verifier tests with conventional Ultra builder and Goblin builder
@@ -218,4 +249,9 @@ HEAVY_TYPED_TEST(GoblinRecursiveVerifierTest, SingleRecursiveVerification)
     TestFixture::test_recursive_verification();
 };
 
+HEAVY_TYPED_TEST(GoblinRecursiveVerifierTest, SingleRecursiveVerificationFailure)
+{
+    TestFixture::test_recursive_verification_fails();
+};
+
 } // namespace proof_system::plonk::stdlib::recursion::honk

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

@@ -98,10 +98,10 @@ std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::ve
     commitments.z_perm = transcript.template receive_from_prover<Commitment>(commitment_labels.z_perm);
     commitments.z_lookup = transcript.template receive_from_prover<Commitment>(commitment_labels.z_lookup);
 
-    // Execute Sumcheck Verifier
+    // Execute Sumcheck Verifier and extract multivariate opening point u = (u_0, ..., u_{d-1}) and purported
+    // multivariate evaluations at u
     auto sumcheck = Sumcheck(key->circuit_size);
-
-    std::optional sumcheck_output = sumcheck.verify(relation_parameters, transcript);
+    auto [multivariate_challenge, purported_evaluations, verified] = sumcheck.verify(relation_parameters, transcript);
 
     info("Sumcheck: num gates = ",
          builder->get_num_gates() - prev_num_gates,
@@ -110,12 +110,6 @@ std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::ve
          ")");
     prev_num_gates = builder->get_num_gates();
 
-    // If Sumcheck does not return an output, sumcheck verification has failed
-    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;
-
     // Compute powers of batching challenge rho
     FF rho = transcript.get_challenge("rho");
     std::vector<FF> rhos = ::proof_system::honk::pcs::gemini::powers_of_rho(rho, Flavor::NUM_ALL_ENTITIES);
@@ -152,7 +146,7 @@ std::array<typename Flavor::GroupElement, 2> UltraRecursiveVerifier_<Flavor>::ve
     }
     // 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);
+    scalars_unshifted[0] = FF(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);