chore: cycle group cleanup #5

barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_ultra_recursive_verifier.test.cpp

@@ -136,7 +136,7 @@ template <typename RecursiveFlavor> class BoomerangRecursiveVerifierTest : publi
         outer_circuit.finalize_circuit(false);
         auto graph = cdg::StaticAnalyzer(outer_circuit);
         auto connected_components = graph.find_connected_components();
-        EXPECT_EQ(connected_components.size(), 2);
+        EXPECT_EQ(connected_components.size(), 5);
         info("Connected components: ", connected_components.size());
         auto variables_in_one_gate = graph.show_variables_in_one_gate(outer_circuit);
         EXPECT_EQ(variables_in_one_gate.size(), 2);

barretenberg/cpp/src/barretenberg/stdlib/eccvm_verifier/eccvm_recursive_verifier.test.cpp

@@ -139,7 +139,7 @@ class ECCVMRecursiveTests : public ::testing::Test {
         }
 
         // Check that the size of the recursive verifier is consistent with historical expectation
-        uint32_t NUM_GATES_EXPECTED = 213923;
+        uint32_t NUM_GATES_EXPECTED = 213307;
         ASSERT_EQ(static_cast<uint32_t>(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED)
             << "Ultra-arithmetized ECCVM Recursive verifier gate count changed! Update this value if you are sure this "
                "is expected.";

barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp

@@ -291,7 +291,7 @@ template <typename RecursiveFlavor> class RecursiveVerifierTest : public testing
         }
         // Check the size of the recursive verifier
         if constexpr (std::same_as<RecursiveFlavor, MegaZKRecursiveFlavor_<UltraCircuitBuilder>>) {
-            uint32_t NUM_GATES_EXPECTED = 796978;
+            uint32_t NUM_GATES_EXPECTED = 796237;
             ASSERT_EQ(static_cast<uint32_t>(outer_circuit.get_num_finalized_gates()), NUM_GATES_EXPECTED)
                 << "MegaZKHonk Recursive verifier changed in Ultra gate count! Update this value if you "
                    "are sure this is expected.";

barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp

@@ -1260,14 +1260,43 @@ template <typename Builder> field_t<Builder> field_t<Builder>::accumulate(const
     return total.normalize();
 }
 
+// WORKTODO: better name!
+template <typename Builder>
+std::pair<field_t<Builder>, field_t<Builder>> field_t<Builder>::split_at(const size_t lsb_index) const
+{
+    // WORKTODO: is this right? depends on how bb::fr is implemented. ideally this would be some predefined constant..
+    const size_t max_bits = 254;
+    ASSERT(lsb_index < max_bits);
+
+    const uint256_t value(this->get_value());
+    const uint256_t lo_val = value.slice(0, lsb_index);
+    const uint256_t hi_val = value.slice(lsb_index, max_bits);
+
+    // If `this` is constant, return constants based on the native hi/lo values
+    if (this->is_constant()) {
+        return { field_t(lo_val), field_t(hi_val) };
+    }
+
+    // Otherwise, create hi/lo witnesses and constrain them to reconstruct `this` as field elements
+    field_t lo = from_witness(get_context(), lo_val);
+    field_t hi = from_witness(get_context(), hi_val);
+    const uint256_t shift = uint256_t(1) << lsb_index;
+    (lo + (hi * shift)).assert_equal(*this);
+
+    // Set the origin tag for the limbs to be that of the original field element
+    lo.set_origin_tag(get_origin_tag());
+    hi.set_origin_tag(get_origin_tag());
+    return { lo, hi };
+}
+
 /**
  * @brief Splits the field element into (lo, hi), where:
  * - lo contains bits [0, lsb_index)
  * - hi contains bits [lsb_index, num_bits)
  */
 template <typename Builder>
-std::pair<field_t<Builder>, field_t<Builder>> field_t<Builder>::split_at(const size_t lsb_index,
-                                                                         const size_t num_bits) const
+std::pair<field_t<Builder>, field_t<Builder>> field_t<Builder>::no_wrap_split_at(const size_t lsb_index,
+                                                                                 const size_t num_bits) const
 {
     ASSERT(lsb_index < num_bits);
     ASSERT(num_bits <= grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH);

barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp

@@ -388,7 +388,9 @@ template <typename Builder_> class field_t {
 
     Builder* get_context() const { return context; }
 
-    std::pair<field_t<Builder>, field_t<Builder>> split_at(
+    std::pair<field_t<Builder>, field_t<Builder>> split_at(const size_t lsb_index) const;
+
+    std::pair<field_t<Builder>, field_t<Builder>> no_wrap_split_at(
         const size_t lsb_index, const size_t num_bits = grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) const;
 
     bool_t<Builder> is_zero() const;

barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp

@@ -930,7 +930,7 @@ template <typename Builder> class stdlib_field : public testing::Test {
         // Lambda to check split_at functionality
         auto check_split_at = [&](const field_ct& a, size_t start, size_t num_bits) {
             const uint256_t a_native = a.get_value();
-            auto split_data = a.split_at(start, num_bits);
+            auto split_data = a.no_wrap_split_at(start, num_bits);
             EXPECT_EQ(split_data.first.get_value(), a_native & ((uint256_t(1) << start) - 1));
             EXPECT_EQ(split_data.second.get_value(), (a_native >> start) & ((uint256_t(1) << num_bits) - 1));
 
@@ -1424,7 +1424,7 @@ template <typename Builder> class stdlib_field : public testing::Test {
 
         // Split preserves tags
         const size_t num_bits = uint256_t(a.get_value()).get_msb() + 1;
-        auto split_data = a.split_at(num_bits / 2, num_bits);
+        auto split_data = a.no_wrap_split_at(num_bits / 2, num_bits);
         EXPECT_EQ(split_data.first.get_origin_tag(), submitted_value_origin_tag);
         EXPECT_EQ(split_data.second.get_origin_tag(), submitted_value_origin_tag);
 

barretenberg/cpp/src/barretenberg/stdlib/primitives/group/cycle_group.test.cpp

@@ -20,7 +20,8 @@
     using Group = typename Curve::Group;                                                                               \
     using bool_ct = stdlib::bool_t<Builder>;                                                                           \
     using witness_ct = stdlib::witness_t<Builder>;                                                                     \
-    using cycle_scalar_ct = cycle_group_ct::cycle_scalar;
+    using cycle_scalar_ct = cycle_group_ct::cycle_scalar;                                                              \
+    using BigScalarField = typename cycle_scalar_ct::BigScalarField;
 
 using namespace bb;
 
@@ -771,22 +772,26 @@ TYPED_TEST(CycleGroupTest, TestBatchMulGeneralMSM)
         // 1: add entry where point, scalar are witnesses
         expected += (element * scalar);
         points.emplace_back(cycle_group_ct::from_witness(&builder, element));
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar1(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar1));
 
         // 2: add entry where point is constant, scalar is witness
         expected += (element * scalar);
         points.emplace_back(cycle_group_ct(element));
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar2(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar2));
 
         // 3: add entry where point is witness, scalar is constant
         expected += (element * scalar);
         points.emplace_back(cycle_group_ct::from_witness(&builder, element));
-        scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar));
+        BigScalarField big_scalar3(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar3));
 
         // 4: add entry where point is constant, scalar is constant
         expected += (element * scalar);
         points.emplace_back(cycle_group_ct(element));
-        scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar));
+        BigScalarField big_scalar4(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar4));
     }
 
     // Here and in the following cases assign different tags to points and scalars and get the union of them back
