git.delta.rocks / unique-network / refs/commits / 25bffe721f74

--- a/pallets/scheduler-v2/src/lib.rs
+++ b/pallets/scheduler-v2/src/lib.rs
@@ -96,7 +96,7 @@
 	BoundedVec, RuntimeDebug, DispatchErrorWithPostInfo,
 };
 use sp_core::H160;
-use sp_std::{borrow::Borrow, cmp::Ordering, marker::PhantomData, prelude::*};
+use sp_std::{cmp::Ordering, marker::PhantomData, prelude::*};
 pub use weights::WeightInfo;
 
 pub use pallet::*;
@@ -254,9 +254,9 @@
 }
 
 impl<T: Config> BlockAgenda<T> {
-	fn try_push(&mut self, scheduled: ScheduledOf<T>) -> Option<u32> {
+	fn try_push(&mut self, scheduled: ScheduledOf<T>) -> Result<u32, ScheduledOf<T>> {
 		if self.free_places == 0 {
-			return None;
+			return Err(scheduled);
 		}
 
 		self.free_places = self.free_places.saturating_sub(1);
@@ -264,14 +264,14 @@
 		if (self.agenda.len() as u32) < T::MaxScheduledPerBlock::get() {
 			// will always succeed due to the above check.
 			let _ = self.agenda.try_push(Some(scheduled));
-			Some((self.agenda.len() - 1) as u32)
+			Ok((self.agenda.len() - 1) as u32)
 		} else {
 			match self.agenda.iter().position(|i| i.is_none()) {
 				Some(hole_index) => {
 					self.agenda[hole_index] = Some(scheduled);
-					Some(hole_index as u32)
+					Ok(hole_index as u32)
 				}
-				None => unreachable!("free_places > 0; qed"),
+				None => unreachable!("free_places was greater than 0; qed"),
 			}
 		}
 	}
@@ -473,11 +473,6 @@
 		},
 		/// The call for the provided hash was not found so the task has been aborted.
 		CallUnavailable {
-			task: TaskAddress<T::BlockNumber>,
-			id: Option<[u8; 32]>,
-		},
-		/// The given task was unable to be renewed since the agenda is full at that block.
-		PeriodicFailed {
 			task: TaskAddress<T::BlockNumber>,
 			id: Option<[u8; 32]>,
 		},
@@ -688,12 +683,24 @@
 		Ok(when)
 	}
 
-	fn place_task(
+	fn mandatory_place_task(when: T::BlockNumber, what: ScheduledOf<T>) {
+		Self::place_task(when, what, true).expect("mandatory place task always succeeds; qed");
+	}
+
+	fn try_place_task(
 		when: T::BlockNumber,
 		what: ScheduledOf<T>,
-	) -> Result<TaskAddress<T::BlockNumber>, (DispatchError, ScheduledOf<T>)> {
+	) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
+		Self::place_task(when, what, false)
+	}
+
+	fn place_task(
+		mut when: T::BlockNumber,
+		what: ScheduledOf<T>,
+		is_mandatory: bool,
+	) -> Result<TaskAddress<T::BlockNumber>, DispatchError> {
 		let maybe_name = what.maybe_id;
-		let index = Self::push_to_agenda(when, what)?;
+		let index = Self::push_to_agenda(&mut when, what, is_mandatory)?;
 		let address = (when, index);
 		if let Some(name) = maybe_name {
 			Lookup::<T>::insert(name, address)
@@ -706,13 +713,24 @@
 	}
 
 	fn push_to_agenda(
-		when: T::BlockNumber,
-		what: ScheduledOf<T>,
-	) -> Result<u32, (DispatchError, ScheduledOf<T>)> {
-		let mut agenda = Agenda::<T>::get(when);
-		let index = agenda
-			.try_push(what.clone())
-			.ok_or((<Error<T>>::AgendaIsExhausted.into(), what))?;
+		when: &mut T::BlockNumber,
+		mut what: ScheduledOf<T>,
+		is_mandatory: bool,
+	) -> Result<u32, DispatchError> {
+		let mut agenda;
+
+		let index = loop {
+			agenda = Agenda::<T>::get(*when);
+
+			match agenda.try_push(what) {
+				Ok(index) => break index,
+				Err(returned_what) if is_mandatory => {
+					what = returned_what;
+					when.saturating_inc();
+				}
+				Err(_) => return Err(<Error<T>>::AgendaIsExhausted.into()),
+			}
+		};
 
 		Agenda::<T>::insert(when, agenda);
 		Ok(index)
@@ -740,7 +758,7 @@
 			origin,
 			_phantom: PhantomData,
 		};
-		Self::place_task(when, task).map_err(|x| x.0)
+		Self::try_place_task(when, task)
 	}
 
 	fn do_cancel(
@@ -809,7 +827,7 @@
 			origin,
 			_phantom: Default::default(),
 		};
