srml: im-online: fix received heartbeats pruning (#3724)

* srml: im-online: fix pruning of received heartbeats

* srml: im-online: add test for received heartbeats pruning

* srml: im-online: remove unused variables from test

* node: bump spec_version

substrate/node/runtime/src/lib.rs

@@ -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: 165,
-	impl_version: 167,
+	spec_version: 166,
+	impl_version: 166,
 	apis: RUNTIME_API_VERSIONS,
 };
 

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

@@ -437,9 +437,6 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
 	fn on_new_session<'a, I: 'a>(_changed: bool, validators: I, _queued_validators: I)
 		where I: Iterator<Item=(&'a T::AccountId, T::AuthorityId)>
 	{
-		// Reset heartbeats
-		<ReceivedHeartbeats>::remove_prefix(&<session::Module<T>>::current_index());
-
 		// Tell the offchain worker to start making the next session's heartbeats.
 		<GossipAt<T>>::put(<system::Module<T>>::block_number());
 
@@ -484,6 +481,10 @@ 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/tests.rs

@@ -216,3 +216,32 @@ fn should_generate_heartbeats() {
 		});
 	});
 }
+
+#[test]
+fn should_cleanup_received_heartbeats_on_session_end() {
+	with_externalities(&mut new_test_ext(), || {
+		advance_session();
+
+		VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3]));
+		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]);
+
+		// send an heartbeat from authority id 0 at session 2
+		let _ = heartbeat(1, 2, 0, 1.into()).unwrap();
+
+		// the heartbeat is stored
+		assert!(!ImOnline::received_heartbeats(&2, &0).is_empty());
+
+		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());
+	});
+}