Merge pull request #5 from ArnaudBrousseau/arkworks-suggestions

Serialization simplification and better errors

src/errors.rs

@@ -0,0 +1,44 @@
+//! Error enum to wrap underlying failures
+use std::error::Error;
+use std::fmt;
+
+use ark_serialize::SerializationError;
+
+/// Error enum to wrap underlying failures in BLS operations, or wrap errors from dependencies.
+/// Inspired by the excellent https://blog.burntsushi.net/rust-error-handling
+#[derive(Debug)]
+pub enum BLSError {
+    /// Happens when the infinity bit is set in an encoding point, but the rest of the bytes aren't correctly zero'd
+    BadInfinityEncoding(usize),
+    /// Error coming from `ark_serialize` upon deserialization
+    DeserializationError(SerializationError),
+    /// Error coming from `I2OSP` (see RFC 8017, section 4.1)
+    /// <https://datatracker.ietf.org/doc/html/rfc8017#section-4.1>
+    IntegerTooLarge(u64, usize),
+    NoSignaturesToAggregate,
+    SignatureNotInCorrectSubgroup,
+    WrongSizeForSignature(usize),
+    WrongSizeForPublicKey(usize),
+}
+
+impl Error for BLSError {}
+
+impl fmt::Display for BLSError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            BLSError::BadInfinityEncoding(i) => write!(f, "Bad encoding: infinity bit set but found non-zero bits in byte at index {i}"),
+            BLSError::DeserializationError(ref err) => err.fmt(f),
+            BLSError::IntegerTooLarge(ref n, l) => write!(f, "Integer too large: cannot fit {n} into a byte string of length {l}"),
+            BLSError::NoSignaturesToAggregate => write!(f, "Cannot aggregate signatures: no signatures were passed in"),
+            BLSError::SignatureNotInCorrectSubgroup => write!(f, "Signature point is not in the correct subgroup. Please check the passed in secret key value."),
+            BLSError::WrongSizeForSignature(l) => write!(f, "Signature bytes must have length 96 or 192. Got {l}"),
+            BLSError::WrongSizeForPublicKey(l) => write!(f, "Public key bytes must have length 48 or 96. Got {l}"),
+        }
+    }
+}
+
+impl From<SerializationError> for BLSError {
+    fn from(err: SerializationError) -> BLSError {
+        BLSError::DeserializationError(err)
+    }
+}

src/lib.rs

@@ -37,9 +37,11 @@ use hkdf::Hkdf;
 use num_bigint::{BigInt, Sign};
 use sha2::{Digest, Sha256};
 
+pub mod errors;
 mod serialization;
 pub mod types;
 
+use errors::*;
 use types::*;
 
 /// Domain separation tags to use if you're working with Ethereum
@@ -71,7 +73,7 @@ fn pairing(u: G2AffinePoint, v: G1AffinePoint) -> BLS12381Pairing {
 fn i2osp(x: u64, x_len: usize) -> Result<Vec<u8>, BLSError> {
     // 1
     if x > 256_u64.pow(x_len.try_into().unwrap()) {
-        return Err(BLSError::IntegerTooLarge);
+        return Err(BLSError::IntegerTooLarge(x, x_len));
     }
 
     // 2
@@ -156,14 +158,22 @@ pub fn point_to_signature_uncompressed(p: G2AffinePoint) -> Signature {
 /// Invoke the appropriate deserialization routine depending on signature variant
 /// For minimal-pubkey-size: `pubkey_to_point(ostr) := octets_to_point_E1(ostr)`
 pub fn pubkey_to_point(pk: &PublicKey) -> Result<G1AffinePoint, BLSError> {
-    serialization::octets_to_point_e1(pk)
+    match pk.len() {
+        48 => serialization::octets_to_point_e1(pk),
+        96 => serialization::octets_to_point_e1_uncompressed(pk),
+        _ => Err(BLSError::WrongSizeForPublicKey(pk.len())),
+    }
 }
 
 /// ([spec link](https://www.ietf.org/archive/id/draft-irtf-cfrg-bls-signature-05.html#section-2.2))
 /// Invoke the appropriate deserialization routine depending on signature variant
 /// For minimal-pubkey-size: signature_to_point(ostr) := octets_to_point_E2(ostr)
 pub fn signature_to_point(signature: &Signature) -> Result<G2AffinePoint, BLSError> {
-    serialization::octets_to_point_e2(signature)
+    match signature.len() {
+        96 => serialization::octets_to_point_e2(signature),
+        192 => serialization::octets_to_point_e2_uncompressed(signature),
+        _ => Err(BLSError::WrongSizeForSignature(signature.len())),
+    }
 }
 
 /// ([spec link](https://www.ietf.org/archive/id/draft-irtf-cfrg-bls-signature-05.html#section-2.3))
@@ -207,7 +217,8 @@ pub fn keygen(ikm: &Octets) -> SecretKey {
         // we use the libm crate, since core doesn't have support for math.
         let l = libm::ceil((3_f64 * BLSFr::MODULUS_BIT_SIZE as f64) / 16_f64) as u64;
         let info = i2osp(l, 2).expect("unable to convert L to octet bytes");
-        let mut okm = [0u8; 42];
+        // .try_into().unwrap() is okay here, L is a static value!
+        let mut okm = vec![0u8; l.try_into().unwrap()];
         hk.expand(&info, &mut okm).expect("unable to expand HKDF");
 
         // 4
@@ -518,20 +529,20 @@ fn subgroup_check_e2(p: G2AffinePoint) -> bool {
 
 #[cfg(test)]
 mod test {
+    use super::*;
     use hex::ToHex;
     use hex_literal::hex;
     use num_bigint::ToBigInt;
     use rand_core::{OsRng, RngCore};
 
-    use crate::serialization::octets_to_point_e2;
-
-    use super::*;
-
     #[test]
     fn test_i2osp() {
         assert_eq!(i2osp(1, 1).unwrap(), vec![0b00000001],);
         assert_eq!(i2osp(255, 1).unwrap(), vec![0b11111111],);
-        assert_eq!(i2osp(257, 1).unwrap_err(), BLSError::IntegerTooLarge,);
+        assert_eq!(
+            i2osp(257, 1).unwrap_err().to_string(),
+            "Integer too large: cannot fit 257 into a byte string of length 1"
+        );
         assert_eq!(i2osp(259, 2).unwrap(), vec![0b00000001, 0b00000011],);
     }
 
@@ -648,8 +659,8 @@ mod test {
         // To do this we parse the coordinates as an uncompressed G2 point, then compare to the point obtained from parsing our actual signature.
         // They should be the same!
         assert_eq!(
-            octets_to_point_e2(&signature).unwrap(),
-            octets_to_point_e2(
+            signature_to_point(&signature).unwrap(),
+            signature_to_point(
                 &hex::decode(format!(
                     "{}{}{}{}",
                     x_c1.encode_hex::<String>(),