ff: Add Ord bound to PrimeField

bellman/src/groth16/tests/dummy_engine.rs

@@ -50,6 +50,18 @@ impl ConditionallySelectable for Fr {
     }
 }
 
+impl Ord for Fr {
+    fn cmp(&self, other: &Fr) -> Ordering {
+        (self.0).0.cmp(&(other.0).0)
+    }
+}
+
+impl PartialOrd for Fr {
+    fn partial_cmp(&self, other: &Fr) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 impl Neg for Fr {
     type Output = Self;
 

ff/src/lib.rs

@@ -258,7 +258,7 @@ impl fmt::Display for PrimeFieldDecodingError {
 
 /// This represents an element of a prime field.
 pub trait PrimeField:
-    Field + From<u64> + BitAnd<u64, Output = u64> + Shr<u32, Output = Self>
+    Field + Ord + From<u64> + BitAnd<u64, Output = u64> + Shr<u32, Output = Self>
 {
     /// The prime field can be converted back and forth into this biginteger
     /// representation.

zcash_primitives/src/jubjub/fs.rs

@@ -272,6 +272,20 @@ impl ConstantTimeEq for Fs {
     }
 }
 
+impl Ord for Fs {
+    #[inline(always)]
+    fn cmp(&self, other: &Fs) -> ::std::cmp::Ordering {
+        self.into_repr().cmp(&other.into_repr())
+    }
+}
+
+impl PartialOrd for Fs {
+    #[inline(always)]
+    fn partial_cmp(&self, other: &Fs) -> Option<::std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 impl ::std::fmt::Display for Fs {
     fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
         write!(f, "Fs({})", self.into_repr())

zcash_primitives/src/jubjub/tests.rs

@@ -1,6 +1,6 @@
 use super::{edwards, montgomery, JubjubEngine, JubjubParams, PrimeOrder};
 
-use ff::{Field, PrimeField, PrimeFieldRepr, SqrtField};
+use ff::{Field, PrimeField, SqrtField};
 use std::ops::{AddAssign, MulAssign, Neg, SubAssign};
 
 use rand_core::{RngCore, SeedableRng};
@@ -370,32 +370,26 @@ fn test_jubjub_params<E: JubjubEngine>(params: &E::Params) {
         // Check that the number of windows per generator
         // in the Pedersen hash does not allow for collisions
 
-        let mut cur = E::Fs::one().into_repr();
+        let mut cur = E::Fs::one();
 
-        let mut max = E::Fs::char();
+        let max = (-E::Fs::one()) >> 1;
-        {
-            max.sub_noborrow(&E::Fs::one().into_repr());
-            max.div2();
-        }
 
-        let mut pacc = E::Fs::zero().into_repr();
+        let mut pacc = E::Fs::zero();
-        let mut nacc = E::Fs::char();
+        let mut nacc = E::Fs::zero();
 
         for _ in 0..params.pedersen_hash_chunks_per_generator() {
             // tmp = cur * 4
-            let mut tmp = cur;
+            let tmp = cur.double().double();
-            tmp.mul2();
-            tmp.mul2();
 
-            pacc.add_nocarry(&tmp);
+            pacc += &tmp;
-            nacc.sub_noborrow(&tmp);
+            nacc -= &tmp; // The first subtraction wraps intentionally.
 
             assert!(pacc < max);
             assert!(pacc < nacc);
 
             // cur = cur * 16
             for _ in 0..4 {
-                cur.mul2();
+                cur = cur.double();
             }
         }
     }