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

Enable RefundSurplus and DepositAsset instructions for XCM Transactor #2283

Closed
wants to merge 16 commits into from

Conversation

Agusrodri
Copy link
Contributor

@Agusrodri Agusrodri commented May 5, 2023

What does it do?

Currently, xcm-transactor pallet and precompile don't allow senders to make usage of RefundSurplus and DepositAsset instructions, which normally causes assets get trapped on the destination chain. This PR enables such functionality.

Changes

  • Modified transact_through_signed(), transact_through_derivative() and transact_through_sovereign() functions to accept an additional bool parameter (called refund) that indicates if RefundSurplus and DepositAsset instructions will be used. These instructions are included inside an appendix that will execute them at the end of the execution of the whole XCM message.
  • Added transactThroughSignedRefund(), transactThroughSignedMultilocationRefund(), transactThroughDerivativeRefund() and transactThroughDerivativeMultilocationRefund() functions to xcm-transactor precompile V2. These functions receive the same parameters as the normal previous ones, but this time adding the appendix with RefundSurplus and DepositAsset behind the scenes. It was thought this way to not break any kind of existing compatibility.

Notes

  • If the appendix is set within transact_through_derivative() or transact_through_sovereign(), the remaining surplus will be refunded to the sovereign account.

@Agusrodri Agusrodri added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. D10-breaksdocs Changes to code that require changes in documentation breaking Needs to be mentioned in breaking changes labels May 5, 2023
@Agusrodri Agusrodri requested review from girazoki and librelois May 5, 2023 19:13
@Agusrodri Agusrodri added breaking Needs to be mentioned in breaking changes and removed breaking Needs to be mentioned in breaking changes labels May 5, 2023
@Agusrodri Agusrodri marked this pull request as draft May 8, 2023 23:33
@Agusrodri Agusrodri marked this pull request as ready for review May 9, 2023 00:57
@crystalin
Copy link
Collaborator

@Agusrodri this will need to be updated. I merged master but there are some compilation errors

@Agusrodri
Copy link
Contributor Author

Agusrodri commented May 26, 2023

@Agusrodri this will need to be updated. I merged master but there are some compilation errors

@crystalin yes, I think this PR will be closed in favor of a new one containing this and other necessary changes for the xcm-transactor precompile to be upgraded to V3.

@Agusrodri Agusrodri marked this pull request as draft May 26, 2023 14:20
@Agusrodri Agusrodri closed this Jun 8, 2023
@Agusrodri
Copy link
Contributor Author

Closed in favor of #2338

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. D10-breaksdocs Changes to code that require changes in documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants