Decouple the session validators from im-online (#7127)

* Decouple the session validators from im-online

* .

* Add SessionInterface trait in im-online

Add ValidatorId in im-online Trait

Make im-online compile

Make substrate binary compile

* Fix merging issue

* Make all compile

* Fix tests

* Avoid using frame dep in primitives via pallet-session-common

* Merge ValidatorSet into SessionInterface trait

Wrap a few too long lines

Add some docs

* Move pallet-sesion-common into pallet-session

* Move SessionInterface to sp-session and impl it in session pallet

Ref https://github.com/paritytech/substrate/pull/7127#discussion_r494892472

* Split put historical::FullValidatorIdentification trait

* Fix line width

* Fix staking mock

* Fix session doc test

* Simplify <T as ValidatorIdentification<AccountId>>::ValidatorId as ValidatorId<T>

* Nits

* Clean up.

* Make it compile by commenting out report_offence_im_online bench

* Tests

* Nits

* Move OneSessionHandler to sp-session

* Fix tests

* Add some docs

* .

* Fix typo

* Rename to ValidatorSet::session_index()

* Add some more docs

* .

* Remove extra empty line

* Fix line width check

.

* Apply suggestions from code review

* Cleaup Cargo.toml

* Aura has migrated to Pallet now

Co-authored-by: Tomasz Drwięga <tomasz@parity.io>

substrate/frame/im-online/Cargo.toml

@@ -19,7 +19,6 @@ codec = { package = "parity-scale-codec", version = "2.0.0", default-features =
 sp-core = { version = "2.0.0", default-features = false, path = "../../primitives/core" }
 sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" }
 serde = { version = "1.0.101", optional = true }
-pallet-session = { version = "2.0.0", default-features = false, path = "../session" }
 sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" }
 sp-runtime = { version = "2.0.0", default-features = false, path = "../../primitives/runtime" }
 sp-staking = { version = "2.0.0", default-features = false, path = "../../primitives/staking" }
@@ -28,8 +27,11 @@ frame-system = { version = "2.0.0", default-features = false, path = "../system"
 
 frame-benchmarking = { version = "2.0.0", default-features = false, path = "../benchmarking", optional = true }
 
+[dev-dependencies]
+pallet-session = { version = "2.0.0", path = "../session" }
+
 [features]
-default = ["std", "pallet-session/historical"]
+default = ["std"]
 std = [
 	"sp-application-crypto/std",
 	"pallet-authorship/std",
@@ -37,7 +39,6 @@ std = [
 	"sp-core/std",
 	"sp-std/std",
 	"serde",
-	"pallet-session/std",
 	"sp-io/std",
 	"sp-runtime/std",
 	"sp-staking/std",

substrate/frame/im-online/src/lib.rs

@@ -79,7 +79,6 @@ use codec::{Encode, Decode};
 use sp_core::offchain::OpaqueNetworkState;
 use sp_std::prelude::*;
 use sp_std::convert::TryInto;
-use pallet_session::historical::IdentificationTuple;
 use sp_runtime::{
 	offchain::storage::StorageValueRef,
 	RuntimeDebug,
@@ -95,7 +94,7 @@ use sp_staking::{
 };
 use frame_support::{
 	decl_module, decl_event, decl_storage, Parameter, debug, decl_error,
-	traits::Get,
+	traits::{Get, ValidatorSet, ValidatorSetWithIdentification, OneSessionHandler},
 };
 use frame_system::ensure_none;
 use frame_system::offchain::{
@@ -227,7 +226,19 @@ pub struct Heartbeat<BlockNumber>
 	pub validators_len: u32,
 }
 
-pub trait Config: SendTransactionTypes<Call<Self>> + pallet_session::historical::Config {
+/// A type for representing the validator id in a session.
+pub type ValidatorId<T> = <
+	<T as Config>::ValidatorSet as ValidatorSet<<T as frame_system::Config>::AccountId>
+>::ValidatorId;
+
+/// A tuple of (ValidatorId, Identification) where `Identification` is the full identification of `ValidatorId`.
+pub type IdentificationTuple<T> = (
+	ValidatorId<T>,
+	<<T as Config>::ValidatorSet as
+		ValidatorSetWithIdentification<<T as frame_system::Config>::AccountId>>::Identification,
+);
+
+pub trait Config: SendTransactionTypes<Call<Self>> + frame_system::Config {
 	/// The identifier type for an authority.
 	type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord;
 
@@ -242,6 +253,9 @@ pub trait Config: SendTransactionTypes<Call<Self>> + pallet_session::historical:
 	/// there is a chance the authority will produce a block and they won't be necessary.
 	type SessionDuration: Get<Self::BlockNumber>;
 
+	/// A type for retrieving the validators supposed to be online in a session.
+	type ValidatorSet: ValidatorSetWithIdentification<Self::AccountId>;
+
 	/// A type that gives us the ability to submit unresponsiveness offence reports.
 	type ReportUnresponsiveness:
 		ReportOffence<
@@ -293,10 +307,10 @@ decl_storage! {
 			double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) AuthIndex
 			=> Option<Vec<u8>>;
 
-		/// For each session index, we keep a mapping of `T::ValidatorId` to the
+		/// For each session index, we keep a mapping of `ValidatorId<T>` to the
 		/// number of blocks authored by the given authority.
 		AuthoredBlocks get(fn authored_blocks):
-			double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) T::ValidatorId
+			double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) ValidatorId<T>
 			=> u32;
 	}
 	add_extra_genesis {
@@ -345,7 +359,7 @@ decl_module! {
 		) {
 			ensure_none(origin)?;
 
-			let current_session = <pallet_session::Module<T>>::current_index();
+			let current_session = T::ValidatorSet::session_index();
 			let exists = <ReceivedHeartbeats>::contains_key(
 				&current_session,
 				&heartbeat.authority_index
@@ -397,12 +411,15 @@ type OffchainResult<T, A> = Result<A, OffchainErr<<T as frame_system::Config>::B
 
 /// Keep track of number of authored blocks per authority, uncles are counted as
 /// well since they're a valid proof of being online.
-impl<T: Config + pallet_authorship::Config> pallet_authorship::EventHandler<T::ValidatorId, T::BlockNumber> for Module<T> {
-	fn note_author(author: T::ValidatorId) {
+impl<
+	T: Config + pallet_authorship::Config,
+> pallet_authorship::EventHandler<ValidatorId<T>, T::BlockNumber> for Module<T>
+{
+	fn note_author(author: ValidatorId<T>) {
 		Self::note_authorship(author);
 	}
 
-	fn note_uncle(author: T::ValidatorId, _age: T::BlockNumber) {
+	fn note_uncle(author: ValidatorId<T>, _age: T::BlockNumber) {
 		Self::note_authorship(author);
 	}
 }
@@ -413,7 +430,7 @@ impl<T: Config> Module<T> {
 	/// authored at least one block, during the current session. Otherwise
 	/// `false`.
 	pub fn is_online(authority_index: AuthIndex) -> bool {
-		let current_validators = <pallet_session::Module<T>>::validators();
+		let current_validators = T::ValidatorSet::validators();
 
 		if authority_index >= current_validators.len() as u32 {
 			return false;
@@ -424,8 +441,8 @@ impl<T: Config> Module<T> {
 		Self::is_online_aux(authority_index, authority)
 	}
 
-	fn is_online_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool {
-		let current_session = <pallet_session::Module<T>>::current_index();
+	fn is_online_aux(authority_index: AuthIndex, authority: &ValidatorId<T>) -> bool {
+		let current_session = T::ValidatorSet::session_index();
 
 		<ReceivedHeartbeats>::contains_key(&current_session, &authority_index) ||
 			<AuthoredBlocks<T>>::get(
@@ -437,13 +454,13 @@ impl<T: Config> Module<T> {
 	/// Returns `true` if a heartbeat has been received for the authority at `authority_index` in
 	/// the authorities series, during the current session. Otherwise `false`.
 	pub fn received_heartbeat_in_current_session(authority_index: AuthIndex) -> bool {
-		let current_session = <pallet_session::Module<T>>::current_index();
+		let current_session = T::ValidatorSet::session_index();
 		<ReceivedHeartbeats>::contains_key(&current_session, &authority_index)
 	}
 
 	/// Note that the given authority has authored a block in the current session.
-	fn note_authorship(author: T::ValidatorId) {
-		let current_session = <pallet_session::Module<T>>::current_index();
+	fn note_authorship(author: ValidatorId<T>) {
+		let current_session = T::ValidatorSet::session_index();
 
 		<AuthoredBlocks<T>>::mutate(
 			&current_session,
@@ -460,8 +477,8 @@ impl<T: Config> Module<T> {
 			return Err(OffchainErr::TooEarly(heartbeat_after))
 		}
 
-		let session_index = <pallet_session::Module<T>>::current_index();
-		let validators_len = <pallet_session::Module<T>>::validators().len() as u32;
+		let session_index = T::ValidatorSet::session_index();
+		let validators_len = Keys::<T>::decode_len().unwrap_or_default() as u32;
 
 		Ok(Self::local_authority_keys()
 			.map(move |(authority_index, key)|
@@ -614,7 +631,7 @@ impl<T: Config> sp_runtime::BoundToRuntimeAppPublic for Module<T> {
 	type Public = T::AuthorityId;
 }
 
-impl<T: Config> pallet_session::OneSessionHandler<T::AccountId> for Module<T> {
+impl<T: Config> OneSessionHandler<T::AccountId> for Module<T> {
 	type Key = T::AuthorityId;
 
 	fn on_genesis_session<'a, I: 'a>(validators: I)
@@ -639,22 +656,24 @@ impl<T: Config> pallet_session::OneSessionHandler<T::AccountId> for Module<T> {
 	}
 
 	fn on_before_session_ending() {
-		let session_index = <pallet_session::Module<T>>::current_index();
+		let session_index = T::ValidatorSet::session_index();
 		let keys = Keys::<T>::get();
-		let current_validators = <pallet_session::Module<T>>::validators();
+		let current_validators = T::ValidatorSet::validators();
 
 		let offenders = current_validators.into_iter().enumerate()
 			.filter(|(index, id)|
 				!Self::is_online_aux(*index as u32, id)
 			).filter_map(|(_, id)|
-				T::FullIdentificationOf::convert(id.clone()).map(|full_id| (id, full_id))
+				<T::ValidatorSet as ValidatorSetWithIdentification<T::AccountId>>::IdentificationOf::convert(
+					id.clone()
+				).map(|full_id| (id, full_id))
 			).collect::<Vec<IdentificationTuple<T>>>();
 
 		// Remove all received heartbeats and number of authored blocks from the
 		// current session, they have already been processed and won't be needed
 		// anymore.
-		<ReceivedHeartbeats>::remove_prefix(&<pallet_session::Module<T>>::current_index());
-		<AuthoredBlocks<T>>::remove_prefix(&<pallet_session::Module<T>>::current_index());
+		<ReceivedHeartbeats>::remove_prefix(&T::ValidatorSet::session_index());
+		<AuthoredBlocks<T>>::remove_prefix(&T::ValidatorSet::session_index());
 
 		if offenders.is_empty() {
 			Self::deposit_event(RawEvent::AllGood);
@@ -691,7 +710,7 @@ impl<T: Config> frame_support::unsigned::ValidateUnsigned for Module<T> {
 			}
 
 			// check if session index from heartbeat is recent
-			let current_session = <pallet_session::Module<T>>::current_index();
+			let current_session = T::ValidatorSet::session_index();
 			if heartbeat.session_index != current_session {
 				return InvalidTransaction::Stale.into();
 			}

substrate/frame/im-online/src/mock.rs

@@ -179,6 +179,7 @@ impl Config for Runtime {
 	type AuthorityId = UintAuthorityId;
 	type Event = ();
 	type ReportUnresponsiveness = OffenceHandler;
+	type ValidatorSet = Historical;
 	type SessionDuration = Period;
 	type UnsignedPriority = UnsignedPriority;
 	type WeightInfo = ();
@@ -195,6 +196,7 @@ impl<LocalCall> frame_system::offchain::SendTransactionTypes<LocalCall> for Runt
 pub type ImOnline = Module<Runtime>;
 pub type System = frame_system::Module<Runtime>;
 pub type Session = pallet_session::Module<Runtime>;
+pub type Historical = pallet_session::historical::Module<Runtime>;
 
 pub fn advance_session() {
 	let now = System::block_number().max(1);