From ed1023839919cdc3a11368c11e6a5f173535705b Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Mon, 3 Apr 2023 21:00:05 -0500 Subject: [PATCH 01/10] check for supported multilocation type when creating/updating assets Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 51af3f703..6dc008abe 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -261,6 +261,9 @@ pub mod pallet { /// Location Already Exists LocationAlreadyExists, + /// MultiLocation Type Not Supported + LocationNotSupported, + /// An error occured while creating a new asset at the [`AssetRegistry`]. ErrorCreatingAsset, @@ -348,6 +351,7 @@ pub mod pallet { !LocationAssetId::::contains_key(&location), Error::::LocationAlreadyExists ); + ensure!(Self::contains(&location), Error::::LocationNotSupported); let asset_id = Self::next_asset_id_and_increment()?; >::AssetRegistry::create_asset( asset_id, @@ -399,6 +403,7 @@ pub mod pallet { !LocationAssetId::::contains_key(&location), Error::::LocationAlreadyExists ); + ensure!(Self::contains(&location), Error::::LocationNotSupported); // change the ledger state. let old_location = AssetIdLocation::::get(asset_id).ok_or(Error::::UpdateNonExistentAsset)?; From 38562ec360f08c32dd57da72fcbf91cf2b263910 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Mon, 3 Apr 2023 23:01:26 -0500 Subject: [PATCH 02/10] check for compatibility only if location is convertible to a Multilocation Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 6dc008abe..dfa61ea14 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -351,7 +351,9 @@ pub mod pallet { !LocationAssetId::::contains_key(&location), Error::::LocationAlreadyExists ); - ensure!(Self::contains(&location), Error::::LocationNotSupported); + if let Some(multi) = location.clone().into() { + ensure!(Self::contains(&multi), Error::::LocationNotSupported); + } let asset_id = Self::next_asset_id_and_increment()?; >::AssetRegistry::create_asset( asset_id, @@ -403,7 +405,9 @@ pub mod pallet { !LocationAssetId::::contains_key(&location), Error::::LocationAlreadyExists ); - ensure!(Self::contains(&location), Error::::LocationNotSupported); + if let Some(multi) = location.clone().into() { + ensure!(Self::contains(&multi), Error::::LocationNotSupported); + } // change the ledger state. let old_location = AssetIdLocation::::get(asset_id).ok_or(Error::::UpdateNonExistentAsset)?; From e2627b103564fca92c4f746fa5d6a2231a6ea34e Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Mon, 3 Apr 2023 23:36:19 -0500 Subject: [PATCH 03/10] allow types that are used in testing Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index dfa61ea14..4487f8ea2 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -627,13 +627,14 @@ pub mod pallet { } match location.interior { + // Local transfers + Junctions::Here => true, // Send tokens back to relaychain. Junctions::X1(Junction::AccountId32 { .. }) => true, // Send tokens to sibling chain. Junctions::X2(Junction::Parachain(para_id), Junction::AccountId32 { .. }) - | Junctions::X2(Junction::Parachain(para_id), Junction::AccountKey20 { .. }) => { - AllowedDestParaIds::::contains_key(para_id) - } + | Junctions::X2(Junction::Parachain(para_id), Junction::PalletInstance { .. }) + | Junctions::X2(Junction::Parachain(para_id), Junction::AccountKey20 { .. }) => true, // We don't support X3 or longer Junctions. _ => false, } From de063f066942a6c02be89bac90725208209e679a Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 00:12:03 -0500 Subject: [PATCH 04/10] split location shape check into its own fn Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 44 ++++++++++++++++++++++-------- runtime/calamari/src/xcm_config.rs | 1 + 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 4487f8ea2..fdb876e8c 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -352,7 +352,10 @@ pub mod pallet { Error::::LocationAlreadyExists ); if let Some(multi) = location.clone().into() { - ensure!(Self::contains(&multi), Error::::LocationNotSupported); + ensure!( + Self::is_allowed_location_shape(&multi), + Error::::LocationNotSupported + ); } let asset_id = Self::next_asset_id_and_increment()?; >::AssetRegistry::create_asset( @@ -406,7 +409,10 @@ pub mod pallet { Error::::LocationAlreadyExists ); if let Some(multi) = location.clone().into() { - ensure!(Self::contains(&multi), Error::::LocationNotSupported); + ensure!( + Self::is_allowed_location_shape(&multi), + Error::::LocationNotSupported + ); } // change the ledger state. let old_location = @@ -612,20 +618,12 @@ pub mod pallet { Ok(()) } } - } - /// Check the multilocation destination is supported by calamari/manta. - impl Contains for Pallet - where - T: Config, - { - #[inline] - fn contains(location: &MultiLocation) -> bool { + fn is_allowed_location_shape(location: &MultiLocation) -> bool { // check parents if location.parents != 1 { return false; } - match location.interior { // Local transfers Junctions::Here => true, @@ -634,13 +632,35 @@ pub mod pallet { // Send tokens to sibling chain. Junctions::X2(Junction::Parachain(para_id), Junction::AccountId32 { .. }) | Junctions::X2(Junction::Parachain(para_id), Junction::PalletInstance { .. }) - | Junctions::X2(Junction::Parachain(para_id), Junction::AccountKey20 { .. }) => true, + | Junctions::X2(Junction::Parachain(para_id), Junction::AccountKey20 { .. }) => { + true + } // We don't support X3 or longer Junctions. _ => false, } } } + /// impl used by xtokens to filter multilocations allowed to send to + impl Contains for Pallet + where + T: Config, + { + #[inline] + fn contains(location: &MultiLocation) -> bool { + if !Self::is_allowed_location_shape(location) { + return false; + } + + // if sending to sibling, only siblings whose assets we registered are allowed + if let Junctions::X2(Junction::Parachain(para_id), _) = location.interior { + AllowedDestParaIds::::contains_key(para_id) + } else { + true + } + } + } + /// Get min-xcm-fee for reserve chain by multilocation. impl GetByKey> for Pallet where diff --git a/runtime/calamari/src/xcm_config.rs b/runtime/calamari/src/xcm_config.rs index 28dfe1558..f32e548be 100644 --- a/runtime/calamari/src/xcm_config.rs +++ b/runtime/calamari/src/xcm_config.rs @@ -350,6 +350,7 @@ impl orml_xtokens::Config for Runtime { type LocationInverter = LocationInverter; type MaxAssetsForTransfer = MaxAssetsForTransfer; type MinXcmFee = AssetManager; + /// filter multilocations our chain is allowed to send XCM to type MultiLocationsFilter = AssetManager; type ReserveProvider = AbsoluteReserveProvider; } From 3badfdcf9ba025d33218dad5d56c0f169afa67a8 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 00:15:24 -0500 Subject: [PATCH 05/10] fmt Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index fdb876e8c..932aae287 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -630,11 +630,9 @@ pub mod pallet { // Send tokens back to relaychain. Junctions::X1(Junction::AccountId32 { .. }) => true, // Send tokens to sibling chain. - Junctions::X2(Junction::Parachain(para_id), Junction::AccountId32 { .. }) - | Junctions::X2(Junction::Parachain(para_id), Junction::PalletInstance { .. }) - | Junctions::X2(Junction::Parachain(para_id), Junction::AccountKey20 { .. }) => { - true - } + Junctions::X2(Junction::Parachain { .. }, Junction::AccountId32 { .. }) + | Junctions::X2(Junction::Parachain { .. }, Junction::PalletInstance { .. }) + | Junctions::X2(Junction::Parachain { .. }, Junction::AccountKey20 { .. }) => true, // We don't support X3 or longer Junctions. _ => false, } From beb9bad6f7eccc67ebc8e0d6530cbeeaa14e7e87 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 00:22:45 -0500 Subject: [PATCH 06/10] comment Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 932aae287..775ec2fa7 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -639,7 +639,7 @@ pub mod pallet { } } - /// impl used by xtokens to filter multilocations allowed to send to + /// impl used by xtokens as `MultiLocationsFilter`. Defines where we're allowed to send to impl Contains for Pallet where T: Config, From 5536ef2c8aad49b2aae8923394e9c6bd7dc86520 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 00:23:42 -0500 Subject: [PATCH 07/10] comment Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 775ec2fa7..5e2c52039 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -619,6 +619,7 @@ pub mod pallet { } } + /// defines what types of locations can be registered as assets fn is_allowed_location_shape(location: &MultiLocation) -> bool { // check parents if location.parents != 1 { From dede51edacb7920bf0d98f71c9413bd4c8c9fc58 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 00:43:36 -0500 Subject: [PATCH 08/10] disallow junction types that dont make sense for parents=0 Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 5e2c52039..9072d8e23 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -621,19 +621,21 @@ pub mod pallet { /// defines what types of locations can be registered as assets fn is_allowed_location_shape(location: &MultiLocation) -> bool { - // check parents - if location.parents != 1 { + let p = location.parents; + if p > 1 { return false; } match location.interior { - // Local transfers - Junctions::Here => true, - // Send tokens back to relaychain. - Junctions::X1(Junction::AccountId32 { .. }) => true, + // Local transfers or relay asset + Junctions::Here => p == 0 || p == 1, + // Send tokens to relaychain. + Junctions::X1(Junction::AccountId32 { .. }) => p == 1, // Send tokens to sibling chain. Junctions::X2(Junction::Parachain { .. }, Junction::AccountId32 { .. }) | Junctions::X2(Junction::Parachain { .. }, Junction::PalletInstance { .. }) - | Junctions::X2(Junction::Parachain { .. }, Junction::AccountKey20 { .. }) => true, + | Junctions::X2(Junction::Parachain { .. }, Junction::AccountKey20 { .. }) => { + p == 1 + } // We don't support X3 or longer Junctions. _ => false, } @@ -650,7 +652,10 @@ pub mod pallet { if !Self::is_allowed_location_shape(location) { return false; } - + // xtokens / XCM must not be used to send transfers local to our parachain + if location.parents != 1 { + return false; + } // if sending to sibling, only siblings whose assets we registered are allowed if let Junctions::X2(Junction::Parachain(para_id), _) = location.interior { AllowedDestParaIds::::contains_key(para_id) From 21fe850196c644498a302907b79d3c52ff3ca159 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 00:45:38 -0500 Subject: [PATCH 09/10] parents == 0 check Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 9072d8e23..754a92a2a 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -653,7 +653,7 @@ pub mod pallet { return false; } // xtokens / XCM must not be used to send transfers local to our parachain - if location.parents != 1 { + if location.parents == 0 { return false; } // if sending to sibling, only siblings whose assets we registered are allowed From a41e44bb212ee7e42ea96e034ea8ddb7aeb8fec4 Mon Sep 17 00:00:00 2001 From: Adam Reif Date: Tue, 4 Apr 2023 02:27:17 -0500 Subject: [PATCH 10/10] split asset multilocations from dest multilocs in the contains Signed-off-by: Adam Reif --- pallets/asset-manager/src/lib.rs | 48 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/pallets/asset-manager/src/lib.rs b/pallets/asset-manager/src/lib.rs index 754a92a2a..39b411d79 100644 --- a/pallets/asset-manager/src/lib.rs +++ b/pallets/asset-manager/src/lib.rs @@ -353,7 +353,7 @@ pub mod pallet { ); if let Some(multi) = location.clone().into() { ensure!( - Self::is_allowed_location_shape(&multi), + Self::is_allowed_asset_location(&multi), Error::::LocationNotSupported ); } @@ -410,7 +410,7 @@ pub mod pallet { ); if let Some(multi) = location.clone().into() { ensure!( - Self::is_allowed_location_shape(&multi), + Self::is_allowed_asset_location(&multi), Error::::LocationNotSupported ); } @@ -620,47 +620,51 @@ pub mod pallet { } /// defines what types of locations can be registered as assets - fn is_allowed_location_shape(location: &MultiLocation) -> bool { + fn is_allowed_asset_location(location: &MultiLocation) -> bool { let p = location.parents; if p > 1 { return false; } match location.interior { - // Local transfers or relay asset + // Local or relay asset Junctions::Here => p == 0 || p == 1, - // Send tokens to relaychain. - Junctions::X1(Junction::AccountId32 { .. }) => p == 1, + // A parachain native asset + Junctions::X1(Junction::Parachain { .. }) => p == 1, // Send tokens to sibling chain. - Junctions::X2(Junction::Parachain { .. }, Junction::AccountId32 { .. }) - | Junctions::X2(Junction::Parachain { .. }, Junction::PalletInstance { .. }) - | Junctions::X2(Junction::Parachain { .. }, Junction::AccountKey20 { .. }) => { - p == 1 - } - // We don't support X3 or longer Junctions. + Junctions::X2(Junction::Parachain { .. }, Junction::PalletInstance { .. }) + | Junctions::X2(Junction::Parachain { .. }, Junction::GeneralKey { .. }) => p == 1, + Junctions::X3( + Junction::Parachain { .. }, + Junction::PalletInstance { .. }, + Junction::GeneralIndex { .. }, + ) => p == 1, _ => false, } } } - /// impl used by xtokens as `MultiLocationsFilter`. Defines where we're allowed to send to + /// impl used by xtokens as `MultiLocationsFilter`. Defines where we're allowed to **send** to impl Contains for Pallet where T: Config, { #[inline] fn contains(location: &MultiLocation) -> bool { - if !Self::is_allowed_location_shape(location) { - return false; - } // xtokens / XCM must not be used to send transfers local to our parachain - if location.parents == 0 { + // and we don't support nested chains + if location.parents != 1 { return false; } - // if sending to sibling, only siblings whose assets we registered are allowed - if let Junctions::X2(Junction::Parachain(para_id), _) = location.interior { - AllowedDestParaIds::::contains_key(para_id) - } else { - true + match location.interior { + // Send tokens to an account on the relaychain. + Junctions::X1(Junction::AccountId32 { .. }) => true, + // Send tokens to an account on a sibling chain. + Junctions::X2(Junction::Parachain(para_id), Junction::AccountId32 { .. }) + | Junctions::X2(Junction::Parachain(para_id), Junction::AccountKey20 { .. }) => { + AllowedDestParaIds::::contains_key(para_id) + } + // We don't support X3 or longer Junctions. + _ => false, } } }