From fecbf128c5dcb9b1ddff8fde56dab0f34cfcc7af Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 21 Mar 2023 10:27:43 +0200 Subject: [PATCH 1/3] XCM ExportMessage benchmark support Signed-off-by: acatangiu --- runtime/kusama/src/lib.rs | 5 +++++ runtime/kusama/src/weights/xcm/mod.rs | 3 ++- runtime/rococo/src/lib.rs | 5 +++++ runtime/rococo/src/weights/xcm/mod.rs | 3 ++- runtime/westend/src/lib.rs | 9 +++++++-- runtime/westend/src/weights/xcm/mod.rs | 2 +- .../src/generic/benchmarking.rs | 17 +++++++++++++++++ xcm/pallet-xcm-benchmarks/src/generic/mock.rs | 5 +++++ xcm/pallet-xcm-benchmarks/src/generic/mod.rs | 10 +++++++++- xcm/xcm-executor/src/traits/export.rs | 2 +- 10 files changed, 54 insertions(+), 7 deletions(-) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index f514504bef70..965c5e4d5d06 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -2079,6 +2079,11 @@ sp_api::impl_runtime_apis! { // Kusama doesn't support asset locking Err(BenchmarkError::Skip) } + + fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + // Kusama doesn't support exporting messages + Err(BenchmarkError::Skip) + } } let whitelist: Vec = vec![ diff --git a/runtime/kusama/src/weights/xcm/mod.rs b/runtime/kusama/src/weights/xcm/mod.rs index 1a3630ac243e..09c6bfa3a37a 100644 --- a/runtime/kusama/src/weights/xcm/mod.rs +++ b/runtime/kusama/src/weights/xcm/mod.rs @@ -243,7 +243,8 @@ impl XcmWeightInfo for KusamaXcmWeight { Weight::MAX } fn export_message(_: &NetworkId, _: &Junctions, _: &Xcm<()>) -> Weight { - Weight::MAX // todo fix + // Kusama relay should not support export message operations + Weight::MAX } fn lock_asset(_: &MultiAsset, _: &MultiLocation) -> Weight { // Kusama does not currently support asset locking operations diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index cdf60a5a477b..2f4de9d16079 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -2087,6 +2087,11 @@ sp_api::impl_runtime_apis! { // Rococo doesn't support asset locking Err(BenchmarkError::Skip) } + + fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + // Rococo doesn't support exporting messages + Err(BenchmarkError::Skip) + } } let whitelist: Vec = vec![ diff --git a/runtime/rococo/src/weights/xcm/mod.rs b/runtime/rococo/src/weights/xcm/mod.rs index fc430805b5b8..fe894d3ad2e8 100644 --- a/runtime/rococo/src/weights/xcm/mod.rs +++ b/runtime/rococo/src/weights/xcm/mod.rs @@ -243,7 +243,8 @@ impl XcmWeightInfo for RococoXcmWeight { Weight::MAX } fn export_message(_: &NetworkId, _: &Junctions, _: &Xcm<()>) -> Weight { - Weight::MAX // todo fix + // Rococo relay should not support export message operations + Weight::MAX } fn lock_asset(_: &MultiAsset, _: &MultiLocation) -> Weight { // Rococo does not currently support asset locking operations diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 12de36fb7954..18722b1c1c4d 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1741,8 +1741,8 @@ sp_api::impl_runtime_apis! { impl runtime_parachains::disputes::slashing::benchmarking::Config for Runtime {} use xcm::latest::{ - AssetId::*, Fungibility::*, Junction, Junctions::*, MultiAsset, MultiAssets, - MultiLocation, Response, + AssetId::*, Fungibility::*, InteriorMultiLocation, Junction, Junctions::*, + MultiAsset, MultiAssets, MultiLocation, NetworkId, Response, }; use xcm_config::{Westmint, TokenLocation}; @@ -1818,6 +1818,11 @@ sp_api::impl_runtime_apis! { // Westend doesn't support asset locking Err(BenchmarkError::Skip) } + + fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + // Westend doesn't support exporting messages + Err(BenchmarkError::Skip) + } } type XcmBalances = pallet_xcm_benchmarks::fungible::Pallet::; diff --git a/runtime/westend/src/weights/xcm/mod.rs b/runtime/westend/src/weights/xcm/mod.rs index 8b8c5339702d..4fb27c12096a 100644 --- a/runtime/westend/src/weights/xcm/mod.rs +++ b/runtime/westend/src/weights/xcm/mod.rs @@ -246,7 +246,7 @@ impl XcmWeightInfo for WestendXcmWeight { Weight::MAX } fn export_message(_: &NetworkId, _: &Junctions, _: &Xcm<()>) -> Weight { - // Westend does not currently support export message operations + // Westend relay should not support export message operations Weight::MAX } fn lock_asset(_: &MultiAsset, _: &MultiLocation) -> Weight { diff --git a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index 3976c482eb8d..6fc501d1cdb7 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -496,6 +496,23 @@ benchmarks! { assert_eq!(executor.origin(), &Some(X1(alias).relative_to(&universal_location))); } + export_message { + let mut executor = new_executor::(Here.into_location()); + let (network, destination) = T::bridged_destination()?; + let expected_message = Xcm(vec![TransferAsset { + assets: (Here, 100u128).into(), + beneficiary: Parachain(2).into(), + }]); + let xcm = Xcm(vec![ExportMessage { + network, destination, xcm: expected_message.clone(), + }]); + }: { + executor.bench_process(xcm)?; + } verify { + // The execute completing successfully is as good as we can check. + // TODO: Potentially add new trait to XcmSender to detect a queued outgoing message. #4426 + } + set_fees_mode { let mut executor = new_executor::(Default::default()); executor.set_fees_mode(FeesMode { jit_withdraw: false }); diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs index 79f9f28e32f5..e0a9206ce214 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs @@ -186,6 +186,11 @@ impl generic::Config for Test { let assets: MultiAsset = (Concrete(Here.into()), 100).into(); Ok((Default::default(), Default::default(), assets)) } + + fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + // No MessageExporter in tests + Err(BenchmarkError::Skip) + } } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mod.rs b/xcm/pallet-xcm-benchmarks/src/generic/mod.rs index 8fee41142fa8..fefd8d73c0d6 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/mod.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/mod.rs @@ -28,7 +28,10 @@ pub mod pallet { dispatch::{Dispatchable, GetDispatchInfo}, pallet_prelude::Encode, }; - use xcm::latest::{Junction, MultiAsset, MultiAssets, MultiLocation, Response}; + use xcm::latest::{ + InteriorMultiLocation, Junction, MultiAsset, MultiAssets, MultiLocation, NetworkId, + Response, + }; #[pallet::config] pub trait Config: frame_system::Config + crate::Config { @@ -71,6 +74,11 @@ pub mod pallet { /// Return an unlocker, owner and assets that can be locked and unlocked. fn unlockable_asset() -> Result<(MultiLocation, MultiLocation, MultiAsset), BenchmarkError>; + + /// A valid `(NetworkId, InteriorMultiLocation)` we can successfully export message to. + /// + /// If set to `Err`, benchmarks which rely on `export_message` will be skipped. + fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError>; } #[pallet::pallet] diff --git a/xcm/xcm-executor/src/traits/export.rs b/xcm/xcm-executor/src/traits/export.rs index 61b76addfe4c..ef8daa3c828d 100644 --- a/xcm/xcm-executor/src/traits/export.rs +++ b/xcm/xcm-executor/src/traits/export.rs @@ -31,7 +31,7 @@ use xcm::latest::prelude::*; /// destination must accept the local location to represent that location or the operation will /// fail. pub trait ExportXcm { - /// Intermediate value which connects the two phaases of the export operation. + /// Intermediate value which connects the two phases of the export operation. type Ticket; /// Check whether the given `message` is deliverable to the given `destination` on `network`, From c0c5d556e1c630f327751124309f854c6fbbebb5 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 21 Mar 2023 14:28:28 +0200 Subject: [PATCH 2/3] address review comments --- runtime/kusama/src/lib.rs | 3 ++- runtime/rococo/src/lib.rs | 3 ++- runtime/westend/src/lib.rs | 3 ++- .../src/generic/benchmarking.rs | 12 +++++------- xcm/pallet-xcm-benchmarks/src/generic/mock.rs | 3 ++- xcm/pallet-xcm-benchmarks/src/generic/mod.rs | 5 +++-- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 965c5e4d5d06..860868f6e116 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -2080,7 +2080,8 @@ sp_api::impl_runtime_apis! { Err(BenchmarkError::Skip) } - fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + fn export_message_origin_and_destination( + ) -> Result<(MultiLocation, NetworkId, InteriorMultiLocation), BenchmarkError> { // Kusama doesn't support exporting messages Err(BenchmarkError::Skip) } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 2f4de9d16079..3cedcf8a752c 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -2088,7 +2088,8 @@ sp_api::impl_runtime_apis! { Err(BenchmarkError::Skip) } - fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + fn export_message_origin_and_destination( + ) -> Result<(MultiLocation, NetworkId, InteriorMultiLocation), BenchmarkError> { // Rococo doesn't support exporting messages Err(BenchmarkError::Skip) } diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 18722b1c1c4d..b71f51163b5b 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1819,7 +1819,8 @@ sp_api::impl_runtime_apis! { Err(BenchmarkError::Skip) } - fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + fn export_message_origin_and_destination( + ) -> Result<(MultiLocation, NetworkId, InteriorMultiLocation), BenchmarkError> { // Westend doesn't support exporting messages Err(BenchmarkError::Skip) } diff --git a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index 6fc501d1cdb7..063b5b547ee4 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -497,14 +497,12 @@ benchmarks! { } export_message { - let mut executor = new_executor::(Here.into_location()); - let (network, destination) = T::bridged_destination()?; - let expected_message = Xcm(vec![TransferAsset { - assets: (Here, 100u128).into(), - beneficiary: Parachain(2).into(), - }]); + let (origin, network, destination) = T::export_message_origin_and_destination()?; + let mut executor = new_executor::(origin); + // Actual exported message has no bearing on this instruction's weight, use empty message. + let inner_xcm = Xcm(vec![]); let xcm = Xcm(vec![ExportMessage { - network, destination, xcm: expected_message.clone(), + network, destination, xcm: inner_xcm, }]); }: { executor.bench_process(xcm)?; diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs index e0a9206ce214..c76a9f275983 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/mock.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/mock.rs @@ -187,7 +187,8 @@ impl generic::Config for Test { Ok((Default::default(), Default::default(), assets)) } - fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError> { + fn export_message_origin_and_destination( + ) -> Result<(MultiLocation, NetworkId, InteriorMultiLocation), BenchmarkError> { // No MessageExporter in tests Err(BenchmarkError::Skip) } diff --git a/xcm/pallet-xcm-benchmarks/src/generic/mod.rs b/xcm/pallet-xcm-benchmarks/src/generic/mod.rs index fefd8d73c0d6..b12ac0ba2308 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/mod.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/mod.rs @@ -75,10 +75,11 @@ pub mod pallet { /// Return an unlocker, owner and assets that can be locked and unlocked. fn unlockable_asset() -> Result<(MultiLocation, MultiLocation, MultiAsset), BenchmarkError>; - /// A valid `(NetworkId, InteriorMultiLocation)` we can successfully export message to. + /// A `(MultiLocation, NetworkId, InteriorMultiLocation)` we can successfully export message to. /// /// If set to `Err`, benchmarks which rely on `export_message` will be skipped. - fn bridged_destination() -> Result<(NetworkId, InteriorMultiLocation), BenchmarkError>; + fn export_message_origin_and_destination( + ) -> Result<(MultiLocation, NetworkId, InteriorMultiLocation), BenchmarkError>; } #[pallet::pallet] From be7a2b4d29b5e8211304e39c8ec40e279a0638dc Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 22 Mar 2023 15:15:31 +0200 Subject: [PATCH 3/3] include inner-xcm in ExportMessage benchmark --- xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs | 10 ++++++++-- xcm/xcm-executor/src/lib.rs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs index 063b5b547ee4..19aecdd47c89 100644 --- a/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs +++ b/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs @@ -497,10 +497,16 @@ benchmarks! { } export_message { + let x in 1 .. 1000; + // The `inner_xcm` influences `ExportMessage` total weight based on + // `inner_xcm.encoded_size()`, so for this benchmark use smallest encoded instruction + // to approximate weight per "unit" of encoded size; then actual weight can be estimated + // to be `inner_xcm.encoded_size() * benchmarked_unit`. + // Use `ClearOrigin` as the small encoded instruction. + let inner_xcm = Xcm(vec![ClearOrigin; x as usize]); + // Get `origin`, `network` and `destination` from configured runtime. let (origin, network, destination) = T::export_message_origin_and_destination()?; let mut executor = new_executor::(origin); - // Actual exported message has no bearing on this instruction's weight, use empty message. - let inner_xcm = Xcm(vec![]); let xcm = Xcm(vec![ExportMessage { network, destination, xcm: inner_xcm, }]); diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index c4d3e2768aae..4e63ad91479f 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -820,7 +820,7 @@ impl XcmExecutor { Ok(()) }, ExportMessage { network, destination, xcm } => { - // The actual message send to the bridge for forwarding is prepended with `UniversalOrigin` + // The actual message sent to the bridge for forwarding is prepended with `UniversalOrigin` // and `DescendOrigin` in order to ensure that the message is executed with this Origin. // // Prepend the desired message with instructions which effectively rewrite the origin.