Backport DepositReserveAsset fix

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/Cargo.toml

@@ -30,3 +30,4 @@ asset-hub-rococo-runtime = { version = "0.10.0", path = "../../../../../runtimes
 emulated-integration-tests-common = { path = "../../../common", default-features = false, version = "1.0.0" }
 penpal-runtime = { version = "0.12.0", path = "../../../../../runtimes/testing/penpal" }
 rococo-system-emulated-network = { version = "0.1.0", path = "../../../networks/rococo-system" }
+cumulus-pallet-parachain-system = { path = "../../../../../../pallets/parachain-system", default-features = false }

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs

@@ -35,7 +35,7 @@ pub use emulated_integration_tests_common::{
 	test_parachain_is_trusted_teleporter,
 	xcm_emulator::{
 		assert_expected_events, bx, helpers::weight_within_threshold, Chain, Parachain as Para,
-		RelayChain as Relay, Test, TestArgs, TestContext, TestExt,
+		RelayChain as Relay, Test, TestArgs, TestContext, TestExt, Parachain as ParachainTrait,
 	},
 	xcm_helpers::{xcm_transact_paid_execution, xcm_transact_unpaid_execution},
 	PROOF_SIZE_THRESHOLD, REF_TIME_THRESHOLD, XCM_V3,
@@ -53,6 +53,8 @@ pub use rococo_system_emulated_network::{
 	PenpalAParaReceiver as PenpalAReceiver, PenpalAParaSender as PenpalASender,
 	RococoRelay as Rococo, RococoRelayReceiver as RococoReceiver,
 	RococoRelaySender as RococoSender,
+	PenpalBParaReceiver as PenpalBReceiver, PenpalBParaSender as PenpalBSender,
+	PenpalBPara as PenpalB,
 };
 
 pub const ASSET_ID: u32 = 1;
@@ -65,6 +67,7 @@ pub type RelayToParaTest = Test<Rococo, PenpalA>;
 pub type SystemParaToRelayTest = Test<AssetHubRococo, Rococo>;
 pub type SystemParaToParaTest = Test<AssetHubRococo, PenpalA>;
 pub type ParaToSystemParaTest = Test<PenpalA, AssetHubRococo>;
+pub type ParaToParaTest = Test<PenpalA, PenpalB, Rococo>;
 
 /// Returns a `TestArgs` instance to be used for the Relay Chain across integration tests
 pub fn relay_test_args(

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/reserve_transfer.rs

@@ -14,6 +14,7 @@
 // limitations under the License.
 
 use crate::*;
+use frame_support::traits::Get;
 use asset_hub_rococo_runtime::xcm_config::XcmConfig as AssetHubRococoXcmConfig;
 use penpal_runtime::xcm_config::XcmConfig as PenpalRococoXcmConfig;
 use rococo_runtime::xcm_config::XcmConfig as RococoXcmConfig;
@@ -110,6 +111,66 @@ fn para_to_system_para_sender_assertions(t: ParaToSystemParaTest) {
 	);
 }
 
+fn para_to_para_sender_assertions(t: ParaToParaTest) {
+	type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
+	PenpalA::assert_xcm_pallet_attempted_complete(None);
+	assert_expected_events!(
+		PenpalA,
+		vec![
+			// Amount to reserve transfer is transferred to Parachain's Sovereign account
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Withdraw { who, amount }
+			) => {
+				who: *who == t.sender.account_id,
+				amount: *amount == t.args.amount,
+			},
+			// XCM sent to relay reserve
+			RuntimeEvent::ParachainSystem(
+				cumulus_pallet_parachain_system::Event::UpwardMessageSent { .. }
+			) => {},
+		]
+	);
+}
+
+fn para_to_para_relay_hop_assertions(t: ParaToParaTest) {
+	type RuntimeEvent = <Rococo as Chain>::RuntimeEvent;
+	let sov_penpal_a_on_rococo =
+		Rococo::sovereign_account_id_of(Rococo::child_location_of(PenpalA::para_id()));
+	let sov_penpal_b_on_rococo =
+		Rococo::sovereign_account_id_of(Rococo::child_location_of(PenpalB::para_id()));
+	assert_expected_events!(
+		Rococo,
+		vec![
+			// Withdrawn from sender parachain SA
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Withdraw { who, amount }
+			) => {
+				who: *who == sov_penpal_a_on_rococo,
+				amount: *amount == t.args.amount,
+			},
+			// Deposited to receiver parachain SA
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Deposit { who, .. }
+			) => {
+				who: *who == sov_penpal_b_on_rococo,
+			},
+			RuntimeEvent::MessageQueue(
+				pallet_message_queue::Event::Processed { success: true, .. }
+			) => {},
+		]
+	);
+}
+
+fn para_to_para_receiver_assertions(_: ParaToParaTest) {
+	type RuntimeEvent = <PenpalB as Chain>::RuntimeEvent;
+	assert_expected_events!(
+		PenpalB,
+		vec![
+			RuntimeEvent::Balances(pallet_balances::Event::Deposit { .. }) => {},
+		]
+	);
+}
+
 fn para_to_system_para_receiver_assertions(t: ParaToSystemParaTest) {
 	type RuntimeEvent = <AssetHubRococo as Chain>::RuntimeEvent;
 
@@ -175,6 +236,121 @@ fn system_para_to_para_assets_receiver_assertions<Test>(_: Test) {
 	);
 }
 
+// function assumes fees and assets have the same remote reserve
+fn remote_reserve_transfer_program(
+	reserve: MultiLocation,
+	dest: MultiLocation,
+	beneficiary: MultiLocation,
+	assets: Vec<MultiAsset>,
+	fees: MultiAsset,
+	weight_limit: WeightLimit,
+) -> Xcm<penpal_runtime::RuntimeCall> {
+	let max_assets = assets.len() as u32;
+	let context = X1(Parachain(<PenpalA as ParachainTrait>::ParachainInfo::get().into()));
+	// we spend up to half of fees for execution on reserve and other half for execution on
+	// destination
+	let (fees_half_1, fees_half_2) = match fees.fun {
+		Fungible(amount) => {
+			let fee1 = amount.saturating_div(2);
+			let fee2 = amount.saturating_sub(fee1);
+			assert!(fee1 > 0);
+			assert!(fee2 > 0);
+			(MultiAsset::from((fees.id.clone(), fee1)), MultiAsset::from((fees.id.clone(), fee2)))
+		},
+		NonFungible(_) => unreachable!(),
+	};
+	// identifies fee item as seen by `reserve` - to be used at reserve chain
+	let reserve_fees = fees_half_1.reanchored(&reserve, context).unwrap();
+	// identifies fee item as seen by `dest` - to be used at destination chain
+	let dest_fees = fees_half_2.reanchored(&dest, context).unwrap();
+	// identifies `dest` as seen by `reserve`
+	let dest = dest.reanchored(&reserve, context).unwrap();
+	// xcm to be executed at dest
+	let xcm_on_dest = Xcm(vec![
+		BuyExecution { fees: dest_fees, weight_limit: weight_limit.clone() },
+		DepositAsset { assets: Wild(AllCounted(max_assets)), beneficiary },
+	]);
+	// xcm to be executed on reserve
+	let xcm_on_reserve = Xcm(vec![
+		BuyExecution { fees: reserve_fees, weight_limit },
+		DepositReserveAsset { assets: Wild(AllCounted(max_assets)), dest, xcm: xcm_on_dest },
+	]);
+	Xcm(vec![
+		WithdrawAsset(assets.into()),
+		InitiateReserveWithdraw {
+			assets: Wild(AllCounted(max_assets)),
+			reserve,
+			xcm: xcm_on_reserve,
+		},
+	])
+}
+
+fn para_to_para_remote_reserve_transfer_native_assets(t: ParaToParaTest) -> DispatchResult {
+	let xcm = remote_reserve_transfer_program(
+		Parent.into(),
+		t.args.dest,
+		t.args.beneficiary,
+		t.args.assets.clone().into_inner(),
+		t.args.assets.into_inner().pop().unwrap(),
+		t.args.weight_limit,
+	);
+	<PenpalA as PenpalAPallet>::PolkadotXcm::execute(
+		t.signed_origin,
+		bx!(xcm::VersionedXcm::V3(xcm)),
+		Weight::MAX,
+	)
+	.unwrap();
+	Ok(())
+}
+
+/// Reserve Transfers of native asset from Parachain to Parachain (through Relay reserve) should
+/// work
+#[test]
+fn reserve_transfer_native_asset_from_para_to_para() {
+	// Init values for Penpal Parachain
+	let destination = PenpalA::sibling_location_of(PenpalB::para_id());
+	let beneficiary_id = PenpalBReceiver::get();
+	let amount_to_send: Balance = ASSET_HUB_ROCOCO_ED * 10000;
+	let assets = (Parent, amount_to_send).into();
+
+	let test_args = TestContext {
+		sender: PenpalASender::get(),
+		receiver: PenpalBReceiver::get(),
+		args: para_test_args(destination, beneficiary_id, amount_to_send, assets, None, 0),
+	};
+
+	let mut test = ParaToParaTest::new(test_args);
+
+	let sender_balance_before = test.sender.balance;
+	let receiver_balance_before = test.receiver.balance;
+
+	let sender_as_seen_by_relay = Rococo::child_location_of(PenpalA::para_id());
+	let sov_of_sender_on_relay = Rococo::sovereign_account_id_of(sender_as_seen_by_relay);
+
+	// fund the PenpalA's SA on Rococo with the native tokens held in reserve
+	Rococo::fund_accounts(vec![(sov_of_sender_on_relay.into(), amount_to_send * 2)]);
+
+	test.set_assertion::<PenpalA>(para_to_para_sender_assertions);
+	test.set_assertion::<Rococo>(para_to_para_relay_hop_assertions);
+	test.set_assertion::<PenpalB>(para_to_para_receiver_assertions);
+	test.set_dispatchable::<PenpalA>(para_to_para_remote_reserve_transfer_native_assets);
+	test.assert();
+
+	let sender_balance_after = test.sender.balance;
+	let receiver_balance_after = test.receiver.balance;
+
+	let delivery_fees = PenpalA::execute_with(|| {
+		xcm_helpers::transfer_assets_delivery_fees::<
+			<PenpalRococoXcmConfig as xcm_executor::Config>::XcmSender,
+		>(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest)
+	});
+
+	// Sender's balance is reduced
+	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
+	// Receiver's balance is increased
+	assert!(receiver_balance_after > receiver_balance_before);
+}
+
 fn relay_to_para_limited_reserve_transfer_assets(t: RelayToParaTest) -> DispatchResult {
 	<Rococo as RococoPallet>::XcmPallet::limited_reserve_transfer_assets(
 		t.signed_origin,

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs

@@ -155,34 +155,29 @@ match_types! {
 	};
 }
 
-pub type Barrier = TrailingSetTopicAsId<
-	DenyThenTry<
-		DenyReserveTransferToRelayChain,
+pub type Barrier = TrailingSetTopicAsId<(
+	TakeWeightCredit,
+	// Expected responses are OK.
+	AllowKnownQueryResponses<PolkadotXcm>,
+	// Allow XCMs with some computed origins to pass through.
+	WithComputedOrigin<
 		(
-			TakeWeightCredit,
-			// Expected responses are OK.
-			AllowKnownQueryResponses<PolkadotXcm>,
-			// Allow XCMs with some computed origins to pass through.
-			WithComputedOrigin<
-				(
-					// If the message is one that immediately attempts to pay for execution, then
-					// allow it.
-					AllowTopLevelPaidExecutionFrom<Everything>,
-					// System Assets parachain, parent and its exec plurality get free
-					// execution
-					AllowExplicitUnpaidExecutionFrom<(
-						CommonGoodAssetsParachain,
-						ParentOrParentsExecutivePlurality,
-					)>,
-					// Subscriptions for version tracking are OK.
-					AllowSubscriptionsFrom<Everything>,
-				),
-				UniversalLocation,
-				ConstU32<8>,
-			>,
+			// If the message is one that immediately attempts to pay for execution, then
+			// allow it.
+			AllowTopLevelPaidExecutionFrom<Everything>,
+			// System Assets parachain, parent and its exec plurality get free
+			// execution
+			AllowExplicitUnpaidExecutionFrom<(
+				CommonGoodAssetsParachain,
+				ParentOrParentsExecutivePlurality,
+			)>,
+			// Subscriptions for version tracking are OK.
+			AllowSubscriptionsFrom<Everything>,
 		),
+		UniversalLocation,
+		ConstU32<8>,
 	>,
->;
+)>;
 
 /// Type alias to conveniently refer to `frame_system`'s `Config::AccountId`.
 pub type AccountIdOf<R> = <R as frame_system::Config>::AccountId;
@@ -259,7 +254,7 @@ impl xcm_executor::Config for XcmConfig {
 	type OriginConverter = XcmOriginToTransactDispatchOrigin;
 	type IsReserve = Reserves;
 	// no teleport trust established with other chains
-	type IsTeleporter = NativeAsset;
+	type IsTeleporter = ();
 	type UniversalLocation = UniversalLocation;
 	type Barrier = Barrier;
 	type Weigher = FixedWeightBounds<UnitWeightCost, RuntimeCall, MaxInstructions>;
@@ -298,7 +293,7 @@ impl pallet_xcm::Config for Runtime {
 	type SendXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
 	type XcmRouter = XcmRouter;
 	type ExecuteXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
-	type XcmExecuteFilter = Nothing;
+	type XcmExecuteFilter = Everything;
 	// ^ Disable dispatchable execute on the XCM pallet.
 	// Needs to be `Everything` for local testing.
 	type XcmExecutor = XcmExecutor<XcmConfig>;

polkadot/xcm/xcm-executor/Cargo.toml

@@ -4,7 +4,7 @@ description = "An abstract and configurable XCM message executor."
 authors.workspace = true
 edition.workspace = true
 license.workspace = true
-version = "5.0.0"
+version = "5.0.1"
 
 [dependencies]
 impl-trait-for-tuples = "0.2.2"

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

@@ -630,6 +630,20 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				Ok(())
 			},
 			DepositReserveAsset { assets, dest, xcm } => {
+				// we need to do this take/put cycle to solve wildcards and get exact assets to be
+				// weighed
+				let to_weigh = self.holding.saturating_take(assets.clone());
+				self.holding.subsume_assets(to_weigh.clone());
+
+				let mut message_to_weigh =
+					vec![ReserveAssetDeposited(to_weigh.into()), ClearOrigin];
+				message_to_weigh.extend(xcm.0.clone().into_iter());
+				let (_, fee) =
+					validate_send::<Config::XcmSender>(dest.clone(), Xcm(message_to_weigh))?;
+				// set aside fee to be charged by XcmSender
+				let parked_fee = self.holding.saturating_take(fee.into());
+
+				// now take assets to deposit (excluding parked_fee)
 				let deposited = self.holding.saturating_take(assets);
 				for asset in deposited.assets_iter() {
 					Config::AssetTransactor::deposit_asset(&asset, &dest, Some(&self.context))?;
@@ -639,6 +653,9 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				let assets = Self::reanchored(deposited, &dest, None);
 				let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
+
+				// put back parked_fee in holding register to be charged by XcmSender
+				self.holding.subsume_assets(parked_fee);
 				self.send(dest, Xcm(message), FeeReason::DepositReserveAsset)?;
 				Ok(())
 			},