diff --git a/cumulus/pallets/collator-selection/Cargo.toml b/cumulus/pallets/collator-selection/Cargo.toml index a2b2158f1769..affa32970302 100644 --- a/cumulus/pallets/collator-selection/Cargo.toml +++ b/cumulus/pallets/collator-selection/Cargo.toml @@ -24,6 +24,7 @@ sp-staking = { path = "../../../substrate/primitives/staking", default-features frame-support = { path = "../../../substrate/frame/support", default-features = false, version = "26.0.0" } frame-system = { path = "../../../substrate/frame/system", default-features = false, version = "26.0.0" } pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false, version = "26.0.0" } +pallet-balances = { path = "../../../substrate/frame/balances", default-features = false, version = "26.0.0" } pallet-session = { path = "../../../substrate/frame/session", default-features = false, version = "26.0.0" } frame-benchmarking = { path = "../../../substrate/frame/benchmarking", default-features = false, optional = true, version = "26.0.0" } @@ -35,7 +36,6 @@ sp-tracing = { path = "../../../substrate/primitives/tracing" } sp-runtime = { path = "../../../substrate/primitives/runtime" } pallet-timestamp = { path = "../../../substrate/frame/timestamp" } sp-consensus-aura = { path = "../../../substrate/primitives/consensus/aura" } -pallet-balances = { path = "../../../substrate/frame/balances" } pallet-aura = { path = "../../../substrate/frame/aura" } [features] @@ -54,6 +54,7 @@ std = [ "frame-system/std", "log/std", "pallet-authorship/std", + "pallet-balances/std", "pallet-session/std", "rand/std", "scale-info/std", diff --git a/cumulus/pallets/collator-selection/src/lib.rs b/cumulus/pallets/collator-selection/src/lib.rs index 7449f4d68c7e..61a68676466e 100644 --- a/cumulus/pallets/collator-selection/src/lib.rs +++ b/cumulus/pallets/collator-selection/src/lib.rs @@ -118,8 +118,8 @@ pub mod pallet { use sp_staking::SessionIndex; use sp_std::vec::Vec; - /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + /// The in-code storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; diff --git a/cumulus/pallets/collator-selection/src/migration.rs b/cumulus/pallets/collator-selection/src/migration.rs index 58b4cc5b06a1..6ee0b707c442 100644 --- a/cumulus/pallets/collator-selection/src/migration.rs +++ b/cumulus/pallets/collator-selection/src/migration.rs @@ -17,9 +17,107 @@ //! A module that is responsible for migration of storage for Collator Selection. use super::*; -use frame_support::traits::OnRuntimeUpgrade; +use frame_support::traits::{OnRuntimeUpgrade, UncheckedOnRuntimeUpgrade}; use log; +/// Migrate to v2. Should have been part of . +pub mod v2 { + use super::*; + use frame_support::{ + pallet_prelude::*, + storage_alias, + traits::{Currency, ReservableCurrency}, + }; + use sp_runtime::traits::{Saturating, Zero}; + #[cfg(feature = "try-runtime")] + use sp_std::vec::Vec; + + /// [`UncheckedMigrationToV2`] wrapped in a + /// [`VersionedMigration`](frame_support::migrations::VersionedMigration), ensuring the + /// migration is only performed when on-chain version is 1. + pub type MigrationToV2 = frame_support::migrations::VersionedMigration< + 1, + 2, + UncheckedMigrationToV2, + Pallet, + ::DbWeight, + >; + + #[storage_alias] + pub type Candidates = StorageValue< + Pallet, + BoundedVec::AccountId, <::Currency as Currency<::AccountId>>::Balance>, ::MaxCandidates>, + ValueQuery, + >; + + /// Migrate to V2. + pub struct UncheckedMigrationToV2(sp_std::marker::PhantomData); + impl UncheckedOnRuntimeUpgrade for UncheckedMigrationToV2 { + fn on_runtime_upgrade() -> Weight { + let mut weight = Weight::zero(); + let mut count: u64 = 0; + // candidates who exist under the old `Candidates` key + let candidates = Candidates::::take(); + + // New candidates who have registered since the upgrade. Under normal circumstances, + // this should not exist because the migration should be applied when the upgrade + // happens. But in Polkadot/Kusama we messed this up, and people registered under + // `CandidateList` while their funds were locked in `Candidates`. + let new_candidate_list = CandidateList::::get(); + if new_candidate_list.len().is_zero() { + // The new list is empty, so this is essentially being applied correctly. We just + // put the candidates into the new storage item. + CandidateList::::put(&candidates); + // 1 write for the new list + weight.saturating_accrue(T::DbWeight::get().reads_writes(0, 1)); + } else { + // Oops, the runtime upgraded without the migration. There are new candidates in + // `CandidateList`. So, let's just refund the old ones and assume they have already + // started participating in the new system. + for candidate in candidates { + let err = T::Currency::unreserve(&candidate.who, candidate.deposit); + if err > Zero::zero() { + log::error!( + target: LOG_TARGET, + "{:?} balance was unable to be unreserved from {:?}", + err, &candidate.who, + ); + } + count.saturating_inc(); + } + weight.saturating_accrue( + <::WeightInfo as pallet_balances::WeightInfo>::force_unreserve().saturating_mul(count.into()), + ); + } + + log::info!( + target: LOG_TARGET, + "Unreserved locked bond of {} candidates, upgraded storage to version 2", + count, + ); + + weight.saturating_accrue(T::DbWeight::get().reads_writes(3, 2)); + weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::DispatchError> { + let number_of_candidates = Candidates::::get().to_vec().len(); + Ok((number_of_candidates as u32).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_number_of_candidates: Vec) -> Result<(), sp_runtime::DispatchError> { + let new_number_of_candidates = Candidates::::get().to_vec().len(); + assert_eq!( + new_number_of_candidates, 0 as usize, + "after migration, the candidates map should be empty" + ); + Ok(()) + } + } +} + /// Version 1 Migration /// This migration ensures that any existing `Invulnerables` storage lists are sorted. pub mod v1 { @@ -90,3 +188,136 @@ pub mod v1 { } } } + +#[cfg(all(feature = "try-runtime", test))] +mod tests { + use super::*; + use crate::{ + migration::v2::Candidates, + mock::{new_test_ext, Balances, Test}, + }; + use frame_support::{ + traits::{Currency, ReservableCurrency, StorageVersion}, + BoundedVec, + }; + use sp_runtime::traits::ConstU32; + + #[test] + fn migrate_to_v2_with_new_candidates() { + new_test_ext().execute_with(|| { + let storage_version = StorageVersion::new(1); + storage_version.put::>(); + + let one = 1u64; + let two = 2u64; + let three = 3u64; + let deposit = 10u64; + + // Set balance to 100 + Balances::make_free_balance_be(&one, 100u64); + Balances::make_free_balance_be(&two, 100u64); + Balances::make_free_balance_be(&three, 100u64); + + // Reservations: 10 for the "old" candidacy and 10 for the "new" + Balances::reserve(&one, 10u64).unwrap(); // old + Balances::reserve(&two, 20u64).unwrap(); // old + new + Balances::reserve(&three, 10u64).unwrap(); // new + + // Candidate info + let candidate_one = CandidateInfo { who: one, deposit }; + let candidate_two = CandidateInfo { who: two, deposit }; + let candidate_three = CandidateInfo { who: three, deposit }; + + // Storage lists + let bounded_candidates = + BoundedVec::, ConstU32<20>>::try_from(vec![ + candidate_one.clone(), + candidate_two.clone(), + ]) + .expect("it works"); + let bounded_candidate_list = + BoundedVec::, ConstU32<20>>::try_from(vec![ + candidate_two.clone(), + candidate_three.clone(), + ]) + .expect("it works"); + + // Set storage + Candidates::::put(bounded_candidates); + CandidateList::::put(bounded_candidate_list.clone()); + + // Sanity check + assert_eq!(Balances::free_balance(one), 90); + assert_eq!(Balances::free_balance(two), 80); + assert_eq!(Balances::free_balance(three), 90); + + // Run migration + v2::MigrationToV2::::on_runtime_upgrade(); + + let new_storage_version = StorageVersion::get::>(); + assert_eq!(new_storage_version, 2); + + // 10 should have been unreserved from the old candidacy + assert_eq!(Balances::free_balance(one), 100); + assert_eq!(Balances::free_balance(two), 90); + assert_eq!(Balances::free_balance(three), 90); + // The storage item should be gone + assert!(Candidates::::get().is_empty()); + // The new storage item should be preserved + assert_eq!(CandidateList::::get(), bounded_candidate_list); + }); + } + + #[test] + fn migrate_to_v2_without_new_candidates() { + new_test_ext().execute_with(|| { + let storage_version = StorageVersion::new(1); + storage_version.put::>(); + + let one = 1u64; + let two = 2u64; + let deposit = 10u64; + + // Set balance to 100 + Balances::make_free_balance_be(&one, 100u64); + Balances::make_free_balance_be(&two, 100u64); + + // Reservations + Balances::reserve(&one, 10u64).unwrap(); // old + Balances::reserve(&two, 10u64).unwrap(); // old + + // Candidate info + let candidate_one = CandidateInfo { who: one, deposit }; + let candidate_two = CandidateInfo { who: two, deposit }; + + // Storage lists + let bounded_candidates = + BoundedVec::, ConstU32<20>>::try_from(vec![ + candidate_one.clone(), + candidate_two.clone(), + ]) + .expect("it works"); + + // Set storage + Candidates::::put(bounded_candidates.clone()); + + // Sanity check + assert_eq!(Balances::free_balance(one), 90); + assert_eq!(Balances::free_balance(two), 90); + + // Run migration + v2::MigrationToV2::::on_runtime_upgrade(); + + let new_storage_version = StorageVersion::get::>(); + assert_eq!(new_storage_version, 2); + + // Nothing changes deposit-wise + assert_eq!(Balances::free_balance(one), 90); + assert_eq!(Balances::free_balance(two), 90); + // The storage item should be gone + assert!(Candidates::::get().is_empty()); + // The new storage item should have the info now + assert_eq!(CandidateList::::get(), bounded_candidates); + }); + } +} diff --git a/cumulus/parachain-template/runtime/src/lib.rs b/cumulus/parachain-template/runtime/src/lib.rs index 7a064e227d4c..9e63ed36b5c7 100644 --- a/cumulus/parachain-template/runtime/src/lib.rs +++ b/cumulus/parachain-template/runtime/src/lib.rs @@ -115,6 +115,13 @@ pub type SignedExtra = ( pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; +/// Migrations to apply on runtime upgrade. +pub type Migrations = ( + pallet_collator_selection::migration::v2::MigrationToV2, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); + /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< Runtime, diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index fed9dc80932f..a4fe057e8b1e 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -959,7 +959,67 @@ pub type SignedExtra = ( pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (pallet_collator_selection::migration::v1::MigrateToV1,); +#[allow(deprecated)] +pub type Migrations = ( + InitStorageVersions, + // unreleased + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + pallet_collator_selection::migration::v2::MigrationToV2, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); + +/// Migration to initialize storage versions for pallets added after genesis. +/// +/// This is now done automatically (see ), +/// but some pallets had made it in and had storage set in them for this parachain before it was +/// merged. +pub struct InitStorageVersions; + +impl frame_support::traits::OnRuntimeUpgrade for InitStorageVersions { + fn on_runtime_upgrade() -> Weight { + use frame_support::traits::{GetStorageVersion, StorageVersion}; + + let mut writes = 0; + + if PolkadotXcm::on_chain_storage_version() == StorageVersion::new(0) { + PolkadotXcm::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + if Multisig::on_chain_storage_version() == StorageVersion::new(0) { + Multisig::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + if Assets::on_chain_storage_version() == StorageVersion::new(0) { + Assets::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + if Uniques::on_chain_storage_version() == StorageVersion::new(0) { + Uniques::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + if Nfts::on_chain_storage_version() == StorageVersion::new(0) { + Nfts::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + if ForeignAssets::on_chain_storage_version() == StorageVersion::new(0) { + ForeignAssets::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + if PoolAssets::on_chain_storage_version() == StorageVersion::new(0) { + PoolAssets::in_code_storage_version().put::(); + writes.saturating_inc(); + } + + ::DbWeight::get().reads_writes(7, writes) + } +} /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index d45337776361..1e03bde66985 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -939,7 +939,7 @@ pub type Migrations = ( // v9420 pallet_nfts::migration::v1::MigrateToV1, // unreleased - pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, // unreleased migrations::NativeAssetParents0ToParents1Migration, // unreleased diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/lib.rs index 1e59c60596ef..7e9a9a47cfc0 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/lib.rs @@ -110,8 +110,11 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( - // unreleased - pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + pallet_broker::migration::MigrateV0ToV1, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/lib.rs index 72c2ccce8911..9e7fcd5137c3 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/lib.rs @@ -110,8 +110,9 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( - // unreleased - pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs index 00eb8d1594fb..50ade9fac810 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs @@ -115,7 +115,7 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( - pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, pallet_multisig::migrations::v1::MigrateToV1, InitStorageVersions, ); diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs index 5db8dbe2384f..fadd328db01b 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs @@ -115,7 +115,7 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( - pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, pallet_multisig::migrations::v1::MigrateToV1, InitStorageVersions, ); diff --git a/cumulus/parachains/runtimes/collectives/collectives-polkadot/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-polkadot/src/lib.rs index 206f46140606..7e48b9d8912d 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-polkadot/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-polkadot/src/lib.rs @@ -96,13 +96,70 @@ use xcm_config::{GovernanceLocation, XcmOriginToTransactDispatchOrigin}; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; +use sp_runtime::{ + create_runtime_str, generic, impl_opaque_keys, + traits::Block as BlockT, + transaction_validity::{TransactionSource, TransactionValidity}, + ApplyExtrinsicResult, DispatchError, MultiAddress, Perbill, +}; +use sp_std::prelude::*; +#[cfg(feature = "std")] +use sp_version::NativeVersion; +use sp_version::RuntimeVersion; +use testnet_parachains_constants::rococo::{consensus::*, currency::*, fee::WeightToFee, time::*}; +use weights::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight}; +use xcm::latest::prelude::*; +use xcm_config::{ + FellowshipLocation, GovernanceLocation, RocRelayLocation, XcmOriginToTransactDispatchOrigin, +}; -// Polkadot imports -use pallet_xcm::{EnsureXcm, IsVoiceOfBody}; -use polkadot_runtime_common::{BlockHashCount, SlowAdjustingFeeUpdate}; -use xcm::latest::{prelude::*, BodyId}; +/// The address format for describing accounts. +pub type Address = MultiAddress; -use weights::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight}; +/// Block type as expected by this runtime. +pub type Block = generic::Block; + +/// A Block signed with a Justification +pub type SignedBlock = generic::SignedBlock; + +/// BlockId type as expected by this runtime. +pub type BlockId = generic::BlockId; + +/// The SignedExtension to the basic transaction logic. +pub type SignedExtra = ( + frame_system::CheckNonZeroSender, + frame_system::CheckSpecVersion, + frame_system::CheckTxVersion, + frame_system::CheckGenesis, + frame_system::CheckEra, + frame_system::CheckNonce, + frame_system::CheckWeight, + pallet_transaction_payment::ChargeTransactionPayment, + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim, +); + +/// Unchecked extrinsic type as expected by this runtime. +pub type UncheckedExtrinsic = + generic::UncheckedExtrinsic; + +/// Migrations to apply on runtime upgrade. +pub type Migrations = ( + pallet_collator_selection::migration::v2::MigrationToV2, + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + pallet_broker::migration::MigrateV0ToV1, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); + +/// Executive: handles dispatch to the various modules. +pub type Executive = frame_executive::Executive< + Runtime, + Block, + frame_system::ChainContext, + Runtime, + AllPalletsWithSystem, + Migrations, +>; impl_opaque_keys! { pub struct SessionKeys { diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 54cb898fd6cb..e0bb58459c65 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -713,7 +713,11 @@ pub type UncheckedExtrinsic = /// `OnRuntimeUpgrade`. Included migrations must be idempotent. type Migrations = ( // unreleased - pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, + // unreleased + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index 4c66e780ba9f..bc51f2e4a632 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -99,6 +99,8 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( + pallet_collator_selection::migration::v1::MigrateToV1, + pallet_collator_selection::migration::v2::MigrationToV2, cumulus_pallet_parachain_system::migration::Migration, cumulus_pallet_xcmp_queue::migration::MigrationToV3, pallet_contracts::Migration, diff --git a/prdoc/pr_4229.prdoc b/prdoc/pr_4229.prdoc new file mode 100644 index 000000000000..05af8e062a32 --- /dev/null +++ b/prdoc/pr_4229.prdoc @@ -0,0 +1,10 @@ +title: "Fix Stuck Collator Funds" + +doc: + - audience: Runtime Dev + description: | + Fixes stuck collator funds by providing a migration that should have been in PR 1340. + +crates: + - name: pallet-collator-selection + bump: patch