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

@@ -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",
 ]
 

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,
 			};

substrate/srml/authority-discovery/Cargo.toml

@@ -5,33 +5,32 @@ authors = ["Parity Technologies <admin@parity.io>"]
 edition = "2018"
 
 [dependencies]
-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 }
+codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] }
+primitives = { package = "substrate-primitives",  path = "../../core/primitives", 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",
 ]

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(),