From 55b720fcb5845d0e220cf57c719014b461c1dd27 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Tue, 25 Feb 2025 15:50:59 +0100 Subject: [PATCH 1/9] adjust core-fellowship benchmarks to allow for MaxRank=1 --- .../frame/core-fellowship/src/benchmarking.rs | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/substrate/frame/core-fellowship/src/benchmarking.rs b/substrate/frame/core-fellowship/src/benchmarking.rs index ac0d489953c1f..609499f4bc15f 100644 --- a/substrate/frame/core-fellowship/src/benchmarking.rs +++ b/substrate/frame/core-fellowship/src/benchmarking.rs @@ -149,19 +149,20 @@ mod benchmarks { fn bump_demote() -> Result<(), BenchmarkError> { set_benchmark_params::()?; - let member = make_member::(2)?; + let member = make_member::(T::MaxRank::get().try_into().unwrap())?; // Set it to the max value to ensure that any possible auto-demotion period has passed. frame_system::Pallet::::set_block_number(BlockNumberFor::::max_value()); ensure_evidence::(&member)?; assert!(Member::::contains_key(&member)); - assert_eq!(T::Members::rank_of(&member), Some(2)); + assert_eq!(T::Members::rank_of(&member), Some(T::MaxRank::get().try_into().unwrap())); #[extrinsic_call] CoreFellowship::::bump(RawOrigin::Signed(member.clone()), member.clone()); assert!(Member::::contains_key(&member)); - assert_eq!(T::Members::rank_of(&member), Some(1)); + let _new_rank = T::MaxRank::get() as usize; + assert_eq!(T::Members::rank_of(&member), Some(_new_rank.saturating_sub(1) as u16)); assert!(!MemberEvidence::::contains_key(&member)); Ok(()) } @@ -198,16 +199,23 @@ mod benchmarks { params.min_promotion_period = BoundedVec::try_from(vec![Zero::zero(); max_rank]).unwrap(); Params::::put(¶ms); - let member = make_member::(1)?; - - // Set it to the max value to ensure that any possible auto-demotion period has passed. + // Start at rank 0 to allow at least one promotion. + let current_rank = 0; + let member = make_member::(current_rank)?; + + // Set `to_rank` dynamically based on `max_rank`. + let max_rank = T::MaxRank::get(); + let to_rank = (current_rank + 1).min(max_rank.try_into().unwrap()); // Ensure `to_rank` <= `max_rank`. + + // Set block number to avoid auto-demotion. frame_system::Pallet::::set_block_number(BlockNumberFor::::max_value()); ensure_evidence::(&member)?; - + #[extrinsic_call] - _(RawOrigin::Root, member.clone(), 2u8.into()); - - assert_eq!(T::Members::rank_of(&member), Some(2)); + _(RawOrigin::Root, member.clone(), to_rank); + + // Assert the new rank matches `to_rank` (not a hardcoded value). + assert_eq!(T::Members::rank_of(&member), Some(to_rank)); assert!(!MemberEvidence::::contains_key(&member)); Ok(()) } @@ -215,8 +223,12 @@ mod benchmarks { /// Benchmark the `promote_fast` extrinsic to promote someone up to `r`. #[benchmark] fn promote_fast(r: Linear<1, { T::MaxRank::get() as u32 }>) -> Result<(), BenchmarkError> { + let max_rank = T::MaxRank::get() as u32; + let r = r.min(max_rank); let r = r.try_into().expect("r is too large"); + let member = make_member::(0)?; + log::info!("Benchmark promote_fast: r={} MaxRank={}", r, T::MaxRank::get()); ensure_evidence::(&member)?; From 3481b97a79c70fda638b9f2eb6b7c79dfbe43f77 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 2 Mar 2025 14:29:37 +0100 Subject: [PATCH 2/9] compe dynamic clamping --- .../frame/core-fellowship/src/benchmarking.rs | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/substrate/frame/core-fellowship/src/benchmarking.rs b/substrate/frame/core-fellowship/src/benchmarking.rs index 609499f4bc15f..c747e1a7d0823 100644 --- a/substrate/frame/core-fellowship/src/benchmarking.rs +++ b/substrate/frame/core-fellowship/src/benchmarking.rs @@ -21,6 +21,7 @@ use super::*; use crate::Pallet as CoreFellowship; +use sp_runtime::SaturatedConversion; use alloc::{boxed::Box, vec}; use frame_benchmarking::v2::*; @@ -149,20 +150,23 @@ mod benchmarks { fn bump_demote() -> Result<(), BenchmarkError> { set_benchmark_params::()?; - let member = make_member::(T::MaxRank::get().try_into().unwrap())?; + let max_rank: RankOf = T::MaxRank::get().try_into().unwrap(); + let initial_rank = max_rank.saturated_into::(); + + let member = make_member::(max_rank)?; // Set it to the max value to ensure that any possible auto-demotion period has passed. frame_system::Pallet::::set_block_number(BlockNumberFor::::max_value()); ensure_evidence::(&member)?; + assert!(Member::::contains_key(&member)); - assert_eq!(T::Members::rank_of(&member), Some(T::MaxRank::get().try_into().unwrap())); + assert_eq!(T::Members::rank_of(&member), Some(initial_rank)); #[extrinsic_call] CoreFellowship::::bump(RawOrigin::Signed(member.clone()), member.clone()); assert!(Member::::contains_key(&member)); - let _new_rank = T::MaxRank::get() as usize; - assert_eq!(T::Members::rank_of(&member), Some(_new_rank.saturating_sub(1) as u16)); + assert_eq!(T::Members::rank_of(&member), Some(initial_rank.saturating_sub(1))); assert!(!MemberEvidence::::contains_key(&member)); Ok(()) } @@ -196,24 +200,25 @@ mod benchmarks { // Ensure that the `min_promotion_period` wont get in our way. let mut params = Params::::get(); let max_rank = T::MaxRank::get().try_into().unwrap(); + + // Get minimum promotion period. params.min_promotion_period = BoundedVec::try_from(vec![Zero::zero(); max_rank]).unwrap(); Params::::put(¶ms); // Start at rank 0 to allow at least one promotion. - let current_rank = 0; + let current_rank = 0; let member = make_member::(current_rank)?; - + // Set `to_rank` dynamically based on `max_rank`. - let max_rank = T::MaxRank::get(); let to_rank = (current_rank + 1).min(max_rank.try_into().unwrap()); // Ensure `to_rank` <= `max_rank`. - + // Set block number to avoid auto-demotion. frame_system::Pallet::::set_block_number(BlockNumberFor::::max_value()); ensure_evidence::(&member)?; - + #[extrinsic_call] _(RawOrigin::Root, member.clone(), to_rank); - + // Assert the new rank matches `to_rank` (not a hardcoded value). assert_eq!(T::Members::rank_of(&member), Some(to_rank)); assert!(!MemberEvidence::::contains_key(&member)); @@ -223,19 +228,20 @@ mod benchmarks { /// Benchmark the `promote_fast` extrinsic to promote someone up to `r`. #[benchmark] fn promote_fast(r: Linear<1, { T::MaxRank::get() as u32 }>) -> Result<(), BenchmarkError> { - let max_rank = T::MaxRank::get() as u32; - let r = r.min(max_rank); - let r = r.try_into().expect("r is too large"); - + // Get target rank for promotion. + let max_rank = T::MaxRank::get(); + let target_rank = r.min(max_rank).try_into().unwrap(); + + // Begin with Candidate. let member = make_member::(0)?; log::info!("Benchmark promote_fast: r={} MaxRank={}", r, T::MaxRank::get()); ensure_evidence::(&member)?; #[extrinsic_call] - _(RawOrigin::Root, member.clone(), r); + _(RawOrigin::Root, member.clone(), target_rank); - assert_eq!(T::Members::rank_of(&member), Some(r)); + assert_eq!(T::Members::rank_of(&member), Some(target_rank)); assert!(!MemberEvidence::::contains_key(&member)); Ok(()) } From 283bff912b788a505e022656b7248dcec1cbb084 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 2 Mar 2025 14:58:17 +0100 Subject: [PATCH 3/9] add prdoc --- prdoc/pr_7720.prdoc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 prdoc/pr_7720.prdoc diff --git a/prdoc/pr_7720.prdoc b/prdoc/pr_7720.prdoc new file mode 100644 index 0000000000000..0ac05c2e982e2 --- /dev/null +++ b/prdoc/pr_7720.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: ... + +doc: + - audience: Runtime Dev + description: | + Enables reliable benchmarking for constrained configurations like `MaxRank=1` while + maintaining compatibility with larger rank ranges. + +crates: + - name: pallet-core-fellowship + bump: minor From cf353eb526004ce992898c0974c98942c7548087 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 2 Mar 2025 14:59:08 +0100 Subject: [PATCH 4/9] pr_doc title --- prdoc/pr_7720.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_7720.prdoc b/prdoc/pr_7720.prdoc index 0ac05c2e982e2..c8748337088b7 100644 --- a/prdoc/pr_7720.prdoc +++ b/prdoc/pr_7720.prdoc @@ -1,7 +1,7 @@ # Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 # See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json -title: ... +title: Clamp Core Fellowship Benchmarks to Runtime MaxRank Configuration doc: - audience: Runtime Dev From bf7d20ddef28b531c7a45263eb26389db3b44247 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 9 Mar 2025 02:11:50 +0100 Subject: [PATCH 5/9] add ConvertU16ToU32 for BoundedVec --- .../collectives-westend/src/ambassador/mod.rs | 2 +- .../collectives-westend/src/fellowship/mod.rs | 2 +- .../pallet_core_fellowship_ambassador_core.rs | 2 +- .../pallet_core_fellowship_fellowship_core.rs | 2 +- substrate/bin/node/runtime/src/lib.rs | 2 +- .../frame/core-fellowship/src/benchmarking.rs | 25 +++++++------- substrate/frame/core-fellowship/src/lib.rs | 33 ++++++++++++------- .../core-fellowship/src/tests/integration.rs | 4 +-- .../frame/core-fellowship/src/tests/unit.rs | 4 +-- .../frame/core-fellowship/src/weights.rs | 6 ++-- 10 files changed, 47 insertions(+), 35 deletions(-) diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs index 8b0842697237f..9711a34589de8 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs @@ -223,7 +223,7 @@ impl pallet_core_fellowship::Config for Runtime { type PromoteOrigin = PromoteOrigin; type FastPromoteOrigin = Self::PromoteOrigin; type EvidenceSize = ConstU32<65536>; - type MaxRank = ConstU32<9>; + type MaxRank = ConstU16<9>; } pub type AmbassadorSalaryInstance = pallet_salary::Instance2; diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs index 3fcaeb1dfc4fc..ceadc131c384e 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs @@ -211,7 +211,7 @@ impl pallet_core_fellowship::Config for Runtime { >; type FastPromoteOrigin = Self::PromoteOrigin; type EvidenceSize = ConstU32<65536>; - type MaxRank = ConstU32<9>; + type MaxRank = ConstU16<9>; } pub type FellowshipSalaryInstance = pallet_salary::Instance1; diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs index c47746be78e97..4f24def3ec505 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs @@ -193,7 +193,7 @@ impl pallet_core_fellowship::WeightInfo for WeightInfo< /// Proof: `AmbassadorCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) /// The range of component `r` is `[1, 9]`. /// The range of component `r` is `[1, 9]`. - fn promote_fast(r: u32, ) -> Weight { + fn promote_fast(r: u16, ) -> Weight { // Proof Size summary in bytes: // Measured: `65968` // Estimated: `69046 + r * (2489 ±0)` diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs index 0ca5c19b88aeb..db9254d5a6d7e 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs @@ -193,7 +193,7 @@ impl pallet_core_fellowship::WeightInfo for WeightInfo< /// Proof: `FellowshipCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) /// The range of component `r` is `[1, 9]`. /// The range of component `r` is `[1, 9]`. - fn promote_fast(r: u32, ) -> Weight { + fn promote_fast(r: u16, ) -> Weight { // Proof Size summary in bytes: // Measured: `65996` // Estimated: `69046 + r * (2489 ±0)` diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 22e266cc18678..9b0ac420b32b7 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2297,7 +2297,7 @@ impl pallet_core_fellowship::Config for Runtime { type PromoteOrigin = EnsureRootWithSuccess>; type FastPromoteOrigin = Self::PromoteOrigin; type EvidenceSize = ConstU32<16_384>; - type MaxRank = ConstU32<9>; + type MaxRank = ConstU16<9>; } parameter_types! { diff --git a/substrate/frame/core-fellowship/src/benchmarking.rs b/substrate/frame/core-fellowship/src/benchmarking.rs index c747e1a7d0823..9f47b5218b7e6 100644 --- a/substrate/frame/core-fellowship/src/benchmarking.rs +++ b/substrate/frame/core-fellowship/src/benchmarking.rs @@ -21,7 +21,6 @@ use super::*; use crate::Pallet as CoreFellowship; -use sp_runtime::SaturatedConversion; use alloc::{boxed::Box, vec}; use frame_benchmarking::v2::*; @@ -57,7 +56,7 @@ mod benchmarks { } fn set_benchmark_params, I: 'static>() -> Result<(), BenchmarkError> { - let max_rank = T::MaxRank::get().try_into().unwrap(); + let max_rank = T::MaxRank::get() as usize; let params = ParamsType { active_salary: BoundedVec::try_from(vec![100u32.into(); max_rank]).unwrap(), passive_salary: BoundedVec::try_from(vec![10u32.into(); max_rank]).unwrap(), @@ -72,7 +71,7 @@ mod benchmarks { #[benchmark] fn set_params() -> Result<(), BenchmarkError> { - let max_rank = T::MaxRank::get().try_into().unwrap(); + let max_rank = T::MaxRank::get() as usize; let params = ParamsType { active_salary: BoundedVec::try_from(vec![100u32.into(); max_rank]).unwrap(), passive_salary: BoundedVec::try_from(vec![10u32.into(); max_rank]).unwrap(), @@ -90,7 +89,7 @@ mod benchmarks { #[benchmark] fn set_partial_params() -> Result<(), BenchmarkError> { - let max_rank = T::MaxRank::get().try_into().unwrap(); + let max_rank = T::MaxRank::get() as usize; // Set up the initial default state for the Params storage let params = ParamsType { @@ -150,10 +149,9 @@ mod benchmarks { fn bump_demote() -> Result<(), BenchmarkError> { set_benchmark_params::()?; - let max_rank: RankOf = T::MaxRank::get().try_into().unwrap(); - let initial_rank = max_rank.saturated_into::(); + let initial_rank = T::MaxRank::get(); - let member = make_member::(max_rank)?; + let member = make_member::(initial_rank)?; // Set it to the max value to ensure that any possible auto-demotion period has passed. frame_system::Pallet::::set_block_number(BlockNumberFor::::max_value()); @@ -199,10 +197,11 @@ mod benchmarks { fn promote() -> Result<(), BenchmarkError> { // Ensure that the `min_promotion_period` wont get in our way. let mut params = Params::::get(); - let max_rank = T::MaxRank::get().try_into().unwrap(); + let max_rank = T::MaxRank::get(); // Get minimum promotion period. - params.min_promotion_period = BoundedVec::try_from(vec![Zero::zero(); max_rank]).unwrap(); + params.min_promotion_period = + BoundedVec::try_from(vec![Zero::zero(); max_rank as usize]).unwrap(); Params::::put(¶ms); // Start at rank 0 to allow at least one promotion. @@ -210,7 +209,7 @@ mod benchmarks { let member = make_member::(current_rank)?; // Set `to_rank` dynamically based on `max_rank`. - let to_rank = (current_rank + 1).min(max_rank.try_into().unwrap()); // Ensure `to_rank` <= `max_rank`. + let to_rank = (current_rank + 1).min(max_rank); // Ensure `to_rank` <= `max_rank`. // Set block number to avoid auto-demotion. frame_system::Pallet::::set_block_number(BlockNumberFor::::max_value()); @@ -227,10 +226,12 @@ mod benchmarks { /// Benchmark the `promote_fast` extrinsic to promote someone up to `r`. #[benchmark] - fn promote_fast(r: Linear<1, { T::MaxRank::get() as u32 }>) -> Result<(), BenchmarkError> { + fn promote_fast( + r: Linear<1, { ConvertU16ToU32::::get() }>, + ) -> Result<(), BenchmarkError> { // Get target rank for promotion. let max_rank = T::MaxRank::get(); - let target_rank = r.min(max_rank).try_into().unwrap(); + let target_rank = (r as u16).min(max_rank); // Begin with Candidate. let member = make_member::(0)?; diff --git a/substrate/frame/core-fellowship/src/lib.rs b/substrate/frame/core-fellowship/src/lib.rs index 3d2da6e9f717e..1aac8d8ef5fce 100644 --- a/substrate/frame/core-fellowship/src/lib.rs +++ b/substrate/frame/core-fellowship/src/lib.rs @@ -163,6 +163,13 @@ impl< } } +pub struct ConvertU16ToU32(PhantomData); +impl> Get for ConvertU16ToU32 { + fn get() -> u32 { + Inner::get() as u32 + } +} + /// The status of a single member. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub struct MemberStatus { @@ -238,15 +245,18 @@ pub mod pallet { /// /// Increasing this value is supported, but decreasing it may lead to a broken state. #[pallet::constant] - type MaxRank: Get; + type MaxRank: Get; } - pub type ParamsOf = - ParamsType<>::Balance, BlockNumberFor, >::MaxRank>; + pub type ParamsOf = ParamsType< + >::Balance, + BlockNumberFor, + ConvertU16ToU32<>::MaxRank>, + >; pub type PartialParamsOf = ParamsType< Option<>::Balance>, Option>, - >::MaxRank, + ConvertU16ToU32<>::MaxRank>, >; pub type MemberStatusOf = MemberStatus>; pub type RankOf = <>::Members as RankedMembers>::Rank; @@ -523,7 +533,7 @@ pub mod pallet { /// This is useful for out-of-band promotions, hence it has its own `FastPromoteOrigin` to /// be (possibly) more restrictive than `PromoteOrigin`. Note that the member must already /// be inducted. - #[pallet::weight(T::WeightInfo::promote_fast(*to_rank as u32))] + #[pallet::weight(T::WeightInfo::promote_fast(*to_rank))] #[pallet::call_index(10)] pub fn promote_fast( origin: OriginFor, @@ -534,7 +544,7 @@ pub mod pallet { Ok(allow_rank) => ensure!(allow_rank >= to_rank, Error::::NoPermission), Err(origin) => ensure_root(origin)?, } - ensure!(to_rank as u32 <= T::MaxRank::get(), Error::::InvalidRank); + ensure!(to_rank <= T::MaxRank::get(), Error::::InvalidRank); let curr_rank = T::Members::rank_of(&who).ok_or(Error::::Unranked)?; ensure!(to_rank > curr_rank, Error::::UnexpectedRank); @@ -681,8 +691,8 @@ pub mod pallet { /// /// Only elements in the base slice which has a new value in the new slice will be updated. pub(crate) fn set_partial_params_slice( - base_slice: &mut BoundedVec>::MaxRank>, - new_slice: BoundedVec, >::MaxRank>, + base_slice: &mut BoundedVec>, + new_slice: BoundedVec, ConvertU16ToU32>, ) { for (base_element, new_element) in base_slice.iter_mut().zip(new_slice) { if let Some(element) = new_element { @@ -713,9 +723,10 @@ pub mod pallet { /// Rank 1 becomes index 0, rank `RANK_COUNT` becomes index `RANK_COUNT - 1`. Any rank not /// in the range `1..=RANK_COUNT` is `None`. pub(crate) fn rank_to_index(rank: RankOf) -> Option { - match TryInto::::try_into(rank) { - Ok(r) if r as u32 <= >::MaxRank::get() && r > 0 => Some(r - 1), - _ => return None, + if rank == 0 || rank > T::MaxRank::get() { + None + } else { + Some((rank - 1) as usize) } } diff --git a/substrate/frame/core-fellowship/src/tests/integration.rs b/substrate/frame/core-fellowship/src/tests/integration.rs index b2149336547d3..db411f91d269f 100644 --- a/substrate/frame/core-fellowship/src/tests/integration.rs +++ b/substrate/frame/core-fellowship/src/tests/integration.rs @@ -27,7 +27,7 @@ use frame_support::{ }; use frame_system::EnsureSignedBy; use pallet_ranked_collective::{EnsureRanked, Geometric, Rank}; -use sp_core::{ConstU32, Get}; +use sp_core::Get; use sp_runtime::{ bounded_vec, traits::{Convert, ReduceBy, ReplaceWithDefault, TryMorphInto}, @@ -82,7 +82,7 @@ impl Config for Test { type PromoteOrigin = TryMapSuccess, u64>, TryMorphInto>; type FastPromoteOrigin = Self::PromoteOrigin; type EvidenceSize = EvidenceSize; - type MaxRank = ConstU32<9>; + type MaxRank = ConstU16<9>; } /// Convert the tally class into the minimum rank required to vote on the poll. diff --git a/substrate/frame/core-fellowship/src/tests/unit.rs b/substrate/frame/core-fellowship/src/tests/unit.rs index f4418ed439d0c..f8ae5afcc1f23 100644 --- a/substrate/frame/core-fellowship/src/tests/unit.rs +++ b/substrate/frame/core-fellowship/src/tests/unit.rs @@ -26,7 +26,7 @@ use frame_support::{ assert_noop, assert_ok, derive_impl, hypothetically, ord_parameter_types, pallet_prelude::Weight, parameter_types, - traits::{tokens::GetSalary, ConstU32, IsInVec, TryMapSuccess}, + traits::{tokens::GetSalary, ConstU16, ConstU32, IsInVec, TryMapSuccess}, }; use frame_system::EnsureSignedBy; use sp_runtime::{bounded_vec, traits::TryMorphInto, BuildStorage, DispatchError, DispatchResult}; @@ -119,7 +119,7 @@ impl Config for Test { type PromoteOrigin = TryMapSuccess, u64>, TryMorphInto>; type FastPromoteOrigin = Self::PromoteOrigin; type EvidenceSize = ConstU32<1024>; - type MaxRank = ConstU32<9>; + type MaxRank = ConstU16<9>; } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/substrate/frame/core-fellowship/src/weights.rs b/substrate/frame/core-fellowship/src/weights.rs index 9c0fffd3d05ec..25f6310addc92 100644 --- a/substrate/frame/core-fellowship/src/weights.rs +++ b/substrate/frame/core-fellowship/src/weights.rs @@ -79,7 +79,7 @@ pub trait WeightInfo { fn set_active() -> Weight; fn induct() -> Weight; fn promote() -> Weight; - fn promote_fast(r: u32, ) -> Weight; + fn promote_fast(r: u16, ) -> Weight; fn offboard() -> Weight; fn import() -> Weight; fn import_member() -> Weight; @@ -225,7 +225,7 @@ impl WeightInfo for SubstrateWeight { /// Storage: `RankedCollective::IdToIndex` (r:0 w:9) /// Proof: `RankedCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) /// The range of component `r` is `[1, 9]`. - fn promote_fast(r: u32, ) -> Weight { + fn promote_fast(r: u16, ) -> Weight { // Proof Size summary in bytes: // Measured: `16665` // Estimated: `19894 + r * (2489 ±0)` @@ -447,7 +447,7 @@ impl WeightInfo for () { /// Storage: `RankedCollective::IdToIndex` (r:0 w:9) /// Proof: `RankedCollective::IdToIndex` (`max_values`: None, `max_size`: Some(54), added: 2529, mode: `MaxEncodedLen`) /// The range of component `r` is `[1, 9]`. - fn promote_fast(r: u32, ) -> Weight { + fn promote_fast(r: u16, ) -> Weight { // Proof Size summary in bytes: // Measured: `16665` // Estimated: `19894 + r * (2489 ±0)` From 6c6ff9c3033dcfaca0f0c112b15a715e0d4f7282 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 9 Mar 2025 02:37:23 +0100 Subject: [PATCH 6/9] try-runtime features usize --- substrate/frame/core-fellowship/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/core-fellowship/src/migration.rs b/substrate/frame/core-fellowship/src/migration.rs index b1e27d1e79363..216523f0c7364 100644 --- a/substrate/frame/core-fellowship/src/migration.rs +++ b/substrate/frame/core-fellowship/src/migration.rs @@ -73,7 +73,7 @@ impl, I: 'static> UncheckedOnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { ensure!( - T::MaxRank::get() >= v0::RANK_COUNT as u32, + T::MaxRank::get() as usize >= v0::RANK_COUNT, "pallet-core-fellowship: new bound should not truncate" ); Ok(Default::default()) From f08a7af9fee960ceae8d77c1b25386a674d6c6d7 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 9 Mar 2025 02:52:52 +0100 Subject: [PATCH 7/9] remove log check in benchmarking code --- substrate/frame/core-fellowship/src/benchmarking.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/frame/core-fellowship/src/benchmarking.rs b/substrate/frame/core-fellowship/src/benchmarking.rs index 9f47b5218b7e6..7fdec9748516f 100644 --- a/substrate/frame/core-fellowship/src/benchmarking.rs +++ b/substrate/frame/core-fellowship/src/benchmarking.rs @@ -235,7 +235,6 @@ mod benchmarks { // Begin with Candidate. let member = make_member::(0)?; - log::info!("Benchmark promote_fast: r={} MaxRank={}", r, T::MaxRank::get()); ensure_evidence::(&member)?; From 9b17f02f709fc9699f8f194e9e95f109dde7d434 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 13 Mar 2025 13:00:16 +0100 Subject: [PATCH 8/9] update prdoc --- prdoc/pr_7720.prdoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/prdoc/pr_7720.prdoc b/prdoc/pr_7720.prdoc index c8748337088b7..7f2119e380ca8 100644 --- a/prdoc/pr_7720.prdoc +++ b/prdoc/pr_7720.prdoc @@ -12,3 +12,7 @@ doc: crates: - name: pallet-core-fellowship bump: minor + - name: collectives-westend-runtime + bump: minor + - name: kitchensink-runtime + bump: minor From 7531822fc522f90717ddfca1eef02ba8f39e4129 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Wed, 9 Apr 2025 15:36:07 +0100 Subject: [PATCH 9/9] prdoc major, downstream developer note --- prdoc/pr_7720.prdoc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_7720.prdoc b/prdoc/pr_7720.prdoc index 7f2119e380ca8..48ad47f21277e 100644 --- a/prdoc/pr_7720.prdoc +++ b/prdoc/pr_7720.prdoc @@ -6,12 +6,19 @@ title: Clamp Core Fellowship Benchmarks to Runtime MaxRank Configuration doc: - audience: Runtime Dev description: | - Enables reliable benchmarking for constrained configurations like `MaxRank=1` while + + This change modifies the `MaxRank` type in the Core Fellowship pallet configuration from + `u32` to `u16`. Runtime developers updating to this version must: + - Change any `ConstU32` usages for `MaxRank` to `ConstU16` + + Enabling reliable benchmarking for constrained configurations like `MaxRank=1` while maintaining compatibility with larger rank ranges. + + crates: - name: pallet-core-fellowship - bump: minor + bump: major - name: collectives-westend-runtime bump: minor - name: kitchensink-runtime