Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add migration to fix coretime state #458

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8b09d7f
Add migration to fix coretime state (WIP)
seadanda Sep 20, 2024
d210d8d
Update timeslice
seadanda Sep 21, 2024
64770cc
Update timeslice
seadanda Sep 21, 2024
4dd1326
Fix compilation without `try-runtime` feature
tdimitrov Sep 23, 2024
0367a60
Remove hardcoded core ids and obtain them from sales instead
tdimitrov Sep 23, 2024
ddf33fc
Remove a duplicated `use`
tdimitrov Sep 23, 2024
79d2e8f
Revert "Remove hardcoded core ids and obtain them from sales instead"
tdimitrov Sep 24, 2024
63b534b
Fix todos and update try-runtime tests
tdimitrov Sep 27, 2024
a0a2ff3
Basic migration logic.
Sep 29, 2024
d98f0e6
fmt
Sep 29, 2024
0f809df
Fix tests
Sep 29, 2024
3349c15
Add useful traces
Sep 29, 2024
aaf3fe0
Add missing tests:
Sep 29, 2024
b5a58d0
Fix imports
Sep 29, 2024
641e72f
Replace remaining useless debug_asserts
Sep 29, 2024
ac10dc3
New lines
Sep 29, 2024
41b53cb
More checks
Sep 29, 2024
e697de8
Cleanup
Sep 29, 2024
73d2256
Merge pull request #6 from eskimor/rk-coretime-migration-fix
eskimor Sep 29, 2024
74e4efc
Fixes, cleanup
Sep 29, 2024
3bfd626
Merge branch 'rk-coretime-migration-fix' into donal-coretime-migratio…
Sep 29, 2024
91076d7
Fix feature propagation
Sep 29, 2024
5b6d997
Add weights
Sep 30, 2024
cb57ec5
Add Changelog entry
Sep 30, 2024
6bc0754
Merge remote-tracking branch 'origin/main' into donal-coretime-migrat…
Sep 30, 2024
8075c6a
Add rpc node for polkadot-coretime so tests are run.
Sep 30, 2024
836feb6
Merge branch 'main' into donal-coretime-migration-fix
eskimor Oct 1, 2024
483c1a2
Update CHANGELOG.md
bkchr Oct 1, 2024
a1acf1c
Update CHANGELOG.md
bkchr Oct 1, 2024
50d429b
Merge branch 'main' into donal-coretime-migration-fix
fellowship-merge-bot[bot] Oct 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion system-parachains/coretime/coretime-polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -117,6 +118,7 @@ pub type UncheckedExtrinsic =
pub type Migrations = (
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
migrations::FixMigration,
);

/// Executive: handles dispatch to the various modules.
Expand All @@ -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,
Expand Down
352 changes: 352 additions & 0 deletions system-parachains/coretime/coretime-polkadot/src/migrations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,352 @@
// 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, CoreAssignment::Pool, CoreMask, LeaseRecordItem, Leases, LeasesRecordOf,
PotentialRenewalId, PotentialRenewals, Schedule, ScheduleItem, WeightInfo, Workplan,
};

use sp_std::vec::Vec;

#[cfg(feature = "try-runtime")]
use pallet_broker::{CoreAssignment::Task, PotentialRenewalRecord, SaleInfo, SaleInfoRecordOf};
#[cfg(feature = "try-runtime")]
use sp_runtime::TryRuntimeError;

/// The log target.
const TARGET: &str = "runtime::bootstrapping::fix-migration";

// Alias into the broker weights for this runtime.
type BrokerWeights = weights::pallet_broker::WeightInfo<Runtime>;

pub struct FixMigration;

