Drop region (#232)

* impl drop_region

* update tests

* update runtime config

* benchmarking

* fix processor/mock

* fix

* fix market mock

* fix benchmark

* solve timeslice issue

* update weight

---------

Co-authored-by: Szegoo <[email protected]>

pallets/market/src/mock.rs

@@ -139,6 +139,8 @@ impl pallet_regions::Config for Test {
 	type IsmpDispatcher = MockDispatcher<Self>;
 	type StateMachineHeightProvider = MockStateMachineHeightProvider;
 	type Timeout = ConstU64<1000>;
+	type RCBlockNumberProvider = RelayBlockNumberProvider;
+	type TimeslicePeriod = ConstU64<80>;
 	type UnsignedPriority = RegionsUnsignedPriority;
 	type WeightInfo = ();
 }

pallets/processor/src/mock.rs

@@ -167,6 +167,18 @@ parameter_types! {
 	pub const RegionsUnsignedPriority: TransactionPriority = TransactionPriority::max_value();
 }
 
+parameter_types! {
+	pub static RelayBlockNumber: u64 = 0;
+}
+
+pub struct RelayBlockNumberProvider;
+impl BlockNumberProvider for RelayBlockNumberProvider {
+	type BlockNumber = u64;
+	fn current_block_number() -> Self::BlockNumber {
+		RelayBlockNumber::get()
+	}
+}
+
 impl pallet_regions::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type Currency = Balances;
@@ -175,6 +187,8 @@ impl pallet_regions::Config for Test {
 	type StateMachineHeightProvider = MockStateMachineHeightProvider;
 	type Timeout = ConstU64<1000>;
 	type UnsignedPriority = RegionsUnsignedPriority;
+	type RCBlockNumberProvider = RelayBlockNumberProvider;
+	type TimeslicePeriod = ConstU64<80>;
 	type WeightInfo = ();
 }
 
@@ -191,18 +205,6 @@ impl FeeHandler<AccountId, u64> for OrderCreationFeeHandler {
 	}
 }
 
