Add block provider to core-fellowship

cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs

@@ -223,6 +223,7 @@ impl pallet_core_fellowship::Config<AmbassadorCoreInstance> for Runtime {
 	type FastPromoteOrigin = Self::PromoteOrigin;
 	type EvidenceSize = ConstU32<65536>;
 	type MaxRank = ConstU32<9>;
+	type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
 }
 
 pub type AmbassadorSalaryInstance = pallet_salary::Instance2;

cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs

@@ -22,7 +22,7 @@ use crate::{
 	weights,
 	xcm_config::{FellowshipAdminBodyId, LocationToAccountId, TreasurerBodyId, UsdtAssetHub},
 	AccountId, AssetRate, Balance, Balances, FellowshipReferenda, GovernanceLocation,
-	ParachainInfo, Preimage, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Scheduler,
+	ParachainInfo, Preimage, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Scheduler, System,
 	WestendTreasuryAccount, DAYS,
 };
 use cumulus_primitives_core::ParaId;
@@ -211,6 +211,7 @@ impl pallet_core_fellowship::Config<FellowshipCoreInstance> for Runtime {
 	type FastPromoteOrigin = Self::PromoteOrigin;
 	type EvidenceSize = ConstU32<65536>;
 	type MaxRank = ConstU32<9>;
+	type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
 }
 
 pub type FellowshipSalaryInstance = pallet_salary::Instance1;
@@ -333,5 +334,5 @@ impl pallet_treasury::Config<FellowshipTreasuryInstance> for Runtime {
 		sp_core::ConstU8<1>,
 		ConstU32<1000>,
 	>;
-	type BlockNumberProvider = crate::System;
+	type BlockNumberProvider = System;
 }

cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs

@@ -759,19 +759,62 @@ pub type UncheckedExtrinsic =
 /// All migrations executed on runtime upgrade as a nested tuple of types implementing
 /// `OnRuntimeUpgrade`. Included migrations must be idempotent.
 type Migrations = (
-	// unreleased
-	pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
-	// unreleased
-	cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
-	cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5<Runtime>,
 	// permanent
 	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 	// unreleased
-	pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, FellowshipCoreInstance>,
-	// unreleased
-	pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, AmbassadorCoreInstance>,
+	pallet_core_fellowship::migration::MigrateV1ToV2<
+		Runtime,
+		BlockNumberConverter,
+		FellowshipCoreInstance,
+	>,
+	pallet_core_fellowship::migration::MigrateV1ToV2<
+		Runtime,
+		BlockNumberConverter,
+		AmbassadorCoreInstance,
+	>,
 );
 
+// Helpers for the core fellowship pallet v1->v2 storage migration.
+use sp_runtime::traits::BlockNumberProvider;
+type CoreFellowshipLocalBlockNumber = <System as BlockNumberProvider>::BlockNumber;
+type CoreFellowshipNewBlockNumber = <cumulus_pallet_parachain_system::RelaychainDataProvider<
+	Runtime,
+> as BlockNumberProvider>::BlockNumber;
+pub struct BlockNumberConverter;
+impl
+	pallet_core_fellowship::migration::v2::ConvertBlockNumber<
+		CoreFellowshipLocalBlockNumber,
+		CoreFellowshipNewBlockNumber,
+	> for BlockNumberConverter
+{
+	/// The equivalent moment in time from the perspective of the relay chain, starting from a
+	/// local moment in time (system block number).
+	fn equivalent_moment_in_time(
+		local_moment: CoreFellowshipLocalBlockNumber,
+	) -> CoreFellowshipNewBlockNumber {
+		let local_block_number = System::block_number();
+		let local_duration = u32::abs_diff(local_block_number, local_moment);
+		let relay_duration = Self::equivalent_block_duration(local_duration); // 6s to 6s
+		let relay_block_number = ParachainSystem::last_relay_block_number();
+		if local_block_number >= local_moment {
+			// Moment was in past.
+			relay_block_number.saturating_sub(relay_duration)
+		} else {
+			// Moment is in future.
+			relay_block_number.saturating_add(relay_duration)
+		}
+	}
+
+	/// The equivalent duration from the perspective of the relay chain, starting from
+	/// a local duration (number of block). Identity function for Westend, since both
+	/// relay and collectives chain run 6s block times.
+	fn equivalent_block_duration(
+		local_duration: CoreFellowshipLocalBlockNumber,
+	) -> CoreFellowshipNewBlockNumber {
+		local_duration
+	}
+}
+
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<
 	Runtime,

