diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 50c033bdc7e28c4760278518ec5b048105c893ff..3fff6312f333f0c66efce79554f7a7dd3a58377c 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -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 { diff --git a/substrate/frame/staking/src/migrations.rs b/substrate/frame/staking/src/migrations.rs index 14846da8a5d54e41e5fa2b1da9c885e8bdd1dc67..101cac0a31348e0ae42925c0ec49eefcc62ece81 100644 --- a/substrate/frame/staking/src/migrations.rs +++ b/substrate/frame/staking/src/migrations.rs @@ -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::*; diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index bd2d8cdc32ce9e10d71f8fa7c422b69d9b70ea7a..70d00c2b648d8cb04132ea312e5205137a129870 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -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); @@ -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)) } diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 7656eec80a5ffaeecba56a8e5b0866b36e991cf2..68aa97db8a324236684ac4c2205671fd63110e74 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -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::{ @@ -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. @@ -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(); @@ -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 { diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index e53464195de230b41c358f694637dbc16c24d09d..c999020c281673c1abab95095e6ee35521a7c20c 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -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`]. diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index b76126f0c5d04c11bacb243a0bc9b9943c8baac2..d14d8c4a75f2e95d81c2642d10ec17b3585093b2 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -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); }) } @@ -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)], @@ -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)], @@ -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); @@ -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); @@ -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!( @@ -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);