Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix Benchmark Regressions in Polkadot Parachain Auction System #5139

Merged
merged 5 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions runtime/common/src/auctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 2 additions & 2 deletions runtime/common/src/crowdloan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down
36 changes: 26 additions & 10 deletions runtime/common/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
],
);
Expand All @@ -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))
],
);
Expand All @@ -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))
],
);
Expand All @@ -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 -------
Expand All @@ -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 -------
Expand All @@ -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);
Expand Down
10 changes: 9 additions & 1 deletion runtime/common/src/slots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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());
Expand All @@ -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)
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down