From 0031d49d1ec083c62a4e2b5bf594b7f45f84ab0d Mon Sep 17 00:00:00 2001
From: Ankan <10196091+Ank4n@users.noreply.github.com>
Date: Mon, 29 Apr 2024 17:55:45 +0200
Subject: [PATCH] [Staking] Not allow reap stash for virtual stakers (#4311)
Related to https://github.com/paritytech/polkadot-sdk/pull/3905.
Since virtual stakers does not have a real balance, they should not be
allowed to be reaped.
The proper reaping for agents slashed will be done in a separate PR.
---
prdoc/pr_4311.prdoc | 9 ++
substrate/frame/staking/src/pallet/impls.rs | 4 +-
substrate/frame/staking/src/pallet/mod.rs | 8 ++
substrate/frame/staking/src/tests.rs | 93 +++++++++++++++++++++
4 files changed, 113 insertions(+), 1 deletion(-)
create mode 100644 prdoc/pr_4311.prdoc
diff --git a/prdoc/pr_4311.prdoc b/prdoc/pr_4311.prdoc
new file mode 100644
index 00000000000..cf32acaf008
--- /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 4eb24311ab3..5b2a55303e2 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 9c968d88344..16ad510c562 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 d05752f54be..3cb51604aa6 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::*;
--
GitLab