Fix leases with gaps and time slice calculation in MigrateToCoretime (

paritytech#5380)

Backport fixes from
polkadot-fellows/runtimes#426

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<N> {
 		/// If parachain is a lease holding parachain, return the block at which the lease expires.
 		fn get_parachain_lease_in_blocks(para: ParaId) -> Option<N>;
+		// All parachains holding a lease, no matter if there are gaps in the slots or not.
+		fn get_all_parachains_with_leases() -> Vec<ParaId>;
 	}
 
 	/// 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<T, SendXcm, LegacyLease>(
+	pub struct MigrateToCoretime<T, SendXcm, LegacyLease, const TIMESLICE_PERIOD: u32>(
 		core::marker::PhantomData<(T, SendXcm, LegacyLease)>,
 	);
 
-	impl<T: Config, SendXcm: xcm::v4::SendXcm, LegacyLease: GetLegacyLease<BlockNumberFor<T>>>
-		MigrateToCoretime<T, SendXcm, LegacyLease>
+	impl<
+			T: Config,
+			SendXcm: xcm::v4::SendXcm,
+			LegacyLease: GetLegacyLease<BlockNumberFor<T>>,
+			const TIMESLICE_PERIOD: u32,
+		> MigrateToCoretime<T, SendXcm, LegacyLease, TIMESLICE_PERIOD>
 	{
 		fn already_migrated() -> bool {
 			// We are using the assigner coretime because the coretime pallet doesn't has any
@@ -94,15 +99,16 @@ mod v_coretime {
 			T: Config + crate::dmp::Config,
 			SendXcm: xcm::v4::SendXcm,
 			LegacyLease: GetLegacyLease<BlockNumberFor<T>>,
-		> OnRuntimeUpgrade for MigrateToCoretime<T, SendXcm, LegacyLease>
+			const TIMESLICE_PERIOD: u32,
+		> OnRuntimeUpgrade for MigrateToCoretime<T, SendXcm, LegacyLease, TIMESLICE_PERIOD>
 	{
 		fn on_runtime_upgrade() -> Weight {
 			if Self::already_migrated() {
 				return Weight::zero()
 			}
 
 			log::info!("Migrating existing parachains to coretime.");
-			migrate_to_coretime::<T, SendXcm, LegacyLease>()
+			migrate_to_coretime::<T, SendXcm, LegacyLease, TIMESLICE_PERIOD>()
 		}
 
 		#[cfg(feature = "try-runtime")]
@@ -111,7 +117,7 @@ mod v_coretime {
 				return Ok(Vec::new())
 			}
 
-			let legacy_paras = paras::Parachains::<T>::get();
+			let legacy_paras = LegacyLease::get_all_parachains_with_leases();
 			let config = configuration::ActiveConfig::<T>::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<BlockNumberFor<T>>,
+		const TIMESLICE_PERIOD: u32,
 	>() -> Weight {
-		let legacy_paras = paras::Parachains::<T>::get();
+		let legacy_paras = LegacyLease::get_all_parachains_with_leases();
 		let legacy_count = legacy_paras.len() as u32;
 		let now = frame_system::Pallet::<T>::block_number();
 		for (core, para_id) in legacy_paras.into_iter().enumerate() {
@@ -175,7 +182,6 @@ mod v_coretime {
 		}
 
 		let config = configuration::ActiveConfig::<T>::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::<T>::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::<T, SendXcm, LegacyLease>() {
+		if let Err(err) =
+			migrate_send_assignments_to_coretime_chain::<T, SendXcm, LegacyLease, TIMESLICE_PERIOD>(
+			) {
 			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<BlockNumberFor<T>>,
+		const TIMESLICE_PERIOD: u32,
 	>() -> result::Result<(), SendError> {
-		let legacy_paras = paras::Parachains::<T>::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::<T>(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::<T>(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::<SendXcm>(

polkadot/runtime/rococo/src/lib.rs

@@ -1580,6 +1580,13 @@ pub mod migrations {
 				<slots::Pallet<Runtime> as Leaser<BlockNumber>>::lease_period_index(now)?;
 			Some(index.saturating_add(lease.len() as u32).saturating_mul(LeasePeriod::get()))
 		}
+
+		fn get_all_parachains_with_leases() -> Vec<ParaId> {
+			slots::Leases::<Runtime>::iter()
+				.filter(|(_, lease)| !lease.is_empty())
+				.map(|(para, _)| para)
+				.collect::<Vec<_>>()
+		}
 	}
 
 	parameter_types! {
@@ -1661,7 +1668,7 @@ pub mod migrations {
         pallet_identity::migration::versioned::V0ToV1<Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
         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>,
+        coretime::migration::MigrateToCoretime<Runtime, crate::xcm_config::XcmRouter, GetLegacyLeaseImpl, TIMESLICE_PERIOD>,
         parachains_configuration::migration::v12::MigrateToV12<Runtime>,
         parachains_on_demand::migration::MigrateV0ToV1<Runtime>,
 

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<BlockNumber> for GetLegacyLeaseImpl {
-		fn get_parachain_lease_in_blocks(para: ParaId) -> Option<BlockNumber> {
-			let now = frame_system::Pallet::<Runtime>::block_number();
-			let lease = slots::Leases::<Runtime>::get(para);
-			if lease.is_empty() {
-				return None;
-			}
-			// Lease not yet started, ignore:
-			if lease.iter().any(Option::is_none) {
-				return None;
-			}
-			let (index, _) =
-				<slots::Pallet<Runtime> as Leaser<BlockNumber>>::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.

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