Revert and use dedicated error variant

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

bridges/snowbridge/pallets/outbound-queue/src/process_message_impl.rs

@@ -18,7 +18,7 @@ impl<T: Config> ProcessMessage for Pallet<T> {
 	) -> Result<bool, ProcessMessageError> {
 		let weight = T::WeightInfo::do_process_message();
 		if meter.try_consume(weight).is_err() {
-			return Err(ProcessMessageError::Overweight(Some(weight)))
+			return Err(ProcessMessageError::Overweight(weight))
 		}
 		Self::do_process_message(origin, message)
 	}

bridges/snowbridge/pallets/outbound-queue/src/test.rs

@@ -139,9 +139,7 @@ fn process_message_fails_on_overweight_message() {
 		let mut meter = WeightMeter::with_limit(Weight::from_parts(1, 1));
 		assert_noop!(
 			OutboundQueue::process_message(encoded.as_slice(), origin, &mut meter, &mut [0u8; 32]),
-			ProcessMessageError::Overweight(Some(
-				<Test as Config>::WeightInfo::do_process_message()
-			))
+			ProcessMessageError::Overweight(<Test as Config>::WeightInfo::do_process_message())
 		);
 	})
 }

polkadot/runtime/parachains/src/mock.rs

@@ -603,7 +603,7 @@ impl ProcessMessage for TestProcessMessage {
 			Err(_) => return Err(ProcessMessageError::Corrupt), // same as the real `ProcessMessage`
 		};
 		if meter.try_consume(required).is_err() {
-			return Err(ProcessMessageError::Overweight(Some(required)))
+			return Err(ProcessMessageError::Overweight(required))
 		}
 
 		let mut processed = Processed::get();

polkadot/xcm/xcm-builder/src/barriers.rs

@@ -47,7 +47,7 @@ impl ShouldExecute for TakeWeightCredit {
 		properties.weight_credit = properties
 			.weight_credit
 			.checked_sub(&max_weight)
-			.ok_or(ProcessMessageError::Overweight(Some(max_weight)))?;
+			.ok_or(ProcessMessageError::Overweight(max_weight))?;
 		Ok(())
 	}
 }
@@ -104,7 +104,7 @@ impl<T: Contains<Location>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T>
 					*weight_limit = Limited(max_weight);
 					Ok(())
 				},
-				_ => Err(ProcessMessageError::Overweight(Some(max_weight))),
+				_ => Err(ProcessMessageError::Overweight(max_weight)),
 			})?;
 		Ok(())
 	}
@@ -304,7 +304,7 @@ impl<T: Contains<Location>> ShouldExecute for AllowExplicitUnpaidExecutionFrom<T
 		instructions.matcher().match_next_inst(|inst| match inst {
 			UnpaidExecution { weight_limit: Limited(m), .. } if m.all_gte(max_weight) => Ok(()),
 			UnpaidExecution { weight_limit: Unlimited, .. } => Ok(()),
-			_ => Err(ProcessMessageError::Overweight(Some(max_weight))),
+			_ => Err(ProcessMessageError::Overweight(max_weight)),
 		})?;
 		Ok(())
 	}

polkadot/xcm/xcm-builder/src/process_xcm_message.rs

@@ -77,7 +77,7 @@ impl<
 				meter.remaining(),
 			);
 
-			return Err(ProcessMessageError::Overweight(Some(required)))
+			return Err(ProcessMessageError::Overweight(required))
 		}
 
 		let (consumed, result) = match XcmExecutor::execute(origin.into(), pre, id, Weight::zero())
@@ -103,7 +103,7 @@ impl<
 					"XCM message execution error: {error:?}",
 				);
 				let error = match error {
-					xcm::latest::Error::ExceedsStackLimit => ProcessMessageError::Overweight(None),
+					xcm::latest::Error::ExceedsStackLimit => ProcessMessageError::StackLimitReached,
 					_ => ProcessMessageError::Unsupported,
 				};
 
@@ -188,7 +188,7 @@ mod tests {
 				&mut WeightMeter::new(),
 				&mut [0; 32]
 			),
-			ProcessMessageError::Overweight(None)
+			ProcessMessageError::StackLimitReached,
 		);
 	}
 
@@ -203,7 +203,7 @@ mod tests {
 				let mut id = [0; 32];
 				assert_err!(
 					Processor::process_message(msg, ORIGIN, meter, &mut id),
-					Overweight(Some(1000.into()))
+					Overweight(1000.into())
 				);
 				assert_eq!(meter.consumed(), 0.into());
 			}

polkadot/xcm/xcm-builder/src/tests/barriers.rs

@@ -42,7 +42,7 @@ fn take_weight_credit_barrier_should_work() {
 		Weight::from_parts(10, 10),
 		&mut properties,
 	);
-	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(10, 10)))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(10, 10))));
 	assert_eq!(properties.weight_credit, Weight::zero());
 }
 
@@ -163,15 +163,15 @@ fn allow_explicit_unpaid_should_work() {
 		Weight::from_parts(20, 20),
 		&mut props(Weight::zero()),
 	);
-	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(20, 20)))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20))));
 
 	let r = AllowExplicitUnpaidExecutionFrom::<IsInVec<AllowExplicitUnpaidFrom>>::should_execute(
 		&Parent.into(),
 		bad_message2.inner_mut(),
 		Weight::from_parts(20, 20),
 		&mut props(Weight::zero()),
 	);
-	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(20, 20)))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20))));
 
 	let r = AllowExplicitUnpaidExecutionFrom::<IsInVec<AllowExplicitUnpaidFrom>>::should_execute(
 		&Parent.into(),
@@ -195,7 +195,7 @@ fn allow_explicit_unpaid_should_work() {
 		Weight::from_parts(20, 20),
 		&mut props(Weight::zero()),
 	);
-	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(20, 20)))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20))));
 
 	let r = AllowExplicitUnpaidExecutionFrom::<IsInVec<AllowExplicitUnpaidFrom>>::should_execute(
 		&Parent.into(),
@@ -234,7 +234,7 @@ fn allow_paid_should_work() {
 		Weight::from_parts(30, 30),
 		&mut props(Weight::zero()),
 	);
-	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(30, 30)))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(30, 30))));
 
 	let fees = (Parent, 1).into();
 	let mut paying_message = Xcm::<()>(vec![
@@ -272,7 +272,7 @@ fn allow_paid_should_work() {
 		Weight::from_parts(20, 20),
 		&mut props(Weight::zero()),
 	);
-	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(20, 20)))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(20, 20))));
 
 	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
 		&Parent.into(),

polkadot/xcm/xcm-simulator/src/lib.rs

