diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp index 03e4d731d72..3c4cad8dac4 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/mega_circuit_builder.test.cpp @@ -149,7 +149,7 @@ TEST(MegaCircuitBuilder, GoblinEccOpQueueUltraOps) builder.queue_ecc_eq(); // Check that the ultra ops recorded in the EccOpQueue match the ops recorded in the wires - auto ultra_ops = builder.op_queue->get_aggregate_transcript(); + auto ultra_ops = builder.op_queue->construct_current_ultra_ops_subtable_columns(); for (size_t i = 1; i < 4; ++i) { for (size_t j = 0; j < builder.blocks.ecc_op.size(); ++j) { auto op_wire_val = builder.variables[builder.blocks.ecc_op.wires[i][j]]; @@ -203,4 +203,4 @@ TEST(MegaCircuitBuilder, CompleteSelectorPartitioningCheck) } } -} // namespace bb \ No newline at end of file +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/mock_kernel_pinning.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/mock_kernel_pinning.test.cpp index 5765a9902dd..6bf11bc07b6 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/mock_kernel_pinning.test.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/mock_kernel_pinning.test.cpp @@ -33,7 +33,8 @@ TEST_F(MockKernelTest, PinFoldingKernelSizes) Builder circuit = circuit_producer.create_next_circuit(ivc); ivc.accumulate(circuit); + EXPECT_FALSE(circuit.blocks.has_overflow); // trace oveflow mechanism should not be triggered } EXPECT_EQ(ivc.fold_output.accumulator->proving_key.log_circuit_size, 19); -} \ No newline at end of file +} diff --git a/barretenberg/cpp/src/barretenberg/constants.hpp b/barretenberg/cpp/src/barretenberg/constants.hpp index 53f097b30c4..8ef4e470724 100644 --- a/barretenberg/cpp/src/barretenberg/constants.hpp +++ b/barretenberg/cpp/src/barretenberg/constants.hpp @@ -24,4 +24,6 @@ static constexpr uint32_t MASKING_OFFSET = 4; // For ZK Flavors: the number of the commitments required by Libra and SmallSubgroupIPA. static constexpr uint32_t NUM_LIBRA_COMMITMENTS = 3; static constexpr uint32_t NUM_LIBRA_EVALUATIONS = 4; -} // namespace bb \ No newline at end of file + +static constexpr uint32_t MERGE_PROOF_SIZE = 65; // used to ensure mock proofs are generated correctly +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp index 7a6bc5e8864..4d77b3fbce1 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.cpp @@ -241,11 +241,15 @@ ClientIVC::MergeProof create_dummy_merge_proof() using FF = ClientIVC::FF; std::vector<FF> proof; + proof.reserve(MERGE_PROOF_SIZE); FF mock_val(5); auto mock_commitment = curve::BN254::AffineElement::one(); std::vector<FF> mock_commitment_frs = field_conversion::convert_to_bn254_frs(mock_commitment); + // Populate mock subtable size + proof.emplace_back(mock_val); + // There are 12 entities in the merge protocol (4 columns x 3 components; aggregate transcript, previous aggregate // transcript, current transcript contribution) const size_t NUM_TRANSCRIPT_ENTITIES = 12; @@ -265,6 +269,8 @@ ClientIVC::MergeProof create_dummy_merge_proof() proof.emplace_back(val); } + ASSERT(proof.size() == MERGE_PROOF_SIZE); + return proof; } diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.test.cpp index 78a75d6c239..5c14bc33dd9 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ivc_recursion_constraint.test.cpp @@ -136,6 +136,15 @@ class IvcRecursionConstraintTest : public ::testing::Test { } }; +/** + * @brief Check that the size of a mock merge proof matches expectation + */ +TEST_F(IvcRecursionConstraintTest, MockMergeProofSize) +{ + ClientIVC::MergeProof merge_proof = create_dummy_merge_proof(); + EXPECT_EQ(merge_proof.size(), MERGE_PROOF_SIZE); +} + /** * @brief Test IVC accumulation of a one app and one kernel; The kernel includes a recursive oink verification for the * app, specified via an ACIR RecursionConstraint. diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp index 7d472685f16..ead7e0ad55c 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp @@ -113,10 +113,10 @@ class ECCVMCircuitBuilder { std::vector<std::pair<size_t, size_t>> msm_mul_index; std::vector<size_t> msm_sizes; - const auto& raw_ops = op_queue->get_raw_ops(); + const auto& eccvm_ops = op_queue->get_eccvm_ops(); size_t op_idx = 0; // populate opqueue and mul indices - for (const auto& op : raw_ops) { + for (const auto& op : eccvm_ops) { if (op.mul) { if ((op.z1 != 0 || op.z2 != 0) && !op.base_point.is_point_at_infinity()) { msm_opqueue_index.push_back(op_idx); @@ -131,7 +131,7 @@ class ECCVMCircuitBuilder { op_idx++; } // if last op is a mul we have not correctly computed the total number of msms - if (raw_ops.back().mul && active_mul_count > 0) { + if (eccvm_ops.back().mul && active_mul_count > 0) { msm_sizes.push_back(active_mul_count); msm_count++; } @@ -143,7 +143,7 @@ class ECCVMCircuitBuilder { parallel_for_range(msm_opqueue_index.size(), [&](size_t start, size_t end) { for (size_t i = start; i < end; i++) { - const auto& op = raw_ops[msm_opqueue_index[i]]; + const auto& op = eccvm_ops[msm_opqueue_index[i]]; auto [msm_index, mul_index] = msm_mul_index[i]; if (op.z1 != 0 && !op.base_point.is_point_at_infinity()) { ASSERT(result.size() > msm_index); diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp index 0ee34da7aa7..fd31725893e 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp @@ -454,7 +454,7 @@ TEST(ECCVMCircuitBuilderTests, AddProducesDouble) * @details Currently, Goblin does not support clean initialization, which means that we have to create mock ECCOpQueue * to avoid commiting to zero polynomials. This test localizes the issue to the problem with populating ECCVM Transcript * rows in the method \ref bb::ECCVMTranscriptBuilder::compute_rows "compute rows". Namely, we are loosing the point at - * infinity contribution to the 'transcipt_Px' polynomials while parsing the raw ops of ECCOpQueue. + * infinity contribution to the 'transcipt_Px' polynomials while parsing the eccvm ops of ECCOpQueue. * * More specifically, in this test we add a simple MSM with the point at infinity multiplied by \f$0\f$. While the ECCVM * computes the result of this op correctly, i.e. outputs the point at infinity, the computation of 'transcript_Px' is @@ -474,7 +474,7 @@ TEST(ECCVMCircuitBuilderTests, InfinityFailure) bb::srs::init_grumpkin_crs_factory(bb::srs::get_grumpkin_crs_path()); // Add the same operations to the ECC op queue; the native computation is performed under the hood. - auto op_queue = std::make_shared<bb::ECCOpQueue>(); + std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>(); for (size_t i = 0; i < 1; i++) { op_queue->mul_accumulate(P1, Fr(0)); @@ -482,7 +482,7 @@ TEST(ECCVMCircuitBuilderTests, InfinityFailure) auto eccvm_builder = ECCVMCircuitBuilder(op_queue); - auto transcript_rows = ECCVMTranscriptBuilder::compute_rows(op_queue->get_raw_ops(), 1); + auto transcript_rows = ECCVMTranscriptBuilder::compute_rows(op_queue->get_eccvm_ops(), 1); // check that the corresponding op is mul bool row_op_code_correct = transcript_rows[1].opcode == 4; diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp index d8fc7ca34fe..d189d5efb53 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp @@ -533,7 +533,7 @@ class ECCVMFlavor { { // compute rows for the three different sections of the ECCVM execution trace const auto transcript_rows = - ECCVMTranscriptBuilder::compute_rows(builder.op_queue->get_raw_ops(), builder.get_number_of_muls()); + ECCVMTranscriptBuilder::compute_rows(builder.op_queue->get_eccvm_ops(), builder.get_number_of_muls()); const std::vector<MSM> msms = builder.get_msms(); const auto point_table_rows = ECCVMPointTablePrecomputationBuilder::compute_rows(CircuitBuilder::get_flattened_scalar_muls(msms)); @@ -1461,4 +1461,4 @@ class ECCVMFlavor { // NOLINTEND(cppcoreguidelines-avoid-const-or-ref-data-members) -} // namespace bb \ No newline at end of file +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp index 8eef6b58e48..3adb4631676 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp @@ -405,4 +405,4 @@ TEST_F(ECCVMTranscriptTests, StructureTest) prover.transcript->deserialize_full_transcript(); EXPECT_EQ(static_cast<typename Flavor::Commitment>(prover.transcript->transcript_Px_comm), one_group_val * rand_val); -} \ No newline at end of file +} diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index e7e89185d58..785e9fcc4c5 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -75,7 +75,7 @@ class GoblinProver { // commitments (https://github.com/AztecProtocol/barretenberg/issues/871) which would otherwise appear in the // first round of the merge protocol. To be removed once the issue has been resolved. commitment_key = bn254_commitment_key ? bn254_commitment_key : nullptr; - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue, commitment_key); + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); } /** * @brief Construct a MegaHonk proof and a merge proof for the present circuit. diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index b63aaa9091d..4964a6d7337 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -138,8 +138,7 @@ class GoblinMockCircuits { * * @param op_queue */ - static void perform_op_queue_interactions_for_mock_first_circuit( - std::shared_ptr<bb::ECCOpQueue>& op_queue, std::shared_ptr<CommitmentKey> commitment_key = nullptr) + static void perform_op_queue_interactions_for_mock_first_circuit(std::shared_ptr<bb::ECCOpQueue>& op_queue) { PROFILE_THIS(); @@ -147,23 +146,6 @@ class GoblinMockCircuits { // Add some goblinized ecc ops MockCircuits::construct_goblin_ecc_op_circuit(builder); - - op_queue->set_size_data(); - - // Manually compute the op queue transcript commitments (which would normally be done by the merge prover) -#ifndef __wasm__ - // TODO(Adam): This is crashing wasm-in-browser. It doesn't make sense to call this in browser... - bb::srs::init_crs_factory(bb::srs::get_ignition_crs_path()); -#endif - auto bn254_commitment_key = - commitment_key ? commitment_key : std::make_shared<CommitmentKey>(op_queue->get_current_size()); - std::array<Point, Flavor::NUM_WIRES> op_queue_commitments; - size_t idx = 0; - for (auto& entry : op_queue->get_aggregate_transcript()) { - op_queue_commitments[idx++] = bn254_commitment_key->commit({ 0, entry }); - } - // Store the commitment data for use by the prover of the next circuit - op_queue->set_commitment_data(op_queue_commitments); } /** diff --git a/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/mega_execution_trace.hpp b/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/mega_execution_trace.hpp index 42b0eb09ec0..cac4d6cc09b 100644 --- a/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/mega_execution_trace.hpp +++ b/barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/mega_execution_trace.hpp @@ -294,7 +294,7 @@ static constexpr TraceStructure CLIENT_IVC_BENCH_STRUCTURE{ .ecc_op = 1 << 10, .elliptic = 9000, .aux = 136000, .poseidon2_external = 2500, - .poseidon2_internal = 14000, + .poseidon2_internal = 14500, .overflow = 0 }; /** diff --git a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp index 76ec71551ad..462ea1bd4d2 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp @@ -199,4 +199,4 @@ TYPED_TEST(ECCVMRecursiveTests, IndependentVKHash) { TestFixture::test_independent_vk_hash(); }; -} // namespace bb \ No newline at end of file +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_recursive_verifier.cpp b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_recursive_verifier.cpp index 6f3ea52555c..41fa0356b54 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_recursive_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_recursive_verifier.cpp @@ -8,9 +8,17 @@ MergeRecursiveVerifier_<CircuitBuilder>::MergeRecursiveVerifier_(CircuitBuilder* {} /** - * @brief Construct recursive verifier for Goblin Merge protocol, up to but not including the pairing + * @brief Computes inputs to a pairing check that, if verified, establishes proper construction of the aggregate Goblin + * ECC op queue polynomials T_j, j = 1,2,3,4. + * @details Let T_j be the jth column of the aggregate ecc op table after prepending the subtable columns t_j containing + * the contribution from a single circuit. T_{j,prev} corresponds to the columns of the aggregate table at the + * previous stage. For each column we have the relationship T_j = t_j + right_shift(T_{j,prev}, k), where k is the + * length of the subtable columns t_j. This protocol demonstrates, assuming the length of t is at most k, that the + * aggregate ecc op table has been constructed correctly via the simple Schwartz-Zippel check: * - * @tparam Flavor + * T_j(\kappa) = t_j(\kappa) + \kappa^k * (T_{j,prev}(\kappa)). + * + * @tparam CircuitBuilder * @param proof * @return std::array<typename Flavor::GroupElement, 2> Inputs to final pairing */ @@ -18,49 +26,52 @@ template <typename CircuitBuilder> std::array<typename bn254<CircuitBuilder>::Element, 2> MergeRecursiveVerifier_<CircuitBuilder>::verify_proof( const HonkProof& proof) { - // transform it into stdlib proof + // Transform proof into a stdlib object StdlibProof<CircuitBuilder> stdlib_proof = bb::convert_native_proof_to_stdlib(builder, proof); transcript = std::make_shared<Transcript>(stdlib_proof); - // Receive commitments [t_i^{shift}], [T_{i-1}], and [T_i] - std::array<Commitment, NUM_WIRES> C_T_prev; - std::array<Commitment, NUM_WIRES> C_t_shift; - std::array<Commitment, NUM_WIRES> C_T_current; + FF subtable_size = transcript->template receive_from_prover<FF>("subtable_size"); + + // Receive table column polynomial commitments [t_j], [T_{j,prev}], and [T_j], j = 1,2,3,4 + std::array<Commitment, NUM_WIRES> t_commitments; + std::array<Commitment, NUM_WIRES> T_prev_commitments; + std::array<Commitment, NUM_WIRES> T_commitments; for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - C_T_prev[idx] = transcript->template receive_from_prover<Commitment>("T_PREV_" + std::to_string(idx + 1)); - C_t_shift[idx] = transcript->template receive_from_prover<Commitment>("t_SHIFT_" + std::to_string(idx + 1)); - C_T_current[idx] = transcript->template receive_from_prover<Commitment>("T_CURRENT_" + std::to_string(idx + 1)); + std::string suffix = std::to_string(idx); + t_commitments[idx] = transcript->template receive_from_prover<Commitment>("t_CURRENT_" + suffix); + T_prev_commitments[idx] = transcript->template receive_from_prover<Commitment>("T_PREV_" + suffix); + T_commitments[idx] = transcript->template receive_from_prover<Commitment>("T_CURRENT_" + suffix); } FF kappa = transcript->template get_challenge<FF>("kappa"); - // Receive transcript poly evaluations and add corresponding univariate opening claims {(\kappa, p(\kappa), [p(X)]} + // Receive evaluations t_j(\kappa), T_{j,prev}(\kappa), T_j(\kappa), j = 1,2,3,4 + std::array<FF, NUM_WIRES> t_evals; std::array<FF, NUM_WIRES> T_prev_evals; - std::array<FF, NUM_WIRES> t_shift_evals; - std::array<FF, NUM_WIRES> T_current_evals; + std::array<FF, NUM_WIRES> T_evals; std::vector<OpeningClaim> opening_claims; for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - T_prev_evals[idx] = transcript->template receive_from_prover<FF>("T_prev_eval_" + std::to_string(idx + 1)); - opening_claims.emplace_back(OpeningClaim{ { kappa, T_prev_evals[idx] }, C_T_prev[idx] }); + t_evals[idx] = transcript->template receive_from_prover<FF>("t_eval_" + std::to_string(idx + 1)); + opening_claims.emplace_back(OpeningClaim{ { kappa, t_evals[idx] }, t_commitments[idx] }); } for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - t_shift_evals[idx] = transcript->template receive_from_prover<FF>("t_shift_eval_" + std::to_string(idx + 1)); - opening_claims.emplace_back(OpeningClaim{ { kappa, t_shift_evals[idx] }, C_t_shift[idx] }); + T_prev_evals[idx] = transcript->template receive_from_prover<FF>("T_prev_eval_" + std::to_string(idx + 1)); + opening_claims.emplace_back(OpeningClaim{ { kappa, T_prev_evals[idx] }, T_prev_commitments[idx] }); } for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - T_current_evals[idx] = - transcript->template receive_from_prover<FF>("T_current_eval_" + std::to_string(idx + 1)); - opening_claims.emplace_back(OpeningClaim{ { kappa, T_current_evals[idx] }, C_T_current[idx] }); + T_evals[idx] = transcript->template receive_from_prover<FF>("T_eval_" + std::to_string(idx + 1)); + opening_claims.emplace_back(OpeningClaim{ { kappa, T_evals[idx] }, T_commitments[idx] }); } - // Check the identity T_i(\kappa) = T_{i-1}(\kappa) + t_i^{shift}(\kappa) + // Check the identity T_j(\kappa) = t_j(\kappa) + \kappa^m * T_{j,prev}(\kappa) for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - T_current_evals[idx].assert_equal(T_prev_evals[idx] + t_shift_evals[idx]); + FF T_prev_shifted_eval_reconstructed = T_prev_evals[idx] * kappa.pow(subtable_size); + T_evals[idx].assert_equal(t_evals[idx] + T_prev_shifted_eval_reconstructed); } FF alpha = transcript->template get_challenge<FF>("alpha"); - // Constuct batched commitment and batched evaluation from constituents using batching challenge \alpha + // Constuct inputs to batched commitment and batched evaluation from constituents using batching challenge \alpha std::vector<FF> scalars; std::vector<Commitment> commitments; scalars.emplace_back(FF(builder, 1)); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_verifier.test.cpp index a7887a9c96e..7f3f302ad44 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/merge_verifier.test.cpp @@ -99,4 +99,4 @@ TYPED_TEST(RecursiveMergeVerifierTest, SingleRecursiveVerification) TestFixture::test_recursive_merge_verification(); }; -} // namespace bb::stdlib::recursion::goblin \ No newline at end of file +} // namespace bb::stdlib::recursion::goblin diff --git a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.test.cpp index abe273f388c..6e6780293d9 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/translator_vm_verifier/translator_recursive_verifier.test.cpp @@ -199,4 +199,4 @@ TYPED_TEST(TranslatorRecursiveTests, IndependentVKHash) GTEST_SKIP() << "Not built for this parameter"; } }; -} // namespace bb \ No newline at end of file +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp index d796755d46a..ebe2588bf2e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp @@ -50,10 +50,13 @@ template <typename FF> class MegaCircuitBuilder_ : public UltraCircuitBuilder_<M , op_queue(std::move(op_queue_in)) { PROFILE_THIS(); + // Instantiate the subtable to be populated with goblin ecc ops from this circuit + op_queue->initialize_new_subtable(); // Set indices to constants corresponding to Goblin ECC op codes set_goblin_ecc_op_code_constant_variables(); }; + MegaCircuitBuilder_(std::shared_ptr<ECCOpQueue> op_queue_in) : MegaCircuitBuilder_(0, op_queue_in) {} @@ -79,6 +82,9 @@ template <typename FF> class MegaCircuitBuilder_ : public UltraCircuitBuilder_<M : UltraCircuitBuilder_<MegaExecutionTraceBlocks>(/*size_hint=*/0, witness_values, public_inputs, varnum) , op_queue(std::move(op_queue_in)) { + // Instantiate the subtable to be populated with goblin ecc ops from this circuit + op_queue->initialize_new_subtable(); + // Set indices to constants corresponding to Goblin ECC op codes set_goblin_ecc_op_code_constant_variables(); }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp index 33b462a6860..2f2634c180b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp @@ -2,38 +2,27 @@ #include "barretenberg/ecc/curves/bn254/bn254.hpp" #include "barretenberg/eccvm/eccvm_builder_types.hpp" +#include "barretenberg/polynomials/polynomial.hpp" #include "barretenberg/stdlib/primitives/bigfield/constants.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp" #include "barretenberg/stdlib_circuit_builders/op_queue/eccvm_row_tracker.hpp" namespace bb { -enum EccOpCode { NULL_OP, ADD_ACCUM, MUL_ACCUM, EQUALITY }; - -struct UltraOp { - using Fr = curve::BN254::ScalarField; - EccOpCode op_code = NULL_OP; - Fr op; - Fr x_lo; - Fr x_hi; - Fr y_lo; - Fr y_hi; - Fr z_1; - Fr z_2; - bool return_is_infinity; -}; - /** * @brief Used to construct execution trace representations of elliptic curve operations. - * - * @details Currently the targets in execution traces are: four advice wires in UltraCircuitBuilder and 5 wires in the - * ECCVM. In each case, the variable values are stored in this class, since the same values will need to be used later - * by the TranslationVMCircuitBuilder. The circuit builders will store witness indices which are indices in the - * ultra (resp. eccvm) ops members of this class (rather than in the builder's variables array). + * @details Constructs and stores tables of ECC operations in two formats: the ECCVM format and the + * Ultra-arithmetization (width-4) format. The ECCVM format is used to construct the execution trace for the ECCVM + * circuit, while the Ultra-arithmetization is used in the Mega circuits and the Translator VM. Both tables are + * constructed via successive pre-pending of subtables of the same format, where each subtable represents the operations + * of a single circuit. + * TODO(https://github.com/AztecProtocol/barretenberg/issues/1267): consider possible efficiency improvements */ class ECCOpQueue { using Curve = curve::BN254; using Point = Curve::AffineElement; using Fr = Curve::ScalarField; using Fq = Curve::BaseField; // Grumpkin's scalar field + static constexpr size_t ULTRA_TABLE_WIDTH = UltraEccOpsTable::TABLE_WIDTH; Point point_at_infinity = Curve::Group::affine_point_at_infinity; // The operations written to the queue are also performed natively; the result is stored in accumulator @@ -41,21 +30,61 @@ class ECCOpQueue { static constexpr size_t DEFAULT_NON_NATIVE_FIELD_LIMB_BITS = stdlib::NUM_LIMB_BITS_IN_FIELD_SIMULATION; - std::vector<bb::eccvm::VMOperation<Curve::Group>> raw_ops; - std::array<std::vector<Fr>, 4> ultra_ops; // ops encoded in the width-4 Ultra format + EccvmOpsTable eccvm_ops_table; // table of ops in the ECCVM format + UltraEccOpsTable ultra_ops_table; // table of ops in the Ultra-arithmetization format - size_t current_ultra_ops_size = 0; // M_i - size_t previous_ultra_ops_size = 0; // M_{i-1} + // Storage for the reconstructed eccvm ops table in contiguous memory. (Intended to be constructed once and for all + // prior to ECCVM construction to avoid repeated prepending of subtables in physical memory). + std::vector<bb::eccvm::VMOperation<Curve::Group>> eccvm_ops_reconstructed; - std::array<Point, 4> ultra_ops_commitments; - - // Tracks numer of muls and size of eccvm in real time as the op queue is updated + // Tracks number of muls and size of eccvm in real time as the op queue is updated EccvmRowTracker eccvm_row_tracker; public: using ECCVMOperation = bb::eccvm::VMOperation<Curve::Group>; - const std::vector<ECCVMOperation>& get_raw_ops() { return raw_ops; } + // Constructor that instantiates an initial ECC op subtable + ECCOpQueue() { initialize_new_subtable(); } + + // Initialize a new subtable of ECCVM ops and Ultra ops corresponding to an individual circuit + void initialize_new_subtable() + { + eccvm_ops_table.create_new_subtable(); + ultra_ops_table.create_new_subtable(); + } + + // Construct polynomials corresponding to the columns of the full aggregate ultra ecc ops table + std::array<Polynomial<Fr>, ULTRA_TABLE_WIDTH> construct_ultra_ops_table_columns() const + { + return ultra_ops_table.construct_table_columns(); + } + + // Construct polys corresponding to the columns of the aggregate ultra ops table, excluding the most recent subtable + std::array<Polynomial<Fr>, ULTRA_TABLE_WIDTH> construct_previous_ultra_ops_table_columns() const + { + return ultra_ops_table.construct_previous_table_columns(); + } + + // Construct polynomials corresponding to the columns of the current subtable of ultra ecc ops + std::array<Polynomial<Fr>, ULTRA_TABLE_WIDTH> construct_current_ultra_ops_subtable_columns() const + { + return ultra_ops_table.construct_current_ultra_ops_subtable_columns(); + } + + // Reconstruct the full table of eccvm ops in contiguous memory from the independent subtables + void construct_full_eccvm_ops_table() { eccvm_ops_reconstructed = eccvm_ops_table.get_reconstructed(); } + + size_t get_ultra_ops_table_num_rows() const { return ultra_ops_table.ultra_table_size(); } + size_t get_current_ultra_ops_subtable_num_rows() const { return ultra_ops_table.current_ultra_subtable_size(); } + + // Get the full table of ECCVM ops in contiguous memory; construct it if it has not been constructed already + std::vector<ECCVMOperation>& get_eccvm_ops() + { + if (eccvm_ops_reconstructed.empty()) { + construct_full_eccvm_ops_table(); + } + return eccvm_ops_reconstructed; + } /** * @brief Get the number of rows in the 'msm' column section, for all msms in the circuit @@ -86,19 +115,22 @@ class ECCOpQueue { } /** - * @brief A fuzzing only method for setting raw ops directly + * @brief A fuzzing only method for setting eccvm ops directly * */ - void set_raw_ops_for_fuzzing(std::vector<ECCVMOperation>& raw_ops_in) { raw_ops = raw_ops_in; } + void set_eccvm_ops_for_fuzzing(std::vector<ECCVMOperation>& eccvm_ops_in) + { + eccvm_ops_reconstructed = eccvm_ops_in; + } /** - * @brief A testing only method that adds an erroneous equality op to the raw ops + * @brief A testing only method that adds an erroneous equality op to the eccvm ops * @brief May be used to ensure that ECCVM responds as expected when encountering a bad op * */ void add_erroneous_equality_op_for_testing() { - raw_ops.emplace_back(ECCVMOperation{ .eq = true, .reset = true, .base_point = Point::random_element() }); + append_eccvm_op(ECCVMOperation{ .eq = true, .reset = true, .base_point = Point::random_element() }); } /** @@ -106,64 +138,10 @@ class ECCOpQueue { * @warning This is for testing purposes only. Currently no valid use case. * */ - void empty_row_for_testing() - { - raw_ops.emplace_back(ECCVMOperation{ .base_point = point_at_infinity }); - - eccvm_row_tracker.update_cached_msms(raw_ops.back()); - } + void empty_row_for_testing() { append_eccvm_op(ECCVMOperation{ .base_point = point_at_infinity }); } Point get_accumulator() { return accumulator; } - /** - * @brief Set the current and previous size of the ultra_ops transcript - * - * @details previous_ultra_ops_size = M_{i-1} is needed by the prover to extract the previous aggregate op - * queue transcript T_{i-1} from the current one T_i. This method should be called when a circuit is 'finalized'. - */ - void set_size_data() - { - previous_ultra_ops_size = current_ultra_ops_size; - current_ultra_ops_size = ultra_ops[0].size(); - } - - [[nodiscard]] size_t get_previous_size() const { return previous_ultra_ops_size; } - [[nodiscard]] size_t get_current_size() const { return current_ultra_ops_size; } - - void set_commitment_data(std::array<Point, 4>& commitments) { ultra_ops_commitments = commitments; } - const auto& get_ultra_ops_commitments() { return ultra_ops_commitments; } - - /** - * @brief Get a 'view' of the current ultra ops object - * - * @return std::vector<std::span<Fr>> - */ - std::vector<std::span<Fr>> get_aggregate_transcript() - { - std::vector<std::span<Fr>> result; - result.reserve(ultra_ops.size()); - for (auto& entry : ultra_ops) { - result.emplace_back(entry); - } - return result; - } - - /** - * @brief Get a 'view' of the previous ultra ops object - * - * @return std::vector<std::span<Fr>> - */ - std::vector<std::span<Fr>> get_previous_aggregate_transcript() - { - std::vector<std::span<Fr>> result; - result.reserve(ultra_ops.size()); - // Construct T_{i-1} as a view of size M_{i-1} into T_i - for (auto& entry : ultra_ops) { - result.emplace_back(entry.begin(), previous_ultra_ops_size); - } - return result; - } - /** * @brief Write point addition op to queue and natively perform addition * @@ -174,9 +152,8 @@ class ECCOpQueue { // Update the accumulator natively accumulator = accumulator + to_add; - // Store the raw operation - raw_ops.emplace_back(ECCVMOperation{ .add = true, .base_point = to_add }); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); + // Store the eccvm operation + append_eccvm_op(ECCVMOperation{ .add = true, .base_point = to_add }); // Construct and store the operation in the ultra op format return construct_and_populate_ultra_ops(ADD_ACCUM, to_add); @@ -195,15 +172,14 @@ class ECCOpQueue { // Construct and store the operation in the ultra op format UltraOp ultra_op = construct_and_populate_ultra_ops(MUL_ACCUM, to_mul, scalar); - // Store the raw operation - raw_ops.emplace_back(ECCVMOperation{ + // Store the eccvm operation + append_eccvm_op(ECCVMOperation{ .mul = true, .base_point = to_mul, .z1 = ultra_op.z_1, .z2 = ultra_op.z_2, .mul_scalar_full = scalar, }); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); return ultra_op; } @@ -214,9 +190,8 @@ class ECCOpQueue { */ UltraOp no_op() { - // Store raw operation - raw_ops.emplace_back(ECCVMOperation{}); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); + // Store eccvm operation + append_eccvm_op(ECCVMOperation{}); // Construct and store the operation in the ultra op format return construct_and_populate_ultra_ops(NULL_OP, accumulator); @@ -232,15 +207,23 @@ class ECCOpQueue { auto expected = accumulator; accumulator.self_set_infinity(); - // Store raw operation - raw_ops.emplace_back(ECCVMOperation{ .eq = true, .reset = true, .base_point = expected }); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); + // Store eccvm operation + append_eccvm_op(ECCVMOperation{ .eq = true, .reset = true, .base_point = expected }); // Construct and store the operation in the ultra op format return construct_and_populate_ultra_ops(EQUALITY, expected); } private: + /** + * @brief Append an eccvm operation to the eccvm ops table; update the eccvm row tracker + * + */ + void append_eccvm_op(const ECCVMOperation& op) + { + eccvm_row_tracker.update_cached_msms(op); + eccvm_ops_table.push(op); + } /** * @brief Given an ecc operation and its inputs, decompose into ultra format and populate ultra_ops * @@ -286,29 +269,10 @@ class ECCOpQueue { ultra_op.z_2 = z_2.to_montgomery_form(); } - append_to_ultra_ops(ultra_op); + ultra_ops_table.push(ultra_op); return ultra_op; } - - /** - * @brief Populate two rows of the ultra ops,representing a complete ECC operation - * @note Only the first 'op' field is utilized so the second is explicitly set to 0 - * - * @param tuple - */ - void append_to_ultra_ops(UltraOp tuple) - { - ultra_ops[0].emplace_back(tuple.op); - ultra_ops[1].emplace_back(tuple.x_lo); - ultra_ops[2].emplace_back(tuple.x_hi); - ultra_ops[3].emplace_back(tuple.y_lo); - - ultra_ops[0].emplace_back(0); - ultra_ops[1].emplace_back(tuple.y_hi); - ultra_ops[2].emplace_back(tuple.z_1); - ultra_ops[3].emplace_back(tuple.z_2); - } }; } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index a0d2596da28..213713b6773 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -3,25 +3,47 @@ using namespace bb; +class ECCOpQueueTest { + public: + using Curve = curve::BN254; + using G1 = Curve::AffineElement; + using Fr = Curve::ScalarField; + + // Perform some basic interactions with the ECC op queue to mock the behavior of a single circuit + static void populate_an_arbitrary_subtable_of_ops(const std::shared_ptr<bb::ECCOpQueue>& op_queue) + { + auto P1 = G1::random_element(); + auto P2 = G1::random_element(); + auto z = Fr::random_element(); + + op_queue->initialize_new_subtable(); + op_queue->add_accumulate(P1); + op_queue->mul_accumulate(P2, z); + op_queue->eq_and_reset(); + } +}; + TEST(ECCOpQueueTest, Basic) { + using G1 = ECCOpQueueTest::G1; + ECCOpQueue op_queue; - const auto& raw_ops = op_queue.get_raw_ops(); op_queue.add_accumulate(bb::g1::affine_one); - EXPECT_EQ(raw_ops[0].base_point, bb::g1::affine_one); op_queue.empty_row_for_testing(); - EXPECT_EQ(raw_ops[1].add, false); + const auto& eccvm_ops = op_queue.get_eccvm_ops(); + EXPECT_EQ(eccvm_ops[0].base_point, G1::one()); + EXPECT_EQ(eccvm_ops[1].add, false); } TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) { - using point = g1::affine_element; - using scalar = fr; + using G1 = ECCOpQueueTest::G1; + using Fr = ECCOpQueueTest::Fr; // Compute a simple point accumulation natively - auto P1 = point::random_element(); - auto P2 = point::random_element(); - auto z = scalar::random_element(); + auto P1 = G1::random_element(); + auto P2 = G1::random_element(); + auto z = Fr::random_element(); auto P_expected = P1 + P2 * z; // Add the same operations to the ECC op queue; the native computation is performed under the hood. @@ -36,3 +58,43 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) op_queue.eq_and_reset(); EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } + +// Check that the ECC op queue correctly constructs the table column polynomials for the full table, the previous table, +// and the current subtable via successive prepending of subtables +TEST(ECCOpQueueTest, ColumnPolynomialConstruction) +{ + using Fr = fr; + + // Instantiate an EccOpQueue and populate it with several subtables of ECC ops + auto op_queue = std::make_shared<bb::ECCOpQueue>(); + + // Lambda for checking that the table column polynomials reconstructed by the op queue have the correct relationship + auto check_table_column_polynomials = [&](const std::shared_ptr<bb::ECCOpQueue>& op_queue) { + // Construct column polynomials corresponding to the full table (T), the previous table (T_prev), and the + // current subtable (t_current) + auto table_polynomials = op_queue->construct_ultra_ops_table_columns(); + auto prev_table_polynomials = op_queue->construct_previous_ultra_ops_table_columns(); + auto subtable_polynomials = op_queue->construct_current_ultra_ops_subtable_columns(); + // Check that the table polynomials are constructed correctly by checking the following identity at a single + // point: T(x) = t_current(x) + x^k * T_prev(x), where k is the size of the current subtable + const size_t current_subtable_size = op_queue->get_current_ultra_ops_subtable_num_rows(); + + // Check T(x) = t_current(x) + x^k * T_prev(x) at a single random challenge point + Fr eval_challenge = Fr::random_element(); + for (auto [table_poly, prev_table_poly, subtable_poly] : + zip_view(table_polynomials, prev_table_polynomials, subtable_polynomials)) { + Fr table_eval = table_poly.evaluate(eval_challenge); // T(x) + Fr subtable_eval = subtable_poly.evaluate(eval_challenge); // t_current(x) + Fr shifted_previous_table_eval = + prev_table_poly.evaluate(eval_challenge) * eval_challenge.pow(current_subtable_size); // x^k * T_prev(x) + EXPECT_EQ(table_eval, subtable_eval + shifted_previous_table_eval); + } + }; + + // Check that the table polynomials have the correct form after each subtable concatenation + const size_t NUM_SUBTABLES = 5; + for (size_t i = 0; i < NUM_SUBTABLES; ++i) { + ECCOpQueueTest::populate_an_arbitrary_subtable_of_ops(op_queue); + check_table_column_polynomials(op_queue); + } +} diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp index 890e117a478..f8db7c38146 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp @@ -2,10 +2,25 @@ #include "barretenberg/ecc/curves/bn254/bn254.hpp" #include "barretenberg/eccvm/eccvm_builder_types.hpp" -#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" +#include "barretenberg/polynomials/polynomial.hpp" #include <deque> namespace bb { +enum EccOpCode { NULL_OP, ADD_ACCUM, MUL_ACCUM, EQUALITY }; + +struct UltraOp { + using Fr = curve::BN254::ScalarField; + EccOpCode op_code = NULL_OP; + Fr op; + Fr x_lo; + Fr x_hi; + Fr y_lo; + Fr y_hi; + Fr z_1; + Fr z_2; + bool return_is_infinity; +}; + /** * @brief A table of ECC operations * @details The table is constructed via concatenation of subtables of ECC operations. The table concatentation protocol @@ -29,13 +44,19 @@ template <typename OpFormat> class EccOpsTable { return total; } + size_t num_subtables() const { return table.size(); } + auto& get() const { return table; } void push(const OpFormat& op) { table.front().push_back(op); } void create_new_subtable(size_t size_hint = 0) { - std::vector<OpFormat> new_subtable; + // If there is a single subtable and it is empty, dont create a new one + if (table.size() == 1 && table.front().empty()) { + return; + } + Subtable new_subtable; new_subtable.reserve(size_hint); table.insert(table.begin(), std::move(new_subtable)); } @@ -53,9 +74,22 @@ template <typename OpFormat> class EccOpsTable { } return table.front().front(); // should never reach here } + + // highly inefficient copy-based reconstruction of the table for use in ECCVM/Translator + std::vector<OpFormat> get_reconstructed() const + { + std::vector<OpFormat> reconstructed_table; + reconstructed_table.reserve(size()); + for (const auto& subtable : table) { + for (const auto& op : subtable) { + reconstructed_table.push_back(op); + } + } + return reconstructed_table; + } }; -using RawEccOpsTable = EccOpsTable<eccvm::VMOperation<curve::BN254::Group>>; +using EccvmOpsTable = EccOpsTable<eccvm::VMOperation<curve::BN254::Group>>; /** * @brief Stores a table of elliptic curve operations represented in the Ultra format @@ -71,40 +105,90 @@ using RawEccOpsTable = EccOpsTable<eccvm::VMOperation<curve::BN254::Group>>; * polynomials in the proving system. */ class UltraEccOpsTable { + public: + static constexpr size_t TABLE_WIDTH = 4; // dictated by the number of wires in the Ultra arithmetization + static constexpr size_t NUM_ROWS_PER_OP = 2; // A single ECC op is split across two width-4 rows + + private: using Curve = curve::BN254; using Fr = Curve::ScalarField; using UltraOpsTable = EccOpsTable<UltraOp>; + using TableView = std::array<std::span<Fr>, TABLE_WIDTH>; + using ColumnPolynomials = std::array<Polynomial<Fr>, TABLE_WIDTH>; UltraOpsTable table; - static constexpr size_t TABLE_WIDTH = 4; public: size_t size() const { return table.size(); } + size_t ultra_table_size() const { return table.size() * NUM_ROWS_PER_OP; } + size_t current_ultra_subtable_size() const { return table.get()[0].size() * NUM_ROWS_PER_OP; } + size_t previous_ultra_table_size() const { return (ultra_table_size() - current_ultra_subtable_size()); } void create_new_subtable(size_t size_hint = 0) { table.create_new_subtable(size_hint); } void push(const UltraOp& op) { table.push(op); } + // Construct the columns of the full ultra ecc ops table + ColumnPolynomials construct_table_columns() const + { + const size_t poly_size = ultra_table_size(); + const size_t subtable_start_idx = 0; // include all subtables + const size_t subtable_end_idx = table.num_subtables(); + + return construct_column_polynomials_from_subtables(poly_size, subtable_start_idx, subtable_end_idx); + } + + // Construct the columns of the previous full ultra ecc ops table + ColumnPolynomials construct_previous_table_columns() const + { + const size_t poly_size = previous_ultra_table_size(); + const size_t subtable_start_idx = 1; // exclude the 0th subtable + const size_t subtable_end_idx = table.num_subtables(); + + return construct_column_polynomials_from_subtables(poly_size, subtable_start_idx, subtable_end_idx); + } + + // Construct the columns of the current ultra ecc ops subtable + ColumnPolynomials construct_current_ultra_ops_subtable_columns() const + { + const size_t poly_size = current_ultra_subtable_size(); + const size_t subtable_start_idx = 0; + const size_t subtable_end_idx = 1; // include only the 0th subtable + + return construct_column_polynomials_from_subtables(poly_size, subtable_start_idx, subtable_end_idx); + } + + private: /** - * @brief Populate the provided array of columns with the width-4 representation of the table data - * @todo multithreaded this functionality + * @brief Construct polynomials corresponding to the columns of the reconstructed ultra ops table for the given + * range of subtables + * TODO(https://github.com/AztecProtocol/barretenberg/issues/1267): multithread this functionality * @param target_columns */ - void populate_column_data(std::array<std::span<Fr>, TABLE_WIDTH>& target_columns) + ColumnPolynomials construct_column_polynomials_from_subtables(const size_t poly_size, + const size_t subtable_start_idx, + const size_t subtable_end_idx) const { + ColumnPolynomials column_polynomials; + for (auto& poly : column_polynomials) { + poly = Polynomial<Fr>(poly_size); + } + size_t i = 0; - for (const auto& subtable : table.get()) { + for (size_t subtable_idx = subtable_start_idx; subtable_idx < subtable_end_idx; ++subtable_idx) { + const auto& subtable = table.get()[subtable_idx]; for (const auto& op : subtable) { - target_columns[0][i] = op.op; - target_columns[1][i] = op.x_lo; - target_columns[2][i] = op.x_hi; - target_columns[3][i] = op.y_lo; + column_polynomials[0].at(i) = op.op; + column_polynomials[1].at(i) = op.x_lo; + column_polynomials[2].at(i) = op.x_hi; + column_polynomials[3].at(i) = op.y_lo; i++; - target_columns[0][i] = 0; // only the first 'op' field is utilized - target_columns[1][i] = op.y_hi; - target_columns[2][i] = op.z_1; - target_columns[3][i] = op.z_2; + column_polynomials[0].at(i) = 0; // only the first 'op' field is utilized + column_polynomials[1].at(i) = op.y_hi; + column_polynomials[2].at(i) = op.z_1; + column_polynomials[3].at(i) = op.z_2; i++; } } + return column_polynomials; } }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp index b8ccfe4c77b..748d224500a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp @@ -43,16 +43,15 @@ class EccOpsTableTest : public ::testing::Test { size_t size() const { return columns[0].size(); } }; - // Mock raw ops table that constructs a concatenated table from successively prepended subtables - struct MockRawOpsTable { - std::vector<ECCVMOperation> raw_ops; - void append(const ECCVMOperation& op) { raw_ops.push_back(op); } + // Mock eccvm ops table that constructs a concatenated table from successively prepended subtables + struct MockEccvmOpsTable { + std::vector<ECCVMOperation> eccvm_ops; - MockRawOpsTable(const auto& subtable_ops) + MockEccvmOpsTable(const auto& subtable_ops) { for (auto& ops : std::ranges::reverse_view(subtable_ops)) { for (const auto& op : ops) { - append(op); + eccvm_ops.push_back(op); } } } @@ -73,7 +72,7 @@ class EccOpsTableTest : public ::testing::Test { return op; } - static ECCVMOperation random_raw_op() + static ECCVMOperation random_eccvm_op() { return ECCVMOperation{ .mul = true, .base_point = curve::BN254::Group::affine_element::random_element(), @@ -88,8 +87,6 @@ TEST(EccOpsTableTest, UltraOpsTable) { using Fr = fr; - constexpr size_t NUM_ROWS_PER_OP = 2; // Each ECC op is represented by two rows in the ultra ops table - // Construct sets of ultra ops, each representing those added by a single circuit const size_t NUM_SUBTABLES = 3; std::array<std::vector<UltraOp>, NUM_SUBTABLES> subtable_ultra_ops; @@ -117,14 +114,7 @@ TEST(EccOpsTableTest, UltraOpsTable) EXPECT_EQ(ultra_ops_table.size(), expected_num_ops); // Construct polynomials corresponding to the columns of the ultra ops table - const size_t expected_table_size = expected_num_ops * NUM_ROWS_PER_OP; - std::array<Polynomial<Fr>, 4> ultra_ops_table_polynomials; - std::array<std::span<fr>, 4> column_spans; - for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { - column = Polynomial<Fr>(expected_table_size); - column_span = column.coeffs(); - } - ultra_ops_table.populate_column_data(column_spans); + std::array<Polynomial<Fr>, 4> ultra_ops_table_polynomials = ultra_ops_table.construct_table_columns(); // Check that the ultra ops table constructed by the op queue matches the expected table for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { @@ -134,39 +124,42 @@ TEST(EccOpsTableTest, UltraOpsTable) } } -// Ensure RawOpsTable correctly constructs a concatenated table from successively prepended subtables -TEST(EccOpsTableTest, RawOpsTable) +// Ensure EccvmOpsTable correctly constructs a concatenated table from successively prepended subtables +TEST(EccOpsTableTest, EccvmOpsTable) { using ECCVMOperation = bb::eccvm::VMOperation<curve::BN254::Group>; - // Construct sets of raw ops, each representing those added by a single circuit + // Construct sets of eccvm ops, each representing those added by a single circuit const size_t NUM_SUBTABLES = 3; - std::array<std::vector<ECCVMOperation>, NUM_SUBTABLES> subtable_raw_ops; + std::array<std::vector<ECCVMOperation>, NUM_SUBTABLES> subtable_eccvm_ops; std::array<size_t, NUM_SUBTABLES> subtable_op_counts = { 4, 2, 7 }; - for (auto [subtable_ops, op_count] : zip_view(subtable_raw_ops, subtable_op_counts)) { + for (auto [subtable_ops, op_count] : zip_view(subtable_eccvm_ops, subtable_op_counts)) { for (size_t i = 0; i < op_count; ++i) { - subtable_ops.push_back(EccOpsTableTest::random_raw_op()); + subtable_ops.push_back(EccOpsTableTest::random_eccvm_op()); } } - // Construct the mock raw ops table which contains the subtables ordered in reverse (as if prepended) - EccOpsTableTest::MockRawOpsTable expected_raw_ops_table(subtable_raw_ops); + // Construct the mock eccvm ops table which contains the subtables ordered in reverse (as if prepended) + EccOpsTableTest::MockEccvmOpsTable expected_eccvm_ops_table(subtable_eccvm_ops); - // Construct the concatenated raw ops table - RawEccOpsTable raw_ops_table; - for (const auto& subtable_ops : subtable_raw_ops) { - raw_ops_table.create_new_subtable(); + // Construct the concatenated eccvm ops table + EccvmOpsTable eccvm_ops_table; + for (const auto& subtable_ops : subtable_eccvm_ops) { + eccvm_ops_table.create_new_subtable(); for (const auto& op : subtable_ops) { - raw_ops_table.push(op); + eccvm_ops_table.push(op); } } // Check that the table has the correct size auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); - EXPECT_EQ(raw_ops_table.size(), expected_num_ops); + EXPECT_EQ(eccvm_ops_table.size(), expected_num_ops); - // Check that the table matches the manually constructed mock table + // Check that accessing the table values via operator[] matches the manually constructed mock table for (size_t i = 0; i < expected_num_ops; ++i) { - EXPECT_EQ(expected_raw_ops_table.raw_ops[i], raw_ops_table[i]); + EXPECT_EQ(expected_eccvm_ops_table.eccvm_ops[i], eccvm_ops_table[i]); } + + // Check that the copy-based reconstruction of the eccvm ops table matches the expected table + EXPECT_EQ(expected_eccvm_ops_table.eccvm_ops, eccvm_ops_table.get_reconstructed()); } diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/translator.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/translator_vm/translator.fuzzer.hpp index eb54ebb9988..a3d747a1032 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/translator.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/translator.fuzzer.hpp @@ -23,15 +23,15 @@ using G1 = curve::BN254::AffineElement; */ std::vector<ECCOpQueue::ECCVMOperation> parse_operations(const unsigned char* data, size_t size) { - std::vector<ECCOpQueue::ECCVMOperation> raw_ops; + std::vector<ECCOpQueue::ECCVMOperation> eccvm_ops; size_t size_left = size; // Just iterate and parse until there's no data left while (size_left >= sizeof(ECCOpQueue::ECCVMOperation)) { - raw_ops.emplace_back((ECCOpQueue::ECCVMOperation*)(data + (size - size_left))); + eccvm_ops.emplace_back((ECCOpQueue::ECCVMOperation*)(data + (size - size_left))); size_left -= sizeof(ECCOpQueue::ECCVMOperation); } - return raw_ops; + return eccvm_ops; } /** @@ -44,7 +44,7 @@ std::vector<ECCOpQueue::ECCVMOperation> parse_operations(const unsigned char* da std::optional<std::tuple<Fq, Fq, std::shared_ptr<ECCOpQueue>>> parse_and_construct_opqueue(const unsigned char* data, size_t size) { - std::vector<ECCOpQueue::ECCVMOperation> raw_ops; + std::vector<ECCOpQueue::ECCVMOperation> eccvm_ops; // Try to parse batching challenge size_t size_left = size; @@ -65,8 +65,8 @@ std::optional<std::tuple<Fq, Fq, std::shared_ptr<ECCOpQueue>>> parse_and_constru size_left -= sizeof(uint256_t); // Try to parse operations - raw_ops = parse_operations(data + (size - size_left), size_left); - if (raw_ops.empty()) { + eccvm_ops = parse_operations(data + (size - size_left), size_left); + if (eccvm_ops.empty()) { return {}; } @@ -76,7 +76,7 @@ std::optional<std::tuple<Fq, Fq, std::shared_ptr<ECCOpQueue>>> parse_and_constru auto padding_element = G1(p_x, p_y); auto padding_scalar = -Fr::one(); auto ecc_op_queue = std::make_shared<ECCOpQueue>(); - ecc_op_queue->set_raw_ops_for_fuzzing(raw_ops); + ecc_op_queue->set_eccvm_ops_for_fuzzing(eccvm_ops); ecc_op_queue->mul_accumulate(padding_element, padding_scalar); // Return the batching challenge, evaluation challenge and the constructed queue diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.cpp index 6ef93887319..05417477b9a 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.cpp @@ -588,13 +588,16 @@ TranslatorCircuitBuilder::AccumulationInput TranslatorCircuitBuilder::compute_wi batching_challenge_v, evaluation_input_x); } + +// TODO(https://github.com/AztecProtocol/barretenberg/issues/1266): Evaluate whether this method can reuse existing data +// in the op queue for improved efficiency void TranslatorCircuitBuilder::feed_ecc_op_queue_into_circuit(const std::shared_ptr<ECCOpQueue> ecc_op_queue) { using Fq = bb::fq; - const auto& raw_ops = ecc_op_queue->get_raw_ops(); + const auto& eccvm_ops = ecc_op_queue->get_eccvm_ops(); std::vector<Fq> accumulator_trace; Fq current_accumulator(0); - if (raw_ops.empty()) { + if (eccvm_ops.empty()) { return; } // Rename for ease of use @@ -603,8 +606,8 @@ void TranslatorCircuitBuilder::feed_ecc_op_queue_into_circuit(const std::shared_ // We need to precompute the accumulators at each step, because in the actual circuit we compute the values starting // from the later indices. We need to know the previous accumulator to create the gate - for (size_t i = 0; i < raw_ops.size(); i++) { - const auto& ecc_op = raw_ops[raw_ops.size() - 1 - i]; + for (size_t i = 0; i < eccvm_ops.size(); i++) { + const auto& ecc_op = eccvm_ops[eccvm_ops.size() - 1 - i]; current_accumulator *= x; const auto [x_256, y_256] = ecc_op.get_base_point_standard_form(); current_accumulator += @@ -615,7 +618,7 @@ void TranslatorCircuitBuilder::feed_ecc_op_queue_into_circuit(const std::shared_ // We don't care about the last value since we'll recompute it during witness generation anyway accumulator_trace.pop_back(); - for (const auto& raw_op : raw_ops) { + for (const auto& eccvm_op : eccvm_ops) { Fq previous_accumulator = 0; // Pop the last value from accumulator trace and use it as previous accumulator if (!accumulator_trace.empty()) { @@ -623,7 +626,7 @@ void TranslatorCircuitBuilder::feed_ecc_op_queue_into_circuit(const std::shared_ accumulator_trace.pop_back(); } // Compute witness values - auto one_accumulation_step = compute_witness_values_for_one_ecc_op(raw_op, previous_accumulator, v, x); + auto one_accumulation_step = compute_witness_values_for_one_ecc_op(eccvm_op, previous_accumulator, v, x); // And put them into the wires create_accumulation_gate(one_accumulation_step); @@ -1055,4 +1058,4 @@ bool TranslatorCircuitBuilder::check_circuit() } return true; }; -} // namespace bb \ No newline at end of file +} // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.fuzzer.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.fuzzer.cpp index aac38c9b9ca..52f5d4d46d7 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.fuzzer.cpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.fuzzer.cpp @@ -23,15 +23,15 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) auto z_1_accumulator = Fq(0); auto z_2_accumulator = Fq(0); // Compute the batched evaluation of polynomials (multiplying by inverse to go from lower to higher) - const auto& raw_ops = op_queue->get_raw_ops(); - for (const auto& ecc_op : raw_ops) { + const auto& eccvm_ops = op_queue->get_eccvm_ops(); + for (const auto& ecc_op : eccvm_ops) { op_accumulator = op_accumulator * x_inv + ecc_op.get_opcode_value(); p_x_accumulator = p_x_accumulator * x_inv + ecc_op.base_point.x; p_y_accumulator = p_y_accumulator * x_inv + ecc_op.base_point.y; z_1_accumulator = z_1_accumulator * x_inv + ecc_op.z1; z_2_accumulator = z_2_accumulator * x_inv + ecc_op.z2; } - Fq x_pow = x.pow(raw_ops.size() - 1); + Fq x_pow = x.pow(eccvm_ops.size() - 1); // Multiply by an appropriate power of x to get rid of the inverses Fq result = ((((z_2_accumulator * batching_challenge + z_1_accumulator) * batching_challenge + p_y_accumulator) * @@ -45,4 +45,4 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) circuit_builder.check_circuit(); (void)result; return 0; -} \ No newline at end of file +} diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.test.cpp index 95492d1acc0..e214a5fe567 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/translator_circuit_builder.test.cpp @@ -105,8 +105,8 @@ TEST(TranslatorCircuitBuilder, SeveralOperationCorrectness) // Get an inverse Fq x_inv = x.invert(); // Compute the batched evaluation of polynomials (multiplying by inverse to go from lower to higher) - const auto& raw_ops = op_queue->get_raw_ops(); - for (const auto& ecc_op : raw_ops) { + const auto& eccvm_ops = op_queue->get_eccvm_ops(); + for (const auto& ecc_op : eccvm_ops) { op_accumulator = op_accumulator * x_inv + ecc_op.get_opcode_value(); const auto [x_u256, y_u256] = ecc_op.get_base_point_standard_form(); p_x_accumulator = p_x_accumulator * x_inv + x_u256; @@ -114,7 +114,7 @@ TEST(TranslatorCircuitBuilder, SeveralOperationCorrectness) z_1_accumulator = z_1_accumulator * x_inv + ecc_op.z1; z_2_accumulator = z_2_accumulator * x_inv + ecc_op.z2; } - Fq x_pow = x.pow(raw_ops.size() - 1); + Fq x_pow = x.pow(eccvm_ops.size() - 1); // Multiply by an appropriate power of x to get rid of the inverses Fq result = ((((z_2_accumulator * batching_challenge + z_1_accumulator) * batching_challenge + p_y_accumulator) * @@ -130,4 +130,4 @@ TEST(TranslatorCircuitBuilder, SeveralOperationCorrectness) EXPECT_TRUE(circuit_builder.check_circuit()); // Check the computation result is in line with what we've computed EXPECT_EQ(result, circuit_builder.get_computation_result()); -} \ No newline at end of file +} diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp index ff8f002abc3..50e6d68793e 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp @@ -89,6 +89,30 @@ template <typename Flavor> class MegaHonkTests : public ::testing::Test { TYPED_TEST_SUITE(MegaHonkTests, FlavorTypes); +/** + * @brief Check that size of a merge proof matches the corresponding constant + * @details This is useful for ensuring correct construction of mock merge proofs + * + */ +TYPED_TEST(MegaHonkTests, MergeProofSizeCheck) +{ + using Flavor = TypeParam; + using MergeProver = MergeProver_<Flavor>; + + // Constuct an op queue and populate it with some arbitrary ops + auto op_queue = std::make_shared<bb::ECCOpQueue>(); + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); + + auto builder = typename Flavor::CircuitBuilder{ op_queue }; + GoblinMockCircuits::construct_simple_circuit(builder); + + // Construct a merge proof and ensure its size matches expectation; if not, the constant may need to be updated + MergeProver merge_prover{ op_queue }; + auto merge_proof = merge_prover.construct_proof(); + + EXPECT_EQ(merge_proof.size(), MERGE_PROOF_SIZE); +} + /** * @brief Test proof construction/verification for a circuit with ECC op gates, public inputs, and basic arithmetic * gates @@ -309,17 +333,6 @@ TYPED_TEST(MegaHonkTests, MultipleCircuitsHonkAndMerge) auto merge_verified = this->construct_and_verify_merge_proof(op_queue); EXPECT_TRUE(merge_verified); } - - // Compute the commitments to the aggregate op queue directly and check that they match those that were computed - // iteratively during transcript aggregation by the provers and stored in the op queue. - size_t aggregate_op_queue_size = op_queue->get_current_size(); - auto ultra_ops = op_queue->get_aggregate_transcript(); - auto commitment_key = std::make_shared<typename Flavor::CommitmentKey>(aggregate_op_queue_size); - size_t idx = 0; - for (const auto& result : op_queue->get_ultra_ops_commitments()) { - auto expected = commitment_key->commit({ /* start index */ 0, ultra_ops[idx++] }); - EXPECT_EQ(result, expected); - } } /** diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp index 00d3757db44..7d872aec97f 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp @@ -5,9 +5,8 @@ namespace bb { /** * @brief Create MergeProver - * @details We require an SRS at least as large as the current op queue size in order to commit to the shifted - * per-circuit contribution t_i^{shift} - * + * @details We require an SRS at least as large as the current ultra ecc ops table + * TODO(https://github.com/AztecProtocol/barretenberg/issues/1267): consider possible efficiency improvements */ template <class Flavor> MergeProver_<Flavor>::MergeProver_(const std::shared_ptr<ECCOpQueue>& op_queue, @@ -15,22 +14,21 @@ MergeProver_<Flavor>::MergeProver_(const std::shared_ptr<ECCOpQueue>& op_queue, : op_queue(op_queue) { // Update internal size data in the op queue that allows for extraction of e.g. previous aggregate transcript - op_queue->set_size_data(); pcs_commitment_key = - commitment_key ? commitment_key : std::make_shared<CommitmentKey>(op_queue->get_current_size()); + commitment_key ? commitment_key : std::make_shared<CommitmentKey>(op_queue->get_ultra_ops_table_num_rows()); } /** - * @brief Prove proper construction of the aggregate Goblin ECC op queue polynomials T_i^(j), j = 1,2,3,4. - * @details Let T_i^(j) be the jth column of the aggregate op queue after incorporating the contribution from the - * present circuit. T_{i-1}^(j) corresponds to the aggregate op queue at the previous stage and $t_i^(j)$ represents - * the contribution from the present circuit only. For each j, we have the relationship T_i = T_{i-1} + right_shift(t_i, - * M_{i-1}), where the shift magnitude M_{i-1} is the honest length of T_{i-1}. This protocol demonstrates, assuming the - * length of T_{i-1} is at most M_{i-1}, that the aggregate op queue has been constructed correctly via a simple - * Schwartz-Zippel check. Evaluations are proven via batched KZG. + * @brief Prove proper construction of the aggregate Goblin ECC op queue polynomials T_j, j = 1,2,3,4. + * @details Let T_j be the jth column of the aggregate ecc op table after prepending the subtable columns t_j containing + * the contribution from the present circuit. T_{j,prev} corresponds to the columns of the aggregate table at the + * previous stage. For each column we have the relationship T_j = t_j + right_shift(T_{j,prev}, k), where k is the + * length of the subtable columns t_j. This protocol demonstrates, assuming the length of t is at most k, that the + * aggregate ecc op table has been constructed correctly via the simple Schwartz-Zippel check: + * + * T_j(\kappa) = t_j(\kappa) + \kappa^k * (T_{j,prev}(\kappa)). * - * TODO(#746): Prove connection between t_i^{shift}, committed to herein, and t_i, used in the main protocol. See issue - * for details (https://github.com/AztecProtocol/barretenberg/issues/746). + * TODO(https://github.com/AztecProtocol/barretenberg/issues/1270): connect [t_j] used herein those used in PG verifier * * @return honk::proof */ @@ -38,74 +36,64 @@ template <typename Flavor> HonkProof MergeProver_<Flavor>::construct_proof() { transcript = std::make_shared<Transcript>(); - size_t N = op_queue->get_current_size(); + // Extract columns of the full table T_j, the previous table T_{j,prev}, and the current subtable t_j + std::array<Polynomial, NUM_WIRES> T_current = op_queue->construct_ultra_ops_table_columns(); + std::array<Polynomial, NUM_WIRES> T_prev = op_queue->construct_previous_ultra_ops_table_columns(); + std::array<Polynomial, NUM_WIRES> t_current = op_queue->construct_current_ultra_ops_subtable_columns(); - // Extract T_i, T_{i-1} - auto T_current = op_queue->get_aggregate_transcript(); - auto T_prev = op_queue->get_previous_aggregate_transcript(); - // TODO(#723): Cannot currently support an empty T_{i-1}. Need to be able to properly handle zero commitment. + // TODO(#723): Cannot currently support an empty T_prev. Need to be able to properly handle zero commitment. ASSERT(T_prev[0].size() > 0); - ASSERT(T_current[0].size() > T_prev[0].size()); // Must have some new ops to accumulate otherwise C_t_shift = 0 + ASSERT(T_current[0].size() > T_prev[0].size()); // Must have some new ops to accumulate otherwise [t_j] = 0 - // Construct t_i^{shift} as T_i - T_{i-1} - std::array<Polynomial, NUM_WIRES> t_shift; - for (size_t i = 0; i < NUM_WIRES; ++i) { - t_shift[i] = Polynomial(T_current[i]); - t_shift[i] -= { 0, T_prev[i] }; - } + const size_t current_table_size = T_current[0].size(); + const size_t current_subtable_size = t_current[0].size(); - // Compute/get commitments [t_i^{shift}], [T_{i-1}], and [T_i] and add to transcript - std::array<Commitment, NUM_WIRES> C_T_current; - for (size_t idx = 0; idx < t_shift.size(); ++idx) { - // Get previous transcript commitment [T_{i-1}] from op queue - const auto& C_T_prev = op_queue->get_ultra_ops_commitments()[idx]; - // Compute commitment [t_i^{shift}] directly - auto C_t_shift = pcs_commitment_key->commit(t_shift[idx]); - // Compute updated aggregate transcript commitment as [T_i] = [T_{i-1}] + [t_i^{shift}] - C_T_current[idx] = C_T_prev + C_t_shift; - - std::string suffix = std::to_string(idx + 1); - transcript->send_to_verifier("T_PREV_" + suffix, C_T_prev); - transcript->send_to_verifier("t_SHIFT_" + suffix, C_t_shift); - transcript->send_to_verifier("T_CURRENT_" + suffix, C_T_current[idx]); - } + transcript->send_to_verifier("subtable_size", static_cast<uint32_t>(current_subtable_size)); - // Store the commitments [T_{i}] (to be used later in subsequent iterations as [T_{i-1}]). - op_queue->set_commitment_data(C_T_current); + // Compute/get commitments [t^{shift}], [T_prev], and [T] and add to transcript + for (size_t idx = 0; idx < NUM_WIRES; ++idx) { + // Compute commitments + Commitment t_commitment = pcs_commitment_key->commit(t_current[idx]); + Commitment T_prev_commitment = pcs_commitment_key->commit(T_prev[idx]); + Commitment T_commitment = pcs_commitment_key->commit(T_current[idx]); + + std::string suffix = std::to_string(idx); + transcript->send_to_verifier("t_CURRENT_" + suffix, t_commitment); + transcript->send_to_verifier("T_PREV_" + suffix, T_prev_commitment); + transcript->send_to_verifier("T_CURRENT_" + suffix, T_commitment); + } - // Compute evaluations T_i(\kappa), T_{i-1}(\kappa), t_i^{shift}(\kappa), add to transcript. For each polynomial - // we add a univariate opening claim {p(X), (\kappa, p(\kappa))} to the set of claims to be checked via batched KZG. + // Compute evaluations T_j(\kappa), T_{j,prev}(\kappa), t_j(\kappa), add to transcript. For each polynomial we add a + // univariate opening claim {p(X), (\kappa, p(\kappa))} to the set of claims to be checked via batched KZG. FF kappa = transcript->template get_challenge<FF>("kappa"); // Add univariate opening claims for each polynomial. std::vector<OpeningClaim> opening_claims; - // Compute evaluation T_{i-1}(\kappa) + // Compute evaluation t(\kappa) for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - auto polynomial = Polynomial(T_prev[idx]); - auto evaluation = polynomial.evaluate(kappa); - transcript->send_to_verifier("T_prev_eval_" + std::to_string(idx + 1), evaluation); - opening_claims.emplace_back(OpeningClaim{ polynomial, { kappa, evaluation } }); + FF evaluation = t_current[idx].evaluate(kappa); + transcript->send_to_verifier("t_eval_" + std::to_string(idx), evaluation); + opening_claims.emplace_back(OpeningClaim{ std::move(t_current[idx]), { kappa, evaluation } }); } - // Compute evaluation t_i^{shift}(\kappa) + // Compute evaluation T_prev(\kappa) for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - auto evaluation = t_shift[idx].evaluate(kappa); - transcript->send_to_verifier("t_shift_eval_" + std::to_string(idx + 1), evaluation); - opening_claims.emplace_back(OpeningClaim{ t_shift[idx], { kappa, evaluation } }); + FF evaluation = T_prev[idx].evaluate(kappa); + transcript->send_to_verifier("T_prev_eval_" + std::to_string(idx), evaluation); + opening_claims.emplace_back(OpeningClaim{ T_prev[idx], { kappa, evaluation } }); } - // Compute evaluation T_i(\kappa) + // Compute evaluation T(\kappa) for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - auto polynomial = Polynomial(T_current[idx]); - auto evaluation = polynomial.evaluate(kappa); - transcript->send_to_verifier("T_current_eval_" + std::to_string(idx + 1), evaluation); - opening_claims.emplace_back(OpeningClaim{ polynomial, { kappa, evaluation } }); + FF evaluation = T_current[idx].evaluate(kappa); + transcript->send_to_verifier("T_eval_" + std::to_string(idx), evaluation); + opening_claims.emplace_back(OpeningClaim{ std::move(T_current[idx]), { kappa, evaluation } }); } FF alpha = transcript->template get_challenge<FF>("alpha"); - // Construct batched polynomial to opened via KZG - auto batched_polynomial = Polynomial(N); - auto batched_eval = FF(0); - auto alpha_pow = FF(1); + // Construct batched polynomial to be opened via KZG + Polynomial batched_polynomial(current_table_size); + FF batched_eval(0); + FF alpha_pow(1); for (auto& claim : opening_claims) { batched_polynomial.add_scaled(claim.polynomial, alpha_pow); batched_eval += alpha_pow * claim.opening_pair.evaluation; @@ -113,12 +101,8 @@ template <typename Flavor> HonkProof MergeProver_<Flavor>::construct_proof() } // Construct and commit to KZG quotient polynomial q = (f - v) / (X - kappa) - auto quotient = batched_polynomial; - quotient.at(0) -= batched_eval; - quotient.factor_roots(kappa); - - auto quotient_commitment = pcs_commitment_key->commit(quotient); - transcript->send_to_verifier("KZG:W", quotient_commitment); + OpeningClaim batched_claim = { std::move(batched_polynomial), { kappa, batched_eval } }; + PCS::compute_opening_proof(pcs_commitment_key, batched_claim, transcript); return transcript->proof_data; } diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_verifier.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_verifier.cpp index 13f2bf21ba0..9569d44038d 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_verifier.cpp @@ -10,68 +10,78 @@ MergeVerifier_<Flavor>::MergeVerifier_() , pcs_verification_key(std::make_unique<VerifierCommitmentKey>()){}; /** - * @brief Verify proper construction of the aggregate Goblin ECC op queue polynomials T_i^(j), j = 1,2,3,4. - * @details Let T_i^(j) be the jth column of the aggregate op queue after incorporating the contribution from the - * present circuit. T_{i-1}^(j) corresponds to the aggregate op queue at the previous stage and $t_i^(j)$ represents - * the contribution from the present circuit only. For each j, we have the relationship T_i = T_{i-1} + right_shift(t_i, - * M_{i-1}), where the shift magnitude M_{i-1} is the honest length of T_{i-1}. This protocol verfies, assuming the - * length of T_{i-1} is at most M_{i-1}, that the aggregate op queue has been constructed correctly via a simple - * Schwartz-Zippel check. Evaluations are checked via batched KZG. + * @brief Verify proper construction of the aggregate Goblin ECC op queue polynomials T_j, j = 1,2,3,4. + * @details Let T_j be the jth column of the aggregate ecc op table after prepending the subtable columns t_j containing + * the contribution from a single circuit. T_{j,prev} corresponds to the columns of the aggregate table at the + * previous stage. For each column we have the relationship T_j = t_j + right_shift(T_{j,prev}, k), where k is the + * length of the subtable columns t_j. This protocol demonstrates, assuming the length of t is at most k, that the + * aggregate ecc op table has been constructed correctly via the simple Schwartz-Zippel check: + * + * T_j(\kappa) = t_j(\kappa) + \kappa^k * (T_{j,prev}(\kappa)). * * @tparam Flavor - * @return bool + * @return bool Verification result */ template <typename Flavor> bool MergeVerifier_<Flavor>::verify_proof(const HonkProof& proof) { transcript = std::make_shared<Transcript>(proof); - // Receive commitments [t_i^{shift}], [T_{i-1}], and [T_i] - std::array<Commitment, Flavor::NUM_WIRES> C_T_prev; - std::array<Commitment, Flavor::NUM_WIRES> C_t_shift; - std::array<Commitment, Flavor::NUM_WIRES> C_T_current; + uint32_t subtable_size = transcript->template receive_from_prover<uint32_t>("subtable_size"); + + // Receive table column polynomial commitments [t_j], [T_{j,prev}], and [T_j], j = 1,2,3,4 + std::array<Commitment, Flavor::NUM_WIRES> t_commitments; + std::array<Commitment, Flavor::NUM_WIRES> T_prev_commitments; + std::array<Commitment, Flavor::NUM_WIRES> T_commitments; for (size_t idx = 0; idx < Flavor::NUM_WIRES; ++idx) { - C_T_prev[idx] = transcript->template receive_from_prover<Commitment>("T_PREV_" + std::to_string(idx + 1)); - C_t_shift[idx] = transcript->template receive_from_prover<Commitment>("t_SHIFT_" + std::to_string(idx + 1)); - C_T_current[idx] = transcript->template receive_from_prover<Commitment>("T_CURRENT_" + std::to_string(idx + 1)); + std::string suffix = std::to_string(idx); + t_commitments[idx] = transcript->template receive_from_prover<Commitment>("t_CURRENT_" + suffix); + T_prev_commitments[idx] = transcript->template receive_from_prover<Commitment>("T_PREV_" + suffix); + T_commitments[idx] = transcript->template receive_from_prover<Commitment>("T_CURRENT_" + suffix); } FF kappa = transcript->template get_challenge<FF>("kappa"); - // Receive transcript poly evaluations and add corresponding univariate opening claims {(\kappa, p(\kappa), [p(X)]} + // Receive evaluations t_j(\kappa), T_{j,prev}(\kappa), T_j(\kappa), j = 1,2,3,4 + std::array<FF, Flavor::NUM_WIRES> t_evals; std::array<FF, Flavor::NUM_WIRES> T_prev_evals; - std::array<FF, Flavor::NUM_WIRES> t_shift_evals; - std::array<FF, Flavor::NUM_WIRES> T_current_evals; - std::vector<OpeningClaim> opening_claims; + std::array<FF, Flavor::NUM_WIRES> T_evals; for (size_t idx = 0; idx < Flavor::NUM_WIRES; ++idx) { - T_prev_evals[idx] = transcript->template receive_from_prover<FF>("T_prev_eval_" + std::to_string(idx + 1)); - opening_claims.emplace_back(OpeningClaim{ { kappa, T_prev_evals[idx] }, C_T_prev[idx] }); + t_evals[idx] = transcript->template receive_from_prover<FF>("t_eval_" + std::to_string(idx)); } for (size_t idx = 0; idx < Flavor::NUM_WIRES; ++idx) { - t_shift_evals[idx] = transcript->template receive_from_prover<FF>("t_shift_eval_" + std::to_string(idx + 1)); - opening_claims.emplace_back(OpeningClaim{ { kappa, t_shift_evals[idx] }, C_t_shift[idx] }); + T_prev_evals[idx] = transcript->template receive_from_prover<FF>("T_prev_eval_" + std::to_string(idx)); } for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - T_current_evals[idx] = - transcript->template receive_from_prover<FF>("T_current_eval_" + std::to_string(idx + 1)); - opening_claims.emplace_back(OpeningClaim{ { kappa, T_current_evals[idx] }, C_T_current[idx] }); + T_evals[idx] = transcript->template receive_from_prover<FF>("T_eval_" + std::to_string(idx)); } - // Check the identity T_i(\kappa) = T_{i-1}(\kappa) + t_i^{shift}(\kappa). If it fails, return false + // Check the identity T_j(\kappa) = t_j(\kappa) + \kappa^m * T_{j,prev}(\kappa). If it fails, return false bool identity_checked = true; for (size_t idx = 0; idx < NUM_WIRES; ++idx) { - identity_checked = identity_checked && (T_current_evals[idx] == T_prev_evals[idx] + t_shift_evals[idx]); + FF T_prev_shifted_eval_reconstructed = T_prev_evals[idx] * kappa.pow(subtable_size); + bool current_check = T_evals[idx] == t_evals[idx] + T_prev_shifted_eval_reconstructed; + identity_checked = identity_checked && current_check; } FF alpha = transcript->template get_challenge<FF>("alpha"); // Construct batched commitment and evaluation from constituents - auto batched_commitment = opening_claims[0].commitment; - auto batched_eval = opening_claims[0].opening_pair.evaluation; + Commitment batched_commitment = t_commitments[0]; + FF batched_eval = t_evals[0]; auto alpha_pow = alpha; - for (size_t idx = 1; idx < opening_claims.size(); ++idx) { - auto& claim = opening_claims[idx]; - batched_commitment = batched_commitment + (claim.commitment * alpha_pow); - batched_eval += alpha_pow * claim.opening_pair.evaluation; + for (size_t idx = 1; idx < NUM_WIRES; ++idx) { + batched_commitment = batched_commitment + (t_commitments[idx] * alpha_pow); + batched_eval += alpha_pow * t_evals[idx]; + alpha_pow *= alpha; + } + for (size_t idx = 0; idx < NUM_WIRES; ++idx) { + batched_commitment = batched_commitment + (T_prev_commitments[idx] * alpha_pow); + batched_eval += alpha_pow * T_prev_evals[idx]; + alpha_pow *= alpha; + } + for (size_t idx = 0; idx < NUM_WIRES; ++idx) { + batched_commitment = batched_commitment + (T_commitments[idx] * alpha_pow); + batched_eval += alpha_pow * T_evals[idx]; alpha_pow *= alpha; } diff --git a/barretenberg/cpp/src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc.test.cpp index 5177113a1f0..21656305671 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_vanilla_client_ivc/ultra_vanilla_client_ivc.test.cpp @@ -90,4 +90,4 @@ TEST_F(UltraVanillaClientIVCTests, PrecomputedVerificationKeys) EXPECT_TRUE(ivc_2.prove_and_verify(circuit_source_with_vks)); }; -// TODO(https://github.com/AztecProtocol/barretenberg/issues/1177) Implement failure tests \ No newline at end of file +// TODO(https://github.com/AztecProtocol/barretenberg/issues/1177) Implement failure tests