diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index e8c6ca85267d2..1452b8250f116 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -11,10 +11,11 @@ use frame_election_provider_support::SortedListProvider; use frame_support::{ensure, traits::Get}; use frame_system::RawOrigin as Origin; use pallet_nomination_pools::{ - BalanceOf, BondedPoolInner, BondedPools, Delegators, Metadata, MinCreateBond, MinJoinBond, - Pallet as Pools, PoolRoles, PoolState, RewardPools, SubPoolsStorage, + BalanceOf, BondedPoolInner, BondedPools, ConfigOp, Delegators, MaxDelegators, + MaxDelegatorsPerPool, MaxPools, Metadata, MinCreateBond, MinJoinBond, Pallet as Pools, + PoolRoles, PoolState, RewardPools, SubPoolsStorage, }; -use sp_runtime::traits::{StaticLookup, Zero}; +use sp_runtime::traits::{Bounded, StaticLookup, Zero}; use sp_staking::{EraIndex, StakingInterface}; // `frame_benchmarking::benchmarks!` macro needs this use pallet_nomination_pools::Call; @@ -570,6 +571,22 @@ frame_benchmarking::benchmarks! { verify { assert_eq!(Metadata::::get(&1), metadata); } + + set_configs { + }:_( + Origin::Root, + ConfigOp::Set(BalanceOf::::max_value()), + ConfigOp::Set(BalanceOf::::max_value()), + ConfigOp::Set(u32::MAX), + ConfigOp::Set(u32::MAX), + ConfigOp::Set(u32::MAX) + ) verify { + assert_eq!(MinJoinBond::::get(), BalanceOf::::max_value()); + assert_eq!(MinCreateBond::::get(), BalanceOf::::max_value()); + assert_eq!(MaxPools::::get(), Some(u32::MAX)); + assert_eq!(MaxDelegators::::get(), Some(u32::MAX)); + assert_eq!(MaxDelegatorsPerPool::::get(), Some(u32::MAX)); + } } // TODO: consider benchmarking slashing logic with pools diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 0f1127ed71178..a17407f0a64a3 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -299,6 +299,7 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] +use codec::Codec; use frame_support::{ ensure, pallet_prelude::{MaxEncodedLen, *}, @@ -313,7 +314,7 @@ use scale_info::TypeInfo; use sp_core::U256; use sp_runtime::traits::{AccountIdConversion, Bounded, Convert, Saturating, StaticLookup, Zero}; use sp_staking::{EraIndex, OnStakerSlash, StakingInterface}; -use sp_std::{collections::btree_map::BTreeMap, ops::Div, vec::Vec}; +use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, ops::Div, vec::Vec}; #[cfg(test)] mod mock; @@ -336,6 +337,17 @@ pub type PoolId = u32; const POINTS_TO_BALANCE_INIT_RATIO: u32 = 1; +/// Possible operations on the configuration values of this pallet. +#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, PartialEq, Clone)] +pub enum ConfigOp { + /// Don't change. + Noop, + /// Set the given value. + Set(T), + /// Remove from storage. + Remove, +} + /// Extrinsics that bond some funds to the pool. enum PoolBond { Create, @@ -1512,15 +1524,45 @@ pub mod pallet { Ok(()) } - // Set - // * `min_join_bond` - // * `min_create_bond` - // * `max_pools` - // * `max_delegators_per_pool` - // * `max_delegators` - // pub fn set_parameters(origin: OriginFor, ) -> DispatchResult { + /// Update configurations for the nomination pools. The origin must for this call must be + /// Root. + /// + /// # Arguments + /// + /// * `min_join_bond` - Set [`Self::MinJoinBond`]. + /// * `min_create_bond` - Set [`Self::MinCreateBond`]. + /// * `max_pools` - Set [`Self::MaxPools`]. + /// * `max_delegators` - Set [`Self::MaxDelegators`]. + /// * `max_delegators_per_pool` - Set [`Self::MaxDelegatorsPerPool`]. + #[pallet::weight(T::WeightInfo::set_configs())] + pub fn set_configs( + origin: OriginFor, + min_join_bond: ConfigOp>, + min_create_bond: ConfigOp>, + max_pools: ConfigOp, + max_delegators: ConfigOp, + max_delegators_per_pool: ConfigOp, + ) -> DispatchResult { + ensure_root(origin)?; + + macro_rules! config_op_exp { + ($storage:ty, $op:ident) => { + match $op { + ConfigOp::Noop => (), + ConfigOp::Set(v) => <$storage>::put(v), + ConfigOp::Remove => <$storage>::kill(), + } + }; + } + + config_op_exp!(MinJoinBond::, min_join_bond); + config_op_exp!(MinCreateBond::, min_create_bond); + config_op_exp!(MaxPools::, max_pools); + config_op_exp!(MaxDelegators::, max_delegators); + config_op_exp!(MaxDelegatorsPerPool::, max_delegators_per_pool); - // } + Ok(()) + } } #[pallet::hooks] diff --git a/frame/nomination-pools/src/sanity.rs b/frame/nomination-pools/src/sanity.rs index 389065452ed11..1a40110d8ee94 100644 --- a/frame/nomination-pools/src/sanity.rs +++ b/frame/nomination-pools/src/sanity.rs @@ -12,7 +12,7 @@ fn bonding_pools_checks() { let mut delegators_seen = 0; for (_account, pool) in BondedPools::::iter() { assert!(pool.delegator_counter >= 1); - assert!(pool.delegator_counter <= MaxDelegatorsPerPool::::get().unwrap()); + assert!(MaxDelegatorsPerPool::::get().map_or(true, |max| pool.delegator_counter <= max)); delegators_seen += pool.delegator_counter; } assert_eq!(delegators_seen, Delegators::::count()); diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 51c1d33e3a1ca..0a024a298623c 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -1,6 +1,6 @@ use super::*; use crate::mock::*; -use frame_support::{assert_noop, assert_ok}; +use frame_support::{assert_noop, assert_ok, assert_storage_noop}; macro_rules! sub_pools_with_era { ($($k:expr => $v:expr),* $(,)?) => {{ @@ -2350,3 +2350,52 @@ mod set_metadata { }); } } + +mod set_configs { + use super::*; + + #[test] + fn set_configs_works() { + ExtBuilder::default().build_and_execute(|| { + // Setting works + assert_ok!(Pools::set_configs( + Origin::root(), + ConfigOp::Set(1 as Balance), + ConfigOp::Set(2 as Balance), + ConfigOp::Set(3u32), + ConfigOp::Set(4u32), + ConfigOp::Set(5u32), + )); + assert_eq!(MinJoinBond::::get(), 1); + assert_eq!(MinCreateBond::::get(), 2); + assert_eq!(MaxPools::::get(), Some(3)); + assert_eq!(MaxDelegators::::get(), Some(4)); + assert_eq!(MaxDelegatorsPerPool::::get(), Some(5)); + + // Noop does nothing + assert_storage_noop!(assert_ok!(Pools::set_configs( + Origin::root(), + ConfigOp::Noop, + ConfigOp::Noop, + ConfigOp::Noop, + ConfigOp::Noop, + ConfigOp::Noop, + ))); + + // Removing works + assert_ok!(Pools::set_configs( + Origin::root(), + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove, + ConfigOp::Remove + )); + assert_eq!(MinJoinBond::::get(), 0); + assert_eq!(MinCreateBond::::get(), 0); + assert_eq!(MaxPools::::get(), None); + assert_eq!(MaxDelegators::::get(), None); + assert_eq!(MaxDelegatorsPerPool::::get(), None); + }); + } +} diff --git a/frame/nomination-pools/src/weights.rs b/frame/nomination-pools/src/weights.rs index 373bc28b5d356..4468b39caacf9 100644 --- a/frame/nomination-pools/src/weights.rs +++ b/frame/nomination-pools/src/weights.rs @@ -53,6 +53,7 @@ pub trait WeightInfo { fn nominate() -> Weight; fn set_state_other() -> Weight; fn set_metadata() -> Weight; + fn set_configs() -> Weight; } /// Weights for pallet_nomination_pools using the Substrate node and recommended hardware. @@ -174,7 +175,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(28 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } - fn set_state_other() -> Weight { (79_587_000 as Weight) .saturating_add(T::DbWeight::get().reads(28 as Weight)) @@ -185,7 +185,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(28 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } - + fn set_configs() -> Weight { + (79_587_000 as Weight) + .saturating_add(T::DbWeight::get().reads(28 as Weight)) + .saturating_add(T::DbWeight::get().writes(5 as Weight)) + } } // For backwards compatibility and tests @@ -317,4 +321,9 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(28 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) } + fn set_configs() -> Weight { + (79_587_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(28 as Weight)) + .saturating_add(RocksDbWeight::get().writes(5 as Weight)) + } }