This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
XCM: Remove & replace XCM Convert trait
#7329
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gavofyork
added
A0-please_review
gavofyork
changed the title
Convert traitConvert trait
KiChjang
reviewed
Jun 5, 2023
| }; | ||
| #[allow(deprecated)] | ||
| pub use super::{Identity, JustTry}; |
There was a problem hiding this comment.
Are we going to continue exporting these types after deprecation by re-exporting from sp_runtime? What's the plan otherwise?
There was a problem hiding this comment.
We'll remove them eventually (hence deeprecation), but they're here to minimise breaking changes.
muharem
approved these changes
Jun 5, 2023
bkontur
approved these changes
Jun 5, 2023
liamaharon
approved these changes
Jun 5, 2023
Co-authored-by: Muharem Ismailov <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
Co-authored-by: Liam Aharon <[email protected]>
|
bot merge |
|
This pull request has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/multichain-friendly-account-abstraction/1298/20 |
|
This pull request has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/multichain-friendly-account-abstraction/1298/21 |
16 tasks
This was referenced Dec 6, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cumulus Companion: paritytech/cumulus#2688
The
Converttrait had a name collision with a similar trait in Substrate causing confusion; the trait itself was used for 4 things, each quite different and was better replaced by using more job-specific traits and more general conversion traits from Substrate.API changes
xcm_executor::traits::Convertremoved.xcm_executor::traits::{Encoded, Decoded}removed.xcm_executor::traits::{JustTry, Identity}deprecated (usesp_runtimeequivalents instead).xcm_executor::traits::ConvertLocationadded.xcm_builder::location_conversion::DescribeLocationadded.xcm_builder::location_conversion::HashedDescriptionand family added, which should be used to replace usages ofForeignChainAliasAccount.ForeignChainAliasAccountdeprecated.Origin Conversion
EnsureXcmOrigin:Conversionbound changes fromxcm_executor::traits::Convert<RuntimeOrigin, MultiLocation>toTryConvert<RuntimeOrigin, MultiLocation>.Location Conversion
LocationConverterbound changes fromxcm_executor::traits::Convert<MultiLocation, RuntimeOrigin::AccountId>toConvertLocation<RuntimeOrigin::AccountId>.AccountIdConverterbound changes fromxcm_executor::traits::Convert<MultiLocation, AccountId>toConvertLocation<AccountId>.pallet_xcm::Config::SovereignAccountOfbound changes fromxcm_executor::traits::Convert<MultiLocation, Self::AccountId>toConvertLocation<Self::AccountId>.Asset Conversion
ConvertClassIdbound changes fromxcm_executor::traits::Convert<[u8; 32], ClassId>toMaybeEquivalence<[u8; 32], ClassId>(same forAssetId).ConvertInstanceIdbound changes fromxcm_executor::traits::Convert<AssetInstance, InstanceId>toMaybeEquivalence<AssetInstance, InstanceId>.ConvertBalancebound changes fromxcm_executor::traits::Convert<u128, Balance>toMaybeEquivalence<u128, Balance>.Migrating code
Since almost all trait-implementors have also been changed in line with the bounds, there should be a good degree of compatibility for runtime maintainers. However, if there are custom implementations of the
xcm_executor::traits::Converttrait, then these will need to be altered into an implementation of eitherConvertLocation,MaybeEquivalenceorTryConvert(depending on what kind of conversion it is performing).The functions provided in the new traits are slightly different; if the
xcm_executor::traits::Convertwas being used directly (e.g. in tests) then the code will need to be adapted. Theimpl Borrowparameter types are not used in any of the new traits, instead favouring simpler pass-by-value (TryConvert) or pass-by-reference (ConvertLocation,MaybeEquivalence). The return types used in the new location conversion and asset conversion traits areOption, notResult(as was the case inxcm_executor::traits::Convert. Naming is also slightly different;convert_locationis called instead ofconvertfor location conversion.convert_refandreverse_refbecomeconvertandconvert_backfor asset conversion. For origin conversion,convertbecomestry_convert.This PR deprecates
ForeignChainAliasAccount; if it is already in production usage in your runtime, it may be replaced withHashedDescription<AccountId, LegacyDescribeForeignChainAccount>. If not already in use, you are encouraged to useHashedDescriptionwith the modules appropriate to your expected usage, such asHashedDescription<AccountId, DescribeFamily<DescribeAllTerminal>>.