From 03fde9873735775ce46b9d640968330fd69d4917 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 20 Oct 2022 16:35:51 +1100 Subject: [PATCH] bls: uncompressed serialization --- Cargo.lock | 11 ++-- crypto/bls/Cargo.toml | 12 +++- crypto/bls/benches/compress_decompress.rs | 64 ++++++++++++++++++++ crypto/bls/src/generic_public_key.rs | 24 ++++++++ crypto/bls/src/impls/blst.rs | 23 ++++++- crypto/bls/src/impls/fake_crypto.rs | 12 +++- crypto/bls/src/impls/milagro.rs | 16 +++-- crypto/bls/src/lib.rs | 4 +- crypto/bls/tests/tests.rs | 5 ++ testing/ef_tests/src/cases/bls_verify_msg.rs | 9 ++- 10 files changed, 164 insertions(+), 16 deletions(-) create mode 100644 crypto/bls/benches/compress_decompress.rs diff --git a/Cargo.lock b/Cargo.lock index b93793d6d7..026ff52841 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/crypto/bls/Cargo.toml b/crypto/bls/Cargo.toml index 9ac468d227..3053982506 100644 --- a/crypto/bls/Cargo.toml +++ b/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 diff --git a/crypto/bls/benches/compress_decompress.rs b/crypto/bls/benches/compress_decompress.rs new file mode 100644 index 0000000000..3053cf1f9a --- /dev/null +++ b/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::>(); + 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); diff --git a/crypto/bls/src/generic_public_key.rs b/crypto/bls/src/generic_public_key.rs index 847d039c62..c0011d4f5b 100644 --- a/crypto/bls/src/generic_public_key.rs +++ b/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; + + /// 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; } /// 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 { if bytes == &INFINITY_PUBLIC_KEY[..] { @@ -75,6 +92,13 @@ where }) } } + + /// Deserialize `self` from compressed bytes. + pub fn deserialize_uncompressed(bytes: &[u8]) -> Result { + Ok(Self { + point: Pub::deserialize_uncompressed(bytes)?, + }) + } } impl Eq for GenericPublicKey {} diff --git a/crypto/bls/src/impls/blst.rs b/crypto/bls/src/impls/blst.rs index bd28abff9f..10a073c6c8 100644 --- a/crypto/bls/src/impls/blst.rs +++ b/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 { // 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 { + 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::deserialize(bytes); + let key = result?; + Ok(key) + } } /// A wrapper that allows for `PartialEq` and `Clone` impls. diff --git a/crypto/bls/src/impls/fake_crypto.rs b/crypto/bls/src/impls/fake_crypto.rs index f2d8b79b98..a09fb347e6 100644 --- a/crypto/bls/src/impls/fake_crypto.rs +++ b/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 { let mut pubkey = Self::infinity(); pubkey.0[..].copy_from_slice(&bytes[0..PUBLIC_KEY_BYTES_LEN]); Ok(pubkey) } + + fn deserialize_uncompressed(_: &[u8]) -> Result { + panic!("fake_crypto does not support uncompressed keys") + } } impl Eq for PublicKey {} diff --git a/crypto/bls/src/impls/milagro.rs b/crypto/bls/src/impls/milagro.rs index eb4767d3c7..4c65933103 100644 --- a/crypto/bls/src/impls/milagro.rs +++ b/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::from_bytes(bytes).map_err(Into::into) } + + fn deserialize_uncompressed(bytes: &[u8]) -> Result { + Self::from_uncompressed_bytes(bytes).map_err(Into::into) + } } impl TAggregatePublicKey for milagro::AggregatePublicKey { diff --git a/crypto/bls/src/lib.rs b/crypto/bls/src/lib.rs index 750e1bd5b8..e588b3b355 100644 --- a/crypto/bls/src/lib.rs +++ b/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; diff --git a/crypto/bls/tests/tests.rs b/crypto/bls/tests/tests.rs index ad498dbfa8..40c0a4a793 100644 --- a/crypto/bls/tests/tests.rs +++ b/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, diff --git a/testing/ef_tests/src/cases/bls_verify_msg.rs b/testing/ef_tests/src/cases/bls_verify_msg.rs index 779b3cf75f..ac81c2a9bd 100644 --- a/testing/ef_tests/src/cases/bls_verify_msg.rs +++ b/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);