From 64741c5a15115cc9f9e9ec6f5231d0f09989cc25 Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Wed, 1 Nov 2023 19:55:39 +0100 Subject: [PATCH] refactor: use collection-locality --- pallets/foreign-assets/src/lib.rs | 138 ++++++++++++++++-------------- 1 file changed, 72 insertions(+), 66 deletions(-) diff --git a/pallets/foreign-assets/src/lib.rs b/pallets/foreign-assets/src/lib.rs index 6e15d6d222..dabec285ea 100644 --- a/pallets/foreign-assets/src/lib.rs +++ b/pallets/foreign-assets/src/lib.rs @@ -23,6 +23,8 @@ #![cfg_attr(not(feature = "std"), no_std)] #![allow(clippy::unused_unit)] +use core::ops::Deref; + use frame_support::{dispatch::DispatchResult, pallet_prelude::*, traits::EnsureOrigin, PalletId}; use frame_system::pallet_prelude::*; use pallet_common::{ @@ -247,11 +249,13 @@ impl Pallet { /// If the multilocation doesn't match the patterns listed above, /// or the `` points to a foreign collection, /// `None` is returned, identifying that the given multilocation doesn't correspond to a local collection. - fn local_asset_location_to_collection(asset_location: &MultiLocation) -> Option { + fn local_asset_location_to_collection( + asset_location: &MultiLocation, + ) -> Option { let self_location = T::SelfLocation::get(); if *asset_location == Here.into() || *asset_location == self_location { - Some(NATIVE_FUNGIBLE_COLLECTION_ID) + Some(CollectionLocality::Local(NATIVE_FUNGIBLE_COLLECTION_ID)) } else if asset_location.parents == self_location.parents { match asset_location .interior @@ -262,7 +266,7 @@ impl Pallet { Self::collection_to_foreign_reserve_location(collection_id) .is_none() - .then_some(collection_id) + .then_some(CollectionLocality::Local(collection_id)) } _ => None, } @@ -271,22 +275,24 @@ impl Pallet { } } - /// Converts an asset ID to a Unique Network's collection (either foreign or a local one). + /// Converts an asset ID to a Unique Network's collection locality (either foreign or a local one). /// /// The function will check if the asset's reserve location has the corresponding - /// foreign collection on Unique Network, and will return the collection ID if found. + /// foreign collection on Unique Network, + /// and will return the "foreign" locality containing the collection ID if found. /// /// If no corresponding foreign collection is found, the function will check /// if the asset's reserve location corresponds to a local collection. - /// If the local collection is found, its ID is returned. + /// If the local collection is found, the "local" locality with the collection ID is returned. /// /// If all of the above have failed, the `AssetIdConversionFailed` error will be returned. - fn asset_to_collection(asset_id: &AssetId) -> Result { + fn asset_to_collection(asset_id: &AssetId) -> Result { let AssetId::Concrete(asset_reserve_location) = asset_id else { return Err(XcmExecutorError::AssetNotHandled.into()); }; Self::foreign_reserve_location_to_collection(asset_reserve_location) + .map(CollectionLocality::Foreign) .or_else(|| Self::local_asset_location_to_collection(asset_reserve_location)) .ok_or_else(|| XcmExecutorError::AssetIdConversionFailed.into()) } @@ -297,40 +303,24 @@ impl Pallet { /// `AssetInstance::Index()`. /// /// If the asset instance is not in the valid format or the `` can't fit into the valid token ID, - /// the `AssetNotFound` error will be returned. - fn local_asset_instance_to_token_id( - asset_instance: &AssetInstance, - ) -> Result { + /// `None` will be returned. + fn local_asset_instance_to_token_id(asset_instance: &AssetInstance) -> Option { match asset_instance { - AssetInstance::Index(token_id) => Ok(TokenId( - (*token_id) - .try_into() - .map_err(|_| XcmError::AssetNotFound)?, - )), - _ => Err(XcmError::AssetNotFound), + AssetInstance::Index(token_id) => Some(TokenId((*token_id).try_into().ok()?)), + _ => None, } } /// Obtains the token ID of the `asset_instance` in the collection. - /// - /// Returns `Ok(None)` only if the `asset_instance` is a part of a foreign collection - /// and the item hasn't yet been created on this blockchain. - /// - /// If the `asset_instance` exists and it is a part of a foreign collection, the function will return `Ok(Some())`. - /// - /// If the `asset_instance` is a part of a local collection, - /// the function will return either `Ok(Some())` or an error if the token is not found. fn asset_instance_to_token_id( - collection_id: CollectionId, + collection_locality: CollectionLocality, asset_instance: &AssetInstance, - ) -> Result, XcmError> { - if >::contains_key(collection_id) { - Ok(Self::foreign_reserve_asset_instance_to_token_id( - collection_id, - asset_instance, - )) - } else { - Self::local_asset_instance_to_token_id(asset_instance).map(Some) + ) -> Option { + match collection_locality { + CollectionLocality::Local(_) => Self::local_asset_instance_to_token_id(asset_instance), + CollectionLocality::Foreign(collection_id) => { + Self::foreign_reserve_asset_instance_to_token_id(collection_id, asset_instance) + } } } @@ -380,20 +370,26 @@ impl Pallet { /// or creates a foreign item. fn deposit_asset_instance( xcm_ext: &dyn XcmExtensions, - collection_id: CollectionId, + collection_locality: CollectionLocality, asset_instance: &AssetInstance, to: T::CrossAccountId, ) -> XcmResult { - let deposit_result = if let Some(token_id) = - Self::asset_instance_to_token_id(collection_id, asset_instance)? - { - let depositor = &Self::pallet_account(); - let from = depositor; - let amount = 1; - - xcm_ext.transfer_item(depositor, from, &to, token_id, amount, &ZeroBudget) - } else { - Self::create_foreign_asset_instance(xcm_ext, collection_id, asset_instance, to) + let token_id = Self::asset_instance_to_token_id(collection_locality, asset_instance); + + let deposit_result = match (collection_locality, token_id) { + (_, Some(token_id)) => { + let depositor = &Self::pallet_account(); + let from = depositor; + let amount = 1; + + xcm_ext.transfer_item(depositor, from, &to, token_id, amount, &ZeroBudget) + } + (CollectionLocality::Foreign(collection_id), None) => { + Self::create_foreign_asset_instance(xcm_ext, collection_id, asset_instance, to) + } + (CollectionLocality::Local(_), None) => { + return Err(XcmError::AssetNotFound); + } }; deposit_result @@ -405,11 +401,11 @@ impl Pallet { /// Transfers the asset instance to the pallet's account. fn withdraw_asset_instance( xcm_ext: &dyn XcmExtensions, - collection_id: CollectionId, + collection_locality: CollectionLocality, asset_instance: &AssetInstance, from: T::CrossAccountId, ) -> XcmResult { - let token_id = Self::asset_instance_to_token_id(collection_id, asset_instance)? + let token_id = Self::asset_instance_to_token_id(collection_locality, asset_instance) .ok_or(XcmError::AssetNotFound)?; let depositor = &from; @@ -448,9 +444,9 @@ impl TransactAsset for Pallet { let to = T::LocationToAccountId::convert_location(to) .ok_or(XcmExecutorError::AccountIdConversionFailed)?; - let collection_id = Self::asset_to_collection(&what.id)?; - let dispatch = - T::CollectionDispatch::dispatch(collection_id).map_err(|_| XcmError::AssetNotFound)?; + let collection_locality = Self::asset_to_collection(&what.id)?; + let dispatch = T::CollectionDispatch::dispatch(*collection_locality) + .map_err(|_| XcmError::AssetNotFound)?; let collection = dispatch.as_dyn(); let xcm_ext = collection.xcm_extensions().ok_or(XcmError::Unimplemented)?; @@ -467,7 +463,7 @@ impl TransactAsset for Pallet { .map_err(|_| XcmError::FailedToTransactAsset("fungible item deposit failed")), Fungibility::NonFungible(asset_instance) => { - Self::deposit_asset_instance(xcm_ext, collection_id, &asset_instance, to) + Self::deposit_asset_instance(xcm_ext, collection_locality, &asset_instance, to) } } } @@ -480,9 +476,9 @@ impl TransactAsset for Pallet { let from = T::LocationToAccountId::convert_location(from) .ok_or(XcmExecutorError::AccountIdConversionFailed)?; - let collection_id = Self::asset_to_collection(&what.id)?; - let dispatch = - T::CollectionDispatch::dispatch(collection_id).map_err(|_| XcmError::AssetNotFound)?; + let collection_locality = Self::asset_to_collection(&what.id)?; + let dispatch = T::CollectionDispatch::dispatch(*collection_locality) + .map_err(|_| XcmError::AssetNotFound)?; let collection = dispatch.as_dyn(); let xcm_ext = collection.xcm_extensions().ok_or(XcmError::NoPermission)?; @@ -493,7 +489,7 @@ impl TransactAsset for Pallet { .map_err(|_| XcmError::FailedToTransactAsset("fungible item withdraw failed"))?, Fungibility::NonFungible(asset_instance) => { - Self::withdraw_asset_instance(xcm_ext, collection_id, &asset_instance, from)?; + Self::withdraw_asset_instance(xcm_ext, collection_locality, &asset_instance, from)?; } } @@ -512,10 +508,10 @@ impl TransactAsset for Pallet { let to = T::LocationToAccountId::convert_location(to) .ok_or(XcmExecutorError::AccountIdConversionFailed)?; - let collection_id = Self::asset_to_collection(&what.id)?; + let collection_locality = Self::asset_to_collection(&what.id)?; - let dispatch = - T::CollectionDispatch::dispatch(collection_id).map_err(|_| XcmError::AssetNotFound)?; + let dispatch = T::CollectionDispatch::dispatch(*collection_locality) + .map_err(|_| XcmError::AssetNotFound)?; let collection = dispatch.as_dyn(); let xcm_ext = collection.xcm_extensions().ok_or(XcmError::NoPermission)?; @@ -533,7 +529,7 @@ impl TransactAsset for Pallet { } Fungibility::NonFungible(asset_instance) => { - token_id = Self::asset_instance_to_token_id(collection_id, &asset_instance)? + token_id = Self::asset_instance_to_token_id(collection_locality, &asset_instance) .ok_or(XcmError::AssetNotFound)?; amount = 1; @@ -549,6 +545,23 @@ impl TransactAsset for Pallet { } } +#[derive(Clone, Copy)] +pub enum CollectionLocality { + Local(CollectionId), + Foreign(CollectionId), +} + +impl Deref for CollectionLocality { + type Target = CollectionId; + + fn deref(&self) -> &Self::Target { + match self { + Self::Local(id) => id, + Self::Foreign(id) => id, + } + } +} + pub struct CurrencyIdConvert(PhantomData); impl sp_runtime::traits::Convert> for CurrencyIdConvert @@ -566,13 +579,6 @@ impl sp_runtime::traits::Convert> } } -pub use frame_support::{ - traits::{ - fungibles::Balanced, tokens::currency::Currency as CurrencyT, OnUnbalanced as OnUnbalancedT, - }, - weights::{WeightToFee, WeightToFeePolynomial}, -}; - #[derive(Encode, Decode, Eq, Debug, Clone, PartialEq, TypeInfo, MaxEncodedLen)] pub enum ForeignCollectionMode { NFT,