From 64361ac19a3c74ec50329403e45a3a98fc40ac5b Mon Sep 17 00:00:00 2001
From: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Date: Thu, 30 Nov 2023 12:58:39 +0100
Subject: [PATCH] Withdraw Assets Before Checking Out in `OnReapIdentity` impl
 (#2552)

Follow up to fix a bug from #1814 discovered in XCM emulator testing.

I mistakenly thought that checking out an asset would withdraw it from
the sender. This actually withdraws the asset before checking out.

---------

Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
---
 polkadot/runtime/rococo/src/impls.rs  | 24 +++++++++++++++++++++++-
 polkadot/runtime/westend/src/impls.rs | 24 +++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs
index 71b1091eeb6..eddbfacc3b1 100644
--- a/polkadot/runtime/rococo/src/impls.rs
+++ b/polkadot/runtime/rococo/src/impls.rs
@@ -79,6 +79,8 @@ impl<Runtime, AccountId> ToParachainIdentityReaper<Runtime, AccountId> {
 	}
 }
 
+// Note / Warning: This implementation should only be used in a transactional context. If not, then
+// an error could result in assets being burned.
 impl<Runtime, AccountId> OnReapIdentity<AccountId> for ToParachainIdentityReaper<Runtime, AccountId>
 where
 	Runtime: frame_system::Config + pallet_xcm::Config,
@@ -100,6 +102,19 @@ where
 		// Do `check_out` accounting since the XCM Executor's `InitiateTeleport` doesn't support
 		// unpaid teleports.
 
+		// withdraw the asset from `who`
+		let who_origin =
+			Junction::AccountId32 { network: None, id: who.clone().into() }.into_location();
+		let _withdrawn = xcm_config::LocalAssetTransactor::withdraw_asset(&roc, &who_origin, None)
+			.map_err(|err| {
+				log::error!(
+					target: "runtime::on_reap_identity",
+					"withdraw_asset(what: {:?}, who_origin: {:?}) error: {:?}",
+					roc, who_origin, err
+				);
+				pallet_xcm::Error::<Runtime>::LowBalance
+			})?;
+
 		// check out
 		xcm_config::LocalAssetTransactor::can_check_out(
 			&destination,
@@ -107,7 +122,14 @@ where
 			// not used in AssetTransactor
 			&XcmContext { origin: None, message_id: [0; 32], topic: None },
 		)
-		.map_err(|_| pallet_xcm::Error::<Runtime>::CannotCheckOutTeleport)?;
+		.map_err(|err| {
+			log::error!(
+				target: "runtime::on_reap_identity",
+				"can_check_out(destination: {:?}, asset: {:?}, _) error: {:?}",
+				destination, roc, err
+			);
+			pallet_xcm::Error::<Runtime>::CannotCheckOutTeleport
+		})?;
 		xcm_config::LocalAssetTransactor::check_out(
 			&destination,
 			&roc,
diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs
index 80105594965..5f23bd373b1 100644
--- a/polkadot/runtime/westend/src/impls.rs
+++ b/polkadot/runtime/westend/src/impls.rs
@@ -79,6 +79,8 @@ impl<Runtime, AccountId> ToParachainIdentityReaper<Runtime, AccountId> {
 	}
 }
 
+// Note / Warning: This implementation should only be used in a transactional context. If not, then
+// an error could result in assets being burned.
 impl<Runtime, AccountId> OnReapIdentity<AccountId> for ToParachainIdentityReaper<Runtime, AccountId>
 where
 	Runtime: frame_system::Config + pallet_xcm::Config,
@@ -100,6 +102,19 @@ where
 		// Do `check_out` accounting since the XCM Executor's `InitiateTeleport` doesn't support
 		// unpaid teleports.
 
+		// withdraw the asset from `who`
+		let who_origin =
+			Junction::AccountId32 { network: None, id: who.clone().into() }.into_location();
+		let _withdrawn = xcm_config::LocalAssetTransactor::withdraw_asset(&wnd, &who_origin, None)
+			.map_err(|err| {
+				log::error!(
+					target: "runtime::on_reap_identity",
+					"withdraw_asset(what: {:?}, who_origin: {:?}) error: {:?}",
+					wnd, who_origin, err
+				);
+				pallet_xcm::Error::<Runtime>::LowBalance
+			})?;
+
 		// check out
 		xcm_config::LocalAssetTransactor::can_check_out(
 			&destination,
@@ -107,7 +122,14 @@ where
 			// not used in AssetTransactor
 			&XcmContext { origin: None, message_id: [0; 32], topic: None },
 		)
-		.map_err(|_| pallet_xcm::Error::<Runtime>::CannotCheckOutTeleport)?;
+		.map_err(|err| {
+			log::error!(
+				target: "runtime::on_reap_identity",
+				"can_check_out(destination: {:?}, asset: {:?}, _) error: {:?}",
+				destination, wnd, err
+			);
+			pallet_xcm::Error::<Runtime>::CannotCheckOutTeleport
+		})?;
 		xcm_config::LocalAssetTransactor::check_out(
 			&destination,
 			&wnd,
-- 
GitLab