-
Notifications
You must be signed in to change notification settings - Fork 599
chore: chonk rec ver 0 #20506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: chonk rec ver 0 #20506
Changes from all commits
68fb333
3c91f33
cfa4b75
ad61db6
caef3a7
c4a2a47
51ea620
37c207b
6d9e044
a49c09b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -341,9 +341,6 @@ template <typename Curve> class SmallSubgroupIPAVerifier { | |
| */ | ||
| static void handle_edge_cases(const FF& vanishing_poly_eval) | ||
| { | ||
|
|
||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/1194). Handle edge cases in PCS | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are both closed - I think we made the explicit decision that they were not needed |
||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/1186). Insecure pattern. | ||
| bool evaluation_challenge_in_small_subgroup = false; | ||
| if constexpr (Curve::is_stdlib_type) { | ||
| evaluation_challenge_in_small_subgroup = (vanishing_poly_eval.get_value() == FF(0).get_value()); | ||
|
|
||
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| #include "barretenberg/goblin/goblin_verifier.hpp" | ||
| #include "barretenberg/circuit_checker/circuit_checker.hpp" | ||
| #include "barretenberg/common/assert.hpp" | ||
| #include "barretenberg/common/test.hpp" | ||
| #include "barretenberg/goblin/goblin.hpp" | ||
| #include "barretenberg/goblin/mock_circuits.hpp" | ||
|
|
@@ -27,41 +26,19 @@ class GoblinRecursiveVerifierTests : public testing::Test { | |
| using RecursiveMergeCommitments = bb::GoblinRecursiveVerifier::MergeVerifier::InputCommitments; | ||
| using Transcript = UltraStdlibTranscript; | ||
| using FF = TranslatorFlavor::FF; | ||
| using BF = TranslatorFlavor::BF; | ||
| static void SetUpTestSuite() { bb::srs::init_file_crs_factory(bb::srs::bb_crs_path()); } | ||
|
|
||
| // Compute the size of a Translator commitment (in bb::fr's) | ||
| static constexpr size_t comm_frs = FrCodec::calc_num_fields<Commitment>(); // 4 | ||
| static constexpr size_t eval_frs = FrCodec::calc_num_fields<FF>(); // 1 | ||
|
|
||
| struct ProverOutput { | ||
| GoblinProof proof; | ||
| MergeCommitments merge_commitments; | ||
| RecursiveMergeCommitments recursive_merge_commitments; | ||
| }; | ||
| // TODO(https://github.com/AztecProtocol/barretenberg/issues/1298): | ||
| // Better recursion testing - create more flexible proof tampering tests. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm relying on the independent ECCVM/Translator tests for most of the coverage here. Just trying to target things that are particular to the connective tissue of Goblin here |
||
| // Tamper with the `op` commitment in the merge commitments (op commitments are no longer in translator proof) | ||
| static void tamper_with_op_commitment(MergeCommitments& merge_commitments) | ||
| { | ||
| // The first commitment in merged table is the `op` wire commitment | ||
| merge_commitments.t_commitments[0] = merge_commitments.t_commitments[0] * FF(2); | ||
| }; | ||
|
|
||
| // Translator proof ends with [..., Libra:quotient_eval, Shplonk:Q, KZG:W]. We invalidate the proof by multiplying | ||
| // the eval by 2 (it leads to a Libra consistency check failure). | ||
| static void tamper_with_libra_eval(HonkProof& translator_proof) | ||
| { | ||
| // Proof tail size | ||
| static constexpr size_t tail_size = 2 * comm_frs + eval_frs; // 2*4 + 1 = 9 | ||
|
|
||
| // Index of the target field (one fr) from the beginning | ||
| const size_t idx = translator_proof.size() - tail_size; | ||
|
|
||
| // Tamper: multiply by 2 (or tweak however you like) | ||
| translator_proof[idx] = translator_proof[idx] + translator_proof[idx]; | ||
| }; | ||
|
|
||
| // ECCVM pre-IPA proof ends with evaluations including `op`. We tamper with the `op` evaluation. | ||
| // The structure is: [..., op_eval, x_lo_y_hi_eval, x_hi_z_1_eval, y_lo_z_2_eval, IPA_proof...] | ||
| // So op_eval is 3 fields before the IPA proof starts. | ||
|
|
@@ -238,22 +215,27 @@ TEST_F(GoblinRecursiveVerifierTests, IndependentVKHash) | |
| } | ||
|
|
||
| /** | ||
| * @brief Ensure failure of the goblin recursive verification circuit for a bad ECCVM proof | ||
| * | ||
| * @brief Tampered merge commitments cause the Translator pairing check to fail | ||
| * @details Tests the Merge-Translator cross-component connection: merge_commitments flow into the Translator verifier | ||
| * as op queue wire commitments. A mismatch causes the KZG pairing check to fail (not a circuit failure). | ||
| */ | ||
| TEST_F(GoblinRecursiveVerifierTests, ECCVMFailure) | ||
| TEST_F(GoblinRecursiveVerifierTests, MergeToTranslatorBindingFailure) | ||
| { | ||
| BB_DISABLE_ASSERTS(); // Avoid on_curve assertion failure in cycle_group etc | ||
| Builder builder; | ||
| auto [proof, merge_commitments, _] = create_goblin_prover_output(); | ||
|
|
||
| auto [proof, merge_commitments, recursive_merge_commitments] = create_goblin_prover_output(&builder); | ||
| // Tamper with the op commitment in merge commitments (used by Translator verifier) | ||
| MergeCommitments tampered_merge_commitments = merge_commitments; | ||
| tamper_with_op_commitment(tampered_merge_commitments); | ||
| Builder builder; | ||
|
|
||
| // Tamper with the ECCVM proof | ||
| for (auto& val : proof.eccvm_proof) { | ||
| if (val > 0) { // tamper by finding the first non-zero value and incrementing it by 1 | ||
| val += 1; | ||
| break; | ||
| } | ||
| RecursiveMergeCommitments recursive_merge_commitments; | ||
| for (size_t idx = 0; idx < MegaFlavor::NUM_WIRES; idx++) { | ||
| recursive_merge_commitments.t_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, tampered_merge_commitments.t_commitments[idx]); | ||
| recursive_merge_commitments.T_prev_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, tampered_merge_commitments.T_prev_commitments[idx]); | ||
| recursive_merge_commitments.t_commitments[idx].fix_witness(); | ||
| recursive_merge_commitments.T_prev_commitments[idx].fix_witness(); | ||
| } | ||
|
|
||
| auto transcript = std::make_shared<Transcript>(); | ||
|
|
@@ -262,101 +244,34 @@ TEST_F(GoblinRecursiveVerifierTests, ECCVMFailure) | |
| transcript, stdlib_proof, recursive_merge_commitments, MergeSettings::APPEND | ||
| }; | ||
| auto goblin_rec_verifier_output = verifier.reduce_to_pairing_check_and_ipa_opening(); | ||
| EXPECT_FALSE(CircuitChecker::check(builder)); | ||
|
|
||
| srs::init_file_crs_factory(bb::srs::bb_crs_path()); | ||
| auto crs_factory = srs::get_grumpkin_crs_factory(); | ||
| VerifierCommitmentKey<curve::Grumpkin> grumpkin_verifier_commitment_key(1 << CONST_ECCVM_LOG_N, crs_factory); | ||
| OpeningClaim<curve::Grumpkin> native_claim = goblin_rec_verifier_output.ipa_claim.get_native_opening_claim(); | ||
| auto native_ipa_transcript = std::make_shared<NativeTranscript>(goblin_rec_verifier_output.ipa_proof.get_value()); | ||
|
|
||
| bool native_result = | ||
| IPA<curve::Grumpkin>::reduce_verify(grumpkin_verifier_commitment_key, native_claim, native_ipa_transcript); | ||
| EXPECT_FALSE(native_result); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Ensure failure of the goblin recursive verification circuit for a bad Translator proof | ||
| * | ||
| */ | ||
| TEST_F(GoblinRecursiveVerifierTests, TranslatorFailure) | ||
| { | ||
| auto [proof, merge_commitments, _] = create_goblin_prover_output(); | ||
|
|
||
| // Tamper with the op commitment in merge commitments (used by Translator verifier) | ||
| { | ||
| MergeCommitments tampered_merge_commitments = merge_commitments; | ||
| tamper_with_op_commitment(tampered_merge_commitments); | ||
| Builder builder; | ||
|
|
||
| RecursiveMergeCommitments recursive_merge_commitments; | ||
| for (size_t idx = 0; idx < MegaFlavor::NUM_WIRES; idx++) { | ||
| recursive_merge_commitments.t_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, tampered_merge_commitments.t_commitments[idx]); | ||
| recursive_merge_commitments.T_prev_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, tampered_merge_commitments.T_prev_commitments[idx]); | ||
| recursive_merge_commitments.t_commitments[idx].fix_witness(); | ||
| recursive_merge_commitments.T_prev_commitments[idx].fix_witness(); | ||
| } | ||
|
|
||
| auto transcript = std::make_shared<Transcript>(); | ||
| GoblinStdlibProof stdlib_proof(builder, proof); | ||
| bb::GoblinRecursiveVerifier verifier{ | ||
| transcript, stdlib_proof, recursive_merge_commitments, MergeSettings::APPEND | ||
| }; | ||
| auto goblin_rec_verifier_output = verifier.reduce_to_pairing_check_and_ipa_opening(); | ||
|
|
||
| // Aggregate merge + translator pairing points | ||
| goblin_rec_verifier_output.translator_pairing_points.aggregate(goblin_rec_verifier_output.merge_pairing_points); | ||
|
|
||
| // Circuit is correct but pairing check should fail | ||
| EXPECT_TRUE(CircuitChecker::check(builder)); | ||
|
|
||
| // Check that the pairing fails natively | ||
| bb::PairingPoints<curve::BN254> native_pairing_points( | ||
| goblin_rec_verifier_output.translator_pairing_points.P0().get_value(), | ||
| goblin_rec_verifier_output.translator_pairing_points.P1().get_value()); | ||
| bool pairing_result = native_pairing_points.check(); | ||
| EXPECT_FALSE(pairing_result); | ||
| } | ||
| // Tamper with the Translator proof non - preamble values | ||
| { | ||
| auto tampered_proof = proof; | ||
| tamper_with_libra_eval(tampered_proof.translator_proof); | ||
|
|
||
| Builder builder; | ||
| // Aggregate merge + translator pairing points | ||
| goblin_rec_verifier_output.translator_pairing_points.aggregate(goblin_rec_verifier_output.merge_pairing_points); | ||
|
|
||
| RecursiveMergeCommitments recursive_merge_commitments; | ||
| for (size_t idx = 0; idx < MegaFlavor::NUM_WIRES; idx++) { | ||
| recursive_merge_commitments.t_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, merge_commitments.t_commitments[idx]); | ||
| recursive_merge_commitments.T_prev_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, merge_commitments.T_prev_commitments[idx]); | ||
| recursive_merge_commitments.t_commitments[idx].fix_witness(); | ||
| recursive_merge_commitments.T_prev_commitments[idx].fix_witness(); | ||
| } | ||
| // Circuit is correct but pairing check should fail | ||
| EXPECT_TRUE(CircuitChecker::check(builder)); | ||
|
|
||
| auto transcript = std::make_shared<Transcript>(); | ||
| GoblinStdlibProof stdlib_proof(builder, tampered_proof); | ||
| bb::GoblinRecursiveVerifier verifier{ | ||
| transcript, stdlib_proof, recursive_merge_commitments, MergeSettings::APPEND | ||
| }; | ||
| [[maybe_unused]] auto goblin_rec_verifier_output = verifier.reduce_to_pairing_check_and_ipa_opening(); | ||
| EXPECT_FALSE(CircuitChecker::check(builder)); | ||
| } | ||
| // Check that the pairing fails natively | ||
| bb::PairingPoints<curve::BN254> native_pairing_points( | ||
| goblin_rec_verifier_output.translator_pairing_points.P0().get_value(), | ||
| goblin_rec_verifier_output.translator_pairing_points.P1().get_value()); | ||
| bool pairing_result = native_pairing_points.check(); | ||
| EXPECT_FALSE(pairing_result); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Ensure failure of the goblin recursive verification circuit for bad translation evaluations | ||
| * | ||
| * @brief Tampered ECCVM translation evaluations cause the Translator circuit to fail | ||
| * @details Tests the ECCVM-Translator cross-component connection: translation evaluations (op, Px, Py, z1, z2) from | ||
| * the ECCVM proof become `accumulated_result` in the Translator verifier. Tampering with these causes the Translator's | ||
| * relation constraints to fail in-circuit. | ||
| */ | ||
| TEST_F(GoblinRecursiveVerifierTests, TranslationEvaluationsFailure) | ||
| TEST_F(GoblinRecursiveVerifierTests, ECCVMToTranslatorBindingFailure) | ||
| { | ||
| Builder builder; | ||
|
|
||
| auto [proof, merge_commitments, recursive_merge_commitments] = create_goblin_prover_output(&builder); | ||
|
|
||
| // Tamper with the `op` evaluation in the ECCVM proof using the helper function | ||
| // Tamper with the `op` evaluation in the ECCVM proof | ||
| tamper_with_eccvm_op_eval(proof.eccvm_proof); | ||
|
|
||
| auto transcript = std::make_shared<Transcript>(); | ||
|
|
@@ -368,67 +283,4 @@ TEST_F(GoblinRecursiveVerifierTests, TranslationEvaluationsFailure) | |
|
|
||
| EXPECT_FALSE(CircuitChecker::check(builder)); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Ensure failure of the goblin recursive verification circuit for bad translation evaluations | ||
| * | ||
| */ | ||
| TEST_F(GoblinRecursiveVerifierTests, TranslatorMergeConsistencyFailure) | ||
| { | ||
|
|
||
| { | ||
|
|
||
| Builder builder; | ||
|
|
||
| auto [proof, merge_commitments, recursive_merge_commitments] = create_goblin_prover_output(&builder); | ||
|
|
||
| // Check natively that the proof is correct. | ||
| auto native_transcript = std::make_shared<NativeTranscript>(); | ||
| bb::GoblinVerifier native_verifier(native_transcript, proof, merge_commitments, MergeSettings::APPEND); | ||
| auto native_result = native_verifier.reduce_to_pairing_check_and_ipa_opening(); | ||
| // Aggregate merge + translator pairing points before checking | ||
| native_result.translator_pairing_points.aggregate(native_result.merge_pairing_points); | ||
| bool pairing_verified = native_result.translator_pairing_points.check(); | ||
| auto ipa_transcript = std::make_shared<NativeTranscript>(native_result.ipa_proof); | ||
| auto ipa_vk = VerifierCommitmentKey<curve::Grumpkin>{ ECCVMFlavor::ECCVM_FIXED_SIZE }; | ||
| bool ipa_verified = IPA<curve::Grumpkin>::reduce_verify(ipa_vk, native_result.ipa_claim, ipa_transcript); | ||
| EXPECT_TRUE(pairing_verified && ipa_verified); | ||
|
|
||
| // Tamper with the op commitment in merge commitments (used by Translator verifier) | ||
| MergeCommitments tampered_merge_commitments = merge_commitments; | ||
| tamper_with_op_commitment(tampered_merge_commitments); | ||
|
|
||
| // Construct and check the Goblin Recursive Verifier circuit | ||
|
|
||
| RecursiveMergeCommitments tampered_recursive_merge_commitments; | ||
| for (size_t idx = 0; idx < MegaFlavor::NUM_WIRES; idx++) { | ||
| tampered_recursive_merge_commitments.t_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, tampered_merge_commitments.t_commitments[idx]); | ||
| tampered_recursive_merge_commitments.T_prev_commitments[idx] = | ||
| RecursiveCommitment::from_witness(&builder, tampered_merge_commitments.T_prev_commitments[idx]); | ||
| tampered_recursive_merge_commitments.t_commitments[idx].fix_witness(); | ||
| tampered_recursive_merge_commitments.T_prev_commitments[idx].fix_witness(); | ||
| } | ||
|
|
||
| auto transcript = std::make_shared<Transcript>(); | ||
| GoblinStdlibProof stdlib_proof(builder, proof); | ||
| bb::GoblinRecursiveVerifier verifier{ | ||
| transcript, stdlib_proof, tampered_recursive_merge_commitments, MergeSettings::APPEND | ||
| }; | ||
| auto goblin_rec_verifier_output = verifier.reduce_to_pairing_check_and_ipa_opening(); | ||
|
|
||
| // Aggregate merge + translator pairing points | ||
| goblin_rec_verifier_output.translator_pairing_points.aggregate(goblin_rec_verifier_output.merge_pairing_points); | ||
|
|
||
| // Circuit is correct but pairing check should fail | ||
| EXPECT_TRUE(CircuitChecker::check(builder)); | ||
|
|
||
| // Check that the pairing fails natively | ||
| bb::PairingPoints<curve::BN254> native_pairing_points( | ||
| goblin_rec_verifier_output.translator_pairing_points.P0().get_value(), | ||
| goblin_rec_verifier_output.translator_pairing_points.P1().get_value()); | ||
| bool pairing_result = native_pairing_points.check(); | ||
| EXPECT_FALSE(pairing_result); | ||
| } | ||
| } | ||
| } // namespace bb::stdlib::recursion::honk | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some formatting fixes to get this to display well