Add BlockNumberProvider to salary pallet config

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

@@ -270,4 +270,6 @@ impl pallet_salary::Config<AmbassadorSalaryInstance> for Runtime {
 	type PayoutPeriod = ConstU32<{ 15 * DAYS }>;
 	// Total monthly salary budget.
 	type Budget = ConstU128<{ 10_000 * DOLLARS }>;
+
+	type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
 }

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

@@ -257,6 +257,8 @@ impl pallet_salary::Config<FellowshipSalaryInstance> for Runtime {
 	type PayoutPeriod = ConstU32<{ 15 * DAYS }>;
 	// Total monthly salary budget.
 	type Budget = ConstU128<{ 100_000 * USDT_UNITS }>;
+
+	type BlockNumberProvider = cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>;
 }
 
 parameter_types! {

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

@@ -49,9 +49,11 @@ extern crate alloc;
 pub use ambassador::pallet_ambassador_origins;
 
 use alloc::{vec, vec::Vec};
-use ambassador::AmbassadorCoreInstance;
+use ambassador::{AmbassadorCoreInstance, AmbassadorSalaryInstance};
 use cumulus_pallet_parachain_system::RelayNumberMonotonicallyIncreases;
-use fellowship::{pallet_fellowship_origins, Fellows, FellowshipCoreInstance};
+use fellowship::{
+	pallet_fellowship_origins, Fellows, FellowshipCoreInstance, FellowshipSalaryInstance,
+};
 use impls::{AllianceProposalProvider, EqualOrGreatestRootCmp};
 use sp_api::impl_runtime_apis;
 use sp_core::{crypto::KeyTypeId, OpaqueMetadata};
@@ -770,8 +772,58 @@ type Migrations = (
 	pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, FellowshipCoreInstance>,
 	// unreleased
 	pallet_core_fellowship::migration::MigrateV0ToV1<Runtime, AmbassadorCoreInstance>,
+	// unreleased
+	pallet_salary::migration::MigrateV0ToV1<
+		Runtime,
+		SalaryBlockNumberConverter,
+		FellowshipSalaryInstance,
+	>,
+	pallet_salary::migration::MigrateV0ToV1<
+		Runtime,
+		SalaryBlockNumberConverter,
+		AmbassadorSalaryInstance,
+	>,
 );
 
+// Helpers for the salary pallet v0->v1 storage migration.
+use sp_runtime::traits::BlockNumberProvider;
+type SalaryLocalBlockNumber = <System as BlockNumberProvider>::BlockNumber;
+type SalaryNewBlockNumber = <cumulus_pallet_parachain_system::RelaychainDataProvider<Runtime>
+	as BlockNumberProvider>::BlockNumber;
+pub struct SalaryBlockNumberConverter;
+impl pallet_salary::migration::v1::ConvertBlockNumber<SalaryLocalBlockNumber, SalaryNewBlockNumber>
+	for SalaryBlockNumberConverter
+{
+	/// Simply convert the types. Cycle index storage item uses block number but is agnostic to the
+	/// time that denotes for instance.
+	fn convert(local: SalaryLocalBlockNumber) -> SalaryNewBlockNumber {
+		local
+	}
+
+	/// 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: SalaryLocalBlockNumber) -> SalaryNewBlockNumber {
+		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: SalaryLocalBlockNumber) -> SalaryNewBlockNumber {
+		local_duration
+	}
+}
+
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<
 	Runtime,

polkadot/xcm/xcm-builder/src/tests/pay/salary.rs

@@ -113,6 +113,7 @@ impl pallet_salary::Config for Test {
 	type RegistrationPeriod = RegistrationPeriod;
 	type PayoutPeriod = PayoutPeriod;
 	type Budget = Budget;
+	type BlockNumberProvider = System;
 }
 
 /// Scenario:

prdoc/pr_7000.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-salary 
+    config such that a block provider can be set for use in the pallet. This would 
+    usually be the frame system pallet or the appropriate relay chain. Previously 
+    it defaulted to the frame system pallet.
+- audience: Runtime User
+  description: |-
+    This PR updates the Westend runtime to use the relaychain's block number for
+    salary pallet logic. The type remains the same, but the values have
+    shifted.
+crates:
+- name: pallet-salary
+  bump: major
+- name: collectives-westend-runtime
+  bump: minor
+- name: kitchensink-runtime
+  bump: patch
+- name: staging-xcm-builder
+  bump: patch

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

@@ -1978,6 +1978,7 @@ impl pallet_salary::Config for Runtime {
 	type RegistrationPeriod = ConstU32<200>;
 	type PayoutPeriod = ConstU32<200>;
 	type Budget = Budget;
+	type BlockNumberProvider = System;
 }
 
 impl pallet_core_fellowship::Config for Runtime {

substrate/frame/salary/src/benchmarking.rs

@@ -20,9 +20,9 @@
 #![cfg(feature = "runtime-benchmarks")]
 
 use super::*;
-use crate::Pallet as Salary;
-
+use crate::{pallet::BlockNumberFor as SalaryBlockNumberFor, Pallet as Salary};
 use frame::benchmarking::prelude::*;
+
 const SEED: u32 = 0;
 
 fn ensure_member_with_salary<T: Config<I>, I: 'static>(who: &T::AccountId) {
@@ -44,6 +44,14 @@ fn ensure_member_with_salary<T: Config<I>, I: 'static>(who: &T::AccountId) {
 mod benchmarks {
 	use super::*;
 
+	fn get_block_number<T: Config<I>, I: 'static>() -> SalaryBlockNumberFor<T, I> {
+		T::BlockNumberProvider::current_block_number()
+	}
+
+	fn set_block_number<T: Config<I>, I: 'static>(n: SalaryBlockNumberFor<T, I>) {
+		T::BlockNumberProvider::set_block_number(n);
+	}
+
 	#[benchmark]
 	fn init() {
 		let caller: T::AccountId = whitelisted_caller();
@@ -58,7 +66,7 @@ mod benchmarks {
 	fn bump() {
 		let caller: T::AccountId = whitelisted_caller();
 		Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
+		set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
 
 		#[extrinsic_call]
 		_(RawOrigin::Signed(caller.clone()));
@@ -84,7 +92,7 @@ mod benchmarks {
 		ensure_member_with_salary::<T, I>(&caller);
 		Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
 		Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
+		set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
 		Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
 
 		#[extrinsic_call]
@@ -99,9 +107,9 @@ mod benchmarks {
 		ensure_member_with_salary::<T, I>(&caller);
 		Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
 		Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
+		set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
 		Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + T::RegistrationPeriod::get());
+		set_block_number::<T, I>(get_block_number::<T, I>() + T::RegistrationPeriod::get());
 
 		let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller);
 		T::Paymaster::ensure_successful(&caller, (), salary);
@@ -125,9 +133,9 @@ mod benchmarks {
 		ensure_member_with_salary::<T, I>(&caller);
 		Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
 		Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
+		set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
 		Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + T::RegistrationPeriod::get());
+		set_block_number::<T, I>(get_block_number::<T, I>() + T::RegistrationPeriod::get());
 
 		let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller);
 		let recipient: T::AccountId = account("recipient", 0, SEED);
@@ -152,9 +160,9 @@ mod benchmarks {
 		ensure_member_with_salary::<T, I>(&caller);
 		Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
 		Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());
+		set_block_number::<T, I>(get_block_number::<T, I>() + Salary::<T, I>::cycle_period());
 		Salary::<T, I>::bump(RawOrigin::Signed(caller.clone()).into()).unwrap();
-		System::<T>::set_block_number(System::<T>::block_number() + T::RegistrationPeriod::get());
+		set_block_number::<T, I>(get_block_number::<T, I>() + T::RegistrationPeriod::get());
 
 		let salary = T::Salary::get_salary(T::Members::rank_of(&caller).unwrap(), &caller);
 		let recipient: T::AccountId = account("recipient", 0, SEED);

substrate/frame/salary/src/lib.rs

@@ -19,6 +19,8 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
+extern crate alloc;
+
 use core::marker::PhantomData;
 use frame::{
 	prelude::*,
@@ -30,6 +32,7 @@ mod tests;
 
 #[cfg(feature = "runtime-benchmarks")]
 mod benchmarking;
+pub mod migration;
 pub mod weights;
 
 pub use pallet::*;
@@ -42,15 +45,15 @@ pub type Cycle = u32;
 #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)]
 pub struct StatusType<CycleIndex, BlockNumber, Balance> {
 	/// The index of the "current cycle" (i.e. the last cycle being processed).
-	cycle_index: CycleIndex,
+	pub cycle_index: CycleIndex,
 	/// The first block of the "current cycle" (i.e. the last cycle being processed).
-	cycle_start: BlockNumber,
+	pub cycle_start: BlockNumber,
 	/// The total budget available for all payments in the current cycle.
-	budget: Balance,
+	pub budget: Balance,
 	/// The total amount of the payments registered in the current cycle.
-	total_registrations: Balance,
+	pub total_registrations: Balance,
 	/// The total amount of unregistered payments which have been made in the current cycle.
-	total_unregistered_paid: Balance,
+	pub total_unregistered_paid: Balance,
 }
 
 /// The state of a specific payment claim.
@@ -78,7 +81,12 @@ pub struct ClaimantStatus<CycleIndex, Balance, Id> {
 #[frame::pallet]
 pub mod pallet {
 	use super::*;
+
+	/// The in-code storage version.
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+
 	#[pallet::pallet]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T, I = ()>(PhantomData<(T, I)>);
 
 	#[pallet::config]
@@ -112,27 +120,36 @@ pub mod pallet {
 		/// The number of blocks between sequential payout cycles is the sum of this and
 		/// `PayoutPeriod`.
 		#[pallet::constant]
-		type RegistrationPeriod: Get<BlockNumberFor<Self>>;
+		type RegistrationPeriod: Get<BlockNumberFor<Self, I>>;
 
 		/// The number of blocks within a cycle which accounts have to claim the payout.
 		///
 		/// The number of blocks between sequential payout cycles is the sum of this and
 		/// `RegistrationPeriod`.
 		#[pallet::constant]
-		type PayoutPeriod: Get<BlockNumberFor<Self>>;
+		type PayoutPeriod: Get<BlockNumberFor<Self, I>>;
 
 		/// The total budget per cycle.
 		///
 		/// This may change over the course of a cycle without any problem.
 		#[pallet::constant]
 		type Budget: Get<BalanceOf<Self, I>>;
+
+		/// 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 CycleIndexOf<T> = BlockNumberFor<T>;
+	pub type BlockNumberFor<T, I> =
+		<<T as Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
+	pub type CycleIndexOf<T, I> = BlockNumberFor<T, I>;
 	pub type BalanceOf<T, I> = <<T as Config<I>>::Paymaster as Pay>::Balance;
 	pub type IdOf<T, I> = <<T as Config<I>>::Paymaster as Pay>::Id;
-	pub type StatusOf<T, I> = StatusType<CycleIndexOf<T>, BlockNumberFor<T>, BalanceOf<T, I>>;
-	pub type ClaimantStatusOf<T, I> = ClaimantStatus<CycleIndexOf<T>, BalanceOf<T, I>, IdOf<T, I>>;
+	pub type StatusOf<T, I> = StatusType<CycleIndexOf<T, I>, BlockNumberFor<T, I>, BalanceOf<T, I>>;
+	pub type ClaimantStatusOf<T, I> =
+		ClaimantStatus<CycleIndexOf<T, I>, BalanceOf<T, I>, IdOf<T, I>>;
 
 	/// The overall status of the system.
 	#[pallet::storage]
@@ -159,7 +176,7 @@ pub mod pallet {
 			id: <T::Paymaster as Pay>::Id,
 		},
 		/// The next cycle begins.
-		CycleStarted { index: CycleIndexOf<T> },
+		CycleStarted { index: CycleIndexOf<T, I> },
 		/// A member swapped their account.
 		Swapped { who: T::AccountId, new_who: T::AccountId },
 	}
@@ -205,7 +222,7 @@ pub mod pallet {
 		#[pallet::call_index(0)]
 		pub fn init(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
 			let _ = ensure_signed(origin)?;
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			ensure!(!Status::<T, I>::exists(), Error::<T, I>::AlreadyStarted);
 			let status = StatusType {
 				cycle_index: Zero::zero(),
@@ -227,7 +244,7 @@ pub mod pallet {
 		#[pallet::call_index(1)]
 		pub fn bump(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
 			let _ = ensure_signed(origin)?;
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			let cycle_period = Self::cycle_period();
 			let mut status = Status::<T, I>::get().ok_or(Error::<T, I>::NotStarted)?;
 			status.cycle_start.saturating_accrue(cycle_period);
@@ -273,7 +290,7 @@ pub mod pallet {
 			let rank = T::Members::rank_of(&who).ok_or(Error::<T, I>::NotMember)?;
 			let mut status = Status::<T, I>::get().ok_or(Error::<T, I>::NotStarted)?;
 			let mut claimant = Claimant::<T, I>::get(&who).ok_or(Error::<T, I>::NotInducted)?;
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			ensure!(
 				now < status.cycle_start + T::RegistrationPeriod::get(),
 				Error::<T, I>::TooLate
@@ -373,17 +390,17 @@ pub mod pallet {
 		pub fn status() -> Option<StatusOf<T, I>> {
 			Status::<T, I>::get()
 		}
-		pub fn last_active(who: &T::AccountId) -> Result<CycleIndexOf<T>, DispatchError> {
+		pub fn last_active(who: &T::AccountId) -> Result<CycleIndexOf<T, I>, DispatchError> {
 			Ok(Claimant::<T, I>::get(&who).ok_or(Error::<T, I>::NotInducted)?.last_active)
 		}
-		pub fn cycle_period() -> BlockNumberFor<T> {
+		pub fn cycle_period() -> BlockNumberFor<T, I> {
 			T::RegistrationPeriod::get() + T::PayoutPeriod::get()
 		}
 		fn do_payout(who: T::AccountId, beneficiary: T::AccountId) -> DispatchResult {
 			let mut status = Status::<T, I>::get().ok_or(Error::<T, I>::NotStarted)?;
 			let mut claimant = Claimant::<T, I>::get(&who).ok_or(Error::<T, I>::NotInducted)?;
 
-			let now = frame_system::Pallet::<T>::block_number();
+			let now = T::BlockNumberProvider::current_block_number();
 			ensure!(
 				now >= status.cycle_start + T::RegistrationPeriod::get(),
 				Error::<T, I>::TooEarly,