From 69541e15978c47edf079f0c37ec6f332e8d77206 Mon Sep 17 00:00:00 2001
From: Bryan Chen <xlchen1291@gmail.com>
Date: Mon, 20 Nov 2023 19:37:13 +1300
Subject: [PATCH] ensure xcm outcome is always complete

---
 polkadot/xcm/pallet-xcm/src/lib.rs       | 16 +++++++++++++---
 polkadot/xcm/pallet-xcm/src/tests/mod.rs |  2 +-
 polkadot/xcm/src/v3/traits.rs            |  4 ++--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs
index 38ea7555fc327..2f924506955b6 100644
--- a/polkadot/xcm/pallet-xcm/src/lib.rs
+++ b/polkadot/xcm/pallet-xcm/src/lib.rs
@@ -1010,6 +1010,10 @@ pub mod pallet {
 			max_weight: Weight,
 		) -> DispatchResultWithPostInfo {
 			let outcome = <Self as ExecuteController<_, _>>::execute(origin, message, max_weight)?;
+			outcome.clone().ensure_complete().map_err(|error| {
+				log::error!(target: "runtime::xcm", "XCM execution failed with error {:?}", error);
+				Error::<T>::LocalExecutionIncomplete
+			})?;
 			Ok(Some(outcome.weight_used().saturating_add(T::WeightInfo::execute())).into())
 		}
 
@@ -1495,13 +1499,19 @@ impl<T: Config> Pallet<T> {
 		let outcome =
 			T::XcmExecutor::execute_xcm_in_credit(origin, local_xcm, hash, weight, weight);
 		Self::deposit_event(Event::Attempted { outcome: outcome.clone() });
-		if let Some(remote_xcm) = remote_xcm {
-			outcome.ensure_complete().map_err(|_| Error::<T>::LocalExecutionIncomplete)?;
+		outcome.ensure_complete().map_err(|error| {
+			log::error!(target: "runtime::xcm", "XCM execution failed with error {:?}", error);
+			Error::<T>::LocalExecutionIncomplete
+		})?;
 
+		if let Some(remote_xcm) = remote_xcm {
 			let (ticket, price) = validate_send::<T::XcmRouter>(dest, remote_xcm.clone())
 				.map_err(Error::<T>::from)?;
 			if origin != Here.into_location() {
-				Self::charge_fees(origin, price).map_err(|_| Error::<T>::FeesNotMet)?;
+				Self::charge_fees(origin, price).map_err(|error| {
+					log::error!(target: "runtime::xcm", "Unable to charge fee with error {:?}", error);
+					Error::<T>::FeesNotMet
+				})?;
 			}
 			let message_id = T::XcmRouter::deliver(ticket).map_err(Error::<T>::from)?;
 
diff --git a/polkadot/xcm/pallet-xcm/src/tests/mod.rs b/polkadot/xcm/pallet-xcm/src/tests/mod.rs
index 72814e507f2a0..49121a4383e9c 100644
--- a/polkadot/xcm/pallet-xcm/src/tests/mod.rs
+++ b/polkadot/xcm/pallet-xcm/src/tests/mod.rs
@@ -445,7 +445,7 @@ fn trapped_assets_can_be_claimed() {
 		assert_eq!(AssetTraps::<Test>::iter().collect::<Vec<_>>(), vec![]);
 
 		let weight = BaseXcmWeight::get() * 3;
-		assert_ok!(XcmPallet::execute(
+		assert_ok!(<XcmPallet as xcm_builder::ExecuteController<_, _>>::execute(
 			RuntimeOrigin::signed(ALICE),
 			Box::new(VersionedXcm::from(Xcm(vec![
 				ClaimAsset { assets: (Here, SEND_AMOUNT).into(), ticket: Here.into() },
diff --git a/polkadot/xcm/src/v3/traits.rs b/polkadot/xcm/src/v3/traits.rs
index 1043d17b71066..6054bf1456a5d 100644
--- a/polkadot/xcm/src/v3/traits.rs
+++ b/polkadot/xcm/src/v3/traits.rs
@@ -275,9 +275,9 @@ pub enum Outcome {
 }
 
 impl Outcome {
-	pub fn ensure_complete(self) -> Result {
+	pub fn ensure_complete(self) -> result::Result<Weight, Error> {
 		match self {
-			Outcome::Complete(_) => Ok(()),
+			Outcome::Complete(weight) => Ok(weight),
 			Outcome::Incomplete(_, e) => Err(e),
 			Outcome::Error(e) => Err(e),
 		}