Revise how staking configurations are set

frame/staking/src/benchmarking.rs

@@ -18,7 +18,7 @@
 //! Staking pallet benchmarking.
 
 use super::*;
-use crate::Pallet as Staking;
+use crate::{ConfigOp, Pallet as Staking};
 use testing_utils::*;
 
 use codec::Decode;
@@ -852,16 +852,15 @@ benchmarks! {
 		assert_eq!(targets.len() as u32, v);
 	}
 
-	set_staking_configs {
-		// This function always does the same thing... just write to 4 storage items.
-	}: _(
+	set_staking_configs_all_set {
+	}: set_staking_configs(
 		RawOrigin::Root,
-		BalanceOf::<T>::max_value(),
-		BalanceOf::<T>::max_value(),
-		Some(u32::MAX),
-		Some(u32::MAX),
-		Some(Percent::max_value()),
-		Perbill::max_value()
+		ConfigOp::Set(BalanceOf::<T>::max_value()),
+		ConfigOp::Set(BalanceOf::<T>::max_value()),
+		ConfigOp::Set(Some(u32::MAX)),
+		ConfigOp::Set(Some(u32::MAX)),
+		ConfigOp::Set(Some(Percent::max_value())),
+		ConfigOp::Set(Perbill::max_value())
 	) verify {
 		assert_eq!(MinNominatorBond::<T>::get(), BalanceOf::<T>::max_value());
 		assert_eq!(MinValidatorBond::<T>::get(), BalanceOf::<T>::max_value());
@@ -871,6 +870,24 @@ benchmarks! {
 		assert_eq!(MinCommission::<T>::get(), Perbill::from_percent(100));
 	}
 
+	set_staking_configs_all_remove {
+	}: set_staking_configs(
+		RawOrigin::Root,
+		ConfigOp::Remove,
+		ConfigOp::Remove,
+		ConfigOp::Remove,
+		ConfigOp::Remove,
+		ConfigOp::Remove,
+		ConfigOp::Remove
+	) verify {
+		assert!(!MinNominatorBond::<T>::exists());
+		assert!(!MinValidatorBond::<T>::exists());
+		assert!(!MaxNominatorsCount::<T>::exists());
+		assert!(!MaxValidatorsCount::<T>::exists());
+		assert!(!ChillThreshold::<T>::exists());
+		assert!(!MinCommission::<T>::exists());
+	}
+
 	chill_other {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
@@ -886,12 +903,12 @@ benchmarks! {
 
 		Staking::<T>::set_staking_configs(
 			RawOrigin::Root.into(),
-			BalanceOf::<T>::max_value(),
-			BalanceOf::<T>::max_value(),
-			Some(0),
-			Some(0),
-			Some(Percent::from_percent(0)),
-			Zero::zero(),
+			ConfigOp::Set(BalanceOf::<T>::max_value()),
+			ConfigOp::Set(BalanceOf::<T>::max_value()),
+			ConfigOp::Set(Some(0)),
+			ConfigOp::Set(Some(0)),
+			ConfigOp::Set(Some(Percent::from_percent(0))),
+			ConfigOp::Set(Zero::zero()),
 		)?;
 
 		let caller = whitelisted_caller();

frame/staking/src/pallet/mod.rs

@@ -19,6 +19,7 @@
 
 use frame_election_provider_support::SortedListProvider;
 use frame_support::{
+	dispatch::Codec,
 	pallet_prelude::*,
 	traits::{
 		Currency, CurrencyToVote, EnsureOrigin, EstimateNextNewSession, Get, LockIdentifier,
@@ -32,7 +33,7 @@ use sp_runtime::{
 	DispatchError, Perbill, Percent,
 };
 use sp_staking::{EraIndex, SessionIndex};
-use sp_std::{convert::From, prelude::*};
+use sp_std::{cmp::max, convert::From, prelude::*};
 
 mod impls;
 
@@ -61,6 +62,16 @@ pub mod pallet {
 	#[pallet::without_storage_info]
 	pub struct Pallet<T>(_);
 
+	#[derive(TypeInfo, Debug, Clone, Encode, Decode, PartialEq)]
+	pub enum ConfigOp<T: Default + Codec> {
+		/// Don't change.
+		Noop,
+		/// Set the given value.
+		Set(T),
+		/// Remove from storage.
+		Remove,
+	}
+
 	#[pallet::config]
 	pub trait Config: frame_system::Config + SendTransactionTypes<Call<Self>> {
 		/// The staking balance.
@@ -1513,23 +1524,39 @@ pub mod pallet {
 		///
 		/// NOTE: Existing nominators and validators will not be affected by this update.
 		/// to kick people under the new limits, `chill_other` should be called.
-		#[pallet::weight(T::WeightInfo::set_staking_configs())]
+		// We assume the worst case for this call is either: all items are set or all items are
+		// removed.
+		#[pallet::weight(max(
+			T::WeightInfo::set_staking_configs_all_set(),
+			T::WeightInfo::set_staking_configs_all_remove()
+		))]
 		pub fn set_staking_configs(
 			origin: OriginFor<T>,
-			min_nominator_bond: BalanceOf<T>,
-			min_validator_bond: BalanceOf<T>,
-			max_nominator_count: Option<u32>,
-			max_validator_count: Option<u32>,
-			chill_threshold: Option<Percent>,
-			min_commission: Perbill,
+			min_nominator_bond: ConfigOp<BalanceOf<T>>,
+			min_validator_bond: ConfigOp<BalanceOf<T>>,
+			max_nominator_count: ConfigOp<Option<u32>>,
+			max_validator_count: ConfigOp<Option<u32>>,
+			chill_threshold: ConfigOp<Option<Percent>>,
+			min_commission: ConfigOp<Perbill>,
 		) -> DispatchResult {
 			ensure_root(origin)?;
-			MinNominatorBond::<T>::set(min_nominator_bond);
-			MinValidatorBond::<T>::set(min_validator_bond);
-			MaxNominatorsCount::<T>::set(max_nominator_count);
-			MaxValidatorsCount::<T>::set(max_validator_count);
-			ChillThreshold::<T>::set(chill_threshold);
-			MinCommission::<T>::set(min_commission);
+
+			macro_rules! config_op_exp {
+				($storage:ty, $op:ident) => {
+					match $op {
+						ConfigOp::Noop => (),
+						ConfigOp::Set(v) => <$storage>::set(v),
+						ConfigOp::Remove => <$storage>::kill(),
+					}
+				};
+			}
+
+			config_op_exp!(MinNominatorBond<T>, min_nominator_bond);
+			config_op_exp!(MinValidatorBond<T>, min_validator_bond);
+			config_op_exp!(MaxNominatorsCount<T>, max_nominator_count);
+			config_op_exp!(MaxValidatorsCount<T>, max_validator_count);
+			config_op_exp!(ChillThreshold<T>, chill_threshold);
+			config_op_exp!(MinCommission<T>, min_commission);
 			Ok(())
 		}