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::transfer_assets_using_type_and_then() supports custom actions on destination #4260

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

acatangiu
Copy link
Contributor

@acatangiu acatangiu commented Apr 23, 2024

Continuation of #3695

Change transfer_assets_using_type_and_then() 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 (and rename) 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

@acatangiu acatangiu added the T6-XCM This PR/Issue is related to XCM. label Apr 23, 2024
@acatangiu acatangiu self-assigned this Apr 23, 2024
@acatangiu acatangiu requested a review from a team as a code owner April 23, 2024 15:50
…estination

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 an API change for `transfer_assets_using_type()`, but it is ok as the
previous version has not been yet released. Thus, its first release will include the
new API proposed by this PR.

Signed-off-by: Adrian Catangiu <[email protected]>
@acatangiu acatangiu force-pushed the pallet-xcm-extend-transfer-xt branch from 6a07074 to ee88f2e Compare April 23, 2024 15:51
fees_transfer_type: Box<TransferType>,
custom_xcm_on_dest: Box<VersionedXcm<()>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't have much context regarding this change. A couple of questions. Not sure if they make sense:

  1. Should this be a blob ? Just as we did for send and execute ?
  2. Isn't it risky to allow the user to perform arbitrary xcm execution here ?
  3. Nit: seems unexpected to pass a custom_xcm_on_dest here. Partially because of the name. transfer_assets_using_type makes me think automatically of the asset being deposited at the destination chain. And also because the other transfer-related extrinsics seem to deposit the assets from what I understand. Maybe renaming the method to something like transfer_assets_using_type_and_then would make this more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. With UncheckedExtrinsic decoding improvements #4255 we don't need the blob hack anymore.
  2. No, a ClearOrigin is run before these user-controlled instructions. Only thing they can do is operate on the holding register (which they loaded in the first place).
  3. Yes, I like the name suggestion will rename to transfer_assets_using_type_and_then

polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
@acatangiu acatangiu added the R0-silent Changes should not be mentioned in any release notes label Apr 24, 2024
@acatangiu
Copy link
Contributor Author

@bkontur @serban300 I made the suggested changes, edited the prdoc from the original PR, and added emulated tests. PTAL

@acatangiu acatangiu added this pull request to the merge queue Apr 24, 2024
Merged via the queue into paritytech:master with commit e0584a1 Apr 24, 2024
131 of 138 checks passed
@acatangiu acatangiu deleted the pallet-xcm-extend-transfer-xt branch April 24, 2024 09:09
@acatangiu acatangiu changed the title pallet-xcm::transfer_assets_using_type() supports custom actions on destination pallet-xcm::transfer_assets_using_type_and_then() supports custom actions on destination May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants