-
Notifications
You must be signed in to change notification settings - Fork 44
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
RFC: add complex asset transfers instructions #54
Conversation
cc @mrshiposha @xlc |
…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 polkadot-fellows/xcm-format#54 --------- Signed-off-by: Adrian Catangiu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion! My only comment is about naming and the number of instructions. I want to make sure these instructions are simple to reason about. Right now, because of the names, it's hard to figure out what they do if I don't look at the documentation.
There was the option of having only one huge instruction.
When you only care about a specific transfer type, like when doing a simple teleport, you would have to add a bunch of None
s.
I prefer the two-step approach you described, first configure the asset transfer and then execute it, because you don't want to add a bunch of None
s all the time.
That said, I'd have only one previous instruction PrepareAssetTransfer
that receives a vector of a TransferType
enum. This instruction sets asides assets for teleports, local reserve and destination reserve.
enum Instruction {
...
PrepareAssetTransfer(Vec<AssetTransferType>),
...
}
enum AssetTransferType {
Teleport(AssetFilter),
LocalReserve(AssetFilter),
DestinationReserve(AssetFilter),
}
I think this strikes a good balance between having just one instruction or a lot of them. It makes XCM programs easier to reason about.
What do you think?
PS: We could also probably do this in the one big instruction, further coupling concepts that work together. It would always require specifying a vector of AssetTransferType
, even if it has only one element. That makes it good UX even when just doing a simple teleport.
enum Instruction {
...
InitiateAssetTransfer {
destination: Location,
assets: Vec<AssetTransferType>,
remote_fees: Option<AssetFilter>,
remote_xcm: Xcm<()>,
},
...
}
rest of the assets are handled by subsequent instructions, thus allowing | ||
[single asset buy execution](https://github.com/paritytech/polkadot-sdk/issues/2423). | ||
|
||
Open Q: for `remote_fees == None`, should we append `UnpaidExecution` or do nothing? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to add UnpaidExecution
here. It would be very useful as right now system parachains have to teleport with unpaid execution and there are currently no instructions that handle this use-case correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine unpaid asset transfers, but haven't seen any in practice. Do you know of any practical usecases so we can make sure we properly support them?
enum Instruction {
...
InitiateAssetTransfer {
destination: Location,
assets: Vec<AssetTransferType>,
remote_fees: Option<AssetFilter>,
remote_xcm: Xcm<()>,
},
...
} Yes, I like this ^ |
|
||
### What is the impact of not doing this? | ||
|
||
Current multi-chain transfers are forced to happen in multiple programs per asset per "hop", resulting in very poor UX. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For someone not having done a lot of XCM, one can only imagine why this is a very poor UX. What problems does a developer actually face with the current design?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The developer simply can not write an XCM program that can, for example, transfer both DOT (local reserve deposit) and HDX (destination reserve withdraw) from AssetHub to HydraDX. It is simply not possible with the current instruction set.
The only XCM instructions available currently (XCMv4) for transferring assets are DepositReserveAsset
, InitiateReserveWithdraw
and InitiateTeleport
instructions. They initiate asset transfers on execution, but they are opinionated in the type of transfer to use. Combining them is not possible, because as a result of their individual execution, a message containing a ClearOrigin
instruction is sent to the destination chain (for security reasons - more details available in their docs), making subsequent transfers impossible.
This pull request has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/xcm-user-and-developer-experience-improvements/4511/21 |
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/7 |
Unfortunately, following the repo migration to the new org, I lost write access to the PR (upstream) branch. I've opened the PR from a fork branch here #63 Closing this one. |
Summary
This RFC proposes new instructions that provide a way to initiate asset transfers which transfer multiple types (teleports, local-reserve, destination-reserve) of assets, on remote chains using XCM alone.
The currently existing instructions are too opinionated and force each XCM asset transfer to a single transfer type (teleport, local-reserve, destination-reserve). This results in inability to combine different types of transfers in single transfer which results in overall poor UX when trying to move assets across chains.
Example usage
Proof-of-Concept
I also have a proof-of-concept implementation and a proof-of-concept asset transfer test, transferring 2 different types of assets across 3 chains.