From cb9d1d4fb39e0f180f59f0a3fd321ee324577f33 Mon Sep 17 00:00:00 2001
From: ledwards2225 <l.edwards.d@gmail.com>
Date: Tue, 19 Sep 2023 21:19:12 +0000
Subject: [PATCH 1/2] fix and reenable some honk ultra composer tests

---
 .../honk/composer/ultra_composer.test.cpp     | 147 ++++++++----------
 1 file changed, 63 insertions(+), 84 deletions(-)

diff --git a/barretenberg/cpp/src/barretenberg/honk/composer/ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/honk/composer/ultra_composer.test.cpp
index 65a15a2249f..6daa41d2f14 100644
--- a/barretenberg/cpp/src/barretenberg/honk/composer/ultra_composer.test.cpp
+++ b/barretenberg/cpp/src/barretenberg/honk/composer/ultra_composer.test.cpp
@@ -51,6 +51,9 @@ void ensure_non_zero(auto& polynomial)
 }
 
 class UltraHonkComposerTests : public ::testing::Test {
+  public:
+    using fr = barretenberg::fr;
+
   protected:
     static void SetUpTestSuite() { barretenberg::srs::init_crs_factory("../srs_db/ignition"); }
 };
@@ -91,7 +94,6 @@ TEST_F(UltraHonkComposerTests, ANonZeroPolynomialIsAGoodPolynomial)
  */
 TEST_F(UltraHonkComposerTests, PublicInputs)
 {
-    using fr = barretenberg::fr;
     auto builder = proof_system::UltraCircuitBuilder();
     size_t num_gates = 10;
 
@@ -116,7 +118,6 @@ TEST_F(UltraHonkComposerTests, PublicInputs)
 
 TEST_F(UltraHonkComposerTests, XorConstraint)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
 
     uint32_t left_value = engine.get_random_uint32();
@@ -145,7 +146,6 @@ TEST_F(UltraHonkComposerTests, XorConstraint)
 
 TEST_F(UltraHonkComposerTests, create_gates_from_plookup_accumulators)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
 
     barretenberg::fr input_value = fr::random_element();
@@ -241,7 +241,6 @@ TEST_F(UltraHonkComposerTests, create_gates_from_plookup_accumulators)
 
 TEST_F(UltraHonkComposerTests, test_no_lookup_proof)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
 
     for (size_t i = 0; i < 16; ++i) {
@@ -303,7 +302,6 @@ TEST_F(UltraHonkComposerTests, test_elliptic_gate)
 
 TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     fr a = fr::random_element();
     fr b = -a;
@@ -332,7 +330,6 @@ TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation)
 
 TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation_and_cycles)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     fr a = fr::random_element();
     fr c = -a;
@@ -371,7 +368,6 @@ TEST_F(UltraHonkComposerTests, non_trivial_tag_permutation_and_cycles)
 
 TEST_F(UltraHonkComposerTests, bad_tag_permutation)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     fr a = fr::random_element();
     fr b = -a;
@@ -399,7 +395,6 @@ TEST_F(UltraHonkComposerTests, bad_tag_permutation)
 // same as above but with turbocomposer to check reason of failue is really tag mismatch
 TEST_F(UltraHonkComposerTests, bad_tag_turbo_permutation)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     fr a = fr::random_element();
     fr b = -a;
@@ -425,7 +420,6 @@ TEST_F(UltraHonkComposerTests, bad_tag_turbo_permutation)
 
 TEST_F(UltraHonkComposerTests, sort_widget)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     fr a = fr::one();
     fr b = fr(2);