-parameter_types! {
-	pub static RelayBlockNumber: u64 = 0;
-}
-
-pub struct RelayBlockNumberProvider;
-impl BlockNumberProvider for RelayBlockNumberProvider {
-	type BlockNumber = u64;
-	fn current_block_number() -> Self::BlockNumber {
-		RelayBlockNumber::get()
-	}
-}
-
 pub struct OrderToAccountId;
 impl Convert<OrderId, AccountId> for OrderToAccountId {
 	fn convert(order: OrderId) -> AccountId {

pallets/regions/src/benchmarking.rs

@@ -70,6 +70,26 @@ mod benchmarks {
 		Ok(())
 	}
 
+	#[benchmark]
+	fn drop_region() -> Result<(), BenchmarkError> {
+		let caller: T::AccountId = whitelisted_caller();
+		let owner: T::AccountId = account("alice", 0, SEED);
+
+		let region_id = RegionId { begin: 0, core: 72, mask: CoreMask::complete() };
+		let record: RegionRecordOf<T> = RegionRecord { end: 0, owner, paid: None };
+
+		assert_ok!(crate::Pallet::<T>::mint_into(&region_id.into(), &caller));
+		assert_ok!(crate::Pallet::<T>::request_region_record(RawOrigin::None.into(), region_id));
+		assert_ok!(crate::Pallet::<T>::set_record(region_id, record));
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(caller.clone()), region_id);
+
+		assert_last_event::<T>(Event::RegionDropped { region_id, who: caller }.into());
+
+		Ok(())
+	}
+
 	#[benchmark]
 	fn on_accept() -> Result<(), BenchmarkError> {
 		let module = IsmpModuleCallback::<T>::default();

pallets/regions/src/lib.rs

@@ -28,12 +28,16 @@ use ismp::{
 };
 use ismp_parachain::PARACHAIN_CONSENSUS_ID;
 pub use pallet::*;
-use pallet_broker::RegionId;
+use pallet_broker::{RegionId, Timeslice};
 use pallet_ismp::{weights::IsmpModuleWeight, ModuleId};
+use primitives::StateMachineHeightProvider;
 use region_primitives::{Record, Region, RegionFactory};
 use scale_info::prelude::{format, vec, vec::Vec};
 use sp_core::H256;
-use sp_runtime::traits::Zero;
+use sp_runtime::{
+	traits::{BlockNumberProvider, Zero},
+	SaturatedConversion,
+};
 
 #[cfg(test)]
 mod mock;
@@ -53,7 +57,6 @@ mod types;
 use types::*;
 
 pub mod primitives;
-use primitives::StateMachineHeightProvider;
 
 pub mod weights;
 pub use weights::WeightInfo;
@@ -67,6 +70,10 @@ pub const PALLET_ID: ModuleId = ModuleId::Pallet(PalletId(*b"regionsp"));
 const REGION_NOT_FOUND: u8 = 1;
 const REGION_NOT_UNAVAILABLE: u8 = 2;
 
+/// Relay chain block number.
+pub type RCBlockNumberOf<T> =
+	<<T as crate::Config>::RCBlockNumberProvider as BlockNumberProvider>::BlockNumber;
+
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
@@ -87,6 +94,15 @@ pub mod pallet {
 		/// The Coretime chain from which we read region state.
 		type CoretimeChain: Get<StateMachine>;
 
+		/// Type for getting the current relay chain block.
+		///
+		/// This is used for determining the current timeslice.
+		type RCBlockNumberProvider: BlockNumberProvider;
+
+		/// Number of Relay-chain blocks per timeslice.
+		#[pallet::constant]
+		type TimeslicePeriod: Get<RCBlockNumberOf<Self>>;
+
 		/// The ISMP dispatcher.
 		type IsmpDispatcher: IsmpDispatcher<Account = Self::AccountId, Balance = BalanceOf<Self>>
 			+ Default;
@@ -139,23 +155,30 @@ pub mod pallet {
 		},
 		/// A region was minted via a cross chain transfer.
 		RegionMinted {
-			/// minted region id
+			/// id of the minted region
 			region_id: RegionId,
 		},
 		/// A region was burnt.
 		RegionBurnt {
-			/// burnt region id
+			/// id of the burnt region
 			region_id: RegionId,
 		},
 		/// A region was locked.
 		RegionLocked {
-			/// locked region id
+			/// id of the locked region
 			region_id: RegionId,
 		},
 		/// A region was unlocked.
 		RegionUnlocked {
-			/// unlocked region id
+			/// id of the unlocked region
+			region_id: RegionId,
+		},
+		/// An expired region was dropped.
+		RegionDropped {
+			/// id of the dropped region
 			region_id: RegionId,
+			/// the account that dropped the region
+			who: T::AccountId,
 		},
 	}
 
@@ -172,6 +195,8 @@ pub mod pallet {
 		IsmpDispatchError,
 		/// The record must be unavailable to be able to re-request it.
 		NotUnavailable,
+		/// The region record is not available.
+		NotAvailable,
 		/// The given region id is not valid.
 		InvalidRegionId,
 		/// Failed to get the latest height of the Coretime chain.
@@ -180,6 +205,8 @@ pub mod pallet {
 		RegionLocked,
 		/// Region isn't locked.
 		RegionNotLocked,
+		/// Region is not expired.
+		RegionNotExpired,
 	}
 
 	#[pallet::call]
@@ -215,6 +242,36 @@ pub mod pallet {
 
 			Ok(())
 		}
+
+		#[pallet::call_index(2)]
+		#[pallet::weight(T::WeightInfo::drop_region())]
+		pub fn drop_region(origin: OriginFor<T>, region_id: RegionId) -> DispatchResult {
+			let who = ensure_signed(origin)?;
+
+			let region = Regions::<T>::get(region_id).ok_or(Error::<T>::UnknownRegion)?;
+
+			ensure!(region.record.is_available(), Error::<T>::NotAvailable);
+
+			if let Record::Available(record) = region.record {
+				// Cannot drop a region that is not expired yet.
+
+				// Allowing region removal 1 timeslice before it truly expires makes writing
+				// benchmarks much easier. With this we can set the start and end to 0 and be able
+				// to drop the region without having to modify the current timeslice.
+				let current_timeslice = Self::current_timeslice();
+				#[cfg(feature = "runtime-benchmarks")]
+				ensure!(record.end <= current_timeslice, Error::<T>::RegionNotExpired);
+				#[cfg(not(feature = "runtime-benchmarks"))]
+				ensure!(record.end < current_timeslice, Error::<T>::RegionNotExpired);
+
+				Regions::<T>::remove(region_id);
+
+				Self::deposit_event(Event::RegionDropped { region_id, who });
+				Ok(())
+			} else {
+				Err(Error::<T>::NotAvailable.into())
+			}
+		}
 	}
 
 	impl<T: Config> Pallet<T> {
@@ -308,6 +365,12 @@ pub mod pallet {
 
 			Ok(key)
 		}
+
+		pub(crate) fn current_timeslice() -> Timeslice {
+			let latest_rc_block = T::RCBlockNumberProvider::current_block_number();
+			let timeslice_period = T::TimeslicePeriod::get();
+			(latest_rc_block / timeslice_period).saturated_into()
+		}
 	}
 
 	#[pallet::validate_unsigned]

pallets/regions/src/mock.rs

@@ -18,7 +18,7 @@ use frame_support::{pallet_prelude::*, parameter_types, traits::Everything};
 use ismp::{consensus::StateMachineId, host::StateMachine};
 use sp_core::{ConstU64, H256};
 use sp_runtime::{
-	traits::{BlakeTwo256, IdentityLookup},
+	traits::{BlakeTwo256, BlockNumberProvider, IdentityLookup},
 	BuildStorage,
 };
 
@@ -95,6 +95,18 @@ impl StateMachineHeightProvider for MockStateMachineHeightProvider {
 	}
 }
 
+parameter_types! {
+	pub static RelayBlockNumber: u64 = 0;
+}
+
+pub struct RelayBlockNumberProvider;
+impl BlockNumberProvider for RelayBlockNumberProvider {
+	type BlockNumber = u64;
+	fn current_block_number() -> Self::BlockNumber {
+		RelayBlockNumber::get()
+	}
+}
+
 impl crate::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type Currency = Balances;
@@ -103,6 +115,8 @@ impl crate::Config for Test {
 	type StateMachineHeightProvider = MockStateMachineHeightProvider;
 	type Timeout = ConstU64<1000>;
 	type UnsignedPriority = RegionsUnsignedPriority;
+	type RCBlockNumberProvider = RelayBlockNumberProvider;
+	type TimeslicePeriod = ConstU64<80>;
 	type WeightInfo = ();
 }
 

pallets/regions/src/tests.rs

@@ -529,3 +529,45 @@ fn utils_read_value_works() {
 		);
 	});
 }
