Skip to content

Commit

Permalink
Fix migrations for pallet-xcm (paritytech#6148)
Browse files Browse the repository at this point in the history
Relates to: paritytech#4826
Relates to: paritytech#3214

## Description

`pallet-xcm` stores some operational data that uses `Versioned*` XCM
types. When we add a new XCM version (XV), we deprecate XV-2 and remove
XV-3. Without proper migration, this can lead to issues with
[undecodable
storage](https://github.com/paritytech/polkadot-sdk/actions/runs/11381324568/job/31662577532?pr=6092),
as was identified on the XCMv5 branch where XCMv2 was removed.

This PR extends the existing `MigrateToLatestXcmVersion` to include
migration for the `Queries`, `LockedFungibles`, and
`RemoteLockedFungibles` storage types. Additionally, more checks were
added to `try_state` for these types.

## TODO
- [x] create tracking issue for `polkadot-fellows`
polkadot-fellows/runtimes#492
- [x] Add missing `MigrateToLatestXcmVersion` for westend
- [x] fix pallet-xcm `Queries`
- fails for Westend
https://github.com/paritytech/polkadot-sdk/actions/runs/11381324568/job/31662577532?pr=6092
- `V2` was removed from `Versioned*` stuff, but we have a live data with
V2 e.g. Queries - e.g. Kusama or Polkadot relay chains
```
VersionNotifier: {
        origin: {
          V2: {
            parents: 0
            interior: {
              X1: {
                Parachain: 2,124
              }
            }
          }
        }
        isActive: true
      } 
```

![image](https://github.com/user-attachments/assets/f59f761b-46a7-4def-8aea-45c4e41c0a00)
- [x] fix also for `RemoteLockedFungibles` 
- [x] fix also for `LockedFungibles`

## Follow-ups

- [ ] deploy on Westend chains before XCMv5
- [ ] paritytech#6188

---------

Co-authored-by: command-bot <>
Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
  • Loading branch information
3 people authored and mordamax committed Oct 29, 2024
1 parent 59d9e51 commit 2421880
Show file tree
Hide file tree
Showing 10 changed files with 610 additions and 14 deletions.
2 changes: 2 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1809,6 +1809,8 @@ pub mod migrations {
>,
parachains_shared::migration::MigrateToV1<Runtime>,
parachains_scheduler::migration::MigrateV2ToV3<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl pallet_xcm::Config for Runtime {
type UniversalLocation = UniversalLocation;
// No version discovery needed
const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 0;
type AdvertisedXcmVersion = frame::traits::ConstU32<3>;
type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion;
type AdminOrigin = frame_system::EnsureRoot<AccountId>;
// No locking
type TrustedLockers = ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl pallet_xcm::Config for Runtime {
type UniversalLocation = UniversalLocation;
// No version discovery needed
const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 0;
type AdvertisedXcmVersion = frame::traits::ConstU32<3>;
type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion;
type AdminOrigin = frame_system::EnsureRoot<AccountId>;
// No locking
type TrustedLockers = ();
Expand Down
42 changes: 40 additions & 2 deletions polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2807,6 +2807,44 @@ impl<T: Config> Pallet<T> {
/// set.
#[cfg(any(feature = "try-runtime", test))]
pub fn do_try_state() -> Result<(), TryRuntimeError> {
use migration::data::NeedsMigration;

// Take the minimum version between `SafeXcmVersion` and `latest - 1` and ensure that the
// operational data is stored at least at that version, for example, to prevent issues when
// removing older XCM versions.
let minimal_allowed_xcm_version = if let Some(safe_xcm_version) = SafeXcmVersion::<T>::get()
{
XCM_VERSION.saturating_sub(1).min(safe_xcm_version)
} else {
XCM_VERSION.saturating_sub(1)
};

// check `Queries`
ensure!(
!Queries::<T>::iter_values()
.any(|data| data.needs_migration(minimal_allowed_xcm_version)),
TryRuntimeError::Other("`Queries` data should be migrated to the higher xcm version!")
);

// check `LockedFungibles`
ensure!(
!LockedFungibles::<T>::iter_values()
.any(|data| data.needs_migration(minimal_allowed_xcm_version)),
TryRuntimeError::Other(
"`LockedFungibles` data should be migrated to the higher xcm version!"
)
);

// check `RemoteLockedFungibles`
ensure!(
!RemoteLockedFungibles::<T>::iter()
.any(|(key, data)| key.needs_migration(minimal_allowed_xcm_version) ||
data.needs_migration(minimal_allowed_xcm_version)),
TryRuntimeError::Other(
"`RemoteLockedFungibles` data should be migrated to the higher xcm version!"
)
);

// if migration has been already scheduled, everything is ok and data will be eventually
// migrated
if CurrentMigration::<T>::exists() {
Expand Down Expand Up @@ -2887,7 +2925,7 @@ impl<T: Config> xcm_executor::traits::Enact for UnlockTicket<T> {
let mut maybe_remove_index = None;
let mut locked = BalanceOf::<T>::zero();
let mut found = false;
// We could just as well do with with an into_iter, filter_map and collect, however this way
// We could just as well do with an into_iter, filter_map and collect, however this way
// avoids making an allocation.
for (i, x) in locks.iter_mut().enumerate() {
if x.1.try_as::<_>().defensive() == Ok(&self.unlocker) {
Expand Down Expand Up @@ -3268,7 +3306,7 @@ impl<T: Config> OnResponse for Pallet<T> {
});
return Weight::zero()
}
return match maybe_notify {
match maybe_notify {
Some((pallet_index, call_index)) => {
// This is a bit horrible, but we happen to know that the `Call` will
// be built by `(pallet_index: u8, call_index: u8, QueryId, Response)`.
Expand Down
Loading

0 comments on commit 2421880

Please sign in to comment.