-		Self::place_task(when, task).map_err(|x| x.0)
+		Self::try_place_task(when, task)
 	}
 
 	fn do_cancel_named(origin: Option<T::PalletsOrigin>, id: TaskName) -> DispatchResult {
@@ -1075,18 +1093,7 @@
 							task.maybe_periodic = None;
 						}
 						let wake = now.saturating_add(period);
-						match Self::place_task(wake, task) {
-							Ok(_) => {}
-							Err((_, task)) => {
-								// TODO: Leave task in storage somewhere for it to be rescheduled
-								// manually.
-								T::Preimages::drop(&task.call);
-								Self::deposit_event(Event::PeriodicFailed {
-									task: (when, agenda_index),
-									id: task.maybe_id,
-								});
-							}
-						}
+						Self::mandatory_place_task(wake, task);
 					}
 					_ => {
 						if let Some(ref id) = task.maybe_id {

319			.into(),319			.into(),
320		);320		);
321		// The call is still in the agenda.321		// The call is still in the agenda.
322		assert!(Agenda::<Test>::get(4)[0].is_some());322		assert!(Agenda::<Test>::get(4).agenda[0].is_some());
323	});323	});
324}324}
325325
326#[test]326#[test]
327fn scheduler_handles_periodic_failure() {327fn scheduler_periodic_tasks_always_find_place() {
328	let max_weight: Weight = <Test as Config>::MaximumWeight::get();328	let max_weight: Weight = <Test as Config>::MaximumWeight::get();
329	let max_per_block = <Test as Config>::MaxScheduledPerBlock::get();329	let max_per_block = <Test as Config>::MaxScheduledPerBlock::get();
330330
357			));357			));
358		}358		}
359359
360		// Going to block 24 will emit a `PeriodicFailed` event.
361		run_to_block(24);360		run_to_block(24);
362		assert_eq!(logger::log().len(), 6);361		assert_eq!(logger::log().len(), 6);
363362
363		// The periodic task should be postponed
364		assert_eq!(364		assert_eq!(<Agenda<Test>>::get(29).agenda.len(), 1);
365			System::events().last().unwrap().event,365
366			crate::Event::PeriodicFailed {366		run_to_block(27); // will call on_initialize(28)
367				task: (24, 0),367		assert_eq!(logger::log().len(), 6);
368				id: None368
369			}369		run_to_block(28); // will call on_initialize(29)
370			.into(),370		assert_eq!(logger::log().len(), 7);
371		);
372	});371	});
373}372}
596		));595		));
597		run_to_block(3);596		run_to_block(3);
598		// Scheduled calls are in the agenda.597		// Scheduled calls are in the agenda.
599		assert_eq!(Agenda::<Test>::get(4).len(), 2);598		assert_eq!(Agenda::<Test>::get(4).agenda.len(), 2);
600		assert!(logger::log().is_empty());599		assert!(logger::log().is_empty());
601		assert_ok!(Scheduler::cancel_named(RuntimeOrigin::root(), [1u8; 32]));600		assert_ok!(Scheduler::cancel_named(RuntimeOrigin::root(), [1u8; 32]));
602		assert_ok!(Scheduler::cancel(RuntimeOrigin::root(), 4, 1));601		assert_ok!(Scheduler::cancel(RuntimeOrigin::root(), 4, 1));
669		));668		));
670		run_to_block(3);669		run_to_block(3);
671		// Scheduled calls are in the agenda.670		// Scheduled calls are in the agenda.
672		assert_eq!(Agenda::<Test>::get(4).len(), 2);671		assert_eq!(Agenda::<Test>::get(4).agenda.len(), 2);
673		assert!(logger::log().is_empty());672		assert!(logger::log().is_empty());
674		assert_ok!(Scheduler::cancel_named(673		assert_ok!(Scheduler::cancel_named(
675			system::RawOrigin::Signed(1).into(),674			system::RawOrigin::Signed(1).into(),
739		));738		));
740		run_to_block(3);739		run_to_block(3);
741		// Scheduled calls are in the agenda.740		// Scheduled calls are in the agenda.
742		assert_eq!(Agenda::<Test>::get(4).len(), 2);741		assert_eq!(Agenda::<Test>::get(4).agenda.len(), 2);
743		assert!(logger::log().is_empty());742		assert!(logger::log().is_empty());
744		assert_noop!(743		assert_noop!(
745			Scheduler::cancel_named(system::RawOrigin::Signed(2).into(), [1u8; 32]),744			Scheduler::cancel_named(system::RawOrigin::Signed(2).into(), [1u8; 32]),