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

pallet-xcm: add new extrinsic for asset transfers using explicit XCM transfer types #3695

Merged
merged 37 commits into from Apr 12, 2024

Conversation

acatangiu
Copy link
Contributor

@acatangiu acatangiu commented Mar 14, 2024

Description

Add transfer_assets_using() for transferring assets from local chain to destination chain using explicit XCM transfer types such as:

  • TransferType::LocalReserve: transfer assets to sovereign account of destination chain and forward a notification XCM to dest to mint and deposit reserve-based assets to beneficiary.
  • TransferType::DestinationReserve: burn local assets and forward a notification to dest chain to withdraw the reserve assets from this chain's sovereign account and deposit them to beneficiary.
  • TransferType::RemoteReserve(reserve): burn local assets, forward XCM to reserve chain to move reserves from this chain's SA to dest chain's SA, and forward another XCM to dest to mint and deposit reserve-based assets to beneficiary. Typically the remote reserve is Asset Hub.
  • TransferType::Teleport: burn local assets and forward XCM to dest chain to mint/teleport assets and deposit them to beneficiary.

By default, an asset's reserve is its origin chain. But sometimes we may want to explicitly use another chain as reserve (as long as allowed by runtime IsReserve filter).

This is very helpful for transferring assets with multiple configured reserves (such as Asset Hub ForeignAssets), when the transfer strictly depends on the used reserve.

E.g. For transferring Foreign Assets over a bridge, Asset Hub must be used as the reserve location.

Example usage scenarios

Transfer bridged ethereum ERC20-tokenX between ecosystem parachains.

ERC20-tokenX is registered on AssetHub as a ForeignAsset by the Polkadot<>Ethereum bridge (Snowbridge). Its asset_id is something like (parents:2, (GlobalConsensus(Ethereum), Address(tokenX_contract))). Its original reserve is Ethereum (only we can't use Ethereum as a reserve in local transfers); but, since tokenX is also registered on AssetHub as a ForeignAsset, we can use AssetHub as a reserve.

With this PR we can transfer tokenX from ParaA to ParaB while using AssetHub as a reserve.

Transfer AssetHub ForeignAssets between parachains

AssetA created on ParaA but also registered as foreign asset on Asset Hub. Can use AssetHub as a reserve.

And all of the above can be done while still controlling transfer type for fees so mixing assets in same transfer is supported.

Tests

Added integration tests for showcasing:

  • transferring local (not bridged) assets from parachain over bridge using local Asset Hub reserve,
  • transferring foreign assets from parachain to Asset Hub,
  • transferring foreign assets from Asset Hub to parachain,
  • transferring foreign assets from parachain to parachain using local Asset Hub reserve.

Later Edit: NOTE:

This PR has a followup PR that slightly changes the name and API: #4260

Add `transfer_assets_using_reserve()` for transferring assets from local
chain to destination chain using an explicit reserve location (typically
local Asset Hub).

By default, an asset's reserve is its origin chain. But sometimes we may
want to explicitly use another chain as reserve (as long as allowed by
runtime `IsReserve` filter).

This is very helpful for transferring assets with multiple configured
reserves (such as Asset Hub ForeignAssets), when the transfer strictly
depends on the used reserve.
E.g. For transferring Foreign Assets over a bridge, Asset Hub must be
used as the reserve location.
@acatangiu acatangiu added T6-XCM This PR/Issue is related to XCM. T2-pallets This PR/Issue is related to a particular pallet. T15-bridges This PR/Issue is related to bridges. labels Mar 14, 2024
@acatangiu acatangiu self-assigned this Mar 14, 2024
@acatangiu acatangiu requested a review from a team as a code owner March 14, 2024 11:54
@paritytech-review-bot paritytech-review-bot bot requested review from a team March 14, 2024 11:55
@franciscoaguirre
Copy link
Contributor

Just to double check. This two reserves issue happens because a token is teleported between two locations, right? It's not because a token is reserved transferred somewhere and then reserved transferred again from that main reserve into some sort of secondary reserve.

@acatangiu
Copy link
Contributor Author

Just to double check. This two reserves issue happens because a token is teleported between two locations, right? It's not because a token is reserved transferred somewhere and then reserved transferred again from that main reserve into some sort of secondary reserve.

It is the former. Teleporting assets between chains requires the same level of trust as allowing those chains to act as reserves for said asset. With teleports, it is conceptually the same asset on both chains, unlike reserve-based transfers where the real asset stays put within its borders while outside the borders (i.e. destination chain) a derivative asset is used to represent it.


This PR adds utility function for reserve-based transfers (not teleports) of a given asset, but using one of the assets secondary reserve locations (any other chain where it can be teleported to/from).

I added some example scenarios to the PR description for more clarity.

@acatangiu
Copy link
Contributor Author

cc @vgeddes @alistair-singh

@acatangiu
Copy link
Contributor Author

@bkontur addressed all comments, PTAL

Co-authored-by: Branislav Kontur <bkontur@gmail.com>
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 12, 2024 09:39
native_amount_to_send,
assets.into(),
None,
fee_asset_item,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thinking, do we want to change also this TestArgs to take just fee_asset_id instead of fee_asset_item index? Probably, it will cause lots of change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not in this PR..

@acatangiu
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Apr 12, 2024

@acatangiu https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5900488 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 4-fb45e933-062d-4f75-b56f-7f9a33901903 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 12, 2024

@acatangiu Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5900488 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5900488/artifacts/download.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 12, 2024 10:02
Copy link
Contributor

@bkontur bkontur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm and lots of tests are added for new cases, cool :)

