Skip to content

Commit

Permalink
Make BEEFY client keystore generic over BEEFY AuthorityId type (par…
Browse files Browse the repository at this point in the history
…itytech#2258)

This is the significant step to make BEEFY client able to handle both
ECDSA and (ECDSA, BLS) type signature. The idea is having BEEFY Client
generic on crypto types makes migration to new types smoother.

This makes the BEEFY Keystore generic over AuthorityId and extends its
tests to cover the case when the AuthorityId is of type (ECDSA,
BLS12-377)

---------

Co-authored-by: Davide Galassi <[email protected]>
Co-authored-by: Robert Hambrock <[email protected]>
  • Loading branch information
3 people authored Feb 8, 2024
1 parent a1e5f3c commit a9364d4
Show file tree
Hide file tree
Showing 21 changed files with 744 additions and 234 deletions.
9 changes: 9 additions & 0 deletions substrate/client/consensus/beefy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,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-consensus-beefy/bls-experimental",
"sp-core/bls-experimental",
]
18 changes: 12 additions & 6 deletions substrate/client/consensus/beefy/src/communication/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,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, test_utils::Keyring, Commitment, MmrRootHash,
Payload, SignedCommitment, VoteMessage,
};
use sp_keystore::{testing::MemoryKeystore, Keystore};

Expand All @@ -507,10 +507,13 @@ pub(crate) mod tests {
}
}

pub fn sign_commitment<BN: Encode>(who: &Keyring, commitment: &Commitment<BN>) -> Signature {
pub fn sign_commitment<BN: Encode>(
who: &Keyring<AuthorityId>,
commitment: &Commitment<BN>,
) -> 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<AuthorityId> = Some(store.into()).into();
beefy_keystore.sign(&who.public(), &commitment.encode()).unwrap()
}

Expand Down Expand Up @@ -538,7 +541,10 @@ pub(crate) mod tests {
.validators()
.iter()
.map(|validator: &AuthorityId| {
Some(sign_commitment(&Keyring::from_public(validator).unwrap(), &commitment))
Some(sign_commitment(
&Keyring::<AuthorityId>::from_public(validator).unwrap(),
&commitment,
))
})
.collect();

Expand All @@ -547,7 +553,7 @@ pub(crate) mod tests {

#[test]
fn should_validate_messages() {
let keys = vec![Keyring::Alice.public()];
let keys = vec![Keyring::<AuthorityId>::Alice.public()];
let validator_set = ValidatorSet::<AuthorityId>::new(keys.clone(), 0).unwrap();
let (gv, mut report_stream) =
GossipValidator::<Block>::new(Arc::new(Mutex::new(KnownPeers::new())));
Expand Down
9 changes: 5 additions & 4 deletions substrate/client/consensus/beefy/src/justification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub(crate) fn verify_with_validator_set<Block: BlockT>(
.as_ref()
.map(|sig| {
signatures_checked += 1;
BeefyKeystore::verify(id, sig, &message[..])
BeefyKeystore::verify(*id, sig, &message[..])
})
.unwrap_or(false)
})
Expand All @@ -93,7 +93,8 @@ pub(crate) fn verify_with_validator_set<Block: BlockT>(
#[cfg(test)]
pub(crate) mod tests {
use sp_consensus_beefy::{
known_payloads, Commitment, Keyring, Payload, SignedCommitment, VersionedFinalityProof,
known_payloads, test_utils::Keyring, Commitment, Payload, SignedCommitment,
VersionedFinalityProof,
};
use substrate_test_runtime_client::runtime::Block;

Expand All @@ -103,7 +104,7 @@ pub(crate) mod tests {
pub(crate) fn new_finality_proof(
block_num: NumberFor<Block>,
validator_set: &ValidatorSet<AuthorityId>,
keys: &[Keyring],
keys: &[Keyring<AuthorityId>],
) -> BeefyVersionedFinalityProof<Block> {
let commitment = Commitment {
payload: Payload::from_single_entry(known_payloads::MMR_ROOT_ID, vec![]),
Expand Down Expand Up @@ -174,7 +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::<AuthorityId>::Dave.sign(&bad_signed_commitment.commitment.encode()));
match verify_with_validator_set::<Block>(block_num, &validator_set, &bad_proof.into()) {
Err((ConsensusError::InvalidJustification, 3)) => (),
e => assert!(false, "Got unexpected {:?}", e),
Expand Down
Loading

0 comments on commit a9364d4

Please sign in to comment.