From 8b09d7fbcd1fd4dcde74879b97d80df3985be47f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Fri, 20 Sep 2024 16:43:39 +0100 Subject: [PATCH 01/25] Add migration to fix coretime state (WIP) --- .../coretime/coretime-polkadot/src/lib.rs | 4 +- .../coretime-polkadot/src/migrations.rs | 332 ++++++++++++++++++ 2 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 system-parachains/coretime/coretime-polkadot/src/migrations.rs diff --git a/system-parachains/coretime/coretime-polkadot/src/lib.rs b/system-parachains/coretime/coretime-polkadot/src/lib.rs index 7b1cdf1ce2..6ae37db720 100644 --- a/system-parachains/coretime/coretime-polkadot/src/lib.rs +++ b/system-parachains/coretime/coretime-polkadot/src/lib.rs @@ -23,6 +23,7 @@ include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); mod coretime; +mod migrations; // Genesis preset configurations. pub mod genesis_config_presets; #[cfg(test)] @@ -117,6 +118,7 @@ pub type UncheckedExtrinsic = pub type Migrations = ( // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + migrations::FixMigration, ); /// Executive: handles dispatch to the various modules. @@ -140,7 +142,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("coretime-polkadot"), impl_name: create_runtime_str!("coretime-polkadot"), authoring_version: 1, - spec_version: 1_003_000, + spec_version: 1_003_003, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 0, diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs new file mode 100644 index 0000000000..c7fa86be72 --- /dev/null +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -0,0 +1,332 @@ +// Copyright (C) Parity Technologies and the various Polkadot contributors, see Contributions.md +// for a list of specific contributors. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Migration to fix the coretime migration lease offset-related issues +//! +//! This fixes a problem with the leases where the relay migration did not take the lease offset +//! into consideration, so the end of leases is 64 days short, in some cases leading to them being +//! dropped completely. + +use crate::{weights, Runtime, RuntimeOrigin}; +use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; +use pallet_broker::{CoreAssignment, Leases, LeasesRecordOf, WeightInfo}; +#[cfg(feature = "try-runtime")] +use pallet_broker::{ + CoreAssignment::{Pool, Task}, + CoreMask, LeaseRecordItem, PotentialRenewalId, PotentialRenewalRecord, PotentialRenewals, + SaleInfo, SaleInfoRecordOf, Schedule, ScheduleItem, Workplan, +}; +#[cfg(feature = "try-runtime")] +use sp_runtime::TryRuntimeError; +#[cfg(feature = "try-runtime")] +use sp_std::vec::Vec; + +/// The log target. +const TARGET: &str = "runtime::bootstrapping::fix-migration"; + +// Alias into the broker weights for this runtime. +type BrokerWeights = weights::pallet_broker::WeightInfo; + +pub struct FixMigration; + +impl OnRuntimeUpgrade for FixMigration { + fn on_runtime_upgrade() -> Weight { + if PotentialRenewals::::get(PotentialRenewalId { core: 6, when: 292605 }).is_none() + { + // Idempotency check - this core will never be renewable at this timeslice ever again. + log::error!(target: TARGET, "This migration includes hardcoded values not relevant to this runtime. Bailing."); + return ::DbWeight::get().reads(1); + } + + // Add leases for 2040, 2094, 3333, 2106, 2101, 2093 with a properly calculated end + // timeslice. + let leases: LeasesRecordOf = LEASES + .iter() + .map(|(until, task)| LeaseRecordItem { until: *until, task: *task }) + .collect::>() + .try_into() + .expect("Within range of bounded vec"); + Leases::::put(leases); + + // This reorders the cores, so the existing entries to the workplan need to be overwritten. + for (&(para_id, _), core_id) in LEASES.iter().zip(51u16..55u16) { + // Add to the workplan at timeslice 287565 using the new cores. + let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: CoreAssignment::Task(para_id), + }])); + + Workplan::::insert((292605, core_id), workplan_entry); + } + + // Remove the existing 6 PotentialRenewals. + for &(core, when) in INCORRECT_RENEWAL_IDS.iter() { + PotentialRenewals::::remove(PotentialRenewalId { core, when }); + } + + // Sort the parachains who can renew. They are currently missing from the broker state + // entirely. + // TODO double check the core ids, these should be on top of the last available in the sale. + for (&(para_id, _), core_id) in POTENTIAL_RENEWALS.iter().zip(56u16..61u16) { + // Add to the workplan at timeslice 287565 using the new cores. + let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: CoreAssignment::Task(para_id), + }])); + + Workplan::::insert((292605, core_id), workplan_entry); + } + + // Add cores to the system - this will take 2 sessions to kick in. + // 1 core for the system on-demand pool, 5 for open market, 5 for reservations and 51 + // parachains, of which 46 have leases and 5 are up for renewal. + let core_count = pallet_broker::Reservations::::decode_len().unwrap_or(0) as u16 + + pallet_broker::Leases::::decode_len().unwrap_or(0) as u16 + + 5 + 6; + + match pallet_broker::Pallet::::request_core_count( + RuntimeOrigin::root(), + core_count, + ) { + Ok(_) => log::info!(target: TARGET, "Request for 62 cores sent."), + Err(_) => log::error!(target: TARGET, "Request for 62 cores failed to send."), + } + + let pool_assignment = Schedule::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: Pool, + }])); + + // Reserve the system pool core - this kicks in after two sale period boundaries. + match pallet_broker::Pallet::::reserve( + RuntimeOrigin::root(), + pool_assignment.clone(), + ) { + Ok(_) => log::info!(target: TARGET, "Pool core reserved."), + Err(_) => log::error!(target: TARGET, "Pool core reservation failed."), + } + + // Add the system pool core to the workplan for the next cycle (287565) on the last new core + // (core 61) + Workplan::::insert((292605, 61), pool_assignment.clone()); + + // Add the system pool core to the workplan starting now. + let now = 287000; // TODO - this needs to be after the cores have just added have been processed, which takes + // two sessions. Currently just a placeholder. This part is probably better as a call to + // assign_core on the relay as part of the referendum instead of here. + Workplan::::insert((now, 61), pool_assignment); + + // TODO finalise the weights here. + ::DbWeight::get() + .writes(1) + .saturating_mul(LEASES.len() as u64) + .saturating_add(BrokerWeights::request_core_count(62)) + .saturating_add(::DbWeight::get().reads(1)) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + // Idempotency check - this core will never be renewable at this timeslice ever again. + if PotentialRenewals::::get(PotentialRenewalId { core: 6, when: 292605 }).is_none() + { + return Ok(Vec::new()) + } + let sale_info = SaleInfo::::get().unwrap(); + let leases = Leases::::get(); + let pre_upgrade_state = (sale_info, leases); + Ok(pre_upgrade_state.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { + use pallet_broker::LeasesRecordOf; + + if state.is_empty() { + return Ok(()) + } + let (prev_sale_info, prev_leases): (SaleInfoRecordOf, LeasesRecordOf) = + Decode::decode(&mut &state[..]).unwrap(); + + log::info!(target: TARGET, "Checking migration."); + + let sale_info = SaleInfo::::get().unwrap(); + + // Check the sale start has not changed. + assert_eq!(sale_info, prev_sale_info); + + // The workplan entries start from the region begin reported by the new SaleInfo. + let workplan_start = sale_info.region_begin; + + // Check the reservations are still in the workplan out of an abundance of caution. + for (core_id, task) in [Task(1001), Task(1002), Task(1000), Task(1004), Task(1005)] + .into_iter() + .enumerate() + { + assert_eq!( + Workplan::::get((workplan_start, core_id as u16)), + Some(Schedule::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: task, + }]))) + ); + } + + // Make sure we've got all the leases. + let leases = Leases::::get(); + assert_eq!(leases.len(), LEASES.iter().filter(|(_, l)| sale_info.region_end <= *l).count()); + + // Make something out of the leases that is easier to check against. + let leases_vec: Vec<(u32, u32)> = + leases.iter().map(|LeaseRecordItem { until, task }| (*task, *until)).collect(); + + // Iterate through hardcoded leases and check they're all correctly in state and scheduled + // in the workplan. + for (i, (para_id, until)) in LEASES.iter().enumerate() { + // Add the system parachains as an offset - these should come before the leases. + let core_id = i as u16 + 4; + + assert!(leases_vec.contains(&(*until, *para_id))); + + // This is the entry found in Workplan + let workload = Schedule::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: Task(*para_id), + }])); + + // They should all be in the workplan for next region. + assert_eq!(Workplan::::get((workplan_start, core_id)), Some(workload)); + } + + // For the leases we had before their lease should extend for an additional 11520 + // timeslices (64 days). + for LeaseRecordItem { task, until } in prev_leases.iter() { + log::error!("{task}, {until}"); + assert!(leases_vec.contains(&(*until + 11520, *task))) + } + + // Iterate through hardcoded potential renewals and check they're all correctly in state + // and scheduled in the workplan. + for (i, (para_id, until)) in POTENTIAL_RENEWALS.iter().enumerate() { + // Add the system parachains as an offset - these should come before + // the leases. + let core_id = i as u16 + 4; + + // This is the entry found in Workplan and PotentialRenewals. + let workload = Schedule::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: Task(*para_id), + }])); + + // Make sure they're not in the leases. + assert!(!leases.contains(&LeaseRecordItem { until: *until, task: *para_id })); + + // Ensure they can renew in the next region. + assert_eq!( + PotentialRenewals::::get(PotentialRenewalId { + core: core_id, + when: sale_info.region_end + 5040, + }), + Some(PotentialRenewalRecord { + price: 1_000_000_000_000, + completion: pallet_broker::CompletionStatus::Complete(workload.clone()) + }) + ); + + // They should all be in the workplan for next sale. + assert_eq!(Workplan::::get((workplan_start, core_id)), Some(workload)); + } + + // Walk the workplan at timeslice 287565 and make sure there is an entry for every 62 cores. + for i in 0..61 { + // TODO - finalise + assert!(Workplan::::get((287565, i)).is_some()); + } + + // Ensure we have requested the correct number of cores. + assert!(frame_system::Pallet::::read_events_no_consensus().any(|e| { + match e.event { + crate::RuntimeEvent::Broker( + pallet_broker::Event::::CoreCountRequested { core_count }, + ) => { + log::info!("{core_count:?}"); + + core_count == 62 + }, + _ => false, + } + })); + + Ok(()) + } +} + +// Incorrect potential renewals in state +const INCORRECT_RENEWAL_IDS: [(u16, u32); 6] = + [(6, 292605), (5, 292605), (13, 292605), (15, 292605), (47, 292605), (44, 292605)]; + +// Hardcoded para ids and their end timeslice. +// Taken from https://github.com/SBalaguer/coretime-migration/blob/master/polkadot-output-200924.json +const POTENTIAL_RENEWALS: [(u32, u32); 5] = + [(2048, 283680), (3375, 283680), (3358, 283680), (2053, 283680), (2056, 283680)]; + +const LEASES: [(u32, u32); 46] = [ + (2094, 298800), + (2040, 298800), + (3333, 298800), + (2106, 298800), + (2093, 298800), + (2101, 298800), + (3369, 313920), + (2000, 313920), + (3338, 329040), + (2004, 329040), + (3344, 344160), + (3345, 344160), + (3340, 344160), + (2031, 344160), + (3346, 344160), + (2026, 344160), + (2006, 344160), + (2035, 359280), + (2032, 359280), + (2025, 359280), + (2002, 359280), + (2034, 359280), + (2012, 359280), + (3354, 359280), + (3367, 374400), + (2030, 374400), + (2046, 374400), + (3366, 374400), + (2037, 374400), + (2043, 374400), + (2013, 374400), + (3370, 389520), + (2086, 389520), + (2051, 389520), + (2008, 389520), + (3359, 389520), + (3377, 389520), + (3373, 389520), + (2092, 404640), + (3378, 404640), + (2104, 404640), + (3389, 404640), + (2090, 404640), + (3397, 404640), + (2091, 404640), + (3388, 404640), +]; From d210d8d89e96d3dac02b23032eec2081bec18814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Sat, 21 Sep 2024 17:38:19 +0100 Subject: [PATCH 02/25] Update timeslice --- system-parachains/coretime/coretime-polkadot/src/migrations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index c7fa86be72..80da46b32e 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -69,7 +69,7 @@ impl OnRuntimeUpgrade for FixMigration { assignment: CoreAssignment::Task(para_id), }])); - Workplan::::insert((292605, core_id), workplan_entry); + Workplan::::insert((287565, core_id), workplan_entry); } // Remove the existing 6 PotentialRenewals. From 64770cc8d6f81f46896a29b525df9bbc834e0e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Sat, 21 Sep 2024 17:38:28 +0100 Subject: [PATCH 03/25] Update timeslice --- system-parachains/coretime/coretime-polkadot/src/migrations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 80da46b32e..287506cd79 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -87,7 +87,7 @@ impl OnRuntimeUpgrade for FixMigration { assignment: CoreAssignment::Task(para_id), }])); - Workplan::::insert((292605, core_id), workplan_entry); + Workplan::::insert((287565, core_id), workplan_entry); } // Add cores to the system - this will take 2 sessions to kick in. From 4dd132688f4e37b8cee321aac63a8c9b2eca4ef5 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 23 Sep 2024 13:24:19 +0300 Subject: [PATCH 04/25] Fix compilation without `try-runtime` feature --- .../coretime/coretime-polkadot/src/migrations.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 287506cd79..0a2a59b28b 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -22,17 +22,17 @@ use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; -use pallet_broker::{CoreAssignment, Leases, LeasesRecordOf, WeightInfo}; -#[cfg(feature = "try-runtime")] use pallet_broker::{ - CoreAssignment::{Pool, Task}, - CoreMask, LeaseRecordItem, PotentialRenewalId, PotentialRenewalRecord, PotentialRenewals, - SaleInfo, SaleInfoRecordOf, Schedule, ScheduleItem, Workplan, + CoreAssignment, CoreAssignment::Pool, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, + PotentialRenewalId, PotentialRenewals, Schedule, ScheduleItem, WeightInfo, Workplan, }; + +use sp_std::vec::Vec; + #[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeError; +use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf}; #[cfg(feature = "try-runtime")] -use sp_std::vec::Vec; +use sp_runtime::TryRuntimeError; /// The log target. const TARGET: &str = "runtime::bootstrapping::fix-migration"; From 0367a60f1edd7ad09e7d89262264a03497c9dab0 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 23 Sep 2024 13:34:15 +0300 Subject: [PATCH 05/25] Remove hardcoded core ids and obtain them from sales instead --- .../coretime/coretime-polkadot/src/migrations.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 0a2a59b28b..d1ece3d8e6 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -24,13 +24,13 @@ use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ CoreAssignment, CoreAssignment::Pool, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, - PotentialRenewalId, PotentialRenewals, Schedule, ScheduleItem, WeightInfo, Workplan, + PotentialRenewalId, PotentialRenewals, SaleInfo, Schedule, ScheduleItem, WeightInfo, Workplan, }; use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] -use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf}; +use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfoRecordOf}; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; @@ -77,10 +77,15 @@ impl OnRuntimeUpgrade for FixMigration { PotentialRenewals::::remove(PotentialRenewalId { core, when }); } + // Get the last sale info to obtain information about cores + let sale_info = SaleInfo::::get().expect("SaleInfo should be set"); + // Sort the parachains who can renew. They are currently missing from the broker state // entirely. - // TODO double check the core ids, these should be on top of the last available in the sale. - for (&(para_id, _), core_id) in POTENTIAL_RENEWALS.iter().zip(56u16..61u16) { + for (&(para_id, _), core_id) in POTENTIAL_RENEWALS + .iter() + .zip((sale_info.first_core + sale_info.cores_offered)..POTENTIAL_RENEWALS.len() as u16) + { // Add to the workplan at timeslice 287565 using the new cores. let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), @@ -134,7 +139,7 @@ impl OnRuntimeUpgrade for FixMigration { .writes(1) .saturating_mul(LEASES.len() as u64) .saturating_add(BrokerWeights::request_core_count(62)) - .saturating_add(::DbWeight::get().reads(1)) + .saturating_add(::DbWeight::get().reads(2)) } #[cfg(feature = "try-runtime")] From ddf33fcd38f3665796d65b9ed447948885f78ea3 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 23 Sep 2024 14:12:53 +0300 Subject: [PATCH 06/25] Remove a duplicated `use` --- system-parachains/coretime/coretime-polkadot/src/migrations.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index d1ece3d8e6..cfd34010d0 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -157,8 +157,6 @@ impl OnRuntimeUpgrade for FixMigration { #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { - use pallet_broker::LeasesRecordOf; - if state.is_empty() { return Ok(()) } From 79d2e8f02f5493421f90635b3b185ca89c190808 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Tue, 24 Sep 2024 17:05:51 +0300 Subject: [PATCH 07/25] Revert "Remove hardcoded core ids and obtain them from sales instead" This reverts commit 0367a60f1edd7ad09e7d89262264a03497c9dab0. --- .../coretime/coretime-polkadot/src/migrations.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index cfd34010d0..2abc40a66c 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -24,13 +24,13 @@ use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ CoreAssignment, CoreAssignment::Pool, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, - PotentialRenewalId, PotentialRenewals, SaleInfo, Schedule, ScheduleItem, WeightInfo, Workplan, + PotentialRenewalId, PotentialRenewals, Schedule, ScheduleItem, WeightInfo, Workplan, }; use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] -use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfoRecordOf}; +use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf}; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; @@ -77,15 +77,10 @@ impl OnRuntimeUpgrade for FixMigration { PotentialRenewals::::remove(PotentialRenewalId { core, when }); } - // Get the last sale info to obtain information about cores - let sale_info = SaleInfo::::get().expect("SaleInfo should be set"); - // Sort the parachains who can renew. They are currently missing from the broker state // entirely. - for (&(para_id, _), core_id) in POTENTIAL_RENEWALS - .iter() - .zip((sale_info.first_core + sale_info.cores_offered)..POTENTIAL_RENEWALS.len() as u16) - { + // TODO double check the core ids, these should be on top of the last available in the sale. + for (&(para_id, _), core_id) in POTENTIAL_RENEWALS.iter().zip(56u16..61u16) { // Add to the workplan at timeslice 287565 using the new cores. let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), @@ -139,7 +134,7 @@ impl OnRuntimeUpgrade for FixMigration { .writes(1) .saturating_mul(LEASES.len() as u64) .saturating_add(BrokerWeights::request_core_count(62)) - .saturating_add(::DbWeight::get().reads(2)) + .saturating_add(::DbWeight::get().reads(1)) } #[cfg(feature = "try-runtime")] From 63b534b7f44ffd0bcf7810be08b175d1dd304da6 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Fri, 27 Sep 2024 15:23:21 +0300 Subject: [PATCH 08/25] Fix todos and update try-runtime tests --- .../coretime-polkadot/src/migrations.rs | 112 +++++++++++------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 2abc40a66c..afd0e3e66f 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -52,7 +52,7 @@ impl OnRuntimeUpgrade for FixMigration { } // Add leases for 2040, 2094, 3333, 2106, 2101, 2093 with a properly calculated end - // timeslice. + // timeslice. Add 11520 for all other leases. let leases: LeasesRecordOf = LEASES .iter() .map(|(until, task)| LeaseRecordItem { until: *until, task: *task }) @@ -62,7 +62,7 @@ impl OnRuntimeUpgrade for FixMigration { Leases::::put(leases); // This reorders the cores, so the existing entries to the workplan need to be overwritten. - for (&(para_id, _), core_id) in LEASES.iter().zip(51u16..55u16) { + for (&(para_id, _), core_id) in LEASES.iter().zip(51u16..56u16) { // Add to the workplan at timeslice 287565 using the new cores. let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), @@ -77,8 +77,8 @@ impl OnRuntimeUpgrade for FixMigration { PotentialRenewals::::remove(PotentialRenewalId { core, when }); } - // Sort the parachains who can renew. They are currently missing from the broker state - // entirely. + // Sort the parachains who can renew. They are currently missing from the broker + // state entirely. // TODO double check the core ids, these should be on top of the last available in the sale. for (&(para_id, _), core_id) in POTENTIAL_RENEWALS.iter().zip(56u16..61u16) { // Add to the workplan at timeslice 287565 using the new cores. @@ -168,16 +168,15 @@ impl OnRuntimeUpgrade for FixMigration { // The workplan entries start from the region begin reported by the new SaleInfo. let workplan_start = sale_info.region_begin; + let system_chains = [Task(1001), Task(1002), Task(1000), Task(1004), Task(1005)]; + // Check the reservations are still in the workplan out of an abundance of caution. - for (core_id, task) in [Task(1001), Task(1002), Task(1000), Task(1004), Task(1005)] - .into_iter() - .enumerate() - { + for (core_id, task) in system_chains.iter().enumerate() { assert_eq!( Workplan::::get((workplan_start, core_id as u16)), Some(Schedule::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), - assignment: task, + assignment: task.clone(), }]))) ); } @@ -194,7 +193,7 @@ impl OnRuntimeUpgrade for FixMigration { // in the workplan. for (i, (para_id, until)) in LEASES.iter().enumerate() { // Add the system parachains as an offset - these should come before the leases. - let core_id = i as u16 + 4; + let core_id = i as u16 + 5; assert!(leases_vec.contains(&(*until, *para_id))); @@ -218,9 +217,9 @@ impl OnRuntimeUpgrade for FixMigration { // Iterate through hardcoded potential renewals and check they're all correctly in state // and scheduled in the workplan. for (i, (para_id, until)) in POTENTIAL_RENEWALS.iter().enumerate() { - // Add the system parachains as an offset - these should come before - // the leases. - let core_id = i as u16 + 4; + // Add the system parachains and leases as an offset. + // system chains + leases + new cores = 5 + 46 + 5 = 56. + let core_id = i as u16 + 56; // This is the entry found in Workplan and PotentialRenewals. let workload = Schedule::truncate_from(Vec::from([ScheduleItem { @@ -249,8 +248,31 @@ impl OnRuntimeUpgrade for FixMigration { // Walk the workplan at timeslice 287565 and make sure there is an entry for every 62 cores. for i in 0..61 { - // TODO - finalise - assert!(Workplan::::get((287565, i)).is_some()); + let entry = Workplan::::get((287565, i)).expect("Entry should exist"); + assert_eq!(entry.len(), 1); + assert_eq!(entry.get(0).unwrap().mask, CoreMask::complete()); + if i < 5 { + // system chains + assert_eq!(entry.get(0).unwrap().assignment, system_chains[i as usize]); + } else if i < 51 { + // leases + assert_eq!( + entry.get(0).unwrap().assignment, + Task(LEASES.get(i as usize - 5).unwrap().0) + ); + } else if i < 56 { + // 5 new cores + assert_eq!( + entry.get(0).unwrap().assignment, + Task(LEASES.get(i as usize - 51).unwrap().0) + ); + } else { + // 5 potential renewals + assert_eq!( + entry.get(0).unwrap().assignment, + Task(POTENTIAL_RENEWALS.get(i as usize - 56).unwrap().0) + ); + } } // Ensure we have requested the correct number of cores. @@ -283,48 +305,48 @@ const POTENTIAL_RENEWALS: [(u32, u32); 5] = const LEASES: [(u32, u32); 46] = [ (2094, 298800), (2040, 298800), + (2035, 359280), + (3344, 344160), + (3370, 389520), + (3367, 374400), + (2086, 389520), + (2032, 359280), (3333, 298800), + (2051, 389520), (2106, 298800), - (2093, 298800), - (2101, 298800), (3369, 313920), - (2000, 313920), - (3338, 329040), - (2004, 329040), - (3344, 344160), - (3345, 344160), - (3340, 344160), - (2031, 344160), - (3346, 344160), - (2026, 344160), - (2006, 344160), - (2035, 359280), - (2032, 359280), + (2008, 389520), (2025, 359280), + (2000, 313920), + (2092, 404640), (2002, 359280), - (2034, 359280), - (2012, 359280), - (3354, 359280), - (3367, 374400), + (3359, 389520), (2030, 374400), + (3378, 404640), + (2104, 404640), (2046, 374400), - (3366, 374400), - (2037, 374400), - (2043, 374400), - (2013, 374400), - (3370, 389520), - (2086, 389520), - (2051, 389520), - (2008, 389520), - (3359, 389520), + (3345, 344160), + (3340, 344160), + (3338, 329040), + (2004, 329040), (3377, 389520), (3373, 389520), - (2092, 404640), - (3378, 404640), - (2104, 404640), + (2031, 344160), (3389, 404640), + (3366, 374400), + (2037, 374400), + (2034, 359280), (2090, 404640), + (3346, 344160), + (2012, 359280), (3397, 404640), + (2043, 374400), (2091, 404640), + (2093, 298800), + (2026, 344160), (3388, 404640), + (2101, 298800), + (3354, 359280), + (2006, 344160), + (2013, 374400), ]; From a0a2ff3ee9fa2d7362247ea4dd5afdc3a8ce7fcb Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 12:41:54 +0200 Subject: [PATCH 09/25] Basic migration logic. Renewals likely still off. --- .../coretime/coretime-polkadot/Cargo.toml | 1 + .../coretime-polkadot/src/migrations.rs | 191 +++++++++++++++++- 2 files changed, 188 insertions(+), 4 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/Cargo.toml b/system-parachains/coretime/coretime-polkadot/Cargo.toml index 164d621dfe..669f50daac 100644 --- a/system-parachains/coretime/coretime-polkadot/Cargo.toml +++ b/system-parachains/coretime/coretime-polkadot/Cargo.toml @@ -42,6 +42,7 @@ pallet-transaction-payment = { workspace = true } pallet-transaction-payment-rpc-runtime-api = { workspace = true } pallet-utility = { workspace = true } sp-api = { workspace = true } +sp-arithmetic = { workspace = true } sp-block-builder = { workspace = true } sp-consensus-aura = { workspace = true } sp-core = { workspace = true } diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index afd0e3e66f..80a43a8f38 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -20,19 +20,23 @@ //! into consideration, so the end of leases is 64 days short, in some cases leading to them being //! dropped completely. +extern crate alloc; + use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ - CoreAssignment, CoreAssignment::Pool, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, - PotentialRenewalId, PotentialRenewals, Schedule, ScheduleItem, WeightInfo, Workplan, + CompletionStatus, CoreAssignment::{self, Pool}, CoreIndex, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, PotentialRenewalId, PotentialRenewals, SalePerformance, Schedule, ScheduleItem, TaskId, Timeslice, WeightInfo, Workplan, AdaptPrice, }; use sp_std::vec::Vec; +use alloc::collections::btree_map::BTreeMap; +use sp_arithmetic::traits::Saturating; #[cfg(feature = "try-runtime")] use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf}; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; +use system_parachains_constants::polkadot::currency::UNITS; /// The log target. const TARGET: &str = "runtime::bootstrapping::fix-migration"; @@ -42,8 +46,155 @@ type BrokerWeights = weights::pallet_broker::WeightInfo; pub struct FixMigration; -impl OnRuntimeUpgrade for FixMigration { - fn on_runtime_upgrade() -> Weight { +impl FixMigration { + + fn get_first_usable_core() -> Option { + let sale_info = SaleInfo::::get()?; + // Cores between first_core and cores_offered are for sale and my not be used anymore: + Some(sale_info.first_core + sale_info.cores_offered) + } + + fn add_pool_core(first_core: &mut CoreIndex) { + let Some(sale_info) = SaleInfo::::get() else { + log::error!(target: TARGET, "Retrieving `SaleInfo` failed!"); + return + }; + + let schedule = BoundedVec::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), assignment: CoreAssignment::Pool}])); + let result = pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule.clone()); + debug_assert!(result.is_ok()); + // But we want it now(ish): add it to the workplan. + Workplan::::insert((sale_info.region_begin, *first_core), schedule); + first_core.saturating_inc(); + } + + fn extend_short_leases() { + let short_leases: BTreeMap = [ + (2035, 359280), + (3344, 344160), + (3370, 389520), + (3367, 374400), + (2086, 389520), + (2032, 359280), + (2051, 389520), + (3369, 313920), + (2008, 389520), + (2025, 359280), + (2000, 313920), + (2092, 404640), + (2002, 359280), + (3359, 389520), + (2030, 374400), + (3378, 404640), + (2104, 404640), + (2046, 374400), + (3345, 344160), + (3340, 344160), + (3338, 329040), + (2004, 329040), + (3377, 389520), + (3373, 389520), + (2031, 344160), + (3389, 404640), + (3366, 374400), + (2037, 374400), + (2034, 359280), + (2090, 404640), + (3346, 344160), + (2012, 359280), + (3397, 404640), + (2043, 374400), + (2091, 404640), + (2026, 344160), + (3388, 404640), + (3354, 359280), + (2006, 344160), + (2013, 374400), + ].into(); + + let leases = Leases::::get(); + let leases = BoundedVec::truncate_from(leases.into_iter().map(|mut l| { + let Some(new_until) = short_leases.get(&l.task) else { + return l + }; + debug_assert!(*new_until > l.until); + l.until = *new_until; + l + }).collect()); + Leases::::put(leases); + } + + // Undo rotate_sale effect on leases that wrongly expired. + // + // Needs to be called before `give_dropped_leases_renewal_rights_and_workplan_entry`. + fn remove_premature_renewals_add_back_leases() { + let premature_renewals = [ + (2094, 298800), + (2040, 298800), + (3333, 298800), + (2106, 298800), + (2093, 298800), + (2101, 298800), + ]; + + debug_assert_eq!(PotentialRenewals::::iter().count(), premature_renewals.len()); + let result = PotentialRenewals::::clear(premature_renewals.len() as u32, None); + debug_assert!(result.maybe_cursor.is_none()); + + for (task, until) in premature_renewals { + let result = pallet_broker::Pallet::::set_lease(RuntimeOrigin::root(), task, until); + debug_assert!(result.is_ok()); + } + } + + fn give_dropped_leases_renewal_rights_and_workplan_entry(first_core: &mut CoreIndex) { + let Some(sale_info) = SaleInfo::::get() else { + log::error!(target: TARGET, "Retrieving `SaleInfo` failed!"); + return + }; + + let dropped_leases = [ + (2048, 283680), + (3375, 283680), + (3358, 283680), + (2053, 283680), + (2056, 283680), + ]; + + // Leases should have been added, but removed again by rotate_sale - replaces with work + // plan items + renewal rights. + // https://github.com/paritytech/polkadot-sdk/blob/f170af615c0dc413482100892758b236d1fda93b/substrate/frame/broker/src/tick_impls.rs#L212 + // So we need to: + // - Don't change leases, already correct. + // - Add work plan entry. + // - add to potential renewals + + for (task, _) in dropped_leases { + // Workplan + let mask = CoreMask::complete(); + let assignment = CoreAssignment::Task(task); + // DONE: enough to start at next rotation? + // - Yes all good, assignments are all for next rotation already. Hence relay chain + // state is persisted for now. + let schedule = BoundedVec::truncate_from(Vec::from([ScheduleItem { mask, assignment }])); + Workplan::::insert((sale_info.region_begin, *first_core), schedule.clone()); + + // Renewal: + let new_prices = ::PriceAdapter::adapt_price(SalePerformance::from_sale(&sale_info)); + debug_assert_eq!(new_prices.target_price, 100*UNITS); + let renewal_id = PotentialRenewalId { core: *first_core, when: sale_info.region_end }; + let record = PotentialRenewalRecord { + price: new_prices.target_price, + completion: CompletionStatus::Complete(schedule), + }; + PotentialRenewals::::insert(renewal_id, &record); + + first_core.saturating_inc() + } + + } + + fn on_runtime_upgrade_donal() -> Weight { if PotentialRenewals::::get(PotentialRenewalId { core: 6, when: 292605 }).is_none() { // Idempotency check - this core will never be renewable at this timeslice ever again. @@ -129,6 +280,38 @@ impl OnRuntimeUpgrade for FixMigration { // assign_core on the relay as part of the referendum instead of here. Workplan::::insert((now, 61), pool_assignment); + // TODO finalise the weights here. + ::DbWeight::get() + .writes(1) + .saturating_mul(LEASES.len() as u64) + .saturating_add(BrokerWeights::request_core_count(62)) + .saturating_add(::DbWeight::get().reads(1)) + } +} + +impl OnRuntimeUpgrade for FixMigration { + fn on_runtime_upgrade() -> Weight { + if PotentialRenewals::::get(PotentialRenewalId { core: 6, when: 292605 }).is_none() + { + // Idempotency check - this core will never be renewable at this timeslice ever again. + log::error!(target: TARGET, "This migration includes hardcoded values not relevant to this runtime. Bailing."); + return ::DbWeight::get().reads(1); + } + + let Some(mut first_core) = Self::get_first_usable_core() else { + log::error!(target: TARGET, "Retrieving `SaleInfo` (first_core) failed!"); + // Return dummy weight. This should really not happen and if it does we have bigger + // problems than wrong weights. + return ::DbWeight::get() + .writes(50) + }; + + Self::add_pool_core(&mut first_core); + Self::extend_short_leases(); + Self::remove_premature_renewals_add_back_leases(); + Self::give_dropped_leases_renewal_rights_and_workplan_entry(&mut first_core); + + // TODO finalise the weights here. ::DbWeight::get() .writes(1) From d98f0e65fe8ecdac58f8bbc9d90fc289a23594c8 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 12:43:34 +0200 Subject: [PATCH 10/25] fmt --- .../coretime-polkadot/src/migrations.rs | 331 +++++++++--------- 1 file changed, 169 insertions(+), 162 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 80a43a8f38..b6eccdd2a4 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -25,12 +25,16 @@ extern crate alloc; use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ - CompletionStatus, CoreAssignment::{self, Pool}, CoreIndex, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, PotentialRenewalId, PotentialRenewals, SalePerformance, Schedule, ScheduleItem, TaskId, Timeslice, WeightInfo, Workplan, AdaptPrice, + AdaptPrice, CompletionStatus, + CoreAssignment::{self, Pool}, + CoreIndex, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, PotentialRenewalId, + PotentialRenewals, SalePerformance, Schedule, ScheduleItem, TaskId, Timeslice, WeightInfo, + Workplan, }; -use sp_std::vec::Vec; use alloc::collections::btree_map::BTreeMap; use sp_arithmetic::traits::Saturating; +use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf}; @@ -47,152 +51,157 @@ type BrokerWeights = weights::pallet_broker::WeightInfo; pub struct FixMigration; impl FixMigration { + fn get_first_usable_core() -> Option { + let sale_info = SaleInfo::::get()?; + // Cores between first_core and cores_offered are for sale and my not be used anymore: + Some(sale_info.first_core + sale_info.cores_offered) + } + + fn add_pool_core(first_core: &mut CoreIndex) { + let Some(sale_info) = SaleInfo::::get() else { + log::error!(target: TARGET, "Retrieving `SaleInfo` failed!"); + return + }; + + let schedule = BoundedVec::truncate_from(Vec::from([ScheduleItem { + mask: CoreMask::complete(), + assignment: CoreAssignment::Pool, + }])); + let result = + pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule.clone()); + debug_assert!(result.is_ok()); + // But we want it now(ish): add it to the workplan. + Workplan::::insert((sale_info.region_begin, *first_core), schedule); + first_core.saturating_inc(); + } + + fn extend_short_leases() { + let short_leases: BTreeMap = [ + (2035, 359280), + (3344, 344160), + (3370, 389520), + (3367, 374400), + (2086, 389520), + (2032, 359280), + (2051, 389520), + (3369, 313920), + (2008, 389520), + (2025, 359280), + (2000, 313920), + (2092, 404640), + (2002, 359280), + (3359, 389520), + (2030, 374400), + (3378, 404640), + (2104, 404640), + (2046, 374400), + (3345, 344160), + (3340, 344160), + (3338, 329040), + (2004, 329040), + (3377, 389520), + (3373, 389520), + (2031, 344160), + (3389, 404640), + (3366, 374400), + (2037, 374400), + (2034, 359280), + (2090, 404640), + (3346, 344160), + (2012, 359280), + (3397, 404640), + (2043, 374400), + (2091, 404640), + (2026, 344160), + (3388, 404640), + (3354, 359280), + (2006, 344160), + (2013, 374400), + ] + .into(); + + let leases = Leases::::get(); + let leases = BoundedVec::truncate_from( + leases + .into_iter() + .map(|mut l| { + let Some(new_until) = short_leases.get(&l.task) else { return l }; + debug_assert!(*new_until > l.until); + l.until = *new_until; + l + }) + .collect(), + ); + Leases::::put(leases); + } - fn get_first_usable_core() -> Option { - let sale_info = SaleInfo::::get()?; - // Cores between first_core and cores_offered are for sale and my not be used anymore: - Some(sale_info.first_core + sale_info.cores_offered) - } - - fn add_pool_core(first_core: &mut CoreIndex) { - let Some(sale_info) = SaleInfo::::get() else { - log::error!(target: TARGET, "Retrieving `SaleInfo` failed!"); - return - }; - - let schedule = BoundedVec::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), assignment: CoreAssignment::Pool}])); - let result = pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule.clone()); - debug_assert!(result.is_ok()); - // But we want it now(ish): add it to the workplan. - Workplan::::insert((sale_info.region_begin, *first_core), schedule); - first_core.saturating_inc(); - } - - fn extend_short_leases() { - let short_leases: BTreeMap = [ - (2035, 359280), - (3344, 344160), - (3370, 389520), - (3367, 374400), - (2086, 389520), - (2032, 359280), - (2051, 389520), - (3369, 313920), - (2008, 389520), - (2025, 359280), - (2000, 313920), - (2092, 404640), - (2002, 359280), - (3359, 389520), - (2030, 374400), - (3378, 404640), - (2104, 404640), - (2046, 374400), - (3345, 344160), - (3340, 344160), - (3338, 329040), - (2004, 329040), - (3377, 389520), - (3373, 389520), - (2031, 344160), - (3389, 404640), - (3366, 374400), - (2037, 374400), - (2034, 359280), - (2090, 404640), - (3346, 344160), - (2012, 359280), - (3397, 404640), - (2043, 374400), - (2091, 404640), - (2026, 344160), - (3388, 404640), - (3354, 359280), - (2006, 344160), - (2013, 374400), - ].into(); - - let leases = Leases::::get(); - let leases = BoundedVec::truncate_from(leases.into_iter().map(|mut l| { - let Some(new_until) = short_leases.get(&l.task) else { - return l - }; - debug_assert!(*new_until > l.until); - l.until = *new_until; - l - }).collect()); - Leases::::put(leases); - } - - // Undo rotate_sale effect on leases that wrongly expired. - // - // Needs to be called before `give_dropped_leases_renewal_rights_and_workplan_entry`. - fn remove_premature_renewals_add_back_leases() { - let premature_renewals = [ - (2094, 298800), - (2040, 298800), - (3333, 298800), - (2106, 298800), - (2093, 298800), - (2101, 298800), - ]; - - debug_assert_eq!(PotentialRenewals::::iter().count(), premature_renewals.len()); - let result = PotentialRenewals::::clear(premature_renewals.len() as u32, None); - debug_assert!(result.maybe_cursor.is_none()); - - for (task, until) in premature_renewals { - let result = pallet_broker::Pallet::::set_lease(RuntimeOrigin::root(), task, until); - debug_assert!(result.is_ok()); - } - } - - fn give_dropped_leases_renewal_rights_and_workplan_entry(first_core: &mut CoreIndex) { - let Some(sale_info) = SaleInfo::::get() else { - log::error!(target: TARGET, "Retrieving `SaleInfo` failed!"); - return - }; - - let dropped_leases = [ - (2048, 283680), - (3375, 283680), - (3358, 283680), - (2053, 283680), - (2056, 283680), - ]; - - // Leases should have been added, but removed again by rotate_sale - replaces with work - // plan items + renewal rights. - // https://github.com/paritytech/polkadot-sdk/blob/f170af615c0dc413482100892758b236d1fda93b/substrate/frame/broker/src/tick_impls.rs#L212 - // So we need to: - // - Don't change leases, already correct. - // - Add work plan entry. - // - add to potential renewals - - for (task, _) in dropped_leases { - // Workplan - let mask = CoreMask::complete(); - let assignment = CoreAssignment::Task(task); - // DONE: enough to start at next rotation? - // - Yes all good, assignments are all for next rotation already. Hence relay chain - // state is persisted for now. - let schedule = BoundedVec::truncate_from(Vec::from([ScheduleItem { mask, assignment }])); - Workplan::::insert((sale_info.region_begin, *first_core), schedule.clone()); - - // Renewal: - let new_prices = ::PriceAdapter::adapt_price(SalePerformance::from_sale(&sale_info)); - debug_assert_eq!(new_prices.target_price, 100*UNITS); - let renewal_id = PotentialRenewalId { core: *first_core, when: sale_info.region_end }; - let record = PotentialRenewalRecord { - price: new_prices.target_price, - completion: CompletionStatus::Complete(schedule), - }; - PotentialRenewals::::insert(renewal_id, &record); - - first_core.saturating_inc() - } - - } + // Undo rotate_sale effect on leases that wrongly expired. + // + // Needs to be called before `give_dropped_leases_renewal_rights_and_workplan_entry`. + fn remove_premature_renewals_add_back_leases() { + let premature_renewals = [ + (2094, 298800), + (2040, 298800), + (3333, 298800), + (2106, 298800), + (2093, 298800), + (2101, 298800), + ]; + + debug_assert_eq!(PotentialRenewals::::iter().count(), premature_renewals.len()); + let result = PotentialRenewals::::clear(premature_renewals.len() as u32, None); + debug_assert!(result.maybe_cursor.is_none()); + + for (task, until) in premature_renewals { + let result = + pallet_broker::Pallet::::set_lease(RuntimeOrigin::root(), task, until); + debug_assert!(result.is_ok()); + } + } + + fn give_dropped_leases_renewal_rights_and_workplan_entry(first_core: &mut CoreIndex) { + let Some(sale_info) = SaleInfo::::get() else { + log::error!(target: TARGET, "Retrieving `SaleInfo` failed!"); + return + }; + + let dropped_leases = + [(2048, 283680), (3375, 283680), (3358, 283680), (2053, 283680), (2056, 283680)]; + + // Leases should have been added, but removed again by rotate_sale - replaces with work + // plan items + renewal rights. + // https://github.com/paritytech/polkadot-sdk/blob/f170af615c0dc413482100892758b236d1fda93b/substrate/frame/broker/src/tick_impls.rs#L212 + // So we need to: + // - Don't change leases, already correct. + // - Add work plan entry. + // - add to potential renewals + + for (task, _) in dropped_leases { + // Workplan + let mask = CoreMask::complete(); + let assignment = CoreAssignment::Task(task); + // DONE: enough to start at next rotation? + // - Yes all good, assignments are all for next rotation already. Hence relay chain + // state is persisted for now. + let schedule = + BoundedVec::truncate_from(Vec::from([ScheduleItem { mask, assignment }])); + Workplan::::insert((sale_info.region_begin, *first_core), schedule.clone()); + + // Renewal: + let new_prices = ::PriceAdapter::adapt_price( + SalePerformance::from_sale(&sale_info), + ); + debug_assert_eq!(new_prices.target_price, 100 * UNITS); + let renewal_id = PotentialRenewalId { core: *first_core, when: sale_info.region_end }; + let record = PotentialRenewalRecord { + price: new_prices.target_price, + completion: CompletionStatus::Complete(schedule), + }; + PotentialRenewals::::insert(renewal_id, &record); + + first_core.saturating_inc() + } + } fn on_runtime_upgrade_donal() -> Weight { if PotentialRenewals::::get(PotentialRenewalId { core: 6, when: 292605 }).is_none() @@ -276,8 +285,8 @@ impl FixMigration { // Add the system pool core to the workplan starting now. let now = 287000; // TODO - this needs to be after the cores have just added have been processed, which takes - // two sessions. Currently just a placeholder. This part is probably better as a call to - // assign_core on the relay as part of the referendum instead of here. + // two sessions. Currently just a placeholder. This part is probably better as a call to + // assign_core on the relay as part of the referendum instead of here. Workplan::::insert((now, 61), pool_assignment); // TODO finalise the weights here. @@ -298,19 +307,17 @@ impl OnRuntimeUpgrade for FixMigration { return ::DbWeight::get().reads(1); } - let Some(mut first_core) = Self::get_first_usable_core() else { - log::error!(target: TARGET, "Retrieving `SaleInfo` (first_core) failed!"); - // Return dummy weight. This should really not happen and if it does we have bigger - // problems than wrong weights. - return ::DbWeight::get() - .writes(50) - }; - - Self::add_pool_core(&mut first_core); - Self::extend_short_leases(); - Self::remove_premature_renewals_add_back_leases(); - Self::give_dropped_leases_renewal_rights_and_workplan_entry(&mut first_core); - + let Some(mut first_core) = Self::get_first_usable_core() else { + log::error!(target: TARGET, "Retrieving `SaleInfo` (first_core) failed!"); + // Return dummy weight. This should really not happen and if it does we have bigger + // problems than wrong weights. + return ::DbWeight::get().writes(50) + }; + + Self::add_pool_core(&mut first_core); + Self::extend_short_leases(); + Self::remove_premature_renewals_add_back_leases(); + Self::give_dropped_leases_renewal_rights_and_workplan_entry(&mut first_core); // TODO finalise the weights here. ::DbWeight::get() From 0f809df0a33aaedc093be33d5dcacb73dfcdadac Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 16:43:47 +0200 Subject: [PATCH 11/25] Fix tests --- .../coretime-polkadot/src/migrations.rs | 156 +++++------------- 1 file changed, 40 insertions(+), 116 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index b6eccdd2a4..bc9e6d87ea 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -188,13 +188,10 @@ impl FixMigration { Workplan::::insert((sale_info.region_begin, *first_core), schedule.clone()); // Renewal: - let new_prices = ::PriceAdapter::adapt_price( - SalePerformance::from_sale(&sale_info), - ); debug_assert_eq!(new_prices.target_price, 100 * UNITS); let renewal_id = PotentialRenewalId { core: *first_core, when: sale_info.region_end }; let record = PotentialRenewalRecord { - price: new_prices.target_price, + price: 100 * UNITS, completion: CompletionStatus::Complete(schedule), }; PotentialRenewals::::insert(renewal_id, &record); @@ -202,100 +199,7 @@ impl FixMigration { first_core.saturating_inc() } } - - fn on_runtime_upgrade_donal() -> Weight { - if PotentialRenewals::::get(PotentialRenewalId { core: 6, when: 292605 }).is_none() - { - // Idempotency check - this core will never be renewable at this timeslice ever again. - log::error!(target: TARGET, "This migration includes hardcoded values not relevant to this runtime. Bailing."); - return ::DbWeight::get().reads(1); } - - // Add leases for 2040, 2094, 3333, 2106, 2101, 2093 with a properly calculated end - // timeslice. Add 11520 for all other leases. - let leases: LeasesRecordOf = LEASES - .iter() - .map(|(until, task)| LeaseRecordItem { until: *until, task: *task }) - .collect::>() - .try_into() - .expect("Within range of bounded vec"); - Leases::::put(leases); - - // This reorders the cores, so the existing entries to the workplan need to be overwritten. - for (&(para_id, _), core_id) in LEASES.iter().zip(51u16..56u16) { - // Add to the workplan at timeslice 287565 using the new cores. - let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { - mask: CoreMask::complete(), - assignment: CoreAssignment::Task(para_id), - }])); - - Workplan::::insert((287565, core_id), workplan_entry); - } - - // Remove the existing 6 PotentialRenewals. - for &(core, when) in INCORRECT_RENEWAL_IDS.iter() { - PotentialRenewals::::remove(PotentialRenewalId { core, when }); - } - - // Sort the parachains who can renew. They are currently missing from the broker - // state entirely. - // TODO double check the core ids, these should be on top of the last available in the sale. - for (&(para_id, _), core_id) in POTENTIAL_RENEWALS.iter().zip(56u16..61u16) { - // Add to the workplan at timeslice 287565 using the new cores. - let workplan_entry = Schedule::truncate_from(Vec::from([ScheduleItem { - mask: CoreMask::complete(), - assignment: CoreAssignment::Task(para_id), - }])); - - Workplan::::insert((287565, core_id), workplan_entry); - } - - // Add cores to the system - this will take 2 sessions to kick in. - // 1 core for the system on-demand pool, 5 for open market, 5 for reservations and 51 - // parachains, of which 46 have leases and 5 are up for renewal. - let core_count = pallet_broker::Reservations::::decode_len().unwrap_or(0) as u16 + - pallet_broker::Leases::::decode_len().unwrap_or(0) as u16 + - 5 + 6; - - match pallet_broker::Pallet::::request_core_count( - RuntimeOrigin::root(), - core_count, - ) { - Ok(_) => log::info!(target: TARGET, "Request for 62 cores sent."), - Err(_) => log::error!(target: TARGET, "Request for 62 cores failed to send."), - } - - let pool_assignment = Schedule::truncate_from(Vec::from([ScheduleItem { - mask: CoreMask::complete(), - assignment: Pool, - }])); - - // Reserve the system pool core - this kicks in after two sale period boundaries. - match pallet_broker::Pallet::::reserve( - RuntimeOrigin::root(), - pool_assignment.clone(), - ) { - Ok(_) => log::info!(target: TARGET, "Pool core reserved."), - Err(_) => log::error!(target: TARGET, "Pool core reservation failed."), - } - - // Add the system pool core to the workplan for the next cycle (287565) on the last new core - // (core 61) - Workplan::::insert((292605, 61), pool_assignment.clone()); - - // Add the system pool core to the workplan starting now. - let now = 287000; // TODO - this needs to be after the cores have just added have been processed, which takes - // two sessions. Currently just a placeholder. This part is probably better as a call to - // assign_core on the relay as part of the referendum instead of here. - Workplan::::insert((now, 61), pool_assignment); - - // TODO finalise the weights here. - ::DbWeight::get() - .writes(1) - .saturating_mul(LEASES.len() as u64) - .saturating_add(BrokerWeights::request_core_count(62)) - .saturating_add(::DbWeight::get().reads(1)) - } } impl OnRuntimeUpgrade for FixMigration { @@ -314,11 +218,18 @@ impl OnRuntimeUpgrade for FixMigration { return ::DbWeight::get().writes(50) }; - Self::add_pool_core(&mut first_core); Self::extend_short_leases(); Self::remove_premature_renewals_add_back_leases(); Self::give_dropped_leases_renewal_rights_and_workplan_entry(&mut first_core); + Self::add_pool_core(&mut first_core); + match pallet_broker::Pallet::::request_core_count( + RuntimeOrigin::root(), + first_core, + ) { + Ok(_) => log::info!(target: TARGET, "Request for 62 cores sent."), + Err(_) => log::error!(target: TARGET, "Request for 62 cores failed to send."), + } // TODO finalise the weights here. ::DbWeight::get() .writes(1) @@ -385,7 +296,11 @@ impl OnRuntimeUpgrade for FixMigration { // Add the system parachains as an offset - these should come before the leases. let core_id = i as u16 + 5; - assert!(leases_vec.contains(&(*until, *para_id))); + assert!( + leases_vec.contains(&(*para_id, *until)), + "Lease entry not found: {:?}", + *para_id + ); // This is the entry found in Workplan let workload = Schedule::truncate_from(Vec::from([ScheduleItem { @@ -394,14 +309,18 @@ impl OnRuntimeUpgrade for FixMigration { }])); // They should all be in the workplan for next region. - assert_eq!(Workplan::::get((workplan_start, core_id)), Some(workload)); + assert_eq!( + Workplan::::get((workplan_start, core_id)), + Some(workload), + "Workplan entry not found/invalid." + ); } // For the leases we had before their lease should extend for an additional 11520 // timeslices (64 days). for LeaseRecordItem { task, until } in prev_leases.iter() { log::error!("{task}, {until}"); - assert!(leases_vec.contains(&(*until + 11520, *task))) + assert!(leases_vec.contains(&(*task, *until + 11520))) } // Iterate through hardcoded potential renewals and check they're all correctly in state @@ -421,15 +340,19 @@ impl OnRuntimeUpgrade for FixMigration { assert!(!leases.contains(&LeaseRecordItem { until: *until, task: *para_id })); // Ensure they can renew in the next region. + let renewal_entry = PotentialRenewals::::get(PotentialRenewalId { + core: core_id, + // TODO: Where are these 5040 coming from?! + // when: sale_info.region_end + 5040, + when: sale_info.region_end, + }) + .unwrap(); assert_eq!( - PotentialRenewals::::get(PotentialRenewalId { - core: core_id, - when: sale_info.region_end + 5040, - }), - Some(PotentialRenewalRecord { - price: 1_000_000_000_000, + renewal_entry, + PotentialRenewalRecord { + price: 100 * UNITS, completion: pallet_broker::CompletionStatus::Complete(workload.clone()) - }) + } ); // They should all be in the workplan for next sale. @@ -437,7 +360,11 @@ impl OnRuntimeUpgrade for FixMigration { } // Walk the workplan at timeslice 287565 and make sure there is an entry for every 62 cores. - for i in 0..61 { + for i in 0..62 { + if i >= 51 && i < 56 { + // Cores offered for sale, we don't know anything about them for sure. + continue; + } let entry = Workplan::::get((287565, i)).expect("Entry should exist"); assert_eq!(entry.len(), 1); assert_eq!(entry.get(0).unwrap().mask, CoreMask::complete()); @@ -450,18 +377,15 @@ impl OnRuntimeUpgrade for FixMigration { entry.get(0).unwrap().assignment, Task(LEASES.get(i as usize - 5).unwrap().0) ); - } else if i < 56 { - // 5 new cores - assert_eq!( - entry.get(0).unwrap().assignment, - Task(LEASES.get(i as usize - 51).unwrap().0) - ); - } else { + } else if i <= 60 { // 5 potential renewals assert_eq!( entry.get(0).unwrap().assignment, Task(POTENTIAL_RENEWALS.get(i as usize - 56).unwrap().0) ); + } else { + // Pool core: + assert_eq!(entry.get(0).unwrap().assignment, Pool,); } } From 3349c1582875c92b43f8e2d2274d18f00f57b73c Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 17:03:33 +0200 Subject: [PATCH 12/25] Add useful traces --- .../coretime/coretime-polkadot/src/migrations.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index bc9e6d87ea..5efceab7c6 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -272,6 +272,7 @@ impl OnRuntimeUpgrade for FixMigration { let system_chains = [Task(1001), Task(1002), Task(1000), Task(1004), Task(1005)]; // Check the reservations are still in the workplan out of an abundance of caution. + log::trace!(target: TARGET, "Checking system chains"); for (core_id, task) in system_chains.iter().enumerate() { assert_eq!( Workplan::::get((workplan_start, core_id as u16)), @@ -283,6 +284,7 @@ impl OnRuntimeUpgrade for FixMigration { } // Make sure we've got all the leases. + log::trace!(target: TARGET, "Checking leases"); let leases = Leases::::get(); assert_eq!(leases.len(), LEASES.iter().filter(|(_, l)| sale_info.region_end <= *l).count()); @@ -292,10 +294,12 @@ impl OnRuntimeUpgrade for FixMigration { // Iterate through hardcoded leases and check they're all correctly in state and scheduled // in the workplan. + log::trace!(target: TARGET, "Checking leases more thoroughly"); for (i, (para_id, until)) in LEASES.iter().enumerate() { // Add the system parachains as an offset - these should come before the leases. let core_id = i as u16 + 5; + log::trace!(target: TARGET, "Checking lease existence, core: {:?}, lease: {:?}", core_id, para_id); assert!( leases_vec.contains(&(*para_id, *until)), "Lease entry not found: {:?}", @@ -309,6 +313,7 @@ impl OnRuntimeUpgrade for FixMigration { }])); // They should all be in the workplan for next region. + log::trace!(target: TARGET, "Checking workplan existence"); assert_eq!( Workplan::::get((workplan_start, core_id)), Some(workload), @@ -318,6 +323,7 @@ impl OnRuntimeUpgrade for FixMigration { // For the leases we had before their lease should extend for an additional 11520 // timeslices (64 days). + log::trace!(target: TARGET, "Checking lease durations"); for LeaseRecordItem { task, until } in prev_leases.iter() { log::error!("{task}, {until}"); assert!(leases_vec.contains(&(*task, *until + 11520))) @@ -325,6 +331,7 @@ impl OnRuntimeUpgrade for FixMigration { // Iterate through hardcoded potential renewals and check they're all correctly in state // and scheduled in the workplan. + log::trace!(target: TARGET, "Checking potential renewals"); for (i, (para_id, until)) in POTENTIAL_RENEWALS.iter().enumerate() { // Add the system parachains and leases as an offset. // system chains + leases + new cores = 5 + 46 + 5 = 56. @@ -337,9 +344,11 @@ impl OnRuntimeUpgrade for FixMigration { }])); // Make sure they're not in the leases. + log::trace!(target: TARGET, "Potential renewal not in leases?"); assert!(!leases.contains(&LeaseRecordItem { until: *until, task: *para_id })); // Ensure they can renew in the next region. + log::trace!(target: TARGET, "Potential renewal exists as expected? core_id: {:?}, when: {:?}, para: {:?}", core_id, sale_info.region_end, para_id); let renewal_entry = PotentialRenewals::::get(PotentialRenewalId { core: core_id, // TODO: Where are these 5040 coming from?! @@ -347,6 +356,7 @@ impl OnRuntimeUpgrade for FixMigration { when: sale_info.region_end, }) .unwrap(); + log::trace!(target: TARGET, "Renewal entry found: {:?} {:?}", renewal_entry.price / UNITS, renewal_entry.completion); assert_eq!( renewal_entry, PotentialRenewalRecord { @@ -356,18 +366,23 @@ impl OnRuntimeUpgrade for FixMigration { ); // They should all be in the workplan for next sale. + log::trace!(target: TARGET, "Workplan entry exists as expected?"); assert_eq!(Workplan::::get((workplan_start, core_id)), Some(workload)); } // Walk the workplan at timeslice 287565 and make sure there is an entry for every 62 cores. + log::trace!(target: TARGET, "Checking workplan"); for i in 0..62 { if i >= 51 && i < 56 { // Cores offered for sale, we don't know anything about them for sure. continue; } + log::trace!(target: TARGET, "Core: {:?}", i); let entry = Workplan::::get((287565, i)).expect("Entry should exist"); + log::trace!(target: TARGET, "Found entry"); assert_eq!(entry.len(), 1); assert_eq!(entry.get(0).unwrap().mask, CoreMask::complete()); + log::trace!(target: TARGET, "Entry complete"); if i < 5 { // system chains assert_eq!(entry.get(0).unwrap().assignment, system_chains[i as usize]); @@ -390,6 +405,7 @@ impl OnRuntimeUpgrade for FixMigration { } // Ensure we have requested the correct number of cores. + log::trace!(target: TARGET, "Checking requested core count"); assert!(frame_system::Pallet::::read_events_no_consensus().any(|e| { match e.event { crate::RuntimeEvent::Broker( From aaf3fe08e7fe6bfaed69da3d29004caffdd33ef6 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 17:38:07 +0200 Subject: [PATCH 13/25] Add missing tests: - PotentialRenewals size - pool core - actual reservations --- .../coretime-polkadot/src/migrations.rs | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 5efceab7c6..f2959799d7 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -126,7 +126,6 @@ impl FixMigration { .into_iter() .map(|mut l| { let Some(new_until) = short_leases.get(&l.task) else { return l }; - debug_assert!(*new_until > l.until); l.until = *new_until; l }) @@ -148,7 +147,6 @@ impl FixMigration { (2101, 298800), ]; - debug_assert_eq!(PotentialRenewals::::iter().count(), premature_renewals.len()); let result = PotentialRenewals::::clear(premature_renewals.len() as u32, None); debug_assert!(result.maybe_cursor.is_none()); @@ -188,7 +186,6 @@ impl FixMigration { Workplan::::insert((sale_info.region_begin, *first_core), schedule.clone()); // Renewal: - debug_assert_eq!(new_prices.target_price, 100 * UNITS); let renewal_id = PotentialRenewalId { core: *first_core, when: sale_info.region_end }; let record = PotentialRenewalRecord { price: 100 * UNITS, @@ -269,11 +266,24 @@ impl OnRuntimeUpgrade for FixMigration { // The workplan entries start from the region begin reported by the new SaleInfo. let workplan_start = sale_info.region_begin; - let system_chains = [Task(1001), Task(1002), Task(1000), Task(1004), Task(1005)]; + let system_chains = [Task(1001), Task(1002), Task(1000), Task(1004), Task(1005), Pool]; + let reservations = Reservations::::get(); + assert_eq!(reservations.len(), system_chains.len()); // Check the reservations are still in the workplan out of an abundance of caution. log::trace!(target: TARGET, "Checking system chains"); - for (core_id, task) in system_chains.iter().enumerate() { + for (i, (task, reservation)) in + system_chains.iter().zip(reservations.into_iter()).enumerate() + { + log::trace!(target: TARGET, "Checking system chain {:?}", i); + let core_id = if i < 5 { + i + } else { + // 51 ... first core for sale + // 5 ... five cores for sale + // 5 ... the dropped leases + 51 + 5 + 5 + }; assert_eq!( Workplan::::get((workplan_start, core_id as u16)), Some(Schedule::truncate_from(Vec::from([ScheduleItem { @@ -281,6 +291,10 @@ impl OnRuntimeUpgrade for FixMigration { assignment: task.clone(), }]))) ); + + assert_eq!(reservation.len(), 1); + let reservation = reservation.into_iter().next().unwrap(); + assert_eq!(reservation.assignment, *task,); } // Make sure we've got all the leases. @@ -364,6 +378,7 @@ impl OnRuntimeUpgrade for FixMigration { completion: pallet_broker::CompletionStatus::Complete(workload.clone()) } ); + assert_eq!(PotentialRenewals::::iter().count(), POTENTIAL_RENEWALS.len()); // They should all be in the workplan for next sale. log::trace!(target: TARGET, "Workplan entry exists as expected?"); From b5a58d0be0aca2da41effd8019149ed15f2490fd Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 17:39:26 +0200 Subject: [PATCH 14/25] Fix imports --- .../coretime/coretime-polkadot/src/migrations.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index f2959799d7..c1e6430e89 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -25,10 +25,10 @@ extern crate alloc; use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ - AdaptPrice, CompletionStatus, + CompletionStatus, CoreAssignment::{self, Pool}, CoreIndex, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, PotentialRenewalId, - PotentialRenewals, SalePerformance, Schedule, ScheduleItem, TaskId, Timeslice, WeightInfo, + PotentialRenewals, Reservations, Schedule, ScheduleItem, TaskId, Timeslice, WeightInfo, Workplan, }; From 641e72f5687efa88a31facaa8c2682ec1a950f69 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 17:40:25 +0200 Subject: [PATCH 15/25] Replace remaining useless debug_asserts --- .../coretime-polkadot/src/migrations.rs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index c1e6430e89..a3a2972366 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -67,9 +67,11 @@ impl FixMigration { mask: CoreMask::complete(), assignment: CoreAssignment::Pool, }])); - let result = - pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule.clone()); - debug_assert!(result.is_ok()); + if let Err(err) = + pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule.clone()) + { + log::error!(target: TARGET, "Adding pool reservation failed: {:?}", err); + } // But we want it now(ish): add it to the workplan. Workplan::::insert((sale_info.region_begin, *first_core), schedule); first_core.saturating_inc(); @@ -147,13 +149,19 @@ impl FixMigration { (2101, 298800), ]; - let result = PotentialRenewals::::clear(premature_renewals.len() as u32, None); - debug_assert!(result.maybe_cursor.is_none()); + if PotentialRenewals::::clear(premature_renewals.len() as u32, None) + .maybe_cursor + .is_some() + { + log::error!(target: TARGET, "Unexpected cursor from clear!"); + } for (task, until) in premature_renewals { - let result = - pallet_broker::Pallet::::set_lease(RuntimeOrigin::root(), task, until); - debug_assert!(result.is_ok()); + if let Err(err) = + pallet_broker::Pallet::::set_lease(RuntimeOrigin::root(), task, until) + { + log::error!(target: TARGET, "Setting lease failed: {:?}", err); + } } } @@ -196,7 +204,6 @@ impl FixMigration { first_core.saturating_inc() } } - } } impl OnRuntimeUpgrade for FixMigration { From ac10dc3728ff788d7c2d9152307abc16d29f8706 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 17:41:05 +0200 Subject: [PATCH 16/25] New lines --- system-parachains/coretime/coretime-polkadot/src/migrations.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index a3a2972366..43c77bba97 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -234,6 +234,7 @@ impl OnRuntimeUpgrade for FixMigration { Ok(_) => log::info!(target: TARGET, "Request for 62 cores sent."), Err(_) => log::error!(target: TARGET, "Request for 62 cores failed to send."), } + // TODO finalise the weights here. ::DbWeight::get() .writes(1) @@ -277,6 +278,7 @@ impl OnRuntimeUpgrade for FixMigration { let reservations = Reservations::::get(); assert_eq!(reservations.len(), system_chains.len()); + // Check the reservations are still in the workplan out of an abundance of caution. log::trace!(target: TARGET, "Checking system chains"); for (i, (task, reservation)) in @@ -400,6 +402,7 @@ impl OnRuntimeUpgrade for FixMigration { continue; } log::trace!(target: TARGET, "Core: {:?}", i); + let entry = Workplan::::get((287565, i)).expect("Entry should exist"); log::trace!(target: TARGET, "Found entry"); assert_eq!(entry.len(), 1); From 41b53cb7485fd760d2ab5a2942bf4158407e3295 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 19:40:35 +0200 Subject: [PATCH 17/25] More checks --- Cargo.lock | 1 + .../coretime/coretime-polkadot/src/migrations.rs | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index a34e028c5d..9009eb75c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2806,6 +2806,7 @@ dependencies = [ "serde", "serde_json", "sp-api", + "sp-arithmetic 26.0.0", "sp-block-builder", "sp-consensus-aura", "sp-core 34.0.0", diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 43c77bba97..0baf278e7f 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -149,6 +149,10 @@ impl FixMigration { (2101, 298800), ]; + if PotentialRenewals::::iter().count() != premature_renewals.len() { + log::error!(target: TARGET, "Unexpected renewal state!"); + } + if PotentialRenewals::::clear(premature_renewals.len() as u32, None) .maybe_cursor .is_some() @@ -253,6 +257,9 @@ impl OnRuntimeUpgrade for FixMigration { let sale_info = SaleInfo::::get().unwrap(); let leases = Leases::::get(); let pre_upgrade_state = (sale_info, leases); + + assert_eq!(PotentialRenewals::::iter().count(), INCORRECT_RENEWAL_IDS.len()); + Ok(pre_upgrade_state.encode()) } From e697de819859faa47a8ed8f5ac256c3998b82c19 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 20:04:03 +0200 Subject: [PATCH 18/25] Cleanup --- .../coretime/coretime-polkadot/src/migrations.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 0baf278e7f..82af59b2a6 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -381,8 +381,6 @@ impl OnRuntimeUpgrade for FixMigration { log::trace!(target: TARGET, "Potential renewal exists as expected? core_id: {:?}, when: {:?}, para: {:?}", core_id, sale_info.region_end, para_id); let renewal_entry = PotentialRenewals::::get(PotentialRenewalId { core: core_id, - // TODO: Where are these 5040 coming from?! - // when: sale_info.region_end + 5040, when: sale_info.region_end, }) .unwrap(); @@ -410,7 +408,7 @@ impl OnRuntimeUpgrade for FixMigration { } log::trace!(target: TARGET, "Core: {:?}", i); - let entry = Workplan::::get((287565, i)).expect("Entry should exist"); + let entry = Workplan::::get((workplan_start, i)).expect("Entry should exist"); log::trace!(target: TARGET, "Found entry"); assert_eq!(entry.len(), 1); assert_eq!(entry.get(0).unwrap().mask, CoreMask::complete()); From 74e4efcfdba307c3ff63a773da1132c6078d44ab Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 21:28:07 +0200 Subject: [PATCH 19/25] Fixes, cleanup --- .../coretime-polkadot/src/migrations.rs | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 82af59b2a6..51f84aa317 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -25,11 +25,9 @@ extern crate alloc; use crate::{weights, Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ - CompletionStatus, - CoreAssignment::{self, Pool}, - CoreIndex, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf, PotentialRenewalId, - PotentialRenewals, Reservations, Schedule, ScheduleItem, TaskId, Timeslice, WeightInfo, - Workplan, + CompletionStatus, CoreAssignment, CoreIndex, CoreMask, Leases, PotentialRenewalId, + PotentialRenewalRecord, PotentialRenewals, SaleInfo, ScheduleItem, TaskId, Timeslice, + WeightInfo, Workplan, }; use alloc::collections::btree_map::BTreeMap; @@ -37,7 +35,10 @@ use sp_arithmetic::traits::Saturating; use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] -use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf}; +use pallet_broker::{ + CoreAssignment::{Pool, Task}, + LeaseRecordItem, LeasesRecordOf, Reservations, SaleInfoRecordOf, Schedule, +}; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; use system_parachains_constants::polkadot::currency::UNITS; @@ -53,7 +54,7 @@ pub struct FixMigration; impl FixMigration { fn get_first_usable_core() -> Option { let sale_info = SaleInfo::::get()?; - // Cores between first_core and cores_offered are for sale and my not be used anymore: + // Cores between first_core and cores_offered are for sale and are thus not usable. Some(sale_info.first_core + sale_info.cores_offered) } @@ -63,18 +64,19 @@ impl FixMigration { return }; + // Workplan add (speed up by one cycle): let schedule = BoundedVec::truncate_from(Vec::from([ScheduleItem { mask: CoreMask::complete(), assignment: CoreAssignment::Pool, }])); - if let Err(err) = - pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule.clone()) + Workplan::::insert((sale_info.region_begin, *first_core), schedule.clone()); + first_core.saturating_inc(); + + // And reserve: + if let Err(err) = pallet_broker::Pallet::::reserve(RuntimeOrigin::root(), schedule) { log::error!(target: TARGET, "Adding pool reservation failed: {:?}", err); } - // But we want it now(ish): add it to the workplan. - Workplan::::insert((sale_info.region_begin, *first_core), schedule); - first_core.saturating_inc(); } fn extend_short_leases() { @@ -242,7 +244,6 @@ impl OnRuntimeUpgrade for FixMigration { // TODO finalise the weights here. ::DbWeight::get() .writes(1) - .saturating_mul(LEASES.len() as u64) .saturating_add(BrokerWeights::request_core_count(62)) .saturating_add(::DbWeight::get().reads(1)) } @@ -310,7 +311,7 @@ impl OnRuntimeUpgrade for FixMigration { assert_eq!(reservation.len(), 1); let reservation = reservation.into_iter().next().unwrap(); - assert_eq!(reservation.assignment, *task,); + assert_eq!(reservation.assignment, *task); } // Make sure we've got all the leases. @@ -399,11 +400,11 @@ impl OnRuntimeUpgrade for FixMigration { assert_eq!(Workplan::::get((workplan_start, core_id)), Some(workload)); } - // Walk the workplan at timeslice 287565 and make sure there is an entry for every 62 cores. + // Walk the workplan at timeslice 287565 and make sure there is an entry for all 62 cores. log::trace!(target: TARGET, "Checking workplan"); for i in 0..62 { if i >= 51 && i < 56 { - // Cores offered for sale, we don't know anything about them for sure. + // Cores offered for sale, we don't know anything about them. continue; } log::trace!(target: TARGET, "Core: {:?}", i); @@ -434,6 +435,17 @@ impl OnRuntimeUpgrade for FixMigration { } } + log::info!(target: TARGET, "Workplan entries: {:?}", Workplan::::iter().count()); + log::debug!(target: TARGET, "Complete workplan:"); + let workplan_by_core: BTreeMap<_, _> = Workplan::::iter() + .map(|((timeslice, core_index), schedule)| { + (core_index, (schedule.into_iter().next().unwrap().assignment, timeslice)) + }) + .collect(); + for (core_index, (task, timeslice)) in workplan_by_core { + log::debug!(target: TARGET, "{:?}, {:?}, {:?}", core_index, task, timeslice); + } + // Ensure we have requested the correct number of cores. log::trace!(target: TARGET, "Checking requested core count"); assert!(frame_system::Pallet::::read_events_no_consensus().any(|e| { @@ -454,14 +466,18 @@ impl OnRuntimeUpgrade for FixMigration { } // Incorrect potential renewals in state + +#[cfg(feature = "try-runtime")] const INCORRECT_RENEWAL_IDS: [(u16, u32); 6] = [(6, 292605), (5, 292605), (13, 292605), (15, 292605), (47, 292605), (44, 292605)]; // Hardcoded para ids and their end timeslice. // Taken from https://github.com/SBalaguer/coretime-migration/blob/master/polkadot-output-200924.json +#[cfg(feature = "try-runtime")] const POTENTIAL_RENEWALS: [(u32, u32); 5] = [(2048, 283680), (3375, 283680), (3358, 283680), (2053, 283680), (2056, 283680)]; +#[cfg(feature = "try-runtime")] const LEASES: [(u32, u32); 46] = [ (2094, 298800), (2040, 298800), From 91076d7a619300166eb17b1fb16f2b58c12e0508 Mon Sep 17 00:00:00 2001 From: eskimor Date: Sun, 29 Sep 2024 21:51:18 +0200 Subject: [PATCH 20/25] Fix feature propagation --- system-parachains/coretime/coretime-polkadot/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/system-parachains/coretime/coretime-polkadot/Cargo.toml b/system-parachains/coretime/coretime-polkadot/Cargo.toml index 669f50daac..91ef384c04 100644 --- a/system-parachains/coretime/coretime-polkadot/Cargo.toml +++ b/system-parachains/coretime/coretime-polkadot/Cargo.toml @@ -132,6 +132,7 @@ std = [ "serde", "serde_json/std", "sp-api/std", + "sp-arithmetic/std", "sp-block-builder/std", "sp-consensus-aura/std", "sp-core/std", From 5b6d997fd4889b1cc1d9d3b600aeefafee87f182 Mon Sep 17 00:00:00 2001 From: eskimor Date: Mon, 30 Sep 2024 09:03:11 +0200 Subject: [PATCH 21/25] Add weights --- .../coretime-polkadot/src/migrations.rs | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/system-parachains/coretime/coretime-polkadot/src/migrations.rs b/system-parachains/coretime/coretime-polkadot/src/migrations.rs index 51f84aa317..5e06724cac 100644 --- a/system-parachains/coretime/coretime-polkadot/src/migrations.rs +++ b/system-parachains/coretime/coretime-polkadot/src/migrations.rs @@ -22,7 +22,7 @@ extern crate alloc; -use crate::{weights, Runtime, RuntimeOrigin}; +use crate::{Runtime, RuntimeOrigin}; use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; use pallet_broker::{ CompletionStatus, CoreAssignment, CoreIndex, CoreMask, Leases, PotentialRenewalId, @@ -46,8 +46,37 @@ use system_parachains_constants::polkadot::currency::UNITS; /// The log target. const TARGET: &str = "runtime::bootstrapping::fix-migration"; -// Alias into the broker weights for this runtime. -type BrokerWeights = weights::pallet_broker::WeightInfo; +struct Weights; +impl Weights { + fn get_first_usable_core() -> Weight { + ::DbWeight::get().reads(1) + } + + fn add_pool_core() -> Weight { + ::DbWeight::get() + .reads_writes(1, 1) + .saturating_add(::WeightInfo::reserve()) + } + + fn extend_short_leases() -> Weight { + ::DbWeight::get().reads_writes(1, 1) + } + + fn remove_premature_renewals_add_back_leases() -> Weight { + let removed_renewals = INCORRECT_RENEWAL_IDS.len() as u64; + ::DbWeight::get() + .reads_writes(removed_renewals, removed_renewals) + .saturating_add( + ::WeightInfo::set_lease() + .saturating_mul(removed_renewals), + ) + } + + fn give_dropped_leases_renewal_rights_and_workplan_entry() -> Weight { + let added_renewals = POTENTIAL_RENEWALS.len() as u64; + ::DbWeight::get().reads_writes(1, 2 * added_renewals) + } +} pub struct FixMigration; @@ -241,11 +270,12 @@ impl OnRuntimeUpgrade for FixMigration { Err(_) => log::error!(target: TARGET, "Request for 62 cores failed to send."), } - // TODO finalise the weights here. - ::DbWeight::get() - .writes(1) - .saturating_add(BrokerWeights::request_core_count(62)) - .saturating_add(::DbWeight::get().reads(1)) + Weights::get_first_usable_core() + .saturating_add(Weights::extend_short_leases()) + .saturating_add(Weights::remove_premature_renewals_add_back_leases()) + .saturating_add(Weights::give_dropped_leases_renewal_rights_and_workplan_entry()) + .saturating_add(Weights::add_pool_core()) + .saturating_add(::WeightInfo::request_core_count(62)) } #[cfg(feature = "try-runtime")] @@ -467,13 +497,11 @@ impl OnRuntimeUpgrade for FixMigration { // Incorrect potential renewals in state -#[cfg(feature = "try-runtime")] const INCORRECT_RENEWAL_IDS: [(u16, u32); 6] = [(6, 292605), (5, 292605), (13, 292605), (15, 292605), (47, 292605), (44, 292605)]; // Hardcoded para ids and their end timeslice. // Taken from https://github.com/SBalaguer/coretime-migration/blob/master/polkadot-output-200924.json -#[cfg(feature = "try-runtime")] const POTENTIAL_RENEWALS: [(u32, u32); 5] = [(2048, 283680), (3375, 283680), (3358, 283680), (2053, 283680), (2056, 283680)]; From cb57ec5afef09a7957d19231734d18e17688a5f0 Mon Sep 17 00:00:00 2001 From: eskimor Date: Mon, 30 Sep 2024 09:11:28 +0200 Subject: [PATCH 22/25] Add Changelog entry --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9e9397901..a6c5900627 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,19 @@ Changelog for the runtimes governed by the Polkadot Fellowship. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [1.3.3] 30.09.2024 + +### Fixed + +In the initial Coretime migration the leases were off due to the lease offset on +Polkadot. [polkadot-fellows/runtimes#458](https://github.com/polkadot-fellows/runtimes/pull/458) +fixes this via another storage migration on the Coretime chain. + ## [1.3.2] 27.08.2024 ### Fixed -- Kusama: Revert accidental changes to inflation formula ([polkadot-fellows/runtimes#445](tps://github.com/polkadot-fellows/runtimes/pull/445)). +- Kusama: Revert accidental changes to inflation formula ([polkadot-fellows/runtimes#445](https://github.com/polkadot-fellows/runtimes/pull/445)). ## [1.3.1] 23.08.2024 From 8075c6a2df8b06118482d15ffda2ff1f44961b4d Mon Sep 17 00:00:00 2001 From: eskimor Date: Mon, 30 Sep 2024 10:11:48 +0200 Subject: [PATCH 23/25] Add rpc node for polkadot-coretime so tests are run. --- .github/workflows/runtimes-matrix.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/runtimes-matrix.json b/.github/workflows/runtimes-matrix.json index 2107e8d0b0..b38a129e60 100644 --- a/.github/workflows/runtimes-matrix.json +++ b/.github/workflows/runtimes-matrix.json @@ -65,6 +65,7 @@ "name": "coretime-polkadot", "package": "coretime-polkadot-runtime", "path": "system-parachains/coretime/coretime-polkadot", + "uri": "wss://polkadot-coretime-rpc.polkadot.io:443", "is_relay": false }, { From 483c1a2a7ecfb9abbe925a5abdd0460c2ae12c4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 1 Oct 2024 21:06:46 +0200 Subject: [PATCH 24/25] Update CHANGELOG.md --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a803b45a4d..bf578f1b47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Chain-spec generator: propagate the `on_chain_release_build` feature to the chain-spec generator. Without this the live/genesis chain-specs contain a wrongly-configured WASM blob ([polkadot-fellows/runtimes#450](https://github.com/polkadot-fellows/runtimes/pull/450)). -- In the initial Coretime migration the leases were off due to the lease offset on -Polkadot. [polkadot-fellows/runtimes#458](https://github.com/polkadot-fellows/runtimes/pull/458) -fixes this via another storage migration on the Coretime chain. +- Adds a migration to the Polkadot Coretime chain to fix an issue from the initial Coretime migration. [polkadot-fellows/runtimes#458](https://github.com/polkadot-fellows/runtimes/pull/458) ### Added From a1acf1c228845c08952c973c765eef2a5c7b0c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 1 Oct 2024 21:07:19 +0200 Subject: [PATCH 25/25] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf578f1b47..dcf8377f25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Chain-spec generator: propagate the `on_chain_release_build` feature to the chain-spec generator. Without this the live/genesis chain-specs contain a wrongly-configured WASM blob ([polkadot-fellows/runtimes#450](https://github.com/polkadot-fellows/runtimes/pull/450)). -- Adds a migration to the Polkadot Coretime chain to fix an issue from the initial Coretime migration. [polkadot-fellows/runtimes#458](https://github.com/polkadot-fellows/runtimes/pull/458) +- Adds a migration to the Polkadot Coretime chain to fix an issue from the initial Coretime migration. ([polkadot-fellows/runtimes#458](https://github.com/polkadot-fellows/runtimes/pull/458)) ### Added