Fix Benchmark Regressions in Polkadot Parachain Auction System (#5139)

* integration tests use offset

* fix slots

* fix auctions

* add auctions benchmark

* fix crowdloan

polkadot/runtime/common/src/auctions.rs

@@ -1772,6 +1772,10 @@ mod benchmarking {
 
 		// Worst case scenario a new bid comes in which kicks out an existing bid for the same slot.
 		bid {
+			// 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::<T>::set_block_number(offset + One::one());
+
 			// Create a new auction
 			let duration = T::BlockNumber::max_value();
 			let lease_period_index = LeasePeriodOf::<T>::zero();
@@ -1818,8 +1822,12 @@ mod benchmarking {
 		// 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 {
+			// 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::<T>::set_block_number(offset + One::one());
+
 			// Create a new auction
-			let duration: T::BlockNumber = 99u32.into();
+			let duration: T::BlockNumber = lease_length / 2u32.into();
 			let lease_period_index = LeasePeriodOf::<T>::zero();
 			let now = frame_system::Pallet::<T>::block_number();
 			Auctions::<T>::new_auction(RawOrigin::Root.into(), duration, lease_period_index)?;
@@ -1857,8 +1865,12 @@ mod benchmarking {
 
 		// Worst case: 10 bidders taking all wining spots, and winning data is full.
 		cancel_auction {
+			// 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::<T>::set_block_number(offset + One::one());
+
 			// Create a new auction
-			let duration: T::BlockNumber = 99u32.into();
+			let duration: T::BlockNumber = lease_length / 2u32.into();
 			let lease_period_index = LeasePeriodOf::<T>::zero();
 			let now = frame_system::Pallet::<T>::block_number();
 			Auctions::<T>::new_auction(RawOrigin::Root.into(), duration, lease_period_index)?;

polkadot/runtime/common/src/crowdloan/mod.rs

@@ -2014,7 +2014,7 @@ mod benchmarking {
 			let caller: T::AccountId = whitelisted_caller();
 			let contributor = account("contributor", 0, 0);
 			contribute_fund::<T>(&contributor, fund_index);
-			frame_system::Pallet::<T>::set_block_number(200u32.into());
+			frame_system::Pallet::<T>::set_block_number(T::BlockNumber::max_value());
 		}: _(RawOrigin::Signed(caller), contributor.clone(), fund_index)
 		verify {
 			assert_last_event::<T>(Event::<T>::Withdrew(contributor, fund_index, T::MinContribution::get()).into());
@@ -2034,7 +2034,7 @@ mod benchmarking {
 			}
 
 			let caller: T::AccountId = whitelisted_caller();
-			frame_system::Pallet::<T>::set_block_number(200u32.into());
+			frame_system::Pallet::<T>::set_block_number(T::BlockNumber::max_value());
 		}: _(RawOrigin::Signed(caller), fund_index)
 		verify {
 			assert_last_event::<T>(Event::<T>::AllRefunded(fund_index).into());

polkadot/runtime/common/src/integration_tests.rs

@@ -239,7 +239,7 @@ impl auctions::Config for Test {
 
 parameter_types! {
 	pub const LeasePeriod: BlockNumber = 100;
-	pub static LeaseOffset: BlockNumber = 0;
+	pub static LeaseOffset: BlockNumber = 5;
 }
 
 impl slots::Config for Test {
@@ -321,6 +321,11 @@ fn para_origin(id: u32) -> ParaOrigin {
 	ParaOrigin::Parachain(id.into())
 }
 
+fn add_blocks(n: u32) {
+	let block_number = System::block_number();
+	run_to_block(block_number + n);
+}
+
 fn run_to_block(n: u32) {
 	assert!(System::block_number() < n);
 	while System::block_number() < n {
@@ -1307,19 +1312,25 @@ fn gap_bids_work() {
 		));
 
 		// Finish the auction
-		run_to_block(110);
+		run_to_block(110 + LeaseOffset::get());
 
 		// Should have won the lease periods
 		assert_eq!(
 			slots::Leases::<Test>::get(ParaId::from(2000)),
-			// -- 1 --- 2 --- 3 ---------- 4 -------------- 5 -------------- 6 -------------- 7 -------
 			vec![
+				// LP 1
 				None,
+				// LP 2
 				None,
+				// LP 3
 				None,
+				// LP 4
 				Some((account_id(10), 100)),
+				// LP 5
 				Some((account_id(20), 800)),
+				// LP 6
 				Some((account_id(20), 200)),
+				// LP 7
 				Some((account_id(10), 400))
 			],
 		);
@@ -1330,14 +1341,17 @@ fn gap_bids_work() {
 
 		// Progress through the leases and note the correct amount of balance is reserved.
 
-		run_to_block(400);
+		add_blocks(300 + LeaseOffset::get());
 		assert_eq!(
 			slots::Leases::<Test>::get(ParaId::from(2000)),
-			// --------- 4 -------------- 5 -------------- 6 -------------- 7 -------
 			vec![
+				// LP 4
 				Some((account_id(10), 100)),
+				// LP 5
 				Some((account_id(20), 800)),
+				// LP 6
 				Some((account_id(20), 200)),
+				// LP 7
 				Some((account_id(10), 400))
 			],
 		);
@@ -1346,13 +1360,15 @@ fn gap_bids_work() {
 		assert_eq!(Balances::reserved_balance(&account_id(20)), 800);
 
 		// Lease period 4 is done, but nothing is unreserved since user 1 has a debt on lease 7
-		run_to_block(500);
+		add_blocks(100);
 		assert_eq!(
 			slots::Leases::<Test>::get(ParaId::from(2000)),
-			// --------- 5 -------------- 6 -------------- 7 -------
 			vec![
+				// LP 5
 				Some((account_id(20), 800)),
+				// LP 6
 				Some((account_id(20), 200)),
+				// LP 7
 				Some((account_id(10), 400))
 			],
 		);
@@ -1361,7 +1377,7 @@ fn gap_bids_work() {
 		assert_eq!(Balances::reserved_balance(&account_id(20)), 800);
 
 		// Lease period 5 is done, and 20 will unreserve down to 200.
-		run_to_block(600);
+		add_blocks(100);
 		assert_eq!(
 			slots::Leases::<Test>::get(ParaId::from(2000)),
 			// --------- 6 -------------- 7 -------
@@ -1371,7 +1387,7 @@ fn gap_bids_work() {
 		assert_eq!(Balances::reserved_balance(&account_id(20)), 200);
 
 		// Lease period 6 is done, and 20 will unreserve everything.
-		run_to_block(700);
+		add_blocks(100);
 		assert_eq!(
 			slots::Leases::<Test>::get(ParaId::from(2000)),
 			// --------- 7 -------
@@ -1381,7 +1397,7 @@ fn gap_bids_work() {
 		assert_eq!(Balances::reserved_balance(&account_id(20)), 0);
 
 		// All leases are done. Everything is unreserved.
-		run_to_block(800);
+		add_blocks(100);
 		assert_eq!(slots::Leases::<Test>::get(ParaId::from(2000)), vec![]);
 		assert_eq!(Balances::reserved_balance(&account_id(10)), 0);
 		assert_eq!(Balances::reserved_balance(&account_id(20)), 0);

polkadot/runtime/common/src/slots.rs

@@ -981,7 +981,7 @@ mod benchmarking {
 	use super::*;
 	use frame_support::assert_ok;
 	use frame_system::RawOrigin;
-	use sp_runtime::traits::Bounded;
+	use sp_runtime::traits::{Bounded, One};
 
 	use frame_benchmarking::{account, benchmarks, whitelisted_caller};
 
@@ -1015,6 +1015,8 @@ mod benchmarking {
 
 	benchmarks! {
 		force_lease {
+			// If there is an offset, we need to be on that block to be able to do lease things.
+			frame_system::Pallet::<T>::set_block_number(T::LeaseOffset::get() + One::one());
 			let para = ParaId::from(1337);
 			let leaser: T::AccountId = account("leaser", 0, 0);
 			T::Currency::make_free_balance_be(&leaser, BalanceOf::<T>::max_value());
@@ -1035,6 +1037,9 @@ mod benchmarking {
 			let period_begin = 1u32.into();
 			let period_count = 4u32.into();
 
+			// If there is an offset, we need to be on that block to be able to do lease things.
+			frame_system::Pallet::<T>::set_block_number(T::LeaseOffset::get() + One::one());
+
 			// Make T parathreads
 			let paras_info = (0..t).map(|i| {
 				register_a_parathread::<T>(i)
@@ -1085,6 +1090,9 @@ mod benchmarking {
 			let max_people = 8;
 			let (para, _) = register_a_parathread::<T>(1);
 
+			// If there is an offset, we need to be on that block to be able to do lease things.
+			frame_system::Pallet::<T>::set_block_number(T::LeaseOffset::get() + One::one());
+
 			for i in 0 .. max_people {
 				let leaser = account("lease_deposit", i, 0);
 				let amount = T::Currency::minimum_balance();

polkadot/runtime/polkadot/src/lib.rs

@@ -1696,6 +1696,7 @@ mod benches {
 		// Polkadot
 		// NOTE: Make sure to prefix these with `runtime_common::` so
 		// the that path resolves correctly in the generated file.
+		[runtime_common::auctions, Auctions]
 		[runtime_common::claims, Claims]
 		[runtime_common::crowdloan, Crowdloan]
 		[runtime_common::slots, Slots]