Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 37 additions & 19 deletions xtokens/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ use TransferKind::*;

#[frame_support::pallet]
pub mod module {

use super::*;

#[pallet::config]
Expand Down Expand Up @@ -572,22 +571,28 @@ pub mod module {
Error::<T>::TooManyAssetsBeingSent
);

// We check that all assets are valid and share the same reserve
for i in 0..assets.len() {
let mut reserve: Option<MultiLocation> = None;
let asset_len = assets.len();
for i in 0..asset_len {
let asset = assets.get(i).ok_or(Error::<T>::AssetIndexNonExistent)?;
if !asset.is_fungible(None) {
return Err(Error::<T>::NotFungible.into());
}
if fungible_amount(asset).is_zero() {
return Ok(());
Comment thread
xlc marked this conversation as resolved.
Outdated
}
ensure!(
fee.reserve() == asset.reserve(),
Error::<T>::DistinctReserveForAssetAndFee
);
// `assets` includes fee asset, the reserve location is decided by non fee
// asset
if (fee != *asset && reserve.is_none()) || asset_len == 1 {
reserve = asset.reserve();
}
// make sure all non fee assets share the same reserve
if reserve.is_some() {
ensure!(reserve == asset.reserve(), Error::<T>::DistinctReserveForAssetAndFee);
}
}

let (transfer_kind, dest, reserve, recipient) = Self::transfer_kind(&fee, &dest)?;
let (transfer_kind, dest, reserve, recipient) = Self::transfer_kind(reserve, &dest)?;
let mut msg = match transfer_kind {
SelfReserveAsset => {
Self::transfer_self_reserve_asset(assets.clone(), fee, dest.clone(), recipient, dest_weight)?
Expand Down Expand Up @@ -735,22 +740,22 @@ pub mod module {

/// Get the transfer kind.
///
/// Returns `Err` if `asset` and `dest` combination doesn't make sense,
/// else returns a tuple of:
/// Returns `Err` if `dest` combination doesn't make sense, or `reserve`
/// is none, else returns a tuple of:
/// - `transfer_kind`.
/// - asset's `reserve` parachain or relay chain location,
/// - `dest` parachain or relay chain location.
/// - `recipient` location.
fn transfer_kind(
asset: &MultiAsset,
reserve: Option<MultiLocation>,
dest: &MultiLocation,
) -> Result<(TransferKind, MultiLocation, MultiLocation, MultiLocation), DispatchError> {
let (dest, recipient) = Self::ensure_valid_dest(dest)?;

let self_location = T::SelfLocation::get();
ensure!(dest != self_location, Error::<T>::NotCrossChainTransfer);

let reserve = asset.reserve().ok_or(Error::<T>::AssetHasNoReserve)?;
let reserve = reserve.ok_or(Error::<T>::AssetHasNoReserve)?;
let transfer_kind = if reserve == self_location {
SelfReserveAsset
} else if reserve == dest {
Expand All @@ -766,13 +771,13 @@ pub mod module {
impl<T: Config> Pallet<T> {
/// Returns weight of `transfer_multiasset` call.
fn weight_of_transfer_multiasset(asset: &VersionedMultiAsset, dest: &VersionedMultiLocation) -> Weight {
let asset = asset.clone().try_into();
let asset: Result<MultiAsset, _> = asset.clone().try_into();
let dest = dest.clone().try_into();
if let (Ok(asset), Ok(dest)) = (asset, dest) {
if let Ok((transfer_kind, dest, _, reserve)) = Self::transfer_kind(&asset, &dest) {
if let Ok((transfer_kind, dest, _, reserve)) = Self::transfer_kind(asset.reserve(), &dest) {
let mut msg = match transfer_kind {
SelfReserveAsset => Xcm(vec![
WithdrawAsset(MultiAssets::from(asset.clone())),
WithdrawAsset(MultiAssets::from(asset)),
DepositReserveAsset {
assets: All.into(),
max_assets: 1,
Expand All @@ -781,7 +786,7 @@ pub mod module {
},
]),
ToReserve | ToNonReserve => Xcm(vec![
WithdrawAsset(MultiAssets::from(asset.clone())),
WithdrawAsset(MultiAssets::from(asset)),
InitiateReserveWithdraw {
assets: All.into(),
// `dest` is always (equal to) `reserve` in both cases
Expand Down Expand Up @@ -833,11 +838,11 @@ pub mod module {
dest: &VersionedMultiLocation,
) -> Weight {
let assets: Result<MultiAssets, ()> = assets.clone().try_into();

let dest = dest.clone().try_into();
if let (Ok(assets), Ok(dest)) = (assets, dest) {
if let Some(fee) = assets.get(*fee_item as usize) {
if let Ok((transfer_kind, dest, _, reserve)) = Self::transfer_kind(fee, &dest) {
let reserve_location = Self::get_reserve_location(&assets, fee_item);
if let Ok(reserve_location) = reserve_location {
if let Ok((transfer_kind, dest, _, reserve)) = Self::transfer_kind(reserve_location, &dest) {
let mut msg = match transfer_kind {
SelfReserveAsset => Xcm(vec![
WithdrawAsset(assets.clone()),
Expand Down Expand Up @@ -865,6 +870,19 @@ pub mod module {
}
0
}

/// Get reserve location of non fee asset.
fn get_reserve_location(assets: &MultiAssets, fee_item: &u32) -> Result<Option<MultiLocation>, DispatchError> {
Comment thread
zqhxuyuan marked this conversation as resolved.
Outdated
let non_fee_index = match assets.len() {
Comment thread
zqhxuyuan marked this conversation as resolved.
Outdated
1 => 0,
_ => match fee_item {
0 => 1,
_ => 0,
},
};
let asset = assets.get(non_fee_index).ok_or(Error::<T>::AssetIndexNonExistent)?;
Ok(asset.reserve())
}
}

impl<T: Config> XcmTransfer<T::AccountId, T::Balance, T::CurrencyId> for Pallet<T> {
Expand Down
90 changes: 49 additions & 41 deletions xtokens/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,55 @@ fn send_self_parachain_asset_to_sibling_with_distinct_fee() {
});
}

#[test]
fn sending_assets_with_different_reserve_works() {
TestNet::reset();

ParaA::execute_with(|| {
assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 1_000));
});

ParaB::execute_with(|| {
assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), 1_000));
assert_ok!(ParaTokens::deposit(CurrencyId::R, &sibling_a_account(), 1_000));
});

Relay::execute_with(|| {
let _ = RelayBalances::deposit_creating(&para_a_account(), 1_000);
});

ParaA::execute_with(|| {
assert_ok!(ParaXTokens::transfer_multicurrencies(
Some(ALICE).into(),
vec![(CurrencyId::B, 450), (CurrencyId::R, 50)],
1,
Box::new(
(
Parent,
Parachain(2),
Junction::AccountId32 {
network: NetworkId::Any,
id: BOB.into(),
},
)
.into()
),
40,
));
assert_eq!(550, ParaTokens::free_balance(CurrencyId::B, &ALICE));
assert_eq!(950, ParaTokens::free_balance(CurrencyId::R, &ALICE));
});

ParaB::execute_with(|| {
assert_eq!(550, ParaTokens::free_balance(CurrencyId::B, &sibling_a_account()));
assert_eq!(950, ParaTokens::free_balance(CurrencyId::R, &sibling_a_account()));

// It should use 40 for weight, so 450 B and 10 R should reach destination
assert_eq!(450, ParaTokens::free_balance(CurrencyId::B, &BOB));
assert_eq!(10, ParaTokens::free_balance(CurrencyId::R, &BOB));
});
}

#[test]
fn transfer_no_reserve_assets_fails() {
TestNet::reset();
Expand Down Expand Up @@ -972,47 +1021,6 @@ fn specifying_more_than_two_assets_should_error() {
});
}

#[test]
fn sending_assets_with_different_reserve_should_fail() {
TestNet::reset();

ParaA::execute_with(|| {
assert_ok!(ParaTokens::deposit(CurrencyId::B, &ALICE, 1_000));
assert_ok!(ParaTokens::deposit(CurrencyId::R, &ALICE, 1_000));
});

ParaB::execute_with(|| {
assert_ok!(ParaTokens::deposit(CurrencyId::B, &sibling_a_account(), 1_000));
});

Relay::execute_with(|| {
let _ = RelayBalances::deposit_creating(&para_a_account(), 1_000);
});

ParaA::execute_with(|| {
assert_noop!(
ParaXTokens::transfer_multicurrencies(
Some(ALICE).into(),
vec![(CurrencyId::B, 450), (CurrencyId::R, 5000)],
1,
Box::new(
(
Parent,
Parachain(2),
Junction::AccountId32 {
network: NetworkId::Any,
id: BOB.into(),
},
)
.into()
),
40,
),
Error::<para::Runtime>::DistinctReserveForAssetAndFee
);
});
}

#[test]
fn specifying_a_non_existent_asset_index_should_fail() {
TestNet::reset();
Expand Down