bls: uncompressed serialization

Cargo.lock

@@ -128,7 +128,7 @@ dependencies = [
 [[package]]
 name = "amcl"
 version = "0.3.0"
-source = "git+https://github.com/sigp/milagro_bls?tag=v1.4.2#16655aa033175a90c10ef02aa144e2835de23aec"
+source = "git+https://github.com/sigp/milagro_bls?branch=uncompressed#ae944c03a4ae43df29a5cd7d35c7ad82b7cb937a"
 
 [[package]]
 name = "ansi_term"
@@ -578,13 +578,14 @@ version = "0.2.0"
 dependencies = [
  "arbitrary",
  "blst",
+ "criterion",
  "eth2_hashing 0.3.0",
  "eth2_serde_utils",
  "eth2_ssz",
  "ethereum-types 0.12.1",
  "hex",
  "milagro_bls",
- "rand 0.7.3",
+ "rand 0.8.5",
  "serde",
  "serde_derive",
  "tree_hash",
@@ -3987,13 +3988,13 @@ dependencies = [
 
 [[package]]
 name = "milagro_bls"
-version = "1.4.2"
-source = "git+https://github.com/sigp/milagro_bls?tag=v1.4.2#16655aa033175a90c10ef02aa144e2835de23aec"
+version = "1.5.0"
+source = "git+https://github.com/sigp/milagro_bls?branch=uncompressed#ae944c03a4ae43df29a5cd7d35c7ad82b7cb937a"
 dependencies = [
  "amcl",
  "hex",
  "lazy_static",
- "rand 0.7.3",
+ "rand 0.8.5",
  "zeroize",
 ]
 

crypto/bls/Cargo.toml

@@ -7,8 +7,9 @@ edition = "2021"
 [dependencies]
 eth2_ssz = "0.4.1"
 tree_hash = "0.4.1"
-milagro_bls = { git = "https://github.com/sigp/milagro_bls", tag = "v1.4.2", optional = true }
-rand = "0.7.3"
+# FIXME(bls): update this to v1.5+ once issue is fixed
+milagro_bls = { git = "https://github.com/sigp/milagro_bls", branch = "uncompressed", optional = true }
+rand = "0.8.5"
 serde = "1.0.116"
 serde_derive = "1.0.116"
 eth2_serde_utils = "0.1.1"
@@ -26,3 +27,10 @@ milagro = ["milagro_bls"]
 supranational = ["blst"]
 supranational-portable = ["supranational", "blst/portable"]
 supranational-force-adx = ["supranational", "blst/force-adx"]
+
+[dev-dependencies]
+criterion = "0.3.3"
+
+[[bench]]
+name = "compress_decompress"
+harness = false

crypto/bls/benches/compress_decompress.rs

@@ -0,0 +1,64 @@
+use bls::{PublicKey, SecretKey};
+use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
+
+pub fn compress(c: &mut Criterion) {
+    let private_key = SecretKey::random();
+    let public_key = private_key.public_key();
+    c.bench_with_input(
+        BenchmarkId::new("compress", 1),
+        &public_key,
+        |b, public_key| {
+            b.iter(|| public_key.compress());
+        },
+    );
+}
+
+pub fn decompress(c: &mut Criterion) {
+    let private_key = SecretKey::random();
+    let public_key_bytes = private_key.public_key().compress();
+    c.bench_with_input(
+        BenchmarkId::new("decompress", 1),
+        &public_key_bytes,
+        |b, public_key_bytes| {
+            b.iter(|| public_key_bytes.decompress().unwrap());
+        },
+    );
+}
+
+pub fn deserialize_uncompressed(c: &mut Criterion) {
+    let private_key = SecretKey::random();
+    let public_key_bytes = private_key.public_key().serialize_uncompressed();
+    c.bench_with_input(
+        BenchmarkId::new("deserialize_uncompressed", 1),
+        &public_key_bytes,
+        |b, public_key_bytes| {
+            b.iter(|| PublicKey::deserialize_uncompressed(public_key_bytes).unwrap());
+        },
+    );
+}
+
+pub fn compress_all(c: &mut Criterion) {
+    let n = 500_000;
+    let keys = (0..n)
+        .map(|_| {
+            let private_key = SecretKey::random();
+            private_key.public_key()
+        })
+        .collect::<Vec<_>>();
+    c.bench_with_input(BenchmarkId::new("compress", n), &keys, |b, keys| {
+        b.iter(|| {
+            for key in keys {
+                key.compress();
+            }
+        });
+    });
+}
+
+criterion_group!(
+    benches,
+    compress,
+    decompress,
+    deserialize_uncompressed,
+    compress_all
+);
+criterion_main!(benches);

crypto/bls/src/generic_public_key.rs

@@ -11,6 +11,9 @@ use tree_hash::TreeHash;
 /// The byte-length of a BLS public key when serialized in compressed form.
 pub const PUBLIC_KEY_BYTES_LEN: usize = 48;
 
+/// The byte-length of a BLS public key when serialized in uncompressed form.
+pub const PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN: usize = 96;
+
 /// Represents the public key at infinity.
 pub const INFINITY_PUBLIC_KEY: [u8; PUBLIC_KEY_BYTES_LEN] = [
     0xc0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
@@ -23,8 +26,17 @@ pub trait TPublicKey: Sized + Clone {
     /// Serialize `self` as compressed bytes.
     fn serialize(&self) -> [u8; PUBLIC_KEY_BYTES_LEN];
 
+    /// Serialize `self` as uncompressed bytes.
+    fn serialize_uncompressed(&self) -> [u8; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN];
+
     /// Deserialize `self` from compressed bytes.
     fn deserialize(bytes: &[u8]) -> Result<Self, Error>;
+
+    /// Deserialize `self` from uncompressed bytes.
+    ///
+    /// This function *does not* perform thorough checks of the input bytes and should only be
+    /// used with bytes output from `Self::serialize_uncompressed`.
+    fn deserialize_uncompressed(bytes: &[u8]) -> Result<Self, Error>;
 }
 
 /// A BLS public key that is generic across some BLS point (`Pub`).
@@ -65,6 +77,11 @@ where
         self.point.serialize()
     }
 
+    /// Serialize `self` as uncompressed bytes.
+    pub fn serialize_uncompressed(&self) -> [u8; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN] {
+        self.point.serialize_uncompressed()
+    }
+
     /// Deserialize `self` from compressed bytes.
     pub fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         if bytes == &INFINITY_PUBLIC_KEY[..] {
@@ -75,6 +92,13 @@ where
             })
         }
     }
