diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ae025341e..875542ae18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Fix claim queue size ([runtimes#381](/~https://github.com/polkadot-fellows/runtimes/pull/381), [SDK v1.14 #4691](/~https://github.com/paritytech/polkadot-sdk/pull/4691)). - `pallet-referenda`: Ensure to schedule referenda earliest at the next block ([runtimes#381](/~https://github.com/polkadot-fellows/runtimes/pull/381), [SDK v1.14 #4823](/~https://github.com/paritytech/polkadot-sdk/pull/4823)). - Don't partially modify HRMP pages ([runtimes#381](/~https://github.com/polkadot-fellows/runtimes/pull/381), [SDK v1.14 #4710](/~https://github.com/paritytech/polkadot-sdk/pull/4710)). +- Coretime Chain: mitigate behaviour with many assignments on one core ([runtimes#434][/~https://github.com/polkadot-fellows/runtimes/pull/434]). #### From [#322](/~https://github.com/polkadot-fellows/runtimes/pull/322): diff --git a/Cargo.lock b/Cargo.lock index 15ac9c3cbc..0ccf9de0a4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2634,12 +2634,14 @@ dependencies = [ "kusama-runtime-constants", "kusama-system-emulated-network", "pallet-balances", + "pallet-broker", "pallet-identity", "pallet-message-queue", "pallet-xcm", "parachains-common", "parity-scale-codec", "polkadot-runtime-common", + "polkadot-runtime-parachains", "sp-runtime 38.0.0", "staging-kusama-runtime", "staging-xcm", diff --git a/integration-tests/emulated/chains/parachains/coretime/coretime-kusama/src/lib.rs b/integration-tests/emulated/chains/parachains/coretime/coretime-kusama/src/lib.rs index 2876d1886b..c35d01722a 100644 --- a/integration-tests/emulated/chains/parachains/coretime/coretime-kusama/src/lib.rs +++ b/integration-tests/emulated/chains/parachains/coretime/coretime-kusama/src/lib.rs @@ -41,6 +41,7 @@ decl_test_parachains! { pallets = { PolkadotXcm: coretime_kusama_runtime::PolkadotXcm, Balances: coretime_kusama_runtime::Balances, + Broker: coretime_kusama_runtime::Broker, } }, } diff --git a/integration-tests/emulated/tests/coretime/coretime-kusama/Cargo.toml b/integration-tests/emulated/tests/coretime/coretime-kusama/Cargo.toml index 4bc905ff94..8371a02754 100644 --- a/integration-tests/emulated/tests/coretime/coretime-kusama/Cargo.toml +++ b/integration-tests/emulated/tests/coretime/coretime-kusama/Cargo.toml @@ -14,12 +14,14 @@ codec = { workspace = true, default-features = true } sp-runtime = { workspace = true, default-features = true } frame-support = { workspace = true, default-features = true } pallet-balances = { workspace = true, default-features = true } +pallet-broker = { workspace = true, default-features = true } pallet-message-queue = { workspace = true, default-features = true } pallet-identity = { workspace = true, default-features = true } # Polkadot polkadot-runtime-common = { workspace = true, default-features = true } pallet-xcm = { workspace = true, default-features = true } +runtime-parachains = { workspace = true, default-features = true } xcm = { workspace = true, default-features = true } xcm-executor = { workspace = true } xcm-runtime-apis = { workspace = true, default-features = true } diff --git a/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/coretime_interface.rs b/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/coretime_interface.rs new file mode 100644 index 0000000000..435bde8dba --- /dev/null +++ b/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/coretime_interface.rs @@ -0,0 +1,204 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// 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. + +use crate::*; +use frame_support::traits::OnInitialize; +use kusama_runtime_constants::system_parachain::coretime::TIMESLICE_PERIOD; +use pallet_broker::{ConfigRecord, Configuration, CoreAssignment, CoreMask, ScheduleItem}; +use sp_runtime::Perbill; + +#[test] +fn transact_hardcoded_weights_are_sane() { + // There are three transacts with hardcoded weights sent from the Coretime Chain to the Relay + // Chain across the CoretimeInterface which are triggered at various points in the sales cycle. + // - Request core count - triggered directly by `start_sales` or `request_core_count` + // extrinsics. + // - Request revenue info - triggered when each timeslice is committed. + // - Assign core - triggered when an entry is encountered in the workplan for the next + // timeslice. + + // RuntimeEvent aliases to avoid warning from usage of qualified paths in assertions due to + // + type CoretimeEvent = ::RuntimeEvent; + type RelayEvent = ::RuntimeEvent; + + // Reserve a workload, configure broker and start sales. + CoretimeKusama::execute_with(|| { + // Hooks don't run in emulated tests - workaround as we need `on_initialize` to tick things + // along and have no concept of time passing otherwise. + ::Broker::on_initialize( + ::System::block_number(), + ); + + let coretime_root_origin = ::RuntimeOrigin::root(); + + // Create and populate schedule with the worst case assignment on this core. + let mut schedule = Vec::new(); + for i in 0..80 { + schedule.push(ScheduleItem { + mask: CoreMask::void().set(i), + assignment: CoreAssignment::Task(2000 + i), + }) + } + + assert_ok!(::Broker::reserve( + coretime_root_origin.clone(), + schedule.try_into().expect("Vector is within bounds."), + )); + + // Configure broker and start sales. + let config = ConfigRecord { + advance_notice: 1, + interlude_length: 1, + leadin_length: 2, + region_length: 1, + ideal_bulk_proportion: Perbill::from_percent(40), + limit_cores_offered: None, + renewal_bump: Perbill::from_percent(2), + contribution_timeout: 1, + }; + assert_ok!(::Broker::configure( + coretime_root_origin.clone(), + config + )); + assert_ok!(::Broker::start_sales( + coretime_root_origin, + 100, + 0 + )); + assert_eq!( + pallet_broker::Status::<::Runtime>::get() + .unwrap() + .core_count, + 1 + ); + + assert_expected_events!( + CoretimeKusama, + vec![ + CoretimeEvent::Broker( + pallet_broker::Event::ReservationMade { .. } + ) => {}, + CoretimeEvent::Broker( + pallet_broker::Event::CoreCountRequested { core_count: 1 } + ) => {}, + CoretimeEvent::ParachainSystem( + cumulus_pallet_parachain_system::Event::UpwardMessageSent { .. } + ) => {}, + ] + ); + }); + + // Check that the request_core_count message was processed successfully. This will fail if the + // weights are misconfigured. + Kusama::execute_with(|| { + Kusama::assert_ump_queue_processed(true, Some(CoretimeKusama::para_id()), None); + + assert_expected_events!( + Kusama, + vec![ + RelayEvent::MessageQueue( + pallet_message_queue::Event::Processed { success: true, .. } + ) => {}, + ] + ); + }); + + // Keep track of the relay chain block number so we can fast forward while still checking the + // right block. + let mut block_number_cursor = Kusama::ext_wrapper(::System::block_number); + + let config = CoretimeKusama::ext_wrapper(|| { + Configuration::<::Runtime>::get() + .expect("Pallet was configured earlier.") + }); + + // Now run up to the block before the sale is rotated. + while block_number_cursor < TIMESLICE_PERIOD - config.advance_notice - 1 { + CoretimeKusama::execute_with(|| { + // Hooks don't run in emulated tests - workaround. + ::Broker::on_initialize( + ::System::block_number(), + ); + }); + + Kusama::ext_wrapper(|| { + block_number_cursor = ::System::block_number(); + }); + + dbg!(&block_number_cursor); + } + + // In this block we trigger assign core. + CoretimeKusama::execute_with(|| { + // Hooks don't run in emulated tests - workaround. + ::Broker::on_initialize( + ::System::block_number(), + ); + + assert_expected_events!( + CoretimeKusama, + vec![ + CoretimeEvent::Broker( + pallet_broker::Event::SaleInitialized { .. } + ) => {}, + CoretimeEvent::Broker( + pallet_broker::Event::CoreAssigned { .. } + ) => {}, + CoretimeEvent::ParachainSystem( + cumulus_pallet_parachain_system::Event::UpwardMessageSent { .. } + ) => {}, + ] + ); + }); + + // In this block we trigger request revenue. + CoretimeKusama::execute_with(|| { + // Hooks don't run in emulated tests - workaround. + ::Broker::on_initialize( + ::System::block_number(), + ); + + assert_expected_events!( + CoretimeKusama, + vec![ + CoretimeEvent::ParachainSystem( + cumulus_pallet_parachain_system::Event::UpwardMessageSent { .. } + ) => {}, + ] + ); + }); + + // Check that the assign_core and request_revenue_info_at messages were processed successfully. + // This will fail if the weights are misconfigured. + Kusama::execute_with(|| { + Kusama::assert_ump_queue_processed(true, Some(CoretimeKusama::para_id()), None); + + assert_expected_events!( + Kusama, + vec![ + RelayEvent::MessageQueue( + pallet_message_queue::Event::Processed { success: true, .. } + ) => {}, + RelayEvent::MessageQueue( + pallet_message_queue::Event::Processed { success: true, .. } + ) => {}, + RelayEvent::Coretime( + runtime_parachains::coretime::Event::CoreAssigned { .. } + ) => {}, + ] + ); + }); +} diff --git a/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/mod.rs b/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/mod.rs index 516ec37cc1..057dc87e71 100644 --- a/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/mod.rs +++ b/integration-tests/emulated/tests/coretime/coretime-kusama/src/tests/mod.rs @@ -13,4 +13,5 @@ // See the License for the specific language governing permissions and // limitations under the License. +mod coretime_interface; mod teleport; diff --git a/integration-tests/emulated/tests/coretime/coretime-polkadot/src/tests/coretime_interface.rs b/integration-tests/emulated/tests/coretime/coretime-polkadot/src/tests/coretime_interface.rs index f80245794d..144bd37fa9 100644 --- a/integration-tests/emulated/tests/coretime/coretime-polkadot/src/tests/coretime_interface.rs +++ b/integration-tests/emulated/tests/coretime/coretime-polkadot/src/tests/coretime_interface.rs @@ -44,9 +44,9 @@ fn transact_hardcoded_weights_are_sane() { let coretime_root_origin = ::RuntimeOrigin::root(); - // Create and populate schedule with some assignments on this core. + // Create and populate schedule with the worst case assignment on this core. let mut schedule = Vec::new(); - for i in 0..10 { + for i in 0..80 { schedule.push(ScheduleItem { mask: CoreMask::void().set(i), assignment: CoreAssignment::Task(2000 + i), diff --git a/system-parachains/coretime/coretime-kusama/src/coretime.rs b/system-parachains/coretime/coretime-kusama/src/coretime.rs index 06c777e24a..262619111a 100644 --- a/system-parachains/coretime/coretime-kusama/src/coretime.rs +++ b/system-parachains/coretime/coretime-kusama/src/coretime.rs @@ -223,8 +223,6 @@ impl CoretimeInterface for CoretimeAllocator { end_hint: Option>, ) { use crate::coretime::CoretimeProviderCalls::AssignCore; - let assign_core_call = - RelayRuntimePallets::Coretime(AssignCore(core, begin, assignment, end_hint)); // Weight for `assign_core` from Kusama runtime benchmarks: // `ref_time` = 10177115 + (1 * 25000000) + (2 * 100000000) + (80 * 13932) = 236291675 @@ -232,6 +230,38 @@ impl CoretimeInterface for CoretimeAllocator { // Add 5% to each component and round to 2 significant figures. let call_weight = Weight::from_parts(248_000_000, 3800); + // The relay chain currently only allows `assign_core` to be called with a complete mask + // and only ever with increasing `begin`. The assignments must be truncated to avoid + // dropping that core's assignment completely. + + // This shadowing of `assignment` is temporary and can be removed when the relay can accept + // multiple messages to assign a single core. + let assignment = if assignment.len() > 28 { + let mut total_parts = 0u16; + // Account for missing parts with a new `Idle` assignment at the start as + // `assign_core` on the relay assumes this is sorted. We'll add the rest of the + // assignments and sum the parts in one pass, so this is just initialized to 0. + let mut assignment_truncated = vec![(CoreAssignment::Idle, 0)]; + // Truncate to first 27 non-idle assignments. + assignment_truncated.extend( + assignment + .into_iter() + .filter(|(a, _)| *a != CoreAssignment::Idle) + .take(27) + .inspect(|(_, parts)| total_parts += *parts) + .collect::>(), + ); + + // Set the parts of the `Idle` assignment we injected at the start of the vec above. + assignment_truncated[0].1 = 57_600u16.saturating_sub(total_parts); + assignment_truncated + } else { + assignment + }; + + let assign_core_call = + RelayRuntimePallets::Coretime(AssignCore(core, begin, assignment, end_hint)); + let message = Xcm(vec![ Instruction::UnpaidExecution { weight_limit: WeightLimit::Unlimited, diff --git a/system-parachains/coretime/coretime-polkadot/src/coretime.rs b/system-parachains/coretime/coretime-polkadot/src/coretime.rs index c6e076ebc9..cc94caf498 100644 --- a/system-parachains/coretime/coretime-polkadot/src/coretime.rs +++ b/system-parachains/coretime/coretime-polkadot/src/coretime.rs @@ -224,8 +224,6 @@ impl CoretimeInterface for CoretimeAllocator { end_hint: Option>, ) { use crate::coretime::CoretimeProviderCalls::AssignCore; - let assign_core_call = - RelayRuntimePallets::Coretime(AssignCore(core, begin, assignment, end_hint)); // Weight for `assign_core` from Polkadot runtime benchmarks: // `ref_time` = 10177115 + (1 * 25000000) + (2 * 100000000) + (80 * 13932) = 236291675 @@ -234,6 +232,38 @@ impl CoretimeInterface for CoretimeAllocator { // TODO check when benchmarks are rerun let call_weight = Weight::from_parts(248_000_000, 3800); + // The relay chain currently only allows `assign_core` to be called with a complete mask + // and only ever with increasing `begin`. The assignments must be truncated to avoid + // dropping that core's assignment completely. + + // This shadowing of `assignment` is temporary and can be removed when the relay can accept + // multiple messages to assign a single core. + let assignment = if assignment.len() > 28 { + let mut total_parts = 0u16; + // Account for missing parts with a new `Idle` assignment at the start as + // `assign_core` on the relay assumes this is sorted. We'll add the rest of the + // assignments and sum the parts in one pass, so this is just initialized to 0. + let mut assignment_truncated = vec![(CoreAssignment::Idle, 0)]; + // Truncate to first 27 non-idle assignments. + assignment_truncated.extend( + assignment + .into_iter() + .filter(|(a, _)| *a != CoreAssignment::Idle) + .take(27) + .inspect(|(_, parts)| total_parts += *parts) + .collect::>(), + ); + + // Set the parts of the `Idle` assignment we injected at the start of the vec above. + assignment_truncated[0].1 = 57_600u16.saturating_sub(total_parts); + assignment_truncated + } else { + assignment + }; + + let assign_core_call = + RelayRuntimePallets::Coretime(AssignCore(core, begin, assignment, end_hint)); + let message = Xcm(vec![ Instruction::UnpaidExecution { weight_limit: WeightLimit::Unlimited, @@ -246,7 +276,7 @@ impl CoretimeInterface for CoretimeAllocator { }, ]); - match PolkadotXcm::send_xcm(Here, Location::parent(), message) { + match PolkadotXcm::send_xcm(Here, Location::parent(), message.clone()) { Ok(_) => log::debug!( target: "runtime::coretime", "Core assignment sent successfully."