diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 5e087832f0e82..79c240323878b 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; type OriginPrivilegeCmp = EqualOrGreatestRootCmp; type Preimages = Preimage; + type BlockNumberProvider = frame_system::Pallet; } parameter_types! { diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 6195a9e356d47..15d183c301a87 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; type OriginPrivilegeCmp = OriginPrivilegeCmp; type Preimages = Preimage; + type BlockNumberProvider = frame_system::Pallet; } parameter_types! { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 11788c0193e49..3aa3e26d030e6 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; type OriginPrivilegeCmp = frame_support::traits::EqualPrivilegeOnly; type Preimages = Preimage; + type BlockNumberProvider = frame_system::Pallet; } parameter_types! { diff --git a/prdoc/pr_7441.prdoc b/prdoc/pr_7441.prdoc new file mode 100644 index 0000000000000..ef956ff4bebfe --- /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::::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::` without issue. To retain the + previous behavior, set `BlockNumberProvider` to `frame_system::Pallet::`. + +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 6e847a7dc6555..3e11dcc674ec7 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; type OriginPrivilegeCmp = EqualPrivilegeOnly; type Preimages = Preimage; + type BlockNumberProvider = frame_system::Pallet; } impl pallet_glutton::Config for Runtime { diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs index 7777448006848..91f239476610c 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; } #[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)] diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs index c46236586f1f7..10e5f35bbabf1 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; } #[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 ff40e8ef8abfc..e7bd355f3a9bb 100644 --- a/substrate/frame/scheduler/src/benchmarking.rs +++ b/substrate/frame/scheduler/src/benchmarking.rs @@ -49,10 +49,7 @@ fn assert_last_event(generic_event: ::RuntimeEvent) { /// - `None`: aborted (hash without preimage) /// - `Some(true)`: hash resolves into call if possible, plain call otherwise /// - `Some(false)`: plain call -fn fill_schedule( - when: frame_system::pallet_prelude::BlockNumberFor, - n: u32, -) -> Result<(), &'static str> { +fn fill_schedule(when: BlockNumberFor, n: u32) -> Result<(), &'static str> { let t = DispatchTime::At(when); let origin: ::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 468099010bf97..80ba7fd06da07 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 = pub type BoundedCallOf = Bounded<::RuntimeCall, ::Hashing>; +pub type BlockNumberFor = + <::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 { @@ -230,7 +230,7 @@ impl 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 + 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 Hooks> for Pallet { + impl Hooks> for Pallet { /// Execute the scheduled calls - fn on_initialize(now: BlockNumberFor) -> Weight { + fn on_initialize(_now: SystemBlockNumberFor) -> 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 Pallet { fn resolve_time( when: DispatchTime>, ) -> Result, DispatchError> { - let now = frame_system::Pallet::::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 Pallet { 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 a304689a120cc..f3d04f215ee0d 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 43a964bcf1497..a9aea97542acd 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, EnsureSignedBy>; + type OriginPrivilegeCmp = EqualPrivilegeOnly; type MaxScheduledPerBlock = ConstU32<10>; type WeightInfo = TestWeightInfo; - type OriginPrivilegeCmp = EqualPrivilegeOnly; type Preimages = Preimage; + type BlockNumberProvider = frame_system::Pallet; } pub type LoggerCall = logger::Call; diff --git a/substrate/frame/scheduler/src/tests.rs b/substrate/frame/scheduler/src/tests.rs index 7552239341088..d0a3acc05ac7e 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 + ::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) + ::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 + ::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) + ::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 + ::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) + ::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); + ::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 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 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::(); assert_eq_uvec!(