From 258eeb5f09c8280f9c6676ff7396fac266c59cc5 Mon Sep 17 00:00:00 2001 From: chonghe <44791194+chong-he@users.noreply.github.com> Date: Thu, 7 Mar 2024 07:17:42 +0800 Subject: [PATCH] Delete milagro library (#5298) * fix lib.rs and tests.rs * update decode.rs * auto-delete in Cargo.lock * delete milagro in cargo.toml * remove milagro from makefile * remove milagro from the name * delete milagro in comment * delete milagro in cargo.toml * delete in /testing/ef_tests/cargo.toml * delete milagro in the logical OR * delete milagro in /lighthouse/src/main.rs * delete milagro in /crypto/bls/tests/tests.rs * delete milagro in comment * delete milagro in /testing//ef_test/src//cases/bls_eth_aggregate_pubkeys.rs * delete milagro * delete more in lib.rs * delete more in lib.rs * delete more in lib.rs * delete milagro in /crypto/bls/src/lib.rs * delete milagro in crypto/bls/src/mod.rs * delete milagro.rs --- .github/workflows/test-suite.yml | 2 +- Cargo.lock | 18 -- Makefile | 2 - consensus/types/src/attestation.rs | 2 +- crypto/bls/Cargo.toml | 2 - crypto/bls/src/impls/milagro.rs | 194 ------------------ crypto/bls/src/impls/mod.rs | 2 - crypto/bls/src/lib.rs | 30 +-- crypto/bls/tests/tests.rs | 5 - lighthouse/Cargo.toml | 2 - lighthouse/src/main.rs | 2 - testing/ef_tests/Cargo.toml | 1 - .../src/cases/bls_eth_aggregate_pubkeys.rs | 4 - testing/ef_tests/src/decode.rs | 4 +- 14 files changed, 6 insertions(+), 264 deletions(-) delete mode 100644 crypto/bls/src/impls/milagro.rs diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 50ccb02ad..975dc1c8b 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -211,7 +211,7 @@ jobs: channel: stable cache-target: release bins: cargo-nextest - - name: Run consensus-spec-tests with blst, milagro and fake_crypto + - name: Run consensus-spec-tests with blst and fake_crypto run: make nextest-ef - name: Show cache stats if: env.SELF_HOSTED_RUNNERS == 'true' diff --git a/Cargo.lock b/Cargo.lock index 1bdced9c7..135c4efe9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -268,11 +268,6 @@ dependencies = [ "syn 2.0.52", ] -[[package]] -name = "amcl" -version = "0.3.0" -source = "git+https://github.com/sigp/milagro_bls?tag=v1.5.1#d3fc0a40cfe8b72ccda46ba050ee6786a59ce753" - [[package]] name = "android-tzdata" version = "0.1.1" @@ -1162,7 +1157,6 @@ dependencies = [ "ethereum_serde_utils", "ethereum_ssz", "hex", - "milagro_bls", "rand", "serde", "tree_hash", @@ -5468,18 +5462,6 @@ dependencies = [ "quote", ] -[[package]] -name = "milagro_bls" -version = "1.5.1" -source = "git+https://github.com/sigp/milagro_bls?tag=v1.5.1#d3fc0a40cfe8b72ccda46ba050ee6786a59ce753" -dependencies = [ - "amcl", - "hex", - "lazy_static", - "rand", - "zeroize", -] - [[package]] name = "mime" version = "0.3.17" diff --git a/Makefile b/Makefile index 8392d0017..6b6418cb8 100644 --- a/Makefile +++ b/Makefile @@ -143,7 +143,6 @@ run-ef-tests: rm -rf $(EF_TESTS)/.accessed_file_log.txt cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES)" cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),fake_crypto" - cargo test --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),milagro" ./$(EF_TESTS)/check_all_files_accessed.py $(EF_TESTS)/.accessed_file_log.txt $(EF_TESTS)/consensus-spec-tests # Runs EF test vectors with nextest @@ -151,7 +150,6 @@ nextest-run-ef-tests: rm -rf $(EF_TESTS)/.accessed_file_log.txt cargo nextest run --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES)" cargo nextest run --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),fake_crypto" - cargo nextest run --release -p ef_tests --features "ef_tests,$(EF_TEST_FEATURES),milagro" ./$(EF_TESTS)/check_all_files_accessed.py $(EF_TESTS)/.accessed_file_log.txt $(EF_TESTS)/consensus-spec-tests # Run the tests in the `beacon_chain` crate for all known forks. diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index ac4a583cb..d1d75523a 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -125,7 +125,7 @@ mod tests { // Check the in-memory size of an `Attestation`, which is useful for reasoning about memory // and preventing regressions. // - // This test will only pass with `blst`, if we run these tests with Milagro or another + // This test will only pass with `blst`, if we run these tests with another // BLS library in future we will have to make it generic. #[test] fn size_of() { diff --git a/crypto/bls/Cargo.toml b/crypto/bls/Cargo.toml index 1216fc2a9..7aa8e02dc 100644 --- a/crypto/bls/Cargo.toml +++ b/crypto/bls/Cargo.toml @@ -7,7 +7,6 @@ edition = { workspace = true } [dependencies] ethereum_ssz = { workspace = true } tree_hash = { workspace = true } -milagro_bls = { git = "https://github.com/sigp/milagro_bls", tag = "v1.5.1", optional = true } rand = { workspace = true } serde = { workspace = true } ethereum_serde_utils = { workspace = true } @@ -22,7 +21,6 @@ blst = { version = "0.3.3", optional = true } arbitrary = [] default = ["supranational"] fake_crypto = [] -milagro = ["milagro_bls"] supranational = ["blst"] supranational-portable = ["supranational", "blst/portable"] supranational-force-adx = ["supranational", "blst/force-adx"] diff --git a/crypto/bls/src/impls/milagro.rs b/crypto/bls/src/impls/milagro.rs deleted file mode 100644 index eb4767d3c..000000000 --- a/crypto/bls/src/impls/milagro.rs +++ /dev/null @@ -1,194 +0,0 @@ -use crate::{ - generic_aggregate_public_key::TAggregatePublicKey, - generic_aggregate_signature::TAggregateSignature, - generic_public_key::{GenericPublicKey, TPublicKey, PUBLIC_KEY_BYTES_LEN}, - generic_secret_key::{TSecretKey, SECRET_KEY_BYTES_LEN}, - generic_signature::{TSignature, SIGNATURE_BYTES_LEN}, - Error, Hash256, ZeroizeHash, -}; -pub use milagro_bls as milagro; -use rand::thread_rng; -use std::iter::ExactSizeIterator; - -/// Provides the externally-facing, core BLS types. -pub mod types { - pub use super::milagro::AggregatePublicKey; - pub use super::milagro::AggregateSignature; - pub use super::milagro::PublicKey; - pub use super::milagro::SecretKey; - pub use super::milagro::Signature; - pub use super::verify_signature_sets; - pub use super::SignatureSet; -} - -pub type SignatureSet<'a> = crate::generic_signature_set::GenericSignatureSet< - 'a, - milagro::PublicKey, - milagro::AggregatePublicKey, - milagro::Signature, - milagro::AggregateSignature, ->; - -pub fn verify_signature_sets<'a>( - signature_sets: impl ExactSizeIterator>, -) -> bool { - if signature_sets.len() == 0 { - return false; - } - - signature_sets - .map(|signature_set| { - let mut aggregate = milagro::AggregatePublicKey::from_public_key( - signature_set.signing_keys.first().ok_or(())?.point(), - ); - - for signing_key in signature_set.signing_keys.iter().skip(1) { - aggregate.add(signing_key.point()) - } - - if signature_set.signature.point().is_none() { - return Err(()); - } - - Ok(( - signature_set.signature.as_ref(), - aggregate, - signature_set.message, - )) - }) - .collect::, ()>>() - .map(|aggregates| { - milagro::AggregateSignature::verify_multiple_aggregate_signatures( - &mut rand::thread_rng(), - aggregates.iter().map(|(signature, aggregate, message)| { - ( - signature - .point() - .expect("guarded against none by previous check"), - aggregate, - message.as_bytes(), - ) - }), - ) - }) - .unwrap_or(false) -} - -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 - } - - fn deserialize(bytes: &[u8]) -> Result { - Self::from_bytes(bytes).map_err(Into::into) - } -} - -impl TAggregatePublicKey for milagro::AggregatePublicKey { - fn to_public_key(&self) -> GenericPublicKey { - GenericPublicKey::from_point(milagro::PublicKey { - point: self.point.clone(), - }) - } - - fn aggregate(pubkeys: &[GenericPublicKey]) -> Result { - let pubkey_refs = pubkeys.iter().map(|pk| pk.point()).collect::>(); - Ok(milagro::AggregatePublicKey::aggregate(&pubkey_refs)?) - } -} - -impl TSignature for milagro::Signature { - fn serialize(&self) -> [u8; SIGNATURE_BYTES_LEN] { - let mut bytes = [0; SIGNATURE_BYTES_LEN]; - - bytes[..].copy_from_slice(&self.as_bytes()); - - bytes - } - - fn deserialize(bytes: &[u8]) -> Result { - milagro::Signature::from_bytes(&bytes).map_err(Error::MilagroError) - } - - fn verify(&self, pubkey: &milagro::PublicKey, msg: Hash256) -> bool { - self.verify(msg.as_bytes(), pubkey) - } -} - -impl TAggregateSignature - for milagro::AggregateSignature -{ - fn infinity() -> Self { - milagro::AggregateSignature::new() - } - - fn add_assign(&mut self, other: &milagro::Signature) { - self.add(other) - } - - fn add_assign_aggregate(&mut self, other: &Self) { - self.add_aggregate(other) - } - - fn serialize(&self) -> [u8; SIGNATURE_BYTES_LEN] { - let mut bytes = [0; SIGNATURE_BYTES_LEN]; - - bytes[..].copy_from_slice(&self.as_bytes()); - - bytes - } - - fn deserialize(bytes: &[u8]) -> Result { - milagro::AggregateSignature::from_bytes(&bytes).map_err(Error::MilagroError) - } - - fn fast_aggregate_verify( - &self, - msg: Hash256, - pubkeys: &[&GenericPublicKey], - ) -> bool { - let pubkeys = pubkeys.iter().map(|pk| pk.point()).collect::>(); - self.fast_aggregate_verify(msg.as_bytes(), &pubkeys) - } - - fn aggregate_verify( - &self, - msgs: &[Hash256], - pubkeys: &[&GenericPublicKey], - ) -> bool { - let pubkeys = pubkeys.iter().map(|pk| pk.point()).collect::>(); - let msgs = msgs.iter().map(|hash| hash.as_bytes()).collect::>(); - self.aggregate_verify(&msgs, &pubkeys) - } -} - -impl TSecretKey for milagro::SecretKey { - fn random() -> Self { - Self::random(&mut thread_rng()) - } - - fn public_key(&self) -> milagro::PublicKey { - let point = milagro::PublicKey::from_secret_key(self).point; - milagro::PublicKey { point } - } - - fn sign(&self, msg: Hash256) -> milagro::Signature { - let point = milagro::Signature::new(msg.as_bytes(), self).point; - milagro::Signature { point } - } - - fn serialize(&self) -> ZeroizeHash { - let mut bytes = [0; SECRET_KEY_BYTES_LEN]; - - // Takes the right-hand 32 bytes from the secret key. - bytes[..].copy_from_slice(&self.as_bytes()); - - bytes.into() - } - - fn deserialize(bytes: &[u8]) -> Result { - Self::from_bytes(bytes).map_err(Into::into) - } -} diff --git a/crypto/bls/src/impls/mod.rs b/crypto/bls/src/impls/mod.rs index b3f2da77b..d87c3b12b 100644 --- a/crypto/bls/src/impls/mod.rs +++ b/crypto/bls/src/impls/mod.rs @@ -1,5 +1,3 @@ #[cfg(feature = "supranational")] pub mod blst; pub mod fake_crypto; -#[cfg(feature = "milagro")] -pub mod milagro; diff --git a/crypto/bls/src/lib.rs b/crypto/bls/src/lib.rs index 750e1bd5b..fef9804b7 100644 --- a/crypto/bls/src/lib.rs +++ b/crypto/bls/src/lib.rs @@ -9,15 +9,13 @@ //! are supported via compile-time flags. There are three backends supported via features: //! //! - `supranational`: the pure-assembly, highly optimized version from the `blst` crate. -//! - `milagro`: the classic pure-Rust `milagro_bls` crate. //! - `fake_crypto`: an always-returns-valid implementation that is only useful for testing //! scenarios which intend to *ignore* real cryptography. //! //! This crate uses traits to reduce code-duplication between the two implementations. For example, //! the `GenericPublicKey` struct exported from this crate is generic across the `TPublicKey` trait //! (i.e., `PublicKey`). `TPublicKey` is implemented by all three backends (see the -//! `impls.rs` module). When compiling with the `milagro` feature, we export -//! `type PublicKey = GenericPublicKey`. +//! `impls.rs` module). #[macro_use] mod macros; @@ -43,16 +41,11 @@ pub use zeroize_hash::ZeroizeHash; #[cfg(feature = "supranational")] use blst::BLST_ERROR as BlstError; -#[cfg(feature = "milagro")] -use milagro_bls::AmclError; pub type Hash256 = ethereum_types::H256; #[derive(Clone, Debug, PartialEq)] pub enum Error { - /// An error was raised from the Milagro BLS library. - #[cfg(feature = "milagro")] - MilagroError(AmclError), /// An error was raised from the Supranational BLST BLS library. #[cfg(feature = "supranational")] BlstError(BlstError), @@ -66,13 +59,6 @@ pub enum Error { InvalidZeroSecretKey, } -#[cfg(feature = "milagro")] -impl From for Error { - fn from(e: AmclError) -> Error { - Error::MilagroError(e) - } -} - #[cfg(feature = "supranational")] impl From for Error { fn from(e: BlstError) -> Error { @@ -94,8 +80,7 @@ pub mod generics { } /// Defines all the fundamental BLS points which should be exported by this crate by making -/// concrete the generic type parameters using the points from some external BLS library (e.g., -/// Milagro, BLST). +/// concrete the generic type parameters using the points from some external BLS library (e.g.,BLST). macro_rules! define_mod { ($name: ident, $mod: path) => { pub mod $name { @@ -139,8 +124,6 @@ macro_rules! define_mod { }; } -#[cfg(feature = "milagro")] -define_mod!(milagro_implementations, crate::impls::milagro::types); #[cfg(feature = "supranational")] define_mod!(blst_implementations, crate::impls::blst::types); #[cfg(feature = "fake_crypto")] @@ -149,14 +132,7 @@ define_mod!( crate::impls::fake_crypto::types ); -#[cfg(all(feature = "milagro", not(feature = "fake_crypto"),))] -pub use milagro_implementations::*; - -#[cfg(all( - feature = "supranational", - not(feature = "fake_crypto"), - not(feature = "milagro") -))] +#[cfg(all(feature = "supranational", not(feature = "fake_crypto"),))] pub use blst_implementations::*; #[cfg(feature = "fake_crypto")] diff --git a/crypto/bls/tests/tests.rs b/crypto/bls/tests/tests.rs index ad498dbfa..478c1b7dc 100644 --- a/crypto/bls/tests/tests.rs +++ b/crypto/bls/tests/tests.rs @@ -509,8 +509,3 @@ macro_rules! test_suite { mod blst { test_suite!(blst_implementations); } - -#[cfg(all(feature = "milagro", not(debug_assertions)))] -mod milagro { - test_suite!(milagro_implementations); -} diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index ffa4727d7..77b05e9d1 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -14,8 +14,6 @@ write_ssz_files = ["beacon_node/write_ssz_files"] portable = ["bls/supranational-portable"] # Compiles BLST so that it always uses ADX instructions. modern = ["bls/supranational-force-adx"] -# Uses the slower Milagro BLS library, which is written in native Rust. -milagro = ["bls/milagro"] # Support minimal spec (used for testing only). spec-minimal = [] # Support Gnosis spec and Gnosis Beacon Chain. diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index d646b9764..932b125dc 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -23,8 +23,6 @@ fn bls_library_name() -> &'static str { "blst-portable" } else if cfg!(feature = "modern") { "blst-modern" - } else if cfg!(feature = "milagro") { - "milagro" } else { "blst" } diff --git a/testing/ef_tests/Cargo.toml b/testing/ef_tests/Cargo.toml index 8bc36d008..f3d00fa03 100644 --- a/testing/ef_tests/Cargo.toml +++ b/testing/ef_tests/Cargo.toml @@ -7,7 +7,6 @@ edition = { workspace = true } [features] # `ef_tests` feature must be enabled to actually run the tests ef_tests = [] -milagro = ["bls/milagro"] fake_crypto = ["bls/fake_crypto"] portable = ["beacon_chain/portable"] diff --git a/testing/ef_tests/src/cases/bls_eth_aggregate_pubkeys.rs b/testing/ef_tests/src/cases/bls_eth_aggregate_pubkeys.rs index 8783aa141..2a9a393bf 100644 --- a/testing/ef_tests/src/cases/bls_eth_aggregate_pubkeys.rs +++ b/testing/ef_tests/src/cases/bls_eth_aggregate_pubkeys.rs @@ -31,10 +31,6 @@ impl Case for BlsEthAggregatePubkeys { { return Ok(()); } - #[cfg(feature = "milagro")] - Err(bls::Error::MilagroError(_)) if self.output.is_none() => { - return Ok(()); - } Err(e) => return Err(Error::FailedToParseTest(format!("{:?}", e))), }; diff --git a/testing/ef_tests/src/decode.rs b/testing/ef_tests/src/decode.rs index b5c0da53a..e95bddffa 100644 --- a/testing/ef_tests/src/decode.rs +++ b/testing/ef_tests/src/decode.rs @@ -71,9 +71,7 @@ where f(&bytes).map_err(|e| { match e { // NOTE: this is a bit hacky, but seemingly better than the alternatives - ssz::DecodeError::BytesInvalid(message) - if message.contains("Blst") || message.contains("Milagro") => - { + ssz::DecodeError::BytesInvalid(message) if message.contains("Blst") => { Error::InvalidBLSInput(message) } e => Error::FailedToParseTest(format!(