Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XCM: support multiple assets transfers while still enforcing single asset for BuyExecution #2423

Open
acatangiu opened this issue Nov 21, 2023 · 2 comments
Assignees
Labels
I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM.

Comments

@acatangiu
Copy link
Contributor

acatangiu commented Nov 21, 2023

Description

For security reasons AllowTopLevelPaidExecutionFrom barrier enforces that incoming XCM at least attempts to pay for its execution.

Cross-chain asset transfers are currently limited to MAX_ASSETS_FOR_BUY_EXECUTION = 2 because of this. For best security MAX_ASSETS_FOR_BUY_EXECUTION should actually be 1, while for usability it should be as high as possible (allow more than 1 or 2 assets in single transfer).

The underlying problem is that assets and fees are bundled together in same instructions so this MAX_ASSETS_FOR_BUY_EXECUTION limit is being applied to both.

We should fix this with some solution where MAX_ASSETS_FOR_BUY_EXECUTION only applies to fees which should be single asset, while XCM can still contain instructions for transferring multiple other assets.

Solution Proposal

Best fix (IMO) is to make sure cross-chain XCMs explicitly handle "fees asset" and "rest of assets" separately instead of bundling them together.
With a clear limit of max 1 asset allowed for buying execution, it then becomes clear to XCM builders that XCMs sent to be executed on remote chains need to do explicit:

Xcm(vec![
	ReceiveTeleportedAsset(fee)/ReserveAssetDeposited(fee)/WithdrawAsset(fee)/ClaimAsset(fee),
	BuyExecution(fee),
	<rest of the program here>,
])

This also means we need to do breaking changes to TransferReserveAsset, DepositReserveAsset, InitiateReserveWithdraw, InitiateTeleport so xcm-executor can build fully correct "onward XCMs" in the first place.
Maybe take extra Option<fees> parameter to be able to add BuyExecution before adding ClearOrigin and running the rest of custom xcm instructions.

@acatangiu
Copy link
Contributor Author

Related attempt to fix above, but at the XCM-routing level: #2394

@acatangiu acatangiu added T6-XCM This PR/Issue is related to XCM. I5-enhancement An additional feature request. labels Nov 21, 2023
@xlc
Copy link
Contributor

xlc commented Nov 21, 2023

Let's get this into XCM v4

@acatangiu acatangiu self-assigned this Mar 28, 2024
franciscoaguirre added a commit that referenced this issue Oct 23, 2024
A new instruction `InitiateTransfer` is introduced that initiates an
assets transfer from the chain it is executed on, to another chain. The
executed transfer is point-to-point (chain-to-chain) with all of the
transfer properties specified in the instruction parameters. The
instruction also allows specifying another XCM program to be executed on
the remote chain.
If a transfer requires going through multiple hops, an XCM program can
compose this instruction to be used at every chain along the path, on
each hop describing that specific leg of the transfer.

**Note:** Transferring assets that require different paths (chains along
the way) is _not supported within same XCM_ because of the async nature
of cross chain messages. This new instruction, however, enables
initiating transfers for multiple assets that take the same path even if
they require different transfer types along that path.

The usage and composition model of `InitiateTransfer` is the same as
with existing `DepositReserveAsset`, `InitiateReserveWithdraw` and
`InitiateTeleport` instructions. The main difference comes from the
ability to handle assets that have different point-to-point transfer
type between A and B. The other benefit is that it also allows
specifying remote fee payment and transparently appends the required
remote fees logic to the remote XCM.

We can specify the desired transfer type for some asset(s) using:

```rust
/// Specify which type of asset transfer is required for a particular `(asset, dest)` combination.
pub enum AssetTransferFilter {
	/// teleport assets matching `AssetFilter` to `dest`
	Teleport(AssetFilter),
	/// reserve-transfer assets matching `AssetFilter` to `dest`, using the local chain as reserve
	ReserveDeposit(AssetFilter),
	/// reserve-transfer assets matching `AssetFilter` to `dest`, using `dest` as reserve
	ReserveWithdraw(AssetFilter),
}
```

This PR adds 1 new XCM instruction:
```rust
/// Cross-chain transfer matching `assets` in the holding register as follows:
///
/// Assets in the holding register are matched using the given list of `AssetTransferFilter`s,
/// they are then transferred based on their specified transfer type:
///
/// - teleport: burn local assets and append a `ReceiveTeleportedAsset` XCM instruction to
///   the XCM program to be sent onward to the `dest` location,
///
/// - reserve deposit: place assets under the ownership of `dest` within this consensus system
///   (i.e. its sovereign account), and append a `ReserveAssetDeposited` XCM instruction
///   to the XCM program to be sent onward to the `dest` location,
///
/// - reserve withdraw: burn local assets and append a `WithdrawAsset` XCM instruction
///   to the XCM program to be sent onward to the `dest` location,
///
/// The onward XCM is then appended a `ClearOrigin` to allow safe execution of any following
/// custom XCM instructions provided in `remote_xcm`.
///
/// The onward XCM also potentially contains a `BuyExecution` instruction based on the presence
/// of the `remote_fees` parameter (see below).
///
/// If a transfer requires going through multiple hops, an XCM program can compose this instruction
/// to be used at every chain along the path, describing that specific leg of the transfer.
///
/// Parameters:
/// - `dest`: The location of the transfer next hop.
/// - `remote_fees`: If set to `Some(asset_xfer_filter)`, the single asset matching
///   `asset_xfer_filter` in the holding register will be transferred first in the remote XCM
///   program, followed by a `BuyExecution(fee)`, then rest of transfers follow.
///   This guarantees `remote_xcm` will successfully pass a `AllowTopLevelPaidExecutionFrom` barrier.
/// - `remote_xcm`: Custom instructions that will be executed on the `dest` chain. Note that
///   these instructions will be executed after a `ClearOrigin` so their origin will be `None`.
///
/// Safety: No concerns.
///
/// Kind: *Command*.
///
InitiateTransfer {
	destination: Location,
	remote_fees: Option<AssetTransferFilter>,
	assets: Vec<AssetTransferFilter>,
	remote_xcm: Xcm<()>,
}
```

An `InitiateTransfer { .. }` instruction shall transfer to `dest`, all
assets in the `holding` register that match the provided `assets` and
`remote_fees` filters.
These filters identify the assets to be transferred as well as the
transfer type to be used for transferring them.
It shall handle the local side of the transfer, then forward an onward
XCM to `dest` for handling the remote side of the transfer.

It should do so using same mechanisms as existing `DepositReserveAsset`,
`InitiateReserveWithdraw`, `InitiateTeleport` instructions but
practically combining all required XCM instructions to be remotely
executed into a _single_ remote XCM program to be sent over to `dest`.

Furthermore, through `remote_fees: Option<AssetTransferFilter>`, it
shall allow specifying a single asset to be used for fees on `dest`
chain. This single asset shall be remotely handled/received by the
**first instruction** in the onward XCM and shall be followed by a
`BuyExecution` instruction using it.
If `remote_fees` is set to `None`, the **first instruction** in the
onward XCM shall be a `UnpaidExecution` instruction. The rest of the
assets shall be handled by subsequent instructions, thus also finally
allowing [single asset buy
execution](#2423)
barrier security recommendation.

The `BuyExecution` appended to the onward XCM specifies
`WeightLimit::Unlimited`, thus being limited only by the `remote_fees`
asset "amount". This is a deliberate decision for enhancing UX - in
practice, people/dApps care about limiting the amount of fee asset used
and not the actually used weight.

The onward XCM, following the assets transfers instructions,
`ClearOrigin` or `DescendOrigin` instructions shall be appended to stop
acting on behalf of the source chain, then the caller-provided
`remote_xcm` shall also be appended, allowing the caller to control what
to do with the transferred assets.

Closes #5209

---------

Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: command-bot <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request. T6-XCM This PR/Issue is related to XCM.
Projects
Status: Todo
Status: Backlog
Development

No branches or pull requests

2 participants