Remove all uses of `PrimeField::Repr` in generic code

`PrimeField::from_repr` explicitly leaves the endianness opaque. We
therefore can't use it in places we were using `FieldExt::from_bytes`
(which was specifically little-endian) generically, but the previous
commit replaced it everywhere. We now handle generic contexts on a
case-by-case basis:

- Where we needed to convert bitstrings into field elements, we now use
  double-and-add on the field elements directly instead of on bytes.
  This is less efficient, but visible correct (and a future change to
  the `ff` crate APIs could enable the more efficient version).

- `INV_TWO_POW_K`, which is pre-computed for `pallas::Base`, was being
  incorrectly used in a field-generic circuit. We now compute it live.

- `test_zs_and_us` was only used in tests, and hard-coded a field
  element encoding length of 32 bytes. It now uses Pallas concretely.

src/circuit/gadget/sinsemilla.rs

@@ -3,7 +3,7 @@ use crate::circuit::gadget::{
     ecc::{self, EccInstructions},
     utilities::Var,
 };
-use ff::PrimeField;
+use group::ff::{Field, PrimeField};
 use halo2::{circuit::Layouter, plonk::Error};
 use pasta_curves::arithmetic::CurveAffine;
 use std::fmt::Debug;
@@ -194,25 +194,19 @@ where
 
         // Closure to parse a bitstring (little-endian) into a base field element.
         let to_base_field = |bits: &[Option<bool>]| -> Option<C::Base> {
-            assert!(bits.len() <= C::Base::NUM_BITS as usize);
+            // To simplify the following logic, require that the all-ones bitstring is
+            // canonical in the field, by not allowing a length of NUM_BITS.
+            assert!(bits.len() < C::Base::NUM_BITS as usize);
 
             let bits: Option<Vec<bool>> = bits.iter().cloned().collect();
-            let bytes: Option<Vec<u8>> = bits.map(|bits| {
-                // Pad bits to 256 bits
-                let pad_len = 256 - bits.len();
-                let mut bits = bits;
-                bits.extend_from_slice(&vec![false; pad_len]);
-
-                bits.chunks_exact(8)
-                    .map(|byte| byte.iter().rev().fold(0u8, |acc, bit| acc * 2 + *bit as u8))
-                    .collect()
-            });
-            bytes.map(|bytes| {
-                let mut repr = <C::Base as PrimeField>::Repr::default();
-                // The above code assumes the byte representation is 256 bits.
-                assert_eq!(repr.as_ref().len(), 32);
-                repr.as_mut().copy_from_slice(&bytes);
-                C::Base::from_repr(repr).unwrap()
+            bits.map(|bits| {
+                bits.into_iter().rev().fold(C::Base::zero(), |acc, bit| {
+                    if bit {
+                        acc.double() + C::Base::one()
+                    } else {
+                        acc.double()
+                    }
+                })
             })
         };
 

src/circuit/gadget/utilities.rs

@@ -87,27 +87,24 @@ pub fn ternary<F: FieldExt>(a: Expression<F>, b: Expression<F>, c: Expression<F>
 /// Takes a specified subsequence of the little-endian bit representation of a field element.
 /// The bits are numbered from 0 for the LSB.
 pub fn bitrange_subset<F: PrimeFieldBits>(field_elem: &F, bitrange: Range<usize>) -> F {
+    // We can allow a subsequence of length NUM_BITS, because
+    // field_elem.to_le_bits() returns canonical bitstrings.
     assert!(bitrange.end <= F::NUM_BITS as usize);
 
-    let bits: Vec<bool> = field_elem
+    field_elem
         .to_le_bits()
         .iter()
         .by_val()
         .skip(bitrange.start)
         .take(bitrange.end - bitrange.start)
-        .chain(std::iter::repeat(false))
-        .take(256)
-        .collect();
-    let bytearray: Vec<u8> = bits
-        .chunks_exact(8)
-        .map(|byte| byte.iter().rev().fold(0u8, |acc, bit| acc * 2 + *bit as u8))
-        .collect();
-
-    let mut repr = F::Repr::default();
-    // The above code assumes the byte representation is 256 bits.
-    assert_eq!(repr.as_ref().len(), 32);
-    repr.as_mut().copy_from_slice(&bytearray);
-    F::from_repr(repr).unwrap()
+        .rev()
+        .fold(F::zero(), |acc, bit| {
+            if bit {
+                acc.double() + F::one()
+            } else {
+                acc.double()
+            }
+        })
 }
 
 /// Check that an expression is in the small range [0..range),

src/circuit/gadget/utilities/lookup_range_check.rs

@@ -364,7 +364,7 @@ impl<F: FieldExt + PrimeFieldBits, const K: usize> LookupRangeCheckConfig<F, K>
 mod tests {
     use super::LookupRangeCheckConfig;
 
-    use crate::primitives::sinsemilla::{INV_TWO_POW_K, K};
+    use crate::primitives::sinsemilla::K;
     use crate::spec::lebs2ip;
     use ff::{Field, PrimeFieldBits};
     use halo2::{
@@ -431,9 +431,7 @@ mod tests {
                             .collect::<Vec<_>>()
                     };
                     let expected_zs = {
-                        let mut repr = F::Repr::default();
-                        repr.as_mut().copy_from_slice(&INV_TWO_POW_K);
-                        let inv_two_pow_k = F::from_repr(repr).unwrap();
+                        let inv_two_pow_k = F::from(1 << K).invert().unwrap();
                         chunks.iter().fold(vec![element], |mut zs, a_i| {
                             // z_{i + 1} = (z_i - a_i) / 2^{K}
                             let z = (zs[zs.len() - 1] - a_i) * inv_two_pow_k;

src/constants.rs

@@ -248,17 +248,15 @@ fn test_lagrange_coeffs<C: CurveAffine>(base: C, num_windows: usize) {
 //      1. z + y = u^2,
 //      2. z - y is not a square
 // for the y-coordinate of each fixed-base multiple in each window.
-fn test_zs_and_us<C: CurveAffine>(base: C, z: &[u64], u: &[[[u8; 32]; H]], num_windows: usize) {
+fn test_zs_and_us(base: pallas::Affine, z: &[u64], u: &[[[u8; 32]; H]], num_windows: usize) {
     let window_table = compute_window_table(base, num_windows);
 
     for ((u, z), window_points) in u.iter().zip(z.iter()).zip(window_table) {
         for (u, point) in u.iter().zip(window_points.iter()) {
             let y = *point.coordinates().unwrap().y();
-            let mut u_repr = <C::Base as PrimeField>::Repr::default();
-            u_repr.as_mut().copy_from_slice(u);
-            let u = C::Base::from_repr(u_repr).unwrap();
-            assert_eq!(C::Base::from(*z) + y, u * u); // allow either square root
-            assert!(bool::from((C::Base::from(*z) - y).sqrt().is_none()));
+            let u = pallas::Base::from_repr(*u).unwrap();
+            assert_eq!(pallas::Base::from(*z) + y, u * u); // allow either square root
+            assert!(bool::from((pallas::Base::from(*z) - y).sqrt().is_none()));
         }
     }
 }