Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Properly defer slashes (#11823)
Browse files Browse the repository at this point in the history
* initial draft of fixing slashing

* fix test

* Update frame/staking/src/tests.rs

Co-authored-by: Piotr Mikołajczyk <[email protected]>

* last touches

* add more detail about unbonding

* add migration

* fmt

Co-authored-by: Piotr Mikołajczyk <[email protected]>
Co-authored-by: parity-processbot <>
  • Loading branch information
kianenigma and pmikolajczyk41 authored Jul 26, 2022
1 parent 9428b0e commit fdfdc73
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 41 deletions.
11 changes: 6 additions & 5 deletions frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,11 +872,12 @@ enum Releases {
V2_0_0,
V3_0_0,
V4_0_0,
V5_0_0, // blockable validators.
V6_0_0, // removal of all storage associated with offchain phragmen.
V7_0_0, // keep track of number of nominators / validators in map
V8_0_0, // populate `VoterList`.
V9_0_0, // inject validators into `VoterList` as well.
V5_0_0, // blockable validators.
V6_0_0, // removal of all storage associated with offchain phragmen.
V7_0_0, // keep track of number of nominators / validators in map
V8_0_0, // populate `VoterList`.
V9_0_0, // inject validators into `VoterList` as well.
V10_0_0, // remove `EarliestUnappliedSlash`.
}

impl Default for Releases {
Expand Down
23 changes: 23 additions & 0 deletions frame/staking/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,29 @@ use super::*;
use frame_election_provider_support::SortedListProvider;
use frame_support::traits::OnRuntimeUpgrade;

pub mod v10 {
use super::*;
use frame_support::storage_alias;

#[storage_alias]
type EarliestUnappliedSlash<T: Config> = StorageValue<Pallet<T>, EraIndex>;

pub struct MigrateToV10<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToV10<T> {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
if StorageVersion::<T>::get() == Releases::V9_0_0 {
EarliestUnappliedSlash::<T>::kill();
StorageVersion::<T>::put(Releases::V10_0_0);

T::DbWeight::get().reads_writes(1, 1)
} else {
log!(warn, "MigrateToV10 should be removed.");
T::DbWeight::get().reads(1)
}
}
}
}

pub mod v9 {
use super::*;

Expand Down
15 changes: 15 additions & 0 deletions frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ impl ExtBuilder {
ext
}
pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
sp_tracing::try_init_simple();
let mut ext = self.build();
ext.execute_with(test);
ext.execute_with(post_conditions);
Expand Down Expand Up @@ -884,6 +885,20 @@ pub(crate) fn staking_events() -> Vec<crate::Event<Test>> {
.collect()
}

parameter_types! {
static StakingEventsIndex: usize = 0;
}

pub(crate) fn staking_events_since_last_call() -> Vec<crate::Event<Test>> {
let all: Vec<_> = System::events()
.into_iter()
.filter_map(|r| if let Event::Staking(inner) = r.event { Some(inner) } else { None })
.collect();
let seen = StakingEventsIndex::get();
StakingEventsIndex::set(all.len());
all.into_iter().skip(seen).collect()
}

pub(crate) fn balances(who: &AccountId) -> (Balance, Balance) {
(Balances::free_balance(who), Balances::reserved_balance(who))
}
47 changes: 24 additions & 23 deletions frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use frame_support::{
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use pallet_session::historical;
use sp_runtime::{
traits::{Bounded, Convert, SaturatedConversion, Saturating, StaticLookup, Zero},
traits::{Bounded, Convert, One, SaturatedConversion, Saturating, StaticLookup, Zero},
Perbill,
};
use sp_staking::{
Expand Down Expand Up @@ -599,20 +599,17 @@ impl<T: Config> Pallet<T> {

/// Apply previously-unapplied slashes on the beginning of a new era, after a delay.
fn apply_unapplied_slashes(active_era: EraIndex) {
let slash_defer_duration = T::SlashDeferDuration::get();
<Self as Store>::EarliestUnappliedSlash::mutate(|earliest| {
if let Some(ref mut earliest) = earliest {
let keep_from = active_era.saturating_sub(slash_defer_duration);
for era in (*earliest)..keep_from {
let era_slashes = <Self as Store>::UnappliedSlashes::take(&era);
for slash in era_slashes {
slashing::apply_slash::<T>(slash, era);
}
}

*earliest = (*earliest).max(keep_from)
}
})
let era_slashes = <Self as Store>::UnappliedSlashes::take(&active_era);
log!(
debug,
"found {} slashes scheduled to be executed in era {:?}",
era_slashes.len(),
active_era,
);
for slash in era_slashes {
let slash_era = active_era.saturating_sub(T::SlashDeferDuration::get());
slashing::apply_slash::<T>(slash, slash_era);
}
}

/// Add reward points to validators using their stash account ID.
Expand Down Expand Up @@ -1209,11 +1206,6 @@ where
}
};

<Self as Store>::EarliestUnappliedSlash::mutate(|earliest| {
if earliest.is_none() {
*earliest = Some(active_era)
}
});
add_db_reads_writes(1, 1);

let slash_defer_duration = T::SlashDeferDuration::get();
Expand Down Expand Up @@ -1263,9 +1255,18 @@ where
}
} else {
// Defer to end of some `slash_defer_duration` from now.
<Self as Store>::UnappliedSlashes::mutate(active_era, move |for_later| {
for_later.push(unapplied)
});
log!(
debug,
"deferring slash of {:?}% happened in {:?} (reported in {:?}) to {:?}",
slash_fraction,
slash_era,
active_era,
slash_era + slash_defer_duration + 1,
);
<Self as Store>::UnappliedSlashes::mutate(
slash_era.saturating_add(slash_defer_duration).saturating_add(One::one()),
move |for_later| for_later.push(unapplied),
);
add_db_reads_writes(1, 1);
}
} else {
Expand Down
4 changes: 0 additions & 4 deletions frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,10 +477,6 @@ pub mod pallet {
ValueQuery,
>;

/// The earliest era for which we have a pending, unapplied slash.
#[pallet::storage]
pub(crate) type EarliestUnappliedSlash<T> = StorageValue<_, EraIndex>;

/// The last planned session scheduled by the session pallet.
///
/// This is basically in sync with the call to [`pallet_session::SessionManager::new_session`].
Expand Down
118 changes: 109 additions & 9 deletions frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2777,12 +2777,103 @@ fn deferred_slashes_are_deferred() {
assert_eq!(Balances::free_balance(11), 1000);
assert_eq!(Balances::free_balance(101), 2000);

System::reset_events();

// at the start of era 4, slashes from era 1 are processed,
// after being deferred for at least 2 full eras.
mock::start_active_era(4);

assert_eq!(Balances::free_balance(11), 900);
assert_eq!(Balances::free_balance(101), 2000 - (nominated_value / 10));

assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid(3, 11075, 33225),
Event::Slashed(11, 100),
Event::Slashed(101, 12)
]
);
})
}

