From 10ed76437f1355fab5559e992409fc4c7b30788f Mon Sep 17 00:00:00 2001
From: Ankan <10196091+Ank4n@users.noreply.github.com>
Date: Tue, 9 Apr 2024 12:14:19 +0200
Subject: [PATCH] Nomination pool configurations can be managed by custom
origin (#3959)
closes https://github.com/paritytech/polkadot-sdk/issues/3894
Allows Nomination Pool configuration to be set by a custom origin
instead of root.
In runtimes, we would set this to be `StakingAdmin`, same as for
pallet-staking.
---------
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
---
polkadot/runtime/westend/src/lib.rs | 1 +
prdoc/pr_3959.prdoc | 14 ++++++++++
substrate/bin/node/runtime/src/lib.rs | 4 +++
.../test-staking-e2e/src/mock.rs | 1 +
.../nomination-pools/benchmarking/src/mock.rs | 1 +
substrate/frame/nomination-pools/src/lib.rs | 7 +++--
substrate/frame/nomination-pools/src/mock.rs | 13 +++++++--
substrate/frame/nomination-pools/src/tests.rs | 28 +++++++++++++++----
.../nomination-pools/test-staking/src/mock.rs | 1 +
9 files changed, 61 insertions(+), 9 deletions(-)
create mode 100644 prdoc/pr_3959.prdoc
diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs
index 0bb15653c18..b44b448b012 100644
--- a/polkadot/runtime/westend/src/lib.rs
+++ b/polkadot/runtime/westend/src/lib.rs
@@ -1340,6 +1340,7 @@ impl pallet_nomination_pools::Config for Runtime {
type MaxUnbonding = <Self as pallet_staking::Config>::MaxUnlockingChunks;
type PalletId = PoolsPalletId;
type MaxPointsToBalance = MaxPointsToBalance;
+ type AdminOrigin = EitherOf<EnsureRoot<AccountId>, StakingAdmin>;
}
impl pallet_root_testing::Config for Runtime {
diff --git a/prdoc/pr_3959.prdoc b/prdoc/pr_3959.prdoc
new file mode 100644
index 00000000000..33c82003b0e
--- /dev/null
+++ b/prdoc/pr_3959.prdoc
@@ -0,0 +1,14 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Allow StakingAdmin to manage nomination pool configurations
+
+doc:
+ - audience: Runtime User
+ description: |
+ Adds a custom origin to Nomination pool configuration and allows StakingAdmin to be this origin in Westend. Other
+ runtimes could also set this origin to be the same that manages staking-pallet configurations.
+
+crates:
+ - name: pallet-nomination-pools
+ bump: major
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index e49b6885cc4..65d3ba0ed6e 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -917,6 +917,10 @@ impl pallet_nomination_pools::Config for Runtime {
type MaxUnbonding = ConstU32<8>;
type PalletId = NominationPoolsPalletId;
type MaxPointsToBalance = MaxPointsToBalance;
+ type AdminOrigin = EitherOfDiverse<
+ EnsureRoot<AccountId>,
+ pallet_collective::EnsureProportionAtLeast<AccountId, CouncilCollective, 3, 4>,
+ >;
}
parameter_types! {
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 7efcc4701e2..a727e3bf816 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
@@ -281,6 +281,7 @@ impl pallet_nomination_pools::Config for Runtime {
type MaxMetadataLen = ConstU32<256>;
type MaxUnbonding = MaxUnbonding;
type MaxPointsToBalance = frame_support::traits::ConstU8<10>;
+ type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
}
parameter_types! {
diff --git a/substrate/frame/nomination-pools/benchmarking/src/mock.rs b/substrate/frame/nomination-pools/benchmarking/src/mock.rs
index 1c513a1007c..a59f8f3f40e 100644
--- a/substrate/frame/nomination-pools/benchmarking/src/mock.rs
+++ b/substrate/frame/nomination-pools/benchmarking/src/mock.rs
@@ -172,6 +172,7 @@ impl pallet_nomination_pools::Config for Runtime {
type MaxUnbonding = ConstU32<8>;
type PalletId = PoolsPalletId;
type MaxPointsToBalance = MaxPointsToBalance;
+ type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
}
impl crate::Config for Runtime {}
diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs
index 23501cd89d2..0fdb7e3eff5 100644
--- a/substrate/frame/nomination-pools/src/lib.rs
+++ b/substrate/frame/nomination-pools/src/lib.rs
@@ -1657,6 +1657,9 @@ pub mod pallet {
/// The maximum length, in bytes, that a pools metadata maybe.
type MaxMetadataLen: Get<u32>;
+
+ /// The origin that can manage pool configurations.
+ type AdminOrigin: EnsureOrigin<Self::RuntimeOrigin>;
}
/// The sum of funds across all pools.
@@ -2495,7 +2498,7 @@ pub mod pallet {
}
/// Update configurations for the nomination pools. The origin for this call must be
- /// Root.
+ /// [`Config::AdminOrigin`].
///
/// # Arguments
///
@@ -2516,7 +2519,7 @@ pub mod pallet {
max_members_per_pool: ConfigOp<u32>,
global_max_commission: ConfigOp<Perbill>,
) -> DispatchResult {
- ensure_root(origin)?;
+ T::AdminOrigin::ensure_origin(origin)?;
macro_rules! config_op_exp {
($storage:ty, $op:ident) => {
diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs
index 686759604c2..b9301a40095 100644
--- a/substrate/frame/nomination-pools/src/mock.rs
+++ b/substrate/frame/nomination-pools/src/mock.rs
@@ -17,8 +17,11 @@
use super::*;
use crate::{self as pools};
-use frame_support::{assert_ok, derive_impl, parameter_types, traits::fungible::Mutate, PalletId};
-use frame_system::RawOrigin;
+use frame_support::{
+ assert_ok, derive_impl, ord_parameter_types, parameter_types, traits::fungible::Mutate,
+ PalletId,
+};
+use frame_system::{EnsureSignedBy, RawOrigin};
use sp_runtime::{BuildStorage, FixedU128};
use sp_staking::{OnStakingUpdate, Stake};
@@ -289,6 +292,11 @@ parameter_types! {
pub static CheckLevel: u8 = 255;
pub const PoolsPalletId: PalletId = PalletId(*b"py/nopls");
}
+
+ord_parameter_types! {
+ pub const Admin: u128 = 42;
+}
+
impl pools::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();
@@ -303,6 +311,7 @@ impl pools::Config for Runtime {
type MaxMetadataLen = MaxMetadataLen;
type MaxUnbonding = MaxUnbonding;
type MaxPointsToBalance = frame_support::traits::ConstU8<10>;
+ type AdminOrigin = EnsureSignedBy<Admin, AccountId>;
}
type Block = frame_system::mocking::MockBlock<Runtime>;
diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs
index 32b3e9af3cd..f6ef1e6eaac 100644
--- a/substrate/frame/nomination-pools/src/tests.rs
+++ b/substrate/frame/nomination-pools/src/tests.rs
@@ -19,7 +19,11 @@ use super::*;
use crate::{mock::*, Event};
use frame_support::{assert_err, assert_noop, assert_ok, assert_storage_noop};
use pallet_balances::Event as BEvent;
-use sp_runtime::{bounded_btree_map, traits::Dispatchable, FixedU128};
+use sp_runtime::{
+ bounded_btree_map,
+ traits::{BadOrigin, Dispatchable},
+ FixedU128,
+};
macro_rules! unbonding_pools_with_era {
($($k:expr => $v:expr),* $(,)?) => {{
@@ -4996,9 +5000,23 @@ mod set_configs {
#[test]
fn set_configs_works() {
ExtBuilder::default().build_and_execute(|| {
- // Setting works
+ // only admin origin can set configs
+ assert_noop!(
+ Pools::set_configs(
+ RuntimeOrigin::signed(20),
+ ConfigOp::Set(1 as Balance),
+ ConfigOp::Set(2 as Balance),
+ ConfigOp::Set(3u32),
+ ConfigOp::Set(4u32),
+ ConfigOp::Set(5u32),
+ ConfigOp::Set(Perbill::from_percent(6))
+ ),
+ BadOrigin
+ );
+
+ // Setting works by Admin (42)
assert_ok!(Pools::set_configs(
- RuntimeOrigin::root(),
+ RuntimeOrigin::signed(42),
ConfigOp::Set(1 as Balance),
ConfigOp::Set(2 as Balance),
ConfigOp::Set(3u32),
@@ -5015,7 +5033,7 @@ mod set_configs {
// Noop does nothing
assert_storage_noop!(assert_ok!(Pools::set_configs(
- RuntimeOrigin::root(),
+ RuntimeOrigin::signed(42),
ConfigOp::Noop,
ConfigOp::Noop,
ConfigOp::Noop,
@@ -5026,7 +5044,7 @@ mod set_configs {
// Removing works
assert_ok!(Pools::set_configs(
- RuntimeOrigin::root(),
+ RuntimeOrigin::signed(42),
ConfigOp::Remove,
ConfigOp::Remove,
ConfigOp::Remove,
diff --git a/substrate/frame/nomination-pools/test-staking/src/mock.rs b/substrate/frame/nomination-pools/test-staking/src/mock.rs
index 22939ff5e23..2ec47e0d164 100644
--- a/substrate/frame/nomination-pools/test-staking/src/mock.rs
+++ b/substrate/frame/nomination-pools/test-staking/src/mock.rs
@@ -186,6 +186,7 @@ impl pallet_nomination_pools::Config for Runtime {
type MaxUnbonding = ConstU32<8>;
type MaxPointsToBalance = ConstU8<10>;
type PalletId = PoolsPalletId;
+ type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
}
type Block = frame_system::mocking::MockBlock<Runtime>;
--
GitLab