im-online: account for block authorship (#3973)

* im-online: account for block authorship

* im-online: add test for block authorship onlineness

* im-online: cleanup

* im-online: fix test

substrate/Cargo.lock

@@ -4305,6 +4305,7 @@ dependencies = [
  "sr-primitives 2.0.0",
  "sr-staking-primitives 2.0.0",
  "sr-std 2.0.0",
+ "srml-authorship 0.1.0",
  "srml-session 2.0.0",
  "srml-support 2.0.0",
  "srml-system 2.0.0",

substrate/node/runtime/src/lib.rs

@@ -206,7 +206,7 @@ impl authorship::Trait for Runtime {
 	type FindAuthor = session::FindAccountFromAuthorIndex<Self, Babe>;
 	type UncleGenerations = UncleGenerations;
 	type FilterUncle = ();
-	type EventHandler = Staking;
+	type EventHandler = (Staking, ImOnline);
 }
 
 impl_opaque_keys! {

substrate/srml/im-online/Cargo.toml

@@ -6,6 +6,7 @@ edition = "2018"
 
 [dependencies]
 app-crypto = { package = "substrate-application-crypto", path = "../../core/application-crypto", default-features = false }
+authorship = { package = "srml-authorship", path = "../authorship", 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 }
@@ -24,6 +25,7 @@ offchain = { package = "substrate-offchain", path = "../../core/offchain" }
 default = ["std", "session/historical"]
 std = [
 	"app-crypto/std",
+	"authorship/std",
 	"codec/std",
 	"primitives/std",
 	"rstd/std",

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

@@ -214,10 +214,15 @@ decl_storage! {
 		/// The current set of keys that may issue a heartbeat.
 		Keys get(fn keys): Vec<T::AuthorityId>;
 
-		/// For each session index we keep a mapping of `AuthorityId`
+		/// For each session index, we keep a mapping of `AuthIndex`
 		/// to `offchain::OpaqueNetworkState`.
 		ReceivedHeartbeats get(fn received_heartbeats): double_map SessionIndex,
 			blake2_256(AuthIndex) => Vec<u8>;
+
+		/// For each session index, we keep a mapping of `T::ValidatorId` to the
+		/// number of blocks authored by the given authority.
+		AuthoredBlocks get(fn authored_blocks): double_map SessionIndex,
+			blake2_256(T::ValidatorId) => u32;
 	}
 	add_extra_genesis {
 		config(keys): Vec<T::AuthorityId>;
@@ -278,14 +283,63 @@ decl_module! {
 	}
 }
 
+/// Keep track of number of authored blocks per authority, uncles are counted as
+/// well since they're a valid proof of onlineness.
+impl<T: Trait + authorship::Trait> authorship::EventHandler<T::ValidatorId, T::BlockNumber> for Module<T> {
+	fn note_author(author: T::ValidatorId) {
+		Self::note_authorship(author);
+	}
+
+	fn note_uncle(author: T::ValidatorId, _age: T::BlockNumber) {
+		Self::note_authorship(author);
+	}
+}
+
 impl<T: Trait> Module<T> {
+	/// Returns `true` if a heartbeat has been received for the authority at
+	/// `authority_index` in the authorities series or if the authority has
+	/// authored at least one block, during the current session. Otherwise
+	/// `false`.
+	pub fn is_online_in_current_session(authority_index: AuthIndex) -> bool {
+		let current_validators = <session::Module<T>>::validators();
+
+		if authority_index >= current_validators.len() as u32 {
+			return false;
+		}
+
+		let authority = &current_validators[authority_index as usize];
+
+		Self::is_online_in_current_session_aux(authority_index, authority)
+	}
+
+	fn is_online_in_current_session_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool {
+		let current_session = <session::Module<T>>::current_index();
+
+		<ReceivedHeartbeats>::exists(&current_session, &authority_index) ||
+			<AuthoredBlocks<T>>::get(
+				&current_session,
+				authority,
+			) != 0
+	}
+
 	/// 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 is_online_in_current_session(authority_index: AuthIndex) -> bool {
+	pub fn received_heartbeat_in_current_session(authority_index: AuthIndex) -> bool {
 		let current_session = <session::Module<T>>::current_index();
 		<ReceivedHeartbeats>::exists(&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 = <session::Module<T>>::current_index();
+
+		<AuthoredBlocks<T>>::mutate(
+			&current_session,
+			author,
+			|authored| *authored += 1,
+		);
+	}
+
 	pub(crate) fn offchain(now: T::BlockNumber) {
 		let next_gossip = <GossipAt<T>>::get();
 		let check = Self::check_not_yet_gossipped(now, next_gossip);
@@ -461,9 +515,7 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 		let current_validators = <session::Module<T>>::validators();
 
 		for (auth_idx, validator_id) in current_validators.into_iter().enumerate() {
-			let auth_idx = auth_idx as u32;
-			let exists = <ReceivedHeartbeats>::exists(&current_session, &auth_idx);
-			if !exists {
+			if !Self::is_online_in_current_session_aux(auth_idx as u32, &validator_id) {
 				let full_identification = T::FullIdentificationOf::convert(validator_id.clone())
 					.expect(
 						"we got the validator_id from current_validators;
@@ -477,6 +529,12 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<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(&<session::Module<T>>::current_index());
+		<AuthoredBlocks<T>>::remove_prefix(&<session::Module<T>>::current_index());
+
 		if unresponsive.is_empty() {
 			return;
 		}
@@ -489,10 +547,6 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 		};
 
 		T::ReportUnresponsiveness::report_offence(vec![], offence);
-
-		// Remove all received heartbeats from the current session, they have
-		// already been processed and won't be needed anymore.
-		<ReceivedHeartbeats>::remove_prefix(&<session::Module<T>>::current_index());
 	}
 
 	fn on_disabled(_i: usize) {

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

@@ -145,6 +145,17 @@ impl session::historical::Trait for Runtime {
 	type FullIdentificationOf = ConvertInto;
 }
 
+parameter_types! {
+	pub const UncleGenerations: u32 = 5;
+}
+
+impl authorship::Trait for Runtime {
+	type FindAuthor = ();
+	type UncleGenerations = UncleGenerations;
+	type FilterUncle = ();
+	type EventHandler = ImOnline;
+}
+
 impl Trait for Runtime {
 	type AuthorityId = UintAuthorityId;
 	type Event = ();

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

@@ -243,3 +243,33 @@ fn should_cleanup_received_heartbeats_on_session_end() {
 		assert!(ImOnline::received_heartbeats(&2, &0).is_empty());
 	});
 }
+
+#[test]
+fn should_mark_online_validator_when_block_is_authored() {
+	use authorship::EventHandler;
+
+	new_test_ext().execute_with(|| {
+		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]);
+
+		for i in 0..3 {
+			assert!(!ImOnline::is_online_in_current_session(i));
+		}
+
+		// when
+		ImOnline::note_author(1);
+		ImOnline::note_uncle(2, 0);
+
+		// 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));
+	});
+}