From b26154007c64fdf5b7121db52891d9e737e1366b Mon Sep 17 00:00:00 2001 From: Ankan <10196091+Ank4n@users.noreply.github.com> Date: Thu, 27 Feb 2025 01:42:57 +0530 Subject: [PATCH] BACKPORT-CONFLICT --- polkadot/runtime/test-runtime/src/lib.rs | 7 + polkadot/runtime/westend/src/lib.rs | 13 +- prdoc/pr_7685.prdoc | 20 +++ substrate/bin/node/runtime/src/lib.rs | 11 ++ substrate/frame/delegated-staking/src/lib.rs | 10 -- substrate/frame/delegated-staking/src/mock.rs | 6 + .../frame/delegated-staking/src/tests.rs | 83 ++++------- .../test-staking-e2e/src/mock.rs | 23 ++- .../nomination-pools/benchmarking/src/mock.rs | 7 +- substrate/frame/nomination-pools/src/lib.rs | 40 +++++- substrate/frame/nomination-pools/src/mock.rs | 41 ++++++ substrate/frame/nomination-pools/src/tests.rs | 69 +++++++++ .../test-delegate-stake/src/mock.rs | 7 +- substrate/frame/staking/src/lib.rs | 23 +++ substrate/frame/staking/src/mock.rs | 29 ++++ substrate/frame/staking/src/pallet/mod.rs | 131 ++++++++++++++++++ substrate/frame/staking/src/tests.rs | 129 +++++++++++++++++ 17 files changed, 578 insertions(+), 71 deletions(-) create mode 100644 prdoc/pr_7685.prdoc diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index 39b96bbaacfad..16c035b4138bd 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -395,7 +395,14 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = polkadot_runtime_common::StakingBenchmarkingConfig; type EventListeners = (); type WeightInfo = (); +<<<<<<< HEAD type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; +======= + type MaxValidatorSet = MaxAuthorities; + type MaxInvulnerables = ConstU32<20>; + type MaxDisabledValidators = ConstU32<100>; + type Filter = frame_support::traits::Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } parameter_types! { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 800886fe6a0c3..8ca5656cb0be1 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 _}, @@ -755,7 +755,13 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = polkadot_runtime_common::StakingBenchmarkingConfig; type EventListeners = (NominationPools, DelegatedStaking); type WeightInfo = weights::pallet_staking::WeightInfo; +<<<<<<< HEAD type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; +======= + type MaxInvulnerables = frame_support::traits::ConstU32<20>; + type MaxDisabledValidators = ConstU32<100>; + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } impl pallet_fast_unstake::Config for Runtime { @@ -1516,6 +1522,11 @@ impl pallet_nomination_pools::Config for Runtime { type PalletId = PoolsPalletId; type MaxPointsToBalance = MaxPointsToBalance; type AdminOrigin = EitherOf, StakingAdmin>; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } 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 5a2ff3ceb7f6a..b6a05b27ca27f 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -741,7 +741,13 @@ impl pallet_staking::Config for Runtime { type EventListeners = NominationPools; type WeightInfo = pallet_staking::weights::SubstrateWeight; type BenchmarkingConfig = StakingBenchmarkingConfig; +<<<<<<< HEAD type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; +======= + type MaxInvulnerables = ConstU32<20>; + type MaxDisabledValidators = ConstU32<100>; + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } impl pallet_fast_unstake::Config for Runtime { @@ -960,6 +966,11 @@ impl pallet_nomination_pools::Config for Runtime { EnsureRoot, pallet_collective::EnsureProportionAtLeast, >; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } parameter_types! { diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 1d181eb29cab7..bbba504e807f0 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 811d5739f4e98..96a88ae5bee83 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -111,6 +111,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! { @@ -160,6 +161,11 @@ impl pallet_nomination_pools::Config for Runtime { type StakeAdapter = pallet_nomination_pools::adapter::DelegateStake; type AdminOrigin = frame_system::EnsureRoot; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = pallet_staking::AllStakers; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } frame_support::construct_runtime!( diff --git a/substrate/frame/delegated-staking/src/tests.rs b/substrate/frame/delegated-staking/src/tests.rs index b7b82a43771eb..6ffbe1a535840 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 - ); }); } @@ -1361,7 +1324,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(|| { @@ -1375,28 +1338,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 360f14913fcc6..c203ee9f6d71b 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 @@ -53,9 +53,8 @@ use pallet_staking::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; @@ -272,6 +271,26 @@ impl pallet_nomination_pools::Config for Runtime { type MaxUnbonding = MaxUnbonding; type MaxPointsToBalance = frame_support::traits::ConstU8<10>; type AdminOrigin = frame_system::EnsureRoot; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = Nothing; +} + +parameter_types! { + pub const DelegatedStakingPalletId: PalletId = PalletId(*b"py/dlstk"); + pub const SlashRewardFraction: Perbill = Perbill::from_percent(1); +} + +impl pallet_delegated_staking::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type PalletId = DelegatedStakingPalletId; + type Currency = Balances; + type OnSlash = (); + type SlashRewardFraction = SlashRewardFraction; + type RuntimeHoldReason = RuntimeHoldReason; + type CoreStaking = Staking; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } parameter_types! { diff --git a/substrate/frame/nomination-pools/benchmarking/src/mock.rs b/substrate/frame/nomination-pools/benchmarking/src/mock.rs index 15d9e2c56031f..abed1c4fefdec 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::{ @@ -139,6 +139,11 @@ impl pallet_nomination_pools::Config for Runtime { type PalletId = PoolsPalletId; type MaxPointsToBalance = MaxPointsToBalance; type AdminOrigin = frame_system::EnsureRoot; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } parameter_types! { diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 201b0af1d6080..86fbacf8d8f84 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, }; @@ -1650,6 +1650,15 @@ pub mod pallet { /// The origin that can manage pool configurations. type AdminOrigin: EnsureOrigin; +<<<<<<< HEAD +======= + + /// 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; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } /// The sum of funds across all pools. @@ -1950,6 +1959,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(Encode, Decode, PartialEq, TypeInfo, PalletError, RuntimeDebug)] @@ -1987,8 +1999,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 @@ -2013,6 +2026,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); @@ -3012,12 +3028,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)?; @@ -3346,6 +3366,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)?; @@ -3419,6 +3442,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(), @@ -4079,3 +4105,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 cc942039760c0..e2316c42c2489 100644 --- a/substrate/frame/nomination-pools/src/mock.rs +++ b/substrate/frame/nomination-pools/src/mock.rs @@ -56,6 +56,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; @@ -276,6 +277,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; @@ -302,6 +310,11 @@ impl pools::Config for Runtime { type MaxUnbonding = MaxUnbonding; type MaxPointsToBalance = frame_support::traits::ConstU8<10>; type AdminOrigin = EnsureSignedBy; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = RestrictMock; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } type Block = frame_system::mocking::MockBlock; @@ -533,6 +546,34 @@ pub fn reward_imbalance(pool: PoolId) -> RewardImbalance { } } +<<<<<<< HEAD +======= +pub fn set_pool_balance(who: AccountId, amount: Balance) { + StakingMock::set_bonded_balance(who, amount); + DelegateMock::set_agent_balance(who, amount); +} + +pub fn member_delegation(who: AccountId) -> Balance { + ::StakeAdapter::member_delegation_balance(Member::from(who)) + .expect("who must be a pool member") +} + +pub fn pool_balance(id: PoolId) -> Balance { + ::StakeAdapter::total_balance(Pool::from(Pools::generate_bonded_account(id))) + .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)); +} + +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) #[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 06261699a5b23..f7f1b36595974 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -7488,6 +7488,7 @@ mod chill { } } +<<<<<<< HEAD // the test mock is using `TransferStake` and so `DelegateStake` is not tested here. Extrinsics // meant for `DelegateStake` should be gated. // @@ -7544,6 +7545,74 @@ mod delegate_stake { Pools::apply_slash(RuntimeOrigin::signed(10), 11), Error::::NotSupported ); +======= +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)); +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) }); } } 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 d1bc4ef8ff281..1e3a540f22fa3 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs @@ -20,7 +20,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; @@ -265,6 +265,11 @@ impl pallet_nomination_pools::Config for Runtime { type MaxPointsToBalance = ConstU8<10>; type PalletId = PoolsPalletId; type AdminOrigin = EnsureRoot; +<<<<<<< HEAD +======= + type BlockNumberProvider = System; + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } parameter_types! { diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 19d999109d8dd..53fa1b3dd35df 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -312,7 +312,12 @@ use codec::{Decode, Encode, HasCompact, MaxEncodedLen}; use frame_support::{ defensive, defensive_assert, traits::{ +<<<<<<< HEAD ConstU32, Currency, Defensive, DefensiveMax, DefensiveSaturating, Get, LockIdentifier, +======= + tokens::fungible::{Credit, Debt}, + ConstU32, Contains, Defensive, DefensiveMax, DefensiveSaturating, Get, LockIdentifier, +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) }, weights::Weight, BoundedVec, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, @@ -1249,6 +1254,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. diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 4a0209fc5b083..e107708562d51 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -241,6 +241,7 @@ parameter_types! { (BalanceOf, BTreeMap>) = (Zero::zero(), BTreeMap::new()); pub static SlashObserver: BTreeMap> = BTreeMap::new(); + pub static RestrictedAccounts: Vec = Vec::new(); } pub struct EventListenerMock; @@ -258,7 +259,19 @@ impl OnStakingUpdate for EventListenerMock { } } +<<<<<<< HEAD // Disabling threshold for `UpToLimitDisablingStrategy` +======= +pub struct MockedRestrictList; +impl Contains for MockedRestrictList { + fn contains(who: &AccountId) -> bool { + RestrictedAccounts::get().contains(who) + } +} + +// Disabling threshold for `UpToLimitDisablingStrategy` and +// `UpToLimitWithReEnablingDisablingStrategy`` +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) pub(crate) const DISABLING_LIMIT_FACTOR: usize = 3; #[derive_impl(crate::config_preludes::TestDefaultConfig)] @@ -284,7 +297,13 @@ impl crate::pallet::pallet::Config for Test { type HistoryDepth = HistoryDepth; type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; type EventListeners = EventListenerMock; +<<<<<<< HEAD type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; +======= + type MaxInvulnerables = ConstU32<20>; + type MaxDisabledValidators = ConstU32<100>; + type Filter = MockedRestrictList; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } pub struct WeightedNominationsQuota; @@ -928,3 +947,13 @@ pub(crate) fn staking_events_since_last_call() -> Vec> { 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)); +} diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 5210bef853b29..dcbe964543588 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -25,8 +25,17 @@ use frame_election_provider_support::{ use frame_support::{ pallet_prelude::*, traits::{ +<<<<<<< HEAD Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get, InspectLockableCurrency, LockableCurrency, OnUnbalanced, UnixTime, +======= + fungible::{ + hold::{Balanced as FunHoldBalanced, Mutate as FunHoldMutate}, + Inspect, Mutate, Mutate as FunMutate, + }, + Contains, Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get, + InspectLockableCurrency, Nothing, OnUnbalanced, UnixTime, +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) }, weights::Weight, BoundedVec, @@ -296,6 +305,13 @@ pub mod pallet { #[pallet::no_default_bounds] type DisablingStrategy: DisablingStrategy; + #[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; @@ -342,7 +358,11 @@ pub mod pallet { type MaxUnlockingChunks = ConstU32<32>; type MaxControllersInDeprecationBatch = ConstU32<100>; type EventListeners = (); +<<<<<<< HEAD type DisablingStrategy = crate::UpToLimitDisablingStrategy; +======= + type Filter = Nothing; +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) #[cfg(feature = "std")] type BenchmarkingConfig = crate::TestBenchmarkingConfig; type WeightInfo = (); @@ -939,6 +959,18 @@ pub mod pallet { NotEnoughFunds, /// Operation not allowed for virtual stakers. VirtualStakerNotAllowed, +<<<<<<< HEAD +======= + /// Stash could not be reaped as other pallet might depend on it. + CannotReapStash, + /// The stake of this account is already migrated to `Fungible` holds. + 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, +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } #[pallet::hooks] @@ -1018,6 +1050,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()) } @@ -1068,6 +1102,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) } @@ -1654,6 +1689,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; @@ -2151,6 +2188,100 @@ pub mod pallet { ); Ok(()) } +<<<<<<< HEAD +======= + + /// Migrates permissionlessly a stash from locks to holds. + /// + /// This removes the old lock on the stake and creates a hold on it atomically. If all + /// stake cannot be held, the best effort is made to hold as much as possible. The remaining + /// stake is removed from the ledger. + /// + /// The fee is waived if the migration is successful. + #[pallet::call_index(30)] + #[pallet::weight(T::WeightInfo::migrate_currency())] + pub fn migrate_currency( + origin: OriginFor, + stash: T::AccountId, + ) -> DispatchResultWithPostInfo { + let _ = ensure_signed(origin)?; + Self::do_migrate_currency(&stash)?; + + // Refund the transaction fee if successful. + Ok(Pays::No.into()) + } + + /// Manually applies a deferred slash for a given era. + /// + /// Normally, slashes are automatically applied shortly after the start of the `slash_era`. + /// This function exists as a **fallback mechanism** in case slashes were not applied due to + /// unexpected reasons. It allows anyone to manually apply an unapplied slash. + /// + /// ## Parameters + /// - `slash_era`: The staking era in which the slash was originally scheduled. + /// - `slash_key`: A unique identifier for the slash, represented as a tuple: + /// - `stash`: The stash account of the validator being slashed. + /// - `slash_fraction`: The fraction of the stake that was slashed. + /// - `page_index`: The index of the exposure page being processed. + /// + /// ## Behavior + /// - The function is **permissionless**—anyone can call it. + /// - The `slash_era` **must be the current era or a past era**. If it is in the future, the + /// call fails with `EraNotStarted`. + /// - The fee is waived if the slash is successfully applied. + /// + /// ## TODO: Future Improvement + /// - Implement an **off-chain worker (OCW) task** to automatically apply slashes when there + /// is unused block space, improving efficiency. + #[pallet::call_index(31)] + #[pallet::weight(T::WeightInfo::apply_slash())] + pub fn apply_slash( + origin: OriginFor, + slash_era: EraIndex, + slash_key: (T::AccountId, Perbill, u32), + ) -> DispatchResultWithPostInfo { + let _ = ensure_signed(origin)?; + let active_era = ActiveEra::::get().map(|a| a.index).unwrap_or_default(); + ensure!(slash_era <= active_era, Error::::EraNotStarted); + let unapplied_slash = UnappliedSlashes::::take(&slash_era, &slash_key) + .ok_or(Error::::InvalidSlashRecord)?; + slashing::apply_slash::(unapplied_slash, slash_era); + + 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(()) + } +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) } } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index d1dc6c3db659b..5b46dd885bdf6 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -39,7 +39,11 @@ use sp_runtime::{ }; use sp_staking::{ offence::{OffenceDetails, OnOffenceHandler}, +<<<<<<< HEAD SessionIndex, +======= + SessionIndex, Stake, StakingInterface, +>>>>>>> f7e98b40 ([Nomination Pool] Make staking restrictions configurable (#7685)) }; use substrate_test_utils::assert_eq_uvec; @@ -5111,6 +5115,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;