-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[XCM]XcmPaymentApi::query_delivery_fees() allow querying fee using custom asset #7179
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
Closed
programskillforverification
wants to merge
4
commits into
paritytech:master
from
programskillforverification:custom-asset-for-query_delivery_fees
Closed
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
788e873
init
programskillforverification 2ffb8ee
Merge branch 'master' into custom-asset-for-query_delivery_fees
programskillforverification 6c31331
Merge branch 'master' into custom-asset-for-query_delivery_fees
programskillforverification dc171e5
adjust version
programskillforverification File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@acatangiu how can I convert
feeto newassetcorrespondingcustom_assetwithout introduce new dependency?Uh oh!
There was an error while loading. Please reload this page.
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.
you don't change
pallet_xcm::query_delivery_fees()- it provides delivery fee as X DOTyou change
xcm_runtime_apis::fees::XcmPaymentApiimplementation fn query_delivery_fees(dest, msg, asset_id) to convertX DOTto Ycustom_assetusing assets_common::PoolAdapter::::quote_price_tokens_for_exact_tokens()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.
You also need to change the
XcmPaymentApito take customasset_idfor delivery fees.You can either change existing fn query_delivery_fees() - which would be a API breaking change - or you can add a new function..
RuntimeAPIs are versioned so it's not a big deal to break them, but not sure which one's better. @franciscoaguirre thoughts?
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 add a new is better
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.
Runtime APIs are versioned so there's no problem with changing the signature.
You just need to specify the version, leave the old signature and point to the new one.
Example:
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.
assets_commoncauses cyclic package dependency. So directly usepallet-asset-conversion::quote_price_tokens_for_exact_tokens()?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 returning the delivery fees in the required asset (DOT, for example) is as far as this
pallet-xcmhelper is going to get.This pallet doesn't assume the runtime it's running on has
pallet-asset-conversion, or any pallet.So we should call assets_common::PoolAdapter::::quote_price_tokens_for_exact_tokens() on the runtimes we want to swap, so asset-hub-westend.
The implementation in asset-hub-westend would call this
pallet-xcmhelper to get the fee in DOT and then call the asset conversion function to convert it into the custom asset that was passed in