Broker pallet: fix interlacing (#2811)

With the current code, when a user interlaces their region, the end
result will be three regions in the state:
- the non-interlaced region
- first part of the interlaced region
- second part of the interlaced region

The existing implementation retains the non-interlaced region in the
state, leading to a problematic scenario:

1. User 1 acquires a region from the market.
2. User 1 then interlaces this region.
3. Subsequently, User 1 transfers one part of the interlaced regions to
User 2.
Despite this transfer, User 1 retains the ability to assign the entire
original non-interlaced region, which is inconsistent with the fact that
they no longer own one of the interlaced parts.

This PR resolves the issue by removing the original region, ensuring
that only the two new interlaced regions remain in the state.

prdoc/pr_2811.prdoc

@@ -0,0 +1,13 @@
+title: "Interlacing removes the region on which it is performed."
+
+doc:
+  - audience: Runtime User
+    description: |
+      The current implementation of the broker pallet does not remove
+      the region on which the interlacing is performed. This can create
+      a vulnerability, as the original region owner is still allowed to 
+      assign a task to the region even after transferring an interlaced
+      part of it.
+
+crates:
+  - name: "pallet-broker"

substrate/frame/broker/src/dispatchable_impls.rs

@@ -234,6 +234,9 @@ impl<T: Config> Pallet<T> {
 		ensure!(!pivot.is_void(), Error::<T>::VoidPivot);
 		ensure!(pivot != region_id.mask, Error::<T>::CompletePivot);
 
+		// The old region should be removed.
+		Regions::<T>::remove(&region_id);
+
 		let one = RegionId { mask: pivot, ..region_id };
 		Regions::<T>::insert(&one, &region);
 		let other = RegionId { mask: region_id.mask ^ pivot, ..region_id };

substrate/frame/broker/src/tests.rs

@@ -602,6 +602,43 @@ fn interlace_works() {
 	});
 }
 
+#[test]
+fn cant_assign_unowned_region() {
+	TestExt::new().endow(1, 1000).execute_with(|| {
+		assert_ok!(Broker::do_start_sales(100, 1));
+		advance_to(2);
+		let region = Broker::do_purchase(1, u64::max_value()).unwrap();
+		let (region1, region2) =
+			Broker::do_interlace(region, Some(1), CoreMask::from_chunk(0, 30)).unwrap();
+
+		// Transfer the interlaced region to account 2.
+		assert_ok!(Broker::do_transfer(region2, Some(1), 2));
+
+		// The initial owner should not be able to assign the non-interlaced region, since they have
+		// just transferred an interlaced part of it to account 2.
+		assert_noop!(Broker::do_assign(region, Some(1), 1001, Final), Error::<Test>::UnknownRegion);
+
+		// Account 1 can assign only the interlaced region that they did not transfer.
+		assert_ok!(Broker::do_assign(region1, Some(1), 1001, Final));
+		// Account 2 can assign the region they received.
+		assert_ok!(Broker::do_assign(region2, Some(2), 1002, Final));
+
+		advance_to(10);
+		assert_eq!(
+			CoretimeTrace::get(),
+			vec![(
+				6,
+				AssignCore {
+					core: 0,
+					begin: 8,
+					assignment: vec![(Task(1001), 21600), (Task(1002), 36000)],
+					end_hint: None
+				}
+			),]
+		);
+	});
+}
+
 #[test]
 fn interlace_then_partition_works() {
 	TestExt::new().endow(1, 1000).execute_with(|| {