Merge pull request #52 from zcash/field-trait-changes

Migrate to `ff` revision with square root backports

CHANGELOG.md

@@ -6,6 +6,14 @@ and this project adheres to Rust's notion of
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- Migrated to `ff 0.13`, `group 0.13`.
+
+### Removed
+- `pasta_curves::arithmetic::SqrtRatio` (use `ff::Field::{sqrt_ratio, sqrt_alt}`
+  instead).
+- `pasta_curves::arithmetic::SqrtTables` (from public API, as it isn't suitable
+  for generic usage).
 
 ## [0.4.1] - 2022-10-13
 ### Added

Cargo.toml

@@ -72,3 +72,6 @@ sqrt-table = ["alloc", "lazy_static"]
 repr-c = []
 uninline-portable = []
 serde = ["hex", "serde_crate"]
+
+[patch.crates-io]
+ff = { git = "https://github.com/zkcrypto/ff.git", rev = "c070ffbaea8cb17e57f817a91ed0e364ff679b7c" }

src/arithmetic/fields.rs

@@ -4,103 +4,39 @@
 use core::mem::size_of;
 
 use static_assertions::const_assert;
-use subtle::{Choice, ConditionallySelectable, CtOption};
 
 use super::Group;
 
-use core::assert;
-
 #[cfg(feature = "sqrt-table")]
 use alloc::{boxed::Box, vec::Vec};
 #[cfg(feature = "sqrt-table")]
 use core::marker::PhantomData;
 
+#[cfg(feature = "sqrt-table")]
+use subtle::Choice;
+
 const_assert!(size_of::<usize>() >= 4);
 
-/// A trait that exposes additional operations related to calculating square roots of
+/// An internal trait that exposes additional operations related to calculating square roots of
 /// prime-order finite fields.
-pub trait SqrtRatio: ff::PrimeField {
+pub(crate) trait SqrtTableHelpers: ff::PrimeField {
-    /// The value $(T-1)/2$ such that $2^S \cdot T = p - 1$ with $T$ odd.
+    /// Raise this field element to the power $(t-1)/2$.
-    const T_MINUS1_OVER2: [u64; 4];
-
-    /// Raise this field element to the power [`Self::T_MINUS1_OVER2`].
     ///
     /// Field implementations may override this to use an efficient addition chain.
-    fn pow_by_t_minus1_over2(&self) -> Self {
+    fn pow_by_t_minus1_over2(&self) -> Self;
-        ff::Field::pow_vartime(self, &Self::T_MINUS1_OVER2)
-    }
 
     /// Gets the lower 32 bits of this field element when expressed
     /// canonically.
     fn get_lower_32(&self) -> u32;
-
-    /// Computes:
-    ///
-    /// - $(\textsf{true}, \sqrt{\textsf{num}/\textsf{div}})$, if $\textsf{num}$ and
-    ///   $\textsf{div}$ are nonzero and $\textsf{num}/\textsf{div}$ is a square in the
-    ///   field;
-    /// - $(\textsf{true}, 0)$, if $\textsf{num}$ is zero;
-    /// - $(\textsf{false}, 0)$, if $\textsf{num}$ is nonzero and $\textsf{div}$ is zero;
-    /// - $(\textsf{false}, \sqrt{G_S \cdot \textsf{num}/\textsf{div}})$, if
-    ///   $\textsf{num}$ and $\textsf{div}$ are nonzero and $\textsf{num}/\textsf{div}$ is
-    ///   a nonsquare in the field;
-    ///
-    /// where $G_S$ is a non-square.
-    ///
-    /// For `pasta_curves`, $G_S$ is currently [`ff::PrimeField::root_of_unity`], a
-    /// generator of the order $2^S$ subgroup. Users of this crate should not rely on this
-    /// generator being fixed; it may be changed in future crate versions to simplify the
-    /// implementation of the SSWU hash-to-curve algorithm.
-    ///
-    /// The choice of root from sqrt is unspecified.
-    fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) {
-        // General implementation:
-        //
-        // a = num * inv0(div)
-        //   = {    0    if div is zero
-        //     { num/div otherwise
-        //
-        // b = G_S * a
-        //   = {      0      if div is zero
-        //     { G_S*num/div otherwise
-        //
-        // Since G_S is non-square, a and b are either both zero (and both square), or
-        // only one of them is square. We can therefore choose the square root to return
-        // based on whether a is square, but for the boolean output we need to handle the
-        // num != 0 && div == 0 case specifically.
-
-        let a = div.invert().unwrap_or_else(Self::zero) * num;
-        let b = a * Self::root_of_unity();
-        let sqrt_a = a.sqrt();
-        let sqrt_b = b.sqrt();
-
-        let num_is_zero = num.is_zero();
-        let div_is_zero = div.is_zero();
-        let is_square = sqrt_a.is_some();
-        let is_nonsquare = sqrt_b.is_some();
-        assert!(bool::from(
-            num_is_zero | div_is_zero | (is_square ^ is_nonsquare)
-        ));
-
-        (
-            is_square & !(!num_is_zero & div_is_zero),
-            CtOption::conditional_select(&sqrt_b, &sqrt_a, is_square).unwrap(),
-        )
-    }
-
-    /// Equivalent to `Self::sqrt_ratio(self, one())`.
-    fn sqrt_alt(&self) -> (Choice, Self) {
-        Self::sqrt_ratio(self, &Self::one())
-    }
 }
 
 /// This trait is a common interface for dealing with elements of a finite
 /// field.
