From 140a5e864d23fafd79abd5d80f30185c49dda856 Mon Sep 17 00:00:00 2001 From: joepetrowski Date: Wed, 29 Nov 2023 21:05:48 +0100 Subject: [PATCH 1/6] actually withdraw assets before checking out --- polkadot/runtime/rococo/src/impls.rs | 6 ++++++ polkadot/runtime/westend/src/impls.rs | 6 ++++++ polkadot/xcm/pallet-xcm/src/lib.rs | 2 ++ 3 files changed, 14 insertions(+) diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index 71b1091eeb6c..9e818857261f 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -100,6 +100,12 @@ 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(|_| pallet_xcm::Error::::CannotWithdraw)?; + // check out xcm_config::LocalAssetTransactor::can_check_out( &destination, diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs index 801055949654..d3c0832db270 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -100,6 +100,12 @@ 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(|_| pallet_xcm::Error::::CannotWithdraw)?; + // check out xcm_config::LocalAssetTransactor::can_check_out( &destination, diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 74a24b132da7..17546e891e4a 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -514,6 +514,8 @@ pub mod pallet { CannotReanchor, /// Too many assets have been attempted for transfer. TooManyAssets, + /// The amount of the asset requested could not be withdrawn from the origin. + CannotWithdraw, /// Origin is invalid for sending. InvalidOrigin, /// The version of the `Versioned` value used is not able to be interpreted. From 76c42e4a48647fd433c9b07c5bf7ce053e51666b Mon Sep 17 00:00:00 2001 From: joepetrowski Date: Wed, 29 Nov 2023 21:52:12 +0100 Subject: [PATCH 2/6] don't break error encoding --- polkadot/xcm/pallet-xcm/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 17546e891e4a..4cf6cf10fe6e 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -514,8 +514,6 @@ pub mod pallet { CannotReanchor, /// Too many assets have been attempted for transfer. TooManyAssets, - /// The amount of the asset requested could not be withdrawn from the origin. - CannotWithdraw, /// Origin is invalid for sending. InvalidOrigin, /// The version of the `Versioned` value used is not able to be interpreted. @@ -551,6 +549,8 @@ pub mod pallet { TooManyReserves, /// Local XCM execution incomplete. LocalExecutionIncomplete, + /// The amount of the asset requested could not be withdrawn from the origin. + CannotWithdraw, } impl From for Error { From 7002eb0618e6426ccc08a1ecc421b152e322a947 Mon Sep 17 00:00:00 2001 From: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Date: Thu, 30 Nov 2023 06:47:28 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Adrian Catangiu --- polkadot/runtime/rococo/src/impls.rs | 9 ++++++++- polkadot/runtime/westend/src/impls.rs | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index 9e818857261f..255cb7d3f440 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -104,7 +104,14 @@ where 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(|_| pallet_xcm::Error::::CannotWithdraw)?; + .map_err(|err| { + log::error!( + target: "runtime::on_reap_identity", + "withdraw_asset(what: {:?}, who_origin: {:?}) error: {:?}", + roc, who_origin, err + ); + pallet_xcm::Error::::CannotWithdraw + })?; // check out xcm_config::LocalAssetTransactor::can_check_out( diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs index d3c0832db270..369161d13313 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -104,7 +104,14 @@ where 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(|_| pallet_xcm::Error::::CannotWithdraw)?; + .map_err(|err| { + log::error!( + target: "runtime::on_reap_identity", + "withdraw_asset(what: {:?}, who_origin: {:?}) error: {:?}", + wnd, who_origin, err + ); + pallet_xcm::Error::::CannotWithdraw + })?; // check out xcm_config::LocalAssetTransactor::can_check_out( From a2a041ffe1451ddb5e815f274bb1f9b056483b62 Mon Sep 17 00:00:00 2001 From: joepetrowski Date: Thu, 30 Nov 2023 07:00:06 +0100 Subject: [PATCH 4/6] log error messages --- polkadot/runtime/rococo/src/impls.rs | 9 ++++++++- polkadot/runtime/westend/src/impls.rs | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index 255cb7d3f440..a4f2d3c824bc 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -120,7 +120,14 @@ where // not used in AssetTransactor &XcmContext { origin: None, message_id: [0; 32], topic: None }, ) - .map_err(|_| pallet_xcm::Error::::CannotCheckOutTeleport)?; + .map_err(|err| { + log::error!( + target: "runtime::on_reap_identity", + "can_check_out(destination: {:?}, asset: {:?}, _) error: {:?}", + destination, roc, err + ); + pallet_xcm::Error::::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 369161d13313..0e13f69b2ec4 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -120,7 +120,14 @@ where // not used in AssetTransactor &XcmContext { origin: None, message_id: [0; 32], topic: None }, ) - .map_err(|_| pallet_xcm::Error::::CannotCheckOutTeleport)?; + .map_err(|err| { + log::error!( + target: "runtime::on_reap_identity", + "can_check_out(destination: {:?}, asset: {:?}, _) error: {:?}", + destination, wnd, err + ); + pallet_xcm::Error::::CannotCheckOutTeleport + })?; xcm_config::LocalAssetTransactor::check_out( &destination, &wnd, From d238c2126e2d7aaafd6cf4fb12e023ad4cbf4785 Mon Sep 17 00:00:00 2001 From: joepetrowski Date: Thu, 30 Nov 2023 11:53:18 +0100 Subject: [PATCH 5/6] remove new error variant --- polkadot/runtime/rococo/src/impls.rs | 2 +- polkadot/runtime/westend/src/impls.rs | 2 +- polkadot/xcm/pallet-xcm/src/lib.rs | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index a4f2d3c824bc..e8fbe4a1b454 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -110,7 +110,7 @@ where "withdraw_asset(what: {:?}, who_origin: {:?}) error: {:?}", roc, who_origin, err ); - pallet_xcm::Error::::CannotWithdraw + pallet_xcm::Error::::LowBalance })?; // check out diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs index 0e13f69b2ec4..62d87999af76 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -110,7 +110,7 @@ where "withdraw_asset(what: {:?}, who_origin: {:?}) error: {:?}", wnd, who_origin, err ); - pallet_xcm::Error::::CannotWithdraw + pallet_xcm::Error::::LowBalance })?; // check out diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 4cf6cf10fe6e..74a24b132da7 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -549,8 +549,6 @@ pub mod pallet { TooManyReserves, /// Local XCM execution incomplete. LocalExecutionIncomplete, - /// The amount of the asset requested could not be withdrawn from the origin. - CannotWithdraw, } impl From for Error { From c4b797011b0dbd7d99677825df8a26c5797f65b9 Mon Sep 17 00:00:00 2001 From: joepetrowski Date: Thu, 30 Nov 2023 12:09:00 +0100 Subject: [PATCH 6/6] add comment about transactional context --- polkadot/runtime/rococo/src/impls.rs | 2 ++ polkadot/runtime/westend/src/impls.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index e8fbe4a1b454..eddbfacc3b1d 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -79,6 +79,8 @@ impl ToParachainIdentityReaper { } } +// Note / Warning: This implementation should only be used in a transactional context. If not, then +// an error could result in assets being burned. impl OnReapIdentity for ToParachainIdentityReaper where Runtime: frame_system::Config + pallet_xcm::Config, diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs index 62d87999af76..5f23bd373b13 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -79,6 +79,8 @@ impl ToParachainIdentityReaper { } } +// Note / Warning: This implementation should only be used in a transactional context. If not, then +// an error could result in assets being burned. impl OnReapIdentity for ToParachainIdentityReaper where Runtime: frame_system::Config + pallet_xcm::Config,