Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable2412] Backport #7685 #7738

Merged
merged 8 commits into from
Mar 3, 2025
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
15 changes: 15 additions & 0 deletions prdoc/pr_7738.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
title: 'Introduce filter to restrict accounts from staking'

doc:
- audience: Runtime Dev
description: |
Introduce an associated fn `filter` in pallet-staking to restrict some accounts from staking.
This is useful for restricting certain accounts from staking, for example, accounts staking via pools.

crates:
- name: pallet-staking
bump: patch
- name: pallet-nomination-pools
bump: patch
- name: pallet-delegated-staking
bump: patch
5 changes: 4 additions & 1 deletion substrate/frame/delegated-staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use frame_support::{
assert_ok, derive_impl,
pallet_prelude::*,
parameter_types,
traits::{ConstU64, Currency, VariantCountOf},
traits::{ConstU64, Contains, Currency, VariantCountOf},
PalletId,
};

Expand Down Expand Up @@ -111,6 +111,9 @@ impl pallet_staking::Config for Runtime {
type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
type TargetList = pallet_staking::UseValidatorsMap<Self>;
type EventListeners = (Pools, DelegatedStaking);
fn filter(who: &AccountId) -> bool {
pallet_nomination_pools::AllPoolMembers::<Runtime>::contains(who)
}
}

parameter_types! {
Expand Down
44 changes: 29 additions & 15 deletions substrate/frame/delegated-staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ mod pool_integration {
}

#[test]
fn existing_pool_member_can_stake() {
fn existing_pool_member_cannot_stake() {
// A pool member is able to stake directly since staking only uses free funds but once a
// staker, they cannot join/add extra bond to the pool. They can still withdraw funds.
ExtBuilder::default().build_and_execute(|| {
Expand All @@ -1375,28 +1375,42 @@ mod pool_integration {
fund(&delegator, 1000);
assert_ok!(Pools::join(RawOrigin::Signed(delegator).into(), 200, pool_id));

// THEN: they can still stake directly.
// THEN: they cannot stake anymore
assert_noop!(
Staking::bond(
RuntimeOrigin::signed(delegator),
500,
RewardDestination::Account(101)
),
StakingError::<T>::BoundNotMet
);
});
}

#[test]
fn stakers_cannot_join_pool() {
ExtBuilder::default().build_and_execute(|| {
start_era(1);
// GIVEN: a pool.
fund(&200, 1000);
let pool_id = create_pool(200, 800);

// WHEN: an account is a staker.
let staker = 100;
fund(&staker, 1000);

assert_ok!(Staking::bond(
RuntimeOrigin::signed(delegator),
RuntimeOrigin::signed(staker),
500,
RewardDestination::Account(101)
));
assert_ok!(Staking::nominate(
RuntimeOrigin::signed(delegator),
vec![GENESIS_VALIDATOR]
));
assert_ok!(Staking::nominate(RuntimeOrigin::signed(staker), vec![GENESIS_VALIDATOR]));

// The delegator cannot add any extra bond to the pool anymore.
// THEN: they cannot join pool.
assert_noop!(
Pools::bond_extra(RawOrigin::Signed(delegator).into(), BondExtra::FreeBalance(100)),
Pools::join(RawOrigin::Signed(staker).into(), 200, pool_id),
Error::<T>::AlreadyStaking
);

// But they can unbond
assert_ok!(Pools::unbond(RawOrigin::Signed(delegator).into(), delegator, 50));
// and withdraw
start_era(4);
assert_ok!(Pools::withdraw_unbonded(RawOrigin::Signed(delegator).into(), delegator, 0));
});
}

Expand Down
8 changes: 8 additions & 0 deletions substrate/frame/nomination-pools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4079,3 +4079,11 @@ impl<T: Config> sp_staking::OnStakingUpdate<T::AccountId, BalanceOf<T>> for Pall
}
}
}

/// A utility struct that provides a way to check if a given account is a pool member.
pub struct AllPoolMembers<T: Config>(PhantomData<T>);
impl<T: Config> frame_support::traits::Contains<T::AccountId> for AllPoolMembers<T> {
fn contains(t: &T::AccountId) -> bool {
PoolMembers::<T>::contains_key(t)
}
}
1 change: 1 addition & 0 deletions substrate/frame/staking/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ substrate-test-utils = { path = "../../test-utils" }
frame-benchmarking = { default-features = true, path = "../benchmarking" }
frame-election-provider-support = { default-features = true, path = "../election-provider-support" }
rand_chacha = { workspace = true, default-features = true }
frame-support = { features = ["experimental"], default-features = true, workspace = true }

[features]
default = ["std"]
Expand Down
67 changes: 66 additions & 1 deletion substrate/frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ use codec::{Decode, Encode, HasCompact, MaxEncodedLen};
use frame_support::{
defensive, defensive_assert,
traits::{
ConstU32, Currency, Defensive, DefensiveMax, DefensiveSaturating, Get, LockIdentifier,
ConstU32, Contains, Currency, Defensive, DefensiveMax, DefensiveSaturating, Get,
LockIdentifier,
},
weights::Weight,
BoundedVec, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
Expand Down Expand Up @@ -540,6 +541,52 @@ impl<T: Config> StakingLedger<T> {
}
}

/// Sets ledger total to the `new_total`.
///
/// Removes entries from `unlocking` upto `amount` starting from the oldest first.
fn update_total_stake(mut self, new_total: BalanceOf<T>) -> Self {
let old_total = self.total;
self.total = new_total;
debug_assert!(
new_total <= old_total,
"new_total {:?} must be <= old_total {:?}",
new_total,
old_total
);

let to_withdraw = old_total.defensive_saturating_sub(new_total);
// accumulator to keep track of how much is withdrawn.
// First we take out from active.
let mut withdrawn = BalanceOf::<T>::zero();

// first we try to remove stake from active
if self.active >= to_withdraw {
self.active -= to_withdraw;
return self
} else {
withdrawn += self.active;
self.active = BalanceOf::<T>::zero();
}

// start removing from the oldest chunk.
while let Some(last) = self.unlocking.last_mut() {
if withdrawn.defensive_saturating_add(last.value) <= to_withdraw {
withdrawn += last.value;
self.unlocking.pop();
} else {
let diff = to_withdraw.defensive_saturating_sub(withdrawn);
withdrawn += diff;
last.value -= diff;
}

if withdrawn >= to_withdraw {
break
}
}

self
}

/// Re-bond funds that were scheduled for unlocking.
///
/// Returns the updated ledger, and the amount actually rebonded.
Expand Down Expand Up @@ -1249,6 +1296,24 @@ impl<T: Config> EraInfo<T> {
}
}

/// A utility struct that provides a way to check if a given account is a staker.
///
/// This struct implements the `Contains` trait, allowing it to determine whether
/// a particular account is currently staking by checking if the account exists in
/// the staking ledger.
pub struct AllStakers<T: Config>(core::marker::PhantomData<T>);

impl<T: Config> Contains<T::AccountId> for AllStakers<T> {
/// Checks if the given account ID corresponds to a staker.
///
/// # Returns
/// - `true` if the account has an entry in the staking ledger (indicating it is staking).
/// - `false` otherwise.
fn contains(account: &T::AccountId) -> bool {
Ledger::<T>::contains_key(account)
}
}

/// Configurations of the benchmarking of the pallet.
pub trait BenchmarkingConfig {
/// The maximum number of validators to use.
Expand Down
18 changes: 17 additions & 1 deletion substrate/frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ parameter_types! {
(BalanceOf<Test>, BTreeMap<EraIndex, BalanceOf<Test>>) =
(Zero::zero(), BTreeMap::new());
pub static SlashObserver: BTreeMap<AccountId, BalanceOf<Test>> = BTreeMap::new();
pub static RestrictedAccounts: Vec<AccountId> = Vec::new();
}

pub struct EventListenerMock;
Expand All @@ -258,7 +259,8 @@ impl OnStakingUpdate<AccountId, Balance> for EventListenerMock {
}
}

// Disabling threshold for `UpToLimitDisablingStrategy`
// Disabling threshold for `UpToLimitDisablingStrategy` and
// `UpToLimitWithReEnablingDisablingStrategy``
pub(crate) const DISABLING_LIMIT_FACTOR: usize = 3;

#[derive_impl(crate::config_preludes::TestDefaultConfig)]
Expand All @@ -285,6 +287,10 @@ impl crate::pallet::pallet::Config for Test {
type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch;
type EventListeners = EventListenerMock;
type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy<DISABLING_LIMIT_FACTOR>;

fn filter(who: &AccountId) -> bool {
RestrictedAccounts::get().contains(who)
}
}

pub struct WeightedNominationsQuota<const MAX: u32>;
Expand Down Expand Up @@ -928,3 +934,13 @@ pub(crate) fn staking_events_since_last_call() -> Vec<crate::Event<Test>> {
pub(crate) fn balances(who: &AccountId) -> (Balance, Balance) {
(Balances::free_balance(who), Balances::reserved_balance(who))
}

pub(crate) fn restrict(who: &AccountId) {
if !RestrictedAccounts::get().contains(who) {
RestrictedAccounts::mutate(|l| l.push(*who));
}
}

pub(crate) fn remove_from_restrict_list(who: &AccountId) {
RestrictedAccounts::mutate(|l| l.retain(|x| x != who));
}
50 changes: 49 additions & 1 deletion substrate/frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use frame_election_provider_support::{
use frame_support::{
pallet_prelude::*,
traits::{
Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get,
Currency, Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get,
InspectLockableCurrency, LockableCurrency, OnUnbalanced, UnixTime,
},
weights::Weight,
Expand Down Expand Up @@ -306,6 +306,16 @@ pub mod pallet {

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

/// Determines whether a given account should be filtered out from staking operations.
///
/// This function provides a way to exclude certain accounts from bonding to staking. An
/// already bonded account is allowed to withdraw.
///
/// The default implementation does not filter out any accounts.
fn filter(_who: &Self::AccountId) -> bool {
false
}
}

/// Default implementations of [`DefaultConfig`], which can be used to implement [`Config`].
Expand Down Expand Up @@ -1018,6 +1028,8 @@ pub mod pallet {
) -> DispatchResult {
let stash = ensure_signed(origin)?;

ensure!(!T::filter(&stash), Error::<T>::BoundNotMet);

if StakingLedger::<T>::is_bonded(StakingAccount::Stash(stash.clone())) {
return Err(Error::<T>::AlreadyBonded.into())
}
Expand Down Expand Up @@ -1068,6 +1080,7 @@ pub mod pallet {
#[pallet::compact] max_additional: BalanceOf<T>,
) -> DispatchResult {
let stash = ensure_signed(origin)?;
ensure!(!T::filter(&stash), Error::<T>::BoundNotMet);
Self::do_bond_extra(&stash, max_additional)
}

Expand Down Expand Up @@ -1654,6 +1667,8 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
let controller = ensure_signed(origin)?;
let ledger = Self::ledger(Controller(controller))?;

ensure!(!T::filter(&ledger.stash), Error::<T>::BoundNotMet);
ensure!(!ledger.unlocking.is_empty(), Error::<T>::NoUnlockChunk);

let initial_unlocking = ledger.unlocking.len() as u32;
Expand Down Expand Up @@ -2151,6 +2166,39 @@ pub mod pallet {
);
Ok(())
}

/// Adjusts the staking ledger by withdrawing any excess staked amount.
///
/// This function corrects cases where a user's recorded stake in the ledger
/// exceeds their actual staked funds. This situation can arise due to cases such as
/// external slashing by another pallet, leading to an inconsistency between the ledger
/// and the actual stake.
#[pallet::call_index(32)]
#[pallet::weight(T::DbWeight::get().reads_writes(2, 1))]
pub fn withdraw_overstake(origin: OriginFor<T>, stash: T::AccountId) -> DispatchResult {
let _ = ensure_signed(origin)?;

// Virtual stakers are controlled by some other pallet.
ensure!(!Self::is_virtual_staker(&stash), Error::<T>::VirtualStakerNotAllowed);

let ledger = Self::ledger(Stash(stash.clone()))?;
let stash_balance = T::Currency::free_balance(&stash);

// Ensure there is an overstake.
ensure!(ledger.total > stash_balance, Error::<T>::BoundNotMet);

let force_withdraw_amount = ledger.total.defensive_saturating_sub(stash_balance);

// Update the ledger by withdrawing excess stake.
ledger.update_total_stake(stash_balance).update()?;

// Ensure lock is updated.
debug_assert_eq!(T::Currency::balance_locked(crate::STAKING_ID, &stash), stash_balance);

Self::deposit_event(Event::<T>::Withdrawn { stash, amount: force_withdraw_amount });

Ok(())
}
}
}

Expand Down
Loading
Loading