Offline/Slashing improvements (#1665)

* Initial logic

* Fix tests

* Don't punish at all when everyone skipped

* Typo

* Fix tests.

* Update srml/aura/src/lib.rs

Co-Authored-By: gavofyork <github@gavwood.com>

* Simplification.

* Bump runtime verions

substrate/node/runtime/src/lib.rs

@@ -66,8 +66,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("node"),
 	impl_name: create_runtime_str!("substrate-node"),
 	authoring_version: 10,
-	spec_version: 18,
-	impl_version: 18,
+	spec_version: 19,
+	impl_version: 19,
 	apis: RUNTIME_API_VERSIONS,
 };
 
@@ -203,7 +203,7 @@ construct_runtime!(
 		Indices: indices,
 		Balances: balances,
 		Session: session,
-		Staking: staking,
+		Staking: staking::{default, OfflineWorker},
 		Democracy: democracy,
 		Council: council::{Module, Call, Storage, Event<T>},
 		CouncilVoting: council_voting,

substrate/srml/aura/src/lib.rs

@@ -152,28 +152,13 @@ impl AuraReport {
 	pub fn punish<F>(&self, validator_count: usize, mut punish_with: F)
 		where F: FnMut(usize, usize)
 	{
-		let start_slot = self.start_slot % validator_count;
-
-		// the number of times everyone was skipped.
-		let skipped_all = self.skipped / validator_count;
-		// the number of validators who were skipped once after that.
-		let skipped_after = self.skipped % validator_count;
-
-		let iter = (start_slot..validator_count).into_iter()
-			.chain(0..start_slot)
-			.enumerate();
-
-		for (rel_index, actual_index) in iter {
-			let slash_count = skipped_all + if rel_index < skipped_after {
-				1
-			} else {
-				// avoid iterating over all authorities when skipping a couple.
-				if skipped_all == 0 { break }
-				0
-			};
-
-			if slash_count > 0 {
-				punish_with(actual_index, slash_count);
+		// If all validators have been skipped, then it implies some sort of 
+		// systematic problem common to all rather than a minority of validators
+		// unfulfilling their specific duties. In this case, it doesn't make
+		// sense to punish anyone, so we guard against it.
+		if self.skipped < validator_count {
+			for index in 0..self.skipped {
+				punish_with((self.start_slot + index) % validator_count, 1);
 			}
 		}
 	}

substrate/srml/aura/src/tests.rs

@@ -28,22 +28,34 @@ use crate::{AuraReport, HandleReport};
 #[test]
 fn aura_report_gets_skipped_correctly() {
 	let mut report = AuraReport {
-		start_slot: 0,
-		skipped: 30,
+		start_slot: 3,
+		skipped: 15,
 	};
 
 	let mut validators = vec![0; 10];
 	report.punish(10, |idx, count| validators[idx] += count);
+	assert_eq!(validators, vec![0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
 
-	assert_eq!(validators, vec![3; 10]);
+	let mut validators = vec![0; 10];
+	report.skipped = 5;
+	report.punish(10, |idx, count| validators[idx] += count);
+	assert_eq!(validators, vec![0, 0, 0, 1, 1, 1, 1, 1, 0, 0]);
+
+	let mut validators = vec![0; 10];
+	report.start_slot = 8;
+	report.punish(10, |idx, count| validators[idx] += count);
+	assert_eq!(validators, vec![1, 1, 1, 0, 0, 0, 0, 0, 1, 1]);
 
 	let mut validators = vec![0; 4];
+	report.start_slot = 1;
+	report.skipped = 3;
 	report.punish(4, |idx, count| validators[idx] += count);
-	assert_eq!(validators, vec![8, 8, 7, 7]);
+	assert_eq!(validators, vec![0, 1, 1, 1]);
 
+	let mut validators = vec![0; 4];
 	report.start_slot = 2;
 	report.punish(4, |idx, count| validators[idx] += count);
-	assert_eq!(validators, vec![15, 15, 15, 15]);
+	assert_eq!(validators, vec![1, 0, 1, 1]);
 }
 
 #[test]

substrate/srml/staking/src/lib.rs

@@ -57,6 +57,7 @@ mod mock;
 
 mod tests;
 
+const RECENT_OFFLINE_COUNT: usize = 32;
 const DEFAULT_MINIMUM_VALIDATOR_COUNT: u32 = 4;
 
 #[derive(PartialEq, Clone)]
@@ -295,6 +296,9 @@ decl_storage! {
 
 		/// We are forcing a new era.
 		pub ForcingNewEra get(forcing_new_era): Option<()>;
+
+		/// Most recent `RECENT_OFFLINE_COUNT` instances. (who it was, when it was reported, how many instances they were offline for).
+		pub RecentlyOffline get(recently_offline): Vec<(T::AccountId, T::BlockNumber, u32)>;
 	}
 }
 
@@ -510,9 +514,23 @@ impl<T: Trait> Module<T> {
 
 		let slash_count = Self::slash_count(&v);
 		let new_slash_count = slash_count + count as u32;
-		<SlashCount<T>>::insert(v.clone(), new_slash_count);
+		<SlashCount<T>>::insert(&v, new_slash_count);
 		let grace = Self::offline_slash_grace();
 
+		if RECENT_OFFLINE_COUNT > 0 {
+			let item = (v.clone(), <system::Module<T>>::block_number(), count as u32);
+			<RecentlyOffline<T>>::mutate(|v| if v.len() >= RECENT_OFFLINE_COUNT {
+				let index = v.iter()
+					.enumerate()
+					.min_by_key(|(_, (_, block, _))| block)
+					.expect("v is non-empty; qed")
+					.0;
+				v[index] = item;
+			} else {
+				v.push(item);
+			});
+		}
+
 		let event = if new_slash_count > grace {
 			let slash = {
 				let base_slash = Self::current_offline_slash();