Cleanups from code review

Co-authored-by: Jack Grigg <jack@electriccoin.co>

src/constants.rs

@@ -209,7 +209,7 @@ impl<C: CurveAffine> FixedBase<C> for OrchardFixedBase<C> {
     }
 }
 
-pub trait TestFixedBase<C: CurveAffine> {
+trait TestFixedBase<C: CurveAffine> {
     // Test that Lagrange interpolation coefficients reproduce the correct x-coordinate
     // for each fixed-base multiple in each window.
     fn test_lagrange_coeffs(&self, num_windows: usize);

src/constants/util.rs

@@ -1,7 +1,7 @@
 use ff::PrimeField;
 use halo2::arithmetic::{CurveAffine, FieldExt};
 
-/// Decompose a scalar into FIXED_BASE_WINDOW_SIZE bits (little-endian)
+/// Decompose a scalar into `window_num_bits` bits (little-endian)
 /// For a window size of `w`, this returns [k_0, ..., k_n] where each `k_i`
 /// is a `w`-bit value, and `scalar = k_0 + k_1 * w + k_n * w^n`.
 pub fn decompose_scalar_fixed<C: CurveAffine>(
@@ -9,29 +9,18 @@ pub fn decompose_scalar_fixed<C: CurveAffine>(
     scalar_num_bits: usize,
     window_num_bits: usize,
 ) -> Vec<u8> {
-    let mut bits: Vec<bool> = scalar
+    // Pad bits to multiple of window_num_bits
+    let padding = (window_num_bits - (scalar_num_bits % window_num_bits)) % window_num_bits;
+    let bits: Vec<bool> = scalar
         .to_le_bits()
         .into_iter()
         .take(scalar_num_bits)
+        .chain(std::iter::repeat(false).take(padding))
         .collect();
-
-    assert_eq!(bits.len(), scalar_num_bits);
-
-    // Pad bits to multiple of window_num_bits
-    bits.append(&mut vec![
-        false;
-        (window_num_bits
-            - (scalar_num_bits % window_num_bits))
-            % window_num_bits
-    ]);
+    assert_eq!(bits.len(), scalar_num_bits + padding);
 
     bits.chunks_exact(window_num_bits)
-        .map(|chunk| {
-            let mut chunk = chunk.iter();
-            *(chunk.next().unwrap()) as u8
-                + ((*(chunk.next().unwrap()) as u8) << 1)
-                + ((*(chunk.next().unwrap()) as u8) << 2)
-        })
+        .map(|chunk| chunk.iter().rev().fold(0, |acc, b| (acc << 1) + (*b as u8)))
         .collect()
 }
 

src/primitives/sinsemilla/constants.rs

@@ -58,11 +58,8 @@ pub const S_PERSONALIZATION: &str = "z.cash:SinsemillaS";
 /// Creates the Sinsemilla S generators used in each round of the Sinsemilla hash
 pub fn sinsemilla_s_generators<C: CurveAffine>() -> impl Iterator<Item = (C::Base, C::Base)> {
     let hasher = C::CurveExt::hash_to_curve(S_PERSONALIZATION);
-    (0..(1 << K)).map(move |j| {
-        let point = hasher(&(j as u32).to_le_bytes())
-            .to_affine()
-            .coordinates()
-            .unwrap();
+    (0..(1u32 << K)).map(move |j| {
+        let point = hasher(&j.to_le_bytes()).to_affine().coordinates().unwrap();
         (*point.x(), *point.y())
     })
 }
@@ -81,9 +78,10 @@ mod tests {
     #[test]
     fn sinsemilla_s() {
         use super::super::sinsemilla_s::SINSEMILLA_S;
-        let mut sinsemilla_s = sinsemilla_s_generators::<pallas::Affine>();
-        for s in SINSEMILLA_S.iter() {
-            assert_eq!(sinsemilla_s.next().unwrap(), (s.0, s.1));
+        let sinsemilla_s: Vec<_> = sinsemilla_s_generators::<pallas::Affine>().collect();
+        assert_eq!(sinsemilla_s.len(), SINSEMILLA_S.len());
+        for (expected, actual) in sinsemilla_s.iter().zip(&SINSEMILLA_S[..]) {
+            assert_eq!(expected, actual);
         }
     }