Skip to content

Commit

Permalink
Make BEEFY client keystore generic over BEEFY AuthorityId type (#2258)
Browse files Browse the repository at this point in the history
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 <davxy@datawok.net>
Co-authored-by: Robert Hambrock <roberthambrock@gmail.com>
  • Loading branch information
3 people authored Feb 8, 2024
1 parent bc5a758 commit 0a94124
Show file tree
Hide file tree
Showing 22 changed files with 745 additions and 234 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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 0a94124

Please sign in to comment.