Skip to content

Commit e3c97e4

Browse files
authored
[xcm] Use Weight::MAX for reserve_asset_deposited, receive_teleported_asset benchmarks (#1726)
# Description ## Summary Previously, the `pallet_xcm::do_reserve_transfer_assets` and `pallet_xcm::do_teleport_assets` functions relied on weight estimation for remote chain execution, which was based on guesswork derived from the local chain. This approach led to complications for runtimes that did not provide or support specific [XCM configurations](https://github.com/paritytech/polkadot-sdk/blob/7cbe0c76ef8fd2aabf9f07de0156941ce3ed44b0/polkadot/xcm/xcm-executor/src/config.rs#L43-L47) for `IsReserve` or `IsTeleporter`. Consequently, such runtimes had to resort to implementing hard-coded weights for XCM instructions like `reserve_asset_deposited` or `receive_teleported_asset` to support extrinsics such as `pallet_xcm::reserve_transfer_assets` and `pallet_xcm::teleport_assets`, which depended on remote weight estimation. The issue of remote weight estimation was addressed and resolved by [Pull Request #1645](#1645), which removed the need for remote weight estimation. ## Solution As a continuation of this improvement, the current PR proposes further cleanup by removing unnecessary hard-coded values and rectifying benchmark results with `Weight::MAX` that previously used `T::BlockWeights::get().max_block` as an override for unsupported XCM instructions like `ReserveAssetDeposited` and `ReceiveTeleportedAsset`. ## Questions - [x] Can we remove now also `Hardcoded till the XCM pallet is fixed` for `deposit_asset`? E.g. for AssetHubKusama [here](https://github.com/paritytech/polkadot-sdk/blob/7cbe0c76ef8fd2aabf9f07de0156941ce3ed44b0/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/weights/xcm/mod.rs#L129-L134) - [x] Are comments like [this](https://github.com/paritytech/polkadot-sdk/blob/7cbe0c76ef8fd2aabf9f07de0156941ce3ed44b0/polkadot/runtime/kusama/src/weights/xcm/mod.rs#L94) `// Kusama doesn't support ReserveAssetDeposited, so this benchmark has a default weight` still relevant? Shouldnt be removed/changed? ## TODO - [x] `bench bot` regenerate xcm weights for all runtimes - [x] remove hard-coded stuff from system parachain weight files - [ ] when merged, open `polkadot-fellow/runtimes` PR ## References Fixes #1132 Closes #1132 Old polkadot repo [PR](paritytech/polkadot#7546) --------- Co-authored-by: command-bot <>
1 parent ebf4423 commit e3c97e4

File tree

21 files changed

+381
-454
lines changed

21 files changed

+381
-454
lines changed

cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/reserve_transfer.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,8 @@ fn relay_origin_assertions(t: RelayToSystemParaTest) {
3535
);
3636
}
3737