+
+#[test]
+fn drop_region_works() {
+	new_test_ext().execute_with(|| {
+		let region_id = RegionId { begin: 1, core: 81, mask: CoreMask::complete() };
+		let record: RegionRecordOf<Test> = RegionRecord { end: 10, owner: 1, paid: None };
+		let who = 1u32.into();
+
+		assert_noop!(
+			Regions::drop_region(RuntimeOrigin::signed(who), region_id),
+			Error::<Test>::UnknownRegion
+		);
+
+		assert_ok!(Regions::mint_into(&region_id.into(), &2));
+		// region status = Unavailable
+		assert_noop!(
+			Regions::drop_region(RuntimeOrigin::signed(who), region_id),
+			Error::<Test>::NotAvailable
+		);
+
+		assert_ok!(Regions::request_region_record(RuntimeOrigin::none(), region_id));
+		// region status = Pending
+		assert_noop!(
+			Regions::drop_region(RuntimeOrigin::signed(who), region_id),
+			Error::<Test>::NotAvailable
+		);
+
+		assert_ok!(Regions::set_record(region_id, record.clone()));
+
+		assert_noop!(
+			Regions::drop_region(RuntimeOrigin::signed(who), region_id),
+			Error::<Test>::RegionNotExpired
+		);
+
+		RelayBlockNumber::set(11 * 80);
+		assert_ok!(Regions::drop_region(RuntimeOrigin::signed(who), region_id));
+
+		assert!(Regions::regions(region_id).is_none());
+
+		System::assert_last_event(Event::RegionDropped { region_id, who }.into());
+	})
+}

