Enforce a decoding limit in MultiAssets (#1395)

* Enforce a decoding limit in MultiAssets

* ".git/.scripts/commands/fmt/fmt.sh"

* Update polkadot/xcm/src/v3/multiasset.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Just use a BoundedVec

* Conflicts

---------

Co-authored-by: command-bot <>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

polkadot/xcm/src/v3/multiasset.rs

@@ -40,6 +40,7 @@ use core::{
 };
 use parity_scale_codec::{self as codec, Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
+use bounded_collections::{BoundedVec, ConstU32};
 
 /// A general identifier for an instance of a non-fungible asset class.
 #[derive(
@@ -506,9 +507,8 @@ impl TryFrom<OldMultiAsset> for MultiAsset {
 #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct MultiAssets(Vec<MultiAsset>);
 
-/// Maximum number of items we expect in a single `MultiAssets` value. Note this is not (yet)
+/// Maximum number of items in a single `MultiAssets` value that can be decoded.
-/// enforced, and just serves to provide a sensible `max_encoded_len` for `MultiAssets`.
+pub const MAX_ITEMS_IN_MULTIASSETS: usize = 20;
-const MAX_ITEMS_IN_MULTIASSETS: usize = 20;
 
 impl MaxEncodedLen for MultiAssets {
 	fn max_encoded_len() -> usize {
@@ -517,8 +517,9 @@ impl MaxEncodedLen for MultiAssets {
 }
 
 impl Decode for MultiAssets {
-	fn decode<I: codec::Input>(input: &mut I) -> Result<Self, parity_scale_codec::Error> {
+	fn decode<I: codec::Input>(input: &mut I) -> Result<Self, codec::Error> {
-		Self::from_sorted_and_deduplicated(Vec::<MultiAsset>::decode(input)?)
+		let bounded_instructions = BoundedVec::<MultiAsset, ConstU32<{ MAX_ITEMS_IN_MULTIASSETS as u32 }>>::decode(input)?;
+		Self::from_sorted_and_deduplicated(bounded_instructions.into_inner())
 			.map_err(|()| "Out of order".into())
 	}
 }
@@ -974,4 +975,31 @@ mod tests {
 		let r = MultiAssets::from_sorted_and_deduplicated(mixed_bad);
 		assert!(r.is_err());
 	}
+
+	#[test]
+	fn decoding_respects_limit() {
+		use super::*;
+
+		// Having lots of one asset will work since they are deduplicated
+		let lots_of_one_asset: MultiAssets =
+			vec![(GeneralIndex(1), 1u128).into(); MAX_ITEMS_IN_MULTIASSETS + 1].into();
+		let encoded = lots_of_one_asset.encode();
+		assert!(MultiAssets::decode(&mut &encoded[..]).is_ok());
+
+		// Fewer assets than the limit works
+		let mut few_assets: MultiAssets = Vec::new().into();
+		for i in 0..MAX_ITEMS_IN_MULTIASSETS {
+			few_assets.push((GeneralIndex(i as u128), 1u128).into());
+		}
+		let encoded = few_assets.encode();
+		assert!(MultiAssets::decode(&mut &encoded[..]).is_ok());
+
+		// Having lots of different assets will not work
+		let mut too_many_different_assets: MultiAssets = Vec::new().into();
+		for i in 0..MAX_ITEMS_IN_MULTIASSETS + 1 {
+			too_many_different_assets.push((GeneralIndex(i as u128), 1u128).into());
+		}
+		let encoded = too_many_different_assets.encode();
+		assert!(MultiAssets::decode(&mut &encoded[..]).is_err());
+	}
 }