Update Scheduler to have a configurable block provider #7434

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

@@ -628,6 +628,7 @@ impl pallet_scheduler::Config for Runtime {
 	type WeightInfo = weights::pallet_scheduler::WeightInfo<Runtime>;
 	type OriginPrivilegeCmp = EqualOrGreatestRootCmp;
 	type Preimages = Preimage;
+	type BlockNumberProvider = frame_system::Pallet<Runtime>;
 }
 
 parameter_types! {

polkadot/runtime/rococo/src/lib.rs

@@ -344,6 +344,7 @@ impl pallet_scheduler::Config for Runtime {
 	type WeightInfo = weights::pallet_scheduler::WeightInfo<Runtime>;
 	type OriginPrivilegeCmp = OriginPrivilegeCmp;
 	type Preimages = Preimage;
+	type BlockNumberProvider = frame_system::Pallet<Runtime>;
 }
 
 parameter_types! {

polkadot/runtime/westend/src/lib.rs

@@ -250,6 +250,7 @@ impl pallet_scheduler::Config for Runtime {
 	type WeightInfo = weights::pallet_scheduler::WeightInfo<Runtime>;
 	type OriginPrivilegeCmp = frame_support::traits::EqualPrivilegeOnly;
 	type Preimages = Preimage;
+	type BlockNumberProvider = frame_system::Pallet<Runtime>;
 }
 
 parameter_types! {

prdoc/pr_7441.prdoc

@@ -0,0 +1,25 @@
+title: 'Update Scheduler to have a configurable block number provider'
+doc:
+- audience: Runtime Dev
+  description: |-
+    This PR makes `pallet_scheduler` configurable by introducing `BlockNumberProvider` in
+    `pallet_scheduler::Config`. Instead of relying solely on
+    `frame_system::Pallet::<T>::block_number()`, the scheduler can now use any block number source,
+    including external providers like the relay chain.
+
+    Parachains can continue using `frame_system::Pallet::<Runtime>` without issue. To retain the
+    previous behavior, set `BlockNumberProvider` to `frame_system::Pallet::<Runtime>`.
+
+crates:
+- name: collectives-westend-runtime
+  bump: patch
+- name: rococo-runtime
+  bump: patch
+- name: westend-runtime
+  bump: patch
+- name: pallet-democracy
+  bump: patch
+- name: pallet-referenda
+  bump: patch
+- name: pallet-scheduler
+  bump: major

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

@@ -504,6 +504,7 @@ impl pallet_scheduler::Config for Runtime {
 	type WeightInfo = pallet_scheduler::weights::SubstrateWeight<Runtime>;
 	type OriginPrivilegeCmp = EqualPrivilegeOnly;
 	type Preimages = Preimage;
+	type BlockNumberProvider = frame_system::Pallet<Runtime>;
 }
 
 impl pallet_glutton::Config for Runtime {

substrate/frame/democracy/src/tests.rs

@@ -106,6 +106,7 @@ impl pallet_scheduler::Config for Test {
 	type WeightInfo = ();
 	type OriginPrivilegeCmp = EqualPrivilegeOnly;
 	type Preimages = ();
+	type BlockNumberProvider = frame_system::Pallet<Test>;
 }
 
 #[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)]

substrate/frame/referenda/src/mock.rs

@@ -81,6 +81,7 @@ impl pallet_scheduler::Config for Test {
 	type WeightInfo = ();
 	type OriginPrivilegeCmp = EqualPrivilegeOnly;
 	type Preimages = Preimage;
+	type BlockNumberProvider = frame_system::Pallet<Test>;
 }
 #[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)]
 impl pallet_balances::Config for Test {

substrate/frame/scheduler/src/benchmarking.rs

@@ -49,10 +49,7 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
 /// - `None`: aborted (hash without preimage)
 /// - `Some(true)`: hash resolves into call if possible, plain call otherwise
 /// - `Some(false)`: plain call
-fn fill_schedule<T: Config>(
-	when: frame_system::pallet_prelude::BlockNumberFor<T>,
-	n: u32,
-) -> Result<(), &'static str> {
+fn fill_schedule<T: Config>(when: BlockNumberFor<T>, n: u32) -> Result<(), &'static str> {
 	let t = DispatchTime::At(when);
 	let origin: <T as Config>::PalletsOrigin = frame_system::RawOrigin::Root.into();
 	for i in 0..n {

substrate/frame/scheduler/src/lib.rs

@@ -100,17 +100,16 @@ use frame_support::{
 	},
 	weights::{Weight, WeightMeter},
 };
-use frame_system::{
-	pallet_prelude::BlockNumberFor,
-	{self as system},
-};
+use frame_system::{self as system};
 use scale_info::TypeInfo;
 use sp_io::hashing::blake2_256;
 use sp_runtime::{
 	traits::{BadOrigin, Dispatchable, One, Saturating, Zero},
 	BoundedVec, DispatchError, RuntimeDebug,
 };
 
+use sp_runtime::traits::BlockNumberProvider;
+
 pub use pallet::*;
 pub use weights::WeightInfo;
 
@@ -125,6 +124,9 @@ pub type CallOrHashOf<T> =
 pub type BoundedCallOf<T> =
 	Bounded<<T as Config>::RuntimeCall, <T as frame_system::Config>::Hashing>;
 
+pub type BlockNumberFor<T> =
+	<<T as Config>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
+
 /// The configuration of the retry mechanism for a given task along with its current state.
 #[derive(Clone, Copy, RuntimeDebug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo)]
 pub struct RetryConfig<Period> {
@@ -230,7 +232,7 @@ impl<T: WeightInfo> MarginalWeightInfo for T {}
 pub mod pallet {
 	use super::*;
 	use frame_support::{dispatch::PostDispatchInfo, pallet_prelude::*};
-	use frame_system::pallet_prelude::*;
+	use frame_system::pallet_prelude::{BlockNumberFor as SystemBlockNumberFor, OriginFor};
 
 	/// The in-code storage version.
 	const STORAGE_VERSION: StorageVersion = StorageVersion::new(4);
@@ -292,6 +294,30 @@ pub mod pallet {
 
 		/// The preimage provider with which we look up call hashes to get the call.
 		type Preimages: QueryPreimage<H = Self::Hashing> + StorePreimage;
+
+		/// Query the current block number.
+		///
+		/// Must return monotonically increasing values when called from consecutive blocks. It is
+		/// generally expected that the values also do not differ "too much" between consecutive
+		/// blocks. A future addition to this pallet will allow bigger difference between
+		/// consecutive blocks to make it possible to be utilized by parachains with *Agile
+		/// Coretime*. *Agile Coretime* parachains are currently not supported and must continue to
+		/// use their local block number provider.
+		///
+		/// Can be configured to return either:
+		/// - the local block number of the runtime via `frame_system::Pallet`
+		/// - a remote block number, eg from the relay chain through `RelaychainDataProvider`
+		/// - an arbitrary value through a custom implementation of the trait
+		///
+		/// Suggested values:
+		/// - Solo- and Relay-chains should use `frame_system::Pallet`. There are no concerns with
+		///   this configuration.
+		/// - Parachains should also use `frame_system::Pallet` for the time being. The scheduler
+		///   pallet is not yet ready for the case that big numbers of blocks are skipped. In an
+		///   *Agile Coretime* chain with relay chain number provider configured, it could otherwise
+		///   happen that the scheduler will not be able to catch up to its agendas, since too many
+		///   relay blocks are missing if the parachain only produces blocks rarely.
+		type BlockNumberProvider: BlockNumberProvider;
 	}
 
 	#[pallet::storage]
@@ -374,11 +400,12 @@ pub mod pallet {
 	}
 
 	#[pallet::hooks]
-	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+	impl<T: Config> Hooks<SystemBlockNumberFor<T>> for Pallet<T> {
 		/// Execute the scheduled calls
-		fn on_initialize(now: BlockNumberFor<T>) -> Weight {
+		fn on_initialize(_now: SystemBlockNumberFor<T>) -> Weight {
+			let now = T::BlockNumberProvider::current_block_number();
 			let mut weight_counter = WeightMeter::with_limit(T::MaximumWeight::get());
-			Self::service_agendas(&mut weight_counter, now, u32::max_value());
+			Self::service_agendas(&mut weight_counter, now, u32::MAX);
 			weight_counter.consumed()
 		}
 	}
@@ -889,8 +916,7 @@ impl<T: Config> Pallet<T> {
 	fn resolve_time(
 		when: DispatchTime<BlockNumberFor<T>>,
 	) -> Result<BlockNumberFor<T>, DispatchError> {
-		let now = frame_system::Pallet::<T>::block_number();
-
+		let now = T::BlockNumberProvider::current_block_number();
 		let when = match when {
 			DispatchTime::At(x) => x,
 			// The current block has already completed it's scheduled tasks, so
@@ -1165,7 +1191,7 @@ impl<T: Config> Pallet<T> {
 		let mut count_down = max;
 		let service_agenda_base_weight = T::WeightInfo::service_agenda_base(max_items);
 		while count_down > 0 && when <= now && weight.can_consume(service_agenda_base_weight) {
-			if !Self::service_agenda(weight, &mut executed, now, when, u32::max_value()) {
+			if !Self::service_agenda(weight, &mut executed, now, when, u32::MAX) {
 				incomplete_since = incomplete_since.min(when);
 			}
 			when.saturating_inc();

substrate/frame/scheduler/src/migration.rs

@@ -19,7 +19,6 @@
 
 use super::*;
 use frame_support::traits::OnRuntimeUpgrade;
-use frame_system::pallet_prelude::BlockNumberFor;
 
 #[cfg(feature = "try-runtime")]
 use sp_runtime::TryRuntimeError;

substrate/frame/scheduler/src/mock.rs

@@ -223,10 +223,11 @@ impl Config for Test {
 	type RuntimeCall = RuntimeCall;
 	type MaximumWeight = MaximumSchedulerWeight;
 	type ScheduleOrigin = EitherOfDiverse<EnsureRoot<u64>, EnsureSignedBy<One, u64>>;
+	type OriginPrivilegeCmp = EqualPrivilegeOnly;
 	type MaxScheduledPerBlock = ConstU32<10>;
 	type WeightInfo = TestWeightInfo;
-	type OriginPrivilegeCmp = EqualPrivilegeOnly;
 	type Preimages = Preimage;
+	type BlockNumberProvider = frame_system::Pallet<Self>;
 }
 
 pub type LoggerCall = logger::Call<Test>;

substrate/frame/scheduler/src/tests.rs

@@ -1636,8 +1636,9 @@ fn on_initialize_weight_is_correct() {
 		));
 
 		// Will include the named periodic only
+		<Test as Config>::BlockNumberProvider::set_block_number(1);
 		assert_eq!(
-			Scheduler::on_initialize(1),
+			Scheduler::on_initialize(42), // BN unused
 			TestWeightInfo::service_agendas_base() +
 				TestWeightInfo::service_agenda_base(1) +
 				<TestWeightInfo as MarginalWeightInfo>::service_task(None, true, true) +
@@ -1648,8 +1649,9 @@ fn on_initialize_weight_is_correct() {
 		assert_eq!(logger::log(), vec![(root(), 2600u32)]);
 
 		// Will include anon and anon periodic
+		<Test as Config>::BlockNumberProvider::set_block_number(2);
 		assert_eq!(
-			Scheduler::on_initialize(2),
+			Scheduler::on_initialize(123), // BN unused
 			TestWeightInfo::service_agendas_base() +
 				TestWeightInfo::service_agenda_base(2) +
 				<TestWeightInfo as MarginalWeightInfo>::service_task(None, false, true) +
@@ -1663,8 +1665,9 @@ fn on_initialize_weight_is_correct() {
 		assert_eq!(logger::log(), vec![(root(), 2600u32), (root(), 69u32), (root(), 42u32)]);
 
 		// Will include named only
+		<Test as Config>::BlockNumberProvider::set_block_number(3);
 		assert_eq!(
-			Scheduler::on_initialize(3),
+			Scheduler::on_initialize(555), // BN unused
 			TestWeightInfo::service_agendas_base() +
 				TestWeightInfo::service_agenda_base(1) +
 				<TestWeightInfo as MarginalWeightInfo>::service_task(None, true, false) +
@@ -1678,7 +1681,8 @@ fn on_initialize_weight_is_correct() {
 		);
 
 		// Will contain none
-		let actual_weight = Scheduler::on_initialize(4);
+		<Test as Config>::BlockNumberProvider::set_block_number(4);
+		let actual_weight = Scheduler::on_initialize(444); // BN unused
 		assert_eq!(
 			actual_weight,
 			TestWeightInfo::service_agendas_base() + TestWeightInfo::service_agenda_base(0)
@@ -2116,6 +2120,18 @@ fn migration_to_v4_works() {
 	});
 }
 
+impl Into<OriginCaller> for u32 {
+	fn into(self) -> OriginCaller {
+		match self {
+			3u32 => system::RawOrigin::Root.into(),
+			2u32 => system::RawOrigin::None.into(),
+			101u32 => system::RawOrigin::Signed(101).into(),
+			102u32 => system::RawOrigin::Signed(102).into(),
+			_ => unreachable!("test make no use of it"),
+		}
+	}
+}
+
 #[test]
 fn test_migrate_origin() {
 	new_test_ext().execute_with(|| {
@@ -2151,18 +2167,6 @@ fn test_migrate_origin() {
 			frame_support::migration::put_storage_value(b"Scheduler", b"Agenda", &k, old);
 		}
 
-		impl Into<OriginCaller> for u32 {
-			fn into(self) -> OriginCaller {
-				match self {
-					3u32 => system::RawOrigin::Root.into(),
-					2u32 => system::RawOrigin::None.into(),
-					101u32 => system::RawOrigin::Signed(101).into(),
-					102u32 => system::RawOrigin::Signed(102).into(),
-					_ => unreachable!("test make no use of it"),
-				}
-			}
-		}
-
 		Scheduler::migrate_origin::<u32>();
 
 		assert_eq_uvec!(