From 66bc9b3f74044212f44699910351d10fe9c5e88a Mon Sep 17 00:00:00 2001 From: Ankan Date: Mon, 24 Feb 2025 17:46:41 +0530 Subject: [PATCH] add the pool filter to runtimes --- polkadot/runtime/westend/src/lib.rs | 3 +- substrate/bin/node/runtime/src/lib.rs | 1 + substrate/frame/delegated-staking/src/lib.rs | 10 --- substrate/frame/delegated-staking/src/mock.rs | 1 + .../frame/delegated-staking/src/tests.rs | 68 ++++++++----------- .../test-staking-e2e/src/mock.rs | 4 +- .../nomination-pools/benchmarking/src/mock.rs | 3 +- .../test-delegate-stake/src/mock.rs | 3 +- substrate/frame/staking/src/lib.rs | 20 +++++- 9 files changed, 59 insertions(+), 54 deletions(-) diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 5af67ad9bd70a..aa0bb2e42e71c 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 _}, @@ -1547,6 +1547,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/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 7e33f1feeca19..77159ba1ebe90 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1236,6 +1236,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..e874451a15f23 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -164,6 +164,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..df4a20e42e0f1 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 - ); }); } @@ -1390,7 +1353,7 @@ mod pool_integration { // The delegator cannot add any extra bond to the pool anymore. assert_noop!( Pools::bond_extra(RawOrigin::Signed(delegator).into(), BondExtra::FreeBalance(100)), - Error::::AlreadyStaking + PoolsError::::Restricted ); // But they can unbond @@ -1401,6 +1364,35 @@ mod pool_integration { }); } + #[test] + fn stakers_cannot_join_pool() { + // 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(|| { + 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(staker), + 500, + RewardDestination::Account(101) + )); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(staker), vec![GENESIS_VALIDATOR])); + + // THEN: they cannot join pool. + assert_noop!( + Pools::join(RawOrigin::Signed(staker).into(), 200, pool_id), + PoolsError::::Restricted + ); + }); + } + fn create_pool(creator: AccountId, amount: Balance) -> u32 { fund(&creator, amount * 2); assert_ok!(Pools::create( 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/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.