#[test]
fn retroactive_deferred_slashes_two_eras_before() {
ExtBuilder::default().slash_defer_duration(2).build_and_execute(|| {
assert_eq!(BondingDuration::get(), 3);

mock::start_active_era(1);
let exposure_11_at_era1 = Staking::eras_stakers(active_era(), 11);

mock::start_active_era(3);
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure_11_at_era1), reporters: vec![] }],
&[Perbill::from_percent(10)],
1, // should be deferred for two full eras, and applied at the beginning of era 4.
DisableStrategy::Never,
);
System::reset_events();

mock::start_active_era(4);

assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid(3, 7100, 21300),
Event::Slashed(11, 100),
Event::Slashed(101, 12)
]
);
})
}

#[test]
fn retroactive_deferred_slashes_one_before() {
ExtBuilder::default().slash_defer_duration(2).build_and_execute(|| {
assert_eq!(BondingDuration::get(), 3);

mock::start_active_era(1);
let exposure_11_at_era1 = Staking::eras_stakers(active_era(), 11);

// unbond at slash era.
mock::start_active_era(2);
assert_ok!(Staking::chill(Origin::signed(10)));
assert_ok!(Staking::unbond(Origin::signed(10), 100));

mock::start_active_era(3);
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure_11_at_era1), reporters: vec![] }],
&[Perbill::from_percent(10)],
2, // should be deferred for two full eras, and applied at the beginning of era 5.
DisableStrategy::Never,
);
System::reset_events();

