Auto merge of #75 - ebfull:primerepr-fixes, r=ebfull

PrimeRepr improvements

These are API-breaking changes that make `PrimeRepr` a little nicer.

src/bls12_381/fq.rs

@@ -276,7 +276,7 @@ impl PrimeFieldRepr for FqRepr {
     }
 
     #[inline(always)]
-    fn divn(&mut self, mut n: u32) {
+    fn shr(&mut self, mut n: u32) {
         if n >= 64 * 6 {
             *self = Self::from(0);
             return;
@@ -324,7 +324,7 @@ impl PrimeFieldRepr for FqRepr {
     }
 
     #[inline(always)]
-    fn muln(&mut self, mut n: u32) {
+    fn shl(&mut self, mut n: u32) {
         if n >= 64 * 6 {
             *self = Self::from(0);
             return;
@@ -364,25 +364,21 @@ impl PrimeFieldRepr for FqRepr {
     }
 
     #[inline(always)]
-    fn add_nocarry(&mut self, other: &FqRepr) -> bool {
+    fn add_nocarry(&mut self, other: &FqRepr) {
         let mut carry = 0;
 
         for (a, b) in self.0.iter_mut().zip(other.0.iter()) {
             *a = ::adc(*a, *b, &mut carry);
         }
-
-        carry != 0
     }
 
     #[inline(always)]
-    fn sub_noborrow(&mut self, other: &FqRepr) -> bool {
+    fn sub_noborrow(&mut self, other: &FqRepr) {
         let mut borrow = 0;
 
         for (a, b) in self.0.iter_mut().zip(other.0.iter()) {
             *a = ::sbb(*a, *b, &mut borrow);
         }
-
-        borrow != 0
     }
 }
 
@@ -969,29 +965,29 @@ fn test_fq_repr_div2() {
 }
 
 #[test]
-fn test_fq_repr_divn() {
+fn test_fq_repr_shr() {
     let mut a = FqRepr([0xaa5cdd6172847ffd, 0x43242c06aed55287, 0x9ddd5b312f3dd104, 0xc5541fd48046b7e7, 0x16080cf4071e0b05, 0x1225f2901aea514e]);
-    a.divn(0);
+    a.shr(0);
     assert_eq!(
         a,
         FqRepr([0xaa5cdd6172847ffd, 0x43242c06aed55287, 0x9ddd5b312f3dd104, 0xc5541fd48046b7e7, 0x16080cf4071e0b05, 0x1225f2901aea514e])
     );
-    a.divn(1);
+    a.shr(1);
     assert_eq!(
         a,
         FqRepr([0xd52e6eb0b9423ffe, 0x21921603576aa943, 0xceeead98979ee882, 0xe2aa0fea40235bf3, 0xb04067a038f0582, 0x912f9480d7528a7])
     );
-    a.divn(50);
+    a.shr(50);
     assert_eq!(
         a,
         FqRepr([0x8580d5daaa50f54b, 0xab6625e7ba208864, 0x83fa9008d6fcf3bb, 0x19e80e3c160b8aa, 0xbe52035d4a29c2c1, 0x244])
     );
-    a.divn(130);
+    a.shr(130);
     assert_eq!(
         a,
         FqRepr([0xa0fea40235bf3cee, 0x4067a038f0582e2a, 0x2f9480d7528a70b0, 0x91, 0x0, 0x0])
     );
-    a.divn(64);
+    a.shr(64);
     assert_eq!(
         a,
         FqRepr([0x4067a038f0582e2a, 0x2f9480d7528a70b0, 0x91, 0x0, 0x0, 0x0])
@@ -1067,13 +1063,10 @@ fn test_fq_repr_sub_noborrow() {
         assert_eq!(csub_ab, csub_ba);
     }
 
-    // Subtracting q+1 from q should produce a borrow
+    // Subtracting q+1 from q should produce -1 (mod 2**384)
     let mut qplusone = FqRepr([0xb9feffffffffaaab, 0x1eabfffeb153ffff, 0x6730d2a0f6b0f624, 0x64774b84f38512bf, 0x4b1ba7b6434bacd7, 0x1a0111ea397fe69a]);
-    assert!(qplusone.sub_noborrow(&FqRepr([0xb9feffffffffaaac, 0x1eabfffeb153ffff, 0x6730d2a0f6b0f624, 0x64774b84f38512bf, 0x4b1ba7b6434bacd7, 0x1a0111ea397fe69a])));
+    qplusone.sub_noborrow(&FqRepr([0xb9feffffffffaaac, 0x1eabfffeb153ffff, 0x6730d2a0f6b0f624, 0x64774b84f38512bf, 0x4b1ba7b6434bacd7, 0x1a0111ea397fe69a]));
-
+    assert_eq!(qplusone, FqRepr([0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff]));
-    // Subtracting x from x should produce no borrow
-    let mut x = FqRepr([0xb9feffffffffaaac, 0x1eabfffeb153ffff, 0x6730d2a0f6b0f624, 0x64774b84f38512bf, 0x4b1ba7b6434bacd7, 0x1a0111ea397fe69a]);
-    assert!(!x.sub_noborrow(&FqRepr([0xb9feffffffffaaac, 0x1eabfffeb153ffff, 0x6730d2a0f6b0f624, 0x64774b84f38512bf, 0x4b1ba7b6434bacd7, 0x1a0111ea397fe69a])))
 }
 
 #[test]
@@ -1126,13 +1119,10 @@ fn test_fq_repr_add_nocarry() {
         assert_eq!(abc, cba);
     }
 
-    // Adding 1 to (2^384 - 1) should produce a carry
+    // Adding 1 to (2^384 - 1) should produce zero
     let mut x = FqRepr([0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff]);
-    assert!(x.add_nocarry(&FqRepr::from(1)));
+    x.add_nocarry(&FqRepr::from(1));
-
+    assert!(x.is_zero());
-    // Adding 1 to q should not produce a carry
-    let mut x = FqRepr([0xb9feffffffffaaab, 0x1eabfffeb153ffff, 0x6730d2a0f6b0f624, 0x64774b84f38512bf, 0x4b1ba7b6434bacd7, 0x1a0111ea397fe69a]);
-    assert!(!x.add_nocarry(&FqRepr::from(1)));
 }
 
 #[test]

src/bls12_381/fr.rs

@@ -113,7 +113,7 @@ impl PrimeFieldRepr for FrRepr {
     }
 
     #[inline(always)]
-    fn divn(&mut self, mut n: u32) {
+    fn shr(&mut self, mut n: u32) {
         if n >= 64 * 4 {
             *self = Self::from(0);
             return;
@@ -161,7 +161,7 @@ impl PrimeFieldRepr for FrRepr {
     }
 
     #[inline(always)]
-    fn muln(&mut self, mut n: u32) {
+    fn shl(&mut self, mut n: u32) {
         if n >= 64 * 4 {
             *self = Self::from(0);
             return;
@@ -201,25 +201,21 @@ impl PrimeFieldRepr for FrRepr {
     }
 
     #[inline(always)]
-    fn add_nocarry(&mut self, other: &FrRepr) -> bool {
+    fn add_nocarry(&mut self, other: &FrRepr) {
         let mut carry = 0;
 
         for (a, b) in self.0.iter_mut().zip(other.0.iter()) {
             *a = ::adc(*a, *b, &mut carry);
         }
-
-        carry != 0
     }
 
     #[inline(always)]
-    fn sub_noborrow(&mut self, other: &FrRepr) -> bool {
+    fn sub_noborrow(&mut self, other: &FrRepr) {
         let mut borrow = 0;
 
         for (a, b) in self.0.iter_mut().zip(other.0.iter()) {
             *a = ::sbb(*a, *b, &mut borrow);
         }
-
-        borrow != 0
     }
 }
 
@@ -674,29 +670,29 @@ fn test_fr_repr_div2() {
 }
 
 #[test]
-fn test_fr_repr_divn() {
+fn test_fr_repr_shr() {
     let mut a = FrRepr([0xb33fbaec482a283f, 0x997de0d3a88cb3df, 0x9af62d2a9a0e5525, 0x36003ab08de70da1]);
-    a.divn(0);
+    a.shr(0);
     assert_eq!(
         a,
         FrRepr([0xb33fbaec482a283f, 0x997de0d3a88cb3df, 0x9af62d2a9a0e5525, 0x36003ab08de70da1])
     );
-    a.divn(1);
+    a.shr(1);
     assert_eq!(
         a,
         FrRepr([0xd99fdd762415141f, 0xccbef069d44659ef, 0xcd7b16954d072a92, 0x1b001d5846f386d0])
     );
-    a.divn(50);
+    a.shr(50);
     assert_eq!(
         a,
         FrRepr([0xbc1a7511967bf667, 0xc5a55341caa4b32f, 0x75611bce1b4335e, 0x6c0])
     );
-    a.divn(130);
+    a.shr(130);
     assert_eq!(
         a,
         FrRepr([0x1d5846f386d0cd7, 0x1b0, 0x0, 0x0])
     );
-    a.divn(64);
+    a.shr(64);
     assert_eq!(
         a,
         FrRepr([0x1b0, 0x0, 0x0, 0x0])
@@ -772,13 +768,10 @@ fn test_fr_repr_sub_noborrow() {
         assert_eq!(csub_ab, csub_ba);
     }
 
-    // Subtracting r+1 from r should produce a borrow
+    // Subtracting r+1 from r should produce -1 (mod 2**256)
     let mut qplusone = FrRepr([0xffffffff00000001, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48]);
-    assert!(qplusone.sub_noborrow(&FrRepr([0xffffffff00000002, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48])));
+    qplusone.sub_noborrow(&FrRepr([0xffffffff00000002, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48]));
-
+    assert_eq!(qplusone, FrRepr([0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff]));
-    // Subtracting x from x should produce no borrow
-    let mut x = FrRepr([0xffffffff00000001, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48]);
-    assert!(!x.sub_noborrow(&FrRepr([0xffffffff00000001, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48])))
 }
 
 #[test]
@@ -842,13 +835,10 @@ fn test_fr_repr_add_nocarry() {
         assert_eq!(abc, cba);
     }
 
-    // Adding 1 to (2^256 - 1) should produce a carry
+    // Adding 1 to (2^256 - 1) should produce zero
     let mut x = FrRepr([0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff]);
-    assert!(x.add_nocarry(&FrRepr::from(1)));
+    x.add_nocarry(&FrRepr::from(1));
-
+    assert!(x.is_zero());
-    // Adding 1 to r should not produce a carry
-    let mut x = FrRepr([0xffffffff00000001, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48]);
-    assert!(!x.add_nocarry(&FrRepr::from(1)));
 }
 
 #[test]

src/lib.rs

@@ -353,11 +353,11 @@ pub trait PrimeFieldRepr: Sized +
                           AsMut<[u64]> +
                           From<u64>
 {
-    /// Subtract another represetation from this one, returning the borrow bit.
+    /// Subtract another represetation from this one.
-    fn sub_noborrow(&mut self, other: &Self) -> bool;
+    fn sub_noborrow(&mut self, other: &Self);
 
-    /// Add another representation to this one, returning the carry bit.
+    /// Add another representation to this one.
-    fn add_nocarry(&mut self, other: &Self) -> bool;
+    fn add_nocarry(&mut self, other: &Self);
 
     /// Compute the number of bits needed to encode this number. Always a
     /// multiple of 64.
@@ -377,17 +377,16 @@ pub trait PrimeFieldRepr: Sized +
     fn div2(&mut self);
 
     /// Performs a rightwise bitshift of this number by some amount.
-    fn divn(&mut self, amt: u32);
+    fn shr(&mut self, amt: u32);
 
     /// Performs a leftwise bitshift of this number, effectively multiplying
     /// it by 2. Overflow is ignored.
     fn mul2(&mut self);
 
     /// Performs a leftwise bitshift of this number by some amount.
-    fn muln(&mut self, amt: u32);
+    fn shl(&mut self, amt: u32);
 
-    /// Writes this `PrimeFieldRepr` as a big endian integer. Always writes
+    /// Writes this `PrimeFieldRepr` as a big endian integer.
-    /// `(num_bits` / 8) bytes.
     fn write_be<W: Write>(&self, mut writer: W) -> io::Result<()> {
         use byteorder::{WriteBytesExt, BigEndian};
 
@@ -398,8 +397,7 @@ pub trait PrimeFieldRepr: Sized +
         Ok(())
     }
 
-    /// Reads a big endian integer occupying (`num_bits` / 8) bytes into this
+    /// Reads a big endian integer into this representation.
-    /// representation.
     fn read_be<R: Read>(&mut self, mut reader: R) -> io::Result<()> {
         use byteorder::{ReadBytesExt, BigEndian};
 

src/tests/repr.rs

@@ -3,8 +3,8 @@ use ::{PrimeFieldRepr};
 
 pub fn random_repr_tests<R: PrimeFieldRepr>() {
     random_encoding_tests::<R>();
-    random_muln_tests::<R>();
+    random_shl_tests::<R>();
-    random_divn_tests::<R>();
+    random_shr_tests::<R>();
 }
 
 fn random_encoding_tests<R: PrimeFieldRepr>() {
@@ -22,7 +22,7 @@ fn random_encoding_tests<R: PrimeFieldRepr>() {
     }
 }
 
-fn random_muln_tests<R: PrimeFieldRepr>() {
+fn random_shl_tests<R: PrimeFieldRepr>() {
     let mut rng = XorShiftRng::from_seed([0x5dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
 
     for _ in 0..100 {
@@ -36,14 +36,14 @@ fn random_muln_tests<R: PrimeFieldRepr>() {
                 r1.mul2();
             }
 
-            r2.muln(shift);
+            r2.shl(shift);
 
             assert_eq!(r1, r2);
         }
     }
 }
 
-fn random_divn_tests<R: PrimeFieldRepr>() {
+fn random_shr_tests<R: PrimeFieldRepr>() {
     let mut rng = XorShiftRng::from_seed([0x5dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
 
     for _ in 0..100 {
@@ -57,7 +57,7 @@ fn random_divn_tests<R: PrimeFieldRepr>() {
                 r1.div2();
             }
 
-            r2.divn(shift);
+            r2.shr(shift);
 
             assert_eq!(r1, r2);
         }