From 8e0320e5e8e9549fcc26ce4ac6c1307ac665fcbe Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 25 Sep 2023 16:24:41 +0000 Subject: [PATCH 1/7] romoving fixing or editing some old minor todos --- .../cpp/src/barretenberg/honk/proof_system/work_queue.hpp | 1 - .../cpp/src/barretenberg/plonk/composer/composer_lib.cpp | 4 ++-- .../plonk/proof_system/types/polynomial_manifest.hpp | 5 ++--- .../proof_system/circuit_builder/ultra_circuit_builder.cpp | 1 - .../src/barretenberg/proof_system/composer/composer_lib.hpp | 1 - .../proof_system/relations/elliptic_relation.hpp | 2 +- .../stdlib/recursion/honk/transcript/transcript.hpp | 2 +- .../recursion/honk/verifier/ultra_recursive_verifier.cpp | 2 +- barretenberg/cpp/src/barretenberg/stdlib/utility/utility.hpp | 1 - barretenberg/cpp/src/barretenberg/transcript/manifest.hpp | 1 - .../cpp/src/barretenberg/transcript/transcript_wrappers.hpp | 3 --- 11 files changed, 7 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/work_queue.hpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/work_queue.hpp index bd052e5eccf..a95e40e45aa 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/work_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/work_queue.hpp @@ -27,7 +27,6 @@ template 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& transcript; std::shared_ptr commitment_key; std::vector work_item_queue; diff --git a/barretenberg/cpp/src/barretenberg/plonk/composer/composer_lib.cpp b/barretenberg/cpp/src/barretenberg/plonk/composer/composer_lib.cpp index b3a13a11fbb..f12d1f66379 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/composer/composer_lib.cpp +++ b/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)); } diff --git a/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp b/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp index d4148be017b..da671799ebb 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp +++ b/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp @@ -1,5 +1,6 @@ #pragma once +#include "barretenberg/common/throw_or_abort.hpp" #include "barretenberg/proof_system/types/circuit_type.hpp" #include #include @@ -141,7 +142,6 @@ static constexpr PolynomialDescriptor ultra_polynomial_manifest[ULTRA_MANIFEST_S // Simple class allowing for access to a polynomial manifest based on composer type class PolynomialManifest { - // TODO(luke): make this object iterable, i.e. compatible with range-based for loop private: std::vector manifest; @@ -164,8 +164,7 @@ class PolynomialManifest { break; }; default: { - // TODO(luke): reinstate this. Was getting "use of undeclared identifier" error for 'throw_or_abort'. - // throw_or_abort("Received invalid composer type"); + throw_or_abort("Received invalid composer type"); } }; } diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index d88a3c01049..b3289e3ea59 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -58,7 +58,6 @@ template void UltraCircuitBuilder_::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 void UltraCircuitBuilder_::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 diff --git a/barretenberg/cpp/src/barretenberg/proof_system/composer/composer_lib.hpp b/barretenberg/cpp/src/barretenberg/proof_system/composer/composer_lib.hpp index b4934e7186f..df2c83ce6f4 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/composer/composer_lib.hpp +++ b/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) { const size_t num_ecc_op_gates = circuit_constructor.num_ecc_op_gates; gate_offset += num_ecc_op_gates; diff --git a/barretenberg/cpp/src/barretenberg/proof_system/relations/elliptic_relation.hpp b/barretenberg/cpp/src/barretenberg/proof_system/relations/elliptic_relation.hpp index d0d9910195b..511f7f37a23 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/relations/elliptic_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/relations/elliptic_relation.hpp @@ -33,7 +33,7 @@ template 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) { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/transcript/transcript.hpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/transcript/transcript.hpp index 4adb5c0fceb..f02354207cb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/transcript/transcript.hpp +++ b/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 class Transcript { public: diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp index bc2926ac47a..4e3ed1d9c55 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp @@ -152,7 +152,7 @@ std::array UltraRecursiveVerifier_::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); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/utility/utility.hpp b/barretenberg/cpp/src/barretenberg/stdlib/utility/utility.hpp index e17926fa47d..5d0fb4f43b4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/utility/utility.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/utility/utility.hpp @@ -83,7 +83,6 @@ template class StdlibTypesUtility { /** * @brief Construct field_t array from native Univariate type - * TODO(luke): do we need a stdlib Univariate or is Univariate good enough? * @param native_element * @return Univariate */ diff --git a/barretenberg/cpp/src/barretenberg/transcript/manifest.hpp b/barretenberg/cpp/src/barretenberg/transcript/manifest.hpp index c0b251ee11e..4cc4e09cabf 100644 --- a/barretenberg/cpp/src/barretenberg/transcript/manifest.hpp +++ b/barretenberg/cpp/src/barretenberg/transcript/manifest.hpp @@ -67,7 +67,6 @@ class Manifest { bool map_challenges; }; - // TODO(luke): needed only in development; can be deleted when appropriate Manifest() = default; Manifest(std::vector _round_manifests) : round_manifests(_round_manifests) diff --git a/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp b/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp index bd33eff8008..3ec18b95c43 100644 --- a/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp +++ b/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp @@ -53,9 +53,6 @@ class StandardTranscript : public Transcript { const std::string& challenge_map_name) const; std::vector export_transcript() const { return Transcript::export_transcript(); } - - // TODO(luke): temporary function for debugging - barretenberg::fr get_mock_challenge() { return barretenberg::fr::random_element(); }; }; } // namespace transcript From cd906d6b15ca7ad1efffd59bda2add9d9f8e202e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 25 Sep 2023 18:27:46 +0000 Subject: [PATCH 2/7] sumcheck always returns value --- .../honk/proof_system/eccvm_verifier.cpp | 15 +++++----- .../honk/proof_system/ultra_verifier.cpp | 13 +++++---- .../barretenberg/honk/sumcheck/sumcheck.hpp | 13 ++------- .../honk/sumcheck/sumcheck.test.cpp | 28 ++++++++++-------- .../honk/sumcheck/sumcheck_output.hpp | 29 ++----------------- .../honk/sumcheck/sumcheck_round.hpp | 3 +- .../verifier/ultra_recursive_verifier.cpp | 12 ++------ 7 files changed, 40 insertions(+), 73 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp index 74f35e6e4b1..a45349adf1c 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp @@ -186,15 +186,14 @@ template bool ECCVMVerifier_::verify_proof(const plonk // Execute Sumcheck Verifier auto sumcheck = SumcheckVerifier(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.value()) { return false; } - auto [multivariate_challenge, purported_evaluations] = *sumcheck_output; - // Execute Gemini/Shplonk verification: // Construct inputs for Gemini verifier: @@ -254,8 +253,10 @@ template bool ECCVMVerifier_::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_; diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp index 38dc6c2b8b1..6d4bca46f2b 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp @@ -114,15 +114,14 @@ template bool UltraVerifier_::verify_proof(const plonk // Execute Sumcheck Verifier auto sumcheck = SumcheckVerifier(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.value()) { return false; } - auto [multivariate_challenge, purported_evaluations] = *sumcheck_output; - // Execute Gemini/Shplonk verification: // Construct inputs for Gemini verifier: @@ -218,7 +217,9 @@ template bool UltraVerifier_::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_; diff --git a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.hpp b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.hpp index 06847d05509..48f2283700e 100644 --- a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.hpp @@ -172,8 +172,7 @@ template class SumcheckVerifier { * @param relation_parameters * @param transcript */ - std::optional> verify(const proof_system::RelationParameters& relation_parameters, - auto& transcript) + SumcheckOutput verify(const proof_system::RelationParameters& relation_parameters, auto& transcript) { bool verified(true); @@ -204,11 +203,6 @@ template 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 class SumcheckVerifier { checked = (full_honk_relation_purported_value == round.target_total_sum); } verified = verified && checked; - if (!verified) { - return std::nullopt; - } - return SumcheckOutput{ multivariate_challenge, purported_evaluations }; + return SumcheckOutput{ multivariate_challenge, purported_evaluations, verified }; }; }; } // namespace proof_system::honk::sumcheck diff --git a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp index c79d96f5ecc..9d6712c0a3e 100644 --- a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp @@ -107,11 +107,11 @@ TEST_F(SumcheckTests, PolynomialNormalization) auto sumcheck = SumcheckProver(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_point[0]; + FF u_1 = output.challenge_point[1]; + FF u_2 = output.challenge_point[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(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_point[0]; + FF u_1 = output.challenge_point[1]; std::vector 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.purported_evaluations[poly_idx], expected_values[poly_idx]); } } @@ -242,9 +242,11 @@ TEST_F(SumcheckTests, ProverAndVerifierSimple) auto sumcheck_verifier = SumcheckVerifier(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(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 diff --git a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp index c83d85c15a0..7e8ce6b76b8 100644 --- a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include namespace proof_system::honk::sumcheck { @@ -15,31 +16,7 @@ template struct SumcheckOutput { std::vector challenge_point; // Evaluations in `u` of the polynomials used in Sumcheck ClaimedEvaluations purported_evaluations; - - SumcheckOutput() - : purported_evaluations(std::array()){}; - - SumcheckOutput(const std::vector& _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; - }; + // Whether or not the purported multilinear evaluations and final sumcheck evaluation have been confirmed + std::optional verified = false; }; } // namespace proof_system::honk::sumcheck diff --git a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_round.hpp b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_round.hpp index 1c335957c29..353433c756f 100644 --- a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_round.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_round.hpp @@ -424,8 +424,7 @@ template class SumcheckVerifierRound { // with a simulated builder. bool sumcheck_round_failed(false); if constexpr (IsRecursiveFlavor) { - // 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); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp index 4e3ed1d9c55..4b97dd1c1ab 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/ultra_recursive_verifier.cpp @@ -98,10 +98,10 @@ std::array UltraRecursiveVerifier_::ve commitments.z_perm = transcript.template receive_from_prover(commitment_labels.z_perm); commitments.z_lookup = transcript.template receive_from_prover(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 UltraRecursiveVerifier_::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 rhos = ::proof_system::honk::pcs::gemini::powers_of_rho(rho, Flavor::NUM_ALL_ENTITIES); From 13fea990ce4d625caba5069a0833298ce7501e75 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 25 Sep 2023 20:10:04 +0000 Subject: [PATCH 3/7] add failure tests for the honk recursive verifiers --- .../honk/verifier/goblin_verifier.test.cpp | 36 ++++++++++++++++++ .../recursion/honk/verifier/verifier.test.cpp | 37 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp index a74ea38d1f8..814968794f5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp @@ -196,6 +196,37 @@ template 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(#656): For now we gat 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(&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 \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index 557194b433c..da17d1e687e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -180,6 +180,38 @@ template class RecursiveVerifierTest : public testing::Te 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(#656): For now we gat 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 + InnerBuilder inner_circuit; + create_inner_circuit(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(&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 @@ -202,4 +234,9 @@ HEAVY_TYPED_TEST(RecursiveVerifierTest, SingleRecursiveVerification) TestFixture::test_recursive_verification(); }; +HEAVY_TYPED_TEST(RecursiveVerifierTest, SingleRecursiveVerificationFailure) +{ + TestFixture::test_recursive_verification_fails(); +}; + } // namespace proof_system::plonk::stdlib::recursion::honk \ No newline at end of file From c4b3d08b2be3b567707a3fe1d3e5426dbe2b59c8 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 26 Sep 2023 15:41:07 +0000 Subject: [PATCH 4/7] typo --- .../stdlib/recursion/honk/verifier/goblin_verifier.test.cpp | 2 +- .../stdlib/recursion/honk/verifier/verifier.test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp index 814968794f5..543303301a3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp @@ -199,7 +199,7 @@ template class GoblinRecursiveVerifierTest : public testi /** * @brief Construct a verifier circuit for a proof whose data has been tampered with. Expect failure - * TODO(#656): For now we gat a "bad" proof by arbitrarily tampering with bits in a valid proof. It would be + * TODO(#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. */ diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index da17d1e687e..bc4a749943d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -183,7 +183,7 @@ template class RecursiveVerifierTest : public testing::Te /** * @brief Construct a verifier circuit for a proof whose data has been tampered with. Expect failure - * TODO(#656): For now we gat a "bad" proof by arbitrarily tampering with bits in a valid proof. It would be + * TODO(#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. */ From 9f2a9453de53e9f23bb326564c80236160feae46 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 26 Sep 2023 23:17:56 +0000 Subject: [PATCH 5/7] fixes in response to maras comments --- barretenberg/cpp/src/barretenberg/bb/exec_pipe.hpp | 1 + .../barretenberg/honk/proof_system/eccvm_prover.cpp | 4 ++-- .../honk/proof_system/eccvm_verifier.cpp | 2 +- .../barretenberg/honk/proof_system/ultra_prover.cpp | 4 ++-- .../honk/proof_system/ultra_verifier.cpp | 2 +- .../src/barretenberg/honk/sumcheck/sumcheck.test.cpp | 12 ++++++------ .../barretenberg/honk/sumcheck/sumcheck_output.hpp | 8 ++++---- .../cpp/src/barretenberg/numeric/uint128/uint128.hpp | 2 +- .../recursion/honk/verifier/goblin_verifier.test.cpp | 2 +- .../stdlib/recursion/honk/verifier/verifier.test.cpp | 2 +- 10 files changed, 20 insertions(+), 19 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/exec_pipe.hpp b/barretenberg/cpp/src/barretenberg/bb/exec_pipe.hpp index 945901a8107..5ab7fcab34b 100644 --- a/barretenberg/cpp/src/barretenberg/bb/exec_pipe.hpp +++ b/barretenberg/cpp/src/barretenberg/bb/exec_pipe.hpp @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_prover.cpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_prover.cpp index b50d9823b0e..c44589b0578 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_prover.cpp @@ -261,7 +261,7 @@ template void ECCVMProver_::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 void ECCVMProver_::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); diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp index a45349adf1c..80ff34bd587 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/eccvm_verifier.cpp @@ -190,7 +190,7 @@ template bool ECCVMVerifier_::verify_proof(const plonk sumcheck.verify(relation_parameters, transcript); // If Sumcheck did not verify, return false - if (!sumcheck_verified.value()) { + if (sumcheck_verified.has_value() && !sumcheck_verified.value()) { return false; } diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_prover.cpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_prover.cpp index 393e896059c..ad3151b45bd 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_prover.cpp @@ -139,7 +139,7 @@ template void UltraProver_::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 void UltraProver_::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); diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp index 6d4bca46f2b..4d304709c0b 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/ultra_verifier.cpp @@ -118,7 +118,7 @@ template bool UltraVerifier_::verify_proof(const plonk sumcheck.verify(relation_parameters, transcript); // If Sumcheck did not verify, return false - if (!sumcheck_verified.value()) { + if (sumcheck_verified.has_value() && !sumcheck_verified.value()) { return false; } diff --git a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp index 9d6712c0a3e..f8ae29e46e3 100644 --- a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp +++ b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck.test.cpp @@ -109,9 +109,9 @@ TEST_F(SumcheckTests, PolynomialNormalization) auto output = sumcheck.prove(full_polynomials, {}); - FF u_0 = output.challenge_point[0]; - FF u_1 = output.challenge_point[1]; - FF u_2 = output.challenge_point[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 @@ -163,8 +163,8 @@ TEST_F(SumcheckTests, Prover) auto sumcheck = SumcheckProver(multivariate_n, transcript); auto output = sumcheck.prove(full_polynomials, {}); - FF u_0 = output.challenge_point[0]; - FF u_1 = output.challenge_point[1]; + FF u_0 = output.challenge[0]; + FF u_1 = output.challenge[1]; std::vector 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(output.purported_evaluations[poly_idx], expected_values[poly_idx]); + EXPECT_EQ(output.claimed_evaluations[poly_idx], expected_values[poly_idx]); } } diff --git a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp index 7e8ce6b76b8..a274c9df3ab 100644 --- a/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/sumcheck/sumcheck_output.hpp @@ -13,10 +13,10 @@ template struct SumcheckOutput { using FF = typename Flavor::FF; using ClaimedEvaluations = typename Flavor::ClaimedEvaluations; // u = (u_0, ..., u_{d-1}) - std::vector challenge_point; + std::vector challenge; // Evaluations in `u` of the polynomials used in Sumcheck - ClaimedEvaluations purported_evaluations; - // Whether or not the purported multilinear evaluations and final sumcheck evaluation have been confirmed - std::optional verified = false; + ClaimedEvaluations claimed_evaluations; + // Whether or not the claimed multilinear evaluations and final sumcheck evaluation have been confirmed + std::optional verified = false; // optional b/c this struct is shared by the Prover/Verifier }; } // namespace proof_system::honk::sumcheck diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp index fc0410b647d..3dc320abe2d 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp @@ -1,10 +1,10 @@ #pragma once +#include #include #include #ifdef __i386__ #include "barretenberg/common/serialize.hpp" -#include namespace numeric { diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp index 543303301a3..aa7ed4490e2 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp @@ -199,7 +199,7 @@ template class GoblinRecursiveVerifierTest : public testi /** * @brief Construct a verifier circuit for a proof whose data has been tampered with. Expect failure - * TODO(#656): For now we get a "bad" proof by arbitrarily tampering with bits in a valid proof. It would be + * 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. */ diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index bc4a749943d..0da616b18cd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -183,7 +183,7 @@ template class RecursiveVerifierTest : public testing::Te /** * @brief Construct a verifier circuit for a proof whose data has been tampered with. Expect failure - * TODO(#656): For now we get a "bad" proof by arbitrarily tampering with bits in a valid proof. It would be + * 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. */ From 906d6da39841fda0987d908ae381b4ad71277960 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 27 Sep 2023 14:46:22 +0000 Subject: [PATCH 6/7] try to remove plonk changes --- .../plonk/proof_system/types/polynomial_manifest.hpp | 4 ++-- .../cpp/src/barretenberg/transcript/transcript_wrappers.hpp | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp b/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp index da671799ebb..162f21fc432 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp +++ b/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp @@ -1,6 +1,6 @@ #pragma once -#include "barretenberg/common/throw_or_abort.hpp" +// #include "barretenberg/common/throw_or_abort.hpp" #include "barretenberg/proof_system/types/circuit_type.hpp" #include #include @@ -164,7 +164,7 @@ class PolynomialManifest { break; }; default: { - throw_or_abort("Received invalid composer type"); + // throw_or_abort("Received invalid composer type"); } }; } diff --git a/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp b/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp index 3ec18b95c43..bd33eff8008 100644 --- a/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp +++ b/barretenberg/cpp/src/barretenberg/transcript/transcript_wrappers.hpp @@ -53,6 +53,9 @@ class StandardTranscript : public Transcript { const std::string& challenge_map_name) const; std::vector export_transcript() const { return Transcript::export_transcript(); } + + // TODO(luke): temporary function for debugging + barretenberg::fr get_mock_challenge() { return barretenberg::fr::random_element(); }; }; } // namespace transcript From a76b518f71f794a3bf635876e2efd53841d47570 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 27 Sep 2023 15:15:11 +0000 Subject: [PATCH 7/7] remove manifest change --- .../plonk/proof_system/types/polynomial_manifest.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp b/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp index 162f21fc432..d4148be017b 100644 --- a/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp +++ b/barretenberg/cpp/src/barretenberg/plonk/proof_system/types/polynomial_manifest.hpp @@ -1,6 +1,5 @@ #pragma once -// #include "barretenberg/common/throw_or_abort.hpp" #include "barretenberg/proof_system/types/circuit_type.hpp" #include #include @@ -142,6 +141,7 @@ static constexpr PolynomialDescriptor ultra_polynomial_manifest[ULTRA_MANIFEST_S // Simple class allowing for access to a polynomial manifest based on composer type class PolynomialManifest { + // TODO(luke): make this object iterable, i.e. compatible with range-based for loop private: std::vector manifest; @@ -164,6 +164,7 @@ class PolynomialManifest { break; }; default: { + // TODO(luke): reinstate this. Was getting "use of undeclared identifier" error for 'throw_or_abort'. // throw_or_abort("Received invalid composer type"); } };