pallets/regions/src/weights.rs

@@ -55,6 +55,7 @@ use core::marker::PhantomData;
 pub trait WeightInfo {
 	fn transfer() -> Weight;
 	fn request_region_record() -> Weight;
+	fn drop_region() -> Weight;
 	fn on_accept() -> Weight;
 	fn on_response() -> Weight;
 	fn on_timeout() -> Weight;
@@ -95,6 +96,21 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 			.saturating_add(T::DbWeight::get().reads(6_u64))
 			.saturating_add(T::DbWeight::get().writes(3_u64))
 	}
+	/// Storage: `Regions::Regions` (r:1 w:1)
+	/// Proof: `Regions::Regions` (`max_values`: None, `max_size`: Some(119), added: 2594, mode: `MaxEncodedLen`)
+	/// Storage: `ParachainSystem::ValidationData` (r:1 w:0)
+	/// Proof: `ParachainSystem::ValidationData` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	/// Storage: `ParachainSystem::LastRelayChainBlockNumber` (r:1 w:0)
+	/// Proof: `ParachainSystem::LastRelayChainBlockNumber` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	fn drop_region() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `249`
+		//  Estimated: `3584`
+		// Minimum execution time: 19_555_000 picoseconds.
+		Weight::from_parts(20_101_000, 3584)
+			.saturating_add(T::DbWeight::get().reads(3_u64))
+			.saturating_add(T::DbWeight::get().writes(1_u64))
+	}
 	fn on_accept() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `0`
@@ -160,6 +176,21 @@ impl WeightInfo for () {
 			.saturating_add(RocksDbWeight::get().reads(6_u64))
 			.saturating_add(RocksDbWeight::get().writes(3_u64))
 	}
+	/// Storage: `Regions::Regions` (r:1 w:1)
+	/// Proof: `Regions::Regions` (`max_values`: None, `max_size`: Some(119), added: 2594, mode: `MaxEncodedLen`)
+	/// Storage: `ParachainSystem::ValidationData` (r:1 w:0)
+	/// Proof: `ParachainSystem::ValidationData` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	/// Storage: `ParachainSystem::LastRelayChainBlockNumber` (r:1 w:0)
+	/// Proof: `ParachainSystem::LastRelayChainBlockNumber` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	fn drop_region() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `249`
+		//  Estimated: `3584`
+		// Minimum execution time: 19_555_000 picoseconds.
+		Weight::from_parts(20_101_000, 3584)
+			.saturating_add(RocksDbWeight::get().reads(3_u64))
+			.saturating_add(RocksDbWeight::get().writes(1_u64))
+	}
 	fn on_accept() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `0`

runtime/cocos/src/lib.rs

@@ -622,6 +622,8 @@ impl pallet_regions::Config for Runtime {
 	type IsmpDispatcher = Ismp;
 	type StateMachineHeightProvider = StateMachineHeightProvider;
 	type Timeout = ConstU64<300>; // 5 minutes
+	type RCBlockNumberProvider = RelaychainDataProvider<Self>;
+	type TimeslicePeriod = ConstU32<80>;
 	type UnsignedPriority = RegionsUnsignedPriority;
 	type WeightInfo = weights::pallet_regions::WeightInfo<Runtime>;
 }