From c6bdd509b58ed4b498976d6df44d17a889738e1c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 4 Oct 2019 13:45:33 +0200 Subject: [PATCH 01/46] define slashing spans --- srml/staking/src/lib.rs | 1 + srml/staking/src/slashing.rs | 78 ++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 srml/staking/src/slashing.rs diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index edbf7c245fea7..4d5dba3c5c72c 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -248,6 +248,7 @@ mod mock; #[cfg(test)] mod tests; +mod slashing; pub mod inflation; diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs new file mode 100644 index 0000000000000..156b9d454a6c5 --- /dev/null +++ b/srml/staking/src/slashing.rs @@ -0,0 +1,78 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! A slashing implementation for NPoS systems. +//! +//! For the purposes of the economic model, it is easiest to think of each validator +//! of a nominator which nominates only its own identity. +//! +//! The act of nomination signals intent to unify economic identity with the validator - to take part in the +//! rewards of a job well done, and to take part in the punishment of a job done badly. +//! +//! There are 3 main difficulties to account for with slashing in NPoS: +//! - A nominator can nominate multiple validators and be slashed via any of them. +//! - Until slashed, stake is reused from era to era. Nominating with N coins for E eras in a row +//! does not mean you have N*E coins to be slashed - you've only ever had N. +//! - Slashable offences can be found after the fact and out of order. +//! +//! The algorithm implemented in this module tries to balance these 3 difficulties. +//! +//! First, we only slash participants for the _maximum_ slash they receive in some time period, +//! rather than the sum. This ensures a protection from overslashing. +//! +//! Second, we do not want the time period (or "span") that the maximum is computed +//! over to last indefinitely. That would allow participants to begin acting with +//! impunity after some point, fearing no further repercussions. For that reason, we +//! automatically "chill" validators and nominators upon a slash event, requiring them +//! to re-enlist voluntarily (acknowledging the slash) and begin a new slashing span. +//! +//! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ + +use super::{EraIndex, Trait, Module}; +use codec::{HasCompact, Encode, Decode}; + +// A range of start..end eras for a slashing span. +#[derive(Encode, Decode)] +struct SlashingSpan { + start: EraIndex, + length: Option, // the ongoing slashing span has indeterminate length. +} + +/// An encoding of all of a nominator's slashing spans. +#[derive(Default, Encode, Decode)] +pub struct SlashingSpans { + // 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) + // encoded as offsets relative to the slashing span after it. + prior: Vec, +} + +impl SlashingSpans { + // an iterator over all slashing spans in _reverse_ order - most recent first. + fn iter(&'_ self, ) -> impl Iterator + '_ { + let mut last_start = self.last_start; + let last = SlashingSpan { start: last_start, length: None }; + let prior = self.prior.iter().cloned().map(move |length| { + let start = last_start - length; + last_start = start; + + SlashingSpan { start, length: Some(length) } + }); + + rstd::iter::once(last).chain(prior) + } +} From fef2d4c1036775ea91758f491e2b8060bba71101 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 4 Oct 2019 14:04:55 +0200 Subject: [PATCH 02/46] tests and pruning for slashing-spans record --- srml/staking/src/slashing.rs | 118 ++++++++++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 156b9d454a6c5..d303e0b22c36b 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -46,6 +46,7 @@ use codec::{HasCompact, Encode, Decode}; // A range of start..end eras for a slashing span. #[derive(Encode, Decode)] +#[cfg_attr(test, derive(Debug, PartialEq))] struct SlashingSpan { start: EraIndex, length: Option, // the ongoing slashing span has indeterminate length. @@ -63,7 +64,7 @@ pub struct SlashingSpans { impl SlashingSpans { // an iterator over all slashing spans in _reverse_ order - most recent first. - fn iter(&'_ self, ) -> impl Iterator + '_ { + fn iter(&'_ self) -> impl Iterator + '_ { let mut last_start = self.last_start; let last = SlashingSpan { start: last_start, length: None }; let prior = self.prior.iter().cloned().map(move |length| { @@ -75,4 +76,119 @@ impl SlashingSpans { rstd::iter::once(last).chain(prior) } + + // prune the slashing spans against a window, whose start era index is given. + fn prune(&mut self, window_start: EraIndex) { + let old_idx = self.iter() + .skip(1) // skip ongoing span. + .position(|span| span.length.map_or(false, |len| span.start + len <= window_start)); + + if let Some(o) = old_idx { + self.prior.truncate(o); + } + + // readjust the ongoing span, if it started before the beginning of the winow. + self.last_start = rstd::cmp::max(self.last_start, window_start); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn single_slashing_span() { + let spans = SlashingSpans { + last_start: 1000, + prior: Vec::new(), + }; + + assert_eq!(spans.iter().collect::>(), vec![SlashingSpan { start: 1000, length: None }]); + } + + #[test] + fn many_prior_spans() { + let spans = SlashingSpans { + last_start: 1000, + prior: vec![10, 9, 8, 10], + }; + + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 1000, length: None }, + SlashingSpan { start: 990, length: Some(10) }, + SlashingSpan { start: 981, length: Some(9) }, + SlashingSpan { start: 973, length: Some(8) }, + SlashingSpan { start: 963, length: Some(10) }, + ], + ) + } + + #[test] + fn pruning_spans() { + let mut spans = SlashingSpans { + last_start: 1000, + prior: vec![10, 9, 8, 10], + }; + + spans.prune(981); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 1000, length: None }, + SlashingSpan { start: 990, length: Some(10) }, + SlashingSpan { start: 981, length: Some(9) }, + ], + ); + + spans.prune(982); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 1000, length: None }, + SlashingSpan { start: 990, length: Some(10) }, + SlashingSpan { start: 981, length: Some(9) }, + ], + ); + + spans.prune(989); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 1000, length: None }, + SlashingSpan { start: 990, length: Some(10) }, + SlashingSpan { start: 981, length: Some(9) }, + ], + ); + + spans.prune(1000); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 1000, length: None }, + ], + ); + + spans.prune(2000); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 2000, length: None }, + ], + ); + + // now all in one shot. + let mut spans = SlashingSpans { + last_start: 1000, + prior: vec![10, 9, 8, 10], + }; + spans.prune(2000); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { start: 2000, length: None }, + ], + ); + } } From 59ea56a85ea2bcff838242789b32f2a5e933289c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 17 Oct 2019 16:46:52 -0400 Subject: [PATCH 03/46] validators get slashed before nominators --- srml/staking/src/lib.rs | 21 ++- srml/staking/src/slashing.rs | 274 ++++++++++++++++++++++++++++++----- 2 files changed, 254 insertions(+), 41 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 4d5dba3c5c72c..da664ce6e55dd 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -611,9 +611,15 @@ decl_storage! { /// A mapping from still-bonded eras to the first session index of that era. BondedEras: Vec<(EraIndex, SessionIndex)>; - /// All slashes that have occurred in a given era. - EraSlashJournal get(fn era_slash_journal): - map EraIndex => Vec>>; + /// All slashing events on validators and nominators, mapped by era. + SlashInEra: + double_map EraIndex, twox_128(T::AccountId) => Option<(Perbill, BalanceOf)>; + + /// Slashing spans for stash accounts. + SlashingSpans: map T::AccountId => Option; + + /// Records information about the maximum slash of a stash within a slashing span. + SpanSlash get(fn span_slash): map (T::AccountId, slashing::SpanIndex) => BalanceOf; } add_extra_genesis { config(stakers): @@ -1214,7 +1220,8 @@ impl Module { let current_era = CurrentEra::mutate(|s| { *s += 1; *s }); // prune journal for last era. - >::remove(current_era - 1); + // TODO: prune journal for the era just exiting the bonding window. + // >::remove(current_era - 1); CurrentEraStartSessionIndex::mutate(|v| { *v = start_session_index; @@ -1364,6 +1371,8 @@ impl Module { >::remove(stash); >::remove(stash); >::remove(stash); + + // TODO: clear all slashing bookkeeping (slashing spans, etc). } /// Add reward points to validators using their stash account ID. @@ -1493,7 +1502,6 @@ impl OnOffenceHandler OnOffenceHandler OnOffenceHandler>::insert(era_now, journal); // Handle the rest of imbalances T::Slash::on_unbalanced(remaining_imbalance); diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index d303e0b22c36b..b8fe5a6ff6869 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -36,25 +36,41 @@ //! Second, we do not want the time period (or "span") that the maximum is computed //! over to last indefinitely. That would allow participants to begin acting with //! impunity after some point, fearing no further repercussions. For that reason, we -//! automatically "chill" validators and nominators upon a slash event, requiring them -//! to re-enlist voluntarily (acknowledging the slash) and begin a new slashing span. +//! automatically "chill" validators and withdraw a nominator's nomination after a slashing event, +//! requiring them to re-enlist voluntarily (acknowledging the slash) and begin a new +//! slashing span. //! //! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ -use super::{EraIndex, Trait, Module}; +use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill}; +use sr_primitives::traits::Zero; +use support::{StorageValue, StorageMap, StorageDoubleMap}; use codec::{HasCompact, Encode, Decode}; +/// The index of a slashing span - unique to each stash. +pub (crate) type SpanIndex = u32; + // A range of start..end eras for a slashing span. #[derive(Encode, Decode)] #[cfg_attr(test, derive(Debug, PartialEq))] struct SlashingSpan { + index: SpanIndex, start: EraIndex, length: Option, // the ongoing slashing span has indeterminate length. } +impl SlashingSpan { + fn contains_era(&self, era: EraIndex) -> bool { + self.start <= era && self.length.map_or(true, |l| self.start + l > era) + } +} + /// An encoding of all of a nominator's slashing spans. -#[derive(Default, Encode, Decode)] +#[derive(Encode, Decode)] pub struct SlashingSpans { + // the index of the current slashing span of the nominator. different for + // every stash, resets when the account hits free balance 0. + 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) @@ -63,32 +79,192 @@ pub struct SlashingSpans { } impl SlashingSpans { + // creates a new record of slashing spans for a stash, starting at the beginning + // of the bonding period, relative to now. + fn new(window_start: EraIndex) -> Self { + SlashingSpans { + span_index: 0, + last_start: window_start, + prior: Vec::new(), + } + } + + fn bump_span(&mut self, now: EraIndex) { + if now <= self.last_start { return } + + let last_length = now - self.last_start; + self.prior.insert(0, last_length); + self.last_start = now; + self.span_index += 1; + } + // an iterator over all slashing spans in _reverse_ order - most recent first. fn iter(&'_ self) -> impl Iterator + '_ { let mut last_start = self.last_start; - let last = SlashingSpan { start: last_start, length: None }; + let mut index = self.span_index; + let last = SlashingSpan { index, start: last_start, length: None }; let prior = self.prior.iter().cloned().map(move |length| { let start = last_start - length; last_start = start; + index -= 1; - SlashingSpan { start, length: Some(length) } + SlashingSpan { index, start, length: Some(length) } }); rstd::iter::once(last).chain(prior) } // prune the slashing spans against a window, whose start era index is given. - fn prune(&mut self, window_start: EraIndex) { + // + // If this returns `Some`, then it includes a range start..end of all the span + // indices which were pruned. + fn prune(&mut self, window_start: EraIndex) -> Option<(SpanIndex, SpanIndex)> { let old_idx = self.iter() .skip(1) // skip ongoing span. .position(|span| span.length.map_or(false, |len| span.start + len <= window_start)); - if let Some(o) = old_idx { - self.prior.truncate(o); - } + let earliest_span_index = self.span_index - self.prior.len() as SpanIndex; + let pruned = match old_idx { + Some(o) => { + self.prior.truncate(o); + let new_earliest = self.span_index - self.prior.len() as SpanIndex; + Some((earliest_span_index, new_earliest)) + } + None => None, + }; // readjust the ongoing span, if it started before the beginning of the winow. self.last_start = rstd::cmp::max(self.last_start, window_start); + pruned + } +} + +/// Parameters for performing a slash. +pub(crate) struct SlashParams<'a, T: 'a + Trait> { + /// The stash account being slashed. + pub(crate) stash: &'a T::AccountId, + /// The proportion and total amount of the slash (proportion applied to `exposure.total`). + pub(crate) slash: (Perbill, BalanceOf), + /// The exposure of the stash and all nominators. + pub(crate) exposure: &'a Exposure>, + /// The era where the offence occurred. + pub(crate) slash_era: EraIndex, + /// The first era in the current bonding period. + pub(crate) window_start: EraIndex, +} + +/// Slash a validator and their nominators, if necessary. +pub(crate) fn slash(params: SlashParams){ + let SlashParams { + stash, + slash, + exposure, + slash_era, + window_start, + } = params; + let (slash_proportion, slash_amount) = slash; + + // is the slash amount here a maximum for the era? + let era_slash = as Store>::SlashInEra::get(&slash_era, stash) + .map(|(_, amount)| amount) + .unwrap_or(Zero::zero()); + + if slash_amount > era_slash { + as Store>::SlashInEra::insert( + &slash_era, + stash, + &(slash_proportion, slash_amount), + ); + } else { + // we slash based on the max in era - this new event is not the max, + // so neither the validator or any nominators will need an update. + return + } + + // what about the maximum slash in the span? + let mut spans = as Store>::SlashingSpans::get(stash).unwrap_or_else(|| { + SlashingSpans::new(window_start) + }); + span_gc::(stash, spans.prune(window_start)); + + as Store>::SlashingSpans::insert(stash, &spans); + + let target_span = spans.iter().find(|span| span.contains_era(slash_era)); + let span_slash = >::span_slash(&(stash.clone(), target_span.index)); + + let spans = spans; + + // `chill` validator if this is the most recent span. + if spans.span_index == target_span.index { + // TODO + } + + // false if have chewed through all of our own exposure for this era, + // true if nothing has been passed onto nominators. + let has_own_remaining = slash_amount < exposure.own; + + let effective_nominator_slash = if slash_amount > span_slash { + let remaining_slash = slash_amount - span_slash; + let remaining_own = exposure.own.saturating_sub(span_slash); + + // divide up the slash into own and nominator portions. + let (own_slash, nominator_slash) = if remaining_slash >= remaining_own { + (remaining_own, remaining_slash - remaining_own) + } else { + (remaining_slash, 0) + }; + + // Apply `own_slash` to self balance. Since this is the highest slash of + // the span, we actually apply the slash to our own bond. + // TODO + + // note new highest slash in span + as Store>::SpanSlash::insert( + &(stash.clone(), target_span.index), + &slash_amount, + ); + + nominator_slash + } else { + // this validator has already been slashed for more than `slash_amount` within + // this slashing span. That means that we do not actually apply the slash on our own + // bond, but if the nominator slash for this era is now non-zero we have + // to pass that onwards to the nominator calculation. + if has_own_remaining { + 0 + } else { + slash_amount - exposure.own + } + }; + + if effective_nominator_slash == 0 { + // nominators are unaffected. + return + } + + // p_{v,e} is non-zero here - it is the ratio of the remaining slash in the era + // to the amount of exposure held by nominators. + let nominator_slash_proportion = { + let d = Perbill::from_rational_approximation( + effective_nominator_slash, + exposure.total - exposure.own, + ); + let d = Perbill::one() / (exposure.total - exposure.own); + d * effective_nominator_slash; + }; + + // TODO: apply to nominators. +} + +fn slash_nominators() { + +} + +fn span_gc(stash: &T::AccountId, pruned_range: Option<(SpanIndex, SpanIndex)>) { + if let Some((start, end)) = pruned_range { + for span_index in start..end { + as Store>::SpanSlash::remove(&(stash.clone(), span_index)); + } } } @@ -96,19 +272,47 @@ impl SlashingSpans { mod tests { use super::*; + #[test] + fn span_contains_era() { + // unbounded end + let span = SlashingSpan { index: 0, start: 1000, length: None }; + assert!(!span.contains_era(0)); + assert!(!span.contains_era(999)); + + assert!(span.contains_era(1000)); + assert!(span.contains_era(1001)); + assert!(span.contains_era(10000)); + + // bounded end - non-inclusive range. + let span = SlashingSpan { index: 0, start: 1000, length: Some(10) }; + assert!(!span.contains_era(0)); + assert!(!span.contains_era(999)); + + assert!(span.contains_era(1000)); + assert!(span.contains_era(1001)); + assert!(span.contains_era(1009)); + assert!(!span.contains_era(1010)); + assert!(!span.contains_era(1011)); + } + #[test] fn single_slashing_span() { let spans = SlashingSpans { + span_index: 0, last_start: 1000, prior: Vec::new(), }; - assert_eq!(spans.iter().collect::>(), vec![SlashingSpan { start: 1000, length: None }]); + assert_eq!( + spans.iter().collect::>(), + vec![SlashingSpan { index: 0, start: 1000, length: None }], + ); } #[test] fn many_prior_spans() { let spans = SlashingSpans { + span_index: 10, last_start: 1000, prior: vec![10, 9, 8, 10], }; @@ -116,11 +320,11 @@ mod tests { assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 1000, length: None }, - SlashingSpan { start: 990, length: Some(10) }, - SlashingSpan { start: 981, length: Some(9) }, - SlashingSpan { start: 973, length: Some(8) }, - SlashingSpan { start: 963, length: Some(10) }, + SlashingSpan { index: 10, start: 1000, length: None }, + SlashingSpan { index: 9, start: 990, length: Some(10) }, + SlashingSpan { index: 8, start: 981, length: Some(9) }, + SlashingSpan { index: 7, start: 973, length: Some(8) }, + SlashingSpan { index: 6, start: 963, length: Some(10) }, ], ) } @@ -128,66 +332,68 @@ mod tests { #[test] fn pruning_spans() { let mut spans = SlashingSpans { + span_index: 10, last_start: 1000, prior: vec![10, 9, 8, 10], }; - spans.prune(981); + assert_eq!(spans.prune(981), Some((6, 8))); assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 1000, length: None }, - SlashingSpan { start: 990, length: Some(10) }, - SlashingSpan { start: 981, length: Some(9) }, + SlashingSpan { index: 10, start: 1000, length: None }, + SlashingSpan { index: 9, start: 990, length: Some(10) }, + SlashingSpan { index: 8, start: 981, length: Some(9) }, ], ); - spans.prune(982); + assert_eq!(spans.prune(982), None); assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 1000, length: None }, - SlashingSpan { start: 990, length: Some(10) }, - SlashingSpan { start: 981, length: Some(9) }, + SlashingSpan { index: 10, start: 1000, length: None }, + SlashingSpan { index: 9, start: 990, length: Some(10) }, + SlashingSpan { index: 8, start: 981, length: Some(9) }, ], ); - spans.prune(989); + assert_eq!(spans.prune(989), None); assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 1000, length: None }, - SlashingSpan { start: 990, length: Some(10) }, - SlashingSpan { start: 981, length: Some(9) }, + SlashingSpan { index: 10, start: 1000, length: None }, + SlashingSpan { index: 9, start: 990, length: Some(10) }, + SlashingSpan { index: 8, start: 981, length: Some(9) }, ], ); - spans.prune(1000); + assert_eq!(spans.prune(1000), Some((8, 10))); assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 1000, length: None }, + SlashingSpan { index: 10, start: 1000, length: None }, ], ); - spans.prune(2000); + assert_eq!(spans.prune(2000), None); assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 2000, length: None }, + SlashingSpan { index: 10, start: 2000, length: None }, ], ); // now all in one shot. let mut spans = SlashingSpans { + span_index: 10, last_start: 1000, prior: vec![10, 9, 8, 10], }; - spans.prune(2000); + assert_eq!(spans.prune(2000), Some((6, 10))); assert_eq!( spans.iter().collect::>(), vec![ - SlashingSpan { start: 2000, length: None }, + SlashingSpan { index: 10, start: 2000, length: None }, ], ); } From c6c9da8a509a771e24e4d538859cb05f9ba70693 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 17 Oct 2019 18:38:43 -0400 Subject: [PATCH 04/46] apply slash to nominators as well --- srml/staking/src/lib.rs | 11 +- srml/staking/src/slashing.rs | 195 ++++++++++++++++++++++------------- 2 files changed, 131 insertions(+), 75 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index da664ce6e55dd..0ccc310af9239 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -611,10 +611,15 @@ decl_storage! { /// A mapping from still-bonded eras to the first session index of that era. BondedEras: Vec<(EraIndex, SessionIndex)>; - /// All slashing events on validators and nominators, mapped by era. - SlashInEra: + /// All slashing events on validators, mapped by era to the highest slash proportion + /// and slash value of the era. + ValidatorSlashInEra: double_map EraIndex, twox_128(T::AccountId) => Option<(Perbill, BalanceOf)>; + /// All slashing events on nominators, mapped by era to the highest slash value of the era. + NominatorSlashInEra: + double_map EraIndex, twox_128(T::AccountId) => Option>; + /// Slashing spans for stash accounts. SlashingSpans: map T::AccountId => Option; @@ -1054,6 +1059,8 @@ impl Module { exposure: &Exposure>, journal: &mut Vec>>, ) -> NegativeImbalanceOf { + // TODO: move over to `slashing.rs` logic. + // The amount we are actually going to slash (can't be bigger than the validator's total // exposure) let slash = slash.min(exposure.total); diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index b8fe5a6ff6869..62efd588e6a10 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -43,7 +43,7 @@ //! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill}; -use sr_primitives::traits::Zero; +use sr_primitives::traits::{Zero, Saturating}; use support::{StorageValue, StorageMap, StorageDoubleMap}; use codec::{HasCompact, Encode, Decode}; @@ -140,11 +140,12 @@ impl SlashingSpans { } /// Parameters for performing a slash. +#[derive(Clone)] pub(crate) struct SlashParams<'a, T: 'a + Trait> { /// The stash account being slashed. pub(crate) stash: &'a T::AccountId, - /// The proportion and total amount of the slash (proportion applied to `exposure.total`). - pub(crate) slash: (Perbill, BalanceOf), + /// The proportion of the slash. + pub(crate) slash: Perbill, /// The exposure of the stash and all nominators. pub(crate) exposure: &'a Exposure>, /// The era where the offence occurred. @@ -161,19 +162,20 @@ pub(crate) fn slash(params: SlashParams){ exposure, slash_era, window_start, - } = params; - let (slash_proportion, slash_amount) = slash; + } = params.clone(); // is the slash amount here a maximum for the era? - let era_slash = as Store>::SlashInEra::get(&slash_era, stash) - .map(|(_, amount)| amount) - .unwrap_or(Zero::zero()); + let own_slash = slash * exposure.own; + let (prior_slash_p, era_slash) = as Store>::ValidatorSlashInEra::get( + &slash_era, + stash, + ).unwrap_or((Perbill::zero(), Zero::zero())); - if slash_amount > era_slash { - as Store>::SlashInEra::insert( + if own_slash > era_slash { + as Store>::ValidatorSlashInEra::insert( &slash_era, stash, - &(slash_proportion, slash_amount), + &(slash, own_slash), ); } else { // we slash based on the max in era - this new event is not the max, @@ -181,89 +183,136 @@ pub(crate) fn slash(params: SlashParams){ return } - // what about the maximum slash in the span? - let mut spans = as Store>::SlashingSpans::get(stash).unwrap_or_else(|| { - SlashingSpans::new(window_start) - }); - span_gc::(stash, spans.prune(window_start)); + // apply slash to validator. + let mut spans = fetch_spans::(stash, window_start); - as Store>::SlashingSpans::insert(stash, &spans); + let target_span = spans.compare_and_update_span_slash( + slash_era, + own_slash, + ); - let target_span = spans.iter().find(|span| span.contains_era(slash_era)); - let span_slash = >::span_slash(&(stash.clone(), target_span.index)); + if target_span == Some(spans.span_index()) { + // TODO: chill the validator - it misbehaved in the current span. + } - let spans = spans; + slash_nominators::(params, prior_slash_p) +} - // `chill` validator if this is the most recent span. - if spans.span_index == target_span.index { - // TODO - } +fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { + let SlashParams { + stash: validator, + slash, + exposure, + slash_era, + window_start, + } = params; - // false if have chewed through all of our own exposure for this era, - // true if nothing has been passed onto nominators. - let has_own_remaining = slash_amount < exposure.own; + for nominator in &exposure.others { + let stash = &nominator.who; - let effective_nominator_slash = if slash_amount > span_slash { - let remaining_slash = slash_amount - span_slash; - let remaining_own = exposure.own.saturating_sub(span_slash); + // the era slash of a nominator always grows, if the validator + // had a new max slash for the era. + let era_slash = { + let own_slash_prior = prior_slash_p * nominator.value; + let own_slash_by_validator = slash * nominator.value; + let own_slash_difference = own_slash_by_validator.saturating_sub(own_slash_prior); - // divide up the slash into own and nominator portions. - let (own_slash, nominator_slash) = if remaining_slash >= remaining_own { - (remaining_own, remaining_slash - remaining_own) - } else { - (remaining_slash, 0) + let mut era_slash = as Store>::NominatorSlashInEra::get( + &slash_era, + stash, + ).unwrap_or(Zero::zero()); + + era_slash += own_slash_difference; + + as Store>::NominatorSlashInEra::insert( + &slash_era, + stash, + &era_slash, + ); + + era_slash }; - // Apply `own_slash` to self balance. Since this is the highest slash of - // the span, we actually apply the slash to our own bond. - // TODO + // compare the era slash against other eras in the same span. + let mut spans = fetch_spans::(stash, window_start); - // note new highest slash in span - as Store>::SpanSlash::insert( - &(stash.clone(), target_span.index), - &slash_amount, + let target_span = spans.compare_and_update_span_slash( + slash_era, + era_slash, ); - nominator_slash - } else { - // this validator has already been slashed for more than `slash_amount` within - // this slashing span. That means that we do not actually apply the slash on our own - // bond, but if the nominator slash for this era is now non-zero we have - // to pass that onwards to the nominator calculation. - if has_own_remaining { - 0 - } else { - slash_amount - exposure.own + if target_span == Some(spans.span_index()) { + // TODO: chill the nominator. (only from the validator?) } - }; - - if effective_nominator_slash == 0 { - // nominators are unaffected. - return } +} - // p_{v,e} is non-zero here - it is the ratio of the remaining slash in the era - // to the amount of exposure held by nominators. - let nominator_slash_proportion = { - let d = Perbill::from_rational_approximation( - effective_nominator_slash, - exposure.total - exposure.own, - ); - let d = Perbill::one() / (exposure.total - exposure.own); - d * effective_nominator_slash; - }; +// helper struct for managing a set of spans we are currently inspecting. +// writes alterations to disk on drop, but only if a slash has been carried out. +struct InspectingSpans<'a, T: Trait + 'a> { + dirty: bool, + window_start: EraIndex, + stash: &'a T::AccountId, + spans: SlashingSpans, + _marker: rstd::marker::PhantomData, +} - // TODO: apply to nominators. +// fetches the slashing spans record for a stash account, initializing it if necessary. +fn fetch_spans(stash: &T::AccountId, window_start: EraIndex) -> InspectingSpans { + let spans = as Store>::SlashingSpans::get(stash).unwrap_or_else(|| { + let spans = SlashingSpans::new(window_start); + as Store>::SlashingSpans::insert(stash, &spans); + spans + }); + + InspectingSpans { + dirty: false, + window_start, + stash, + spans, + _marker: rstd::marker::PhantomData, + } } -fn slash_nominators() { +impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { + fn span_index(&self) -> SpanIndex { + self.spans.span_index + } + + // compares the slash in an era to the overall current span slash. + // if it's higher, applies the difference of the slashes and then updates the span on disk. + // + // returns the span index of the era, if any. + fn compare_and_update_span_slash( + &mut self, + slash_era: EraIndex, + slash: BalanceOf, + ) -> Option { + let target_span = self.spans.iter().find(|span| span.contains_era(slash_era))?; + let span_slash_key = (self.stash.clone(), target_span.index); + let span_slash = >::span_slash(&span_slash_key); + + if span_slash < slash { + // new maximum span slash. + // TODO: apply difference. + self.dirty = true; + as Store>::SpanSlash::insert(&span_slash_key, &slash); + } + Some(target_span.index) + } } -fn span_gc(stash: &T::AccountId, pruned_range: Option<(SpanIndex, SpanIndex)>) { - if let Some((start, end)) = pruned_range { - for span_index in start..end { - as Store>::SpanSlash::remove(&(stash.clone(), span_index)); +impl<'a, T: 'a + Trait> Drop for InspectingSpans<'a, T> { + fn drop(&mut self) { + // only update on disk if we slashed this account. + if !self.dirty { return } + + if let Some((start, end)) = self.spans.prune(self.window_start) { + as Store>::SlashingSpans::insert(self.stash, &self.spans); + for span_index in start..end { + as Store>::SpanSlash::remove(&(self.stash.clone(), span_index)); + } } } } From 8427efb23433365ba86a956410f38dbbba1a56fa Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 21 Oct 2019 17:22:16 -0400 Subject: [PATCH 05/46] chill and end slashing spans --- srml/staking/src/lib.rs | 9 +++-- srml/staking/src/slashing.rs | 73 +++++++++++++++++++++++++++++++++--- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 0ccc310af9239..1e385f0c40ac0 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -916,9 +916,7 @@ decl_module! { fn chill(origin) { let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or("not a controller")?; - let stash = &ledger.stash; - >::remove(stash); - >::remove(stash); + Self::chill_stash(&ledger.stash); } /// (Re-)set the payment target for a controller. @@ -1030,6 +1028,11 @@ impl Module { // MUTABLES (DANGEROUS) + fn chill_stash(stash: &T::AccountId) { + >::remove(stash); + >::remove(stash); + } + /// Update the ledger for a controller. This will also update the stash lock. The lock will /// will lock the entire funds except paying for further transactions. fn update_ledger( diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 62efd588e6a10..0f04e99f8c4c1 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -40,6 +40,9 @@ //! requiring them to re-enlist voluntarily (acknowledging the slash) and begin a new //! slashing span. //! +//! Typically, you will have a single slashing event per slashing span. Only in the case +//! where a validator releases many misbehaviors at once +//! //! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill}; @@ -89,13 +92,18 @@ impl SlashingSpans { } } - fn bump_span(&mut self, now: EraIndex) { - if now <= self.last_start { return } + // update the slashing spans to reflect the start of a new span at the era after `now` + // returns `true` if a new span was started, `false` otherwise. `false` indicates + // that internal state is unchanged. + fn end_span(&mut self, now: EraIndex) -> bool { + let next_start = now + 1; + if next_start <= self.last_start { return false } - let last_length = now - self.last_start; + let last_length = next_start - self.last_start; self.prior.insert(0, last_length); - self.last_start = now; + self.last_start = next_start; self.span_index += 1; + true } // an iterator over all slashing spans in _reverse_ order - most recent first. @@ -152,6 +160,8 @@ pub(crate) struct SlashParams<'a, T: 'a + Trait> { pub(crate) slash_era: EraIndex, /// The first era in the current bonding period. pub(crate) window_start: EraIndex, + /// The current era. + pub(crate) now: EraIndex, } /// Slash a validator and their nominators, if necessary. @@ -162,6 +172,7 @@ pub(crate) fn slash(params: SlashParams){ exposure, slash_era, window_start, + now, } = params.clone(); // is the slash amount here a maximum for the era? @@ -192,7 +203,10 @@ pub(crate) fn slash(params: SlashParams){ ); if target_span == Some(spans.span_index()) { - // TODO: chill the validator - it misbehaved in the current span. + // chill the validator - it misbehaved in the current span and should + // not continue in the next election. also end the slashing span. + spans.end_span(now); + >::chill_stash(stash); } slash_nominators::(params, prior_slash_p) @@ -205,6 +219,7 @@ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { exposure, slash_era, window_start, + now, } = params; for nominator in &exposure.others { @@ -242,7 +257,9 @@ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { ); if target_span == Some(spans.span_index()) { - // TODO: chill the nominator. (only from the validator?) + // Chill the nominator outright, ending the slashing span. + spans.end_span(now); + >::chill_stash(&stash); } } } @@ -279,6 +296,10 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { self.spans.span_index } + fn end_span(&mut self, now: EraIndex) { + self.dirty = self.spans.end_span(now) || self.dirty; + } + // compares the slash in an era to the overall current span slash. // if it's higher, applies the difference of the slashes and then updates the span on disk. // @@ -446,4 +467,44 @@ mod tests { ], ); } + + #[test] + fn ending_span() { + let mut spans = SlashingSpans { + span_index: 1, + last_start: 10, + prior: Vec::new(), + }; + + assert!(spans.end_span(10)); + + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { index: 2, start: 11, length: None }, + SlashingSpan { index: 1, start: 10, length: Some(1) }, + ], + ); + + assert!(spans.end_span(15)); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { index: 3, start: 16, length: None }, + SlashingSpan { index: 2, start: 11, length: Some(5) }, + SlashingSpan { index: 1, start: 10, length: Some(1) }, + ], + ); + + // does nothing if not a valid end. + assert!(!spans.end_span(15)); + assert_eq!( + spans.iter().collect::>(), + vec![ + SlashingSpan { index: 3, start: 16, length: None }, + SlashingSpan { index: 2, start: 11, length: Some(5) }, + SlashingSpan { index: 1, start: 10, length: Some(1) }, + ], + ); + } } From 428873662ef61e226c133813d2bd730a876d22eb Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 21 Oct 2019 22:15:32 -0400 Subject: [PATCH 06/46] actually perform slashes --- srml/staking/src/lib.rs | 14 ++++----- srml/staking/src/slashing.rs | 57 ++++++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 1e385f0c40ac0..a9fe9bd396938 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1020,19 +1020,13 @@ decl_module! { impl Module { // PUBLIC IMMUTABLES - /// The total balance that can be slashed from a validator controller account as of - /// right now. + /// The total balance that can be slashed from a stash account as of right now. pub fn slashable_balance_of(stash: &T::AccountId) -> BalanceOf { Self::bonded(stash).and_then(Self::ledger).map(|l| l.active).unwrap_or_default() } // MUTABLES (DANGEROUS) - fn chill_stash(stash: &T::AccountId) { - >::remove(stash); - >::remove(stash); - } - /// Update the ledger for a controller. This will also update the stash lock. The lock will /// will lock the entire funds except paying for further transactions. fn update_ledger( @@ -1049,6 +1043,12 @@ impl Module { >::insert(controller, ledger); } + /// Chill a stash account. + fn chill_stash(stash: &T::AccountId) { + >::remove(stash); + >::remove(stash); + } + /// Slash a given validator by a specific amount with given (historical) exposure. /// /// Removes the slash from the validator's balance by preference, diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 0f04e99f8c4c1..c0af945d993ac 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -47,8 +47,8 @@ use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill}; use sr_primitives::traits::{Zero, Saturating}; -use support::{StorageValue, StorageMap, StorageDoubleMap}; -use codec::{HasCompact, Encode, Decode}; +use support::{StorageValue, StorageMap, StorageDoubleMap, traits::Currency}; +use codec::{Encode, Decode}; /// The index of a slashing span - unique to each stash. pub (crate) type SpanIndex = u32; @@ -271,6 +271,7 @@ struct InspectingSpans<'a, T: Trait + 'a> { window_start: EraIndex, stash: &'a T::AccountId, spans: SlashingSpans, + deferred_slashes: Vec>, _marker: rstd::marker::PhantomData, } @@ -287,6 +288,7 @@ fn fetch_spans(stash: &T::AccountId, window_start: EraIndex) -> Inspec window_start, stash, spans, + deferred_slashes: Vec::with_capacity(1), // we usually would only slash once. _marker: rstd::marker::PhantomData, } } @@ -303,7 +305,8 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { // compares the slash in an era to the overall current span slash. // if it's higher, applies the difference of the slashes and then updates the span on disk. // - // returns the span index of the era, if any. + // returns the span index of the era, if any, along with a slash to be lazily applied + // when all book-keeping is done. fn compare_and_update_span_slash( &mut self, slash_era: EraIndex, @@ -314,10 +317,13 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { let span_slash = >::span_slash(&span_slash_key); if span_slash < slash { - // new maximum span slash. - // TODO: apply difference. + // new maximum span slash. apply the difference. + let difference = slash - span_slash; + self.dirty = true; as Store>::SpanSlash::insert(&span_slash_key, &slash); + + self.deferred_slashes.push(slash_lazy(self.stash.clone(), difference)); } Some(target_span.index) @@ -338,6 +344,47 @@ impl<'a, T: 'a + Trait> Drop for InspectingSpans<'a, T> { } } +// perform a slash which is lazily applied on `Drop`. This allows us to register +// that the slash should happen without being in the middle of any bookkeeping. +fn slash_lazy(stash: T::AccountId, value: BalanceOf) -> LazySlash { + LazySlash { stash, value } +} + +struct LazySlash { + stash: T::AccountId, + value: BalanceOf, +} + +impl Drop for LazySlash { + fn drop(&mut self) { + let mut ledger = match >::ledger(&self.stash) { + Some(ledger) => ledger, + None => return, // nothing to do. + }; + + let controller = match >::bonded(&self.stash) { + None => return, // defensive: should always exist. + Some(c) => c, + }; + + let mut value = self.value.min(ledger.active); + + if !value.is_zero() { + ledger.active -= value; + + // Avoid there being a dust balance left in the staking system. + if ledger.active < T::Currency::minimum_balance() { + value += ledger.active; + ledger.active = Zero::zero(); + } + + ledger.total -= value; + T::Currency::slash(&self.stash, value); + >::update_ledger(&controller, &ledger); + } + } +} + #[cfg(test)] mod tests { use super::*; From 372a4ce42e36160fa5947c3469b511b1fe599cce Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 22 Oct 2019 01:26:28 -0400 Subject: [PATCH 07/46] integration (tests failing) --- srml/staking/src/lib.rs | 139 ++++++++++++----------------------- srml/staking/src/slashing.rs | 28 +++++-- srml/staking/src/tests.rs | 7 +- 3 files changed, 72 insertions(+), 102 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index a9fe9bd396938..f1225c98ebb27 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1049,71 +1049,6 @@ impl Module { >::remove(stash); } - /// Slash a given validator by a specific amount with given (historical) exposure. - /// - /// Removes the slash from the validator's balance by preference, - /// and reduces the nominators' balance if needed. - /// - /// Returns the resulting `NegativeImbalance` to allow distributing the slashed amount and - /// pushes an entry onto the slash journal. - fn slash_validator( - stash: &T::AccountId, - slash: BalanceOf, - exposure: &Exposure>, - journal: &mut Vec>>, - ) -> NegativeImbalanceOf { - // TODO: move over to `slashing.rs` logic. - - // The amount we are actually going to slash (can't be bigger than the validator's total - // exposure) - let slash = slash.min(exposure.total); - - // limit what we'll slash of the stash's own to only what's in - // the exposure. - // - // note: this is fine only because we limit reports of the current era. - // otherwise, these funds may have already been slashed due to something - // reported from a prior era. - let already_slashed_own = journal.iter() - .filter(|entry| &entry.who == stash) - .map(|entry| entry.own_slash) - .fold(>::zero(), |a, c| a.saturating_add(c)); - - let own_remaining = exposure.own.saturating_sub(already_slashed_own); - - // The amount we'll slash from the validator's stash directly. - let own_slash = own_remaining.min(slash); - let (mut imbalance, missing) = T::Currency::slash(stash, own_slash); - let own_slash = own_slash - missing; - // The amount remaining that we can't slash from the validator, - // that must be taken from the nominators. - let rest_slash = slash - own_slash; - if !rest_slash.is_zero() { - // The total to be slashed from the nominators. - let total = exposure.total - exposure.own; - if !total.is_zero() { - for i in exposure.others.iter() { - let per_u64 = Perbill::from_rational_approximation(i.value, total); - // best effort - not much that can be done on fail. - imbalance.subsume(T::Currency::slash(&i.who, per_u64 * rest_slash).0) - } - } - } - - journal.push(SlashJournalEntry { - who: stash.clone(), - own_slash: own_slash.clone(), - amount: slash, - }); - - // trigger the event - Self::deposit_event( - RawEvent::Slash(stash.clone(), slash) - ); - - imbalance - } - /// Actually make a payment to a staker. This uses the currency's reward function /// to pay the right payee for the given staker account. fn make_payout(stash: &T::AccountId, amount: BalanceOf) -> Option> { @@ -1508,14 +1443,21 @@ impl OnOffenceHandler>], slash_fraction: &[Perbill], ) { - let mut remaining_imbalance = >::zero(); let slash_reward_fraction = SlashRewardFraction::get(); let era_now = Self::current_era(); + let window_start = era_now.saturating_sub(T::BondingDuration::get()); + let current_era_start_session = CurrentEraStartSessionIndex::get(); + let mut bonded_eras = None; + for (details, slash_fraction) in offenders.iter().zip(slash_fraction) { let stash = &details.offender.0; let exposure = &details.offender.1; + // TODO + // let slash_session = unimplemented!(); + let slash_session = current_era_start_session; + // Skip if the validator is invulnerable. if Self::invulnerables().contains(stash) { continue @@ -1524,42 +1466,51 @@ impl OnOffenceHandler= current_era_start_session { + era_now } else { - remaining_imbalance.subsume(slashed_amount); - } - } + let eras = bonded_eras.get_or_insert_with(|| BondedEras::get()).iter().rev(); + match eras.filter(|&&(_, ref sesh)| sesh <= &slash_session).next() { + None => continue, // before bonding period. defensive - should be filtered out. + Some(&(ref slash_era, _)) => *slash_era, + } + }; + + slashing::slash::(slashing::SlashParams { + stash, + slash: *slash_fraction, + exposure, + slash_era, + window_start, + now: era_now, + }) - // Handle the rest of imbalances - T::Slash::on_unbalanced(remaining_imbalance); + // TODO: remove when rewards are done again. + // let slash_reward = slash_reward_fraction * slashed_amount.peek(); + // if !slash_reward.is_zero() && !details.reporters.is_empty() { + // let (mut reward, rest) = slashed_amount.split(slash_reward); + // // split the reward between reporters equally. Division cannot fail because + // // we guarded against it in the enclosing if. + // let per_reporter = reward.peek() / (details.reporters.len() as u32).into(); + // for reporter in &details.reporters { + // let (reporter_reward, rest) = reward.split(per_reporter); + // reward = rest; + // T::Currency::resolve_creating(reporter, reporter_reward); + // } + // // The rest goes to the treasury. + // remaining_imbalance.subsume(reward); + // remaining_imbalance.subsume(rest); + // } else { + // remaining_imbalance.subsume(slashed_amount); + // } + } } } diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index c0af945d993ac..f55a17d74a118 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -45,13 +45,13 @@ //! //! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ -use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill}; +use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill, SessionInterface}; use sr_primitives::traits::{Zero, Saturating}; -use support::{StorageValue, StorageMap, StorageDoubleMap, traits::Currency}; +use support::{StorageValue, StorageMap, StorageDoubleMap, traits::{Currency, OnUnbalanced}}; use codec::{Encode, Decode}; /// The index of a slashing span - unique to each stash. -pub (crate) type SpanIndex = u32; +pub(crate) type SpanIndex = u32; // A range of start..end eras for a slashing span. #[derive(Encode, Decode)] @@ -203,10 +203,19 @@ pub(crate) fn slash(params: SlashParams){ ); if target_span == Some(spans.span_index()) { + // misbehavior occurred within the current slashing span - take appropriate + // actions. + // chill the validator - it misbehaved in the current span and should // not continue in the next election. also end the slashing span. spans.end_span(now); >::chill_stash(stash); + + // 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 + crate::ForceEra::put(crate::Forcing::ForceNew); + } } slash_nominators::(params, prior_slash_p) @@ -214,7 +223,7 @@ pub(crate) fn slash(params: SlashParams){ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { let SlashParams { - stash: validator, + stash: _, slash, exposure, slash_era, @@ -379,8 +388,17 @@ impl Drop for LazySlash { } ledger.total -= value; - T::Currency::slash(&self.stash, value); + + // TODO: rewards + let (imbalance, _) = T::Currency::slash(&self.stash, value); + T::Slash::on_unbalanced(imbalance); + >::update_ledger(&controller, &ledger); + + // trigger the event + >::deposit_event( + super::RawEvent::Slash(self.stash.clone(), value) + ); } } } diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 2b821764579d4..35a8dbd8bab52 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1651,9 +1651,10 @@ fn reward_validator_slashing_validator_doesnt_overflow() { ]}); // Check slashing - let _ = Staking::slash_validator(&11, reward_slash, &Staking::stakers(&11), &mut Vec::new()); - assert_eq!(Balances::total_balance(&11), stake - 1); - assert_eq!(Balances::total_balance(&2), 1); + // TODO + // let _ = Staking::slash_validator(&11, reward_slash, &Staking::stakers(&11), &mut Vec::new()); + // assert_eq!(Balances::total_balance(&11), stake - 1); + // assert_eq!(Balances::total_balance(&2), 1); }) } From 06af8d0f441e6b477dd260fd535cb0515c74b793 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 24 Oct 2019 17:53:18 -0400 Subject: [PATCH 08/46] prune metadata --- srml/staking/src/lib.rs | 20 ++++++-------------- srml/staking/src/slashing.rs | 36 +++++++++++++++++++++++++++++++----- srml/staking/src/tests.rs | 20 ++++++++++++++------ 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index f1225c98ebb27..13200c5f76dcb 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -432,14 +432,6 @@ pub struct Exposure { pub others: Vec>, } -/// A slashing event occurred, slashing a validator for a given amount of balance. -#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Decode, Default, RuntimeDebug)] -pub struct SlashJournalEntry { - who: AccountId, - amount: Balance, - own_slash: Balance, // the amount of `who`'s own exposure that was slashed -} - pub type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type PositiveImbalanceOf = @@ -1164,10 +1156,6 @@ impl Module { // Increment current era. let current_era = CurrentEra::mutate(|s| { *s += 1; *s }); - // prune journal for last era. - // TODO: prune journal for the era just exiting the bonding window. - // >::remove(current_era - 1); - CurrentEraStartSessionIndex::mutate(|v| { *v = start_session_index; }); @@ -1175,6 +1163,7 @@ impl Module { if current_era > bonding_duration { let first_kept = current_era - bonding_duration; + BondedEras::mutate(|bonded| { bonded.push((current_era, start_session_index)); @@ -1183,7 +1172,10 @@ impl Module { .take_while(|&&(era_idx, _)| era_idx < first_kept) .count(); - bonded.drain(..n_to_prune); + // kill slashing metadata. + for (pruned_era, _) in bonded.drain(..n_to_prune) { + slashing::clear_era_metadata(pruned_era); + } if let Some(&(_, first_session)) = bonded.first() { T::SessionInterface::prune_historical_up_to(first_session); @@ -1317,7 +1309,7 @@ impl Module { >::remove(stash); >::remove(stash); - // TODO: clear all slashing bookkeeping (slashing spans, etc). + slashing::clear_stash_metadata::(stash); } /// Add reward points to validators using their stash account ID. diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index f55a17d74a118..0fcb9af3db467 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -275,6 +275,11 @@ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { // helper struct for managing a set of spans we are currently inspecting. // writes alterations to disk on drop, but only if a slash has been carried out. +// +// NOTE: alterations to slashing metadata should not be done after this is dropped. +// dropping this struct applies any necessary slashes, which can lead to free balance +// being 0, and the account being garbage-collected -- a dead account should get no new +// metadata. struct InspectingSpans<'a, T: Trait + 'a> { dirty: bool, window_start: EraIndex, @@ -359,6 +364,26 @@ fn slash_lazy(stash: T::AccountId, value: BalanceOf) -> LazySlash(obsolete_era: EraIndex) { + as Store>::ValidatorSlashInEra::remove_prefix(&obsolete_era); + as Store>::NominatorSlashInEra::remove_prefix(&obsolete_era); +} + +/// Clear slashing metadata for a dead account. +pub(crate) fn clear_stash_metadata(stash: &T::AccountId) { + let spans = as Store>::SlashingSpans::take(stash); + + // kill slashing-span metadata for account. + // + // this can only happen while the account is staked _if_ they are completely slashed. + // in that case, they may re-bond, but it would count again as span 0. Further ancient + // slashes would slash into this new bond, since metadata has now been cleared. + for span in spans.into_iter().flat_map(|spans| spans) { + as Store>::SpanSlash::remove(&(stash.clone()), span.index); + } +} + struct LazySlash { stash: T::AccountId, value: BalanceOf, @@ -366,16 +391,16 @@ struct LazySlash { impl Drop for LazySlash { fn drop(&mut self) { - let mut ledger = match >::ledger(&self.stash) { - Some(ledger) => ledger, - None => return, // nothing to do. - }; - let controller = match >::bonded(&self.stash) { None => return, // defensive: should always exist. Some(c) => c, }; + let mut ledger = match >::ledger(&controller) { + Some(ledger) => ledger, + None => return, // nothing to do. + }; + let mut value = self.value.min(ledger.active); if !value.is_zero() { @@ -390,6 +415,7 @@ impl Drop for LazySlash { ledger.total -= value; // TODO: rewards + let (imbalance, _) = T::Currency::slash(&self.stash, value); T::Slash::on_unbalanced(imbalance); diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 35a8dbd8bab52..984dc3c6a4831 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1825,14 +1825,14 @@ fn reporters_receive_their_slice() { fn invulnerables_are_not_slashed() { // For invulnerable validators no slashing is performed. ExtBuilder::default().invulnerables(vec![11]).build().execute_with(|| { - #[cfg(feature = "equalize")] - let initial_balance = 1250; - #[cfg(not(feature = "equalize"))] - let initial_balance = 1375; - assert_eq!(Balances::free_balance(&11), 1000); assert_eq!(Balances::free_balance(&21), 2000); - assert_eq!(Staking::stakers(&21).total, initial_balance); + + let exposure = Staking::stakers(&21); + let initial_balance = Staking::slashable_balance_of(&21); + + let nominator_balances: Vec<_> = exposure.others + .iter().map(|o| Balances::free_balance(&o.who)).collect(); Staking::on_offence( &[ @@ -1852,6 +1852,14 @@ fn invulnerables_are_not_slashed() { assert_eq!(Balances::free_balance(&11), 1000); // 2000 - (0.2 * initial_balance) assert_eq!(Balances::free_balance(&21), 2000 - (2 * initial_balance / 10)); + + // ensure that nominators were slashed as well. + for (initial_balance, other) in nominator_balances.into_iter().zip(exposure.others) { + assert_eq!( + Balances::free_balance(&other.who), + initial_balance - (2 * other.value / 10), + ); + } }); } From 2468334e05a0ee62cdce8529fcb7d4b088b6e219 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 24 Oct 2019 17:59:54 -0400 Subject: [PATCH 09/46] fix compilation --- srml/staking/src/lib.rs | 2 +- srml/staking/src/slashing.rs | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 13200c5f76dcb..0f1940ff10154 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1174,7 +1174,7 @@ impl Module { // kill slashing metadata. for (pruned_era, _) in bonded.drain(..n_to_prune) { - slashing::clear_era_metadata(pruned_era); + slashing::clear_era_metadata::(pruned_era); } if let Some(&(_, first_session)) = bonded.first() { diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 0fcb9af3db467..2340543ba02ea 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -372,15 +372,18 @@ pub(crate) fn clear_era_metadata(obsolete_era: EraIndex) { /// Clear slashing metadata for a dead account. pub(crate) fn clear_stash_metadata(stash: &T::AccountId) { - let spans = as Store>::SlashingSpans::take(stash); + let spans = match as Store>::SlashingSpans::take(stash) { + None => return, + Some(s) => s, + }; // kill slashing-span metadata for account. // // this can only happen while the account is staked _if_ they are completely slashed. // in that case, they may re-bond, but it would count again as span 0. Further ancient // slashes would slash into this new bond, since metadata has now been cleared. - for span in spans.into_iter().flat_map(|spans| spans) { - as Store>::SpanSlash::remove(&(stash.clone()), span.index); + for span in spans.iter() { + as Store>::SpanSlash::remove(&(stash.clone(), span.index)); } } From d0854ae3596d15b4907768a73dc5f1e5fb665732 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 25 Oct 2019 16:25:02 -0400 Subject: [PATCH 10/46] some tests for slashing and metadata garbage collection --- srml/balances/src/lib.rs | 1 + srml/staking/src/lib.rs | 12 ++-- srml/staking/src/tests.rs | 127 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 6 deletions(-) diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 71f37cb8f8193..7246bcd0520a5 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -921,6 +921,7 @@ where ) -> (Self::NegativeImbalance, Self::Balance) { let free_balance = Self::free_balance(who); let free_slash = cmp::min(free_balance, value); + Self::set_free_balance(who, free_balance - free_slash); let remaining_slash = value - free_slash; // NOTE: `slash()` prefers free balance, but assumes that reserve balance can be drawn diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 0f1940ff10154..3e13d327ae4e9 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1161,11 +1161,11 @@ impl Module { }); let bonding_duration = T::BondingDuration::get(); - if current_era > bonding_duration { - let first_kept = current_era - bonding_duration; + BondedEras::mutate(|bonded| { + bonded.push((current_era, start_session_index)); - BondedEras::mutate(|bonded| { - bonded.push((current_era, start_session_index)); + if current_era > bonding_duration { + let first_kept = current_era - bonding_duration; // prune out everything that's from before the first-kept index. let n_to_prune = bonded.iter() @@ -1180,8 +1180,8 @@ impl Module { if let Some(&(_, first_session)) = bonded.first() { T::SessionInterface::prune_historical_up_to(first_session); } - }) - } + } + }); // Reassign all Stakers. let (_slot_stake, maybe_new_validators) = Self::select_validators(); diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 984dc3c6a4831..ccd1358e9332e 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1885,3 +1885,130 @@ fn dont_slash_if_fraction_is_zero() { assert_eq!(Staking::force_era(), Forcing::NotForcing); }); } + +#[test] +fn only_slash_for_max_in_era() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(Balances::free_balance(&11), 1000); + + Staking::on_offence( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(50)], + ); + + // The validator has been slashed and has been force-chilled. + assert_eq!(Balances::free_balance(&11), 500); + assert_eq!(Staking::force_era(), Forcing::ForceNew); + + Staking::on_offence( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(25)], + ); + + // The validator has not been slashed additionally. + assert_eq!(Balances::free_balance(&11), 500); + + Staking::on_offence( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(60)], + ); + + // The validator got slashed 10% more. + assert_eq!(Balances::free_balance(&11), 400); + }) +} + +#[test] +fn garbage_collection_after_slashing() { + ExtBuilder::default().existential_deposit(1).build().execute_with(|| { + assert_eq!(Balances::free_balance(&11), 256_000); + + Staking::on_offence( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + ); + + assert_eq!(Balances::free_balance(&11), 256_000 - 25_600); + assert!(::SlashingSpans::get(&11).is_some()); + assert_eq!(::SpanSlash::get(&(11, 0)), 25_600); + + Staking::on_offence( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(100)], + ); + + // validator and nominator slash in era are garbage-collected by era change. + assert_eq!(Balances::free_balance(&11), 0); + assert!(::SlashingSpans::get(&11).is_none()); + assert_eq!(::SpanSlash::get(&(11, 0)), 0); + }) +} + +#[test] +fn garbage_collection_on_window_pruning() { + 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); + let nominated_value = exposure.others.iter().find(|o| o.who == 101).unwrap().value; + + Staking::on_offence( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + ); + + let now = Staking::current_era(); + + assert_eq!(Balances::free_balance(&11), 900); + assert_eq!(Balances::free_balance(&101), 2000 - (nominated_value / 10)); + + assert!(::ValidatorSlashInEra::get(&now, &11).is_some()); + assert!(::NominatorSlashInEra::get(&now, &101).is_some()); + + let slash_era = now; + + // + 1 because we have to exit the bonding window. + for era in (0..(BondingDuration::get() + 1)).map(|offset| offset + now + 1) { + assert!(::ValidatorSlashInEra::get(&now, &11).is_some()); + assert!(::NominatorSlashInEra::get(&now, &101).is_some()); + + start_era(era); + } + + assert!(::ValidatorSlashInEra::get(&now, &11).is_none()); + assert!(::NominatorSlashInEra::get(&now, &101).is_none()); + }) +} From 47f3c547ce9cee6f146592b8af5abdf65c5d6d1e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 25 Oct 2019 18:07:35 -0400 Subject: [PATCH 11/46] correctly pass session index to slash handler --- core/sr-staking-primitives/src/offence.rs | 4 +++ srml/offences/src/lib.rs | 6 +++- srml/staking/src/lib.rs | 37 ++++++++++++----------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/core/sr-staking-primitives/src/offence.rs b/core/sr-staking-primitives/src/offence.rs index db51f75df1bf3..04d887fbe09fc 100644 --- a/core/sr-staking-primitives/src/offence.rs +++ b/core/sr-staking-primitives/src/offence.rs @@ -117,9 +117,12 @@ pub trait OnOffenceHandler { /// the authorities should be slashed and is computed /// according to the `OffenceCount` already. This is of the same length as `offenders.` /// Zero is a valid value for a fraction. + /// + /// The `session` parameter is the session index of the offence. fn on_offence( offenders: &[OffenceDetails], slash_fraction: &[Perbill], + session: SessionIndex, ); } @@ -127,6 +130,7 @@ impl OnOffenceHandler for () { fn on_offence( _offenders: &[OffenceDetails], _slash_fraction: &[Perbill], + _session: SessionIndex, ) {} } diff --git a/srml/offences/src/lib.rs b/srml/offences/src/lib.rs index a6cf4d7956467..6580ede084d9b 100644 --- a/srml/offences/src/lib.rs +++ b/srml/offences/src/lib.rs @@ -150,7 +150,11 @@ where }) .collect::>(); - T::OnOffenceHandler::on_offence(&concurrent_offenders, &slash_perbill); + T::OnOffenceHandler::on_offence( + &concurrent_offenders, + &slash_perbill, + offence.session_index(), + ); } } diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 3e13d327ae4e9..cc2a94a363737 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1434,22 +1434,31 @@ impl OnOffenceHandler>], slash_fraction: &[Perbill], + slash_session: SessionIndex, ) { let slash_reward_fraction = SlashRewardFraction::get(); let era_now = Self::current_era(); let window_start = era_now.saturating_sub(T::BondingDuration::get()); let current_era_start_session = CurrentEraStartSessionIndex::get(); - let mut bonded_eras = None; + + // fast path for current-era report - most likely. + let slash_era = if slash_session >= current_era_start_session { + era_now + } else { + let eras = BondedEras::get(); + + // reverse because it's more likely to find reports from recent eras. + match eras.iter().rev().filter(|&&(_, ref sesh)| sesh <= &slash_session).next() { + None => return, // before bonding period. defensive - should be filtered out. + Some(&(ref slash_era, _)) => *slash_era, + } + }; for (details, slash_fraction) in offenders.iter().zip(slash_fraction) { let stash = &details.offender.0; let exposure = &details.offender.1; - // TODO - // let slash_session = unimplemented!(); - let slash_session = current_era_start_session; - // Skip if the validator is invulnerable. if Self::invulnerables().contains(stash) { continue @@ -1465,16 +1474,6 @@ impl OnOffenceHandler= current_era_start_session { - era_now - } else { - let eras = bonded_eras.get_or_insert_with(|| BondedEras::get()).iter().rev(); - match eras.filter(|&&(_, ref sesh)| sesh <= &slash_session).next() { - None => continue, // before bonding period. defensive - should be filtered out. - Some(&(ref slash_era, _)) => *slash_era, - } - }; - slashing::slash::(slashing::SlashParams { stash, slash: *slash_fraction, @@ -1506,7 +1505,7 @@ impl OnOffenceHandler { _inner: rstd::marker::PhantomData<(T, R)>, } @@ -1518,9 +1517,11 @@ impl ReportOffence O: Offence, { fn report_offence(reporters: Vec, offence: O) { - // disallow any slashing from before the current era. + // disallow any slashing from before the current bonding period. let offence_session = offence.session_index(); - if offence_session >= >::current_era_start_session_index() { + let bonded_eras = BondedEras::get(); + + if bonded_eras.first().filter(|(_, start)| offence_session >= *start).is_some() { R::report_offence(reporters, offence) } else { >::deposit_event( From 3145ac373602d62c9bd93f72cdbf2f874470f3c0 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 25 Oct 2019 18:07:59 -0400 Subject: [PATCH 12/46] test span-max property for nominators and validators --- srml/staking/src/mock.rs | 34 ++++++++- srml/staking/src/slashing.rs | 13 ++-- srml/staking/src/tests.rs | 131 ++++++++++++++++++++++++++++++----- 3 files changed, 154 insertions(+), 24 deletions(-) diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index 41a2ad4802097..b69166d51573a 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -21,10 +21,10 @@ use sr_primitives::Perbill; use sr_primitives::curve::PiecewiseLinear; use sr_primitives::traits::{IdentityLookup, Convert, OpaqueKeys, OnInitialize, SaturatedConversion}; use sr_primitives::testing::{Header, UintAuthorityId}; -use sr_staking_primitives::SessionIndex; +use sr_staking_primitives::{SessionIndex, offence::{OffenceDetails, OnOffenceHandler}}; use primitives::H256; use runtime_io; -use support::{assert_ok, impl_outer_origin, parameter_types, StorageLinkedMap}; +use support::{assert_ok, impl_outer_origin, parameter_types, StorageLinkedMap, StorageValue}; use support::traits::{Currency, Get, FindAuthor}; use crate::{ EraIndex, GenesisConfig, Module, Trait, StakerStatus, ValidatorPrefs, RewardDestination, @@ -451,3 +451,33 @@ pub fn reward_all_elected() { pub fn validator_controllers() -> Vec { Session::validators().into_iter().map(|s| Staking::bonded(&s).expect("no controller for validator")).collect() } + +pub fn on_offence_in_era( + offenders: &[OffenceDetails>], + slash_fraction: &[Perbill], + era: EraIndex, +) { + let bonded_eras = crate::BondedEras::get(); + for &(bonded_era, start_session) in bonded_eras.iter() { + if bonded_era == era { + Staking::on_offence(offenders, slash_fraction, start_session); + return + } else if bonded_era > era { + break + } + } + + if Staking::current_era() == era { + Staking::on_offence(offenders, slash_fraction, Staking::current_era_start_session_index()); + } else { + panic!("cannot slash in era {}", era); + } +} + +pub fn on_offence_now( + offenders: &[OffenceDetails>], + slash_fraction: &[Perbill], +) { + let now = Staking::current_era(); + on_offence_in_era(offenders, slash_fraction, now) +} diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 2340543ba02ea..d01430f2fc2cf 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -56,10 +56,10 @@ pub(crate) type SpanIndex = u32; // A range of start..end eras for a slashing span. #[derive(Encode, Decode)] #[cfg_attr(test, derive(Debug, PartialEq))] -struct SlashingSpan { - index: SpanIndex, - start: EraIndex, - length: Option, // the ongoing slashing span has indeterminate length. +pub(crate) struct SlashingSpan { + pub(crate) index: SpanIndex, + pub(crate) start: EraIndex, + pub(crate) length: Option, // the ongoing slashing span has indeterminate length. } impl SlashingSpan { @@ -107,7 +107,7 @@ impl SlashingSpans { } // an iterator over all slashing spans in _reverse_ order - most recent first. - fn iter(&'_ self) -> impl Iterator + '_ { + pub(crate) fn iter(&'_ self) -> impl Iterator + '_ { let mut last_start = self.last_start; let mut index = self.span_index; let last = SlashingSpan { index, start: last_start, length: None }; @@ -350,11 +350,12 @@ impl<'a, T: 'a + Trait> Drop for InspectingSpans<'a, T> { if !self.dirty { return } if let Some((start, end)) = self.spans.prune(self.window_start) { - as Store>::SlashingSpans::insert(self.stash, &self.spans); for span_index in start..end { as Store>::SpanSlash::remove(&(self.stash.clone(), span_index)); } } + + as Store>::SlashingSpans::insert(self.stash, &self.spans); } } diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index ccd1358e9332e..127b0e40f888d 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -19,7 +19,7 @@ use super::*; use mock::*; use sr_primitives::{assert_eq_error_rate, traits::OnInitialize}; -use sr_staking_primitives::offence::{OffenceDetails, OnOffenceHandler}; +use sr_staking_primitives::offence::OffenceDetails; use support::{assert_ok, assert_noop, assert_eq_uvec, traits::{Currency, ReservableCurrency}}; #[test] @@ -637,7 +637,7 @@ fn nominators_also_get_slashed() { assert_eq!(Balances::total_balance(&2), initial_balance); // 10 goes offline - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1747,7 +1747,7 @@ fn era_is_always_same_length() { #[test] fn offence_forces_new_era() { ExtBuilder::default().build().execute_with(|| { - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1770,7 +1770,7 @@ fn slashing_performed_according_exposure() { assert_eq!(Staking::stakers(&11).own, 1000); // Handle an offence with a historical exposure. - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1803,7 +1803,7 @@ fn reporters_receive_their_slice() { assert_eq!(Staking::stakers(&11).total, initial_balance); - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1834,7 +1834,7 @@ fn invulnerables_are_not_slashed() { let nominator_balances: Vec<_> = exposure.others .iter().map(|o| Balances::free_balance(&o.who)).collect(); - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1869,7 +1869,7 @@ fn dont_slash_if_fraction_is_zero() { ExtBuilder::default().build().execute_with(|| { assert_eq!(Balances::free_balance(&11), 1000); - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1891,7 +1891,7 @@ fn only_slash_for_max_in_era() { ExtBuilder::default().build().execute_with(|| { assert_eq!(Balances::free_balance(&11), 1000); - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1905,7 +1905,7 @@ fn only_slash_for_max_in_era() { assert_eq!(Balances::free_balance(&11), 500); assert_eq!(Staking::force_era(), Forcing::ForceNew); - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1918,7 +1918,7 @@ fn only_slash_for_max_in_era() { // The validator has not been slashed additionally. assert_eq!(Balances::free_balance(&11), 500); - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1938,7 +1938,7 @@ fn garbage_collection_after_slashing() { ExtBuilder::default().existential_deposit(1).build().execute_with(|| { assert_eq!(Balances::free_balance(&11), 256_000); - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1952,7 +1952,7 @@ fn garbage_collection_after_slashing() { assert!(::SlashingSpans::get(&11).is_some()); assert_eq!(::SpanSlash::get(&(11, 0)), 25_600); - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1962,7 +1962,9 @@ fn garbage_collection_after_slashing() { &[Perbill::from_percent(100)], ); - // validator and nominator slash in era are garbage-collected by era change. + // validator and nominator slash in era are garbage-collected by era change, + // so we don't test those here. + assert_eq!(Balances::free_balance(&11), 0); assert!(::SlashingSpans::get(&11).is_none()); assert_eq!(::SpanSlash::get(&(11, 0)), 0); @@ -1980,7 +1982,7 @@ fn garbage_collection_on_window_pruning() { assert_eq!(Balances::free_balance(&101), 2000); let nominated_value = exposure.others.iter().find(|o| o.who == 101).unwrap().value; - Staking::on_offence( + on_offence_now( &[ OffenceDetails { offender: (11, Staking::stakers(&11)), @@ -1998,8 +2000,6 @@ fn garbage_collection_on_window_pruning() { assert!(::ValidatorSlashInEra::get(&now, &11).is_some()); assert!(::NominatorSlashInEra::get(&now, &101).is_some()); - let slash_era = now; - // + 1 because we have to exit the bonding window. for era in (0..(BondingDuration::get() + 1)).map(|offset| offset + now + 1) { assert!(::ValidatorSlashInEra::get(&now, &11).is_some()); @@ -2012,3 +2012,102 @@ fn garbage_collection_on_window_pruning() { assert!(::NominatorSlashInEra::get(&now, &101).is_none()); }) } + +#[test] +fn slashing_nominators_by_span_max() { + ExtBuilder::default().build().execute_with(|| { + start_era(1); + start_era(2); + start_era(3); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&21), 2000); + assert_eq!(Staking::slashable_balance_of(&21), 1000); + + + let exposure_11 = Staking::stakers(&11); + let exposure_21 = Staking::stakers(&21); + assert_eq!(Balances::free_balance(&101), 2000); + let nominated_value_11 = exposure_11.others.iter().find(|o| o.who == 101).unwrap().value; + let nominated_value_21 = exposure_21.others.iter().find(|o| o.who == 101).unwrap().value; + + on_offence_in_era( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + 2, + ); + + assert_eq!(Balances::free_balance(&11), 900); + + let slash_1_amount = nominated_value_11 / 10; + assert_eq!(Balances::free_balance(&101), 2000 - slash_1_amount); + + let expected_spans = vec![ + slashing::SlashingSpan { index: 1, start: 4, length: None }, + slashing::SlashingSpan { index: 0, start: 0, length: Some(4) }, + ]; + + let get_span = |account| ::SlashingSpans::get(&account).unwrap(); + + assert_eq!( + get_span(11).iter().collect::>(), + expected_spans, + ); + + assert_eq!( + get_span(101).iter().collect::>(), + expected_spans, + ); + + // second slash: higher era, higher value, same span. + on_offence_in_era( + &[ + OffenceDetails { + offender: (21, Staking::stakers(&21)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(30)], + 3, + ); + + // 11 was not further slashed, but 21 and 101 were. + assert_eq!(Balances::free_balance(&11), 900); + assert_eq!(Balances::free_balance(&21), 1700); + + let slash_2_amount = 3 * (nominated_value_21 / 10); + assert!(slash_2_amount > slash_1_amount); + + // only the maximum slash in a single span is taken. + assert_eq!(Balances::free_balance(&101), 2000 - slash_2_amount); + + // third slash: in same era and on same validator as first, higher + // in-era value, but lower slash value than slash 2. + on_offence_in_era( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(20)], + 2, + ); + + // 11 was further slashed, but 21 and 101 were not. + assert_eq!(Balances::free_balance(&11), 800); + assert_eq!(Balances::free_balance(&21), 1700); + + let slash_3_amount = 2 * (nominated_value_11 / 10); + assert!(slash_3_amount < slash_2_amount); + assert!(slash_3_amount > slash_1_amount); + + // only the maximum slash in a single span is taken. + assert_eq!(Balances::free_balance(&101), 2000 - slash_2_amount); + }); +} From 13f30839b2eb48a330b4473fdcaad5f5b088cd8b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 25 Oct 2019 18:24:24 -0400 Subject: [PATCH 13/46] test that slashes are summed correctly --- srml/staking/src/tests.rs | 59 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 127b0e40f888d..763c1a600ecbe 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -2022,6 +2022,7 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(&11), 1000); assert_eq!(Balances::free_balance(&21), 2000); + assert_eq!(Balances::free_balance(&101), 2000); assert_eq!(Staking::slashable_balance_of(&21), 1000); @@ -2111,3 +2112,61 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(&101), 2000 - slash_2_amount); }); } + +#[test] +fn slashes_are_summed_across_spans() { + ExtBuilder::default().build().execute_with(|| { + start_era(1); + start_era(2); + start_era(3); + + assert_eq!(Balances::free_balance(&21), 2000); + assert_eq!(Staking::slashable_balance_of(&21), 1000); + + let get_span = |account| ::SlashingSpans::get(&account).unwrap(); + + on_offence_now( + &[ + OffenceDetails { + offender: (21, Staking::stakers(&21)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + ); + + let expected_spans = vec![ + slashing::SlashingSpan { index: 1, start: 4, length: None }, + slashing::SlashingSpan { index: 0, start: 0, length: Some(4) }, + ]; + + assert_eq!(get_span(21).iter().collect::>(), expected_spans); + assert_eq!(Balances::free_balance(&21), 1900); + + // 21 has been force-chilled. re-signal intent to validate. + Staking::validate(Origin::signed(20), Default::default()).unwrap(); + + start_era(4); + + assert_eq!(Staking::slashable_balance_of(&21), 900); + + on_offence_now( + &[ + OffenceDetails { + offender: (21, Staking::stakers(&21)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + ); + + let expected_spans = vec![ + slashing::SlashingSpan { index: 2, start: 5, length: None }, + slashing::SlashingSpan { index: 1, start: 4, length: Some(1) }, + slashing::SlashingSpan { index: 0, start: 0, length: Some(4) }, + ]; + + assert_eq!(get_span(21).iter().collect::>(), expected_spans); + assert_eq!(Balances::free_balance(&21), 1810); + }); +} From 187b3f67e5289e97cc1038c47ea718c78a13f3c7 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 27 Oct 2019 14:38:44 -0400 Subject: [PATCH 14/46] reward value computation --- srml/staking/src/lib.rs | 13 +-- srml/staking/src/slashing.rs | 163 ++++++++++++++++++++++++++--------- srml/staking/src/tests.rs | 4 +- 3 files changed, 133 insertions(+), 47 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index cc2a94a363737..84b038a799692 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -615,8 +615,10 @@ decl_storage! { /// Slashing spans for stash accounts. SlashingSpans: map T::AccountId => Option; - /// Records information about the maximum slash of a stash within a slashing span. - SpanSlash get(fn span_slash): map (T::AccountId, slashing::SpanIndex) => BalanceOf; + /// Records information about the maximum slash of a stash within a slashing span, + /// as well as how much reward has been paid out. + SpanSlash: map (T::AccountId, slashing::SpanIndex) + => slashing::SpanRecord>; } add_extra_genesis { config(stakers): @@ -1436,7 +1438,7 @@ impl OnOffenceHandler OnOffenceHandler(slashing::SlashParams { + let reward_payout = slashing::slash::(slashing::SlashParams { stash, slash: *slash_fraction, exposure, slash_era, window_start, now: era_now, - }) + reward_proportion, + }); // TODO: remove when rewards are done again. // let slash_reward = slash_reward_fraction * slashed_amount.peek(); diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index d01430f2fc2cf..162b0b66db6c0 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -50,6 +50,8 @@ use sr_primitives::traits::{Zero, Saturating}; use support::{StorageValue, StorageMap, StorageDoubleMap, traits::{Currency, OnUnbalanced}}; use codec::{Encode, Decode}; +const REWARD_F1: Perbill = Perbill::from_percent(50); + /// The index of a slashing span - unique to each stash. pub(crate) type SpanIndex = u32; @@ -147,6 +149,25 @@ impl SlashingSpans { } } +/// A slashing-span record for a particular stash. +#[derive(Encode, Decode, Default)] +pub(crate) struct SpanRecord { + slashed: Balance, + paid_out: Balance, +} + +impl SpanRecord { + /// The value of stash balance slashed in this span. + pub(crate) fn amount_slashed(&self) -> &Balance { + &self.slashed + } + + /// The value of rewards paid out from slashes in this span. + pub(crate) fn amount_rewarded(&self) -> &Balance { + &self.paid_out + } +} + /// Parameters for performing a slash. #[derive(Clone)] pub(crate) struct SlashParams<'a, T: 'a + Trait> { @@ -162,10 +183,17 @@ pub(crate) struct SlashParams<'a, T: 'a + Trait> { pub(crate) window_start: EraIndex, /// The current era. pub(crate) now: EraIndex, + /// The maximum percentage of a slash that ever gets paid out. + /// This is f_inf in the paper. + pub(crate) reward_proportion: Perbill, } +/// An unapplied reward payout. +#[must_use = "Reward payout must be explicitly applied"] +pub(crate) struct RewardPayout(Option>); + /// Slash a validator and their nominators, if necessary. -pub(crate) fn slash(params: SlashParams){ +pub(crate) fn slash(params: SlashParams) -> RewardPayout { let SlashParams { stash, slash, @@ -173,8 +201,11 @@ pub(crate) fn slash(params: SlashParams){ slash_era, window_start, now, + reward_proportion, } = params.clone(); + let mut reward_payout = Zero::zero(); + // is the slash amount here a maximum for the era? let own_slash = slash * exposure.own; let (prior_slash_p, era_slash) = as Store>::ValidatorSlashInEra::get( @@ -191,37 +222,48 @@ pub(crate) fn slash(params: SlashParams){ } else { // we slash based on the max in era - this new event is not the max, // so neither the validator or any nominators will need an update. - return + return RewardPayout(None) } // apply slash to validator. - let mut spans = fetch_spans::(stash, window_start); + { + let mut spans = fetch_spans::( + stash, + window_start, + &mut reward_payout, + reward_proportion, + ); - let target_span = spans.compare_and_update_span_slash( - slash_era, - own_slash, - ); - - if target_span == Some(spans.span_index()) { - // misbehavior occurred within the current slashing span - take appropriate - // actions. - - // chill the validator - it misbehaved in the current span and should - // not continue in the next election. also end the slashing span. - spans.end_span(now); - >::chill_stash(stash); - - // 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 - crate::ForceEra::put(crate::Forcing::ForceNew); + let target_span = spans.compare_and_update_span_slash( + slash_era, + own_slash, + ); + + if target_span == Some(spans.span_index()) { + // misbehavior occurred within the current slashing span - take appropriate + // actions. + + // chill the validator - it misbehaved in the current span and should + // not continue in the next election. also end the slashing span. + spans.end_span(now); + >::chill_stash(stash); + + // 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 + crate::ForceEra::put(crate::Forcing::ForceNew); + } } } - slash_nominators::(params, prior_slash_p) + reward_payout += slash_nominators::(params, prior_slash_p); + RewardPayout(Some(reward_payout)) } -fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { +/// Slash nominators. Accepts general parameters and the prior slash percentage of the nominator. +/// +/// Returns the amount of reward to pay out. +fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) -> BalanceOf { let SlashParams { stash: _, slash, @@ -229,8 +271,11 @@ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { slash_era, window_start, now, + reward_proportion, } = params; + let mut reward_payout = Zero::zero(); + for nominator in &exposure.others { let stash = &nominator.who; @@ -258,19 +303,28 @@ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) { }; // compare the era slash against other eras in the same span. - let mut spans = fetch_spans::(stash, window_start); + { + let mut spans = fetch_spans::( + stash, + window_start, + &mut reward_payout, + reward_proportion, + ); - let target_span = spans.compare_and_update_span_slash( - slash_era, - era_slash, - ); + let target_span = spans.compare_and_update_span_slash( + slash_era, + era_slash, + ); - if target_span == Some(spans.span_index()) { - // Chill the nominator outright, ending the slashing span. - spans.end_span(now); - >::chill_stash(&stash); + if target_span == Some(spans.span_index()) { + // Chill the nominator outright, ending the slashing span. + spans.end_span(now); + >::chill_stash(&stash); + } } } + + reward_payout } // helper struct for managing a set of spans we are currently inspecting. @@ -286,11 +340,18 @@ struct InspectingSpans<'a, T: Trait + 'a> { stash: &'a T::AccountId, spans: SlashingSpans, deferred_slashes: Vec>, + paid_out: &'a mut BalanceOf, + reward_proportion: Perbill, _marker: rstd::marker::PhantomData, } // fetches the slashing spans record for a stash account, initializing it if necessary. -fn fetch_spans(stash: &T::AccountId, window_start: EraIndex) -> InspectingSpans { +fn fetch_spans<'a, T: Trait + 'a>( + stash: &'a T::AccountId, + window_start: EraIndex, + paid_out: &'a mut BalanceOf, + reward_proportion: Perbill, +) -> InspectingSpans<'a, T> { let spans = as Store>::SlashingSpans::get(stash).unwrap_or_else(|| { let spans = SlashingSpans::new(window_start); as Store>::SlashingSpans::insert(stash, &spans); @@ -303,6 +364,8 @@ fn fetch_spans(stash: &T::AccountId, window_start: EraIndex) -> Inspec stash, spans, deferred_slashes: Vec::with_capacity(1), // we usually would only slash once. + paid_out, + reward_proportion, _marker: rstd::marker::PhantomData, } } @@ -328,16 +391,38 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { ) -> Option { let target_span = self.spans.iter().find(|span| span.contains_era(slash_era))?; let span_slash_key = (self.stash.clone(), target_span.index); - let span_slash = >::span_slash(&span_slash_key); + let mut span_record = as Store>::SpanSlash::get(&span_slash_key); + let mut changed = false; - if span_slash < slash { + let reward = if span_record.slashed < slash { // new maximum span slash. apply the difference. - let difference = slash - span_slash; + let difference = slash - span_record.slashed; + span_record.slashed = slash; - self.dirty = true; - as Store>::SpanSlash::insert(&span_slash_key, &slash); + // compute reward. + let reward = REWARD_F1 + * (self.reward_proportion * slash).saturating_sub(span_record.paid_out); self.deferred_slashes.push(slash_lazy(self.stash.clone(), difference)); + changed = true; + + reward + } else if span_record.slashed == slash { + // compute reward. no slash difference to apply. + REWARD_F1 * (self.reward_proportion * slash).saturating_sub(span_record.paid_out) + } else { + Zero::zero() + }; + + if !reward.is_zero() { + changed = true; + span_record.paid_out += reward; + *self.paid_out += reward; + } + + if changed { + self.dirty = true; + as Store>::SpanSlash::insert(&span_slash_key, &span_record); } Some(target_span.index) @@ -418,8 +503,6 @@ impl Drop for LazySlash { ledger.total -= value; - // TODO: rewards - let (imbalance, _) = T::Currency::slash(&self.stash, value); T::Slash::on_unbalanced(imbalance); diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 763c1a600ecbe..fa5f1a912a407 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1950,7 +1950,7 @@ fn garbage_collection_after_slashing() { assert_eq!(Balances::free_balance(&11), 256_000 - 25_600); assert!(::SlashingSpans::get(&11).is_some()); - assert_eq!(::SpanSlash::get(&(11, 0)), 25_600); + assert_eq!(::SpanSlash::get(&(11, 0)).amount_slashed(), &25_600); on_offence_now( &[ @@ -1967,7 +1967,7 @@ fn garbage_collection_after_slashing() { assert_eq!(Balances::free_balance(&11), 0); assert!(::SlashingSpans::get(&11).is_none()); - assert_eq!(::SpanSlash::get(&(11, 0)), 0); + assert_eq!(::SpanSlash::get(&(11, 0)).amount_slashed(), &0); }) } From 52ff078e2e1e4505c82778c6b16430071f4cb916 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 27 Oct 2019 18:13:42 -0400 Subject: [PATCH 15/46] implement rewarding --- srml/staking/src/lib.rs | 19 +--- srml/staking/src/slashing.rs | 175 +++++++++++++++++++++++++---------- srml/staking/src/tests.rs | 78 ++++++++++++++-- 3 files changed, 197 insertions(+), 75 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 84b038a799692..3c0183b8a29ff 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1486,24 +1486,7 @@ impl OnOffenceHandler(reward_payout, &details.reporters); } } } diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 162b0b66db6c0..2b91fc6d58308 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -45,11 +45,19 @@ //! //! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ -use super::{EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill, SessionInterface}; +use super::{ + EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill, SessionInterface, + NegativeImbalanceOf, +}; use sr_primitives::traits::{Zero, Saturating}; -use support::{StorageValue, StorageMap, StorageDoubleMap, traits::{Currency, OnUnbalanced}}; +use support::{ + StorageValue, StorageMap, StorageDoubleMap, + traits::{Currency, OnUnbalanced, Imbalance}, +}; use codec::{Encode, Decode}; +/// The proportion of the slashing reward to be paid out on the first slashing detection. +/// This is f_1 in the paper. const REWARD_F1: Perbill = Perbill::from_percent(50); /// The index of a slashing span - unique to each stash. @@ -161,11 +169,6 @@ impl SpanRecord { pub(crate) fn amount_slashed(&self) -> &Balance { &self.slashed } - - /// The value of rewards paid out from slashes in this span. - pub(crate) fn amount_rewarded(&self) -> &Balance { - &self.paid_out - } } /// Parameters for performing a slash. @@ -188,9 +191,9 @@ pub(crate) struct SlashParams<'a, T: 'a + Trait> { pub(crate) reward_proportion: Perbill, } -/// An unapplied reward payout. +/// An unapplied reward payout. Feed into `pay_reporters`. #[must_use = "Reward payout must be explicitly applied"] -pub(crate) struct RewardPayout(Option>); +pub(crate) struct RewardPayout(Option<(BalanceOf, NegativeImbalanceOf)>); /// Slash a validator and their nominators, if necessary. pub(crate) fn slash(params: SlashParams) -> RewardPayout { @@ -205,6 +208,7 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { } = params.clone(); let mut reward_payout = Zero::zero(); + let mut slash_imbalance = >::zero(); // is the slash amount here a maximum for the era? let own_slash = slash * exposure.own; @@ -231,6 +235,7 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { stash, window_start, &mut reward_payout, + &mut slash_imbalance, reward_proportion, ); @@ -256,14 +261,19 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { } } - reward_payout += slash_nominators::(params, prior_slash_p); - RewardPayout(Some(reward_payout)) + reward_payout += slash_nominators::(params, prior_slash_p, &mut slash_imbalance); + + RewardPayout(Some((reward_payout, slash_imbalance))) } /// Slash nominators. Accepts general parameters and the prior slash percentage of the nominator. /// /// Returns the amount of reward to pay out. -fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) -> BalanceOf { +fn slash_nominators( + params: SlashParams, + prior_slash_p: Perbill, + slash_imbalance: &mut NegativeImbalanceOf, +) -> BalanceOf { let SlashParams { stash: _, slash, @@ -308,6 +318,7 @@ fn slash_nominators(params: SlashParams, prior_slash_p: Perbill) -> stash, window_start, &mut reward_payout, + slash_imbalance, reward_proportion, ); @@ -339,8 +350,9 @@ struct InspectingSpans<'a, T: Trait + 'a> { window_start: EraIndex, stash: &'a T::AccountId, spans: SlashingSpans, - deferred_slashes: Vec>, + deferred_slash: Option>, paid_out: &'a mut BalanceOf, + slash_imbalance: &'a mut NegativeImbalanceOf, reward_proportion: Perbill, _marker: rstd::marker::PhantomData, } @@ -350,6 +362,7 @@ fn fetch_spans<'a, T: Trait + 'a>( stash: &'a T::AccountId, window_start: EraIndex, paid_out: &'a mut BalanceOf, + slash_imbalance: &'a mut NegativeImbalanceOf, reward_proportion: Perbill, ) -> InspectingSpans<'a, T> { let spans = as Store>::SlashingSpans::get(stash).unwrap_or_else(|| { @@ -363,7 +376,8 @@ fn fetch_spans<'a, T: Trait + 'a>( window_start, stash, spans, - deferred_slashes: Vec::with_capacity(1), // we usually would only slash once. + deferred_slash: None, + slash_imbalance, paid_out, reward_proportion, _marker: rstd::marker::PhantomData, @@ -379,6 +393,11 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { self.dirty = self.spans.end_span(now) || self.dirty; } + fn defer_slash(&mut self, amount: BalanceOf) { + let total_deferred = self.deferred_slash.get_or_insert_with(Zero::zero); + *total_deferred = *total_deferred + amount; + } + // compares the slash in an era to the overall current span slash. // if it's higher, applies the difference of the slashes and then updates the span on disk. // @@ -403,7 +422,7 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { let reward = REWARD_F1 * (self.reward_proportion * slash).saturating_sub(span_record.paid_out); - self.deferred_slashes.push(slash_lazy(self.stash.clone(), difference)); + self.defer_slash(difference); changed = true; reward @@ -441,13 +460,14 @@ impl<'a, T: 'a + Trait> Drop for InspectingSpans<'a, T> { } as Store>::SlashingSpans::insert(self.stash, &self.spans); - } -} -// perform a slash which is lazily applied on `Drop`. This allows us to register -// that the slash should happen without being in the middle of any bookkeeping. -fn slash_lazy(stash: T::AccountId, value: BalanceOf) -> LazySlash { - LazySlash { stash, value } + // do this AFTER updating span information because it is possible that the + // slash may zero the account and cause garbage collection. any stored data + // after that point would be orphaned. + if let Some(slash) = self.deferred_slash.take() { + apply_slash::(self.stash, slash, self.paid_out, self.slash_imbalance) + } + } } /// Clear slashing metadata for an obsolete era. @@ -473,49 +493,104 @@ pub(crate) fn clear_stash_metadata(stash: &T::AccountId) { } } -struct LazySlash { - stash: T::AccountId, +// apply the slash to a stash account, deducting any missing funds from the reward +// payout, saturating at 0. this is mildly unfair but also an edge-case that +// can only occur when overlapping locked funds have been slashed. +fn apply_slash( + stash: &T::AccountId, value: BalanceOf, -} + reward_payout: &mut BalanceOf, + slash_imbalance: &mut NegativeImbalanceOf, +) { + let controller = match >::bonded(stash) { + None => return, // defensive: should always exist. + Some(c) => c, + }; -impl Drop for LazySlash { - fn drop(&mut self) { - let controller = match >::bonded(&self.stash) { - None => return, // defensive: should always exist. - Some(c) => c, - }; + let mut ledger = match >::ledger(&controller) { + Some(ledger) => ledger, + None => return, // nothing to do. + }; - let mut ledger = match >::ledger(&controller) { - Some(ledger) => ledger, - None => return, // nothing to do. - }; + let mut value = value.min(ledger.active); - let mut value = self.value.min(ledger.active); + if !value.is_zero() { + ledger.active -= value; - if !value.is_zero() { - ledger.active -= value; + // Avoid there being a dust balance left in the staking system. + if ledger.active < T::Currency::minimum_balance() { + value += ledger.active; + ledger.active = Zero::zero(); + } - // Avoid there being a dust balance left in the staking system. - if ledger.active < T::Currency::minimum_balance() { - value += ledger.active; - ledger.active = Zero::zero(); - } + ledger.total -= value; - ledger.total -= value; + let (imbalance, missing) = T::Currency::slash(stash, value); + slash_imbalance.subsume(imbalance); - let (imbalance, _) = T::Currency::slash(&self.stash, value); - T::Slash::on_unbalanced(imbalance); + if !missing.is_zero() { + // deduct overslash from the reward payout + *reward_payout = reward_payout.saturating_sub(missing); + } - >::update_ledger(&controller, &ledger); + >::update_ledger(&controller, &ledger); - // trigger the event - >::deposit_event( - super::RawEvent::Slash(self.stash.clone(), value) - ); + // trigger the event + >::deposit_event( + super::RawEvent::Slash(stash.clone(), value) + ); + } +} + +impl RewardPayout { + fn pay_to(&mut self, reporters: &[T::AccountId]) { + let (reward_payout, slashed_imbalance) = match self.0.take() { + None => return, + Some((payout, slashed_imbalance)) => { + if payout.is_zero() || reporters.is_empty() { + // nobody to pay out to or nothing to pay; + // just treat the whole value as slashed. + T::Slash::on_unbalanced(slashed_imbalance); + return + } + (payout, slashed_imbalance) + }, + }; + + // take rewards out of the slashed imbalance. + let (mut reward_payout, mut value_slashed) = slashed_imbalance.split(reward_payout); + + let per_reporter = reward_payout.peek() / (reporters.len() as u32).into(); + for reporter in reporters { + let (reporter_reward, rest) = reward_payout.split(per_reporter); + reward_payout = rest; + + // this cancels out the reporter reward imbalance internally, leading + // to no change in total issuance. + T::Currency::resolve_creating(reporter, reporter_reward); } + + // the rest goes to the on-slash imbalance handler (e.g. treasury) + value_slashed.subsume(reward_payout); // remainder of reward division remains. + T::Slash::on_unbalanced(value_slashed); + } +} + +impl Drop for RewardPayout { + fn drop(&mut self) { + self.pay_to(&[]); } } + +/// Apply a reward payout to some reporters. +pub(crate) fn pay_reporters( + mut reward_payout: RewardPayout, + reporters: &[T::AccountId], +) { + reward_payout.pay_to(reporters) +} + #[cfg(test)] mod tests { use super::*; diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index fa5f1a912a407..02a84ca7c4249 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1646,15 +1646,28 @@ fn reward_validator_slashing_validator_doesnt_overflow() { // Set staker let _ = Balances::make_free_balance_be(&11, stake); let _ = Balances::make_free_balance_be(&2, stake); + + // only slashes out of bonded stake are applied. without this line, + // it is 0. + Staking::bond(Origin::signed(2), 20000, stake - 1, RewardDestination::default()).unwrap(); >::insert(&11, Exposure { total: stake, own: 1, others: vec![ IndividualExposure { who: 2, value: stake - 1 } ]}); + // Check slashing - // TODO - // let _ = Staking::slash_validator(&11, reward_slash, &Staking::stakers(&11), &mut Vec::new()); - // assert_eq!(Balances::total_balance(&11), stake - 1); - // assert_eq!(Balances::total_balance(&2), 1); + on_offence_now( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(100)], + ); + + assert_eq!(Balances::total_balance(&11), stake - 1); + assert_eq!(Balances::total_balance(&2), 1); }) } @@ -1814,10 +1827,61 @@ fn reporters_receive_their_slice() { &[Perbill::from_percent(50)], ); - // initial_balance x 50% (slash fraction) x 10% (rewards slice) - let reward = initial_balance / 20 / 2; + // F1 * (reward_proportion * slash - 0) + // 50% * (10% * initial_balance / 2) + let reward = (initial_balance / 20) / 2; + let reward_each = reward / 2; // split into two pieces. + assert_eq!(Balances::free_balance(&1), 10 + reward_each); + assert_eq!(Balances::free_balance(&2), 20 + reward_each); + }); +} + +#[test] +fn subsequent_reports_in_same_span_pay_out_less() { + // This test verifies that the reporters of the offence receive their slice from the slashed + // amount. + ExtBuilder::default().build().execute_with(|| { + // The reporters' reward is calculated from the total exposure. + #[cfg(feature = "equalize")] + let initial_balance = 1250; + #[cfg(not(feature = "equalize"))] + let initial_balance = 1125; + + assert_eq!(Staking::stakers(&11).total, initial_balance); + + on_offence_now( + &[OffenceDetails { + offender: ( + 11, + Staking::stakers(&11), + ), + reporters: vec![1], + }], + &[Perbill::from_percent(20)], + ); + + // F1 * (reward_proportion * slash - 0) + // 50% * (10% * initial_balance * 20%) + let reward = (initial_balance / 5) / 20; assert_eq!(Balances::free_balance(&1), 10 + reward); - assert_eq!(Balances::free_balance(&2), 20 + reward); + + on_offence_now( + &[OffenceDetails { + offender: ( + 11, + Staking::stakers(&11), + ), + reporters: vec![1], + }], + &[Perbill::from_percent(50)], + ); + + let prior_payout = reward; + + // F1 * (reward_proportion * slash - prior_payout) + // 50% * (10% * (initial_balance / 2) - prior_payout) + let reward = ((initial_balance / 20) - prior_payout) / 2; + assert_eq!(Balances::free_balance(&1), 10 + prior_payout + reward); }); } From e4d986402e50241e032fa6161045bc3df3363c1b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 27 Oct 2019 18:14:49 -0400 Subject: [PATCH 16/46] add comment about rewards --- srml/staking/src/slashing.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 2b91fc6d58308..0e150a8b9aa76 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -226,6 +226,11 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { } else { // we slash based on the max in era - this new event is not the max, // so neither the validator or any nominators will need an update. + // + // this does lead to a divergence of our system from the paper, which + // pays out some reward even if the latest report is not max-in-era. + // we opt to avoid the nominator lookups and edits and leave more rewards + // for more drastic misbehavior. return RewardPayout(None) } From 459ebe78344341b0cc1146ca1327838df5e50f41 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 27 Oct 2019 19:45:59 -0400 Subject: [PATCH 17/46] do not adjust slash fraction in offences module --- srml/offences/src/lib.rs | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/srml/offences/src/lib.rs b/srml/offences/src/lib.rs index 6580ede084d9b..24d4bdd38a7df 100644 --- a/srml/offences/src/lib.rs +++ b/srml/offences/src/lib.rs @@ -118,37 +118,7 @@ where // The amount new offenders are slashed let new_fraction = O::slash_fraction(offenders_count, validator_set_count); - // The amount previous offenders are slashed additionally. - // - // Since they were slashed in the past, we slash by: - // x = (new - prev) / (1 - prev) - // because: - // Y = X * (1 - prev) - // Z = Y * (1 - x) - // Z = X * (1 - new) - let old_fraction = if previous_offenders_count > 0 { - let previous_fraction = O::slash_fraction( - offenders_count.saturating_sub(previous_offenders_count), - validator_set_count, - ); - let numerator = new_fraction.saturating_sub(previous_fraction); - let denominator = Perbill::one().saturating_sub(previous_fraction); - denominator.saturating_mul(numerator) - } else { - new_fraction.clone() - }; - - // calculate how much to slash - let slash_perbill = concurrent_offenders - .iter() - .map(|details| { - if previous_offenders_count > 0 && new_offenders.contains(&details.offender) { - new_fraction.clone() - } else { - old_fraction.clone() - } - }) - .collect::>(); + let slash_perbill = vec![new_fraction; concurrent_offenders.len()]; T::OnOffenceHandler::on_offence( &concurrent_offenders, From 2d10e2c2a0e367bb6b5d64eb81710425a793c37d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 27 Oct 2019 19:56:57 -0400 Subject: [PATCH 18/46] fix offences tests --- srml/offences/src/lib.rs | 8 ++------ srml/offences/src/mock.rs | 5 ++--- srml/offences/src/tests.rs | 33 +-------------------------------- 3 files changed, 5 insertions(+), 41 deletions(-) diff --git a/srml/offences/src/lib.rs b/srml/offences/src/lib.rs index 24d4bdd38a7df..c8261d3f5a2d4 100644 --- a/srml/offences/src/lib.rs +++ b/srml/offences/src/lib.rs @@ -31,10 +31,7 @@ use rstd::{ use support::{ decl_module, decl_event, decl_storage, Parameter, }; -use sr_primitives::{ - Perbill, - traits::{Hash, Saturating}, -}; +use sr_primitives::traits::Hash; use sr_staking_primitives::{ offence::{Offence, ReportOffence, Kind, OnOffenceHandler, OffenceDetails}, }; @@ -101,7 +98,7 @@ where // Go through all offenders in the offence report and find all offenders that was spotted // in unique reports. let TriageOutcome { - new_offenders, + new_offenders: _, concurrent_offenders, } = match Self::triage_offence_report::(reporters, &time_slot, offenders) { Some(triage) => triage, @@ -113,7 +110,6 @@ where Self::deposit_event(Event::Offence(O::ID, time_slot.encode())); let offenders_count = concurrent_offenders.len() as u32; - let previous_offenders_count = offenders_count - new_offenders.len() as u32; // The amount new offenders are slashed let new_fraction = O::slash_fraction(offenders_count, validator_set_count); diff --git a/srml/offences/src/mock.rs b/srml/offences/src/mock.rs index f9c79390819e0..491c9681b1cd5 100644 --- a/srml/offences/src/mock.rs +++ b/srml/offences/src/mock.rs @@ -46,6 +46,7 @@ impl offence::OnOffenceHandler for OnOff fn on_offence( _offenders: &[OffenceDetails], slash_fraction: &[Perbill], + _offence_session: SessionIndex, ) { ON_OFFENCE_PERBILL.with(|f| { *f.borrow_mut() = slash_fraction.to_vec(); @@ -148,9 +149,7 @@ impl offence::Offence for Offence { } fn session_index(&self) -> SessionIndex { - // session index is not used by the srml-offences directly, but rather it exists only for - // filtering historical reports. - unimplemented!() + 1 } fn slash_fraction( diff --git a/srml/offences/src/tests.rs b/srml/offences/src/tests.rs index 28e655d16bfdd..aa71d1d6206a7 100644 --- a/srml/offences/src/tests.rs +++ b/srml/offences/src/tests.rs @@ -23,6 +23,7 @@ use crate::mock::{ Offences, System, Offence, TestEvent, KIND, new_test_ext, with_on_offence_fractions, offence_reports, }; +use sr_primitives::Perbill; use system::{EventRecord, Phase}; #[test] @@ -48,38 +49,6 @@ fn should_report_an_authority_and_trigger_on_offence() { }); } -#[test] -fn should_calculate_the_fraction_correctly() { - new_test_ext().execute_with(|| { - // given - let time_slot = 42; - assert_eq!(offence_reports(KIND, time_slot), vec![]); - let offence1 = Offence { - validator_set_count: 5, - time_slot, - offenders: vec![5], - }; - let offence2 = Offence { - validator_set_count: 5, - time_slot, - offenders: vec![4], - }; - - // when - Offences::report_offence(vec![], offence1); - with_on_offence_fractions(|f| { - assert_eq!(f.clone(), vec![Perbill::from_percent(25)]); - }); - - Offences::report_offence(vec![], offence2); - - // then - with_on_offence_fractions(|f| { - assert_eq!(f.clone(), vec![Perbill::from_percent(15), Perbill::from_percent(45)]); - }); - }); -} - #[test] fn should_not_report_the_same_authority_twice_in_the_same_slot() { new_test_ext().execute_with(|| { From ababe08d9351ec5c13e3b07451f60f3076982905 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 28 Oct 2019 18:57:52 -0400 Subject: [PATCH 19/46] remove unused new_offenders field --- srml/offences/src/lib.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/srml/offences/src/lib.rs b/srml/offences/src/lib.rs index c8261d3f5a2d4..83a07771ad801 100644 --- a/srml/offences/src/lib.rs +++ b/srml/offences/src/lib.rs @@ -24,10 +24,7 @@ mod mock; mod tests; -use rstd::{ - vec::Vec, - collections::btree_set::BTreeSet, -}; +use rstd::{vec::Vec}; use support::{ decl_module, decl_event, decl_storage, Parameter, }; @@ -97,10 +94,7 @@ where // Go through all offenders in the offence report and find all offenders that was spotted // in unique reports. - let TriageOutcome { - new_offenders: _, - concurrent_offenders, - } = match Self::triage_offence_report::(reporters, &time_slot, offenders) { + let TriageOutcome { concurrent_offenders } = match Self::triage_offence_report::(reporters, &time_slot, offenders) { Some(triage) => triage, // The report contained only duplicates, so there is no need to slash again. None => return, @@ -143,13 +137,13 @@ impl Module { offenders: Vec, ) -> Option> { let mut storage = ReportIndexStorage::::load(time_slot); - let mut new_offenders = BTreeSet::new(); + let mut any_new = false; for offender in offenders { let report_id = Self::report_id::(time_slot, &offender); if !>::exists(&report_id) { - new_offenders.insert(offender.clone()); + any_new = true; >::insert( &report_id, OffenceDetails { @@ -162,7 +156,7 @@ impl Module { } } - if !new_offenders.is_empty() { + if any_new { // Load report details for the all reports happened at the same time. let concurrent_offenders = storage.concurrent_reports .iter() @@ -172,7 +166,6 @@ impl Module { storage.save(); Some(TriageOutcome { - new_offenders, concurrent_offenders, }) } else { @@ -182,8 +175,6 @@ impl Module { } struct TriageOutcome { - /// Offenders that was spotted in the unique reports. - new_offenders: BTreeSet, /// Other reports for the same report kinds. concurrent_offenders: Vec>, } From a6824486ddfa936b1afae85bc594837f245d17f5 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 28 Oct 2019 19:00:34 -0400 Subject: [PATCH 20/46] update runtime version --- node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 3dba79d27ba9b..dd46b26d01e8c 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -83,7 +83,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, }; From d5935690e67e7c54dc19a6b01c84887286e6b21e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 28 Oct 2019 19:51:36 -0400 Subject: [PATCH 21/46] fix up some docs --- srml/staking/src/slashing.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 0e150a8b9aa76..c080b58373006 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -41,7 +41,10 @@ //! slashing span. //! //! Typically, you will have a single slashing event per slashing span. Only in the case -//! where a validator releases many misbehaviors at once +//! where a validator releases many misbehaviors at once, or goes "back in time" to misbehave in +//! eras that have already passed, would you encounter situations where a slashing span +//! has multiple misbehaviors. However, accounting for such cases is necessary +//! to deter a class of "rage-quit" attacks. //! //! Based on research at https://research.web3.foundation/en/latest/polkadot/slashing/npos/ From 036fdbd6ab0d34f5ed1c3773694d3d13185d9846 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 28 Oct 2019 19:59:05 -0400 Subject: [PATCH 22/46] fix some CI failures --- srml/offences/src/lib.rs | 6 +++++- srml/staking/src/slashing.rs | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/srml/offences/src/lib.rs b/srml/offences/src/lib.rs index 83a07771ad801..bacafee469108 100644 --- a/srml/offences/src/lib.rs +++ b/srml/offences/src/lib.rs @@ -94,7 +94,11 @@ where // Go through all offenders in the offence report and find all offenders that was spotted // in unique reports. - let TriageOutcome { concurrent_offenders } = match Self::triage_offence_report::(reporters, &time_slot, offenders) { + let TriageOutcome { concurrent_offenders } = match Self::triage_offence_report::( + reporters, + &time_slot, + offenders, + ) { Some(triage) => triage, // The report contained only duplicates, so there is no need to slash again. None => return, diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index c080b58373006..b8cea38682a2f 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -57,6 +57,7 @@ use support::{ StorageValue, StorageMap, StorageDoubleMap, traits::{Currency, OnUnbalanced, Imbalance}, }; +use rstd::vec::Vec; use codec::{Encode, Decode}; /// The proportion of the slashing reward to be paid out on the first slashing detection. @@ -169,6 +170,7 @@ pub(crate) struct SpanRecord { impl SpanRecord { /// The value of stash balance slashed in this span. + #[cfg(test)] pub(crate) fn amount_slashed(&self) -> &Balance { &self.slashed } From 0d3f17f78cf41c022577a359655271e75d3bf670 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 28 Oct 2019 20:33:34 -0400 Subject: [PATCH 23/46] remove no-std incompatible vec! invocation --- srml/offences/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/srml/offences/src/lib.rs b/srml/offences/src/lib.rs index bacafee469108..ce9a1a0a41bae 100644 --- a/srml/offences/src/lib.rs +++ b/srml/offences/src/lib.rs @@ -24,7 +24,7 @@ mod mock; mod tests; -use rstd::{vec::Vec}; +use rstd::vec::Vec; use support::{ decl_module, decl_event, decl_storage, Parameter, }; @@ -112,7 +112,8 @@ where // The amount new offenders are slashed let new_fraction = O::slash_fraction(offenders_count, validator_set_count); - let slash_perbill = vec![new_fraction; concurrent_offenders.len()]; + let slash_perbill: Vec<_> = (0..concurrent_offenders.len()) + .map(|_| new_fraction.clone()).collect(); T::OnOffenceHandler::on_offence( &concurrent_offenders, From 177e7e67acfc1d3d423d45dcbf5f23840fbcf84c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 28 Oct 2019 23:07:51 -0400 Subject: [PATCH 24/46] try to fix span-max rounding error --- srml/staking/src/slashing.rs | 3 +-- srml/staking/src/tests.rs | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index b8cea38682a2f..2f6fb43396ada 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -411,8 +411,7 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { // compares the slash in an era to the overall current span slash. // if it's higher, applies the difference of the slashes and then updates the span on disk. // - // returns the span index of the era, if any, along with a slash to be lazily applied - // when all book-keeping is done. + // returns the span index of the era where the slash occurred, if any. fn compare_and_update_span_slash( &mut self, slash_era: EraIndex, diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 02a84ca7c4249..0cc818edd55db 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -2109,7 +2109,7 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(&11), 900); - let slash_1_amount = nominated_value_11 / 10; + let slash_1_amount = Perbill::from_percent(10) * nominated_value_11; assert_eq!(Balances::free_balance(&101), 2000 - slash_1_amount); let expected_spans = vec![ @@ -2145,7 +2145,7 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(&11), 900); assert_eq!(Balances::free_balance(&21), 1700); - let slash_2_amount = 3 * (nominated_value_21 / 10); + let slash_2_amount = Perbill::from_percent(30) * nominated_value_21; assert!(slash_2_amount > slash_1_amount); // only the maximum slash in a single span is taken. @@ -2168,7 +2168,7 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(&11), 800); assert_eq!(Balances::free_balance(&21), 1700); - let slash_3_amount = 2 * (nominated_value_11 / 10); + let slash_3_amount = Perbill::from_percent(20) * nominated_value_21; assert!(slash_3_amount < slash_2_amount); assert!(slash_3_amount > slash_1_amount); From ee189aa5f57d597d5948e76a1e234f4fbccfc6bb Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 29 Oct 2019 16:19:14 -0400 Subject: [PATCH 25/46] Update srml/staking/src/slashing.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix type: winow -> window Co-Authored-By: Tomasz Drwięga --- srml/staking/src/slashing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 2f6fb43396ada..bb88c719bb4c8 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -155,7 +155,7 @@ impl SlashingSpans { None => None, }; - // readjust the ongoing span, if it started before the beginning of the winow. + // readjust the ongoing span, if it started before the beginning of the window. self.last_start = rstd::cmp::max(self.last_start, window_start); pruned } From f8e7fabedcf7a6224ae938aa67263576f1ba4494 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 30 Oct 2019 15:54:46 -0700 Subject: [PATCH 26/46] slashes from prior spans don't kick validator again --- srml/staking/src/lib.rs | 17 --------- srml/staking/src/slashing.rs | 50 +++++++++++++++++++++++--- srml/staking/src/tests.rs | 68 ++++++++++++++++++++++++++++++++++-- 3 files changed, 112 insertions(+), 23 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 6c01b2fb854e2..05f1106dcf3cb 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -1488,23 +1488,6 @@ impl OnOffenceHandler>::exists(stash) { - >::remove(stash); - Self::ensure_new_era(); - } - - // calculate the amount to slash - let slash_exposure = exposure.total; - let amount = *slash_fraction * slash_exposure; - - // in some cases `slash_fraction` can be just `0`, - // which means we are not slashing this time. - if amount.is_zero() { - continue; - } - let reward_payout = slashing::slash::(slashing::SlashParams { stash, slash: *slash_fraction, diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 9306325874a1b..fb0b68a3d00b5 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -54,7 +54,7 @@ use super::{ }; use sr_primitives::traits::{Zero, Saturating}; use support::{ - StorageValue, StorageMap, StorageDoubleMap, + StorageMap, StorageDoubleMap, traits::{Currency, OnUnbalanced, Imbalance}, }; use rstd::vec::Vec; @@ -217,12 +217,21 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { // is the slash amount here a maximum for the era? let own_slash = slash * exposure.own; - let (prior_slash_p, era_slash) = as Store>::ValidatorSlashInEra::get( + if slash * exposure.total == Zero::zero() { + // kick out the validator even if they won't be slashed, + // as long as the misbehavior is from their most recent slashing span. + kick_out_if_recent::(params); + return RewardPayout(None) + } + + let (prior_slash_p, _era_slash) = as Store>::ValidatorSlashInEra::get( &slash_era, stash, ).unwrap_or((Perbill::zero(), Zero::zero())); - if own_slash > era_slash { + // compare slash proportions rather than slash values to avoid issues due to rounding + // error. + if slash.deconstruct() > prior_slash_p.deconstruct() { as Store>::ValidatorSlashInEra::insert( &slash_era, stash, @@ -276,6 +285,34 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { RewardPayout(Some((reward_payout, slash_imbalance))) } +// doesn't apply any slash, but kicks out the validator if the misbehavior is from +// the most recent slashing span. +fn kick_out_if_recent( + params: SlashParams, +) { + // these are not updated by era-span or end-span. + let mut reward_payout = Zero::zero(); + let mut slash_imbalance = >::zero(); + let mut spans = fetch_spans::( + params.stash, + params.window_start, + &mut reward_payout, + &mut slash_imbalance, + params.reward_proportion, + ); + + if spans.era_span(params.slash_era).map(|s| s.index) == Some(spans.span_index()) { + spans.end_span(params.now); + >::chill_stash(params.stash); + + // make sure to disable validator till the end of this session + if T::SessionInterface::disable_validator(params.stash).unwrap_or(false) { + // force a new era, to select a new validator set + >::ensure_new_era() + } + } +} + /// Slash nominators. Accepts general parameters and the prior slash percentage of the nominator. /// /// Returns the amount of reward to pay out. @@ -408,6 +445,11 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { *total_deferred = *total_deferred + amount; } + // find the span index of the given era, if covered. + fn era_span(&self, era: EraIndex) -> Option { + self.spans.iter().find(|span| span.contains_era(era)) + } + // compares the slash in an era to the overall current span slash. // if it's higher, applies the difference of the slashes and then updates the span on disk. // @@ -417,7 +459,7 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { slash_era: EraIndex, slash: BalanceOf, ) -> Option { - let target_span = self.spans.iter().find(|span| span.contains_era(slash_era))?; + let target_span = self.era_span(slash_era)?; let span_slash_key = (self.stash.clone(), target_span.index); let mut span_record = as Store>::SpanSlash::get(&span_slash_key); let mut changed = false; diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index f9a4b5b5f7688..47d58915454c1 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1794,7 +1794,7 @@ fn offence_ensures_new_era_without_clobbering() { ExtBuilder::default().build().execute_with(|| { assert_ok!(Staking::force_new_era_always(Origin::ROOT)); - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1813,7 +1813,7 @@ fn offence_ensures_new_era_without_clobbering() { fn offence_deselects_validator_when_slash_is_zero() { ExtBuilder::default().build().execute_with(|| { assert!(>::exists(11)); - Staking::on_offence( + on_offence_now( &[OffenceDetails { offender: ( 11, @@ -1856,6 +1856,70 @@ fn slashing_performed_according_exposure() { }); } +#[test] +fn slash_in_old_span_does_not_deselect() { + ExtBuilder::default().build().execute_with(|| { + start_era(1); + + assert!(>::exists(11)); + on_offence_now( + &[OffenceDetails { + offender: ( + 11, + Staking::stakers(&11), + ), + reporters: vec![], + }], + &[Perbill::from_percent(0)], + ); + assert_eq!(Staking::force_era(), Forcing::ForceNew); + assert!(!>::exists(11)); + + start_era(2); + + Staking::validate(Origin::signed(10), Default::default()).unwrap(); + assert_eq!(Staking::force_era(), Forcing::NotForcing); + assert!(>::exists(11)); + + start_era(3); + + // this staker is in a new slashing span now, having re-registered after + // their prior slash. + + on_offence_in_era( + &[OffenceDetails { + offender: ( + 11, + Staking::stakers(&11), + ), + reporters: vec![], + }], + &[Perbill::from_percent(0)], + 1, + ); + + // not for zero-slash. + assert_eq!(Staking::force_era(), Forcing::NotForcing); + assert!(>::exists(11)); + + on_offence_in_era( + &[OffenceDetails { + offender: ( + 11, + Staking::stakers(&11), + ), + reporters: vec![], + }], + &[Perbill::from_percent(100)], + 1, + ); + + // or non-zero. + assert_eq!(Staking::force_era(), Forcing::NotForcing); + assert!(>::exists(11)); + }); +} + #[test] fn reporters_receive_their_slice() { // This test verifies that the reporters of the offence receive their slice from the slashed From 8a24768f8a469f82e53783e0614737e5c785c589 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 6 Nov 2019 17:57:52 +0100 Subject: [PATCH 27/46] more information for nominators, suppression --- srml/staking/Cargo.toml | 1 + srml/staking/src/lib.rs | 46 ++++++++++++++++++++++++++++++------ srml/staking/src/slashing.rs | 8 +++++++ srml/staking/src/tests.rs | 2 +- 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/srml/staking/Cargo.toml b/srml/staking/Cargo.toml index 67f3ab41d6583..b360e68b4a7a9 100644 --- a/srml/staking/Cargo.toml +++ b/srml/staking/Cargo.toml @@ -27,6 +27,7 @@ srml-staking-reward-curve = { path = "../staking/reward-curve"} [features] equalize = [] +migrate-storage-v1 = [] default = ["std", "equalize"] std = [ "serde", diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 05f1106dcf3cb..43de1118dd947 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -279,7 +279,7 @@ use sr_staking_primitives::{ use sr_primitives::{Serialize, Deserialize}; use system::{ensure_signed, ensure_root}; -use phragmen::{elect, equalize, build_support_map, ExtendedBalance, PhragmenStakedAssignment}; +use phragmen::{ExtendedBalance, PhragmenStakedAssignment}; const DEFAULT_MINIMUM_VALIDATOR_COUNT: u32 = 4; const MAX_NOMINATIONS: usize = 16; @@ -389,6 +389,17 @@ pub struct StakingLedger { pub unlocking: Vec>, } +/// A record of the nominations made by a specific account. +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] +pub struct Nominations { + /// The targets of nomination. + pub targets: Vec, + /// The era the nominations were submitted. + pub submitted_in: EraIndex, + /// Whether the nominations have been suppressed. + pub suppressed: bool, +} + impl< AccountId, Balance: HasCompact + Copy + Saturating, @@ -564,7 +575,7 @@ decl_storage! { pub Validators get(fn validators): linked_map T::AccountId => ValidatorPrefs>; /// The map from nominator stash key to the set of stash keys of all validators to nominate. - pub Nominators get(fn nominators): linked_map T::AccountId => Vec; + pub Nominators get(fn nominators): linked_map T::AccountId => Option>; /// Nominators for a particular account that is in action right now. You can't iterate /// through validators here, but you can find them in the Session module. @@ -893,8 +904,14 @@ decl_module! { .map(|t| T::Lookup::lookup(t)) .collect::, _>>()?; + let nominations = Nominations { + targets, + submitted_in: Self::current_era(), + suppressed: false, + }; + >::remove(stash); - >::insert(stash, targets); + >::insert(stash, &nominations); } /// Declare no desire to either validate or nominate. @@ -1209,11 +1226,26 @@ impl Module { /// /// Returns the new `SlotStake` value and a set of newly selected _stash_ IDs. fn select_validators() -> (BalanceOf, Option>) { - let maybe_phragmen_result = elect::<_, _, _, T::CurrencyToVote>( + let votes = >::enumerate().map(|(nominator, nominations)| { + let Nominations { submitted_in, mut targets, suppressed: _ } = nominations; + + // Filter out nomination targets which were nominated before the most recent + // slashing span. + targets.retain(|stash| { + ::SlashingSpans::get(&stash).map_or( + true, + |spans| submitted_in >= spans.last_start(), + ) + }); + + (nominator, targets) + }).collect::>(); + + let maybe_phragmen_result = phragmen::elect::<_, _, _, T::CurrencyToVote>( Self::validator_count() as usize, Self::minimum_validator_count().max(1) as usize, >::enumerate().map(|(who, _)| who).collect::>(), - >::enumerate().collect(), + votes, Self::slashable_balance_of, true, ); @@ -1229,7 +1261,7 @@ impl Module { let to_balance = |e: ExtendedBalance| >>::convert(e); - let mut supports = build_support_map::<_, _, _, T::CurrencyToVote>( + let mut supports = phragmen::build_support_map::<_, _, _, T::CurrencyToVote>( &elected_stashes, &assignments, Self::slashable_balance_of, @@ -1254,7 +1286,7 @@ impl Module { let tolerance = 0_u128; let iterations = 2_usize; - equalize::<_, _, T::CurrencyToVote, _>( + phragmen::equalize::<_, _, T::CurrencyToVote, _>( staked_assignments, &mut supports, tolerance, diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index fb0b68a3d00b5..fdeb7034ac595 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -136,6 +136,11 @@ impl SlashingSpans { rstd::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 + } + // prune the slashing spans against a window, whose start era index is given. // // If this returns `Some`, then it includes a range start..end of all the span @@ -642,6 +647,9 @@ pub(crate) fn pay_reporters( reward_payout.pay_to(reporters) } +// TODO: function for undoing a slash. +// + #[cfg(test)] mod tests { use super::*; diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 47d58915454c1..c637674e1c209 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -79,7 +79,7 @@ fn basic_setup_works() { Staking::ledger(100), Some(StakingLedger { stash: 101, total: 500, active: 500, unlocking: vec![] }) ); - assert_eq!(Staking::nominators(101), vec![11, 21]); + assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); if cfg!(feature = "equalize") { assert_eq!( From 8aca6f488bb67d1cd458a40f3d6fc67d55acb854 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 8 Nov 2019 23:24:29 +0100 Subject: [PATCH 28/46] ensure ledger is consistent with itself post-slash --- srml/staking/src/mock.rs | 8 ++++++++ srml/staking/src/tests.rs | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index 2dbaea2a78cfb..9589d43a9d575 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -393,6 +393,14 @@ pub fn assert_is_stash(acc: u64) { assert!(Staking::bonded(&acc).is_some(), "Not a stash."); } +pub fn assert_ledger_consistent(stash: u64) { + assert_is_stash(stash); + let ledger = Staking::ledger(stash - 1).unwrap(); + + let real_total: Balance = ledger.unlocking.iter().fold(ledger.active, |a, c| a + c.value); + assert_eq!(real_total, ledger.total); +} + pub fn bond_validator(acc: u64, val: u64) { // a = controller // a + 1 = stash diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index c637674e1c209..33fa06c73809e 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -1917,6 +1917,7 @@ fn slash_in_old_span_does_not_deselect() { // or non-zero. assert_eq!(Staking::force_era(), Forcing::NotForcing); assert!(>::exists(11)); + assert_ledger_consistent(11); }); } @@ -1950,6 +1951,7 @@ fn reporters_receive_their_slice() { let reward_each = reward / 2; // split into two pieces. assert_eq!(Balances::free_balance(&1), 10 + reward_each); assert_eq!(Balances::free_balance(&2), 20 + reward_each); + assert_ledger_consistent(11); }); } @@ -1999,6 +2001,7 @@ fn subsequent_reports_in_same_span_pay_out_less() { // 50% * (10% * (initial_balance / 2) - prior_payout) let reward = ((initial_balance / 20) - prior_payout) / 2; assert_eq!(Balances::free_balance(&1), 10 + prior_payout + reward); + assert_ledger_consistent(11); }); } @@ -2041,6 +2044,8 @@ fn invulnerables_are_not_slashed() { initial_balance - (2 * other.value / 10), ); } + assert_ledger_consistent(11); + assert_ledger_consistent(21); }); } @@ -2063,6 +2068,7 @@ 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_ledger_consistent(11); }); } @@ -2110,6 +2116,7 @@ fn only_slash_for_max_in_era() { // The validator got slashed 10% more. assert_eq!(Balances::free_balance(&11), 400); + assert_ledger_consistent(11); }) } From b165b27d7d22222e1e27f9ede80ff694aa151625 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 8 Nov 2019 23:24:48 +0100 Subject: [PATCH 29/46] implement slash out of unlocking funds also --- srml/staking/src/lib.rs | 110 +++++++++++++++++++++++++++++++---- srml/staking/src/slashing.rs | 14 +---- 2 files changed, 100 insertions(+), 24 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 43de1118dd947..bb7231f79bf6c 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -258,7 +258,7 @@ use support::{ decl_module, decl_event, decl_storage, ensure, traits::{ Currency, OnFreeBalanceZero, OnDilution, LockIdentifier, LockableCurrency, - WithdrawReasons, OnUnbalanced, Imbalance, Get, Time + WithdrawReasons, OnUnbalanced, Imbalance, Get, Time, } }; use session::{historical::OnSessionEnding, SelectInitialValidators}; @@ -269,6 +269,7 @@ use sr_primitives::{ weights::SimpleDispatchInfo, traits::{ Convert, Zero, One, StaticLookup, CheckedSub, Saturating, Bounded, SaturatedConversion, + SimpleArithmetic, } }; use sr_staking_primitives::{ @@ -389,17 +390,6 @@ pub struct StakingLedger { pub unlocking: Vec>, } -/// A record of the nominations made by a specific account. -#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] -pub struct Nominations { - /// The targets of nomination. - pub targets: Vec, - /// The era the nominations were submitted. - pub submitted_in: EraIndex, - /// Whether the nominations have been suppressed. - pub suppressed: bool, -} - impl< AccountId, Balance: HasCompact + Copy + Saturating, @@ -418,6 +408,74 @@ impl< .collect(); Self { total, active: self.active, stash: self.stash, unlocking } } + +} + +impl StakingLedger where + Balance: SimpleArithmetic + Saturating + Copy, +{ + /// Slash the validator for a given amount of balance. This can grow the value + /// of the slash in the case that the validator has less than `minimum_balance` + /// active funds. Returns the amount of funds actually slashed. + /// + /// Slashes from `active` funds first, and then `unlocking`, starting with the + /// chunks that are closest to unlocking. + fn slash( + &mut self, + mut value: Balance, + minimum_balance: Balance, + ) -> Balance { + let pre_total = self.total; + let total = &mut self.total; + let active = &mut self.active; + + let slash_out_of = | + total_remaining: &mut Balance, + target: &mut Balance, + value: &mut Balance, + | { + let mut slash_from_target = (*value).min(*target); + + if !slash_from_target.is_zero() { + *target -= slash_from_target; + + // don't leave a dust balance in the staking system. + if *target <= minimum_balance { + slash_from_target += *target; + *value += rstd::mem::replace(target, Zero::zero()); + } + + *total_remaining = total_remaining.saturating_sub(slash_from_target); + *value -= slash_from_target; + } + }; + + slash_out_of(total, active, &mut value); + + let i = self.unlocking.iter_mut() + .map(|chunk| { + slash_out_of(total, &mut chunk.value, &mut value); + chunk.value + }) + .take_while(|value| value.is_zero()) // take all fully-consumed chunks out. + .count(); + + // kill all drained chunks. + let _ = self.unlocking.drain(..i); + + pre_total.saturating_sub(*total) + } +} + +/// A record of the nominations made by a specific account. +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] +pub struct Nominations { + /// The targets of nomination. + pub targets: Vec, + /// The era the nominations were submitted. + pub submitted_in: EraIndex, + /// Whether the nominations have been suppressed. + pub suppressed: bool, } /// The amount of exposure (to slashing) than an individual nominator has. @@ -443,6 +501,24 @@ pub struct Exposure { pub others: Vec>, } +/// A pending slash record. The value of the slash has been computed but not applied yet, +/// rather deferred for several eras. +#[derive(Encode, Decode, Default, RuntimeDebug)] +pub struct PendingSlashRecord { + /// The era where the slashed offence was actually applied + era: EraIndex, + /// The stash ID of the offending validator. + validator: AccountId, + /// The validator's own slash. + own: Balance, + /// All other slashed stakers and amounts. + others: Vec<(AccountId, Balance)>, + /// Reporters of the offence; bounty payout recipients. + reporters: Vec, + /// The amount of payout. + payout: Balance, +} + pub type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type PositiveImbalanceOf = @@ -523,6 +599,10 @@ pub trait Trait: system::Trait { /// Number of eras that staked funds must remain bonded for. type BondingDuration: Get; + // /// Number of eras that slashes are deferred by, after computation. This + // /// should be less than the bonding duration. + // type SlashDeferDuration: Get; + /// Interface for interacting with a session module. type SessionInterface: self::SessionInterface; @@ -632,6 +712,12 @@ decl_storage! { /// as well as how much reward has been paid out. SpanSlash: map (T::AccountId, slashing::SpanIndex) => slashing::SpanRecord>; + + /// The earliest era for which we have a pending slash. + EarliestPendingSlash: EraIndex; + + /// All pending slashes that have not been applied yet. + PendingSlashes get(pending_slashes): map EraIndex => Vec>>; } add_extra_genesis { config(stakers): diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index fdeb7034ac595..70fcd39f402ca 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -205,7 +205,7 @@ pub(crate) struct SlashParams<'a, T: 'a + Trait> { #[must_use = "Reward payout must be explicitly applied"] pub(crate) struct RewardPayout(Option<(BalanceOf, NegativeImbalanceOf)>); -/// Slash a validator and their nominators, if necessary. +/// Computes a slash of a validator and nominators, if necessary. pub(crate) fn slash(params: SlashParams) -> RewardPayout { let SlashParams { stash, @@ -568,19 +568,9 @@ fn apply_slash( None => return, // nothing to do. }; - let mut value = value.min(ledger.active); + let value = ledger.slash(value, T::Currency::minimum_balance()); if !value.is_zero() { - ledger.active -= value; - - // Avoid there being a dust balance left in the staking system. - if ledger.active < T::Currency::minimum_balance() { - value += ledger.active; - ledger.active = Zero::zero(); - } - - ledger.total -= value; - let (imbalance, missing) = T::Currency::slash(stash, value); slash_imbalance.subsume(imbalance); From 76608abba2bfdda14c5c70dab6adaba59289ac88 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sat, 9 Nov 2019 01:32:56 +0100 Subject: [PATCH 30/46] slashing: create records to be applied after-the-fact --- srml/staking/src/lib.rs | 23 +++-- srml/staking/src/mock.rs | 2 + srml/staking/src/slashing.rs | 162 +++++++++++++++++++---------------- 3 files changed, 103 insertions(+), 84 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index bb7231f79bf6c..0ad5a0f1232e4 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -504,9 +504,7 @@ pub struct Exposure { /// A pending slash record. The value of the slash has been computed but not applied yet, /// rather deferred for several eras. #[derive(Encode, Decode, Default, RuntimeDebug)] -pub struct PendingSlashRecord { - /// The era where the slashed offence was actually applied - era: EraIndex, +pub struct UnappliedSlash { /// The stash ID of the offending validator. validator: AccountId, /// The validator's own slash. @@ -599,9 +597,9 @@ pub trait Trait: system::Trait { /// Number of eras that staked funds must remain bonded for. type BondingDuration: Get; - // /// Number of eras that slashes are deferred by, after computation. This - // /// should be less than the bonding duration. - // type SlashDeferDuration: Get; + /// Number of eras that slashes are deferred by, after computation. This + /// should be less than the bonding duration. + type SlashDeferDuration: Get; /// Interface for interacting with a session module. type SessionInterface: self::SessionInterface; @@ -693,6 +691,10 @@ decl_storage! { /// The rest of the slashed value is handled by the `Slash`. pub SlashRewardFraction get(fn slash_reward_fraction) config(): Perbill; + /// The amount of currency given to reporters of a slash event which was + /// canceled by extraordinary circumstances (e.g. governance). + pub CanceledSlashPayout get(fn canceled_payout) config(): BalanceOf; + /// A mapping from still-bonded eras to the first session index of that era. BondedEras: Vec<(EraIndex, SessionIndex)>; @@ -717,7 +719,7 @@ decl_storage! { EarliestPendingSlash: EraIndex; /// All pending slashes that have not been applied yet. - PendingSlashes get(pending_slashes): map EraIndex => Vec>>; + PendingSlashes get(pending_slashes): map EraIndex => Vec>>; } add_extra_genesis { config(stakers): @@ -1606,7 +1608,7 @@ impl OnOffenceHandler(slashing::SlashParams { + let unapplied = slashing::compute_slash::(slashing::SlashParams { stash, slash: *slash_fraction, exposure, @@ -1616,7 +1618,10 @@ impl OnOffenceHandler(reward_payout, &details.reporters); + if let Some(mut unapplied) = unapplied { + unapplied.reporters = details.reporters.clone(); + slashing::apply_slash::(unapplied); + } } } } diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index 9589d43a9d575..e45abe73938fd 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -109,6 +109,7 @@ parameter_types! { pub const MaximumBlockWeight: u32 = 1024; pub const MaximumBlockLength: u32 = 2 * 1024; pub const AvailableBlockRatio: Perbill = Perbill::one(); + pub const SlashDeferDuration: EraIndex = 10; } impl system::Trait for Test { type Origin = Origin; @@ -202,6 +203,7 @@ impl Trait for Test { type Slash = (); type Reward = (); type SessionsPerEra = SessionsPerEra; + type SlashDeferDuration = SlashDeferDuration; type BondingDuration = BondingDuration; type SessionInterface = Self; type RewardCurve = RewardCurve; diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 70fcd39f402ca..123f077a7dc4c 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -50,7 +50,7 @@ use super::{ EraIndex, Trait, Module, Store, BalanceOf, Exposure, Perbill, SessionInterface, - NegativeImbalanceOf, + NegativeImbalanceOf, UnappliedSlash, }; use sr_primitives::traits::{Zero, Saturating}; use support::{ @@ -201,12 +201,15 @@ pub(crate) struct SlashParams<'a, T: 'a + Trait> { pub(crate) reward_proportion: Perbill, } -/// An unapplied reward payout. Feed into `pay_reporters`. -#[must_use = "Reward payout must be explicitly applied"] -pub(crate) struct RewardPayout(Option<(BalanceOf, NegativeImbalanceOf)>); - -/// Computes a slash of a validator and nominators, if necessary. -pub(crate) fn slash(params: SlashParams) -> RewardPayout { +/// Computes a slash of a validator and nominators. It returns an unapplied +/// record to be applied at some later point. Slashing metadata is updated in storage, +/// since unapplied records are only rarely intended to be dropped. +/// +/// The pending slash record returned does not have initialized reporters. Those have +/// to be set at a higher level, if any. +pub(crate) fn compute_slash(params: SlashParams) + -> Option>> +{ let SlashParams { stash, slash, @@ -218,7 +221,7 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { } = params.clone(); let mut reward_payout = Zero::zero(); - let mut slash_imbalance = >::zero(); + let mut val_slashed = Zero::zero(); // is the slash amount here a maximum for the era? let own_slash = slash * exposure.own; @@ -226,7 +229,7 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { // kick out the validator even if they won't be slashed, // as long as the misbehavior is from their most recent slashing span. kick_out_if_recent::(params); - return RewardPayout(None) + return None; } let (prior_slash_p, _era_slash) = as Store>::ValidatorSlashInEra::get( @@ -250,7 +253,7 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { // pays out some reward even if the latest report is not max-in-era. // we opt to avoid the nominator lookups and edits and leave more rewards // for more drastic misbehavior. - return RewardPayout(None) + return None; } // apply slash to validator. @@ -259,7 +262,7 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { stash, window_start, &mut reward_payout, - &mut slash_imbalance, + &mut val_slashed, reward_proportion, ); @@ -285,9 +288,16 @@ pub(crate) fn slash(params: SlashParams) -> RewardPayout { } } - reward_payout += slash_nominators::(params, prior_slash_p, &mut slash_imbalance); + let mut nominators_slashed = Vec::new(); + reward_payout += slash_nominators::(params, prior_slash_p, &mut nominators_slashed); - RewardPayout(Some((reward_payout, slash_imbalance))) + Some(UnappliedSlash { + validator: stash.clone(), + own: val_slashed, + others: nominators_slashed, + reporters: Vec::new(), + payout: reward_payout, + }) } // doesn't apply any slash, but kicks out the validator if the misbehavior is from @@ -297,12 +307,12 @@ fn kick_out_if_recent( ) { // these are not updated by era-span or end-span. let mut reward_payout = Zero::zero(); - let mut slash_imbalance = >::zero(); + let mut val_slashed = Zero::zero(); let mut spans = fetch_spans::( params.stash, params.window_start, &mut reward_payout, - &mut slash_imbalance, + &mut val_slashed, params.reward_proportion, ); @@ -324,7 +334,7 @@ fn kick_out_if_recent( fn slash_nominators( params: SlashParams, prior_slash_p: Perbill, - slash_imbalance: &mut NegativeImbalanceOf, + nominators_slashed: &mut Vec<(T::AccountId, BalanceOf)>, ) -> BalanceOf { let SlashParams { stash: _, @@ -338,8 +348,10 @@ fn slash_nominators( let mut reward_payout = Zero::zero(); + nominators_slashed.reserve(exposure.others.len()); for nominator in &exposure.others { let stash = &nominator.who; + let mut nom_slashed = Zero::zero(); // the era slash of a nominator always grows, if the validator // had a new max slash for the era. @@ -370,7 +382,7 @@ fn slash_nominators( stash, window_start, &mut reward_payout, - slash_imbalance, + &mut nom_slashed, reward_proportion, ); @@ -382,9 +394,11 @@ fn slash_nominators( if target_span == Some(spans.span_index()) { // Chill the nominator outright, ending the slashing span. spans.end_span(now); - >::chill_stash(&stash); + >::chill_stash(stash); } } + + nominators_slashed.push((stash.clone(), nom_slashed)); } reward_payout @@ -402,9 +416,8 @@ struct InspectingSpans<'a, T: Trait + 'a> { window_start: EraIndex, stash: &'a T::AccountId, spans: SlashingSpans, - deferred_slash: Option>, paid_out: &'a mut BalanceOf, - slash_imbalance: &'a mut NegativeImbalanceOf, + slash_of: &'a mut BalanceOf, reward_proportion: Perbill, _marker: rstd::marker::PhantomData, } @@ -414,7 +427,7 @@ fn fetch_spans<'a, T: Trait + 'a>( stash: &'a T::AccountId, window_start: EraIndex, paid_out: &'a mut BalanceOf, - slash_imbalance: &'a mut NegativeImbalanceOf, + slash_of: &'a mut BalanceOf, reward_proportion: Perbill, ) -> InspectingSpans<'a, T> { let spans = as Store>::SlashingSpans::get(stash).unwrap_or_else(|| { @@ -428,8 +441,7 @@ fn fetch_spans<'a, T: Trait + 'a>( window_start, stash, spans, - deferred_slash: None, - slash_imbalance, + slash_of, paid_out, reward_proportion, _marker: rstd::marker::PhantomData, @@ -445,9 +457,8 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { self.dirty = self.spans.end_span(now) || self.dirty; } - fn defer_slash(&mut self, amount: BalanceOf) { - let total_deferred = self.deferred_slash.get_or_insert_with(Zero::zero); - *total_deferred = *total_deferred + amount; + fn add_slash(&mut self, amount: BalanceOf) { + *self.slash_of += amount; } // find the span index of the given era, if covered. @@ -478,7 +489,7 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { let reward = REWARD_F1 * (self.reward_proportion * slash).saturating_sub(span_record.paid_out); - self.defer_slash(difference); + self.add_slash(difference); changed = true; reward @@ -516,13 +527,6 @@ impl<'a, T: 'a + Trait> Drop for InspectingSpans<'a, T> { } as Store>::SlashingSpans::insert(self.stash, &self.spans); - - // do this AFTER updating span information because it is possible that the - // slash may zero the account and cause garbage collection. any stored data - // after that point would be orphaned. - if let Some(slash) = self.deferred_slash.take() { - apply_slash::(self.stash, slash, self.paid_out, self.slash_imbalance) - } } } @@ -552,11 +556,11 @@ pub(crate) fn clear_stash_metadata(stash: &T::AccountId) { // apply the slash to a stash account, deducting any missing funds from the reward // payout, saturating at 0. this is mildly unfair but also an edge-case that // can only occur when overlapping locked funds have been slashed. -fn apply_slash( +fn do_slash( stash: &T::AccountId, value: BalanceOf, reward_payout: &mut BalanceOf, - slash_imbalance: &mut NegativeImbalanceOf, + slashed_imbalance: &mut NegativeImbalanceOf, ) { let controller = match >::bonded(stash) { None => return, // defensive: should always exist. @@ -572,7 +576,7 @@ fn apply_slash( if !value.is_zero() { let (imbalance, missing) = T::Currency::slash(stash, value); - slash_imbalance.subsume(imbalance); + slashed_imbalance.subsume(imbalance); if !missing.is_zero() { // deduct overslash from the reward payout @@ -588,53 +592,61 @@ fn apply_slash( } } -impl RewardPayout { - fn pay_to(&mut self, reporters: &[T::AccountId]) { - let (reward_payout, slashed_imbalance) = match self.0.take() { - None => return, - Some((payout, slashed_imbalance)) => { - if payout.is_zero() || reporters.is_empty() { - // nobody to pay out to or nothing to pay; - // just treat the whole value as slashed. - T::Slash::on_unbalanced(slashed_imbalance); - return - } - (payout, slashed_imbalance) - }, - }; +/// Apply a previously-unapplied slash. +pub(crate) fn apply_slash(unapplied_slash: UnappliedSlash>) { + let mut slashed_imbalance = NegativeImbalanceOf::::zero(); + let mut reward_payout = unapplied_slash.payout; - // take rewards out of the slashed imbalance. - let (mut reward_payout, mut value_slashed) = slashed_imbalance.split(reward_payout); - - let per_reporter = reward_payout.peek() / (reporters.len() as u32).into(); - for reporter in reporters { - let (reporter_reward, rest) = reward_payout.split(per_reporter); - reward_payout = rest; - - // this cancels out the reporter reward imbalance internally, leading - // to no change in total issuance. - T::Currency::resolve_creating(reporter, reporter_reward); - } + do_slash::( + &unapplied_slash.validator, + unapplied_slash.own, + &mut reward_payout, + &mut slashed_imbalance, + ); - // the rest goes to the on-slash imbalance handler (e.g. treasury) - value_slashed.subsume(reward_payout); // remainder of reward division remains. - T::Slash::on_unbalanced(value_slashed); + for &(ref nominator, nominator_slash) in &unapplied_slash.others { + do_slash::( + &nominator, + nominator_slash, + &mut reward_payout, + &mut slashed_imbalance, + ); } -} -impl Drop for RewardPayout { - fn drop(&mut self) { - self.pay_to(&[]); - } + pay_reporters::(reward_payout, slashed_imbalance, &unapplied_slash.reporters); } -/// Apply a reward payout to some reporters. -pub(crate) fn pay_reporters( - mut reward_payout: RewardPayout, +/// Apply a reward payout to some reporters, paying the rewards out of the slashed imbalance. +fn pay_reporters( + reward_payout: BalanceOf, + slashed_imbalance: NegativeImbalanceOf, reporters: &[T::AccountId], ) { - reward_payout.pay_to(reporters) + if reward_payout.is_zero() || reporters.is_empty() { + // nobody to pay out to or nothing to pay; + // just treat the whole value as slashed. + T::Slash::on_unbalanced(slashed_imbalance); + return + } + + // take rewards out of the slashed imbalance. + let reward_payout = reward_payout.min(slashed_imbalance.peek()); + let (mut reward_payout, mut value_slashed) = slashed_imbalance.split(reward_payout); + + let per_reporter = reward_payout.peek() / (reporters.len() as u32).into(); + for reporter in reporters { + let (reporter_reward, rest) = reward_payout.split(per_reporter); + reward_payout = rest; + + // this cancels out the reporter reward imbalance internally, leading + // to no change in total issuance. + T::Currency::resolve_creating(reporter, reporter_reward); + } + + // the rest goes to the on-slash imbalance handler (e.g. treasury) + value_slashed.subsume(reward_payout); // remainder of reward division remains. + T::Slash::on_unbalanced(value_slashed); } // TODO: function for undoing a slash. From 9c30d7dd592b03809d0c03084214a16ff81429bd Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 11 Nov 2019 18:42:39 +0100 Subject: [PATCH 31/46] queue slashes for a few eras later --- srml/staking/src/lib.rs | 40 +++++++++++++++++++++++++++++------- srml/staking/src/slashing.rs | 2 +- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 0ad5a0f1232e4..d52515bd515d9 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -712,14 +712,14 @@ decl_storage! { /// Records information about the maximum slash of a stash within a slashing span, /// as well as how much reward has been paid out. - SpanSlash: map (T::AccountId, slashing::SpanIndex) - => slashing::SpanRecord>; + SpanSlash: + map (T::AccountId, slashing::SpanIndex) => slashing::SpanRecord>; - /// The earliest era for which we have a pending slash. - EarliestPendingSlash: EraIndex; + /// The earliest era for which we have a pending, unapplied slash. + EarliestUnappliedSlash: Option; - /// All pending slashes that have not been applied yet. - PendingSlashes get(pending_slashes): map EraIndex => Vec>>; + /// All unapplied slashes that are queued for later. + UnappliedSlashes: map EraIndex => Vec>>; } add_extra_genesis { config(stakers): @@ -1306,10 +1306,27 @@ impl Module { // Reassign all Stakers. let (_slot_stake, maybe_new_validators) = Self::select_validators(); + Self::apply_unapplied_slashes(current_era); maybe_new_validators } + /// Apply previously-unapplied slashes on the beginning of a new era, after a delay. + fn apply_unapplied_slashes(current_era: EraIndex) { + let slash_defer_duration = T::SlashDeferDuration::get(); + ::EarliestUnappliedSlash::mutate(|earliest| if let Some(ref mut earliest) = earliest { + let keep_from = current_era.saturating_sub(slash_defer_duration) + 1; + for era in (*earliest)..keep_from { + let era_slashes = ::UnappliedSlashes::take(&era); + for slash in era_slashes { + slashing::apply_slash::(slash); + } + } + + *earliest = (*earliest).max(keep_from) + }) + } + /// Select a new validator set from the assembled stakers and their role preferences. /// /// Returns the new `SlotStake` value and a set of newly selected _stash_ IDs. @@ -1599,6 +1616,12 @@ impl OnOffenceHandler::EarliestUnappliedSlash::mutate(|earliest| { + if earliest.is_none() { + *earliest = Some(era_now) + } + }); + for (details, slash_fraction) in offenders.iter().zip(slash_fraction) { let stash = &details.offender.0; let exposure = &details.offender.1; @@ -1620,7 +1643,10 @@ impl OnOffenceHandler(unapplied); + ::UnappliedSlashes::mutate( + era_now, + move |for_later| for_later.push(unapplied), + ); } } } diff --git a/srml/staking/src/slashing.rs b/srml/staking/src/slashing.rs index 123f077a7dc4c..62662041d0b70 100644 --- a/srml/staking/src/slashing.rs +++ b/srml/staking/src/slashing.rs @@ -328,7 +328,7 @@ fn kick_out_if_recent( } } -/// Slash nominators. Accepts general parameters and the prior slash percentage of the nominator. +/// Slash nominators. Accepts general parameters and the prior slash percentage of the validator. /// /// Returns the amount of reward to pay out. fn slash_nominators( From d3ac5a2ebcccf527c33d487fe1a5eeb3e2d0364d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 11 Nov 2019 21:50:32 +0100 Subject: [PATCH 32/46] method for canceling deferred slashes --- srml/staking/src/lib.rs | 54 +++++++++++++++---- srml/staking/src/mock.rs | 16 +++++- srml/staking/src/tests.rs | 106 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 10 deletions(-) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index d52515bd515d9..88427e64bb08d 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -598,7 +598,8 @@ pub trait Trait: system::Trait { type BondingDuration: Get; /// Number of eras that slashes are deferred by, after computation. This - /// should be less than the bonding duration. + /// should be less than the bonding duration. Set to 0 if slashes should be + /// applied immediately, without opportunity for intervention. type SlashDeferDuration: Get; /// Interface for interacting with a session module. @@ -695,6 +696,9 @@ decl_storage! { /// canceled by extraordinary circumstances (e.g. governance). pub CanceledSlashPayout get(fn canceled_payout) config(): BalanceOf; + /// All unapplied slashes that are queued for later. + pub UnappliedSlashes: map EraIndex => Vec>>; + /// A mapping from still-bonded eras to the first session index of that era. BondedEras: Vec<(EraIndex, SessionIndex)>; @@ -717,9 +721,6 @@ decl_storage! { /// The earliest era for which we have a pending, unapplied slash. EarliestUnappliedSlash: Option; - - /// All unapplied slashes that are queued for later. - UnappliedSlashes: map EraIndex => Vec>>; } add_extra_genesis { config(stakers): @@ -1126,6 +1127,33 @@ decl_module! { ensure_root(origin)?; ForceEra::put(Forcing::ForceAlways); } + + /// Cancel enactment of a deferred slash. Can only be called by the root origin, + /// passing the era and indices of the slashes for that era to kill. + /// + /// # + /// - One storage write. + /// # + #[weight = SimpleDispatchInfo::FreeOperational] + fn cancel_deferred_slash(origin, era: EraIndex, slash_indices: Vec) { + ensure_root(origin)?; + + let mut slash_indices = slash_indices; + slash_indices.sort_unstable(); + let mut unapplied = ::UnappliedSlashes::get(&era); + + for (removed, index) in slash_indices.into_iter().enumerate() { + let index = index as usize; + + // if `index` is not duplicate, `removed` must be <= index. + if removed > index { return Err("duplicate index") } + if index >= unapplied.len() { return Err("slash record index out of bounds") } + + unapplied.remove(index); + } + + ::UnappliedSlashes::insert(&era, &unapplied); + } } } @@ -1315,7 +1343,7 @@ impl Module { fn apply_unapplied_slashes(current_era: EraIndex) { let slash_defer_duration = T::SlashDeferDuration::get(); ::EarliestUnappliedSlash::mutate(|earliest| if let Some(ref mut earliest) = earliest { - let keep_from = current_era.saturating_sub(slash_defer_duration) + 1; + let keep_from = current_era.saturating_sub(slash_defer_duration); for era in (*earliest)..keep_from { let era_slashes = ::UnappliedSlashes::take(&era); for slash in era_slashes { @@ -1622,6 +1650,8 @@ impl OnOffenceHandler OnOffenceHandler::UnappliedSlashes::mutate( - era_now, - move |for_later| for_later.push(unapplied), - ); + if slash_defer_duration == 0 { + // apply right away. + slashing::apply_slash::(unapplied); + } else { + // defer to end of some `slash_defer_duration` from now. + ::UnappliedSlashes::mutate( + era_now, + move |for_later| for_later.push(unapplied), + ); + } } } } diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index e45abe73938fd..18e8e27be9ebe 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -48,6 +48,7 @@ impl Convert for CurrencyToVoteHandler { thread_local! { static SESSION: RefCell<(Vec, HashSet)> = RefCell::new(Default::default()); static EXISTENTIAL_DEPOSIT: RefCell = RefCell::new(0); + static SLASH_DEFER_DURATION: RefCell = RefCell::new(0); } pub struct TestSessionHandler; @@ -87,6 +88,13 @@ impl Get for ExistentialDeposit { } } +pub struct SlashDeferDuration; +impl Get for SlashDeferDuration { + fn get() -> EraIndex { + SLASH_DEFER_DURATION.with(|v| *v.borrow()) + } +} + impl_outer_origin!{ pub enum Origin for Test {} } @@ -109,7 +117,6 @@ parameter_types! { pub const MaximumBlockWeight: u32 = 1024; pub const MaximumBlockLength: u32 = 2 * 1024; pub const AvailableBlockRatio: Perbill = Perbill::one(); - pub const SlashDeferDuration: EraIndex = 10; } impl system::Trait for Test { type Origin = Origin; @@ -215,6 +222,7 @@ pub struct ExtBuilder { nominate: bool, validator_count: u32, minimum_validator_count: u32, + slash_defer_duration: EraIndex, fair: bool, num_validators: Option, invulnerables: Vec, @@ -228,6 +236,7 @@ impl Default for ExtBuilder { nominate: true, validator_count: 2, minimum_validator_count: 0, + slash_defer_duration: 0, fair: true, num_validators: None, invulnerables: vec![], @@ -256,6 +265,10 @@ impl ExtBuilder { self.minimum_validator_count = count; self } + pub fn slash_defer_duration(mut self, eras: EraIndex) -> Self { + self.slash_defer_duration = eras; + self + } pub fn fair(mut self, is_fair: bool) -> Self { self.fair = is_fair; self @@ -270,6 +283,7 @@ impl ExtBuilder { } pub fn set_associated_consts(&self) { EXISTENTIAL_DEPOSIT.with(|v| *v.borrow_mut() = self.existential_deposit); + SLASH_DEFER_DURATION.with(|v| *v.borrow_mut() = self.slash_defer_duration); } pub fn build(self) -> runtime_io::TestExternalities { self.set_associated_consts(); diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 33fa06c73809e..06dcbe0a6c9d6 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -2357,3 +2357,109 @@ fn slashes_are_summed_across_spans() { assert_eq!(Balances::free_balance(&21), 1810); }); } + +#[test] +fn deferred_slashes_are_deferred() { + ExtBuilder::default().slash_defer_duration(2).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); + let nominated_value = exposure.others.iter().find(|o| o.who == 101).unwrap().value; + + on_offence_now( + &[ + OffenceDetails { + offender: (11, Staking::stakers(&11)), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + ); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + start_era(2); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + start_era(3); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + // at the start of era 4, slashes from era 1 are processed, + // after being deferred for at least 2 full eras. + start_era(4); + + assert_eq!(Balances::free_balance(&11), 900); + assert_eq!(Balances::free_balance(&101), 2000 - (nominated_value / 10)); + }) +} + +#[test] +fn remove_deferred() { + ExtBuilder::default().slash_defer_duration(2).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); + let nominated_value = exposure.others.iter().find(|o| o.who == 101).unwrap().value; + + on_offence_now( + &[ + OffenceDetails { + offender: (11, exposure.clone()), + reporters: vec![], + }, + ], + &[Perbill::from_percent(10)], + ); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + start_era(2); + + on_offence_in_era( + &[ + OffenceDetails { + offender: (11, exposure.clone()), + reporters: vec![], + }, + ], + &[Perbill::from_percent(15)], + 1, + ); + + Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![0]).unwrap(); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + start_era(3); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + // at the start of era 4, slashes from era 1 are processed, + // after being deferred for at least 2 full eras. + start_era(4); + + // the first slash for 10% was cancelled, so no effect. + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + start_era(5); + + // 5% slash (15 - 10) processed now. + assert_eq!(Balances::free_balance(&11), 950); + assert_eq!(Balances::free_balance(&101), 2000 - (nominated_value / 20)); + }) +} From a5a8e99e88c26f24b9ab0c18bd8a7a1d9b6c02e6 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 11 Nov 2019 22:51:01 +0100 Subject: [PATCH 33/46] attempt to fix test in CI --- srml/staking/src/tests.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 06dcbe0a6c9d6..a6e0e18cb8b16 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -2458,8 +2458,15 @@ fn remove_deferred() { start_era(5); + let slash_10 = Perbill::from_percent(10); + let slash_15 = Perbill::from_percent(15); + let initial_slash = slash_10 * nominated_value; + + let total_slash = slash_15 * nominated_value; + let actual_slash = total_slash - initial_slash; + // 5% slash (15 - 10) processed now. assert_eq!(Balances::free_balance(&11), 950); - assert_eq!(Balances::free_balance(&101), 2000 - (nominated_value / 20)); + assert_eq!(Balances::free_balance(&101), 2000 - actual_slash); }) } From 5ba349e849d5464fb9a804a34aa781954c43df43 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 13 Nov 2019 23:30:48 +0100 Subject: [PATCH 34/46] storage migration for `Nominators` --- srml/staking/Cargo.toml | 2 +- srml/staking/src/lib.rs | 30 ++++++++++++- srml/staking/src/migration.rs | 84 +++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 srml/staking/src/migration.rs diff --git a/srml/staking/Cargo.toml b/srml/staking/Cargo.toml index b360e68b4a7a9..c3f9adab804d4 100644 --- a/srml/staking/Cargo.toml +++ b/srml/staking/Cargo.toml @@ -27,7 +27,7 @@ srml-staking-reward-curve = { path = "../staking/reward-curve"} [features] equalize = [] -migrate-storage-v1 = [] +migrate = [] default = ["std", "equalize"] std = [ "serde", diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 90020424d3f01..96664986ace05 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -248,6 +248,7 @@ mod mock; #[cfg(test)] mod tests; +mod migration; mod slashing; pub mod inflation; @@ -654,7 +655,10 @@ decl_storage! { pub Validators get(fn validators): linked_map T::AccountId => ValidatorPrefs>; /// The map from nominator stash key to the set of stash keys of all validators to nominate. - pub Nominators get(fn nominators): linked_map T::AccountId => Option>; + /// + /// NOTE: is private so that we can ensure upgraded before all typical accesses. + /// Direct storage APIs can still bypass this protection. + Nominators get(fn nominators): linked_map T::AccountId => Option>; /// Nominators for a particular account that is in action right now. You can't iterate /// through validators here, but you can find them in the Session module. @@ -721,6 +725,9 @@ decl_storage! { /// The earliest era for which we have a pending, unapplied slash. EarliestUnappliedSlash: Option; + + /// The version of storage for upgrade. + StorageVersion: u32; } add_extra_genesis { config(stakers): @@ -779,6 +786,10 @@ decl_module! { fn deposit_event() = default; + fn on_initialize() { + Self::ensure_storage_upgraded(); + } + fn on_finalize() { // Set the start of the first era. if !>::exists() { @@ -965,6 +976,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(750_000)] fn validate(origin, prefs: ValidatorPrefs>) { + Self::ensure_storage_upgraded(); + let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or("not a controller")?; let stash = &ledger.stash; @@ -985,6 +998,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(750_000)] fn nominate(origin, targets: Vec<::Source>) { + Self::ensure_storage_upgraded(); + let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or("not a controller")?; let stash = &ledger.stash; @@ -1186,10 +1201,17 @@ impl Module { /// Chill a stash account. fn chill_stash(stash: &T::AccountId) { + Self::ensure_storage_upgraded(); + >::remove(stash); >::remove(stash); } + /// Ensures storage is upgraded to most recent necessary state. + fn ensure_storage_upgraded() { + migration::perform_migrations::(); + } + /// Actually make a payment to a staker. This uses the currency's reward function /// to pay the right payee for the given staker account. fn make_payout(stash: &T::AccountId, amount: BalanceOf) -> Option> { @@ -1361,6 +1383,8 @@ impl Module { /// Select a new validator set from the assembled stakers and their role preferences. /// /// Returns the new `SlotStake` value and a set of newly selected _stash_ IDs. + /// + /// Assumes storage is coherent with the declaration. fn select_validators() -> (BalanceOf, Option>) { let mut all_nominators: Vec<(T::AccountId, Vec)> = Vec::new(); let all_validator_candidates_iter = >::enumerate(); @@ -1500,6 +1524,8 @@ impl Module { /// - Immediately when an account's balance falls below existential deposit. /// - after a `withdraw_unbond()` call that frees all of a stash's bonded balance. fn kill_stash(stash: &T::AccountId) { + Self::ensure_storage_upgraded(); + if let Some(controller) = >::take(stash) { >::remove(&controller); } @@ -1563,6 +1589,7 @@ impl Module { impl session::OnSessionEnding for Module { fn on_session_ending(_ending: SessionIndex, start_session: SessionIndex) -> Option> { + Self::ensure_storage_upgraded(); Self::new_session(start_session - 1).map(|(new, _old)| new) } } @@ -1571,6 +1598,7 @@ impl OnSessionEnding fn on_session_ending(_ending: SessionIndex, start_session: SessionIndex) -> Option<(Vec, Vec<(T::AccountId, Exposure>)>)> { + Self::ensure_storage_upgraded(); Self::new_session(start_session - 1) } } diff --git a/srml/staking/src/migration.rs b/srml/staking/src/migration.rs new file mode 100644 index 0000000000000..9a1ae6739eb8c --- /dev/null +++ b/srml/staking/src/migration.rs @@ -0,0 +1,84 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Storage migrations for srml-staking. + +#[cfg(feature = "migrate")] +mod inner { + use crate::{Store, Module, Trait}; + use support::StorageLinkedMap; + + /// Indicator of a version of a storage layout. + pub type VersionNumber = u32; + + // the minimum supported version of the migration logic. + const MIN_SUPPORTED_VERSION: VersionNumber = 0; + + // the current expected version of the storage + const CURRENT_VERSION: VersionNumber = 1; + + // migrate storage from v0 to v1. + // + // this upgrades the `Nominators` linked_map value type from `Vec` to + // `Option>` + pub fn to_v1(version: &mut VersionNumber) { + if *version != 0 { return } + *version += 1; + + let now = >::current_era(); + let res = as Store>::Nominators::translate::, _, _>( + |key| key, + |targets| super::Nominations { + targets, + submitted_in: now, + suppressed: false, + }, + ); + + if let Err(e) = res { + support::print("Encountered error in migration of Staking::Nominators map."); + if e.is_none() { + support::print("Staking::Nominators map reinitialized"); + } + } + } + + pub(super) fn perform_migrations() { + as MainStore>::StorageVersion::mutate(|version| { + if *version < MIN_SUPPORTED_VERSION { + support::print("Cannot migrate staking storage because version is less than\ + minimum."); + support::print(*version); + return + } + + if *version == CURRENT_VERSION { return } + + to_v1::run(version); + }); + } +} + +#[cfg(not(feature = "migrate"))] +mod inner { + pub(super) fn perform_migrations() { } +} + +/// Perform all necessary storage migrations to get storage into the expected stsate for current +/// logic. No-op if fully upgraded. +pub(crate) fn perform_migrations() { + inner::perform_migrations::(); +} From 36f3b3477aa8e9f9d25d0b050d06ef34fa3c073c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 13 Nov 2019 23:35:22 +0100 Subject: [PATCH 35/46] update node-runtime to use SlashDeferDuration --- node/runtime/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index c321693f45680..63e728374e162 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -76,8 +76,8 @@ 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: 193, - impl_version: 193, + spec_version: 194, + impl_version: 194, apis: RUNTIME_API_VERSIONS, }; @@ -247,6 +247,7 @@ srml_staking_reward_curve::build! { parameter_types! { pub const SessionsPerEra: sr_staking_primitives::SessionIndex = 6; pub const BondingDuration: staking::EraIndex = 24 * 28; + pub const SlashDeferDuration: staking::EraIndex = 24 * 7; // 1/4 the bonding duration. pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; } @@ -260,6 +261,7 @@ impl staking::Trait for Runtime { type Reward = (); // rewards are minted from the void type SessionsPerEra = SessionsPerEra; type BondingDuration = BondingDuration; + type SlashDeferDuration = SlashDeferDuration; type SessionInterface = Self; type RewardCurve = RewardCurve; } From 11b71f3e114ebf40c1cdadfcfd6e48a724d1666c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 15 Nov 2019 08:27:44 +0100 Subject: [PATCH 36/46] adjust migration entry-points somewhat --- paint/staking/src/lib.rs | 11 +++++++---- paint/staking/src/migration.rs | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/paint/staking/src/lib.rs b/paint/staking/src/lib.rs index 39d630a582339..ac4c6816f1262 100644 --- a/paint/staking/src/lib.rs +++ b/paint/staking/src/lib.rs @@ -1201,8 +1201,6 @@ impl Module { /// Chill a stash account. fn chill_stash(stash: &T::AccountId) { - Self::ensure_storage_upgraded(); - >::remove(stash); >::remove(stash); } @@ -1520,12 +1518,12 @@ impl Module { /// Remove all associated data of a stash account from the staking system. /// + /// Assumes storage is upgraded before calling. + /// /// This is called : /// - Immediately when an account's balance falls below existential deposit. /// - after a `withdraw_unbond()` call that frees all of a stash's bonded balance. fn kill_stash(stash: &T::AccountId) { - Self::ensure_storage_upgraded(); - if let Some(controller) = >::take(stash) { >::remove(&controller); } @@ -1605,6 +1603,7 @@ impl OnSessionEnding impl OnFreeBalanceZero for Module { fn on_free_balance_zero(stash: &T::AccountId) { + Self::ensure_storage_upgraded(); Self::kill_stash(stash); } } @@ -1670,6 +1669,8 @@ impl OnOffenceHandler>::ensure_storage_upgraded(); + let reward_proportion = SlashRewardFraction::get(); let era_now = Self::current_era(); @@ -1745,6 +1746,8 @@ impl ReportOffence O: Offence, { fn report_offence(reporters: Vec, offence: O) { + >::ensure_storage_upgraded(); + // disallow any slashing from before the current bonding period. let offence_session = offence.session_index(); let bonded_eras = BondedEras::get(); diff --git a/paint/staking/src/migration.rs b/paint/staking/src/migration.rs index 9a1ae6739eb8c..dc4fac40dfb7b 100644 --- a/paint/staking/src/migration.rs +++ b/paint/staking/src/migration.rs @@ -16,7 +16,7 @@ //! Storage migrations for srml-staking. -#[cfg(feature = "migrate")] +#[cfg(any(test, feature = "migrate"))] mod inner { use crate::{Store, Module, Trait}; use support::StorageLinkedMap; @@ -72,7 +72,7 @@ mod inner { } } -#[cfg(not(feature = "migrate"))] +#[cfg(not(any(test, feature = "migrate")))] mod inner { pub(super) fn perform_migrations() { } } From 74087bafac25b4eb91c82b401acf0f576edb7799 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 17 Nov 2019 16:28:57 +0100 Subject: [PATCH 37/46] fix migration compilation --- paint/staking/src/migration.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/paint/staking/src/migration.rs b/paint/staking/src/migration.rs index dc4fac40dfb7b..663f3367f0015 100644 --- a/paint/staking/src/migration.rs +++ b/paint/staking/src/migration.rs @@ -19,7 +19,7 @@ #[cfg(any(test, feature = "migrate"))] mod inner { use crate::{Store, Module, Trait}; - use support::StorageLinkedMap; + use support::{StorageLinkedMap, StorageValue}; /// Indicator of a version of a storage layout. pub type VersionNumber = u32; @@ -41,7 +41,7 @@ mod inner { let now = >::current_era(); let res = as Store>::Nominators::translate::, _, _>( |key| key, - |targets| super::Nominations { + |targets| crate::Nominations { targets, submitted_in: now, suppressed: false, @@ -57,7 +57,7 @@ mod inner { } pub(super) fn perform_migrations() { - as MainStore>::StorageVersion::mutate(|version| { + as Store>::StorageVersion::mutate(|version| { if *version < MIN_SUPPORTED_VERSION { support::print("Cannot migrate staking storage because version is less than\ minimum."); @@ -67,7 +67,7 @@ mod inner { if *version == CURRENT_VERSION { return } - to_v1::run(version); + to_v1::(version); }); } } From b8232868c2e3d0d435240b797c5bb7c73d8cfd1a Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 18 Nov 2019 18:04:27 +0100 Subject: [PATCH 38/46] add manual Vec import to migration --- paint/staking/src/migration.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/paint/staking/src/migration.rs b/paint/staking/src/migration.rs index 663f3367f0015..3bb060fb39639 100644 --- a/paint/staking/src/migration.rs +++ b/paint/staking/src/migration.rs @@ -20,6 +20,7 @@ mod inner { use crate::{Store, Module, Trait}; use support::{StorageLinkedMap, StorageValue}; + use rstd::vec::Vec; /// Indicator of a version of a storage layout. pub type VersionNumber = u32; From 1866a5024f7adbe56ea806b7603a038201beb7ac Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 18 Nov 2019 18:29:59 +0100 Subject: [PATCH 39/46] enable migrations feature in node-runtime --- bin/node/runtime/Cargo.toml | 2 +- paint/staking/src/migration.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 2cc7f84e8574c..9a59ba77dc1cb 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -50,7 +50,7 @@ nicks = { package = "paint-nicks", path = "../../../paint/nicks", default-featur offences = { package = "paint-offences", path = "../../../paint/offences", default-features = false } randomness-collective-flip = { package = "paint-randomness-collective-flip", path = "../../../paint/randomness-collective-flip", default-features = false } session = { package = "paint-session", path = "../../../paint/session", default-features = false, features = ["historical"] } -staking = { package = "paint-staking", path = "../../../paint/staking", default-features = false } +staking = { package = "paint-staking", path = "../../../paint/staking", default-features = false, features = ["migrate"] } paint-staking-reward-curve = { path = "../../../paint/staking/reward-curve"} sudo = { package = "paint-sudo", path = "../../../paint/sudo", default-features = false } support = { package = "paint-support", path = "../../../paint/support", default-features = false } diff --git a/paint/staking/src/migration.rs b/paint/staking/src/migration.rs index 3bb060fb39639..abd7a9bff371f 100644 --- a/paint/staking/src/migration.rs +++ b/paint/staking/src/migration.rs @@ -55,6 +55,8 @@ mod inner { support::print("Staking::Nominators map reinitialized"); } } + + support::print("Finished migrating Staking storage to v1."); } pub(super) fn perform_migrations() { From 71064258645df30fbe8a816e88e54228f25b9505 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 18 Nov 2019 19:05:28 +0100 Subject: [PATCH 40/46] bump runtime version --- bin/node/runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index aed41d8d943d4..2f2f1dec3f67f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -77,8 +77,8 @@ 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: 194, - impl_version: 194, + spec_version: 195, + impl_version: 195, apis: RUNTIME_API_VERSIONS, }; From deda33601eda37a71c751f465d4b8a6ed36c9e65 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 22 Nov 2019 15:42:40 +0100 Subject: [PATCH 41/46] update to latest master crate renames --- bin/node/runtime/Cargo.toml | 12 ++++++------ palette/staking/src/lib.rs | 2 ++ {paint => palette}/staking/src/migration.rs | 0 {paint => palette}/staking/src/slashing.rs | 0 4 files changed, 8 insertions(+), 6 deletions(-) rename {paint => palette}/staking/src/migration.rs (100%) rename {paint => palette}/staking/src/slashing.rs (100%) diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 7cc7d2a2bfe9b..4b08f841fc5f1 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -40,7 +40,7 @@ contracts = { package = "pallet-contracts", path = "../../../palette/contracts", contracts-rpc-runtime-api = { package = "pallet-contracts-rpc-runtime-api", path = "../../../palette/contracts/rpc/runtime-api/", default-features = false } democracy = { package = "pallet-democracy", path = "../../../palette/democracy", default-features = false } elections-phragmen = { package = "pallet-elections-phragmen", path = "../../../palette/elections-phragmen", default-features = false } -executive = { package = "pallet-executive", path = "../../../palette/executive", default-features = false } +executive = { package = "palette-executive", path = "../../../palette/executive", default-features = false } finality-tracker = { package = "pallet-finality-tracker", path = "../../../palette/finality-tracker", default-features = false } grandpa = { package = "pallet-grandpa", path = "../../../palette/grandpa", default-features = false } im-online = { package = "pallet-im-online", path = "../../../palette/im-online", default-features = false } @@ -51,14 +51,14 @@ offences = { package = "pallet-offences", path = "../../../palette/offences", de randomness-collective-flip = { package = "pallet-randomness-collective-flip", path = "../../../palette/randomness-collective-flip", default-features = false } session = { package = "pallet-session", path = "../../../palette/session", default-features = false, features = ["historical"] } staking = { package = "pallet-staking", path = "../../../palette/staking", default-features = false, features = ["migrate"] } -paint-staking-reward-curve = { path = "../../../palette/staking/reward-curve"} +pallet-staking-reward-curve = { path = "../../../palette/staking/reward-curve"} sudo = { package = "pallet-sudo", path = "../../../palette/sudo", default-features = false } -support = { package = "pallet-support", path = "../../../palette/support", default-features = false } -system = { package = "pallet-system", path = "../../../palette/system", default-features = false } -system-rpc-runtime-api = { package = "pallet-system-rpc-runtime-api", path = "../../../palette/system/rpc/runtime-api/", default-features = false } +support = { package = "palette-support", path = "../../../palette/support", default-features = false } +system = { package = "palette-system", path = "../../../palette/system", default-features = false } +system-rpc-runtime-api = { package = "palette-system-rpc-runtime-api", path = "../../../palette/system/rpc/runtime-api/", default-features = false } timestamp = { package = "pallet-timestamp", path = "../../../palette/timestamp", default-features = false } treasury = { package = "pallet-treasury", path = "../../../palette/treasury", default-features = false } -utility = { package = "pallet-utility", path = "../../../palette/utility", default-features = false } +utility = { package = "palette-utility", path = "../../../palette/utility", default-features = false } transaction-payment = { package = "pallet-transaction-payment", path = "../../../palette/transaction-payment", default-features = false } transaction-payment-rpc-runtime-api = { package = "pallet-transaction-payment-rpc-runtime-api", path = "../../../palette/transaction-payment/rpc/runtime-api/", default-features = false } diff --git a/palette/staking/src/lib.rs b/palette/staking/src/lib.rs index ab17fa02dc83d..9dcfd84dc7a91 100644 --- a/palette/staking/src/lib.rs +++ b/palette/staking/src/lib.rs @@ -108,6 +108,8 @@ //! determined, a value is deducted from the balance of the validator and all the nominators who //! voted for this validator (values are deducted from the _stash_ account of the slashed entity). //! +//! Slashing logic is further described in the documentation of the `slashing` module. +//! //! Similar to slashing, rewards are also shared among a validator and its associated nominators. //! Yet, the reward funds are not always transferred to the stash account and can be configured. //! See [Reward Calculation](#reward-calculation) for more details. diff --git a/paint/staking/src/migration.rs b/palette/staking/src/migration.rs similarity index 100% rename from paint/staking/src/migration.rs rename to palette/staking/src/migration.rs diff --git a/paint/staking/src/slashing.rs b/palette/staking/src/slashing.rs similarity index 100% rename from paint/staking/src/slashing.rs rename to palette/staking/src/slashing.rs From aee412cbb76a0c9b13f39572f0f08637875b7c5c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 22 Nov 2019 15:59:19 +0100 Subject: [PATCH 42/46] update to use ensure-origin --- bin/node/runtime/src/lib.rs | 2 ++ palette/staking/src/lib.rs | 13 ++++++++++--- palette/staking/src/mock.rs | 1 + 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index be732828affdb..aa45ff4ae3c54 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -265,6 +265,8 @@ impl staking::Trait for Runtime { type SessionsPerEra = SessionsPerEra; type BondingDuration = BondingDuration; type SlashDeferDuration = SlashDeferDuration; + /// A super-majority of the council can cancel the slash. + type SlashCancelOrigin = collective::EnsureProportionAtLeast<_3, _4, AccountId, CouncilCollective>; type SessionInterface = Self; type RewardCurve = RewardCurve; } diff --git a/palette/staking/src/lib.rs b/palette/staking/src/lib.rs index 9dcfd84dc7a91..a602b445bd024 100644 --- a/palette/staking/src/lib.rs +++ b/palette/staking/src/lib.rs @@ -272,7 +272,7 @@ use sr_primitives::{ curve::PiecewiseLinear, traits::{ Convert, Zero, One, StaticLookup, CheckedSub, Saturating, Bounded, SaturatedConversion, - SimpleArithmetic, + SimpleArithmetic, EnsureOrigin, } }; use sr_staking_primitives::{ @@ -605,6 +605,9 @@ pub trait Trait: system::Trait { /// applied immediately, without opportunity for intervention. type SlashDeferDuration: Get; + /// The origin which can cancel a deferred slash. Root can always do this. + type SlashCancelOrigin: EnsureOrigin; + /// Interface for interacting with a session module. type SessionInterface: self::SessionInterface; @@ -1146,7 +1149,8 @@ decl_module! { ForceEra::put(Forcing::ForceAlways); } - /// Cancel enactment of a deferred slash. Can only be called by the root origin, + /// Cancel enactment of a deferred slash. Can be called by either the root origin or + /// the `T::SlashCancelOrigin`. /// passing the era and indices of the slashes for that era to kill. /// /// # @@ -1154,7 +1158,10 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FreeOperational] fn cancel_deferred_slash(origin, era: EraIndex, slash_indices: Vec) { - ensure_root(origin)?; + T::SlashCancelOrigin::try_origin(origin) + .map(|_| ()) + .or_else(ensure_root) + .map_err(|_| "bad origin")?; let mut slash_indices = slash_indices; slash_indices.sort_unstable(); diff --git a/palette/staking/src/mock.rs b/palette/staking/src/mock.rs index 58d06efcbcdb8..f6068a477307c 100644 --- a/palette/staking/src/mock.rs +++ b/palette/staking/src/mock.rs @@ -211,6 +211,7 @@ impl Trait for Test { type Reward = (); type SessionsPerEra = SessionsPerEra; type SlashDeferDuration = SlashDeferDuration; + type SlashCancelOrigin = system::EnsureRoot; type BondingDuration = BondingDuration; type SessionInterface = Self; type RewardCurve = RewardCurve; From d578e2ef59b3f862e09508baf9cbe2ba09a6305c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 22 Nov 2019 22:11:36 +0100 Subject: [PATCH 43/46] Apply suggestions from code review use `ensure!` Co-Authored-By: Gavin Wood --- palette/staking/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/palette/staking/src/lib.rs b/palette/staking/src/lib.rs index a602b445bd024..4536e7f2f401f 100644 --- a/palette/staking/src/lib.rs +++ b/palette/staking/src/lib.rs @@ -1171,8 +1171,8 @@ decl_module! { let index = index as usize; // if `index` is not duplicate, `removed` must be <= index. - if removed > index { return Err("duplicate index") } - if index >= unapplied.len() { return Err("slash record index out of bounds") } + ensure!(removed <= index, "duplicate index"); + ensure!(index < unapplied.len(), "slash record index out of bounds"); unapplied.remove(index); } From a94d403a07212cc9f10b448d9b3ab6c423ba5349 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sat, 23 Nov 2019 01:53:28 +0100 Subject: [PATCH 44/46] fix multi-slash removal --- frame/staking/src/lib.rs | 4 +++ frame/staking/src/tests.rs | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 4536e7f2f401f..9fc1c96464f01 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1172,6 +1172,10 @@ decl_module! { // if `index` is not duplicate, `removed` must be <= index. ensure!(removed <= index, "duplicate index"); + + // all prior removals were from before this index, since the + // list is sorted. + let index = index - removed; ensure!(index < unapplied.len(), "slash record index out of bounds"); unapplied.remove(index); diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index a6e0e18cb8b16..2357f5511078b 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -2470,3 +2470,53 @@ fn remove_deferred() { assert_eq!(Balances::free_balance(&101), 2000 - actual_slash); }) } + +#[test] +fn remove_multi_deferred() { + ExtBuilder::default().slash_defer_duration(2).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(10)], + ); + + on_offence_now( + &[ + OffenceDetails { + offender: (21, Staking::stakers(&21)), + reporters: vec![], + } + ], + &[Perbill::from_percent(10)], + ); + + + on_offence_now( + &[ + OffenceDetails { + offender: (11, exposure.clone()), + reporters: vec![], + }, + ], + &[Perbill::from_percent(25)], + ); + + assert_eq!(::UnappliedSlashes::get(&1).len(), 3); + Staking::cancel_deferred_slash(Origin::ROOT, 1, vec![0, 2]).unwrap(); + + let slashes = ::UnappliedSlashes::get(&1); + assert_eq!(slashes.len(), 1); + assert_eq!(slashes[0].validator, 21); + }) +} From 4fc80f78aac2acad8997fb9cafc1df074645f9e0 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 27 Nov 2019 14:20:49 +0100 Subject: [PATCH 45/46] initialize storage version to current in genesis --- frame/staking/src/lib.rs | 2 ++ frame/staking/src/migration.rs | 13 +++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 9fc1c96464f01..5e4ad4c796158 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -764,6 +764,8 @@ decl_storage! { }, _ => Ok(()) }; } + + StorageVersion::put(migration::CURRENT_VERSION); }); } } diff --git a/frame/staking/src/migration.rs b/frame/staking/src/migration.rs index abd7a9bff371f..e89c6af1b9113 100644 --- a/frame/staking/src/migration.rs +++ b/frame/staking/src/migration.rs @@ -16,21 +16,22 @@ //! Storage migrations for srml-staking. +/// Indicator of a version of a storage layout. +pub type VersionNumber = u32; + +// the current expected version of the storage +pub const CURRENT_VERSION: VersionNumber = 1; + #[cfg(any(test, feature = "migrate"))] mod inner { use crate::{Store, Module, Trait}; use support::{StorageLinkedMap, StorageValue}; use rstd::vec::Vec; - - /// Indicator of a version of a storage layout. - pub type VersionNumber = u32; + use super::{CURRENT_VERSION, VersionNumber}; // the minimum supported version of the migration logic. const MIN_SUPPORTED_VERSION: VersionNumber = 0; - // the current expected version of the storage - const CURRENT_VERSION: VersionNumber = 1; - // migrate storage from v0 to v1. // // this upgrades the `Nominators` linked_map value type from `Vec` to From b7599d7851fb9ccd4001ca91a49ff6d6c36008ae Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 27 Nov 2019 14:23:11 +0100 Subject: [PATCH 46/46] add test for version initialization --- frame/staking/src/tests.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 2357f5511078b..b4e1aa846bc54 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -2520,3 +2520,10 @@ fn remove_multi_deferred() { assert_eq!(slashes[0].validator, 21); }) } + +#[test] +fn version_initialized() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(::StorageVersion::get(), crate::migration::CURRENT_VERSION); + }); +}