@@ -813,10 +818,12 @@ TYPED_TEST(CycleGroupTest, TestBatchMulProducesInfinity)
     auto element = TestFixture::generators[0];
     typename Group::Fr scalar = Group::Fr::random_element(&engine);
     points.emplace_back(cycle_group_ct::from_witness(&builder, element));
-    scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+    BigScalarField big_scalar1(&builder, scalar);
+    scalars.emplace_back(cycle_scalar_ct(big_scalar1));
 
     points.emplace_back(cycle_group_ct::from_witness(&builder, element));
-    scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, -scalar));
+    BigScalarField big_scalar2(&builder, -scalar);
+    scalars.emplace_back(cycle_scalar_ct(big_scalar2));
 
     const auto expected_tag = assign_and_merge_tags(points, scalars);
 
@@ -841,7 +848,8 @@ TYPED_TEST(CycleGroupTest, TestBatchMulMultiplyByZero)
     auto element = TestFixture::generators[0];
     typename Group::Fr scalar = 0;
     points.emplace_back(cycle_group_ct::from_witness(&builder, element));
-    scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+    BigScalarField big_scalar(&builder, scalar);
+    scalars.emplace_back(cycle_scalar_ct(big_scalar));
 
     const auto expected_tag = assign_and_merge_tags(points, scalars);
     auto result = cycle_group_ct::batch_mul(points, scalars);
