From dbef222b54dc4321b72157dac357256188e2a8c1 Mon Sep 17 00:00:00 2001 From: girazoki Date: Fri, 9 Feb 2024 10:15:24 +0100 Subject: [PATCH] check keys registered in add_invulnerable too (#405) * check keys registered in invulnerables too * new weights * typscript api --- pallets/invulnerables/src/benchmarking.rs | 8 +++- pallets/invulnerables/src/lib.rs | 10 ++++ pallets/invulnerables/src/tests.rs | 14 ++++++ pallets/invulnerables/src/weights.rs | 48 ++++++++++--------- .../dancebox/interfaces/augment-api-errors.ts | 2 + .../src/dancebox/interfaces/lookup.ts | 2 +- .../src/dancebox/interfaces/types-lookup.ts | 3 +- .../flashbox/interfaces/augment-api-errors.ts | 2 + .../src/flashbox/interfaces/lookup.ts | 2 +- .../src/flashbox/interfaces/types-lookup.ts | 3 +- 10 files changed, 67 insertions(+), 27 deletions(-) diff --git a/pallets/invulnerables/src/benchmarking.rs b/pallets/invulnerables/src/benchmarking.rs index 4665b9f1d..9ecf48fa5 100644 --- a/pallets/invulnerables/src/benchmarking.rs +++ b/pallets/invulnerables/src/benchmarking.rs @@ -146,7 +146,13 @@ mod benchmarks { frame_support::BoundedVec::try_from(invulnerables).unwrap(); >::put(invulnerables); - let new_invulnerable = invulnerable::(b + 1).0; + let (new_invulnerable, keys) = invulnerable::(b + 1); + >::set_keys( + RawOrigin::Signed(new_invulnerable.clone()).into(), + keys, + Vec::new(), + ) + .unwrap(); #[extrinsic_call] _(origin as T::RuntimeOrigin, new_invulnerable.clone()); diff --git a/pallets/invulnerables/src/lib.rs b/pallets/invulnerables/src/lib.rs index 57695c468..a988839f3 100644 --- a/pallets/invulnerables/src/lib.rs +++ b/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::::NoKeysRegistered + ); >::try_mutate(|invulnerables| -> DispatchResult { if invulnerables.contains(&who) { diff --git a/pallets/invulnerables/src/tests.rs b/pallets/invulnerables/src/tests.rs index c591208c4..b2fa54724 100644 --- a/pallets/invulnerables/src/tests.rs +++ b/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::::NoKeysRegistered + ); + }); +} + #[test] fn invulnerable_limit_works() { new_test_ext().execute_with(|| { diff --git a/pallets/invulnerables/src/weights.rs b/pallets/invulnerables/src/weights.rs index 367d83f0a..96b7afb97 100644 --- a/pallets/invulnerables/src/weights.rs +++ b/pallets/invulnerables/src/weights.rs @@ -74,20 +74,22 @@ impl WeightInfo for SubstrateWeight { .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) diff --git a/typescript-api/src/dancebox/interfaces/augment-api-errors.ts b/typescript-api/src/dancebox/interfaces/augment-api-errors.ts index 1b3864e99..710ad7c0d 100644 --- a/typescript-api/src/dancebox/interfaces/augment-api-errors.ts +++ b/typescript-api/src/dancebox/interfaces/augment-api-errors.ts @@ -189,6 +189,8 @@ declare module "@polkadot/api-base/types/errors" { invulnerables: { /** Account is already an Invulnerable. */ AlreadyInvulnerable: AugmentedError; + /** Account does not have keys registered */ + NoKeysRegistered: AugmentedError; /** Account is not an Invulnerable. */ NotInvulnerable: AugmentedError; /** There are too many Invulnerables. */ diff --git a/typescript-api/src/dancebox/interfaces/lookup.ts b/typescript-api/src/dancebox/interfaces/lookup.ts index 8cd19ffdc..a08cef33c 100644 --- a/typescript-api/src/dancebox/interfaces/lookup.ts +++ b/typescript-api/src/dancebox/interfaces/lookup.ts @@ -3256,7 +3256,7 @@ export default { }, /** Lookup383: pallet_invulnerables::pallet::Error */ PalletInvulnerablesError: { - _enum: ["TooManyInvulnerables", "AlreadyInvulnerable", "NotInvulnerable"], + _enum: ["TooManyInvulnerables", "AlreadyInvulnerable", "NotInvulnerable", "NoKeysRegistered"], }, /** Lookup388: sp_core::crypto::KeyTypeId */ SpCoreCryptoKeyTypeId: "[u8;4]", diff --git a/typescript-api/src/dancebox/interfaces/types-lookup.ts b/typescript-api/src/dancebox/interfaces/types-lookup.ts index 2b4d41b84..ab63c25ef 100644 --- a/typescript-api/src/dancebox/interfaces/types-lookup.ts +++ b/typescript-api/src/dancebox/interfaces/types-lookup.ts @@ -4383,7 +4383,8 @@ declare module "@polkadot/types/lookup" { readonly isTooManyInvulnerables: boolean; readonly isAlreadyInvulnerable: boolean; readonly isNotInvulnerable: boolean; - readonly type: "TooManyInvulnerables" | "AlreadyInvulnerable" | "NotInvulnerable"; + readonly isNoKeysRegistered: boolean; + readonly type: "TooManyInvulnerables" | "AlreadyInvulnerable" | "NotInvulnerable" | "NoKeysRegistered"; } /** @name SpCoreCryptoKeyTypeId (388) */ diff --git a/typescript-api/src/flashbox/interfaces/augment-api-errors.ts b/typescript-api/src/flashbox/interfaces/augment-api-errors.ts index 70a91cd2a..5199c7b7c 100644 --- a/typescript-api/src/flashbox/interfaces/augment-api-errors.ts +++ b/typescript-api/src/flashbox/interfaces/augment-api-errors.ts @@ -112,6 +112,8 @@ declare module "@polkadot/api-base/types/errors" { invulnerables: { /** Account is already an Invulnerable. */ AlreadyInvulnerable: AugmentedError; + /** Account does not have keys registered */ + NoKeysRegistered: AugmentedError; /** Account is not an Invulnerable. */ NotInvulnerable: AugmentedError; /** There are too many Invulnerables. */ diff --git a/typescript-api/src/flashbox/interfaces/lookup.ts b/typescript-api/src/flashbox/interfaces/lookup.ts index 44c3bc07b..474c11ca7 100644 --- a/typescript-api/src/flashbox/interfaces/lookup.ts +++ b/typescript-api/src/flashbox/interfaces/lookup.ts @@ -1606,7 +1606,7 @@ export default { }, /** Lookup287: pallet_invulnerables::pallet::Error */ PalletInvulnerablesError: { - _enum: ["TooManyInvulnerables", "AlreadyInvulnerable", "NotInvulnerable"], + _enum: ["TooManyInvulnerables", "AlreadyInvulnerable", "NotInvulnerable", "NoKeysRegistered"], }, /** Lookup292: sp_core::crypto::KeyTypeId */ SpCoreCryptoKeyTypeId: "[u8;4]", diff --git a/typescript-api/src/flashbox/interfaces/types-lookup.ts b/typescript-api/src/flashbox/interfaces/types-lookup.ts index ae0ebf4eb..b04496346 100644 --- a/typescript-api/src/flashbox/interfaces/types-lookup.ts +++ b/typescript-api/src/flashbox/interfaces/types-lookup.ts @@ -2125,7 +2125,8 @@ declare module "@polkadot/types/lookup" { readonly isTooManyInvulnerables: boolean; readonly isAlreadyInvulnerable: boolean; readonly isNotInvulnerable: boolean; - readonly type: "TooManyInvulnerables" | "AlreadyInvulnerable" | "NotInvulnerable"; + readonly isNoKeysRegistered: boolean; + readonly type: "TooManyInvulnerables" | "AlreadyInvulnerable" | "NotInvulnerable" | "NoKeysRegistered"; } /** @name SpCoreCryptoKeyTypeId (292) */