Skip to content

Commit

Permalink
[Staking] Not allow reap stash for virtual stakers (#4311)
Browse files Browse the repository at this point in the history
Related to #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.
  • Loading branch information
Ank4n authored Apr 29, 2024
1 parent f34d8e3 commit 0031d49
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 1 deletion.
9 changes: 9 additions & 0 deletions prdoc/pr_4311.prdoc
Original file line number Diff line number Diff line change
@@ -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
4 changes: 3 additions & 1 deletion substrate/frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down
8 changes: 8 additions & 0 deletions substrate/frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,8 @@ pub mod pallet {
RewardDestinationRestricted,
/// Not enough funds available to withdraw.
NotEnoughFunds,
/// Operation not allowed for virtual stakers.
VirtualStakerNotAllowed,
}

#[pallet::hooks]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
93 changes: 93 additions & 0 deletions substrate/frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down

0 comments on commit 0031d49

Please sign in to comment.