Lde/cleanup todos (#227)

* Address some todos, make issues for others, clean up others.

barretenberg/cpp/src/barretenberg/honk/circuit_constructors/circuit_constructor_base.hpp

@@ -12,7 +12,7 @@ template <size_t program_width_> class CircuitConstructorBase {
     static constexpr size_t program_width = program_width_;
     std::vector<std::string> selector_names_;
     size_t num_gates = 0;
-    // TODO(Adrian): It would be better to store an array of size program_width_
+    // TODO(#216)(Adrian): It would be better to store an array of size program_width_
     // to make the composer agnostic of the wire name.
     std::vector<uint32_t> w_l;
     std::vector<uint32_t> w_r;
@@ -30,7 +30,7 @@ template <size_t program_width_> class CircuitConstructorBase {
     uint32_t current_tag = DUMMY_TAG;
     // The permutation on variable tags. See
     // https://github.com/AztecProtocol/plonk-with-lookups-private/blob/new-stuff/GenPermuations.pdf
-    // DOCTODO: replace with the relevant wiki link.
+    // DOCTODO(#231): replace with the relevant wiki link.
     std::map<uint32_t, uint32_t> tau;
 
     size_t num_selectors;
@@ -42,7 +42,7 @@ template <size_t program_width_> class CircuitConstructorBase {
     static constexpr uint32_t FIRST_VARIABLE_IN_CLASS = UINT32_MAX - 2;
 
     // Enum values spaced in increments of 30-bits (multiples of 2 ** 30).
-    // TODO(Adrian): This is unused, and this type of hard coded data should be avoided
+    // TODO(#216)(Adrian): This is unused, and this type of hard coded data should be avoided
     // Cody: This is used by compute_wire_copy_cycles in Plonk.
     // enum WireType { LEFT = 0U, RIGHT = (1U << 30U), OUTPUT = (1U << 31U), FOURTH = 0xc0000000 };
 
@@ -65,7 +65,7 @@ template <size_t program_width_> class CircuitConstructorBase {
     virtual size_t get_num_gates() const { return num_gates; }
     virtual void print_num_gates() const { std::cout << num_gates << std::endl; }
     virtual size_t get_num_variables() const { return variables.size(); }
-    // TODO(Adrian): Feels wrong to let the zero_idx be changed.
+    // TODO(#216)(Adrian): Feels wrong to let the zero_idx be changed.
     uint32_t zero_idx = 0;
     uint32_t one_idx = 1;
 
@@ -197,8 +197,8 @@ template <size_t program_width_> class CircuitConstructorBase {
 
     virtual void assert_equal(const uint32_t a_idx, const uint32_t b_idx, std::string const& msg = "assert_equal");
 
-    // TODO(Adrian): This method should belong in the ComposerHelper, where the number of reserved gates can be
-    // correctly set. Cody: I don't know, this method is about circuit construction, seems like it should be here.
+    // TODO(#216)(Adrian): This method should belong in the ComposerHelper, where the number of reserved gates can be
+    // correctly set.
     size_t get_circuit_subgroup_size(const size_t num_gates) const
     {
         auto log2_n = static_cast<size_t>(numeric::get_msb(num_gates));
@@ -238,7 +238,7 @@ template <size_t program_width_> class CircuitConstructorBase {
 
 } // namespace bonk
 
-// TODO(Cody): This may need updating, to deal with the new gate we used to ensure that non multivariate is zero?
+// TODO(#217)(Cody): This will need updating based on the approach we take to ensure no multivariate is zero.
 /**
  * Composer Example: Pythagorean triples.
  *

barretenberg/cpp/src/barretenberg/honk/circuit_constructors/standard_circuit_constructor.hpp

@@ -13,13 +13,14 @@ inline std::vector<std::string> standard_selector_names()
 
 class StandardCircuitConstructor : public CircuitConstructorBase<STANDARD_HONK_WIDTH> {
   public:
-    // TODO: replace this with Honk enums after we have a verifier and no longer depend on plonk prover/verifier
+    // TODO(#216)(Kesha): replace this with Honk enums after we have a verifier and no longer depend on plonk
+    // prover/verifier
     static constexpr plonk::ComposerType type = plonk::ComposerType::STANDARD_HONK;
     static constexpr size_t UINT_LOG2_BASE = 2;
 
     // These are variables that we have used a gate on, to enforce that they are
     // equal to a defined value.
-    // TODO(Adrian): Why is this not in CircuitConstructorBase
+    // TODO(#216)(Adrian): Why is this not in CircuitConstructorBase
     std::map<barretenberg::fr, uint32_t> constant_variable_indices;
 
     StandardCircuitConstructor(const size_t size_hint = 0)
@@ -30,9 +31,10 @@ class StandardCircuitConstructor : public CircuitConstructorBase<STANDARD_HONK_W
         w_o.reserve(size_hint);
         // To effieciently constrain wires to zero, we set the first value of w_1 to be 0, and use copy constraints for
         // all future zero values.
-        // TODO(Adrian): This should be done in a constant way, maybe by initializing the constant_variable_indices map
+        // (#216)(Adrian): This should be done in a constant way, maybe by initializing the constant_variable_indices
+        // map
         zero_idx = put_constant_variable(barretenberg::fr::zero());
-        // TODO(Cody): Ensure that no polynomial is ever zero. Maybe there's a better way.
+        // TODO(#217)(Cody): Ensure that no polynomial is ever zero. Maybe there's a better way.
         one_idx = put_constant_variable(barretenberg::fr::one());
         // 1 * 1 * 1 + 1 * 1 + 1 * 1 + 1 * 1 + -4
         // m           l       r       o        c
@@ -63,7 +65,7 @@ class StandardCircuitConstructor : public CircuitConstructorBase<STANDARD_HONK_W
 
     fixed_group_add_quad previous_add_quad;
 
-    // TODO(Adrian): This should be a virtual overridable method in the base class.
+    // TODO(#216)(Adrian): This should be a virtual overridable method in the base class.
     void fix_witness(const uint32_t witness_index, const barretenberg::fr& witness_value);
 
     std::vector<uint32_t> decompose_into_base4_accumulators(const uint32_t witness_index,
@@ -84,7 +86,7 @@ class StandardCircuitConstructor : public CircuitConstructorBase<STANDARD_HONK_W
     accumulator_triple create_and_constraint(const uint32_t a, const uint32_t b, const size_t num_bits);
     accumulator_triple create_xor_constraint(const uint32_t a, const uint32_t b, const size_t num_bits);
 
-    // TODO(Adrian): The 2 following methods should be virtual in the base class
+    // TODO(#216)(Adrian): The 2 following methods should be virtual in the base class
     uint32_t put_constant_variable(const barretenberg::fr& variable);
 
     size_t get_num_constant_gates() const override { return 0; }

barretenberg/cpp/src/barretenberg/honk/composer/composer_helper/composer_helper_lib.cpp

@@ -63,12 +63,11 @@ void construct_lagrange_selector_forms(const CircuitConstructor& circuit_constru
         for (size_t i = 0; i < selector_values.size(); ++i) {
             selector_poly_lagrange[num_public_inputs + i] = selector_values[i];
         }
-        // TODO(Adrian): We may want to add a unique value (e.g. j+1) in the last position of each selector polynomial
-        // to guard against some edge cases that may occur during the MSM.
-        // If we do so, we should ensure that this does not clash with any other values we want to place at the end of
-        // of the witness vectors.
-        // In later iterations of the Sumcheck, we will be able to efficiently cancel out any checks in the last 2^k
-        // rows, so any randomness or unique values should be placed there.
+        // TODO(#217)(Adrian): We may want to add a unique value (e.g. j+1) in the last position of each selector
+        // polynomial to guard against some edge cases that may occur during the MSM. If we do so, we should ensure that
+        // this does not clash with any other values we want to place at the end of of the witness vectors. In later
+        // iterations of the Sumcheck, we will be able to efficiently cancel out any checks in the last 2^k rows, so any
+        // randomness or unique values should be placed there.
 
         circuit_proving_key->polynomial_cache.put(circuit_constructor.selector_names_[j] + "_lagrange",
                                                   std::move(selector_poly_lagrange));
@@ -98,10 +97,9 @@ void compute_monomial_and_coset_selector_forms(bonk::proving_key* circuit_provin
         barretenberg::polynomial selector_poly_fft(selector_poly, circuit_proving_key->circuit_size * 4 + 4);
         selector_poly_fft.coset_fft(circuit_proving_key->large_domain);
 
-        // TODO(kesha): Delete lagrange polynomial from cache if it's not needed
-        // if (selector_properties[i].requires_lagrange_base_polynomial) {
-        //     key->polynomial_cache.put(selector_properties[i].name + "_lagrange", std::move(selector_poly_lagrange));
-        // }
+        // TODO(#215)(Luke/Kesha): Lagrange polynomials could be deleted from cache here since they are no longer
+        // needed.
+
         circuit_proving_key->polynomial_cache.put(selector_properties[i].name, std::move(selector_poly));
         circuit_proving_key->polynomial_cache.put(selector_properties[i].name + "_fft", std::move(selector_poly_fft));
     }
@@ -127,7 +125,7 @@ std::vector<barretenberg::polynomial> compute_witness_base(const CircuitConstruc
     const size_t num_public_inputs = public_inputs.size();
 
     const size_t num_constraints = std::max(minimum_circuit_size, num_gates + num_public_inputs);
-    // TODO(Adrian): Not a fan of specifying NUM_RANDOMIZED_GATES everywhere,
+    // TODO(#216)(Adrian): Not a fan of specifying NUM_RANDOMIZED_GATES everywhere,
     // Each flavor of Honk should have a "fixed" number of random places to add randomness to.
     // It should be taken care of in as few places possible.
     const size_t subgroup_size =

barretenberg/cpp/src/barretenberg/honk/composer/composer_helper/standard_honk_composer_helper.cpp

@@ -27,7 +27,7 @@ std::shared_ptr<bonk::proving_key> StandardHonkComposerHelper<CircuitConstructor
     const CircuitConstructor& constructor, const size_t minimum_circuit_size, const size_t num_randomized_gates)
 {
     // Initialize circuit_proving_key
-    // TODO: replace composer types.
+    // TODO(#229)(Kesha): replace composer types.
     circuit_proving_key = initialize_proving_key(constructor,
                                                  crs_factory_.get(),
                                                  minimum_circuit_size,

barretenberg/cpp/src/barretenberg/honk/composer/composer_helper/standard_honk_composer_helper.hpp

@@ -25,7 +25,8 @@ template <typename CircuitConstructor> class StandardHonkComposerHelper {
     std::shared_ptr<bonk::proving_key> circuit_proving_key;
     std::vector<barretenberg::polynomial> wire_polynomials;
     std::shared_ptr<bonk::verification_key> circuit_verification_key;
-    // TODO(kesha): we need to put this into the commitment key, so that the composer doesn't have to handle srs at all
+    // TODO(#218)(kesha): we need to put this into the commitment key, so that the composer doesn't have to handle srs
+    // at all
     std::shared_ptr<bonk::ReferenceStringFactory> crs_factory_;
     bool computed_witness = false;
     StandardHonkComposerHelper()
@@ -56,7 +57,7 @@ template <typename CircuitConstructor> class StandardHonkComposerHelper {
 
     template <typename Flavor> StandardProver create_prover(const CircuitConstructor& circuit_constructor);
 
-    // TODO(Adrian): Seems error prone to provide the number of randomized gates
+    // TODO(#216)(Adrian): Seems error prone to provide the number of randomized gates
     // Cody: Where should this go? In the flavor (or whatever that becomes)?
     std::shared_ptr<bonk::proving_key> compute_proving_key_base(
         const CircuitConstructor& circuit_constructor,

barretenberg/cpp/src/barretenberg/honk/composer/composer_helper/standard_plonk_composer_helper.cpp

@@ -33,7 +33,7 @@ std::shared_ptr<bonk::proving_key> StandardPlonkComposerHelper<CircuitConstructo
 {
 
     // Initialize circuit_proving_key
-    // TODO: replace composer types.
+    // TODO(#229)(Kesha): replace composer types.
     circuit_proving_key = initialize_proving_key(
         constructor, crs_factory_.get(), minimum_circuit_size, num_randomized_gates, plonk::ComposerType::STANDARD);
     // Compute lagrange selectors

barretenberg/cpp/src/barretenberg/honk/composer/composer_helper/standard_plonk_composer_helper.hpp

@@ -23,7 +23,8 @@ template <typename CircuitConstructor> class StandardPlonkComposerHelper {
     static constexpr size_t program_width = CircuitConstructor::program_width;
     std::shared_ptr<bonk::proving_key> circuit_proving_key;
     std::shared_ptr<bonk::verification_key> circuit_verification_key;
-    // TODO(kesha): we need to put this into the commitment key, so that the composer doesn't have to handle srs at all
+    // TODO(#218)(kesha): we need to put this into the commitment key, so that the composer doesn't have to handle srs
+    // at all
     std::shared_ptr<bonk::ReferenceStringFactory> crs_factory_;
 
     std::vector<uint32_t> recursive_proof_public_input_indices;
@@ -78,7 +79,7 @@ template <typename CircuitConstructor> class StandardPlonkComposerHelper {
     plonk::Verifier create_verifier(const CircuitConstructor& circuit_constructor);
     plonk::Prover create_prover(const CircuitConstructor& circuit_constructor);
 
-    // TODO(Adrian): Seems error prone to provide the number of randomized gates
+    // TODO(#216)(Adrian): Seems error prone to provide the number of randomized gates
     // Cody: Where should this go? In the flavor (or whatever that becomes)?
     std::shared_ptr<bonk::proving_key> compute_proving_key_base(
         const CircuitConstructor& circuit_constructor,

barretenberg/cpp/src/barretenberg/honk/composer/standard_honk_composer.hpp

@@ -69,10 +69,9 @@ class StandardHonkComposer {
     StandardHonkComposer(const StandardHonkComposer& other) = delete;
     StandardHonkComposer(StandardHonkComposer&& other) = default;
     StandardHonkComposer& operator=(const StandardHonkComposer& other) = delete;
-    // Todo(Cody): This constructor started to be implicitly deleted when I added `n` and `variables` members. This is a
-    // temporary measure until we can rewrite Plonk and all tests using circuit builder methods in place of composer
-    // methods, where appropriate.
-    // StandardHonkComposer& operator=(StandardHonkComposer&& other) = default;
+    // TODO(#230)(Cody): This constructor started to be implicitly deleted when I added `n` and `variables` members.
+    // This is a temporary measure until we can rewrite Plonk and all tests using circuit builder methods in place of
+    // composer methods, where appropriate. StandardHonkComposer& operator=(StandardHonkComposer&& other) = default;
     ~StandardHonkComposer() = default;
 
     size_t get_num_gates() const { return circuit_constructor.get_num_gates(); }

barretenberg/cpp/src/barretenberg/honk/composer/standard_honk_composer.test.cpp

@@ -308,7 +308,7 @@ TEST(StandardHonkComposer, VerificationKeyCreation)
  * @brief A test taking sumcheck relations and applying them to the witness and selector polynomials to ensure that the
  * realtions are correct.
  *
- * TODO(kesha): We'll have to update this function once we add zk, since the relation will be incorrect for he first few
+ * TODO(Kesha): We'll have to update this function once we add zk, since the relation will be incorrect for he first few
  * indices
  *
  */

barretenberg/cpp/src/barretenberg/honk/composer/standard_plonk_composer.hpp

@@ -70,10 +70,9 @@ class StandardPlonkComposer {
     StandardPlonkComposer(const StandardPlonkComposer& other) = delete;
     StandardPlonkComposer(StandardPlonkComposer&& other) = default;
     StandardPlonkComposer& operator=(const StandardPlonkComposer& other) = delete;
-    // Todo(Cody): This constructor started to be implicitly deleted when I added `n` and `variables` members. This is a
-    // temporary measure until we can rewrite Plonk and all tests using circuit builder methods in place of composer
-    // methods, where appropriate.
-    // StandardPlonkComposer& operator=(StandardPlonkComposer&& other) = default;
+    // TODO(#230)(Cody): This constructor started to be implicitly deleted when I added `n` and `variables` members.
+    // This is a temporary measure until we can rewrite Plonk and all tests using circuit builder methods in place of
+    // composer methods, where appropriate. StandardPlonkComposer& operator=(StandardPlonkComposer&& other) = default;
     ~StandardPlonkComposer() = default;
 
     size_t get_num_gates() const { return circuit_constructor.get_num_gates(); }
@@ -188,7 +187,7 @@ class StandardPlonkComposer {
     uint32_t zero_idx = 0;
 
     void compute_witness() { composer_helper.compute_witness(circuit_constructor); };
-    // TODO(Cody): This will not be needed, but maybe something is required for ComposerHelper to be generic?
+    // TODO(#230)(Cody): This will not be needed, but maybe something is required for ComposerHelper to be generic?
     plonk::Verifier create_verifier() { return composer_helper.create_verifier(circuit_constructor); }
     /**
      * Preprocess the circuit. Delegates to create_prover.

barretenberg/cpp/src/barretenberg/honk/pcs/commitment_key.hpp

@@ -24,7 +24,7 @@ namespace kzg {
  * The SRS is given as a list of 𝔾₁ points
  *  { [xʲ]₁ }ⱼ where 'x' is unknown.
  *
- * @todo This class should take ownership of the SRS, and handle reading the file from disk.
+ * TODO(#218)(Adrian): This class should take ownership of the SRS, and handle reading the file from disk.
  */
 class CommitmentKey {
     using Fr = typename barretenberg::g1::Fr;
@@ -44,7 +44,6 @@ class CommitmentKey {
      * @param n
      * @param path
      *
-     * @todo change path to string_view
      */
     CommitmentKey(const size_t num_points, std::string_view path)
         : pippenger_runtime_state(num_points)
@@ -101,7 +100,7 @@ class VerificationKey {
     {
         C pairing_points[2]{ p0, p1 };
         // The final pairing check of step 12.
-        // TODO: try to template parametrise the pairing + fq12 output :/
+        // TODO(Adrian): try to template parametrise the pairing + fq12 output :/
         barretenberg::fq12 result = barretenberg::pairing::reduced_ate_pairing_batch_precomputed(
             pairing_points, verifier_srs.get_precomputed_g2_lines(), 2);
 

barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp

@@ -94,7 +94,7 @@ template <typename Params> class MultilinearReductionScheme {
 
     using Fr = typename Params::Fr;
     using Commitment = typename Params::Commitment;
-    using CommitmentAffine = typename Params::C; // TODO(luke): find a better name
+    using CommitmentAffine = typename Params::C;
     using Polynomial = barretenberg::Polynomial<Fr>;
 
   public:
@@ -150,7 +150,7 @@ template <typename Params> class MultilinearReductionScheme {
 
             // fold the previous polynomial with odd and even parts
             for (size_t i = 0; i < n_l; ++i) {
-                // TODO parallelize
+                // TODO(#219)(Adrian) parallelize
 
                 // fold(Aₗ)[i] = (1-uₗ)⋅even(Aₗ)[i] + uₗ⋅odd(Aₗ)[i]
                 //            = (1-uₗ)⋅Aₗ[2i]      + uₗ⋅Aₗ[2i+1]