+
+    /// Deserialize `self` from compressed bytes.
+    pub fn deserialize_uncompressed(bytes: &[u8]) -> Result<Self, Error> {
+        Ok(Self {
+            point: Pub::deserialize_uncompressed(bytes)?,
+        })
+    }
 }
 
 impl<Pub: TPublicKey> Eq for GenericPublicKey<Pub> {}

crypto/bls/src/impls/blst.rs

@@ -1,10 +1,12 @@
 use crate::{
     generic_aggregate_public_key::TAggregatePublicKey,
     generic_aggregate_signature::TAggregateSignature,
-    generic_public_key::{GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN},
+    generic_public_key::{
+        GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN, PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN,
+    },
     generic_secret_key::TSecretKey,
     generic_signature::{TSignature, SIGNATURE_BYTES_LEN},
-    Error, Hash256, ZeroizeHash, INFINITY_SIGNATURE,
+    BlstError, Error, Hash256, ZeroizeHash, INFINITY_SIGNATURE,
 };
 pub use blst::min_pk as blst_core;
 use blst::{blst_scalar, BLST_ERROR};
@@ -123,6 +125,10 @@ impl TPublicKey for blst_core::PublicKey {
         self.compress()
     }
 
+    fn serialize_uncompressed(&self) -> [u8; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN] {
+        blst_core::PublicKey::serialize(self)
+    }
+
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         // key_validate accepts uncompressed bytes too so enforce byte length here.
         // It also does subgroup checks, noting infinity check is done in `generic_public_key.rs`.
@@ -134,6 +140,19 @@ impl TPublicKey for blst_core::PublicKey {
         }
         Self::key_validate(bytes).map_err(Into::into)
     }
+
+    fn deserialize_uncompressed(bytes: &[u8]) -> Result<Self, Error> {
+        if bytes.len() != PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN {
+            return Err(Error::InvalidByteLength {
+                got: bytes.len(),
+                expected: PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN,
+            });
+        }
+        // Ensure we use the `blst` function rather than the one from this trait.
+        let result: Result<Self, BlstError> = Self::deserialize(bytes);
+        let key = result?;
+        Ok(key)
+    }
 }
 
 /// A wrapper that allows for `PartialEq` and `Clone` impls.

crypto/bls/src/impls/fake_crypto.rs

