From 0541cb0083d4435efff1654902c55c49c7b9b054 Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 27 Oct 2023 12:21:49 -0400 Subject: [PATCH 01/37] Fix `BeefyAuthorityId::verify` for `ecdsa_bls_crypto` not to use `ecdsa_bls::verify` --- .../primitives/consensus/beefy/src/lib.rs | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 5bdf8ce010a1..802567369e7c 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -133,7 +133,7 @@ pub mod bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // `w3f-bls` library uses IETF hashing standard and as such does not exposes + // `w3f-bls` library uses IETF hashing standard and as such does not expose // a choice of hash to field function. // We are directly calling into the library to avoid introducing new host call. // and because BeefyAuthorityId::verify is being called in the runtime so we don't have @@ -157,7 +157,7 @@ pub mod bls_crypto { pub mod ecdsa_bls_crypto { use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa_bls377}; - use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair, Pair as _}; + use sp_core::{bls377, crypto::Wraps, ecdsa, Pair as _}; app_crypto!(ecdsa_bls377, KEY_TYPE); @@ -172,12 +172,30 @@ pub mod ecdsa_bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // `w3f-bls` library uses IETF hashing standard and as such does not exposes - // a choice of hash to field function. - // We are directly calling into the library to avoid introducing new host call. - // and because BeefyAuthorityId::verify is being called in the runtime so we don't have + // We can not call simply call + // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` + // because that invokes ecdsa default verification which perfoms blake2 hash + // which we don't want. As such we need to re-implement the verification + // of signatures part by part instead of relying on paired crypto + let public: &[u8] = self.as_inner_ref().as_ref(); + let signature: &[u8] = signature.as_inner_ref().as_ref(); + let msg_hash = ::hash(msg).into(); + + let ecdsa_public = + ecdsa::Public::try_from(&public[0..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE]); + let ecdsa_signature = + ecdsa::Signature::try_from(&signature[0..ecdsa::SIGNATURE_SERIALIZED_SIZE]); + + let bls_public = bls377::Public::try_from(&public[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..]); + let bls_signature = + bls377::Signature::try_from(&signature[ecdsa::SIGNATURE_SERIALIZED_SIZE..]); - EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref()) + return match (ecdsa_public, ecdsa_signature, bls_public, bls_signature) { + (Ok(ecdsa_public), Ok(ecdsa_signature), Ok(bls_public), Ok(bls_signature)) => + ecdsa::Pair::verify_prehashed(&ecdsa_signature, &msg_hash, &ecdsa_public) && + bls377::Pair::verify(&bls_signature, msg, &bls_public), + _ => false, + }; } } } @@ -257,6 +275,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. +/// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block @@ -505,15 +524,30 @@ mod tests { #[cfg(feature = "bls-experimental")] fn ecdsa_bls_beefy_verify_works() { let msg = &b"test-message"[..]; - let (pair, _) = ecdsa_bls_crypto::Pair::generate(); - - let signature: ecdsa_bls_crypto::Signature = pair.as_inner_ref().sign(&msg).into(); + let (ecdsa_pair, _) = ecdsa_crypto::Pair::generate(); + let (bls_pair, _) = bls_crypto::Pair::generate(); + + let ecdsa_keccak_256_signature = ecdsa_pair.as_inner_ref().sign_prehashed(&keccak_256(msg)); + let bls_signature = bls_pair.sign(&msg); + + let public_vec: Vec = [ + ecdsa_pair.public().as_inner_ref().as_ref(), + bls_pair.public().as_inner_ref().as_ref(), + ] + .concat(); + + let public = ecdsa_bls_crypto::Public::try_from(&public_vec[..]).unwrap(); + let signature_slice: Vec = [ + >::as_ref(&ecdsa_keccak_256_signature), + >::as_ref(&bls_signature.as_inner_ref()), + ] + .concat(); + let signature = ecdsa_bls_crypto::Signature::try_from(&signature_slice[..]).unwrap(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); + assert!(BeefyAuthorityId::::verify(&public, &signature, msg)); - // Other public key doesn't work - let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); - assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); + // Verification doesn't works if we verify function provided by pair_crypto implementation + assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &public)); } } From 099b86d3c743d4d23b4150b94fbc23bb5a0c40b6 Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 27 Oct 2023 12:31:06 -0400 Subject: [PATCH 02/37] Fix comment in `primitives/consensus/beefy/src/lib.rs` --- substrate/primitives/consensus/beefy/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 802567369e7c..1af40b2c4442 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -172,11 +172,11 @@ pub mod ecdsa_bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // We can not call simply call + // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` // because that invokes ecdsa default verification which perfoms blake2 hash // which we don't want. As such we need to re-implement the verification - // of signatures part by part instead of relying on paired crypto + // of signatures part by part instead of relying on `paired_crypto` let public: &[u8] = self.as_inner_ref().as_ref(); let signature: &[u8] = signature.as_inner_ref().as_ref(); let msg_hash = ::hash(msg).into(); From 033ff5701780d7537ccfe14d66b181252db928b5 Mon Sep 17 00:00:00 2001 From: Skalman Date: Tue, 31 Oct 2023 10:10:36 -0400 Subject: [PATCH 03/37] - implement `sign/verify_with_hasher` for `ecdsa_bls377` `Pair` with test - update `substrate/primitives/consensus/beefy/src/lib.rs` accordingly --- .../primitives/consensus/beefy/src/lib.rs | 60 +++++---------- .../primitives/core/src/paired_crypto.rs | 74 ++++++++++++++++++- 2 files changed, 88 insertions(+), 46 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 1af40b2c4442..c78abdeb1ccc 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -157,7 +157,7 @@ pub mod bls_crypto { pub mod ecdsa_bls_crypto { use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa_bls377}; - use sp_core::{bls377, crypto::Wraps, ecdsa, Pair as _}; + use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair}; app_crypto!(ecdsa_bls377, KEY_TYPE); @@ -175,27 +175,12 @@ pub mod ecdsa_bls_crypto { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` // because that invokes ecdsa default verification which perfoms blake2 hash - // which we don't want. As such we need to re-implement the verification - // of signatures part by part instead of relying on `paired_crypto` - let public: &[u8] = self.as_inner_ref().as_ref(); - let signature: &[u8] = signature.as_inner_ref().as_ref(); - let msg_hash = ::hash(msg).into(); - - let ecdsa_public = - ecdsa::Public::try_from(&public[0..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE]); - let ecdsa_signature = - ecdsa::Signature::try_from(&signature[0..ecdsa::SIGNATURE_SERIALIZED_SIZE]); - - let bls_public = bls377::Public::try_from(&public[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..]); - let bls_signature = - bls377::Signature::try_from(&signature[ecdsa::SIGNATURE_SERIALIZED_SIZE..]); - - return match (ecdsa_public, ecdsa_signature, bls_public, bls_signature) { - (Ok(ecdsa_public), Ok(ecdsa_signature), Ok(bls_public), Ok(bls_signature)) => - ecdsa::Pair::verify_prehashed(&ecdsa_signature, &msg_hash, &ecdsa_public) && - bls377::Pair::verify(&bls_signature, msg, &bls_public), - _ => false, - }; + // which we don't want. + EcdsaBlsPair::verify_with_hasher::( + signature.as_inner_ref(), + msg, + self.as_inner_ref(), + ) } } } @@ -524,30 +509,19 @@ mod tests { #[cfg(feature = "bls-experimental")] fn ecdsa_bls_beefy_verify_works() { let msg = &b"test-message"[..]; - let (ecdsa_pair, _) = ecdsa_crypto::Pair::generate(); - let (bls_pair, _) = bls_crypto::Pair::generate(); - - let ecdsa_keccak_256_signature = ecdsa_pair.as_inner_ref().sign_prehashed(&keccak_256(msg)); - let bls_signature = bls_pair.sign(&msg); - - let public_vec: Vec = [ - ecdsa_pair.public().as_inner_ref().as_ref(), - bls_pair.public().as_inner_ref().as_ref(), - ] - .concat(); - - let public = ecdsa_bls_crypto::Public::try_from(&public_vec[..]).unwrap(); - let signature_slice: Vec = [ - >::as_ref(&ecdsa_keccak_256_signature), - >::as_ref(&bls_signature.as_inner_ref()), - ] - .concat(); - let signature = ecdsa_bls_crypto::Signature::try_from(&signature_slice[..]).unwrap(); + let (pair, _) = ecdsa_bls_crypto::Pair::generate(); + + let signature: ecdsa_bls_crypto::Signature = + pair.as_inner_ref().sign_with_hasher::(&msg).into(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&public, &signature, msg)); + assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); // Verification doesn't works if we verify function provided by pair_crypto implementation - assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &public)); + assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &pair.public())); + + // Other public key doesn't work + let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); + assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); } } diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index a97b657e7578..88167bb8a2fe 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -39,7 +39,11 @@ use sp_std::convert::TryFrom; /// ECDSA and BLS12-377 paired crypto scheme #[cfg(feature = "bls-experimental")] pub mod ecdsa_bls377 { - use crate::{bls377, crypto::CryptoTypeId, ecdsa}; + use crate::{ + bls377, + crypto::{CryptoTypeId, Pair as PairT, UncheckedFrom}, + ecdsa, + }; /// An identifier used to match public keys against BLS12-377 keys pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecb7"); @@ -71,6 +75,60 @@ pub mod ecdsa_bls377 { impl super::CryptoType for Pair { type Pair = Pair; } + + #[cfg(feature = "full_crypto")] + impl Pair { + /// hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA + /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates + /// BLS12-377 Signature according to IETF standard and disregard the hasher for the + /// BLS12-377 component + pub fn sign_with_hasher(&self, message: &[u8]) -> Signature + where + ::Out: Into<[u8; 32]>, + { + let msg_hash = ::hash(message).into(); + + let mut raw: [u8; SIGNATURE_LEN] = [0u8; SIGNATURE_LEN]; + raw[..ecdsa::SIGNATURE_SERIALIZED_SIZE] + .copy_from_slice(self.left.sign_prehashed(&msg_hash).as_ref()); + raw[ecdsa::SIGNATURE_SERIALIZED_SIZE..] + .copy_from_slice(self.right.sign(message).as_ref()); + ::Signature::unchecked_from(raw) + } + + /// hashes the `message` with the specified `MsgHasher` and then verifys if the resulting + /// hash was signed by the provided ECDSA public key.. It does not affect the behavoir of + /// BLS12-377 component. It Verify the BLS12-377 signature as it was hashed and signed + /// according to IETF standrad + pub fn verify_with_hasher( + sig: &Signature, + message: &[u8], + public: &Public, + ) -> bool + where + ::Out: Into<[u8; 32]>, + { + let msg_hash = ::hash(message).into(); + + let Ok(left_pub) = public.0[..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE].try_into() else { + return false + }; + let Ok(left_sig) = sig.0[0..ecdsa::SIGNATURE_SERIALIZED_SIZE].try_into() else { + return false + }; + if !ecdsa::Pair::verify_prehashed(&left_sig, &msg_hash, &left_pub) { + return false + } + + let Ok(right_pub) = public.0[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..].try_into() else { + return false + }; + let Ok(right_sig) = sig.0[ecdsa::SIGNATURE_SERIALIZED_SIZE..].try_into() else { + return false + }; + bls377::Pair::verify(&right_sig, message.as_ref(), &right_pub) + } + } } /// Secure seed length. @@ -455,12 +513,12 @@ where #[cfg(all(test, feature = "bls-experimental"))] mod test { use super::*; - use crate::crypto::DEV_PHRASE; + use crate::{crypto::DEV_PHRASE, KeccakHasher}; use ecdsa_bls377::{Pair, Signature}; use crate::{bls377, ecdsa}; - #[test] + #[test] fn test_length_of_paired_ecdsa_and_bls377_public_key_and_signature_is_correct() { assert_eq!( ::Public::LEN, @@ -617,6 +675,16 @@ mod test { assert_eq!(cmp, public); } + #[test] + fn sign_and_verify_with_haser_works() { + let pair = + Pair::from_seed(&(b"12345678901234567890123456789012".as_slice().try_into().unwrap())); + let message = b"Something important"; + let signature = pair.sign_with_hasher::(&message[..]); + + assert!(Pair::verify_with_hasher::(&signature, &message[..], &pair.public())); + } + #[test] fn signature_serialization_works() { let pair = From 76831b7b87250e3db10e66bc0e357d9b33b48d64 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Wed, 1 Nov 2023 14:00:44 -0400 Subject: [PATCH 04/37] Fix typos and improve comment quality Co-authored-by: Davide Galassi --- substrate/primitives/consensus/beefy/src/lib.rs | 2 +- substrate/primitives/core/src/paired_crypto.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index c78abdeb1ccc..738a491f04fb 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -260,7 +260,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. -/// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. +// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 88167bb8a2fe..03567a8b7317 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -676,7 +676,7 @@ mod test { } #[test] - fn sign_and_verify_with_haser_works() { + fn sign_and_verify_with_hasher_works() { let pair = Pair::from_seed(&(b"12345678901234567890123456789012".as_slice().try_into().unwrap())); let message = b"Something important"; From a0c6d83edcaa12cfcad14f14597499bda2de9b36 Mon Sep 17 00:00:00 2001 From: Skalman Date: Thu, 2 Nov 2023 17:34:03 -0400 Subject: [PATCH 05/37] Add ecdsa_bls377_sign_with_keccak256 to the keystore --- substrate/primitives/keystore/src/lib.rs | 28 ++++++++++++++++++++ substrate/primitives/keystore/src/testing.rs | 15 ++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/keystore/src/lib.rs b/substrate/primitives/keystore/src/lib.rs index e415080779cf..8999bf2ec0bc 100644 --- a/substrate/primitives/keystore/src/lib.rs +++ b/substrate/primitives/keystore/src/lib.rs @@ -355,6 +355,24 @@ pub trait Keystore: Send + Sync { msg: &[u8], ) -> Result, Error>; + /// Hashes the `message` using keccak256 and then signs it using ECDSA + /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates + /// BLS12-377 Signature according to IETF standard. + /// + /// Receives [`KeyTypeId`] and a [`ecdsa_bls377::Public`] key to be able to map + /// them to a private key that exists in the keystore. + /// + /// Returns an [`ecdsa_bls377::Signature`] or `None` in case the given `key_type` + /// and `public` combination doesn't exist in the keystore. + /// An `Err` will be returned if generating the signature itself failed. + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign_with_keccak256( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error>; + /// Insert a new secret key. fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()>; @@ -661,6 +679,16 @@ impl Keystore for Arc { (**self).ecdsa_bls377_sign(key_type, public, msg) } + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign_with_keccak256( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error> { + (**self).ecdsa_bls377_sign_with_keccak256(key_type, public, msg) + } + fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> { (**self).insert(key_type, suri, public) } diff --git a/substrate/primitives/keystore/src/testing.rs b/substrate/primitives/keystore/src/testing.rs index 7f5dfd9faa71..df9e4bdf083c 100644 --- a/substrate/primitives/keystore/src/testing.rs +++ b/substrate/primitives/keystore/src/testing.rs @@ -25,7 +25,7 @@ use sp_core::bandersnatch; use sp_core::{bls377, bls381, ecdsa_bls377}; use sp_core::{ crypto::{ByteArray, KeyTypeId, Pair, VrfSecret}, - ecdsa, ed25519, sr25519, + ecdsa, ed25519, sr25519, KeccakHasher, }; use parking_lot::RwLock; @@ -346,6 +346,19 @@ impl Keystore for MemoryKeystore { self.sign::(key_type, public, msg) } + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign_with_keccak256( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error> { + let sig = self + .pair::(key_type, public) + .map(|pair| pair.sign_with_hasher::(msg)); + Ok(sig) + } + fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> { self.keys .write() From 102ad62db34c58e4aa73dfada2038d79e53f383d Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 3 Nov 2023 13:23:57 -0400 Subject: [PATCH 06/37] add and implement `ecdsa_bls377_sign_with_keccak256` make BEEFY keystore generic over `AuthorityId` and works with both (ECDSA) and (ECDSA, BLS377) key types. pass all the BEEFY tests for ECDSA --- substrate/client/consensus/beefy/Cargo.toml | 1 + .../beefy/src/communication/gossip.rs | 2 +- .../consensus/beefy/src/justification.rs | 2 +- .../client/consensus/beefy/src/keystore.rs | 212 ++++++++++++++---- .../client/consensus/beefy/src/worker.rs | 2 +- substrate/client/keystore/src/local.rs | 13 ++ substrate/primitives/keystore/src/testing.rs | 36 ++- 7 files changed, 221 insertions(+), 47 deletions(-) diff --git a/substrate/client/consensus/beefy/Cargo.toml b/substrate/client/consensus/beefy/Cargo.toml index aae5a44d7fa2..4b300b649c44 100644 --- a/substrate/client/consensus/beefy/Cargo.toml +++ b/substrate/client/consensus/beefy/Cargo.toml @@ -36,6 +36,7 @@ sp-core = { path = "../../../primitives/core" } sp-keystore = { path = "../../../primitives/keystore" } sp-mmr-primitives = { path = "../../../primitives/merkle-mountain-range" } sp-runtime = { path = "../../../primitives/runtime" } +sp-std = { path = "../../../primitives/std", default-features = false} [dev-dependencies] serde = "1.0.188" diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index 342cd0511a51..5bbbac502bb7 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -506,7 +506,7 @@ pub(crate) mod tests { pub fn sign_commitment(who: &Keyring, commitment: &Commitment) -> Signature { let store = MemoryKeystore::new(); store.ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&who.to_seed())).unwrap(); - let beefy_keystore: BeefyKeystore = Some(store.into()).into(); + let beefy_keystore: BeefyKeystore = Some(store.into()).into(); beefy_keystore.sign(&who.public(), &commitment.encode()).unwrap() } diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index 483184e2374a..83e11b5b6e97 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -76,7 +76,7 @@ pub(crate) fn verify_with_validator_set( .as_ref() .map(|sig| { signatures_checked += 1; - BeefyKeystore::verify(id, sig, &message[..]) + BeefyKeystore::verify(*id, sig, &message[..]) }) .unwrap_or(false) }) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 925bb0882822..c9151a25c088 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -15,42 +15,86 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use codec::{Codec, Decode}; +use core::fmt::{Debug, Display}; + +use sp_application_crypto::{ + key_types::BEEFY as BEEFY_KEY_TYPE, AppCrypto, AppPublic, ByteArray, RuntimeAppPublic, +}; +#[cfg(feature = "bls-experimental")] +use sp_core::{bls377, ecdsa_bls377}; +use sp_core::{ecdsa, keccak_256}; -use sp_application_crypto::{key_types::BEEFY as BEEFY_KEY_TYPE, RuntimeAppPublic}; -use sp_core::keccak_256; use sp_keystore::KeystorePtr; +use sp_std::marker::PhantomData; use log::warn; -use sp_consensus_beefy::{ - ecdsa_crypto::{Public, Signature}, - BeefyAuthorityId, -}; +use sp_consensus_beefy::{ecdsa_crypto, BeefyAuthorityId}; + +#[cfg(feature = "bls-experimental")] +use sp_consensus_beefy::bls_crypto; use crate::{error, LOG_TARGET}; /// Hasher used for BEEFY signatures. pub(crate) type BeefySignatureHasher = sp_runtime::traits::Keccak256; -/// A BEEFY specific keystore implemented as a `Newtype`. This is basically a -/// wrapper around [`sp_keystore::Keystore`] and allows to customize -/// common cryptographic functionality. -pub(crate) struct BeefyKeystore(Option); +pub trait AuthorityIdBound: + Codec + + Debug + + Clone + + Ord + + Sync + + Send + + AsRef<[u8]> + + ByteArray + + AppPublic + + AppCrypto + + RuntimeAppPublic + + Display + + BeefyAuthorityId +where + ::Signature: Send + Sync, +{ +} -impl BeefyKeystore { +impl AuthorityIdBound for ecdsa_crypto::AuthorityId {} + +// impl + AppPublic + AppCrypto + +// RuntimeAppPublic + BeefyAuthorityId > AuthorityIdBound for T { type +// Signature = AppCrypto::Signature; } + +/// A BEEFY specific keystore i mplemented as a `Newtype`. This is basically a +/// wrapper around [`sp_keystore::Keystore`] and allows to customize +/// commoncryptographic functionality. +pub(crate) struct BeefyKeystore( + Option, + PhantomData AuthorityId>, +) +where + ::Signature: Send + Sync; + +impl BeefyKeystore +where + ::Signature: Send + Sync, +{ /// Check if the keystore contains a private key for one of the public keys /// contained in `keys`. A public key with a matching private key is known /// as a local authority id. /// /// Return the public key for which we also do have a private key. If no /// matching private key is found, `None` will be returned. - pub fn authority_id(&self, keys: &[Public]) -> Option { + pub fn authority_id(&self, keys: &[AuthorityId]) -> Option { let store = self.0.clone()?; // we do check for multiple private keys as a key store sanity check. - let public: Vec = keys + let public: Vec = keys .iter() - .filter(|k| store.has_keys(&[(k.to_raw_vec(), BEEFY_KEY_TYPE)])) + .filter(|k| { + store + .has_keys(&[(::to_raw_vec(k), BEEFY_KEY_TYPE)]) + }) .cloned() .collect(); @@ -71,47 +115,131 @@ impl BeefyKeystore { /// Note that `message` usually will be pre-hashed before being signed. /// /// Return the message signature or an error in case of failure. - pub fn sign(&self, public: &Public, message: &[u8]) -> Result { + pub fn sign( + &self, + public: &AuthorityId, + message: &[u8], + ) -> Result<::Signature, error::Error> { let store = self.0.clone().ok_or_else(|| error::Error::Keystore("no Keystore".into()))?; - let msg = keccak_256(message); - let public = public.as_ref(); - - let sig = store - .ecdsa_sign_prehashed(BEEFY_KEY_TYPE, public, &msg) - .map_err(|e| error::Error::Keystore(e.to_string()))? - .ok_or_else(|| error::Error::Signature("ecdsa_sign_prehashed() failed".to_string()))?; - - // check that `sig` has the expected result type - let sig = sig.clone().try_into().map_err(|_| { - error::Error::Signature(format!("invalid signature {:?} for key {:?}", sig, public)) + //ECDSA shoud do ecdsa_sign_prehashed because it needs to be hashed by keccak_256 + //as such we need to deal with signing case by case + + let signature_byte_array: Vec = match ::CRYPTO_ID { + ecdsa::CRYPTO_ID => { + let msg_hash = keccak_256(message); + let public: ecdsa::Public = ecdsa::Public::try_from(public.as_slice()).unwrap(); + + let sig = store + .ecdsa_sign_prehashed(BEEFY_KEY_TYPE, &public, &msg_hash) + .map_err(|e| error::Error::Keystore(e.to_string()))? + .ok_or_else(|| { + error::Error::Signature("ecdsa_sign_prehashed() failed".to_string()) + })?; + let sig_ref: &[u8] = sig.as_ref(); + sig_ref.to_vec() + }, + + #[cfg(feature = "bls-experimental")] + ecdsa_bls377::CRYPTO_ID => store + .ecdsa_bls377_sign_with_keccak256(BEEFY_KEY_TYPE, public, &msg) + .map_err(|e| error::Error::Keystore(e.to_string()))? + .ok_or_else(|| { + error::Error::AuthorityId::Signature( + "bls377_sign() + failed" + .to_string(), + ) + })? + .as_ref(), + + _ => { + let sig = store + .sign_with( + ::ID, + ::CRYPTO_ID, + public.as_slice(), + message, + ) + .map_err(|e| error::Error::Signature(format!("{}. Key: {:?}", e, public)))? + .ok_or_else(|| { + error::Error::Signature(format!( + "Could not find key in keystore. Key: {:?}", + public + )) + })?; + + sig + }, + }; + + //check that `sig` has the expected result type + let signature = ::Signature::decode( + &mut signature_byte_array.as_slice(), + ) + .map_err(|_| { + error::Error::Signature(format!( + "invalid signature {:?} for key {:?}", + signature_byte_array, public + )) })?; - Ok(sig) + Ok(signature) } /// Returns a vector of [`sp_consensus_beefy::crypto::Public`] keys which are currently /// supported (i.e. found in the keystore). - pub fn public_keys(&self) -> Result, error::Error> { + pub fn public_keys(&self) -> Result, error::Error> { let store = self.0.clone().ok_or_else(|| error::Error::Keystore("no Keystore".into()))?; - let pk: Vec = - store.ecdsa_public_keys(BEEFY_KEY_TYPE).drain(..).map(Public::from).collect(); - - Ok(pk) + let pk = match ::CRYPTO_ID { + ecdsa::CRYPTO_ID => store + .ecdsa_public_keys(BEEFY_KEY_TYPE) + .drain(..) + .map(|pk| AuthorityId::try_from(pk.as_ref())) + .collect::, _>>() + .or_else(|_| { + Err(error::Error::Keystore( + "unable to convert public key into authority id".into(), + )) + }), + + #[cfg(feature = "bls-experimental")] + ecdsa_bls377::CRYPTO_ID => store + .ecdsa_bls377_public_keys(BEEFY_KEY_TYPE) + .drain(..) + .map(|pk| AuthorityId::try_from(pk.as_ref())) + .collect::, _>>() + .or_else(|_| { + Err(error::Error::Keystore( + "unable to convert public key into authority id".into(), + )) + }), + + _ => Err(error::Error::Keystore("key type is not supported by BEEFY Keystore".into())), + }; + + pk } /// Use the `public` key to verify that `sig` is a valid signature for `message`. /// /// Return `true` if the signature is authentic, `false` otherwise. - pub fn verify(public: &Public, sig: &Signature, message: &[u8]) -> bool { + pub fn verify( + public: &AuthorityId, + sig: &::Signature, + message: &[u8], + ) -> bool { BeefyAuthorityId::::verify(public, sig, message) } } -impl From> for BeefyKeystore { - fn from(store: Option) -> BeefyKeystore { - BeefyKeystore(store) +impl From> for BeefyKeystore +where + ::Signature: Send + Sync, +{ + fn from(store: Option) -> BeefyKeystore { + BeefyKeystore(store, PhantomData) } } @@ -218,7 +346,7 @@ pub mod tests { let bob = Keyring::Bob.public(); let charlie = Keyring::Charlie.public(); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); let mut keys = vec![bob, charlie]; @@ -241,7 +369,7 @@ pub mod tests { .unwrap() .into(); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); let msg = b"are you involved or commited?"; @@ -260,7 +388,7 @@ pub mod tests { .ok() .unwrap(); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); let alice = Keyring::Alice.public(); @@ -273,7 +401,7 @@ pub mod tests { #[test] fn sign_no_keystore() { - let store: BeefyKeystore = None.into(); + let store: BeefyKeystore = None.into(); let alice = Keyring::Alice.public(); let msg = b"are you involved or commited"; @@ -293,7 +421,7 @@ pub mod tests { .unwrap() .into(); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); // `msg` and `sig` match let msg = b"are you involved or commited?"; @@ -330,7 +458,7 @@ pub mod tests { let key1: ecdsa_crypto::Public = add_key(BEEFY_KEY_TYPE, None).into(); let key2: ecdsa_crypto::Public = add_key(BEEFY_KEY_TYPE, None).into(); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); let keys = store.public_keys().ok().unwrap(); diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 309d8c5135be..8b3b8e782393 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -330,7 +330,7 @@ pub(crate) struct BeefyWorker { pub payload_provider: P, pub runtime: Arc, pub sync: Arc, - pub key_store: BeefyKeystore, + pub key_store: BeefyKeystore, // communication (created once, but returned and reused if worker is restarted/reinitialized) pub comms: BeefyComms, diff --git a/substrate/client/keystore/src/local.rs b/substrate/client/keystore/src/local.rs index 8089dbba0352..30fbaedce896 100644 --- a/substrate/client/keystore/src/local.rs +++ b/substrate/client/keystore/src/local.rs @@ -391,6 +391,19 @@ impl Keystore for LocalKeystore { self.sign::(key_type, public, msg) } + fn ecdsa_bls377_sign_with_keccak256( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error> { + let sig = self + .pair::(key_type, public) + .map(|pair| pair.sign_with_hasher::(msg)); + Ok(sig) + } + + } } diff --git a/substrate/primitives/keystore/src/testing.rs b/substrate/primitives/keystore/src/testing.rs index df9e4bdf083c..25653ac9a6b0 100644 --- a/substrate/primitives/keystore/src/testing.rs +++ b/substrate/primitives/keystore/src/testing.rs @@ -22,10 +22,10 @@ use crate::{Error, Keystore, KeystorePtr}; #[cfg(feature = "bandersnatch-experimental")] use sp_core::bandersnatch; #[cfg(feature = "bls-experimental")] -use sp_core::{bls377, bls381, ecdsa_bls377}; +use sp_core::{bls377, bls381, ecdsa_bls377, KeccakHasher}; use sp_core::{ crypto::{ByteArray, KeyTypeId, Pair, VrfSecret}, - ecdsa, ed25519, sr25519, KeccakHasher, + ecdsa, ed25519, sr25519, }; use parking_lot::RwLock; @@ -504,6 +504,38 @@ mod tests { assert!(res.is_some()); } + #[test] + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign_with_keccak_works() { + use sp_core::testing::ECDSA_BLS377; + + let store = MemoryKeystore::new(); + + let suri = "//Alice"; + let pair = ecdsa_bls377::Pair::from_string(suri, None).unwrap(); + + let msg = b"this should be a normal unhashed message not "; + + // insert key, sign again + store.insert(ECDSA_BLS377, suri, pair.public().as_ref()).unwrap(); + + let res = store + .ecdsa_bls377_sign_with_keccak256(ECDSA_BLS377, &pair.public(), &msg[..]) + .unwrap(); + + assert!(res.is_some()); + + // does not verify with default out-of-the-box verification + assert!(!ecdsa_bls377::Pair::verify(&res.clone().unwrap(), &msg[..], &pair.public())); + + // should verify using keccak256 as hasher + assert!(ecdsa_bls377::Pair::verify_with_hasher::( + &res.unwrap(), + msg, + &pair.public() + )); + } + #[test] #[cfg(feature = "bandersnatch-experimental")] fn bandersnatch_vrf_sign() { From 25d1afc215abae1f072b33dddbae87bb959a83c8 Mon Sep 17 00:00:00 2001 From: Skalman Date: Thu, 9 Nov 2023 12:10:44 -0500 Subject: [PATCH 07/37] - Make BEEFY Keyring to be generic over `AuthorityId`. - Make BEEFY Keystore test generic over `AuthorityId`. - Implement keystore tests for `ecdsa_bls` crypto. - Fix `bls::Pair::derive` and write test for it. --- substrate/client/consensus/beefy/Cargo.toml | 9 + .../beefy/src/communication/gossip.rs | 16 +- .../consensus/beefy/src/justification.rs | 13 +- .../client/consensus/beefy/src/keystore.rs | 406 ++++++++++++------ substrate/client/consensus/beefy/src/round.rs | 135 ++++-- substrate/client/consensus/beefy/src/tests.rs | 13 +- .../client/consensus/beefy/src/worker.rs | 10 +- substrate/client/keystore/src/local.rs | 9 +- .../primitives/consensus/beefy/Cargo.toml | 3 +- .../consensus/beefy/src/commitment.rs | 2 +- .../primitives/consensus/beefy/src/lib.rs | 37 +- .../consensus/beefy/src/test_utils.rs | 314 +++++++++++++- substrate/primitives/core/src/bls.rs | 26 +- substrate/primitives/core/src/ecdsa.rs | 13 + .../primitives/core/src/paired_crypto.rs | 23 +- substrate/primitives/core/src/sr25519.rs | 13 + 16 files changed, 813 insertions(+), 229 deletions(-) diff --git a/substrate/client/consensus/beefy/Cargo.toml b/substrate/client/consensus/beefy/Cargo.toml index 4b300b649c44..65b941638c30 100644 --- a/substrate/client/consensus/beefy/Cargo.toml +++ b/substrate/client/consensus/beefy/Cargo.toml @@ -48,3 +48,12 @@ sp-consensus-grandpa = { path = "../../../primitives/consensus/grandpa" } sp-keyring = { path = "../../../primitives/keyring" } sp-tracing = { path = "../../../primitives/tracing" } substrate-test-runtime-client = { path = "../../../test-utils/runtime/client" } + +[features] +# This feature adds BLS crypto primitives. It should not be used in production since +# the BLS implementation and interface may still be subject to significant change. +bls-experimental = [ + "sp-application-crypto/bls-experimental", + "sp-core/bls-experimental", + "sp-consensus-beefy/bls-experimental" +] diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index 5bbbac502bb7..c70e5214a13f 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -481,8 +481,8 @@ pub(crate) mod tests { use sc_network_test::Block; use sp_application_crypto::key_types::BEEFY as BEEFY_KEY_TYPE; use sp_consensus_beefy::{ - ecdsa_crypto::Signature, known_payloads, Commitment, Keyring, MmrRootHash, Payload, - SignedCommitment, VoteMessage, + ecdsa_crypto::Signature, known_payloads, Commitment, GenericKeyring, Keyring, MmrRootHash, + Payload, SignedCommitment, VoteMessage, }; use sp_keystore::{testing::MemoryKeystore, Keystore}; @@ -505,7 +505,12 @@ pub(crate) mod tests { pub fn sign_commitment(who: &Keyring, commitment: &Commitment) -> Signature { let store = MemoryKeystore::new(); - store.ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&who.to_seed())).unwrap(); + store + .ecdsa_generate_new( + BEEFY_KEY_TYPE, + Some(&>::to_seed(*who)), + ) + .unwrap(); let beefy_keystore: BeefyKeystore = Some(store.into()).into(); beefy_keystore.sign(&who.public(), &commitment.encode()).unwrap() } @@ -534,7 +539,10 @@ pub(crate) mod tests { .validators() .iter() .map(|validator: &AuthorityId| { - Some(sign_commitment(&Keyring::from_public(validator).unwrap(), &commitment)) + Some(sign_commitment( + &>::from_public(validator).unwrap(), + &commitment, + )) }) .collect(); diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index 83e11b5b6e97..6de9c5b3316f 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -93,7 +93,8 @@ pub(crate) fn verify_with_validator_set( #[cfg(test)] pub(crate) mod tests { use sp_consensus_beefy::{ - known_payloads, Commitment, Keyring, Payload, SignedCommitment, VersionedFinalityProof, + known_payloads, Commitment, GenericKeyring, Keyring, Payload, SignedCommitment, + VersionedFinalityProof, }; use substrate_test_runtime_client::runtime::Block; @@ -111,7 +112,10 @@ pub(crate) mod tests { validator_set_id: validator_set.id(), }; let message = commitment.encode(); - let signatures = keys.iter().map(|key| Some(key.sign(&message))).collect(); + let signatures = keys + .iter() + .map(|key| Some(>::sign(*key, &message))) + .collect(); VersionedFinalityProof::V1(SignedCommitment { commitment, signatures }) } @@ -174,7 +178,10 @@ pub(crate) mod tests { }; // change a signature to a different key *bad_signed_commitment.signatures.first_mut().unwrap() = - Some(Keyring::Dave.sign(&bad_signed_commitment.commitment.encode())); + Some(>::sign( + Keyring::Dave, + &bad_signed_commitment.commitment.encode(), + )); match verify_with_validator_set::(block_num, &validator_set, &bad_proof.into()) { Err((ConsensusError::InvalidJustification, 3)) => (), e => assert!(false, "Got unexpected {:?}", e), diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index c9151a25c088..80e2f807c2b1 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -15,14 +15,11 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use codec::{Codec, Decode}; -use core::fmt::{Debug, Display}; +use codec::Decode; -use sp_application_crypto::{ - key_types::BEEFY as BEEFY_KEY_TYPE, AppCrypto, AppPublic, ByteArray, RuntimeAppPublic, -}; +use sp_application_crypto::{key_types::BEEFY as BEEFY_KEY_TYPE, AppCrypto, RuntimeAppPublic}; #[cfg(feature = "bls-experimental")] -use sp_core::{bls377, ecdsa_bls377}; +use sp_core::ecdsa_bls377; use sp_core::{ecdsa, keccak_256}; use sp_keystore::KeystorePtr; @@ -30,37 +27,13 @@ use sp_std::marker::PhantomData; use log::warn; -use sp_consensus_beefy::{ecdsa_crypto, BeefyAuthorityId}; +use sp_consensus_beefy::{AuthorityIdBound, BeefyAuthorityId, BeefySignatureHasher}; #[cfg(feature = "bls-experimental")] -use sp_consensus_beefy::bls_crypto; +use sp_consensus_beefy::ecdsa_bls_crypto; use crate::{error, LOG_TARGET}; -/// Hasher used for BEEFY signatures. -pub(crate) type BeefySignatureHasher = sp_runtime::traits::Keccak256; - -pub trait AuthorityIdBound: - Codec - + Debug - + Clone - + Ord - + Sync - + Send - + AsRef<[u8]> - + ByteArray - + AppPublic - + AppCrypto - + RuntimeAppPublic - + Display - + BeefyAuthorityId -where - ::Signature: Send + Sync, -{ -} - -impl AuthorityIdBound for ecdsa_crypto::AuthorityId {} - // impl + AppPublic + AppCrypto + // RuntimeAppPublic + BeefyAuthorityId > AuthorityIdBound for T { type // Signature = AppCrypto::Signature; } @@ -141,17 +114,16 @@ where }, #[cfg(feature = "bls-experimental")] - ecdsa_bls377::CRYPTO_ID => store - .ecdsa_bls377_sign_with_keccak256(BEEFY_KEY_TYPE, public, &msg) - .map_err(|e| error::Error::Keystore(e.to_string()))? - .ok_or_else(|| { - error::Error::AuthorityId::Signature( - "bls377_sign() - failed" - .to_string(), - ) - })? - .as_ref(), + ecdsa_bls377::CRYPTO_ID => { + let public: ecdsa_bls377::Public = + ecdsa_bls377::Public::try_from(public.as_slice()).unwrap(); + let sig = store + .ecdsa_bls377_sign_with_keccak256(BEEFY_KEY_TYPE, &public, &message) + .map_err(|e| error::Error::Keystore(e.to_string()))? + .ok_or_else(|| error::Error::Signature("bls377_sign() failed".to_string()))?; + let sig_ref: &[u8] = sig.as_ref(); + sig_ref.to_vec() + }, _ => { let sig = store @@ -208,7 +180,9 @@ where ecdsa_bls377::CRYPTO_ID => store .ecdsa_bls377_public_keys(BEEFY_KEY_TYPE) .drain(..) - .map(|pk| AuthorityId::try_from(pk.as_ref())) + .map(|pk| + AuthorityId::try_from(pk.as_ref()) + ) .collect::, _>>() .or_else(|_| { Err(error::Error::Keystore( @@ -245,8 +219,8 @@ where #[cfg(test)] pub mod tests { - use sp_consensus_beefy::{ecdsa_crypto, Keyring}; - use sp_core::{ecdsa, Pair}; + use sp_consensus_beefy::{ecdsa_crypto, BeefySignerAuthority, GenericKeyring, Keyring}; + use sp_core::Pair as PairT; use sp_keystore::testing::MemoryKeystore; use super::*; @@ -256,149 +230,246 @@ pub mod tests { MemoryKeystore::new().into() } - #[test] - fn verify_should_work() { - let msg = keccak_256(b"I am Alice!"); - let sig = Keyring::Alice.sign(b"I am Alice!"); - - assert!(ecdsa::Pair::verify_prehashed( - &sig.clone().into(), - &msg, - &Keyring::Alice.public().into(), + fn pair_verify_should_work< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >() + where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { + let msg = b"I am Alice!"; + let sig = >::sign(Keyring::Alice, b"I am Alice!"); + + assert!(>::verify( + &>::public(Keyring::Alice), + &sig, + &msg.as_slice(), )); // different public key -> fail - assert!(!ecdsa::Pair::verify_prehashed( - &sig.clone().into(), - &msg, - &Keyring::Bob.public().into(), + assert!(!>::verify( + &>::public(Keyring::Bob), + &sig, + &msg.as_slice(), )); - let msg = keccak_256(b"I am not Alice!"); + let msg = b"I am not Alice!"; // different msg -> fail - assert!( - !ecdsa::Pair::verify_prehashed(&sig.into(), &msg, &Keyring::Alice.public().into(),) - ); + assert!(!>::verify( + &>::public(Keyring::Alice), + &sig, + &msg.as_slice(), + )); } #[test] - fn pair_works() { - let want = ecdsa_crypto::Pair::from_string("//Alice", None) + fn pair_verify_should_work_ecdsa() { + pair_verify_should_work::(); + } + + #[cfg(feature = "bls-experimental")] + #[test] + fn pair_verify_should_work_ecdsa_n_bls() { + pair_verify_should_work::(); + } + + fn pair_works< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >() + where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { + let want = >::KeyPair::from_string("//Alice", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::Alice.pair().to_raw_vec(); + let got = >::pair(Keyring::Alice).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//Bob", None) + let want = >::KeyPair::from_string("//Bob", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::Bob.pair().to_raw_vec(); + let got = >::pair(Keyring::Bob).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//Charlie", None) - .expect("Pair failed") - .to_raw_vec(); - let got = Keyring::Charlie.pair().to_raw_vec(); + let want = + >::KeyPair::from_string("//Charlie", None) + .expect("Pair failed") + .to_raw_vec(); + let got = >::pair(Keyring::Charlie).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//Dave", None) + let want = >::KeyPair::from_string("//Dave", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::Dave.pair().to_raw_vec(); + let got = >::pair(Keyring::Dave).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//Eve", None) + let want = >::KeyPair::from_string("//Eve", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::Eve.pair().to_raw_vec(); + let got = >::pair(Keyring::Eve).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//Ferdie", None) + let want = >::KeyPair::from_string("//Ferdie", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::Ferdie.pair().to_raw_vec(); + let got = >::pair(Keyring::Ferdie).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//One", None) + let want = >::KeyPair::from_string("//One", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::One.pair().to_raw_vec(); + let got = >::pair(Keyring::One).to_raw_vec(); assert_eq!(want, got); - let want = ecdsa_crypto::Pair::from_string("//Two", None) + let want = >::KeyPair::from_string("//Two", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::Two.pair().to_raw_vec(); + let got = >::pair(Keyring::Two).to_raw_vec(); assert_eq!(want, got); } #[test] - fn authority_id_works() { + fn ecdsa_pair_works() { + pair_works::(); + } + + #[cfg(feature = "bls-experimental")] + #[test] + fn ecdsa_n_bls_pair_works() { + pair_works::(); + } + + fn authority_id_works< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >() + where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { let store = keystore(); - let alice: ecdsa_crypto::Public = store - .ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&Keyring::Alice.to_seed())) - .ok() - .unwrap() - .into(); + >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + Some(Keyring::Alice), + ); - let bob = Keyring::Bob.public(); - let charlie = Keyring::Charlie.public(); + let alice = >::public(Keyring::Alice); - let store: BeefyKeystore = Some(store).into(); + let bob = >::public(Keyring::Bob); + let charlie = >::public(Keyring::Charlie); + + let beefy_store: BeefyKeystore = Some(store).into(); let mut keys = vec![bob, charlie]; - let id = store.authority_id(keys.as_slice()); + let id = beefy_store.authority_id(keys.as_slice()); assert!(id.is_none()); keys.push(alice.clone()); - let id = store.authority_id(keys.as_slice()).unwrap(); + let id = beefy_store.authority_id(keys.as_slice()).unwrap(); assert_eq!(id, alice); } + #[cfg(feature = "bls-experimental")] + #[test] + + fn authority_id_works_for_ecdsa() { + authority_id_works::(); + } + + #[cfg(feature = "bls-experimental")] #[test] - fn sign_works() { + + fn authority_id_works_for_ecdsa_n_bls() { + authority_id_works::(); + } + + fn sign_works< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >() + where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { let store = keystore(); - let alice: ecdsa_crypto::Public = store - .ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&Keyring::Alice.to_seed())) - .ok() - .unwrap() - .into(); + >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + Some(Keyring::Alice), + ); + + let alice = >::public(Keyring::Alice); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); let msg = b"are you involved or commited?"; let sig1 = store.sign(&alice, msg).unwrap(); - let sig2 = Keyring::Alice.sign(msg); + let sig2 = >::sign(Keyring::Alice, msg); assert_eq!(sig1, sig2); } #[test] - fn sign_error() { + fn sign_works_for_ecdsa() { + sign_works::(); + } + + #[cfg(feature = "bls-experimental")] + #[test] + fn sign_works_for_ecdsa_n_bls() { + sign_works::(); + } + + fn sign_error< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >( + expected_error_message: &str, + ) where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { let store = keystore(); - store - .ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&Keyring::Bob.to_seed())) - .ok() - .unwrap(); + >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + Some(Keyring::Bob), + ); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); - let alice = Keyring::Alice.public(); + let alice = >::public(Keyring::Alice); let msg = b"are you involved or commited?"; let sig = store.sign(&alice, msg).err().unwrap(); - let err = Error::Signature("ecdsa_sign_prehashed() failed".to_string()); + let err = Error::Signature(expected_error_message.to_string()); assert_eq!(sig, err); } + #[test] + fn sign_error_for_ecdsa() { + sign_error::("ecdsa_sign_prehashed() failed"); + } + + #[cfg(feature = "bls-experimental")] + #[test] + fn sign_error_for_ecdsa_n_bls() { + sign_error::("bls377_sign() failed"); + } + #[test] fn sign_no_keystore() { let store: BeefyKeystore = None.into(); @@ -411,17 +482,25 @@ pub mod tests { assert_eq!(sig, err); } - #[test] - fn verify_works() { + fn verify_works< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >() + where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { let store = keystore(); - let alice: ecdsa_crypto::Public = store - .ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&Keyring::Alice.to_seed())) - .ok() - .unwrap() - .into(); + >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + Some(Keyring::Alice), + ); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); + + let alice = >::public(Keyring::Alice); // `msg` and `sig` match let msg = b"are you involved or commited?"; @@ -433,39 +512,98 @@ pub mod tests { assert!(!BeefyKeystore::verify(&alice, &sig, msg)); } - // Note that we use keys with and without a seed for this test. #[test] - fn public_keys_works() { + fn verify_works_for_ecdsa() { + verify_works::(); + } + + #[cfg(feature = "bls-experimental")] + #[test] + + fn verify_works_for_ecdsa_n_bls() { + verify_works::(); + } + + // Note that we use keys with and without a seed for this test. + fn public_keys_works< + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + >() + where + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + ::Pair: BeefySignerAuthority, + { const TEST_TYPE: sp_application_crypto::KeyTypeId = sp_application_crypto::KeyTypeId(*b"test"); let store = keystore(); - let add_key = - |key_type, seed: Option<&str>| store.ecdsa_generate_new(key_type, seed).unwrap(); - // test keys - let _ = add_key(TEST_TYPE, Some(Keyring::Alice.to_seed().as_str())); - let _ = add_key(TEST_TYPE, Some(Keyring::Bob.to_seed().as_str())); - - let _ = add_key(TEST_TYPE, None); - let _ = add_key(TEST_TYPE, None); + let _ = >::generate_in_store( + store.clone(), + TEST_TYPE, + Some(Keyring::Alice), + ); + let _ = >::generate_in_store( + store.clone(), + TEST_TYPE, + Some(Keyring::Bob), + ); // BEEFY keys - let _ = add_key(BEEFY_KEY_TYPE, Some(Keyring::Dave.to_seed().as_str())); - let _ = add_key(BEEFY_KEY_TYPE, Some(Keyring::Eve.to_seed().as_str())); + let _ = >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + Some(Keyring::Dave), + ); + let _ = >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + Some(Keyring::Eve), + ); + + let _ = >::generate_in_store( + store.clone(), + TEST_TYPE, + None, + ); + let _ = >::generate_in_store( + store.clone(), + TEST_TYPE, + None, + ); - let key1: ecdsa_crypto::Public = add_key(BEEFY_KEY_TYPE, None).into(); - let key2: ecdsa_crypto::Public = add_key(BEEFY_KEY_TYPE, None).into(); + let key1 = >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + None, + ); + let key2 = >::generate_in_store( + store.clone(), + BEEFY_KEY_TYPE, + None, + ); - let store: BeefyKeystore = Some(store).into(); + let store: BeefyKeystore = Some(store).into(); let keys = store.public_keys().ok().unwrap(); assert!(keys.len() == 4); - assert!(keys.contains(&Keyring::Dave.public())); - assert!(keys.contains(&Keyring::Eve.public())); + assert!(keys.contains(&>::public(Keyring::Dave))); + assert!(keys.contains(&>::public(Keyring::Eve))); assert!(keys.contains(&key1)); assert!(keys.contains(&key2)); } + + #[test] + fn public_keys_works_for_ecdsa_keystore() { + public_keys_works::(); + } + + #[cfg(feature = "bls-experimental")] + #[test] + + fn public_keys_works_for_ecdsa_n_bls() { + public_keys_works::(); + } } diff --git a/substrate/client/consensus/beefy/src/round.rs b/substrate/client/consensus/beefy/src/round.rs index 6f400ce47843..a3f8c1baad5f 100644 --- a/substrate/client/consensus/beefy/src/round.rs +++ b/substrate/client/consensus/beefy/src/round.rs @@ -203,8 +203,8 @@ mod tests { use sc_network_test::Block; use sp_consensus_beefy::{ - known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, Keyring, Payload, - SignedCommitment, ValidatorSet, VoteMessage, + known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, GenericKeyring, Keyring, + Payload, SignedCommitment, ValidatorSet, VoteMessage, }; use super::{threshold, AuthorityId, Block as BlockT, RoundTracker, Rounds}; @@ -222,7 +222,10 @@ mod tests { #[test] fn round_tracker() { let mut rt = RoundTracker::default(); - let bob_vote = (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")); + let bob_vote = ( + >::public(Keyring::Bob), + >::sign(Keyring::Bob, b"I am committed"), + ); let threshold = 2; // adding new vote allowed @@ -233,7 +236,10 @@ mod tests { // vote is not done assert!(!rt.is_done(threshold)); - let alice_vote = (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")); + let alice_vote = ( + >::public(Keyring::Alice), + >::sign(Keyring::Alice, b"I am committed"), + ); // adding new vote (self vote this time) allowed assert!(rt.add_vote(alice_vote)); @@ -256,7 +262,11 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], + vec![ + >::public(Keyring::Alice), + >::public(Keyring::Bob), + >::public(Keyring::Charlie), + ], 42, ) .unwrap(); @@ -267,7 +277,11 @@ mod tests { assert_eq!(42, rounds.validator_set_id()); assert_eq!(1, rounds.session_start()); assert_eq!( - &vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], + &vec![ + >::public(Keyring::Alice), + >::public(Keyring::Bob), + >::public(Keyring::Charlie) + ], rounds.validators() ); } @@ -278,9 +292,9 @@ mod tests { let validators = ValidatorSet::::new( vec![ - Keyring::Alice.public(), - Keyring::Bob.public(), - Keyring::Charlie.public(), + >::public(Keyring::Alice), + >::public(Keyring::Bob), + >::public(Keyring::Charlie), Keyring::Eve.public(), ], Default::default(), @@ -295,9 +309,12 @@ mod tests { let block_number = 1; let commitment = Commitment { block_number, payload, validator_set_id }; let mut vote = VoteMessage { - id: Keyring::Alice.public(), + id: >::public(Keyring::Alice), commitment: commitment.clone(), - signature: Keyring::Alice.sign(b"I am committed"), + signature: >::sign( + Keyring::Alice, + b"I am committed", + ), }; // add 1st good vote assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); @@ -305,35 +322,48 @@ mod tests { // double voting (same vote), ok, no effect assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); - vote.id = Keyring::Dave.public(); - vote.signature = Keyring::Dave.sign(b"I am committed"); + vote.id = >::public(Keyring::Dave); + vote.signature = + >::sign(Keyring::Dave, b"I am committed"); // invalid vote (Dave is not a validator) assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Invalid); - vote.id = Keyring::Bob.public(); - vote.signature = Keyring::Bob.sign(b"I am committed"); + vote.id = >::public(Keyring::Bob); + vote.signature = + >::sign(Keyring::Bob, b"I am committed"); // add 2nd good vote assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); - vote.id = Keyring::Charlie.public(); - vote.signature = Keyring::Charlie.sign(b"I am committed"); + vote.id = >::public(Keyring::Charlie); + vote.signature = + >::sign(Keyring::Charlie, b"I am committed"); // add 3rd good vote -> round concluded -> signatures present assert_eq!( rounds.add_vote(vote.clone()), VoteImportResult::RoundConcluded(SignedCommitment { commitment, signatures: vec![ - Some(Keyring::Alice.sign(b"I am committed")), - Some(Keyring::Bob.sign(b"I am committed")), - Some(Keyring::Charlie.sign(b"I am committed")), + Some(>::sign( + Keyring::Alice, + b"I am committed" + )), + Some(>::sign( + Keyring::Bob, + b"I am committed" + )), + Some(>::sign( + Keyring::Charlie, + b"I am committed" + )), None, ] }) ); rounds.conclude(block_number); - vote.id = Keyring::Eve.public(); - vote.signature = Keyring::Eve.sign(b"I am committed"); + vote.id = >::public(Keyring::Eve); + vote.signature = + >::sign(Keyring::Eve, b"I am committed"); // Eve is a validator, but round was concluded, adding vote disallowed assert_eq!(rounds.add_vote(vote), VoteImportResult::Stale); } @@ -343,7 +373,11 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], + vec![ + >::public(Keyring::Alice), + >::public(Keyring::Bob), + >::public(Keyring::Charlie), + ], 42, ) .unwrap(); @@ -358,9 +392,12 @@ mod tests { let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); let commitment = Commitment { block_number, payload, validator_set_id }; let mut vote = VoteMessage { - id: Keyring::Alice.public(), + id: >::public(Keyring::Alice), commitment, - signature: Keyring::Alice.sign(b"I am committed"), + signature: >::sign( + Keyring::Alice, + b"I am committed", + ), }; // add vote for previous session, should fail assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Stale); @@ -389,7 +426,11 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], + vec![ + >::public(Keyring::Alice), + >::public(Keyring::Bob), + >::public(Keyring::Charlie), + ], Default::default(), ) .unwrap(); @@ -401,24 +442,36 @@ mod tests { let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); let commitment = Commitment { block_number: 1, payload, validator_set_id }; let mut alice_vote = VoteMessage { - id: Keyring::Alice.public(), + id: >::public(Keyring::Alice), commitment: commitment.clone(), - signature: Keyring::Alice.sign(b"I am committed"), + signature: >::sign( + Keyring::Alice, + b"I am committed", + ), }; let mut bob_vote = VoteMessage { - id: Keyring::Bob.public(), + id: >::public(Keyring::Bob), commitment: commitment.clone(), - signature: Keyring::Bob.sign(b"I am committed"), + signature: >::sign( + Keyring::Bob, + b"I am committed", + ), }; let mut charlie_vote = VoteMessage { - id: Keyring::Charlie.public(), + id: >::public(Keyring::Charlie), commitment, - signature: Keyring::Charlie.sign(b"I am committed"), + signature: >::sign( + Keyring::Charlie, + b"I am committed", + ), }; let expected_signatures = vec![ - Some(Keyring::Alice.sign(b"I am committed")), - Some(Keyring::Bob.sign(b"I am committed")), - Some(Keyring::Charlie.sign(b"I am committed")), + Some(>::sign(Keyring::Alice, b"I am committed")), + Some(>::sign(Keyring::Bob, b"I am committed")), + Some(>::sign( + Keyring::Charlie, + b"I am committed", + )), ]; // round 1 - only 2 out of 3 vote @@ -464,7 +517,10 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![Keyring::Alice.public(), Keyring::Bob.public()], + vec![ + >::public(Keyring::Alice), + >::public(Keyring::Bob), + ], Default::default(), ) .unwrap(); @@ -478,9 +534,12 @@ mod tests { let commitment2 = Commitment { block_number: 1, payload: payload2, validator_set_id }; let alice_vote1 = VoteMessage { - id: Keyring::Alice.public(), + id: >::public(Keyring::Alice), commitment: commitment1, - signature: Keyring::Alice.sign(b"I am committed"), + signature: >::sign( + Keyring::Alice, + b"I am committed", + ), }; let mut alice_vote2 = alice_vote1.clone(); alice_vote2.commitment = commitment2; diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 902feca16398..e3e5441dac66 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -54,8 +54,8 @@ use sp_consensus_beefy::{ ecdsa_crypto::{AuthorityId, Signature}, known_payloads, mmr::{find_mmr_root_digest, MmrRootProvider}, - BeefyApi, Commitment, ConsensusLog, EquivocationProof, Keyring as BeefyKeyring, MmrRootHash, - OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, ValidatorSetId, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, GenericKeyring, Keyring as BeefyKeyring, + MmrRootHash, OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; use sp_core::H256; @@ -347,13 +347,18 @@ fn add_auth_change_digest(builder: &mut impl BlockBuilderExt, new_auth_set: Beef } pub(crate) fn make_beefy_ids(keys: &[BeefyKeyring]) -> Vec { - keys.iter().map(|&key| key.public().into()).collect() + keys.iter() + .map(|&key| >::public(key).into()) + .collect() } pub(crate) fn create_beefy_keystore(authority: BeefyKeyring) -> KeystorePtr { let keystore = MemoryKeystore::new(); keystore - .ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&authority.to_seed())) + .ecdsa_generate_new( + BEEFY_KEY_TYPE, + Some(&>::to_seed(authority)), + ) .expect("Creates authority key"); keystore.into() } diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 8b3b8e782393..888d1b072344 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -24,7 +24,7 @@ use crate::{ }, error::Error, justification::BeefyVersionedFinalityProof, - keystore::{BeefyKeystore, BeefySignatureHasher}, + keystore::BeefyKeystore, metric_inc, metric_set, metrics::VoterMetrics, round::{Rounds, VoteImportResult}, @@ -42,8 +42,8 @@ use sp_consensus::SyncOracle; use sp_consensus_beefy::{ check_equivocation_proof, ecdsa_crypto::{AuthorityId, Signature}, - BeefyApi, Commitment, ConsensusLog, EquivocationProof, PayloadProvider, ValidatorSet, - VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, + BeefyApi, BeefySignatureHasher, Commitment, ConsensusLog, EquivocationProof, PayloadProvider, + ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; use sp_runtime::{ generic::OpaqueDigestItemId, @@ -1068,7 +1068,7 @@ pub(crate) mod tests { use sp_blockchain::Backend as BlockchainBackendT; use sp_consensus_beefy::{ generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, - mmr::MmrRootProvider, Keyring, Payload, SignedCommitment, + mmr::MmrRootProvider, GenericKeyring, Keyring, Payload, SignedCommitment, }; use sp_runtime::traits::One; use substrate_test_runtime_client::{ @@ -1646,7 +1646,7 @@ pub(crate) mod tests { // now let's try with a bad proof let mut bad_proof = good_proof.clone(); - bad_proof.first.id = Keyring::Charlie.public(); + bad_proof.first.id = >::public(Keyring::Charlie); // bad proofs are simply ignored assert_eq!(worker.report_equivocation(bad_proof), Ok(())); // verify nothing reported to runtime diff --git a/substrate/client/keystore/src/local.rs b/substrate/client/keystore/src/local.rs index 30fbaedce896..c42867295964 100644 --- a/substrate/client/keystore/src/local.rs +++ b/substrate/client/keystore/src/local.rs @@ -37,7 +37,7 @@ use sp_core::bandersnatch; } sp_keystore::bls_experimental_enabled! { -use sp_core::{bls377, bls381, ecdsa_bls377}; +use sp_core::{bls377, bls381, ecdsa_bls377, KeccakHasher}; } use crate::{Error, Result}; @@ -396,9 +396,10 @@ impl Keystore for LocalKeystore { key_type: KeyTypeId, public: &ecdsa_bls377::Public, msg: &[u8], - ) -> Result, Error> { - let sig = self - .pair::(key_type, public) + ) -> std::result::Result, TraitError> { + let sig = self.0 + .read() + .key_pair_by_type::(public, key_type)? .map(|pair| pair.sign_with_hasher::(msg)); Ok(sig) } diff --git a/substrate/primitives/consensus/beefy/Cargo.toml b/substrate/primitives/consensus/beefy/Cargo.toml index 5ff0a2ebc70f..9457be51db5e 100644 --- a/substrate/primitives/consensus/beefy/Cargo.toml +++ b/substrate/primitives/consensus/beefy/Cargo.toml @@ -21,13 +21,14 @@ sp-core = { path = "../../core", default-features = false} sp-io = { path = "../../io", default-features = false} sp-mmr-primitives = { path = "../../merkle-mountain-range", default-features = false} sp-runtime = { path = "../../runtime", default-features = false} +sp-keystore = { path = "../../keystore", default-features = false} sp-std = { path = "../../std", default-features = false} strum = { version = "0.24.1", features = ["derive"], default-features = false } lazy_static = "1.4.0" [dev-dependencies] array-bytes = "6.1" -w3f-bls = { version = "0.1.3", features = ["std"]} +w3f-bls = { version = "0.1.3", features = ["std"]} [features] default = [ "std" ] diff --git a/substrate/primitives/consensus/beefy/src/commitment.rs b/substrate/primitives/consensus/beefy/src/commitment.rs index 5b6ef9ae5ab3..f8be4dbd7916 100644 --- a/substrate/primitives/consensus/beefy/src/commitment.rs +++ b/substrate/primitives/consensus/beefy/src/commitment.rs @@ -397,7 +397,7 @@ mod tests { assert_eq!( encoded, array_bytes::hex2bytes_unchecked( - "046d68343048656c6c6f20576f726c642105000000000000000000000000000000000000000000000004300400000008558455ad81279df0795cc985580e4fb75d72d948d1107b2ac80a09abed4da8480c746cc321f2319a5e99a830e314d10dd3cd68ce3dc0c33c86e99bcb7816f9ba01667603fc041cf9d7147d22bf54b15e5778893d6986b71a929747befd3b4d233fbe668bc480e8865116b94db46ca25a01e03c71955f2582604e415da68f2c3c406b9d5f4ad416230ec5453f05ac16a50d8d0923dfb0413cc956ae3fa6334465bd1f2cacec8e9cd606438390fe2a29dc052d6e1f8105c337a86cdd9aaacdc496577f3db8c55ef9e6fd48f2c5c05a2274707491635d8ba3df64f324575b7b2a34487bca2324b6a0046395a71681be3d0c2a00df61d3b2be0963eb6caa243cc505d327aec73e1bb7ffe9a14b1354b0c406792ac6d6f47c06987c15dec9993f43eefa001d866fe0850d986702c414840f0d9ec0fdc04832ef91ae37c8d49e2f573ca50cb37f152801d489a19395cb04e5fc8f2ab6954b58a3bcc40ef9b6409d2ff7ef07" + "046d68343048656c6c6f20576f726c642105000000000000000000000000000000000000000000000004300400000008558455ad81279df0795cc985580e4fb75d72d948d1107b2ac80a09abed4da8480c746cc321f2319a5e99a830e314d10dd3cd68ce3dc0c33c86e99bcb7816f9ba015dd1c9b2237e54baa93d232cdf83a430b58a5efbc2f86ca1bab173a315ff6f15bef161425750c028055e9a23947b73002889a8b22168628438875a8ef25d76db998a80187b50719471286f054f3b3809b77a0cd87d7fe9c1a9d5d562683e25a70610f0804e92340549a43a7159b77b0c2d6e1f8105c337a86cdd9aaacdc496577f3db8c55ef9e6fd48f2c5c05a2274707491635d8ba3df64f324575b7b2a34487bca2324b6a0046395a71681be3d0c2a001074884b6998c82331bd57ffa0a02cbfd02483c765b9216eab6a1fc119206236bf7971be68acaebff7400edee943240006a6096c9cfa65e9eb4e67f025c27112d14b4574fb208c439500f45cf3a8060f6cf009044f3141cce0364a7c2710a19b1bdf4abf27f86e5e3db08bddd35a7d12" ) ); } diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 738a491f04fb..5af0e9b0800f 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -44,8 +44,9 @@ pub use payload::{known_payloads, BeefyPayloadId, Payload, PayloadProvider}; pub use test_utils::*; use codec::{Codec, Decode, Encode}; +use core::fmt::{Debug, Display}; use scale_info::TypeInfo; -use sp_application_crypto::RuntimeAppPublic; +use sp_application_crypto::{AppCrypto, AppPublic, ByteArray, RuntimeAppPublic}; use sp_core::H256; use sp_runtime::traits::{Hash, Keccak256, NumberFor}; use sp_std::prelude::*; @@ -63,6 +64,30 @@ pub trait BeefyAuthorityId: RuntimeAppPublic { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool; } +/// Hasher used for BEEFY signatures. +pub type BeefySignatureHasher = sp_runtime::traits::Keccak256; + +/// A trait bound which lists all traits which are required to be implemented by +/// a BEEFY AuthorityId type in order to be able to be used in BEEFY Keystore +pub trait AuthorityIdBound: + Codec + + Debug + + Clone + + Ord + + Sync + + Send + + AsRef<[u8]> + + ByteArray + + AppPublic + + AppCrypto + + RuntimeAppPublic + + Display + + BeefyAuthorityId +where + ::Signature: Send + Sync, +{ +} + /// BEEFY cryptographic types for ECDSA crypto /// /// This module basically introduces four crypto types: @@ -74,7 +99,7 @@ pub trait BeefyAuthorityId: RuntimeAppPublic { /// Your code should use the above types as concrete types for all crypto related /// functionality. pub mod ecdsa_crypto { - use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa}; use sp_core::crypto::Wraps; @@ -101,6 +126,7 @@ pub mod ecdsa_crypto { } } } + impl AuthorityIdBound for AuthorityId {} } /// BEEFY cryptographic types for BLS crypto @@ -116,7 +142,7 @@ pub mod ecdsa_crypto { #[cfg(feature = "bls-experimental")] pub mod bls_crypto { - use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, bls377}; use sp_core::{bls377::Pair as BlsPair, crypto::Wraps, Pair as _}; @@ -141,6 +167,7 @@ pub mod bls_crypto { BlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref()) } } + impl AuthorityIdBound for AuthorityId {} } /// BEEFY cryptographic types for (ECDSA,BLS) crypto pair @@ -155,7 +182,7 @@ pub mod bls_crypto { /// functionality. #[cfg(feature = "bls-experimental")] pub mod ecdsa_bls_crypto { - use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa_bls377}; use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair}; @@ -183,6 +210,8 @@ pub mod ecdsa_bls_crypto { ) } } + + impl AuthorityIdBound for AuthorityId {} } /// The `ConsensusEngineId` of BEEFY. diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index b83f657af38e..2f355d5feea8 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -17,9 +17,21 @@ #![cfg(feature = "std")] -use crate::{ecdsa_crypto, Commitment, EquivocationProof, Payload, ValidatorSetId, VoteMessage}; +use core::fmt::Debug; +use sp_runtime::traits::Hash; + +#[cfg(feature = "bls-experimental")] +use crate::ecdsa_bls_crypto; +use crate::{ + ecdsa_crypto, AuthorityIdBound, BeefySignatureHasher, Commitment, EquivocationProof, Payload, + ValidatorSetId, VoteMessage, +}; use codec::Encode; -use sp_core::{ecdsa, keccak_256, Pair}; +use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; +#[cfg(feature = "bls-experimental")] +use sp_core::ecdsa_bls377; +use sp_core::{ecdsa, Pair}; +use sp_keystore::{Keystore, KeystorePtr}; use std::collections::HashMap; use strum::IntoEnumIterator; @@ -36,52 +48,307 @@ pub enum Keyring { One, Two, } +/// Trait representing BEEFY specific generating and signing behavoir of authority id +/// +/// Accepts custom hashing fn for the message and custom convertor fn for the signer. +pub trait BeefySignerAuthority: AppPair { + /// generate a signature. + /// + /// Return `true` if signature over `msg` is valid for this id. + fn sign_with_hasher(&self, message: &[u8]) -> ::Signature; +} + +impl BeefySignerAuthority for ::Pair +where + MsgHash: Hash, + ::Output: Into<[u8; 32]>, +{ + fn sign_with_hasher(&self, message: &[u8]) -> ::Signature { + let hashed_message = ::hash(message).into(); + self.as_inner_ref().sign_prehashed(&hashed_message).into() + } +} + +#[cfg(feature = "bls-experimental")] +impl BeefySignerAuthority for ::Pair +where + MsgHash: Hash, + ::Output: Into<[u8; 32]>, +{ + fn sign_with_hasher(&self, message: &[u8]) -> ::Signature { + self.as_inner_ref().sign_with_hasher::(&message).into() + } +} + +/// Implement Keyring functionalities generically over AuthorityId +pub trait GenericKeyring +where + ::Signature: Send + Sync, +{ + ///The key pair type which is used to perform crypto functionalities for the Keyring + type KeyPair: AppPair; + /// Generate key pair in the given store using the provided seed + fn generate_in_store( + store: KeystorePtr, + key_type: sp_application_crypto::KeyTypeId, + owner: Option, + ) -> AuthorityId; -impl Keyring { /// Sign `msg`. - pub fn sign(self, msg: &[u8]) -> ecdsa_crypto::Signature { - // todo: use custom signature hashing type - let msg = keccak_256(msg); - ecdsa::Pair::from(self).sign_prehashed(&msg).into() + fn sign(self, msg: &[u8]) -> ::Signature; + + /// Return key pair. + fn pair(self) -> Self::KeyPair; + + /// Return public key. + fn public(self) -> AuthorityId; + + /// Return seed string. + fn to_seed(self) -> String; + + /// Get Keyring from public key. + fn from_public(who: &AuthorityId) -> Option; +} + +impl GenericKeyring for Keyring +where + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + ::Pair: BeefySignerAuthority, + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, +{ + type KeyPair = ::Pair; + fn generate_in_store( + store: KeystorePtr, + key_type: sp_application_crypto::KeyTypeId, + owner: Option, + ) -> AuthorityId { + let optional_seed: Option = owner + .map(|owner| >::to_seed(owner)); + + match ::CRYPTO_ID { + ecdsa::CRYPTO_ID => AuthorityId::decode( + &mut Keystore::ecdsa_generate_new(&*store, key_type, optional_seed.as_deref()) + .ok() + .unwrap() + .as_ref(), + ) + .unwrap(), + + #[cfg(feature = "bls-experimental")] + ecdsa_bls377::CRYPTO_ID => { + let pk = Keystore::ecdsa_bls377_generate_new( + &*store, + key_type, + optional_seed.as_deref(), + ) + .ok() + .unwrap(); + let decoded_pk = AuthorityId::decode(&mut pk.as_ref()).unwrap(); + println!( + "Seed: {:?}, before decode: {:?}, after decode: {:?}", + optional_seed, pk, decoded_pk + ); + decoded_pk + }, + + _ => panic!("Requested CRYPTO_ID is not supported by the BEEFY Keyring"), + } + } + /// Sign `msg`. + fn sign(self, msg: &[u8]) -> ::Signature { + let key_pair: Self::KeyPair = >::pair(self); + key_pair.sign_with_hasher(&msg).into() } /// Return key pair. - pub fn pair(self) -> ecdsa_crypto::Pair { - ecdsa::Pair::from_string(self.to_seed().as_str(), None).unwrap().into() + fn pair(self) -> Self::KeyPair { + Self::KeyPair::from_string( + >::to_seed(self).as_str(), + None, + ) + .unwrap() + .into() } /// Return public key. - pub fn public(self) -> ecdsa_crypto::Public { - self.pair().public() + fn public(self) -> AuthorityId { + >::pair(self).public().into() } /// Return seed string. - pub fn to_seed(self) -> String { + fn to_seed(self) -> String { format!("//{}", self) } /// Get Keyring from public key. - pub fn from_public(who: &ecdsa_crypto::Public) -> Option { - Self::iter().find(|&k| &ecdsa_crypto::Public::from(k) == who) + fn from_public(who: &AuthorityId) -> Option { + Self::iter().find(|&k| >::public(k) == *who) } } +// pub(crate) trait SimpleKeyPair: Clone + Sized + Sync + Send { +// type Public: Clone + Codec + Debug + Ord + Sync + Send + Wraps; +// type Signature: Clone + Codec + Debug + Clone + Sync + Send + Wraps; + +// fn generate_in_store(store: KeystorePtr, owner: Keyring) -> Self::Public; + +// fn add_typed_key_to_store( +// store: KeystorePtr, +// key_type: sp_application_crypto::KeyTypeId, +// seed: Option<&str>, +// ) -> Self::Public; + +// fn sign(&self, hashed_message: &[u8]) -> Self::Signature; + +// fn public(&self) -> Self::Public; + +// fn verify(sig: &Self::Signature, hashed_message: &[u8], pubkey: Self::Public) -> bool; + +// fn from_string(s: &str, password_override: Option<&str>) +// -> Result; + +// /// Return a vec filled with raw data. +// fn to_raw_vec(&self) -> Vec; +// } +// Auxiliary traits for ECDSA +// impl SimpleKeyPair for ecdsa_crypto::Pair { +// type Public = ecdsa_crypto::Public; +// type Signature = ecdsa_crypto::Signature; + +// fn geonerate_in_store(store: KeystorePtr, owner: Keyring) -> Self::Public { +// Keystore::ecdsa_generate_new( +// &*store, +// KEY_TYPE, +// Some(&>::to_seed(owner)), +// ) +// .ok() +// o.unwrap() +// .into() +// } + +// fn add_typed_key_to_store( +// store: KeystorePtr, +// key_type: sp_application_crypto::KeyTypeId, +// seed: Option<&str>, +// ) -> Self::Public { +// Keystore::ecdsa_generate_new(&*store, key_type, seed) +// .ok() +// .unwrap() +// .into() +// } + +// fn sign(&self, message: &[u8]) -> Self::Signature { +// let hashed_message = keccak_256(message); +// self.as_inner_ref().sign_prehashed(&hashed_message).into() +// } + +// fn verify( +// sig: &::Signature, +// message: &[u8], +// pubkey: Self::Public, +// ) -> bool { +// let hashed_message = keccak_256(message); +// ecdsa::Pair::verify_prehashed( +// sig.as_inner_ref(), +// &hashed_message, +// pubkey.as_inner_ref(), +// ) +// } + +// fn public(&self) -> Self::Public { +// ::public(self) +// } + +// fn from_string( +// s: &str, +// password_override: Option<&str>, +// ) -> Result { +// ::from_string(s, password_override) +// } + +// /// Return a vec filled with raw data. +// fn to_raw_vec(&self) -> Vec { +// ::to_raw_vec(self) +// } +// } + +// /// Auxiliary traits for ECDSA_BLS377 +// #[cfg(feature = "bls-experimental")] +// impl SimpleKeyPair for ecdas_bls377_crypto::Pair { +// type Public = ecdas_bls377_crypto::Public; +// type Signature = ecdas_bls377_crypto::Signature; + +// fn generate_in_store(store: KeystorePtr, owner: Keyring) -> Self::Public { +// Keystore::ecdsa_bls377_generate_new( +// &*store, +// KEY_TYPE, +// Some(&>::to_seed(owner)), +// ) +// .ok() +// .unwrap() +// .into() +// } + +// fn add_typed_key_to_store( +// store: KeystorePtr, +// key_type: sp_application_crypto::KeyTypeId, +// seed: Option<&str>, +// ) -> Self::Public { +// Keystore::ecdsa_bls377_generate_new(&*store, key_type, seed) +// .ok() +// .unwrap() +// .into() +// } + +// fn sign(&self, message: &[u8]) -> Self::Signature { +// self.sign_with_hasher::(&message).into() + +// } + +// fn verify( +// sig: &::Signature, +// message: &[u8], +// pubkey: Self::Public, +// ) -> bool { +// ecdsa_bls377::Pair::verify_with_hasher::(sig.as_inner_ref(), &message, +// &pubkey.as_inner_ref()) + +// } + +// fn public(&self) -> Self::Public { +// ::public(self) +// } + +// fn from_string( +// s: &str, +// password_override: Option<&str>, +// ) -> Result { +// ::from_string(s, password_override) +// } + +// /// Return a vec filled with raw data. +// fn to_raw_vec(&self) -> Vec { +// ::to_raw_vec(self) +// } +// } + lazy_static::lazy_static! { static ref PRIVATE_KEYS: HashMap = - Keyring::iter().map(|i| (i, i.pair())).collect(); + Keyring::iter().map(|i| (i, >::pair(i))).collect(); static ref PUBLIC_KEYS: HashMap = - PRIVATE_KEYS.iter().map(|(&name, pair)| (name, pair.public())).collect(); + PRIVATE_KEYS.iter().map(|(&name, pair)| (name, sp_application_crypto::Pair::public(pair))).collect(); } impl From for ecdsa_crypto::Pair { fn from(k: Keyring) -> Self { - k.pair() + >::pair(k) } } impl From for ecdsa::Pair { fn from(k: Keyring) -> Self { - k.pair().into() + >::pair(k).into() } } @@ -101,8 +368,15 @@ pub fn generate_equivocation_proof( validator_set_id: ValidatorSetId, keyring: &Keyring| { let commitment = Commitment { validator_set_id, block_number, payload }; - let signature = keyring.sign(&commitment.encode()); - VoteMessage { commitment, id: keyring.public(), signature } + let signature = >::sign( + *keyring, + &commitment.encode(), + ); + VoteMessage { + commitment, + id: >::public(*keyring), + signature, + } }; let first = signed_vote(vote1.0, vote1.1, vote1.2, vote1.3); let second = signed_vote(vote2.0, vote2.1, vote2.2, vote2.3); diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index e519ba1806c4..720520af6bfd 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -452,11 +452,12 @@ impl TraitPair for Pair { fn derive>( &self, path: Iter, - _seed: Option, + seed: Option, ) -> Result<(Self, Option), DeriveError> { - let mut acc: [u8; SECRET_KEY_SERIALIZED_SIZE] = self.0.secret.to_bytes().try_into().expect( - "Secret key serializer returns a vector of SECRET_KEY_SERIALIZED_SIZE size; qed", - ); + let mut acc: [u8; SECRET_KEY_SERIALIZED_SIZE] = + seed.unwrap_or(self.0.secret.to_bytes().try_into().expect( + "Secret key serializer returns a vector of SECRET_KEY_SERIALIZED_SIZE size; qed", + )); for j in path { match j { DeriveJunction::Soft(_cc) => return Err(DeriveError::SoftKeyInPath), @@ -588,12 +589,12 @@ mod test { assert_eq!( public, Public::unchecked_from(array_bytes::hex2array_unchecked( - "6dc6be608fab3c6bd894a606be86db346cc170db85c733853a371f3db54ae1b12052c0888d472760c81b537572a26f00db865e5963aef8634f9917571c51b538b564b2a9ceda938c8b930969ee3b832448e08e33a79e9ddd28af419a3ce45300f5dbc768b067781f44f3fe05a19e6b07b1c4196151ec3f8ea37e4f89a8963030d2101e931276bb9ebe1f20102239d780" + "7a84ca8ce4c37c93c95ecee6a3c0c9a7b9c225093cf2f12dc4f69cbfb847ef9424a18f5755d5a742247d386ff2aabb806bcf160eff31293ea9616976628f77266c8a8cc1d8753be04197bd6cdd8c5c87a148f782c4c1568d599b48833fd539001e580cff64bbc71850605433fcd051f3afc3b74819786f815ffb5272030a8d03e5df61e6183f8fd8ea85f26defa83400" )) ); let message = b""; let signature = - array_bytes::hex2array_unchecked("bbb395bbdee1a35930912034f5fde3b36df2835a0536c865501b0675776a1d5931a3bea2e66eff73b2546c6af2061a8019223e4ebbbed661b2538e0f5823f2c708eb89c406beca8fcb53a5c13dbc7c0c42e4cf2be2942bba96ea29297915a06bd2b1b979c0e2ac8fd4ec684a6b5d110c" + array_bytes::hex2array_unchecked("d1e3013161991e142d8751017d4996209c2ff8a9ee160f373733eda3b4b785ba6edce9f45f87104bbe07aa6aa6eb2780aa705efb2c13d3b317d6409d159d23bdc7cdd5c2a832d1551cf49d811d49c901495e527dbd532e3a462335ce2686009104aba7bc11c5b22be78f3198d2727a0b" ); let expected_signature = Signature::unchecked_from(signature); println!("signature is {:?}", pair.sign(&message[..])); @@ -647,6 +648,19 @@ mod test { assert_eq!(pair1.public(), pair2.public()); } + #[test] + fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + let (pair, phrase, seed) = Pair::generate_with_phrase(None); + let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_seed.public()); + let (repair_phrase, reseed) = + Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); + assert_eq!(seed, reseed); + assert_eq!(pair.public(), repair_phrase.public()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_string.public()); + } + #[test] fn password_does_something() { let (pair1, phrase, _) = Pair::generate_with_phrase(Some("password")); diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 603fa515a30e..a457d6c93326 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -651,6 +651,19 @@ mod test { assert_eq!(pair1.public(), pair2.public()); } + #[test] + fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + let (pair, phrase, seed) = Pair::generate_with_phrase(None); + let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_seed.public()); + let (repair_phrase, reseed) = + Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); + assert_eq!(seed, reseed); + assert_eq!(pair.public(), repair_phrase.public()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_string.public()); + } + #[test] fn password_does_something() { let (pair1, phrase, _) = Pair::generate_with_phrase(Some("password")); diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 03567a8b7317..aaf5efe2b705 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -458,10 +458,11 @@ where path: Iter, seed: Option, ) -> Result<(Self, Option), DeriveError> { - let path: Vec<_> = path.collect(); + let left_path: Vec<_> = path.collect(); + let right_path: Vec<_> = left_path.clone(); - let left = self.left.derive(path.iter().cloned(), seed.map(|s| s.into()))?; - let right = self.right.derive(path.into_iter(), seed.map(|s| s.into()))?; + let left = self.left.derive(left_path.into_iter(), seed.map(|s| s.into()))?; + let right = self.right.derive(right_path.into_iter(), seed.map(|s| s.into()))?; let seed = match (left.1, right.1) { (Some(l), Some(r)) if l.as_ref() == r.as_ref() => Some(l.into()), @@ -540,6 +541,18 @@ mod test { ); } + #[test] + fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + let (pair, phrase, seed) = Pair::generate_with_phrase(None); + let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_seed.public()); + let (repair_phrase, reseed) = + Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); + assert_eq!(seed, reseed); + assert_eq!(pair.public(), repair_phrase.public()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_string.public()); + } #[test] fn seed_and_derive_should_work() { let seed_for_right_and_left: [u8; SECURE_SEED_LEN] = array_bytes::hex2array_unchecked( @@ -597,13 +610,13 @@ mod test { assert_eq!( public, Public::unchecked_from( - array_bytes::hex2array_unchecked("028db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd916dc6be608fab3c6bd894a606be86db346cc170db85c733853a371f3db54ae1b12052c0888d472760c81b537572a26f00db865e5963aef8634f9917571c51b538b564b2a9ceda938c8b930969ee3b832448e08e33a79e9ddd28af419a3ce45300f5dbc768b067781f44f3fe05a19e6b07b1c4196151ec3f8ea37e4f89a8963030d2101e931276bb9ebe1f20102239d780" + array_bytes::hex2array_unchecked("028db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd917a84ca8ce4c37c93c95ecee6a3c0c9a7b9c225093cf2f12dc4f69cbfb847ef9424a18f5755d5a742247d386ff2aabb806bcf160eff31293ea9616976628f77266c8a8cc1d8753be04197bd6cdd8c5c87a148f782c4c1568d599b48833fd539001e580cff64bbc71850605433fcd051f3afc3b74819786f815ffb5272030a8d03e5df61e6183f8fd8ea85f26defa83400" ), ), ); let message = b""; let signature = - array_bytes::hex2array_unchecked("3dde91174bd9359027be59a428b8146513df80a2a3c7eda2194f64de04a69ab97b753169e94db6ffd50921a2668a48b94ca11e3d32c1ff19cfe88890aa7e8f3c00bbb395bbdee1a35930912034f5fde3b36df2835a0536c865501b0675776a1d5931a3bea2e66eff73b2546c6af2061a8019223e4ebbbed661b2538e0f5823f2c708eb89c406beca8fcb53a5c13dbc7c0c42e4cf2be2942bba96ea29297915a06bd2b1b979c0e2ac8fd4ec684a6b5d110c" + array_bytes::hex2array_unchecked("3dde91174bd9359027be59a428b8146513df80a2a3c7eda2194f64de04a69ab97b753169e94db6ffd50921a2668a48b94ca11e3d32c1ff19cfe88890aa7e8f3c00d1e3013161991e142d8751017d4996209c2ff8a9ee160f373733eda3b4b785ba6edce9f45f87104bbe07aa6aa6eb2780aa705efb2c13d3b317d6409d159d23bdc7cdd5c2a832d1551cf49d811d49c901495e527dbd532e3a462335ce2686009104aba7bc11c5b22be78f3198d2727a0b" ); let signature = Signature::unchecked_from(signature); assert!(pair.sign(&message[..]) == signature); diff --git a/substrate/primitives/core/src/sr25519.rs b/substrate/primitives/core/src/sr25519.rs index ffa52ef97d1f..dfeefe130f3c 100644 --- a/substrate/primitives/core/src/sr25519.rs +++ b/substrate/primitives/core/src/sr25519.rs @@ -967,6 +967,19 @@ mod tests { assert!(Pair::verify(&signature, &message[..], &public)); } + #[test] + fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + let (pair, phrase, seed) = Pair::generate_with_phrase(None); + let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_seed.public()); + let (repair_phrase, reseed) = + Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); + assert_eq!(seed, reseed); + assert_eq!(pair.public(), repair_phrase.public()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_string.public()); + } + #[test] fn generated_pair_should_work() { let (pair, _) = Pair::generate(); From c860dd0a1810a0bee3e54c77bc0971ef84119e3d Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Thu, 9 Nov 2023 12:58:03 -0500 Subject: [PATCH 08/37] Improve documention and comments Co-authored-by: Robert Hambrock --- substrate/primitives/consensus/beefy/src/lib.rs | 2 +- substrate/primitives/core/src/paired_crypto.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 738a491f04fb..2684baac407b 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -517,7 +517,7 @@ mod tests { // Verification works if same hashing function is used when signing and verifying. assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); - // Verification doesn't works if we verify function provided by pair_crypto implementation + // Verification doesn't work if we verify function provided by pair_crypto implementation assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &pair.public())); // Other public key doesn't work diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 03567a8b7317..684b8dffa343 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -80,7 +80,7 @@ pub mod ecdsa_bls377 { impl Pair { /// hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates - /// BLS12-377 Signature according to IETF standard and disregard the hasher for the + /// BLS12-377 Signature according to IETF standard and disregards the hasher for the /// BLS12-377 component pub fn sign_with_hasher(&self, message: &[u8]) -> Signature where @@ -96,10 +96,10 @@ pub mod ecdsa_bls377 { ::Signature::unchecked_from(raw) } - /// hashes the `message` with the specified `MsgHasher` and then verifys if the resulting - /// hash was signed by the provided ECDSA public key.. It does not affect the behavoir of - /// BLS12-377 component. It Verify the BLS12-377 signature as it was hashed and signed - /// according to IETF standrad + /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the resulting + /// hash was signed by the provided ECDSA public key. It does not affect the behavior of the + /// BLS12-377 component. It verifies whether the BLS12-377 signature was hashed and signed + /// according to IETF standard pub fn verify_with_hasher( sig: &Signature, message: &[u8], From 8958fde0b96c401be5cd82e233f85ea2e513fd64 Mon Sep 17 00:00:00 2001 From: Skalman Date: Thu, 9 Nov 2023 14:54:21 -0500 Subject: [PATCH 09/37] Remove unused SimpleKeyPair Trait --- .../consensus/beefy/src/test_utils.rs | 146 ------------------ 1 file changed, 146 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 2f355d5feea8..795efbd81012 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -187,152 +187,6 @@ where } } -// pub(crate) trait SimpleKeyPair: Clone + Sized + Sync + Send { -// type Public: Clone + Codec + Debug + Ord + Sync + Send + Wraps; -// type Signature: Clone + Codec + Debug + Clone + Sync + Send + Wraps; - -// fn generate_in_store(store: KeystorePtr, owner: Keyring) -> Self::Public; - -// fn add_typed_key_to_store( -// store: KeystorePtr, -// key_type: sp_application_crypto::KeyTypeId, -// seed: Option<&str>, -// ) -> Self::Public; - -// fn sign(&self, hashed_message: &[u8]) -> Self::Signature; - -// fn public(&self) -> Self::Public; - -// fn verify(sig: &Self::Signature, hashed_message: &[u8], pubkey: Self::Public) -> bool; - -// fn from_string(s: &str, password_override: Option<&str>) -// -> Result; - -// /// Return a vec filled with raw data. -// fn to_raw_vec(&self) -> Vec; -// } -// Auxiliary traits for ECDSA -// impl SimpleKeyPair for ecdsa_crypto::Pair { -// type Public = ecdsa_crypto::Public; -// type Signature = ecdsa_crypto::Signature; - -// fn geonerate_in_store(store: KeystorePtr, owner: Keyring) -> Self::Public { -// Keystore::ecdsa_generate_new( -// &*store, -// KEY_TYPE, -// Some(&>::to_seed(owner)), -// ) -// .ok() -// o.unwrap() -// .into() -// } - -// fn add_typed_key_to_store( -// store: KeystorePtr, -// key_type: sp_application_crypto::KeyTypeId, -// seed: Option<&str>, -// ) -> Self::Public { -// Keystore::ecdsa_generate_new(&*store, key_type, seed) -// .ok() -// .unwrap() -// .into() -// } - -// fn sign(&self, message: &[u8]) -> Self::Signature { -// let hashed_message = keccak_256(message); -// self.as_inner_ref().sign_prehashed(&hashed_message).into() -// } - -// fn verify( -// sig: &::Signature, -// message: &[u8], -// pubkey: Self::Public, -// ) -> bool { -// let hashed_message = keccak_256(message); -// ecdsa::Pair::verify_prehashed( -// sig.as_inner_ref(), -// &hashed_message, -// pubkey.as_inner_ref(), -// ) -// } - -// fn public(&self) -> Self::Public { -// ::public(self) -// } - -// fn from_string( -// s: &str, -// password_override: Option<&str>, -// ) -> Result { -// ::from_string(s, password_override) -// } - -// /// Return a vec filled with raw data. -// fn to_raw_vec(&self) -> Vec { -// ::to_raw_vec(self) -// } -// } - -// /// Auxiliary traits for ECDSA_BLS377 -// #[cfg(feature = "bls-experimental")] -// impl SimpleKeyPair for ecdas_bls377_crypto::Pair { -// type Public = ecdas_bls377_crypto::Public; -// type Signature = ecdas_bls377_crypto::Signature; - -// fn generate_in_store(store: KeystorePtr, owner: Keyring) -> Self::Public { -// Keystore::ecdsa_bls377_generate_new( -// &*store, -// KEY_TYPE, -// Some(&>::to_seed(owner)), -// ) -// .ok() -// .unwrap() -// .into() -// } - -// fn add_typed_key_to_store( -// store: KeystorePtr, -// key_type: sp_application_crypto::KeyTypeId, -// seed: Option<&str>, -// ) -> Self::Public { -// Keystore::ecdsa_bls377_generate_new(&*store, key_type, seed) -// .ok() -// .unwrap() -// .into() -// } - -// fn sign(&self, message: &[u8]) -> Self::Signature { -// self.sign_with_hasher::(&message).into() - -// } - -// fn verify( -// sig: &::Signature, -// message: &[u8], -// pubkey: Self::Public, -// ) -> bool { -// ecdsa_bls377::Pair::verify_with_hasher::(sig.as_inner_ref(), &message, -// &pubkey.as_inner_ref()) - -// } - -// fn public(&self) -> Self::Public { -// ::public(self) -// } - -// fn from_string( -// s: &str, -// password_override: Option<&str>, -// ) -> Result { -// ::from_string(s, password_override) -// } - -// /// Return a vec filled with raw data. -// fn to_raw_vec(&self) -> Vec { -// ::to_raw_vec(self) -// } -// } - lazy_static::lazy_static! { static ref PRIVATE_KEYS: HashMap = Keyring::iter().map(|i| (i, >::pair(i))).collect(); From 3b8ededc87fb5203904cf4725b16407163705d40 Mon Sep 17 00:00:00 2001 From: Skalman Date: Mon, 13 Nov 2023 11:09:31 -0500 Subject: [PATCH 10/37] add reasoning on why in BEEFY we want the ECDSA signature on Keccak hash of the message instead of Blake2 --- substrate/primitives/consensus/beefy/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 2684baac407b..3fd33e934c69 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -175,7 +175,14 @@ pub mod ecdsa_bls_crypto { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` // because that invokes ecdsa default verification which perfoms blake2 hash - // which we don't want. + // which we don't want. This is because ECDSA signatures are meant to be verified + // on Ethereum network where Keccak hasher is siginificantly cheaper than Blake2b. + // See Figure 3 of [OnSc21](https://www.scitepress.org/Papers/2021/106066/106066.pdf) + // for comparison. + // + // - [OnSc21] Onica, E., & Schifirneţ, C. (2021). Towards efficient hashing in + // ethereum smart contracts. Proceedings of the 16th International + // Conference on Software Technologies, () EcdsaBlsPair::verify_with_hasher::( signature.as_inner_ref(), msg, From 8cfccd8696f39a2727a5952d55d8491d0648d3ff Mon Sep 17 00:00:00 2001 From: Skalman Date: Mon, 13 Nov 2023 11:45:32 -0500 Subject: [PATCH 11/37] fmt --- substrate/primitives/core/src/paired_crypto.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 684b8dffa343..aa9994695573 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -96,10 +96,10 @@ pub mod ecdsa_bls377 { ::Signature::unchecked_from(raw) } - /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the resulting - /// hash was signed by the provided ECDSA public key. It does not affect the behavior of the - /// BLS12-377 component. It verifies whether the BLS12-377 signature was hashed and signed - /// according to IETF standard + /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the + /// resulting hash was signed by the provided ECDSA public key. It does not affect the + /// behavior of the BLS12-377 component. It verifies whether the BLS12-377 signature was + /// hashed and signed according to IETF standard pub fn verify_with_hasher( sig: &Signature, message: &[u8], From a15a60da935a52317d8e9975de233a9b9b9a0629 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Mon, 13 Nov 2023 13:08:43 -0500 Subject: [PATCH 12/37] Improve documentation Co-authored-by: Davide Galassi --- substrate/primitives/consensus/beefy/src/lib.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 3fd33e934c69..f79ee3ba0c9d 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -174,15 +174,11 @@ pub mod ecdsa_bls_crypto { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` - // because that invokes ecdsa default verification which perfoms blake2 hash + // because that invokes ECDSA default verification which perfoms Blake2b hash // which we don't want. This is because ECDSA signatures are meant to be verified - // on Ethereum network where Keccak hasher is siginificantly cheaper than Blake2b. + // on Ethereum network where Keccak hasher is significantly cheaper than Blake2b. // See Figure 3 of [OnSc21](https://www.scitepress.org/Papers/2021/106066/106066.pdf) // for comparison. - // - // - [OnSc21] Onica, E., & Schifirneţ, C. (2021). Towards efficient hashing in - // ethereum smart contracts. Proceedings of the 16th International - // Conference on Software Technologies, () EcdsaBlsPair::verify_with_hasher::( signature.as_inner_ref(), msg, From 00b62effe599d36d0ca699438ba996a8bafab96e Mon Sep 17 00:00:00 2001 From: Skalman Date: Mon, 13 Nov 2023 15:24:15 -0500 Subject: [PATCH 13/37] fmt --- substrate/client/consensus/beefy/src/keystore.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 80e2f807c2b1..e21364c4baa5 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -180,9 +180,7 @@ where ecdsa_bls377::CRYPTO_ID => store .ecdsa_bls377_public_keys(BEEFY_KEY_TYPE) .drain(..) - .map(|pk| - AuthorityId::try_from(pk.as_ref()) - ) + .map(|pk| AuthorityId::try_from(pk.as_ref())) .collect::, _>>() .or_else(|_| { Err(error::Error::Keystore( From 3ef0f652f199cafc794d4eee924503138cd6d7c9 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Mon, 27 Nov 2023 06:13:40 -0500 Subject: [PATCH 14/37] Improve comment quality Co-authored-by: Robert Hambrock --- substrate/client/consensus/beefy/src/keystore.rs | 8 ++++---- substrate/primitives/consensus/beefy/src/lib.rs | 2 +- substrate/primitives/consensus/beefy/src/test_utils.rs | 3 ++- substrate/primitives/core/src/paired_crypto.rs | 2 +- substrate/primitives/keystore/src/lib.rs | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index e21364c4baa5..0ea46a47eee3 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -38,9 +38,9 @@ use crate::{error, LOG_TARGET}; // RuntimeAppPublic + BeefyAuthorityId > AuthorityIdBound for T { type // Signature = AppCrypto::Signature; } -/// A BEEFY specific keystore i mplemented as a `Newtype`. This is basically a +/// A BEEFY specific keystore implemented as a `Newtype`. This is basically a /// wrapper around [`sp_keystore::Keystore`] and allows to customize -/// commoncryptographic functionality. +/// common cryptographic functionality. pub(crate) struct BeefyKeystore( Option, PhantomData AuthorityId>, @@ -95,9 +95,9 @@ where ) -> Result<::Signature, error::Error> { let store = self.0.clone().ok_or_else(|| error::Error::Keystore("no Keystore".into()))?; - //ECDSA shoud do ecdsa_sign_prehashed because it needs to be hashed by keccak_256 - //as such we need to deal with signing case by case + // ECDSA should use ecdsa_sign_prehashed since it needs to be hashed by keccak_256 instead of blake2. + // As such we need to deal with producing the signatures case-by-case let signature_byte_array: Vec = match ::CRYPTO_ID { ecdsa::CRYPTO_ID => { let msg_hash = keccak_256(message); diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index bd412faa4f7f..4fd71b298fa5 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -160,7 +160,7 @@ pub mod bls_crypto { { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { // `w3f-bls` library uses IETF hashing standard and as such does not expose - // a choice of hash to field function. + // a choice of hash-to-field function. // We are directly calling into the library to avoid introducing new host call. // and because BeefyAuthorityId::verify is being called in the runtime so we don't have diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 795efbd81012..6aefc52f1fa5 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -48,7 +48,8 @@ pub enum Keyring { One, Two, } -/// Trait representing BEEFY specific generating and signing behavoir of authority id + +/// Trait representing BEEFY specific generation and signing behavior of authority id /// /// Accepts custom hashing fn for the message and custom convertor fn for the signer. pub trait BeefySignerAuthority: AppPair { diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index a26c536d257c..79c02f1a979f 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -78,7 +78,7 @@ pub mod ecdsa_bls377 { #[cfg(feature = "full_crypto")] impl Pair { - /// hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA + /// Hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates /// BLS12-377 Signature according to IETF standard and disregards the hasher for the /// BLS12-377 component diff --git a/substrate/primitives/keystore/src/lib.rs b/substrate/primitives/keystore/src/lib.rs index 8999bf2ec0bc..f6132f5e10f4 100644 --- a/substrate/primitives/keystore/src/lib.rs +++ b/substrate/primitives/keystore/src/lib.rs @@ -356,7 +356,7 @@ pub trait Keystore: Send + Sync { ) -> Result, Error>; /// Hashes the `message` using keccak256 and then signs it using ECDSA - /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates + /// algorithm. It does not affect the behavior of BLS12-377 component. It generates /// BLS12-377 Signature according to IETF standard. /// /// Receives [`KeyTypeId`] and a [`ecdsa_bls377::Public`] key to be able to map From 3f8d533375f3f78ddf315672cff605d94d92ee1c Mon Sep 17 00:00:00 2001 From: Skalman Date: Tue, 5 Dec 2023 12:43:18 -0500 Subject: [PATCH 15/37] drop 'perhaps' from generate_with_phrase_should_be_recoverable_with_from_string_perhaps test and add it to bandersnatch and ed25519 --- substrate/primitives/core/src/bandersnatch.rs | 13 +++++++++++++ substrate/primitives/core/src/bls.rs | 2 +- substrate/primitives/core/src/ecdsa.rs | 2 +- substrate/primitives/core/src/ed25519.rs | 13 +++++++++++++ substrate/primitives/core/src/paired_crypto.rs | 3 ++- substrate/primitives/core/src/sr25519.rs | 2 +- 6 files changed, 31 insertions(+), 4 deletions(-) diff --git a/substrate/primitives/core/src/bandersnatch.rs b/substrate/primitives/core/src/bandersnatch.rs index b91c001a7de0..760cc0862b32 100644 --- a/substrate/primitives/core/src/bandersnatch.rs +++ b/substrate/primitives/core/src/bandersnatch.rs @@ -983,6 +983,19 @@ mod tests { assert!(res.is_err()); } + #[test] + fn generate_with_phrase_should_be_recoverable_with_from_string() { + let (pair, phrase, seed) = Pair::generate_with_phrase(None); + let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_seed.public()); + let (repair_phrase, reseed) = + Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); + assert_eq!(seed, reseed); + assert_eq!(pair.public(), repair_phrase.public()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_string.public()); + } + #[test] fn sign_verify() { let pair = Pair::from_seed(DEV_SEED); diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index 720520af6bfd..11b1deeae8df 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -649,7 +649,7 @@ mod test { } #[test] - fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + fn generate_with_phrase_should_be_recoverable_with_from_string() { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 0e31cad15d8d..9568c1386bf9 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -647,7 +647,7 @@ mod test { } #[test] - fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + fn generate_with_phrase_should_be_recoverable_with_from_string() { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); diff --git a/substrate/primitives/core/src/ed25519.rs b/substrate/primitives/core/src/ed25519.rs index 151a7229315e..8b99e38247da 100644 --- a/substrate/primitives/core/src/ed25519.rs +++ b/substrate/primitives/core/src/ed25519.rs @@ -501,6 +501,19 @@ mod test { ); } + #[test] + fn generate_with_phrase_should_be_recoverable_with_from_string() { + let (pair, phrase, seed) = Pair::generate_with_phrase(None); + let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_seed.public()); + let (repair_phrase, reseed) = + Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); + assert_eq!(seed, reseed); + assert_eq!(pair.public(), repair_phrase.public()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); + assert_eq!(pair.public(), repair_string.public()); + } + #[test] fn test_vector_should_work() { let pair = Pair::from_seed(&array_bytes::hex2array_unchecked( diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index dded976e965e..f48f7f0918c5 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -544,7 +544,7 @@ mod test { } #[test] - fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + fn generate_with_phrase_should_be_recoverable_with_from_string() { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); @@ -555,6 +555,7 @@ mod test { let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); assert_eq!(pair.public(), repair_string.public()); } + #[test] fn seed_and_derive_should_work() { let seed_for_right_and_left: [u8; SECURE_SEED_LEN] = array_bytes::hex2array_unchecked( diff --git a/substrate/primitives/core/src/sr25519.rs b/substrate/primitives/core/src/sr25519.rs index dfeefe130f3c..4018cd5bb120 100644 --- a/substrate/primitives/core/src/sr25519.rs +++ b/substrate/primitives/core/src/sr25519.rs @@ -968,7 +968,7 @@ mod tests { } #[test] - fn generate_with_phrase_should_be_recoverable_with_from_string_perhaps() { + fn generate_with_phrase_should_be_recoverable_with_from_string() { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); From 5f09d22404a2c3470e1ac3a9f6ad0ea426dff300 Mon Sep 17 00:00:00 2001 From: Robert Hambrock Date: Tue, 5 Dec 2023 21:44:46 +0100 Subject: [PATCH 16/37] fmt --- substrate/client/consensus/beefy/src/keystore.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 0ea46a47eee3..8b7c638f2d93 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -95,9 +95,8 @@ where ) -> Result<::Signature, error::Error> { let store = self.0.clone().ok_or_else(|| error::Error::Keystore("no Keystore".into()))?; - - // ECDSA should use ecdsa_sign_prehashed since it needs to be hashed by keccak_256 instead of blake2. - // As such we need to deal with producing the signatures case-by-case + // ECDSA should use ecdsa_sign_prehashed since it needs to be hashed by keccak_256 instead + // of blake2. As such we need to deal with producing the signatures case-by-case let signature_byte_array: Vec = match ::CRYPTO_ID { ecdsa::CRYPTO_ID => { let msg_hash = keccak_256(message); From 6304d1e110d76730499ff38ca341d086c88ead76 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Fri, 19 Jan 2024 17:53:41 +0100 Subject: [PATCH 17/37] Apply suggestions from code review Code and dependency clean-up Co-authored-by: Davide Galassi --- substrate/client/consensus/beefy/Cargo.toml | 1 - substrate/primitives/consensus/beefy/src/test_utils.rs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/client/consensus/beefy/Cargo.toml b/substrate/client/consensus/beefy/Cargo.toml index e2f26b71f857..c79e8b919b38 100644 --- a/substrate/client/consensus/beefy/Cargo.toml +++ b/substrate/client/consensus/beefy/Cargo.toml @@ -36,7 +36,6 @@ sp-core = { path = "../../../primitives/core" } sp-keystore = { path = "../../../primitives/keystore" } sp-mmr-primitives = { path = "../../../primitives/merkle-mountain-range" } sp-runtime = { path = "../../../primitives/runtime" } -sp-std = { path = "../../../primitives/std", default-features = false} [dev-dependencies] serde = "1.0.193" diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 6aefc52f1fa5..13f2eb0176b1 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -53,7 +53,7 @@ pub enum Keyring { /// /// Accepts custom hashing fn for the message and custom convertor fn for the signer. pub trait BeefySignerAuthority: AppPair { - /// generate a signature. + /// Generate a signature. /// /// Return `true` if signature over `msg` is valid for this id. fn sign_with_hasher(&self, message: &[u8]) -> ::Signature; @@ -88,6 +88,7 @@ where { ///The key pair type which is used to perform crypto functionalities for the Keyring type KeyPair: AppPair; + /// Generate key pair in the given store using the provided seed fn generate_in_store( store: KeystorePtr, From 7fc132605dcf34e45db9558cce63a5270928e471 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Fri, 19 Jan 2024 18:25:21 +0100 Subject: [PATCH 18/37] Apply suggestions from code review Co-authored-by: Davide Galassi --- substrate/client/consensus/beefy/src/keystore.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 8b7c638f2d93..dab1efdd8e73 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -34,9 +34,6 @@ use sp_consensus_beefy::ecdsa_bls_crypto; use crate::{error, LOG_TARGET}; -// impl + AppPublic + AppCrypto + -// RuntimeAppPublic + BeefyAuthorityId > AuthorityIdBound for T { type -// Signature = AppCrypto::Signature; } /// A BEEFY specific keystore implemented as a `Newtype`. This is basically a /// wrapper around [`sp_keystore::Keystore`] and allows to customize From 6401a97733c2353df89ba08151f30fbd018e2c5e Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 19 Jan 2024 13:17:56 -0500 Subject: [PATCH 19/37] use std instead of sp_std as we are not in the Runtime --- substrate/client/consensus/beefy/src/keystore.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 8b7c638f2d93..724d5b3dcc66 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -23,7 +23,7 @@ use sp_core::ecdsa_bls377; use sp_core::{ecdsa, keccak_256}; use sp_keystore::KeystorePtr; -use sp_std::marker::PhantomData; +use std::marker::PhantomData; use log::warn; From 425c3d341c9cda99edde0a83cdb1f9b4dc82820f Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 19 Jan 2024 13:19:06 -0500 Subject: [PATCH 20/37] remove sp-std fro Cargo.lock --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 27ce0b716022..958ef3b4c4d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15224,7 +15224,6 @@ dependencies = [ "sp-keystore", "sp-mmr-primitives", "sp-runtime", - "sp-std 8.0.0", "sp-tracing 10.0.0", "substrate-prometheus-endpoint", "substrate-test-runtime-client", From 59a4b5f8328f9b542454672305e383ef61cee6f6 Mon Sep 17 00:00:00 2001 From: Skalman Date: Sat, 20 Jan 2024 14:36:46 -0500 Subject: [PATCH 21/37] Do not re-export internal of primitives/consensus/beefy/src/test_utils --- substrate/client/consensus/beefy/src/communication/gossip.rs | 2 +- substrate/client/consensus/beefy/src/justification.rs | 2 +- substrate/client/consensus/beefy/src/keystore.rs | 2 +- substrate/client/consensus/beefy/src/round.rs | 2 +- substrate/client/consensus/beefy/src/tests.rs | 2 +- substrate/client/consensus/beefy/src/worker.rs | 4 ++-- substrate/primitives/consensus/beefy/src/lib.rs | 4 +--- 7 files changed, 8 insertions(+), 10 deletions(-) diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index c70e5214a13f..eb18f1f4034b 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -481,7 +481,7 @@ pub(crate) mod tests { use sc_network_test::Block; use sp_application_crypto::key_types::BEEFY as BEEFY_KEY_TYPE; use sp_consensus_beefy::{ - ecdsa_crypto::Signature, known_payloads, Commitment, GenericKeyring, Keyring, MmrRootHash, + ecdsa_crypto::Signature, known_payloads, Commitment, test_utils::GenericKeyring, test_utils::Keyring, MmrRootHash, Payload, SignedCommitment, VoteMessage, }; use sp_keystore::{testing::MemoryKeystore, Keystore}; diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index 6de9c5b3316f..00a0e20c1724 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -93,7 +93,7 @@ pub(crate) fn verify_with_validator_set( #[cfg(test)] pub(crate) mod tests { use sp_consensus_beefy::{ - known_payloads, Commitment, GenericKeyring, Keyring, Payload, SignedCommitment, + known_payloads, Commitment, test_utils::GenericKeyring, test_utils::Keyring, Payload, SignedCommitment, VersionedFinalityProof, }; use substrate_test_runtime_client::runtime::Block; diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index e67856c160dd..6066aa5c6451 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -213,7 +213,7 @@ where #[cfg(test)] pub mod tests { - use sp_consensus_beefy::{ecdsa_crypto, BeefySignerAuthority, GenericKeyring, Keyring}; + use sp_consensus_beefy::{ecdsa_crypto, test_utils::BeefySignerAuthority, test_utils::GenericKeyring, test_utils::Keyring}; use sp_core::Pair as PairT; use sp_keystore::testing::MemoryKeystore; diff --git a/substrate/client/consensus/beefy/src/round.rs b/substrate/client/consensus/beefy/src/round.rs index a3f8c1baad5f..29f5f10a4444 100644 --- a/substrate/client/consensus/beefy/src/round.rs +++ b/substrate/client/consensus/beefy/src/round.rs @@ -203,7 +203,7 @@ mod tests { use sc_network_test::Block; use sp_consensus_beefy::{ - known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, GenericKeyring, Keyring, + known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, test_utils::GenericKeyring, test_utils::Keyring, Payload, SignedCommitment, ValidatorSet, VoteMessage, }; diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index e85f1abc083f..85b5de238de8 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -55,7 +55,7 @@ use sp_consensus_beefy::{ ecdsa_crypto::{AuthorityId, Signature}, known_payloads, mmr::{find_mmr_root_digest, MmrRootProvider}, - BeefyApi, Commitment, ConsensusLog, EquivocationProof, GenericKeyring, Keyring as BeefyKeyring, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, test_utils::GenericKeyring, test_utils::Keyring as BeefyKeyring, MmrRootHash, OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index cb2b3285176a..49f888cf011d 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -1076,8 +1076,8 @@ pub(crate) mod tests { use sc_network_test::TestNetFactory; use sp_blockchain::Backend as BlockchainBackendT; use sp_consensus_beefy::{ - generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, - mmr::MmrRootProvider, GenericKeyring, Keyring, Payload, SignedCommitment, + test_utils::generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, + mmr::MmrRootProvider, test_utils::GenericKeyring, test_utils::Keyring, Payload, SignedCommitment, }; use sp_runtime::traits::{Header as HeaderT, One}; use substrate_test_runtime_client::{ diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 9b4d2955cb9d..a5758b74afb1 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -35,13 +35,11 @@ mod commitment; pub mod mmr; mod payload; #[cfg(feature = "std")] -mod test_utils; +pub mod test_utils; pub mod witness; pub use commitment::{Commitment, SignedCommitment, VersionedFinalityProof}; pub use payload::{known_payloads, BeefyPayloadId, Payload, PayloadProvider}; -#[cfg(feature = "std")] -pub use test_utils::*; use codec::{Codec, Decode, Encode}; use core::fmt::{Debug, Display}; From f2956c02249f390bfa7f7b33804da985e234733d Mon Sep 17 00:00:00 2001 From: Skalman Date: Sat, 20 Jan 2024 15:01:34 -0500 Subject: [PATCH 22/37] fix wrong doc for `BeefySignerAuthority::sign_with_hasher` --- substrate/primitives/consensus/beefy/src/test_utils.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 13f2eb0176b1..0d19d4835465 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -53,9 +53,7 @@ pub enum Keyring { /// /// Accepts custom hashing fn for the message and custom convertor fn for the signer. pub trait BeefySignerAuthority: AppPair { - /// Generate a signature. - /// - /// Return `true` if signature over `msg` is valid for this id. + /// Generate and return signature for `message` using custom hashing `MsgHash` fn sign_with_hasher(&self, message: &[u8]) -> ::Signature; } From 3422057065a19d00c0fcc3e2a0f62fedd4cfef59 Mon Sep 17 00:00:00 2001 From: Skalman Date: Wed, 24 Jan 2024 11:13:19 -0500 Subject: [PATCH 23/37] Make BEEFY Keyring itself generic over AuthId using PhantomData. --- .../consensus/beefy/src/test_utils.rs | 113 ++++++++---------- 1 file changed, 47 insertions(+), 66 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 0d19d4835465..d70a4d1dbcab 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -34,11 +34,13 @@ use sp_core::{ecdsa, Pair}; use sp_keystore::{Keystore, KeystorePtr}; use std::collections::HashMap; use strum::IntoEnumIterator; +use std::marker::PhantomData; + /// Set of test accounts using [`crate::ecdsa_crypto`] types. #[allow(missing_docs)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumIter)] -pub enum Keyring { +pub enum Keyring { Alice, Bob, Charlie, @@ -46,7 +48,8 @@ pub enum Keyring { Eve, Ferdie, One, - Two, + Two, + _Marker(PhantomData) } /// Trait representing BEEFY specific generation and signing behavior of authority id @@ -79,52 +82,22 @@ where } } -/// Implement Keyring functionalities generically over AuthorityId -pub trait GenericKeyring -where - ::Signature: Send + Sync, -{ - ///The key pair type which is used to perform crypto functionalities for the Keyring - type KeyPair: AppPair; - /// Generate key pair in the given store using the provided seed - fn generate_in_store( + + +/// Generate key pair in the given store using the provided seed +fn generate_in_store( store: KeystorePtr, key_type: sp_application_crypto::KeyTypeId, - owner: Option, - ) -> AuthorityId; - - /// Sign `msg`. - fn sign(self, msg: &[u8]) -> ::Signature; - - /// Return key pair. - fn pair(self) -> Self::KeyPair; - - /// Return public key. - fn public(self) -> AuthorityId; - - /// Return seed string. - fn to_seed(self) -> String; - - /// Get Keyring from public key. - fn from_public(who: &AuthorityId) -> Option; -} - -impl GenericKeyring for Keyring -where + owner: Option>, + ) -> AuthorityId where AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, ::Pair: BeefySignerAuthority, ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, { - type KeyPair = ::Pair; - fn generate_in_store( - store: KeystorePtr, - key_type: sp_application_crypto::KeyTypeId, - owner: Option, - ) -> AuthorityId { - let optional_seed: Option = owner - .map(|owner| >::to_seed(owner)); + let optional_seed: Option = owner + .map(|owner| owner.to_seed()); match ::CRYPTO_ID { ecdsa::CRYPTO_ID => AuthorityId::decode( @@ -154,17 +127,26 @@ where _ => panic!("Requested CRYPTO_ID is not supported by the BEEFY Keyring"), } - } +} + +/// Implement Keyring functionalities generically over AuthorityId +impl Keyring +where + AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + ::Pair: BeefySignerAuthority, + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, +{ /// Sign `msg`. fn sign(self, msg: &[u8]) -> ::Signature { - let key_pair: Self::KeyPair = >::pair(self); + let key_pair: ::Pair = self.clone().pair(); key_pair.sign_with_hasher(&msg).into() } /// Return key pair. - fn pair(self) -> Self::KeyPair { - Self::KeyPair::from_string( - >::to_seed(self).as_str(), + fn pair(self) -> ::Pair { + ::Pair::from_string( + self.to_seed().as_str(), None, ) .unwrap() @@ -173,7 +155,7 @@ where /// Return public key. fn public(self) -> AuthorityId { - >::pair(self).public().into() + self.clone().pair().public().into() } /// Return seed string. @@ -182,53 +164,52 @@ where } /// Get Keyring from public key. - fn from_public(who: &AuthorityId) -> Option { - Self::iter().find(|&k| >::public(k) == *who) + fn from_public(who: &AuthorityId) -> Option> { + Self::iter().find(|k| k.clone().public() == *who) } } lazy_static::lazy_static! { - static ref PRIVATE_KEYS: HashMap = - Keyring::iter().map(|i| (i, >::pair(i))).collect(); - static ref PUBLIC_KEYS: HashMap = - PRIVATE_KEYS.iter().map(|(&name, pair)| (name, sp_application_crypto::Pair::public(pair))).collect(); + static ref PRIVATE_KEYS: HashMap, ecdsa_crypto::Pair> = + Keyring::iter().map(|i| (i.clone(), i.clone().pair())).collect(); + static ref PUBLIC_KEYS: HashMap, ecdsa_crypto::Public> = + PRIVATE_KEYS.iter().map(|(name, pair)| (name.clone(), sp_application_crypto::Pair::public(pair))).collect(); } -impl From for ecdsa_crypto::Pair { - fn from(k: Keyring) -> Self { - >::pair(k) +impl From> for ecdsa_crypto::Pair { + fn from(k: Keyring) -> Self { + k.clone().pair() } } -impl From for ecdsa::Pair { - fn from(k: Keyring) -> Self { - >::pair(k).into() +impl From> for ecdsa::Pair { + fn from(k: Keyring) -> Self { + k.clone().pair().into() } } -impl From for ecdsa_crypto::Public { - fn from(k: Keyring) -> Self { +impl From> for ecdsa_crypto::Public { + fn from(k: Keyring) -> Self { (*PUBLIC_KEYS).get(&k).cloned().unwrap() } } /// Create a new `EquivocationProof` based on given arguments. pub fn generate_equivocation_proof( - vote1: (u64, Payload, ValidatorSetId, &Keyring), - vote2: (u64, Payload, ValidatorSetId, &Keyring), + vote1: (u64, Payload, ValidatorSetId, &Keyring), + vote2: (u64, Payload, ValidatorSetId, &Keyring), ) -> EquivocationProof { let signed_vote = |block_number: u64, payload: Payload, validator_set_id: ValidatorSetId, - keyring: &Keyring| { + keyring: &Keyring| { let commitment = Commitment { validator_set_id, block_number, payload }; - let signature = >::sign( - *keyring, + let signature = keyring.clone().sign( &commitment.encode(), ); VoteMessage { commitment, - id: >::public(*keyring), + id: keyring.clone().public(), signature, } }; From 7454b6d13e7f5c5615da4c97b4890f7ff298bed7 Mon Sep 17 00:00:00 2001 From: Skalman Date: Thu, 25 Jan 2024 09:19:23 -0500 Subject: [PATCH 24/37] Compatiblize all client/beefy tests to Keyring enum model --- .../beefy/src/communication/gossip.rs | 10 +- .../consensus/beefy/src/justification.rs | 9 +- .../client/consensus/beefy/src/keystore.rs | 84 +++++++------- substrate/client/consensus/beefy/src/round.rs | 106 ++++++++---------- substrate/client/consensus/beefy/src/tests.rs | 18 +-- .../client/consensus/beefy/src/worker.rs | 8 +- .../consensus/beefy/src/test_utils.rs | 28 ++--- 7 files changed, 126 insertions(+), 137 deletions(-) diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index eb18f1f4034b..03c490fef745 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -481,7 +481,7 @@ pub(crate) mod tests { use sc_network_test::Block; use sp_application_crypto::key_types::BEEFY as BEEFY_KEY_TYPE; use sp_consensus_beefy::{ - ecdsa_crypto::Signature, known_payloads, Commitment, test_utils::GenericKeyring, test_utils::Keyring, MmrRootHash, + ecdsa_crypto::Signature, known_payloads, Commitment, test_utils::Keyring, MmrRootHash, Payload, SignedCommitment, VoteMessage, }; use sp_keystore::{testing::MemoryKeystore, Keystore}; @@ -503,12 +503,12 @@ pub(crate) mod tests { } } - pub fn sign_commitment(who: &Keyring, commitment: &Commitment) -> Signature { + pub fn sign_commitment(who: &Keyring, commitment: &Commitment) -> Signature { let store = MemoryKeystore::new(); store .ecdsa_generate_new( BEEFY_KEY_TYPE, - Some(&>::to_seed(*who)), + Some(&who.to_seed()), ) .unwrap(); let beefy_keystore: BeefyKeystore = Some(store.into()).into(); @@ -540,7 +540,7 @@ pub(crate) mod tests { .iter() .map(|validator: &AuthorityId| { Some(sign_commitment( - &>::from_public(validator).unwrap(), + &Keyring::::from_public(validator).unwrap(), &commitment, )) }) @@ -551,7 +551,7 @@ pub(crate) mod tests { #[test] fn should_validate_messages() { - let keys = vec![Keyring::Alice.public()]; + let keys = vec![Keyring::::Alice.public()]; let validator_set = ValidatorSet::::new(keys.clone(), 0).unwrap(); let (gv, mut report_stream) = GossipValidator::::new(Arc::new(Mutex::new(KnownPeers::new()))); diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index 00a0e20c1724..d49b8abf4cf8 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -93,7 +93,7 @@ pub(crate) fn verify_with_validator_set( #[cfg(test)] pub(crate) mod tests { use sp_consensus_beefy::{ - known_payloads, Commitment, test_utils::GenericKeyring, test_utils::Keyring, Payload, SignedCommitment, + known_payloads, Commitment, test_utils::Keyring, Payload, SignedCommitment, VersionedFinalityProof, }; use substrate_test_runtime_client::runtime::Block; @@ -104,7 +104,7 @@ pub(crate) mod tests { pub(crate) fn new_finality_proof( block_num: NumberFor, validator_set: &ValidatorSet, - keys: &[Keyring], + keys: &[Keyring], ) -> BeefyVersionedFinalityProof { let commitment = Commitment { payload: Payload::from_single_entry(known_payloads::MMR_ROOT_ID, vec![]), @@ -114,7 +114,7 @@ pub(crate) mod tests { let message = commitment.encode(); let signatures = keys .iter() - .map(|key| Some(>::sign(*key, &message))) + .map(|key| Some(key.sign(&message))) .collect(); VersionedFinalityProof::V1(SignedCommitment { commitment, signatures }) } @@ -178,8 +178,7 @@ pub(crate) mod tests { }; // change a signature to a different key *bad_signed_commitment.signatures.first_mut().unwrap() = - Some(>::sign( - Keyring::Dave, + Some(Keyring::::Dave.sign( &bad_signed_commitment.commitment.encode(), )); match verify_with_validator_set::(block_num, &validator_set, &bad_proof.into()) { diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 6066aa5c6451..eb79a385bd59 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -213,7 +213,7 @@ where #[cfg(test)] pub mod tests { - use sp_consensus_beefy::{ecdsa_crypto, test_utils::BeefySignerAuthority, test_utils::GenericKeyring, test_utils::Keyring}; + use sp_consensus_beefy::{ecdsa_crypto, test_utils::{BeefySignerAuthority, Keyring, generate_in_store}}; use sp_core::Pair as PairT; use sp_keystore::testing::MemoryKeystore; @@ -233,17 +233,17 @@ pub mod tests { ::Pair: BeefySignerAuthority, { let msg = b"I am Alice!"; - let sig = >::sign(Keyring::Alice, b"I am Alice!"); + let sig = Keyring::::Alice.sign(b"I am Alice!"); assert!(>::verify( - &>::public(Keyring::Alice), + &Keyring::Alice.public(), &sig, &msg.as_slice(), )); // different public key -> fail assert!(!>::verify( - &>::public(Keyring::Bob), + &Keyring::Bob.public(), &sig, &msg.as_slice(), )); @@ -252,7 +252,7 @@ pub mod tests { // different msg -> fail assert!(!>::verify( - &>::public(Keyring::Alice), + &Keyring::Alice.public(), &sig, &msg.as_slice(), )); @@ -277,53 +277,53 @@ pub mod tests { Send + Sync + From<<::Pair as AppCrypto>::Signature>, ::Pair: BeefySignerAuthority, { - let want = >::KeyPair::from_string("//Alice", None) + let want = ::Pair::from_string("//Alice", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Alice).to_raw_vec(); + let got = Keyring::::Alice.pair().to_raw_vec(); assert_eq!(want, got); - let want = >::KeyPair::from_string("//Bob", None) + let want = ::Pair::from_string("//Bob", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Bob).to_raw_vec(); + let got = Keyring::::Bob.pair().to_raw_vec(); assert_eq!(want, got); let want = - >::KeyPair::from_string("//Charlie", None) + ::Pair::from_string("//Charlie", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Charlie).to_raw_vec(); + let got = Keyring::::Charlie.pair().to_raw_vec(); assert_eq!(want, got); - let want = >::KeyPair::from_string("//Dave", None) + let want = ::Pair::from_string("//Dave", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Dave).to_raw_vec(); + let got = Keyring::::Dave.pair().to_raw_vec(); assert_eq!(want, got); - let want = >::KeyPair::from_string("//Eve", None) + let want = ::Pair::from_string("//Eve", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Eve).to_raw_vec(); + let got = Keyring::::Eve.pair().to_raw_vec(); assert_eq!(want, got); - let want = >::KeyPair::from_string("//Ferdie", None) + let want = ::Pair::from_string("//Ferdie", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Ferdie).to_raw_vec(); + let got = Keyring::::Ferdie.pair().to_raw_vec(); assert_eq!(want, got); - let want = >::KeyPair::from_string("//One", None) + let want = ::Pair::from_string("//One", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::One).to_raw_vec(); + let got = Keyring::::One.pair().to_raw_vec(); assert_eq!(want, got); - let want = >::KeyPair::from_string("//Two", None) + let want = ::Pair::from_string("//Two", None) .expect("Pair failed") .to_raw_vec(); - let got = >::pair(Keyring::Two).to_raw_vec(); + let got = Keyring::::Two.pair().to_raw_vec(); assert_eq!(want, got); } @@ -348,16 +348,16 @@ pub mod tests { { let store = keystore(); - >::generate_in_store( + generate_in_store::( store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Alice), ); - let alice = >::public(Keyring::Alice); + let alice = Keyring::::Alice.public(); - let bob = >::public(Keyring::Bob); - let charlie = >::public(Keyring::Charlie); + let bob = Keyring::Bob.public(); + let charlie = Keyring::Charlie.public(); let beefy_store: BeefyKeystore = Some(store).into(); @@ -396,20 +396,20 @@ pub mod tests { { let store = keystore(); - >::generate_in_store( + generate_in_store::( store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Alice), ); - let alice = >::public(Keyring::Alice); + let alice = Keyring::Alice.public(); let store: BeefyKeystore = Some(store).into(); let msg = b"are you involved or commited?"; let sig1 = store.sign(&alice, msg).unwrap(); - let sig2 = >::sign(Keyring::Alice, msg); + let sig2 = Keyring::::Alice.sign( msg); assert_eq!(sig1, sig2); } @@ -436,7 +436,7 @@ pub mod tests { { let store = keystore(); - >::generate_in_store( + generate_in_store::( store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Bob), @@ -444,7 +444,7 @@ pub mod tests { let store: BeefyKeystore = Some(store).into(); - let alice = >::public(Keyring::Alice); + let alice = Keyring::Alice.public(); let msg = b"are you involved or commited?"; let sig = store.sign(&alice, msg).err().unwrap(); @@ -486,7 +486,7 @@ pub mod tests { { let store = keystore(); - >::generate_in_store( + generate_in_store::( store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Alice), @@ -494,7 +494,7 @@ pub mod tests { let store: BeefyKeystore = Some(store).into(); - let alice = >::public(Keyring::Alice); + let alice = Keyring::Alice.public(); // `msg` and `sig` match let msg = b"are you involved or commited?"; @@ -533,46 +533,46 @@ pub mod tests { let store = keystore(); // test keys - let _ = >::generate_in_store( + let _ = generate_in_store::( store.clone(), TEST_TYPE, Some(Keyring::Alice), ); - let _ = >::generate_in_store( + let _ = generate_in_store::( store.clone(), TEST_TYPE, Some(Keyring::Bob), ); // BEEFY keys - let _ = >::generate_in_store( + let _ = generate_in_store::( store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Dave), ); - let _ = >::generate_in_store( + let _ = generate_in_store::( store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Eve), ); - let _ = >::generate_in_store( + let _ = generate_in_store::( store.clone(), TEST_TYPE, None, ); - let _ = >::generate_in_store( + let _ = generate_in_store::( store.clone(), TEST_TYPE, None, ); - let key1 = >::generate_in_store( + let key1 = generate_in_store::( store.clone(), BEEFY_KEY_TYPE, None, ); - let key2 = >::generate_in_store( + let key2 = generate_in_store::( store.clone(), BEEFY_KEY_TYPE, None, @@ -583,8 +583,8 @@ pub mod tests { let keys = store.public_keys().ok().unwrap(); assert!(keys.len() == 4); - assert!(keys.contains(&>::public(Keyring::Dave))); - assert!(keys.contains(&>::public(Keyring::Eve))); + assert!(keys.contains(&Keyring::Dave.public())); + assert!(keys.contains(&Keyring::Eve.public())); assert!(keys.contains(&key1)); assert!(keys.contains(&key2)); } diff --git a/substrate/client/consensus/beefy/src/round.rs b/substrate/client/consensus/beefy/src/round.rs index 29f5f10a4444..9ff9bff3cbb8 100644 --- a/substrate/client/consensus/beefy/src/round.rs +++ b/substrate/client/consensus/beefy/src/round.rs @@ -203,7 +203,7 @@ mod tests { use sc_network_test::Block; use sp_consensus_beefy::{ - known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, test_utils::GenericKeyring, test_utils::Keyring, + known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, test_utils::Keyring, Payload, SignedCommitment, ValidatorSet, VoteMessage, }; @@ -223,8 +223,8 @@ mod tests { fn round_tracker() { let mut rt = RoundTracker::default(); let bob_vote = ( - >::public(Keyring::Bob), - >::sign(Keyring::Bob, b"I am committed"), + Keyring::Bob.public(), + Keyring::::Bob.sign( b"I am committed"), ); let threshold = 2; @@ -237,8 +237,8 @@ mod tests { assert!(!rt.is_done(threshold)); let alice_vote = ( - >::public(Keyring::Alice), - >::sign(Keyring::Alice, b"I am committed"), + Keyring::Alice.public(), + Keyring::::Alice.sign( b"I am committed"), ); // adding new vote (self vote this time) allowed assert!(rt.add_vote(alice_vote)); @@ -263,9 +263,9 @@ mod tests { let validators = ValidatorSet::::new( vec![ - >::public(Keyring::Alice), - >::public(Keyring::Bob), - >::public(Keyring::Charlie), + Keyring::Alice.public(), + Keyring::Bob.public(), + Keyring::Charlie.public(), ], 42, ) @@ -278,9 +278,9 @@ mod tests { assert_eq!(1, rounds.session_start()); assert_eq!( &vec![ - >::public(Keyring::Alice), - >::public(Keyring::Bob), - >::public(Keyring::Charlie) + Keyring::::Alice.public(), + Keyring::::Bob.public(), + Keyring::::Charlie.public() ], rounds.validators() ); @@ -292,9 +292,9 @@ mod tests { let validators = ValidatorSet::::new( vec![ - >::public(Keyring::Alice), - >::public(Keyring::Bob), - >::public(Keyring::Charlie), + Keyring::Alice.public(), + Keyring::Bob.public(), + Keyring::Charlie.public(), Keyring::Eve.public(), ], Default::default(), @@ -309,10 +309,9 @@ mod tests { let block_number = 1; let commitment = Commitment { block_number, payload, validator_set_id }; let mut vote = VoteMessage { - id: >::public(Keyring::Alice), + id: Keyring::Alice.public(), commitment: commitment.clone(), - signature: >::sign( - Keyring::Alice, + signature: Keyring::::Alice.sign( b"I am committed", ), }; @@ -322,37 +321,34 @@ mod tests { // double voting (same vote), ok, no effect assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); - vote.id = >::public(Keyring::Dave); + vote.id = Keyring::Dave.public(); vote.signature = - >::sign(Keyring::Dave, b"I am committed"); + Keyring::::Dave.sign( b"I am committed"); // invalid vote (Dave is not a validator) assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Invalid); - vote.id = >::public(Keyring::Bob); + vote.id = Keyring::Bob.public(); vote.signature = - >::sign(Keyring::Bob, b"I am committed"); + Keyring::::Bob.sign( b"I am committed"); // add 2nd good vote assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); - vote.id = >::public(Keyring::Charlie); + vote.id = Keyring::Charlie.public(); vote.signature = - >::sign(Keyring::Charlie, b"I am committed"); + Keyring::::Charlie.sign(b"I am committed"); // add 3rd good vote -> round concluded -> signatures present assert_eq!( rounds.add_vote(vote.clone()), VoteImportResult::RoundConcluded(SignedCommitment { commitment, signatures: vec![ - Some(>::sign( - Keyring::Alice, + Some(Keyring::::Alice.sign( b"I am committed" )), - Some(>::sign( - Keyring::Bob, + Some(Keyring::::Bob.sign( b"I am committed" )), - Some(>::sign( - Keyring::Charlie, + Some(Keyring::::Charlie.sign( b"I am committed" )), None, @@ -361,9 +357,9 @@ mod tests { ); rounds.conclude(block_number); - vote.id = >::public(Keyring::Eve); + vote.id = Keyring::Eve.public(); vote.signature = - >::sign(Keyring::Eve, b"I am committed"); + Keyring::::Eve.sign(b"I am committed"); // Eve is a validator, but round was concluded, adding vote disallowed assert_eq!(rounds.add_vote(vote), VoteImportResult::Stale); } @@ -374,9 +370,9 @@ mod tests { let validators = ValidatorSet::::new( vec![ - >::public(Keyring::Alice), - >::public(Keyring::Bob), - >::public(Keyring::Charlie), + Keyring::Alice.public(), + Keyring::Bob.public(), + Keyring::Charlie.public(), ], 42, ) @@ -392,10 +388,9 @@ mod tests { let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); let commitment = Commitment { block_number, payload, validator_set_id }; let mut vote = VoteMessage { - id: >::public(Keyring::Alice), + id: Keyring::Alice.public(), commitment, - signature: >::sign( - Keyring::Alice, + signature: Keyring::::Alice.sign( b"I am committed", ), }; @@ -427,9 +422,9 @@ mod tests { let validators = ValidatorSet::::new( vec![ - >::public(Keyring::Alice), - >::public(Keyring::Bob), - >::public(Keyring::Charlie), + Keyring::Alice.public(), + Keyring::Bob.public(), + Keyring::Charlie.public(), ], Default::default(), ) @@ -442,34 +437,30 @@ mod tests { let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); let commitment = Commitment { block_number: 1, payload, validator_set_id }; let mut alice_vote = VoteMessage { - id: >::public(Keyring::Alice), + id: Keyring::Alice.public(), commitment: commitment.clone(), - signature: >::sign( - Keyring::Alice, + signature: Keyring::::Alice.sign( b"I am committed", ), }; let mut bob_vote = VoteMessage { - id: >::public(Keyring::Bob), + id: Keyring::Bob.public(), commitment: commitment.clone(), - signature: >::sign( - Keyring::Bob, + signature: Keyring::::Bob.sign( b"I am committed", ), }; let mut charlie_vote = VoteMessage { - id: >::public(Keyring::Charlie), + id: Keyring::Charlie.public(), commitment, - signature: >::sign( - Keyring::Charlie, + signature: Keyring::::Charlie.sign( b"I am committed", ), }; let expected_signatures = vec![ - Some(>::sign(Keyring::Alice, b"I am committed")), - Some(>::sign(Keyring::Bob, b"I am committed")), - Some(>::sign( - Keyring::Charlie, + Some(Keyring::::Alice.sign( b"I am committed")), + Some(Keyring::::Bob.sign( b"I am committed")), + Some(Keyring::::Charlie.sign( b"I am committed", )), ]; @@ -518,8 +509,8 @@ mod tests { let validators = ValidatorSet::::new( vec![ - >::public(Keyring::Alice), - >::public(Keyring::Bob), + Keyring::Alice.public(), + Keyring::Bob.public(), ], Default::default(), ) @@ -534,10 +525,9 @@ mod tests { let commitment2 = Commitment { block_number: 1, payload: payload2, validator_set_id }; let alice_vote1 = VoteMessage { - id: >::public(Keyring::Alice), + id: Keyring::Alice.public(), commitment: commitment1, - signature: >::sign( - Keyring::Alice, + signature: Keyring::::Alice.sign( b"I am committed", ), }; diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 85b5de238de8..735e261340a4 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -55,7 +55,7 @@ use sp_consensus_beefy::{ ecdsa_crypto::{AuthorityId, Signature}, known_payloads, mmr::{find_mmr_root_digest, MmrRootProvider}, - BeefyApi, Commitment, ConsensusLog, EquivocationProof, test_utils::GenericKeyring, test_utils::Keyring as BeefyKeyring, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, test_utils::Keyring as BeefyKeyring, MmrRootHash, OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; @@ -347,18 +347,18 @@ fn add_auth_change_digest(builder: &mut impl BlockBuilderExt, new_auth_set: Beef .unwrap(); } -pub(crate) fn make_beefy_ids(keys: &[BeefyKeyring]) -> Vec { +pub(crate) fn make_beefy_ids(keys: &[BeefyKeyring]) -> Vec { keys.iter() - .map(|&key| >::public(key).into()) + .map(|key| key.public().into()) .collect() } -pub(crate) fn create_beefy_keystore(authority: BeefyKeyring) -> KeystorePtr { +pub(crate) fn create_beefy_keystore(authority: &BeefyKeyring) -> KeystorePtr { let keystore = MemoryKeystore::new(); keystore .ecdsa_generate_new( BEEFY_KEY_TYPE, - Some(&>::to_seed(authority)), + Some(&authority.to_seed()), ) .expect("Creates authority key"); keystore.into() @@ -389,7 +389,7 @@ async fn voter_init_setup( // Spawns beefy voters. Returns a future to spawn on the runtime. fn initialize_beefy( net: &mut BeefyTestNet, - peers: Vec<(usize, &BeefyKeyring, Arc)>, + peers: Vec<(usize, &BeefyKeyring, Arc)>, min_block_delta: u32, ) -> impl Future where @@ -409,7 +409,7 @@ where for (peer_id, key, api) in peers.into_iter() { let peer = &net.peers[peer_id]; - let keystore = create_beefy_keystore(*key); + let keystore = create_beefy_keystore(key); let (_, _, peer_data) = net.make_block_import(peer.client().clone()); let PeerData { beefy_rpc_links, beefy_voter_links, .. } = peer_data; @@ -467,7 +467,7 @@ async fn run_for(duration: Duration, net: &Arc>) { pub(crate) fn get_beefy_streams( net: &mut BeefyTestNet, // peer index and key - peers: impl Iterator, + peers: impl Iterator)>, ) -> (Vec>, Vec>>) { let mut best_block_streams = Vec::new(); @@ -565,7 +565,7 @@ async fn streams_empty_after_timeout( async fn finalize_block_and_wait_for_beefy( net: &Arc>, // peer index and key - peers: impl Iterator + Clone, + peers: impl Iterator)> + Clone, finalize_target: &H256, expected_beefy: &[u64], ) { diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 49f888cf011d..c6cc51f9914f 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -1077,7 +1077,7 @@ pub(crate) mod tests { use sp_blockchain::Backend as BlockchainBackendT; use sp_consensus_beefy::{ test_utils::generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, - mmr::MmrRootProvider, test_utils::GenericKeyring, test_utils::Keyring, Payload, SignedCommitment, + mmr::MmrRootProvider, test_utils::Keyring, Payload, SignedCommitment, }; use sp_runtime::traits::{Header as HeaderT, One}; use substrate_test_runtime_client::{ @@ -1111,7 +1111,7 @@ pub(crate) mod tests { fn create_beefy_worker( peer: &mut BeefyPeer, - key: &Keyring, + key: &Keyring, min_block_delta: u32, genesis_validator_set: ValidatorSet, ) -> BeefyWorker< @@ -1121,7 +1121,7 @@ pub(crate) mod tests { TestApi, Arc>, > { - let keystore = create_beefy_keystore(*key); + let keystore = create_beefy_keystore(key); let (to_rpc_justif_sender, from_voter_justif_stream) = BeefyVersionedFinalityProofStream::::channel(); @@ -1659,7 +1659,7 @@ pub(crate) mod tests { // now let's try with a bad proof let mut bad_proof = good_proof.clone(); - bad_proof.first.id = >::public(Keyring::Charlie); + bad_proof.first.id = Keyring::Charlie.public(); // bad proofs are simply ignored assert_eq!(worker.report_equivocation(bad_proof), Ok(())); // verify nothing reported to runtime diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index d70a4d1dbcab..802390b4d685 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -86,7 +86,7 @@ where /// Generate key pair in the given store using the provided seed -fn generate_in_store( +pub fn generate_in_store( store: KeystorePtr, key_type: sp_application_crypto::KeyTypeId, owner: Option>, @@ -138,13 +138,13 @@ where Send + Sync + From<<::Pair as AppCrypto>::Signature>, { /// Sign `msg`. - fn sign(self, msg: &[u8]) -> ::Signature { - let key_pair: ::Pair = self.clone().pair(); + pub fn sign(&self, msg: &[u8]) -> ::Signature { + let key_pair: ::Pair = self.pair(); key_pair.sign_with_hasher(&msg).into() } /// Return key pair. - fn pair(self) -> ::Pair { + pub fn pair(&self) -> ::Pair { ::Pair::from_string( self.to_seed().as_str(), None, @@ -154,37 +154,37 @@ where } /// Return public key. - fn public(self) -> AuthorityId { - self.clone().pair().public().into() + pub fn public(&self) -> AuthorityId { + self.pair().public().into() } /// Return seed string. - fn to_seed(self) -> String { + pub fn to_seed(&self) -> String { format!("//{}", self) } /// Get Keyring from public key. - fn from_public(who: &AuthorityId) -> Option> { - Self::iter().find(|k| k.clone().public() == *who) + pub fn from_public(who: &AuthorityId) -> Option> { + Self::iter().find(|k| k.public() == *who) } } lazy_static::lazy_static! { static ref PRIVATE_KEYS: HashMap, ecdsa_crypto::Pair> = - Keyring::iter().map(|i| (i.clone(), i.clone().pair())).collect(); + Keyring::iter().map(|i| (i.clone(), i.pair())).collect(); static ref PUBLIC_KEYS: HashMap, ecdsa_crypto::Public> = PRIVATE_KEYS.iter().map(|(name, pair)| (name.clone(), sp_application_crypto::Pair::public(pair))).collect(); } impl From> for ecdsa_crypto::Pair { fn from(k: Keyring) -> Self { - k.clone().pair() + k.pair() } } impl From> for ecdsa::Pair { fn from(k: Keyring) -> Self { - k.clone().pair().into() + k.pair().into() } } @@ -204,12 +204,12 @@ pub fn generate_equivocation_proof( validator_set_id: ValidatorSetId, keyring: &Keyring| { let commitment = Commitment { validator_set_id, block_number, payload }; - let signature = keyring.clone().sign( + let signature = keyring.sign( &commitment.encode(), ); VoteMessage { commitment, - id: keyring.clone().public(), + id: keyring.public(), signature, } }; From dd686dcacf9143debb8f2420622e298a0b29e45d Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Fri, 26 Jan 2024 10:58:13 -0500 Subject: [PATCH 25/37] Remove Sync + Send Bound from ::Signature Because you have to specify it later explicitly anyway. Co-authored-by: Davide Galassi --- substrate/client/consensus/beefy/src/keystore.rs | 9 ++------- substrate/primitives/consensus/beefy/src/lib.rs | 5 ----- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index eb79a385bd59..327e892e9072 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -41,14 +41,9 @@ use crate::{error, LOG_TARGET}; pub(crate) struct BeefyKeystore( Option, PhantomData AuthorityId>, -) -where - ::Signature: Send + Sync; +); -impl BeefyKeystore -where - ::Signature: Send + Sync, -{ +impl BeefyKeystore { /// Check if the keystore contains a private key for one of the public keys /// contained in `keys`. A public key with a matching private key is known /// as a local authority id. diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index a5758b74afb1..3d7b7727cc8f 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -71,9 +71,6 @@ pub trait AuthorityIdBound: Codec + Debug + Clone - + Ord - + Sync - + Send + AsRef<[u8]> + ByteArray + AppPublic @@ -81,8 +78,6 @@ pub trait AuthorityIdBound: + RuntimeAppPublic + Display + BeefyAuthorityId -where - ::Signature: Send + Sync, { } From 34d80cfbcfc1ea50de16d04a7531a76dbb2c4473 Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 26 Jan 2024 11:17:15 -0500 Subject: [PATCH 26/37] do `cargo fmt` --- .../beefy/src/communication/gossip.rs | 14 ++- .../consensus/beefy/src/justification.rs | 11 +-- .../client/consensus/beefy/src/keystore.rs | 94 +++++-------------- substrate/client/consensus/beefy/src/round.rs | 94 +++++-------------- substrate/client/consensus/beefy/src/tests.rs | 16 ++-- .../client/consensus/beefy/src/worker.rs | 7 +- .../consensus/beefy/src/test_utils.rs | 93 ++++++++---------- 7 files changed, 107 insertions(+), 222 deletions(-) diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index 03c490fef745..f4ab40c1df69 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -481,7 +481,7 @@ pub(crate) mod tests { use sc_network_test::Block; use sp_application_crypto::key_types::BEEFY as BEEFY_KEY_TYPE; use sp_consensus_beefy::{ - ecdsa_crypto::Signature, known_payloads, Commitment, test_utils::Keyring, MmrRootHash, + ecdsa_crypto::Signature, known_payloads, test_utils::Keyring, Commitment, MmrRootHash, Payload, SignedCommitment, VoteMessage, }; use sp_keystore::{testing::MemoryKeystore, Keystore}; @@ -503,14 +503,12 @@ pub(crate) mod tests { } } - pub fn sign_commitment(who: &Keyring, commitment: &Commitment) -> Signature { + pub fn sign_commitment( + who: &Keyring, + commitment: &Commitment, + ) -> Signature { let store = MemoryKeystore::new(); - store - .ecdsa_generate_new( - BEEFY_KEY_TYPE, - Some(&who.to_seed()), - ) - .unwrap(); + store.ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&who.to_seed())).unwrap(); let beefy_keystore: BeefyKeystore = Some(store.into()).into(); beefy_keystore.sign(&who.public(), &commitment.encode()).unwrap() } diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index d49b8abf4cf8..7f1b9e5237c3 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -93,7 +93,7 @@ pub(crate) fn verify_with_validator_set( #[cfg(test)] pub(crate) mod tests { use sp_consensus_beefy::{ - known_payloads, Commitment, test_utils::Keyring, Payload, SignedCommitment, + known_payloads, test_utils::Keyring, Commitment, Payload, SignedCommitment, VersionedFinalityProof, }; use substrate_test_runtime_client::runtime::Block; @@ -112,10 +112,7 @@ pub(crate) mod tests { validator_set_id: validator_set.id(), }; let message = commitment.encode(); - let signatures = keys - .iter() - .map(|key| Some(key.sign(&message))) - .collect(); + let signatures = keys.iter().map(|key| Some(key.sign(&message))).collect(); VersionedFinalityProof::V1(SignedCommitment { commitment, signatures }) } @@ -178,9 +175,7 @@ pub(crate) mod tests { }; // change a signature to a different key *bad_signed_commitment.signatures.first_mut().unwrap() = - Some(Keyring::::Dave.sign( - &bad_signed_commitment.commitment.encode(), - )); + Some(Keyring::::Dave.sign(&bad_signed_commitment.commitment.encode())); match verify_with_validator_set::(block_num, &validator_set, &bad_proof.into()) { Err((ConsensusError::InvalidJustification, 3)) => (), e => assert!(false, "Got unexpected {:?}", e), diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index eb79a385bd59..34695d438694 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -34,7 +34,6 @@ use sp_consensus_beefy::ecdsa_bls_crypto; use crate::{error, LOG_TARGET}; - /// A BEEFY specific keystore implemented as a `Newtype`. This is basically a /// wrapper around [`sp_keystore::Keystore`] and allows to customize /// common cryptographic functionality. @@ -213,7 +212,10 @@ where #[cfg(test)] pub mod tests { - use sp_consensus_beefy::{ecdsa_crypto, test_utils::{BeefySignerAuthority, Keyring, generate_in_store}}; + use sp_consensus_beefy::{ + ecdsa_crypto, + test_utils::{generate_in_store, BeefySignerAuthority, Keyring}, + }; use sp_core::Pair as PairT; use sp_keystore::testing::MemoryKeystore; @@ -289,10 +291,9 @@ pub mod tests { let got = Keyring::::Bob.pair().to_raw_vec(); assert_eq!(want, got); - let want = - ::Pair::from_string("//Charlie", None) - .expect("Pair failed") - .to_raw_vec(); + let want = ::Pair::from_string("//Charlie", None) + .expect("Pair failed") + .to_raw_vec(); let got = Keyring::::Charlie.pair().to_raw_vec(); assert_eq!(want, got); @@ -305,7 +306,7 @@ pub mod tests { let want = ::Pair::from_string("//Eve", None) .expect("Pair failed") .to_raw_vec(); - let got = Keyring::::Eve.pair().to_raw_vec(); + let got = Keyring::::Eve.pair().to_raw_vec(); assert_eq!(want, got); let want = ::Pair::from_string("//Ferdie", None) @@ -348,11 +349,7 @@ pub mod tests { { let store = keystore(); - generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - Some(Keyring::Alice), - ); + generate_in_store::(store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Alice)); let alice = Keyring::::Alice.public(); @@ -396,11 +393,7 @@ pub mod tests { { let store = keystore(); - generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - Some(Keyring::Alice), - ); + generate_in_store::(store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Alice)); let alice = Keyring::Alice.public(); @@ -409,7 +402,7 @@ pub mod tests { let msg = b"are you involved or commited?"; let sig1 = store.sign(&alice, msg).unwrap(); - let sig2 = Keyring::::Alice.sign( msg); + let sig2 = Keyring::::Alice.sign(msg); assert_eq!(sig1, sig2); } @@ -436,11 +429,7 @@ pub mod tests { { let store = keystore(); - generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - Some(Keyring::Bob), - ); + generate_in_store::(store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Bob)); let store: BeefyKeystore = Some(store).into(); @@ -486,11 +475,7 @@ pub mod tests { { let store = keystore(); - generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - Some(Keyring::Alice), - ); + generate_in_store::(store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Alice)); let store: BeefyKeystore = Some(store).into(); @@ -533,50 +518,19 @@ pub mod tests { let store = keystore(); // test keys - let _ = generate_in_store::( - store.clone(), - TEST_TYPE, - Some(Keyring::Alice), - ); - let _ = generate_in_store::( - store.clone(), - TEST_TYPE, - Some(Keyring::Bob), - ); + let _ = generate_in_store::(store.clone(), TEST_TYPE, Some(Keyring::Alice)); + let _ = generate_in_store::(store.clone(), TEST_TYPE, Some(Keyring::Bob)); // BEEFY keys - let _ = generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - Some(Keyring::Dave), - ); - let _ = generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - Some(Keyring::Eve), - ); - - let _ = generate_in_store::( - store.clone(), - TEST_TYPE, - None, - ); - let _ = generate_in_store::( - store.clone(), - TEST_TYPE, - None, - ); - - let key1 = generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - None, - ); - let key2 = generate_in_store::( - store.clone(), - BEEFY_KEY_TYPE, - None, - ); + let _ = + generate_in_store::(store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Dave)); + let _ = generate_in_store::(store.clone(), BEEFY_KEY_TYPE, Some(Keyring::Eve)); + + let _ = generate_in_store::(store.clone(), TEST_TYPE, None); + let _ = generate_in_store::(store.clone(), TEST_TYPE, None); + + let key1 = generate_in_store::(store.clone(), BEEFY_KEY_TYPE, None); + let key2 = generate_in_store::(store.clone(), BEEFY_KEY_TYPE, None); let store: BeefyKeystore = Some(store).into(); diff --git a/substrate/client/consensus/beefy/src/round.rs b/substrate/client/consensus/beefy/src/round.rs index 9ff9bff3cbb8..5ea8889459ec 100644 --- a/substrate/client/consensus/beefy/src/round.rs +++ b/substrate/client/consensus/beefy/src/round.rs @@ -203,8 +203,8 @@ mod tests { use sc_network_test::Block; use sp_consensus_beefy::{ - known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, test_utils::Keyring, - Payload, SignedCommitment, ValidatorSet, VoteMessage, + known_payloads::MMR_ROOT_ID, test_utils::Keyring, Commitment, EquivocationProof, Payload, + SignedCommitment, ValidatorSet, VoteMessage, }; use super::{threshold, AuthorityId, Block as BlockT, RoundTracker, Rounds}; @@ -222,10 +222,7 @@ mod tests { #[test] fn round_tracker() { let mut rt = RoundTracker::default(); - let bob_vote = ( - Keyring::Bob.public(), - Keyring::::Bob.sign( b"I am committed"), - ); + let bob_vote = (Keyring::Bob.public(), Keyring::::Bob.sign(b"I am committed")); let threshold = 2; // adding new vote allowed @@ -236,10 +233,8 @@ mod tests { // vote is not done assert!(!rt.is_done(threshold)); - let alice_vote = ( - Keyring::Alice.public(), - Keyring::::Alice.sign( b"I am committed"), - ); + let alice_vote = + (Keyring::Alice.public(), Keyring::::Alice.sign(b"I am committed")); // adding new vote (self vote this time) allowed assert!(rt.add_vote(alice_vote)); @@ -262,11 +257,7 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![ - Keyring::Alice.public(), - Keyring::Bob.public(), - Keyring::Charlie.public(), - ], + vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], 42, ) .unwrap(); @@ -311,9 +302,7 @@ mod tests { let mut vote = VoteMessage { id: Keyring::Alice.public(), commitment: commitment.clone(), - signature: Keyring::::Alice.sign( - b"I am committed", - ), + signature: Keyring::::Alice.sign(b"I am committed"), }; // add 1st good vote assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); @@ -322,35 +311,26 @@ mod tests { assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); vote.id = Keyring::Dave.public(); - vote.signature = - Keyring::::Dave.sign( b"I am committed"); + vote.signature = Keyring::::Dave.sign(b"I am committed"); // invalid vote (Dave is not a validator) assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Invalid); vote.id = Keyring::Bob.public(); - vote.signature = - Keyring::::Bob.sign( b"I am committed"); + vote.signature = Keyring::::Bob.sign(b"I am committed"); // add 2nd good vote assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); vote.id = Keyring::Charlie.public(); - vote.signature = - Keyring::::Charlie.sign(b"I am committed"); + vote.signature = Keyring::::Charlie.sign(b"I am committed"); // add 3rd good vote -> round concluded -> signatures present assert_eq!( rounds.add_vote(vote.clone()), VoteImportResult::RoundConcluded(SignedCommitment { commitment, signatures: vec![ - Some(Keyring::::Alice.sign( - b"I am committed" - )), - Some(Keyring::::Bob.sign( - b"I am committed" - )), - Some(Keyring::::Charlie.sign( - b"I am committed" - )), + Some(Keyring::::Alice.sign(b"I am committed")), + Some(Keyring::::Bob.sign(b"I am committed")), + Some(Keyring::::Charlie.sign(b"I am committed")), None, ] }) @@ -358,8 +338,7 @@ mod tests { rounds.conclude(block_number); vote.id = Keyring::Eve.public(); - vote.signature = - Keyring::::Eve.sign(b"I am committed"); + vote.signature = Keyring::::Eve.sign(b"I am committed"); // Eve is a validator, but round was concluded, adding vote disallowed assert_eq!(rounds.add_vote(vote), VoteImportResult::Stale); } @@ -369,11 +348,7 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![ - Keyring::Alice.public(), - Keyring::Bob.public(), - Keyring::Charlie.public(), - ], + vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], 42, ) .unwrap(); @@ -390,9 +365,7 @@ mod tests { let mut vote = VoteMessage { id: Keyring::Alice.public(), commitment, - signature: Keyring::::Alice.sign( - b"I am committed", - ), + signature: Keyring::::Alice.sign(b"I am committed"), }; // add vote for previous session, should fail assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Stale); @@ -421,11 +394,7 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![ - Keyring::Alice.public(), - Keyring::Bob.public(), - Keyring::Charlie.public(), - ], + vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], Default::default(), ) .unwrap(); @@ -439,30 +408,22 @@ mod tests { let mut alice_vote = VoteMessage { id: Keyring::Alice.public(), commitment: commitment.clone(), - signature: Keyring::::Alice.sign( - b"I am committed", - ), + signature: Keyring::::Alice.sign(b"I am committed"), }; let mut bob_vote = VoteMessage { id: Keyring::Bob.public(), commitment: commitment.clone(), - signature: Keyring::::Bob.sign( - b"I am committed", - ), + signature: Keyring::::Bob.sign(b"I am committed"), }; let mut charlie_vote = VoteMessage { id: Keyring::Charlie.public(), commitment, - signature: Keyring::::Charlie.sign( - b"I am committed", - ), + signature: Keyring::::Charlie.sign(b"I am committed"), }; let expected_signatures = vec![ - Some(Keyring::::Alice.sign( b"I am committed")), - Some(Keyring::::Bob.sign( b"I am committed")), - Some(Keyring::::Charlie.sign( - b"I am committed", - )), + Some(Keyring::::Alice.sign(b"I am committed")), + Some(Keyring::::Bob.sign(b"I am committed")), + Some(Keyring::::Charlie.sign(b"I am committed")), ]; // round 1 - only 2 out of 3 vote @@ -508,10 +469,7 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![ - Keyring::Alice.public(), - Keyring::Bob.public(), - ], + vec![Keyring::Alice.public(), Keyring::Bob.public()], Default::default(), ) .unwrap(); @@ -527,9 +485,7 @@ mod tests { let alice_vote1 = VoteMessage { id: Keyring::Alice.public(), commitment: commitment1, - signature: Keyring::::Alice.sign( - b"I am committed", - ), + signature: Keyring::::Alice.sign(b"I am committed"), }; let mut alice_vote2 = alice_vote1.clone(); alice_vote2.commitment = commitment2; diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 735e261340a4..1d2e068e8280 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -55,9 +55,10 @@ use sp_consensus_beefy::{ ecdsa_crypto::{AuthorityId, Signature}, known_payloads, mmr::{find_mmr_root_digest, MmrRootProvider}, - BeefyApi, Commitment, ConsensusLog, EquivocationProof, test_utils::Keyring as BeefyKeyring, - MmrRootHash, OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, ValidatorSetId, - VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, + test_utils::Keyring as BeefyKeyring, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, MmrRootHash, OpaqueKeyOwnershipProof, + Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, VoteMessage, + BEEFY_ENGINE_ID, }; use sp_core::H256; use sp_keystore::{testing::MemoryKeystore, Keystore, KeystorePtr}; @@ -348,18 +349,13 @@ fn add_auth_change_digest(builder: &mut impl BlockBuilderExt, new_auth_set: Beef } pub(crate) fn make_beefy_ids(keys: &[BeefyKeyring]) -> Vec { - keys.iter() - .map(|key| key.public().into()) - .collect() + keys.iter().map(|key| key.public().into()).collect() } pub(crate) fn create_beefy_keystore(authority: &BeefyKeyring) -> KeystorePtr { let keystore = MemoryKeystore::new(); keystore - .ecdsa_generate_new( - BEEFY_KEY_TYPE, - Some(&authority.to_seed()), - ) + .ecdsa_generate_new(BEEFY_KEY_TYPE, Some(&authority.to_seed())) .expect("Creates authority key"); keystore.into() } diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index c6cc51f9914f..9fe1921ab057 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -1076,8 +1076,11 @@ pub(crate) mod tests { use sc_network_test::TestNetFactory; use sp_blockchain::Backend as BlockchainBackendT; use sp_consensus_beefy::{ - test_utils::generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, - mmr::MmrRootProvider, test_utils::Keyring, Payload, SignedCommitment, + known_payloads, + known_payloads::MMR_ROOT_ID, + mmr::MmrRootProvider, + test_utils::{generate_equivocation_proof, Keyring}, + Payload, SignedCommitment, }; use sp_runtime::traits::{Header as HeaderT, One}; use substrate_test_runtime_client::{ diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 802390b4d685..ab1baa45c728 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -32,10 +32,8 @@ use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; use sp_core::ecdsa_bls377; use sp_core::{ecdsa, Pair}; use sp_keystore::{Keystore, KeystorePtr}; -use std::collections::HashMap; +use std::{collections::HashMap, marker::PhantomData}; use strum::IntoEnumIterator; -use std::marker::PhantomData; - /// Set of test accounts using [`crate::ecdsa_crypto`] types. #[allow(missing_docs)] @@ -48,8 +46,8 @@ pub enum Keyring { Eve, Ferdie, One, - Two, - _Marker(PhantomData) + Two, + _Marker(PhantomData), } /// Trait representing BEEFY specific generation and signing behavior of authority id @@ -82,51 +80,45 @@ where } } - - - /// Generate key pair in the given store using the provided seed pub fn generate_in_store( - store: KeystorePtr, - key_type: sp_application_crypto::KeyTypeId, - owner: Option>, - ) -> AuthorityId where + store: KeystorePtr, + key_type: sp_application_crypto::KeyTypeId, + owner: Option>, +) -> AuthorityId +where AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, ::Pair: BeefySignerAuthority, ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, { - let optional_seed: Option = owner - .map(|owner| owner.to_seed()); + let optional_seed: Option = owner.map(|owner| owner.to_seed()); - match ::CRYPTO_ID { - ecdsa::CRYPTO_ID => AuthorityId::decode( - &mut Keystore::ecdsa_generate_new(&*store, key_type, optional_seed.as_deref()) - .ok() - .unwrap() - .as_ref(), - ) - .unwrap(), - - #[cfg(feature = "bls-experimental")] - ecdsa_bls377::CRYPTO_ID => { - let pk = Keystore::ecdsa_bls377_generate_new( - &*store, - key_type, - optional_seed.as_deref(), - ) + match ::CRYPTO_ID { + ecdsa::CRYPTO_ID => AuthorityId::decode( + &mut Keystore::ecdsa_generate_new(&*store, key_type, optional_seed.as_deref()) .ok() - .unwrap(); - let decoded_pk = AuthorityId::decode(&mut pk.as_ref()).unwrap(); - println!( - "Seed: {:?}, before decode: {:?}, after decode: {:?}", - optional_seed, pk, decoded_pk - ); - decoded_pk - }, - - _ => panic!("Requested CRYPTO_ID is not supported by the BEEFY Keyring"), - } + .unwrap() + .as_ref(), + ) + .unwrap(), + + #[cfg(feature = "bls-experimental")] + ecdsa_bls377::CRYPTO_ID => { + let pk = + Keystore::ecdsa_bls377_generate_new(&*store, key_type, optional_seed.as_deref()) + .ok() + .unwrap(); + let decoded_pk = AuthorityId::decode(&mut pk.as_ref()).unwrap(); + println!( + "Seed: {:?}, before decode: {:?}, after decode: {:?}", + optional_seed, pk, decoded_pk + ); + decoded_pk + }, + + _ => panic!("Requested CRYPTO_ID is not supported by the BEEFY Keyring"), + } } /// Implement Keyring functionalities generically over AuthorityId @@ -145,12 +137,9 @@ where /// Return key pair. pub fn pair(&self) -> ::Pair { - ::Pair::from_string( - self.to_seed().as_str(), - None, - ) - .unwrap() - .into() + ::Pair::from_string(self.to_seed().as_str(), None) + .unwrap() + .into() } /// Return public key. @@ -204,14 +193,8 @@ pub fn generate_equivocation_proof( validator_set_id: ValidatorSetId, keyring: &Keyring| { let commitment = Commitment { validator_set_id, block_number, payload }; - let signature = keyring.sign( - &commitment.encode(), - ); - VoteMessage { - commitment, - id: keyring.public(), - signature, - } + let signature = keyring.sign(&commitment.encode()); + VoteMessage { commitment, id: keyring.public(), signature } }; let first = signed_vote(vote1.0, vote1.1, vote1.2, vote1.3); let second = signed_vote(vote2.0, vote2.1, vote2.2, vote2.3); From 0885c2a341387c79572ef09c8e43c901d535ab2e Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Mon, 29 Jan 2024 10:42:51 +0100 Subject: [PATCH 27/37] Move function closer to the test --- .../client/consensus/beefy/src/keystore.rs | 39 ++++++++++++++--- .../primitives/consensus/beefy/src/lib.rs | 3 +- .../consensus/beefy/src/test_utils.rs | 43 +------------------ 3 files changed, 37 insertions(+), 48 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index c5902459fb02..23d7e60d725e 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -209,10 +209,10 @@ where pub mod tests { use sp_consensus_beefy::{ ecdsa_crypto, - test_utils::{generate_in_store, BeefySignerAuthority, Keyring}, + test_utils::{BeefySignerAuthority, Keyring}, }; use sp_core::Pair as PairT; - use sp_keystore::testing::MemoryKeystore; + use sp_keystore::{testing::MemoryKeystore, Keystore}; use super::*; use crate::error::Error; @@ -255,6 +255,38 @@ pub mod tests { )); } + /// Generate key pair in the given store using the provided seed + fn generate_in_store( + store: KeystorePtr, + key_type: sp_application_crypto::KeyTypeId, + owner: Option>, + ) -> AuthorityId + where + AuthorityId: + AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, + ::Pair: BeefySignerAuthority, + ::Signature: + Send + Sync + From<<::Pair as AppCrypto>::Signature>, + { + let optional_seed: Option = owner.map(|owner| owner.to_seed()); + + match ::CRYPTO_ID { + ecdsa::CRYPTO_ID => { + let pk = store.ecdsa_generate_new(key_type, optional_seed.as_deref()).ok().unwrap(); + AuthorityId::decode(&mut pk.as_ref()).unwrap() + }, + #[cfg(feature = "bls-experimental")] + ecdsa_bls377::CRYPTO_ID => { + let pk = store + .ecdsa_bls377_generate_new(key_type, optional_seed.as_deref()) + .ok() + .unwrap(); + AuthorityId::decode(&mut pk.as_ref()).unwrap() + }, + _ => panic!("Requested CRYPTO_ID is not supported by the BEEFY Keyring"), + } + } + #[test] fn pair_verify_should_work_ecdsa() { pair_verify_should_work::(); @@ -364,16 +396,13 @@ pub mod tests { assert_eq!(id, alice); } - #[cfg(feature = "bls-experimental")] #[test] - fn authority_id_works_for_ecdsa() { authority_id_works::(); } #[cfg(feature = "bls-experimental")] #[test] - fn authority_id_works_for_ecdsa_n_bls() { authority_id_works::(); } diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 3d7b7727cc8f..69d0295af197 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -32,8 +32,9 @@ //! while GRANDPA uses `ed25519`. mod commitment; -pub mod mmr; mod payload; + +pub mod mmr; #[cfg(feature = "std")] pub mod test_utils; pub mod witness; diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index ab1baa45c728..0efeb686499d 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -31,7 +31,7 @@ use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; #[cfg(feature = "bls-experimental")] use sp_core::ecdsa_bls377; use sp_core::{ecdsa, Pair}; -use sp_keystore::{Keystore, KeystorePtr}; +use sp_keystore::KeystorePtr; use std::{collections::HashMap, marker::PhantomData}; use strum::IntoEnumIterator; @@ -80,47 +80,6 @@ where } } -/// Generate key pair in the given store using the provided seed -pub fn generate_in_store( - store: KeystorePtr, - key_type: sp_application_crypto::KeyTypeId, - owner: Option>, -) -> AuthorityId -where - AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, - ::Pair: BeefySignerAuthority, - ::Signature: - Send + Sync + From<<::Pair as AppCrypto>::Signature>, -{ - let optional_seed: Option = owner.map(|owner| owner.to_seed()); - - match ::CRYPTO_ID { - ecdsa::CRYPTO_ID => AuthorityId::decode( - &mut Keystore::ecdsa_generate_new(&*store, key_type, optional_seed.as_deref()) - .ok() - .unwrap() - .as_ref(), - ) - .unwrap(), - - #[cfg(feature = "bls-experimental")] - ecdsa_bls377::CRYPTO_ID => { - let pk = - Keystore::ecdsa_bls377_generate_new(&*store, key_type, optional_seed.as_deref()) - .ok() - .unwrap(); - let decoded_pk = AuthorityId::decode(&mut pk.as_ref()).unwrap(); - println!( - "Seed: {:?}, before decode: {:?}, after decode: {:?}", - optional_seed, pk, decoded_pk - ); - decoded_pk - }, - - _ => panic!("Requested CRYPTO_ID is not supported by the BEEFY Keyring"), - } -} - /// Implement Keyring functionalities generically over AuthorityId impl Keyring where From edc6a9cafd3a75753c5c45d66ea29b40f0109b55 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Mon, 29 Jan 2024 10:47:31 +0100 Subject: [PATCH 28/37] Fix some warnings --- substrate/client/consensus/beefy/src/keystore.rs | 5 ++--- substrate/primitives/consensus/beefy/src/lib.rs | 4 +++- substrate/primitives/consensus/beefy/src/test_utils.rs | 3 --- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 23d7e60d725e..c95925e9f14f 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -29,9 +29,6 @@ use log::warn; use sp_consensus_beefy::{AuthorityIdBound, BeefyAuthorityId, BeefySignatureHasher}; -#[cfg(feature = "bls-experimental")] -use sp_consensus_beefy::ecdsa_bls_crypto; - use crate::{error, LOG_TARGET}; /// A BEEFY specific keystore implemented as a `Newtype`. This is basically a @@ -207,6 +204,8 @@ where #[cfg(test)] pub mod tests { + #[cfg(feature = "bls-experimental")] + use sp_consensus_beefy::ecdsa_bls_crypto; use sp_consensus_beefy::{ ecdsa_crypto, test_utils::{BeefySignerAuthority, Keyring}, diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 69d0295af197..0f88f1432cdb 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -35,9 +35,11 @@ mod commitment; mod payload; pub mod mmr; +pub mod witness; + +/// Test utilities #[cfg(feature = "std")] pub mod test_utils; -pub mod witness; pub use commitment::{Commitment, SignedCommitment, VersionedFinalityProof}; pub use payload::{known_payloads, BeefyPayloadId, Payload, PayloadProvider}; diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 0efeb686499d..f8a2a15fe846 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -28,10 +28,7 @@ use crate::{ }; use codec::Encode; use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; -#[cfg(feature = "bls-experimental")] -use sp_core::ecdsa_bls377; use sp_core::{ecdsa, Pair}; -use sp_keystore::KeystorePtr; use std::{collections::HashMap, marker::PhantomData}; use strum::IntoEnumIterator; From cd35b5df1447cc7b470edee9ea01b3bf040ddb2b Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Mon, 29 Jan 2024 11:56:33 +0100 Subject: [PATCH 29/37] Fix std feat propagation --- substrate/primitives/consensus/beefy/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/primitives/consensus/beefy/Cargo.toml b/substrate/primitives/consensus/beefy/Cargo.toml index 1433660adaa0..118b5ee26ec3 100644 --- a/substrate/primitives/consensus/beefy/Cargo.toml +++ b/substrate/primitives/consensus/beefy/Cargo.toml @@ -47,6 +47,7 @@ std = [ "sp-io/std", "sp-mmr-primitives/std", "sp-runtime/std", + "sp-keystore/std", "sp-std/std", "strum/std", ] From 3d5cc3609031619ed020386f6c10751cc1a1922b Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Mon, 29 Jan 2024 12:07:11 +0100 Subject: [PATCH 30/37] Fix beefy pallet --- substrate/frame/beefy/src/tests.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index bf5ae19510ce..453cf19a4fe1 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -15,21 +15,21 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::vec; - use codec::Encode; -use sp_consensus_beefy::{ - check_equivocation_proof, generate_equivocation_proof, known_payloads::MMR_ROOT_ID, - Keyring as BeefyKeyring, Payload, ValidatorSet, KEY_TYPE as BEEFY_KEY_TYPE, -}; - -use sp_runtime::DigestItem; +use std::vec; use frame_support::{ assert_err, assert_ok, dispatch::{GetDispatchInfo, Pays}, traits::{Currency, KeyOwnerProofSystem, OnInitialize}, }; +use sp_consensus_beefy::{ + check_equivocation_proof, + known_payloads::MMR_ROOT_ID, + test_utils::{generate_equivocation_proof, Keyring as BeefyKeyring}, + Payload, ValidatorSet, KEY_TYPE as BEEFY_KEY_TYPE, +}; +use sp_runtime::DigestItem; use crate::{mock::*, Call, Config, Error, Weight, WeightInfo}; From 77bdcfa3401f072d943680a187370df544343155 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Mon, 29 Jan 2024 12:08:08 +0100 Subject: [PATCH 31/37] Make taplo happy --- substrate/client/consensus/beefy/Cargo.toml | 2 +- substrate/primitives/consensus/beefy/Cargo.toml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/client/consensus/beefy/Cargo.toml b/substrate/client/consensus/beefy/Cargo.toml index 2bd61c73fa50..56c38bf2e479 100644 --- a/substrate/client/consensus/beefy/Cargo.toml +++ b/substrate/client/consensus/beefy/Cargo.toml @@ -58,6 +58,6 @@ substrate-test-runtime-client = { path = "../../../test-utils/runtime/client" } # the BLS implementation and interface may still be subject to significant change. bls-experimental = [ "sp-application-crypto/bls-experimental", + "sp-consensus-beefy/bls-experimental", "sp-core/bls-experimental", - "sp-consensus-beefy/bls-experimental" ] diff --git a/substrate/primitives/consensus/beefy/Cargo.toml b/substrate/primitives/consensus/beefy/Cargo.toml index 118b5ee26ec3..1985f5e658d7 100644 --- a/substrate/primitives/consensus/beefy/Cargo.toml +++ b/substrate/primitives/consensus/beefy/Cargo.toml @@ -25,7 +25,7 @@ sp-crypto-hashing = { path = "../../crypto/hashing", default-features = false } sp-io = { path = "../../io", default-features = false } sp-mmr-primitives = { path = "../../merkle-mountain-range", default-features = false } sp-runtime = { path = "../../runtime", default-features = false } -sp-keystore = { path = "../../keystore", default-features = false} +sp-keystore = { path = "../../keystore", default-features = false } sp-std = { path = "../../std", default-features = false } strum = { version = "0.24.1", features = ["derive"], default-features = false } lazy_static = "1.4.0" @@ -45,9 +45,9 @@ std = [ "sp-core/std", "sp-crypto-hashing/std", "sp-io/std", + "sp-keystore/std", "sp-mmr-primitives/std", "sp-runtime/std", - "sp-keystore/std", "sp-std/std", "strum/std", ] From ec1ef135efd89f7576da725aee21e03c7c7cad04 Mon Sep 17 00:00:00 2001 From: Skalman Date: Tue, 30 Jan 2024 08:50:08 -0500 Subject: [PATCH 32/37] Add reference for the BLS implementation used in substrate --- substrate/primitives/core/src/bls.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index 72394b7db03d..5a6ac7f925f3 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -15,7 +15,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Simple BLS (Boneh–Lynn–Shacham) Signature API. +//! BLS (Boneh–Lynn–Shacham) Signature along with efficiently verifiable Chaum-Pedersen proof API. +//! Signatures are implemented according to +//! [Efficient Aggregatable BLS Signatures with Chaum-Pedersen Proofs](https://eprint.iacr.org/2022/1611) +//! Hash-to-BLS-curve is using Simplified SWU for AB == 0 +//! [RFC 9380](https://datatracker.ietf.org/doc/rfc9380/) Sect 6.6.3. +//! Chaum-Pedersen proof uses the same hash to field specified in RFC 9380 for the field of the BLS curve. #[cfg(feature = "serde")] use crate::crypto::Ss58Codec; From e4ad7162b52ea4cb62ef09f1cc21cc65b3d1afad Mon Sep 17 00:00:00 2001 From: Skalman Date: Tue, 30 Jan 2024 10:35:47 -0500 Subject: [PATCH 33/37] BEEFY keystore should error if asked to sign with any key beside ECDSA or BLS377 --- .../client/consensus/beefy/src/keystore.rs | 19 +------------------ substrate/primitives/core/src/bls.rs | 3 ++- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 267797c709ac..2ddc938fbc6c 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -111,24 +111,7 @@ impl BeefyKeystore { sig_ref.to_vec() }, - _ => { - let sig = store - .sign_with( - ::ID, - ::CRYPTO_ID, - public.as_slice(), - message, - ) - .map_err(|e| error::Error::Signature(format!("{}. Key: {:?}", e, public)))? - .ok_or_else(|| { - error::Error::Signature(format!( - "Could not find key in keystore. Key: {:?}", - public - )) - })?; - - sig - }, + _ => Err(error::Error::Keystore("key type is not supported by BEEFY Keystore".into()))?, }; //check that `sig` has the expected result type diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index 5a6ac7f925f3..2429a46def09 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -20,7 +20,8 @@ //! [Efficient Aggregatable BLS Signatures with Chaum-Pedersen Proofs](https://eprint.iacr.org/2022/1611) //! Hash-to-BLS-curve is using Simplified SWU for AB == 0 //! [RFC 9380](https://datatracker.ietf.org/doc/rfc9380/) Sect 6.6.3. -//! Chaum-Pedersen proof uses the same hash to field specified in RFC 9380 for the field of the BLS curve. +//! Chaum-Pedersen proof uses the same hash to field specified in RFC 9380 for the field of the BLS +//! curve. #[cfg(feature = "serde")] use crate::crypto::Ss58Codec; From 018f13a57d805ba468d4c82ddc56d5f62faa1eb0 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Thu, 1 Feb 2024 08:51:14 -0500 Subject: [PATCH 34/37] Improve `bls.rs` docs Co-authored-by: Robert Hambrock --- substrate/primitives/core/src/bls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index 2429a46def09..ab2fbae3cb1a 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -20,7 +20,7 @@ //! [Efficient Aggregatable BLS Signatures with Chaum-Pedersen Proofs](https://eprint.iacr.org/2022/1611) //! Hash-to-BLS-curve is using Simplified SWU for AB == 0 //! [RFC 9380](https://datatracker.ietf.org/doc/rfc9380/) Sect 6.6.3. -//! Chaum-Pedersen proof uses the same hash to field specified in RFC 9380 for the field of the BLS +//! Chaum-Pedersen proof uses the same hash-to-field specified in RFC 9380 for the field of the BLS //! curve. #[cfg(feature = "serde")] From 98a872175cd3e89bec3df1cb1723d76d6de7a878 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Thu, 1 Feb 2024 08:57:50 -0500 Subject: [PATCH 35/37] Test the private key constructed from phrase is the same as constructed initially. Co-authored-by: Robert Hambrock --- substrate/primitives/core/src/ecdsa.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index fce6b682dc72..bd34ef53ddcc 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -649,12 +649,15 @@ mod test { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); + assert_eq!(pair.secret, repair_seed.secret); let (repair_phrase, reseed) = Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); assert_eq!(seed, reseed); assert_eq!(pair.public(), repair_phrase.public()); + assert_eq!(pair.secret, repair_phrase.secret); let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); assert_eq!(pair.public(), repair_string.public()); + assert_eq!(pair.secret, repair_string.secret); } #[test] From 2271268cb662f8251dfc35c98344ab564fd55b3d Mon Sep 17 00:00:00 2001 From: Skalman Date: Thu, 1 Feb 2024 10:15:13 -0500 Subject: [PATCH 36/37] Verify secret (in)equality in key recovery tests. --- substrate/primitives/core/src/bls.rs | 5 +++++ substrate/primitives/core/src/ecdsa.rs | 1 + substrate/primitives/core/src/ed25519.rs | 4 ++++ substrate/primitives/core/src/paired_crypto.rs | 5 +++++ substrate/primitives/core/src/sr25519.rs | 3 +++ 5 files changed, 18 insertions(+) diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index ab2fbae3cb1a..b202c92b17a5 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -659,12 +659,16 @@ mod test { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); let (repair_phrase, reseed) = Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); assert_eq!(seed, reseed); assert_eq!(pair.public(), repair_phrase.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); + let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); assert_eq!(pair.public(), repair_string.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); } #[test] @@ -673,6 +677,7 @@ mod test { let (pair2, _) = Pair::from_phrase(&phrase, None).unwrap(); assert_ne!(pair1.public(), pair2.public()); + assert_ne!(pair1.to_raw_vec(), pair2.to_raw_vec()); } #[test] diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index bd34ef53ddcc..f172b3a7d02c 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -666,6 +666,7 @@ mod test { let (pair2, _) = Pair::from_phrase(&phrase, None).unwrap(); assert_ne!(pair1.public(), pair2.public()); + assert_ne!(pair1.secret, pair2.secret); } #[test] diff --git a/substrate/primitives/core/src/ed25519.rs b/substrate/primitives/core/src/ed25519.rs index d3fd985e04bd..60ebd93e12d4 100644 --- a/substrate/primitives/core/src/ed25519.rs +++ b/substrate/primitives/core/src/ed25519.rs @@ -506,12 +506,15 @@ mod test { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); let (repair_phrase, reseed) = Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); assert_eq!(seed, reseed); assert_eq!(pair.public(), repair_phrase.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); assert_eq!(pair.public(), repair_string.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); } #[test] @@ -603,6 +606,7 @@ mod test { let (pair2, _) = Pair::from_phrase(&phrase, None).unwrap(); assert_ne!(pair1.public(), pair2.public()); + assert_ne!(pair1.to_raw_vec(), pair2.to_raw_vec()); } #[test] diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 260d3a358ae3..7b92813306cb 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -548,12 +548,16 @@ mod test { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); + let (repair_phrase, reseed) = Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); assert_eq!(seed, reseed); assert_eq!(pair.public(), repair_phrase.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); assert_eq!(pair.public(), repair_string.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); } #[test] @@ -678,6 +682,7 @@ mod test { let (pair2, _) = Pair::from_phrase(&phrase, None).unwrap(); assert_ne!(pair1.public(), pair2.public()); + assert_ne!(pair1.to_raw_vec(), pair2.to_raw_vec()); } #[test] diff --git a/substrate/primitives/core/src/sr25519.rs b/substrate/primitives/core/src/sr25519.rs index b01387c84ece..7c02afc3cd5f 100644 --- a/substrate/primitives/core/src/sr25519.rs +++ b/substrate/primitives/core/src/sr25519.rs @@ -975,12 +975,15 @@ mod tests { let (pair, phrase, seed) = Pair::generate_with_phrase(None); let repair_seed = Pair::from_seed_slice(seed.as_ref()).expect("seed slice is valid"); assert_eq!(pair.public(), repair_seed.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); let (repair_phrase, reseed) = Pair::from_phrase(phrase.as_ref(), None).expect("seed slice is valid"); assert_eq!(seed, reseed); assert_eq!(pair.public(), repair_phrase.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); let repair_string = Pair::from_string(phrase.as_str(), None).expect("seed slice is valid"); assert_eq!(pair.public(), repair_string.public()); + assert_eq!(pair.to_raw_vec(), repair_seed.to_raw_vec()); } #[test] From 3cf2c72e27d3c315b971aae344d930d1b49a8780 Mon Sep 17 00:00:00 2001 From: Robert Hambrock Date: Thu, 1 Feb 2024 22:15:39 +0100 Subject: [PATCH 37/37] discipline rogue "hash to field"s --- substrate/primitives/core/src/bls.rs | 2 +- substrate/primitives/core/src/paired_crypto.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index b202c92b17a5..0c84d0ba8e6c 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -551,7 +551,7 @@ mod test { "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", ); let pair = Pair::from_seed(&seed); - // we are using hash to field so this is not going to work + // we are using hash-to-field so this is not going to work // assert_eq!(pair.seed(), seed); let path = vec![DeriveJunction::Hard([0u8; 32])]; let derived = pair.derive(path.into_iter(), None).ok().unwrap().0; diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 7b92813306cb..20b32c339bd7 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -566,7 +566,7 @@ mod test { "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", ); let pair = Pair::from_seed(&seed_for_right_and_left); - // we are using hash to field so this is not going to work + // we are using hash-to-field so this is not going to work // assert_eq!(pair.seed(), seed); let path = vec![DeriveJunction::Hard([0u8; 32])]; let derived = pair.derive(path.into_iter(), None).ok().unwrap().0;