Skip to content

XcmPaymentApi::query_weight_to_asset_fee simple common impl#8281

Merged
acatangiu merged 18 commits intoparitytech:masterfrom
UniqueNetwork:workaround/weight-to-fee-common-impl
May 6, 2025
Merged

XcmPaymentApi::query_weight_to_asset_fee simple common impl#8281
acatangiu merged 18 commits intoparitytech:masterfrom
UniqueNetwork:workaround/weight-to-fee-common-impl

Conversation

@mrshiposha
Copy link
Copy Markdown
Contributor

@mrshiposha mrshiposha commented Apr 18, 2025

Description

Add a common implementation for XcmPaymentApi::query_weight_to_asset_fee to pallet-xcm.

This PR is a simple alternative to #8202 (which could still be useful for other reasons).
It uses a workaround instead of a big refactoring.

The workaround is:
Computes the weight cost using the provided WeightTrader.
This function is supposed to be used ONLY in XcmPaymentApi::query_weight_to_asset_fee
Runtime API implementation, as it can introduce a massive change to the total issuance.
The provided WeightTrader must be the same as the one used in the XcmExecutor to ensure
uniformity in the weight cost calculation.

NOTE: Currently this function uses a workaround that should be good enough for all practical
uses: passes u128::MAX / 2 == 2^127 of the specified asset to the WeightTrader as
payment and computes the weight cost as the difference between this and the unspent amount.
Some weight traders could add the provided payment to some account's balance. However,
it should practically never result in overflow because even currencies with a lot of decimal digits
(say 18) usually have the total issuance of billions (x * 10^9) or trillions (x * 10^12) at max,
much less than 2^127 / 10^18 =~ 1.7 * 10^20 (170 billion billion). Thus, any account's balance
most likely holds less than 2^127, so adding 2^127 won't result in u128 overflow.

Integration

The Runtime builders can use the query_weight_to_asset_fee provided by pallet-xcm in
their XcmPaymentApi implementation.

@mrshiposha
Copy link
Copy Markdown
Contributor Author

CC @acatangiu

@mrshiposha mrshiposha changed the title workaround: XcmPaymentApi::query_weight_to_asset_fee common impl workaround: XcmPaymentApi::query_weight_to_asset_fee simple common impl Apr 18, 2025
Copy link
Copy Markdown
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

I like the approach - using the Trader directly is the right way to go 👍

Copy link
Copy Markdown
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Looks good! cc @franciscoaguirre to also have a look, but I think you can go ahead with fully implementing the PR.

@mrshiposha mrshiposha marked this pull request as ready for review April 21, 2025 15:09
@mrshiposha mrshiposha requested a review from a team as a code owner April 21, 2025 15:09
@acatangiu acatangiu changed the title workaround: XcmPaymentApi::query_weight_to_asset_fee simple common impl XcmPaymentApi::query_weight_to_asset_fee simple common impl Apr 23, 2025
Copy link
Copy Markdown
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

I tested the workaround locally using Westend Asset Hub. Is there a proper way to unit-test this?

Yes, I would add following tests:

  • generic test here using native DOT/KSM that can be called for all runtimes in their local unit-tests (e.g. here)
  • Asset Hub Westend unit test here that sets up a few pools then validates this call against a few assets (that have pools)

notes:

@acatangiu
Copy link
Copy Markdown
Contributor

TRY 6 FAIL [ 0.209s] asset-hub-westend-integration-tests tests::swap::xcm_fee_querying_apis_work

some tests are failing

// We just created it, there's none.
assert_eq!(fee_in_usdt_fail, Err(XcmPaymentApiError::AssetNotFound));

// USDT is sufficient asset, so it passes even if there is not enough liquidity yet.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since #8376 was merged recently, sufficient assets can no longer directly buy weight and also have to go through liquidity pool to work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll revert this commit then. Merging master should fix the tests instead

@acatangiu
Copy link
Copy Markdown
Contributor

also needs rebase and conflicts to resolve

@acatangiu acatangiu added this pull request to the merge queue May 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2025
@acatangiu acatangiu added this pull request to the merge queue May 6, 2025
Merged via the queue into paritytech:master with commit 5802306 May 6, 2025
241 of 250 checks passed
castillax pushed a commit that referenced this pull request May 12, 2025
# Description

Add a common implementation for
`XcmPaymentApi::query_weight_to_asset_fee` to `pallet-xcm`.

This PR is a simple alternative to #8202 (which could still be useful
for other reasons).
It uses a workaround instead of a big refactoring.

The workaround is:
Computes the weight cost using the provided `WeightTrader`.
This function is supposed to be used ONLY in
`XcmPaymentApi::query_weight_to_asset_fee`
Runtime API implementation, as it can introduce a massive change to the
total issuance.
The provided `WeightTrader` must be the same as the one used in the
XcmExecutor to ensure
uniformity in the weight cost calculation.