prdoc/pr_6978.prdoc

@@ -0,0 +1,22 @@
+title: 'Adds BlockNumberProvider to pallet-core-fellowship'
+doc:
+- audience: Runtime Dev
+  description: |-
+    This PR adds a parameter 'BlockNumberProvider' to the pallet-core-fellowship 
+    config such that a block provider can be set for use in the pallet. This would 
+    usually be the local system pallet or the appropriate relay chain. Previously 
+    it defaulted to the local system pallet.
+- audience: Runtime User
+  description: |-
+    This PR updates the Westend runtime to use the relaychain's block number for
+    core-fellowship pallet logic. The type remains the same, but the values have
+    shifted.
+crates:
+- name: pallet-core-fellowship
+  bump: major
+- name: sp-runtime
+  bump: major
+- name: collectives-westend-runtime
+  bump: minor
+- name: kitchensink-runtime
+  bump: patch

substrate/bin/node/runtime/src/lib.rs

@@ -1992,6 +1992,7 @@ impl pallet_core_fellowship::Config for Runtime {
 	type FastPromoteOrigin = Self::PromoteOrigin;
 	type EvidenceSize = ConstU32<16_384>;
 	type MaxRank = ConstU32<9>;
+	type BlockNumberProvider = System;
 }
 
 parameter_types! {

substrate/frame/core-fellowship/src/benchmarking.rs

@@ -15,17 +15,18 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! Salary pallet benchmarking.
+//! Core Fellowship pallet benchmarking.
 
 #![cfg(feature = "runtime-benchmarks")]
 
 use super::*;
-use crate::Pallet as CoreFellowship;
+use crate::{BlockNumberFor as CoreFellowshipBlockNumberFor, Pallet as CoreFellowship};
 
 use alloc::{boxed::Box, vec};
 use frame_benchmarking::v2::*;
-use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
+use frame_system::RawOrigin;
 use sp_arithmetic::traits::Bounded;
+use sp_runtime::traits::BlockNumberProvider;
 
 const SEED: u32 = 0;
 
@@ -35,6 +36,10 @@ type BenchResult = Result<(), BenchmarkError>;
 mod benchmarks {
 	use super::*;
 
+	fn set_block_number<T: Config<I>, I: 'static>(n: CoreFellowshipBlockNumberFor<T, I>) {
+		T::BlockNumberProvider::set_block_number(n);
+	}
+
 	fn ensure_evidence<T: Config<I>, I: 'static>(who: &T::AccountId) -> BenchResult {
 		let evidence = BoundedVec::try_from(vec![0; Evidence::<T, I>::bound()]).unwrap();
 		let wish = Wish::Retention;
@@ -133,7 +138,7 @@ mod benchmarks {
 		let member = make_member::<T, I>(0)?;
 
 		// Set it to the max value to ensure that any possible auto-demotion period has passed.
-		frame_system::Pallet::<T>::set_block_number(BlockNumberFor::<T>::max_value());
+		set_block_number::<T, I>(CoreFellowshipBlockNumberFor::<T, I>::max_value());
 		ensure_evidence::<T, I>(&member)?;
 		assert!(Member::<T, I>::contains_key(&member));
 
@@ -152,7 +157,7 @@ mod benchmarks {
 		let member = make_member::<T, I>(2)?;
 
 		// Set it to the max value to ensure that any possible auto-demotion period has passed.
-		frame_system::Pallet::<T>::set_block_number(BlockNumberFor::<T>::max_value());
+		set_block_number::<T, I>(CoreFellowshipBlockNumberFor::<T, I>::max_value());
 		ensure_evidence::<T, I>(&member)?;
 		assert!(Member::<T, I>::contains_key(&member));
 		assert_eq!(T::Members::rank_of(&member), Some(2));
@@ -201,7 +206,7 @@ mod benchmarks {
 		let member = make_member::<T, I>(1)?;
 
 		// Set it to the max value to ensure that any possible auto-demotion period has passed.
-		frame_system::Pallet::<T>::set_block_number(BlockNumberFor::<T>::max_value());
+		set_block_number::<T, I>(CoreFellowshipBlockNumberFor::<T, I>::max_value());
 		ensure_evidence::<T, I>(&member)?;
 
 		#[extrinsic_call]
@@ -281,9 +286,9 @@ mod benchmarks {
 	#[benchmark]
 	fn approve() -> Result<(), BenchmarkError> {
 		let member = make_member::<T, I>(1)?;
-		let then = frame_system::Pallet::<T>::block_number();
+		let then = T::BlockNumberProvider::current_block_number();
 		let now = then.saturating_plus_one();
-		frame_system::Pallet::<T>::set_block_number(now);
+		set_block_number::<T, I>(now);
 		ensure_evidence::<T, I>(&member)?;
 
 		assert_eq!(Member::<T, I>::get(&member).unwrap().last_proof, then);

substrate/frame/core-fellowship/src/lib.rs

@@ -155,11 +155,11 @@ impl<
 #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)]
 pub struct MemberStatus<BlockNumber> {
 	/// Are they currently active?
-	is_active: bool,
+	pub is_active: bool,
 	/// The block number at which we last promoted them.
-	last_promotion: BlockNumber,
+	pub last_promotion: BlockNumber,
 	/// The last time a member was demoted, promoted or proved their rank.
-	last_proof: BlockNumber,
+	pub last_proof: BlockNumber,
 }
 
 #[frame_support::pallet]
@@ -171,8 +171,9 @@ pub mod pallet {
 		traits::{tokens::GetSalary, EnsureOrigin},
 	};
 	use frame_system::{ensure_root, pallet_prelude::*};
+	use sp_runtime::traits::BlockNumberProvider;
 	/// The in-code storage version.
-	const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]
@@ -227,27 +228,34 @@ pub mod pallet {
 		/// Increasing this value is supported, but decreasing it may lead to a broken state.
 		#[pallet::constant]
 		type MaxRank: Get<u32>;
+
+		/// Provides the current block number.
+		///
+		/// This is usually `cumulus_pallet_parachain_system::RelaychainDataProvider` if a
+		/// parachain, or `frame_system::Pallet` if a solo- or relaychain.
+		type BlockNumberProvider: BlockNumberProvider;
 	}
 
+	pub type BlockNumberFor<T, I = ()> =
+		<<T as Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
 	pub type ParamsOf<T, I> =
-		ParamsType<<T as Config<I>>::Balance, BlockNumberFor<T>, <T as Config<I>>::MaxRank>;
+		ParamsType<<T as Config<I>>::Balance, BlockNumberFor<T, I>, <T as Config<I>>::MaxRank>;
 	pub type PartialParamsOf<T, I> = ParamsType<
 		Option<<T as Config<I>>::Balance>,
-		Option<BlockNumberFor<T>>,
+		Option<BlockNumberFor<T, I>>,
 		<T as Config<I>>::MaxRank,
 	>;
-	pub type MemberStatusOf<T> = MemberStatus<BlockNumberFor<T>>;
+	pub type MemberStatusOf<T, I> = MemberStatus<BlockNumberFor<T, I>>;
 	pub type RankOf<T, I> = <<T as Config<I>>::Members as RankedMembers>::Rank;
 
 	/// The overall status of the system.
 	#[pallet::storage]
-	pub(super) type Params<T: Config<I>, I: 'static = ()> =
-		StorageValue<_, ParamsOf<T, I>, ValueQuery>;
+	pub type Params<T: Config<I>, I: 'static = ()> = StorageValue<_, ParamsOf<T, I>, ValueQuery>;
 
 	/// The status of a claimant.
 	#[pallet::storage]
-	pub(super) type Member<T: Config<I>, I: 'static = ()> =
-		StorageMap<_, Twox64Concat, T::AccountId, MemberStatusOf<T>, OptionQuery>;
+	pub type Member<T: Config<I>, I: 'static = ()> =
+		StorageMap<_, Twox64Concat, T::AccountId, MemberStatusOf<T, I>, OptionQuery>;
 
 	/// Some evidence together with the desired outcome for which it was presented.
 	#[pallet::storage]
@@ -343,13 +351,13 @@ pub mod pallet {
 			};
 
 			if demotion_period.is_zero() {
-				return Err(Error::<T, I>::NothingDoing.into())
+				return Err(Error::<T, I>::NothingDoing.into());
 			}
 
 			let demotion_block = member.last_proof.saturating_add(demotion_period);
 
 			// Ensure enough time has passed.
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			if now >= demotion_block {
 				T::Members::demote(&who)?;
 				let maybe_to_rank = T::Members::rank_of(&who);
@@ -363,7 +371,7 @@ pub mod pallet {
 					Event::<T, I>::Offboarded { who }
 				};
 				Self::deposit_event(event);
-				return Ok(Pays::No.into())
+				return Ok(Pays::No.into());
 			}
 
 			Err(Error::<T, I>::NothingDoing.into())
@@ -428,7 +436,7 @@ pub mod pallet {
 			ensure!(rank == at_rank, Error::<T, I>::UnexpectedRank);
 			let mut member = Member::<T, I>::get(&who).ok_or(Error::<T, I>::NotTracked)?;
 
-			member.last_proof = frame_system::Pallet::<T>::block_number();
+			member.last_proof = T::BlockNumberProvider::current_block_number();
 			Member::<T, I>::insert(&who, &member);
 
 			Self::dispose_evidence(who.clone(), at_rank, Some(at_rank));
@@ -452,7 +460,7 @@ pub mod pallet {
 			ensure!(T::Members::rank_of(&who).is_none(), Error::<T, I>::Ranked);
 
 			T::Members::induct(&who)?;
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			Member::<T, I>::insert(
 				&who,
 				MemberStatus { is_active: true, last_promotion: now, last_proof: now },
@@ -485,7 +493,7 @@ pub mod pallet {
 			);
 
 			let mut member = Member::<T, I>::get(&who).ok_or(Error::<T, I>::NotTracked)?;
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 
 			let params = Params::<T, I>::get();
 			let rank_index = Self::rank_to_index(to_rank).ok_or(Error::<T, I>::InvalidRank)?;
@@ -528,7 +536,7 @@ pub mod pallet {
 			ensure!(to_rank > curr_rank, Error::<T, I>::UnexpectedRank);
 
 			let mut member = Member::<T, I>::get(&who).ok_or(Error::<T, I>::NotTracked)?;
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			member.last_promotion = now;
 			member.last_proof = now;
 
@@ -687,7 +695,7 @@ pub mod pallet {
 			ensure!(!Member::<T, I>::contains_key(&who), Error::<T, I>::AlreadyInducted);
 			let rank = T::Members::rank_of(&who).ok_or(Error::<T, I>::Unranked)?;
 
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			Member::<T, I>::insert(
 				&who,
 				MemberStatus { is_active: true, last_promotion: 0u32.into(), last_proof: now },
@@ -775,15 +783,15 @@ impl<T: Config<I>, I: 'static> RankedMembersSwapHandler<T::AccountId, u16> for P
 	fn swapped(old: &T::AccountId, new: &T::AccountId, _rank: u16) {
 		if old == new {
 			defensive!("Should not try to swap with self");
-			return
+			return;
 		}
 		if !Member::<T, I>::contains_key(old) {
 			defensive!("Should not try to swap non-member");
-			return
+			return;
 		}
 		if Member::<T, I>::contains_key(new) {
 			defensive!("Should not try to overwrite existing member");
-			return
+			return;
 		}
 
 		if let Some(member) = Member::<T, I>::take(old) {