From 677422c22b176b1e06df837de68d3b4e15f82696 Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Fri, 22 Nov 2024 17:16:44 +0100 Subject: [PATCH 1/4] started work on migration, stuck on call error --- polkadot/runtime/common/src/auctions.rs | 51 +++++++++++++++++-------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 78f20d918bab..9ff138bd971a 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -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,34 @@ 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 { + + #[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()); @@ -1839,15 +1848,20 @@ 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 { + + #[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 { + #[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()); @@ -1886,16 +1900,19 @@ mod benchmarking { } } - }: { + #[extrinsic_call] Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); - } verify { + 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 +1920,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)?; @@ -1920,9 +1936,12 @@ mod benchmarking { 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!( From 047fb150e96f44816e5324eb196270abe92c36fb Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Fri, 22 Nov 2024 18:04:14 +0100 Subject: [PATCH 2/4] fix error and fmt --- polkadot/runtime/common/src/auctions.rs | 82 ++++++++++++++++--------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 9ff138bd971a..6c55556ecd21 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); } } @@ -492,8 +492,8 @@ impl Pallet { let mut outgoing_winner = Some((bidder.clone(), para, amount)); swap(&mut current_winning[range_index], &mut outgoing_winner); if let Some((who, para, _amount)) = outgoing_winner { - if auction_status.is_starting() && - current_winning + if auction_status.is_starting() + && current_winning .iter() .filter_map(Option::as_ref) .all(|&(ref other, other_para, _)| other != &who || other_para != para) @@ -543,8 +543,8 @@ impl Pallet { &mut raw_offset.as_ref(), ) .expect("secure hashes should always be bigger than the block number; qed"); - let offset = (raw_offset_block_number % ending_period) / - T::SampleLength::get().max(One::one()); + let offset = (raw_offset_block_number % ending_period) + / T::SampleLength::get().max(One::one()); let auction_counter = AuctionCounter::::get(); Self::deposit_event(Event::::WinningOffset { @@ -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)); } } } @@ -592,9 +592,9 @@ impl Pallet { let period_count = LeasePeriodOf::::from(range.len() as u32); match T::Leaser::lease_out(para, &leaser, amount, period_begin, period_count) { - Err(LeaseError::ReserveFailed) | - Err(LeaseError::AlreadyEnded) | - Err(LeaseError::NoLeasePeriod) => { + Err(LeaseError::ReserveFailed) + | Err(LeaseError::AlreadyEnded) + | Err(LeaseError::NoLeasePeriod) => { // Should never happen since we just unreserved this amount (and our offset is // from the present period). But if it does, there's not much we can do. }, @@ -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 }); } @@ -1782,17 +1782,20 @@ mod benchmarking { 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)?; + let origin = T::InitiateOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; - #[extrinsic_call] + #[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()); + assert_last_event::( + Event::::AuctionStarted { + auction_index: AuctionCounter::::get(), + lease_period: LeasePeriodOf::::max_value(), + ending: BlockNumberFor::::max_value(), + } + .into(), + ); Ok(()) } @@ -1819,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, @@ -1850,7 +1863,14 @@ mod benchmarking { assert_eq!(CurrencyOf::::reserved_balance(&first_bidder), first_amount); #[extrinsic_call] - _(RawOrigin::Signed(caller.clone()), new_para, auction_index, first_slot, last_slot, bigger_amount); + _( + 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); @@ -1882,7 +1902,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()); } @@ -1896,12 +1916,18 @@ 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, + ); } } - #[extrinsic_call] - Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); + #[block] + { + let _ = Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); + } let auction_index = AuctionCounter::::get(); assert_last_event::(Event::::AuctionClosed { auction_index }.into()); @@ -1932,7 +1958,7 @@ 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()); From fccd816b25a4150c11c90be06284ef28306b1506 Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Wed, 27 Nov 2024 16:03:52 +0100 Subject: [PATCH 3/4] nightly fmt --- polkadot/runtime/common/src/auctions.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 6c55556ecd21..8194b0430855 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -492,8 +492,8 @@ impl Pallet { let mut outgoing_winner = Some((bidder.clone(), para, amount)); swap(&mut current_winning[range_index], &mut outgoing_winner); if let Some((who, para, _amount)) = outgoing_winner { - if auction_status.is_starting() - && current_winning + if auction_status.is_starting() && + current_winning .iter() .filter_map(Option::as_ref) .all(|&(ref other, other_para, _)| other != &who || other_para != para) @@ -543,8 +543,8 @@ impl Pallet { &mut raw_offset.as_ref(), ) .expect("secure hashes should always be bigger than the block number; qed"); - let offset = (raw_offset_block_number % ending_period) - / T::SampleLength::get().max(One::one()); + let offset = (raw_offset_block_number % ending_period) / + T::SampleLength::get().max(One::one()); let auction_counter = AuctionCounter::::get(); Self::deposit_event(Event::::WinningOffset { @@ -592,9 +592,9 @@ impl Pallet { let period_count = LeasePeriodOf::::from(range.len() as u32); match T::Leaser::lease_out(para, &leaser, amount, period_begin, period_count) { - Err(LeaseError::ReserveFailed) - | Err(LeaseError::AlreadyEnded) - | Err(LeaseError::NoLeasePeriod) => { + Err(LeaseError::ReserveFailed) | + Err(LeaseError::AlreadyEnded) | + Err(LeaseError::NoLeasePeriod) => { // Should never happen since we just unreserved this amount (and our offset is // from the present period). But if it does, there's not much we can do. }, @@ -1872,14 +1872,15 @@ mod benchmarking { bigger_amount, ); - // Confirms that we unreserved funds from a previous bidder, which is worst case scenario. + // 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. + // 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. From 3f0ad42d377595bcbba08d9e358e001a97caee20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 10 Dec 2024 22:37:24 +0100 Subject: [PATCH 4/4] Update polkadot/runtime/common/src/auctions.rs --- polkadot/runtime/common/src/auctions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/runtime/common/src/auctions.rs b/polkadot/runtime/common/src/auctions.rs index 8194b0430855..3ac1ba2dc4e0 100644 --- a/polkadot/runtime/common/src/auctions.rs +++ b/polkadot/runtime/common/src/auctions.rs @@ -1927,7 +1927,7 @@ mod benchmarking { #[block] { - let _ = Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); + Auctions::::on_initialize(duration + now + T::EndingPeriod::get()); } let auction_index = AuctionCounter::::get();