Trait Bounds for AssetId vs. MultiLocation

[joepetrowski]

In order to support bridged/parachain assets on Statemint, we want to be able to use type AssetId = MultiLocation. However, MultiLocation does not meet the bounds of AssetId.

From c290950:

type AssetId: Member
	+ Parameter
	+ Default
	+ Copy
	+ HasCompact
	+ MaybeSerializeDeserialize
	+ MaxEncodedLen
	+ TypeInfo;

From gav-xcm-v3 (at aef0231):

#[derive(
	Copy, Clone, Decode, Encode, Eq, PartialEq, Ord, PartialOrd, Debug, TypeInfo, MaxEncodedLen,
)]
#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
pub struct MultiLocation {
	/// The number of parent junctions at the beginning of this `MultiLocation`.
	pub parents: u8,
	/// The interior (i.e. non-parent) junctions that this `MultiLocation` contains.
	pub interior: Junctions,
}

We should either implement HasCompact, MaybeSerializeDeserialize for MultiLocation, or relax the bounds on AssetId.

Steps to reproduce

Try to compile cumulus/joe-bridge-hub-westmint-assets with this configuration:

type MultiLocationForAssetId = MultiLocation;

/// Assets managed by some foreign location.
type ForeignAssetsInstance = pallet_assets::Instance2;
impl pallet_assets::Config<ForeignAssetsInstance> for Runtime {
	type RuntimeEvent = RuntimeEvent;
	type Balance = Balance;
	type AssetId = MultiLocationForAssetId; // <-- here
	type Currency = Balances;
	type CreateOrigin = ForeignCreators;
	type ForceOrigin = AssetsForceOrigin;
	type AssetDeposit = AssetDeposit;
	type MetadataDepositBase = MetadataDepositBase;
	type MetadataDepositPerByte = MetadataDepositPerByte;
	type ApprovalDeposit = ApprovalDeposit;
	type StringLimit = AssetsStringLimit;
	type Freezer = ();
	type Extra = ();
	type WeightInfo = weights::pallet_assets::WeightInfo<Runtime>;
	type AssetAccountDeposit = AssetAccountDeposit;
}

[Szegoo]

It seems like removing HasCompact would result in increasing the max_recursion limit to a great value. Also since the AssetId type has to implement the MaybeSerializeDeserialize trait

/// A type that implements Serialize, DeserializeOwned and Debug when in std environment.
trait MaybeSerializeDeserialize: DeserializeOwned, Serialize;

which means that the AssetId type also needs to implement DeserializeOwned which doesn't have a procedural macro, so it has to be implemented by hand AFAIU. And we cannot remove this trait since AssetId has to implement Serialize/Deserialize functionality.

So I think that the solution here is to implement these traits for MultiLocation. Also, I am probably not the person that should make the decision here :P, I was just curious about this and investigated a bit.

[joepetrowski]

And we cannot remove this trait since AssetId has to implement Serialize/Deserialize functionality.

I'm not sure of the implementation constraints, but I do question this since the Uniques pallet's CollectionId is considerably more lenient.

type CollectionId: Member + Parameter + MaxEncodedLen + Copy;

I agree with you that the solution is probably to implement these for MultiLocation, but I don't fully understand why.

[Szegoo]

@joepetrowski You are right that is a bit weird. Maybe the reason for this is that the AssetId is used in the definition of GenesisConfig for the pallet.
When I removed these trait bounds locally and made them the same as the ones in the unique pallet the error message was pointing to the GenesisConfig struct definition saying that Serialize/Deserialize(i.e. MaybeSerializeDeserialize) has to be implemented on AssetId.

[joepetrowski]

I learned that MaybeSerializeDeserialize is needed for a genesis config because it needs to be able to read the JSON chain spec.