refactor: Drop tranche multiLocation

[NunoAlexandre]

Closes #1336

We make AssetMetadata.location of a Tranche simply None since having a MultiLocation for Tranche tokens implies they are XCM-transferrable when in fact we have specific Xcm configs in place to forbid tranche tokens transfers. Tranche tokens are only transferrable through Centrifuge Connectors, which guarantees that the permissioned-nature of tranche tokens are respected.

Changes

• 1. We set the AssetMetadata.location of a Tranche token to None
• 2. We add a migration for all the runtimes that ensure 1. for all the already registered Tranche tokens

To Do

• Write a migration that, for every CurrencyId::Tranche in entry in the AssetRegistry, updates that asset by setting location to None.
  • Centrifuge
  • Altair
  • Development

Discussion

Can we dry the migration so that we define it once for all all runtimes? We need to reference directly on the (external dependency) orml_asset_registry which I don't think we can abstract over the runtime it's in 🤔

[mustermeiszer]

I would really like us to stick with the naming for upgrades, so use UpgradeAltair1028 instead of just the plain migration. Makes it so much easier in the future to remove what needs to be removed.

Otherwise, looks good!

For approval:

Need clarification whether the following will never fail:

• let location: MultiLocation = old_location.clone().try_into().map_err(|()| Error::<T>::BadVersion)?;

[thea-leake]

Looks good! Left some comments about crates we can likely remove from the pool-system deps.

[NunoAlexandre]

I would really like us to stick with the naming for upgrades, so use UpgradeAltair1028 instead of just the plain migration. Makes it so much easier in the future to remove what needs to be removed.

Otherwise, looks good!

For approval:

Need clarification whether the following will never fail:

• let location: MultiLocation = old_location.clone().try_into().map_err(|()| Error::<T>::BadVersion)?;

@mustermeiszer what's the context for this code snippet? 🤔

[mustermeiszer]

Just the only error that could happen behind the expect call in the upgrade logic. Not really a blocker though. . Was just curious or wanted to be sure they expect never gets triggered

[NunoAlexandre]

Just the only error that could happen behind the expect call in the upgrade logic. Not really a blocker though. . Was just curious or wanted to be sure they expect never gets triggered

Yeah, that's a valid concern. Maybe we can just log in case it fails?

[NunoAlexandre]

@wischli @mustermeiszer I believe I have addressed everything, thanks for the feedback and I look forward for the next round :)

[mustermeiszer]

LGTM!

[wischli]

LGTM! I left two nitpicks which definitely don't need to be addressed.

UPDATE: Seeing the conflicts, I would recommend to move your runtime migration code into its own module inside runtime/$runtime/migrations.rs. In #1342, I ensured all imports tied to a specific migration where part of the module's scope such that deprecated migrations can be removed seamlessly without breaking clippy.

[NunoAlexandre]

@wischli @mustermeiszer could you approve this again? :)

[lemunozm]

Re-approving based on previous approvals 😃