diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index 9e39d9d24e..00d1c6dd72 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -148,6 +148,7 @@ runtime-benchmarks = [ "pallet-elections-phragmen/runtime-benchmarks", "pallet-identity/runtime-benchmarks", "pallet-im-online/runtime-benchmarks", + "pallet-scheduler/runtime-benchmarks", "pallet-society/runtime-benchmarks", "pallet-staking/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 294aac8ae3..b8d2c51e27 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -967,6 +967,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, b"identity", Identity); add_benchmark!(params, batches, b"im-online", ImOnline); add_benchmark!(params, batches, b"offences", OffencesBench::); + add_benchmark!(params, batches, b"scheduler", Scheduler); add_benchmark!(params, batches, b"session", SessionBench::); add_benchmark!(params, batches, b"staking", Staking); add_benchmark!(params, batches, b"system", SystemBench::); diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index 4459a1aaa5..9896f987ed 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -69,7 +69,7 @@ fn add_referendum(n: u32) -> Result { ); let referendum_index: ReferendumIndex = ReferendumCount::get() - 1; let _ = T::Scheduler::schedule_named( - (DEMOCRACY_ID, referendum_index), + (DEMOCRACY_ID, referendum_index).encode(), 0.into(), None, 63, diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index d924ca9666..df1ea0d531 100644 --- a/substrate/frame/democracy/src/lib.rs +++ b/substrate/frame/democracy/src/lib.rs @@ -832,7 +832,7 @@ decl_module! { #[weight = (0, DispatchClass::Operational)] fn cancel_queued(origin, which: ReferendumIndex) { ensure_root(origin)?; - T::Scheduler::cancel_named((DEMOCRACY_ID, which)) + T::Scheduler::cancel_named((DEMOCRACY_ID, which).encode()) .map_err(|_| Error::::ProposalMissing)?; } @@ -1659,7 +1659,7 @@ impl Module { }); if T::Scheduler::schedule_named( - (DEMOCRACY_ID, index), + (DEMOCRACY_ID, index).encode(), when, None, 63, diff --git a/substrate/frame/scheduler/Cargo.toml b/substrate/frame/scheduler/Cargo.toml index 8a511cd389..6cc9161eea 100644 --- a/substrate/frame/scheduler/Cargo.toml +++ b/substrate/frame/scheduler/Cargo.toml @@ -11,13 +11,14 @@ description = "FRAME example pallet" [dependencies] serde = { version = "1.0.101", optional = true } codec = { package = "parity-scale-codec", version = "1.2.0", default-features = false } -frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../benchmarking" } frame-support = { version = "2.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "2.0.0-dev", default-features = false, path = "../system" } sp-runtime = { version = "2.0.0-dev", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "2.0.0-dev", default-features = false, path = "../../primitives/std" } sp-io = { version = "2.0.0-dev", default-features = false, path = "../../primitives/io" } +frame-benchmarking = { version = "2.0.0-dev", default-features = false, path = "../benchmarking", optional = true } + [dev-dependencies] sp-core = { version = "2.0.0-dev", path = "../../primitives/core", default-features = false } @@ -33,3 +34,4 @@ std = [ "sp-io/std", "sp-std/std" ] +runtime-benchmarks = ["frame-benchmarking"] diff --git a/substrate/frame/scheduler/src/benchmarking.rs b/substrate/frame/scheduler/src/benchmarking.rs new file mode 100644 index 0000000000..ac9c19f7c3 --- /dev/null +++ b/substrate/frame/scheduler/src/benchmarking.rs @@ -0,0 +1,158 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Scheduler pallet benchmarking. + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; +use sp_std::{vec, prelude::*}; +use frame_system::RawOrigin; +use frame_support::{ensure, traits::OnInitialize}; +use frame_benchmarking::benchmarks; + +use crate::Module as Scheduler; +use frame_system::Module as System; + +const MAX_SCHEDULED: u32 = 50; + +// Add `n` named items to the schedule +fn fill_schedule (when: T::BlockNumber, n: u32) -> Result<(), &'static str> { + // Essentially a no-op call. + let call = frame_system::Call::set_storage(vec![]); + for i in 0..n { + // Named schedule is strictly heavier than anonymous + Scheduler::::do_schedule_named( + i.encode(), + when, + // Add periodicity + Some((T::BlockNumber::one(), 100)), + // HARD_DEADLINE priority means it gets executed no matter what + 0, + call.clone().into(), + )?; + } + ensure!(Agenda::::get(when).len() == n as usize, "didn't fill schedule"); + Ok(()) +} + +benchmarks! { + _ { } + + schedule { + let s in 0 .. MAX_SCHEDULED; + let when = T::BlockNumber::one(); + let periodic = Some((T::BlockNumber::one(), 100)); + let priority = 0; + // Essentially a no-op call. + let call = Box::new(frame_system::Call::set_storage(vec![]).into()); + + fill_schedule::(when, s)?; + }: _(RawOrigin::Root, when, periodic, priority, call) + verify { + ensure!( + Agenda::::get(when).len() == (s + 1) as usize, + "didn't add to schedule" + ); + } + + cancel { + let s in 1 .. MAX_SCHEDULED; + let when: T::BlockNumber = 2.into(); + + fill_schedule::(when, s)?; + assert_eq!(Agenda::::get(when).len(), s as usize); + }: _(RawOrigin::Root, when, 0) + verify { + ensure!( + Lookup::::get(0.encode()).is_none(), + "didn't remove from lookup" + ); + // Removed schedule is NONE + ensure!( + Agenda::::get(when)[0].is_none(), + "didn't remove from schedule" + ); + } + + schedule_named { + let s in 0 .. MAX_SCHEDULED; + let id = s.encode(); + let when = T::BlockNumber::one(); + let periodic = Some((T::BlockNumber::one(), 100)); + let priority = 0; + // Essentially a no-op call. + let call = Box::new(frame_system::Call::set_storage(vec![]).into()); + + fill_schedule::(when, s)?; + }: _(RawOrigin::Root, id, when, periodic, priority, call) + verify { + ensure!( + Agenda::::get(when).len() == (s + 1) as usize, + "didn't add to schedule" + ); + } + + cancel_named { + let s in 1 .. MAX_SCHEDULED; + let when = T::BlockNumber::one(); + + fill_schedule::(when, s)?; + }: _(RawOrigin::Root, 0.encode()) + verify { + ensure!( + Lookup::::get(0.encode()).is_none(), + "didn't remove from lookup" + ); + // Removed schedule is NONE + ensure!( + Agenda::::get(when)[0].is_none(), + "didn't remove from schedule" + ); + } + + on_initialize { + let s in 0 .. MAX_SCHEDULED; + let when = T::BlockNumber::one(); + fill_schedule::(when, s)?; + }: { Scheduler::::on_initialize(T::BlockNumber::one()); } + verify { + assert_eq!(System::::event_count(), s); + // Next block should have all the schedules again + ensure!( + Agenda::::get(when + T::BlockNumber::one()).len() == s as usize, + "didn't append schedule" + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{new_test_ext, Test}; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks() { + new_test_ext().execute_with(|| { + assert_ok!(test_benchmark_schedule::()); + assert_ok!(test_benchmark_cancel::()); + assert_ok!(test_benchmark_schedule_named::()); + assert_ok!(test_benchmark_cancel_named::()); + assert_ok!(test_benchmark_on_initialize::()); + }); + } +} diff --git a/substrate/frame/scheduler/src/lib.rs b/substrate/frame/scheduler/src/lib.rs index 975331059b..87b5aafdf3 100644 --- a/substrate/frame/scheduler/src/lib.rs +++ b/substrate/frame/scheduler/src/lib.rs @@ -44,15 +44,18 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] +mod benchmarking; + use sp_std::prelude::*; use codec::{Encode, Decode}; use sp_runtime::{RuntimeDebug, traits::{Zero, One}}; use frame_support::{ - dispatch::{Dispatchable, DispatchResult, Parameter}, decl_module, decl_storage, decl_event, + decl_module, decl_storage, decl_event, decl_error, + dispatch::{Dispatchable, DispatchError, DispatchResult, Parameter}, traits::{Get, schedule}, weights::{GetDispatchInfo, Weight}, }; -use frame_system as system; +use frame_system::{self as system, ensure_root}; /// Our pallet's configuration trait. All our types and constants go in here. If the /// pallet is dependent on specific other pallets, then their configuration traits @@ -67,7 +70,7 @@ pub trait Trait: system::Trait { type Origin: From>; /// The aggregated call type. - type Call: Parameter + Dispatchable::Origin> + GetDispatchInfo; + type Call: Parameter + Dispatchable::Origin> + GetDispatchInfo + From>; /// The maximum weight that may be scheduled per block for any dispatchables of less priority /// than `schedule::HARD_DEADLINE`. @@ -105,16 +108,112 @@ decl_storage! { decl_event!( pub enum Event where ::BlockNumber { - Scheduled(BlockNumber), + Scheduled(BlockNumber, u32), + Canceled(BlockNumber, u32), Dispatched(TaskAddress, Option>, DispatchResult), } ); +decl_error! { + pub enum Error for Module { + /// Failed to schedule a call + FailedToSchedule, + /// Failed to cancel a scheduled call + FailedToCancel, + } +} + decl_module! { // Simple declaration of the `Module` type. Lets the macro know what its working on. pub struct Module for enum Call where origin: ::Origin { fn deposit_event() = default; + /// Anonymously schedule a task. + /// + /// # + /// - S = Number of already scheduled calls + /// - Base Weight: 22.29 + .126 * S µs + /// - DB Weight: + /// - Read: Agenda + /// - Write: Agenda + /// - Will use base weight of 25 which should be good for up to 30 scheduled calls + /// # + #[weight = 25_000_000 + T::DbWeight::get().reads_writes(1, 1)] + fn schedule(origin, + when: T::BlockNumber, + maybe_periodic: Option>, + priority: schedule::Priority, + call: Box<::Call>, + ) { + ensure_root(origin)?; + let _ = Self::do_schedule(when, maybe_periodic, priority, *call); + } + + /// Cancel an anonymously scheduled task. + /// + /// # + /// - S = Number of already scheduled calls + /// - Base Weight: 22.15 + 2.869 * S µs + /// - DB Weight: + /// - Read: Agenda + /// - Write: Agenda, Lookup + /// - Will use base weight of 100 which should be good for up to 30 scheduled calls + /// # + #[weight = 100_000_000 + T::DbWeight::get().reads_writes(1, 2)] + fn cancel(origin, when: T::BlockNumber, index: u32) { + ensure_root(origin)?; + Self::do_cancel((when, index))?; + } + + /// Schedule a named task. + /// + /// # + /// - S = Number of already scheduled calls + /// - Base Weight: 29.6 + .159 * S µs + /// - DB Weight: + /// - Read: Agenda, Lookup + /// - Write: Agenda, Lookup + /// - Will use base weight of 35 which should be good for more than 30 scheduled calls + /// # + #[weight = 35_000_000 + T::DbWeight::get().reads_writes(2, 2)] + fn schedule_named(origin, + id: Vec, + when: T::BlockNumber, + maybe_periodic: Option>, + priority: schedule::Priority, + call: Box<::Call>, + ) { + ensure_root(origin)?; + Self::do_schedule_named(id, when, maybe_periodic, priority, *call)?; + } + + /// Cancel a named scheduled task. + /// + /// # + /// - S = Number of already scheduled calls + /// - Base Weight: 24.91 + 2.907 * S µs + /// - DB Weight: + /// - Read: Agenda, Lookup + /// - Write: Agenda, Lookup + /// - Will use base weight of 100 which should be good for up to 30 scheduled calls + /// # + #[weight = 100_000_000 + T::DbWeight::get().reads_writes(2, 2)] + fn cancel_named(origin, id: Vec) { + ensure_root(origin)?; + Self::do_cancel_named(id)?; + } + + /// Execute the scheduled calls + /// + /// # + /// - S = Number of already scheduled calls + /// - N = Named scheduled calls + /// - P = Periodic Calls + /// - Base Weight: 9.243 + 23.45 * S µs + /// - DB Weight: + /// - Read: Agenda + Lookup * N + Agenda(Future) * P + /// - Write: Agenda + Lookup * N + Agenda(future) * P + /// # fn on_initialize(now: T::BlockNumber) -> Weight { let limit = T::MaximumWeight::get(); let mut queued = Agenda::::take(now).into_iter() @@ -122,14 +221,32 @@ decl_module! { .filter_map(|(index, s)| s.map(|inner| (index as u32, inner))) .collect::>(); queued.sort_by_key(|(_, s)| s.priority); - let mut result = 0; + let base_weight: Weight = T::DbWeight::get().reads_writes(1, 2) // Agenda + Agenda(next) + .saturating_add(10_000_000); // Base Weight + let mut total_weight: Weight = 0; queued.into_iter() .enumerate() - .scan(0, |cumulative_weight, (order, (index, s))| { - *cumulative_weight += s.call.get_dispatch_info().weight; + .scan(base_weight, |cumulative_weight, (order, (index, s))| { + *cumulative_weight = cumulative_weight + .saturating_add(s.call.get_dispatch_info().weight) + .saturating_add(25_000_000); // Base multiplier + + if s.maybe_id.is_some() { + // Remove/Modify Lookup + *cumulative_weight = cumulative_weight.saturating_add(T::DbWeight::get().writes(1)); + } + if s.maybe_periodic.is_some() { + // Read/Write Agenda for future block + *cumulative_weight = cumulative_weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); + } + Some((order, index, *cumulative_weight, s)) }) .filter_map(|(order, index, cumulative_weight, mut s)| { + // We allow a scheduled call if any is true: + // - It's priority is `HARD_DEADLINE` + // - It does not push the weight past the limit. + // - It is the first item in the schedule if s.priority <= schedule::HARD_DEADLINE || cumulative_weight <= limit || order == 0 { let r = s.call.clone().dispatch(system::RawOrigin::Root.into()); let maybe_id = s.maybe_id.clone(); @@ -140,6 +257,7 @@ decl_module! { s.maybe_periodic = None; } let next = now + period; + // If scheduled is named, place it's information in `Lookup` if let Some(ref id) = s.maybe_id { let next_index = Agenda::::decode_len(now + period).unwrap_or(0); Lookup::::insert(id, (next, next_index as u32)); @@ -155,7 +273,7 @@ decl_module! { maybe_id, r.map(|_| ()).map_err(|e| e.error) )); - result = cumulative_weight; + total_weight = cumulative_weight; None } else { Some(Some(s)) @@ -166,20 +284,18 @@ decl_module! { Agenda::::append(next, unused); }); - result + total_weight } } } -impl schedule::Anon::Call> for Module { - type Address = TaskAddress; - - fn schedule( +impl Module { + fn do_schedule( when: T::BlockNumber, maybe_periodic: Option>, priority: schedule::Priority, call: ::Call - ) -> Self::Address { + ) -> TaskAddress { // sanitize maybe_periodic let maybe_periodic = maybe_periodic .filter(|p| p.1 > 1 && !p.0.is_zero()) @@ -187,35 +303,33 @@ impl schedule::Anon::Call> for Module .map(|(p, c)| (p, c - 1)); let s = Some(Scheduled { maybe_id: None, priority, call, maybe_periodic }); Agenda::::append(when, s); - (when, Agenda::::decode_len(when).unwrap_or(1) as u32 - 1) + let index = Agenda::::decode_len(when).unwrap_or(1) as u32 - 1; + Self::deposit_event(RawEvent::Scheduled(when, index)); + (when, index) } - fn cancel((when, index): Self::Address) -> Result<(), ()> { + fn do_cancel((when, index): TaskAddress) -> Result<(), DispatchError> { if let Some(s) = Agenda::::mutate(when, |agenda| agenda.get_mut(index as usize).and_then(Option::take)) { if let Some(id) = s.maybe_id { - Lookup::::remove(id) + Lookup::::remove(id); } + Self::deposit_event(RawEvent::Canceled(when, index)); Ok(()) } else { - Err(()) + Err(Error::::FailedToCancel)? } } -} -impl schedule::Named::Call> for Module { - type Address = TaskAddress; - - fn schedule_named( - id: impl Encode, + fn do_schedule_named( + id: Vec, when: T::BlockNumber, maybe_periodic: Option>, priority: schedule::Priority, call: ::Call, - ) -> Result { - // determine id and ensure it is unique - let id = id.encode(); + ) -> Result, DispatchError> { + // ensure id it is unique if Lookup::::contains_key(&id) { - return Err(()) + return Err(Error::::FailedToSchedule)? } // sanitize maybe_periodic @@ -229,28 +343,65 @@ impl schedule::Named::Call> for Module let index = Agenda::::decode_len(when).unwrap_or(1) as u32 - 1; let address = (when, index); Lookup::::insert(&id, &address); + Self::deposit_event(RawEvent::Scheduled(when, index)); Ok(address) } - fn cancel_named(id: impl Encode) -> Result<(), ()> { - if let Some((when, index)) = id.using_encoded(|d| Lookup::::take(d)) { + fn do_cancel_named(id: Vec) -> Result<(), DispatchError> { + if let Some((when, index)) = Lookup::::take(id) { let i = index as usize; Agenda::::mutate(when, |agenda| if let Some(s) = agenda.get_mut(i) { *s = None }); + Self::deposit_event(RawEvent::Canceled(when, index)); Ok(()) } else { - Err(()) + Err(Error::::FailedToCancel)? } } } +impl schedule::Anon::Call> for Module { + type Address = TaskAddress; + + fn schedule( + when: T::BlockNumber, + maybe_periodic: Option>, + priority: schedule::Priority, + call: ::Call + ) -> Self::Address { + Self::do_schedule(when, maybe_periodic, priority, call) + } + + fn cancel((when, index): Self::Address) -> Result<(), ()> { + Self::do_cancel((when, index)).map_err(|_| ()) + } +} + +impl schedule::Named::Call> for Module { + type Address = TaskAddress; + + fn schedule_named( + id: Vec, + when: T::BlockNumber, + maybe_periodic: Option>, + priority: schedule::Priority, + call: ::Call, + ) -> Result { + Self::do_schedule_named(id, when, maybe_periodic, priority, call).map_err(|_| ()) + } + + fn cancel_named(id: Vec) -> Result<(), ()> { + Self::do_cancel_named(id).map_err(|_| ()) + } +} + #[cfg(test)] mod tests { use super::*; use frame_support::{ impl_outer_event, impl_outer_origin, impl_outer_dispatch, parameter_types, assert_ok, - traits::{OnInitialize, OnFinalize, schedule::{Anon, Named}}, - weights::{DispatchClass, FunctionOf, Pays} + traits::{OnInitialize, OnFinalize}, + weights::{DispatchClass, FunctionOf, Pays, constants::RocksDbWeight}, }; use sp_core::H256; // The testing primitives are very useful for avoiding having to work with signatures @@ -331,7 +482,7 @@ mod tests { pub struct Test; parameter_types! { pub const BlockHashCount: u64 = 250; - pub const MaximumBlockWeight: Weight = 1024; + pub const MaximumBlockWeight: Weight = 2_000_000_000_000; pub const MaximumBlockLength: u32 = 2 * 1024; pub const AvailableBlockRatio: Perbill = Perbill::one(); } @@ -348,7 +499,7 @@ mod tests { type Event = (); type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; - type DbWeight = (); + type DbWeight = RocksDbWeight; type BlockExecutionWeight = (); type ExtrinsicBaseWeight = (); type MaximumBlockLength = MaximumBlockLength; @@ -362,14 +513,11 @@ mod tests { impl logger::Trait for Test { type Event = (); } - parameter_types! { - pub const MaximumWeight: Weight = 10_000; - } impl Trait for Test { type Event = (); type Origin = Origin; type Call = Call; - type MaximumWeight = MaximumWeight; + type MaximumWeight = MaximumBlockWeight; } type System = system::Module; type Logger = logger::Module; @@ -377,7 +525,7 @@ mod tests { // This function basically just builds a genesis storage key/value store according to // our desired mockup. - fn new_test_ext() -> sp_io::TestExternalities { + pub fn new_test_ext() -> sp_io::TestExternalities { let t = system::GenesisConfig::default().build_storage::().unwrap(); t.into() } @@ -393,7 +541,7 @@ mod tests { #[test] fn basic_scheduling_works() { new_test_ext().execute_with(|| { - Scheduler::schedule(4, None, 127, Call::Logger(logger::Call::log(42, 1000))); + Scheduler::do_schedule(4, None, 127, Call::Logger(logger::Call::log(42, 1000))); run_to_block(3); assert!(logger::log().is_empty()); run_to_block(4); @@ -407,7 +555,7 @@ mod tests { fn periodic_scheduling_works() { new_test_ext().execute_with(|| { // at #4, every 3 blocks, 3 times. - Scheduler::schedule(4, Some((3, 3)), 127, Call::Logger(logger::Call::log(42, 1000))); + Scheduler::do_schedule(4, Some((3, 3)), 127, Call::Logger(logger::Call::log(42, 1000))); run_to_block(3); assert!(logger::log().is_empty()); run_to_block(4); @@ -429,12 +577,12 @@ mod tests { fn cancel_named_scheduling_works_with_normal_cancel() { new_test_ext().execute_with(|| { // at #4. - Scheduler::schedule_named(1u32, 4, None, 127, Call::Logger(logger::Call::log(69, 1000))).unwrap(); - let i = Scheduler::schedule(4, None, 127, Call::Logger(logger::Call::log(42, 1000))); + Scheduler::do_schedule_named(1u32.encode(), 4, None, 127, Call::Logger(logger::Call::log(69, 1000))).unwrap(); + let i = Scheduler::do_schedule(4, None, 127, Call::Logger(logger::Call::log(42, 1000))); run_to_block(3); assert!(logger::log().is_empty()); - assert_ok!(Scheduler::cancel_named(1u32)); - assert_ok!(Scheduler::cancel(i)); + assert_ok!(Scheduler::do_cancel_named(1u32.encode())); + assert_ok!(Scheduler::do_cancel(i)); run_to_block(100); assert!(logger::log().is_empty()); }); @@ -444,17 +592,17 @@ mod tests { fn cancel_named_periodic_scheduling_works() { new_test_ext().execute_with(|| { // at #4, every 3 blocks, 3 times. - Scheduler::schedule_named(1u32, 4, Some((3, 3)), 127, Call::Logger(logger::Call::log(42, 1000))).unwrap(); + Scheduler::do_schedule_named(1u32.encode(), 4, Some((3, 3)), 127, Call::Logger(logger::Call::log(42, 1000))).unwrap(); // same id results in error. - assert!(Scheduler::schedule_named(1u32, 4, None, 127, Call::Logger(logger::Call::log(69, 1000))).is_err()); + assert!(Scheduler::do_schedule_named(1u32.encode(), 4, None, 127, Call::Logger(logger::Call::log(69, 1000))).is_err()); // different id is ok. - Scheduler::schedule_named(2u32, 8, None, 127, Call::Logger(logger::Call::log(69, 1000))).unwrap(); + Scheduler::do_schedule_named(2u32.encode(), 8, None, 127, Call::Logger(logger::Call::log(69, 1000))).unwrap(); run_to_block(3); assert!(logger::log().is_empty()); run_to_block(4); assert_eq!(logger::log(), vec![42u32]); run_to_block(6); - assert_ok!(Scheduler::cancel_named(1u32)); + assert_ok!(Scheduler::do_cancel_named(1u32.encode())); run_to_block(100); assert_eq!(logger::log(), vec![42u32, 69u32]); }); @@ -463,8 +611,9 @@ mod tests { #[test] fn scheduler_respects_weight_limits() { new_test_ext().execute_with(|| { - Scheduler::schedule(4, None, 127, Call::Logger(logger::Call::log(42, 6000))); - Scheduler::schedule(4, None, 127, Call::Logger(logger::Call::log(69, 6000))); + Scheduler::do_schedule(4, None, 127, Call::Logger(logger::Call::log(42, MaximumBlockWeight::get() / 2))); + Scheduler::do_schedule(4, None, 127, Call::Logger(logger::Call::log(69, MaximumBlockWeight::get() / 2))); + // 69 and 42 do not fit together run_to_block(4); assert_eq!(logger::log(), vec![42u32]); run_to_block(5); @@ -475,8 +624,9 @@ mod tests { #[test] fn scheduler_respects_hard_deadlines_more() { new_test_ext().execute_with(|| { - Scheduler::schedule(4, None, 0, Call::Logger(logger::Call::log(42, 6000))); - Scheduler::schedule(4, None, 0, Call::Logger(logger::Call::log(69, 6000))); + Scheduler::do_schedule(4, None, 0, Call::Logger(logger::Call::log(42, MaximumBlockWeight::get() / 2))); + Scheduler::do_schedule(4, None, 0, Call::Logger(logger::Call::log(69, MaximumBlockWeight::get() / 2))); + // With base weights, 69 and 42 should not fit together, but do because of hard deadlines run_to_block(4); assert_eq!(logger::log(), vec![42u32, 69u32]); }); @@ -485,8 +635,8 @@ mod tests { #[test] fn scheduler_respects_priority_ordering() { new_test_ext().execute_with(|| { - Scheduler::schedule(4, None, 1, Call::Logger(logger::Call::log(42, 6000))); - Scheduler::schedule(4, None, 0, Call::Logger(logger::Call::log(69, 6000))); + Scheduler::do_schedule(4, None, 1, Call::Logger(logger::Call::log(42, MaximumBlockWeight::get() / 2))); + Scheduler::do_schedule(4, None, 0, Call::Logger(logger::Call::log(69, MaximumBlockWeight::get() / 2))); run_to_block(4); assert_eq!(logger::log(), vec![69u32, 42u32]); }); @@ -495,31 +645,76 @@ mod tests { #[test] fn scheduler_respects_priority_ordering_with_soft_deadlines() { new_test_ext().execute_with(|| { - Scheduler::schedule(4, None, 255, Call::Logger(logger::Call::log(42, 5000))); - Scheduler::schedule(4, None, 127, Call::Logger(logger::Call::log(69, 5000))); - Scheduler::schedule(4, None, 126, Call::Logger(logger::Call::log(2600, 6000))); + Scheduler::do_schedule(4, None, 255, Call::Logger(logger::Call::log(42, MaximumBlockWeight::get() / 3))); + Scheduler::do_schedule(4, None, 127, Call::Logger(logger::Call::log(69, MaximumBlockWeight::get() / 2))); + Scheduler::do_schedule(4, None, 126, Call::Logger(logger::Call::log(2600, MaximumBlockWeight::get() / 2))); + + // 2600 does not fit with 69 or 42, but has higher priority, so will go through run_to_block(4); assert_eq!(logger::log(), vec![2600u32]); + // 69 and 42 fit together run_to_block(5); assert_eq!(logger::log(), vec![2600u32, 69u32, 42u32]); }); } #[test] - fn initialize_weight_is_correct() { + fn on_initialize_weight_is_correct() { new_test_ext().execute_with(|| { - Scheduler::schedule(1, None, 255, Call::Logger(logger::Call::log(3, 1000))); - Scheduler::schedule(1, None, 128, Call::Logger(logger::Call::log(42, 5000))); - Scheduler::schedule(1, None, 127, Call::Logger(logger::Call::log(69, 5000))); - Scheduler::schedule(1, None, 126, Call::Logger(logger::Call::log(2600, 6000))); - let weight = Scheduler::on_initialize(1); - assert_eq!(weight, 6000); - let weight = Scheduler::on_initialize(2); - assert_eq!(weight, 10000); - let weight = Scheduler::on_initialize(3); - assert_eq!(weight, 1000); - let weight = Scheduler::on_initialize(4); - assert_eq!(weight, 0); + let base_weight: Weight = ::DbWeight::get().reads_writes(1, 2) + 10_000_000; + let base_multiplier = 25_000_000; + let named_multiplier = ::DbWeight::get().writes(1); + let periodic_multiplier = ::DbWeight::get().reads_writes(1, 1); + + // Named + assert_ok!(Scheduler::do_schedule_named(1u32.encode(), 1, None, 255, Call::Logger(logger::Call::log(3, MaximumBlockWeight::get() / 3)))); + // Anon Periodic + Scheduler::do_schedule(1, Some((1000, 3)), 128, Call::Logger(logger::Call::log(42, MaximumBlockWeight::get() / 3))); + // Anon + Scheduler::do_schedule(1, None, 127, Call::Logger(logger::Call::log(69, MaximumBlockWeight::get() / 2))); + // Named Periodic + assert_ok!(Scheduler::do_schedule_named(2u32.encode(), 1, Some((1000, 3)), 126, Call::Logger(logger::Call::log(2600, MaximumBlockWeight::get() / 2)))); + + // Will include the named periodic only + let actual_weight = Scheduler::on_initialize(1); + let call_weight = MaximumBlockWeight::get() / 2; + assert_eq!(actual_weight, call_weight + base_weight + base_multiplier + named_multiplier + periodic_multiplier); + assert_eq!(logger::log(), vec![2600u32]); + + // Will include anon and anon periodic + let actual_weight = Scheduler::on_initialize(2); + let call_weight = MaximumBlockWeight::get() / 2 + MaximumBlockWeight::get() / 3; + assert_eq!(actual_weight, call_weight + base_weight + base_multiplier * 2 + periodic_multiplier); + assert_eq!(logger::log(), vec![2600u32, 69u32, 42u32]); + + // Will include named only + let actual_weight = Scheduler::on_initialize(3); + let call_weight = MaximumBlockWeight::get() / 3; + assert_eq!(actual_weight, call_weight + base_weight + base_multiplier + named_multiplier); + assert_eq!(logger::log(), vec![2600u32, 69u32, 42u32, 3u32]); + + // Will contain none + let actual_weight = Scheduler::on_initialize(4); + assert_eq!(actual_weight, 0); + }); + } + + #[test] + fn root_calls_works() { + new_test_ext().execute_with(|| { + let call = Box::new(Call::Logger(logger::Call::log(69, 1000))); + let call2 = Box::new(Call::Logger(logger::Call::log(42, 1000))); + assert_ok!(Scheduler::schedule_named(Origin::ROOT, 1u32.encode(), 4, None, 127, call)); + assert_ok!(Scheduler::schedule(Origin::ROOT, 4, None, 127, call2)); + run_to_block(3); + // Scheduled calls are in the agenda. + assert_eq!(Agenda::::get(4).len(), 2); + assert!(logger::log().is_empty()); + assert_ok!(Scheduler::cancel_named(Origin::ROOT, 1u32.encode())); + assert_ok!(Scheduler::cancel(Origin::ROOT, 4, 1)); + // Scheduled calls are made NONE, so should not effect state + run_to_block(100); + assert!(logger::log().is_empty()); }); } } diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index eaf94f44f8..0230937fc4 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -1241,7 +1241,7 @@ pub mod schedule { /// /// - `id`: The identity of the task. This must be unique and will return an error if not. fn schedule_named( - id: impl Encode, + id: Vec, when: BlockNumber, maybe_periodic: Option>, priority: Priority, @@ -1255,7 +1255,7 @@ pub mod schedule { /// /// NOTE: This guaranteed to work only *before* the point that it is due to be executed. /// If it ends up being delayed beyond the point of execution, then it cannot be cancelled. - fn cancel_named(id: impl Encode) -> Result<(), ()>; + fn cancel_named(id: Vec) -> Result<(), ()>; } }