38-
fn system_para_dest_assertions_incomplete(_t: RelayToSystemParaTest) {
39-
AssetHubWestend::assert_dmp_queue_incomplete(
40-
Some(Weight::from_parts(1_000_000_000, 0)),
41-
Some(Error::UntrustedReserveLocation),
42-
);
38+
fn system_para_dest_assertions(_t: RelayToSystemParaTest) {
39+
AssetHubWestend::assert_dmp_queue_error(Error::WeightNotComputable);
4340
}
4441

4542
fn system_para_to_relay_assertions(_t: SystemParaToRelayTest) {
@@ -178,7 +175,7 @@ fn limited_reserve_transfer_native_asset_from_relay_to_system_para_fails() {
178175
let receiver_balance_before = test.receiver.balance;
179176

180177
test.set_assertion::<Westend>(relay_origin_assertions);
181-
test.set_assertion::<AssetHubWestend>(system_para_dest_assertions_incomplete);
178+
test.set_assertion::<AssetHubWestend>(system_para_dest_assertions);
182179
test.set_dispatchable::<Westend>(relay_limited_reserve_transfer_assets);
183180
test.assert();
184181

@@ -237,7 +234,7 @@ fn reserve_transfer_native_asset_from_relay_to_system_para_fails() {
237234
let receiver_balance_before = test.receiver.balance;
238235

239236
test.set_assertion::<Westend>(relay_origin_assertions);
240-
test.set_assertion::<AssetHubWestend>(system_para_dest_assertions_incomplete);
237+
test.set_assertion::<AssetHubWestend>(system_para_dest_assertions);
241238
test.set_dispatchable::<Westend>(relay_reserve_transfer_assets);
242239
test.assert();
243240

cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/send.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ fn send_xcm_from_para_to_system_para_paying_fee_with_assets_works() {
119119
type RuntimeEvent = <AssetHubWestend as Chain>::RuntimeEvent;
120120

121121
AssetHubWestend::assert_xcmp_queue_success(Some(Weight::from_parts(
122-
2_176_414_000,
123-
203_593,
122+
16_290_336_000,
123+
562_893,
124124
)));
125125

126126
assert_expected_events!(

cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/teleport.rs

Lines changed: 82 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ fn para_origin_assertions(t: SystemParaToRelayTest) {
9797
fn para_dest_assertions(t: RelayToSystemParaTest) {
9898
type RuntimeEvent = <AssetHubWestend as Chain>::RuntimeEvent;
9999

100-
AssetHubWestend::assert_dmp_queue_complete(Some(Weight::from_parts(164_733_000, 0)));
100+
AssetHubWestend::assert_dmp_queue_complete(Some(Weight::from_parts(164_793_000, 3593)));
101101

102102
assert_expected_events!(
103103
AssetHubWestend,
@@ -142,16 +142,15 @@ fn system_para_limited_teleport_assets(t: SystemParaToRelayTest) -> DispatchResu
142142
)
143143
}
144144

145-
// TODO: Uncomment when https://github.com/paritytech/polkadot/pull/7424 is merged
146-
// fn system_para_teleport_assets(t: SystemParaToRelayTest) -> DispatchResult {
147-
// <AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::teleport_assets(
148-
// t.signed_origin,
149-
// bx!(t.args.dest),
150-
// bx!(t.args.beneficiary),
151-
// bx!(t.args.assets),
152-
// t.args.fee_asset_item,
153-
// )
154-
// }
145+
fn system_para_teleport_assets(t: SystemParaToRelayTest) -> DispatchResult {
146+
<AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::teleport_assets(
147+
t.signed_origin,
148+
bx!(t.args.dest.into()),
149+
bx!(t.args.beneficiary.into()),
150+
bx!(t.args.assets.into()),
151+
t.args.fee_asset_item,
152+
)
153+
}
155154

156155
/// Limited Teleport of native asset from Relay Chain to the System Parachain should work
157156
#[test]
@@ -286,78 +285,75 @@ fn teleport_native_assets_from_relay_to_system_para_works() {
286285
assert!(receiver_balance_after > receiver_balance_before);
287286
}
288287

289-
// TODO: Uncomment when https://github.com/paritytech/polkadot/pull/7424 is merged
290-
291-
// Right now it is failing in the Relay Chain with a
292-
// `messageQueue.ProcessingFailed` event `error: Unsupported`.
293-
// The reason is the `Weigher` in `pallet_xcm` is not properly calculating the `remote_weight`
294-
// and it cause an `Overweight` error in `AllowTopLevelPaidExecutionFrom` barrier
295-
296-
// /// Teleport of native asset from System Parachains to the Relay Chain
297-
// /// should work when there is enough balance in Relay Chain's `CheckAccount`
298-
// #[test]
299-
// fn teleport_native_assets_back_from_system_para_to_relay_works() {
300-
// // Dependency - Relay Chain's `CheckAccount` should have enough balance
301-
// teleport_native_assets_from_relay_to_system_para_works();
302-
303-
// // Init values for Relay Chain
304-
// let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
305-
// let test_args = TestContext {
306-
// sender: AssetHubWestendSender::get(),
307-
// receiver: WestendReceiver::get(),
308-
// args: get_para_dispatch_args(amount_to_send),
309-
// };
310-
311-
// let mut test = SystemParaToRelayTest::new(test_args);
312-
313-
// let sender_balance_before = test.sender.balance;
314-
// let receiver_balance_before = test.receiver.balance;
315-
316-
// test.set_assertion::<AssetHubWestend>(para_origin_assertions);
317-
// test.set_assertion::<Westend>(relay_dest_assertions);
318-
// test.set_dispatchable::<AssetHubWestend>(system_para_teleport_assets);
319-
// test.assert();
320-
321-
// let sender_balance_after = test.sender.balance;
322-
// let receiver_balance_after = test.receiver.balance;
323-
324-
// // Sender's balance is reduced
325-
// assert_eq!(sender_balance_before - amount_to_send, sender_balance_after);
326-
// // Receiver's balance is increased
327-
// assert!(receiver_balance_after > receiver_balance_before);
328-
// }
329-
330-
// /// Teleport of native asset from System Parachain to Relay Chain
331-
// /// shouldn't work when there is not enough balance in Relay Chain's `CheckAccount`
332-
// #[test]
333-
// fn teleport_native_assets_from_system_para_to_relay_fails() {
334-
// // Init values for Relay Chain
335-
// let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
336-
// let assets = (Parent, amount_to_send).into();
337-
//
338-
// let test_args = TestContext {
339-
// sender: AssetHubWestendSender::get(),
340-
// receiver: WestendReceiver::get(),
341-
// args: system_para_test_args(amount_to_send),
342-
// assets,
343-
// None
344-
// };
345-
346-
// let mut test = SystemParaToRelayTest::new(test_args);
347-
348-
// let sender_balance_before = test.sender.balance;
349-
// let receiver_balance_before = test.receiver.balance;
350-
351-
// test.set_assertion::<AssetHubWestend>(para_origin_assertions);
352-
// test.set_assertion::<Westend>(relay_dest_assertions);
353-
// test.set_dispatchable::<AssetHubWestend>(system_para_teleport_assets);
354-
// test.assert();
355-
356-
// let sender_balance_after = test.sender.balance;
357-
// let receiver_balance_after = test.receiver.balance;
358-
359-
// // Sender's balance is reduced
360-
// assert_eq!(sender_balance_before - amount_to_send, sender_balance_after);
361-
// // Receiver's balance does not change
362-
// assert_eq!(receiver_balance_after, receiver_balance_before);
363-
// }
288+
/// Teleport of native asset from System Parachains to the Relay Chain
289+
/// should work when there is enough balance in Relay Chain's `CheckAccount`
290+
#[test]
291+
fn teleport_native_assets_back_from_system_para_to_relay_works() {
292+
// Dependency - Relay Chain's `CheckAccount` should have enough balance
293+
teleport_native_assets_from_relay_to_system_para_works();
294+
295+
// Init values for Relay Chain
296+
let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
297+
let destination = AssetHubWestend::parent_location();
298+
let beneficiary_id = WestendReceiver::get();
299+
let assets = (Parent, amount_to_send).into();
300+
301+
let test_args = TestContext {
302+
sender: AssetHubWestendSender::get(),
303+
receiver: WestendReceiver::get(),
304+
args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
305+
};
306+
307+
let mut test = SystemParaToRelayTest::new(test_args);
308+
309+
let sender_balance_before = test.sender.balance;
310+
let receiver_balance_before = test.receiver.balance;
311+
312+
test.set_assertion::<AssetHubWestend>(para_origin_assertions);
313+
test.set_assertion::<Westend>(relay_dest_assertions);
314+
test.set_dispatchable::<AssetHubWestend>(system_para_teleport_assets);
315+
test.assert();
316+
317+
let sender_balance_after = test.sender.balance;
318+
let receiver_balance_after = test.receiver.balance;
319+
320+
// Sender's balance is reduced
321+
assert_eq!(sender_balance_before - amount_to_send, sender_balance_after);
322+
// Receiver's balance is increased
323+
assert!(receiver_balance_after > receiver_balance_before);
324+
}
325+
326+
/// Teleport of native asset from System Parachain to Relay Chain
327+
/// shouldn't work when there is not enough balance in Relay Chain's `CheckAccount`
328+
#[test]
329+
fn teleport_native_assets_from_system_para_to_relay_fails() {
330+
// Init values for Relay Chain
331+
let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
332+
let destination = AssetHubWestend::parent_location();
333+
let beneficiary_id = WestendReceiver::get();
334+
let assets = (Parent, amount_to_send).into();
335+
336+
let test_args = TestContext {
337+
sender: AssetHubWestendSender::get(),
338+
receiver: WestendReceiver::get(),
339+
args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
340+
};
341+
342+
let mut test = SystemParaToRelayTest::new(test_args);
343+
344+
let sender_balance_before = test.sender.balance;
345+
let receiver_balance_before = test.receiver.balance;
346+
347+
test.set_assertion::<AssetHubWestend>(para_origin_assertions);
348+
test.set_assertion::<Westend>(relay_dest_assertions_fail);
349+
test.set_dispatchable::<AssetHubWestend>(system_para_teleport_assets);
350+
test.assert();
351+
352+
let sender_balance_after = test.sender.balance;
353+
let receiver_balance_after = test.receiver.balance;
354+
355+
// Sender's balance is reduced
356+
assert_eq!(sender_balance_before - amount_to_send, sender_balance_after);
357+
// Receiver's balance does not change
358+
assert_eq!(receiver_balance_after, receiver_balance_before);
359+
}

cumulus/parachains/integration-tests/emulated/common/src/impls.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,22 @@ macro_rules! impl_assert_events_helpers_for_parachain {
503503
);
504504
}
505505

506+
/// Asserts a XCM from Relay Chain is executed with error
507+
pub fn assert_dmp_queue_error(
508+
expected_error: $crate::impls::Error,
509+
) {
510+
$crate::impls::assert_expected_events!(
511+
Self,
512+
vec![
513+
[<$chain RuntimeEvent>]::DmpQueue($crate::impls::cumulus_pallet_dmp_queue::Event::ExecutedDownward {
514+
outcome: $crate::impls::Outcome::Error(error), ..
515+
}) => {
516+
error: *error == expected_error,
517+
},
518+
]
519+
);
520+
}
521+
506522
/// Asserts a XCM from another Parachain is completely executed
507523
pub fn assert_xcmp_queue_success(expected_weight: Option<$crate::impls::Weight>) {
508524
$crate::impls::assert_expected_events!(

cumulus/parachains/runtimes/assets/asset-hub-kusama/src/weights/xcm/mod.rs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,8 @@ impl<Call> XcmWeightInfo<Call> for AssetHubKusamaXcmWeight<Call> {
6161
fn withdraw_asset(assets: &MultiAssets) -> Weight {
6262
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::withdraw_asset())
6363
}
64-
// Currently there is no trusted reserve (`IsReserve = ()`),
65-
// but we need this hack for `pallet_xcm::reserve_transfer_assets`
66-
// (TODO) fix https://github.com/paritytech/polkadot/pull/7424
67-
// (TODO) fix https://github.com/paritytech/polkadot/pull/7546
68-
fn reserve_asset_deposited(_assets: &MultiAssets) -> Weight {
69-
// TODO: if we change `IsReserve = ...` then use this line...
70-
// TODO: or if remote weight estimation is fixed, then remove
71-
// TODO: hardcoded - fix https://github.com/paritytech/cumulus/issues/1974
72-
let hardcoded_weight = Weight::from_parts(1_000_000_000_u64, 0);
73-
hardcoded_weight.min(XcmFungibleWeight::<Runtime>::reserve_asset_deposited())
64+
fn reserve_asset_deposited(assets: &MultiAssets) -> Weight {
65+
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::reserve_asset_deposited())
7466
}
7567
fn receive_teleported_asset(assets: &MultiAssets) -> Weight {
7668
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::receive_teleported_asset())
@@ -127,10 +119,7 @@ impl<Call> XcmWeightInfo<Call> for AssetHubKusamaXcmWeight<Call> {
127119
}
128120

129121
fn deposit_asset(assets: &MultiAssetFilter, _dest: &MultiLocation) -> Weight {
130-
// Hardcoded till the XCM pallet is fixed
131-
let hardcoded_weight = Weight::from_parts(1_000_000_000_u64, 0);
132-
let weight = assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::deposit_asset());
133-
hardcoded_weight.min(weight)
122+
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::deposit_asset())
134123
}
135124
fn deposit_reserve_asset(
136125
assets: &MultiAssetFilter,

0 commit comments

Comments
 (0)