From aac5b1e364d29c33fb26d4422c29a83966729be2 Mon Sep 17 00:00:00 2001
From: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Date: Fri, 26 May 2023 16:41:10 -0300
Subject: [PATCH] XCM: Fix issue with RequestUnlock (#7278)
* XCM: Fix issue with RequestUnlock
* Leave API changes for v4
* Fix clippy errors
* Fix tests
---------
Co-authored-by: parity-processbot <>
---
polkadot/xcm/xcm-builder/src/tests/locking.rs | 8 +++--
polkadot/xcm/xcm-executor/src/lib.rs | 15 +++++++++-
polkadot/xcm/xcm-simulator/example/src/lib.rs | 26 +++++++++++++++--
.../xcm-simulator/example/src/parachain.rs | 29 +++++++++++++++----
4 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/polkadot/xcm/xcm-builder/src/tests/locking.rs b/polkadot/xcm/xcm-builder/src/tests/locking.rs
index 7c408c999b5..f4ef618ac0e 100644
--- a/polkadot/xcm/xcm-builder/src/tests/locking.rs
+++ b/polkadot/xcm/xcm-builder/src/tests/locking.rs
@@ -136,6 +136,8 @@ fn remote_unlock_roundtrip_should_work() {
set_send_price((Parent, 10u128));
// We have been told by Parachain #1 that Account #3 has locked funds which we can unlock.
+ // Previously, we must have sent a LockAsset instruction to Parachain #1.
+ // This caused Parachain #1 to send us the NoteUnlockable instruction.
let message =
Xcm(vec![NoteUnlockable { asset: (Parent, 100u128).into(), owner: (3u64,).into() }]);
let hash = fake_message_hash(&message);
@@ -169,8 +171,10 @@ fn remote_unlock_roundtrip_should_work() {
assert_eq!(r, Outcome::Complete(Weight::from_parts(40, 40)));
assert_eq!(asset_list((3u64,)), vec![(Parent, 990u128).into()]);
- let expected_msg =
- Xcm::<()>(vec![UnlockAsset { target: (3u64,).into(), asset: (Parent, 100u128).into() }]);
+ let expected_msg = Xcm::<()>(vec![UnlockAsset {
+ target: (Parent, Parachain(42), 3u64).into(),
+ asset: (Parent, 100u128).into(),
+ }]);
let expected_hash = fake_message_hash(&expected_msg);
assert_eq!(sent_xcm(), vec![((Parent, Parachain(1)).into(), expected_msg, expected_hash)]);
assert_eq!(
diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs
index ce9d3d4644e..16f53fd6503 100644
--- a/polkadot/xcm/xcm-executor/src/lib.rs
+++ b/polkadot/xcm/xcm-executor/src/lib.rs
@@ -876,9 +876,11 @@ impl<Config: config::Config> XcmExecutor<Config> {
RequestUnlock { asset, locker } => {
let origin = *self.origin_ref().ok_or(XcmError::BadOrigin)?;
let remote_asset = Self::try_reanchor(asset.clone(), &locker)?.0;
+ let remote_target = Self::try_reanchor_multilocation(origin, &locker)?.0;
let reduce_ticket =
Config::AssetLocker::prepare_reduce_unlockable(locker, asset, origin)?;
- let msg = Xcm::<()>(vec![UnlockAsset { asset: remote_asset, target: origin }]);
+ let msg =
+ Xcm::<()>(vec![UnlockAsset { asset: remote_asset, target: remote_target }]);
let (ticket, price) = validate_send::<Config::XcmSender>(locker, msg)?;
self.take_fee(price, FeeReason::RequestUnlock)?;
reduce_ticket.enact()?;
@@ -990,6 +992,17 @@ impl<Config: config::Config> XcmExecutor<Config> {
Ok((asset, reanchor_context))
}
+ fn try_reanchor_multilocation(
+ location: MultiLocation,
+ destination: &MultiLocation,
+ ) -> Result<(MultiLocation, InteriorMultiLocation), XcmError> {
+ let reanchor_context = Config::UniversalLocation::get();
+ let location = location
+ .reanchored(&destination, reanchor_context)
+ .map_err(|_| XcmError::ReanchorFailed)?;
+ Ok((location, reanchor_context))
+ }
+
/// NOTE: Any assets which were unable to be reanchored are introduced into `failed_bin`.
fn reanchored(
mut assets: Assets,
diff --git a/polkadot/xcm/xcm-simulator/example/src/lib.rs b/polkadot/xcm/xcm-simulator/example/src/lib.rs
index 33a5b2c70a9..bd5ebb0b472 100644
--- a/polkadot/xcm/xcm-simulator/example/src/lib.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/lib.rs
@@ -272,7 +272,7 @@ mod tests {
}
#[test]
- fn remote_locking() {
+ fn remote_locking_and_unlocking() {
MockNet::reset();
let locked_amount = 100;
@@ -280,7 +280,7 @@ mod tests {
ParaB::execute_with(|| {
let message = Xcm(vec![LockAsset {
asset: (Here, locked_amount).into(),
- unlocker: (Parachain(1),).into(),
+ unlocker: Parachain(1).into(),
}]);
assert_ok!(ParachainPalletXcm::send_xcm(Here, Parent, message.clone()));
});
@@ -306,6 +306,28 @@ mod tests {
}])]
);
});
+
+ ParaB::execute_with(|| {
+ // Request unlocking part of the funds on the relay chain
+ let message = Xcm(vec![RequestUnlock {
+ asset: (Parent, locked_amount - 50).into(),
+ locker: Parent.into(),
+ }]);
+ assert_ok!(ParachainPalletXcm::send_xcm(Here, (Parent, Parachain(1)), message));
+ });
+
+ Relay::execute_with(|| {
+ use pallet_balances::{BalanceLock, Reasons};
+ // Lock is reduced
+ assert_eq!(
+ relay_chain::Balances::locks(&child_account_id(2)),
+ vec![BalanceLock {
+ id: *b"py/xcmlk",
+ amount: locked_amount - 50,
+ reasons: Reasons::All
+ }]
+ );
+ });
}
/// Scenario:
diff --git a/polkadot/xcm/xcm-simulator/example/src/parachain.rs b/polkadot/xcm/xcm-simulator/example/src/parachain.rs
index cea5a93ec0a..39a2e27470b 100644
--- a/polkadot/xcm/xcm-simulator/example/src/parachain.rs
+++ b/polkadot/xcm/xcm-simulator/example/src/parachain.rs
@@ -17,9 +17,10 @@
//! Parachain runtime mock.
use codec::{Decode, Encode};
+use core::marker::PhantomData;
use frame_support::{
construct_runtime, parameter_types,
- traits::{EnsureOrigin, EnsureOriginWithArg, Everything, EverythingBut, Nothing},
+ traits::{ContainsPair, EnsureOrigin, EnsureOriginWithArg, Everything, EverythingBut, Nothing},
weights::{constants::WEIGHT_REF_TIME_PER_SECOND, Weight},
};
@@ -27,7 +28,7 @@ use frame_system::EnsureRoot;
use sp_core::{ConstU32, H256};
use sp_runtime::{
testing::Header,
- traits::{Hash, IdentityLookup},
+ traits::{Get, Hash, IdentityLookup},
AccountId32,
};
use sp_std::prelude::*;
@@ -238,7 +239,7 @@ impl Config for XcmConfig {
type Trader = FixedRateOfFungible<KsmPerSecondPerByte, ()>;
type ResponseHandler = ();
type AssetTrap = ();
- type AssetLocker = ();
+ type AssetLocker = PolkadotXcm;
type AssetExchanger = ();
type AssetClaims = ();
type SubscriptionService = ();
@@ -325,7 +326,7 @@ pub mod mock_msg_queue {
Ok(xcm) => {
let location = (Parent, Parachain(sender.into()));
match T::XcmExecutor::execute_xcm(location, xcm, message_hash, max_weight) {
- Outcome::Error(e) => (Err(e.clone()), Event::Fail(Some(hash), e)),
+ Outcome::Error(e) => (Err(e), Event::Fail(Some(hash), e)),
Outcome::Complete(w) => (Ok(w), Event::Success(Some(hash))),
// As far as the caller is concerned, this was dispatched without error, so
// we just report the weight used.
@@ -349,7 +350,7 @@ pub mod mock_msg_queue {
let _ = XcmpMessageFormat::decode(&mut data_ref)
.expect("Simulator encodes with versioned xcm format; qed");
- let mut remaining_fragments = &data_ref[..];
+ let mut remaining_fragments = data_ref;
while !remaining_fragments.is_empty() {
if let Ok(xcm) =
VersionedXcm::<T::RuntimeCall>::decode(&mut remaining_fragments)
@@ -403,6 +404,22 @@ parameter_types! {
pub ReachableDest: Option<MultiLocation> = Some(Parent.into());
}
+pub struct TrustedLockerCase<T>(PhantomData<T>);
+impl<T: Get<(MultiLocation, MultiAssetFilter)>> ContainsPair<MultiLocation, MultiAsset>
+ for TrustedLockerCase<T>
+{
+ fn contains(origin: &MultiLocation, asset: &MultiAsset) -> bool {
+ let (o, a) = T::get();
+ a.matches(asset) && &o == origin
+ }
+}
+
+parameter_types! {
+ pub RelayTokenForRelay: (MultiLocation, MultiAssetFilter) = (Parent.into(), Wild(AllOf { id: Concrete(Parent.into()), fun: WildFungible }));
+}
+
+pub type TrustedLockers = TrustedLockerCase<RelayTokenForRelay>;
+
impl pallet_xcm::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type SendXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
@@ -420,7 +437,7 @@ impl pallet_xcm::Config for Runtime {
type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion;
type Currency = Balances;
type CurrencyMatcher = ();
- type TrustedLockers = ();
+ type TrustedLockers = TrustedLockers;
type SovereignAccountOf = LocationToAccountId;
type MaxLockers = ConstU32<8>;
type MaxRemoteLockConsumers = ConstU32<0>;
--
GitLab