Add error logic to decoding methods.

src/bls12_381/ec.rs

@@ -111,10 +111,6 @@ macro_rules! curve_impl {
                 self.infinity
             }
 
-            fn is_valid(&self) -> bool {
-                self.is_on_curve() && self.is_in_correct_subgroup()
-            }
-
             fn mul<S: Into<<Self::Scalar as PrimeField>::Repr>>(&self, by: S) -> $projective {
                 let mut res = $projective::zero();
 
@@ -560,7 +556,7 @@ macro_rules! curve_impl {
 pub mod g1 {
     use rand::{Rand, Rng};
     use super::super::{Fq, Fr, FrRepr, FqRepr};
-    use ::{CurveProjective, CurveAffine, PrimeField, SqrtField, PrimeFieldRepr, Field, BitIterator, EncodedPoint};
+    use ::{CurveProjective, CurveAffine, PrimeField, SqrtField, PrimeFieldRepr, Field, BitIterator, EncodedPoint, GroupDecodingError};
 
     curve_impl!(G1, G1Affine, G1Prepared, Fq, Fr, G1Uncompressed, G1Compressed);
 
@@ -583,7 +579,18 @@ pub mod g1 {
 
         fn empty() -> Self { G1Uncompressed([0; 96]) }
         fn size() -> usize { 96 }
-        fn into_affine_unchecked(&self) -> Result<G1Affine, ()> {
+        fn into_affine(&self) -> Result<G1Affine, GroupDecodingError> {
+            let affine = self.into_affine_unchecked()?;
+
+            if !affine.is_on_curve() {
+                Err(GroupDecodingError::NotOnCurve)
+            } else if !affine.is_in_correct_subgroup() {
+                Err(GroupDecodingError::NotInSubgroup)
+            } else {
+                Ok(affine)
+            }
+        }
+        fn into_affine_unchecked(&self) -> Result<G1Affine, GroupDecodingError> {
             use byteorder::{ReadBytesExt, BigEndian};
 
             // Create a copy of this representation.
@@ -591,7 +598,7 @@ pub mod g1 {
 
             if copy[0] & (1 << 7) != 0 {
                 // Distinguisher bit is set, but this should be uncompressed!
-                return Err(())
+                return Err(GroupDecodingError::UnexpectedCompressionMode)
             }
 
             if copy[0] & (1 << 6) != 0 {
@@ -603,13 +610,13 @@ pub mod g1 {
                 if copy.iter().all(|b| *b == 0) {
                     Ok(G1Affine::zero())
                 } else {
-                    Err(())
+                    Err(GroupDecodingError::UnexpectedInformation)
                 }
             } else {
                 if copy[0] & (1 << 5) != 0 {
                     // The bit indicating the y-coordinate should be lexicographically
                     // largest is set, but this is an uncompressed element.
-                    return Err(())
+                    return Err(GroupDecodingError::UnexpectedInformation)
                 }
 
                 // Unset the three most significant bits.
@@ -631,8 +638,8 @@ pub mod g1 {
                 }
 
                 Ok(G1Affine {
-                    x: Fq::from_repr(x)?,
+                    x: Fq::from_repr(x).map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate", e))?,
-                    y: Fq::from_repr(y)?,
+                    y: Fq::from_repr(y).map_err(|e| GroupDecodingError::CoordinateDecodingError("y coordinate", e))?,
                     infinity: false
                 })
             }
@@ -681,7 +688,18 @@ pub mod g1 {
 
         fn empty() -> Self { G1Compressed([0; 48]) }
         fn size() -> usize { 48 }
-        fn into_affine_unchecked(&self) -> Result<G1Affine, ()> {
+        fn into_affine(&self) -> Result<G1Affine, GroupDecodingError> {
+            let affine = self.into_affine_unchecked()?;
+
+            // NB: Decompression guarantees that it is on the curve already.
+
+            if !affine.is_in_correct_subgroup() {
+                Err(GroupDecodingError::NotInSubgroup)
+            } else {
+                Ok(affine)
+            }
+        }
+        fn into_affine_unchecked(&self) -> Result<G1Affine, GroupDecodingError> {
             use byteorder::{ReadBytesExt, BigEndian};
 
             // Create a copy of this representation.
@@ -689,7 +707,7 @@ pub mod g1 {
 
             if copy[0] & (1 << 7) == 0 {
                 // Distinguisher bit isn't set.
-                return Err(())
+                return Err(GroupDecodingError::UnexpectedCompressionMode)
             }
 
             if copy[0] & (1 << 6) != 0 {
@@ -701,7 +719,7 @@ pub mod g1 {
                 if copy.iter().all(|b| *b == 0) {
                     Ok(G1Affine::zero())
                 } else {
-                    Err(())
+                    Err(GroupDecodingError::UnexpectedInformation)
                 }
             } else {
                 // Determine if the intended y coordinate must be greater
@@ -722,7 +740,7 @@ pub mod g1 {
                 }
 
                 // Interpret as Fq element.
-                let x = Fq::from_repr(x)?;
+                let x = Fq::from_repr(x).map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate", e))?;
 
                 // Compute x^3 + b
                 let mut x3b = x;
@@ -747,7 +765,7 @@ pub mod g1 {
                     },
                     None => {
                         // Point must not be on the curve.
-                        Err(())
+                        Err(GroupDecodingError::NotOnCurve)
                     }
                 }
             }
@@ -873,7 +891,7 @@ pub mod g1 {
                     infinity: false
                 };
 
-                assert!(!p.is_valid());
+                assert!(!p.is_in_correct_subgroup());
 
                 let mut g1 = G1::zero();
 
@@ -895,7 +913,7 @@ pub mod g1 {
                     assert_eq!(i, 4);
                     let g1 = G1Affine::from(g1);
 
-                    assert!(g1.is_valid());
+                    assert!(g1.is_in_correct_subgroup());
 
                     assert_eq!(g1, G1Affine::one());
                     break;
@@ -918,7 +936,6 @@ pub mod g1 {
             };
             assert!(!p.is_on_curve());
             assert!(p.is_in_correct_subgroup());
-            assert!(!p.is_valid());
         }
 
         // Reject point on a twist (b = 3)
@@ -930,7 +947,6 @@ pub mod g1 {
             };
             assert!(!p.is_on_curve());
             assert!(!p.is_in_correct_subgroup());
-            assert!(!p.is_valid());
         }
 
         // Reject point in an invalid subgroup
@@ -943,7 +959,6 @@ pub mod g1 {
             };
             assert!(p.is_on_curve());
             assert!(!p.is_in_correct_subgroup());
-            assert!(!p.is_valid());
         }
     }
 
@@ -1019,9 +1034,9 @@ pub mod g1 {
             infinity: false
         };
 
-        assert!(a.is_valid());
+        assert!(a.is_on_curve() && a.is_in_correct_subgroup());
-        assert!(b.is_valid());
+        assert!(b.is_on_curve() && b.is_in_correct_subgroup());
-        assert!(c.is_valid());
+        assert!(c.is_on_curve() && c.is_in_correct_subgroup());
 
         let mut tmp1 = a.into_projective();
         tmp1.add_assign(&b.into_projective());
@@ -1097,7 +1112,7 @@ pub mod g1 {
 pub mod g2 {
     use rand::{Rand, Rng};
     use super::super::{Fq2, Fr, Fq, FrRepr, FqRepr};
-    use ::{CurveProjective, CurveAffine, PrimeField, SqrtField, PrimeFieldRepr, Field, BitIterator, EncodedPoint};
+    use ::{CurveProjective, CurveAffine, PrimeField, SqrtField, PrimeFieldRepr, Field, BitIterator, EncodedPoint, GroupDecodingError};
 
     curve_impl!(G2, G2Affine, G2Prepared, Fq2, Fr, G2Uncompressed, G2Compressed);
 
@@ -1120,7 +1135,18 @@ pub mod g2 {
 
         fn empty() -> Self { G2Uncompressed([0; 192]) }
         fn size() -> usize { 192 }
-        fn into_affine_unchecked(&self) -> Result<G2Affine, ()> {
+        fn into_affine(&self) -> Result<G2Affine, GroupDecodingError> {
+            let affine = self.into_affine_unchecked()?;
+
+            if !affine.is_on_curve() {
+                Err(GroupDecodingError::NotOnCurve)
+            } else if !affine.is_in_correct_subgroup() {
+                Err(GroupDecodingError::NotInSubgroup)
+            } else {
+                Ok(affine)
+            }
+        }
+        fn into_affine_unchecked(&self) -> Result<G2Affine, GroupDecodingError> {
             use byteorder::{ReadBytesExt, BigEndian};
 
             // Create a copy of this representation.
@@ -1128,7 +1154,7 @@ pub mod g2 {
 
             if copy[0] & (1 << 7) != 0 {
                 // Distinguisher bit is set, but this should be uncompressed!
-                return Err(())
+                return Err(GroupDecodingError::UnexpectedCompressionMode)
             }
 
             if copy[0] & (1 << 6) != 0 {
@@ -1140,13 +1166,13 @@ pub mod g2 {
                 if copy.iter().all(|b| *b == 0) {
                     Ok(G2Affine::zero())
                 } else {
-                    Err(())
+                    Err(GroupDecodingError::UnexpectedInformation)
                 }
             } else {
                 if copy[0] & (1 << 5) != 0 {
                     // The bit indicating the y-coordinate should be lexicographically
                     // largest is set, but this is an uncompressed element.
-                    return Err(())
+                    return Err(GroupDecodingError::UnexpectedInformation)
                 }
 
                 // Unset the three most significant bits.
@@ -1179,12 +1205,12 @@ pub mod g2 {
 
                 Ok(G2Affine {
                     x: Fq2 {
-                        c0: Fq::from_repr(x_c0)?,
+                        c0: Fq::from_repr(x_c0).map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate (c0)", e))?,
-                        c1: Fq::from_repr(x_c1)?
+                        c1: Fq::from_repr(x_c1).map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate (c1)", e))?,
                     },
                     y: Fq2 {
-                        c0: Fq::from_repr(y_c0)?,
+                        c0: Fq::from_repr(y_c0).map_err(|e| GroupDecodingError::CoordinateDecodingError("y coordinate (c0)", e))?,
-                        c1: Fq::from_repr(y_c1)?
+                        c1: Fq::from_repr(y_c1).map_err(|e| GroupDecodingError::CoordinateDecodingError("y coordinate (c1)", e))?,
                     },
                     infinity: false
                 })
@@ -1242,7 +1268,18 @@ pub mod g2 {
 
         fn empty() -> Self { G2Compressed([0; 96]) }
         fn size() -> usize { 96 }
-        fn into_affine_unchecked(&self) -> Result<G2Affine, ()> {
+        fn into_affine(&self) -> Result<G2Affine, GroupDecodingError> {
+            let affine = self.into_affine_unchecked()?;
+
+            // NB: Decompression guarantees that it is on the curve already.
+
+            if !affine.is_in_correct_subgroup() {
+                Err(GroupDecodingError::NotInSubgroup)
+            } else {
+                Ok(affine)
+            }
+        }
+        fn into_affine_unchecked(&self) -> Result<G2Affine, GroupDecodingError> {
             use byteorder::{ReadBytesExt, BigEndian};
 
             // Create a copy of this representation.
@@ -1250,7 +1287,7 @@ pub mod g2 {
 
             if copy[0] & (1 << 7) == 0 {
                 // Distinguisher bit isn't set.
-                return Err(())
+                return Err(GroupDecodingError::UnexpectedCompressionMode)
             }
 
             if copy[0] & (1 << 6) != 0 {
@@ -1262,7 +1299,7 @@ pub mod g2 {
                 if copy.iter().all(|b| *b == 0) {
                     Ok(G2Affine::zero())
                 } else {
-                    Err(())
+                    Err(GroupDecodingError::UnexpectedInformation)
                 }
             } else {
                 // Determine if the intended y coordinate must be greater
@@ -1289,8 +1326,8 @@ pub mod g2 {
 
                 // Interpret as Fq element.
                 let x = Fq2 {
-                    c0: Fq::from_repr(x_c0)?,
+                    c0: Fq::from_repr(x_c0).map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate (c0)", e))?,
-                    c1: Fq::from_repr(x_c1)?
+                    c1: Fq::from_repr(x_c1).map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate (c1)", e))?
                 };
 
                 // Compute x^3 + b
@@ -1316,7 +1353,7 @@ pub mod g2 {
                     },
                     None => {
                         // Point must not be on the curve.
-                        Err(())
+                        Err(GroupDecodingError::NotOnCurve)
                     }
                 }
             }
@@ -1446,7 +1483,7 @@ pub mod g2 {
                     infinity: false
                 };
 
-                assert!(!p.is_valid());
+                assert!(!p.is_in_correct_subgroup());
 
                 let mut g2 = G2::zero();
 
@@ -1468,7 +1505,7 @@ pub mod g2 {
                     assert_eq!(i, 2);
                     let g2 = G2Affine::from(g2);
 
-                    assert!(g2.is_valid());
+                    assert!(g2.is_in_correct_subgroup());
 
                     assert_eq!(g2, G2Affine::one());
                     break;
@@ -1497,7 +1534,6 @@ pub mod g2 {
             };
             assert!(!p.is_on_curve());
             assert!(p.is_in_correct_subgroup());
-            assert!(!p.is_valid());
         }
 
         // Reject point on a twist (b = 2 * (u + 1))
@@ -1515,7 +1551,6 @@ pub mod g2 {
             };
             assert!(!p.is_on_curve());
             assert!(!p.is_in_correct_subgroup());
-            assert!(!p.is_valid());
         }
 
         // Reject point in an invalid subgroup
@@ -1534,7 +1569,6 @@ pub mod g2 {
             };
             assert!(p.is_on_curve());
             assert!(!p.is_in_correct_subgroup());
-            assert!(!p.is_valid());
         }
     }
 

src/bls12_381/fq.rs

@@ -1,4 +1,4 @@
-use ::{Field, PrimeField, SqrtField, PrimeFieldRepr};
+use ::{Field, PrimeField, SqrtField, PrimeFieldRepr, PrimeFieldDecodingError};
 use std::cmp::Ordering;
 use super::fq2::Fq2;
 
@@ -401,14 +401,14 @@ impl From<Fq> for FqRepr {
 impl PrimeField for Fq {
     type Repr = FqRepr;
 
-    fn from_repr(r: FqRepr) -> Result<Fq, ()> {
+    fn from_repr(r: FqRepr) -> Result<Fq, PrimeFieldDecodingError> {
         let mut r = Fq(r);
         if r.is_valid() {
             r.mul_assign(&Fq(R2));
 
             Ok(r)
         } else {
-            Err(())
+            Err(PrimeFieldDecodingError::NotInField)
         }
     }
 
@@ -1740,6 +1740,6 @@ fn test_fq_ordering() {
     // FqRepr's ordering is well-tested, but we still need to make sure the Fq
     // elements aren't being compared in Montgomery form.
     for i in 0..100 {
-        assert!(Fq::from_repr(FqRepr::from(i+1)) > Fq::from_repr(FqRepr::from(i)));
+        assert!(Fq::from_repr(FqRepr::from(i+1)).unwrap() > Fq::from_repr(FqRepr::from(i)).unwrap());
     }
 }

src/bls12_381/fr.rs

@@ -1,4 +1,4 @@
-use ::{Field, PrimeField, SqrtField, PrimeFieldRepr};
+use ::{Field, PrimeField, SqrtField, PrimeFieldRepr, PrimeFieldDecodingError};
 
 // r = 52435875175126190479447740508185965837690552500527637822603658699938581184513
 const MODULUS: FrRepr = FrRepr([0xffffffff00000001, 0x53bda402fffe5bfe, 0x3339d80809a1d805, 0x73eda753299d7d48]);
@@ -222,14 +222,14 @@ impl From<Fr> for FrRepr {
 impl PrimeField for Fr {
     type Repr = FrRepr;
 
-    fn from_repr(r: FrRepr) -> Result<Fr, ()> {
+    fn from_repr(r: FrRepr) -> Result<Fr, PrimeFieldDecodingError> {
         let mut r = Fr(r);
         if r.is_valid() {
             r.mul_assign(&Fr(R2));
 
             Ok(r)
         } else {
-            Err(())
+            Err(PrimeFieldDecodingError::NotInField)
         }
     }
 

src/lib.rs

@@ -28,6 +28,7 @@ pub mod bls12_381;
 pub mod wnaf;
 
 use std::fmt;
+use std::error::Error;
 
 /// An "engine" is a collection of types (fields, elliptic curve groups, etc.)
 /// with well-defined relationships. In particular, the G1/G2 curve groups are
@@ -179,9 +180,6 @@ pub trait CurveAffine: Copy +
     /// additive identity.
     fn is_zero(&self) -> bool;
 
-    /// Determines if this point is on the curve and in the correct subgroup.
-    fn is_valid(&self) -> bool;
-
     /// Negates this element.
     fn negate(&mut self);
 
@@ -224,21 +222,17 @@ pub trait EncodedPoint: Sized +
     fn size() -> usize;
 
     /// Converts an `EncodedPoint` into a `CurveAffine` element,
-    /// if the point is valid.
+    /// if the encoding represents a valid element.
-    fn into_affine(&self) -> Result<Self::Affine, ()> {
+    fn into_affine(&self) -> Result<Self::Affine, GroupDecodingError>;
-        let affine = self.into_affine_unchecked()?;
-
-        if affine.is_valid() {
-            Ok(affine)
-        } else {
-            Err(())
-        }
-    }
 
     /// Converts an `EncodedPoint` into a `CurveAffine` element,
-    /// without checking if it's a valid point. Caller must be careful
+    /// without guaranteeing that the encoding represents a valid
-    /// when using this, as misuse can violate API invariants.
+    /// element. This is useful when the caller knows the encoding is
-    fn into_affine_unchecked(&self) -> Result<Self::Affine, ()>;
+    /// valid already.
+    ///
+    /// If the encoding is invalid, this can break API invariants,
+    /// so caution is strongly encouraged.
+    fn into_affine_unchecked(&self) -> Result<Self::Affine, GroupDecodingError>;
 
     /// Creates an `EncodedPoint` from an affine point, as long as the
     /// point is not the point at infinity.
@@ -368,6 +362,65 @@ pub trait PrimeFieldRepr: Sized +
     fn mul2(&mut self);
 }
 
+#[derive(Debug)]
+pub enum PrimeFieldDecodingError {
+    // The encoded value is not in the field
+    NotInField
+}
+
+impl Error for PrimeFieldDecodingError {
+    fn description(&self) -> &str {
+        match self {
+            &PrimeFieldDecodingError::NotInField => "not an element in the field"
+        }
+    }
+}
+
+impl fmt::Display for PrimeFieldDecodingError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        write!(f, "{}", self.description())
+    }
+}
+
+#[derive(Debug)]
+pub enum GroupDecodingError {
+    /// The coordinate(s) do not lie on the curve.
+    NotOnCurve,
+    /// The element is not part of the r-order subgroup.
+    NotInSubgroup,
+    /// One of the coordinates could not be decoded
+    CoordinateDecodingError(&'static str, PrimeFieldDecodingError),
+    /// The compression mode of the encoded elemnet was not as expected
+    UnexpectedCompressionMode,
+    /// The encoding contained bits that should not have been set
+    UnexpectedInformation
+}
+
+impl Error for GroupDecodingError {
+    fn description(&self) -> &str {
+        match self {
+            &GroupDecodingError::NotOnCurve => "coordinate(s) do not lie on the curve",
+            &GroupDecodingError::NotInSubgroup => "the element is not part of an r-order subgroup",
+            &GroupDecodingError::CoordinateDecodingError(..) => "coordinate(s) could not be decoded",
+            &GroupDecodingError::UnexpectedCompressionMode => "encoding has unexpected compression mode",
+            &GroupDecodingError::UnexpectedInformation => "encoding has unexpected information"
+        }
+    }
+}
+
+impl fmt::Display for GroupDecodingError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        match self {
+            &GroupDecodingError::CoordinateDecodingError(description, ref err) => {
+                write!(f, "{} decoding error: {}", description, err)
+            },
+            _ => {
+                write!(f, "{}", self.description())
+            }
+        }
+    }
+}
+
 /// This represents an element of a prime field.
 pub trait PrimeField: Field
 {
@@ -376,7 +429,7 @@ pub trait PrimeField: Field
     type Repr: PrimeFieldRepr + From<Self>;
 
     /// Convert this prime field element into a biginteger representation.
-    fn from_repr(Self::Repr) -> Result<Self, ()>;
+    fn from_repr(Self::Repr) -> Result<Self, PrimeFieldDecodingError>;
 
     /// Convert a biginteger reprensentation into a prime field element, if
     /// the number is an element of the field.