Add events for im_online (#3991)

* Add AllGood event for im_online

* Another event just in case.

* Bump runtime

substrate/core/network/src/protocol.rs

@@ -35,7 +35,6 @@ use sr_primitives::traits::{
 };
 use message::{BlockAnnounce, BlockAttributes, Direction, FromBlock, Message, RequestId};
 use message::generic::{Message as GenericMessage, ConsensusMessage};
-use event::Event;
 use consensus_gossip::{ConsensusGossip, MessageRecipient as GossipMessageRecipient};
 use light_dispatch::{LightDispatch, LightDispatchNetwork, RequestData};
 use specialization::NetworkSpecialization;

substrate/core/network/src/protocol/specialization.rs

@@ -43,7 +43,7 @@ pub trait NetworkSpecialization<B: BlockT>: Send + Sync + 'static {
 
 	/// Called when a network-specific event arrives.
 	#[deprecated(note = "This method is never called; please use `with_dht_event_tx` when building the service")]
-	fn on_event(&mut self, event: Event) {}
+	fn on_event(&mut self, _event: Event) {}
 
 	/// Called on abort.
 	#[deprecated(note = "This method is never called; aborting corresponds to dropping the object")]

substrate/core/sr-primitives/src/generic/checked_extrinsic.rs

@@ -36,8 +36,7 @@ pub struct CheckedExtrinsic<AccountId, Call, Extra> {
 	pub function: Call,
 }
 
-impl<AccountId, Call, Extra, Origin> traits::Applyable
-for
+impl<AccountId, Call, Extra, Origin> traits::Applyable for
 	CheckedExtrinsic<AccountId, Call, Extra>
 where
 	AccountId: Member + MaybeDisplay,

substrate/node/runtime/src/lib.rs

@@ -81,7 +81,7 @@ 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: 190,
+	spec_version: 191,
 	impl_version: 191,
 	apis: RUNTIME_API_VERSIONS,
 };

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

@@ -37,7 +37,7 @@
 //!
 //! ### Public Functions
 //!
-//! - `is_online_in_current_session` - True if the validator sent a heartbeat in the current session.
+//! - `is_online` - True if the validator sent a heartbeat in the current session.
 //!
 //! ## Usage
 //!
@@ -52,7 +52,7 @@
 //! 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
 //! 		pub fn is_online(origin, authority_index: u32) -> Result {
 //! 			let _sender = ensure_signed(origin)?;
-//! 			let _is_online = <im_online::Module<T>>::is_online_in_current_session(authority_index);
+//! 			let _is_online = <im_online::Module<T>>::is_online(authority_index);
 //! 			Ok(())
 //! 		}
 //! 	}