NOTE: Currently this function uses a workaround that should be good
enough for all practical
uses: passes `u128::MAX / 2 == 2^127` of the specified asset to the
`WeightTrader` as
payment and computes the weight cost as the difference between this and
the unspent amount.
Some weight traders could add the provided payment to some account's
balance. However,
it should practically never result in overflow because even currencies
with a lot of decimal digits
(say 18) usually have the total issuance of billions (`x * 10^9`) or
trillions (`x * 10^12`) at max,
much less than `2^127 / 10^18 =~ 1.7 * 10^20` (170 billion billion).
Thus, any account's balance
most likely holds less than `2^127`, so adding `2^127` won't result in
`u128` overflow.


## Integration

The Runtime builders can use the `query_weight_to_asset_fee` provided by
`pallet-xcm` in
their XcmPaymentApi implementation.

---------

Co-authored-by: Adrian Catangiu <adrian@parity.io>
fellowship-merge-bot bot pushed a commit to polkadot-fellows/runtimes that referenced this pull request Aug 7, 2025
This brings in `stable2506` Polkadot SDK, and integrates many new
features.

Integrated breaking changes to be verified by the original authors:

- [x] ~paritytech/polkadot-sdk#8127 @kianenigma
@Ank4n~
     This will come in with AHM, and not before.
- [x] paritytech/polkadot-sdk#7597 @gui1117 
- [x] paritytech/polkadot-sdk#8254 @bkchr 
- [x] paritytech/polkadot-sdk#7592 @bkontur 
- [x] paritytech/polkadot-sdk#8382
@UtkarshBhardwaj007
- [x] paritytech/polkadot-sdk#8021 @serban300 
- [x] paritytech/polkadot-sdk#8344 @serban300 
- [x] paritytech/polkadot-sdk#8262 @athei 
- [x] paritytech/polkadot-sdk#8584 @athei 
- [x] paritytech/polkadot-sdk#8299 @skunert
- [x] paritytech/polkadot-sdk#8652 @pgherveou 
- [x] paritytech/polkadot-sdk#8554 @pgherveou 
- [x] paritytech/polkadot-sdk#8281 @mrshiposha 
- [x] paritytech/polkadot-sdk#7730
@franciscoaguirre
- [x] paritytech/polkadot-sdk#8599 @yrong
@claravanstaden
- [x] paritytech/polkadot-sdk#8531 @bkontur 
- [x] paritytech/polkadot-sdk#8409 @kianenigma 
- [x] paritytech/polkadot-sdk#9137
@franciscoaguirre
- [x] paritytech/polkadot-sdk#7944 @bkontur 
- [x] paritytech/polkadot-sdk#8179 @bkontur 
- [x] paritytech/polkadot-sdk#8037 @yrong

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: claravanstaden <claravanstaden64@gmail.com>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Alain Brenzikofer <alain@integritee.network>
Co-authored-by: kianenigma <kian@parity.io>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: ron <yrong1997@gmail.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Overkillus <maciej.zyszkiewicz@parity.io>
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
# Description

Add a common implementation for
`XcmPaymentApi::query_weight_to_asset_fee` to `pallet-xcm`.

This PR is a simple alternative to #8202 (which could still be useful
for other reasons).
It uses a workaround instead of a big refactoring.

The workaround is:
Computes the weight cost using the provided `WeightTrader`.
This function is supposed to be used ONLY in
`XcmPaymentApi::query_weight_to_asset_fee`
Runtime API implementation, as it can introduce a massive change to the
total issuance.
The provided `WeightTrader` must be the same as the one used in the
XcmExecutor to ensure
uniformity in the weight cost calculation.

NOTE: Currently this function uses a workaround that should be good
enough for all practical
uses: passes `u128::MAX / 2 == 2^127` of the specified asset to the
`WeightTrader` as
payment and computes the weight cost as the difference between this and
the unspent amount.
Some weight traders could add the provided payment to some account's
balance. However,
it should practically never result in overflow because even currencies
with a lot of decimal digits
(say 18) usually have the total issuance of billions (`x * 10^9`) or
trillions (`x * 10^12`) at max,
much less than `2^127 / 10^18 =~ 1.7 * 10^20` (170 billion billion).
Thus, any account's balance
most likely holds less than `2^127`, so adding `2^127` won't result in
`u128` overflow.


## Integration

The Runtime builders can use the `query_weight_to_asset_fee` provided by
`pallet-xcm` in
their XcmPaymentApi implementation.

---------

Co-authored-by: Adrian Catangiu <adrian@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants