Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[xcm] ConvertedConcreteId convert to MultiAsset (fungible) back-and-forth #6760

Closed
wants to merge 1 commit into from
Closed

[xcm] ConvertedConcreteId convert to MultiAsset (fungible) back-and-forth #6760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 55 additions & 0 deletions xcm/xcm-builder/src/asset_conversion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,32 @@ impl<
Ok((what, amount))
}
}

impl<
AssetId: Clone,
Balance: Clone,
ConvertAssetId: Convert<MultiLocation, AssetId>,
ConvertBalance: Convert<u128, Balance>,
> Convert<(AssetId, Balance), MultiAsset>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KiChjang
it would be cool and useful to have backwards conversion (AssetId, Balance) -> MultiAsset for ConvertedConcreteId/MatchedConvertedConcreteId automatically,

if I add the similar impl for ConvertedConcreteId for non-fungibles

impl<
		ClassId: Clone,
		InstanceId: Clone,
		ConvertClassId: Convert<MultiLocation, ClassId>,
		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
	> Convert<(ClassId, InstanceId), MultiAsset>
 	for ConvertedConcreteId<ClassId, InstanceId, ConvertClassId, ConvertInstanceId>

it errors with:

error[E0119]: conflicting implementations of trait

so maybe, some new trait to extend traits MatchesFungibles / MatchesNonFungibles or some other solution?
or we dont want to support this here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you first give some context as to what problem this PR is trying to solve? It's hard to reason about your solutions here when I don't have much idea what's going on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sure, no problem, I updated here desc: #6760 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but doesn't reverse_ref already does the thing that you expect it to?

for ConvertedConcreteId<AssetId, Balance, ConvertAssetId, ConvertBalance>
{
fn convert_ref(value: impl Borrow<(AssetId, Balance)>) -> Result<MultiAsset, ()> {
let (asset_id, balance) = value.borrow();
match ConvertAssetId::reverse_ref(asset_id) {
Ok(asset_id_as_multilocation) => match ConvertBalance::reverse_ref(balance) {
Ok(amount) => Ok((asset_id_as_multilocation, amount).into()),
Err(_) => Err(()),
},
Err(_) => Err(()),
}
}

fn reverse_ref(value: impl Borrow<MultiAsset>) -> Result<(AssetId, Balance), ()> {
<Self as MatchesFungibles<AssetId, Balance>>::matches_fungibles(value.borrow())
.map_err(|_| ())
}
}

impl<
ClassId: Clone,
InstanceId: Clone,
Expand Down Expand Up @@ -147,3 +173,32 @@ impl<
pub type ConvertedConcreteAssetId<A, B, C, O> = ConvertedConcreteId<A, B, C, O>;
#[deprecated = "Use `ConvertedAbstractId` instead"]
pub type ConvertedAbstractAssetId<A, B, C, O> = ConvertedAbstractId<A, B, C, O>;

#[cfg(test)]
mod tests {
use super::*;

use xcm_executor::traits::{Identity, JustTry};

#[test]
fn converted_concrete_id_fungible_multi_asset_conversion_roundtrip_works() {
type Converter = ConvertedConcreteId<MultiLocation, u64, Identity, JustTry>;

let location = MultiLocation::new(0, X1(GlobalConsensus(ByGenesis([0; 32]))));
let amount = 123456_u64;
let expected_multi_asset = MultiAsset {
id: Concrete(MultiLocation::new(0, X1(GlobalConsensus(ByGenesis([0; 32]))))),
fun: Fungible(123456_u128),
};

assert_eq!(
Converter::matches_fungibles(&expected_multi_asset).map_err(|_| ()),
Ok((location, amount))
);

assert_eq!(Converter::reverse_ref(&expected_multi_asset), Ok((location, amount)));
assert_eq!(Converter::reverse(expected_multi_asset.clone()), Ok((location, amount)));
assert_eq!(Converter::convert_ref((location, amount)), Ok(expected_multi_asset.clone()));
assert_eq!(Converter::convert((location, amount)), Ok(expected_multi_asset));
}
}