mock::start_active_era(4);
assert_eq!(
staking_events_since_last_call(),
vec![Event::StakersElected, Event::EraPaid(3, 11075, 33225)]
);

assert_eq!(Staking::ledger(10).unwrap().total, 1000);
// slash happens after the next line.
mock::start_active_era(5);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid(4, 11075, 33225),
Event::Slashed(11, 100),
Event::Slashed(101, 12)
]
);

// their ledger has already been slashed.
assert_eq!(Staking::ledger(10).unwrap().total, 900);
assert_ok!(Staking::unbond(Origin::signed(10), 1000));
assert_eq!(Staking::ledger(10).unwrap().total, 900);
})
}

Expand Down Expand Up @@ -2871,6 +2962,7 @@ fn remove_deferred() {
assert_eq!(Balances::free_balance(101), 2000);
let nominated_value = exposure.others.iter().find(|o| o.who == 101).unwrap().value;

// deferred to start of era 4.
on_offence_now(
&[OffenceDetails { offender: (11, exposure.clone()), reporters: vec![] }],
&[Perbill::from_percent(10)],
Expand All @@ -2881,6 +2973,7 @@ fn remove_deferred() {

mock::start_active_era(2);

// reported later, but deferred to start of era 4 as well.
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure.clone()), reporters: vec![] }],
&[Perbill::from_percent(15)],
Expand All @@ -2894,7 +2987,8 @@ fn remove_deferred() {
Error::<Test>::EmptyTargets
);

assert_ok!(Staking::cancel_deferred_slash(Origin::root(), 1, vec![0]));
// cancel one of them.
assert_ok!(Staking::cancel_deferred_slash(Origin::root(), 4, vec![0]));

assert_eq!(Balances::free_balance(11), 1000);
assert_eq!(Balances::free_balance(101), 2000);
Expand All @@ -2906,13 +3000,19 @@ fn remove_deferred() {

// at the start of era 4, slashes from era 1 are processed,
// after being deferred for at least 2 full eras.
System::reset_events();
mock::start_active_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);

mock::start_active_era(5);
// the first slash for 10% was cancelled, but the 15% one
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid(3, 11075, 33225),
Event::Slashed(11, 50),
Event::Slashed(101, 7)
]
);

let slash_10 = Perbill::from_percent(10);
let slash_15 = Perbill::from_percent(15);
Expand Down Expand Up @@ -2965,7 +3065,7 @@ fn remove_multi_deferred() {
&[Perbill::from_percent(25)],
);

assert_eq!(<Staking as Store>::UnappliedSlashes::get(&1).len(), 5);
assert_eq!(<Staking as Store>::UnappliedSlashes::get(&4).len(), 5);

// fails if list is not sorted
assert_noop!(
Expand All @@ -2983,9 +3083,9 @@ fn remove_multi_deferred() {
Error::<Test>::InvalidSlashIndex
);

assert_ok!(Staking::cancel_deferred_slash(Origin::root(), 1, vec![0, 2, 4]));
assert_ok!(Staking::cancel_deferred_slash(Origin::root(), 4, vec![0, 2, 4]));

let slashes = <Staking as Store>::UnappliedSlashes::get(&1);
let slashes = <Staking as Store>::UnappliedSlashes::get(&4);
assert_eq!(slashes.len(), 2);
assert_eq!(slashes[0].validator, 21);
assert_eq!(slashes[1].validator, 42);
Expand Down

0 comments on commit fdfdc73

Please sign in to comment.