pallet-xcm: enhance `reserve_transfer_assets` to support all types of reserves

[acatangiu]

Context

When reserve transferring asset A from chain X to chain Y, the reserve location for A can fall in one of three scenarios:

1. LocalReserve: reserve is on source chain X (Here),
2. DestinationReserve: reserve is on target/destination chain Y,
3. ThirdPartyReserve: reserve is on third-party chain Z.

Problem

Currently, pallet-xcm::reserve_transfer_assets() extrinsic only covers scenario 1 above - even worse, it automatically assumes it is being called under scenario 1, without doing all the proper checks.

This is very limiting in terms of reserve-based transfers supported by pallet-xcm.

Solution proposal

Enhance pallet-xcm::reserve_transfer_assets() with support for all scenarios above - it should figure out by itself which scenario it is operating under based on input parameters (ideally without changing function/call API).

Once it figures out which scenario it is operating under, implement the following:

LocalReserve

Reserve is local chain reserve_chain == source_chain.

Same as current implementation:

let context = T::UniversalLocation::get();
let fees = assets.get(fee_asset_item).reanchored(&dest, context);
Xcm(vec![TransferReserveAsset {
    assets,
    dest,
    xcm: Xcm(vec![
        BuyExecution { fees, weight_limit },
        DepositAsset { assets: Wild(AllCounted(max_assets)), beneficiary }
    ]),
}])

DestinationReserve

Implementation when reserve_chain == destination_chain:

let context = T::UniversalLocation::get();
let fees = assets.get(fee_asset_item).reanchored(&dest, context);
Xcm(vec![
    WithdrawAsset(assets),
    InitiateReserveWithdraw {
        assets: Wild(AllCounted(max_assets)),
        reserve: dest,
        xcm: Xcm(vec![
            BuyExecution { fees, weight_limit },
            DepositAsset { assets: Wild(AllCounted(max_assets)), beneficiary }
        ]),
    },
])

ThirdPartyReserve

Implementation when reserve_chain != source_chain && reserve_chain != destination_chain:

let context = T::UniversalLocation::get();
let reserve_fees = assets.get(fee_asset_item).reanchored(&reserve, context);
let dest_fees = assets.get(fee_asset_item).reanchored(&dest, context);
let dest = dest.reanchored(&reserve, context);
Xcm(vec![
    WithdrawAsset(assets),
    InitiateReserveWithdraw {
        assets: Wild(AllCounted(max_assets)),
        reserve,
        xcm: Xcm(vec![
            BuyExecution { fees: reserve_fees, weight_limit },
            DepositReserveAsset {
                assets: Wild(AllCounted(max_assets)),
                dest,
                xcm: Xcm(vec![
                    BuyExecution { fees: dest_fees, weight_limit },
                    DepositAsset { assets: Wild(AllCounted(max_assets)), beneficiary }
                ]),
            },
        ]),
    },
])

[acatangiu]

Looking at the xtokens implementation, I realize for ThirdPartyReserve scenario we need to send XCM programs described above twice if transferred asset(s) reserve is different than fee-asset reserve, once to the reserve1 then dest, second to fee-reserve then dest.

[acatangiu]

Will actually try to make it more general-purpose and send as many XCM programs as required to as many reserves as are required to transfer all assets.

[acatangiu]

Some current (possibly dumb) questions around how teleports and reserve-based transfers work together:

Both native-asset and foreign-assets are allowed to be teleported to AssetHub, where the assets are defined as (from AssetHub context):

• NativeAsset: AssetId: Concrete(Multilocation { parents: 1, interior: Junctions::Here})
• ForeignAssetPara42 (FAP42): AssetId: Concrete(Multilocation { parents: 1, interior: X1(Parachain(42))})

Let's look at the more generic FAP42 example:

1. Alice on Para42 teleports 100 FAP42 to AssetHub,
2. Alice later wants to reserve-transfer FAP42 from AssetHub to Para84
3. FAP42's reserve is Para42 so we're in "ThirdPartyReserve" scenario, with InitiateReserveWithdraw being executed locally with reserve set to Para42,
   • which will "burn" assets here and send vec![WithdrawAsset(assets), ClearOrigin] to Para42 with AH as origin,
   • on Para42, WithdrawAsset will fail because FAP42 balance of SA-of-AH-on-Para42 is zero, FAP42s have been teleported to AH, not reserve-transferred.

So it becomes obvious that pallet-xcm needs to know which assets are teleportable-where and which have to be reserve-based, and reserve_transfer_assets() has to use either InitiateTeleport or InitiateReserveWithdraw accordingly

• Q1: is the above correct/sane?

The existing IsTeleporter is configured in xcm_executor::Config but not available to pallet-xcm.

• Q2: should I just make it a config option in pallet_xcm as well, or re-export it from executor to pallet-xcm? smth like:

• type XcmExecutor: ExecuteXcm<::RuntimeCall>;
• type XcmExecutor: ExecuteXcm<::RuntimeCall> + AssetFilters;

Note: pallet-xcm already has XcmTeleportFilter and XcmReserveTransferFilter associated types, but afaict these filter what assets originis allowed to operate on. We need a filter to say for asset and target combo, do we teleport or do we reserve-transfer?

[acatangiu]

Transfers will check IsReserve/IsTeleport to reason whether to teleport or reserve-transfer the asset to the destination.

[acatangiu]

Transfers will check IsReserve/IsTeleport to reason whether to teleport or reserve-transfer the asset to the destination.

Proposed logic:

if IsTeleporter::contains(asset, dest) {
    // we trust destination for teleporting asset
    return TransferType::Teleport
} else if IsReserve::contains(asset, dest) {
    // we trust destination as asset reserve location
    return TransferType::DestinationReserve
}

// try to determine reserve location based on asset id/location
let asset_location = match asset.id {
    Concrete(location) => location.chain_location(),
    _ => fail!(),
};
if asset_location == MultiLocation::here() || IsTeleporter::contains(asset, &asset_location) {
    // local asset, or remote location that allows local teleports => local reserve
    TransferType::LocalReserve
} else if IsReserve::contains(asset, &asset_location) {
    // remote location that is recognized as reserve location for asset
    TransferType::RemoteReserve(asset_location)
} else {
    // remote location that is not configured either as teleporter or reserve => cannot
    // determine asset reserve
    Error::<T>::UnknownReserve
}

[bkontur]

We need a filter to say for asset and target combo, do we teleport or do we reserve-transfer?

In the very first version of asset transfer with custom extrinsic, exactly for this I had trait ResolveAssetTransferKind:

pub trait ResolveAssetTransferKind {
    fn resolve(asset: &MultiAsset, target_location: &MultiLocation) -> AssetTransferKind;
}

which was part of pallet's Config:

/// Transfer kind resolver for `asset` to `target_location`.
type AssetTransferKindResolver: ResolveAssetTransferKind;

My basic idea was to move IFs like IsReserve::contains or IsTeleporter::contains out of extrinsic implementation to be more generic and also to give possibility to create runtime dependent logic for resolving supported transfer kinds or maybe disable all like type AssetTransferKindResolver = () or type AssetTransferKindResolver = Nothing. And here was some basic implementation of ResolveAssetTransferKind.

[bkontur]

The existing IsTeleporter is configured in xcm_executor::Config but not available to pallet-xcm.

type XcmExecutor: ExecuteXcm<::RuntimeCall> + AssetFilters;

So this trait ResolveAssetTransferKind is just an idea how to separate Proposed logic above out of pallet_xcm and do not make it ExecuteXcm-dependent. Also with this solution, we dont need IsTeleporter / IsReserve in pallet_xcm. And probably generic pallet_xcm should not do unnecessary assumptions about type XcmExecutor: ExecuteXcm.

For example, lets say that any other team is working on their own implementation of ExecuteXcm and they dont need reserve-transfer but they need pallet_xcm, so with + AssetFilters we will enforce them to implement dummy AssetFilters. So maybe something like type AssetTransferKindResolver = () is better for them.

[acatangiu]

The existing IsTeleporter is configured in xcm_executor::Config but not available to pallet-xcm.

type XcmExecutor: ExecuteXcm<::RuntimeCall> + AssetFilters;

So this trait ResolveAssetTransferKind is just an idea how to separate Proposed logic above out of pallet_xcm and do not make it ExecuteXcm-dependent. Also with this solution, we dont need IsTeleporter / IsReserve in pallet_xcm. And probably generic pallet_xcm should not do unnecessary assumptions about type XcmExecutor: ExecuteXcm.

For example, lets say that any other team is working on their own implementation of ExecuteXcm and they dont need reserve-transfer but they need pallet_xcm, so with + AssetFilters we will enforce them to implement dummy AssetFilters. So maybe something like type AssetTransferKindResolver = () is better for them.

I thought about this, but ultimately decided against it because moving it to runtime configuration means basically duplicating IsTeleporter and IsTrusted at the runtime xcm_config level -> duplicated code + chances for the two (xcm-executor::config & pallet-xcm::config) to become out of sync.

The used trait AssetTransferFilter is implemented on XcmExecutor. Using any custom executor allows you to also implement custom AssetTransferFilter so if your executor doesn't care about transfers, then AssetTransferFilter = ().

But this way, all runtimes that are using XcmExecutor, automatically get AssetTransferFilter which is already configured to match the executor's capabilities (config/knowledge of teleporters and reserves).

[acatangiu]

We could move this transfer type resolver from pallet-xcm into xcm-executor and expose just resolve_transfer_type(asset, dest) through executor trait AssetTransferFilter impl.

Either way, I still believe this should be tied to the executor - it ultimately holds the logic for handling all transfer-related XCM instructions and already uses IsTeleport and IsReserve to reason about what works how. So it makes sense to expose this stuff, make it clear what the executor's requirements and capabilities are instead of leaving it to runtime developers to manually try to match them when configuring XCM transfers pallets (like pallet-xcm).