diff --git a/substrate/core/sr-primitives/src/traits.rs b/substrate/core/sr-primitives/src/traits.rs index 3c2b51d7fd..b2bb7ab805 100644 --- a/substrate/core/sr-primitives/src/traits.rs +++ b/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`. diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index 3719130eda..42d89c57ad 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/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, }; diff --git a/substrate/srml/session/src/lib.rs b/substrate/srml/session/src/lib.rs index a74edf52d7..3ae7c98012 100644 --- a/substrate/srml/session/src/lib.rs +++ b/substrate/srml/session/src/lib.rs @@ -359,10 +359,16 @@ impl Module { } /// Disable the validator of index `i`. - pub fn disable(i: usize) { + pub fn disable_index(i: usize) { T::SessionHandler::on_disabled(i); >::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 OnFreeBalanceZero for Module { diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index 1f6e023d18..086f78bd53 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -1210,8 +1210,7 @@ impl Module { .map(|x| x.min(slash_exposure)) .unwrap_or(slash_exposure); let _ = Self::slash_validator(&stash, slash); - >::remove(&stash); - let _ = Self::apply_force_new_era(); + let _ = >::disable(&controller); RawEvent::OfflineSlash(stash.clone(), slash) } else { diff --git a/substrate/srml/staking/src/mock.rs b/substrate/srml/staking/src/mock.rs index 5eca0d524a..ea4fcbd8f9 100644 --- a/substrate/srml/staking/src/mock.rs +++ b/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 for CurrencyToVoteHandler { } } +thread_local! { + static SESSION: RefCell<(Vec, HashSet)> = RefCell::new(Default::default()); +} + pub struct TestSessionHandler; impl session::SessionHandler for TestSessionHandler { - fn on_new_session(_changed: bool, _validators: &[(AccountId, Ks)]) { + fn on_new_session(_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::{ // 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::{ @@ -225,7 +244,14 @@ impl ExtBuilder { let _ = timestamp::GenesisConfig::{ 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 } } diff --git a/substrate/srml/staking/src/tests.rs b/substrate/srml/staking/src/tests.rs index 555248493e..1124261989 100644 --- a/substrate/srml/staking/src/tests.rs +++ b/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!(!>::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)); }); }