From 86191d86dca99e43b1da63746f4a8d8e72a854d4 Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Thu, 19 Oct 2023 10:44:49 +0000 Subject: [PATCH 1/3] feat: xtokens non-fungible multiassets support PoC --- xtokens/src/lib.rs | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index fe479f27a..9e43bfb13 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -526,25 +526,39 @@ pub mod module { let asset_len = assets.len(); for i in 0..asset_len { let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; - ensure!( - matches!(asset.fun, Fungibility::Fungible(x) if !x.is_zero()), - Error::::InvalidAsset - ); - // `assets` includes fee, the reserve location is decided by non fee asset - if (fee != *asset && non_fee_reserve.is_none()) || asset_len == 1 { - non_fee_reserve = T::ReserveProvider::reserve(asset); - } - // make sure all non fee assets share the same reserve - if non_fee_reserve.is_some() { + + if fee == *asset { + // Fee payment can only be made by using fungibles ensure!( - non_fee_reserve == T::ReserveProvider::reserve(asset), - Error::::DistinctReserveForAssetAndFee + matches!(asset.fun, Fungibility::Fungible(x) if !x.is_zero()), + Error::::InvalidAsset ); + } else { + match asset.fun { + Fungibility::Fungible(x) => ensure!(!x.is_zero(), Error::::InvalidAsset), + Fungibility::NonFungible(AssetInstance::Undefined) => { + return Err(Error::::InvalidAsset.into()) + } + _ => {} + } + + // `assets` includes fee, the reserve location is decided by non fee asset + if non_fee_reserve.is_none() { + non_fee_reserve = T::ReserveProvider::reserve(asset); + } + + // make sure all non fee assets share the same reserve + if non_fee_reserve.is_some() { + ensure!( + non_fee_reserve == T::ReserveProvider::reserve(asset), + Error::::DistinctReserveForAssetAndFee + ); + } } } let fee_reserve = T::ReserveProvider::reserve(&fee); - if fee_reserve != non_fee_reserve { + if asset_len > 1 && fee_reserve != non_fee_reserve { // Current only support `ToReserve` with relay-chain asset as fee. other case // like `NonReserve` or `SelfReserve` with relay-chain fee is not support. ensure!(non_fee_reserve == dest.chain_part(), Error::::InvalidAsset); @@ -610,7 +624,7 @@ pub mod module { origin_location, assets.clone(), fee.clone(), - non_fee_reserve, + fee_reserve, &dest, None, dest_weight_limit, From 64e09c57268fe6551c7fb182f368c3c1dfa77d6a Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Fri, 17 Nov 2023 15:34:23 +0100 Subject: [PATCH 2/3] test: nft/fee related tests for multiasset transfer --- xtokens/src/tests.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 3c0806e5e..654063fb7 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -1721,3 +1721,100 @@ fn send_with_insufficient_weight_limit() { assert_eq!(ParaTokens::free_balance(CurrencyId::A, &BOB), 0); }); } + +#[test] +fn send_multiasset_with_zero_fee_should_yield_an_error() { + TestNet::reset(); + + let asset_id: AssetId = X1(Junction::from(BoundedVec::try_from(b"A".to_vec()).unwrap())).into(); + ParaA::execute_with(|| { + assert_noop!( + ParaXTokens::transfer_multiasset_with_fee( + Some(ALICE).into(), + Box::new((asset_id, 100).into()), + Box::new((asset_id, Fungibility::Fungible(0)).into()), + Box::new( + MultiLocation::new( + 1, + X2( + Parachain(2), + Junction::AccountId32 { + network: None, + id: BOB.into() + }, + ) + ) + .into() + ), + WeightLimit::Unlimited, + ), + Error::::InvalidAsset + ); + }); +} + +#[test] +fn send_undefined_nft_should_yield_an_error() { + TestNet::reset(); + + let fee_id: AssetId = X1(Junction::from(BoundedVec::try_from(b"A".to_vec()).unwrap())).into(); + let nft_id: AssetId = X1(Junction::GeneralIndex(42)).into(); + + ParaA::execute_with(|| { + assert_noop!( + ParaXTokens::transfer_multiasset_with_fee( + Some(ALICE).into(), + Box::new((nft_id, Undefined).into()), + Box::new((fee_id, 100).into()), + Box::new( + MultiLocation::new( + 1, + X2( + Parachain(2), + Junction::AccountId32 { + network: None, + id: BOB.into() + }, + ) + ) + .into() + ), + WeightLimit::Unlimited, + ), + Error::::InvalidAsset + ); + }); +} + +#[test] +fn nfts_cannot_be_fee_assets() { + TestNet::reset(); + + let asset_id: AssetId = X1(Junction::from(BoundedVec::try_from(b"A".to_vec()).unwrap())).into(); + let nft_id: AssetId = X1(Junction::GeneralIndex(42)).into(); + + ParaA::execute_with(|| { + assert_noop!( + ParaXTokens::transfer_multiasset_with_fee( + Some(ALICE).into(), + Box::new((asset_id, 100).into()), + Box::new((nft_id, Index(1)).into()), + Box::new( + MultiLocation::new( + 1, + X2( + Parachain(2), + Junction::AccountId32 { + network: None, + id: BOB.into() + }, + ) + ) + .into() + ), + WeightLimit::Unlimited, + ), + Error::::InvalidAsset + ); + }); +} From dbcc78990e5fddeb360c6c1e2c3bcb7a3cf84da3 Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Fri, 17 Nov 2023 15:34:53 +0100 Subject: [PATCH 3/3] fix: do_transfer_multiassets zero fee issue --- xtokens/src/lib.rs | 47 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 9e43bfb13..3f7433447 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -520,6 +520,13 @@ pub mod module { T::MultiLocationsFilter::contains(&dest), Error::::NotSupportedMultiLocation ); + + // Fee payment can only be made by using the non-zero amount of fungibles + ensure!( + matches!(fee.fun, Fungibility::Fungible(x) if !x.is_zero()), + Error::::InvalidAsset + ); + let origin_location = T::AccountIdToMultiLocation::convert(who.clone()); let mut non_fee_reserve: Option = None; @@ -527,33 +534,23 @@ pub mod module { for i in 0..asset_len { let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; - if fee == *asset { - // Fee payment can only be made by using fungibles - ensure!( - matches!(asset.fun, Fungibility::Fungible(x) if !x.is_zero()), - Error::::InvalidAsset - ); - } else { - match asset.fun { - Fungibility::Fungible(x) => ensure!(!x.is_zero(), Error::::InvalidAsset), - Fungibility::NonFungible(AssetInstance::Undefined) => { - return Err(Error::::InvalidAsset.into()) - } - _ => {} - } + match asset.fun { + Fungibility::Fungible(x) => ensure!(!x.is_zero(), Error::::InvalidAsset), + Fungibility::NonFungible(AssetInstance::Undefined) => return Err(Error::::InvalidAsset.into()), + _ => {} + } - // `assets` includes fee, the reserve location is decided by non fee asset - if non_fee_reserve.is_none() { - non_fee_reserve = T::ReserveProvider::reserve(asset); - } + // `assets` includes fee, the reserve location is decided by non fee asset + if non_fee_reserve.is_none() && asset.id != fee.id { + non_fee_reserve = T::ReserveProvider::reserve(asset); + } - // make sure all non fee assets share the same reserve - if non_fee_reserve.is_some() { - ensure!( - non_fee_reserve == T::ReserveProvider::reserve(asset), - Error::::DistinctReserveForAssetAndFee - ); - } + // make sure all non fee assets share the same reserve + if non_fee_reserve.is_some() { + ensure!( + non_fee_reserve == T::ReserveProvider::reserve(asset), + Error::::DistinctReserveForAssetAndFee + ); } }