@@ -869,14 +877,16 @@ TYPED_TEST(CycleGroupTest, TestBatchMulInputsAreInfinity)
         cycle_group_ct point = cycle_group_ct::from_witness(&builder, element);
         point.set_point_at_infinity(witness_ct(&builder, true));
         points.emplace_back(point);
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar1(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar1));
     }
     // is_infinity = constant
     {
         cycle_group_ct point = cycle_group_ct::from_witness(&builder, element);
         point.set_point_at_infinity(true);
         points.emplace_back(point);
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar2(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar2));
     }
 
     const auto expected_tag = assign_and_merge_tags(points, scalars);
@@ -907,14 +917,16 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseInLookupTable)
         // 1: add entry where point is constant, scalar is witness
         expected += (element * scalar);
         points.emplace_back(element);
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar));
         scalars_native.emplace_back(uint256_t(scalar));
 
         // 2: add entry where point is constant, scalar is constant
         element = plookup::fixed_base::table::rhs_generator_point();
         expected += (element * scalar);
         points.emplace_back(element);
-        scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar));
+        BigScalarField big_scalar_const(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar_const));
         scalars_native.emplace_back(uint256_t(scalar));
     }
     const auto expected_tag = assign_and_merge_tags(points, scalars);
@@ -946,22 +958,25 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseSomeInLookupTable)
         // 1: add entry where point is constant, scalar is witness
         expected += (element * scalar);
         points.emplace_back(element);
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar));
         scalars_native.emplace_back(scalar);
 
         // 2: add entry where point is constant, scalar is constant
         element = plookup::fixed_base::table::rhs_generator_point();
         expected += (element * scalar);
         points.emplace_back(element);
-        scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar));
+        BigScalarField big_scalar_const(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar_const));
         scalars_native.emplace_back(scalar);
 
         // 3: add entry where point is constant, scalar is witness
         scalar = Group::Fr::random_element(&engine);
         element = Group::one * Group::Fr::random_element(&engine);
         expected += (element * scalar);
         points.emplace_back(element);
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar2(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar2));
         scalars_native.emplace_back(scalar);
     }
     const auto expected_tag = assign_and_merge_tags(points, scalars);
@@ -989,11 +1004,13 @@ TYPED_TEST(CycleGroupTest, TestBatchMulFixedBaseZeroScalars)
 
         // 1: add entry where point is constant, scalar is witness
         points.emplace_back((element));
-        scalars.emplace_back(cycle_group_ct::cycle_scalar::from_witness(&builder, scalar));
+        BigScalarField big_scalar(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar));
 
         // 2: add entry where point is constant, scalar is constant
         points.emplace_back((element));
