Fix tracking validator set in ImOnline (#3596)

* Use session::validators instead of staking::current_elected

* Basic test framework.

* Initialize validators, attempt to heartbeat.

* Use dummy crypto for im-online testing.

* Remove printlns.

* Finish test, make it invalid.

* Add reporting test.

* Finalize the test.

* Remove dumbness.

* Updates.

* Update AuRa

* Update srml/im-online/src/tests.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Derive Ord

* Add some more tests.

* Remove stray todo.

* Bump runtime version.

* Bump impl-trait-for-tuples.

* Enforce new version of trait-for-tuples.

substrate/Cargo.lock

@@ -1381,7 +1381,7 @@ dependencies = [
 
 [[package]]
 name = "impl-trait-for-tuples"
-version = "0.1.0"
+version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "proc-macro2 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3833,7 +3833,7 @@ dependencies = [
 name = "sr-primitives"
 version = "2.0.0"
 dependencies = [
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "integer-sqrt 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3946,7 +3946,7 @@ dependencies = [
 name = "srml-authorship"
 version = "0.1.0"
 dependencies = [
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-scale-codec 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "sr-io 2.0.0",
  "sr-primitives 2.0.0",
@@ -4105,7 +4105,7 @@ dependencies = [
 name = "srml-finality-tracker"
 version = "2.0.0"
 dependencies = [
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-scale-codec 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.97 (registry+https://github.com/rust-lang/crates.io-index)",
  "sr-io 2.0.0",
@@ -4163,6 +4163,7 @@ dependencies = [
  "srml-support 2.0.0",
  "srml-system 2.0.0",
  "substrate-application-crypto 2.0.0",
+ "substrate-offchain 2.0.0",
  "substrate-primitives 2.0.0",
 ]
 
@@ -4242,7 +4243,7 @@ dependencies = [
 name = "srml-session"
 version = "2.0.0"
 dependencies = [
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-scale-codec 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "safe-mix 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4300,7 +4301,7 @@ name = "srml-support"
 version = "2.0.0"
 dependencies = [
  "bitmask 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "once_cell 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-scale-codec 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "paste 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4366,7 +4367,7 @@ name = "srml-system"
 version = "2.0.0"
 dependencies = [
  "criterion 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-scale-codec 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "safe-mix 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.97 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -4382,7 +4383,7 @@ dependencies = [
 name = "srml-timestamp"
 version = "2.0.0"
 dependencies = [
- "impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-scale-codec 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.97 (registry+https://github.com/rust-lang/crates.io-index)",
  "sr-io 2.0.0",
@@ -4541,7 +4542,6 @@ dependencies = [
  "sr-primitives 2.0.0",
  "substrate-authority-discovery-primitives 2.0.0",
  "substrate-client 2.0.0",
- "substrate-keystore 2.0.0",
  "substrate-network 2.0.0",
  "substrate-peerset 2.0.0",
  "substrate-primitives 2.0.0",
@@ -6620,7 +6620,7 @@ dependencies = [
 "checksum impl-codec 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "78c441b3d2b5e24b407161e76d482b7bbd29b5da357707839ac40d95152f031f"
 "checksum impl-serde 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "5158079de9d4158e0ce1de3ae0bd7be03904efc40b3d7dd8b8c301cbf6b52b56"
 "checksum impl-serde 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7d26be4b97d738552ea423f76c4f681012ff06c3fa36fa968656b3679f60b4a1"
-"checksum impl-trait-for-tuples 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3a9213bd15aa3f974ed007e12e520c435af21e0bb9b016c0874f05eec30034cf"
+"checksum impl-trait-for-tuples 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7b0df44cb13008e863c3d80788d5f4cb0f94d09b31bb0190a8ecc05151b2ac8a"
 "checksum indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7e81a7c05f79578dbc15793d8b619db9ba32b4577003ef3af1a91c416798c58d"
 "checksum integer-sqrt 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "ea155abb3ba6f382a75f1418988c05fe82959ed9ce727de427f9cfd425b0c903"
 "checksum interleaved-ordered 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "141340095b15ed7491bd3d4ced9d20cebfb826174b6bb03386381f62b01e3d77"

substrate/core/application-crypto/src/lib.rs

@@ -213,6 +213,7 @@ macro_rules! app_crypto {
 		}
 
 		impl $crate::RuntimeAppPublic for Public where $public: $crate::RuntimePublic<Signature=$sig> {
+			const ID: $crate::KeyTypeId = $key_type;
 			type Signature = Signature;
 
 			fn all() -> $crate::Vec<Self> {

substrate/core/application-crypto/src/traits.rs

@@ -96,7 +96,10 @@ pub trait RuntimePublic: Sized {
 }
 
 /// A runtime interface for an application's public key.
-pub trait RuntimeAppPublic: Sized {
+pub trait RuntimeAppPublic: Sized  {
+	/// An identifier for this application-specific key type.
+	const ID: KeyTypeId;
+
 	/// The signature that will be generated when signing with the corresponding private key.
 	type Signature: Codec + MaybeDebugHash + Eq + PartialEq + Clone;
 

substrate/core/authority-discovery/Cargo.toml

@@ -15,7 +15,6 @@ client = { package = "substrate-client", path = "../../core/client" }
 codec = { package = "parity-scale-codec", default-features = false, version = "1.0.3" }
 derive_more = "0.14.0"
 futures = "0.1"
-keystore = { package = "substrate-keystore", path = "../../core/keystore" }
 libp2p = { version = "0.12.0", default-features = false, features = ["secp256k1", "libp2p-websocket"] }
 log = "0.4"
 network = { package = "substrate-network", path = "../../core/network" }

substrate/core/offchain/src/testing.rs

@@ -67,6 +67,8 @@ pub struct State {
 	pub persistent_storage: client::in_mem::OffchainStorage,
 	/// Local storage
 	pub local_storage: client::in_mem::OffchainStorage,
+	/// A vector of transactions submitted from the runtime.
+	pub transactions: Vec<Vec<u8>>,
 }
 
 impl State {
@@ -138,12 +140,17 @@ impl offchain::Externalities for TestOffchainExt {
 		unimplemented!("not needed in tests so far")
 	}
 
-	fn submit_transaction(&mut self, _ex: Vec<u8>) -> Result<(), ()> {
+	fn submit_transaction(&mut self, ex: Vec<u8>) -> Result<(), ()> {
-		unimplemented!("not needed in tests so far")
+		let mut state = self.0.write();
+		state.transactions.push(ex);
+		Ok(())
 	}
 
 	fn network_state(&self) -> Result<OpaqueNetworkState, ()> {
-		unimplemented!("not needed in tests so far")
+		Ok(OpaqueNetworkState {
+			peer_id: Default::default(),
+			external_addresses: vec![],
+		})
 	}
 
 	fn timestamp(&mut self) -> Timestamp {

substrate/core/primitives/src/offchain.rs

@@ -160,7 +160,7 @@ pub struct OpaqueNetworkState {
 }
 
 /// Simple blob to hold a `PeerId` without committing to its format.
-#[derive(Clone, Eq, PartialEq, Encode, Decode)]
+#[derive(Default, Clone, Eq, PartialEq, Encode, Decode)]
 #[cfg_attr(feature = "std", derive(Debug))]
 pub struct OpaquePeerId(pub Vec<u8>);
 

substrate/core/sr-primitives/Cargo.toml

@@ -16,7 +16,7 @@ runtime_io = { package = "sr-io", path = "../sr-io", default-features = false }
 log = { version = "0.4", optional = true }
 paste = { version = "0.1"}
 rand = { version = "0.7.0", optional = true }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [dev-dependencies]
 serde_json = "1.0"

substrate/core/sr-primitives/src/lib.rs

@@ -56,7 +56,7 @@ pub use generic::{DigestItem, Digest};
 
 /// Re-export this since it's part of the API of this crate.
 pub use primitives::crypto::{key_types, KeyTypeId, CryptoType};
-pub use app_crypto::AppKey;
+pub use app_crypto::RuntimeAppPublic;
 
 /// Justification type.
 pub type Justification = Vec<u8>;

substrate/core/sr-primitives/src/testing.rs

@@ -17,7 +17,7 @@
 //! Testing utilities.
 
 use serde::{Serialize, Serializer, Deserialize, de::Error as DeError, Deserializer};
-use std::{fmt::Debug, ops::Deref, fmt};
+use std::{fmt::Debug, ops::Deref, fmt, cell::RefCell};
 use crate::codec::{Codec, Encode, Decode};
 use crate::traits::{
 	self, Checkable, Applyable, BlakeTwo256, OpaqueKeys, ValidateUnsigned,
@@ -30,9 +30,15 @@ use primitives::{crypto::{CryptoType, Dummy, key_types, Public}, U256};
 use crate::transaction_validity::{TransactionValidity, TransactionValidityError};
 
 /// Authority Id
-#[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize)]
+#[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize, PartialOrd, Ord)]
 pub struct UintAuthorityId(pub u64);
 
+impl From<u64> for UintAuthorityId {
+	fn from(id: u64) -> Self {
+		UintAuthorityId(id)
+	}
+}
+
 impl UintAuthorityId {
 	/// Convert this authority id into a public key.
 	pub fn to_public_key<T: Public>(&self) -> T {
@@ -47,34 +53,44 @@ impl CryptoType for UintAuthorityId {
 
 impl AsRef<[u8]> for UintAuthorityId {
 	fn as_ref(&self) -> &[u8] {
+		// Unsafe, i know, but it's test code and it's just there because it's really convenient to
+		// keep `UintAuthorityId` as a u64 under the hood.
 		unsafe {
 			std::slice::from_raw_parts(&self.0 as *const u64 as *const _, std::mem::size_of::<u64>())
 		}
 	}
 }
 
+thread_local! {
+	/// A list of all UintAuthorityId keys returned to the runtime.
+	static ALL_KEYS: RefCell<Vec<UintAuthorityId>> = RefCell::new(vec![]);
+}
+
+impl UintAuthorityId {
+	/// Set the list of keys returned by the runtime call for all keys of that type.
+	pub fn set_all_keys<T: Into<UintAuthorityId>>(keys: impl IntoIterator<Item=T>) {
+		ALL_KEYS.with(|l| *l.borrow_mut() = keys.into_iter().map(Into::into).collect())
+	}
+}
+
 impl app_crypto::RuntimeAppPublic for UintAuthorityId {
+	const ID: KeyTypeId = key_types::DUMMY;
+
 	type Signature = u64;
 
 	fn all() -> Vec<Self> {
-		unimplemented!("`all()` not available for `UintAuthorityId`.")
+		ALL_KEYS.with(|l| l.borrow().clone())
 	}
 
-	#[cfg(feature = "std")]
 	fn generate_pair(_: Option<&str>) -> Self {
 		use rand::RngCore;
 		UintAuthorityId(rand::thread_rng().next_u64())
 	}
 
-	#[cfg(not(feature = "std"))]
-	fn generate_pair(_: Option<&str>) -> Self {
-		unimplemented!("`generate_pair` not implemented for `UIntAuthorityId` on `no_std`.")
-	}
-
 	fn sign<M: AsRef<[u8]>>(&self, msg: &M) -> Option<Self::Signature> {
 		let mut signature = [0u8; 8];
 		msg.as_ref().iter()
-			.chain(rstd::iter::repeat(&42u8))
+			.chain(std::iter::repeat(&42u8))
 			.take(8)
 			.enumerate()
 			.for_each(|(i, v)| { signature[i] = *v; });
@@ -85,7 +101,7 @@ impl app_crypto::RuntimeAppPublic for UintAuthorityId {
 	fn verify<M: AsRef<[u8]>>(&self, msg: &M, signature: &Self::Signature) -> bool {
 		let mut msg_signature = [0u8; 8];
 		msg.as_ref().iter()
-			.chain(rstd::iter::repeat(&42))
+			.chain(std::iter::repeat(&42))
 			.take(8)
 			.enumerate()
 			.for_each(|(i, v)| { msg_signature[i] = *v; });
@@ -97,19 +113,16 @@ impl app_crypto::RuntimeAppPublic for UintAuthorityId {
 impl OpaqueKeys for UintAuthorityId {
 	type KeyTypeIds = std::iter::Cloned<std::slice::Iter<'static, KeyTypeId>>;
 
-	fn key_ids() -> Self::KeyTypeIds { [key_types::DUMMY].iter().cloned() }
+	fn key_ids() -> Self::KeyTypeIds {
-	// Unsafe, i know, but it's test code and it's just there because it's really convenient to
+		[key_types::DUMMY].iter().cloned()
-	// keep `UintAuthorityId` as a u64 under the hood.
-	fn get_raw(&self, _: KeyTypeId) -> &[u8] {
-		unsafe {
-			std::slice::from_raw_parts(
-				&self.0 as *const _ as *const u8,
-				std::mem::size_of::<u64>(),
-			)
-		}
 	}
+
+	fn get_raw(&self, _: KeyTypeId) -> &[u8] {
+		self.as_ref()
+	}
+
 	fn get<T: Decode>(&self, _: KeyTypeId) -> Option<T> {
-		self.0.using_encoded(|mut x| T::decode(&mut x)).ok()
+		self.using_encoded(|mut x| T::decode(&mut x)).ok()
 	}
 }
 

substrate/core/sr-primitives/src/traits.rs

@@ -39,7 +39,7 @@ use rstd::ops::{
 	Add, Sub, Mul, Div, Rem, AddAssign, SubAssign, MulAssign, DivAssign,
 	RemAssign, Shl, Shr
 };
-use crate::AppKey;
+use app_crypto::AppKey;
 use impl_trait_for_tuples::impl_for_tuples;
 
 /// A lazy value.

substrate/core/sr-staking-primitives/src/lib.rs

@@ -17,16 +17,7 @@
 //! A crate which contains primitives that are useful for implementation that uses staking
 //! approaches in general. Definitions related to sessions, slashing, etc go here.
 
-use rstd::vec::Vec;
-
 pub mod offence;
 
 /// Simple index type with which we can count sessions.
 pub type SessionIndex = u32;
-
-/// A trait for getting the currently elected validator set without coupling to the module that
-/// provides this information.
-pub trait CurrentElectedSet<ValidatorId> {
-	/// Returns the validator ids for the currently elected validator set.
-	fn current_elected_set() -> Vec<ValidatorId>;
-}

substrate/node/runtime/src/lib.rs

@@ -83,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
 	spec_version: 157,
-	impl_version: 158,
+	impl_version: 159,
 	apis: RUNTIME_API_VERSIONS,
 };
 
@@ -409,7 +409,6 @@ impl im_online::Trait for Runtime {
 	type Event = Event;
 	type SubmitTransaction = SubmitTransaction;
 	type ReportUnresponsiveness = Offences;
-	type CurrentElectedSet = staking::CurrentElectedStashAccounts<Runtime>;
 }
 
 impl offences::Trait for Runtime {

substrate/srml/aura/src/lib.rs

@@ -53,8 +53,8 @@ use support::{
 	decl_storage, decl_module, Parameter, storage::StorageValue, traits::{Get, FindAuthor},
 	ConsensusEngineId,
 };
-use app_crypto::AppPublic;
 use sr_primitives::{
+	RuntimeAppPublic,
 	traits::{SaturatedConversion, Saturating, Zero, Member, IsMember}, generic::DigestItem,
 };
 use timestamp::OnTimestampSet;
@@ -142,7 +142,7 @@ impl ProvideInherentData for InherentDataProvider {
 
 pub trait Trait: timestamp::Trait {
 	/// The identifier type for an authority.
-	type AuthorityId: Member + Parameter + AppPublic + Default;
+	type AuthorityId: Member + Parameter + RuntimeAppPublic + Default;
 }
 
 decl_storage! {

substrate/srml/authority-discovery/src/lib.rs

@@ -137,7 +137,6 @@ mod tests {
 	use sr_primitives::testing::{Header, UintAuthorityId};
 	use sr_primitives::traits::{ConvertInto, IdentityLookup, OpaqueKeys};
 	use sr_primitives::Perbill;
-	use sr_staking_primitives::CurrentElectedSet;
 	use support::{impl_outer_origin, parameter_types};
 
 	type AuthorityDiscovery = Module<Test>;
@@ -149,13 +148,6 @@ mod tests {
 
 	type AuthorityId = im_online::sr25519::AuthorityId;
 
-	pub struct DummyCurrentElectedSet<T>(std::marker::PhantomData<T>);
-	impl<T> CurrentElectedSet<T> for DummyCurrentElectedSet<T> {
-		fn current_elected_set() -> Vec<T> {
-			vec![]
-		}
-	}
-
 	pub struct TestOnSessionEnding;
 	impl session::OnSessionEnding<AuthorityId> for TestOnSessionEnding {
 		fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option<Vec<AuthorityId>> {
@@ -189,7 +181,6 @@ mod tests {
 			UncheckedExtrinsic<(), im_online::Call<Test>, (), ()>,
 		>;
 		type ReportUnresponsiveness = ();
-		type CurrentElectedSet = DummyCurrentElectedSet<AuthorityId>;
 	}
 
 	pub type BlockNumber = u64;

substrate/srml/authorship/Cargo.toml

@@ -14,7 +14,7 @@ sr-primitives = { path = "../../core/sr-primitives", default-features = false }
 support = { package = "srml-support", path = "../support", default-features = false }
 system = { package = "srml-system", path = "../system", default-features = false }
 runtime-io ={ package = "sr-io", path = "../../core/sr-io", default-features = false }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [features]
 default = ["std"]

substrate/srml/finality-tracker/Cargo.toml

@@ -12,7 +12,7 @@ rstd = { package = "sr-std", path = "../../core/sr-std", default-features = fals
 sr-primitives = { path = "../../core/sr-primitives", default-features = false }
 support = { package = "srml-support", path = "../support", default-features = false }
 srml-system = { path = "../system", default-features = false }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [dev-dependencies]
 primitives = { package = "substrate-primitives",  path = "../../core/primitives", default-features = false }

substrate/srml/im-online/Cargo.toml

@@ -17,6 +17,9 @@ sr-staking-primitives = { path = "../../core/sr-staking-primitives", default-fea
 support = { package = "srml-support", path = "../support", default-features = false }
 system = { package = "srml-system", path = "../system", default-features = false }
 
+[dev-dependencies]
+offchain = { package = "substrate-offchain", path = "../../core/offchain" }
+
 [features]
 default = ["std", "session/historical"]
 std = [

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

@@ -67,7 +67,10 @@
 // Ensure we're `no_std` when compiling for Wasm.
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use app_crypto::{AppPublic, RuntimeAppPublic};
+mod mock;
+mod tests;
+
+use app_crypto::RuntimeAppPublic;
 use codec::{Encode, Decode};
 use primitives::offchain::{OpaqueNetworkState, StorageKind};
 use rstd::prelude::*;
@@ -80,7 +83,7 @@ use sr_primitives::{
 	},
 };
 use sr_staking_primitives::{
-	SessionIndex, CurrentElectedSet,
+	SessionIndex,
 	offence::{ReportOffence, Offence, Kind},
 };
 use support::{
@@ -136,15 +139,15 @@ pub mod ed25519 {
 	pub type AuthorityId = app_ed25519::Public;
 }
 
-// The local storage database key under which the worker progress status
+/// The local storage database key under which the worker progress status
-// is tracked.
+/// is tracked.
 const DB_KEY: &[u8] = b"srml/im-online-worker-status";
 
-// It's important to persist the worker state, since e.g. the
+/// It's important to persist the worker state, since e.g. the
-// server could be restarted while starting the gossip process, but before
+/// server could be restarted while starting the gossip process, but before
-// finishing it. With every execution of the off-chain worker we check
+/// finishing it. With every execution of the off-chain worker we check
-// if we need to recover and resume gossipping or if there is already
+/// if we need to recover and resume gossipping or if there is already
-// another off-chain worker in the process of gossipping.
+/// another off-chain worker in the process of gossipping.
 #[derive(Encode, Decode, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "std", derive(Debug))]
 struct WorkerStatus<BlockNumber> {
@@ -152,7 +155,8 @@ struct WorkerStatus<BlockNumber> {
 	gossipping_at: BlockNumber,
 }
 
-// Error which may occur while executing the off-chain code.
+/// Error which may occur while executing the off-chain code.
+#[cfg_attr(feature = "std", derive(Debug))]
 enum OffchainErr {
 	DecodeWorkerStatus,
 	FailedSigning,
@@ -187,7 +191,7 @@ pub struct Heartbeat<BlockNumber>
 
 pub trait Trait: system::Trait + session::historical::Trait {
 	/// The identifier type for an authority.
-	type AuthorityId: Member + Parameter + AppPublic + RuntimeAppPublic + Default;
+	type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord;
 
 	/// The overarching event type.
 	type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
@@ -205,9 +209,6 @@ pub trait Trait: system::Trait + session::historical::Trait {
 			IdentificationTuple<Self>,
 			UnresponsivenessOffence<IdentificationTuple<Self>>,
 		>;
-
-	/// A type that returns a validator id from the current elected set of the era.
-	type CurrentElectedSet: CurrentElectedSet<<Self as session::Trait>::ValidatorId>;
 }
 
 decl_event!(
@@ -272,6 +273,10 @@ decl_module! {
 					&heartbeat.authority_index,
 					&network_state
 				);
+			} else if exists {
+				Err("Duplicated heartbeat.")?
+			} else {
+				Err("Non existent public key.")?
 			}
 		}
 
@@ -293,7 +298,7 @@ impl<T: Trait> Module<T> {
 		<ReceivedHeartbeats>::exists(&current_session, &authority_index)
 	}
 
-	fn offchain(now: T::BlockNumber) {
+	pub(crate) fn offchain(now: T::BlockNumber) {
 		let next_gossip = <GossipAt<T>>::get();
 		let check = Self::check_not_yet_gossipped(now, next_gossip);
 		let (curr_worker_status, not_yet_gossipped) = match check {
@@ -450,19 +455,16 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 		let current_session = <session::Module<T>>::current_index();
 
 		let keys = Keys::<T>::get();
-		let current_elected = T::CurrentElectedSet::current_elected_set();
+		let current_validators = <session::Module<T>>::validators();
 
-		// The invariant is that these two are of the same length.
+		for (auth_idx, validator_id) in current_validators.into_iter().enumerate() {
-		// TODO: What to do: Uncomment, ignore, a third option?
-		// assert_eq!(keys.len(), current_elected.len());
-
-		for (auth_idx, validator_id) in current_elected.into_iter().enumerate() {
 			let auth_idx = auth_idx as u32;
-			if !<ReceivedHeartbeats>::exists(&current_session, &auth_idx) {
+			let exists = <ReceivedHeartbeats>::exists(&current_session, &auth_idx);
+			if !exists {
 				let full_identification = T::FullIdentificationOf::convert(validator_id.clone())
 					.expect(
-						"we got the validator_id from current_elected;
+						"we got the validator_id from current_validators;
-						current_elected is set of currently elected validators;
+						current_validators is set of currently acting validators;
 						the mapping between the validator id and its full identification should be valid;
 						thus `FullIdentificationOf::convert` can't return `None`;
 						qed",
@@ -472,6 +474,10 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 			}
 		}
 
+		if unresponsive.is_empty() {
+			return;
+		}
+
 		let validator_set_count = keys.len() as u32;
 		let offence = UnresponsivenessOffence {
 			session_index: current_session,
@@ -533,6 +539,7 @@ impl<T: Trait> support::unsigned::ValidateUnsigned for Module<T> {
 }
 
 /// An offence that is filed if a validator didn't send a heartbeat message.
+#[cfg_attr(feature = "std", derive(Clone, Debug, PartialEq, Eq))]
 pub struct UnresponsivenessOffence<Offender> {
 	/// The current session index in which we report the unresponsive validators.
 	///
@@ -577,28 +584,3 @@ impl<Offender: Clone> Offence<Offender> for UnresponsivenessOffence<Offender> {
 		Perbill::from_parts(p as u32)
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use super::*;
-
-	#[test]
-	fn test_unresponsiveness_slash_fraction() {
-		// A single case of unresponsiveness is not slashed.
-		assert_eq!(
-			UnresponsivenessOffence::<()>::slash_fraction(1, 50),
-			Perbill::zero(),
-		);
-
-		assert_eq!(
-			UnresponsivenessOffence::<()>::slash_fraction(3, 50),
-			Perbill::from_parts(6000000), // 0.6%
-		);
-
-		// One third offline should be punished around 5%.
-		assert_eq!(
-			UnresponsivenessOffence::<()>::slash_fraction(17, 50),
-			Perbill::from_parts(48000000), // 4.8%
-		);
-	}
-}

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

@@ -0,0 +1,162 @@
+// Copyright 2019 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Substrate is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Test utilities
+
+#![cfg(test)]
+
+use std::cell::RefCell;
+
+use crate::{Module, Trait};
+use sr_primitives::Perbill;
+use sr_staking_primitives::{SessionIndex, offence::ReportOffence};
+use sr_primitives::testing::{Header, UintAuthorityId, TestXt};
+use sr_primitives::traits::{IdentityLookup, BlakeTwo256, ConvertInto};
+use primitives::{H256, Blake2Hasher};
+use support::{impl_outer_origin, impl_outer_dispatch, parameter_types};
+use {runtime_io, system};
+
+impl_outer_origin!{
+	pub enum Origin for Runtime {}
+}
+
+impl_outer_dispatch! {
+	pub enum Call for Runtime where origin: Origin {
+		imonline::ImOnline,
+	}
+}
+
+thread_local! {
+	pub static VALIDATORS: RefCell<Option<Vec<u64>>> = RefCell::new(Some(vec![1, 2, 3]));
+}
+
+pub struct TestOnSessionEnding;
+impl session::OnSessionEnding<u64> for TestOnSessionEnding {
+	fn on_session_ending(_ending_index: SessionIndex, _will_apply_at: SessionIndex)
+		-> Option<Vec<u64>>
+	{
+		VALIDATORS.with(|l| l.borrow_mut().take())
+	}
+}
+
+impl session::historical::OnSessionEnding<u64, u64> for TestOnSessionEnding {
+	fn on_session_ending(_ending_index: SessionIndex, _will_apply_at: SessionIndex)
+		-> Option<(Vec<u64>, Vec<(u64, u64)>)>
+	{
+		VALIDATORS.with(|l| l
+			.borrow_mut()
+			.take()
+			.map(|validators| {
+				let full_identification = validators.iter().map(|v| (*v, *v)).collect();
+				(validators, full_identification)
+			})
+		)
+	}
+}
+
+/// An extrinsic type used for tests.
+pub type Extrinsic = TestXt<Call, ()>;
+type SubmitTransaction = system::offchain::TransactionSubmitter<(), Call, Extrinsic>;
+type IdentificationTuple = (u64, u64);
+type Offence = crate::UnresponsivenessOffence<IdentificationTuple>;
+
+thread_local! {
+	pub static OFFENCES: RefCell<Vec<(Vec<u64>, Offence)>> = RefCell::new(vec![]);
+}
+
+/// A mock offence report handler.
+pub struct OffenceHandler;
+impl ReportOffence<u64, IdentificationTuple, Offence> for OffenceHandler {
+	fn report_offence(reporters: Vec<u64>, offence: Offence) {
+		OFFENCES.with(|l| l.borrow_mut().push((reporters, offence)));
+	}
+}
+
+pub fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> {
+	let t = system::GenesisConfig::default().build_storage::<Runtime>().unwrap();
+	t.into()
+}
+
+
+#[derive(Clone, PartialEq, Eq, Debug)]
+pub struct Runtime;
+
+parameter_types! {
+	pub const BlockHashCount: u64 = 250;
+	pub const MaximumBlockWeight: u32 = 1024;
+	pub const MaximumBlockLength: u32 = 2 * 1024;
+	pub const AvailableBlockRatio: Perbill = Perbill::one();
+}
+
+impl system::Trait for Runtime {
+	type Origin = Origin;
+	type Index = u64;
+	type BlockNumber = u64;
+	type Call = Call;
+	type Hash = H256;
+	type Hashing = BlakeTwo256;
+	type AccountId = u64;
+	type Lookup = IdentityLookup<Self::AccountId>;
+	type Header = Header;
+	type WeightMultiplierUpdate = ();
+	type Event = ();
+	type BlockHashCount = BlockHashCount;
+	type MaximumBlockWeight = MaximumBlockWeight;
+	type MaximumBlockLength = MaximumBlockLength;
+	type AvailableBlockRatio = AvailableBlockRatio;
+	type Version = ();
+}
+
+parameter_types! {
+	pub const Period: u64 = 1;
+	pub const Offset: u64 = 0;
+}
+
+impl session::Trait for Runtime {
+	type ShouldEndSession = session::PeriodicSessions<Period, Offset>;
+	type OnSessionEnding = session::historical::NoteHistoricalRoot<Runtime, TestOnSessionEnding>;
+	type SessionHandler = (ImOnline, );
+	type ValidatorId = u64;
+	type ValidatorIdOf = ConvertInto;
+	type Keys = UintAuthorityId;
+	type Event = ();
+	type SelectInitialValidators = ();
+}
+
+impl session::historical::Trait for Runtime {
+	type FullIdentification = u64;
+	type FullIdentificationOf = ConvertInto;
+}
+
+impl Trait for Runtime {
+	type AuthorityId = UintAuthorityId;
+	type Event = ();
+	type Call = Call;
+	type SubmitTransaction = SubmitTransaction;
+	type ReportUnresponsiveness = OffenceHandler;
+}
+
+/// Im Online module.
+pub type ImOnline = Module<Runtime>;
+pub type System = system::Module<Runtime>;
+pub type Session = session::Module<Runtime>;
+
+pub fn advance_session() {
+	let now = System::block_number();
+	System::set_block_number(now + 1);
+	Session::rotate_session();
+	assert_eq!(Session::current_index(), (now / Period::get()) as u32);
+}

substrate/srml/im-online/src/tests.rs

@@ -0,0 +1,218 @@
+// Copyright 2019 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Substrate is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Tests for the im-online module.
+
+#![cfg(test)]
+
+use super::*;
+use crate::mock::*;
+use offchain::testing::TestOffchainExt;
+use primitives::offchain::OpaquePeerId;
+use runtime_io::with_externalities;
+use support::{dispatch, assert_noop};
+use sr_primitives::testing::UintAuthorityId;
+
+
+#[test]
+fn test_unresponsiveness_slash_fraction() {
+	// A single case of unresponsiveness is not slashed.
+	assert_eq!(
+		UnresponsivenessOffence::<()>::slash_fraction(1, 50),
+		Perbill::zero(),
+	);
+
+	assert_eq!(
+		UnresponsivenessOffence::<()>::slash_fraction(3, 50),
+		Perbill::from_parts(6000000), // 0.6%
+	);
+
+	// One third offline should be punished around 5%.
+	assert_eq!(
+		UnresponsivenessOffence::<()>::slash_fraction(17, 50),
+		Perbill::from_parts(48000000), // 4.8%
+	);
+}
+
+#[test]
+fn should_report_offline_validators() {
+	with_externalities(&mut new_test_ext(), || {
+		// given
+		let block = 1;
+		System::set_block_number(block);
+		// buffer new validators
+		Session::rotate_session();
+		// enact the change and buffer another one
+		let validators = vec![1, 2, 3, 4, 5, 6];
+		VALIDATORS.with(|l| *l.borrow_mut() = Some(validators.clone()));
+		Session::rotate_session();
+
+		// when
+		// we end current session and start the next one
+		Session::rotate_session();
+
+		// then
+		let offences = OFFENCES.with(|l| l.replace(vec![]));
+		assert_eq!(offences, vec![
+			(vec![], UnresponsivenessOffence {
+				session_index: 2,
+				validator_set_count: 3,
+				offenders: vec![
+					(1, 1),
+					(2, 2),
+					(3, 3),
+				],
+			})
+		]);
+
+		// should not report when heartbeat is sent
+		for (idx, v) in validators.into_iter().take(4).enumerate() {
+			let _ = heartbeat(block, 3, idx as u32, v.into()).unwrap();
+		}
+		Session::rotate_session();
+
+		// then
+		let offences = OFFENCES.with(|l| l.replace(vec![]));
+		assert_eq!(offences, vec![
+			(vec![], UnresponsivenessOffence {
+				session_index: 3,
+				validator_set_count: 6,
+				offenders: vec![
+					(5, 5),
+					(6, 6),
+				],
+			})
+		]);
+	});
+}
+
+fn heartbeat(
+	block_number: u64,
+	session_index: u32,
+	authority_index: u32,
+	id: UintAuthorityId,
+) -> dispatch::Result {
+	let heartbeat = Heartbeat {
+		block_number,
+		network_state: OpaqueNetworkState {
+			peer_id: OpaquePeerId(vec![1]),
+			external_addresses: vec![],
+		},
+		session_index,
+		authority_index,
+	};
+	let signature = id.sign(&heartbeat.encode()).unwrap();
+
+	ImOnline::heartbeat(
+		Origin::system(system::RawOrigin::None),
+		heartbeat,
+		signature
+	)
+}
+
+#[test]
+fn should_mark_online_validator_when_heartbeat_is_received() {
+	with_externalities(&mut new_test_ext(), || {
+		advance_session();
+		// given
+		VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6]));
+		assert_eq!(Session::validators(), Vec::<u64>::new());
+		// enact the change and buffer another one
+		advance_session();
+
+		assert_eq!(Session::current_index(), 2);
+		assert_eq!(Session::validators(), vec![1, 2, 3]);
+
+		assert!(!ImOnline::is_online_in_current_session(0));
+		assert!(!ImOnline::is_online_in_current_session(1));
+		assert!(!ImOnline::is_online_in_current_session(2));
+
+		// when
+		let _ = heartbeat(1, 2, 0, 1.into()).unwrap();
+
+		// then
+		assert!(ImOnline::is_online_in_current_session(0));
+		assert!(!ImOnline::is_online_in_current_session(1));
+		assert!(!ImOnline::is_online_in_current_session(2));
+
+		// and when
+		let _ = heartbeat(1, 2, 2, 3.into()).unwrap();
+
+		// then
+		assert!(ImOnline::is_online_in_current_session(0));
+		assert!(!ImOnline::is_online_in_current_session(1));
+		assert!(ImOnline::is_online_in_current_session(2));
+	});
+}
+
+#[test]
+fn late_heartbeat_should_fail() {
+	with_externalities(&mut new_test_ext(), || {
+		advance_session();
+		// given
+		VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 4, 4, 5, 6]));
+		assert_eq!(Session::validators(), Vec::<u64>::new());
+		// enact the change and buffer another one
+		advance_session();
+
+		assert_eq!(Session::current_index(), 2);
+		assert_eq!(Session::validators(), vec![1, 2, 3]);
+
+		// when
+		assert_noop!(heartbeat(1, 3, 0, 1.into()), "Outdated heartbeat received.");
+		assert_noop!(heartbeat(1, 1, 0, 1.into()), "Outdated heartbeat received.");
+	});
+}
+
+#[test]
+fn should_generate_heartbeats() {
+	let mut ext = new_test_ext();
+	let (offchain, state) = TestOffchainExt::new();
+	ext.set_offchain_externalities(offchain);
+
+	with_externalities(&mut ext, || {
+		// given
+		let block = 1;
+		System::set_block_number(block);
+		// buffer new validators
+		Session::rotate_session();
+		// enact the change and buffer another one
+		VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6]));
+		Session::rotate_session();
+
+		// when
+		UintAuthorityId::set_all_keys(vec![0, 1, 2]);
+		ImOnline::offchain(2);
+
+		// then
+		let transaction = state.write().transactions.pop().unwrap();
+		// All validators have `0` as their session key, so we generate 3 transactions.
+		assert_eq!(state.read().transactions.len(), 2);
+		// check stuff about the transaction.
+		let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap();
+		let heartbeat = match ex.1 {
+			crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h,
+			e => panic!("Unexpected call: {:?}", e),
+		};
+
+		assert_eq!(heartbeat, Heartbeat {
+			block_number: 2,
+			network_state: runtime_io::network_state().unwrap(),
+			session_index: 2,
+			authority_index: 2,
+		});
+	});
+}

substrate/srml/session/Cargo.toml

@@ -16,7 +16,7 @@ system = { package = "srml-system", path = "../system", default-features = false
 timestamp = { package = "srml-timestamp", path = "../timestamp", default-features = false }
 substrate-trie = { path = "../../core/trie", default-features = false, optional = true }
 runtime-io ={ package = "sr-io", path = "../../core/sr-io", default-features = false }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [dev-dependencies]
 primitives = { package = "substrate-primitives",  path = "../../core/primitives" }

substrate/srml/session/src/lib.rs

@@ -121,7 +121,7 @@
 
 use rstd::{prelude::*, marker::PhantomData, ops::{Sub, Rem}};
 use codec::Decode;
-use sr_primitives::{KeyTypeId, AppKey};
+use sr_primitives::{KeyTypeId, RuntimeAppPublic};
 use sr_primitives::weights::SimpleDispatchInfo;
 use sr_primitives::traits::{Convert, Zero, Member, OpaqueKeys};
 use sr_staking_primitives::SessionIndex;
@@ -222,7 +222,7 @@ pub trait SessionHandler<ValidatorId> {
 /// A session handler for specific key type.
 pub trait OneSessionHandler<ValidatorId> {
 	/// The key type expected.
-	type Key: Decode + Default + AppKey;
+	type Key: Decode + Default + RuntimeAppPublic;
 
 	fn on_genesis_session<'a, I: 'a>(validators: I)
 		where I: Iterator<Item=(&'a ValidatorId, Self::Key)>, ValidatorId: 'a;
@@ -262,7 +262,7 @@ impl<AId> SessionHandler<AId> for Tuple {
 		for_tuples!(
 			#(
 				let our_keys: Box<dyn Iterator<Item=_>> = Box::new(validators.iter()
-					.map(|k| (&k.0, k.1.get::<Tuple::Key>(<Tuple::Key as AppKey>::ID)
+					.map(|k| (&k.0, k.1.get::<Tuple::Key>(<Tuple::Key as RuntimeAppPublic>::ID)
 						.unwrap_or_default())));
 
 				Tuple::on_genesis_session(our_keys);
@@ -278,10 +278,10 @@ impl<AId> SessionHandler<AId> for Tuple {
 		for_tuples!(
 			#(
 				let our_keys: Box<dyn Iterator<Item=_>> = Box::new(validators.iter()
-					.map(|k| (&k.0, k.1.get::<Tuple::Key>(<Tuple::Key as AppKey>::ID)
+					.map(|k| (&k.0, k.1.get::<Tuple::Key>(<Tuple::Key as RuntimeAppPublic>::ID)
 						.unwrap_or_default())));
 				let queued_keys: Box<dyn Iterator<Item=_>> = Box::new(queued_validators.iter()
-					.map(|k| (&k.0, k.1.get::<Tuple::Key>(<Tuple::Key as AppKey>::ID)
+					.map(|k| (&k.0, k.1.get::<Tuple::Key>(<Tuple::Key as RuntimeAppPublic>::ID)
 						.unwrap_or_default())));
 				Tuple::on_new_session(changed, our_keys, queued_keys);
 			)*

substrate/srml/staking/src/lib.rs

@@ -269,7 +269,7 @@ use sr_primitives::traits::{
 };
 use phragmen::{elect, equalize, Support, SupportMap, ExtendedBalance, ACCURACY};
 use sr_staking_primitives::{
-	SessionIndex, CurrentElectedSet,
+	SessionIndex,
 	offence::{OnOffenceHandler, OffenceDetails, Offence, ReportOffence},
 };
 #[cfg(feature = "std")]
@@ -1586,12 +1586,3 @@ impl<T, Reporter, Offender, R, O> ReportOffence<Reporter, Offender, O>
 		}
 	}
 }
-
-/// Returns the currently elected validator set represented by their stash accounts.
-pub struct CurrentElectedStashAccounts<T>(rstd::marker::PhantomData<T>);
-
-impl<T: Trait> CurrentElectedSet<T::AccountId> for CurrentElectedStashAccounts<T> {
-	fn current_elected_set() -> Vec<T::AccountId> {
-		<Module<T>>::current_elected()
-	}
-}

substrate/srml/support/Cargo.toml

@@ -17,7 +17,7 @@ srml-support-procedural = { package = "srml-support-procedural", path = "./proce
 paste = "0.1"
 once_cell = { version = "0.1.6", default-features = false, optional = true }
 bitmask = { version = "0.5", default-features = false }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [dev-dependencies]
 pretty_assertions = "0.6.1"

substrate/srml/system/Cargo.toml

@@ -14,7 +14,7 @@ runtime-io ={ package = "sr-io", path = "../../core/sr-io", default-features = f
 sr-primitives = { path = "../../core/sr-primitives", default-features = false }
 sr-version = { path = "../../core/sr-version", default-features = false }
 support = { package = "srml-support", path = "../support", default-features = false }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [dev-dependencies]
 criterion = "0.2"

substrate/srml/timestamp/Cargo.toml

@@ -12,7 +12,7 @@ sr-primitives = { path = "../../core/sr-primitives", default-features = false }
 inherents = { package = "substrate-inherents", path = "../../core/inherents", default-features = false }
 support = { package = "srml-support", path = "../support", default-features = false }
 system = { package = "srml-system", path = "../system", default-features = false }
-impl-trait-for-tuples = "0.1"
+impl-trait-for-tuples = "0.1.1"
 
 [dev-dependencies]
 runtime-io ={ package = "sr-io", path = "../../core/sr-io" }