fix: edge case in the ECCVM related to splitting scalars (#16816)

Addresses [Issue
1529](AztecProtocol/barretenberg#1529), which
found an edge case which caused the ECCVM prover to fail. There was just
a `<=128` where there should have been a `<128`. Adds a bit of testing
for the size of the `splitting_scalar_with_endomorphism`method.

---------

Co-authored-by: notnotraju <[email protected]>

Author: notnotraju

barretenberg/cpp/src/barretenberg/ecc/curves/bn254/fq.test.cpp

@@ -385,6 +385,9 @@ TEST(fq, SplitIntoEndomorphismScalars)
     k1.self_to_montgomery_form();
     k2.self_to_montgomery_form();
 
+    EXPECT_LT(uint256_t(k1).get_msb(), 128);
+    EXPECT_LT(uint256_t(k2).get_msb(), 128);
+
     result = k2 * fq::cube_root_of_unity();
     result = k1 - result;
 
@@ -407,6 +410,37 @@ TEST(fq, SplitIntoEndomorphismScalarsSimple)
     k1.self_to_montgomery_form();
     k2.self_to_montgomery_form();
 
+    EXPECT_LT(uint256_t(k1).get_msb(), 128);
+    EXPECT_LT(uint256_t(k2).get_msb(), 128);
+
+    fq beta = fq::cube_root_of_unity();
+    result = k2 * beta;
+    result = k1 - result;
+
+    result.self_from_montgomery_form();
+    for (size_t i = 0; i < 4; ++i) {
+        EXPECT_EQ(result.data[i], k.data[i]);
+    }
+}
+
+TEST(fq, SplitIntoEndomorphismEdgeCase)
+{
+
+    fq input = { 0, 0, 1, 0 }; // 2^128
+    fq k = { 0, 0, 0, 0 };
+    fq k1 = { 0, 0, 0, 0 };
+    fq k2 = { 0, 0, 0, 0 };
+    fq::__copy(input, k);
+
+    fq::split_into_endomorphism_scalars(k, k1, k2);
+
+    fq result{ 0, 0, 0, 0 };
+    k1.self_to_montgomery_form();
+    k2.self_to_montgomery_form();
+
+    EXPECT_LT(uint256_t(k1).get_msb(), 128);
+    EXPECT_LT(uint256_t(k2).get_msb(), 128);
+
     fq beta = fq::cube_root_of_unity();
     result = k2 * beta;
     result = k1 - result;

barretenberg/cpp/src/barretenberg/eccvm/eccvm.fuzzer.cpp

@@ -186,27 +186,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size)
 
             // Use pre-computed scalar selected by scalar_indices
             Fr scalar = precomputed_scalars[op.scalar_index % precomputed_scalars.size()];
-            // TODO(@Rumata888): remove this if we fix the completeness issue
-            // Convert scalar to endomorphism scalars and check that none exceed 128 bits
-            auto converted = scalar.from_montgomery_form();
-            uint256_t converted_u256(scalar);
-            uint256_t k1_u256, k2_u256;
-            Fr z_1 = 0;
-            Fr z_2 = 0;
-
-            if (converted_u256.get_msb() <= 128) {
-                k1_u256 = converted_u256;
-                k2_u256 = 0;
-            } else {
-                bb::fr::split_into_endomorphism_scalars(converted, z_1, z_2);
-                k1_u256 = uint256_t(z_1.to_montgomery_form());
-                k2_u256 = uint256_t(z_2.to_montgomery_form());
-            }
-
-            if (k1_u256.get_msb() >= 128 || k2_u256.get_msb() >= 128) {
-                // Skip this operation if endomorphism scalars are too large
-                continue;
-            }
 
             typename G1::element point_to_multiply = points[generator_index];
             if (should_negate) {

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

@@ -127,6 +127,30 @@ TEST_F(ECCVMTests, Zeroes)
 
     ASSERT_TRUE(verified);
 }
+TEST_F(ECCVMTests, ScalarEdgeCase)
+{
+    using Curve = curve::BN254;
+    using G1 = Curve::Element;
+    using Fr = Curve::ScalarField;
+
+    std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
+    G1 a = G1::one();
+
+    op_queue->mul_accumulate(a, Fr(uint256_t(1) << 128));
+    op_queue->eq_and_reset();
+    op_queue->merge();
+    ECCVMCircuitBuilder builder{ op_queue };
+
+    std::shared_ptr<Transcript> prover_transcript = std::make_shared<Transcript>();
+    ECCVMProver prover(builder, prover_transcript);
+    ECCVMProof proof = prover.construct_proof();
+
+    std::shared_ptr<Transcript> verifier_transcript = std::make_shared<Transcript>();
+    ECCVMVerifier verifier(verifier_transcript);
+    bool verified = verifier.verify_proof(proof);
+
+    ASSERT_TRUE(verified);
+}
 /**
  * @brief Check that size of a ECCVM proof matches the corresponding constant
  *@details If this test FAILS, then the following (non-exhaustive) list should probably be updated as well:

barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp

@@ -311,7 +311,7 @@ class ECCOpQueue {
         Fr z_2 = 0;
         auto converted = scalar.from_montgomery_form();
         uint256_t converted_u256(scalar);
-        if (converted_u256.get_msb() <= 128) {
+        if (converted_u256.get_msb() < 128) {
             ultra_op.z_1 = scalar;
             ultra_op.z_2 = 0;
         } else {