-        scalars.emplace_back(typename cycle_group_ct::cycle_scalar(scalar));
+        BigScalarField big_scalar_const(&builder, scalar);
+        scalars.emplace_back(cycle_scalar_ct(big_scalar_const));
     }
     const auto expected_tag = assign_and_merge_tags(points, scalars);
     auto result = cycle_group_ct::batch_mul(points, scalars);
@@ -1014,7 +1031,6 @@ TYPED_TEST(CycleGroupTest, TestMul)
     // case 1, general MSM with inputs that are combinations of constant and witnesses
     {
         cycle_group_ct point;
-        typename cycle_group_ct::cycle_scalar scalar;
         cycle_group_ct result;
         for (size_t i = 0; i < num_muls; ++i) {
             auto element = TestFixture::generators[i];
@@ -1023,7 +1039,8 @@ TYPED_TEST(CycleGroupTest, TestMul)
 
             // 1: add entry where point, scalar are witnesses
             point = (cycle_group_ct::from_witness(&builder, element));
-            scalar = (cycle_group_ct::cycle_scalar::from_witness(&builder, native_scalar));
+            BigScalarField big_scalar1(&builder, native_scalar);
+            cycle_scalar_ct scalar = (cycle_scalar_ct(big_scalar1));
             point.set_origin_tag(submitted_value_origin_tag);
             scalar.set_origin_tag(challenge_origin_tag);
             result = point * scalar;
@@ -1032,16 +1049,21 @@ TYPED_TEST(CycleGroupTest, TestMul)
 
             // 2: add entry where point is constant, scalar is witness
             point = (cycle_group_ct(element));
-            scalar = (cycle_group_ct::cycle_scalar::from_witness(&builder, native_scalar));
+            BigScalarField big_scalar2(&builder, native_scalar);
+            scalar = (cycle_scalar_ct(big_scalar2));
 
             EXPECT_EQ((result).get_value(), (expected_result));
 
             // 3: add entry where point is witness, scalar is constant
             point = (cycle_group_ct::from_witness(&builder, element));
+            BigScalarField big_scalar3(&builder, native_scalar);
+            scalar = (cycle_scalar_ct(big_scalar3));
             EXPECT_EQ((result).get_value(), (expected_result));
 
             // 4: add entry where point is constant, scalar is constant
             point = (cycle_group_ct(element));
+            BigScalarField big_scalar4(&builder, native_scalar);
+            scalar = (cycle_scalar_ct(big_scalar4));
             EXPECT_EQ((result).get_value(), (expected_result));
         }
     }
@@ -1156,7 +1178,8 @@ TYPED_TEST(CycleGroupTest, MixedLengthScalarsIsNotSupported)
 
     // First scalar: 256 bits
     uint256_t scalar1_value = uint256_t(123456789);
-    scalars.push_back(cycle_group_ct::cycle_scalar::from_witness(&builder, typename Curve::ScalarField(scalar1_value)));
+    BigScalarField big_scalar1(&builder, typename Curve::ScalarField(scalar1_value));
+    scalars.push_back(cycle_scalar_ct(big_scalar1));
 
     // Second scalar: 128 bits
     uint256_t scalar2_value = uint256_t(987654321);
@@ -1188,8 +1211,10 @@ TYPED_TEST(CycleGroupTest, TestFixedBaseBatchMul)
     auto scalar1_val = Group::Fr::random_element(&engine);
     auto scalar2_val = Group::Fr::random_element(&engine);
 
-    scalars.push_back(cycle_scalar_ct::from_witness(&builder, scalar1_val));
-    scalars.push_back(cycle_scalar_ct::from_witness(&builder, scalar2_val));
+    BigScalarField big_scalar1(&builder, scalar1_val);
+    BigScalarField big_scalar2(&builder, scalar2_val);
+    scalars.push_back(cycle_scalar_ct(big_scalar1));
+    scalars.push_back(cycle_scalar_ct(big_scalar2));
     points.push_back(cycle_group_ct(lhs_generator)); // constant point
     points.push_back(cycle_group_ct(rhs_generator)); // constant point