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

Commit

Permalink
Fix Benchmark Regressions in Polkadot Parachain Auction System (#5139)
Browse files Browse the repository at this point in the history
* integration tests use offset

* fix slots

* fix auctions

* add auctions benchmark

* fix crowdloan
  • Loading branch information
shawntabrizi committed Mar 17, 2022
1 parent 685481b commit f170bd6
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 15 deletions.
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

0 comments on commit f170bd6

Please sign in to comment.