diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 80630818de7e6..e312aedbec1f3 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -28,7 +28,7 @@ use frame_support::{ traits::{Currency, CurrencyToVote, Get, Imbalance}, }; use sp_runtime::{ - traits::{StaticLookup, Zero}, + traits::{Bounded, One, StaticLookup, Zero}, Perbill, Percent, }; use sp_staking::SessionIndex; @@ -38,7 +38,6 @@ pub use frame_benchmarking::{ account, benchmarks, impl_benchmark_test_suite, whitelist_account, whitelisted_caller, }; use frame_system::RawOrigin; -use sp_runtime::traits::{Bounded, One}; const SEED: u32 = 0; const MAX_SPANS: u32 = 100; @@ -695,7 +694,7 @@ benchmarks! { let stash = scenario.origin_stash1.clone(); add_slashing_spans::(&stash, s); - T::Currency::make_free_balance_be(&stash, T::Currency::minimum_balance()); + Ledger::::insert(&controller, StakingLedger { active: T::Currency::minimum_balance() - One::one(), total: T::Currency::minimum_balance() - One::one(), ..Default::default() }); assert!(Bonded::::contains_key(&stash)); assert!(T::SortedListProvider::contains(&stash)); diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 8e97a90e07544..197c2eed325a1 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -1425,33 +1425,37 @@ pub mod pallet { Ok(()) } - /// Remove all data structure concerning a staker/stash once its balance is at the minimum. - /// This is essentially equivalent to `withdraw_unbonded` except it can be called by anyone - /// and the target `stash` must have no funds left beyond the ED. + /// Remove all data structures concerning a staker/stash once it is at a state where it can + /// be considered `dust` in the staking system. The requirements are: /// - /// This can be called from any origin. + /// 1. the `total_balance` of the stash is below existential deposit. + /// 2. or, the `ledger.total` of the stash is below existential deposit. /// - /// - `stash`: The stash account to reap. Its balance must be zero. + /// The former can happen in cases like a slash; the latter when a fully unbonded account + /// is still receiving staking rewards in `RewardDestination::Staked`. /// - /// # - /// Complexity: O(S) where S is the number of slashing spans on the account. - /// DB Weight: - /// - Reads: Stash Account, Bonded, Slashing Spans, Locks - /// - Writes: Bonded, Slashing Spans (if S > 0), Ledger, Payee, Validators, Nominators, - /// Stash Account, Locks - /// - Writes Each: SpanSlash * S - /// # + /// It can be called by anyone, as long as `stash` meets the above requirements. + /// + /// Refunds the transaction fees upon successful execution. #[pallet::weight(T::WeightInfo::reap_stash(*num_slashing_spans))] pub fn reap_stash( - _origin: OriginFor, + origin: OriginFor, stash: T::AccountId, num_slashing_spans: u32, - ) -> DispatchResult { - let at_minimum = T::Currency::total_balance(&stash) == T::Currency::minimum_balance(); - ensure!(at_minimum, Error::::FundedTarget); + ) -> DispatchResultWithPostInfo { + let _ = ensure_signed(origin)?; + + let ed = T::Currency::minimum_balance(); + let reapable = T::Currency::total_balance(&stash) < ed || + Self::ledger(Self::bonded(stash.clone()).ok_or(Error::::NotStash)?) + .map(|l| l.total) + .unwrap_or_default() < ed; + ensure!(reapable, Error::::FundedTarget); + Self::kill_stash(&stash, num_slashing_spans)?; T::Currency::remove_lock(STAKING_ID, &stash); - Ok(()) + + Ok(Pays::No.into()) } /// Remove the given nominations from the calling validator. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index d6d92d5bd57fc..8e8a7ee636d8d 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -1633,115 +1633,49 @@ fn reward_to_stake_works() { } #[test] -fn on_free_balance_zero_stash_removes_validator() { - // Tests that validator storage items are cleaned up when stash is empty - // Tests that storage items are untouched when controller is empty +fn reap_stash_works() { ExtBuilder::default() .existential_deposit(10) .balance_factor(10) .build_and_execute(|| { - // Check the balance of the validator account + // given assert_eq!(Balances::free_balance(10), 10); - // Check the balance of the stash account - assert_eq!(Balances::free_balance(11), 10 * 1000); - // Check these two accounts are bonded - assert_eq!(Staking::bonded(&11), Some(10)); - - // Set payee information - assert_ok!(Staking::set_payee(Origin::signed(10), RewardDestination::Stash)); - - // Check storage items that should be cleaned up - assert!(>::contains_key(&10)); - assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); - - // Reduce free_balance of controller to 0 - let _ = Balances::slash(&10, Balance::max_value()); - - // Check the balance of the stash account has not been touched assert_eq!(Balances::free_balance(11), 10 * 1000); - // Check these two accounts are still bonded assert_eq!(Staking::bonded(&11), Some(10)); - // Check storage items have not changed assert!(>::contains_key(&10)); assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); - // Reduce free_balance of stash to 0 - let _ = Balances::slash(&11, Balance::max_value()); - // Check total balance of stash - assert_eq!(Balances::total_balance(&11), 10); - - // Reap the stash - assert_ok!(Staking::reap_stash(Origin::none(), 11, 0)); - - // Check storage items do not exist - assert!(!>::contains_key(&10)); - assert!(!>::contains_key(&11)); - assert!(!>::contains_key(&11)); - assert!(!>::contains_key(&11)); - assert!(!>::contains_key(&11)); - }); -} - -#[test] -fn on_free_balance_zero_stash_removes_nominator() { - // Tests that nominator storage items are cleaned up when stash is empty - // Tests that storage items are untouched when controller is empty - ExtBuilder::default() - .existential_deposit(10) - .balance_factor(10) - .build_and_execute(|| { - // Make 10 a nominator - assert_ok!(Staking::nominate(Origin::signed(10), vec![20])); - // Check that account 10 is a nominator - assert!(>::contains_key(11)); - // Check the balance of the nominator account - assert_eq!(Balances::free_balance(10), 10); - // Check the balance of the stash account - assert_eq!(Balances::free_balance(11), 10_000); - - // Set payee information - assert_ok!(Staking::set_payee(Origin::signed(10), RewardDestination::Stash)); - - // Check storage items that should be cleaned up - assert!(>::contains_key(&10)); - assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); - - // Reduce free_balance of controller to 0 - let _ = Balances::slash(&10, Balance::max_value()); - // Check total balance of account 10 - assert_eq!(Balances::total_balance(&10), 0); - - // Check the balance of the stash account has not been touched - assert_eq!(Balances::free_balance(11), 10_000); - // Check these two accounts are still bonded - assert_eq!(Staking::bonded(&11), Some(10)); - - // Check storage items have not changed - assert!(>::contains_key(&10)); - assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); + // stash is not reapable + assert_noop!( + Staking::reap_stash(Origin::signed(20), 11, 0), + Error::::FundedTarget + ); + // controller or any other account is not reapable + assert_noop!(Staking::reap_stash(Origin::signed(20), 10, 0), Error::::NotStash); - // Reduce free_balance of stash to 0 - let _ = Balances::slash(&11, Balance::max_value()); - // Check total balance of stash - assert_eq!(Balances::total_balance(&11), 10); + // no easy way to cause an account to go below ED, we tweak their staking ledger + // instead. + Ledger::::insert( + 10, + StakingLedger { + stash: 11, + total: 5, + active: 5, + unlocking: vec![], + claimed_rewards: vec![], + }, + ); - // Reap the stash - assert_ok!(Staking::reap_stash(Origin::none(), 11, 0)); + // reap-able + assert_ok!(Staking::reap_stash(Origin::signed(20), 11, 0)); - // Check storage items do not exist + // then assert!(!>::contains_key(&10)); assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); - assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); }); } @@ -2556,10 +2490,10 @@ fn garbage_collection_after_slashing() { // reap_stash respects num_slashing_spans so that weight is accurate assert_noop!( - Staking::reap_stash(Origin::none(), 11, 0), + Staking::reap_stash(Origin::signed(20), 11, 0), Error::::IncorrectSlashingSpans ); - assert_ok!(Staking::reap_stash(Origin::none(), 11, 2)); + assert_ok!(Staking::reap_stash(Origin::signed(20), 11, 2)); assert!(::SlashingSpans::get(&11).is_none()); assert_eq!(::SpanSlash::get(&(11, 0)).amount_slashed(), &0);