XCM coretime region transfers (#3455)

This PR introduces changes enabling the transfer of coretime regions via XCM. TL;DR: There are two primary issues that are resolved in this PR: 1. The `mint` and `burn` functions were not implemented for coretime regions. These operations are essential for moving assets to and from the XCM holding register. 2. The transfer of non-fungible assets through XCM was previously disallowed. This was due to incorrectly benchmarking non-fungible asset transfers via XCM, which led to assigning it a weight of `Weight::Max`, effectively preventing its execution. ### `mint_into` and `burn` implementation This PR addresses the issue with cross-chain transferring regions back to the Coretime chain. Remote reserve transfers are performed by withdrawing and depositing the asset to and from the holding registry. This requires the asset to support burning and minting functionality. This PR adds burning and minting; however, they work a bit differently than usual so that the associated region record is not lost when burning. Instead of removing all the data, burning will set the owner of the region to `None`, and when minting it back, it will set it to an actual value. So, when cross-chain transferring, withdrawing into the registry will remove the region from its original owner, and when depositing it from the registry, it will set its owner to another account This was originally implemented in this PR: #3455, however we decided to move all of it to this single PR (#3455 (comment)) ### Fixes made in this PR - Update the `XcmReserveTransferFilter` on coretime chain since it is meant as a reserve chain for coretime regions. - Update the XCM benchmark to use `AssetTransactor` instead of assuming `pallet-balances` for fungible transfers. - Update the XCM benchmark to properly measure weight consumption for nonfungible reserve asset transfers. ATM reserve transfers via the extrinsic do not work since the weight for it is set to `Weight::max()`. Closes: #865 --------- Co-authored-by: Branislav Kontur <[email protected]> Co-authored-by: Francisco Aguirre <[email protected]> Co-authored-by: Dónal Murray <[email protected]>
paritytech · Apr 17, 2024 · e6f3106 · e6f3106
1 parent 4be9f93
commit e6f3106
Show file tree

Hide file tree

Showing 16 changed files with 368 additions and 94 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs
@@ -109,6 +109,7 @@ pub type UncheckedExtrinsic =
 /// Migrations to apply on runtime upgrade.
 pub type Migrations = (
 	cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
+	pallet_broker::migration::MigrateV0ToV1<Runtime>,
 	// permanent
 	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
@@ -719,11 +720,20 @@ impl_runtime_apis! {
 
 			use pallet_xcm::benchmarking::Pallet as PalletXcmExtrinsicsBenchmark;
 			impl pallet_xcm::benchmarking::Config for Runtime {
-				type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper<
-					xcm_config::XcmConfig,
-					ExistentialDepositAsset,
-					xcm_config::PriceForParentDelivery,
-				>;
+				type DeliveryHelper = (
+					cumulus_primitives_utility::ToParentDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						xcm_config::PriceForParentDelivery,
+					>,
+					polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						PriceForSiblingParachainDelivery,
+						RandomParaId,
+						ParachainSystem,
+					>
+				);
 
 				fn reachable_dest() -> Option<Location> {
 					Some(Parent.into())
@@ -741,8 +751,21 @@ impl_runtime_apis! {
 				}
 
 				fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> {
-					// Reserve transfers are disabled
-					None
+					// Coretime chain can reserve transfer regions to some random parachain.
+
+					// Properties of a mock region:
+					let core = 0;
+					let begin = 0;
+					let end = 42;
+
+					let region_id = pallet_broker::Pallet::<Runtime>::issue(core, begin, end, None, None);
+					Some((
+						Asset {
+							fun: NonFungible(Index(region_id.into())),
+							id: AssetId(xcm_config::BrokerPalletLocation::get())
+						},
+						ParentThen(Parachain(RandomParaId::get().into()).into()).into(),
+					))
 				}
 
 				fn get_asset() -> Asset {
@@ -758,15 +781,25 @@ impl_runtime_apis! {
 					RocRelayLocation::get(),
 					ExistentialDeposit::get()
 				).into());
+				pub const RandomParaId: ParaId = ParaId::new(43211234);
 			}
 
 			impl pallet_xcm_benchmarks::Config for Runtime {
 				type XcmConfig = xcm_config::XcmConfig;
-				type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper<
-					xcm_config::XcmConfig,
-					ExistentialDepositAsset,
-					xcm_config::PriceForParentDelivery,
-				>;
+				type DeliveryHelper = (
+					cumulus_primitives_utility::ToParentDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						xcm_config::PriceForParentDelivery,
+					>,
+					polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						PriceForSiblingParachainDelivery,
+						RandomParaId,
+						ParachainSystem,
+					>
+				);
 				type AccountIdConverter = xcm_config::LocationToAccountId;
 				fn valid_destination() -> Result<Location, BenchmarkError> {
 					Ok(RocRelayLocation::get())

diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs
@@ -293,7 +293,7 @@ impl pallet_xcm::Config for Runtime {
 	type XcmExecuteFilter = Everything;
 	type XcmExecutor = XcmExecutor<XcmConfig>;
 	type XcmTeleportFilter = Everything;
-	type XcmReserveTransferFilter = Nothing; // This parachain is not meant as a reserve location.
+	type XcmReserveTransferFilter = Everything;
 	type Weigher = WeightInfoBounds<
 		crate::weights::xcm::CoretimeRococoXcmWeight<RuntimeCall>,
 		RuntimeCall,

diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs
@@ -109,6 +109,7 @@ pub type UncheckedExtrinsic =
 /// Migrations to apply on runtime upgrade.
 pub type Migrations = (
 	cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
+	pallet_broker::migration::MigrateV0ToV1<Runtime>,
 	// permanent
 	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
@@ -218,6 +219,7 @@ impl pallet_authorship::Config for Runtime {
 
 parameter_types! {
 	pub const ExistentialDeposit: Balance = EXISTENTIAL_DEPOSIT;
+	pub const RandomParaId: ParaId = ParaId::new(43211234);
 }
 
 impl pallet_balances::Config for Runtime {
@@ -710,11 +712,20 @@ impl_runtime_apis! {
 
 			use pallet_xcm::benchmarking::Pallet as PalletXcmExtrinsicsBenchmark;
 			impl pallet_xcm::benchmarking::Config for Runtime {
-				type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper<
-					xcm_config::XcmConfig,
-					ExistentialDepositAsset,
-					xcm_config::PriceForParentDelivery,
-				>;
+				type DeliveryHelper = (
+					cumulus_primitives_utility::ToParentDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						xcm_config::PriceForParentDelivery,
+					>,
+					polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						PriceForSiblingParachainDelivery,
+						RandomParaId,
+						ParachainSystem,
+					>
+				);
 
 				fn reachable_dest() -> Option<Location> {
 					Some(Parent.into())
@@ -732,8 +743,21 @@ impl_runtime_apis! {
 				}
 
 				fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> {
-					// Reserve transfers are disabled
-					None
+					// Coretime chain can reserve transfer regions to some random parachain.
+
+					// Properties of a mock region:
+					let core = 0;
+					let begin = 0;
+					let end = 42;
+
+					let region_id = pallet_broker::Pallet::<Runtime>::issue(core, begin, end, None, None);
+					Some((
+						Asset {
+							fun: NonFungible(Index(region_id.into())),
+							id: AssetId(xcm_config::BrokerPalletLocation::get())
+						},
+						ParentThen(Parachain(RandomParaId::get().into()).into()).into(),
+					))
 				}
 
 				fn get_asset() -> Asset {
@@ -753,11 +777,22 @@ impl_runtime_apis! {
 
 			impl pallet_xcm_benchmarks::Config for Runtime {
 				type XcmConfig = xcm_config::XcmConfig;
-				type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper<
-					xcm_config::XcmConfig,
-					ExistentialDepositAsset,
-					xcm_config::PriceForParentDelivery,
-				>;
+
+				type DeliveryHelper = (
+					cumulus_primitives_utility::ToParentDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						xcm_config::PriceForParentDelivery,
+					>,
+					polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper<
+						xcm_config::XcmConfig,
+						ExistentialDepositAsset,
+						PriceForSiblingParachainDelivery,
+						RandomParaId,
+						ParachainSystem,
+					>
+				);
+
 				type AccountIdConverter = xcm_config::LocationToAccountId;
 				fn valid_destination() -> Result<Location, BenchmarkError> {
 					Ok(TokenRelayLocation::get())

diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs
@@ -300,7 +300,7 @@ impl pallet_xcm::Config for Runtime {
 	type XcmExecuteFilter = Everything;
 	type XcmExecutor = XcmExecutor<XcmConfig>;
 	type XcmTeleportFilter = Everything;
-	type XcmReserveTransferFilter = Nothing; // This parachain is not meant as a reserve location.
+	type XcmReserveTransferFilter = Everything;
 	type Weigher = WeightInfoBounds<
 		crate::weights::xcm::CoretimeWestendXcmWeight<RuntimeCall>,
 		RuntimeCall,

diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs
@@ -18,10 +18,7 @@ use super::*;
 use bounded_collections::{ConstU32, WeakBoundedVec};
 use codec::Encode;
 use frame_benchmarking::{benchmarks, whitelisted_caller, BenchmarkError, BenchmarkResult};
-use frame_support::{
-	traits::fungible::{Inspect, Mutate},
-	weights::Weight,
-};
+use frame_support::{assert_ok, weights::Weight};
 use frame_system::RawOrigin;
 use sp_std::prelude::*;
 use xcm::{latest::prelude::*, v2};
@@ -90,11 +87,6 @@ pub trait Config: crate::Config {
 }
 
 benchmarks! {
-	where_clause {
-		where
-			T: pallet_balances::Config,
-			<T as pallet_balances::Config>::Balance: From<u128> + Into<u128>,
-	}
 	send {
 		let send_origin =
 			T::SendXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
@@ -129,11 +121,7 @@ benchmarks! {
 			BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)),
 		)?;
 
-		let transferred_amount = match &asset.fun {
-			Fungible(amount) => *amount,
-			_ => return Err(BenchmarkError::Stop("Benchmark asset not fungible")),
-		}.into();
-		let assets: Assets = asset.into();
+		let assets: Assets = asset.clone().into();
 
 		let caller: T::AccountId = whitelisted_caller();
 		let send_origin = RawOrigin::Signed(caller.clone());
@@ -150,35 +138,40 @@ benchmarks! {
 			FeeReason::ChargeFees,
 		);
 
-		// Actual balance (e.g. `ensure_successful_delivery` could drip delivery fees, ...)
-		let balance = <pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller);
-		// Add transferred_amount to origin
-		<pallet_balances::Pallet<T> as Mutate<_>>::mint_into(&caller, transferred_amount)?;
-		// verify initial balance
-		let balance = balance + transferred_amount;
-		assert_eq!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller), balance);
+		match &asset.fun {
+			Fungible(amount) => {
+				// Add transferred_amount to origin
+				<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::deposit_asset(
+					&Asset { fun: Fungible(*amount), id: asset.id },
+					&origin_location,
+					None,
+				).map_err(|error| {
+				  log::error!("Fungible asset couldn't be deposited, error: {:?}", error);
+				  BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))
+				})?;
+			},
+			NonFungible(instance) => {
+				<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::deposit_asset(&asset, &origin_location, None)
+					.map_err(|error| {
+						log::error!("Nonfungible asset couldn't be deposited, error: {:?}", error);
+						BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))
+					})?;
+			}
+		};
 
 		let recipient = [0u8; 32];
 		let versioned_dest: VersionedLocation = destination.into();
 		let versioned_beneficiary: VersionedLocation =
 			AccountId32 { network: None, id: recipient.into() }.into();
 		let versioned_assets: VersionedAssets = assets.into();
 	}: _<RuntimeOrigin<T>>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0)
-	verify {
-		// verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees)
-		assert!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller) <= balance - transferred_amount);
-	}
 
 	reserve_transfer_assets {
 		let (asset, destination) = T::reserve_transferable_asset_and_dest().ok_or(
 			BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)),
 		)?;
 
-		let transferred_amount = match &asset.fun {
-			Fungible(amount) => *amount,
-			_ => return Err(BenchmarkError::Stop("Benchmark asset not fungible")),
-		}.into();
-		let assets: Assets = asset.into();
+		let assets: Assets = asset.clone().into();
 
 		let caller: T::AccountId = whitelisted_caller();
 		let send_origin = RawOrigin::Signed(caller.clone());
@@ -195,23 +188,50 @@ benchmarks! {
 			FeeReason::ChargeFees,
 		);
 
-		// Actual balance (e.g. `ensure_successful_delivery` could drip delivery fees, ...)
-		let balance = <pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller);
-		// Add transferred_amount to origin
-		<pallet_balances::Pallet<T> as Mutate<_>>::mint_into(&caller, transferred_amount)?;
-		// verify initial balance
-		let balance = balance + transferred_amount;
-		assert_eq!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller), balance);
+		match &asset.fun {
+			Fungible(amount) => {
+				// Add transferred_amount to origin
+				<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::deposit_asset(
+					&Asset { fun: Fungible(*amount), id: asset.id.clone() },
+					&origin_location,
+					None,
+				).map_err(|error| {
+				  log::error!("Fungible asset couldn't be deposited, error: {:?}", error);
+				  BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))
+				})?;
+			},
+			NonFungible(instance) => {
+				<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::deposit_asset(&asset, &origin_location, None)
+					.map_err(|error| {
+						log::error!("Nonfungible asset couldn't be deposited, error: {:?}", error);
+						BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))
+					})?;
+			}
+		};
 
 		let recipient = [0u8; 32];
-		let versioned_dest: VersionedLocation = destination.into();
+		let versioned_dest: VersionedLocation = destination.clone().into();
 		let versioned_beneficiary: VersionedLocation =
 			AccountId32 { network: None, id: recipient.into() }.into();
 		let versioned_assets: VersionedAssets = assets.into();
 	}: _<RuntimeOrigin<T>>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0)
 	verify {
-		// verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees)
-		assert!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller) <= balance - transferred_amount);
+		match &asset.fun {
+			Fungible(amount) => {
+				assert_ok!(<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::withdraw_asset(
+					&Asset { fun: Fungible(*amount), id: asset.id },
+					&destination,
+					None,
+				));
+			},
+			NonFungible(instance) => {
+				assert_ok!(<T::XcmExecutor as XcmAssetTransfers>::AssetTransactor::withdraw_asset(
+					&asset,
+					&destination,
+					None,
+				));
+			}
+		};
 	}
 
 	transfer_assets {

diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc
@@ -0,0 +1,28 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Region reserve transfers fix
+
+doc:
+  - audience: Runtime User
+    description: |
+      This PR introduces changes enabling the transfer of coretime regions via XCM.
+      There are two primary issues that are resolved in this PR:
+      1. The mint and burn functions were not implemented for coretime regions. These operations
+        are essential for moving assets to and from the XCM holding register.
+      2. The transfer of non-fungible assets through XCM was previously disallowed. This was due 
+        to incorrectly benchmarking non-fungible asset transfers via XCM, which led to assigning 
+        it a weight of Weight::Max, effectively preventing its execution.
+
+migrations: 
+  db: []
+  runtime: 
+    - reference: pallet-broker
+      description: |
+        The region owner is optional.
+
+crates: 
+  - name: pallet-broker
+  - name: pallet-xcm
+  - name: coretime-rococo-runtime
+  - name: coretime-westend-runtime