[pallet-broker] add extrinsic to remove an assignment (paritytech#7080)

# Description

paritytech#6929 requests more extrinsics for "managing the network's coretime
allocations without needing to dabble with migration+runtime upgrade or
set/kill storage patterns"

This pull request implements the remove_assignment() extrinsic.


## Integration

Downstream projects need to benchmark the weight for the
remove_assignment() extrinsic.

---------

Co-authored-by: Jonathan Brown <jbrown@acuity.network>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Dónal Murray <donal.murray@parity.io>

prdoc/pr_7080.prdoc

@@ -0,0 +1,17 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "[pallet-broker] add extrinsic to remove an assignment"
+
+doc:
+  - audience: Runtime User
+    description: |
+      A new `remove_assignment` extrinsic is introduced to the broker pallet to allow an assignment to be removed by the root origin.
+
+crates:
+  - name: pallet-broker
+    bump: major
+  - name: coretime-rococo-runtime
+    bump: patch
+  - name: coretime-westend-runtime
+    bump: patch

substrate/frame/broker/src/benchmarking.rs

@@ -1225,6 +1225,33 @@ mod benches {
 		Ok(())
 	}
 
+	#[benchmark]
+	fn remove_assignment() -> Result<(), BenchmarkError> {
+		setup_and_start_sale::<T>()?;
+
+		advance_to::<T>(2);
+
+		let caller: T::AccountId = whitelisted_caller();
+		T::Currency::set_balance(
+			&caller.clone(),
+			T::Currency::minimum_balance().saturating_add(10_000_000u32.into()),
+		);
+
+		let region = Broker::<T>::do_purchase(caller.clone(), 10_000_000u32.into())
+			.expect("Offer not high enough for configuration.");
+
+		Broker::<T>::do_assign(region, None, 1000, Provisional)
+			.map_err(|_| BenchmarkError::Weightless)?;
+
+		let origin =
+			T::AdminOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
+
+		#[extrinsic_call]
+		_(origin as T::RuntimeOrigin, region);
+
+		Ok(())
+	}
+
 	// Implements a test for each benchmark. Execute with:
 	// `cargo test -p pallet-broker --features runtime-benchmarks`.
 	impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);

substrate/frame/broker/src/dispatchable_impls.rs

@@ -350,6 +350,14 @@ impl<T: Config> Pallet<T> {
 		Ok(())
 	}
 
+	pub(crate) fn do_remove_assignment(region_id: RegionId) -> DispatchResult {
+		let workplan_key = (region_id.begin, region_id.core);
+		ensure!(Workplan::<T>::contains_key(&workplan_key), Error::<T>::AssignmentNotFound);
+		Workplan::<T>::remove(&workplan_key);
+		Self::deposit_event(Event::<T>::AssignmentRemoved { region_id });
+		Ok(())
+	}
+
 	pub(crate) fn do_pool(
 		region_id: RegionId,
 		maybe_check_owner: Option<T::AccountId>,

substrate/frame/broker/src/lib.rs

@@ -279,6 +279,11 @@ pub mod pallet {
 			/// The task to which the Region was assigned.
 			task: TaskId,
 		},
+		/// An assignment has been removed from the workplan.
+		AssignmentRemoved {
+			/// The Region which was removed from the workplan.
+			region_id: RegionId,
+		},
 		/// A Region has been added to the Instantaneous Coretime Pool.
 		Pooled {
 			/// The Region which was added to the Instantaneous Coretime Pool.
@@ -558,6 +563,8 @@ pub mod pallet {
 		SovereignAccountNotFound,
 		/// Attempted to disable auto-renewal for a core that didn't have it enabled.
 		AutoRenewalNotEnabled,
+		/// Attempted to force remove an assignment that doesn't exist.
+		AssignmentNotFound,
 		/// Needed to prevent spam attacks.The amount of credits the user attempted to purchase is
 		/// below `T::MinimumCreditPurchase`.
 		CreditPurchaseTooSmall,
@@ -996,6 +1003,16 @@ pub mod pallet {
 			Self::do_remove_lease(task)
 		}
 
+		/// Remove an assignment from the Workplan.
+		///
+		/// - `origin`: Must be Root or pass `AdminOrigin`.
+		/// - `region_id`: The Region to be removed from the workplan.
+		#[pallet::call_index(26)]
+		pub fn remove_assignment(origin: OriginFor<T>, region_id: RegionId) -> DispatchResult {
+			T::AdminOrigin::ensure_origin_or_root(origin)?;
+			Self::do_remove_assignment(region_id)
+		}
+
 		#[pallet::call_index(99)]
 		#[pallet::weight(T::WeightInfo::swap_leases())]
 		pub fn swap_leases(origin: OriginFor<T>, id: TaskId, other: TaskId) -> DispatchResult {

substrate/frame/broker/src/tests.rs

@@ -25,7 +25,10 @@ use frame_support::{
 };
 use frame_system::RawOrigin::Root;
 use pretty_assertions::assert_eq;
-use sp_runtime::{traits::Get, Perbill, TokenError};
+use sp_runtime::{
+	traits::{BadOrigin, Get},
+	Perbill, TokenError,
+};
 use CoreAssignment::*;
 use CoretimeTraceItem::*;
 use Finality::*;
@@ -1848,6 +1851,25 @@ fn disable_auto_renew_works() {
 	});
 }
 
+#[test]
+fn remove_assignment_works() {
+	TestExt::new().endow(1, 1000).execute_with(|| {
+		assert_ok!(Broker::do_start_sales(100, 1));
+		advance_to(2);
+		let region_id = Broker::do_purchase(1, u64::max_value()).unwrap();
+		assert_ok!(Broker::do_assign(region_id, Some(1), 1001, Final));
+		let workplan_key = (region_id.begin, region_id.core);
+		assert_ne!(Workplan::<Test>::get(workplan_key), None);
+		assert_noop!(Broker::remove_assignment(RuntimeOrigin::signed(2), region_id), BadOrigin);
+		assert_ok!(Broker::remove_assignment(RuntimeOrigin::root(), region_id));
+		assert_eq!(Workplan::<Test>::get(workplan_key), None);
+		assert_noop!(
+			Broker::remove_assignment(RuntimeOrigin::root(), region_id),
+			Error::<Test>::AssignmentNotFound
+		);
+	});
+}
+
 #[test]
 fn start_sales_sets_correct_core_count() {
 	TestExt::new().endow(1, 1000).execute_with(|| {