@@ -200,9 +200,14 @@ pub trait Trait: system::Trait + session::historical::Trait {
 decl_event!(
 	pub enum Event<T> where
 		<T as Trait>::AuthorityId,
+		IdentificationTuple = IdentificationTuple<T>,
 	{
 		/// A new heartbeat was received from `AuthorityId`
 		HeartbeatReceived(AuthorityId),
+		/// At the end of the session, no offence was committed.
+		AllGood,
+		/// At the end of the session, at least once validator was found to be offline.
+		SomeOffline(Vec<IdentificationTuple>),
 	}
 );
 
@@ -217,7 +222,7 @@ decl_storage! {
 		/// 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>;
+			blake2_256(AuthIndex) => Option<Vec<u8>>;
 
 		/// For each session index, we keep a mapping of `T::ValidatorId` to the
 		/// number of blocks authored by the given authority.
@@ -249,8 +254,8 @@ decl_module! {
 				&heartbeat.authority_index
 			);
 			let keys = Keys::<T>::get();
-			let public = keys.get(heartbeat.authority_index as usize);
-			if let (true, Some(public)) = (!exists, public) {
+			let maybe_public = keys.get(heartbeat.authority_index as usize);
+			if let (false, Some(public)) = (exists, maybe_public) {
 				let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| {
 					public.verify(&encoded_heartbeat, &signature)
 				});
@@ -300,7 +305,7 @@ impl<T: Trait> Module<T> {
 	/// `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 {
+	pub fn is_online(authority_index: AuthIndex) -> bool {
 		let current_validators = <session::Module<T>>::validators();
 
 		if authority_index >= current_validators.len() as u32 {
@@ -309,10 +314,10 @@ impl<T: Trait> Module<T> {
 
 		let authority = &current_validators[authority_index as usize];
 
-		Self::is_online_in_current_session_aux(authority_index, authority)
+		Self::is_online_aux(authority_index, authority)
 	}
 
-	fn is_online_in_current_session_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool {
+	fn is_online_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool {
 		let current_session = <session::Module<T>>::current_index();
 
 		<ReceivedHeartbeats>::exists(&current_session, &authority_index) ||
@@ -507,27 +512,16 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 	}
 
 	fn on_before_session_ending() {
-		let mut unresponsive = vec![];
-
-		let current_session = <session::Module<T>>::current_index();
-
+		let session_index = <session::Module<T>>::current_index();
 		let keys = Keys::<T>::get();
 		let current_validators = <session::Module<T>>::validators();
 
-		for (auth_idx, validator_id) in current_validators.into_iter().enumerate() {
-			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;
-						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",
-					);
-
-				unresponsive.push((validator_id, full_identification));
-			}
-		}
+		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))
+			).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
@@ -535,18 +529,15 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 		<ReceivedHeartbeats>::remove_prefix(&<session::Module<T>>::current_index());
 		<AuthoredBlocks<T>>::remove_prefix(&<session::Module<T>>::current_index());
 
-		if unresponsive.is_empty() {
-			return;
+		if offenders.is_empty() {
+			Self::deposit_event(RawEvent::AllGood);
+		} else {
+			Self::deposit_event(RawEvent::SomeOffline(offenders.clone()));
+
+			let validator_set_count = keys.len() as u32;
+			let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders };
+			T::ReportUnresponsiveness::report_offence(vec![], offence);
 		}
-
-		let validator_set_count = keys.len() as u32;
-		let offence = UnresponsivenessOffence {
-			session_index: current_session,
-			validator_set_count,
-			offenders: unresponsive,
-		};
-
-		T::ReportUnresponsiveness::report_offence(vec![], offence);
 	}
 
 	fn on_disabled(_i: usize) {
@@ -559,7 +550,7 @@ impl<T: Trait> support::unsigned::ValidateUnsigned for Module<T> {
 
 	fn validate_unsigned(call: &Self::Call) -> TransactionValidity {
 		if let Call::heartbeat(heartbeat, signature) = call {
-			if <Module<T>>::is_online_in_current_session(heartbeat.authority_index) {
+			if <Module<T>>::is_online(heartbeat.authority_index) {
 				// we already received a heartbeat for this authority
 				return InvalidTransaction::Stale.into();
 			}

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

@@ -134,25 +134,25 @@ fn should_mark_online_validator_when_heartbeat_is_received() {
 		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));
+		assert!(!ImOnline::is_online(0));
+		assert!(!ImOnline::is_online(1));
+		assert!(!ImOnline::is_online(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));
+		assert!(ImOnline::is_online(0));
+		assert!(!ImOnline::is_online(1));
+		assert!(!ImOnline::is_online(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));
+		assert!(ImOnline::is_online(0));
+		assert!(!ImOnline::is_online(1));
+		assert!(ImOnline::is_online(2));
 	});
 }
 
@@ -233,14 +233,14 @@ fn should_cleanup_received_heartbeats_on_session_end() {
 		let _ = heartbeat(1, 2, 0, 1.into()).unwrap();
 
 		// the heartbeat is stored
-		assert!(!ImOnline::received_heartbeats(&2, &0).is_empty());
+		assert!(!ImOnline::received_heartbeats(&2, &0).is_none());
 
 		advance_session();
 
 		// after the session has ended we have already processed the heartbeat
 		// message, so any messages received on the previous session should have
 		// been pruned.
-		assert!(ImOnline::received_heartbeats(&2, &0).is_empty());
+		assert!(ImOnline::received_heartbeats(&2, &0).is_none());
 	});
 }
 
@@ -260,7 +260,7 @@ fn should_mark_online_validator_when_block_is_authored() {
 		assert_eq!(Session::validators(), vec![1, 2, 3]);
 
 		for i in 0..3 {
-			assert!(!ImOnline::is_online_in_current_session(i));
+			assert!(!ImOnline::is_online(i));
 		}
 
 		// when
@@ -268,8 +268,8 @@ fn should_mark_online_validator_when_block_is_authored() {
 		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));
+		assert!(ImOnline::is_online(0));
+		assert!(ImOnline::is_online(1));
+		assert!(!ImOnline::is_online(2));
 	});
 }