add checks to signer changes

crates/shared/src/constants.rs

@@ -64,6 +64,11 @@ pub const NETWORK_PARENT_KEY: &str = "NETWORK_PARENT_KEY_FOR_ENTROPY_";
 /// Total signers on the network with the parent key
 pub const TOTAL_SIGNERS: u8 = 3;
 
+/// Max signers, for bounding of total signers for benches,
+/// Can be changed but requires a re-run of benches
+pub const MAX_SIGNERS: u8 = 15;
+
+
 /// Threshold for those signers
 pub const SIGNER_THRESHOLD: u8 = 2;
 

pallets/parameters/Cargo.toml

@@ -17,6 +17,11 @@ frame-support     ={ version="29.0.0", default-features=false }
 frame-system      ={ version="29.0.0", default-features=false }
 sp-runtime        ={ version="32.0.0", default-features=false }
 sp-std            ={ version="14.0.0", default-features=false }
+pallet-session    ={ version="29.0.0", default-features=false }
+
+entropy-shared={ version="0.2.0", path="../../crates/shared", features=[
+  "wasm-no-std",
+], default-features=false }
 
 [dev-dependencies]
 sp-core={ version="29.0.0" }
@@ -32,6 +37,7 @@ runtime-benchmarks=[
 std=[
   "frame-support/std",
   "frame-system/std",
+  "pallet-session/std",
   "scale-info/std",
   "sp-runtime/std",
   "sp-std/std",

pallets/parameters/src/lib.rs

@@ -33,6 +33,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![allow(clippy::unused_unit)]
 
+use entropy_shared::MAX_SIGNERS;
 use frame_support::pallet_prelude::*;
 use frame_system::pallet_prelude::*;
 use sp_runtime::DispatchResult;
@@ -56,7 +57,7 @@ pub mod module {
     use super::*;
 
     #[pallet::config]
-    pub trait Config: frame_system::Config {
+    pub trait Config: frame_system::Config + pallet_session::Config {
         type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 
         /// The origin which may set filter.
@@ -84,8 +85,11 @@ pub mod module {
             assert!(self.total_signers >= self.threshold, "Threshold is larger then signer");
             RequestLimit::<T>::put(self.request_limit);
             MaxInstructionsPerPrograms::<T>::put(self.max_instructions_per_programs);
-            let signer_info =
-                SignersSize { total_signers: self.total_signers, threshold: self.threshold };
+            let signer_info = SignersSize {
+                total_signers: self.total_signers,
+                threshold: self.threshold,
+                last_session_change: 0,
+            };
             SignersInfo::<T>::put(signer_info);
         }
     }
@@ -96,12 +100,19 @@ pub mod module {
         ThresholdGreaterThenSigners,
         /// Threhsold has to be more than 0
         ThrehsoldTooLow,
+        /// Signers over max signers, can happen however needs a benchmark rerun
+        TooManySigners,
+        /// Signers can only change by one at a time
+        SignerDiffTooLarge,
+        /// Can only do one change per session
+        OneChangePerSession,
     }
 
     #[derive(Clone, Encode, Decode, Eq, PartialEqNoBound, RuntimeDebug, TypeInfo, Default)]
     pub struct SignersSize {
         pub threshold: u8,
         pub total_signers: u8,
+        pub last_session_change: u32,
     }
 
     #[pallet::event]
@@ -137,7 +148,7 @@ pub mod module {
     #[pallet::call]
     impl<T: Config> Pallet<T> {
         #[pallet::call_index(0)]
-        #[pallet::weight(T::WeightInfo::change_request_limit())]
+        #[pallet::weight( <T as Config>::WeightInfo::change_request_limit())]
         pub fn change_request_limit(origin: OriginFor<T>, request_limit: u32) -> DispatchResult {
             T::UpdateOrigin::ensure_origin(origin)?;
             RequestLimit::<T>::put(request_limit);
@@ -146,7 +157,7 @@ pub mod module {
         }
 
         #[pallet::call_index(1)]
-        #[pallet::weight(T::WeightInfo::max_instructions_per_programs())]
+        #[pallet::weight( <T as Config>::WeightInfo::max_instructions_per_programs())]
         pub fn change_max_instructions_per_programs(
             origin: OriginFor<T>,
             max_instructions_per_programs: u64,
@@ -161,7 +172,7 @@ pub mod module {
 
         /// Changes the threshold related parameters for signing.
         #[pallet::call_index(2)]
-        #[pallet::weight(T::WeightInfo::change_signers_info())]
+        #[pallet::weight( <T as Config>::WeightInfo::change_signers_info())]
         pub fn change_signers_info(
             origin: OriginFor<T>,
             total_signers: u8,
@@ -170,7 +181,19 @@ pub mod module {
             T::UpdateOrigin::ensure_origin(origin)?;
             ensure!(total_signers >= threshold, Error::<T>::ThresholdGreaterThenSigners);
             ensure!(threshold > 0, Error::<T>::ThrehsoldTooLow);
-            let signer_info = SignersSize { total_signers, threshold };
+            ensure!(total_signers <= MAX_SIGNERS, Error::<T>::TooManySigners);
+            let old_signer_info = Self::signers_info();
+            ensure!(
+                old_signer_info.total_signers.abs_diff(total_signers) > 1,
+                Error::<T>::SignerDiffTooLarge
+            );
+            let current_session = pallet_session::Pallet::<T>::current_index();
+            ensure!(
+                current_session > old_signer_info.last_session_change,
+                Error::<T>::OneChangePerSession
+            );
+            let signer_info =
+                SignersSize { total_signers, threshold, last_session_change: current_session };
             SignersInfo::<T>::put(&signer_info);
             Self::deposit_event(Event::SignerInfoChanged { signer_info });
             Ok(())

pallets/parameters/src/tests.rs

@@ -17,6 +17,7 @@
 
 #![cfg(test)]
 
+use entropy_shared::MAX_SIGNERS;
 use frame_support::{assert_noop, assert_ok};
 use mock::*;
 use sp_runtime::traits::BadOrigin;
@@ -99,5 +100,11 @@ fn signer_info_changed() {
             Parameters::change_signers_info(RuntimeOrigin::root(), 0, 0),
             Error::<Runtime>::ThrehsoldTooLow,
         );
+
+        // Fails too many signers
+        assert_noop!(
+            Parameters::change_signers_info(RuntimeOrigin::root(), MAX_SIGNERS + 1, 1),
+            Error::<Runtime>::TooManySigners,
+        );
     });
 }

pallets/registry/src/benchmarking.rs

@@ -14,7 +14,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 //! Benchmarking setup for pallet-propgation
-use entropy_shared::{TOTAL_SIGNERS, VERIFICATION_KEY_LENGTH};
+use entropy_shared::{MAX_SIGNERS, VERIFICATION_KEY_LENGTH};
 use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
 use frame_support::{
     traits::{Currency, Get},
@@ -79,20 +79,20 @@ benchmarks! {
   }
 
   confirm_jump_start_done {
-    let c in 0 .. TOTAL_SIGNERS as u32;
+    let c in 0 .. MAX_SIGNERS as u32;
     let sig_req_account: T::AccountId = whitelisted_caller();
     let validator_account: T::AccountId = whitelisted_caller();
     let expected_verifying_key = BoundedVec::default();
 
     let mut accounts = vec![];
-    for i in 0..TOTAL_SIGNERS {
+    for i in 0..MAX_SIGNERS {
         accounts.push(account::<T::AccountId>("ts_account", i as u32, SEED));
     }
 
-    let validators = add_non_syncing_validators::<T>(TOTAL_SIGNERS as u32, 0);
+    let validators = add_non_syncing_validators::<T>(MAX_SIGNERS as u32, 0);
     <Validators<T>>::set(validators.clone());
 
-    for i in 0..TOTAL_SIGNERS {
+    for i in 0..MAX_SIGNERS {
         <ThresholdToStash<T>>::insert(accounts[i as usize].clone(), &validators[i as usize]);
     }
 
@@ -112,15 +112,15 @@ benchmarks! {
   }
 
   confirm_jump_start_confirm {
-    let c in 0 .. TOTAL_SIGNERS as u32;
+    let c in 0 .. MAX_SIGNERS as u32;
     let sig_req_account: T::AccountId = whitelisted_caller();
     let validator_account: T::AccountId = whitelisted_caller();
     let threshold_account: T::AccountId = whitelisted_caller();
     let expected_verifying_key = BoundedVec::default();
 
     // add validators and a registering user
-    for i in 0..TOTAL_SIGNERS {
-        let validators = add_non_syncing_validators::<T>(TOTAL_SIGNERS as u32, 0);
+    for i in 0..MAX_SIGNERS {
+        let validators = add_non_syncing_validators::<T>(MAX_SIGNERS as u32, 0);
         <Validators<T>>::set(validators.clone());
         <ThresholdToStash<T>>::insert(&threshold_account, &validators[i as usize]);
     }

pallets/registry/src/lib.rs

@@ -53,7 +53,7 @@ pub mod weights;
 
 #[frame_support::pallet]
 pub mod pallet {
-    use entropy_shared::{NETWORK_PARENT_KEY, TOTAL_SIGNERS, VERIFICATION_KEY_LENGTH};
+    use entropy_shared::{NETWORK_PARENT_KEY, TOTAL_SIGNERS, MAX_SIGNERS, VERIFICATION_KEY_LENGTH};
     use frame_support::{
         dispatch::{DispatchResultWithPostInfo, Pays},
         pallet_prelude::*,
@@ -286,8 +286,8 @@ pub mod pallet {
         /// Allows validators to signal a successful network jumpstart
         #[pallet::call_index(1)]
         #[pallet::weight({
-                <T as Config>::WeightInfo::confirm_jump_start_confirm(TOTAL_SIGNERS as u32)
-                .max(<T as Config>::WeightInfo::confirm_jump_start_done(TOTAL_SIGNERS as u32))
+                <T as Config>::WeightInfo::confirm_jump_start_confirm(MAX_SIGNERS as u32)
+                .max(<T as Config>::WeightInfo::confirm_jump_start_done(MAX_SIGNERS as u32))
         })]
         pub fn confirm_jump_start(
             origin: OriginFor<T>,
@@ -327,7 +327,8 @@ pub mod pallet {
             // ensure that registration was indeed successful.
             //
             // If it fails we'll need to allow another jumpstart.
-            if jump_start_info.confirmations.len() == (TOTAL_SIGNERS as usize - 1) {
+            let signers_amount = pallet_parameters::Pallet::<T>::signers_info().total_signers;
+            if jump_start_info.confirmations.len() == (signers_amount as usize - 1) {
                 // registration finished, lock call
                 jump_start_info.confirmations.push(validator_stash);
                 let confirmations = jump_start_info.confirmations.len();

pallets/staking/src/benchmarking.rs

@@ -15,7 +15,7 @@
 
 //! Benchmarking setup for pallet-propgation
 #![allow(unused_imports)]
-use entropy_shared::TOTAL_SIGNERS;
+use entropy_shared::MAX_SIGNERS;
 use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
 use frame_support::{
     assert_ok, ensure,
@@ -178,10 +178,10 @@ benchmarks! {
   }
 
   confirm_key_reshare_confirmed {
-    let c in 0 .. TOTAL_SIGNERS as u32;
+    let c in 0 .. MAX_SIGNERS as u32;
     // leave a space for two as not to rotate and only confirm rotation
     let confirmation_num = c.checked_sub(2).unwrap_or(0);
-    let signer_num =  TOTAL_SIGNERS - 1;
+    let signer_num =  MAX_SIGNERS - 1;
     let caller: T::AccountId = whitelisted_caller();
     let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();
     let second_signer: T::AccountId = account("second_signer", 0, SEED);
@@ -205,7 +205,7 @@ benchmarks! {
 
   confirm_key_reshare_completed {
     // once less confirmation to always flip to rotate
-    let confirmation_num = TOTAL_SIGNERS as usize - 1;
+    let confirmation_num = MAX_SIGNERS as usize - 1;
 
     let caller: T::AccountId = whitelisted_caller();
     let validator_id_res = <T as pallet_session::Config>::ValidatorId::try_from(caller.clone()).or(Err(Error::<T>::InvalidValidatorId)).unwrap();

pallets/staking/src/lib.rs

@@ -58,7 +58,7 @@ use sp_staking::SessionIndex;
 #[frame_support::pallet]
 pub mod pallet {
     use entropy_shared::{
-        ValidatorInfo, X25519PublicKey, TEST_RESHARE_BLOCK_NUMBER, TOTAL_SIGNERS,
+        ValidatorInfo, X25519PublicKey, TEST_RESHARE_BLOCK_NUMBER, MAX_SIGNERS,
         VERIFICATION_KEY_LENGTH,
     };
     use frame_support::{
@@ -480,7 +480,7 @@ pub mod pallet {
 
         #[pallet::call_index(5)]
         #[pallet::weight(({
-            <T as Config>::WeightInfo::confirm_key_reshare_confirmed(TOTAL_SIGNERS as u32)
+            <T as Config>::WeightInfo::confirm_key_reshare_confirmed(MAX_SIGNERS as u32)
             .max(<T as Config>::WeightInfo::confirm_key_reshare_completed())
     }, DispatchClass::Operational))]
         pub fn confirm_key_reshare(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
@@ -513,6 +513,7 @@ pub mod pallet {
                 Self::deposit_event(Event::SignerConfirmed(validator_stash));
                 Ok(Pays::No.into())
             }
+            // TODO weight is pays no but want a more accurate weight for max signers vs current signers
         }
     }