[XCM] Treat recursion limit error as transient in the MQ

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(weight))
+			return Err(ProcessMessageError::Overweight(Some(weight)))
 		}
 		Self::do_process_message(origin, message)
 	}

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

@@ -139,7 +139,9 @@ 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(<Test as Config>::WeightInfo::do_process_message())
+			ProcessMessageError::Overweight(Some(
+				<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(required))
+			return Err(ProcessMessageError::Overweight(Some(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(max_weight))?;
+			.ok_or(ProcessMessageError::Overweight(Some(max_weight)))?;
 		Ok(())
 	}
 }
@@ -104,7 +104,7 @@ impl<T: Contains<Location>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T>
 					*weight_limit = Limited(max_weight);
 					Ok(())
 				},
-				_ => Err(ProcessMessageError::Overweight(max_weight)),
+				_ => Err(ProcessMessageError::Overweight(Some(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(max_weight)),
+			_ => Err(ProcessMessageError::Overweight(Some(max_weight))),
 		})?;
 		Ok(())
 	}

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

@@ -77,7 +77,7 @@ impl<
 				meter.remaining(),
 			);
 
-			return Err(ProcessMessageError::Overweight(required))
+			return Err(ProcessMessageError::Overweight(Some(required)))
 		}
 
 		let (consumed, result) = match XcmExecutor::execute(origin.into(), pre, id, Weight::zero())
@@ -102,7 +102,12 @@ impl<
 					target: LOG_TARGET,
 					"XCM message execution error: {error:?}",
 				);
-				(required, Err(ProcessMessageError::Unsupported))
+				let error = match error {
+					xcm::latest::Error::ExceedsStackLimit => ProcessMessageError::Overweight(None),
+					_ => ProcessMessageError::Unsupported,
+				};
+
+				(required, Err(error))
 			},
 		};
 		meter.consume(consumed);
@@ -148,6 +153,45 @@ mod tests {
 		}
 	}
 
+	#[test]
+	fn process_message_exceeds_limits_fails() {
+		struct MockedExecutor;
+		impl ExecuteXcm<()> for MockedExecutor {
+			type Prepared = xcm_executor::WeighedMessage<()>;
+			fn prepare(
+				message: xcm::latest::Xcm<()>,
+			) -> core::result::Result<Self::Prepared, xcm::latest::Xcm<()>> {
+				Ok(xcm_executor::WeighedMessage::new(Weight::zero(), message))
+			}
+			fn execute(
+				_: impl Into<Location>,
+				_: Self::Prepared,
+				_: &mut XcmHash,
+				_: Weight,
+			) -> Outcome {
+				Outcome::Error { error: xcm::latest::Error::ExceedsStackLimit }
+			}
+			fn charge_fees(_location: impl Into<Location>, _fees: Assets) -> xcm::latest::Result {
+				unreachable!()
+			}
+		}
+
+		type Processor = ProcessXcmMessage<Junction, MockedExecutor, ()>;
+
+		let xcm = VersionedXcm::V4(xcm::latest::Xcm::<()>(vec![
+			xcm::latest::Instruction::<()>::ClearOrigin,
+		]));
+		assert_err!(
+			Processor::process_message(
+				&xcm.encode(),
+				ORIGIN,
+				&mut WeightMeter::new(),
+				&mut [0; 32]
+			),
+			ProcessMessageError::Overweight(None)
+		);
+	}
+
 	#[test]
 	fn process_message_overweight_fails() {
 		for msg in [v3_xcm(true), v3_xcm(false), v3_xcm(false), v2_xcm(false)] {
@@ -159,7 +203,7 @@ mod tests {
 				let mut id = [0; 32];
 				assert_err!(
 					Processor::process_message(msg, ORIGIN, meter, &mut id),
-					Overweight(1000.into())
+					Overweight(Some(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(Weight::from_parts(10, 10))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(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(Weight::from_parts(20, 20))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(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(Weight::from_parts(20, 20))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(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(Weight::from_parts(20, 20))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(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(Weight::from_parts(30, 30))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(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(Weight::from_parts(20, 20))));
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Some(Weight::from_parts(20, 20)))));
 
 	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
 		&Parent.into(),

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

@@ -182,6 +182,13 @@ impl<C> PreparedMessage for WeighedMessage<C> {
 	}
 }
 
+#[cfg(feature = "std")]
+impl<C> WeighedMessage<C> {
+	pub fn new(weight: Weight, message: Xcm<C>) -> Self {
+		Self(weight, message)
+	}
+}
+
 impl<Config: config::Config> ExecuteXcm<Config::RuntimeCall> for XcmExecutor<Config> {
 	type Prepared = WeighedMessage<Config::RuntimeCall>;
 	fn prepare(

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)),
+								return Err($crate::XcmError::WeightLimitReached(required.expect("Processor must know the max weight."))),
 							// Not really the correct error, but there is no "undecodable".
 							Err(_) => return Err($crate::XcmError::Unimplemented),
 							Ok(_) => (),

prdoc/pr_4202.prdoc

@@ -0,0 +1,14 @@
+title: "Treat XCM ExceedsStackLimit errors as transient in the MQ pallet"
+
+doc:
+  - audience: Runtime User
+    description: |
+      Fixes an issue where the MessageQueue can incorrectly assume that a message will permanently fail to process and disallow retrial of it.
+
+crates:
+  - name: frame-support
+    bump: major
+  - name: pallet-message-queue
+    bump: patch
+  - name: staging-xcm-builder
+    bump: patch

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

@@ -1437,20 +1437,22 @@ impl<T: Config> Pallet<T> {
 		let prev_consumed = meter.consumed();
 
 		match T::MessageProcessor::process_message(message, origin.clone(), meter, &mut id) {
-			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(Overweight(required)) => {
+				if 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(Yield) => {
 				// Processing should be reattempted later.

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

@@ -191,7 +191,7 @@ impl ProcessMessage for RecordingMessageProcessor {
 			MessagesProcessed::set(m);
 			Ok(true)
 		} else {
-			Err(ProcessMessageError::Overweight(required))
+			Err(ProcessMessageError::Overweight(Some(required)))
 		}
 	}
 }
@@ -254,7 +254,7 @@ impl ProcessMessage for CountingMessageProcessor {
 			NumMessagesProcessed::set(NumMessagesProcessed::get() + 1);
 			Ok(true)
 		} else {
-			Err(ProcessMessageError::Overweight(required))
+			Err(ProcessMessageError::Overweight(Some(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(required))
+			Err(ProcessMessageError::Overweight(Some(required)))
 		}
 	}
 }

substrate/frame/support/src/traits/messages.rs

@@ -37,8 +37,8 @@ pub enum ProcessMessageError {
 	Unsupported,
 	/// Message processing was not attempted because it was not certain that the weight limit
 	/// would be respected. The parameter gives the maximum weight which the message could take
-	/// to process.
-	Overweight(Weight),
+	/// to process. If none is given, then the worst case is assumed.
+	Overweight(Option<Weight>),
 	/// The queue wants to give up its current processing slot.
 	///
 	/// Hints the message processor to cease servicing this queue and proceed to the next