diff --git a/bridges/modules/beefy/src/lib.rs b/bridges/modules/beefy/src/lib.rs index 02ecc9e03eddc..02635953625d2 100644 --- a/bridges/modules/beefy/src/lib.rs +++ b/bridges/modules/beefy/src/lib.rs @@ -58,8 +58,6 @@ pub type BridgedBlockHash = bp_runtime::HashOf>; /// Pallet initialization data. pub type InitializationDataOf = InitializationData, bp_beefy::MmrHashOf>>; -/// BEEFY commitment hasher, used by configured bridged chain. -pub type BridgedBeefyCommitmentHasher = bp_beefy::BeefyCommitmentHasher>; /// BEEFY validator id, used by configured bridged chain. pub type BridgedBeefyAuthorityId = bp_beefy::BeefyAuthorityIdOf>; /// BEEFY validator set, used by configured bridged chain. diff --git a/bridges/modules/beefy/src/mock.rs b/bridges/modules/beefy/src/mock.rs index 3b751ddf066c9..18966b3b58c4f 100644 --- a/bridges/modules/beefy/src/mock.rs +++ b/bridges/modules/beefy/src/mock.rs @@ -17,8 +17,8 @@ use crate as beefy; use crate::{ utils::get_authorities_mmr_root, BridgedBeefyAuthoritySet, BridgedBeefyAuthoritySetInfo, - BridgedBeefyCommitmentHasher, BridgedBeefyMmrLeafExtra, BridgedBeefySignedCommitment, - BridgedMmrHash, BridgedMmrHashing, BridgedMmrProof, + BridgedBeefyMmrLeafExtra, BridgedBeefySignedCommitment, BridgedMmrHash, BridgedMmrHashing, + BridgedMmrProof, }; use bp_beefy::{BeefyValidatorSignatureOf, ChainWithBeefy, Commitment, MmrDataOrHash}; @@ -33,7 +33,7 @@ use sp_runtime::{ }; pub use sp_consensus_beefy::ecdsa_crypto::{AuthorityId as BeefyId, Pair as BeefyPair}; -use sp_core::crypto::Wraps; +use sp_consensus_beefy::test_utils::BeefySignerAuthority; use sp_runtime::traits::Keccak256; pub type TestAccountId = u64; @@ -44,7 +44,6 @@ pub type TestBridgedAuthoritySetInfo = BridgedBeefyAuthoritySetInfo; pub type TestBridgedCommitment = BridgedBeefySignedCommitment; pub type TestBridgedValidatorSignature = BeefyValidatorSignatureOf; -pub type TestBridgedCommitmentHasher = BridgedBeefyCommitmentHasher; pub type TestBridgedMmrHashing = BridgedMmrHashing; pub type TestBridgedMmrHash = BridgedMmrHash; pub type TestBridgedBeefyMmrLeafExtra = BridgedBeefyMmrLeafExtra; @@ -105,7 +104,6 @@ impl Chain for TestBridgedChain { } impl ChainWithBeefy for TestBridgedChain { - type CommitmentHasher = Keccak256; type MmrHashing = Keccak256; type MmrHash = ::Output; type BeefyMmrLeafExtra = (); @@ -184,12 +182,11 @@ pub fn sign_commitment( let random_validators = rand::seq::index::sample(&mut rand::thread_rng(), total_validators, signature_count); - let commitment_hash = TestBridgedCommitmentHasher::hash(&commitment.encode()); let mut signatures = vec![None; total_validators]; for validator_idx in random_validators.iter() { let validator = &validator_pairs[validator_idx]; signatures[validator_idx] = - Some(validator.as_inner_ref().sign_prehashed(commitment_hash.as_fixed_bytes()).into()); + Some(BeefySignerAuthority::sign(validator, &commitment.encode())); } TestBridgedCommitment { commitment, signatures } diff --git a/bridges/primitives/beefy/src/lib.rs b/bridges/primitives/beefy/src/lib.rs index 84012bf8e08a8..289692a561b09 100644 --- a/bridges/primitives/beefy/src/lib.rs +++ b/bridges/primitives/beefy/src/lib.rs @@ -52,11 +52,6 @@ use sp_std::prelude::*; /// primitives. Some of types can be configured in low-level pallets, but are constrained /// when BEEFY+MMR bundle is used. pub trait ChainWithBeefy: Chain { - /// The hashing algorithm used to compute the digest of the BEEFY commitment. - /// - /// Corresponds to the hashing algorithm, used by `sc_consensus_beefy::BeefyKeystore`. - type CommitmentHasher: sp_runtime::traits::Hash; - /// The hashing algorithm used to build the MMR. /// /// The same algorithm is also used to compute merkle roots in BEEFY @@ -84,7 +79,7 @@ pub trait ChainWithBeefy: Chain { /// A way to identify a BEEFY validator. /// /// Corresponds to the `BeefyId` field of the `pallet-beefy` configuration. - type AuthorityId: BeefyAuthorityId + Parameter; + type AuthorityId: BeefyAuthorityId + Parameter; /// A way to convert validator id to its raw representation in the BEEFY merkle tree. /// @@ -105,8 +100,6 @@ pub type BeefyValidatorSignatureOf = /// Signed BEEFY commitment used by given Substrate chain. pub type BeefySignedCommitmentOf = SignedCommitment, BeefyValidatorSignatureOf>; -/// Hash algorithm, used to compute the digest of the BEEFY commitment before signing it. -pub type BeefyCommitmentHasher = ::CommitmentHasher; /// Hash algorithm used in Beefy MMR construction by given Substrate chain. pub type MmrHashingOf = ::MmrHashing; /// Hash type, used in MMR construction by given Substrate chain. diff --git a/prdoc/pr_10763.prdoc b/prdoc/pr_10763.prdoc new file mode 100644 index 0000000000000..c7e5b186dd71c --- /dev/null +++ b/prdoc/pr_10763.prdoc @@ -0,0 +1,21 @@ +title: Make some BEEFY keystore logic more generic +doc: +- audience: Node Dev + description: |- + This PR: + 1. makes some BEEFY keystore methods more generic: + - `sign()` + - `public_keys()` + This is done by implementing the specific logic in the `BeefyAuthorityId`. + 2. Removes the `BeefyAuthorityId::SignatureHasher` since for some algorithms it doesn't make sense to have a hasher. + + Also since now the `BeefyAuthorityId` implements both the signing and the verification logic, we should have better consistency. + + Related to https://github.com/paritytech/polkadot-sdk/pull/8707#discussion_r2673377834 +crates: +- name: sp-consensus-beefy + bump: major +- name: sc-consensus-beefy + bump: patch +- name: pallet-beefy + bump: patch diff --git a/substrate/client/consensus/beefy/src/fisherman.rs b/substrate/client/consensus/beefy/src/fisherman.rs index 2b2683b35f0a8..45708164fb21d 100644 --- a/substrate/client/consensus/beefy/src/fisherman.rs +++ b/substrate/client/consensus/beefy/src/fisherman.rs @@ -23,7 +23,7 @@ use sp_api::ProvideRuntimeApi; use sp_application_crypto::RuntimeAppPublic; use sp_blockchain::HeaderBackend; use sp_consensus_beefy::{ - check_double_voting_proof, AuthorityIdBound, BeefyApi, BeefySignatureHasher, DoubleVotingProof, + check_double_voting_proof, AuthorityIdBound, BeefyApi, DoubleVotingProof, OpaqueKeyOwnershipProof, ValidatorSetId, }; use sp_runtime::{ @@ -131,7 +131,7 @@ where (active_rounds.validators(), active_rounds.validator_set_id()); let offender_id = proof.offender_id(); - if !check_double_voting_proof::<_, _, BeefySignatureHasher>(&proof) { + if !check_double_voting_proof::<_, _>(&proof) { debug!(target: LOG_TARGET, "🥩 Skipping report for bad equivocation {:?}", proof); return Ok(()); } diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index 9ff7c3cf54f68..5e9e3945a9f61 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -20,8 +20,7 @@ use codec::DecodeAll; use sp_application_crypto::RuntimeAppPublic; use sp_consensus::Error as ConsensusError; use sp_consensus_beefy::{ - AuthorityIdBound, BeefySignatureHasher, KnownSignature, ValidatorSet, ValidatorSetId, - VersionedFinalityProof, + AuthorityIdBound, KnownSignature, ValidatorSet, ValidatorSetId, VersionedFinalityProof, }; use sp_runtime::traits::{Block as BlockT, NumberFor}; @@ -61,11 +60,10 @@ pub(crate) fn verify_with_validator_set<'a, Block: BlockT, AuthorityId: Authorit > { match proof { VersionedFinalityProof::V1(signed_commitment) => { - let signatories = signed_commitment - .verify_signatures::<_, BeefySignatureHasher>(target_number, validator_set) - .map_err(|checked_signatures| { - (ConsensusError::InvalidJustification, checked_signatures) - })?; + let signatories = + signed_commitment.verify_signatures::<_>(target_number, validator_set).map_err( + |checked_signatures| (ConsensusError::InvalidJustification, checked_signatures), + )?; if signatories.len() >= crate::round::threshold(validator_set.len()) { Ok(signatories) diff --git a/substrate/client/consensus/beefy/src/keystore.rs b/substrate/client/consensus/beefy/src/keystore.rs index 888a11db89cbe..c064a034c9c9f 100644 --- a/substrate/client/consensus/beefy/src/keystore.rs +++ b/substrate/client/consensus/beefy/src/keystore.rs @@ -18,15 +18,12 @@ use codec::Decode; use log::warn; -use sp_application_crypto::{key_types::BEEFY as BEEFY_KEY_TYPE, AppCrypto, RuntimeAppPublic}; -#[cfg(feature = "bls-experimental")] -use sp_core::ecdsa_bls381; -use sp_core::{ecdsa, keccak_256}; +use sp_application_crypto::{key_types::BEEFY as BEEFY_KEY_TYPE, RuntimeAppPublic}; use sp_keystore::KeystorePtr; use std::marker::PhantomData; -use sp_consensus_beefy::{AuthorityIdBound, BeefyAuthorityId, BeefySignatureHasher}; +use sp_consensus_beefy::{AuthorityIdBound, BeefyAuthorityId}; use crate::{error, LOG_TARGET}; @@ -82,48 +79,22 @@ impl BeefyKeystore { ) -> 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 - 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_bls381::CRYPTO_ID => { - let public: ecdsa_bls381::Public = - ecdsa_bls381::Public::try_from(public.as_slice()).unwrap(); - let sig = store - .ecdsa_bls381_sign_with_keccak256(BEEFY_KEY_TYPE, &public, &message) - .map_err(|e| error::Error::Keystore(e.to_string()))? - .ok_or_else(|| error::Error::Signature("bls381_sign() failed".to_string()))?; - let sig_ref: &[u8] = sig.as_ref(); - sig_ref.to_vec() - }, - - _ => Err(error::Error::Keystore("key type is not supported by BEEFY Keystore".into()))?, - }; - - //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 - )) - })?; + let raw_signature = BeefyAuthorityId::try_sign_with_store(public, store, message) + .map_err(|e| error::Error::Keystore(e.to_string()))? + .ok_or_else(|| { + error::Error::Signature(format!( + "{}::try_sign_with_store() failed", + core::any::type_name::(), + )) + })?; + let signature = + ::Signature::decode(&mut raw_signature.as_ref()) + .map_err(|_| { + error::Error::Signature(format!( + "invalid signature {:?} for key {:?}", + raw_signature, public + )) + })?; Ok(signature) } @@ -133,34 +104,13 @@ impl BeefyKeystore { pub fn public_keys(&self) -> Result, error::Error> { let store = self.0.clone().ok_or_else(|| error::Error::Keystore("no Keystore".into()))?; - 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_bls381::CRYPTO_ID => store - .ecdsa_bls381_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 + ::get_all_public_keys_from_store(store) + .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())) + }) } /// Use the `public` key to verify that `sig` is a valid signature for `message`. @@ -171,7 +121,7 @@ impl BeefyKeystore { sig: &::Signature, message: &[u8], ) -> bool { - BeefyAuthorityId::::verify(public, sig, message) + BeefyAuthorityId::verify(public, sig, message) } } @@ -183,13 +133,16 @@ impl From> for BeefyKeystore< #[cfg(test)] pub mod tests { + use sp_application_crypto::AppCrypto; #[cfg(feature = "bls-experimental")] use sp_consensus_beefy::ecdsa_bls_crypto; use sp_consensus_beefy::{ ecdsa_crypto, test_utils::{BeefySignerAuthority, Keyring}, }; - use sp_core::Pair as PairT; + #[cfg(feature = "bls-experimental")] + use sp_core::ecdsa_bls381; + use sp_core::{ecdsa, Pair as PairT}; use sp_keystore::{testing::MemoryKeystore, Keystore}; use super::*; @@ -205,19 +158,19 @@ pub mod tests { where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { let msg = b"I am Alice!"; let sig = Keyring::::Alice.sign(b"I am Alice!"); - assert!(>::verify( + assert!(::verify( &Keyring::Alice.public(), &sig, &msg.as_slice(), )); // different public key -> fail - assert!(!>::verify( + assert!(!::verify( &Keyring::Bob.public(), &sig, &msg.as_slice(), @@ -226,7 +179,7 @@ pub mod tests { let msg = b"I am not Alice!"; // different msg -> fail - assert!(!>::verify( + assert!(!::verify( &Keyring::Alice.public(), &sig, &msg.as_slice(), @@ -242,7 +195,7 @@ pub mod tests { where AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, { @@ -282,7 +235,7 @@ pub mod tests { where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { let want = ::Pair::from_string("//Alice", None) .expect("Pair failed") @@ -350,7 +303,7 @@ pub mod tests { where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { let store = keystore(); @@ -391,7 +344,7 @@ pub mod tests { where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { let store = keystore(); @@ -427,7 +380,7 @@ pub mod tests { ) where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { let store = keystore(); @@ -446,13 +399,17 @@ pub mod tests { #[test] fn sign_error_for_ecdsa() { - sign_error::("ecdsa_sign_prehashed() failed"); + sign_error::( + "sp_consensus_beefy::ecdsa_crypto::Public::try_sign_with_store() failed", + ); } #[cfg(feature = "bls-experimental")] #[test] fn sign_error_for_ecdsa_n_bls() { - sign_error::("bls381_sign() failed"); + sign_error::( + "sp_consensus_beefy::ecdsa_bls_crypto::Public::try_sign_with_store() failed", + ); } #[test] @@ -473,7 +430,7 @@ pub mod tests { where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { let store = keystore(); @@ -512,7 +469,7 @@ pub mod tests { where ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, { const TEST_TYPE: sp_application_crypto::KeyTypeId = sp_application_crypto::KeyTypeId(*b"test"); diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index d2c843917271f..29fc59cb89619 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -70,8 +70,7 @@ pub mod pallet { /// Authority identifier type type BeefyId: Member + Parameter - // todo: use custom signature hashing type instead of hardcoded `Keccak256` - + BeefyAuthorityId + + BeefyAuthorityId + MaybeSerializeDeserialize + MaxEncodedLen; diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index 108b6b4813d7d..c002954e12223 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -223,8 +223,6 @@ pub fn test_authorities() -> Vec { #[test] fn should_sign_and_verify() { - use sp_runtime::traits::Keccak256; - let set_id = 3; let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); @@ -236,7 +234,7 @@ fn should_sign_and_verify() { (1, payload1.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _>(&equivocation_proof)); // generate an equivocation proof, with two votes in different rounds for // different payloads signed by the same key @@ -245,7 +243,7 @@ fn should_sign_and_verify() { (2, payload2.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _>(&equivocation_proof)); // generate an equivocation proof, with two votes by different authorities let equivocation_proof = generate_double_voting_proof( @@ -253,7 +251,7 @@ fn should_sign_and_verify() { (1, payload2.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _>(&equivocation_proof)); // generate an equivocation proof, with two votes in different set ids let equivocation_proof = generate_double_voting_proof( @@ -261,7 +259,7 @@ fn should_sign_and_verify() { (1, payload2.clone(), set_id + 1, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _>(&equivocation_proof)); // generate an equivocation proof, with two votes in the same round for // different payloads signed by the same key @@ -271,7 +269,7 @@ fn should_sign_and_verify() { (1, payload2, set_id, &BeefyKeyring::Bob), ); // expect valid equivocation proof - assert!(check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(check_double_voting_proof::<_, _>(&equivocation_proof)); } trait ReportEquivocationFn: diff --git a/substrate/primitives/consensus/beefy/src/commitment.rs b/substrate/primitives/consensus/beefy/src/commitment.rs index 49547ed877ecc..f45b11e1f3558 100644 --- a/substrate/primitives/consensus/beefy/src/commitment.rs +++ b/substrate/primitives/consensus/beefy/src/commitment.rs @@ -20,7 +20,6 @@ use codec::{Decode, DecodeWithMemTracking, Encode, Error, Input}; use core::cmp; use scale_info::TypeInfo; use sp_application_crypto::RuntimeAppPublic; -use sp_runtime::traits::Hash; use crate::{BeefyAuthorityId, Payload, ValidatorSet, ValidatorSetId}; @@ -143,15 +142,14 @@ impl SignedCommitment { /// at the block where the commitment was generated. /// /// Returns the valid validator-signature pairs if the commitment can be verified. - pub fn verify_signatures<'a, TAuthorityId, MsgHash>( + pub fn verify_signatures<'a, TAuthorityId>( &'a self, target_number: TBlockNumber, validator_set: &'a ValidatorSet, ) -> Result>, u32> where TBlockNumber: Clone + Encode + PartialEq, - TAuthorityId: RuntimeAppPublic + BeefyAuthorityId, - MsgHash: Hash, + TAuthorityId: RuntimeAppPublic + BeefyAuthorityId, { if self.signatures.len() != validator_set.len() || self.commitment.validator_set_id != validator_set.id() || diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index fe7ef724c1fa7..51a91b829fca5 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -50,37 +50,44 @@ use alloc::vec::Vec; use codec::{Codec, Decode, DecodeWithMemTracking, Encode}; use core::fmt::{Debug, Display}; use scale_info::TypeInfo; +pub use sp_application_crypto::key_types::BEEFY as KEY_TYPE; use sp_application_crypto::{AppPublic, RuntimeAppPublic}; use sp_core::H256; +#[cfg(feature = "std")] +use sp_keystore::KeystorePtr; use sp_runtime::{ - traits::{Hash, Header as HeaderT, Keccak256, NumberFor}, + traits::{Header as HeaderT, Keccak256, NumberFor}, OpaqueValue, }; use sp_weights::Weight; - -/// Key type for BEEFY module. -pub const KEY_TYPE: sp_core::crypto::KeyTypeId = sp_application_crypto::key_types::BEEFY; +use KEY_TYPE as BEEFY_KEY_TYPE; /// Trait representing BEEFY authority id, including custom signature verification. -/// -/// Accepts custom hashing fn for the message and custom convertor fn for the signer. -pub trait BeefyAuthorityId: RuntimeAppPublic { +pub trait BeefyAuthorityId: RuntimeAppPublic { + /// Get all the public keys of the current type from a provided `Keystore`. + #[cfg(feature = "std")] + fn get_all_public_keys_from_store(store: KeystorePtr) -> Vec>; + + /// Sign a message using the private key associated to the current public key. + /// + /// We can't access the private key directly, so we need to receive the store that contains it. + #[cfg(feature = "std")] + fn try_sign_with_store( + &self, + store: KeystorePtr, + msg: &[u8], + ) -> Result + Debug>, sp_keystore::Error>; + /// Verify a signature. /// /// Return `true` if signature over `msg` is valid for this id. 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: - Ord - + AppPublic - + Display - + BeefyAuthorityId + Ord + AppPublic + Display + BeefyAuthorityId { /// Necessary bounds on the Signature associated with the AuthorityId type BoundedSignature: Debug + Eq + PartialEq + Clone + TypeInfo + Codec + Send + Sync; @@ -97,11 +104,20 @@ pub trait AuthorityIdBound: /// Your code should use the above types as concrete types for all crypto related /// functionality. pub mod ecdsa_crypto { - use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + #[cfg(feature = "std")] + use super::Vec; + use super::{AuthorityIdBound, BeefyAuthorityId, RuntimeAppPublic, BEEFY_KEY_TYPE}; + #[cfg(feature = "std")] + use core::fmt::Debug; use sp_application_crypto::{app_crypto, ecdsa}; use sp_core::crypto::Wraps; + #[cfg(feature = "std")] + use sp_core::ByteArray; + use sp_crypto_hashing::keccak_256; + #[cfg(feature = "std")] + use sp_keystore::KeystorePtr; - app_crypto!(ecdsa, KEY_TYPE); + app_crypto!(ecdsa, BEEFY_KEY_TYPE); /// Identity of a BEEFY authority using ECDSA as its crypto. pub type AuthorityId = Public; @@ -109,12 +125,25 @@ pub mod ecdsa_crypto { /// Signature for a BEEFY authority using ECDSA as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId - where - ::Output: Into<[u8; 32]>, - { + impl BeefyAuthorityId for AuthorityId { + #[cfg(feature = "std")] + fn get_all_public_keys_from_store(store: KeystorePtr) -> Vec> { + store.ecdsa_public_keys(BEEFY_KEY_TYPE) + } + + #[cfg(feature = "std")] + fn try_sign_with_store( + &self, + store: sp_keystore::KeystorePtr, + msg: &[u8], + ) -> Result + Debug>, sp_keystore::Error> { + let msg_hash = keccak_256(msg); + let public = ecdsa::Public::try_from(self.as_slice()).unwrap(); + store.ecdsa_sign_prehashed(BEEFY_KEY_TYPE, &public, &msg_hash) + } + fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - let msg_hash = ::hash(msg).into(); + let msg_hash = keccak_256(msg); match sp_io::crypto::secp256k1_ecdsa_recover_compressed( signature.as_inner_ref().as_ref(), &msg_hash, @@ -142,11 +171,17 @@ pub mod ecdsa_crypto { #[cfg(feature = "bls-experimental")] pub mod bls_crypto { - use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + #[cfg(feature = "std")] + use super::Vec; + use super::{AuthorityIdBound, BeefyAuthorityId, RuntimeAppPublic, BEEFY_KEY_TYPE}; + #[cfg(feature = "std")] + use core::fmt::Debug; use sp_application_crypto::{app_crypto, bls381}; - use sp_core::{bls381::Pair as BlsPair, crypto::Wraps, Pair as _}; + use sp_core::{bls381::Pair as BlsPair, crypto::Wraps, ByteArray, Pair as _}; + #[cfg(feature = "std")] + use sp_keystore::KeystorePtr; - app_crypto!(bls381, KEY_TYPE); + app_crypto!(bls381, BEEFY_KEY_TYPE); /// Identity of a BEEFY authority using BLS as its crypto. pub type AuthorityId = Public; @@ -154,10 +189,22 @@ pub mod bls_crypto { /// Signature for a BEEFY authority using BLS as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId - where - ::Output: Into<[u8; 32]>, - { + impl BeefyAuthorityId for AuthorityId { + #[cfg(feature = "std")] + fn get_all_public_keys_from_store(store: KeystorePtr) -> Vec> { + store.bls381_public_keys(BEEFY_KEY_TYPE) + } + + #[cfg(feature = "std")] + fn try_sign_with_store( + &self, + store: sp_keystore::KeystorePtr, + msg: &[u8], + ) -> Result + Debug>, sp_keystore::Error> { + let public = bls381::Public::try_from(self.as_slice()).unwrap(); + store.bls381_sign(BEEFY_KEY_TYPE, &public, msg) + } + 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. @@ -184,11 +231,18 @@ pub mod bls_crypto { /// functionality. #[cfg(feature = "bls-experimental")] pub mod ecdsa_bls_crypto { - use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + #[cfg(feature = "std")] + use super::Vec; + use super::{AuthorityIdBound, BeefyAuthorityId, RuntimeAppPublic, BEEFY_KEY_TYPE}; + #[cfg(feature = "std")] + use core::fmt::Debug; use sp_application_crypto::{app_crypto, ecdsa_bls381}; - use sp_core::{crypto::Wraps, ecdsa_bls381::Pair as EcdsaBlsPair}; + use sp_core::{crypto::Wraps, ecdsa_bls381::Pair as EcdsaBlsPair, ByteArray}; + #[cfg(feature = "std")] + use sp_keystore::KeystorePtr; + use sp_runtime::traits::Keccak256; - app_crypto!(ecdsa_bls381, KEY_TYPE); + app_crypto!(ecdsa_bls381, BEEFY_KEY_TYPE); /// Identity of a BEEFY authority using (ECDSA,BLS) as its crypto. pub type AuthorityId = Public; @@ -196,11 +250,22 @@ pub mod ecdsa_bls_crypto { /// Signature for a BEEFY authority using (ECDSA,BLS) as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId - where - H: Hash, - H::Output: Into<[u8; 32]>, - { + impl BeefyAuthorityId for AuthorityId { + #[cfg(feature = "std")] + fn get_all_public_keys_from_store(store: KeystorePtr) -> Vec> { + store.ecdsa_bls381_public_keys(BEEFY_KEY_TYPE) + } + + #[cfg(feature = "std")] + fn try_sign_with_store( + &self, + store: sp_keystore::KeystorePtr, + msg: &[u8], + ) -> Result + Debug>, sp_keystore::Error> { + let public = ecdsa_bls381::Public::try_from(self.as_slice()).unwrap(); + store.ecdsa_bls381_sign_with_keccak256(BEEFY_KEY_TYPE, &public, &msg) + } + fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` @@ -209,7 +274,7 @@ pub mod ecdsa_bls_crypto { // 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. - EcdsaBlsPair::verify_with_hasher::( + EcdsaBlsPair::verify_with_hasher::( signature.as_inner_ref(), msg, self.as_inner_ref(), @@ -372,29 +437,27 @@ pub struct FutureBlockVotingProof { /// Check a commitment signature by encoding the commitment and /// verifying the provided signature using the expected authority id. -pub fn check_commitment_signature( +pub fn check_commitment_signature( commitment: &Commitment, authority_id: &Id, signature: &::Signature, ) -> bool where - Id: BeefyAuthorityId, + Id: BeefyAuthorityId, Number: Clone + Encode + PartialEq, - MsgHash: Hash, { let encoded_commitment = commitment.encode(); - BeefyAuthorityId::::verify(authority_id, signature, &encoded_commitment) + BeefyAuthorityId::verify(authority_id, signature, &encoded_commitment) } /// Verifies the equivocation proof by making sure that both votes target /// different blocks and that its signatures are valid. -pub fn check_double_voting_proof( +pub fn check_double_voting_proof( report: &DoubleVotingProof::Signature>, ) -> bool where - Id: BeefyAuthorityId + PartialEq, + Id: BeefyAuthorityId + PartialEq, Number: Clone + Encode + PartialEq, - MsgHash: Hash, { let first = &report.first; let second = &report.second; @@ -555,8 +618,7 @@ mod tests { use super::*; use sp_application_crypto::ecdsa::{self, Public}; use sp_core::crypto::{Pair, Wraps}; - use sp_crypto_hashing::{blake2_256, keccak_256}; - use sp_runtime::traits::{BlakeTwo256, Keccak256}; + use sp_crypto_hashing::keccak_256; #[test] fn validator_set() { @@ -576,39 +638,15 @@ mod tests { let msg = &b"test-message"[..]; let (pair, _) = ecdsa_crypto::Pair::generate(); - let keccak_256_signature: ecdsa_crypto::Signature = + let signature: ecdsa_crypto::Signature = pair.as_inner_ref().sign_prehashed(&keccak_256(msg)).into(); - let blake2_256_signature: ecdsa_crypto::Signature = - pair.as_inner_ref().sign_prehashed(&blake2_256(msg)).into(); - - // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&pair.public(), &keccak_256_signature, msg)); - assert!(BeefyAuthorityId::::verify( - &pair.public(), - &blake2_256_signature, - msg - )); - // Verification fails if distinct hashing functions are used when signing and verifying. - assert!(!BeefyAuthorityId::::verify(&pair.public(), &blake2_256_signature, msg)); - assert!(!BeefyAuthorityId::::verify( - &pair.public(), - &keccak_256_signature, - msg - )); + // Verification works if same key is used when signing and verifying. + assert!(BeefyAuthorityId::verify(&pair.public(), &signature, msg)); // Other public key doesn't work let (other_pair, _) = ecdsa_crypto::Pair::generate(); - assert!(!BeefyAuthorityId::::verify( - &other_pair.public(), - &keccak_256_signature, - msg, - )); - assert!(!BeefyAuthorityId::::verify( - &other_pair.public(), - &blake2_256_signature, - msg, - )); + assert!(!BeefyAuthorityId::verify(&other_pair.public(), &signature, msg,)); } #[test] @@ -620,11 +658,11 @@ mod tests { let signature: bls_crypto::Signature = pair.as_inner_ref().sign(&msg).into(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); + assert!(BeefyAuthorityId::verify(&pair.public(), &signature, msg)); // Other public key doesn't work let (other_pair, _) = bls_crypto::Pair::generate(); - assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); + assert!(!BeefyAuthorityId::verify(&other_pair.public(), &signature, msg,)); } #[test] @@ -637,13 +675,13 @@ mod tests { pair.as_inner_ref().sign_with_hasher::(&msg).into(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); + assert!(BeefyAuthorityId::verify(&pair.public(), &signature, msg)); // 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 let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); - assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); + assert!(!BeefyAuthorityId::verify(&other_pair.public(), &signature, msg,)); } } diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index 4460bcefd45f9..10ffb156a0ce9 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -18,14 +18,15 @@ #[cfg(feature = "bls-experimental")] use crate::ecdsa_bls_crypto; use crate::{ - ecdsa_crypto, AuthorityIdBound, BeefySignatureHasher, Commitment, DoubleVotingProof, - ForkVotingProof, FutureBlockVotingProof, Payload, ValidatorSetId, VoteMessage, + ecdsa_crypto, AuthorityIdBound, Commitment, DoubleVotingProof, ForkVotingProof, + FutureBlockVotingProof, Payload, ValidatorSetId, VoteMessage, }; use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; use sp_core::{ecdsa, Pair}; -use sp_runtime::traits::{BlockNumber, Hash, Header as HeaderT}; +use sp_runtime::traits::{BlockNumber, Header as HeaderT}; use codec::Encode; +use sp_crypto_hashing::keccak_256; use std::{collections::HashMap, marker::PhantomData, sync::LazyLock}; use strum::IntoEnumIterator; @@ -44,33 +45,26 @@ pub enum Keyring { _Marker(PhantomData), } -/// Trait representing BEEFY specific generation and signing behavior 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 { - /// Generate and return signature for `message` using custom hashing `MsgHash` - fn sign_with_hasher(&self, message: &[u8]) -> ::Signature; +/// The trait mimics `BeefyAuthorityId` signing, but uses the private key instead of a keystore. +/// This is needed for testing purposes. +pub trait BeefySignerAuthority: AppPair { + /// Generate and return signature for `message`. + fn sign(&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(); +impl BeefySignerAuthority for ::Pair { + fn sign(&self, message: &[u8]) -> ::Signature { + let hashed_message = keccak_256(message); 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() +impl BeefySignerAuthority for ::Pair { + fn sign(&self, message: &[u8]) -> ::Signature { + self.as_inner_ref().sign(&message).into() } } @@ -78,14 +72,14 @@ where impl Keyring where AuthorityId: AuthorityIdBound + From<<::Pair as AppCrypto>::Public>, - ::Pair: BeefySignerAuthority, + ::Pair: BeefySignerAuthority, ::Signature: Send + Sync + From<<::Pair as AppCrypto>::Signature>, { /// Sign `msg`. pub fn sign(&self, msg: &[u8]) -> ::Signature { let key_pair: ::Pair = self.pair(); - key_pair.sign_with_hasher(&msg).into() + BeefySignerAuthority::sign(&key_pair, msg).into() } /// Return key pair.