From 645a6f40927c8acea3f32a6363242456de009321 Mon Sep 17 00:00:00 2001
From: seemantaggarwal <32275622+seemantaggarwal@users.noreply.github.com>
Date: Thu, 13 Feb 2025 06:00:37 +0530
Subject: [PATCH] Update Scheduler to have a configurable block provider #7434
(#7441)
Follow up from
https://github.com/paritytech/polkadot-sdk/pull/6362#issuecomment-2629744365
The goal of this PR is to have the scheduler pallet work on a parachain
which does not produce blocks on a regular schedule, thus can use the
relay chain as a block provider.
Because blocks are not produced regularly, we cannot make the assumption
that block number increases monotonically, and thus have new logic to
handle multiple spend periods passing between blocks.
Requirement:
instead of using the hard coded system block number. We add an
associated type BlockNumberProvider
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
---
.../collectives-westend/src/lib.rs | 1 +
polkadot/runtime/rococo/src/lib.rs | 1 +
polkadot/runtime/westend/src/lib.rs | 1 +
prdoc/pr_7441.prdoc | 25 +++++++++
substrate/bin/node/runtime/src/lib.rs | 1 +
substrate/frame/democracy/src/tests.rs | 1 +
substrate/frame/referenda/src/mock.rs | 1 +
substrate/frame/scheduler/src/benchmarking.rs | 5 +-
substrate/frame/scheduler/src/lib.rs | 53 ++++++++++++++-----
substrate/frame/scheduler/src/migration.rs | 1 -
substrate/frame/scheduler/src/mock.rs | 3 +-
substrate/frame/scheduler/src/tests.rs | 36 +++++++------
12 files changed, 95 insertions(+), 34 deletions(-)
create mode 100644 prdoc/pr_7441.prdoc
diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs
index 5e087832f0e..79c24032387 100644
--- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs
+++ b/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! {
diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs
index 6195a9e356d..15d183c301a 100644
--- a/polkadot/runtime/rococo/src/lib.rs
+++ b/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! {
diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs
index 11788c0193e..3aa3e26d030 100644
--- a/polkadot/runtime/westend/src/lib.rs
+++ b/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! {
diff --git a/prdoc/pr_7441.prdoc b/prdoc/pr_7441.prdoc
new file mode 100644
index 00000000000..ef956ff4beb
--- /dev/null
+++ b/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
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 6e847a7dc65..3e11dcc674e 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/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 {
diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs
index 77774480068..91f23947661 100644
--- a/substrate/frame/democracy/src/tests.rs
+++ b/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)]
diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs
index c46236586f1..10e5f35bbab 100644
--- a/substrate/frame/referenda/src/mock.rs
+++ b/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 {
diff --git a/substrate/frame/scheduler/src/benchmarking.rs b/substrate/frame/scheduler/src/benchmarking.rs
index ff40e8ef8ab..e7bd355f3a9 100644
--- a/substrate/frame/scheduler/src/benchmarking.rs
+++ b/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 {
diff --git a/substrate/frame/scheduler/src/lib.rs b/substrate/frame/scheduler/src/lib.rs
index 468099010bf..80ba7fd06da 100644
--- a/substrate/frame/scheduler/src/lib.rs
+++ b/substrate/frame/scheduler/src/lib.rs
@@ -100,14 +100,11 @@ 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},
+ traits::{BadOrigin, BlockNumberProvider, Dispatchable, One, Saturating, Zero},
BoundedVec, DispatchError, RuntimeDebug,
};
@@ -125,6 +122,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 +230,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 +292,35 @@ 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.
+ ///
+ /// There is currently no migration provided to "hot-swap" block number providers and it is
+ /// therefore highly advised to stay with the default (local) values. If you still want to
+ /// swap block number providers on the fly, then please at least ensure that you do not run
+ /// any pallet migration in the same runtime upgrade.
+ type BlockNumberProvider: BlockNumberProvider;
}
#[pallet::storage]
@@ -374,11 +403,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 +919,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 +1194,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();
diff --git a/substrate/frame/scheduler/src/migration.rs b/substrate/frame/scheduler/src/migration.rs
index a304689a120..f3d04f215ee 100644
--- a/substrate/frame/scheduler/src/migration.rs
+++ b/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;
diff --git a/substrate/frame/scheduler/src/mock.rs b/substrate/frame/scheduler/src/mock.rs
index 43a964bcf14..a9aea97542a 100644
--- a/substrate/frame/scheduler/src/mock.rs
+++ b/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>;
diff --git a/substrate/frame/scheduler/src/tests.rs b/substrate/frame/scheduler/src/tests.rs
index 75522393410..d0a3acc05ac 100644
--- a/substrate/frame/scheduler/src/tests.rs
+++ b/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!(
--
GitLab