Cleanups and clippy fixes.

Co-authored-by: Jack Grigg <jack@electriccoin.co>
Co-authored-by: Daira Hopwood <daira@jacaranda.org>

src/circuit/gadget/ecc/chip.rs

@@ -246,7 +246,7 @@ impl EccChip {
             short_config.create_gate(meta);
         }
 
-        // Create gate ths is only used in fixed-base mul using a base field element.
+        // Create gate that is only used in fixed-base mul using a base field element.
         {
             let base_field_config: mul_fixed::base_field_elem::Config = (&config).into();
             base_field_config.create_gate(meta);
@@ -300,7 +300,8 @@ pub struct EccScalarFixedShort {
 /// Each `a_i` is in the range [0..2^3).
 ///
 /// `windows` = [z_1, ..., z_85], where we expect z_85 = 0.
-/// Since z_0 is initialized as α, we store it as `base_field_elem`.
+/// Since z_0 is initialized as the scalar α, we store it as
+/// `base_field_elem`.
 #[derive(Clone, Debug)]
 struct EccBaseFieldElemFixed {
     base_field_elem: CellValue<pallas::Base>,

src/circuit/gadget/ecc/chip/mul.rs

@@ -147,7 +147,7 @@ impl Config {
                 // Initialize the accumulator `acc = [2]base`
                 let acc = self
                     .add_config
-                    .assign_region(&base, &base, offset, &mut region)?;
+                    .assign_region(base, base, offset, &mut region)?;
 
                 // Increase the offset by 1 after complete addition.
                 let offset = offset + 1;
@@ -170,7 +170,7 @@ impl Config {
                 let (x_a, y_a, zs_incomplete_hi) = self.hi_config.double_and_add(
                     &mut region,
                     offset,
-                    &base,
+                    base,
                     bits_incomplete_hi,
                     (X(acc.x), Y(acc.y), z_init),
                 )?;
@@ -180,13 +180,13 @@ impl Config {
                 let (x_a, y_a, zs_incomplete_lo) = self.lo_config.double_and_add(
                     &mut region,
                     offset,
-                    &base,
+                    base,
                     bits_incomplete_lo,
                     (x_a, y_a, *z),
                 )?;
 
                 // Move from incomplete addition to complete addition.
-                // Inside incomplete::double_and_add, the offset was increase once after initialization
+                // Inside incomplete::double_and_add, the offset was increased once after initialization
                 // of the running sum.
                 // Then, the final assignment of double-and-add was made on row + offset + 1.
                 // Outside of incomplete addition, we must account for these offset increases by adding
@@ -235,11 +235,13 @@ impl Config {
                 }
 
                 let zs = {
-                    let mut zs = vec![z_init];
-                    zs.extend_from_slice(&zs_incomplete_hi);
-                    zs.extend_from_slice(&zs_incomplete_lo);
-                    zs.extend_from_slice(&zs_complete);
-                    zs.extend_from_slice(&[z_0]);
+                    let mut zs = std::iter::empty()
+                        .chain(Some(z_init))
+                        .chain(zs_incomplete_hi.into_iter())
+                        .chain(zs_incomplete_lo.into_iter())
+                        .chain(zs_complete.into_iter())
+                        .chain(Some(z_0))
+                        .collect::<Vec<_>>();
                     assert_eq!(zs.len(), pallas::Scalar::NUM_BITS as usize + 1);
 
                     // This reverses zs to give us [z_0, z_1, ..., z_{254}, z_{255}].

src/circuit/gadget/ecc/chip/mul/complete.rs

@@ -119,7 +119,7 @@ impl Config {
         };
 
         // Store interstitial running sum `z`s in vector
-        let mut zs: Vec<Z<pallas::Base>> = Vec::new();
+        let mut zs: Vec<Z<pallas::Base>> = Vec::with_capacity(bits.len());
 
         // Complete addition
         for (iter, k) in bits.iter().enumerate() {

src/circuit/gadget/ecc/chip/mul/incomplete.rs

@@ -175,7 +175,7 @@ impl Config {
                     .chain(Some(("bool_check", q_mul.clone() * bool_check)))
                     .chain(Some(("gradient_1", q_mul.clone() * gradient_1)))
                     .chain(Some(("secant_line", q_mul.clone() * secant_line)))
-                    .chain(Some(("gradient_2", q_mul.clone() * gradient_2)))
+                    .chain(Some(("gradient_2", q_mul * gradient_2)))
             };
 
             // q_mul == 2
@@ -194,13 +194,13 @@ impl Config {
 
                 // The base used in double-and-add remains constant. We check that its
                 // x- and y- coordinates are the same throughout.
-                let x_p_check = x_p_cur.clone() - x_p_next;
-                let y_p_check = y_p_cur.clone() - y_p_next;
+                let x_p_check = x_p_cur - x_p_next;
+                let y_p_check = y_p_cur - y_p_next;
 
                 std::iter::empty()
                     .chain(Some(("x_p_check", q_mul_is_two.clone() * x_p_check)))
                     .chain(Some(("y_p_check", q_mul_is_two.clone() * y_p_check)))
-                    .chain(for_loop(meta, q_mul_is_two.clone(), y_a_next))
+                    .chain(for_loop(meta, q_mul_is_two, y_a_next))
             };
 
             // q_mul == 3
@@ -312,7 +312,7 @@ impl Config {
         let offset = offset + 1;
 
         // Initialise vector to store all interstitial `z` running sum values.
-        let mut zs: Vec<Z<pallas::Base>> = Vec::new();
+        let mut zs: Vec<Z<pallas::Base>> = Vec::with_capacity(bits.len());
 
         // Incomplete addition
         for (row, k) in bits.iter().enumerate() {

src/circuit/gadget/ecc/chip/mul/overflow.rs

@@ -67,8 +67,7 @@ impl Config {
 
             // q = 2^254 + t_q is the Pallas scalar field modulus.
             // We cast t_q into the base field to check alpha + t_q (mod p).
-            let t_q = pallas::Base::from_u128(T_Q);
-            let t_q = Expression::Constant(t_q);
+            let t_q = Expression::Constant(pallas::Base::from_u128(T_Q));
 
             // z_0 - alpha - t_q = 0 (mod p)
             let recovery = z_0 - alpha - t_q;
@@ -237,6 +236,7 @@ impl Config {
             num_words,
             false,
         )?;
+        // (s - (2^0 s_0 + 2^1 s_1 + ... + 2^129 s_129)) / 2^130
         Ok(zs[zs.len() - 1])
     }
 }

src/circuit/gadget/ecc/chip/mul_fixed/short.rs

@@ -106,14 +106,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         self.q_mul_fixed_short
             .enable(region, offset + NUM_WINDOWS)?;
 
-        // Assign final `x, y` to `x_p, y_p` columns and return final point
-        let x_val = magnitude_mul.x.value();
-        let x_var = region.assign_advice(
-            || "x_var",
-            self.super_config.x_p,
-            offset + NUM_WINDOWS,
-            || x_val.ok_or(Error::SynthesisError),
-        )?;
+        // Assign final `y` to `y_p` column and return final point
         let y_var = region.assign_advice(
             || "y_var",
             self.super_config.y_p,
@@ -122,7 +115,7 @@ impl<const NUM_WINDOWS: usize> Config<NUM_WINDOWS> {
         )?;
 
         let result = EccPoint {
-            x: CellValue::new(x_var, x_val),
+            x: magnitude_mul.x,
             y: CellValue::new(y_var, y_val),
         };
 

src/circuit/gadget/ecc/chip/witness_scalar_fixed.rs

@@ -36,6 +36,8 @@ impl Config {
             let q_scalar_fixed = meta.query_selector(self.q_scalar_fixed);
             let window = meta.query_advice(self.window, Rotation::cur());
 
+            // Constrain each window to a 3-bit value:
+            // 1 * (window - 0) * (window - 1) * ... * (window - 7)
             let range_check =
                 (0..constants::H).fold(Expression::Constant(pallas::Base::one()), |acc, i| {
                     acc * (window.clone() - Expression::Constant(pallas::Base::from_u64(i as u64)))
@@ -44,6 +46,9 @@ impl Config {
         });
     }
 
+    /// Witnesses the given scalar as `NUM_WINDOWS` 3-bit windows.
+    ///
+    /// The scalar is allowed to be non-canonical.
     fn decompose_scalar_fixed<const NUM_WINDOWS: usize, const SCALAR_NUM_BITS: usize>(
         &self,
         scalar: Option<pallas::Scalar>,

src/circuit/gadget/ecc/chip/witness_scalar_fixed/short.rs

@@ -85,7 +85,7 @@ impl Config {
         let sign_cell = region.assign_advice(
             || "sign",
             self.super_config.window,
-            NUM_WINDOWS_SHORT,
+            offset + NUM_WINDOWS_SHORT,
             || sign.ok_or(Error::SynthesisError),
         )?;