From 88cfaf79ef3630b0950d2e4e608a6621fec9731a Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Thu, 20 Jan 2022 17:58:55 +0100 Subject: [PATCH 01/10] fix AssetsFrom to allow more than 1 junction (after the prefix) --- .../parachains-common/src/impls.rs | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index 83e42bedf9b..7069dc4a114 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -108,9 +108,16 @@ pub struct AssetsFrom(PhantomData); impl> FilterAssetLocation for AssetsFrom { fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool { let loc = T::get(); + log::trace!(target: "xcm::AssetsFrom", "loc: {:?}, origin: {:?}", loc, origin); &loc == origin && - matches!(asset, MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } - if asset_loc.match_and_split(&loc).is_some()) + match asset { + MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } => { + loc.parent_count() == asset_loc.parent_count() && + loc.interior().iter().zip(asset_loc.interior().iter()) + .all(|(prefix_junction, asset_junction)| prefix_junction == asset_junction) + }, + _ => false, + } } } @@ -282,5 +289,21 @@ mod tests { ), "AssetsFrom should allow assets from any of its interior locations" ); + + // make sure assets can have more than one junction inside the prefix location + let asset_location = SomeSiblingParachain::get() + .clone() + .pushed_with_interior(PalletInstance(50)) + .unwrap() + .pushed_with_interior(GeneralIndex(42)) + .expect("multilocation will only have 2 junctions; qed"); + let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; + assert!( + AssetsFrom::::filter_asset_location( + &asset, + &SomeSiblingParachain::get() + ), + "AssetsFrom should allow assets from any of its interior locations" + ); } } From 827235e87eaae60c45bf03da0847ba2312d5ee0f Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Thu, 20 Jan 2022 18:01:29 +0100 Subject: [PATCH 02/10] allow KSM from Statemint --- polkadot-parachains/rococo-parachain/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/polkadot-parachains/rococo-parachain/src/lib.rs b/polkadot-parachains/rococo-parachain/src/lib.rs index 6d036e13def..073457de3fe 100644 --- a/polkadot-parachains/rococo-parachain/src/lib.rs +++ b/polkadot-parachains/rococo-parachain/src/lib.rs @@ -75,7 +75,7 @@ use xcm_builder::{ EnsureXcmOrigin, FixedWeightBounds, IsConcrete, LocationInverter, NativeAsset, ParentAsSuperuser, ParentIsDefault, RelayChainAsNative, SiblingParachainAsNative, SiblingParachainConvertsVia, SignedAccountId32AsNative, SignedToAccountId32, - SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, + SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, Case, }; use xcm_executor::{Config, XcmExecutor}; @@ -391,9 +391,17 @@ parameter_types! { // Statemint's Assets pallet index pub StatemintAssetsPalletLocation: MultiLocation = MultiLocation::new(1, X2(Parachain(1000), PalletInstance(50))); + pub KsmFilter: MultiAssetFilter = MultiAssetFilter::Wild(WildMultiAsset::AllOf{ id: (1, Here).into(), fun: WildFungibility::Fungible }); + pub KsmFromStatemint: (MultiAssetFilter, MultiLocation) = ( + KsmFilter::get(), StatemintLocation::get() + ); } -pub type Reserves = (NativeAsset, AssetsFrom); +pub type Reserves = ( + NativeAsset, + AssetsFrom, + Case, +); pub struct XcmConfig; impl Config for XcmConfig { From 6a2d2aeb18fb81f9cf623afb9593a5a884b9be1f Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 25 Jan 2022 13:41:43 +0100 Subject: [PATCH 03/10] cargo +nightly fmt --- polkadot-parachains/parachains-common/src/impls.rs | 8 ++++---- polkadot-parachains/rococo-parachain/src/lib.rs | 12 ++++-------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index 7069dc4a114..81d6e540a2a 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -111,11 +111,11 @@ impl> FilterAssetLocation for AssetsFrom { log::trace!(target: "xcm::AssetsFrom", "loc: {:?}, origin: {:?}", loc, origin); &loc == origin && match asset { - MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } => { + MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } => loc.parent_count() == asset_loc.parent_count() && - loc.interior().iter().zip(asset_loc.interior().iter()) - .all(|(prefix_junction, asset_junction)| prefix_junction == asset_junction) - }, + loc.interior().iter().zip(asset_loc.interior().iter()).all( + |(prefix_junction, asset_junction)| prefix_junction == asset_junction, + ), _ => false, } } diff --git a/polkadot-parachains/rococo-parachain/src/lib.rs b/polkadot-parachains/rococo-parachain/src/lib.rs index 073457de3fe..b27758eedea 100644 --- a/polkadot-parachains/rococo-parachain/src/lib.rs +++ b/polkadot-parachains/rococo-parachain/src/lib.rs @@ -71,11 +71,11 @@ use pallet_xcm::{EnsureXcm, IsMajorityOfBody, XcmPassthrough}; use polkadot_parachain::primitives::Sibling; use xcm::latest::prelude::*; use xcm_builder::{ - AccountId32Aliases, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, CurrencyAdapter, - EnsureXcmOrigin, FixedWeightBounds, IsConcrete, LocationInverter, NativeAsset, + AccountId32Aliases, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, Case, + CurrencyAdapter, EnsureXcmOrigin, FixedWeightBounds, IsConcrete, LocationInverter, NativeAsset, ParentAsSuperuser, ParentIsDefault, RelayChainAsNative, SiblingParachainAsNative, SiblingParachainConvertsVia, SignedAccountId32AsNative, SignedToAccountId32, - SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, Case, + SovereignSignedViaLocation, TakeWeightCredit, UsingComponents, }; use xcm_executor::{Config, XcmExecutor}; @@ -397,11 +397,7 @@ parameter_types! { ); } -pub type Reserves = ( - NativeAsset, - AssetsFrom, - Case, -); +pub type Reserves = (NativeAsset, AssetsFrom, Case); pub struct XcmConfig; impl Config for XcmConfig { From 76bf9d3b4917501d345b7299f8c31b95ff4a42b9 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 25 Jan 2022 13:51:17 +0100 Subject: [PATCH 04/10] add log crate to parachains-common --- Cargo.lock | 1 + polkadot-parachains/parachains-common/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 01a0ad13299..db42baed86a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6264,6 +6264,7 @@ dependencies = [ "frame-executive", "frame-support", "frame-system", + "log", "pallet-asset-tx-payment", "pallet-assets", "pallet-authorship", diff --git a/polkadot-parachains/parachains-common/Cargo.toml b/polkadot-parachains/parachains-common/Cargo.toml index 6099b2036c3..64ac36f3b35 100644 --- a/polkadot-parachains/parachains-common/Cargo.toml +++ b/polkadot-parachains/parachains-common/Cargo.toml @@ -11,6 +11,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] # External dependencies codec = { package = "parity-scale-codec", version = "2.3.0", features = ["derive"], default-features = false } +log = { version = "0.4.14", default-features = false } scale-info = { version = "1.0.0", default-features = false, features = ["derive"] } # Substrate dependencies From 99b3a94087a8e0103584dce60d636b76e5be7611 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 26 Jan 2022 16:46:16 +0100 Subject: [PATCH 05/10] fix AssetsFrom to not accept assets that don't share the prefix ccccccnckuhdkurdfdlbnirhtfcdncknvceelecdeeek --- .../parachains-common/src/impls.rs | 95 +++++++++++++------ 1 file changed, 65 insertions(+), 30 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index d2782fd9608..f790d050759 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -108,6 +108,7 @@ impl> FilterAssetLocation for AssetsFrom { match asset { MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } => loc.parent_count() == asset_loc.parent_count() && + asset_loc.len() >= loc.len() && loc.interior().iter().zip(asset_loc.interior().iter()).all( |(prefix_junction, asset_junction)| prefix_junction == asset_junction, ), @@ -267,39 +268,73 @@ mod tests { }); } - #[test] - fn assets_from_filters_correctly() { + mod assets_from { + use super::*; + parameter_types! { pub SomeSiblingParachain: MultiLocation = MultiLocation::new(1, X1(Parachain(1234))); } - let asset_location = SomeSiblingParachain::get() - .clone() - .pushed_with_interior(GeneralIndex(42)) - .expect("multilocation will only have 2 junctions; qed"); - let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; - assert!( - AssetsFrom::::filter_asset_location( - &asset, - &SomeSiblingParachain::get() - ), - "AssetsFrom should allow assets from any of its interior locations" - ); - - // make sure assets can have more than one junction inside the prefix location - let asset_location = SomeSiblingParachain::get() - .clone() - .pushed_with_interior(PalletInstance(50)) - .unwrap() - .pushed_with_interior(GeneralIndex(42)) - .expect("multilocation will only have 2 junctions; qed"); - let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; - assert!( - AssetsFrom::::filter_asset_location( - &asset, - &SomeSiblingParachain::get() - ), - "AssetsFrom should allow assets from any of its interior locations" - ); + #[test] + fn accepts_native_asset() { + let asset_location = SomeSiblingParachain::get(); + let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; + assert!( + AssetsFrom::::filter_asset_location( + &asset, + &SomeSiblingParachain::get() + ), + "AssetsFrom should allow the native asset" + ); + } + + #[test] + fn filters_correctly() { + let asset_location = SomeSiblingParachain::get() + .clone() + .pushed_with_interior(GeneralIndex(42)) + .expect("multilocation will only have 2 junctions; qed"); + let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; + assert!( + AssetsFrom::::filter_asset_location( + &asset, + &SomeSiblingParachain::get() + ), + "AssetsFrom should allow assets from any of its interior locations" + ); + } + + #[test] + fn accepts_long_suffix() { + // make sure assets can have more than one junction inside the prefix location + let asset_location = SomeSiblingParachain::get() + .clone() + .pushed_with_interior(PalletInstance(50)) + .unwrap() + .pushed_with_interior(GeneralIndex(42)) + .expect("multilocation will only have 3 junctions; qed"); + let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; + assert!( + AssetsFrom::::filter_asset_location( + &asset, + &SomeSiblingParachain::get() + ), + "AssetsFrom should allow assets from any of its interior locations" + ); + } + + #[test] + fn rejects_short_location() { + let asset_location = MultiLocation::parent(); + // make sure assets can have more than one junction inside the prefix location + let asset = MultiAsset { id: Concrete(asset_location), fun: 1_000_000.into() }; + assert!( + !AssetsFrom::::filter_asset_location( + &asset, + &SomeSiblingParachain::get() + ), + "AssetsFrom should filter assets not from the origin" + ); + } } } From e95949454bedaaa24e0c70b04a3f0425a8219c93 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 26 Jan 2022 17:01:45 +0100 Subject: [PATCH 06/10] extract match_prefix --- .../parachains-common/src/impls.rs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index f790d050759..12b6374b6c8 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -98,20 +98,37 @@ where } } +/// Tests `loc` whether it contains `prefix`. +/// Similar to `MultiLocation::match_and_split` but allows arbitrary suffixes (instead of just one +/// junction). +/// +/// ``` +/// # use xcm::prelude::*; +/// # use parachains_common::impls::match_prefix; +/// let prefix = MultiLocation::new(1, X1(Parachain(1234))); +/// let loc = MultiLocation::new(1, X2(Parachain(1234), GeneralIndex(5))); +/// assert!(match_prefix(&prefix, &loc)); +/// let failing = MultiLocation::new(1, Here); +/// assert!(!match_prefix(&prefix, &failing)); +/// ``` +pub fn match_prefix(prefix: &MultiLocation, loc: &MultiLocation) -> bool { + prefix.parent_count() == loc.parent_count() && + loc.len() >= prefix.len() && + prefix.interior().iter().zip(loc.interior().iter()).all( + |(prefix_junction, junction)| prefix_junction == junction, + ) +} + /// Asset filter that allows all assets from a certain location. pub struct AssetsFrom(PhantomData); impl> FilterAssetLocation for AssetsFrom { fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool { - let loc = T::get(); - log::trace!(target: "xcm::AssetsFrom", "loc: {:?}, origin: {:?}", loc, origin); - &loc == origin && + let prefix = T::get(); + log::trace!(target: "xcm::AssetsFrom", "prefix: {:?}, origin: {:?}", prefix, origin); + &prefix == origin && match asset { MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } => - loc.parent_count() == asset_loc.parent_count() && - asset_loc.len() >= loc.len() && - loc.interior().iter().zip(asset_loc.interior().iter()).all( - |(prefix_junction, asset_junction)| prefix_junction == asset_junction, - ), + match_prefix(&prefix, asset_loc), _ => false, } } From 529e18591fc3106869de7b406d55f799b45601c7 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 26 Jan 2022 17:04:34 +0100 Subject: [PATCH 07/10] formatting --- polkadot-parachains/parachains-common/src/impls.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index 12b6374b6c8..72bb8c45224 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -114,9 +114,11 @@ where pub fn match_prefix(prefix: &MultiLocation, loc: &MultiLocation) -> bool { prefix.parent_count() == loc.parent_count() && loc.len() >= prefix.len() && - prefix.interior().iter().zip(loc.interior().iter()).all( - |(prefix_junction, junction)| prefix_junction == junction, - ) + prefix + .interior() + .iter() + .zip(loc.interior().iter()) + .all(|(prefix_junction, junction)| prefix_junction == junction) } /// Asset filter that allows all assets from a certain location. From 0c6fac5ab52d2866c3bd22971cc8db8b5f4f0d16 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 1 Feb 2022 17:48:17 +0100 Subject: [PATCH 08/10] rename match_prefix and make private in anticipation of upstreaming --- .../parachains-common/src/impls.rs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index 72bb8c45224..7c33708f9f7 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -98,20 +98,12 @@ where } } -/// Tests `loc` whether it contains `prefix`. +/// Tests `loc` whether it starts with `prefix`. /// Similar to `MultiLocation::match_and_split` but allows arbitrary suffixes (instead of just one /// junction). -/// -/// ``` -/// # use xcm::prelude::*; -/// # use parachains_common::impls::match_prefix; -/// let prefix = MultiLocation::new(1, X1(Parachain(1234))); -/// let loc = MultiLocation::new(1, X2(Parachain(1234), GeneralIndex(5))); -/// assert!(match_prefix(&prefix, &loc)); -/// let failing = MultiLocation::new(1, Here); -/// assert!(!match_prefix(&prefix, &failing)); -/// ``` -pub fn match_prefix(prefix: &MultiLocation, loc: &MultiLocation) -> bool { +// TODO: Replace with version implemented on MultiLocation here: +// https://github.com/paritytech/polkadot/pull/4827 +fn matches_prefix(prefix: &MultiLocation, loc: &MultiLocation) -> bool { prefix.parent_count() == loc.parent_count() && loc.len() >= prefix.len() && prefix @@ -130,7 +122,7 @@ impl> FilterAssetLocation for AssetsFrom { &prefix == origin && match asset { MultiAsset { id: AssetId::Concrete(asset_loc), fun: Fungible(_a) } => - match_prefix(&prefix, asset_loc), + matches_prefix(&prefix, asset_loc), _ => false, } } @@ -287,6 +279,15 @@ mod tests { }); } + #[test] + fn test_matches_prefix_works() { + let prefix = MultiLocation::new(1, X1(Parachain(1234))); + let loc = MultiLocation::new(1, X2(Parachain(1234), GeneralIndex(5))); + assert!(matches_prefix(&prefix, &loc)); + let failing = MultiLocation::new(1, Here); + assert!(!matches_prefix(&prefix, &failing)); + } + mod assets_from { use super::*; From 8f3d73ef6688dd7c3255db465b522abb543e795c Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 1 Feb 2022 17:52:35 +0100 Subject: [PATCH 09/10] formatting --- polkadot-parachains/parachains-common/src/impls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index 7c33708f9f7..37ea05cab77 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -281,10 +281,10 @@ mod tests { #[test] fn test_matches_prefix_works() { - let prefix = MultiLocation::new(1, X1(Parachain(1234))); - let loc = MultiLocation::new(1, X2(Parachain(1234), GeneralIndex(5))); + let prefix = MultiLocation::new(1, X1(Parachain(1234))); + let loc = MultiLocation::new(1, X2(Parachain(1234), GeneralIndex(5))); assert!(matches_prefix(&prefix, &loc)); - let failing = MultiLocation::new(1, Here); + let failing = MultiLocation::new(1, Here); assert!(!matches_prefix(&prefix, &failing)); } From e00772d62cb24ed34dc376002d329fd3ec5f690e Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Fri, 18 Feb 2022 10:10:07 +0000 Subject: [PATCH 10/10] update TODO comment pointing to upstream starts_with PR --- polkadot-parachains/parachains-common/src/impls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot-parachains/parachains-common/src/impls.rs b/polkadot-parachains/parachains-common/src/impls.rs index 37ea05cab77..ad06e3e3d94 100644 --- a/polkadot-parachains/parachains-common/src/impls.rs +++ b/polkadot-parachains/parachains-common/src/impls.rs @@ -101,8 +101,8 @@ where /// Tests `loc` whether it starts with `prefix`. /// Similar to `MultiLocation::match_and_split` but allows arbitrary suffixes (instead of just one /// junction). -// TODO: Replace with version implemented on MultiLocation here: -// https://github.com/paritytech/polkadot/pull/4827 +// TODO: Replace usage with `starts_with` implemented on MultiLocation here once XCM v3 is merged: +// https://github.com/paritytech/polkadot/pull/4835 fn matches_prefix(prefix: &MultiLocation, loc: &MultiLocation) -> bool { prefix.parent_count() == loc.parent_count() && loc.len() >= prefix.len() &&