fix: ECCVM correctly handles points at infinity and group operation edge cases

barretenberg/cpp/src/barretenberg/benchmark/relations_bench/relations.bench.cpp

@@ -117,6 +117,7 @@ BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMPointTableRelation<Fq>>)
 BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMSetRelation<Fq>>);
 BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMTranscriptRelation<Fq>>);
 BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMWnafRelation<Fq>>);
+BENCHMARK(execute_relation_for_values<ECCVMFlavor, ECCVMBoolsRelation<Fq>>);
 
 } // namespace bb::benchmark::relations
 

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp

@@ -119,7 +119,7 @@ TEST_F(ClientIVCTests, BasicFailure)
  * @brief Prove and verify accumulation of an arbitrary set of circuits
  *
  */
-TEST_F(ClientIVCTests, DISABLED_BasicLarge)
+TEST_F(ClientIVCTests, BasicLarge)
 {
     ClientIVC ivc;
 
@@ -142,7 +142,7 @@ TEST_F(ClientIVCTests, DISABLED_BasicLarge)
  * @brief Using a structured trace allows for the accumulation of circuits of varying size
  *
  */
-TEST_F(ClientIVCTests, DISABLED_BasicStructured)
+TEST_F(ClientIVCTests, BasicStructured)
 {
     ClientIVC ivc;
     ivc.structured_flag = true;

barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp

@@ -120,15 +120,10 @@ class ECCVMCircuitBuilder {
         // populate opqueue and mul indices
         for (const auto& op : raw_ops) {
             if (op.mul) {
-                if (op.z1 != 0 || op.z2 != 0) {
+                if ((op.z1 != 0 || op.z2 != 0) && !op.base_point.is_point_at_infinity()) {
                     msm_opqueue_index.push_back(op_idx);
                     msm_mul_index.emplace_back(msm_count, active_mul_count);
-                }
-                if (op.z1 != 0) {
-                    active_mul_count++;
-                }
-                if (op.z2 != 0) {
-                    active_mul_count++;
+                    active_mul_count += static_cast<size_t>(op.z1 != 0) + static_cast<size_t>(op.z2 != 0);
                 }
             } else if (active_mul_count > 0) {
                 msm_sizes.push_back(active_mul_count);
@@ -138,7 +133,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) {
+        if (raw_ops.back().mul && active_mul_count > 0) {
             msm_sizes.push_back(active_mul_count);
             msm_count++;
         }
@@ -152,7 +147,7 @@ class ECCVMCircuitBuilder {
             for (size_t i = start; i < end; i++) {
                 const auto& op = raw_ops[msm_opqueue_index[i]];
                 auto [msm_index, mul_index] = msm_mul_index[i];
-                if (op.z1 != 0) {
+                if (op.z1 != 0 && !op.base_point.is_point_at_infinity()) {
                     ASSERT(result.size() > msm_index);
                     ASSERT(result[msm_index].size() > mul_index);
                     result[msm_index][mul_index] = (ScalarMul{
@@ -165,7 +160,7 @@ class ECCVMCircuitBuilder {
                     });
                     mul_index++;
                 }
-                if (op.z2 != 0) {
+                if (op.z2 != 0 && !op.base_point.is_point_at_infinity()) {
                     ASSERT(result.size() > msm_index);
                     ASSERT(result[msm_index].size() > mul_index);
                     auto endo_point = AffineElement{ op.base_point.x * FF::cube_root_of_unity(), -op.base_point.y };

barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp

@@ -18,8 +18,10 @@ TEST(ECCVMCircuitBuilderTests, BaseCase)
     typename G1::element a = generators[0];
     typename G1::element b = generators[1];
     typename G1::element c = generators[2];
+    typename G1::element point_at_infinity = G1::point_at_infinity;
     Fr x = Fr::random_element(&engine);
     Fr y = Fr::random_element(&engine);
+    Fr zero_scalar = 0;
 
     std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
 
@@ -29,14 +31,49 @@ TEST(ECCVMCircuitBuilderTests, BaseCase)
     op_queue->mul_accumulate(b, y);
     op_queue->add_accumulate(a);
     op_queue->mul_accumulate(b, x);
+    op_queue->no_op();
+    op_queue->add_accumulate(b);
     op_queue->eq_and_reset();
     op_queue->add_accumulate(c);
     op_queue->mul_accumulate(a, x);
+    op_queue->mul_accumulate(point_at_infinity, x);
     op_queue->mul_accumulate(b, x);
     op_queue->eq_and_reset();
     op_queue->mul_accumulate(a, x);
     op_queue->mul_accumulate(b, x);
+    op_queue->mul_accumulate(point_at_infinity, zero_scalar);
     op_queue->mul_accumulate(c, x);
+    op_queue->eq_and_reset();
+    op_queue->mul_accumulate(point_at_infinity, zero_scalar);
+    op_queue->mul_accumulate(point_at_infinity, x);
+    op_queue->mul_accumulate(point_at_infinity, zero_scalar);
+    op_queue->add_accumulate(a);
+    op_queue->eq_and_reset();
+    op_queue->add_accumulate(a);
+    op_queue->add_accumulate(point_at_infinity);
+    op_queue->eq_and_reset();
+    op_queue->add_accumulate(point_at_infinity);
+    op_queue->eq_and_reset();
+    op_queue->mul_accumulate(point_at_infinity, x);
+    op_queue->mul_accumulate(point_at_infinity, -x);
+    op_queue->eq_and_reset();
+    op_queue->add_accumulate(a);
+    op_queue->mul_accumulate(point_at_infinity, x);
+    op_queue->mul_accumulate(point_at_infinity, -x);
+    op_queue->add_accumulate(a);
+    op_queue->add_accumulate(a);
+    op_queue->eq_and_reset();
+
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+TEST(ECCVMCircuitBuilderTests, NoOp)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    op_queue->no_op();
 
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit, &engine);
@@ -72,6 +109,109 @@ TEST(ECCVMCircuitBuilderTests, Mul)
     EXPECT_EQ(result, true);
 }
 
+TEST(ECCVMCircuitBuilderTests, MulInfinity)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+    Fr x = Fr::random_element(&engine);
+    G1::element b = -a * x;
+    // G1::affine_element c = G1::affine_point_at_infinity;
+    op_queue->add_accumulate(b);
+    op_queue->mul_accumulate(a, x);
+    op_queue->eq_and_reset();
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+// Validate we do not trigger edge cases of addition formulae when we have identical mul inputs
+TEST(ECCVMCircuitBuilderTests, MulOverIdenticalInputs)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+    Fr x = Fr::random_element(&engine);
+    op_queue->mul_accumulate(a, x);
+    op_queue->mul_accumulate(a, x);
+    op_queue->eq_and_reset();
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+TEST(ECCVMCircuitBuilderTests, MSMProducesInfinity)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+    Fr x = Fr::random_element(&engine);
+    op_queue->add_accumulate(a);
+    op_queue->mul_accumulate(a, x);
+    op_queue->mul_accumulate(a, -x);
+    op_queue->eq_and_reset();
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+TEST(ECCVMCircuitBuilderTests, MSMOverPointAtInfinity)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element point_at_infinity = G1::point_at_infinity;
+    typename G1::element b = generators[0];
+    Fr x = Fr::random_element(&engine);
+    Fr zero_scalar = 0;
+
+    // validate including points at infinity in a multiscalar multiplication does not effect result
+    {
+        op_queue->mul_accumulate(b, x);
+        op_queue->mul_accumulate(point_at_infinity, x);
+        op_queue->eq_and_reset();
+        ECCVMCircuitBuilder circuit{ op_queue };
+        bool result = ECCVMTraceChecker::check(circuit);
+        EXPECT_EQ(result, true);
+    }
+    // validate multiplying a point at infinity by nonzero scalar produces point at infinity
+    {
+        op_queue->mul_accumulate(point_at_infinity, x);
+        op_queue->eq_and_reset();
+        ECCVMCircuitBuilder circuit{ op_queue };
+        bool result = ECCVMTraceChecker::check(circuit);
+        EXPECT_EQ(result, true);
+    }
+    // validate multiplying a point by zero produces point at infinity
+    {
+        op_queue->mul_accumulate(b, zero_scalar);
+        op_queue->eq_and_reset();
+        ECCVMCircuitBuilder circuit{ op_queue };
+        bool result = ECCVMTraceChecker::check(circuit);
+        EXPECT_EQ(result, true);
+    }
+    // validate multiplying a point at infinity by zero produces a point at infinity
+    {
+        op_queue->mul_accumulate(point_at_infinity, zero_scalar);
+        op_queue->eq_and_reset();
+        ECCVMCircuitBuilder circuit{ op_queue };
+        bool result = ECCVMTraceChecker::check(circuit);
+        EXPECT_EQ(result, true);
+    }
+    // validate an MSM made entirely of points at infinity / zero scalars produces a point at infinity
+    {
+        op_queue->mul_accumulate(point_at_infinity, x);
+        op_queue->mul_accumulate(b, zero_scalar);
+        op_queue->eq_and_reset();
+        ECCVMCircuitBuilder circuit{ op_queue };
+        bool result = ECCVMTraceChecker::check(circuit);
+        EXPECT_EQ(result, true);
+    }
+}
+
 TEST(ECCVMCircuitBuilderTests, ShortMul)
 {
     std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
@@ -243,3 +383,67 @@ TEST(ECCVMCircuitBuilderTests, MSM)
     bool result = ECCVMTraceChecker::check(circuit, &engine);
     EXPECT_EQ(result, true);
 }
+
+TEST(ECCVMCircuitBuilderTests, EqAgainstPointAtInfinity)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+    a.self_set_infinity();
+
+    op_queue->add_accumulate(a);
+    op_queue->eq_and_reset();
+
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+TEST(ECCVMCircuitBuilderTests, AddPointAtInfinity)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+    typename G1::element b = generators[0];
+    b.self_set_infinity();
+
+    op_queue->add_accumulate(a);
+    op_queue->add_accumulate(b);
+    op_queue->eq_and_reset();
+
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+TEST(ECCVMCircuitBuilderTests, AddProducesPointAtInfinity)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+
+    op_queue->add_accumulate(a);
+    op_queue->add_accumulate(-a);
+    op_queue->eq_and_reset();
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}
+
+TEST(ECCVMCircuitBuilderTests, AddProducesDouble)
+{
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+
+    auto generators = G1::derive_generators("test generators", 3);
+    typename G1::element a = generators[0];
+
+    op_queue->add_accumulate(a);
+    op_queue->add_accumulate(a);
+    op_queue->eq_and_reset();
+    ECCVMCircuitBuilder circuit{ op_queue };
+    bool result = ECCVMTraceChecker::check(circuit);
+    EXPECT_EQ(result, true);
+}