@@ -329,7 +329,7 @@ macro_rules! decl_test_network {
 						);
 						match r {
 							Err($crate::ProcessMessageError::Overweight(required)) =>
-								return Err($crate::XcmError::WeightLimitReached(required.expect("Processor must know the max weight."))),
+								return Err($crate::XcmError::WeightLimitReached(required)),
 							// Not really the correct error, but there is no "undecodable".
 							Err(_) => return Err($crate::XcmError::Unimplemented),
 							Ok(_) => (),

substrate/frame/message-queue/src/lib.rs

@@ -765,6 +765,13 @@ enum MessageExecutionStatus {
 	Processed,
 	/// The message was processed and resulted in a, possibly permanent, error.
 	Unprocessable { permanent: bool },
+	/// The stack depth limit was reached.
+	///
+	/// We cannot just return `Unprocessable` in this case, because the processability of the
+	/// message depends on how the function was called. This may be a permanent error if it was
+	/// called by a top-level function, or a transient error if it was already called in a nested
+	/// function.
+	StackLimitReached,
 }
 
 impl<T: Config> Pallet<T> {
@@ -984,7 +991,8 @@ impl<T: Config> Pallet<T> {
 			// additional overweight event being deposited.
 		) {
 			Overweight | InsufficientWeight => Err(Error::<T>::InsufficientWeight),
-			Unprocessable { permanent: false } => Err(Error::<T>::TemporarilyUnprocessable),
+			StackLimitReached | Unprocessable { permanent: false } =>
+				Err(Error::<T>::TemporarilyUnprocessable),
 			Unprocessable { permanent: true } | Processed => {
 				page.note_processed_at_pos(pos);
 				book_state.message_count.saturating_dec();
@@ -1250,7 +1258,7 @@ impl<T: Config> Pallet<T> {
 		let is_processed = match res {
 			InsufficientWeight => return ItemExecutionStatus::Bailed,
 			Unprocessable { permanent: false } => return ItemExecutionStatus::NoProgress,
-			Processed | Unprocessable { permanent: true } => true,
+			Processed | Unprocessable { permanent: true } | StackLimitReached => true,
 			Overweight => false,
 		};
 
@@ -1437,22 +1445,20 @@ impl<T: Config> Pallet<T> {
 		let prev_consumed = meter.consumed();
 
 		match T::MessageProcessor::process_message(message, origin.clone(), meter, &mut id) {
-			Err(Overweight(maybe_required)) => {
-				if maybe_required.map_or(true, |r| r.any_gt(overweight_limit)) {
-					// Permanently overweight.
-					Self::deposit_event(Event::<T>::OverweightEnqueued {
-						id,
-						origin,
-						page_index,
-						message_index,
-					});
-
-					MessageExecutionStatus::Overweight
-				} else {
-					// Temporarily overweight - save progress and stop processing this
-					// queue.
-					MessageExecutionStatus::InsufficientWeight
-				}
+			Err(Overweight(w)) if w.any_gt(overweight_limit) => {
+				// Permanently overweight.
+				Self::deposit_event(Event::<T>::OverweightEnqueued {
+					id,
+					origin,
+					page_index,
+					message_index,
+				});
+				MessageExecutionStatus::Overweight
+			},
+			Err(Overweight(_)) => {
+				// Temporarily overweight - save progress and stop processing this
+				// queue.
+				MessageExecutionStatus::InsufficientWeight
 			},
 			Err(Yield) => {
 				// Processing should be reattempted later.
@@ -1463,6 +1469,10 @@ impl<T: Config> Pallet<T> {
 				Self::deposit_event(Event::<T>::ProcessingFailed { id: id.into(), origin, error });
 				MessageExecutionStatus::Unprocessable { permanent: true }
 			},
+			Err(error @ StackLimitReached) => {
+				Self::deposit_event(Event::<T>::ProcessingFailed { id: id.into(), origin, error });
+				MessageExecutionStatus::StackLimitReached
+			},
 			Ok(success) => {
 				// Success
 				let weight_used = meter.consumed().saturating_sub(prev_consumed);

substrate/frame/message-queue/src/mock.rs

@@ -191,13 +191,14 @@ impl ProcessMessage for RecordingMessageProcessor {
 			MessagesProcessed::set(m);
 			Ok(true)
 		} else {
-			Err(ProcessMessageError::Overweight(Some(required)))
+			Err(ProcessMessageError::Overweight(required))
 		}
 	}
 }
 
 parameter_types! {
 	pub static Callback: Box<fn (&MessageOrigin, u32)> = Box::new(|_, _| {});
+	pub static IgnoreStackOvError: bool = false;
 }
 
 /// Processed a mocked message. Messages that end with `badformat`, `corrupt`, `unsupported` or
@@ -216,6 +217,8 @@ fn processing_message(msg: &[u8], origin: &MessageOrigin) -> Result<(), ProcessM
 		Err(ProcessMessageError::Unsupported)
 	} else if msg.ends_with("yield") {
 		Err(ProcessMessageError::Yield)
+	} else if msg.ends_with("stacklimitreached") && !IgnoreStackOvError::get() {
+		Err(ProcessMessageError::StackLimitReached)
 	} else {
 		Ok(())
 	}
@@ -254,7 +257,7 @@ impl ProcessMessage for CountingMessageProcessor {
 			NumMessagesProcessed::set(NumMessagesProcessed::get() + 1);
 			Ok(true)
 		} else {
-			Err(ProcessMessageError::Overweight(Some(required)))
+			Err(ProcessMessageError::Overweight(required))
 		}
 	}
 }

substrate/frame/message-queue/src/mock_helpers.rs

@@ -71,7 +71,7 @@ where
 		if meter.try_consume(required).is_ok() {
 			Ok(true)
 		} else {
-			Err(ProcessMessageError::Overweight(Some(required)))
+			Err(ProcessMessageError::Overweight(required))
 		}
 	}
 }