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

Switch runtimes benchmarking for delivering to sibling parachains instead of Parent #7321

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

EleisonC
Copy link

Title: Update XCM benchmarks for sibling parachain delivery (closes #7211)

Description:

This PR updates XCM benchmarking configurations for testnet system parachains to reflect delivery to sibling parachains instead of the Parent relay chain.

Integration:

Replaced ToParentDeliveryHelper with ToParachainDeliveryHelper.

Updated benchmark destinations

@EleisonC EleisonC changed the title switch destination Parent to AH Switch runtimes benchmarking for delivering to sibling parachains instead of Parent Jan 23, 2025
@EleisonC
Copy link
Author

Hey @bkontur

I am Kindly requesting for your feedback on this. PR
runtime - assets hub westend

Copy link
Contributor

@bkontur bkontur left a comment

Choose a reason for hiding this comment

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

the point here for AssetHubWestend was to change location Parent to the RandomParaLocation for delivery (but only those Parent which represents destination and not the AssetId)

@@ -825,7 +825,7 @@ impl cumulus_pallet_aura_ext::Config for Runtime {}

parameter_types! {
/// The asset ID for the asset that we use to pay for message delivery fees.
pub FeeAssetId: AssetId = AssetId(xcm_config::WestendLocation::get());
pub FeeAssetId: AssetId = AssetId(xcm_config::AssetHubWestend::get());
Copy link
Contributor

Choose a reason for hiding this comment

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

no change here, WestendLocation means Location { parents: 1, interior: Here } which represents relaychain native token from the parachain's perspective, which is WND (or DOT for Polkadot, KSM for Kusama...).

Suggested change
pub FeeAssetId: AssetId = AssetId(xcm_config::AssetHubWestend::get());
pub FeeAssetId: AssetId = AssetId(xcm_config::WestendLocation::get());

impl cumulus_pallet_session_benchmarking::Config for Runtime {}

parameter_types! {
pub ExistentialDepositAsset: Option<Asset> = Some((
WestendLocation::get(),
AssetHubWestend::get(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
AssetHubWestend::get(),
WestendLocation::get(),

ExistentialDeposit::get()
).into());
pub const RandomParaId: ParaId = ParaId::new(43211234);
pub const SiblingAssetHubId: ParaId = ParaId::new(AssetHubId::get());
Copy link
Contributor

Choose a reason for hiding this comment

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

AssetHub does not know any other sibling AssetHub, RandomParaId is good here

Suggested change
pub const SiblingAssetHubId: ParaId = ParaId::new(AssetHubId::get());
pub const RandomParaId: ParaId = ParaId::new(43211234);
pub RandomParaLocation: Location = ParentThen(Parachain(RandomParaId::get().into()).into()).into();

@@ -1893,34 +1893,34 @@ impl_runtime_apis! {
xcm_config::XcmConfig,
ExistentialDepositAsset,
PriceForSiblingParachainDelivery,
RandomParaId,
SiblingAssetHubId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
SiblingAssetHubId,
RandomParaId,

ParachainSystem,
>
);

fn reachable_dest() -> Option<Location> {
Some(Parent.into())
Some(AssetHubWestend::get())
Copy link
Contributor

Choose a reason for hiding this comment

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

please, everywhere where you changed Location (location and not assetId) Parent to AssetHubWestend, you need to change Parent tot RandomParaLocation

Suggested change
Some(AssetHubWestend::get())
Some(RandomParaLocation::get())

xcm_config::XcmConfig,
ExistentialDepositAsset,
PriceForSiblingParachainDelivery,
SiblingAssetHubId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
SiblingAssetHubId,
RandomParaId,

fn valid_destination() -> Result<Location, BenchmarkError> {
Ok(WestendLocation::get())
Ok(AssetHubWestend::get())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Ok(AssetHubWestend::get())
Ok(RandomParaLocation::get())

@@ -2042,7 +2043,7 @@ impl_runtime_apis! {
}
})
.chain(core::iter::once(Asset { id: AssetId(Here.into()), fun: Fungible(u128::MAX) }))
.chain(core::iter::once(Asset { id: AssetId(WestendLocation::get()), fun: Fungible(1_000_000 * UNITS) }))
.chain(core::iter::once(Asset { id: AssetId(AssetHubWestend::get()), fun: Fungible(1_000_000 * UNITS) }))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.chain(core::iter::once(Asset { id: AssetId(AssetHubWestend::get()), fun: Fungible(1_000_000 * UNITS) }))
.chain(core::iter::once(Asset { id: AssetId(WestendLocation::get()), fun: Fungible(1_000_000 * UNITS) }))

Comment on lines +2058 to +2059
AssetHubWestend::get(),
Asset { fun: Fungible(UNITS), id: AssetId(AssetHubWestend::get()) },
Copy link
Contributor

Choose a reason for hiding this comment

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

we want to simulate scenario, where RandomParaLocation can teleport us native token aka WND aka WestendLocation

Suggested change
AssetHubWestend::get(),
Asset { fun: Fungible(UNITS), id: AssetId(AssetHubWestend::get()) },
RandomParaLocation::get(),
Asset { fun: Fungible(UNITS), id: AssetId(WestendLocation::get()) },

Comment on lines +67 to +68
pub const AssetHubId: u32 = 1000;
pub const AssetHubWestend: Location = Parachain(AssetHubId::get()).into();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub const AssetHubId: u32 = 1000;
pub const AssetHubWestend: Location = Parachain(AssetHubId::get()).into();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup testnet system parachain runtimes benchmarking for delivering to sibling parachains instead of Parent
2 participants