diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index fc489e3bc685e1dd4666fa5d702cfee3313084e5..117aa849b133f5dc16dc9e69046931a246cb729d 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -405,6 +405,7 @@ impl pallet_staking::Config for Runtime { type MaxValidatorSet = MaxAuthorities; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type Filter = frame_support::traits::Nothing; } parameter_types! { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index ddc26f4e645b67f0d760c328baef84aca8707159..741957d84737778b65aadfd71f9e46b6db5ff96a 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -37,7 +37,7 @@ use frame_support::{ traits::{ fungible::HoldConsideration, tokens::UnityOrOuterConversion, ConstU32, Contains, EitherOf, EitherOfDiverse, EnsureOriginWithArg, EverythingBut, FromContains, InstanceFilter, - KeyOwnerProofSystem, LinearStoragePrice, ProcessMessage, ProcessMessageError, + KeyOwnerProofSystem, LinearStoragePrice, Nothing, ProcessMessage, ProcessMessageError, VariantCountOf, WithdrawReasons, }, weights::{ConstantMultiplier, WeightMeter, WeightToFee as _}, @@ -772,6 +772,7 @@ impl pallet_staking::Config for Runtime { type WeightInfo = weights::pallet_staking::WeightInfo<Runtime>; type MaxInvulnerables = frame_support::traits::ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type Filter = Nothing; } impl pallet_fast_unstake::Config for Runtime { @@ -1547,6 +1548,7 @@ impl pallet_nomination_pools::Config for Runtime { type MaxPointsToBalance = MaxPointsToBalance; type AdminOrigin = EitherOf<EnsureRoot<AccountId>, StakingAdmin>; type BlockNumberProvider = System; + type Filter = Nothing; } parameter_types! { diff --git a/prdoc/pr_7685.prdoc b/prdoc/pr_7685.prdoc new file mode 100644 index 0000000000000000000000000000000000000000..23cb7b09297e9c846fe4e8d041dcd61aa1c42521 --- /dev/null +++ b/prdoc/pr_7685.prdoc @@ -0,0 +1,20 @@ +title: 'Introduce filters to restrict accounts from staking' + +doc: + - audience: Runtime Dev + description: | + Introduce filters to restrict accounts from staking. + This is useful for restricting certain accounts from staking, for example, accounts staking via pools, and vice + versa. + +crates: + - name: pallet-staking + bump: minor + - name: pallet-nomination-pools + bump: minor + - name: westend-runtime + bump: patch + - name: pallet-delegated-staking + bump: patch + - name: pallet-nomination-pools-benchmarking + bump: patch diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 7e33f1feeca19e3f516b79dc1703c1d126229020..22e266cc1867853a58241cb59560b70b01b079f3 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -879,6 +879,7 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = StakingBenchmarkingConfig; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type Filter = Nothing; } impl pallet_fast_unstake::Config for Runtime { @@ -1236,6 +1237,7 @@ impl pallet_nomination_pools::Config for Runtime { pallet_collective::EnsureProportionAtLeast<AccountId, CouncilCollective, 3, 4>, >; type BlockNumberProvider = System; + type Filter = Nothing; } parameter_types! { diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index fadc8d290d6f9e7ff05e718b57e0bdaa5e685e4a..6df5a6b3eb569c2c75833330f50c7e28f9cf3185 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -308,9 +308,6 @@ pub mod pallet { // Existing `agent` cannot register again and a delegator cannot become an `agent`. ensure!(!Self::is_agent(&who) && !Self::is_delegator(&who), Error::<T>::NotAllowed); - // They cannot be already a direct staker in the staking pallet. - ensure!(!Self::is_direct_staker(&who), Error::<T>::AlreadyStaking); - // Reward account cannot be same as `agent` account. ensure!(reward_account != who, Error::<T>::InvalidRewardDestination); @@ -407,7 +404,6 @@ pub mod pallet { // Ensure delegator is sane. ensure!(!Self::is_agent(&delegator), Error::<T>::NotAllowed); ensure!(!Self::is_delegator(&delegator), Error::<T>::NotAllowed); - ensure!(!Self::is_direct_staker(&delegator), Error::<T>::AlreadyStaking); // ensure agent is sane. ensure!(Self::is_agent(&agent), Error::<T>::NotAgent); @@ -442,12 +438,6 @@ pub mod pallet { Error::<T>::InvalidDelegation ); - // Implementation note: Staking uses deprecated locks (similar to freeze) which are not - // mutually exclusive of holds. This means, if we allow delegating for existing stakers, - // already staked funds might be reused for delegation. We avoid that by just blocking - // this. - ensure!(!Self::is_direct_staker(&delegator), Error::<T>::AlreadyStaking); - // ensure agent is sane. ensure!(Self::is_agent(&agent), Error::<T>::NotAgent); diff --git a/substrate/frame/delegated-staking/src/mock.rs b/substrate/frame/delegated-staking/src/mock.rs index 003d3380f66815a19ecafd96d71359d3a6c9e8e5..44068ee5a7f31d5428ab47f0ccc670620fc28324 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -114,6 +114,7 @@ impl pallet_staking::Config for Runtime { type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>; type TargetList = pallet_staking::UseValidatorsMap<Self>; type EventListeners = (Pools, DelegatedStaking); + type Filter = pallet_nomination_pools::AllPoolMembers<Self>; } parameter_types! { @@ -164,6 +165,7 @@ impl pallet_nomination_pools::Config for Runtime { pallet_nomination_pools::adapter::DelegateStake<Self, Staking, DelegatedStaking>; type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>; type BlockNumberProvider = System; + type Filter = pallet_staking::AllStakers<Runtime>; } frame_support::construct_runtime!( diff --git a/substrate/frame/delegated-staking/src/tests.rs b/substrate/frame/delegated-staking/src/tests.rs index c764e2741a2a4a0149853f2279bd8b556ba4b20b..05961d6f824db93329632a7c5c687fc4cdb33ff7 100644 --- a/substrate/frame/delegated-staking/src/tests.rs +++ b/substrate/frame/delegated-staking/src/tests.rs @@ -65,31 +65,6 @@ fn cannot_become_agent() { DelegatedStaking::register_agent(RawOrigin::Signed(100).into(), 100), Error::<T>::InvalidRewardDestination ); - - // an existing validator cannot become agent - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(mock::GENESIS_VALIDATOR).into(), - 100 - ), - Error::<T>::AlreadyStaking - ); - - // an existing direct staker to `CoreStaking` cannot become an agent. - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(mock::GENESIS_NOMINATOR_ONE).into(), - 100 - ), - Error::<T>::AlreadyStaking - ); - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(mock::GENESIS_NOMINATOR_TWO).into(), - 100 - ), - Error::<T>::AlreadyStaking - ); }); } @@ -637,18 +612,6 @@ mod staking_integration { DelegatedStaking::register_agent(RawOrigin::Signed(202).into(), 203), Error::<T>::NotAllowed ); - // existing staker cannot become a delegate - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(GENESIS_NOMINATOR_ONE).into(), - 201 - ), - Error::<T>::AlreadyStaking - ); - assert_noop!( - DelegatedStaking::register_agent(RawOrigin::Signed(GENESIS_VALIDATOR).into(), 201), - Error::<T>::AlreadyStaking - ); }); } @@ -1362,7 +1325,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(|| { @@ -1376,28 +1339,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>::Restricted + ); + }); + } + + #[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)), - Error::<T>::AlreadyStaking + Pools::join(RawOrigin::Signed(staker).into(), 200, pool_id), + PoolsError::<T>::Restricted ); - - // 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)); }); } diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index 120deff96a75eb0d297efc9bc017b2946f39d86c..aa9b314d0068e9fc78a619cafcba8a5a638a6d38 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -54,9 +54,8 @@ use pallet_staking::{ActiveEra, CurrentEra, ErasStartSessionIndex, StakerStatus} use parking_lot::RwLock; use std::sync::Arc; -use frame_support::derive_impl; - use crate::{log, log_current_time}; +use frame_support::{derive_impl, traits::Nothing}; pub const INIT_TIMESTAMP: BlockNumber = 30_000; pub const BLOCK_TIME: BlockNumber = 1000; @@ -290,6 +289,7 @@ impl pallet_nomination_pools::Config for Runtime { type MaxPointsToBalance = frame_support::traits::ConstU8<10>; type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>; type BlockNumberProvider = System; + type Filter = Nothing; } parameter_types! { diff --git a/substrate/frame/nomination-pools/benchmarking/src/mock.rs b/substrate/frame/nomination-pools/benchmarking/src/mock.rs index 2e73ad7cf4fc000034e94ee11e24a84dc43fe7e9..1dcfb86b75cf6c798cbc7ffc6afe0330f16e8a8d 100644 --- a/substrate/frame/nomination-pools/benchmarking/src/mock.rs +++ b/substrate/frame/nomination-pools/benchmarking/src/mock.rs @@ -21,7 +21,7 @@ use frame_support::{ derive_impl, pallet_prelude::*, parameter_types, - traits::{ConstU64, VariantCountOf}, + traits::{ConstU64, Nothing, VariantCountOf}, PalletId, }; use sp_runtime::{ @@ -141,6 +141,7 @@ impl pallet_nomination_pools::Config for Runtime { type MaxPointsToBalance = MaxPointsToBalance; type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>; type BlockNumberProvider = System; + type Filter = Nothing; } parameter_types! { diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 39a7cf05b3abadff82aeb8ab8b34d9918eccbb23..a62c4223b3b1913beeede8c45627684b9a14e180 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -364,7 +364,7 @@ use frame_support::{ traits::{ fungible::{Inspect, InspectFreeze, Mutate, MutateFreeze}, tokens::{Fortitude, Preservation}, - Defensive, DefensiveOption, DefensiveResult, DefensiveSaturating, Get, + Contains, Defensive, DefensiveOption, DefensiveResult, DefensiveSaturating, Get, }, DefaultNoBound, PalletError, }; @@ -1729,6 +1729,9 @@ pub mod pallet { /// Provider for the block number. Normally this is the `frame_system` pallet. type BlockNumberProvider: BlockNumberProvider; + + /// Restrict some accounts from participating in a nomination pool. + type Filter: Contains<Self::AccountId>; } /// The sum of funds across all pools. @@ -2049,6 +2052,9 @@ pub mod pallet { NotMigrated, /// This call is not allowed in the current state of the pallet. NotSupported, + /// Account is restricted from participation in pools. This may happen if the account is + /// staking in another way already. + Restricted, } #[derive( @@ -2088,8 +2094,9 @@ pub mod pallet { #[pallet::call] impl<T: Config> Pallet<T> { - /// Stake funds with a pool. The amount to bond is transferred from the member to the pool - /// account and immediately increases the pools bond. + /// Stake funds with a pool. The amount to bond is delegated (or transferred based on + /// [`adapter::StakeStrategyType`]) from the member to the pool account and immediately + /// increases the pool's bond. /// /// The method of transferring the amount to the pool account is determined by /// [`adapter::StakeStrategyType`]. If the pool is configured to use @@ -2114,6 +2121,9 @@ pub mod pallet { // ensure pool is not in an un-migrated state. ensure!(!Self::api_pool_needs_delegate_migration(pool_id), Error::<T>::NotMigrated); + // ensure account is not restricted from joining the pool. + ensure!(!T::Filter::contains(&who), Error::<T>::Restricted); + ensure!(amount >= MinJoinBond::<T>::get(), Error::<T>::MinimumBondNotMet); // If a member already exists that means they already belong to a pool ensure!(!PoolMembers::<T>::contains_key(&who), Error::<T>::AccountBelongsToOtherPool); @@ -3148,12 +3158,16 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let _caller = ensure_signed(origin)?; + // ensure `DelegateStake` strategy is used. ensure!( T::StakeAdapter::strategy_type() == adapter::StakeStrategyType::Delegate, Error::<T>::NotSupported ); + // ensure member is not restricted from joining the pool. let member_account = T::Lookup::lookup(member_account)?; + ensure!(!T::Filter::contains(&member_account), Error::<T>::Restricted); + let member = PoolMembers::<T>::get(&member_account).ok_or(Error::<T>::PoolMemberNotFound)?; @@ -3482,6 +3496,9 @@ impl<T: Config> Pallet<T> { bouncer: AccountIdLookupOf<T>, pool_id: PoolId, ) -> DispatchResult { + // ensure depositor is not restricted from joining the pool. + ensure!(!T::Filter::contains(&who), Error::<T>::Restricted); + let root = T::Lookup::lookup(root)?; let nominator = T::Lookup::lookup(nominator)?; let bouncer = T::Lookup::lookup(bouncer)?; @@ -3555,6 +3572,9 @@ impl<T: Config> Pallet<T> { member_account: T::AccountId, extra: BondExtra<BalanceOf<T>>, ) -> DispatchResult { + // ensure account is not restricted from joining the pool. + ensure!(!T::Filter::contains(&member_account), Error::<T>::Restricted); + if signer != member_account { ensure!( ClaimPermissions::<T>::get(&member_account).can_bond_extra(), @@ -4224,3 +4244,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> Contains<T::AccountId> for AllPoolMembers<T> { + fn contains(t: &T::AccountId) -> bool { + PoolMembers::<T>::contains_key(t) + } +} diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs index 88ef82a1562001b82c5676a262f0795c0575baf1..b97d98d66948e0202101cd7f7f4596d132c15968 100644 --- a/substrate/frame/nomination-pools/src/mock.rs +++ b/substrate/frame/nomination-pools/src/mock.rs @@ -58,6 +58,7 @@ parameter_types! { pub static MaxUnbonding: u32 = 8; pub static StakingMinBond: Balance = 10; pub storage Nominations: Option<Vec<AccountId>> = None; + pub static RestrictedAccounts: Vec<AccountId> = Vec::new(); } pub struct StakingMock; @@ -451,6 +452,13 @@ impl Convert<U256, Balance> for U256ToBalance { } } +pub struct RestrictMock; +impl Contains<AccountId> for RestrictMock { + fn contains(who: &AccountId) -> bool { + RestrictedAccounts::get().contains(who) + } +} + parameter_types! { pub static PostUnbondingPoolsWindow: u32 = 2; pub static MaxMetadataLen: u32 = 2; @@ -478,6 +486,7 @@ impl pools::Config for Runtime { type MaxPointsToBalance = frame_support::traits::ConstU8<10>; type AdminOrigin = EnsureSignedBy<Admin, AccountId>; type BlockNumberProvider = System; + type Filter = RestrictMock; } type Block = frame_system::mocking::MockBlock<Runtime>; @@ -713,6 +722,16 @@ pub fn pool_balance(id: PoolId) -> Balance { .expect("who must be a bonded pool account") } +pub fn add_to_restrict_list(who: &AccountId) { + if !RestrictedAccounts::get().contains(who) { + RestrictedAccounts::mutate(|l| l.push(*who)); + } +} + +pub fn remove_from_restrict_list(who: &AccountId) { + RestrictedAccounts::mutate(|l| l.retain(|x| x != who)); +} + #[cfg(test)] mod test { use super::*; diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index e2922e22fa98949c219d377b53cd2f0aba3c4964..9926e4bccb6f9da1502c6e506cf5c598135cc5f8 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -7568,3 +7568,73 @@ mod chill { }) } } + +mod filter { + use super::*; + + #[test] + fn restricted_accounts_cannot_join() { + ExtBuilder::default().build_and_execute(|| { + // GIVEN + let alice = 301; + Currency::set_balance(&alice, 20_000); + + // WHEN alice is restricted from participating in pools + add_to_restrict_list(&alice); + + // THEN alice cannot join any pool + assert_noop!( + Pools::join(RuntimeOrigin::signed(alice), 10, 1), + Error::<Runtime>::Restricted + ); + // neither she can create a new pool + assert_noop!( + Pools::create(RuntimeOrigin::signed(alice), 1000, alice, alice, alice), + Error::<Runtime>::Restricted + ); + + // WHEN alice is removed from restricted accounts. + remove_from_restrict_list(&alice); + + // THEN alice can join a pool + assert_ok!(Pools::join(RuntimeOrigin::signed(alice), 10, 1)); + + // WHEN alice is restricted while being in a pool + add_to_restrict_list(&alice); + + // THEN she cannot bond extra funds to the pool + assert_noop!( + Pools::bond_extra(RuntimeOrigin::signed(alice), BondExtra::FreeBalance(10)), + Error::<Runtime>::Restricted + ); + assert_noop!( + Pools::bond_extra(RuntimeOrigin::signed(alice), BondExtra::Rewards), + Error::<Runtime>::Restricted + ); + // nor anyone else can bond her rewards on her behalf + assert_noop!( + Pools::bond_extra_other(RuntimeOrigin::signed(20), alice, BondExtra::Rewards), + Error::<Runtime>::Restricted + ); + + // but she can claim rewards + deposit_rewards(10); + assert_ok!(Pools::claim_payout(RuntimeOrigin::signed(alice))); + // someone else can claim rewards on her behalf + deposit_rewards(10); + assert_ok!(Pools::claim_payout_other(RuntimeOrigin::signed(20), alice)); + // can unbond + assert_ok!(Pools::unbond(RuntimeOrigin::signed(alice), alice, 5)); + // and withdraw + CurrentEra::set(3); + assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(alice), alice, 0)); + + // WHEN alice is removed from restrict list + remove_from_restrict_list(&alice); + + // THEN she can bond extra funds to the pool + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(alice), BondExtra::FreeBalance(10))); + assert_ok!(Pools::bond_extra(RuntimeOrigin::signed(alice), BondExtra::Rewards)); + }); + } +} diff --git a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs index 84d23a994e6e4bf3bd2b796913a6ab30a186b1de..cc7ea7c029ba89c1bcea962202b8cb96e211a5e8 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs @@ -23,7 +23,7 @@ use frame_support::{ assert_ok, derive_impl, pallet_prelude::*, parameter_types, - traits::{ConstU64, ConstU8, VariantCountOf}, + traits::{ConstU64, ConstU8, Nothing, VariantCountOf}, PalletId, }; use frame_system::EnsureRoot; @@ -270,6 +270,7 @@ impl pallet_nomination_pools::Config for Runtime { type PalletId = PoolsPalletId; type AdminOrigin = EnsureRoot<AccountId>; type BlockNumberProvider = System; + type Filter = Nothing; } parameter_types! { diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index a5fe16e500b8f29bbba27eb5667cde1b73a1793b..6e8742e31c6e62de7f64d54bc2932f7e9ed7b29e 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -350,7 +350,7 @@ use frame_support::{ defensive, defensive_assert, traits::{ tokens::fungible::{Credit, Debt}, - ConstU32, Defensive, DefensiveMax, DefensiveSaturating, Get, LockIdentifier, + ConstU32, Contains, Defensive, DefensiveMax, DefensiveSaturating, Get, LockIdentifier, }, weights::Weight, BoundedVec, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, WeakBoundedVec, @@ -1335,6 +1335,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 for snapshot creation. diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 43cff11d80427c563b1ce51c16c199595c00626c..4546dbf74594691d02a4ae1f076b83b26b044cdb 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -319,6 +319,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; @@ -336,6 +337,13 @@ impl OnStakingUpdate<AccountId, Balance> for EventListenerMock { } } +pub struct MockedRestrictList; +impl Contains<AccountId> for MockedRestrictList { + fn contains(who: &AccountId) -> bool { + RestrictedAccounts::get().contains(who) + } +} + // Disabling threshold for `UpToLimitDisablingStrategy` and // `UpToLimitWithReEnablingDisablingStrategy`` pub(crate) const DISABLING_LIMIT_FACTOR: usize = 3; @@ -367,6 +375,7 @@ impl crate::pallet::pallet::Config for Test { type EventListeners = EventListenerMock; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type Filter = MockedRestrictList; } pub struct WeightedNominationsQuota<const MAX: u32>; @@ -1068,3 +1077,13 @@ pub(crate) fn to_bounded_supports( > { supports.try_into().unwrap() } + +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)); +} diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 6bd52e2bce166befd6fd20340a06a83b36342a56..2514fbd2537d7cb0821f67ce307ea03bb61e38b9 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -28,8 +28,8 @@ use frame_support::{ hold::{Balanced as FunHoldBalanced, Mutate as FunHoldMutate}, Inspect, Mutate, Mutate as FunMutate, }, - Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get, - InspectLockableCurrency, OnUnbalanced, UnixTime, + Contains, Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get, + InspectLockableCurrency, Nothing, OnUnbalanced, UnixTime, }, weights::Weight, BoundedBTreeSet, BoundedVec, @@ -331,6 +331,13 @@ pub mod pallet { #[pallet::constant] type MaxDisabledValidators: Get<u32>; + #[pallet::no_default_bounds] + /// Filter some accounts from participating in staking. + /// + /// This is useful for example to blacklist an account that is participating in staking in + /// another way (such as pools). + type Filter: Contains<Self::AccountId>; + /// Some parameters of the benchmarking. #[cfg(feature = "std")] type BenchmarkingConfig: BenchmarkingConfig; @@ -390,6 +397,7 @@ pub mod pallet { type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; type EventListeners = (); + type Filter = Nothing; #[cfg(feature = "std")] type BenchmarkingConfig = crate::TestBenchmarkingConfig; type WeightInfo = (); @@ -1145,6 +1153,9 @@ pub mod pallet { AlreadyMigrated, /// Era not yet started. EraNotStarted, + /// Account is restricted from participation in staking. This may happen if the account is + /// staking in another way already, such as via pool. + Restricted, } #[pallet::hooks] @@ -1402,6 +1413,8 @@ pub mod pallet { ) -> DispatchResult { let stash = ensure_signed(origin)?; + ensure!(!T::Filter::contains(&stash), Error::<T>::Restricted); + if StakingLedger::<T>::is_bonded(StakingAccount::Stash(stash.clone())) { return Err(Error::<T>::AlreadyBonded.into()) } @@ -1449,6 +1462,7 @@ pub mod pallet { #[pallet::compact] max_additional: BalanceOf<T>, ) -> DispatchResult { let stash = ensure_signed(origin)?; + ensure!(!T::Filter::contains(&stash), Error::<T>::Restricted); Self::do_bond_extra(&stash, max_additional) } @@ -2032,6 +2046,8 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let controller = ensure_signed(origin)?; let ledger = Self::ledger(Controller(controller))?; + + ensure!(!T::Filter::contains(&ledger.stash), Error::<T>::Restricted); ensure!(!ledger.unlocking.is_empty(), Error::<T>::NoUnlockChunk); let initial_unlocking = ledger.unlocking.len() as u32; @@ -2588,5 +2604,37 @@ pub mod pallet { Ok(Pays::No.into()) } + + /// 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)?; + + let ledger = Self::ledger(Stash(stash.clone()))?; + let actual_stake = asset::staked::<T>(&stash); + let force_withdraw_amount = ledger.total.defensive_saturating_sub(actual_stake); + + // ensure there is something to force unstake. + ensure!(!force_withdraw_amount.is_zero(), Error::<T>::BoundNotMet); + + // we ignore if active is 0. It implies the locked amount is not actively staked. The + // account can still get away from potential slash, but we can't do much better here. + StakingLedger { + total: actual_stake, + active: ledger.active.saturating_sub(force_withdraw_amount), + ..ledger + } + .update()?; + + Self::deposit_event(Event::<T>::Withdrawn { stash, amount: force_withdraw_amount }); + + Ok(()) + } } } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 64639648073ba5cb28ebdece09828305bfb92dbf..cde313d2577058139cfd72517975375833ec3d99 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -49,7 +49,7 @@ use sp_runtime::{ }; use sp_staking::{ offence::{OffenceDetails, OnOffenceHandler}, - SessionIndex, StakingInterface, + SessionIndex, Stake, StakingInterface, }; use substrate_test_utils::assert_eq_uvec; @@ -5040,6 +5040,131 @@ fn on_finalize_weight_is_nonzero() { }) } +#[test] +fn restricted_accounts_can_only_withdraw() { + ExtBuilder::default().build_and_execute(|| { + start_active_era(1); + // alice is a non blacklisted account. + let alice = 301; + let _ = Balances::make_free_balance_be(&alice, 500); + // alice can bond + assert_ok!(Staking::bond(RuntimeOrigin::signed(alice), 100, RewardDestination::Staked)); + // and bob is a blacklisted account + let bob = 302; + let _ = Balances::make_free_balance_be(&bob, 500); + restrict(&bob); + + // Bob cannot bond + assert_noop!( + Staking::bond(RuntimeOrigin::signed(bob), 100, RewardDestination::Staked,), + Error::<Test>::Restricted + ); + + // alice is blacklisted now and cannot bond anymore + restrict(&alice); + assert_noop!( + Staking::bond_extra(RuntimeOrigin::signed(alice), 100), + Error::<Test>::Restricted + ); + // but she can unbond her existing bond + assert_ok!(Staking::unbond(RuntimeOrigin::signed(alice), 100)); + + // she cannot rebond the unbonded amount + start_active_era(2); + assert_noop!(Staking::rebond(RuntimeOrigin::signed(alice), 50), Error::<Test>::Restricted); + + // move to era when alice fund can be withdrawn + start_active_era(4); + // alice can withdraw now + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(alice), 0)); + // she still cannot bond + assert_noop!( + Staking::bond(RuntimeOrigin::signed(alice), 100, RewardDestination::Staked,), + Error::<Test>::Restricted + ); + + // bob is removed from restrict list + remove_from_restrict_list(&bob); + // bob can bond now + assert_ok!(Staking::bond(RuntimeOrigin::signed(bob), 100, RewardDestination::Staked)); + // and bond extra + assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(bob), 100)); + + start_active_era(6); + // unbond also works. + assert_ok!(Staking::unbond(RuntimeOrigin::signed(bob), 100)); + // bob can withdraw as well. + start_active_era(9); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(bob), 0)); + }) +} + +#[test] +fn permissionless_withdraw_overstake() { + ExtBuilder::default().build_and_execute(|| { + // Given Alice, Bob and Charlie with some stake. + let alice = 301; + let bob = 302; + let charlie = 303; + let _ = Balances::make_free_balance_be(&alice, 500); + let _ = Balances::make_free_balance_be(&bob, 500); + let _ = Balances::make_free_balance_be(&charlie, 500); + assert_ok!(Staking::bond(RuntimeOrigin::signed(alice), 100, RewardDestination::Staked)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(bob), 100, RewardDestination::Staked)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(charlie), 100, RewardDestination::Staked)); + + // WHEN: charlie is partially unbonding. + assert_ok!(Staking::unbond(RuntimeOrigin::signed(charlie), 90)); + let charlie_ledger = StakingLedger::<Test>::get(StakingAccount::Stash(charlie)).unwrap(); + + // AND: alice and charlie ledger having higher value than actual stake. + Ledger::<Test>::insert(alice, StakingLedger::<Test>::new(alice, 200)); + Ledger::<Test>::insert( + charlie, + StakingLedger { stash: charlie, total: 200, active: 200 - 90, ..charlie_ledger }, + ); + + // THEN overstake can be permissionlessly withdrawn. + System::reset_events(); + + // Alice stake is corrected. + assert_eq!( + <Staking as StakingInterface>::stake(&alice).unwrap(), + Stake { total: 200, active: 200 } + ); + assert_ok!(Staking::withdraw_overstake(RuntimeOrigin::signed(1), alice)); + assert_eq!( + <Staking as StakingInterface>::stake(&alice).unwrap(), + Stake { total: 100, active: 100 } + ); + + // Charlie who is partially withdrawing also gets their stake corrected. + assert_eq!( + <Staking as StakingInterface>::stake(&charlie).unwrap(), + Stake { total: 200, active: 110 } + ); + assert_ok!(Staking::withdraw_overstake(RuntimeOrigin::signed(1), charlie)); + assert_eq!( + <Staking as StakingInterface>::stake(&charlie).unwrap(), + Stake { total: 200 - 100, active: 110 - 100 } + ); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::Withdrawn { stash: alice, amount: 200 - 100 }, + Event::Withdrawn { stash: charlie, amount: 200 - 100 } + ] + ); + + // but Bob ledger is fine and that cannot be withdrawn. + assert_noop!( + Staking::withdraw_overstake(RuntimeOrigin::signed(1), bob), + Error::<Test>::BoundNotMet + ); + }); +} + mod election_data_provider { use super::*; use frame_election_provider_support::ElectionDataProvider;