XCM: Limit the max number of assets weighable in MultiAssets (#6786)

* Limit the max number of assets weighable in MultiAssets * Apply suggestions from code review Co-authored-by: Gavin Wood <gavin@parity.io> * Add tests * cargo fmt * Include proof size weight in tests * Add comments --------- Co-authored-by: Gavin Wood <gavin@parity.io>
paritytech · Feb 28, 2023 · 9d74985 · 9d74985
1 parent c9f0ed2
commit 9d74985
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 9 deletions.
diff --git a/runtime/kusama/src/weights/xcm/mod.rs b/runtime/kusama/src/weights/xcm/mod.rs
@@ -48,7 +48,7 @@ trait WeighMultiAssets {
 }
 
 // Kusama only knows about one asset, the balances pallet.
-const MAX_ASSETS: u32 = 1;
+const MAX_ASSETS: u64 = 1;
 
 impl WeighMultiAssets for MultiAssetFilter {
 	fn weigh_multi_assets(&self, balances_weight: Weight) -> Weight {
@@ -62,9 +62,12 @@ impl WeighMultiAssets for MultiAssetFilter {
 					AssetTypes::Unknown => Weight::MAX,
 				})
 				.fold(Weight::zero(), |acc, x| acc.saturating_add(x)),
+			// We don't support any NFTs on Kusama, so these two variants will always match
+			// only 1 kind of fungible asset.
 			Self::Wild(AllOf { .. } | AllOfCounted { .. }) => balances_weight,
-			Self::Wild(AllCounted(count)) => balances_weight.saturating_mul(*count as u64),
-			Self::Wild(All) => balances_weight.saturating_mul(MAX_ASSETS as u64),
+			Self::Wild(AllCounted(count)) =>
+				balances_weight.saturating_mul(MAX_ASSETS.min(*count as u64)),
+			Self::Wild(All) => balances_weight.saturating_mul(MAX_ASSETS),
 		}
 	}
 }
@@ -275,3 +278,11 @@ impl<RuntimeCall> XcmWeightInfo<RuntimeCall> for KusamaXcmWeight<RuntimeCall> {
 		XcmGeneric::<Runtime>::unpaid_execution()
 	}
 }
+
+#[test]
+fn all_counted_has_a_sane_weight_upper_limit() {
+	let assets = MultiAssetFilter::Wild(AllCounted(4294967295));
+	let weight = Weight::from_parts(1000, 1000);
+
+	assert_eq!(assets.weigh_multi_assets(weight), weight * MAX_ASSETS);
+}
diff --git a/runtime/kusama/src/xcm_config.rs b/runtime/kusama/src/xcm_config.rs
@@ -457,3 +457,19 @@ impl pallet_xcm::Config for Runtime {
 	#[cfg(feature = "runtime-benchmarks")]
 	type ReachableDest = ReachableDest;
 }
+
+#[test]
+fn karura_liquid_staking_xcm_has_sane_weight_upper_limt() {
+	use parity_scale_codec::Decode;
+	use xcm::VersionedXcm;
+	use xcm_executor::traits::WeightBounds;
+	let blob = hex_literal::hex!("02140004000000000700e40b540213000000000700e40b54020006010700c817a804341801000006010b00c490bf4302140d010003ffffffff000100411f");
+	let Ok(VersionedXcm::V2(old_xcm)) =
+		VersionedXcm::<super::RuntimeCall>::decode(&mut &blob[..]) else { panic!("can't decode XCM blob") };
+	let mut xcm: Xcm<super::RuntimeCall> =
+		old_xcm.try_into().expect("conversion from v2 to v3 failed");
+	let weight = <XcmConfig as xcm_executor::Config>::Weigher::weight(&mut xcm)
+		.expect("weighing XCM failed");
+
+	assert_eq!(weight, Weight::from_parts(20_313_281_000, 65536));
+}
diff --git a/runtime/rococo/src/weights/xcm/mod.rs b/runtime/rococo/src/weights/xcm/mod.rs
@@ -48,7 +48,7 @@ trait WeighMultiAssets {
 }
 
 // Rococo only knows about one asset, the balances pallet.
-const MAX_ASSETS: u32 = 1;
+const MAX_ASSETS: u64 = 1;
 
 impl WeighMultiAssets for MultiAssetFilter {
 	fn weigh_multi_assets(&self, balances_weight: Weight) -> Weight {
@@ -62,9 +62,12 @@ impl WeighMultiAssets for MultiAssetFilter {
 					AssetTypes::Unknown => Weight::MAX,
 				})
 				.fold(Weight::zero(), |acc, x| acc.saturating_add(x)),
+			// We don't support any NFTs on Rococo, so these two variants will always match
+			// only 1 kind of fungible asset.
 			Self::Wild(AllOf { .. } | AllOfCounted { .. }) => balances_weight,
-			Self::Wild(AllCounted(count)) => balances_weight.saturating_mul(*count as u64),
-			Self::Wild(All) => balances_weight.saturating_mul(MAX_ASSETS as u64),
+			Self::Wild(AllCounted(count)) =>
+				balances_weight.saturating_mul(MAX_ASSETS.min(*count as u64)),
+			Self::Wild(All) => balances_weight.saturating_mul(MAX_ASSETS),
 		}
 	}
 }
@@ -275,3 +278,11 @@ impl<RuntimeCall> XcmWeightInfo<RuntimeCall> for RococoXcmWeight<RuntimeCall> {
 		XcmGeneric::<Runtime>::unpaid_execution()
 	}
 }
+
+#[test]
+fn all_counted_has_a_sane_weight_upper_limit() {
+	let assets = MultiAssetFilter::Wild(AllCounted(4294967295));
+	let weight = Weight::from_parts(1000, 1000);
+
+	assert_eq!(assets.weigh_multi_assets(weight), weight * MAX_ASSETS);
+}
diff --git a/runtime/westend/src/weights/xcm/mod.rs b/runtime/westend/src/weights/xcm/mod.rs
@@ -51,7 +51,7 @@ trait WeighMultiAssets {
 }
 
 // Westend only knows about one asset, the balances pallet.
-const MAX_ASSETS: u32 = 1;
+const MAX_ASSETS: u64 = 1;
 
 impl WeighMultiAssets for MultiAssetFilter {
 	fn weigh_multi_assets(&self, balances_weight: Weight) -> Weight {
@@ -65,9 +65,12 @@ impl WeighMultiAssets for MultiAssetFilter {
 					AssetTypes::Unknown => Weight::MAX,
 				})
 				.fold(Weight::zero(), |acc, x| acc.saturating_add(x)),
+			// We don't support any NFTs on Westend, so these two variants will always match
+			// only 1 kind of fungible asset.
 			Self::Wild(AllOf { .. } | AllOfCounted { .. }) => balances_weight,
-			Self::Wild(AllCounted(count)) => balances_weight.saturating_mul(*count as u64),
-			Self::Wild(All) => balances_weight.saturating_mul(MAX_ASSETS as u64),
+			Self::Wild(AllCounted(count)) =>
+				balances_weight.saturating_mul(MAX_ASSETS.min(*count as u64)),
+			Self::Wild(All) => balances_weight.saturating_mul(MAX_ASSETS),
 		}
 	}
 }
@@ -279,3 +282,11 @@ impl<RuntimeCall> XcmWeightInfo<RuntimeCall> for WestendXcmWeight<RuntimeCall> {
 		XcmGeneric::<Runtime>::unpaid_execution()
 	}
 }
+
+#[test]
+fn all_counted_has_a_sane_weight_upper_limit() {
+	let assets = MultiAssetFilter::Wild(AllCounted(4294967295));
+	let weight = Weight::from_parts(1000, 1000);
+
+	assert_eq!(assets.weigh_multi_assets(weight), weight * MAX_ASSETS);
+}