More robust punishment (#3952)

* Introduce new option "always force new era".

* Take appropriate action, even for small offences.

- Deselect the offender in all circumstances
- Ensure that deselection forces a new era
- Ensure that forcing a new era works with the always-forcing.

* Bump runtime

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

substrate/srml/staking/src/lib.rs

@@ -536,6 +536,8 @@ pub enum Forcing {
 	ForceNew,
 	/// Avoid a new era indefinitely.
 	ForceNone,
+	/// Force a new era at the end of all sessions indefinitely.
+	ForceAlways,
 }
 
 impl Default for Forcing {
@@ -956,7 +958,7 @@ decl_module! {
 		}
 
 		/// The ideal number of validators.
-		#[weight = SimpleDispatchInfo::FixedOperational(150_000)]
+		#[weight = SimpleDispatchInfo::FreeOperational]
 		fn set_validator_count(origin, #[compact] new: u32) {
 			ensure_root(origin)?;
 			ValidatorCount::put(new);
@@ -969,7 +971,7 @@ decl_module! {
 		/// # <weight>
 		/// - No arguments.
 		/// # </weight>
-		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
+		#[weight = SimpleDispatchInfo::FreeOperational]
 		fn force_no_eras(origin) {
 			ensure_root(origin)?;
 			ForceEra::put(Forcing::ForceNone);
@@ -981,14 +983,14 @@ decl_module! {
 		/// # <weight>
 		/// - No arguments.
 		/// # </weight>
-		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
+		#[weight = SimpleDispatchInfo::FreeOperational]
 		fn force_new_era(origin) {
 			ensure_root(origin)?;
 			ForceEra::put(Forcing::ForceNew);
 		}
 
 		/// Set the validators who cannot be slashed (if any).
-		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
+		#[weight = SimpleDispatchInfo::FreeOperational]
 		fn set_invulnerables(origin, validators: Vec<T::AccountId>) {
 			ensure_root(origin)?;
 			<Invulnerables<T>>::put(validators);
@@ -1004,6 +1006,17 @@ decl_module! {
 			// remove all staking-related information.
 			Self::kill_stash(&stash);
 		}
+
+		/// Force there to be a new era at the end of sessions indefinitely.
+		///
+		/// # <weight>
+		/// - One storage write
+		/// # </weight>
+		#[weight = SimpleDispatchInfo::FreeOperational]
+		fn force_new_era_always(origin) {
+			ensure_root(origin)?;
+			ForceEra::put(Forcing::ForceAlways);
+		}
 	}
 }
 
@@ -1155,6 +1168,7 @@ impl<T: Trait> Module<T> {
 		let era_length = session_index.checked_sub(Self::current_era_start_session_index()).unwrap_or(0);
 		match ForceEra::get() {
 			Forcing::ForceNew => ForceEra::kill(),
+			Forcing::ForceAlways => (),
 			Forcing::NotForcing if era_length >= T::SessionsPerEra::get() => (),
 			_ => return None,
 		}
@@ -1406,6 +1420,14 @@ impl<T: Trait> Module<T> {
 			}
 		});
 	}
+
+	/// Ensures that at the end of the current session there will be a new era.
+	fn ensure_new_era() {
+		match ForceEra::get() {
+			Forcing::ForceAlways | Forcing::ForceNew => (),
+			_ => ForceEra::put(Forcing::ForceNew),
+		}
+	}
 }
 
 impl<T: Trait> session::OnSessionEnding<T::AccountId> for Module<T> {
@@ -1502,6 +1524,13 @@ impl <T: Trait> OnOffenceHandler<T::AccountId, session::historical::Identificati
 				continue
 			}
 
+			// Auto deselect validator on any offence and force a new era if they haven't previously
+			// been deselected.
+			if <Validators<T>>::exists(stash) {
+				<Validators<T>>::remove(stash);
+				Self::ensure_new_era();
+			}
+
 			// calculate the amount to slash
 			let slash_exposure = exposure.total;
 			let amount = *slash_fraction * slash_exposure;
@@ -1514,7 +1543,7 @@ impl <T: Trait> OnOffenceHandler<T::AccountId, session::historical::Identificati
 			// make sure to disable validator till the end of this session
 			if T::SessionInterface::disable_validator(stash).unwrap_or(false) {
 				// force a new era, to select a new validator set
-				ForceEra::put(Forcing::ForceNew);
+				Self::ensure_new_era();
 			}
 			// actually slash the validator
 			let slashed_amount = Self::slash_validator(stash, amount, exposure, &mut journal);

substrate/srml/staking/src/tests.rs

@@ -775,7 +775,7 @@ fn forcing_new_era_works() {
 		assert_eq!(Staking::current_era(), 1);
 
 		// back to normal.
-		// this immediatelly starts a new session.
+		// this immediately starts a new session.
 		ForceEra::put(Forcing::NotForcing);
 		start_session(7);
 		assert_eq!(Staking::current_era(), 2);
@@ -783,9 +783,23 @@ fn forcing_new_era_works() {
 		assert_eq!(Staking::current_era(), 2);
 
 		// forceful change
-		ForceEra::put(Forcing::ForceNew);
+		ForceEra::put(Forcing::ForceAlways);
 		start_session(9);
 		assert_eq!(Staking::current_era(), 3);
+		start_session(10);
+		assert_eq!(Staking::current_era(), 4);
+		start_session(11);
+		assert_eq!(Staking::current_era(), 5);
+
+		// just one forceful change
+		ForceEra::put(Forcing::ForceNew);
+		start_session(12);
+		assert_eq!(Staking::current_era(), 6);
+
+		assert_eq!(ForceEra::get(), Forcing::NotForcing);
+		start_session(13);
+		assert_eq!(Staking::current_era(), 6);
+
 	});
 }
 
@@ -1761,6 +1775,45 @@ fn offence_forces_new_era() {
 	});
 }
 
+#[test]
+fn offence_ensures_new_era_without_clobbering() {
+	ExtBuilder::default().build().execute_with(|| {
+		assert_ok!(Staking::force_new_era_always(Origin::ROOT));
+
+		Staking::on_offence(
+			&[OffenceDetails {
+				offender: (
+					11,
+					Staking::stakers(&11),
+				),
+				reporters: vec![],
+			}],
+			&[Perbill::from_percent(5)],
+		);
+
+		assert_eq!(Staking::force_era(), Forcing::ForceAlways);
+	});
+}
+
+#[test]
+fn offence_deselects_validator_when_slash_is_zero() {
+	ExtBuilder::default().build().execute_with(|| {
+		assert!(<Validators<Test>>::exists(11));
+		Staking::on_offence(
+			&[OffenceDetails {
+				offender: (
+					11,
+					Staking::stakers(&11),
+				),
+				reporters: vec![],
+			}],
+			&[Perbill::from_percent(0)],
+		);
+		assert_eq!(Staking::force_era(), Forcing::ForceNew);
+		assert!(!<Validators<Test>>::exists(11));
+	});
+}
+
 #[test]
 fn slashing_performed_according_exposure() {
 	// This test checks that slashing is performed according the exposure (or more precisely,
@@ -1873,6 +1926,5 @@ fn dont_slash_if_fraction_is_zero() {
 
 		// The validator hasn't been slashed. The new era is not forced.
 		assert_eq!(Balances::free_balance(&11), 1000);
-		assert_eq!(Staking::force_era(), Forcing::NotForcing);
 	});
 }