DOT transfers using pallet-xcm's `transfer_assets` after AHM

[mrshiposha]

While looking at the AHM migration guide and the corresponding tests, I didn't find a test for the case of DOT transfer between two non-reserve chains using pallet-xcm's transfer_assets extrinsic (which should deduce the reserve location automatically).

I see that the tests directly execute XCM programs. However, programs are usually created by extrinsics, such as transfer_assets.

From the current code, I conclude that after AHM, when transferring DOT from Parachain A to Parachain B using polkadotXcm.transferAssets, Parachain A won't be able to deduce the reserve location:

polkadot-sdk/polkadot/xcm/pallet-xcm/src/lib.rs

Lines 1990 to 1991 in 0447d26

	let transfer_type =
	T::XcmExecutor::determine_for(&asset, dest).map_err(Error::<T>::from)?;

polkadot-sdk/polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs

Lines 45 to 85 in 3515d4a

	pub trait XcmAssetTransfers {
	/// Combinations of (Asset, Location) pairs which we trust as reserves. Meaning
	/// reserve-based-transfers are to be used for assets matching this filter.
	type IsReserve: ContainsPair<Asset, Location>;
	/// Combinations of (Asset, Location) pairs which we trust as teleporters. Meaning teleports are
	/// to be used for assets matching this filter.
	type IsTeleporter: ContainsPair<Asset, Location>;
	/// How to withdraw and deposit an asset.
	type AssetTransactor: TransactAsset;
	/// Determine transfer type to be used for transferring `asset` from local chain to `dest`.
	fn determine_for(asset: &Asset, dest: &Location) -> Result<TransferType, Error> {
	if Self::IsTeleporter::contains(asset, dest) {
	// we trust destination for teleporting asset
	return Ok(TransferType::Teleport)
	} else if Self::IsReserve::contains(asset, dest) {
	// we trust destination as asset reserve location
	return Ok(TransferType::DestinationReserve)
	}
	// try to determine reserve location based on asset id/location
	let asset_location = asset.id.0.chain_location();
	if asset_location == Location::here() ||
	Self::IsTeleporter::contains(asset, &asset_location)
	{
	// if the asset is local, then it's a local reserve
	// it's also a local reserve if the asset's location is not `here` but it's a location
	// where it can be teleported to `here` => local reserve
	Ok(TransferType::LocalReserve)
	} else if Self::IsReserve::contains(asset, &asset_location) {
	// remote location that is recognized as reserve location for asset
	Ok(TransferType::RemoteReserve(asset_location.into()))
	} else {
	// remote location that is not configured either as teleporter or reserve => cannot
	// determine asset reserve
	Err(Error::UnknownReserve)
	}
	}
	}

Do we have a test for that?

If the problem does exist, a possible solution would be to have a ReserveProvider associated type in the XcmAssetTransfers trait, similar to the ORML pallet-xtokens:
https://github.com/open-web3-stack/open-runtime-module-library/blob/2fd6442fc9b03717e491e82b9d8754a863e41c9a/xtokens/src/lib.rs#L130-L132

CC @acatangiu @franciscoaguirre @xlc

[mrshiposha]

Parachain A won't be able to deduce the reserve location

For instance, transferring DOT from Parachain 2000 to Parachain 2001.
Assuming IsReserve is already expecting Asset Hub to be the reserve for DOT:

1. determine_for is called with asset = DOT, dest = Parachain 2001.
2. IsTeleporter::contains(asset, dest) is false, IsReserve::contains(asset, dest) is false.
3. asset_location variable will be assigned {parents: 1, interior: Here} since the DOT XCM ID doesn't have a "chain part".
4. IsTeleporter::contains(asset, asset_location) is false
5. IsReserve::contains(asset, asset_location) is also false since the Relay isn't the DOT's reserve location anymore
6. We got Err(Error::UnknownReserve).

[acatangiu]

Good find @mrshiposha! This is indeed a problem we need to fix. Unfortunately, relying on on-chain fix alone will not work in practice, as most likely not all parachains will upgrade to ingest the fix in time for AHM.

Currently exploring avenues for tackling this offchain as well.

[franciscoaguirre]

As discussed with @mrshiposha, reserve_transfer_assets and limited_reserve_transfer_assets also have this issue since they call the same find_fee_and_assets_transfer_types function.

The recommendation is to use transfer_assets_using_type_and_then or execute as per this document describing the issue in more depth: https://docs.google.com/document/d/1KbqUskGUhCVzx23KeAEHU-1AzBrUEDBW2IU5cSyVOMc/edit?usp=sharing

[mrshiposha]

Ooph, it seems not only the ParaA->ParaB case is affected.

AH->Para is also affected.

For instance, transferring DOT from AH to Parachain A.

1. determine_for is called with asset = DOT, dest = Parachain A.
2. IsTeleporter::contains(asset, dest) is false, IsReserve::contains(asset, dest) is false.
3. asset_location variable will be assigned {parents: 1, interior: Here} since the DOT XCM ID doesn't have a "chain part".
4. IsTeleporter::contains(asset, asset_location) is false
5. IsReserve::contains(asset, asset_location) is also false since the Relay isn't the DOT's reserve location anymore
6. We got Err(Error::UnknownReserve).

So, only Para->AH will work. The rest of the cases are broken (Para->Para, AH->Para).
Am I wrong?

If it is true, how would a user send DOT from AH to a different non-system parachain? What would they use? polkadotXcm.execute?

[mrshiposha]

BTW, for the Para->Para case, polkadotXcm.execute can't always be the answer. Some chains might restrain the ExecuteXcmOrigin in the pallet-xcm's config, so not everyone is even able to call it.

UPD: This is irrelevant; ExecuteXcmOrigin is used for transfer_assets and friends as well.

[mrshiposha]

@franciscoaguirre @acatangiu, please verify my points above. This looks bad.

[franciscoaguirre]

With this patch, we're actually disabling reserve transfers of DOT|KSM|PAS, so AH->Para was already included in that.

The first immediate alternative is transfer_assets_using_type_and_then, where you need to specify the reserve you want to use.

Another alternative which most chains aren't configured yet to use is execute.

The paraspell SDK uses execute if the chain supports it but transfer_assets_using_type_and_then otherwise.

[franciscoaguirre]

I just checked and AH->Para is NOT affected.

For instance, transferring DOT from AH to Parachain A.

1. `determine_for` is called with `asset = DOT`, `dest = Parachain A`.

2. `IsTeleporter::contains(asset, dest)` is false, `IsReserve::contains(asset, dest)` is false.

3. `asset_location` variable will be assigned `{parents: 1, interior: Here}` since the DOT XCM ID doesn't have a "chain part".

4. `IsTeleporter::contains(asset, asset_location)` is false

Point 4. is true since AH uses ConcreteAssetFromSystem<DotLocation> which returns true for the Relay.

However, remote reserves are still an issue.

The patch already in place for transfer_assets makes the call fail whenever DOT|KSM|PAS|WND and determining reserves is involved. The same patch should be put on limited_reserve_transfer_assets and reserve_transfer_assets to keep things consistent. Off-chain apps should use the alternatives and parachain runtimes should ingest the patches.