Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

sp-beefy: align authority id key type with its signature type #13131

Merged
merged 3 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions client/beefy/src/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use log::warn;

use beefy_primitives::{
crypto::{Public, Signature},
BeefyVerify, KEY_TYPE,
BeefyAuthorityId, KEY_TYPE,
};

use crate::error;
Expand Down Expand Up @@ -99,7 +99,7 @@ impl BeefyKeystore {
///
/// Return `true` if the signature is authentic, `false` otherwise.
pub fn verify(public: &Public, sig: &Signature, message: &[u8]) -> bool {
BeefyVerify::<Keccak256>::verify(sig, message, public)
BeefyAuthorityId::<Keccak256>::verify(public, sig, message)
}
}

Expand Down
4 changes: 2 additions & 2 deletions client/beefy/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ where
warn!(target: "beefy", "🥩 Buffer vote dropped for round: {:?}", block_num)
}
} else {
error!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num);
warn!(target: "beefy", "🥩 Buffer vote dropped for round: {:?}.", block_num);
}
},
RoundAction::Drop => (),
Expand Down Expand Up @@ -528,7 +528,7 @@ where
if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS {
self.pending_justifications.entry(block_num).or_insert(justification);
} else {
error!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num);
warn!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num);
}
},
RoundAction::Drop => (),
Expand Down
52 changes: 25 additions & 27 deletions primitives/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,14 @@ use sp_std::prelude::*;
/// Key type for BEEFY module.
pub const KEY_TYPE: sp_application_crypto::KeyTypeId = sp_application_crypto::KeyTypeId(*b"beef");

/// Trait representing BEEFY authority id.
pub trait BeefyAuthorityId: RuntimeAppPublic {}

/// Means of verification for a BEEFY authority signature.
/// 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 BeefyVerify<MsgHash: Hash> {
/// Type of the signer.
type Signer: BeefyAuthorityId;

pub trait BeefyAuthorityId<MsgHash: Hash>: RuntimeAppPublic {
/// Verify a signature.
///
/// Return `true` if signature is valid for the value.
fn verify(&self, msg: &[u8], signer: &Self::Signer) -> bool;
/// Return `true` if signature over `msg` is valid for this id.
fn verify(&self, signature: &<Self as RuntimeAppPublic>::Signature, msg: &[u8]) -> bool;
}

/// BEEFY cryptographic types
Expand All @@ -78,7 +72,7 @@ pub trait BeefyVerify<MsgHash: Hash> {
/// The current underlying crypto scheme used is ECDSA. This can be changed,
/// without affecting code restricted against the above listed crypto types.
pub mod crypto {
use super::{BeefyAuthorityId, BeefyVerify, Hash};
use super::{BeefyAuthorityId, Hash, RuntimeAppPublic};
use sp_application_crypto::{app_crypto, ecdsa};
use sp_core::crypto::Wraps;
app_crypto!(ecdsa, crate::KEY_TYPE);
Expand All @@ -89,21 +83,17 @@ pub mod crypto {
/// Signature for a BEEFY authority using ECDSA as its crypto.
pub type AuthoritySignature = Signature;

impl BeefyAuthorityId for AuthorityId {}

impl<MsgHash: Hash> BeefyVerify<MsgHash> for AuthoritySignature
impl<MsgHash: Hash> BeefyAuthorityId<MsgHash> for AuthorityId
where
<MsgHash as Hash>::Output: Into<[u8; 32]>,
{
type Signer = AuthorityId;

fn verify(&self, msg: &[u8], signer: &Self::Signer) -> bool {
fn verify(&self, signature: &<Self as RuntimeAppPublic>::Signature, msg: &[u8]) -> bool {
let msg_hash = <MsgHash as Hash>::hash(msg).into();
match sp_io::crypto::secp256k1_ecdsa_recover_compressed(
self.as_inner_ref().as_ref(),
signature.as_inner_ref().as_ref(),
&msg_hash,
) {
Ok(raw_pubkey) => raw_pubkey.as_ref() == AsRef::<[u8]>::as_ref(signer),
Ok(raw_pubkey) => raw_pubkey.as_ref() == AsRef::<[u8]>::as_ref(self),
_ => false,
}
}
Expand Down Expand Up @@ -248,23 +238,31 @@ mod tests {
pair.as_inner_ref().sign_prehashed(&blake2_256(msg)).into();

// Verification works if same hashing function is used when signing and verifying.
assert!(BeefyVerify::<Keccak256>::verify(&keccak_256_signature, msg, &pair.public()));
assert!(BeefyVerify::<BlakeTwo256>::verify(&blake2_256_signature, msg, &pair.public()));
assert!(BeefyAuthorityId::<Keccak256>::verify(&pair.public(), &keccak_256_signature, msg));
assert!(BeefyAuthorityId::<BlakeTwo256>::verify(
&pair.public(),
&blake2_256_signature,
msg
));
// Verification fails if distinct hashing functions are used when signing and verifying.
assert!(!BeefyVerify::<Keccak256>::verify(&blake2_256_signature, msg, &pair.public()));
assert!(!BeefyVerify::<BlakeTwo256>::verify(&keccak_256_signature, msg, &pair.public()));
assert!(!BeefyAuthorityId::<Keccak256>::verify(&pair.public(), &blake2_256_signature, msg));
assert!(!BeefyAuthorityId::<BlakeTwo256>::verify(
&pair.public(),
&keccak_256_signature,
msg
));

// Other public key doesn't work
let (other_pair, _) = crypto::Pair::generate();
assert!(!BeefyVerify::<Keccak256>::verify(
assert!(!BeefyAuthorityId::<Keccak256>::verify(
&other_pair.public(),
&keccak_256_signature,
msg,
&other_pair.public()
));
assert!(!BeefyVerify::<BlakeTwo256>::verify(
assert!(!BeefyAuthorityId::<BlakeTwo256>::verify(
&other_pair.public(),
&blake2_256_signature,
msg,
&other_pair.public()
));
}
}