Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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