diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index fc489e3bc685e..117aa849b133f 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 ddc26f4e645b6..741957d847377 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; 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, 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 0000000000000..23cb7b09297e9 --- /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 7e33f1feeca19..22e266cc18678 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, >; 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 fadc8d290d6f9..6df5a6b3eb569 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::::NotAllowed); - // They cannot be already a direct staker in the staking pallet. - ensure!(!Self::is_direct_staker(&who), Error::::AlreadyStaking); - // Reward account cannot be same as `agent` account. ensure!(reward_account != who, Error::::InvalidRewardDestination); @@ -407,7 +404,6 @@ pub mod pallet { // Ensure delegator is sane. ensure!(!Self::is_agent(&delegator), Error::::NotAllowed); ensure!(!Self::is_delegator(&delegator), Error::::NotAllowed); - ensure!(!Self::is_direct_staker(&delegator), Error::::AlreadyStaking); // ensure agent is sane. ensure!(Self::is_agent(&agent), Error::::NotAgent); @@ -442,12 +438,6 @@ pub mod pallet { Error::::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::::AlreadyStaking); - // ensure agent is sane. ensure!(Self::is_agent(&agent), Error::::NotAgent); diff --git a/substrate/frame/delegated-staking/src/mock.rs b/substrate/frame/delegated-staking/src/mock.rs index 003d3380f6681..44068ee5a7f31 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; type TargetList = pallet_staking::UseValidatorsMap; type EventListeners = (Pools, DelegatedStaking); + type Filter = pallet_nomination_pools::AllPoolMembers; } parameter_types! { @@ -164,6 +165,7 @@ impl pallet_nomination_pools::Config for Runtime { pallet_nomination_pools::adapter::DelegateStake; type AdminOrigin = frame_system::EnsureRoot; type BlockNumberProvider = System; + type Filter = pallet_staking::AllStakers; } frame_support::construct_runtime!( diff --git a/substrate/frame/delegated-staking/src/tests.rs b/substrate/frame/delegated-staking/src/tests.rs index c764e2741a2a4..05961d6f824db 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::::InvalidRewardDestination ); - - // an existing validator cannot become agent - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(mock::GENESIS_VALIDATOR).into(), - 100 - ), - Error::::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::::AlreadyStaking - ); - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(mock::GENESIS_NOMINATOR_TWO).into(), - 100 - ), - Error::::AlreadyStaking - ); }); } @@ -637,18 +612,6 @@ mod staking_integration { DelegatedStaking::register_agent(RawOrigin::Signed(202).into(), 203), Error::::NotAllowed ); - // existing staker cannot become a delegate - assert_noop!( - DelegatedStaking::register_agent( - RawOrigin::Signed(GENESIS_NOMINATOR_ONE).into(), - 201 - ), - Error::::AlreadyStaking - ); - assert_noop!( - DelegatedStaking::register_agent(RawOrigin::Signed(GENESIS_VALIDATOR).into(), 201), - Error::::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::::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::::AlreadyStaking + Pools::join(RawOrigin::Signed(staker).into(), 200, pool_id), + PoolsError::::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 120deff96a75e..aa9b314d0068e 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; 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 2e73ad7cf4fc0..1dcfb86b75cf6 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; 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 39a7cf05b3aba..a62c4223b3b19 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; } /// 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 Pallet { - /// 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::::NotMigrated); + // ensure account is not restricted from joining the pool. + ensure!(!T::Filter::contains(&who), Error::::Restricted); + ensure!(amount >= MinJoinBond::::get(), Error::::MinimumBondNotMet); // If a member already exists that means they already belong to a pool ensure!(!PoolMembers::::contains_key(&who), Error::::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::::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::::Restricted); + let member = PoolMembers::::get(&member_account).ok_or(Error::::PoolMemberNotFound)?; @@ -3482,6 +3496,9 @@ impl Pallet { bouncer: AccountIdLookupOf, pool_id: PoolId, ) -> DispatchResult { + // ensure depositor is not restricted from joining the pool. + ensure!(!T::Filter::contains(&who), Error::::Restricted); + let root = T::Lookup::lookup(root)?; let nominator = T::Lookup::lookup(nominator)?; let bouncer = T::Lookup::lookup(bouncer)?; @@ -3555,6 +3572,9 @@ impl Pallet { member_account: T::AccountId, extra: BondExtra>, ) -> DispatchResult { + // ensure account is not restricted from joining the pool. + ensure!(!T::Filter::contains(&member_account), Error::::Restricted); + if signer != member_account { ensure!( ClaimPermissions::::get(&member_account).can_bond_extra(), @@ -4224,3 +4244,11 @@ impl sp_staking::OnStakingUpdate> for Pall } } } + +/// A utility struct that provides a way to check if a given account is a pool member. +pub struct AllPoolMembers(PhantomData); +impl Contains for AllPoolMembers { + fn contains(t: &T::AccountId) -> bool { + PoolMembers::::contains_key(t) + } +} diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs index 88ef82a156200..b97d98d66948e 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> = None; + pub static RestrictedAccounts: Vec = Vec::new(); } pub struct StakingMock; @@ -451,6 +452,13 @@ impl Convert for U256ToBalance { } } +pub struct RestrictMock; +impl Contains 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; type BlockNumberProvider = System; + type Filter = RestrictMock; } type Block = frame_system::mocking::MockBlock; @@ -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 e2922e22fa989..9926e4bccb6f9 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::::Restricted + ); + // neither she can create a new pool + assert_noop!( + Pools::create(RuntimeOrigin::signed(alice), 1000, alice, alice, alice), + Error::::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::::Restricted + ); + assert_noop!( + Pools::bond_extra(RuntimeOrigin::signed(alice), BondExtra::Rewards), + Error::::Restricted + ); + // nor anyone else can bond her rewards on her behalf + assert_noop!( + Pools::bond_extra_other(RuntimeOrigin::signed(20), alice, BondExtra::Rewards), + Error::::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 84d23a994e6e4..cc7ea7c029ba8 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; 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 a5fe16e500b8f..6e8742e31c6e6 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 EraInfo { } } +/// 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(core::marker::PhantomData); + +impl Contains for AllStakers { + /// 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::::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 43cff11d80427..4546dbf745946 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -319,6 +319,7 @@ parameter_types! { (BalanceOf, BTreeMap>) = (Zero::zero(), BTreeMap::new()); pub static SlashObserver: BTreeMap> = BTreeMap::new(); + pub static RestrictedAccounts: Vec = Vec::new(); } pub struct EventListenerMock; @@ -336,6 +337,13 @@ impl OnStakingUpdate for EventListenerMock { } } +pub struct MockedRestrictList; +impl Contains 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; @@ -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 6bd52e2bce166..2514fbd2537d7 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; + #[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; + /// 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::::Restricted); + if StakingLedger::::is_bonded(StakingAccount::Stash(stash.clone())) { return Err(Error::::AlreadyBonded.into()) } @@ -1449,6 +1462,7 @@ pub mod pallet { #[pallet::compact] max_additional: BalanceOf, ) -> DispatchResult { let stash = ensure_signed(origin)?; + ensure!(!T::Filter::contains(&stash), Error::::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::::Restricted); ensure!(!ledger.unlocking.is_empty(), Error::::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, stash: T::AccountId) -> DispatchResult { + let _ = ensure_signed(origin)?; + + let ledger = Self::ledger(Stash(stash.clone()))?; + let actual_stake = asset::staked::(&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::::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::::Withdrawn { stash, amount: force_withdraw_amount }); + + Ok(()) + } } } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 64639648073ba..cde313d257705 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::::Restricted + ); + + // alice is blacklisted now and cannot bond anymore + restrict(&alice); + assert_noop!( + Staking::bond_extra(RuntimeOrigin::signed(alice), 100), + Error::::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::::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::::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::::get(StakingAccount::Stash(charlie)).unwrap(); + + // AND: alice and charlie ledger having higher value than actual stake. + Ledger::::insert(alice, StakingLedger::::new(alice, 200)); + Ledger::::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!( + ::stake(&alice).unwrap(), + Stake { total: 200, active: 200 } + ); + assert_ok!(Staking::withdraw_overstake(RuntimeOrigin::signed(1), alice)); + assert_eq!( + ::stake(&alice).unwrap(), + Stake { total: 100, active: 100 } + ); + + // Charlie who is partially withdrawing also gets their stake corrected. + assert_eq!( + ::stake(&charlie).unwrap(), + Stake { total: 200, active: 110 } + ); + assert_ok!(Staking::withdraw_overstake(RuntimeOrigin::signed(1), charlie)); + assert_eq!( + ::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::::BoundNotMet + ); + }); +} + mod election_data_provider { use super::*; use frame_election_provider_support::ElectionDataProvider;