diff --git a/polkadot/runtime/parachains/src/coretime/migration.rs b/polkadot/runtime/parachains/src/coretime/migration.rs index 4e75088675590..d4be135aad657 100644 --- a/polkadot/runtime/parachains/src/coretime/migration.rs +++ b/polkadot/runtime/parachains/src/coretime/migration.rs @@ -24,7 +24,6 @@ mod v_coretime { use crate::{ assigner_coretime, configuration, coretime::{mk_coretime_call, Config, PartsOf57600, WeightInfo}, - paras, }; use alloc::{vec, vec::Vec}; #[cfg(feature = "try-runtime")] @@ -51,18 +50,24 @@ mod v_coretime { pub trait GetLegacyLease { /// If parachain is a lease holding parachain, return the block at which the lease expires. fn get_parachain_lease_in_blocks(para: ParaId) -> Option; + // All parachains holding a lease, no matter if there are gaps in the slots or not. + fn get_all_parachains_with_leases() -> Vec; } /// Migrate a chain to use coretime. /// /// This assumes that the `Coretime` and the `AssignerCoretime` pallets are added at the same /// time to a runtime. - pub struct MigrateToCoretime( + pub struct MigrateToCoretime( core::marker::PhantomData<(T, SendXcm, LegacyLease)>, ); - impl>> - MigrateToCoretime + impl< + T: Config, + SendXcm: xcm::v4::SendXcm, + LegacyLease: GetLegacyLease>, + const TIMESLICE_PERIOD: u32, + > MigrateToCoretime { fn already_migrated() -> bool { // We are using the assigner coretime because the coretime pallet doesn't has any @@ -94,7 +99,8 @@ mod v_coretime { T: Config + crate::dmp::Config, SendXcm: xcm::v4::SendXcm, LegacyLease: GetLegacyLease>, - > OnRuntimeUpgrade for MigrateToCoretime + const TIMESLICE_PERIOD: u32, + > OnRuntimeUpgrade for MigrateToCoretime { fn on_runtime_upgrade() -> Weight { if Self::already_migrated() { @@ -102,7 +108,7 @@ mod v_coretime { } log::info!("Migrating existing parachains to coretime."); - migrate_to_coretime::() + migrate_to_coretime::() } #[cfg(feature = "try-runtime")] @@ -111,7 +117,7 @@ mod v_coretime { return Ok(Vec::new()) } - let legacy_paras = paras::Parachains::::get(); + let legacy_paras = LegacyLease::get_all_parachains_with_leases(); let config = configuration::ActiveConfig::::get(); let total_core_count = config.scheduler_params.num_cores + legacy_paras.len() as u32; @@ -154,8 +160,9 @@ mod v_coretime { T: Config, SendXcm: xcm::v4::SendXcm, LegacyLease: GetLegacyLease>, + const TIMESLICE_PERIOD: u32, >() -> Weight { - let legacy_paras = paras::Parachains::::get(); + let legacy_paras = LegacyLease::get_all_parachains_with_leases(); let legacy_count = legacy_paras.len() as u32; let now = frame_system::Pallet::::block_number(); for (core, para_id) in legacy_paras.into_iter().enumerate() { @@ -175,7 +182,6 @@ mod v_coretime { } let config = configuration::ActiveConfig::::get(); - // num_cores was on_demand_cores until now: for on_demand in 0..config.scheduler_params.num_cores { let core = CoreIndex(legacy_count.saturating_add(on_demand as _)); let r = assigner_coretime::Pallet::::assign_core( @@ -193,7 +199,9 @@ mod v_coretime { c.scheduler_params.num_cores = total_cores; }); - if let Err(err) = migrate_send_assignments_to_coretime_chain::() { + if let Err(err) = + migrate_send_assignments_to_coretime_chain::( + ) { log::error!("Sending legacy chain data to coretime chain failed: {:?}", err); } @@ -210,8 +218,9 @@ mod v_coretime { T: Config, SendXcm: xcm::v4::SendXcm, LegacyLease: GetLegacyLease>, + const TIMESLICE_PERIOD: u32, >() -> result::Result<(), SendError> { - let legacy_paras = paras::Parachains::::get(); + let legacy_paras = LegacyLease::get_all_parachains_with_leases(); let legacy_paras_count = legacy_paras.len(); let (system_chains, lease_holding): (Vec<_>, Vec<_>) = legacy_paras.into_iter().partition(IsSystem::is_system); @@ -224,7 +233,7 @@ mod v_coretime { mk_coretime_call::(crate::coretime::CoretimeCalls::Reserve(schedule)) }); - let leases = lease_holding.into_iter().filter_map(|p| { + let mut leases = lease_holding.into_iter().filter_map(|p| { log::trace!(target: "coretime-migration", "Preparing sending of lease holding para {:?}", p); let Some(valid_until) = LegacyLease::get_parachain_lease_in_blocks(p) else { log::error!("Lease holding chain with no lease information?!"); @@ -237,10 +246,7 @@ mod v_coretime { return None }, }; - // We assume the coretime chain set this parameter to the recommended value in RFC-1: - const TIME_SLICE_PERIOD: u32 = 80; - let round_up = if valid_until % TIME_SLICE_PERIOD > 0 { 1 } else { 0 }; - let time_slice = valid_until / TIME_SLICE_PERIOD + TIME_SLICE_PERIOD * round_up; + let time_slice = (valid_until + TIMESLICE_PERIOD - 1) / TIMESLICE_PERIOD; log::trace!(target: "coretime-migration", "Sending of lease holding para {:?}, valid_until: {:?}, time_slice: {:?}", p, valid_until, time_slice); Some(mk_coretime_call::(crate::coretime::CoretimeCalls::SetLease(p.into(), time_slice))) }); @@ -269,16 +275,30 @@ mod v_coretime { }); let reservation_content = message_content.clone().chain(reservations).collect(); - let pool_content = message_content.clone().chain(pool).collect(); - let leases_content = message_content.clone().chain(leases).collect(); + let leases_content_1 = message_content + .clone() + .chain(leases.by_ref().take(legacy_paras_count / 2)) // split in two messages to avoid overweighted XCM + .collect(); + let leases_content_2 = message_content.clone().chain(leases).collect(); let set_core_count_content = message_content.clone().chain(set_core_count).collect(); - - let messages = vec![ - Xcm(reservation_content), - Xcm(pool_content), - Xcm(leases_content), - Xcm(set_core_count_content), - ]; + // If `pool_content` is empty don't send a blank XCM message + let messages = if core_count as usize > legacy_paras_count { + let pool_content = message_content.clone().chain(pool).collect(); + vec![ + Xcm(reservation_content), + Xcm(pool_content), + Xcm(leases_content_1), + Xcm(leases_content_2), + Xcm(set_core_count_content), + ] + } else { + vec![ + Xcm(reservation_content), + Xcm(leases_content_1), + Xcm(leases_content_2), + Xcm(set_core_count_content), + ] + }; for message in messages { send_xcm::( diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 30b915d32deba..31713755b9b20 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1580,6 +1580,13 @@ pub mod migrations { as Leaser>::lease_period_index(now)?; Some(index.saturating_add(lease.len() as u32).saturating_mul(LeasePeriod::get())) } + + fn get_all_parachains_with_leases() -> Vec { + slots::Leases::::iter() + .filter(|(_, lease)| !lease.is_empty()) + .map(|(para, _)| para) + .collect::>() + } } parameter_types! { @@ -1661,7 +1668,7 @@ pub mod migrations { pallet_identity::migration::versioned::V0ToV1, parachains_configuration::migration::v11::MigrateToV11, // This needs to come after the `parachains_configuration` above as we are reading the configuration. - coretime::migration::MigrateToCoretime, + coretime::migration::MigrateToCoretime, parachains_configuration::migration::v12::MigrateToV12, parachains_on_demand::migration::MigrateV0ToV1, diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 5f8d5d4249383..519c7dcde54eb 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -65,7 +65,7 @@ use polkadot_runtime_common::{ VersionedLocatableAsset, VersionedLocationConverter, }, paras_registrar, paras_sudo_wrapper, prod_or_fast, slots, - traits::{Leaser, OnSwap}, + traits::OnSwap, BalanceToU256, BlockHashCount, BlockLength, CurrencyToVote, SlowAdjustingFeeUpdate, U256ToBalance, }; @@ -1775,24 +1775,6 @@ pub type Migrations = migrations::Unreleased; pub mod migrations { use super::*; - pub struct GetLegacyLeaseImpl; - impl coretime::migration::GetLegacyLease for GetLegacyLeaseImpl { - fn get_parachain_lease_in_blocks(para: ParaId) -> Option { - let now = frame_system::Pallet::::block_number(); - let lease = slots::Leases::::get(para); - if lease.is_empty() { - return None; - } - // Lease not yet started, ignore: - if lease.iter().any(Option::is_none) { - return None; - } - let (index, _) = - as Leaser>::lease_period_index(now)?; - Some(index.saturating_add(lease.len() as u32).saturating_mul(LeasePeriod::get())) - } - } - /// Unreleased migrations. Add new ones here: pub type Unreleased = ( // This is only needed for Westend. diff --git a/prdoc/pr_5380.prdoc b/prdoc/pr_5380.prdoc new file mode 100644 index 0000000000000..75063e3353439 --- /dev/null +++ b/prdoc/pr_5380.prdoc @@ -0,0 +1,15 @@ +title: Fix leases with gaps and time slice calculation in MigrateToCoretime + +doc: + - audience: Runtime Dev + description: | + Agile Coretime storage migration wasn't transferring correctly leases with gaps and was + miscalculating time lease period. This patch provides fixes for both issues. + +crates: + - name: rococo-runtime + bump: patch + - name: westend-runtime + bump: major + - name: polkadot-runtime-parachains + bump: patch \ No newline at end of file