From cc4805b51ecea818ca46d96babb6f35f9450b893 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 31 Jan 2024 04:01:44 +0100 Subject: [PATCH] Add limits to XCMv4 (#3114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some more work regarding XCMv4. Two limits from v3 were not transferred over, those are: - The instructions limit - The number of assets limit Both of these are now in v4. For some reason `AssetInstance` increased in size, don't know why CI didn't catch that before. --------- Co-authored-by: Bastian Köcher Co-authored-by: command-bot <> --- polkadot/xcm/src/lib.rs | 2 +- polkadot/xcm/src/v3/mod.rs | 1 - polkadot/xcm/src/v4/asset.rs | 65 +++++++++++++++++++++++++++++++++++- polkadot/xcm/src/v4/mod.rs | 56 +++++++++++++++++++++++++++++-- 4 files changed, 119 insertions(+), 5 deletions(-) diff --git a/polkadot/xcm/src/lib.rs b/polkadot/xcm/src/lib.rs index f0cbe98533..561d5175b4 100644 --- a/polkadot/xcm/src/lib.rs +++ b/polkadot/xcm/src/lib.rs @@ -658,7 +658,7 @@ fn size_limits() { (crate::latest::Junctions, 16), (crate::latest::Junction, 88), (crate::latest::Response, 40), - (crate::latest::AssetInstance, 40), + (crate::latest::AssetInstance, 48), (crate::latest::NetworkId, 48), (crate::latest::BodyId, 32), (crate::latest::Assets, 24), diff --git a/polkadot/xcm/src/v3/mod.rs b/polkadot/xcm/src/v3/mod.rs index 1172cbf43e..d4e2da07a2 100644 --- a/polkadot/xcm/src/v3/mod.rs +++ b/polkadot/xcm/src/v3/mod.rs @@ -69,7 +69,6 @@ pub const VERSION: super::Version = 3; /// An identifier for a query. pub type QueryId = u64; -// TODO (v4): Use `BoundedVec` instead of `Vec` #[derive(Derivative, Default, Encode, TypeInfo)] #[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))] #[codec(encode_bound())] diff --git a/polkadot/xcm/src/v4/asset.rs b/polkadot/xcm/src/v4/asset.rs index 92bc3ff247..bdff0c2723 100644 --- a/polkadot/xcm/src/v4/asset.rs +++ b/polkadot/xcm/src/v4/asset.rs @@ -33,6 +33,7 @@ use crate::v3::{ WildFungibility as OldWildFungibility, WildMultiAsset as OldWildAsset, }; use alloc::{vec, vec::Vec}; +use bounded_collections::{BoundedVec, ConstU32}; use core::{ cmp::Ordering, convert::{TryFrom, TryInto}, @@ -562,7 +563,9 @@ impl MaxEncodedLen for Assets { impl Decode for Assets { fn decode(input: &mut I) -> Result { - Self::from_sorted_and_deduplicated(Vec::::decode(input)?) + let bounded_instructions = + BoundedVec::>::decode(input)?; + Self::from_sorted_and_deduplicated(bounded_instructions.into_inner()) .map_err(|()| "Out of order".into()) } } @@ -1002,4 +1005,64 @@ mod tests { let r = Assets::from_sorted_and_deduplicated(mixed_bad); assert!(r.is_err()); } + + #[test] + fn reanchor_preserves_sorting() { + use super::*; + use alloc::vec; + + let reanchor_context: Junctions = Parachain(2000).into(); + let dest = Location::new(1, []); + + let asset_1: Asset = (Location::new(0, [PalletInstance(50), GeneralIndex(1)]), 10).into(); + let mut asset_1_reanchored = asset_1.clone(); + assert!(asset_1_reanchored.reanchor(&dest, &reanchor_context).is_ok()); + assert_eq!( + asset_1_reanchored, + (Location::new(0, [Parachain(2000), PalletInstance(50), GeneralIndex(1)]), 10).into() + ); + + let asset_2: Asset = (Location::new(1, []), 10).into(); + let mut asset_2_reanchored = asset_2.clone(); + assert!(asset_2_reanchored.reanchor(&dest, &reanchor_context).is_ok()); + assert_eq!(asset_2_reanchored, (Location::new(0, []), 10).into()); + + let asset_3: Asset = (Location::new(1, [Parachain(1000)]), 10).into(); + let mut asset_3_reanchored = asset_3.clone(); + assert!(asset_3_reanchored.reanchor(&dest, &reanchor_context).is_ok()); + assert_eq!(asset_3_reanchored, (Location::new(0, [Parachain(1000)]), 10).into()); + + let mut assets: Assets = vec![asset_1.clone(), asset_2.clone(), asset_3.clone()].into(); + assert_eq!(assets.clone(), vec![asset_1.clone(), asset_2.clone(), asset_3.clone()].into()); + + assert!(assets.reanchor(&dest, &reanchor_context).is_ok()); + assert_eq!(assets, vec![asset_2_reanchored, asset_3_reanchored, asset_1_reanchored].into()); + } + + #[test] + fn decoding_respects_limit() { + use super::*; + + // Having lots of one asset will work since they are deduplicated + let lots_of_one_asset: Assets = + vec![(GeneralIndex(1), 1u128).into(); MAX_ITEMS_IN_ASSETS + 1].into(); + let encoded = lots_of_one_asset.encode(); + assert!(Assets::decode(&mut &encoded[..]).is_ok()); + + // Fewer assets than the limit works + let mut few_assets: Assets = Vec::new().into(); + for i in 0..MAX_ITEMS_IN_ASSETS { + few_assets.push((GeneralIndex(i as u128), 1u128).into()); + } + let encoded = few_assets.encode(); + assert!(Assets::decode(&mut &encoded[..]).is_ok()); + + // Having lots of different assets will not work + let mut too_many_different_assets: Assets = Vec::new().into(); + for i in 0..MAX_ITEMS_IN_ASSETS + 1 { + too_many_different_assets.push((GeneralIndex(i as u128), 1u128).into()); + } + let encoded = too_many_different_assets.encode(); + assert!(Assets::decode(&mut &encoded[..]).is_err()); + } } diff --git a/polkadot/xcm/src/v4/mod.rs b/polkadot/xcm/src/v4/mod.rs index 3b57ba1b13..30ee485589 100644 --- a/polkadot/xcm/src/v4/mod.rs +++ b/polkadot/xcm/src/v4/mod.rs @@ -30,7 +30,10 @@ use core::{ result, }; use derivative::Derivative; -use parity_scale_codec::{self, Decode, Encode, MaxEncodedLen}; +use parity_scale_codec::{ + self, decode_vec_with_len, Compact, Decode, Encode, Error as CodecError, Input as CodecInput, + MaxEncodedLen, +}; use scale_info::TypeInfo; mod asset; @@ -59,7 +62,7 @@ pub const VERSION: super::Version = 4; /// An identifier for a query. pub type QueryId = u64; -#[derive(Derivative, Default, Encode, Decode, TypeInfo)] +#[derive(Derivative, Default, Encode, TypeInfo)] #[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))] #[codec(encode_bound())] #[codec(decode_bound())] @@ -68,6 +71,26 @@ pub struct Xcm(pub Vec>); pub const MAX_INSTRUCTIONS_TO_DECODE: u8 = 100; +environmental::environmental!(instructions_count: u8); + +impl Decode for Xcm { + fn decode(input: &mut I) -> core::result::Result { + instructions_count::using_once(&mut 0, || { + let number_of_instructions: u32 = >::decode(input)?.into(); + instructions_count::with(|count| { + *count = count.saturating_add(number_of_instructions as u8); + if *count > MAX_INSTRUCTIONS_TO_DECODE { + return Err(CodecError::from("Max instructions exceeded")) + } + Ok(()) + }) + .expect("Called in `using` context and thus can not return `None`; qed")?; + let decoded_instructions = decode_vec_with_len(input, number_of_instructions as usize)?; + Ok(Self(decoded_instructions)) + }) + } +} + impl Xcm { /// Create an empty instance. pub fn new() -> Self { @@ -1454,4 +1477,33 @@ mod tests { let new_xcm: Xcm<()> = old_xcm.try_into().unwrap(); assert_eq!(new_xcm, xcm); } + + #[test] + fn decoding_respects_limit() { + let max_xcm = Xcm::<()>(vec![ClearOrigin; MAX_INSTRUCTIONS_TO_DECODE as usize]); + let encoded = max_xcm.encode(); + assert!(Xcm::<()>::decode(&mut &encoded[..]).is_ok()); + + let big_xcm = Xcm::<()>(vec![ClearOrigin; MAX_INSTRUCTIONS_TO_DECODE as usize + 1]); + let encoded = big_xcm.encode(); + assert!(Xcm::<()>::decode(&mut &encoded[..]).is_err()); + + let nested_xcm = Xcm::<()>(vec![ + DepositReserveAsset { + assets: All.into(), + dest: Here.into(), + xcm: max_xcm, + }; + (MAX_INSTRUCTIONS_TO_DECODE / 2) as usize + ]); + let encoded = nested_xcm.encode(); + assert!(Xcm::<()>::decode(&mut &encoded[..]).is_err()); + + let even_more_nested_xcm = Xcm::<()>(vec![SetAppendix(nested_xcm); 64]); + let encoded = even_more_nested_xcm.encode(); + assert_eq!(encoded.len(), 342530); + // This should not decode since the limit is 100 + assert_eq!(MAX_INSTRUCTIONS_TO_DECODE, 100, "precondition"); + assert!(Xcm::<()>::decode(&mut &encoded[..]).is_err()); + } }