allow to specify schedule time as a relative value (#6578)

* allow to specify schedule time as a relative value

* bump version

* line width

* fix benchmarks

* rename and updates
This commit is contained in:
Xiliang Chen
2020-07-15 17:19:21 +12:00
committed by GitHub
parent c802d425d9
commit 8f4329823a
6 changed files with 169 additions and 39 deletions
+2 -2
View File
@@ -97,8 +97,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// and set impl_version to 0. If only runtime // and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as // implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version. // is and increment impl_version.
spec_version: 255, spec_version: 256,
impl_version: 1, impl_version: 0,
apis: RUNTIME_API_VERSIONS, apis: RUNTIME_API_VERSIONS,
transaction_version: 1, transaction_version: 1,
}; };
@@ -22,7 +22,7 @@ use super::*;
use frame_benchmarking::{benchmarks, account}; use frame_benchmarking::{benchmarks, account};
use frame_support::{ use frame_support::{
IterableStorageMap, IterableStorageMap,
traits::{Currency, Get, EnsureOrigin, OnInitialize, UnfilteredDispatchable}, traits::{Currency, Get, EnsureOrigin, OnInitialize, UnfilteredDispatchable, schedule::DispatchTime},
}; };
use frame_system::{RawOrigin, Module as System, self, EventRecord}; use frame_system::{RawOrigin, Module as System, self, EventRecord};
use sp_runtime::traits::{Bounded, One}; use sp_runtime::traits::{Bounded, One};
@@ -76,7 +76,7 @@ fn add_referendum<T: Trait>(n: u32) -> Result<ReferendumIndex, &'static str> {
let referendum_index: ReferendumIndex = ReferendumCount::get() - 1; let referendum_index: ReferendumIndex = ReferendumCount::get() - 1;
T::Scheduler::schedule_named( T::Scheduler::schedule_named(
(DEMOCRACY_ID, referendum_index).encode(), (DEMOCRACY_ID, referendum_index).encode(),
1.into(), DispatchTime::At(1.into()),
None, None,
63, 63,
system::RawOrigin::Root.into(), system::RawOrigin::Root.into(),
+2 -2
View File
@@ -163,7 +163,7 @@ use frame_support::{
weights::{Weight, DispatchClass}, weights::{Weight, DispatchClass},
traits::{ traits::{
Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get, Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get,
OnUnbalanced, BalanceStatus, schedule::Named as ScheduleNamed, EnsureOrigin OnUnbalanced, BalanceStatus, schedule::{Named as ScheduleNamed, DispatchTime}, EnsureOrigin
}, },
dispatch::DispatchResultWithPostInfo, dispatch::DispatchResultWithPostInfo,
}; };
@@ -1688,7 +1688,7 @@ impl<T: Trait> Module<T> {
if T::Scheduler::schedule_named( if T::Scheduler::schedule_named(
(DEMOCRACY_ID, index).encode(), (DEMOCRACY_ID, index).encode(),
when, DispatchTime::At(when),
None, None,
63, 63,
system::RawOrigin::Root.into(), system::RawOrigin::Root.into(),
@@ -39,7 +39,7 @@ fn fill_schedule<T: Trait> (when: T::BlockNumber, n: u32) -> Result<(), &'static
// Named schedule is strictly heavier than anonymous // Named schedule is strictly heavier than anonymous
Scheduler::<T>::do_schedule_named( Scheduler::<T>::do_schedule_named(
i.encode(), i.encode(),
when, DispatchTime::At(when),
// Add periodicity // Add periodicity
Some((T::BlockNumber::one(), 100)), Some((T::BlockNumber::one(), 100)),
// HARD_DEADLINE priority means it gets executed no matter what // HARD_DEADLINE priority means it gets executed no matter what
+151 -30
View File
@@ -55,11 +55,11 @@ mod benchmarking;
use sp_std::{prelude::*, marker::PhantomData, borrow::Borrow}; use sp_std::{prelude::*, marker::PhantomData, borrow::Borrow};
use codec::{Encode, Decode, Codec}; use codec::{Encode, Decode, Codec};
use sp_runtime::{RuntimeDebug, traits::{Zero, One, BadOrigin}}; use sp_runtime::{RuntimeDebug, traits::{Zero, One, BadOrigin, Saturating}};
use frame_support::{ use frame_support::{
decl_module, decl_storage, decl_event, decl_error, IterableStorageMap, decl_module, decl_storage, decl_event, decl_error, IterableStorageMap,
dispatch::{Dispatchable, DispatchError, DispatchResult, Parameter}, dispatch::{Dispatchable, DispatchError, DispatchResult, Parameter},
traits::{Get, schedule, OriginTrait, EnsureOrigin, IsType}, traits::{Get, schedule::{self, DispatchTime}, OriginTrait, EnsureOrigin, IsType},
weights::{GetDispatchInfo, Weight}, weights::{GetDispatchInfo, Weight},
}; };
use frame_system::{self as system}; use frame_system::{self as system};
@@ -219,7 +219,7 @@ decl_module! {
) { ) {
T::ScheduleOrigin::ensure_origin(origin.clone())?; T::ScheduleOrigin::ensure_origin(origin.clone())?;
let origin = <T as Trait>::Origin::from(origin); let origin = <T as Trait>::Origin::from(origin);
Self::do_schedule(when, maybe_periodic, priority, origin.caller().clone(), *call)?; Self::do_schedule(DispatchTime::At(when), maybe_periodic, priority, origin.caller().clone(), *call)?;
} }
/// Cancel an anonymously scheduled task. /// Cancel an anonymously scheduled task.
@@ -259,7 +259,9 @@ decl_module! {
) { ) {
T::ScheduleOrigin::ensure_origin(origin.clone())?; T::ScheduleOrigin::ensure_origin(origin.clone())?;
let origin = <T as Trait>::Origin::from(origin); let origin = <T as Trait>::Origin::from(origin);
Self::do_schedule_named(id, when, maybe_periodic, priority, origin.caller().clone(), *call)?; Self::do_schedule_named(
id, DispatchTime::At(when), maybe_periodic, priority, origin.caller().clone(), *call
)?;
} }
/// Cancel a named scheduled task. /// Cancel a named scheduled task.
@@ -279,6 +281,45 @@ decl_module! {
Self::do_cancel_named(Some(origin.caller().clone()), id)?; Self::do_cancel_named(Some(origin.caller().clone()), id)?;
} }
/// Anonymously schedule a task after a delay.
///
/// # <weight>
/// Same as [`schedule`].
/// # </weight>
#[weight = 25_000_000 + T::DbWeight::get().reads_writes(1, 1)]
fn schedule_after(origin,
after: T::BlockNumber,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority,
call: Box<<T as Trait>::Call>,
) {
T::ScheduleOrigin::ensure_origin(origin.clone())?;
let origin = <T as Trait>::Origin::from(origin);
Self::do_schedule(
DispatchTime::After(after), maybe_periodic, priority, origin.caller().clone(), *call
)?;
}
/// Schedule a named task after a delay.
///
/// # <weight>
/// Same as [`schedule_named`].
/// # </weight>
#[weight = 35_000_000 + T::DbWeight::get().reads_writes(2, 2)]
fn schedule_named_after(origin,
id: Vec<u8>,
after: T::BlockNumber,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority,
call: Box<<T as Trait>::Call>,
) {
T::ScheduleOrigin::ensure_origin(origin.clone())?;
let origin = <T as Trait>::Origin::from(origin);
Self::do_schedule_named(
id, DispatchTime::After(after), maybe_periodic, priority, origin.caller().clone(), *call
)?;
}
/// Execute the scheduled calls /// Execute the scheduled calls
/// ///
/// # <weight> /// # <weight>
@@ -395,13 +436,20 @@ impl<T: Trait> Module<T> {
} }
fn do_schedule( fn do_schedule(
when: T::BlockNumber, when: DispatchTime<T::BlockNumber>,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>, maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority, priority: schedule::Priority,
origin: T::PalletsOrigin, origin: T::PalletsOrigin,
call: <T as Trait>::Call call: <T as Trait>::Call
) -> Result<TaskAddress<T::BlockNumber>, DispatchError> { ) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
if when <= frame_system::Module::<T>::block_number() { let now = frame_system::Module::<T>::block_number();
let when = match when {
DispatchTime::At(x) => x,
DispatchTime::After(x) => now.saturating_add(x)
};
if when <= now {
return Err(Error::<T>::TargetBlockNumberInPast.into()) return Err(Error::<T>::TargetBlockNumberInPast.into())
} }
@@ -451,7 +499,7 @@ impl<T: Trait> Module<T> {
fn do_schedule_named( fn do_schedule_named(
id: Vec<u8>, id: Vec<u8>,
when: T::BlockNumber, when: DispatchTime<T::BlockNumber>,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>, maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority, priority: schedule::Priority,
origin: T::PalletsOrigin, origin: T::PalletsOrigin,
@@ -462,7 +510,14 @@ impl<T: Trait> Module<T> {
return Err(Error::<T>::FailedToSchedule)? return Err(Error::<T>::FailedToSchedule)?
} }
if when <= frame_system::Module::<T>::block_number() { let now = frame_system::Module::<T>::block_number();
let when = match when {
DispatchTime::At(x) => x,
DispatchTime::After(x) => now.saturating_add(x)
};
if when <= now {
return Err(Error::<T>::TargetBlockNumberInPast.into()) return Err(Error::<T>::TargetBlockNumberInPast.into())
} }
@@ -512,7 +567,7 @@ impl<T: Trait> schedule::Anon<T::BlockNumber, <T as Trait>::Call, T::PalletsOrig
type Address = TaskAddress<T::BlockNumber>; type Address = TaskAddress<T::BlockNumber>;
fn schedule( fn schedule(
when: T::BlockNumber, when: DispatchTime<T::BlockNumber>,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>, maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority, priority: schedule::Priority,
origin: T::PalletsOrigin, origin: T::PalletsOrigin,
@@ -531,7 +586,7 @@ impl<T: Trait> schedule::Named<T::BlockNumber, <T as Trait>::Call, T::PalletsOri
fn schedule_named( fn schedule_named(
id: Vec<u8>, id: Vec<u8>,
when: T::BlockNumber, when: DispatchTime<T::BlockNumber>,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>, maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority, priority: schedule::Priority,
origin: T::PalletsOrigin, origin: T::PalletsOrigin,
@@ -716,7 +771,7 @@ mod tests {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
let call = Call::Logger(logger::Call::log(42, 1000)); let call = Call::Logger(logger::Call::log(42, 1000));
assert!(!<Test as frame_system::Trait>::BaseCallFilter::filter(&call)); assert!(!<Test as frame_system::Trait>::BaseCallFilter::filter(&call));
let _ = Scheduler::do_schedule(4, None, 127, root(), call); let _ = Scheduler::do_schedule(DispatchTime::At(4), None, 127, root(), call);
run_to_block(3); run_to_block(3);
assert!(logger::log().is_empty()); assert!(logger::log().is_empty());
run_to_block(4); run_to_block(4);
@@ -726,12 +781,28 @@ mod tests {
}); });
} }
#[test]
fn schedule_after_works() {
new_test_ext().execute_with(|| {
run_to_block(2);
let call = Call::Logger(logger::Call::log(42, 1000));
assert!(!<Test as frame_system::Trait>::BaseCallFilter::filter(&call));
let _ = Scheduler::do_schedule(DispatchTime::After(3), None, 127, root(), call);
run_to_block(4);
assert!(logger::log().is_empty());
run_to_block(5);
assert_eq!(logger::log(), vec![(root(), 42u32)]);
run_to_block(100);
assert_eq!(logger::log(), vec![(root(), 42u32)]);
});
}
#[test] #[test]
fn periodic_scheduling_works() { fn periodic_scheduling_works() {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
// at #4, every 3 blocks, 3 times. // at #4, every 3 blocks, 3 times.
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, Some((3, 3)), 127, root(), Call::Logger(logger::Call::log(42, 1000)) DispatchTime::At(4), Some((3, 3)), 127, root(), Call::Logger(logger::Call::log(42, 1000))
); );
run_to_block(3); run_to_block(3);
assert!(logger::log().is_empty()); assert!(logger::log().is_empty());
@@ -755,10 +826,10 @@ mod tests {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
// at #4. // at #4.
Scheduler::do_schedule_named( Scheduler::do_schedule_named(
1u32.encode(), 4, None, 127, root(), Call::Logger(logger::Call::log(69, 1000)) 1u32.encode(), DispatchTime::At(4), None, 127, root(), Call::Logger(logger::Call::log(69, 1000))
).unwrap(); ).unwrap();
let i = Scheduler::do_schedule( let i = Scheduler::do_schedule(
4, None, 127, root(), Call::Logger(logger::Call::log(42, 1000)) DispatchTime::At(4), None, 127, root(), Call::Logger(logger::Call::log(42, 1000))
).unwrap(); ).unwrap();
run_to_block(3); run_to_block(3);
assert!(logger::log().is_empty()); assert!(logger::log().is_empty());
@@ -774,15 +845,25 @@ mod tests {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
// at #4, every 3 blocks, 3 times. // at #4, every 3 blocks, 3 times.
Scheduler::do_schedule_named( Scheduler::do_schedule_named(
1u32.encode(), 4, Some((3, 3)), 127, root(), Call::Logger(logger::Call::log(42, 1000)) 1u32.encode(),
DispatchTime::At(4),
Some((3, 3)),
127,
root(),
Call::Logger(logger::Call::log(42, 1000))
).unwrap(); ).unwrap();
// same id results in error. // same id results in error.
assert!(Scheduler::do_schedule_named( assert!(Scheduler::do_schedule_named(
1u32.encode(), 4, None, 127, root(), Call::Logger(logger::Call::log(69, 1000)) 1u32.encode(),
DispatchTime::At(4),
None,
127,
root(),
Call::Logger(logger::Call::log(69, 1000))
).is_err()); ).is_err());
// different id is ok. // different id is ok.
Scheduler::do_schedule_named( Scheduler::do_schedule_named(
2u32.encode(), 8, None, 127, root(), Call::Logger(logger::Call::log(69, 1000)) 2u32.encode(), DispatchTime::At(8), None, 127, root(), Call::Logger(logger::Call::log(69, 1000))
).unwrap(); ).unwrap();
run_to_block(3); run_to_block(3);
assert!(logger::log().is_empty()); assert!(logger::log().is_empty());
@@ -799,10 +880,17 @@ mod tests {
fn scheduler_respects_weight_limits() { fn scheduler_respects_weight_limits() {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 127, root(), Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
127,
root(),
Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 2))
); );
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 127, root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
127,
root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2))
); );
// 69 and 42 do not fit together // 69 and 42 do not fit together
run_to_block(4); run_to_block(4);
@@ -816,10 +904,18 @@ mod tests {
fn scheduler_respects_hard_deadlines_more() { fn scheduler_respects_hard_deadlines_more() {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 0, root(), Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
0,
root(),
Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 2))
); );
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 0, root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
0,
root(),
Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2))
); );
// With base weights, 69 and 42 should not fit together, but do because of hard deadlines // With base weights, 69 and 42 should not fit together, but do because of hard deadlines
run_to_block(4); run_to_block(4);
@@ -831,10 +927,18 @@ mod tests {
fn scheduler_respects_priority_ordering() { fn scheduler_respects_priority_ordering() {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 1, root(), Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
1,
root(),
Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 2))
); );
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 0, root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
0,
root(),
Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2))
); );
run_to_block(4); run_to_block(4);
assert_eq!(logger::log(), vec![(root(), 69u32), (root(), 42u32)]); assert_eq!(logger::log(), vec![(root(), 69u32), (root(), 42u32)]);
@@ -845,13 +949,22 @@ mod tests {
fn scheduler_respects_priority_ordering_with_soft_deadlines() { fn scheduler_respects_priority_ordering_with_soft_deadlines() {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 255, root(), Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 3)) DispatchTime::At(4),
None,
255,
root(), Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 3))
); );
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 127, root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
127,
root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2))
); );
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
4, None, 126, root(), Call::Logger(logger::Call::log(2600, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(4),
None,
126,
root(), Call::Logger(logger::Call::log(2600, MaximumSchedulerWeight::get() / 2))
); );
// 2600 does not fit with 69 or 42, but has higher priority, so will go through // 2600 does not fit with 69 or 42, but has higher priority, so will go through
@@ -874,21 +987,29 @@ mod tests {
// Named // Named
assert_ok!( assert_ok!(
Scheduler::do_schedule_named( Scheduler::do_schedule_named(
1u32.encode(), 1, None, 255, root(), 1u32.encode(), DispatchTime::At(1), None, 255, root(),
Call::Logger(logger::Call::log(3, MaximumSchedulerWeight::get() / 3)) Call::Logger(logger::Call::log(3, MaximumSchedulerWeight::get() / 3))
) )
); );
// Anon Periodic // Anon Periodic
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
1, Some((1000, 3)), 128, root(), Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 3)) DispatchTime::At(1),
Some((1000, 3)),
128,
root(),
Call::Logger(logger::Call::log(42, MaximumSchedulerWeight::get() / 3))
); );
// Anon // Anon
let _ = Scheduler::do_schedule( let _ = Scheduler::do_schedule(
1, None, 127, root(), Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2)) DispatchTime::At(1),
None,
127,
root(),
Call::Logger(logger::Call::log(69, MaximumSchedulerWeight::get() / 2))
); );
// Named Periodic // Named Periodic
assert_ok!(Scheduler::do_schedule_named( assert_ok!(Scheduler::do_schedule_named(
2u32.encode(), 1, Some((1000, 3)), 126, root(), 2u32.encode(), DispatchTime::At(1), Some((1000, 3)), 126, root(),
Call::Logger(logger::Call::log(2600, MaximumSchedulerWeight::get() / 2))) Call::Logger(logger::Call::log(2600, MaximumSchedulerWeight::get() / 2)))
); );
+11 -2
View File
@@ -1490,6 +1490,15 @@ pub mod schedule {
/// higher priority. /// higher priority.
pub type Priority = u8; pub type Priority = u8;
/// The dispatch time of a scheduled task.
#[derive(Encode, Decode, Copy, Clone, PartialEq, Eq, RuntimeDebug)]
pub enum DispatchTime<BlockNumber> {
/// At specified block.
At(BlockNumber),
/// After specified number of blocks.
After(BlockNumber),
}
/// The highest priority. We invert the value so that normal sorting will place the highest /// The highest priority. We invert the value so that normal sorting will place the highest
/// priority at the beginning of the list. /// priority at the beginning of the list.
pub const HIGHEST_PRIORITY: Priority = 0; pub const HIGHEST_PRIORITY: Priority = 0;
@@ -1510,7 +1519,7 @@ pub mod schedule {
/// ///
/// Infallible. /// Infallible.
fn schedule( fn schedule(
when: BlockNumber, when: DispatchTime<BlockNumber>,
maybe_periodic: Option<Period<BlockNumber>>, maybe_periodic: Option<Period<BlockNumber>>,
priority: Priority, priority: Priority,
origin: Origin, origin: Origin,
@@ -1540,7 +1549,7 @@ pub mod schedule {
/// - `id`: The identity of the task. This must be unique and will return an error if not. /// - `id`: The identity of the task. This must be unique and will return an error if not.
fn schedule_named( fn schedule_named(
id: Vec<u8>, id: Vec<u8>,
when: BlockNumber, when: DispatchTime<BlockNumber>,
maybe_periodic: Option<Period<BlockNumber>>, maybe_periodic: Option<Period<BlockNumber>>,
priority: Priority, priority: Priority,
origin: Origin, origin: Origin,