From 12577fe2a932eab4faa66da9f9fb4b1f6acd3976 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 12 Oct 2021 08:19:27 -0400 Subject: [PATCH 1/3] found issue --- runtime/westend/src/tests.rs | 48 ++++++++++++++++++++++++++ runtime/westend/src/weights/xcm/mod.rs | 5 +-- xcm/xcm-builder/src/weight.rs | 11 ++++-- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/runtime/westend/src/tests.rs b/runtime/westend/src/tests.rs index f7d4e749c252..dd2822df0fef 100644 --- a/runtime/westend/src/tests.rs +++ b/runtime/westend/src/tests.rs @@ -49,3 +49,51 @@ fn call_size() { If the limit is too strong, maybe consider increase the limit to 300.", ); } + +#[test] +fn xcm_weight() { + use frame_support::dispatch::GetDispatchInfo; + let weight = pallet_xcm::Call::::teleport_assets { + dest: Box::new(xcm::VersionedMultiLocation::V1(MultiLocation::parent())), + beneficiary: Box::new(xcm::VersionedMultiLocation::V1(MultiLocation::parent())), + assets: Box::new(( + Concrete(MultiLocation::parent()), + Fungible(200_000) + ).into()), + fee_asset_item: 0 + }.get_dispatch_info().weight; + + println!("{:?} max: {:?}", weight, u64::MAX); + assert!(false); +} + +#[test] +fn xcm_weight_2() { + use sp_std::convert::TryInto; + use xcm_executor::traits::WeightBounds; + let assets: Box = Box::new(( + Concrete(MultiLocation::parent()), + Fungible(200_000) + ).into()); + let dest: Box = Box::new( + xcm::VersionedMultiLocation::V1(MultiLocation::parent()) + ); + let maybe_assets: Result = (*assets.clone()).try_into(); + let maybe_dest: Result = (*dest.clone()).try_into(); + let final_weight = match (maybe_assets, maybe_dest) { + (Ok(assets), Ok(dest)) => { + use sp_std::vec; + let mut message = Xcm(vec![ + WithdrawAsset(assets), + InitiateTeleport { assets: Wild(All), dest, xcm: Xcm(vec![]) }, + ]); + let result = ::Weigher::weight(&mut message); + println!("result: {:?}", result); + result.map_or(Weight::max_value(), |w| 100_000_000 + w) + }, + _ => Weight::max_value(), + }; + + println!("{:?} max: {:?}", final_weight, u64::MAX); + assert!(false); +} diff --git a/runtime/westend/src/weights/xcm/mod.rs b/runtime/westend/src/weights/xcm/mod.rs index 1b0d7c1ef485..8bf43917650d 100644 --- a/runtime/westend/src/weights/xcm/mod.rs +++ b/runtime/westend/src/weights/xcm/mod.rs @@ -20,9 +20,9 @@ pub enum AssetTypes { impl From<&MultiAsset> for AssetTypes { fn from(asset: &MultiAsset) -> Self { match asset { - MultiAsset { id: Concrete(MultiLocation { parents: 0, interior: Here }), .. } => + _ => AssetTypes::Balances, - _ => AssetTypes::Unknown, + //_ => AssetTypes::Unknown, } } } @@ -44,6 +44,7 @@ impl WeighMultiAssets for MultiAssetFilter { AssetTypes::Unknown => Weight::MAX, }) .fold(0, |acc, x| acc.saturating_add(x)), + Self::Wild(_) => balances_weight, _ => Weight::MAX, } } diff --git a/xcm/xcm-builder/src/weight.rs b/xcm/xcm-builder/src/weight.rs index 1e84a42e2ada..bc4b2644deae 100644 --- a/xcm/xcm-builder/src/weight.rs +++ b/xcm/xcm-builder/src/weight.rs @@ -92,6 +92,9 @@ where let mut r: Weight = 0; *instrs_limit = instrs_limit.checked_sub(message.0.len() as u32).ok_or(())?; for m in message.0.iter() { + sp_std::if_std! { + println!("r: {:?}", r); + } r = r.checked_add(Self::instr_weight_with_limit(m, instrs_limit)?).ok_or(())?; } Ok(r) @@ -101,7 +104,7 @@ where instrs_limit: &mut u32, ) -> Result { use xcm::GetWeight; - instruction + let weight = instruction .weight() .checked_add(match instruction { Transact { require_weight_at_most, .. } => *require_weight_at_most, @@ -109,7 +112,11 @@ where Self::weight_with_limit(xcm, instrs_limit)?, _ => 0, }) - .ok_or(()) + .ok_or(()); + sp_std::if_std! { + println!("instruction: {:?}, weight: {:?}", instruction, weight); + } + weight } } From fbe39e7909cf577a632529ce3ebcfa8e9bcfdb28 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 12 Oct 2021 09:42:30 -0400 Subject: [PATCH 2/3] fix up --- runtime/westend/src/tests.rs | 54 ++++++-------------------- runtime/westend/src/weights/xcm/mod.rs | 11 +++--- 2 files changed, 18 insertions(+), 47 deletions(-) diff --git a/runtime/westend/src/tests.rs b/runtime/westend/src/tests.rs index dd2822df0fef..841089f93ead 100644 --- a/runtime/westend/src/tests.rs +++ b/runtime/westend/src/tests.rs @@ -51,49 +51,19 @@ fn call_size() { } #[test] -fn xcm_weight() { +fn sanity_check_teleport_assets_weight() { + // This test sanity checks that at least 50 teleports can exist in a block. + // Usually when XCM runs into an issue, it will return a weight of `Weight::MAX`, + // so this test will certainly ensure that this problem does not occur. use frame_support::dispatch::GetDispatchInfo; let weight = pallet_xcm::Call::::teleport_assets { - dest: Box::new(xcm::VersionedMultiLocation::V1(MultiLocation::parent())), - beneficiary: Box::new(xcm::VersionedMultiLocation::V1(MultiLocation::parent())), - assets: Box::new(( - Concrete(MultiLocation::parent()), - Fungible(200_000) - ).into()), - fee_asset_item: 0 - }.get_dispatch_info().weight; + dest: Box::new(xcm::VersionedMultiLocation::V1(MultiLocation::here())), + beneficiary: Box::new(xcm::VersionedMultiLocation::V1(MultiLocation::here())), + assets: Box::new((Concrete(MultiLocation::here()), Fungible(200_000)).into()), + fee_asset_item: 0, + } + .get_dispatch_info() + .weight; - println!("{:?} max: {:?}", weight, u64::MAX); - assert!(false); -} - -#[test] -fn xcm_weight_2() { - use sp_std::convert::TryInto; - use xcm_executor::traits::WeightBounds; - let assets: Box = Box::new(( - Concrete(MultiLocation::parent()), - Fungible(200_000) - ).into()); - let dest: Box = Box::new( - xcm::VersionedMultiLocation::V1(MultiLocation::parent()) - ); - let maybe_assets: Result = (*assets.clone()).try_into(); - let maybe_dest: Result = (*dest.clone()).try_into(); - let final_weight = match (maybe_assets, maybe_dest) { - (Ok(assets), Ok(dest)) => { - use sp_std::vec; - let mut message = Xcm(vec![ - WithdrawAsset(assets), - InitiateTeleport { assets: Wild(All), dest, xcm: Xcm(vec![]) }, - ]); - let result = ::Weigher::weight(&mut message); - println!("result: {:?}", result); - result.map_or(Weight::max_value(), |w| 100_000_000 + w) - }, - _ => Weight::max_value(), - }; - - println!("{:?} max: {:?}", final_weight, u64::MAX); - assert!(false); + assert!(weight * 50 < BlockWeights::get().max_block); } diff --git a/runtime/westend/src/weights/xcm/mod.rs b/runtime/westend/src/weights/xcm/mod.rs index 8bf43917650d..70c5731a7853 100644 --- a/runtime/westend/src/weights/xcm/mod.rs +++ b/runtime/westend/src/weights/xcm/mod.rs @@ -20,9 +20,9 @@ pub enum AssetTypes { impl From<&MultiAsset> for AssetTypes { fn from(asset: &MultiAsset) -> Self { match asset { - _ => + MultiAsset { id: Concrete(MultiLocation { parents: 0, interior: Here }), .. } => AssetTypes::Balances, - //_ => AssetTypes::Unknown, + _ => AssetTypes::Unknown, } } } @@ -31,7 +31,9 @@ trait WeighMultiAssets { fn weigh_multi_assets(&self, balances_weight: Weight) -> Weight; } -// TODO wild case +// Westend only knows about one asset, the balances pallet. +const MAX_ASSETS: u32 = 1; + impl WeighMultiAssets for MultiAssetFilter { fn weigh_multi_assets(&self, balances_weight: Weight) -> Weight { match self { @@ -44,8 +46,7 @@ impl WeighMultiAssets for MultiAssetFilter { AssetTypes::Unknown => Weight::MAX, }) .fold(0, |acc, x| acc.saturating_add(x)), - Self::Wild(_) => balances_weight, - _ => Weight::MAX, + Self::Wild(_) => (MAX_ASSETS as Weight).saturating_mul(balances_weight), } } } From 6f5a20b69a3740aba3712f486abbbe807fe04141 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 12 Oct 2021 09:43:34 -0400 Subject: [PATCH 3/3] remove printlns --- xcm/xcm-builder/src/weight.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/xcm/xcm-builder/src/weight.rs b/xcm/xcm-builder/src/weight.rs index bc4b2644deae..1e84a42e2ada 100644 --- a/xcm/xcm-builder/src/weight.rs +++ b/xcm/xcm-builder/src/weight.rs @@ -92,9 +92,6 @@ where let mut r: Weight = 0; *instrs_limit = instrs_limit.checked_sub(message.0.len() as u32).ok_or(())?; for m in message.0.iter() { - sp_std::if_std! { - println!("r: {:?}", r); - } r = r.checked_add(Self::instr_weight_with_limit(m, instrs_limit)?).ok_or(())?; } Ok(r) @@ -104,7 +101,7 @@ where instrs_limit: &mut u32, ) -> Result { use xcm::GetWeight; - let weight = instruction + instruction .weight() .checked_add(match instruction { Transact { require_weight_at_most, .. } => *require_weight_at_most, @@ -112,11 +109,7 @@ where Self::weight_with_limit(xcm, instrs_limit)?, _ => 0, }) - .ok_or(()); - sp_std::if_std! { - println!("instruction: {:?}, weight: {:?}", instruction, weight); - } - weight + .ok_or(()) } }