@@ -444,8 +438,6 @@ TEST_F(UltraHonkComposerTests, sort_widget)
 
 TEST_F(UltraHonkComposerTests, sort_with_edges_gate)
 {
-    using fr = barretenberg::fr;
-
     fr a = fr::one();
     fr b = fr(2);
     fr c = fr(3);
@@ -617,7 +609,6 @@ TEST_F(UltraHonkComposerTests, range_constraint)
 
 TEST_F(UltraHonkComposerTests, range_with_gates)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     auto idx = add_variables(circuit_builder, { 1, 2, 3, 4, 5, 6, 7, 8 });
     for (size_t i = 0; i < idx.size(); i++) {
@@ -637,7 +628,6 @@ TEST_F(UltraHonkComposerTests, range_with_gates)
 
 TEST_F(UltraHonkComposerTests, range_with_gates_where_range_is_not_a_power_of_two)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     auto idx = add_variables(circuit_builder, { 1, 2, 3, 4, 5, 6, 7, 8 });
     for (size_t i = 0; i < idx.size(); i++) {
@@ -657,7 +647,6 @@ TEST_F(UltraHonkComposerTests, range_with_gates_where_range_is_not_a_power_of_tw
 
 TEST_F(UltraHonkComposerTests, sort_widget_complex)
 {
-    using fr = barretenberg::fr;
     {
 
         auto circuit_builder = proof_system::UltraCircuitBuilder();
@@ -686,7 +675,6 @@ TEST_F(UltraHonkComposerTests, sort_widget_complex)
 
 TEST_F(UltraHonkComposerTests, sort_widget_neg)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     fr a = fr::one();
     fr b = fr(2);
@@ -705,7 +693,6 @@ TEST_F(UltraHonkComposerTests, sort_widget_neg)
 
 TEST_F(UltraHonkComposerTests, composed_range_constraint)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
     auto c = fr::random_element();
     auto d = uint256_t(c).slice(0, 133);
@@ -720,7 +707,6 @@ TEST_F(UltraHonkComposerTests, composed_range_constraint)
 
 TEST_F(UltraHonkComposerTests, non_native_field_multiplication)
 {
-    using fr = barretenberg::fr;
     using fq = barretenberg::fq;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
 
@@ -778,7 +764,6 @@ TEST_F(UltraHonkComposerTests, non_native_field_multiplication)
 
 TEST_F(UltraHonkComposerTests, rom)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
 
     uint32_t rom_values[8]{
@@ -821,7 +806,6 @@ TEST_F(UltraHonkComposerTests, rom)
 
 TEST_F(UltraHonkComposerTests, ram)
 {
-    using fr = barretenberg::fr;
     auto circuit_builder = proof_system::UltraCircuitBuilder();
 
     uint32_t ram_values[8]{
@@ -884,6 +868,66 @@ TEST_F(UltraHonkComposerTests, ram)
     prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
 }
 
+TEST_F(UltraHonkComposerTests, range_checks_on_duplicates)
+{
+    auto circuit_builder = proof_system::UltraCircuitBuilder();
+
+    uint32_t a = circuit_builder.add_variable(100);
+    uint32_t b = circuit_builder.add_variable(100);
+    uint32_t c = circuit_builder.add_variable(100);
+    uint32_t d = circuit_builder.add_variable(100);
+
+    circuit_builder.assert_equal(a, b);
+    circuit_builder.assert_equal(a, c);
+    circuit_builder.assert_equal(a, d);
+
+    circuit_builder.create_new_range_constraint(a, 1000);
+    circuit_builder.create_new_range_constraint(b, 1001);
+    circuit_builder.create_new_range_constraint(c, 999);
+    circuit_builder.create_new_range_constraint(d, 1000);
+
+    circuit_builder.create_big_add_gate(
+        {
+            a,
+            b,
+            c,
+            d,
+            0,
+            0,
+            0,
+            0,
+            0,
+        },
+        false);
+
+    auto composer = UltraComposer();
+    prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
+}
+
+// Ensure copy constraints added on variables smaller than 2^14, which have been previously
+// range constrained, do not break the set equivalence checks because of indices mismatch.
+// 2^14 is DEFAULT_PLOOKUP_RANGE_BITNUM i.e. the maximum size before a variable gets sliced
+// before range constraints are applied to it.
+TEST_F(UltraHonkComposerTests, range_constraint_small_variable)
+{
+    auto circuit_builder = proof_system::UltraCircuitBuilder();
+
+    uint16_t mask = (1 << 8) - 1;
+    int a = engine.get_random_uint16() & mask;
+    uint32_t a_idx = circuit_builder.add_variable(fr(a));
+    uint32_t b_idx = circuit_builder.add_variable(fr(a));
+    ASSERT_NE(a_idx, b_idx);
+    uint32_t c_idx = circuit_builder.add_variable(fr(a));
+    ASSERT_NE(c_idx, b_idx);
+    circuit_builder.create_range_constraint(b_idx, 8, "bad range");
+    circuit_builder.assert_equal(a_idx, b_idx);
+    circuit_builder.create_range_constraint(c_idx, 8, "bad range");
+    circuit_builder.assert_equal(a_idx, c_idx);
+
+    auto composer = UltraComposer();
+    prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
+}
+
 TEST(UltraGrumpkinHonkComposer, XorConstraint)
 {
     using fr = barretenberg::fr;
@@ -915,69 +959,4 @@ TEST(UltraGrumpkinHonkComposer, XorConstraint)
     auto composer = UltraGrumpkinComposer();
     prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
 }
-
-// TODO(#378)(luke): this is a recent update from Zac and fails; do we need a corresponding bug fix in ultra circuit
-// c_Fonstructor? TEST(UltraHonkComposerTests, range_checks_on_duplicates)
-// {
-//     auto composer = UltraComposer();
-
-//     uint32_t a = circuit_builder.add_variable(100);
-//     uint32_t b = circuit_builder.add_variable(100);
-//     uint32_t c = circuit_builder.add_variable(100);
-//     uint32_t d = circuit_builder.add_variable(100);
-
-//     circuit_builder.assert_equal(a, b);
-//     circuit_builder.assert_equal(a, c);
-//     circuit_builder.assert_equal(a, d);
-
-//     circuit_builder.create_new_range_constraint(a, 1000);
-//     circuit_builder.create_new_range_constraint(b, 1001);
-//     circuit_builder.create_new_range_constraint(c, 999);
-//     circuit_builder.create_new_range_constraint(d, 1000);
-
-//     circuit_builder.create_big_add_gate(
-//         {
-//             a,
-//             b,
-//             c,
-//             d,
-//             0,
-//             0,
-//             0,
-//             0,
-//             0,
-//         },
-//         false);
-
-//     prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
-// }
-
-// TODO(#378)(luke): this is a new test from Zac; ultra circuit constructor does not yet have create_range_constraint
-// implemented.
-// // Ensure copy constraints added on variables smaller than 2^14, which have been previously
-// // range constrained, do not break the set equivalence checks because of indices mismatch.
-// // 2^14 is DEFAULT_PLOOKUP_RANGE_BITNUM i.e. the maximum size before a variable gets sliced
-// // before range constraints are applied to it.
-// T_FEST(UltraHonkComposerTests, range_constraint_small_variable)
-// {
-//     auto composer = UltraComposer();
-//     uint16_t mask = (1 << 8) - 1;
-//     int a = engine.get_random_uint16() & mask;
-//     uint32_t a_idx = circuit_builder.add_variable(fr(a));
-//     uint32_t b_idx = circuit_builder.add_variable(fr(a));
-//     ASSERT_NE(a_idx, b_idx);
-//     uint32_t c_idx = circuit_builder.add_variable(fr(a));
-//     ASSERT_NE(c_idx, b_idx);
-//     composer.create_range_constraint(b_idx, 8, "bad range");
-//     circuit_builder.assert_equal(a_idx, b_idx);
-//     composer.create_range_constraint(c_idx, 8, "bad range");
-//     circuit_builder.assert_equal(a_idx, c_idx);
-
-//     auto prover = composer.create_prover(circuit_builder);
-//     auto proof = prover.construct_proof();
-//     auto verifier = composer.create_verifier(circuit_builder);
-//     bool result = verifier.verify_proof(proof);
-//     EXPECT_EQ(result, true);
-// }
-
 } // namespace test_ultra_honk_composer
\ No newline at end of file

From 6c94f50798e052ea84a147d65407f8ac559c5931 Mon Sep 17 00:00:00 2001
From: ledwards2225 <l.edwards.d@gmail.com>
Date: Tue, 19 Sep 2023 21:27:44 +0000
Subject: [PATCH 2/2] remove duplicated commented out code

---
 .../circuit_builder/ultra_circuit_builder.hpp | 134 ------------------
 1 file changed, 134 deletions(-)

diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
index f34a3a6dbb0..1e5deb37773 100644
--- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
+++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp
@@ -877,140 +877,6 @@ template <typename FF> class UltraCircuitBuilder_ : public CircuitBuilderBase<ar
                   << ", range " << rangecount << ", non native field gates " << nnfcount << ", goblin ecc op gates "
                   << num_ecc_op_gates << "), pubinp = " << this->public_inputs.size() << std::endl;
     }
-    // /**
-    //  * @brief Get the final number of gates in a circuit, which consists of the sum of:
-    //  * 1) Current number number of actual gates
-    //  * 2) Number of public inputs, as we'll need to add a gate for each of them
-    //  * 3) Number of Rom array-associated gates
-    //  * 4) Number of range-list associated gates
-    //  * 5) Number of non-native field multiplication gates.
-    //  *
-    //  *
-    //  * @param count return arument, number of existing gates
-    //  * @param rangecount return argument, extra gates due to range checks
-    //  * @param romcount return argument, extra gates due to rom reads
-    //  * @param ramcount return argument, extra gates due to ram read/writes
-    //  * @param nnfcount return argument, extra gates due to queued non native field gates
-    //  */
-    // void get_num_gates_split_into_components(
-    //     size_t& count, size_t& rangecount, size_t& romcount, size_t& ramcount, size_t& nnfcount) const
-    // {
-    //     count = num_gates;
-    //     // each ROM gate adds +1 extra gate due to the rom reads being copied to a sorted list set
-    //     for (size_t i = 0; i < rom_arrays.size(); ++i) {
-    //         for (size_t j = 0; j < rom_arrays[i].state.size(); ++j) {
-    //             if (rom_arrays[i].state[j][0] == UNINITIALIZED_MEMORY_RECORD) {
-    //                 romcount += 2;
-    //             }
-    //         }
-    //         romcount += (rom_arrays[i].records.size());
-    //         romcount += 1; // we add an addition gate after procesing a rom array
-    //     }
-
-    //     constexpr size_t gate_width = ultra_settings::program_width;
-    //     // each RAM gate adds +2 extra gates due to the ram reads being copied to a sorted list set,
-    //     // as well as an extra gate to validate timestamps
-    //     std::vector<size_t> ram_timestamps;
-    //     std::vector<size_t> ram_range_sizes;
-    //     std::vector<size_t> ram_range_exists;
-    //     for (size_t i = 0; i < ram_arrays.size(); ++i) {
-    //         for (size_t j = 0; j < ram_arrays[i].state.size(); ++j) {
-    //             if (ram_arrays[i].state[j] == UNINITIALIZED_MEMORY_RECORD) {
-    //                 ramcount += NUMBER_OF_GATES_PER_RAM_ACCESS;
-    //             }
-    //         }
-    //         ramcount += (ram_arrays[i].records.size() * NUMBER_OF_GATES_PER_RAM_ACCESS);
-    //         ramcount += NUMBER_OF_ARITHMETIC_GATES_PER_RAM_ARRAY; // we add an addition gate after procesing a
-    //         ram array
-
-    //         // there will be 'max_timestamp' number of range checks, need to calculate.
-    //         const auto max_timestamp = ram_arrays[i].access_count - 1;
-
-    //         // if a range check of length `max_timestamp` already exists, we are double counting.
-    //         // We record `ram_timestamps` to detect and correct for this error when we process range lists.
-    //         ram_timestamps.push_back(max_timestamp);
-    //         size_t padding = (gate_width - (max_timestamp % gate_width)) % gate_width;
-    //         if (max_timestamp == gate_width)
-    //             padding += gate_width;
-    //         const size_t ram_range_check_list_size = max_timestamp + padding;
-
-    //         size_t ram_range_check_gate_count = (ram_range_check_list_size / gate_width);
-    //         ram_range_check_gate_count += 1; // we need to add 1 extra addition gates for every distinct range
-    //         list
-
-    //         ram_range_sizes.push_back(ram_range_check_gate_count);
-    //         ram_range_exists.push_back(false);
-    //         // rangecount += ram_range_check_gate_count;
-    //     }
-    //     for (const auto& list : range_lists) {
-    //         auto list_size = list.second.variable_indices.size();
-    //         size_t padding = (gate_width - (list.second.variable_indices.size() % gate_width)) % gate_width;
-    //         if (list.second.variable_indices.size() == gate_width)
-    //             padding += gate_width;
-    //         list_size += padding;
-
-    //         for (size_t i = 0; i < ram_timestamps.size(); ++i) {
-    //             if (list.second.target_range == ram_timestamps[i]) {
-    //                 ram_range_exists[i] = true;
-    //             }
-    //         }
-    //         rangecount += (list_size / gate_width);
-    //         rangecount += 1; // we need to add 1 extra addition gates for every distinct range list
-    //     }
-    //     // update rangecount to include the ram range checks the composer will eventually be creating
-    //     for (size_t i = 0; i < ram_range_sizes.size(); ++i) {
-    //         if (!ram_range_exists[i]) {
-    //             rangecount += ram_range_sizes[i];
-    //         }
-    //     }
-    //     std::vector<cached_non_native_field_multiplication> nnf_copy(cached_non_native_field_multiplications);
-    //     // update nnfcount
-    //     std::sort(nnf_copy.begin(), nnf_copy.end());
-
-    //     auto last = std::unique(nnf_copy.begin(), nnf_copy.end());
-    //     const size_t num_nnf_ops = static_cast<size_t>(std::distance(nnf_copy.begin(), last));
-    //     nnfcount = num_nnf_ops * GATES_PER_NON_NATIVE_FIELD_MULTIPLICATION_ARITHMETIC;
-    // }
-
-    // /**
-    //  * @brief Get the final number of gates in a circuit, which consists of the sum of:
-    //  * 1) Current number number of actual gates
-    //  * 2) Number of public inputs, as we'll need to add a gate for each of them
-    //  * 3) Number of Rom array-associated gates
-    //  * 4) Number of range-list associated gates
-    //  * 5) Number of non-native field multiplication gates.
-    //  *
-    //  * @return size_t
-    //  */
-    // virtual size_t get_num_gates() const override
-    // {
-    //     // if circuit finalised already added extra gates
-    //     if (circuit_finalised) {
-    //         return num_gates;
-    //     }
-    //     size_t count = 0;
-    //     size_t rangecount = 0;
-    //     size_t romcount = 0;
-    //     size_t ramcount = 0;
-    //     size_t nnfcount = 0;
-    //     get_num_gates_split_into_components(count, rangecount, romcount, ramcount, nnfcount);
-    //     return count + romcount + ramcount + rangecount + nnfcount;
-    // }
-
-    // virtual void print_num_gates() const override
-    // {
-    //     size_t count = 0;
-    //     size_t rangecount = 0;
-    //     size_t romcount = 0;
-    //     size_t ramcount = 0;
-    //     size_t nnfcount = 0;
-    //     get_num_gates_split_into_components(count, rangecount, romcount, ramcount, nnfcount);
-
-    //     size_t total = count + romcount + ramcount + rangecount;
-    //     std::cout << "gates = " << total << " (arith " << count << ", rom " << romcount << ", ram " << ramcount
-    //               << ", range " << rangecount << ", non native field gates " << nnfcount
-    //               << "), pubinp = " << public_inputs.size() << std::endl;
-    // }
 
     void assert_equal_constant(const uint32_t a_idx, const FF& b, std::string const& msg = "assert equal constant")
     {