Coretime Chain: mitigate behaviour with many assignments on one core (#…

…434)

We can handle a maximum of 28 assignments inside one XCM, while it's
possible to have 80 (if a region is interlaced 79 times).
This can be chunked on the coretime chain side but currently the
scheduler on the relay makes assumptions that means we can't send more
than one chunk for a given core.

I've made a mitigation here that requires no sdk release, but it just
truncates the additional assignments until we can extend the relay to
support this. This means that the first 27 assignments are taken, the
final 28th is used to pad with idle to complete the mask (the relay also
assumes that every schedule is complete). Any other assignments are
dropped.

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: fellowship-merge-bot[bot] <151052383+fellowship-merge-bot[bot]@users.noreply.github.com>

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):
 

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,
 		}
 	},
 }

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 }

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
+	// </~https://github.com/rust-lang/rust/issues/86935>
+	type CoretimeEvent = <CoretimeKusama as Chain>::RuntimeEvent;
+	type RelayEvent = <Kusama as Chain>::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.
+		<CoretimeKusama as CoretimeKusamaPallet>::Broker::on_initialize(
+			<CoretimeKusama as Chain>::System::block_number(),
+		);
+
+		let coretime_root_origin = <CoretimeKusama as Chain>::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!(<CoretimeKusama as CoretimeKusamaPallet>::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!(<CoretimeKusama as CoretimeKusamaPallet>::Broker::configure(
+			coretime_root_origin.clone(),
+			config
+		));
+		assert_ok!(<CoretimeKusama as CoretimeKusamaPallet>::Broker::start_sales(
+			coretime_root_origin,
+			100,
+			0
+		));
+		assert_eq!(
+			pallet_broker::Status::<<CoretimeKusama as Chain>::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(<Kusama as Chain>::System::block_number);
+
+	let config = CoretimeKusama::ext_wrapper(|| {
+		Configuration::<<CoretimeKusama as Chain>::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.
+			<CoretimeKusama as CoretimeKusamaPallet>::Broker::on_initialize(
+				<CoretimeKusama as Chain>::System::block_number(),
+			);
+		});
+
+		Kusama::ext_wrapper(|| {
+			block_number_cursor = <Kusama as Chain>::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.
+		<CoretimeKusama as CoretimeKusamaPallet>::Broker::on_initialize(
+			<CoretimeKusama as Chain>::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.
+		<CoretimeKusama as CoretimeKusamaPallet>::Broker::on_initialize(
+			<CoretimeKusama as Chain>::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 { .. }
+				) => {},
+			]
+		);
+	});
+}

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;

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 = <CoretimePolkadot as Chain>::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),

system-parachains/coretime/coretime-kusama/src/coretime.rs

@@ -223,15 +223,45 @@ impl CoretimeInterface for CoretimeAllocator {
 		end_hint: Option<RCBlockNumberOf<Self>>,
 	) {
 		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
 		// `proof_size` = 3612
 		// 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::<Vec<_>>(),
+			);
+
+			// 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,

system-parachains/coretime/coretime-polkadot/src/coretime.rs

@@ -224,8 +224,6 @@ impl CoretimeInterface for CoretimeAllocator {
 		end_hint: Option<RCBlockNumberOf<Self>>,
 	) {
 		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::<Vec<_>>(),
+			);
+
+			// 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."