-pub trait FieldExt: SqrtRatio + From<bool> + Ord + Group<Scalar = Self> {
+pub trait FieldExt: ff::PrimeField + From<bool> + Ord + Group<Scalar = Self> {
     /// Modulus of the field written as a string for display purposes
     const MODULUS: &'static str;
 
-    /// Inverse of `PrimeField::root_of_unity()`
+    /// Inverse of `PrimeField::ROOT_OF_UNITY`
     const ROOT_OF_UNITY_INV: Self;
 
     /// Generator of the $t-order$ multiplicative subgroup
@@ -119,77 +55,11 @@ pub trait FieldExt: SqrtRatio + From<bool> + Ord + Group<Scalar = Self> {
     /// byte representation of an integer.
     fn from_bytes_wide(bytes: &[u8; 64]) -> Self;
 
-    /// Exponentiates `self` by `by`, where `by` is a little-endian order
-    /// integer exponent.
-    fn pow(&self, by: &[u64; 4]) -> Self {
-        let mut res = Self::one();
-        for e in by.iter().rev() {
-            for i in (0..64).rev() {
-                res = res.square();
-                let mut tmp = res;
-                tmp *= self;
-                res.conditional_assign(&tmp, (((*e >> i) & 0x1) as u8).into());
-            }
-        }
-        res
-    }
-
     /// Gets the lower 128 bits of this field element when expressed
     /// canonically.
     fn get_lower_128(&self) -> u128;
 }
 
-/// Tonelli–Shanks' square-root algorithm for `p mod 16 = 1`.
-///
-/// https://eprint.iacr.org/2012/685.pdf (page 12, algorithm 5)
-///
-/// `tm1d2` should be set to `(t - 1) // 2`, where `t = (modulus - 1) >> F::S`.
-#[cfg(not(feature = "sqrt-table"))]
-#[cfg_attr(docsrs, doc(cfg(not(feature = "sqrt-table"))))]
-pub(crate) fn sqrt_tonelli_shanks<F: ff::PrimeField, S: AsRef<[u64]>>(
-    f: &F,
-    tm1d2: S,
-) -> CtOption<F> {
-    use subtle::ConstantTimeEq;
-
-    // w = self^((t - 1) // 2)
-    let w = f.pow_vartime(tm1d2);
-
-    let mut v = F::S;
-    let mut x = w * f;
-    let mut b = x * w;
-
-    // Initialize z as the 2^S root of unity.
-    let mut z = F::root_of_unity();
-
-    for max_v in (1..=F::S).rev() {
-        let mut k = 1;
-        let mut tmp = b.square();
-        let mut j_less_than_v: Choice = 1.into();
-
-        for j in 2..max_v {
-            let tmp_is_one = tmp.ct_eq(&F::one());
-            let squared = F::conditional_select(&tmp, &z, tmp_is_one).square();
-            tmp = F::conditional_select(&squared, &tmp, tmp_is_one);
-            let new_z = F::conditional_select(&z, &squared, tmp_is_one);
-            j_less_than_v &= !j.ct_eq(&v);
-            k = u32::conditional_select(&j, &k, tmp_is_one);
-            z = F::conditional_select(&z, &new_z, j_less_than_v);
-        }
-
-        let result = x * z;
-        x = F::conditional_select(&result, &x, b.ct_eq(&F::one()));
-        z = z.square();
-        b *= z;
-        v = k;
-    }
-
-    CtOption::new(
-        x,
-        (x * x).ct_eq(f), // Only return Some if it's the square root.
-    )
-}
-
 /// Parameters for a perfect hash function used in square root computation.
 #[cfg(feature = "sqrt-table")]
 #[cfg_attr(docsrs, doc(cfg(feature = "sqrt-table")))]
@@ -201,7 +71,7 @@ struct SqrtHasher<F: FieldExt> {
 }
 
 #[cfg(feature = "sqrt-table")]
-impl<F: FieldExt> SqrtHasher<F> {
+impl<F: FieldExt + SqrtTableHelpers> SqrtHasher<F> {
     /// Returns a perfect hash of x for use with SqrtTables::inv.
     fn hash(&self, x: &F) -> usize {
         // This is just the simplest constant-time perfect hash construction that could
@@ -216,7 +86,7 @@ impl<F: FieldExt> SqrtHasher<F> {
 #[cfg(feature = "sqrt-table")]
 #[cfg_attr(docsrs, doc(cfg(feature = "sqrt-table")))]
 #[derive(Debug)]
-pub struct SqrtTables<F: FieldExt> {
+pub(crate) struct SqrtTables<F: FieldExt> {
     hasher: SqrtHasher<F>,
     inv: Vec<u8>,
     g0: Box<[F; 256]>,
@@ -226,7 +96,7 @@ pub struct SqrtTables<F: FieldExt> {
 }
 
 #[cfg(feature = "sqrt-table")]
-impl<F: FieldExt> SqrtTables<F> {
+impl<F: FieldExt + SqrtTableHelpers> SqrtTables<F> {
     /// Build tables given parameters for the perfect hash.
     pub fn new(hash_xor: u32, hash_mod: usize) -> Self {
         use alloc::vec;
@@ -237,10 +107,10 @@ impl<F: FieldExt> SqrtTables<F> {
             marker: PhantomData,
         };
 
-        let mut gtab = (0..4).scan(F::root_of_unity(), |gi, _| {
+        let mut gtab = (0..4).scan(F::ROOT_OF_UNITY, |gi, _| {
             // gi == ROOT_OF_UNITY^(256^i)
             let gtab_i: Vec<F> = (0..256)
-                .scan(F::one(), |acc, _| {
+                .scan(F::ONE, |acc, _| {
                     let res = *acc;
                     *acc *= *gi;
                     Some(res)
@@ -335,7 +205,7 @@ impl<F: FieldExt> SqrtTables<F> {
 
         let sqdiv = res.square() * div;
         let is_square = (sqdiv - num).is_zero();
-        let is_nonsquare = (sqdiv - F::root_of_unity() * num).is_zero();
+        let is_nonsquare = (sqdiv - F::ROOT_OF_UNITY * num).is_zero();
         assert!(bool::from(
             num.is_zero() | div.is_zero() | (is_square ^ is_nonsquare)
         ));
@@ -352,7 +222,7 @@ impl<F: FieldExt> SqrtTables<F> {
 
         let sq = res.square();
         let is_square = (sq - u).is_zero();
-        let is_nonsquare = (sq - F::root_of_unity() * u).is_zero();
+        let is_nonsquare = (sq - F::ROOT_OF_UNITY * u).is_zero();
         assert!(bool::from(u.is_zero() | (is_square ^ is_nonsquare)));
 
         (is_square, res)

src/curves.rs

@@ -897,7 +897,7 @@ macro_rules! impl_projective_curve_ext {
             use super::hashtocurve;
 
             Box::new(move |message| {
-                let mut us = [Field::zero(); 2];
+                let mut us = [Field::ZERO; 2];
                 hashtocurve::hash_to_field($name::CURVE_ID, domain_prefix, message, &mut us);
                 let q0 = hashtocurve::map_to_curve_simple_swu::<$base, $name, $iso>(
                     &us[0],
@@ -1110,7 +1110,7 @@ impl Ep {
         0x4000000000000000,
     ]);
 
-    /// `(F::root_of_unity().invert().unwrap() * z).sqrt().unwrap()`
+    /// `(F::ROOT_OF_UNITY.invert().unwrap() * z).sqrt().unwrap()`
     pub const THETA: Fp = Fp::from_raw([
         0xca330bcc09ac318e,
         0x51f64fc4dc888857,
@@ -1210,7 +1210,7 @@ impl Eq {
         0x4000000000000000,
     ]);
 
-    /// `(F::root_of_unity().invert().unwrap() * z).sqrt().unwrap()`
+    /// `(F::ROOT_OF_UNITY.invert().unwrap() * z).sqrt().unwrap()`
     pub const THETA: Fq = Fq::from_raw([
         0x632cae9872df1b5d,
         0x38578ccadf03ac27,

src/fields/fp.rs

@@ -1,7 +1,7 @@
 use core::fmt;
 use core::ops::{Add, Mul, Neg, Sub};
 
-use ff::PrimeField;
+use ff::{Field, PrimeField};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
@@ -11,7 +11,7 @@ use lazy_static::lazy_static;
 #[cfg(feature = "bits")]
 use ff::{FieldBits, PrimeFieldBits};
 
-use crate::arithmetic::{adc, mac, sbb, FieldExt, Group, SqrtRatio};
+use crate::arithmetic::{adc, mac, sbb, FieldExt, Group, SqrtTableHelpers};
 
 #[cfg(feature = "sqrt-table")]
 use crate::arithmetic::SqrtTables;
@@ -173,6 +173,18 @@ impl<'a, 'b> Mul<&'b Fp> for &'a Fp {
 impl_binops_additive!(Fp, Fp);
 impl_binops_multiplicative!(Fp, Fp);
 
+impl<T: ::core::borrow::Borrow<Fp>> ::core::iter::Sum<T> for Fp {
+    fn sum<I: Iterator<Item = T>>(iter: I) -> Self {
+        iter.fold(Self::ZERO, |acc, item| acc + item.borrow())
+    }
+}
+
+impl<T: ::core::borrow::Borrow<Fp>> ::core::iter::Product<T> for Fp {
+    fn product<I: Iterator<Item = T>>(iter: I) -> Self {
+        iter.fold(Self::ONE, |acc, item| acc * item.borrow())
+    }
+}
+
 /// INV = -(p^{-1} mod 2^64) mod 2^64
 const INV: u64 = 0x992d30ecffffffff;
 
@@ -232,6 +244,7 @@ const DELTA: Fp = Fp::from_raw([
 ]);
 
 /// `(t - 1) // 2` where t * 2^s + 1 = p with t odd.
+#[cfg(any(test, not(feature = "sqrt-table")))]
 const T_MINUS1_OVER2: [u64; 4] = [
     0x04a6_7c8d_cc96_9876,
     0x0000_0000_1123_4c7e,
@@ -480,6 +493,9 @@ impl Group for Fp {
 }
 
 impl ff::Field for Fp {
+    const ZERO: Self = Self::zero();
+    const ONE: Self = Self::one();
+
     fn random(mut rng: impl RngCore) -> Self {
         Self::from_u512([
             rng.next_u64(),
@@ -493,14 +509,6 @@ impl ff::Field for Fp {
         ])
     }
 
-    fn zero() -> Self {
-        Self::zero()
-    }
-
-    fn one() -> Self {
-        Self::one()
-    }
-
     fn double(&self) -> Self {
         self.double()
     }
@@ -510,6 +518,21 @@ impl ff::Field for Fp {
         self.square()
     }
 
+    fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) {
+        #[cfg(feature = "sqrt-table")]
+        {
+            FP_TABLES.sqrt_ratio(num, div)
+        }
+
+        #[cfg(not(feature = "sqrt-table"))]
+        ff::helpers::sqrt_ratio_generic(num, div)
+    }
+
+    #[cfg(feature = "sqrt-table")]
+    fn sqrt_alt(&self) -> (Choice, Self) {
+        FP_TABLES.sqrt_alt(self)
+    }
+
     /// Computes the square root of this element, if it exists.
     fn sqrt(&self) -> CtOption<Self> {
         #[cfg(feature = "sqrt-table")]
@@ -519,7 +542,7 @@ impl ff::Field for Fp {
         }
 
         #[cfg(not(feature = "sqrt-table"))]
-        crate::arithmetic::sqrt_tonelli_shanks(self, &T_MINUS1_OVER2)
+        ff::helpers::sqrt_tonelli_shanks(self, &T_MINUS1_OVER2)
     }
 
     /// Computes the multiplicative inverse of this element,
@@ -559,7 +582,9 @@ impl ff::PrimeField for Fp {
 
     const NUM_BITS: u32 = 255;
     const CAPACITY: u32 = 254;
+    const MULTIPLICATIVE_GENERATOR: Self = GENERATOR;
     const S: u32 = S;
+    const ROOT_OF_UNITY: Self = ROOT_OF_UNITY;
 
     fn from_repr(repr: Self::Repr) -> CtOption<Self> {
         let mut tmp = Fp([0, 0, 0, 0]);
@@ -604,14 +629,6 @@ impl ff::PrimeField for Fp {
     fn is_odd(&self) -> Choice {
         Choice::from(self.to_repr()[0] & 1)
     }
-
-    fn multiplicative_generator() -> Self {
-        GENERATOR
-    }
-
-    fn root_of_unity() -> Self {
-        ROOT_OF_UNITY
-    }
 }
 
 #[cfg(all(feature = "bits", not(target_pointer_width = "64")))]
@@ -669,9 +686,7 @@ lazy_static! {
     static ref FP_TABLES: SqrtTables<Fp> = SqrtTables::new(0x11BE, 1098);
 }
 
-impl SqrtRatio for Fp {
+impl SqrtTableHelpers for Fp {
-    const T_MINUS1_OVER2: [u64; 4] = T_MINUS1_OVER2;
-
     fn pow_by_t_minus1_over2(&self) -> Self {
         let sqr = |x: Fp, i: u32| (0..i).fold(x, |x, _| x.square());
 
@@ -709,16 +724,6 @@ impl SqrtRatio for Fp {
 
         tmp.0[0] as u32
     }
-
-    #[cfg(feature = "sqrt-table")]
-    fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) {
-        FP_TABLES.sqrt_ratio(num, div)
-    }
-
-    #[cfg(feature = "sqrt-table")]
-    fn sqrt_alt(&self) -> (Choice, Self) {
-        FP_TABLES.sqrt_alt(self)
-    }
 }
 
 impl FieldExt for Fp {
@@ -792,9 +797,6 @@ impl ec_gpu::GpuField for Fp {
     }
 }
 
-#[cfg(test)]
-use ff::Field;
-
 #[test]
 fn test_inv() {
     // Compute -(r^{-1} mod 2^64) mod 2^64 by exponentiating
@@ -840,8 +842,8 @@ fn test_sqrt_ratio_and_alt() {
     assert!(v_alt == v);
 
     // (false, sqrt(ROOT_OF_UNITY * num/div)), if num and div are nonzero and num/div is a nonsquare in the field
-    let num = num * Fp::root_of_unity();
+    let num = num * Fp::ROOT_OF_UNITY;
-    let expected = Fp::TWO_INV * Fp::root_of_unity() * Fp::from(5).invert().unwrap();
+    let expected = Fp::TWO_INV * Fp::ROOT_OF_UNITY * Fp::from(5).invert().unwrap();
     let (is_square, v) = Fp::sqrt_ratio(&num, &div);
     assert!(!bool::from(is_square));
     assert!(v == expected || (-v) == expected);
@@ -888,14 +890,14 @@ fn test_zeta() {
 #[test]
 fn test_root_of_unity() {
     assert_eq!(
-        Fp::root_of_unity().pow_vartime(&[1 << Fp::S, 0, 0, 0]),
+        Fp::ROOT_OF_UNITY.pow_vartime(&[1 << Fp::S, 0, 0, 0]),
         Fp::one()
     );
 }
 
 #[test]
 fn test_inv_root_of_unity() {
-    assert_eq!(Fp::ROOT_OF_UNITY_INV, Fp::root_of_unity().invert().unwrap());
+    assert_eq!(Fp::ROOT_OF_UNITY_INV, Fp::ROOT_OF_UNITY.invert().unwrap());
 }
 
 #[test]
@@ -908,7 +910,7 @@ fn test_delta() {
     assert_eq!(Fp::DELTA, GENERATOR.pow(&[1u64 << Fp::S, 0, 0, 0]));
     assert_eq!(
         Fp::DELTA,
-        Fp::multiplicative_generator().pow(&[1u64 << Fp::S, 0, 0, 0])
+        Fp::MULTIPLICATIVE_GENERATOR.pow(&[1u64 << Fp::S, 0, 0, 0])
     );
 }
 

src/fields/fq.rs

@@ -1,7 +1,7 @@
 use core::fmt;
 use core::ops::{Add, Mul, Neg, Sub};
 
-use ff::PrimeField;
+use ff::{Field, PrimeField};
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
@@ -11,7 +11,7 @@ use lazy_static::lazy_static;
 #[cfg(feature = "bits")]
 use ff::{FieldBits, PrimeFieldBits};
 
-use crate::arithmetic::{adc, mac, sbb, FieldExt, Group, SqrtRatio};
+use crate::arithmetic::{adc, mac, sbb, FieldExt, Group, SqrtTableHelpers};
 
 #[cfg(feature = "sqrt-table")]
 use crate::arithmetic::SqrtTables;
@@ -173,6 +173,18 @@ impl<'a, 'b> Mul<&'b Fq> for &'a Fq {
 impl_binops_additive!(Fq, Fq);
 impl_binops_multiplicative!(Fq, Fq);
 
+impl<T: ::core::borrow::Borrow<Fq>> ::core::iter::Sum<T> for Fq {
+    fn sum<I: Iterator<Item = T>>(iter: I) -> Self {
+        iter.fold(Self::ZERO, |acc, item| acc + item.borrow())
+    }
+}
+
+impl<T: ::core::borrow::Borrow<Fq>> ::core::iter::Product<T> for Fq {
+    fn product<I: Iterator<Item = T>>(iter: I) -> Self {
+        iter.fold(Self::ONE, |acc, item| acc * item.borrow())
+    }
+}
+
 /// INV = -(q^{-1} mod 2^64) mod 2^64
 const INV: u64 = 0x8c46eb20ffffffff;
 
@@ -232,6 +244,7 @@ const DELTA: Fq = Fq::from_raw([
 ]);
 
 /// `(t - 1) // 2` where t * 2^s + 1 = p with t odd.
+#[cfg(any(test, not(feature = "sqrt-table")))]
 const T_MINUS1_OVER2: [u64; 4] = [
     0x04ca_546e_c623_7590,
     0x0000_0000_1123_4c7e,
@@ -480,6 +493,9 @@ impl Group for Fq {
 }
 
 impl ff::Field for Fq {
+    const ZERO: Self = Self::zero();
+    const ONE: Self = Self::one();
+
     fn random(mut rng: impl RngCore) -> Self {
         Self::from_u512([
             rng.next_u64(),
@@ -493,14 +509,6 @@ impl ff::Field for Fq {
         ])
     }
 
-    fn zero() -> Self {
-        Self::zero()
-    }
-
-    fn one() -> Self {
-        Self::one()
-    }
-
     fn double(&self) -> Self {
         self.double()
     }
@@ -510,6 +518,21 @@ impl ff::Field for Fq {
         self.square()
     }
 
+    fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) {
+        #[cfg(feature = "sqrt-table")]
+        {
+            FQ_TABLES.sqrt_ratio(num, div)
+        }
+
+        #[cfg(not(feature = "sqrt-table"))]
+        ff::helpers::sqrt_ratio_generic(num, div)
+    }
+
+    #[cfg(feature = "sqrt-table")]
+    fn sqrt_alt(&self) -> (Choice, Self) {
+        FQ_TABLES.sqrt_alt(self)
+    }
+
     /// Computes the square root of this element, if it exists.
     fn sqrt(&self) -> CtOption<Self> {
         #[cfg(feature = "sqrt-table")]
@@ -519,7 +542,7 @@ impl ff::Field for Fq {
         }
 
         #[cfg(not(feature = "sqrt-table"))]
-        crate::arithmetic::sqrt_tonelli_shanks(self, &T_MINUS1_OVER2)
+        ff::helpers::sqrt_tonelli_shanks(self, &T_MINUS1_OVER2)
     }
 
     /// Computes the multiplicative inverse of this element,
@@ -559,7 +582,9 @@ impl ff::PrimeField for Fq {
 
     const NUM_BITS: u32 = 255;
     const CAPACITY: u32 = 254;
+    const MULTIPLICATIVE_GENERATOR: Self = GENERATOR;
     const S: u32 = S;
+    const ROOT_OF_UNITY: Self = ROOT_OF_UNITY;
 
     fn from_repr(repr: Self::Repr) -> CtOption<Self> {
         let mut tmp = Fq([0, 0, 0, 0]);
@@ -604,14 +629,6 @@ impl ff::PrimeField for Fq {
     fn is_odd(&self) -> Choice {
         Choice::from(self.to_repr()[0] & 1)
     }
-
-    fn multiplicative_generator() -> Self {
-        GENERATOR
-    }
-
-    fn root_of_unity() -> Self {
-        ROOT_OF_UNITY
-    }
 }
 
 #[cfg(all(feature = "bits", not(target_pointer_width = "64")))]
@@ -668,9 +685,7 @@ lazy_static! {
     static ref FQ_TABLES: SqrtTables<Fq> = SqrtTables::new(0x116A9E, 1206);
 }
 
-impl SqrtRatio for Fq {
+impl SqrtTableHelpers for Fq {
-    const T_MINUS1_OVER2: [u64; 4] = T_MINUS1_OVER2;
-
     fn pow_by_t_minus1_over2(&self) -> Self {
         let sqr = |x: Fq, i: u32| (0..i).fold(x, |x, _| x.square());
 
@@ -708,16 +723,6 @@ impl SqrtRatio for Fq {
 
         tmp.0[0] as u32
     }
-
-    #[cfg(feature = "sqrt-table")]
-    fn sqrt_ratio(num: &Self, div: &Self) -> (Choice, Self) {
-        FQ_TABLES.sqrt_ratio(num, div)
-    }
-
-    #[cfg(feature = "sqrt-table")]
-    fn sqrt_alt(&self) -> (Choice, Self) {
-        FQ_TABLES.sqrt_alt(self)
-    }
 }
 
 impl FieldExt for Fq {
@@ -791,9 +796,6 @@ impl ec_gpu::GpuField for Fq {
     }
 }
 
-#[cfg(test)]
-use ff::Field;
-
 #[test]
 fn test_inv() {
     // Compute -(r^{-1} mod 2^64) mod 2^64 by exponentiating
@@ -839,8 +841,8 @@ fn test_sqrt_ratio_and_alt() {
     assert!(v_alt == v);
 
     // (false, sqrt(ROOT_OF_UNITY * num/div)), if num and div are nonzero and num/div is a nonsquare in the field
-    let num = num * Fq::root_of_unity();
+    let num = num * Fq::ROOT_OF_UNITY;
-    let expected = Fq::TWO_INV * Fq::root_of_unity() * Fq::from(5).invert().unwrap();
+    let expected = Fq::TWO_INV * Fq::ROOT_OF_UNITY * Fq::from(5).invert().unwrap();
     let (is_square, v) = Fq::sqrt_ratio(&num, &div);
     assert!(!bool::from(is_square));
     assert!(v == expected || (-v) == expected);
@@ -886,14 +888,14 @@ fn test_zeta() {
 #[test]
 fn test_root_of_unity() {
     assert_eq!(
-        Fq::root_of_unity().pow_vartime(&[1 << Fq::S, 0, 0, 0]),
+        Fq::ROOT_OF_UNITY.pow_vartime(&[1 << Fq::S, 0, 0, 0]),
         Fq::one()
     );
 }
 
 #[test]
 fn test_inv_root_of_unity() {
-    assert_eq!(Fq::ROOT_OF_UNITY_INV, Fq::root_of_unity().invert().unwrap());
+    assert_eq!(Fq::ROOT_OF_UNITY_INV, Fq::ROOT_OF_UNITY.invert().unwrap());
 }
 
 #[test]
@@ -906,7 +908,7 @@ fn test_delta() {
     assert_eq!(Fq::DELTA, GENERATOR.pow(&[1u64 << Fq::S, 0, 0, 0]));
     assert_eq!(
         Fq::DELTA,
-        Fq::multiplicative_generator().pow(&[1u64 << Fq::S, 0, 0, 0])
+        Fq::MULTIPLICATIVE_GENERATOR.pow(&[1u64 << Fq::S, 0, 0, 0])
     );
 }
 

src/hashtocurve.rs

@@ -136,7 +136,7 @@ pub fn map_to_curve_simple_swu<F: FieldExt, C: CurveExt<Base = F>, I: CurveExt<B
     let b = I::b();
     let z_u2 = z * u.square();
     let ta = z_u2.square() + z_u2;
-    let num_x1 = b * (ta + F::one());
+    let num_x1 = b * (ta + F::ONE);
     let div = a * F::conditional_select(&-ta, &z, ta.is_zero());
     let num2_x1 = num_x1.square();
     let div2 = div.square();