Disable validators but don't kick them (#2870)

* Disable validators but don't kick them

* Bump runtime

* Remove stray printlns

substrate/core/sr-primitives/src/traits.rs

@@ -819,7 +819,7 @@ pub trait ValidateUnsigned {
 
 /// Opaque datatype that may be destructured into a series of raw byte slices (which represent
 /// individual keys).
-pub trait OpaqueKeys {
+pub trait OpaqueKeys: Clone {
 	/// Return the number of encoded keys.
 	fn count() -> usize { 0 }
 	/// Get the raw bytes of key with index `i`.

substrate/node/runtime/src/lib.rs

@@ -58,7 +58,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("node"),
 	impl_name: create_runtime_str!("substrate-node"),
 	authoring_version: 10,
-	spec_version: 94,
+	spec_version: 95,
 	impl_version: 95,
 	apis: RUNTIME_API_VERSIONS,
 };

substrate/srml/session/src/lib.rs

@@ -359,10 +359,16 @@ impl<T: Trait> Module<T> {
 	}
 
 	/// Disable the validator of index `i`.
-	pub fn disable(i: usize) {
+	pub fn disable_index(i: usize) {
 		T::SessionHandler::on_disabled(i);
 		<Changed<T>>::put(true);
 	}
+
+	/// Disable the validator identified by `c`. (If using with the staking module, this would be
+	/// their *controller* account.)
+	pub fn disable(c: &T::AccountId) -> rstd::result::Result<(), ()> {
+		Self::validators().iter().position(|i| i == c).map(Self::disable_index).ok_or(())
+	}
 }
 
 impl<T: Trait> OnFreeBalanceZero<T::AccountId> for Module<T> {

substrate/srml/staking/src/lib.rs

@@ -1210,8 +1210,7 @@ impl<T: Trait> Module<T> {
 					.map(|x| x.min(slash_exposure))
 					.unwrap_or(slash_exposure);
 				let _ = Self::slash_validator(&stash, slash);
-				<Validators<T>>::remove(&stash);
-				let _ = Self::apply_force_new_era();
+				let _ = <session::Module<T>>::disable(&controller);
 
 				RawEvent::OfflineSlash(stash.clone(), slash)
 			} else {

substrate/srml/staking/src/mock.rs

@@ -16,6 +16,7 @@
 
 //! Test utilities
 
+use std::{collections::HashSet, cell::RefCell};
 use primitives::{BuildStorage, Perbill};
 use primitives::traits::{IdentityLookup, Convert, OpaqueKeys, OnInitialize};
 use primitives::testing::{Header, UintAuthorityId};
@@ -39,15 +40,32 @@ impl Convert<u128, u64> for CurrencyToVoteHandler {
 	}
 }
 
+thread_local! {
+	static SESSION: RefCell<(Vec<AccountId>, HashSet<AccountId>)> = RefCell::new(Default::default());
+}
+
 pub struct TestSessionHandler;
 impl session::SessionHandler<AccountId> for TestSessionHandler {
-	fn on_new_session<Ks: OpaqueKeys>(_changed: bool, _validators: &[(AccountId, Ks)]) {
+	fn on_new_session<Ks: OpaqueKeys>(_changed: bool, validators: &[(AccountId, Ks)]) {
+		SESSION.with(|x|
+			*x.borrow_mut() = (validators.iter().map(|x| x.0.clone()).collect(), HashSet::new())
+		);
 	}
 
-	fn on_disabled(_validator_index: usize) {
+	fn on_disabled(validator_index: usize) {
+		SESSION.with(|d| {
+			let mut d = d.borrow_mut();
+			let value = d.0[validator_index];
+			println!("on_disabled {} -> {}", validator_index, value);
+			d.1.insert(value);
+		})
 	}
 }
 
+pub fn is_disabled(validator: AccountId) -> bool {
+	SESSION.with(|d| d.borrow().1.contains(&validator))
+}
+
 impl_outer_origin!{
 	pub enum Origin for Test {}
 }
@@ -167,9 +185,10 @@ impl ExtBuilder {
 		} else {
 			1
 		};
+		let validators = if self.validator_pool { vec![10, 20, 30, 40] } else { vec![10, 20] };
 		let _ = session::GenesisConfig::<Test>{
 			// NOTE: if config.nominate == false then 100 is also selected in the initial round.
-			validators: if self.validator_pool { vec![10, 20, 30, 40] } else { vec![10, 20] },
+			validators,
 			keys: vec![],
 		}.assimilate_storage(&mut t, &mut c);
 		let _ = balances::GenesisConfig::<Test>{
@@ -225,7 +244,14 @@ impl ExtBuilder {
 		let _ = timestamp::GenesisConfig::<Test>{
 			minimum_period: 5,
 		}.assimilate_storage(&mut t, &mut c);
-		t.into()
+		let mut ext = t.into();
+		runtime_io::with_externalities(&mut ext, || {
+			let validators = Session::validators();
+			SESSION.with(|x|
+				*x.borrow_mut() = (validators.clone(), HashSet::new())
+			);
+		});
+		ext
 	}
 }
 

substrate/srml/staking/src/tests.rs

@@ -154,7 +154,7 @@ fn invulnerability_should_work() {
 }
 
 #[test]
-fn offline_should_slash_and_kick() {
+fn offline_should_slash_and_disable() {
 	// Test that an offline validator gets slashed and kicked
 	with_externalities(&mut ExtBuilder::default().build(), || {
 		// Give account 10 some balance
@@ -169,6 +169,8 @@ fn offline_should_slash_and_kick() {
 		assert_eq!(Staking::slash_count(&11), 0);
 		// Account 10 has the funds we just gave it
 		assert_eq!(Balances::free_balance(&11), 1000);
+		// Account 10 is not yet disabled.
+		assert!(!is_disabled(10));
 		// Report account 10 as offline, one greater than unstake threshold
 		Staking::on_offline_validator(10, 4);
 		// Confirm user has been reported
@@ -176,10 +178,8 @@ fn offline_should_slash_and_kick() {
 		// Confirm balance has been reduced by 2^unstake_threshold * offline_slash() * amount_at_stake.
 		let slash_base = Staking::offline_slash() * Staking::stakers(11).total;
 		assert_eq!(Balances::free_balance(&11), 1000 - 2_u64.pow(3) * slash_base);
-		// Confirm account 10 has been removed as a validator
-		assert!(!<Validators<Test>>::exists(&11));
-		// A new era is forced due to slashing
-		assert!(Staking::forcing_new_era());
+		// Confirm account 10 has been disabled.
+		assert!(is_disabled(10));
 	});
 }
 
@@ -218,7 +218,7 @@ fn offline_grace_should_delay_slashing() {
 		// User gets slashed
 		assert!(Balances::free_balance(&11) < 70);
 		// New era is forced
-		assert!(Staking::forcing_new_era());
+		assert!(is_disabled(10));
 	});
 }
 
@@ -723,7 +723,7 @@ fn nominators_also_get_slashed() {
 		assert_eq!(Balances::total_balance(&2), initial_balance - nominator_slash);
 		check_exposure_all();
 		// Because slashing happened.
-		assert!(Staking::forcing_new_era());
+		assert!(is_disabled(10));
 	});
 }