Skip to content

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_id: Box<VersionedAssetId>,
remote_fees_id: Box<VersionedAssetId>,
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 [Tracking issue] 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

@acatangiu acatangiu added the R0-no-crate-publish-required The change does not require any crates to be re-published. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

R0-no-crate-publish-required The change does not require any crates to be re-published. T6-XCM This PR/Issue is related to XCM.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants