check keys registered in add_invulnerable too

pallets/invulnerables/src/benchmarking.rs

@@ -146,7 +146,13 @@ mod benchmarks {
             frame_support::BoundedVec::try_from(invulnerables).unwrap();
         <Invulnerables<T>>::put(invulnerables);
 
-        let new_invulnerable = invulnerable::<T>(b + 1).0;
+        let (new_invulnerable, keys) = invulnerable::<T>(b + 1);
+        <session::Pallet<T>>::set_keys(
+            RawOrigin::Signed(new_invulnerable.clone()).into(),
+            keys,
+            Vec::new(),
+        )
+        .unwrap();
 
         #[extrinsic_call]
         _(origin as T::RuntimeOrigin, new_invulnerable.clone());

pallets/invulnerables/src/lib.rs

@@ -160,6 +160,8 @@ pub mod pallet {
         AlreadyInvulnerable,
         /// Account is not an Invulnerable.
         NotInvulnerable,
+        /// Account does not have keys registered
+        NoKeysRegistered,
     }
 
     #[pallet::call]
@@ -221,6 +223,14 @@ pub mod pallet {
             who: T::AccountId,
         ) -> DispatchResultWithPostInfo {
             T::UpdateOrigin::ensure_origin(origin)?;
+            // don't let one unprepared collator ruin things for everyone.
+            let collator_id = T::CollatorIdOf::convert(who.clone());
+
+            // Ensure it has keys registered
+            ensure!(
+                collator_id.map_or(false, |key| T::CollatorRegistration::is_registered(&key)),
+                Error::<T>::NoKeysRegistered
+            );
 
             <Invulnerables<T>>::try_mutate(|invulnerables| -> DispatchResult {
                 if invulnerables.contains(&who) {

pallets/invulnerables/src/tests.rs

@@ -85,6 +85,20 @@ fn add_invulnerable_works() {
     });
 }
 
+#[test]
+fn add_invulnerable_does_not_work_if_not_registered() {
+    new_test_ext().execute_with(|| {
+        initialize_to_block(1);
+        assert_eq!(Invulnerables::invulnerables(), vec![1, 2]);
+        let new = 42;
+
+        assert_noop!(
+            Invulnerables::add_invulnerable(RuntimeOrigin::signed(RootAccount::get()), new),
+            Error::<Test>::NoKeysRegistered
+        );
+    });
+}
+
 #[test]
 fn invulnerable_limit_works() {
     new_test_ext().execute_with(|| {

pallets/invulnerables/src/weights.rs

@@ -74,20 +74,22 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 			.saturating_add(Weight::from_parts(56_720, 0).saturating_mul(b.into()))
 			.saturating_add(T::DbWeight::get().writes(1))
 	}
-	/// Storage: Invulnerables Invulnerables (r:1 w:1)
-	/// Proof: Invulnerables Invulnerables (max_values: Some(1), max_size: Some(3202), added: 3697, mode: MaxEncodedLen)
+	/// Storage: `Session::NextKeys` (r:1 w:0)
+	/// Proof: `Session::NextKeys` (`max_values`: None, `max_size`: None, mode: `Measured`)
+	/// Storage: `Invulnerables::Invulnerables` (r:1 w:1)
+	/// Proof: `Invulnerables::Invulnerables` (`max_values`: Some(1), `max_size`: Some(3202), added: 3697, mode: `MaxEncodedLen`)
 	/// The range of component `b` is `[1, 99]`.
 	fn add_invulnerable(b: u32, ) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `70 + b * (32 ±0)`
-		//  Estimated: `4687`
-		// Minimum execution time: 6_840_000 picoseconds.
-		Weight::from_parts(7_533_824, 0)
-			.saturating_add(Weight::from_parts(0, 4687))
-			// Standard Error: 384
-			.saturating_add(Weight::from_parts(19_372, 0).saturating_mul(b.into()))
-			.saturating_add(T::DbWeight::get().reads(1))
-			.saturating_add(T::DbWeight::get().writes(1))
+		//  Measured:  `548 + b * (36 ±0)`
+		//  Estimated: `4687 + b * (37 ±0)`
+		// Minimum execution time: 16_117_000 picoseconds.
+		Weight::from_parts(18_327_160, 4687)
+			// Standard Error: 1_293
+			.saturating_add(Weight::from_parts(94_608, 0).saturating_mul(b.into()))
+			.saturating_add(T::DbWeight::get().reads(2_u64))
+			.saturating_add(T::DbWeight::get().writes(1_u64))
+			.saturating_add(Weight::from_parts(0, 37).saturating_mul(b.into()))
 	}
 	/// Storage: Invulnerables Invulnerables (r:1 w:1)
 	/// Proof: Invulnerables Invulnerables (max_values: Some(1), max_size: Some(3202), added: 3697, mode: MaxEncodedLen)
@@ -155,20 +157,22 @@ impl WeightInfo for () {
 			.saturating_add(Weight::from_parts(56_720, 0).saturating_mul(b.into()))
 			.saturating_add(RocksDbWeight::get().writes(1))
 	}
-	/// Storage: Invulnerables Invulnerables (r:1 w:1)
-	/// Proof: Invulnerables Invulnerables (max_values: Some(1), max_size: Some(3202), added: 3697, mode: MaxEncodedLen)
+	/// Storage: `Session::NextKeys` (r:1 w:0)
+	/// Proof: `Session::NextKeys` (`max_values`: None, `max_size`: None, mode: `Measured`)
+	/// Storage: `Invulnerables::Invulnerables` (r:1 w:1)
+	/// Proof: `Invulnerables::Invulnerables` (`max_values`: Some(1), `max_size`: Some(3202), added: 3697, mode: `MaxEncodedLen`)
 	/// The range of component `b` is `[1, 99]`.
 	fn add_invulnerable(b: u32, ) -> Weight {
 		// Proof Size summary in bytes:
-		//  Measured:  `70 + b * (32 ±0)`
-		//  Estimated: `4687`
-		// Minimum execution time: 6_840_000 picoseconds.
-		Weight::from_parts(7_533_824, 0)
-			.saturating_add(Weight::from_parts(0, 4687))
-			// Standard Error: 384
-			.saturating_add(Weight::from_parts(19_372, 0).saturating_mul(b.into()))
-			.saturating_add(RocksDbWeight::get().reads(1))
-			.saturating_add(RocksDbWeight::get().writes(1))
+		//  Measured:  `548 + b * (36 ±0)`
+		//  Estimated: `4687 + b * (37 ±0)`
+		// Minimum execution time: 16_117_000 picoseconds.
+		Weight::from_parts(18_327_160, 4687)
+			// Standard Error: 1_293
+			.saturating_add(Weight::from_parts(94_608, 0).saturating_mul(b.into()))
+			.saturating_add(RocksDbWeight::get().reads(2_u64))
+			.saturating_add(RocksDbWeight::get().writes(1_u64))
+			.saturating_add(Weight::from_parts(0, 37).saturating_mul(b.into()))
 	}
 	/// Storage: Invulnerables Invulnerables (r:1 w:1)
 	/// Proof: Invulnerables Invulnerables (max_values: Some(1), max_size: Some(3202), added: 3697, mode: MaxEncodedLen)