From a7b1284fb722513a9dd3c956ca270f44e0f245b1 Mon Sep 17 00:00:00 2001 From: Max Inden <mail@max-inden.de> Date: Tue, 1 Oct 2019 12:28:19 +0200 Subject: [PATCH] srml/authority-discovery: Abstract session key type (#3698) * srml/authority-discovery: Abstract session key type Previously `srml/authority-discovery` dependet on the `srml/im-online` session key type directly. With this patch `srml/authority-discovery` is generic over the session key type it is going to use, as long as it implements the RuntimeAppPublic trait. With this patch one can use the `srml/authority-discovery` module without the `srml/im-online` module. Next to the above, this patch configures `node/runtime` to use the babe session keys for the authority discovery module. * srml/authority-discovery: Fix line length * srml/authority-discovery/Cargo: Move babe to dev-dependencies * node/runtime: Bump implementation version * srml/authority-discovery: Add doc comment for authority discovery Trait --- substrate/Cargo.lock | 2 +- substrate/node/runtime/src/lib.rs | 16 ++-- substrate/srml/authority-discovery/Cargo.toml | 15 ++-- substrate/srml/authority-discovery/src/lib.rs | 73 +++++++++---------- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 41e8e690137..1d9d2f01b99 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -3966,11 +3966,11 @@ dependencies = [ "sr-primitives 2.0.0", "sr-staking-primitives 2.0.0", "sr-std 2.0.0", - "srml-im-online 0.1.0", "srml-session 2.0.0", "srml-support 2.0.0", "srml-system 2.0.0", "substrate-application-crypto 2.0.0", + "substrate-consensus-babe-primitives 2.0.0", "substrate-primitives 2.0.0", ] diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index b7bb26ee00d..87bf313ac4f 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -29,7 +29,7 @@ use node_primitives::{ AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, Moment, Signature, ContractExecResult, }; -use babe_primitives::{AuthorityId as BabeId}; +use babe_primitives::{AuthorityId as BabeId, AuthoritySignature as BabeSignature}; use grandpa::fg_primitives; use client::{ block_builder::api::{self as block_builder_api, InherentData, CheckInherentsResult}, @@ -50,7 +50,7 @@ use elections::VoteIndex; use version::NativeVersion; use primitives::OpaqueMetadata; use grandpa::{AuthorityId as GrandpaId, AuthorityWeight as GrandpaWeight}; -use im_online::sr25519::{AuthorityId as ImOnlineId, AuthoritySignature as ImOnlineSignature}; +use im_online::sr25519::{AuthorityId as ImOnlineId}; use authority_discovery_primitives::{AuthorityId as EncodedAuthorityId, Signature as EncodedSignature}; use codec::{Encode, Decode}; use system::offchain::TransactionSubmitter; @@ -84,8 +84,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 167, - impl_version: 167, + spec_version: 168, + impl_version: 168, apis: RUNTIME_API_VERSIONS, }; @@ -437,7 +437,9 @@ impl offences::Trait for Runtime { type OnOffenceHandler = Staking; } -impl authority_discovery::Trait for Runtime {} +impl authority_discovery::Trait for Runtime { + type AuthorityId = BabeId; +} impl grandpa::Trait for Runtime { type Event = Event; @@ -635,12 +637,12 @@ impl_runtime_apis! { } fn verify(payload: &Vec<u8>, signature: &EncodedSignature, authority_id: &EncodedAuthorityId) -> bool { - let signature = match ImOnlineSignature::decode(&mut &signature.0[..]) { + let signature = match BabeSignature::decode(&mut &signature.0[..]) { Ok(s) => s, _ => return false, }; - let authority_id = match ImOnlineId::decode(&mut &authority_id.0[..]) { + let authority_id = match BabeId::decode(&mut &authority_id.0[..]) { Ok(id) => id, _ => return false, }; diff --git a/substrate/srml/authority-discovery/Cargo.toml b/substrate/srml/authority-discovery/Cargo.toml index 06573484578..e9647984ede 100644 --- a/substrate/srml/authority-discovery/Cargo.toml +++ b/substrate/srml/authority-discovery/Cargo.toml @@ -5,33 +5,32 @@ authors = ["Parity Technologies <admin@parity.io>"] edition = "2018" [dependencies] +app-crypto = { package = "substrate-application-crypto", path = "../../core/application-crypto", default-features = false } codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] } -sr-primitives = { path = "../../core/sr-primitives", default-features = false } primitives = { package = "substrate-primitives", path = "../../core/primitives", default-features = false } -app-crypto = { package = "substrate-application-crypto", path = "../../core/application-crypto", default-features = false } rstd = { package = "sr-std", path = "../../core/sr-std", default-features = false } +runtime-io = { package = "sr-io", path = "../../core/sr-io", default-features = false } serde = { version = "1.0", optional = true } session = { package = "srml-session", path = "../session", default-features = false } -im-online = { package = "srml-im-online", path = "../im-online", default-features = false } +sr-primitives = { path = "../../core/sr-primitives", default-features = false } support = { package = "srml-support", path = "../support", default-features = false } -runtime-io = { package = "sr-io", path = "../../core/sr-io", default-features = false } system = { package = "srml-system", path = "../system", default-features = false } [dev-dependencies] +babe-primitives = { package = "substrate-consensus-babe-primitives", path = "../../core/consensus/babe/primitives", default-features = false } sr-staking-primitives = { path = "../../core/sr-staking-primitives", default-features = false } [features] default = ["std"] std = [ + "app-crypto/std", "codec/std", - "sr-primitives/std", "primitives/std", "rstd/std", + "runtime-io/std", "serde", "session/std", - "im-online/std", + "sr-primitives/std", "support/std", - "runtime-io/std", "system/std", - "app-crypto/std", ] diff --git a/substrate/srml/authority-discovery/src/lib.rs b/substrate/srml/authority-discovery/src/lib.rs index 7a4a104f1ca..4f759ba38a2 100644 --- a/substrate/srml/authority-discovery/src/lib.rs +++ b/substrate/srml/authority-discovery/src/lib.rs @@ -22,29 +22,29 @@ //! //! ## Dependencies //! -//! This module depends on the [I’m online module](../srml_im_online/index.html) -//! using its session key. +//! This module depends on an externally defined session key type, specified via +//! `Trait::AuthorityId` in the respective node runtime implementation. // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] use app_crypto::RuntimeAppPublic; +use codec::{Decode, Encode}; use rstd::prelude::*; use support::{decl_module, decl_storage}; -pub trait Trait: system::Trait + session::Trait + im_online::Trait {} - -type AuthorityIdFor<T> = <T as im_online::Trait>::AuthorityId; -type AuthoritySignatureFor<T> = - <<T as im_online::Trait>::AuthorityId as RuntimeAppPublic>::Signature; +/// The module's config trait. +pub trait Trait: system::Trait + session::Trait { + type AuthorityId: RuntimeAppPublic + Default + Decode + Encode + PartialEq; +} decl_storage! { trait Store for Module<T: Trait> as AuthorityDiscovery { /// The current set of keys that may issue a heartbeat. - Keys get(keys): Vec<AuthorityIdFor<T>>; + Keys get(keys): Vec<T::AuthorityId>; } add_extra_genesis { - config(keys): Vec<AuthorityIdFor<T>>; + config(keys): Vec<T::AuthorityId>; build(|config| Module::<T>::initialize_keys(&config.keys)) } } @@ -59,10 +59,10 @@ impl<T: Trait> Module<T> { /// set, otherwise this function returns None. The restriction might be /// softened in the future in case a consumer needs to learn own authority /// identifier. - fn authority_id() -> Option<AuthorityIdFor<T>> { + fn authority_id() -> Option<T::AuthorityId> { let authorities = Keys::<T>::get(); - let local_keys = <AuthorityIdFor<T>>::all(); + let local_keys = T::AuthorityId::all(); authorities.into_iter().find_map(|authority| { if local_keys.contains(&authority) { @@ -74,12 +74,17 @@ impl<T: Trait> Module<T> { } /// Retrieve authority identifiers of the current authority set. - pub fn authorities() -> Vec<AuthorityIdFor<T>> { + pub fn authorities() -> Vec<T::AuthorityId> { Keys::<T>::get() } /// Sign the given payload with the private key corresponding to the given authority id. - pub fn sign(payload: &Vec<u8>) -> Option<(AuthoritySignatureFor<T>, AuthorityIdFor<T>)> { + pub fn sign( + payload: &Vec<u8>, + ) -> Option<( + <<T as Trait>::AuthorityId as RuntimeAppPublic>::Signature, + T::AuthorityId, + )> { let authority_id = Module::<T>::authority_id()?; authority_id.sign(payload).map(|s| (s, authority_id)) } @@ -88,13 +93,13 @@ impl<T: Trait> Module<T> { /// authority identifier. pub fn verify( payload: &Vec<u8>, - signature: AuthoritySignatureFor<T>, - authority_id: AuthorityIdFor<T>, + signature: <<T as Trait>::AuthorityId as RuntimeAppPublic>::Signature, + authority_id: T::AuthorityId, ) -> bool { authority_id.verify(payload, &signature) } - fn initialize_keys(keys: &[AuthorityIdFor<T>]) { + fn initialize_keys(keys: &[T::AuthorityId]) { if !keys.is_empty() { assert!(Keys::<T>::get().is_empty(), "Keys are already initialized!"); Keys::<T>::put_ref(keys); @@ -103,7 +108,7 @@ impl<T: Trait> Module<T> { } impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> { - type Key = AuthorityIdFor<T>; + type Key = T::AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) where @@ -144,9 +149,11 @@ mod tests { #[derive(Clone, Eq, PartialEq)] pub struct Test; - impl Trait for Test {} + impl Trait for Test { + type AuthorityId = babe_primitives::AuthorityId; + } - type AuthorityId = im_online::sr25519::AuthorityId; + type AuthorityId = babe_primitives::AuthorityId; pub struct TestOnSessionEnding; impl session::OnSessionEnding<AuthorityId> for TestOnSessionEnding { @@ -176,18 +183,6 @@ mod tests { type FullIdentificationOf = (); } - impl im_online::Trait for Test { - type AuthorityId = AuthorityId; - type Call = im_online::Call<Test>; - type Event = (); - type SubmitTransaction = system::offchain::TransactionSubmitter< - (), - im_online::Call<Test>, - UncheckedExtrinsic<(), im_online::Call<Test>, (), ()>, - >; - type ReportUnresponsiveness = (); - } - pub type BlockNumber = u64; parameter_types! { @@ -243,13 +238,13 @@ mod tests { let key_store = KeyStore::new(); key_store .write() - .sr25519_generate_new(key_types::IM_ONLINE, None) + .sr25519_generate_new(key_types::BABE, None) .expect("Generates key."); // Retrieve key to later check if we got the right one. let public_key = key_store .read() - .sr25519_public_keys(key_types::IM_ONLINE) + .sr25519_public_keys(key_types::BABE) .pop() .unwrap(); let authority_id = AuthorityId::from(public_key); @@ -283,7 +278,7 @@ mod tests { let key_store = KeyStore::new(); key_store .write() - .sr25519_generate_new(key_types::IM_ONLINE, None) + .sr25519_generate_new(key_types::BABE, None) .expect("Generates key."); // Build genesis. @@ -317,13 +312,13 @@ mod tests { let key_store = KeyStore::new(); key_store .write() - .sr25519_generate_new(key_types::IM_ONLINE, None) + .sr25519_generate_new(key_types::BABE, None) .expect("Generates key."); // Retrieve key to later check if we got the right one. let public_key = key_store .read() - .sr25519_public_keys(key_types::IM_ONLINE) + .sr25519_public_keys(key_types::BABE) .pop() .unwrap(); let authority_id = AuthorityId::from(public_key); @@ -347,7 +342,11 @@ mod tests { let payload = String::from("test payload").into_bytes(); let (sig, authority_id) = AuthorityDiscovery::sign(&payload).expect("signature"); - assert!(AuthorityDiscovery::verify(&payload, sig.clone(), authority_id.clone(),)); + assert!(AuthorityDiscovery::verify( + &payload, + sig.clone(), + authority_id.clone(), + )); assert!(!AuthorityDiscovery::verify( &String::from("other payload").into_bytes(), -- GitLab