Nomination pool configurations can be managed by custom origin (parit…

…ytech#3959) closes paritytech#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 <[email protected]>
acatangiu · Apr 9, 2024 · 10ed764 · 10ed764
1 parent 6ce126a
commit 10ed764
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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>;