diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 272174a9965a9..ae5c354e6151e 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -186,10 +186,12 @@ decl_error! { pub enum Error for Module { /// Failed to schedule a call FailedToSchedule, - /// Failed to cancel a scheduled call - FailedToCancel, + /// Cannot find the scheduled call. + NotFound, /// Given target block number is in the past. TargetBlockNumberInPast, + /// Reschedule failed because it does not change scheduled time. + RescheduleNoChange, } } @@ -467,13 +469,7 @@ impl Module { )); } - fn do_schedule( - when: DispatchTime, - maybe_periodic: Option>, - priority: schedule::Priority, - origin: T::PalletsOrigin, - call: ::Call - ) -> Result, DispatchError> { + fn resolve_time(when: DispatchTime) -> Result { let now = frame_system::Module::::block_number(); let when = match when { @@ -485,6 +481,18 @@ impl Module { return Err(Error::::TargetBlockNumberInPast.into()) } + Ok(when) + } + + fn do_schedule( + when: DispatchTime, + maybe_periodic: Option>, + priority: schedule::Priority, + origin: T::PalletsOrigin, + call: ::Call + ) -> Result, DispatchError> { + let when = Self::resolve_time(when)?; + // sanitize maybe_periodic let maybe_periodic = maybe_periodic .filter(|p| p.1 > 1 && !p.0.is_zero()) @@ -531,10 +539,34 @@ impl Module { Self::deposit_event(RawEvent::Canceled(when, index)); Ok(()) } else { - Err(Error::::FailedToCancel)? + Err(Error::::NotFound)? } } + fn do_reschedule( + (when, index): TaskAddress, + new_time: DispatchTime, + ) -> Result, DispatchError> { + let new_time = Self::resolve_time(new_time)?; + + if new_time == when { + return Err(Error::::RescheduleNoChange.into()); + } + + Agenda::::try_mutate(when, |agenda| -> DispatchResult { + let task = agenda.get_mut(index as usize).ok_or(Error::::NotFound)?; + let task = task.take().ok_or(Error::::NotFound)?; + Agenda::::append(new_time, Some(task)); + Ok(()) + })?; + + let new_index = Agenda::::decode_len(new_time).unwrap_or(1) as u32 - 1; + Self::deposit_event(RawEvent::Canceled(when, index)); + Self::deposit_event(RawEvent::Scheduled(new_time, new_index)); + + Ok((new_time, new_index)) + } + fn do_schedule_named( id: Vec, when: DispatchTime, @@ -548,16 +580,7 @@ impl Module { return Err(Error::::FailedToSchedule)? } - let now = frame_system::Module::::block_number(); - - let when = match when { - DispatchTime::At(x) => x, - DispatchTime::After(x) => now.saturating_add(x) - }; - - if when <= now { - return Err(Error::::TargetBlockNumberInPast.into()) - } + let when = Self::resolve_time(when)?; // sanitize maybe_periodic let maybe_periodic = maybe_periodic @@ -601,10 +624,41 @@ impl Module { Self::deposit_event(RawEvent::Canceled(when, index)); Ok(()) } else { - Err(Error::::FailedToCancel)? + Err(Error::::NotFound)? } }) } + + fn do_reschedule_named( + id: Vec, + new_time: DispatchTime, + ) -> Result, DispatchError> { + let new_time = Self::resolve_time(new_time)?; + + Lookup::::try_mutate_exists(id, |lookup| -> Result, DispatchError> { + let (when, index) = lookup.ok_or(Error::::NotFound)?; + + if new_time == when { + return Err(Error::::RescheduleNoChange.into()); + } + + Agenda::::try_mutate(when, |agenda| -> DispatchResult { + let task = agenda.get_mut(index as usize).ok_or(Error::::NotFound)?; + let task = task.take().ok_or(Error::::NotFound)?; + Agenda::::append(new_time, Some(task)); + + Ok(()) + })?; + + let new_index = Agenda::::decode_len(new_time).unwrap_or(1) as u32 - 1; + Self::deposit_event(RawEvent::Canceled(when, index)); + Self::deposit_event(RawEvent::Scheduled(new_time, new_index)); + + *lookup = Some((new_time, new_index)); + + Ok((new_time, new_index)) + }) + } } impl schedule::Anon::Call, T::PalletsOrigin> for Module { @@ -623,6 +677,17 @@ impl schedule::Anon::Call, T::PalletsOrig fn cancel((when, index): Self::Address) -> Result<(), ()> { Self::do_cancel(None, (when, index)).map_err(|_| ()) } + + fn reschedule( + address: Self::Address, + when: DispatchTime, + ) -> Result { + Self::do_reschedule(address, when) + } + + fn next_dispatch_time((when, index): Self::Address) -> Result { + Agenda::::get(when).get(index as usize).ok_or(()).map(|_| when) + } } impl schedule::Named::Call, T::PalletsOrigin> for Module { @@ -642,6 +707,17 @@ impl schedule::Named::Call, T::PalletsOri fn cancel_named(id: Vec) -> Result<(), ()> { Self::do_cancel_named(None, id).map_err(|_| ()) } + + fn reschedule_named( + id: Vec, + when: DispatchTime, + ) -> Result { + Self::do_reschedule_named(id, when) + } + + fn next_dispatch_time(id: Vec) -> Result { + Lookup::::get(id).and_then(|(when, index)| Agenda::::get(when).get(index as usize).map(|_| when)).ok_or(()) + } } #[cfg(test)] @@ -868,6 +944,95 @@ mod tests { }); } + #[test] + fn reschedule_works() { + new_test_ext().execute_with(|| { + let call = Call::Logger(logger::Call::log(42, 1000)); + assert!(!::BaseCallFilter::filter(&call)); + assert_eq!(Scheduler::do_schedule(DispatchTime::At(4), None, 127, root(), call).unwrap(), (4, 0)); + + run_to_block(3); + assert!(logger::log().is_empty()); + + assert_eq!(Scheduler::do_reschedule((4, 0), DispatchTime::At(6)).unwrap(), (6, 0)); + + assert_noop!(Scheduler::do_reschedule((6, 0), DispatchTime::At(6)), Error::::RescheduleNoChange); + + run_to_block(4); + assert!(logger::log().is_empty()); + + run_to_block(6); + assert_eq!(logger::log(), vec![(root(), 42u32)]); + + run_to_block(100); + assert_eq!(logger::log(), vec![(root(), 42u32)]); + }); + } + + #[test] + fn reschedule_named_works() { + new_test_ext().execute_with(|| { + let call = Call::Logger(logger::Call::log(42, 1000)); + assert!(!::BaseCallFilter::filter(&call)); + assert_eq!(Scheduler::do_schedule_named( + 1u32.encode(), DispatchTime::At(4), None, 127, root(), call + ).unwrap(), (4, 0)); + + run_to_block(3); + assert!(logger::log().is_empty()); + + assert_eq!(Scheduler::do_reschedule_named(1u32.encode(), DispatchTime::At(6)).unwrap(), (6, 0)); + + assert_noop!(Scheduler::do_reschedule_named(1u32.encode(), DispatchTime::At(6)), Error::::RescheduleNoChange); + + run_to_block(4); + assert!(logger::log().is_empty()); + + run_to_block(6); + assert_eq!(logger::log(), vec![(root(), 42u32)]); + + run_to_block(100); + assert_eq!(logger::log(), vec![(root(), 42u32)]); + }); + } + + #[test] + fn reschedule_named_perodic_works() { + new_test_ext().execute_with(|| { + let call = Call::Logger(logger::Call::log(42, 1000)); + assert!(!::BaseCallFilter::filter(&call)); + assert_eq!(Scheduler::do_schedule_named( + 1u32.encode(), DispatchTime::At(4), Some((3, 3)), 127, root(), call + ).unwrap(), (4, 0)); + + run_to_block(3); + assert!(logger::log().is_empty()); + + assert_eq!(Scheduler::do_reschedule_named(1u32.encode(), DispatchTime::At(5)).unwrap(), (5, 0)); + assert_eq!(Scheduler::do_reschedule_named(1u32.encode(), DispatchTime::At(6)).unwrap(), (6, 0)); + + run_to_block(5); + assert!(logger::log().is_empty()); + + run_to_block(6); + assert_eq!(logger::log(), vec![(root(), 42u32)]); + + assert_eq!(Scheduler::do_reschedule_named(1u32.encode(), DispatchTime::At(10)).unwrap(), (10, 0)); + + run_to_block(9); + assert_eq!(logger::log(), vec![(root(), 42u32)]); + + run_to_block(10); + assert_eq!(logger::log(), vec![(root(), 42u32), (root(), 42u32)]); + + run_to_block(13); + assert_eq!(logger::log(), vec![(root(), 42u32), (root(), 42u32), (root(), 42u32)]); + + run_to_block(100); + assert_eq!(logger::log(), vec![(root(), 42u32), (root(), 42u32), (root(), 42u32)]); + }); + } + #[test] fn cancel_named_scheduling_works_with_normal_cancel() { new_test_ext().execute_with(|| { diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 6a9f834969018..1e1a1293790d2 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -1529,11 +1529,9 @@ pub mod schedule { /// An address which can be used for removing a scheduled task. type Address: Codec + Clone + Eq + EncodeLike + Debug; - /// Schedule a one-off dispatch to happen at the beginning of some block in the future. + /// Schedule a dispatch to happen at the beginning of some block in the future. /// /// This is not named. - /// - /// Infallible. fn schedule( when: DispatchTime, maybe_periodic: Option>, @@ -1553,6 +1551,22 @@ pub mod schedule { /// NOTE2: This will not work to cancel periodic tasks after their initial execution. For /// that, you must name the task explicitly using the `Named` trait. fn cancel(address: Self::Address) -> Result<(), ()>; + + /// Reschedule a task. For one-off tasks, this dispatch is guaranteed to succeed + /// only if it is executed *before* the currently scheduled block. For periodic tasks, + /// this dispatch is guaranteed to succeed only before the *initial* execution; for + /// others, use `reschedule_named`. + /// + /// Will return an error if the `address` is invalid. + fn reschedule( + address: Self::Address, + when: DispatchTime, + ) -> Result; + + /// Return the next dispatch time for a given task. + /// + /// Will return an error if the `address` is invalid. + fn next_dispatch_time(address: Self::Address) -> Result; } /// A type that can be used as a scheduler. @@ -1560,7 +1574,7 @@ pub mod schedule { /// An address which can be used for removing a scheduled task. type Address: Codec + Clone + Eq + EncodeLike + sp_std::fmt::Debug; - /// Schedule a one-off dispatch to happen at the beginning of some block in the future. + /// Schedule a dispatch to happen at the beginning of some block in the future. /// /// - `id`: The identity of the task. This must be unique and will return an error if not. fn schedule_named( @@ -1580,6 +1594,18 @@ 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: Vec) -> Result<(), ()>; + + /// Reschedule a task. For one-off tasks, this dispatch is guaranteed to succeed + /// only if it is executed *before* the currently scheduled block. + fn reschedule_named( + id: Vec, + when: DispatchTime, + ) -> Result; + + /// Return the next dispatch time for a given task. + /// + /// Will return an error if the `id` is invalid. + fn next_dispatch_time(id: Vec) -> Result; } }