Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

reschedule #6860

Merged
9 commits merged into from
Oct 7, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 134 additions & 21 deletions frame/scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ decl_error! {
pub enum Error for Module<T: Trait> {
/// Failed to schedule a call
FailedToSchedule,
/// Failed to cancel a scheduled call
FailedToCancel,
/// Cannot found the scheduled call.
xlc marked this conversation as resolved.
Show resolved Hide resolved
NotFound,
/// Given target block number is in the past.
TargetBlockNumberInPast,
}
Expand Down Expand Up @@ -438,13 +438,7 @@ impl<T: Trait> Module<T> {
}
}

fn do_schedule(
when: DispatchTime<T::BlockNumber>,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority,
origin: T::PalletsOrigin,
call: <T as Trait>::Call
) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
fn resolve_time(when: DispatchTime<T::BlockNumber>) -> Result<T::BlockNumber, DispatchError> {
let now = frame_system::Module::<T>::block_number();

let when = match when {
Expand All @@ -456,6 +450,18 @@ impl<T: Trait> Module<T> {
return Err(Error::<T>::TargetBlockNumberInPast.into())
}

Ok(when)
}

fn do_schedule(
when: DispatchTime<T::BlockNumber>,
maybe_periodic: Option<schedule::Period<T::BlockNumber>>,
priority: schedule::Priority,
origin: T::PalletsOrigin,
call: <T as Trait>::Call
) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
let when = Self::resolve_time(when)?;

// sanitize maybe_periodic
let maybe_periodic = maybe_periodic
.filter(|p| p.1 > 1 && !p.0.is_zero())
Expand Down Expand Up @@ -496,10 +502,30 @@ impl<T: Trait> Module<T> {
Self::deposit_event(RawEvent::Canceled(when, index));
Ok(())
} else {
Err(Error::<T>::FailedToCancel)?
Err(Error::<T>::NotFound)?
}
}

fn do_reschedule(
(when, index): TaskAddress<T::BlockNumber>,
new_time: DispatchTime<T::BlockNumber>,
) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
let new_time = Self::resolve_time(new_time)?;

Agenda::<T>::try_mutate(when, |agenda| -> DispatchResult {
let task = agenda.get_mut(index as usize).ok_or(Error::<T>::NotFound)?;
let task = task.take().ok_or(Error::<T>::NotFound)?;
Agenda::<T>::append(new_time, Some(task));
Ok(())
})?;
gui1117 marked this conversation as resolved.
Show resolved Hide resolved

let new_index = Agenda::<T>::decode_len(new_time).unwrap_or(1) as u32 - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its more efficient to get the length from the try_mutate above right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agenda::<T>::append was used which doesn't decode the whole thing, it does have the length info but not exposed

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<u8>,
when: DispatchTime<T::BlockNumber>,
Expand All @@ -513,16 +539,7 @@ impl<T: Trait> Module<T> {
return Err(Error::<T>::FailedToSchedule)?
}

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())
}
let when = Self::resolve_time(when)?;

// sanitize maybe_periodic
let maybe_periodic = maybe_periodic
Expand Down Expand Up @@ -560,10 +577,36 @@ impl<T: Trait> Module<T> {
Self::deposit_event(RawEvent::Canceled(when, index));
Ok(())
} else {
Err(Error::<T>::FailedToCancel)?
Err(Error::<T>::NotFound)?
}
})
}

fn do_reschedule_named(
id: Vec<u8>,
new_time: DispatchTime<T::BlockNumber>,
) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
let new_time = Self::resolve_time(new_time)?;

Lookup::<T>::try_mutate_exists(id, |lookup| -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
let (when, index) = lookup.ok_or(Error::<T>::NotFound)?;
Agenda::<T>::try_mutate(when, |agenda| -> DispatchResult {
let task = agenda.get_mut(index as usize).ok_or(Error::<T>::NotFound)?;
let task = task.take().ok_or(Error::<T>::NotFound)?;
Agenda::<T>::append(new_time, Some(task));

Ok(())
})?;

let new_index = Agenda::<T>::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<T: Trait> schedule::Anon<T::BlockNumber, <T as Trait>::Call, T::PalletsOrigin> for Module<T> {
Expand All @@ -582,6 +625,17 @@ impl<T: Trait> schedule::Anon<T::BlockNumber, <T as Trait>::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<T::BlockNumber>,
) -> Result<Self::Address, DispatchError> {
Self::do_reschedule(address, when)
}

fn next_dispatch_time((when, index): Self::Address) -> Result<T::BlockNumber, ()> {
Agenda::<T>::get(when).get(index as usize).ok_or(()).map(|_| when)
}
}

impl<T: Trait> schedule::Named<T::BlockNumber, <T as Trait>::Call, T::PalletsOrigin> for Module<T> {
Expand All @@ -601,6 +655,17 @@ impl<T: Trait> schedule::Named<T::BlockNumber, <T as Trait>::Call, T::PalletsOri
fn cancel_named(id: Vec<u8>) -> Result<(), ()> {
Self::do_cancel_named(None, id).map_err(|_| ())
}

fn reschedule_named(
id: Vec<u8>,
when: DispatchTime<T::BlockNumber>,
) -> Result<Self::Address, DispatchError> {
Self::do_reschedule_named(id, when)
}

fn next_dispatch_time(id: Vec<u8>) -> Result<T::BlockNumber, ()> {
Lookup::<T>::get(id).and_then(|(when, index)| Agenda::<T>::get(when).get(index as usize).map(|_| when)).ok_or(())
}
}

#[cfg(test)]
Expand Down Expand Up @@ -824,6 +889,54 @@ mod tests {
});
}

#[test]
fn reschedule_works() {
new_test_ext().execute_with(|| {
let call = Call::Logger(logger::Call::log(42, 1000));
assert!(!<Test as frame_system::Trait>::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));

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() {
xlc marked this conversation as resolved.
Show resolved Hide resolved
new_test_ext().execute_with(|| {
let call = Call::Logger(logger::Call::log(42, 1000));
assert!(!<Test as frame_system::Trait>::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));

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 cancel_named_scheduling_works_with_normal_cancel() {
new_test_ext().execute_with(|| {
Expand Down
41 changes: 37 additions & 4 deletions frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1521,11 +1521,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<BlockNumber>,
maybe_periodic: Option<Period<BlockNumber>>,
Expand All @@ -1545,14 +1543,33 @@ 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 to a different time.
///
/// Will return an error if the `address` is invalid.
///
/// 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.
///
/// 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.
xlc marked this conversation as resolved.
Show resolved Hide resolved
fn reschedule(
address: Self::Address,
when: DispatchTime<BlockNumber>,
) -> Result<Self::Address, DispatchError>;

/// 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<BlockNumber, ()>;
}

/// A type that can be used as a scheduler.
pub trait Named<BlockNumber, Call, Origin> {
/// 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(
Expand All @@ -1572,6 +1589,22 @@ 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<u8>) -> Result<(), ()>;

/// Reschedule a task to a different time.
///
/// Will return an error if the `id` is invalid.
///
/// 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.
xlc marked this conversation as resolved.
Show resolved Hide resolved
fn reschedule_named(
id: Vec<u8>,
when: DispatchTime<BlockNumber>,
) -> Result<Self::Address, DispatchError>;

/// Return the next dispatch time for a given task.
///
/// Will return an error if the `id` is invalid.
fn next_dispatch_time(id: Vec<u8>) -> Result<BlockNumber, ()>;
}
}

Expand Down