Add an adapter for configuring AssetExchanger #5130
Merged
Conversation
This file contains 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 was referenced Jul 24, 2024
gui1117
reviewed
Jul 27, 2024
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Outdated
Show resolved
Hide resolved
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Show resolved
Hide resolved
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Outdated
Show resolved
Hide resolved
acatangiu
approved these changes
Jul 29, 2024
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Outdated
Show resolved
Hide resolved
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Outdated
Show resolved
Hide resolved
|
The CI pipeline was cancelled due to failure one of the required jobs. |
muharem
reviewed
Jul 30, 2024
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Show resolved
Hide resolved
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Show resolved
Hide resolved
acatangiu
approved these changes
Aug 1, 2024
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
muharem
reviewed
Aug 2, 2024
Comment on lines
+57
to
+58
| /// - quote(give, want, maximal) = resulting_want -> exchange(give, resulting_want, maximal) ✅ | ||
| /// - quote(give, want, minimal) = required_give -> exchange(required_give_amount, want, |
There was a problem hiding this comment.
this useful explanation.
would not this be simpler if we do something like:
quote_exchange(give: Assets, want: AssetIds) -> Option<Assets>;
quote_price(give: AssetIds, want: Assets) -> Option<Assets>;
There was a problem hiding this comment.
Hmm, it makes more sense to use AssetId instead of the assets and the amount whenever we don't need the amount.
However, there are two things I like about the API as-is.
One, by specifying the fungibility we get to also specify instances for NFTs.
Two, the addition of maximal even though it's weird it matches better with exchange_asset, so you know which one matches with which one.
polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/adapter.rs
Show resolved
Hide resolved
muharem
approved these changes
Aug 2, 2024
ordian
added a commit
that referenced
this pull request
Aug 6, 2024
* master: (51 commits) Remove unused feature gated code from the minimal template (#5237) make polkadot-parachain startup errors pretty (#5214) Coretime auto-renew (#4424) network/strategy: Backoff and ban overloaded peers to avoid submitting the same request multiple times (#5029) Fix frame crate usage doc (#5222) beefy: Tolerate pruned state on runtime API call (#5197) rpc: Enable ChainSpec for polkadot-parachain (#5205) Add an adapter for configuring AssetExchanger (#5130) Replace env_logger with sp_tracing (#5065) Adjust sync templates flow to use new release branch (#5182) litep2p/discovery: Publish authority records with external addresses only (#5176) Run UI tests in CI for some other crates (#5167) Remove `pallet::getter` usage from the pallet-balances (#4967) pallet-timestamp: `UnixTime::now` implementation logs error only if called at genesis (#5055) [CI] Cache try-runtime check (#5179) [Backport] version bumps and the prdocs reordering from stable2407 (#5178) [subsystem-benchmark] Update availability-distribution-regression-bench baseline after recent subsystem changes (#5180) Remove pallet::getter usage from proxy (#4963) Remove pallet::getter macro usage from pallet-election-provider-multi-phase (#4487) [email protected] (#5177) ...
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 2, 2024
# Context Fees can already be paid in other assets locally thanks to the Trader implementations we have. This doesn't work when sending messages because delivery fees go through a different mechanism altogether. The idea is to fix this leveraging the `AssetExchanger` config item that's able to turn the asset the user wants to pay fees in into the asset the router expects for delivery fees. # Main addition An adapter was needed to use `pallet-asset-conversion` for exchanging assets in XCM. This was created in #5130. The XCM executor was modified to use `AssetExchanger` (when available) to swap assets to pay for delivery fees. ## Limitations We can only pay for delivery fees in different assets in intermediate hops. We can't pay in different assets locally. The first hop will always need the native token of the chain (or whatever is specified in the `XcmRouter`). This is a byproduct of using the `BuyExecution` instruction to know which asset should be used for delivery fee payment. Since this instruction is not present when executing an XCM locally, we are left with this limitation. To illustrate this limitation, I'll show two scenarios. All chains involved have pools. ### Scenario 1 Parachain A --> Parachain B Here, parachain A can use any asset in a pool with its native asset to pay for local execution fees. However, as of now we can't use those for local delivery fees. This means transfers from A to B need some amount of A's native token to pay for delivery fees. ### Scenario 2 Parachain A --> Parachain C --> Parachain B Here, Parachain C's remote delivery fees can be paid with any asset in a pool with its native asset. This allows a reserve asset transfer between A and B with C as the reserve to only need A's native token at the starting hop. After that, it could all be pool assets. ## Future work The fact that delivery fees go through a totally different mechanism results in a lot of bugs and pain points. Unfortunately, this is not so easy to solve in a backwards compatible manner. Delivery fees will be integrated into the language in future XCM versions, following polkadot-fellows/xcm-format#53. Old PR: #4375.
x3c41a
added a commit
that referenced
this pull request
Sep 4, 2024
# Context Fees can already be paid in other assets locally thanks to the Trader implementations we have. This doesn't work when sending messages because delivery fees go through a different mechanism altogether. The idea is to fix this leveraging the `AssetExchanger` config item that's able to turn the asset the user wants to pay fees in into the asset the router expects for delivery fees. # Main addition An adapter was needed to use `pallet-asset-conversion` for exchanging assets in XCM. This was created in #5130. The XCM executor was modified to use `AssetExchanger` (when available) to swap assets to pay for delivery fees. ## Limitations We can only pay for delivery fees in different assets in intermediate hops. We can't pay in different assets locally. The first hop will always need the native token of the chain (or whatever is specified in the `XcmRouter`). This is a byproduct of using the `BuyExecution` instruction to know which asset should be used for delivery fee payment. Since this instruction is not present when executing an XCM locally, we are left with this limitation. To illustrate this limitation, I'll show two scenarios. All chains involved have pools. ### Scenario 1 Parachain A --> Parachain B Here, parachain A can use any asset in a pool with its native asset to pay for local execution fees. However, as of now we can't use those for local delivery fees. This means transfers from A to B need some amount of A's native token to pay for delivery fees. ### Scenario 2 Parachain A --> Parachain C --> Parachain B Here, Parachain C's remote delivery fees can be paid with any asset in a pool with its native asset. This allows a reserve asset transfer between A and B with C as the reserve to only need A's native token at the starting hop. After that, it could all be pool assets. ## Future work The fact that delivery fees go through a totally different mechanism results in a lot of bugs and pain points. Unfortunately, this is not so easy to solve in a backwards compatible manner. Delivery fees will be integrated into the language in future XCM versions, following polkadot-fellows/xcm-format#53. Old PR: #4375.
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Added a new adapter to xcm-builder, the
SingleAssetExchangeAdapter.This adapter makes it easy to use
pallet-asset-conversionfor configuring theAssetExchangerXCM config item.I also took the liberty of adding a new function to the
AssetExchangetrait, with the following signature:The signature is meant to be fairly symmetric to that of
exchange_asset.The way they interact can be seen in the doc comment for it in the
AssetExchangetrait.This is a breaking change but is needed for #5131.
Another idea is to create a new trait for this but that would require setting it in the XCM config which is also breaking.
Old PR: #4375.