@acatangiu acatangiu added this pull request to the merge queue Apr 12, 2024
Merged via the queue into paritytech:master with commit 1e971b8 Apr 12, 2024
131 of 136 checks passed
@acatangiu acatangiu deleted the transfer-using-explicit-reserve branch April 12, 2024 17:18
@albertov19
Copy link

Hey @acatangiu, one scenario that is currently not covered by the PolkadotXCM pallet is when you try to transfer a token from Moonbeam to AH (reserve chain) that is not an XCM-fee token (not sufficient in AH). Moonbeam wants to pay that XCM execution with, for example, DOT.

Moonbeam only has the relay chain as DOT reserve, and we can't have AH because then we would have 2 reserve chains for 1 asset, which could create many new problems with SA's balances.

So, ideally, we could have a way in which we do something like:

Moonbeam (InitiateReserveWithdraw) -> Polkadot (WithdrawAsset, ClearOrigin, ....)

Polkadot  -> AssetHub (XCM Instructions to take out the Token and send it to address

The main problem with the approach above is the ClearOrigin, which deletes all information that the message is being routed from Moonbeam. Is there an alternate scenario that this can be done?

Mainly because we have new assets being created in AH, and they are not sufficient assets, meaning that to send them back, users need to pay with either USDC/USDT.

XTokens allowed for such an approach in which 1 XCM is sent to the Relay Chain to send DOT to Parachains SA account in AH while another XCM was sent to AH to withdraw DOT to buy execution to transfer the token. However, this approach relied on the first XCM message executing successfully, so teams would usually have a DOT buffer in AH SA.

@acatangiu
Copy link
Contributor Author

That is correct, and I don't believe PolkadotXCM will or even could solve this problem (in a generic way for any asset and any chain).

Moonbeam only has the relay chain as DOT reserve, and we can't have AH because then we would have 2 reserve chains for 1 asset, which could create many new problems with SA's balances.

This is the same problem HydraDX and other parachains too are probably facing.
Unfortunately, there is no alternative scalable option I can see other than allowing these multiple reserves and (auto)balancing SAs.

The problem is not limited to DOT. The problem ultimately comes from having both reserve-based transfers as well as teleports allowed in our ecosystem. Once you allow a teleport between two chains, then both of those should be able to act as reserves, otherwise the teleport doesn't make sense.

E.g.: parachain assets registered as ForeignAssets on AH and teleported there. The "owner" parachain will want to allow AH to act as a reserve for them, and expose their asset to all the other chains/bridges/CEXs/etc that are already integrated with AH.
The "users" of their asset will have the same issue: the asset's reserve is split between AH and the "owner" parachain.

XTokens allowed for such an approach in which 1 XCM is sent to the Relay Chain to send DOT to Parachains SA account in AH while another XCM was sent to AH to withdraw DOT to buy execution to transfer the token. However, this approach relied on the first XCM message executing successfully, so teams would usually have a DOT buffer in AH SA.

Exactly! Ultimately, parachains need "some buffer" in AH SA anyway.

I am hoping that with some smarter way of keeping SAs balanced, this problem will go away and workarounds like sending multiple XCMs and waiting for them to happen in a certain sequence will not be needed anymore.


The silver lining here is that the problem is very generic and asset/chain agnostic, and we can decide on some mechanism to auto-balance these SAs, that every chain can use.
E.g. a new pallet_sa_management where root/admin can register several SA locations per asset along with some % distribution for each, that periodically polls the remote balances of those SAs and moves liquidity around to stay within configured distribution %.

@albertov19
Copy link

The silver lining here is that the problem is very generic and asset/chain agnostic, and we can decide on some mechanism to auto-balance these SAs, that every chain can use.
E.g. a new pallet_sa_management where root/admin can register several SA locations per asset along with some % distribution for each, that periodically polls the remote balances of those SAs and moves liquidity around to stay within configured distribution %.

The main issue is SA management. It would then require users to go through undesirable routes to get their assets where they want to (for example, SA in Polkadot has 100 DOT, and in AH, it has 20 DOT, and a user wants to get 40 DOT in Polkadot). This creates bad UX.

What about creating a call in which you can route XCM and assets through another chain? I guess the application of this is too specific. And could we do something similar to what xTokens is doing, but just from a UI point of view?

@acatangiu
Copy link
Contributor Author

acatangiu commented Apr 22, 2024

The main issue is SA management. It would then require users to go through undesirable routes to get their assets where they want to (for example, SA in Polkadot has 100 DOT, and in AH, it has 20 DOT, and a user wants to get 40 DOT in Polkadot). This creates bad UX.

In practice, liquidity should be much larger, and with some periodic automated balancing mechanism this should never happen (unless the user is some whale moving a lot of liquidity at once).
E.g. Moonbeam right now has 2,521,028 DOT in reserve in its SA on the relay chain (5Ec4AhPVjsshXjh8ynp6MwaJTJBnen3pkHiiyDhHfie5VWkN). A very simplistic 50/50 split with SA on AH that is rebalanced once a day would give Moonbeam users a window of 24h to withdraw 1.25M DOT from Moonbeam before reserve runs out.

A somewhat smarter rebalancer: component be deployed on target chains that tracks SA balance and requests liquidity from owner chain if it falls below a threshold, thus triggering rebalancing on-demand.

What about creating a call in which you can route XCM and assets through another chain? I guess the application of this is too specific. And could we do something similar to what xTokens is doing, but just from a UI point of view?

Could be done, but don't know of a way to do it generically, chain/asset agnostic - and without a generic mechanism I strongly recommend against it as it will not scale. We will fragment the ecosystem even further with tokens and chains having custom workarounds and special gotchas.

There is also an efficiency disadvantage (not really important, but worth mentioning): at the end of the day this is also an automated SA rebalancing mechanism, it just happens on every transfer regardless of need (with the user paying for two extra XCMs each time: to 1. signal the main reserve to 2. send liquidity to secondary reserve).

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/managing-sas-on-multiple-reserve-chains-for-same-asset/7538/1

@acatangiu
Copy link
Contributor Author

I created a forum topic to get more visibility and ideas on the problem: https://forum.polkadot.network/t/managing-sas-on-multiple-reserve-chains-for-same-asset/7538

@xlc
Copy link
Contributor

xlc commented Apr 23, 2024

I raised this issue a year ago paritytech/cumulus#2398

@acatangiu
Copy link
Contributor Author

I raised this issue a year ago paritytech/cumulus#2398

Apparently it takes more than raising an issue to solve an ecosystem-wide problem :)

Please join the discussion on the forum, help me engage all the parachains, and collectively decide on a solution.

I can commit to implement whatever we, the ecosystem, decide is necessary, if help is required from Parity.

github-merge-queue bot pushed a commit that referenced this pull request Apr 24, 2024
…estination (#4260)

Change `transfer_assets_using_type()` to not assume `DepositAssets` as
the intended use of the assets on the destination.

Instead provides the caller with the ability to specify custom XCM that
be executed on `dest` chain as the last step of the transfer, thus
allowing custom usecases for the transferred assets. E.g. some are
used/swapped/etc there, while some are sent further to yet another
chain.

Note: this is a follow-up on
#3695, bringing in an API
change for `transfer_assets_using_type()`. This is ok as the previous
version has not been yet released. Thus, its first release will include
the new API proposed by this PR.

This allows usecases such as:
https://forum.polkadot.network/t/managing-sas-on-multiple-reserve-chains-for-same-asset/7538/4

BTW: all this pallet-xcm asset transfers code will be massively reduced
once we have paritytech/xcm-format#54

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet. T6-XCM This PR/Issue is related to XCM. T10-tests This PR/Issue is related to tests. T15-bridges This PR/Issue is related to bridges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants