Fix DepositReserveAsset fees payment (#3340)

The `fee` should be calculated with the reanchored asset, otherwise it
could lead to a failure where the set aside fee ends up not being
enough.

@acatangiu

---------

Co-authored-by: Adrian Catangiu <adrian@parity.io>

Cargo.lock

@@ -22323,10 +22323,12 @@ dependencies = [
  "pallet-transaction-payment",
  "pallet-xcm",
  "parity-scale-codec",
+ "polkadot-service",
  "polkadot-test-client",
  "polkadot-test-runtime",
  "polkadot-test-service",
  "sp-consensus",
+ "sp-core",
  "sp-keyring",
  "sp-runtime",
  "sp-state-machine",

polkadot/node/test/service/src/chain_spec.rs

@@ -169,6 +169,7 @@ fn polkadot_testnet_genesis(
 				paras_availability_period: 4,
 				no_show_slots: 10,
 				minimum_validation_upgrade_delay: 5,
+				max_downward_message_size: 1024,
 				..Default::default()
 			},
 		}

polkadot/runtime/test-runtime/src/xcm_config.rs

@@ -16,14 +16,16 @@
 
 use frame_support::{
 	parameter_types,
-	traits::{Everything, Nothing},
+	traits::{Everything, Get, Nothing},
 	weights::Weight,
 };
 use frame_system::EnsureRoot;
+use polkadot_runtime_parachains::FeeTracker;
+use runtime_common::xcm_sender::{ChildParachainRouter, PriceForMessageDelivery};
 use xcm::latest::prelude::*;
 use xcm_builder::{
 	AllowUnpaidExecutionFrom, EnsureXcmOrigin, FixedWeightBounds, FrameTransactionalProcessor,
-	SignedAccountId32AsNative, SignedToAccountId32,
+	SignedAccountId32AsNative, SignedToAccountId32, WithUniqueTopic,
 };
 use xcm_executor::{
 	traits::{TransactAsset, WeightTrader},
@@ -36,6 +38,8 @@ parameter_types! {
 	pub const MaxInstructions: u32 = 100;
 	pub const MaxAssetsIntoHolding: u32 = 16;
 	pub const UniversalLocation: xcm::latest::InteriorLocation = xcm::latest::Junctions::Here;
+	pub TokenLocation: Location = Here.into_location();
+	pub FeeAssetId: AssetId = AssetId(TokenLocation::get());
 }
 
 /// Type to convert an `Origin` type value into a `Location` value which represents an interior
@@ -45,17 +49,43 @@ pub type LocalOriginToLocation = (
 	SignedToAccountId32<crate::RuntimeOrigin, crate::AccountId, AnyNetwork>,
 );
 
-pub struct DoNothingRouter;
-impl SendXcm for DoNothingRouter {
-	type Ticket = ();
-	fn validate(_dest: &mut Option<Location>, _msg: &mut Option<Xcm<()>>) -> SendResult<()> {
-		Ok(((), Assets::new()))
-	}
-	fn deliver(_: ()) -> Result<XcmHash, SendError> {
-		Ok([0; 32])
+/// Implementation of [`PriceForMessageDelivery`], returning a different price
+/// based on whether a message contains a reanchored asset or not.
+/// This implementation ensures that messages with non-reanchored assets return higher
+/// prices than messages with reanchored assets.
+/// Useful for `deposit_reserve_asset_works_for_any_xcm_sender` integration test.
+pub struct TestDeliveryPrice<A, F>(sp_std::marker::PhantomData<(A, F)>);
+impl<A: Get<AssetId>, F: FeeTracker> PriceForMessageDelivery for TestDeliveryPrice<A, F> {
+	type Id = F::Id;
+
+	fn price_for_delivery(_: Self::Id, msg: &Xcm<()>) -> Assets {
+		let base_fee: super::Balance = 1_000_000;
+
+		let parents = msg.iter().find_map(|xcm| match xcm {
+			ReserveAssetDeposited(assets) => {
+				let AssetId(location) = &assets.inner().first().unwrap().id;
+				Some(location.parents)
+			},
+			_ => None,
+		});
+
+		// If no asset is found, price defaults to `base_fee`.
+		let amount = base_fee
+			.saturating_add(base_fee.saturating_mul(parents.unwrap_or(0) as super::Balance));
+
+		(A::get(), amount).into()
 	}
 }
 
+pub type PriceForChildParachainDelivery = TestDeliveryPrice<FeeAssetId, super::Dmp>;
+
+/// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our
+/// individual routers.
+pub type XcmRouter = WithUniqueTopic<
+	// Only one router so far - use DMP to communicate with child parachains.
+	ChildParachainRouter<super::Runtime, super::Xcm, PriceForChildParachainDelivery>,
+>;
+
 pub type Barrier = AllowUnpaidExecutionFrom<Everything>;
 
 pub struct DummyAssetTransactor;
@@ -99,7 +129,7 @@ type OriginConverter = (
 pub struct XcmConfig;
 impl xcm_executor::Config for XcmConfig {
 	type RuntimeCall = super::RuntimeCall;
-	type XcmSender = DoNothingRouter;
+	type XcmSender = XcmRouter;
 	type AssetTransactor = DummyAssetTransactor;
 	type OriginConverter = OriginConverter;
 	type IsReserve = ();
@@ -133,7 +163,7 @@ impl pallet_xcm::Config for crate::Runtime {
 	type UniversalLocation = UniversalLocation;
 	type SendXcmOrigin = EnsureXcmOrigin<crate::RuntimeOrigin, LocalOriginToLocation>;
 	type Weigher = FixedWeightBounds<BaseXcmWeight, crate::RuntimeCall, MaxInstructions>;
-	type XcmRouter = DoNothingRouter;
+	type XcmRouter = XcmRouter;
 	type XcmExecuteFilter = Everything;
 	type XcmExecutor = xcm_executor::XcmExecutor<XcmConfig>;
 	type XcmTeleportFilter = Everything;

polkadot/xcm/xcm-executor/integration-tests/Cargo.toml

@@ -20,6 +20,7 @@ pallet-xcm = { path = "../../pallet-xcm" }
 polkadot-test-client = { path = "../../../node/test/client" }
 polkadot-test-runtime = { path = "../../../runtime/test-runtime" }
 polkadot-test-service = { path = "../../../node/test/service" }
+polkadot-service = { path = "../../../node/service" }
 sp-consensus = { path = "../../../../substrate/primitives/consensus/common" }
 sp-keyring = { path = "../../../../substrate/primitives/keyring" }
 sp-runtime = { path = "../../../../substrate/primitives/runtime", default-features = false }
@@ -27,6 +28,7 @@ sp-state-machine = { path = "../../../../substrate/primitives/state-machine" }
 xcm = { package = "staging-xcm", path = "../..", default-features = false }
 xcm-executor = { package = "staging-xcm-executor", path = ".." }
 sp-tracing = { path = "../../../../substrate/primitives/tracing" }
+sp-core = { path = "../../../../substrate/primitives/core" }
 
 [features]
 default = ["std"]

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

@@ -19,12 +19,14 @@
 
 use codec::Encode;
 use frame_support::{dispatch::GetDispatchInfo, weights::Weight};
+use polkadot_service::chain_spec::get_account_id_from_seed;
 use polkadot_test_client::{
 	BlockBuilderExt, ClientBlockImportExt, DefaultTestClientBuilderExt, InitPolkadotBlockBuilder,
 	TestClientBuilder, TestClientBuilderExt,
 };
 use polkadot_test_runtime::{pallet_test_notifier, xcm_config::XcmConfig};
 use polkadot_test_service::construct_extrinsic;
+use sp_core::sr25519;
 use sp_runtime::traits::Block;
 use sp_state_machine::InspectState;
 use xcm::{latest::prelude::*, VersionedResponse, VersionedXcm};
@@ -323,3 +325,84 @@ fn query_response_elicits_handler() {
 		)));
 	});
 }
+
+/// Simulates a cross-chain message from Parachain to Parachain through Relay Chain
+/// that deposits assets into the reserve of the destination.
+/// Regression test for `DepostiReserveAsset` changes in
+/// <https://github.com/paritytech/polkadot-sdk/pull/3340>
+#[test]
+fn deposit_reserve_asset_works_for_any_xcm_sender() {
+	sp_tracing::try_init_simple();
+	let mut client = TestClientBuilder::new().build();
+
+	// Init values for the simulated origin Parachain
+	let amount_to_send: u128 = 1_000_000_000_000;
+	let assets: Assets = (Parent, amount_to_send).into();
+	let fee_asset_item = 0;
+	let max_assets = assets.len() as u32;
+	let fees = assets.get(fee_asset_item as usize).unwrap().clone();
+	let weight_limit = Unlimited;
+	let reserve = Location::parent();
+	let dest = Location::new(1, [Parachain(2000)]);
+	let beneficiary_id = get_account_id_from_seed::<sr25519::Public>("Alice");
+	let beneficiary = Location::new(0, [AccountId32 { network: None, id: beneficiary_id.into() }]);
+
+	// spends up to half of fees for execution on reserve and other half for execution on
+	// destination
+	let fee1 = amount_to_send.saturating_div(2);
+	let fee2 = amount_to_send.saturating_sub(fee1);
+	let fees_half_1 = Asset::from((fees.id.clone(), Fungible(fee1)));
+	let fees_half_2 = Asset::from((fees.id.clone(), Fungible(fee2)));
+
+	let reserve_context = <XcmConfig as xcm_executor::Config>::UniversalLocation::get();
+	// identifies fee item as seen by `reserve` - to be used at reserve chain
+	let reserve_fees = fees_half_1.reanchored(&reserve, &reserve_context).unwrap();
+	// identifies fee item as seen by `dest` - to be used at destination chain
+	let dest_fees = fees_half_2.reanchored(&dest, &reserve_context).unwrap();
+	// identifies assets as seen by `reserve` - to be used at reserve chain
+	let assets_reanchored = assets.reanchored(&reserve, &reserve_context).unwrap();
+	// identifies `dest` as seen by `reserve`
+	let dest = dest.reanchored(&reserve, &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 at reserve
+	let msg = Xcm(vec![
+		WithdrawAsset(assets_reanchored),
+		ClearOrigin,
+		BuyExecution { fees: reserve_fees, weight_limit },
+		DepositReserveAsset { assets: Wild(AllCounted(max_assets)), dest, xcm: xcm_on_dest },
+	]);
+
+	let mut block_builder = client.init_polkadot_block_builder();
+
+	// Simulate execution of an incoming XCM message at the reserve chain
+	let execute = construct_extrinsic(
+		&client,
+		polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
+			message: Box::new(VersionedXcm::from(msg)),
+			max_weight: Weight::from_parts(1_000_000_000, 1024 * 1024),
+		}),
+		sp_keyring::Sr25519Keyring::Alice,
+		0,
+	);
+
+	block_builder.push_polkadot_extrinsic(execute).expect("pushes extrinsic");
+
+	let block = block_builder.build().expect("Finalizes the block").block;
+	let block_hash = block.hash();
+
+	futures::executor::block_on(client.import(sp_consensus::BlockOrigin::Own, block))
+		.expect("imports the block");
+
+	client.state_at(block_hash).expect("state should exist").inspect_state(|| {
+		assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!(
+			r.event,
+			polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::Attempted {
+				outcome: Outcome::Complete { .. }
+			}),
+		)));
+	});
+}

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

@@ -826,9 +826,9 @@ impl<Config: config::Config> XcmExecutor<Config> {
 					// be weighed
 					let to_weigh = self.holding.saturating_take(assets.clone());
 					self.holding.subsume_assets(to_weigh.clone());
-
+					let to_weigh_reanchored = Self::reanchored(to_weigh, &dest, None);
 					let mut message_to_weigh =
-						vec![ReserveAssetDeposited(to_weigh.into()), ClearOrigin];
+						vec![ReserveAssetDeposited(to_weigh_reanchored), ClearOrigin];
 					message_to_weigh.extend(xcm.0.clone().into_iter());
 					let (_, fee) =
 						validate_send::<Config::XcmSender>(dest.clone(), Xcm(message_to_weigh))?;