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

More xcm::v4 cleanup and xcm_fee_payment_runtime_api::XcmPaymentApi nits #4355

Merged
merged 3 commits into from
May 2, 2024

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented May 2, 2024

This PR:

  • changes xcm::v4 to xcm::prelude imports for coretime stuff
  • changes query_acceptable_payment_assets / query_weight_to_asset_fee implementations to be more resilient to the XCM version change
  • adds xcm_fee_payment_runtime_api::XcmPaymentApi to the AssetHubRococo/Westend exposing a native token as acceptable payment asset

Continuation of: #3607

Closes: #4297

Possible follow-ups

  • add all sufficient assets (Assets, ForeignAssets) as acceptable payment assets ?

@bkontur bkontur added R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM. labels May 2, 2024
@bkontur bkontur requested a review from a team as a code owner May 2, 2024 11:41
Comment on lines +2214 to +2227
match asset.try_as::<AssetId>() {
Ok(asset_id) if asset_id.0 == xcm_config::TokenLocation::get() => {
// for native token
Ok(WeightToFee::weight_to_fee(&weight))
},
Ok(asset_id) => {
log::trace!(target: "xcm::xcm_fee_payment_runtime_api", "query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!");
Err(XcmPaymentApiError::AssetNotFound)
},
Err(_) => {
log::trace!(target: "xcm::xcm_fee_payment_runtime_api", "query_weight_to_asset_fee - failed to convert asset: {asset:?}!");
Err(XcmPaymentApiError::VersionedConversionFailed)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this code is duplicated for Rococo and Westend. Could we move it into a method and reuse it ?

Seems to also be the case for query_acceptable_payment_assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks duplicated now, because everywhere we expose just a native token, but for AHs we could add at least USDT/USDC or maybe all sufficient assets, then this code would look different. Also I am not sure about other runtimes and their WeightToFee impls.

Yes, I was thinking about extracting e.g. Vec conversion with into_version at least, but not sure about the rest. What do you think? Any suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. Then I think we can see how we can make this more generic later when we add this for multiple runtimes.

Copy link
Contributor

@franciscoaguirre franciscoaguirre 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. We should do the follow-up later. I still need to merge #3872 and create a follow-up where I implement this and the payment api on all runtimes. So it could probably go there.

@bkontur bkontur added this pull request to the merge queue May 2, 2024
Merged via the queue into master with commit 30a1972 May 2, 2024
151 of 154 checks passed
@bkontur bkontur deleted the bko-xcm-v4-cleanup branch May 2, 2024 14:30
dcolley added a commit to metaspan/polkadot-sdk that referenced this pull request May 6, 2024
* 'master' of https://github.com/metaspan/polkadot-sdk: (65 commits)
  Introduces `TypeWithDefault<T, D: Get<T>>` (paritytech#4034)
  Publish `polkadot-sdk-frame`  crate (paritytech#4370)
  Add validate field to prdoc (paritytech#4368)
  State trie migration on asset-hub westend and collectives westend (paritytech#4185)
  Fix: dust unbonded for zero existential deposit (paritytech#4364)
  Bridge: added subcommand to relay single parachain header (paritytech#4365)
  Bridge: fix zombienet tests (paritytech#4367)
  [WIP][CI] Add more GHA jobs (paritytech#4270)
  Allow for 0 existential deposit in benchmarks for `pallet_staking`, `pallet_session`, and `pallet_balances` (paritytech#4346)
  Deprecate `NativeElseWasmExecutor` (paritytech#4329)
  More `xcm::v4` cleanup and `xcm_fee_payment_runtime_api::XcmPaymentApi` nits (paritytech#4355)
  sc-tracing: enable env-filter feature (paritytech#4357)
  deps: update jsonrpsee to v0.22.5 (paritytech#4330)
  Add PoV-reclaim enablement guide to polkadot-sdk-docs (paritytech#4244)
  cargo: Update experimental litep2p to latest version (paritytech#4344)
  Bridge: ignore client errors when calling recently added `*_free_headers_interval` methods (paritytech#4350)
  Make parachain template great again (and async backing ready) (paritytech#4295)
  [Backport] Version bumps and reorg prdocs from 1.11.0 (paritytech#4336)
  HRMP - set `DefaultChannelSizeAndCapacityWithSystem` with dynamic values according to the `ActiveConfig` (paritytech#4332)
  Statement Distribution Per Peer Rate Limit (paritytech#3444)
  ...
fgamundi pushed a commit to moondance-labs/polkadot-sdk that referenced this pull request Jul 17, 2024
…i` nits (paritytech#4355)

This PR:
- changes `xcm::v4` to `xcm::prelude` imports for coretime stuff
- changes `query_acceptable_payment_assets` /
`query_weight_to_asset_fee` implementations to be more resilient to the
XCM version change
- adds `xcm_fee_payment_runtime_api::XcmPaymentApi` to the
AssetHubRococo/Westend exposing a native token as acceptable payment
asset

Continuation of: paritytech#3607

Closes: paritytech#4297

## Possible follow-ups

- [ ] add all sufficient assets (`Assets`, `ForeignAssets`) as
acceptable payment assets ?
fgamundi pushed a commit to moondance-labs/polkadot-sdk that referenced this pull request Jul 17, 2024
…i` nits (paritytech#4355)

This PR:
- changes `xcm::v4` to `xcm::prelude` imports for coretime stuff
- changes `query_acceptable_payment_assets` /
`query_weight_to_asset_fee` implementations to be more resilient to the
XCM version change
- adds `xcm_fee_payment_runtime_api::XcmPaymentApi` to the
AssetHubRococo/Westend exposing a native token as acceptable payment
asset

Continuation of: paritytech#3607

Closes: paritytech#4297

## Possible follow-ups

- [ ] add all sufficient assets (`Assets`, `ForeignAssets`) as
acceptable payment assets ?
girazoki pushed a commit to moondance-labs/polkadot-sdk that referenced this pull request Aug 1, 2024
…i` nits (paritytech#4355)

This PR:
- changes `xcm::v4` to `xcm::prelude` imports for coretime stuff
- changes `query_acceptable_payment_assets` /
`query_weight_to_asset_fee` implementations to be more resilient to the
XCM version change
- adds `xcm_fee_payment_runtime_api::XcmPaymentApi` to the
AssetHubRococo/Westend exposing a native token as acceptable payment
asset

Continuation of: paritytech#3607

Closes: paritytech#4297

## Possible follow-ups

- [ ] add all sufficient assets (`Assets`, `ForeignAssets`) as
acceptable payment assets ?
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…i` nits (paritytech#4355)

This PR:
- changes `xcm::v4` to `xcm::prelude` imports for coretime stuff
- changes `query_acceptable_payment_assets` /
`query_weight_to_asset_fee` implementations to be more resilient to the
XCM version change
- adds `xcm_fee_payment_runtime_api::XcmPaymentApi` to the
AssetHubRococo/Westend exposing a native token as acceptable payment
asset

Continuation of: paritytech#3607

Closes: paritytech#4297

## Possible follow-ups

- [ ] add all sufficient assets (`Assets`, `ForeignAssets`) as
acceptable payment assets ?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Investigate usage of xcm::v4
4 participants