diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 326a01599034c..c547545c389a5 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1482,11 +1482,11 @@ impl Module { let Nominations { submitted_in, mut targets, suppressed: _ } = nominations; // Filter out nomination targets which were nominated before the most recent - // slashing span. + // non-zero slash. targets.retain(|stash| { ::SlashingSpans::get(&stash).map_or( true, - |spans| submitted_in >= spans.last_start(), + |spans| submitted_in >= spans.last_nonzero_slash(), ) }); diff --git a/frame/staking/src/migration.rs b/frame/staking/src/migration.rs index bb020b0fc0e5f..6cb472375a48d 100644 --- a/frame/staking/src/migration.rs +++ b/frame/staking/src/migration.rs @@ -20,12 +20,14 @@ pub type VersionNumber = u32; // the current expected version of the storage -pub const CURRENT_VERSION: VersionNumber = 1; +pub const CURRENT_VERSION: VersionNumber = 2; +/// The inner logic of migrations. #[cfg(any(test, feature = "migrate"))] -mod inner { +pub mod inner { use crate::{Store, Module, Trait}; - use frame_support::{StorageLinkedMap, StorageValue}; + use frame_support::{StorageLinkedMap, StoragePrefixedMap, StorageValue}; + use codec::{Encode, Decode}; use sp_std::vec::Vec; use super::{CURRENT_VERSION, VersionNumber}; @@ -60,6 +62,55 @@ mod inner { frame_support::print("Finished migrating Staking storage to v1."); } + // migrate storage from v1 to v2: adds another field to the `SlashingSpans` + // struct. + pub fn to_v2(version: &mut VersionNumber) { + use crate::{EraIndex, slashing::SpanIndex}; + #[derive(Decode)] + struct V1SlashingSpans { + span_index: SpanIndex, + last_start: EraIndex, + prior: Vec, + } + + #[derive(Encode)] + struct V2SlashingSpans { + span_index: SpanIndex, + last_start: EraIndex, + last_nonzero_slash: EraIndex, + prior: Vec, + } + + if *version != 1 { return } + *version += 1; + + let prefix = as Store>::SlashingSpans::final_prefix(); + let mut current_key = prefix.to_vec(); + loop { + let maybe_next_key = sp_io::storage::next_key(¤t_key[..]) + .filter(|v| v.starts_with(&prefix[..])); + + match maybe_next_key { + Some(next_key) => { + let maybe_spans = sp_io::storage::get(&next_key[..]) + .and_then(|v| V1SlashingSpans::decode(&mut &v[..]).ok()); + if let Some(spans) = maybe_spans { + let new_val = V2SlashingSpans { + span_index: spans.span_index, + last_start: spans.last_start, + last_nonzero_slash: spans.last_start, + prior: spans.prior, + }.encode(); + + sp_io::storage::set(&next_key[..], &new_val[..]); + } + current_key = next_key; + } + None => break, + } + } + } + pub(super) fn perform_migrations() { as Store>::StorageVersion::mutate(|version| { if *version < MIN_SUPPORTED_VERSION { @@ -72,6 +123,7 @@ mod inner { if *version == CURRENT_VERSION { return } to_v1::(version); + to_v2::(version); }); } } diff --git a/frame/staking/src/slashing.rs b/frame/staking/src/slashing.rs index 7322b9a1d3118..df36b1c763c4b 100644 --- a/frame/staking/src/slashing.rs +++ b/frame/staking/src/slashing.rs @@ -90,7 +90,9 @@ pub struct SlashingSpans { span_index: SpanIndex, // the start era of the most recent (ongoing) slashing span. last_start: EraIndex, - // all prior slashing spans start indices, in reverse order (most recent first) + // the last era at which a non-zero slash occurred. + last_nonzero_slash: EraIndex, + // all prior slashing spans' start indices, in reverse order (most recent first) // encoded as offsets relative to the slashing span after it. prior: Vec, } @@ -102,6 +104,10 @@ impl SlashingSpans { SlashingSpans { span_index: 0, last_start: window_start, + // initialize to zero, as this structure is lazily created until + // the first slash is applied. setting equal to `window_start` would + // put a time limit on nominations. + last_nonzero_slash: 0, prior: Vec::new(), } } @@ -136,9 +142,9 @@ impl SlashingSpans { sp_std::iter::once(last).chain(prior) } - /// Yields the era index where the last (current) slashing span started. - pub(crate) fn last_start(&self) -> EraIndex { - self.last_start + /// Yields the era index where the most recent non-zero slash occurred. + pub(crate) fn last_nonzero_slash(&self) -> EraIndex { + self.last_nonzero_slash } // prune the slashing spans against a window, whose start era index is given. @@ -457,8 +463,12 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { self.dirty = self.spans.end_span(now) || self.dirty; } - fn add_slash(&mut self, amount: BalanceOf) { + // add some value to the slash of the staker. + // invariant: the staker is being slashed for non-zero value here + // although `amount` may be zero, as it is only a difference. + fn add_slash(&mut self, amount: BalanceOf, slash_era: EraIndex) { *self.slash_of += amount; + self.spans.last_nonzero_slash = sp_std::cmp::max(self.spans.last_nonzero_slash, slash_era); } // find the span index of the given era, if covered. @@ -489,7 +499,7 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { let reward = REWARD_F1 * (self.reward_proportion * slash).saturating_sub(span_record.paid_out); - self.add_slash(difference); + self.add_slash(difference, slash_era); changed = true; reward @@ -681,6 +691,7 @@ mod tests { let spans = SlashingSpans { span_index: 0, last_start: 1000, + last_nonzero_slash: 0, prior: Vec::new(), }; @@ -695,6 +706,7 @@ mod tests { let spans = SlashingSpans { span_index: 10, last_start: 1000, + last_nonzero_slash: 0, prior: vec![10, 9, 8, 10], }; @@ -715,6 +727,7 @@ mod tests { let mut spans = SlashingSpans { span_index: 10, last_start: 1000, + last_nonzero_slash: 0, prior: vec![10, 9, 8, 10], }; @@ -768,6 +781,7 @@ mod tests { let mut spans = SlashingSpans { span_index: 10, last_start: 1000, + last_nonzero_slash: 0, prior: vec![10, 9, 8, 10], }; assert_eq!(spans.prune(2000), Some((6, 10))); @@ -784,6 +798,7 @@ mod tests { let mut spans = SlashingSpans { span_index: 1, last_start: 10, + last_nonzero_slash: 0, prior: Vec::new(), }; diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index c290c6a8581d4..bb1fe32025ebc 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -18,12 +18,13 @@ use super::*; use mock::*; +use codec::Encode; use sp_runtime::{assert_eq_error_rate, traits::{OnInitialize, BadOrigin}}; use sp_staking::offence::OffenceDetails; use frame_support::{ assert_ok, assert_noop, traits::{Currency, ReservableCurrency}, - dispatch::DispatchError, + dispatch::DispatchError, StorageMap, }; use substrate_test_utils::assert_eq_uvec; @@ -2710,7 +2711,95 @@ fn slash_kicks_validators_not_nominators() { // and make sure that the vote will be ignored even if the validator // re-registers. - let last_slash = ::SlashingSpans::get(&11).unwrap().last_start(); + let last_slash = ::SlashingSpans::get(&11).unwrap().last_nonzero_slash(); assert!(nominations.submitted_in < last_slash); }); } + +#[test] +fn migration_v2() { + ExtBuilder::default().build().execute_with(|| { + use crate::{EraIndex, slashing::SpanIndex}; + + #[derive(Encode)] + struct V1SlashingSpans { + span_index: SpanIndex, + last_start: EraIndex, + prior: Vec, + } + + // inject old-style values directly into storage. + let set = |stash, spans: V1SlashingSpans| { + let key = ::SlashingSpans::hashed_key_for(stash); + sp_io::storage::set(&key, &spans.encode()); + }; + + let spans_11 = V1SlashingSpans { + span_index: 10, + last_start: 1, + prior: vec![0], + }; + + let spans_21 = V1SlashingSpans { + span_index: 1, + last_start: 5, + prior: vec![], + }; + + set(11, spans_11); + set(21, spans_21); + + ::StorageVersion::put(1); + + // perform migration. + crate::migration::inner::to_v2::(&mut 1); + + assert_eq!( + ::SlashingSpans::get(&11).unwrap().last_nonzero_slash(), + 1, + ); + + assert_eq!( + ::SlashingSpans::get(&21).unwrap().last_nonzero_slash(), + 5, + ); + }); +} + +#[test] +fn zero_slash_keeps_nominators() { + ExtBuilder::default().build().execute_with(|| { + start_era(1); + + assert_eq!(Balances::free_balance(&11), 1000); + + let exposure = Staking::stakers(&11); + assert_eq!(Balances::free_balance(&101), 2000); + + on_offence_now( + &[ + OffenceDetails { + offender: (11, exposure.clone()), + reporters: vec![], + }, + ], + &[Perbill::from_percent(0)], + ); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + // This is the best way to check that the validator was chilled; `get` will + // return default value. + for (stash, _) in ::Validators::enumerate() { + assert!(stash != 11); + } + + let nominations = ::Nominators::get(&101).unwrap(); + + // and make sure that the vote will not be ignored, because the slash was + // zero. + let last_slash = ::SlashingSpans::get(&11).unwrap().last_nonzero_slash(); + assert!(nominations.submitted_in >= last_slash); + }); +}