Skip to content

Commit

Permalink
Fix XcmPaymentApi::query_weight_to_asset_fee version conversion (#6459
Browse files Browse the repository at this point in the history 
)

The `query_weight_to_asset_fee` function was trying to convert versions
by using `try_as`, this function [doesn't convert from a versioned to a
concrete
type](https://github.com/paritytech/polkadot-sdk/blob/0156ca8f959d5cf3787c18113ce48acaaf1a8345/polkadot/xcm/src/lib.rs#L131).
This would cause all calls with a lower version to fail.

The correct function to use is the good old
[try_into](https://github.com/paritytech/polkadot-sdk/blob/0156ca8f959d5cf3787c18113ce48acaaf1a8345/polkadot/xcm/src/lib.rs#L184).
Now those calls work :)

---------

Co-authored-by: command-bot <>
Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
  • Loading branch information
@franciscoaguirre @bkontur @actions-user
3 people authored Nov 26, 2024
1 parent fc315ac commit 139691b
Show file tree
Hide file tree
Showing 36 changed files with 483 additions and 82 deletions.
13 changes: 13 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cumulus/parachains/runtimes/assets/asset-hub-rococo/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ snowbridge-router-primitives = { workspace = true }

[dev-dependencies]
asset-test-utils = { workspace = true, default-features = true }
parachains-runtimes-test-utils = { workspace = true, default-features = true }

[build-dependencies]
substrate-wasm-builder = { optional = true, workspace = true, default-features = true }
Expand Down
28 changes: 11 additions & 17 deletions cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1415,37 +1415,31 @@ impl_runtime_apis! {
// We accept the native token to pay fees.
let mut acceptable_assets = vec![AssetId(native_token.clone())];
// We also accept all assets in a pool with the native token.
let assets_in_pool_with_native = assets_common::get_assets_in_pool_with::<
Runtime,
xcm::v5::Location
>(&native_token).map_err(|()| XcmPaymentApiError::VersionedConversionFailed)?.into_iter();
acceptable_assets.extend(assets_in_pool_with_native);
acceptable_assets.extend(
assets_common::PoolAdapter::<Runtime>::get_assets_in_pool_with(native_token)
.map_err(|()| XcmPaymentApiError::VersionedConversionFailed)?
);
PolkadotXcm::query_acceptable_payment_assets(xcm_version, acceptable_assets)
}

fn query_weight_to_asset_fee(weight: Weight, asset: VersionedAssetId) -> Result<u128, XcmPaymentApiError> {
let native_asset = xcm_config::TokenLocation::get();
let fee_in_native = WeightToFee::weight_to_fee(&weight);
match asset.try_as::<AssetId>() {
let latest_asset_id: Result<AssetId, ()> = asset.clone().try_into();
match latest_asset_id {
Ok(asset_id) if asset_id.0 == native_asset => {
// for native token
Ok(fee_in_native)
},
Ok(asset_id) => {
let assets_in_pool_with_this_asset: Vec<_> = assets_common::get_assets_in_pool_with::<
Runtime,
xcm::v5::Location
>(&asset_id.0).map_err(|()| XcmPaymentApiError::VersionedConversionFailed)?;
if assets_in_pool_with_this_asset
.into_iter()
.map(|asset_id| asset_id.0)
.any(|location| location == native_asset) {
pallet_asset_conversion::Pallet::<Runtime>::quote_price_tokens_for_exact_tokens(
asset_id.clone().0,
// Try to get current price of `asset_id` in `native_asset`.
if let Ok(Some(swapped_in_native)) = assets_common::PoolAdapter::<Runtime>::quote_price_tokens_for_exact_tokens(
asset_id.0.clone(),
native_asset,
fee_in_native,
true, // We include the fee.
).ok_or(XcmPaymentApiError::AssetNotFound)
) {
Ok(swapped_in_native)
} else {
log::trace!(target: "xcm::xcm_runtime_apis", "query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!");
Err(XcmPaymentApiError::AssetNotFound)
Expand Down
24 changes: 20 additions & 4 deletions cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ use asset_hub_rococo_runtime::{
ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot,
TokenLocation, TrustBackedAssetsPalletLocation, XcmConfig,
},
AllPalletsWithoutSystem, AssetConversion, AssetDeposit, Assets, Balances, CollatorSelection,
ExistentialDeposit, ForeignAssets, ForeignAssetsInstance, MetadataDepositBase,
MetadataDepositPerByte, ParachainSystem, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin,
SessionKeys, TrustBackedAssetsInstance, XcmpQueue,
AllPalletsWithoutSystem, AssetConversion, AssetDeposit, Assets, Balances, Block,
CollatorSelection, ExistentialDeposit, ForeignAssets, ForeignAssetsInstance,
MetadataDepositBase, MetadataDepositPerByte, ParachainSystem, Runtime, RuntimeCall,
RuntimeEvent, RuntimeOrigin, SessionKeys, TrustBackedAssetsInstance, XcmpQueue,
};
use asset_test_utils::{
test_cases_over_bridge::TestBridgingConfig, CollatorSessionKey, CollatorSessionKeys,
Expand Down Expand Up @@ -1471,3 +1471,19 @@ fn location_conversion_works() {
assert_eq!(got, expected, "{}", tc.description);
}
}

#[test]
fn xcm_payment_api_works() {
parachains_runtimes_test_utils::test_cases::xcm_payment_api_with_native_token_works::<
Runtime,
RuntimeCall,
RuntimeOrigin,
Block,
>();
asset_test_utils::test_cases::xcm_payment_api_with_pools_works::<
Runtime,
RuntimeCall,
RuntimeOrigin,
Block,
>();
}
1 change: 1 addition & 0 deletions cumulus/parachains/runtimes/assets/asset-hub-westend/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ snowbridge-router-primitives = { workspace = true }

[dev-dependencies]
asset-test-utils = { workspace = true, default-features = true }
parachains-runtimes-test-utils = { workspace = true, default-features = true }

[build-dependencies]
substrate-wasm-builder = { optional = true, workspace = true, default-features = true }
Expand Down
29 changes: 11 additions & 18 deletions cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1528,38 +1528,31 @@ impl_runtime_apis! {
// We accept the native token to pay fees.
let mut acceptable_assets = vec![AssetId(native_token.clone())];
// We also accept all assets in a pool with the native token.
let assets_in_pool_with_native = assets_common::get_assets_in_pool_with::<
Runtime,
xcm::v5::Location
>(&native_token).map_err(|()| XcmPaymentApiError::VersionedConversionFailed)?.into_iter();
acceptable_assets.extend(assets_in_pool_with_native);
acceptable_assets.extend(
assets_common::PoolAdapter::<Runtime>::get_assets_in_pool_with(native_token)
.map_err(|()| XcmPaymentApiError::VersionedConversionFailed)?
);
PolkadotXcm::query_acceptable_payment_assets(xcm_version, acceptable_assets)
}

fn query_weight_to_asset_fee(weight: Weight, asset: VersionedAssetId) -> Result<u128, XcmPaymentApiError> {
let native_asset = xcm_config::WestendLocation::get();
let fee_in_native = WeightToFee::weight_to_fee(&weight);
match asset.try_as::<AssetId>() {
let latest_asset_id: Result<AssetId, ()> = asset.clone().try_into();
match latest_asset_id {
Ok(asset_id) if asset_id.0 == native_asset => {
// for native asset
Ok(fee_in_native)
},
Ok(asset_id) => {
// We recognize assets in a pool with the native one.
let assets_in_pool_with_this_asset: Vec<_> = assets_common::get_assets_in_pool_with::<
Runtime,
xcm::v5::Location
>(&asset_id.0).map_err(|()| XcmPaymentApiError::VersionedConversionFailed)?;
if assets_in_pool_with_this_asset
.into_iter()
.map(|asset_id| asset_id.0)
.any(|location| location == native_asset) {
pallet_asset_conversion::Pallet::<Runtime>::quote_price_tokens_for_exact_tokens(
asset_id.clone().0,
// Try to get current price of `asset_id` in `native_asset`.
if let Ok(Some(swapped_in_native)) = assets_common::PoolAdapter::<Runtime>::quote_price_tokens_for_exact_tokens(
asset_id.0.clone(),
native_asset,
fee_in_native,
true, // We include the fee.
).ok_or(XcmPaymentApiError::AssetNotFound)
) {
Ok(swapped_in_native)
} else {
log::trace!(target: "xcm::xcm_runtime_apis", "query_weight_to_asset_fee - unhandled asset_id: {asset_id:?}!");
Err(XcmPaymentApiError::AssetNotFound)
Expand Down
18 changes: 17 additions & 1 deletion cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use asset_hub_westend_runtime::{
ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot,
TrustBackedAssetsPalletLocation, WestendLocation, XcmConfig,
},
AllPalletsWithoutSystem, Assets, Balances, ExistentialDeposit, ForeignAssets,
AllPalletsWithoutSystem, Assets, Balances, Block, ExistentialDeposit, ForeignAssets,
ForeignAssetsInstance, MetadataDepositBase, MetadataDepositPerByte, ParachainSystem,
PolkadotXcm, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, SessionKeys,
TrustBackedAssetsInstance, XcmpQueue,
Expand Down Expand Up @@ -1446,3 +1446,19 @@ fn location_conversion_works() {
assert_eq!(got, expected, "{}", tc.description);
}
}

#[test]
fn xcm_payment_api_works() {
parachains_runtimes_test_utils::test_cases::xcm_payment_api_with_native_token_works::<
Runtime,
RuntimeCall,
RuntimeOrigin,
Block,
>();
asset_test_utils::test_cases::xcm_payment_api_with_pools_works::<
Runtime,
RuntimeCall,
RuntimeOrigin,
Block,
>();
}
79 changes: 58 additions & 21 deletions cumulus/parachains/runtimes/assets/common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ extern crate alloc;
use crate::matching::{LocalLocationPattern, ParentLocation};
use alloc::vec::Vec;
use codec::{Decode, EncodeLike};
use core::cmp::PartialEq;
use core::{cmp::PartialEq, marker::PhantomData};
use frame_support::traits::{Equals, EverythingBut};
use parachains_common::{AssetIdForTrustBackedAssets, CollectionId, ItemId};
use sp_runtime::traits::TryConvertInto;
Expand Down Expand Up @@ -137,24 +137,62 @@ pub type PoolAssetsConvertedConcreteId<PoolAssetsPalletLocation, Balance> =
TryConvertInto,
>;

/// Returns an iterator of all assets in a pool with `asset`.
///
/// Should only be used in runtime APIs since it iterates over the whole
/// `pallet_asset_conversion::Pools` map.
///
/// It takes in any version of an XCM Location but always returns the latest one.
/// This is to allow some margin of migrating the pools when updating the XCM version.
///
/// An error of type `()` is returned if the version conversion fails for XCM locations.
/// This error should be mapped by the caller to a more descriptive one.
pub fn get_assets_in_pool_with<
Runtime: pallet_asset_conversion::Config<PoolId = (L, L)>,
L: TryInto<Location> + Clone + Decode + EncodeLike + PartialEq,
>(
asset: &L,
) -> Result<Vec<AssetId>, ()> {
pallet_asset_conversion::Pools::<Runtime>::iter_keys()
.filter_map(|(asset_1, asset_2)| {
/// Adapter implementation for accessing pools (`pallet_asset_conversion`) that uses `AssetKind` as
/// a `xcm::v*` which could be different from the `xcm::latest`.
pub struct PoolAdapter<Runtime>(PhantomData<Runtime>);
impl<
Runtime: pallet_asset_conversion::Config<PoolId = (L, L), AssetKind = L>,
L: TryFrom<Location> + TryInto<Location> + Clone + Decode + EncodeLike + PartialEq,
> PoolAdapter<Runtime>
{
/// Returns a vector of all assets in a pool with `asset`.
///
/// Should only be used in runtime APIs since it iterates over the whole
/// `pallet_asset_conversion::Pools` map.
///
/// It takes in any version of an XCM Location but always returns the latest one.
/// This is to allow some margin of migrating the pools when updating the XCM version.
///
/// An error of type `()` is returned if the version conversion fails for XCM locations.
/// This error should be mapped by the caller to a more descriptive one.
pub fn get_assets_in_pool_with(asset: Location) -> Result<Vec<AssetId>, ()> {
// convert latest to the `L` version.
let asset: L = asset.try_into().map_err(|_| ())?;
Self::iter_assets_in_pool_with(&asset)
.map(|location| {
// convert `L` to the latest `AssetId`
location.try_into().map_err(|_| ()).map(AssetId)
})
.collect::<Result<Vec<_>, _>>()
}

/// Provides a current prices. Wrapper over
/// `pallet_asset_conversion::Pallet::<T>::quote_price_tokens_for_exact_tokens`.
///
/// An error of type `()` is returned if the version conversion fails for XCM locations.
/// This error should be mapped by the caller to a more descriptive one.
pub fn quote_price_tokens_for_exact_tokens(
asset_1: Location,
asset_2: Location,
amount: Runtime::Balance,
include_fees: bool,
) -> Result<Option<Runtime::Balance>, ()> {
// Convert latest to the `L` version.
let asset_1: L = asset_1.try_into().map_err(|_| ())?;
let asset_2: L = asset_2.try_into().map_err(|_| ())?;

// Quote swap price.
Ok(pallet_asset_conversion::Pallet::<Runtime>::quote_price_tokens_for_exact_tokens(
asset_1,
asset_2,
amount,
include_fees,
))
}

/// Helper function for filtering pool.
pub fn iter_assets_in_pool_with(asset: &L) -> impl Iterator<Item = L> + '_ {
pallet_asset_conversion::Pools::<Runtime>::iter_keys().filter_map(|(asset_1, asset_2)| {
if asset_1 == *asset {
Some(asset_2)
} else if asset_2 == *asset {
Expand All @@ -163,8 +201,7 @@ pub fn get_assets_in_pool_with<
None
}
})
.map(|location| location.try_into().map_err(|_| ()).map(AssetId))
.collect::<Result<Vec<_>, _>>()
}
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 139691b

Please sign in to comment.