diff --git a/prdoc/pr_4311.prdoc b/prdoc/pr_4311.prdoc new file mode 100644 index 0000000000000000000000000000000000000000..cf32acaf00895995da73df90b7f6be1053d77ac9 --- /dev/null +++ b/prdoc/pr_4311.prdoc @@ -0,0 +1,9 @@ +title: Not allow reap stash for virtual stakers. + +doc: + - audience: Runtime Dev + description: | + Add guards to staking dispathables to prevent virtual stakers to be reaped. + +crates: +- name: pallet-staking diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 4eb24311ab341efdfd344dce1f54defadac7d4d1..5b2a55303e2c855a814f0d4f9705fb7d89b2767c 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -87,10 +87,12 @@ impl<T: Config> Pallet<T> { StakingLedger::<T>::paired_account(Stash(stash.clone())) } - /// Inspects and returns the corruption state of a ledger and bond, if any. + /// Inspects and returns the corruption state of a ledger and direct bond, if any. /// /// Note: all operations in this method access directly the `Bonded` and `Ledger` storage maps /// instead of using the [`StakingLedger`] API since the bond and/or ledger may be corrupted. + /// It is also meant to check state for direct bonds and may not work as expected for virtual + /// bonds. pub(crate) fn inspect_bond_state( stash: &T::AccountId, ) -> Result<LedgerIntegrityState, Error<T>> { diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 9c968d88344413a250dead39fe9500029a3b5372..16ad510c562bff4590fa1b7ebc7b3daf3145a2a4 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -868,6 +868,8 @@ pub mod pallet { RewardDestinationRestricted, /// Not enough funds available to withdraw. NotEnoughFunds, + /// Operation not allowed for virtual stakers. + VirtualStakerNotAllowed, } #[pallet::hooks] @@ -1634,6 +1636,9 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; + // virtual stakers should not be allowed to be reaped. + ensure!(!Self::is_virtual_staker(&stash), Error::<T>::VirtualStakerNotAllowed); + let ed = T::Currency::minimum_balance(); let reapable = T::Currency::total_balance(&stash) < ed || Self::ledger(Stash(stash.clone())).map(|l| l.total).unwrap_or_default() < ed; @@ -1994,6 +1999,9 @@ pub mod pallet { ) -> DispatchResult { T::AdminOrigin::ensure_origin(origin)?; + // cannot restore ledger for virtual stakers. + ensure!(!Self::is_virtual_staker(&stash), Error::<T>::VirtualStakerNotAllowed); + let current_lock = T::Currency::balance_locked(crate::STAKING_ID, &stash); let stash_balance = T::Currency::free_balance(&stash); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index d05752f54be7c568e433e7f8c0ae2aa3770879b4..3cb51604aa6bb1fc7abe06af069d666e70b223c6 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7205,6 +7205,99 @@ mod staking_unchecked { assert_eq!(SlashObserver::get().get(&101).unwrap(), &nominator_share); }) } + + #[test] + fn virtual_stakers_cannot_be_reaped() { + ExtBuilder::default() + // we need enough validators such that disables are allowed. + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .build_and_execute(|| { + // make 101 only nominate 11. + assert_ok!(Staking::nominate(RuntimeOrigin::signed(101), vec![11])); + + mock::start_active_era(1); + + // slash all stake. + let slash_percent = Perbill::from_percent(100); + let initial_exposure = Staking::eras_stakers(active_era(), &11); + // 101 is a nominator for 11 + assert_eq!(initial_exposure.others.first().unwrap().who, 101); + // make 101 a virtual nominator + <Staking as StakingUnchecked>::migrate_to_virtual_staker(&101); + // set payee different to self. + assert_ok!(<Staking as StakingInterface>::update_payee(&101, &102)); + + // cache values + let validator_balance = Balances::free_balance(&11); + let validator_stake = Staking::ledger(11.into()).unwrap().total; + let nominator_balance = Balances::free_balance(&101); + let nominator_stake = Staking::ledger(101.into()).unwrap().total; + + // 11 goes offline + on_offence_now( + &[OffenceDetails { + offender: (11, initial_exposure.clone()), + reporters: vec![], + }], + &[slash_percent], + ); + + // both stakes must have been decreased to 0. + assert_eq!(Staking::ledger(101.into()).unwrap().active, 0); + assert_eq!(Staking::ledger(11.into()).unwrap().active, 0); + + // all validator stake is slashed + assert_eq_error_rate!( + validator_balance - validator_stake, + Balances::free_balance(&11), + 1 + ); + // Because slashing happened. + assert!(is_disabled(11)); + + // Virtual nominator's balance is not slashed. + assert_eq!(Balances::free_balance(&101), nominator_balance); + // Slash is broadcasted to slash observers. + assert_eq!(SlashObserver::get().get(&101).unwrap(), &nominator_stake); + + // validator can be reaped. + assert_ok!(Staking::reap_stash(RuntimeOrigin::signed(10), 11, u32::MAX)); + // nominator is a virtual staker and cannot be reaped. + assert_noop!( + Staking::reap_stash(RuntimeOrigin::signed(10), 101, u32::MAX), + Error::<Test>::VirtualStakerNotAllowed + ); + }) + } + + #[test] + fn restore_ledger_not_allowed_for_virtual_stakers() { + ExtBuilder::default().has_stakers(true).build_and_execute(|| { + setup_double_bonded_ledgers(); + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + // 333 is corrupted + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); + // migrate to virtual staker. + <Staking as StakingUnchecked>::migrate_to_virtual_staker(&333); + + // recover the ledger won't work for virtual staker + assert_noop!( + Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None), + Error::<Test>::VirtualStakerNotAllowed + ); + + // migrate 333 back to normal staker + <VirtualStakers<Test>>::remove(333); + + // try restore again + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + }) + } } mod ledger { use super::*;