From 9dcdf8133cbae2184caffd9070d18c1217ac5194 Mon Sep 17 00:00:00 2001 From: Ludovic_Domingues Date: Wed, 11 Dec 2024 02:52:29 +0100 Subject: [PATCH] Migration of polkadot-runtime-common auctions benchmarking to v2 (#6613) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Migrated polkadot-runtime-common auctions benchmarking to the new benchmarking syntax v2. This is part of #6202 --------- Co-authored-by: Giuseppe Re Co-authored-by: Bastian Köcher --- polkadot/runtime/common/src/auctions.rs | 120 ++++++++++++++++-------- 1 file changed, 83 insertions(+), 37 deletions(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 78f20d918bab..3ac1ba2dc4e0 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -339,10 +339,10 @@ impl Auctioneer> for Pallet { let sample_length = T::SampleLength::get().max(One::one()); let sample = after_early_end / sample_length; let sub_sample = after_early_end % sample_length; - return AuctionStatus::EndingPeriod(sample, sub_sample) + return AuctionStatus::EndingPeriod(sample, sub_sample); } else { // This is safe because of the comparison operator above - return AuctionStatus::VrfDelay(after_early_end - ending_period) + return AuctionStatus::VrfDelay(after_early_end - ending_period); } } @@ -559,7 +559,7 @@ impl Pallet { #[allow(deprecated)] Winning::::remove_all(None); AuctionInfo::::kill(); - return Some((res, lease_period_index)) + return Some((res, lease_period_index)); } } } @@ -765,11 +765,11 @@ mod tests { let (current_lease_period, _) = Self::lease_period_index(now).ok_or(LeaseError::NoLeasePeriod)?; if period_begin < current_lease_period { - return Err(LeaseError::AlreadyEnded) + return Err(LeaseError::AlreadyEnded); } for period in period_begin..(period_begin + period_count) { if leases.contains_key(&(para, period)) { - return Err(LeaseError::AlreadyLeased) + return Err(LeaseError::AlreadyLeased); } leases.insert((para, period), LeaseData { leaser: *leaser, amount }); } @@ -1718,7 +1718,7 @@ mod benchmarking { use polkadot_runtime_parachains::paras; use sp_runtime::{traits::Bounded, SaturatedConversion}; - use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; + use frame_benchmarking::v2::*; fn assert_last_event(generic_event: ::RuntimeEvent) { let events = frame_system::Pallet::::events(); @@ -1772,25 +1772,37 @@ mod benchmarking { } } - benchmarks! { - where_clause { where T: pallet_babe::Config + paras::Config } + #[benchmarks( + where T: pallet_babe::Config + paras::Config, + )] + mod benchmarks { + use super::*; - new_auction { + #[benchmark] + fn new_auction() -> Result<(), BenchmarkError> { let duration = BlockNumberFor::::max_value(); let lease_period_index = LeasePeriodOf::::max_value(); - let origin = - T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(origin, duration, lease_period_index) - verify { - assert_last_event::(Event::::AuctionStarted { - auction_index: AuctionCounter::::get(), - lease_period: LeasePeriodOf::::max_value(), - ending: BlockNumberFor::::max_value(), - }.into()); + let origin = T::InitiateOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, duration, lease_period_index); + + assert_last_event::( + Event::::AuctionStarted { + auction_index: AuctionCounter::::get(), + lease_period: LeasePeriodOf::::max_value(), + ending: BlockNumberFor::::max_value(), + } + .into(), + ); + + Ok(()) } // Worst case scenario a new bid comes in which kicks out an existing bid for the same slot. - bid { + #[benchmark] + fn bid() -> Result<(), BenchmarkError> { // If there is an offset, we need to be on that block to be able to do lease things. let (_, offset) = T::Leaser::lease_period_length(); frame_system::Pallet::::set_block_number(offset + One::one()); @@ -1810,8 +1822,18 @@ mod benchmarking { CurrencyOf::::make_free_balance_be(&owner, BalanceOf::::max_value()); let worst_head_data = T::Registrar::worst_head_data(); let worst_validation_code = T::Registrar::worst_validation_code(); - T::Registrar::register(owner.clone(), para, worst_head_data.clone(), worst_validation_code.clone())?; - T::Registrar::register(owner, new_para, worst_head_data, worst_validation_code.clone())?; + T::Registrar::register( + owner.clone(), + para, + worst_head_data.clone(), + worst_validation_code.clone(), + )?; + T::Registrar::register( + owner, + new_para, + worst_head_data, + worst_validation_code.clone(), + )?; assert_ok!(paras::Pallet::::add_trusted_validation_code( frame_system::Origin::::Root.into(), worst_validation_code, @@ -1839,15 +1861,28 @@ mod benchmarking { CurrencyOf::::make_free_balance_be(&caller, BalanceOf::::max_value()); let bigger_amount = CurrencyOf::::minimum_balance().saturating_mul(10u32.into()); assert_eq!(CurrencyOf::::reserved_balance(&first_bidder), first_amount); - }: _(RawOrigin::Signed(caller.clone()), new_para, auction_index, first_slot, last_slot, bigger_amount) - verify { - // Confirms that we unreserved funds from a previous bidder, which is worst case scenario. + + #[extrinsic_call] + _( + RawOrigin::Signed(caller.clone()), + new_para, + auction_index, + first_slot, + last_slot, + bigger_amount, + ); + + // Confirms that we unreserved funds from a previous bidder, which is worst case + // scenario. assert_eq!(CurrencyOf::::reserved_balance(&caller), bigger_amount); + + Ok(()) } - // Worst case: 10 bidders taking all wining spots, and we need to calculate the winner for auction end. - // Entire winner map should be full and removed at the end of the benchmark. - on_initialize { + // Worst case: 10 bidders taking all wining spots, and we need to calculate the winner for + // auction end. Entire winner map should be full and removed at the end of the benchmark. + #[benchmark] + fn on_initialize() -> Result<(), BenchmarkError> { // If there is an offset, we need to be on that block to be able to do lease things. let (lease_length, offset) = T::Leaser::lease_period_length(); frame_system::Pallet::::set_block_number(offset + One::one()); @@ -1868,7 +1903,7 @@ mod benchmarking { let winning_data = Winning::::get(BlockNumberFor::::from(0u32)).unwrap(); // Make winning map full - for i in 0u32 .. (T::EndingPeriod::get() / T::SampleLength::get()).saturated_into() { + for i in 0u32..(T::EndingPeriod::get() / T::SampleLength::get()).saturated_into() { Winning::::insert(BlockNumberFor::::from(i), winning_data.clone()); } @@ -1882,20 +1917,29 @@ mod benchmarking { let authorities = pallet_babe::Pallet::::authorities(); // Check for non empty authority set since it otherwise emits a No-OP warning. if !authorities.is_empty() { - pallet_babe::Pallet::::enact_epoch_change(authorities.clone(), authorities, None); + pallet_babe::Pallet::::enact_epoch_change( + authorities.clone(), + authorities, + None, + ); } } - }: { - Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); - } verify { + #[block] + { + Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); + } + let auction_index = AuctionCounter::::get(); assert_last_event::(Event::::AuctionClosed { auction_index }.into()); assert!(Winning::::iter().count().is_zero()); + + Ok(()) } // Worst case: 10 bidders taking all wining spots, and winning data is full. - cancel_auction { + #[benchmark] + fn cancel_auction() -> Result<(), BenchmarkError> { // If there is an offset, we need to be on that block to be able to do lease things. let (lease_length, offset) = T::Leaser::lease_period_length(); frame_system::Pallet::::set_block_number(offset + One::one()); @@ -1903,7 +1947,6 @@ mod benchmarking { // Create a new auction let duration: BlockNumberFor = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); - let now = frame_system::Pallet::::block_number(); let origin = T::InitiateOrigin::try_successful_origin() .expect("InitiateOrigin has no successful origin required for the benchmark"); Auctions::::new_auction(origin, duration, lease_period_index)?; @@ -1916,13 +1959,16 @@ mod benchmarking { } // Make winning map full - for i in 0u32 .. (T::EndingPeriod::get() / T::SampleLength::get()).saturated_into() { + for i in 0u32..(T::EndingPeriod::get() / T::SampleLength::get()).saturated_into() { Winning::::insert(BlockNumberFor::::from(i), winning_data.clone()); } assert!(AuctionInfo::::get().is_some()); - }: _(RawOrigin::Root) - verify { + + #[extrinsic_call] + _(RawOrigin::Root); + assert!(AuctionInfo::::get().is_none()); + Ok(()) } impl_benchmark_test_suite!(