pallet-scheduler: Send CallUnavailable event if the call isn't present (#1161)

The event was already send before, but it was done at the wrong
position. The pull request all changes the `execute_dispatch` signature
to highlight that there is only one error type.

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Javier Viola <javier@parity.io>

substrate/frame/scheduler/src/lib.rs

@@ -1096,7 +1096,14 @@ impl<T: Config> Pallet<T> {
 
 		let (call, lookup_len) = match T::Preimages::peek(&task.call) {
 			Ok(c) => c,
-			Err(_) => return Err((Unavailable, Some(task))),
+			Err(_) => {
+				Self::deposit_event(Event::CallUnavailable {
+					task: (when, agenda_index),
+					id: task.maybe_id,
+				});
+
+				return Err((Unavailable, Some(task)))
+			},
 		};
 
 		let _ = weight.try_consume(T::WeightInfo::service_task(
@@ -1106,15 +1113,7 @@ impl<T: Config> Pallet<T> {
 		));
 
 		match Self::execute_dispatch(weight, task.origin.clone(), call) {
-			Err(Unavailable) => {
-				debug_assert!(false, "Checked to exist with `peek`");
-				Self::deposit_event(Event::CallUnavailable {
-					task: (when, agenda_index),
-					id: task.maybe_id,
-				});
-				Err((Unavailable, Some(task)))
-			},
-			Err(Overweight) if is_first => {
+			Err(()) if is_first => {
 				T::Preimages::drop(&task.call);
 				Self::deposit_event(Event::PermanentlyOverweight {
 					task: (when, agenda_index),
@@ -1122,7 +1121,7 @@ impl<T: Config> Pallet<T> {
 				});
 				Err((Unavailable, Some(task)))
 			},
-			Err(Overweight) => Err((Overweight, Some(task))),
+			Err(()) => Err((Overweight, Some(task))),
 			Ok(result) => {
 				Self::deposit_event(Event::Dispatched {
 					task: (when, agenda_index),
@@ -1162,11 +1161,13 @@ impl<T: Config> Pallet<T> {
 	///
 	/// NOTE: Only the weight for this function will be counted (origin lookup, dispatch and the
 	/// call itself).
+	///
+	/// Returns an error if the call is overweight.
 	fn execute_dispatch(
 		weight: &mut WeightMeter,
 		origin: T::PalletsOrigin,
 		call: <T as Config>::RuntimeCall,
-	) -> Result<DispatchResult, ServiceTaskError> {
+	) -> Result<DispatchResult, ()> {
 		let base_weight = match origin.as_system_ref() {
 			Some(&RawOrigin::Signed(_)) => T::WeightInfo::execute_dispatch_signed(),
 			_ => T::WeightInfo::execute_dispatch_unsigned(),
@@ -1176,7 +1177,7 @@ impl<T: Config> Pallet<T> {
 		let max_weight = base_weight.saturating_add(call_weight);
 
 		if !weight.can_consume(max_weight) {
-			return Err(Overweight)
+			return Err(())
 		}
 
 		let dispatch_origin = origin.into();

substrate/frame/scheduler/src/tests.rs

@@ -1878,3 +1878,42 @@ fn reschedule_named_last_task_removes_agenda() {
 		assert!(Agenda::<Test>::get(when).len() == 0);
 	});
 }
+
+/// Ensures that an unvailable call sends an event.
+#[test]
+fn unavailable_call_is_detected() {
+	use frame_support::traits::schedule::v3::Named;
+
+	new_test_ext().execute_with(|| {
+		let call =
+			RuntimeCall::Logger(LoggerCall::log { i: 42, weight: Weight::from_parts(10, 0) });
+		let hash = <Test as frame_system::Config>::Hashing::hash_of(&call);
+		let len = call.using_encoded(|x| x.len()) as u32;
+		// Important to use here `Bounded::Lookup` to ensure that we request the hash.
+		let bound = Bounded::Lookup { hash, len };
+
+		let name = [1u8; 32];
+
+		// Schedule a call.
+		let _address = <Scheduler as Named<_, _, _>>::schedule_named(
+			name,
+			DispatchTime::At(4),
+			None,
+			127,
+			root(),
+			bound.clone(),
+		)
+		.unwrap();
+
+		// Ensure the preimage isn't available
+		assert!(!Preimage::have(&bound));
+
+		// Executes in block 4.
+		run_to_block(4);
+
+		assert_eq!(
+			System::events().last().unwrap().event,
+			crate::Event::CallUnavailable { task: (4, 0), id: Some(name) }.into()
+		);
+	});
+}