impl OnRuntimeUpgrade for FixMigration {
fn on_runtime_upgrade() -> Weight {
if PotentialRenewals::<Runtime>::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 <Runtime as frame_system::Config>::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<Runtime> = LEASES
.iter()
.map(|(until, task)| LeaseRecordItem { until: *until, task: *task })
.collect::<Vec<_>>()
.try_into()
.expect("Within range of bounded vec");
Leases::<Runtime>::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) {
seadanda marked this conversation as resolved.
Show resolved Hide resolved
// 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::<Runtime>::insert((287565, core_id), workplan_entry);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: This is indeed the region_begin.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Check on what occasions this is sent to the relay chain.

}

// Remove the existing 6 PotentialRenewals.
for &(core, when) in INCORRECT_RENEWAL_IDS.iter() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

PotentialRenewals::<Runtime>::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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked that the first core is indeed 56. There are 5 renewals in POTENTIAL_RENEWALS s.
image

// 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::<Runtime>::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::<Runtime>::decode_len().unwrap_or(0) as u16 +
pallet_broker::Leases::<Runtime>::decode_len().unwrap_or(0) as u16 +
5 + 6;

match pallet_broker::Pallet::<Runtime>::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::<Runtime>::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::<Runtime>::insert((292605, 61), pool_assignment.clone());
seadanda marked this conversation as resolved.
Show resolved Hide resolved

// 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
seadanda marked this conversation as resolved.
Show resolved Hide resolved
// 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::<Runtime>::insert((now, 61), pool_assignment);

// TODO finalise the weights here.
<Runtime as frame_system::Config>::DbWeight::get()
.writes(1)
.saturating_mul(LEASES.len() as u64)
.saturating_add(BrokerWeights::request_core_count(62))
.saturating_add(<Runtime as frame_system::Config>::DbWeight::get().reads(1))
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
// Idempotency check - this core will never be renewable at this timeslice ever again.
if PotentialRenewals::<Runtime>::get(PotentialRenewalId { core: 6, when: 292605 }).is_none()
{
return Ok(Vec::new())
}
let sale_info = SaleInfo::<Runtime>::get().unwrap();
let leases = Leases::<Runtime>::get();
let pre_upgrade_state = (sale_info, leases);
Ok(pre_upgrade_state.encode())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(state: Vec<u8>) -> Result<(), TryRuntimeError> {
if state.is_empty() {
return Ok(())
}
let (prev_sale_info, prev_leases): (SaleInfoRecordOf<Runtime>, LeasesRecordOf<Runtime>) =
Decode::decode(&mut &state[..]).unwrap();

log::info!(target: TARGET, "Checking migration.");

let sale_info = SaleInfo::<Runtime>::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;

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 system_chains.iter().enumerate() {
assert_eq!(
Workplan::<Runtime>::get((workplan_start, core_id as u16)),
Some(Schedule::truncate_from(Vec::from([ScheduleItem {
mask: CoreMask::complete(),
assignment: task.clone(),
}])))
);
}

// Make sure we've got all the leases.
let leases = Leases::<Runtime>::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 + 5;

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::<Runtime>::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 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 {
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::<Runtime>::get(PotentialRenewalId {
seadanda marked this conversation as resolved.
Show resolved Hide resolved
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::<Runtime>::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 {
let entry = Workplan::<Runtime>::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.
assert!(frame_system::Pallet::<Runtime>::read_events_no_consensus().any(|e| {
match e.event {
crate::RuntimeEvent::Broker(
pallet_broker::Event::<Runtime>::CoreCountRequested { core_count },
) => {
log::info!("{core_count:?}");

core_count == 62
},
_ => false,
}
}));

Ok(())
}
}

// Incorrect potential renewals in state
const INCORRECT_RENEWAL_IDS: [(u16, u32); 6] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: all leases added below (compared to coretime chain state) are to be found here.

[(6, 292605), (5, 292605), (13, 292605), (15, 292605), (47, 292605), (44, 292605)];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with chain state: Matching. (This is the entire list of currently registered potential renewals, as expected.)


// 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] = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List fully verified. Complete and correct.

(2094, 298800),
(2040, 298800),
(2035, 359280),
(3344, 344160),
(3370, 389520),
(3367, 374400),
(2086, 389520),
(2032, 359280),
(3333, 298800),
(2051, 389520),
(2106, 298800),
(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),
(2093, 298800),
(2026, 344160),
(3388, 404640),
(2101, 298800),
(3354, 359280),
(2006, 344160),
(2013, 374400),
];
Loading