From 835383c5f0f273fba391054aca0b18426db1282e Mon Sep 17 00:00:00 2001 From: maramihali Date: Thu, 7 Mar 2024 15:24:05 +0000 Subject: [PATCH 1/4] remove functionality from eccvm that modifies the op queue --- .../benchmark/goblin_bench/eccvm.bench.cpp | 20 +-- .../eccvm/eccvm_composer.test.cpp | 64 ++++---- .../eccvm/eccvm_transcript.test.cpp | 37 ++--- .../eccvm/eccvm_circuit_builder.hpp | 65 -------- .../eccvm/eccvm_circuit_builder.test.cpp | 147 ++++++++++-------- 5 files changed, 144 insertions(+), 189 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/eccvm.bench.cpp b/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/eccvm.bench.cpp index 2805fde8286..ac1efd77f72 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/eccvm.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/eccvm.bench.cpp @@ -15,7 +15,7 @@ namespace { Builder generate_trace(size_t target_num_gates) { - Builder builder; + std::shared_ptr op_queue = std::make_shared(); using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; @@ -26,20 +26,20 @@ Builder generate_trace(size_t target_num_gates) Fr x = Fr::random_element(); Fr y = Fr::random_element(); - typename G1::element expected_1 = (a * x) + a + a + (b * y) + (b * x) + (b * x); - // Each loop adds 163 gates. Note: builder.get_num_gates() is very expensive here (bug?) and it's actually painful // to use a `while` loop size_t num_iterations = target_num_gates / 163; for (size_t _ = 0; _ < num_iterations; _++) { - builder.add_accumulate(a); - builder.mul_accumulate(a, x); - builder.mul_accumulate(b, x); - builder.mul_accumulate(b, y); - builder.add_accumulate(a); - builder.mul_accumulate(b, x); - builder.eq_and_reset(expected_1); + op_queue->add_accumulate(a); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(b, y); + op_queue->add_accumulate(a); + op_queue->mul_accumulate(b, x); + op_queue->eq(); } + + Builder builder{ op_queue }; return builder; } diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_composer.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_composer.test.cpp index 812f842dee3..f9e2b72f39b 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_composer.test.cpp @@ -33,9 +33,9 @@ TYPED_TEST_SUITE(ECCVMComposerTests, FlavorTypes); namespace { auto& engine = numeric::get_debug_randomness(); } -template ECCVMCircuitBuilder generate_trace(numeric::RNG* engine = nullptr) +template ECCVMCircuitBuilder generate_circuit(numeric::RNG* engine = nullptr) { - ECCVMCircuitBuilder result; + std::shared_ptr op_queue = std::make_shared(); using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; @@ -47,38 +47,35 @@ template ECCVMCircuitBuilder generate_trace(numeric::R Fr x = Fr::random_element(engine); Fr y = Fr::random_element(engine); - typename G1::element expected_1 = (a * x) + a + a + (b * y) + (b * x) + (b * x); - typename G1::element expected_2 = (a * x) + c + (b * x); - - result.add_accumulate(a); - result.mul_accumulate(a, x); - result.mul_accumulate(b, x); - result.mul_accumulate(b, y); - result.add_accumulate(a); - result.mul_accumulate(b, x); - result.eq_and_reset(expected_1); - result.add_accumulate(c); - result.mul_accumulate(a, x); - result.mul_accumulate(b, x); - result.eq_and_reset(expected_2); - result.mul_accumulate(a, x); - result.mul_accumulate(b, x); - result.mul_accumulate(c, x); - - return result; + op_queue->add_accumulate(a); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(b, y); + op_queue->add_accumulate(a); + op_queue->mul_accumulate(b, x); + op_queue->eq(); + op_queue->add_accumulate(c); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->eq(); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(c, x); + ECCVMCircuitBuilder builder{ op_queue }; + return builder; } TYPED_TEST(ECCVMComposerTests, BaseCase) { using Flavor = TypeParam; - auto circuit_constructor = generate_trace(&engine); + auto builder = generate_circuit(&engine); auto composer = ECCVMComposer_(); - auto prover = composer.create_prover(circuit_constructor); + auto prover = composer.create_prover(builder); auto proof = prover.construct_proof(); - auto verifier = composer.create_verifier(circuit_constructor); + auto verifier = composer.create_verifier(builder); bool verified = verifier.verify_proof(proof); ASSERT_TRUE(verified); @@ -87,16 +84,23 @@ TYPED_TEST(ECCVMComposerTests, BaseCase) TYPED_TEST(ECCVMComposerTests, EqFails) { using Flavor = TypeParam; - using G1 = typename Flavor::CycleGroup; - auto circuit_constructor = generate_trace(&engine); - // create an eq opcode that is not satisfied - circuit_constructor.eq_and_reset(G1::affine_one); + using ECCVMOperation = eccvm::VMOperation; + auto builder = generate_circuit(&engine); + // Tamper with the eq op such that the expected value is incorect + builder.op_queue->raw_ops.emplace_back(ECCVMOperation{ .add = false, + .mul = false, + .eq = true, + .reset = true, + .base_point = G1::affine_one, + .z1 = 0, + .z2 = 0, + .mul_scalar_full = 0 }); auto composer = ECCVMComposer_(); - auto prover = composer.create_prover(circuit_constructor); + auto prover = composer.create_prover(builder); auto proof = prover.construct_proof(); - auto verifier = composer.create_verifier(circuit_constructor); + auto verifier = composer.create_verifier(builder); bool verified = verifier.verify_proof(proof); ASSERT_FALSE(verified); } diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp index f1ab178db34..d7f41fd4424 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp @@ -185,7 +185,7 @@ template class ECCVMTranscriptTests : public ::testing::Test { } ECCVMCircuitBuilder generate_trace(numeric::RNG* engine = nullptr) { - ECCVMCircuitBuilder result; + std::shared_ptr op_queue = std::make_shared(); using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; @@ -197,25 +197,22 @@ template class ECCVMTranscriptTests : public ::testing::Test { Fr x = Fr::random_element(engine); Fr y = Fr::random_element(engine); - typename G1::element expected_1 = (a * x) + a + a + (b * y) + (b * x) + (b * x); - typename G1::element expected_2 = (a * x) + c + (b * x); - - result.add_accumulate(a); - result.mul_accumulate(a, x); - result.mul_accumulate(b, x); - result.mul_accumulate(b, y); - result.add_accumulate(a); - result.mul_accumulate(b, x); - result.eq_and_reset(expected_1); - result.add_accumulate(c); - result.mul_accumulate(a, x); - result.mul_accumulate(b, x); - result.eq_and_reset(expected_2); - result.mul_accumulate(a, x); - result.mul_accumulate(b, x); - result.mul_accumulate(c, x); - - return result; + op_queue->add_accumulate(a); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(b, y); + op_queue->add_accumulate(a); + op_queue->mul_accumulate(b, x); + op_queue->eq(); + op_queue->add_accumulate(c); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->eq(); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(c, x); + ECCVMCircuitBuilder builder{ op_queue }; + return builder; } }; diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp index a60c840d379..88ad2e42908 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp @@ -41,9 +41,6 @@ template class ECCVMCircuitBuilder { using ScalarMul = bb::eccvm::ScalarMul; using ProverPolynomials = typename Flavor::ProverPolynomials; - ECCVMCircuitBuilder() - : op_queue(std::make_shared()){}; - ECCVMCircuitBuilder(std::shared_ptr& op_queue) : op_queue(op_queue){}; @@ -177,68 +174,6 @@ template class ECCVMCircuitBuilder { return result; } - void add_accumulate(const AffineElement& to_add) - { - op_queue->raw_ops.emplace_back(VMOperation{ - .add = true, - .mul = false, - .eq = false, - .reset = false, - .base_point = to_add, - .z1 = 0, - .z2 = 0, - .mul_scalar_full = 0, - }); - } - - void mul_accumulate(const AffineElement& to_mul, const CycleScalar& scalar) - { - CycleScalar z1 = 0; - CycleScalar z2 = 0; - auto converted = scalar.from_montgomery_form(); - CycleScalar::split_into_endomorphism_scalars(converted, z1, z2); - z1 = z1.to_montgomery_form(); - z2 = z2.to_montgomery_form(); - op_queue->raw_ops.emplace_back(VMOperation{ - .add = false, - .mul = true, - .eq = false, - .reset = false, - .base_point = to_mul, - .z1 = z1, - .z2 = z2, - .mul_scalar_full = scalar, - }); - } - - void eq_and_reset(const AffineElement& expected) - { - op_queue->raw_ops.emplace_back(VMOperation{ - .add = false, - .mul = false, - .eq = true, - .reset = true, - .base_point = expected, - .z1 = 0, - .z2 = 0, - .mul_scalar_full = 0, - }); - } - - void empty_row() - { - op_queue->raw_ops.emplace_back(VMOperation{ - .add = false, - .mul = false, - .eq = false, - .reset = false, - .base_point = CycleGroup::affine_point_at_infinity, - .z1 = 0, - .z2 = 0, - .mul_scalar_full = 0, - }); - } - /** * @brief Compute the ECCVM flavor polynomial data required to generate an ECCVM Proof * diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp index 54e0b7a8a79..9df0ecf6430 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp @@ -20,7 +20,6 @@ TYPED_TEST(ECCVMCircuitBuilderTests, BaseCase) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; typename G1::element b = generators[1]; @@ -28,24 +27,24 @@ TYPED_TEST(ECCVMCircuitBuilderTests, BaseCase) Fr x = Fr::random_element(&engine); Fr y = Fr::random_element(&engine); - typename G1::element expected_1 = (a * x) + a + a + (b * y) + (b * x) + (b * x); - typename G1::element expected_2 = (a * x) + c + (b * x); - - circuit.add_accumulate(a); - circuit.mul_accumulate(a, x); - circuit.mul_accumulate(b, x); - circuit.mul_accumulate(b, y); - circuit.add_accumulate(a); - circuit.mul_accumulate(b, x); - circuit.eq_and_reset(expected_1); - circuit.add_accumulate(c); - circuit.mul_accumulate(a, x); - circuit.mul_accumulate(b, x); - circuit.eq_and_reset(expected_2); - circuit.mul_accumulate(a, x); - circuit.mul_accumulate(b, x); - circuit.mul_accumulate(c, x); - + std::shared_ptr op_queue = std::make_shared(); + + op_queue->add_accumulate(a); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(b, y); + op_queue->add_accumulate(a); + op_queue->mul_accumulate(b, x); + op_queue->eq(); + op_queue->add_accumulate(c); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->eq(); + op_queue->mul_accumulate(a, x); + op_queue->mul_accumulate(b, x); + op_queue->mul_accumulate(c, x); + + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -54,13 +53,14 @@ TYPED_TEST(ECCVMCircuitBuilderTests, Add) { using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; - circuit.add_accumulate(a); + op_queue->add_accumulate(a); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -70,13 +70,15 @@ TYPED_TEST(ECCVMCircuitBuilderTests, Mul) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); + auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - circuit.mul_accumulate(a, x); + op_queue->mul_accumulate(a, x); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -86,7 +88,8 @@ TYPED_TEST(ECCVMCircuitBuilderTests, ShortMul) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); + auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; @@ -96,9 +99,10 @@ TYPED_TEST(ECCVMCircuitBuilderTests, ShortMul) small_x.data[1] = engine.get_random_uint64() & 0xFFFFFFFFFFFFULL; Fr x = small_x; - circuit.mul_accumulate(a, x); - circuit.eq_and_reset(a * small_x); + op_queue->mul_accumulate(a, x); + op_queue->eq(); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -108,14 +112,24 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EqFails) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + using ECCVMOperation = eccvm::VMOperation; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - circuit.mul_accumulate(a, x); - circuit.eq_and_reset(a); + op_queue->mul_accumulate(a, x); + // Tamper with the eq op such that the expected value is incorect + op_queue->raw_ops.emplace_back(ECCVMOperation{ .add = false, + .mul = false, + .eq = true, + .reset = true, + .base_point = a, + .z1 = 0, + .z2 = 0, + .mul_scalar_full = 0 }); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, false); } @@ -123,10 +137,11 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EqFails) TYPED_TEST(ECCVMCircuitBuilderTests, EmptyRow) { using Flavor = TypeParam; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); - circuit.empty_row(); + op_queue->empty_row(); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -136,18 +151,17 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EmptyRowBetweenOps) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - typename G1::element expected_1 = (a * x); - - circuit.mul_accumulate(a, x); - circuit.empty_row(); - circuit.eq_and_reset(expected_1); + op_queue->mul_accumulate(a, x); + op_queue->empty_row(); + op_queue->eq(); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -157,17 +171,16 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EndWithEq) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - typename G1::element expected_1 = (a * x); - - circuit.mul_accumulate(a, x); - circuit.eq_and_reset(expected_1); + op_queue->mul_accumulate(a, x); + op_queue->eq(); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -177,18 +190,17 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EndWithAdd) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - typename G1::element expected_1 = (a * x); - - circuit.mul_accumulate(a, x); - circuit.eq_and_reset(expected_1); - circuit.add_accumulate(a); + op_queue->mul_accumulate(a, x); + op_queue->eq(); + op_queue->add_accumulate(a); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -198,16 +210,17 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EndWithMul) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - circuit.add_accumulate(a); - circuit.eq_and_reset(a); - circuit.mul_accumulate(a, x); + op_queue->add_accumulate(a); + op_queue->eq(); + op_queue->mul_accumulate(a, x); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -217,16 +230,18 @@ TYPED_TEST(ECCVMCircuitBuilderTests, EndWithNoop) using Flavor = TypeParam; using G1 = typename Flavor::CycleGroup; using Fr = typename G1::Fr; - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); auto generators = G1::derive_generators("test generators", 3); typename G1::element a = generators[0]; Fr x = Fr::random_element(&engine); - circuit.add_accumulate(a); - circuit.eq_and_reset(a); - circuit.mul_accumulate(a, x); - circuit.empty_row(); + op_queue->add_accumulate(a); + op_queue->eq(); + op_queue->mul_accumulate(a, x); + + op_queue->empty_row(); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } @@ -240,7 +255,7 @@ TYPED_TEST(ECCVMCircuitBuilderTests, MSM) static constexpr size_t max_num_msms = 9; auto generators = G1::derive_generators("test generators", max_num_msms); - const auto try_msms = [&](const size_t num_msms, auto& circuit) { + const auto try_msms = [&](const size_t num_msms, auto& op_queue) { std::vector points; std::vector scalars; typename G1::element expected = G1::point_at_infinity; @@ -248,24 +263,28 @@ TYPED_TEST(ECCVMCircuitBuilderTests, MSM) points.emplace_back(generators[i]); scalars.emplace_back(Fr::random_element(&engine)); expected += (points[i] * scalars[i]); - circuit.mul_accumulate(points[i], scalars[i]); + op_queue->mul_accumulate(points[i], scalars[i]); } - circuit.eq_and_reset(expected); + op_queue->eq(); }; // single msms for (size_t j = 1; j < max_num_msms; ++j) { using Flavor = TypeParam; - ECCVMCircuitBuilder circuit; - try_msms(j, circuit); + std::shared_ptr op_queue = std::make_shared(); + + try_msms(j, op_queue); + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } // chain msms - ECCVMCircuitBuilder circuit; + std::shared_ptr op_queue = std::make_shared(); + for (size_t j = 1; j < 9; ++j) { - try_msms(j, circuit); + try_msms(j, op_queue); } + ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); } From 0f792e327e46442393ded2b0dba22be13df8c17e Mon Sep 17 00:00:00 2001 From: maramihali Date: Fri, 8 Mar 2024 12:17:11 +0000 Subject: [PATCH 2/4] populate ultra ops only through function --- .../goblin_ultra_circuit_builder.cpp | 10 +---- .../proof_system/op_queue/ecc_op_queue.hpp | 37 +++++++++++++------ .../ultra_honk/goblin_ultra_composer.test.cpp | 3 +- .../barretenberg/ultra_honk/merge_prover.cpp | 2 +- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp index 61def907060..0c3b11b4161 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp @@ -170,15 +170,7 @@ ecc_op_tuple GoblinUltraCircuitBuilder_::decompose_ecc_operands(uint32_t op_ z_2 = z_2.to_montgomery_form(); // Populate ultra ops in OpQueue with the decomposed operands - op_queue->ultra_ops[0].emplace_back(this->variables[op_idx]); - op_queue->ultra_ops[1].emplace_back(x_lo); - op_queue->ultra_ops[2].emplace_back(x_hi); - op_queue->ultra_ops[3].emplace_back(y_lo); - - op_queue->ultra_ops[0].emplace_back(this->zero_idx); - op_queue->ultra_ops[1].emplace_back(y_hi); - op_queue->ultra_ops[2].emplace_back(z_1); - op_queue->ultra_ops[3].emplace_back(z_2); + op_queue->populate_ultra_ops({ this->variables[op_idx], x_lo, x_hi, y_lo, y_hi, z_1, z_2 }); // Add variables for decomposition and get indices needed for op wires auto x_lo_idx = this->add_variable(x_lo); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp index 3b1a6866967..3f433e06543 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp @@ -7,6 +7,17 @@ namespace bb { enum EccOpCode { NULL_OP, ADD_ACCUM, MUL_ACCUM, EQUALITY }; +struct UltraOpTuple { + using Fr = curve::BN254::ScalarField; + Fr op; + Fr x_lo; + Fr x_hi; + Fr y_lo; + Fr y_hi; + Fr z_1; + Fr z_2; +}; + /** * @brief Used to construct execution trace representations of elliptic curve operations. * @@ -34,7 +45,6 @@ class ECCOpQueue { size_t previous_ultra_ops_size = 0; // M_{i-1} std::array ultra_ops_commitments; - std::array previous_ultra_ops_commitments; Point get_accumulator() { return accumulator; } @@ -75,7 +85,6 @@ class ECCOpQueue { previous_ultra_ops_size += previous.ultra_ops[0].size(); // Update commitments ultra_ops_commitments = previous.ultra_ops_commitments; - previous_ultra_ops_commitments = previous.previous_ultra_ops_commitments; } /** * @brief Prepend the information from the previous queue (used before accumulation/merge proof to be able to run @@ -104,10 +113,7 @@ class ECCOpQueue { lhs.previous_ultra_ops_size = rhs.previous_ultra_ops_size; rhs.previous_ultra_ops_size = temp; // Swap commitments - auto commit_temp = lhs.previous_ultra_ops_commitments; - lhs.previous_ultra_ops_commitments = rhs.previous_ultra_ops_commitments; - rhs.previous_ultra_ops_commitments = commit_temp; - commit_temp = lhs.ultra_ops_commitments; + auto commit_temp = lhs.ultra_ops_commitments; lhs.ultra_ops_commitments = rhs.ultra_ops_commitments; rhs.ultra_ops_commitments = commit_temp; } @@ -127,11 +133,7 @@ class ECCOpQueue { [[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& commitments) - { - previous_ultra_ops_commitments = ultra_ops_commitments; - ultra_ops_commitments = commitments; - } + void set_commitment_data(std::array& commitments) { ultra_ops_commitments = commitments; } /** * @brief Get a 'view' of the current ultra ops object @@ -281,6 +283,19 @@ class ECCOpQueue { .mul_scalar_full = 0, }); } + + void populate_ultra_ops(UltraOpTuple 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/ultra_honk/goblin_ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp index 72ffc358c0e..bed11a6fb4b 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp @@ -199,10 +199,11 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsHonkAndMerge) // 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->current_ultra_ops_size; + auto ultra_ops = op_queue->get_aggregate_transcript(); auto commitment_key = std::make_shared(aggregate_op_queue_size); size_t idx = 0; for (auto& result : op_queue->ultra_ops_commitments) { - auto expected = commitment_key->commit(op_queue->ultra_ops[idx++]); + auto expected = commitment_key->commit(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 e20a8708e69..f2ef8d4490b 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp @@ -11,10 +11,10 @@ namespace bb { template MergeProver_::MergeProver_(const std::shared_ptr& op_queue) : op_queue(op_queue) - , pcs_commitment_key(std::make_shared(op_queue->ultra_ops[0].size())) { // 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 = std::make_shared(op_queue->current_ultra_ops_size); } /** From 33fc34c47b5af3b059fc6e47288e0d06bdf3f7c8 Mon Sep 17 00:00:00 2001 From: maramihali Date: Fri, 8 Mar 2024 13:04:55 +0000 Subject: [PATCH 3/4] rename from review --- .../circuit_builder/eccvm/eccvm_circuit_builder.test.cpp | 6 +++--- .../cpp/src/barretenberg/ultra_honk/merge_prover.cpp | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp index 9df0ecf6430..b68a470111f 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp @@ -255,7 +255,7 @@ TYPED_TEST(ECCVMCircuitBuilderTests, MSM) static constexpr size_t max_num_msms = 9; auto generators = G1::derive_generators("test generators", max_num_msms); - const auto try_msms = [&](const size_t num_msms, auto& op_queue) { + const auto compute_msms = [&](const size_t num_msms, auto& op_queue) { std::vector points; std::vector scalars; typename G1::element expected = G1::point_at_infinity; @@ -273,7 +273,7 @@ TYPED_TEST(ECCVMCircuitBuilderTests, MSM) using Flavor = TypeParam; std::shared_ptr op_queue = std::make_shared(); - try_msms(j, op_queue); + compute_msms(j, op_queue); ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); EXPECT_EQ(result, true); @@ -282,7 +282,7 @@ TYPED_TEST(ECCVMCircuitBuilderTests, MSM) std::shared_ptr op_queue = std::make_shared(); for (size_t j = 1; j < 9; ++j) { - try_msms(j, op_queue); + compute_msms(j, op_queue); } ECCVMCircuitBuilder circuit{ op_queue }; bool result = circuit.check_circuit(); diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp index f2ef8d4490b..be515b69c58 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp @@ -14,6 +14,7 @@ MergeProver_::MergeProver_(const std::shared_ptr& 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(); + // Get the appropriate commitment based on the updated ultra ops size pcs_commitment_key = std::make_shared(op_queue->current_ultra_ops_size); } From 504534cfe8a49921278213f099e3e2b79837f8ce Mon Sep 17 00:00:00 2001 From: maramihali Date: Fri, 8 Mar 2024 13:21:16 +0000 Subject: [PATCH 4/4] remanimng and comment from review --- .../proof_system/op_queue/ecc_op_queue.hpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp index 3f433e06543..3443fbdd2e0 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp @@ -7,7 +7,7 @@ namespace bb { enum EccOpCode { NULL_OP, ADD_ACCUM, MUL_ACCUM, EQUALITY }; -struct UltraOpTuple { +struct UltraOp { using Fr = curve::BN254::ScalarField; Fr op; Fr x_lo; @@ -284,7 +284,13 @@ class ECCOpQueue { }); } - void populate_ultra_ops(UltraOpTuple tuple) + /** + * @brief Populate two rows of the ultra ops,representing a complete ECC operation. Note that this has 7 inputs so + * the second row of ultra_ops[0] (storing the opcodes) will be set to 0. + * + * @param tuple + */ + void populate_ultra_ops(UltraOp tuple) { ultra_ops[0].emplace_back(tuple.op); ultra_ops[1].emplace_back(tuple.x_lo);