@@ -1,7 +1,9 @@
 use crate::{
     generic_aggregate_public_key::TAggregatePublicKey,
     generic_aggregate_signature::TAggregateSignature,
-    generic_public_key::{GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN},
+    generic_public_key::{
+        GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN, PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN,
+    },
     generic_secret_key::{TSecretKey, SECRET_KEY_BYTES_LEN},
     generic_signature::{TSignature, SIGNATURE_BYTES_LEN},
     Error, Hash256, ZeroizeHash, INFINITY_PUBLIC_KEY, INFINITY_SIGNATURE,
@@ -46,11 +48,19 @@ impl TPublicKey for PublicKey {
         self.0
     }
 
+    fn serialize_uncompressed(&self) -> [u8; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN] {
+        panic!("fake_crypto does not support uncompressed keys")
+    }
+
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         let mut pubkey = Self::infinity();
         pubkey.0[..].copy_from_slice(&bytes[0..PUBLIC_KEY_BYTES_LEN]);
         Ok(pubkey)
     }
+
+    fn deserialize_uncompressed(_: &[u8]) -> Result<Self, Error> {
+        panic!("fake_crypto does not support uncompressed keys")
+    }
 }
 
 impl Eq for PublicKey {}

crypto/bls/src/impls/milagro.rs

@@ -1,7 +1,9 @@
 use crate::{
     generic_aggregate_public_key::TAggregatePublicKey,
     generic_aggregate_signature::TAggregateSignature,
-    generic_public_key::{GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN},
+    generic_public_key::{
+        GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN, PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN,
+    },
     generic_secret_key::{TSecretKey, SECRET_KEY_BYTES_LEN},
     generic_signature::{TSignature, SIGNATURE_BYTES_LEN},
     Error, Hash256, ZeroizeHash,
@@ -76,14 +78,20 @@ pub fn verify_signature_sets<'a>(
 
 impl TPublicKey for milagro::PublicKey {
     fn serialize(&self) -> [u8; PUBLIC_KEY_BYTES_LEN] {
-        let mut bytes = [0; PUBLIC_KEY_BYTES_LEN];
-        bytes[..].copy_from_slice(&self.as_bytes());
-        bytes
+        self.as_bytes()
+    }
+
+    fn serialize_uncompressed(&self) -> [u8; PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN] {
+        self.as_uncompressed_bytes()
     }
 
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         Self::from_bytes(bytes).map_err(Into::into)
     }
+
+    fn deserialize_uncompressed(bytes: &[u8]) -> Result<Self, Error> {
+        Self::from_uncompressed_bytes(bytes).map_err(Into::into)
+    }
 }
 
 impl TAggregatePublicKey<milagro::PublicKey> for milagro::AggregatePublicKey {

crypto/bls/src/lib.rs

@@ -35,7 +35,9 @@ mod zeroize_hash;
 
 pub mod impls;
 
-pub use generic_public_key::{INFINITY_PUBLIC_KEY, PUBLIC_KEY_BYTES_LEN};
+pub use generic_public_key::{
+    INFINITY_PUBLIC_KEY, PUBLIC_KEY_BYTES_LEN, PUBLIC_KEY_UNCOMPRESSED_BYTES_LEN,
+};
 pub use generic_secret_key::SECRET_KEY_BYTES_LEN;
 pub use generic_signature::{INFINITY_SIGNATURE, SIGNATURE_BYTES_LEN};
 pub use get_withdrawal_credentials::get_withdrawal_credentials;

crypto/bls/tests/tests.rs

@@ -341,6 +341,11 @@ macro_rules! test_suite {
                 .assert_single_message_verify(true)
         }
 
+        #[test]
+        fn deserialize_infinity_public_key() {
+            PublicKey::deserialize(&bls::INFINITY_PUBLIC_KEY).unwrap_err();
+        }
+
         /// A helper struct to make it easer to deal with `SignatureSet` lifetimes.
         struct OwnedSignatureSet {
             signature: AggregateSignature,

testing/ef_tests/src/cases/bls_verify_msg.rs

@@ -1,7 +1,7 @@
 use super::*;
 use crate::case_result::compare_result;
 use crate::impl_bls_load_case;
-use bls::{PublicKeyBytes, Signature, SignatureBytes};
+use bls::{PublicKey, PublicKeyBytes, Signature, SignatureBytes};
 use serde_derive::Deserialize;
 use std::convert::TryInto;
 use types::Hash256;
@@ -30,6 +30,13 @@ impl Case for BlsVerify {
             .try_into()
             .and_then(|signature: Signature| {
                 let pk = self.input.pubkey.decompress()?;
+
+                // Check serialization roundtrip.
+                let pk_uncompressed = pk.serialize_uncompressed();
+                let pk_from_uncompressed = PublicKey::deserialize_uncompressed(&pk_uncompressed)
+                    .expect("uncompressed serialization should round-trip");
+                assert_eq!(pk_from_uncompressed, pk);
+
                 Ok(signature.verify(&pk, Hash256::from_slice(&message)))
             })
             .unwrap_or(false);