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 7 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
207 changes: 186 additions & 21 deletions frame/scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,12 @@ decl_error! {
pub enum Error for Module<T: Trait> {
/// 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,
}
}

Expand Down Expand Up @@ -438,13 +440,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 +452,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 +504,34 @@ 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)?;

if new_time == when {
return Err(Error::<T>::RescheduleNoChange.into());
}

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 +545,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 +583,41 @@ 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)?;

if new_time == when {
return Err(Error::<T>::RescheduleNoChange.into());
}

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 +636,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 +666,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 +900,95 @@ 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));

assert_noop!(Scheduler::do_reschedule((6, 0), DispatchTime::At(6)), Error::<Test>::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() {
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));

assert_noop!(Scheduler::do_reschedule_named(1u32.encode(), DispatchTime::At(6)), Error::<Test>::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!(!<Test as frame_system::Trait>::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(|| {
Expand Down
34 changes: 30 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,30 @@ 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<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 +1586,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<u8>) -> 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<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