From 3a29f1024dbf94ea2a9667418958ad75fd6e921d Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Fri, 4 Mar 2022 10:00:59 +0800 Subject: [PATCH 1/6] Add WhiteListingMultiLocations trait --- xtokens/src/lib.rs | 26 ++++++++++++++++++++++++++ xtokens/src/mock/para.rs | 1 + 2 files changed, 27 insertions(+) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 5a6fd3810..3e4095d49 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -52,6 +52,29 @@ enum TransferKind { } use TransferKind::*; +pub trait WhiteListingMultiLocations { + fn get_supported_locations(&self, currency_id: CurrencyId) -> Option; + fn get_all_supported_locations(&self) -> Option>; + fn get_xcm_fee(&self, currency_id: CurrencyId) -> Balance; +} + +impl WhiteListingMultiLocations for () +where + Balance: Zero, +{ + fn get_supported_locations(&self, _: CurrencyId) -> Option { + None + } + + fn get_all_supported_locations(&self) -> Option> { + None + } + + fn get_xcm_fee(&self, _: CurrencyId) -> Balance { + Zero::zero() + } +} + #[frame_support::pallet] pub mod module { @@ -86,6 +109,9 @@ pub mod module { /// XCM executor. type XcmExecutor: ExecuteXcm; + /// + type WhiteListingMultiLocations: WhiteListingMultiLocations; + /// Means of measuring the weight consumed by an XCM message locally. type Weigher: WeightBounds; diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index ef46b3de3..3b1be35db 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -279,6 +279,7 @@ impl orml_xtokens::Config for Runtime { type CurrencyIdConvert = CurrencyIdConvert; type AccountIdToMultiLocation = AccountIdToMultiLocation; type SelfLocation = SelfLocation; + type WhiteListingMultiLocations = (); type XcmExecutor = XcmExecutor; type Weigher = FixedWeightBounds; type BaseXcmWeight = BaseXcmWeight; From 5cd065fcb74fde33e29dda83e99d093cd7ecab7b Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Fri, 11 Mar 2022 12:18:35 +0800 Subject: [PATCH 2/6] Use Contains to filter multilocations Signed-off-by: Dengjianping --- xtokens/src/lib.rs | 48 +++++++++++++++++++--------------------- xtokens/src/mock/para.rs | 46 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 27 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 3e4095d49..1de111d9e 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -22,7 +22,13 @@ #![allow(clippy::unused_unit)] #![allow(clippy::large_enum_variant)] -use frame_support::{log, pallet_prelude::*, require_transactional, traits::Get, transactional, Parameter}; +use frame_support::{ + log, + pallet_prelude::*, + require_transactional, + traits::{Contains, Get}, + transactional, Parameter, +}; use frame_system::{ensure_signed, pallet_prelude::*}; use sp_runtime::{ traits::{AtLeast32BitUnsigned, Convert, MaybeSerializeDeserialize, Member, Zero}, @@ -52,29 +58,6 @@ enum TransferKind { } use TransferKind::*; -pub trait WhiteListingMultiLocations { - fn get_supported_locations(&self, currency_id: CurrencyId) -> Option; - fn get_all_supported_locations(&self) -> Option>; - fn get_xcm_fee(&self, currency_id: CurrencyId) -> Balance; -} - -impl WhiteListingMultiLocations for () -where - Balance: Zero, -{ - fn get_supported_locations(&self, _: CurrencyId) -> Option { - None - } - - fn get_all_supported_locations(&self) -> Option> { - None - } - - fn get_xcm_fee(&self, _: CurrencyId) -> Balance { - Zero::zero() - } -} - #[frame_support::pallet] pub mod module { @@ -110,7 +93,7 @@ pub mod module { type XcmExecutor: ExecuteXcm; /// - type WhiteListingMultiLocations: WhiteListingMultiLocations; + type WhiteListingMultiLocations: Contains; /// Means of measuring the weight consumed by an XCM message locally. type Weigher: WeightBounds; @@ -180,6 +163,8 @@ pub mod module { TooManyAssetsBeingSent, /// The specified index does not exist in a MultiAssets struct AssetIndexNonExistent, + /// Not supported location + NotInWhiteListLocation, } #[pallet::hooks] @@ -394,6 +379,10 @@ pub mod module { T::CurrencyIdConvert::convert(currency_id).ok_or(Error::::NotCrossChainTransferableCurrency)?; ensure!(!amount.is_zero(), Error::::ZeroAmount); + ensure!( + T::WhiteListingMultiLocations::contains(&location), + Error::::NotInWhiteListLocation + ); let asset: MultiAsset = (location, amount.into()).into(); Self::do_transfer_multiassets(who, vec![asset.clone()].into(), asset, dest, dest_weight) @@ -412,6 +401,10 @@ pub mod module { ensure!(!amount.is_zero(), Error::::ZeroAmount); ensure!(!fee.is_zero(), Error::::ZeroFee); + ensure!( + T::WhiteListingMultiLocations::contains(&location), + Error::::NotInWhiteListLocation + ); let asset = (location.clone(), amount.into()).into(); let fee_asset: MultiAsset = (location, fee.into()).into(); @@ -468,6 +461,11 @@ pub mod module { let location: MultiLocation = T::CurrencyIdConvert::convert(currency_id.clone()) .ok_or(Error::::NotCrossChainTransferableCurrency)?; ensure!(!amount.is_zero(), Error::::ZeroAmount); + ensure!( + T::WhiteListingMultiLocations::contains(&location), + Error::::NotInWhiteListLocation + ); + // Push contains saturated addition, so we should be able to use it safely assets.push((location, (*amount).into()).into()) } diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 3b1be35db..d0b3d6470 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -3,7 +3,7 @@ use crate as orml_xtokens; use frame_support::{ construct_runtime, parameter_types, - traits::{Everything, Get, Nothing}, + traits::{Contains, Everything, Get, Nothing}, weights::{constants::WEIGHT_PER_SECOND, Weight}, }; use frame_system::EnsureRoot; @@ -270,6 +270,48 @@ parameter_types! { pub SelfLocation: MultiLocation = MultiLocation::new(1, X1(Parachain(ParachainInfo::get().into()))); pub const BaseXcmWeight: Weight = 100_000_000; pub const MaxAssetsForTransfer: usize = 2; + pub SupportedMultiLocations: Vec = vec![ + MultiLocation::new( + 1, + X2( + Parachain(1), + GeneralKey("A".into()) + ) + ), + MultiLocation::new( + 1, + X2( + Parachain(1), + GeneralKey("A1".into()) + ) + ), + MultiLocation::new( + 1, + X2( + Parachain(2), + GeneralKey("B".into()) + ) + ), + MultiLocation::new( + 1, + X2( + Parachain(2), + GeneralKey("B1".into()) + ) + ), + MultiLocation::new( + 1, + Junctions::Here + ) + ]; +} + +pub struct WhiteListingMultiLocations; +impl Contains for WhiteListingMultiLocations { + fn contains(location: &MultiLocation) -> bool { + dbg!(&location); + SupportedMultiLocations::get().contains(location) + } } impl orml_xtokens::Config for Runtime { @@ -279,7 +321,7 @@ impl orml_xtokens::Config for Runtime { type CurrencyIdConvert = CurrencyIdConvert; type AccountIdToMultiLocation = AccountIdToMultiLocation; type SelfLocation = SelfLocation; - type WhiteListingMultiLocations = (); + type WhiteListingMultiLocations = WhiteListingMultiLocations; type XcmExecutor = XcmExecutor; type Weigher = FixedWeightBounds; type BaseXcmWeight = BaseXcmWeight; From c7c36e804b2536150c0d81f4c532e4934ffa5203 Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Fri, 11 Mar 2022 12:42:02 +0800 Subject: [PATCH 3/6] Remove dbg Signed-off-by: Dengjianping --- xtokens/src/mock/para.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index e080802bb..6105b5e26 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -316,7 +316,6 @@ parameter_types! { pub struct WhiteListingMultiLocations; impl Contains for WhiteListingMultiLocations { fn contains(location: &MultiLocation) -> bool { - dbg!(&location); SupportedMultiLocations::get().contains(location) } } From 32bb4c99a5950c4cf37067ca4ed91f7d4799fb1d Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Mon, 14 Mar 2022 20:56:55 +0800 Subject: [PATCH 4/6] Add a test case to cover filter function Signed-off-by: Dengjianping --- xtokens/src/lib.rs | 20 +++++++------- xtokens/src/mock/para.rs | 58 +++++++++++++++++++--------------------- xtokens/src/tests.rs | 29 ++++++++++++++++++++ 3 files changed, 67 insertions(+), 40 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 2516286cd..d8fd0990b 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -94,8 +94,8 @@ pub mod module { /// XCM executor. type XcmExecutor: ExecuteXcm; - /// - type WhiteListingMultiLocations: Contains; + /// MultiLocation filter + type MultiLocationsFilter: Contains; /// Means of measuring the weight consumed by an XCM message locally. type Weigher: WeightBounds; @@ -167,8 +167,8 @@ pub mod module { AssetIndexNonExistent, /// Fee is not enough. FeeNotEnough, - /// Not supported location - NotInWhiteListLocation, + /// Not supported MultiLocation + NotSupportedMultiLocation, } #[pallet::hooks] @@ -384,8 +384,8 @@ pub mod module { ensure!(!amount.is_zero(), Error::::ZeroAmount); ensure!( - T::WhiteListingMultiLocations::contains(&location), - Error::::NotInWhiteListLocation + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation ); let asset: MultiAsset = (location, amount.into()).into(); @@ -406,8 +406,8 @@ pub mod module { ensure!(!amount.is_zero(), Error::::ZeroAmount); ensure!(!fee.is_zero(), Error::::ZeroFee); ensure!( - T::WhiteListingMultiLocations::contains(&location), - Error::::NotInWhiteListLocation + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation ); let asset = (location.clone(), amount.into()).into(); @@ -466,8 +466,8 @@ pub mod module { .ok_or(Error::::NotCrossChainTransferableCurrency)?; ensure!(!amount.is_zero(), Error::::ZeroAmount); ensure!( - T::WhiteListingMultiLocations::contains(&location), - Error::::NotInWhiteListLocation + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation ); // Push contains saturated addition, so we should be able to use it safely diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 6105b5e26..81ce642c5 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -273,50 +273,48 @@ parameter_types! { pub SupportedMultiLocations: Vec = vec![ MultiLocation::new( 1, - X2( - Parachain(1), - GeneralKey("A".into()) - ) + X1(Parachain(1)) ), MultiLocation::new( 1, - X2( - Parachain(1), - GeneralKey("A1".into()) - ) + X1(Parachain(2)) ), MultiLocation::new( 1, - X2( - Parachain(2), - GeneralKey("B".into()) - ) + X1(Parachain(3)) ), + // There's a test that uses para id = 100 MultiLocation::new( 1, - X2( - Parachain(2), - GeneralKey("B1".into()) - ) - ), - MultiLocation::new( - 1, - X2( - Parachain(2), - GeneralKey("B2".into()) - ) - ), - MultiLocation::new( - 1, - Junctions::Here + X1(Parachain(100)) ) ]; } pub struct WhiteListingMultiLocations; impl Contains for WhiteListingMultiLocations { - fn contains(location: &MultiLocation) -> bool { - SupportedMultiLocations::get().contains(location) + fn contains(dest: &MultiLocation) -> bool { + if dest.parent_count() != 1 { + return false; + } + + if let Junctions::X1(Junction::AccountId32 { .. }) = dest.interior() { + // parent = 1, and the junctions is the variant X1 + // means the asset will be sent to relaychain. + true + } else { + // if the asset is sent to parachain, + // the junctions must have the variant Parachain + dest.interior().iter().any(|i| match i { + Junction::Parachain(parachain_id) => SupportedMultiLocations::get().iter().any(|location| { + location + .interior + .iter() + .any(|junc| *junc == Junction::Parachain(*parachain_id)) + }), + _ => false, // No parachain id found. + }) + } } } @@ -337,7 +335,7 @@ impl orml_xtokens::Config for Runtime { type CurrencyIdConvert = CurrencyIdConvert; type AccountIdToMultiLocation = AccountIdToMultiLocation; type SelfLocation = SelfLocation; - type WhiteListingMultiLocations = WhiteListingMultiLocations; + type MultiLocationsFilter = WhiteListingMultiLocations; type MinXcmFee = ParachainMinFee; type XcmExecutor = XcmExecutor; type Weigher = FixedWeightBounds; diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index bb4ce5f60..3385121e8 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -1335,3 +1335,32 @@ fn send_with_zero_amount() { // TODO: should have more tests after https://github.com/paritytech/polkadot/issues/4996 } + +#[test] +fn unsupported_multilocation_should_be_filtered() { + TestNet::reset(); + + ParaB::execute_with(|| { + assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 1_000)); + assert_noop!( + ParaXTokens::transfer( + Some(ALICE).into(), + CurrencyId::B, + 500, + Box::new( + ( + Parent, + Parachain(4), // parachain 4 is not supported list. + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into(), + }, + ) + .into() + ), + 40, + ), + Error::::NotSupportedMultiLocation + ); + }); +} From eed40a217c5de4ee0f5dffcbedd852d4bc3a4b09 Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Tue, 15 Mar 2022 18:05:44 +0800 Subject: [PATCH 5/6] Add filter check to do_transfer_multiassets and update related test case Signed-off-by: Dengjianping --- xtokens/src/lib.rs | 12 ++++++++---- xtokens/src/mock/para.rs | 5 +++-- xtokens/src/tests.rs | 22 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index abdd00443..6165264f4 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -458,6 +458,10 @@ pub mod module { currencies.len() <= T::MaxAssetsForTransfer::get(), Error::::TooManyAssetsBeingSent ); + ensure!( + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation + ); let mut assets = MultiAssets::new(); @@ -470,10 +474,6 @@ pub mod module { let location: MultiLocation = T::CurrencyIdConvert::convert(currency_id.clone()) .ok_or(Error::::NotCrossChainTransferableCurrency)?; ensure!(!amount.is_zero(), Error::::ZeroAmount); - ensure!( - T::MultiLocationsFilter::contains(&dest), - Error::::NotSupportedMultiLocation - ); // Push contains saturated addition, so we should be able to use it safely assets.push((location, (*amount).into()).into()) @@ -500,6 +500,10 @@ pub mod module { assets.len() <= T::MaxAssetsForTransfer::get(), Error::::TooManyAssetsBeingSent ); + ensure!( + T::MultiLocationsFilter::contains(&dest), + Error::::NotSupportedMultiLocation + ); let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); let mut non_fee_reserve: Option = None; diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 81ce642c5..93664687d 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -294,12 +294,13 @@ parameter_types! { pub struct WhiteListingMultiLocations; impl Contains for WhiteListingMultiLocations { fn contains(dest: &MultiLocation) -> bool { - if dest.parent_count() != 1 { + // Consider children parachain, means parent = 0 + if dest.parent_count() != 0 && dest.parent_count() != 1 { return false; } if let Junctions::X1(Junction::AccountId32 { .. }) = dest.interior() { - // parent = 1, and the junctions is the variant X1 + // parent = 1 or 0, and the junctions is the variant X1 // means the asset will be sent to relaychain. true } else { diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 3385121e8..2b68d35e8 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -1342,6 +1342,7 @@ fn unsupported_multilocation_should_be_filtered() { ParaB::execute_with(|| { assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 1_000)); + assert_ok!(ParaTokens::deposit(CurrencyId::B1, &ALICE, 1_000)); assert_noop!( ParaXTokens::transfer( Some(ALICE).into(), @@ -1362,5 +1363,26 @@ fn unsupported_multilocation_should_be_filtered() { ), Error::::NotSupportedMultiLocation ); + + assert_noop!( + ParaXTokens::transfer_multicurrencies( + Some(ALICE).into(), + vec![(CurrencyId::B1, 50), (CurrencyId::B, 450)], + 0, + Box::new( + ( + Parent, + Parachain(4), + Junction::AccountId32 { + network: NetworkId::Any, + id: BOB.into(), + }, + ) + .into() + ), + 40, + ), + Error::::NotSupportedMultiLocation + ); }); } From 8ff5aa1a2a77a829a9f72ae8749548dbf0ac3989 Mon Sep 17 00:00:00 2001 From: Dengjianping Date: Fri, 18 Mar 2022 22:05:12 +0800 Subject: [PATCH 6/6] Use macro match_type Signed-off-by: Dengjianping --- xtokens/src/mock/para.rs | 60 ++++++++-------------------------------- 1 file changed, 12 insertions(+), 48 deletions(-) diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 93664687d..609dd7b8c 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -2,8 +2,8 @@ use super::{Amount, Balance, CurrencyId, CurrencyIdConvert, ParachainXcmRouter}; use crate as orml_xtokens; use frame_support::{ - construct_runtime, parameter_types, - traits::{Contains, Everything, Get, Nothing}, + construct_runtime, match_type, parameter_types, + traits::{Everything, Get, Nothing}, weights::{constants::WEIGHT_PER_SECOND, Weight}, }; use frame_system::EnsureRoot; @@ -270,53 +270,17 @@ parameter_types! { pub SelfLocation: MultiLocation = MultiLocation::new(1, X1(Parachain(ParachainInfo::get().into()))); pub const BaseXcmWeight: Weight = 100_000_000; pub const MaxAssetsForTransfer: usize = 3; - pub SupportedMultiLocations: Vec = vec![ - MultiLocation::new( - 1, - X1(Parachain(1)) - ), - MultiLocation::new( - 1, - X1(Parachain(2)) - ), - MultiLocation::new( - 1, - X1(Parachain(3)) - ), - // There's a test that uses para id = 100 - MultiLocation::new( - 1, - X1(Parachain(100)) - ) - ]; } -pub struct WhiteListingMultiLocations; -impl Contains for WhiteListingMultiLocations { - fn contains(dest: &MultiLocation) -> bool { - // Consider children parachain, means parent = 0 - if dest.parent_count() != 0 && dest.parent_count() != 1 { - return false; - } - - if let Junctions::X1(Junction::AccountId32 { .. }) = dest.interior() { - // parent = 1 or 0, and the junctions is the variant X1 - // means the asset will be sent to relaychain. - true - } else { - // if the asset is sent to parachain, - // the junctions must have the variant Parachain - dest.interior().iter().any(|i| match i { - Junction::Parachain(parachain_id) => SupportedMultiLocations::get().iter().any(|location| { - location - .interior - .iter() - .any(|junc| *junc == Junction::Parachain(*parachain_id)) - }), - _ => false, // No parachain id found. - }) - } - } +match_type! { + pub type ParentOrParachains: impl Contains = { + MultiLocation { parents: 0, interior: X1(Junction::AccountId32 { .. }) } | + MultiLocation { parents: 1, interior: X1(Junction::AccountId32 { .. }) } | + MultiLocation { parents: 1, interior: X2(Parachain(1), Junction::AccountId32 { .. }) } | + MultiLocation { parents: 1, interior: X2(Parachain(2), Junction::AccountId32 { .. }) } | + MultiLocation { parents: 1, interior: X2(Parachain(3), Junction::AccountId32 { .. }) } | + MultiLocation { parents: 1, interior: X2(Parachain(100), Junction::AccountId32 { .. }) } + }; } parameter_type_with_key! { @@ -336,7 +300,7 @@ impl orml_xtokens::Config for Runtime { type CurrencyIdConvert = CurrencyIdConvert; type AccountIdToMultiLocation = AccountIdToMultiLocation; type SelfLocation = SelfLocation; - type MultiLocationsFilter = WhiteListingMultiLocations; + type MultiLocationsFilter = ParentOrParachains; type MinXcmFee = ParachainMinFee; type XcmExecutor = XcmExecutor; type Weigher = FixedWeightBounds;