diff --git a/barretenberg/cpp/src/barretenberg/goblin/CMakeLists.txt b/barretenberg/cpp/src/barretenberg/goblin/CMakeLists.txt index ea1c0d52b916..0ff44b47db2f 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/CMakeLists.txt +++ b/barretenberg/cpp/src/barretenberg/goblin/CMakeLists.txt @@ -1 +1 @@ -barretenberg_module(goblin ultra_honk eccvm translator_vm stdlib_honk_verifier stdlib_pedersen_commitment stdlib_sha256 ) \ No newline at end of file +barretenberg_module(goblin ultra_honk eccvm translator_vm stdlib_honk_verifier stdlib_pedersen_commitment stdlib_sha256 ) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index e307bf8635bc..582d64e1e301 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -63,12 +63,8 @@ class GoblinProver { public: GoblinProver(const std::shared_ptr>& bn254_commitment_key = nullptr) - { // Mocks the interaction of a first circuit with the op queue due to the inability to currently handle zero - // 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(bn254_commitment_key) + {} /** * @brief Construct a merge proof for the goblin ECC ops in the provided circuit diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index 4964a6d73372..f6b11123a589 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -117,35 +117,6 @@ class GoblinMockCircuits { stdlib::generate_ecdsa_verification_test_circuit(builder, 1); stdlib::generate_merkle_membership_test_circuit(builder, 10); } - - // TODO(https://github.com/AztecProtocol/barretenberg/issues/911): We require goblin ops to be added to the - // function circuit because we cannot support zero commtiments. While the builder handles this at - // DeciderProvingKey creation stage via the add_gates_to_ensure_all_polys_are_non_zero function for other - // MegaHonk circuits (where we don't explicitly need to add goblin ops), in ClientIVC merge proving happens - // prior to folding where the absense of goblin ecc ops will result in zero commitments. - MockCircuits::construct_goblin_ecc_op_circuit(builder); - } - - /** - * @brief Mock the interactions of a simple curcuit with the op_queue - * @todo The transcript aggregation protocol in the Goblin proof system can not yet support an empty "previous - * transcript" (see issue #723) because the corresponding commitments are zero / the point at infinity. This - * function mocks the interactions with the op queue of a fictional "first" circuit. This way, when we go to - * generate a proof over our first "real" circuit, the transcript aggregation protocol can proceed nominally. - * The mock data is valid in the sense that it can be processed by all stages of Goblin as if it came from a - * genuine circuit. - * - * - * @param op_queue - */ - static void perform_op_queue_interactions_for_mock_first_circuit(std::shared_ptr& op_queue) - { - PROFILE_THIS(); - - bb::MegaCircuitBuilder builder{ op_queue }; - - // Add some goblinized ecc ops - MockCircuits::construct_goblin_ecc_op_circuit(builder); } /** diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_delta_range_constraint_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_delta_range_constraint_relation.hpp index bb3aaaa1b3ab..540b0f888a5c 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_delta_range_constraint_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_delta_range_constraint_relation.hpp @@ -9,7 +9,7 @@ template class TranslatorDeltaRangeConstraintRelationImpl { // 1 + polynomial degree of this relation static constexpr size_t RELATION_LENGTH = - 7; // degree((lagrange_real_last - 1)(lagrange_masking - 1) * D(D - 1)(D - 2)(D - 3)) = 5 + 7; // degree((lagrange_real_last - 1)(lagrange_masking - 1) * D(D - 1)(D - 2)(D - 3)) = 6 static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ 7, // ordered_range_constraints_0 step in {0,1,2,3} subrelation diff --git a/barretenberg/cpp/src/barretenberg/srs/global_crs.hpp b/barretenberg/cpp/src/barretenberg/srs/global_crs.hpp index 119d170d83ee..1f62246ea737 100644 --- a/barretenberg/cpp/src/barretenberg/srs/global_crs.hpp +++ b/barretenberg/cpp/src/barretenberg/srs/global_crs.hpp @@ -5,8 +5,6 @@ namespace bb::srs { -// The call comes from GoblinProver ctor and call to perform_op_queue_interactions_for_mock_first_circuit. -// Delete the ifdef block in fn above once resolved. inline std::string get_ignition_crs_path() { const char* env_var = std::getenv("IGNITION_CRS_PATH"); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/CMakeLists.txt b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/CMakeLists.txt index 609d9d179c8a..a6505a2f1c45 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/CMakeLists.txt +++ b/barretenberg/cpp/src/barretenberg/stdlib/goblin_verifier/CMakeLists.txt @@ -5,4 +5,4 @@ barretenberg_module( stdlib_honk_verifier stdlib_eccvm_verifier stdlib_translator_vm_verifier -) \ No newline at end of file +) 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 c1fa8405d4ee..bde2df6f622f 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 @@ -46,8 +46,6 @@ template class RecursiveMergeVerifierTest : public test static void test_recursive_merge_verification() { auto op_queue = std::make_shared(); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/800) Testing cleanup - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); InnerBuilder sample_circuit{ op_queue }; GoblinMockCircuits::construct_simple_circuit(sample_circuit); 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 50e6d68793e8..7a2997829e9c 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp @@ -99,15 +99,11 @@ TYPED_TEST(MegaHonkTests, MergeProofSizeCheck) using Flavor = TypeParam; using MergeProver = MergeProver_; - // Constuct an op queue and populate it with some arbitrary ops - auto op_queue = std::make_shared(); - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); - - auto builder = typename Flavor::CircuitBuilder{ op_queue }; + auto builder = typename Flavor::CircuitBuilder{}; 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 }; + MergeProver merge_prover{ builder.op_queue }; auto merge_proof = merge_prover.construct_proof(); EXPECT_EQ(merge_proof.size(), MERGE_PROOF_SIZE); @@ -235,10 +231,7 @@ TYPED_TEST(MegaHonkTests, DynamicVirtualSizeIncrease) TYPED_TEST(MegaHonkTests, SingleCircuit) { using Flavor = TypeParam; - auto op_queue = std::make_shared(); - - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); - auto builder = typename Flavor::CircuitBuilder{ op_queue }; + auto builder = typename Flavor::CircuitBuilder{}; GoblinMockCircuits::construct_simple_circuit(builder); @@ -247,7 +240,7 @@ TYPED_TEST(MegaHonkTests, SingleCircuit) EXPECT_TRUE(honk_verified); // Construct and verify Goblin ECC op queue Merge proof - auto merge_verified = this->construct_and_verify_merge_proof(op_queue); + auto merge_verified = this->construct_and_verify_merge_proof(builder.op_queue); EXPECT_TRUE(merge_verified); } @@ -261,9 +254,6 @@ TYPED_TEST(MegaHonkTests, MultipleCircuitsMergeOnly) using Flavor = TypeParam; // Instantiate EccOpQueue. This will be shared across all circuits in the series auto op_queue = std::make_shared(); - - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); - // Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each. size_t NUM_CIRCUITS = 3; for (size_t i = 0; i < NUM_CIRCUITS; ++i) { @@ -288,9 +278,6 @@ TYPED_TEST(MegaHonkTests, MultipleCircuitsHonkOnly) // Instantiate EccOpQueue. This will be shared across all circuits in the series auto op_queue = std::make_shared(); - - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); - // Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each. size_t NUM_CIRCUITS = 3; for (size_t i = 0; i < NUM_CIRCUITS; ++i) { @@ -315,9 +302,6 @@ TYPED_TEST(MegaHonkTests, MultipleCircuitsHonkAndMerge) // Instantiate EccOpQueue. This will be shared across all circuits in the series auto op_queue = std::make_shared(); - - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); - // Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each. size_t NUM_CIRCUITS = 3; for (size_t i = 0; i < NUM_CIRCUITS; ++i) { diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp index 0127e9ec6047..2d64775dbc1d 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp @@ -12,11 +12,9 @@ template MergeProver_::MergeProver_(const std::shared_ptr& op_queue, std::shared_ptr commitment_key) : op_queue(op_queue) -{ - // Update internal size data in the op queue that allows for extraction of e.g. previous aggregate transcript - pcs_commitment_key = - commitment_key ? commitment_key : std::make_shared(op_queue->get_ultra_ops_table_num_rows()); -} + , pcs_commitment_key(commitment_key ? commitment_key + : std::make_shared(op_queue->get_ultra_ops_table_num_rows())) +{} /** * @brief Prove proper construction of the aggregate Goblin ECC op queue polynomials T_j, j = 1,2,3,4. @@ -41,10 +39,6 @@ template MergeProver_::MergeProof MergeProver_ std::array T_prev = op_queue->construct_previous_ultra_ops_table_columns(); std::array t_current = op_queue->construct_current_ultra_ops_subtable_columns(); - // 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 [t_j] = 0 - const size_t current_table_size = T_current[0].size(); const size_t current_subtable_size = t_current[0].size();