From d236732b3fa27ec9ce8bc7a99b13d3f8efa6be4b Mon Sep 17 00:00:00 2001 From: cuteolaf Date: Fri, 9 Aug 2024 03:59:35 -0700 Subject: [PATCH] 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 --- pallets/market/src/mock.rs | 2 + pallets/processor/src/mock.rs | 26 +++---- pallets/regions/src/benchmarking.rs | 20 ++++++ pallets/regions/src/lib.rs | 77 +++++++++++++++++++-- pallets/regions/src/mock.rs | 16 ++++- pallets/regions/src/tests.rs | 42 +++++++++++ pallets/regions/src/weights.rs | 31 +++++++++ runtime/cocos/src/lib.rs | 2 + runtime/cocos/src/weights/pallet_regions.rs | 37 +++++++--- 9 files changed, 222 insertions(+), 31 deletions(-) diff --git a/pallets/market/src/mock.rs b/pallets/market/src/mock.rs index cb9fe790..f43f8909 100644 --- a/pallets/market/src/mock.rs +++ b/pallets/market/src/mock.rs @@ -139,6 +139,8 @@ impl pallet_regions::Config for Test { type IsmpDispatcher = MockDispatcher; type StateMachineHeightProvider = MockStateMachineHeightProvider; type Timeout = ConstU64<1000>; + type RCBlockNumberProvider = RelayBlockNumberProvider; + type TimeslicePeriod = ConstU64<80>; type UnsignedPriority = RegionsUnsignedPriority; type WeightInfo = (); } diff --git a/pallets/processor/src/mock.rs b/pallets/processor/src/mock.rs index a860e789..8982b311 100644 --- a/pallets/processor/src/mock.rs +++ b/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 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 for OrderToAccountId { fn convert(order: OrderId) -> AccountId { diff --git a/pallets/regions/src/benchmarking.rs b/pallets/regions/src/benchmarking.rs index 7bdd7a1e..5ad6afed 100644 --- a/pallets/regions/src/benchmarking.rs +++ b/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 = RegionRecord { end: 0, owner, paid: None }; + + assert_ok!(crate::Pallet::::mint_into(®ion_id.into(), &caller)); + assert_ok!(crate::Pallet::::request_region_record(RawOrigin::None.into(), region_id)); + assert_ok!(crate::Pallet::::set_record(region_id, record)); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), region_id); + + assert_last_event::(Event::RegionDropped { region_id, who: caller }.into()); + + Ok(()) + } + #[benchmark] fn on_accept() -> Result<(), BenchmarkError> { let module = IsmpModuleCallback::::default(); diff --git a/pallets/regions/src/lib.rs b/pallets/regions/src/lib.rs index fefb4880..5c49a5c5 100644 --- a/pallets/regions/src/lib.rs +++ b/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 = + <::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; + /// 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>; + /// The ISMP dispatcher. type IsmpDispatcher: IsmpDispatcher> + 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, region_id: RegionId) -> DispatchResult { + let who = ensure_signed(origin)?; + + let region = Regions::::get(region_id).ok_or(Error::::UnknownRegion)?; + + ensure!(region.record.is_available(), Error::::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::::RegionNotExpired); + #[cfg(not(feature = "runtime-benchmarks"))] + ensure!(record.end < current_timeslice, Error::::RegionNotExpired); + + Regions::::remove(region_id); + + Self::deposit_event(Event::RegionDropped { region_id, who }); + Ok(()) + } else { + Err(Error::::NotAvailable.into()) + } + } } impl Pallet { @@ -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] diff --git a/pallets/regions/src/mock.rs b/pallets/regions/src/mock.rs index 14156d49..8ede1f27 100644 --- a/pallets/regions/src/mock.rs +++ b/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 = (); } diff --git a/pallets/regions/src/tests.rs b/pallets/regions/src/tests.rs index d4d460e1..616b174d 100644 --- a/pallets/regions/src/tests.rs +++ b/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 = RegionRecord { end: 10, owner: 1, paid: None }; + let who = 1u32.into(); + + assert_noop!( + Regions::drop_region(RuntimeOrigin::signed(who), region_id), + Error::::UnknownRegion + ); + + assert_ok!(Regions::mint_into(®ion_id.into(), &2)); + // region status = Unavailable + assert_noop!( + Regions::drop_region(RuntimeOrigin::signed(who), region_id), + Error::::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::::NotAvailable + ); + + assert_ok!(Regions::set_record(region_id, record.clone())); + + assert_noop!( + Regions::drop_region(RuntimeOrigin::signed(who), region_id), + Error::::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()); + }) +} diff --git a/pallets/regions/src/weights.rs b/pallets/regions/src/weights.rs index cf003ee9..17cd6b3d 100644 --- a/pallets/regions/src/weights.rs +++ b/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 WeightInfo for SubstrateWeight { .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` diff --git a/runtime/cocos/src/lib.rs b/runtime/cocos/src/lib.rs index d890f04e..1711a2e0 100644 --- a/runtime/cocos/src/lib.rs +++ b/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; + type TimeslicePeriod = ConstU32<80>; type UnsignedPriority = RegionsUnsignedPriority; type WeightInfo = weights::pallet_regions::WeightInfo; } diff --git a/runtime/cocos/src/weights/pallet_regions.rs b/runtime/cocos/src/weights/pallet_regions.rs index 0b424104..3bfb5227 100644 --- a/runtime/cocos/src/weights/pallet_regions.rs +++ b/runtime/cocos/src/weights/pallet_regions.rs @@ -16,7 +16,7 @@ //! Autogenerated weights for `pallet_regions` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 -//! DATE: 2024-08-04, STEPS: `20`, REPEAT: `50`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-08-09, STEPS: `20`, REPEAT: `50`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `cocos-instance-1`, CPU: `Intel(R) Xeon(R) CPU @ 2.80GHz` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("cocos")`, DB CACHE: `1024` @@ -60,8 +60,8 @@ impl pallet_regions::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `135` // Estimated: `3584` - // Minimum execution time: 15_790_000 picoseconds. - Weight::from_parts(16_278_000, 3584) + // Minimum execution time: 15_823_000 picoseconds. + Weight::from_parts(16_381_000, 3584) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -81,17 +81,32 @@ impl pallet_regions::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `527` // Estimated: `3992` - // Minimum execution time: 48_966_000 picoseconds. - Weight::from_parts(50_182_000, 3992) + // Minimum execution time: 48_905_000 picoseconds. + Weight::from_parts(49_910_000, 3992) .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` // Estimated: `0` - // Minimum execution time: 780_000 picoseconds. - Weight::from_parts(854_000, 0) + // Minimum execution time: 809_000 picoseconds. + Weight::from_parts(925_000, 0) } /// Storage: `Regions::Regions` (r:1 w:1) /// Proof: `Regions::Regions` (`max_values`: None, `max_size`: Some(119), added: 2594, mode: `MaxEncodedLen`) @@ -99,8 +114,8 @@ impl pallet_regions::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `135` // Estimated: `3584` - // Minimum execution time: 13_042_000 picoseconds. - Weight::from_parts(13_403_000, 3584) + // Minimum execution time: 12_846_000 picoseconds. + Weight::from_parts(13_208_000, 3584) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -110,8 +125,8 @@ impl pallet_regions::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `135` // Estimated: `3584` - // Minimum execution time: 8_437_000 picoseconds. - Weight::from_parts(8_824_000, 3584) + // Minimum execution time: 8_408_000 picoseconds. + Weight::from_parts(8_687_000, 3584) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) }