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

Don't allow bids for a ParaId where there is an overlapping lease period #3361

Merged
18 commits merged into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
53 changes: 53 additions & 0 deletions runtime/common/src/auctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ pub mod pallet {
NotAuction,
/// Auction has already ended.
AuctionEnded,
/// The para is already leased out for part of this range.
AlreadyLeasedOut,
}

/// Number of auctions started so far.
Expand Down Expand Up @@ -418,6 +420,9 @@ impl<T: Config> Pallet<T> {
let now = frame_system::Pallet::<T>::block_number();
ensure!(now < late_end, Error::<T>::AuctionEnded);

// We also make sure that the bid is not for any existing leases the para already has.
ensure!(!T::Leaser::already_leased(para, first_slot, last_slot), Error::<T>::AlreadyLeasedOut);

// Our range.
let range = SlotRange::new_bounded(first_lease_period, first_slot, last_slot)?;
// Range as an array index.
Expand Down Expand Up @@ -753,6 +758,18 @@ mod tests {
fn lease_period_index() -> Self::LeasePeriod {
(System::block_number() / Self::lease_period()).into()
}

fn already_leased(
para_id: ParaId,
first_period: Self::LeasePeriod,
last_period: Self::LeasePeriod
) -> bool {
leases().into_iter().any(|((para, period), _data)| {
para == para_id &&
first_period <= period &&
period <= last_period
})
}
}

ord_parameter_types!{
Expand Down Expand Up @@ -1325,6 +1342,42 @@ mod tests {
});
}

#[test]
fn handle_bid_checks_existing_lease_periods() {
new_test_ext().execute_with(|| {
run_to_block(1);
assert_ok!(Auctions::new_auction(Origin::signed(6), 5, 1));
assert_ok!(Auctions::bid(Origin::signed(1), 0.into(), 1, 2, 3, 1));
assert_eq!(Balances::reserved_balance(1), 1);
assert_eq!(Balances::free_balance(1), 9);
run_to_block(9);

assert_eq!(leases(), vec![
((0.into(), 2), LeaseData { leaser: 1, amount: 1 }),
((0.into(), 3), LeaseData { leaser: 1, amount: 1 }),
]);
assert_eq!(TestLeaser::deposit_held(0.into(), &1), 1);

// Para 1 just won an auction above and won some lease periods.
// No bids can work which overlap these periods.
assert_ok!(Auctions::new_auction(Origin::signed(6), 5, 1));
assert_noop!(
Auctions::bid(Origin::signed(1), 0.into(), 2, 1, 4, 1),
Error::<Test>::AlreadyLeasedOut,
);
assert_noop!(
Auctions::bid(Origin::signed(1), 0.into(), 2, 1, 2, 1),
Error::<Test>::AlreadyLeasedOut,
);
assert_noop!(
Auctions::bid(Origin::signed(1), 0.into(), 2, 3, 4, 1),
Error::<Test>::AlreadyLeasedOut,
);
// This is okay, not an overlapping bid.
assert_ok!(Auctions::bid(Origin::signed(1), 0.into(), 2, 1, 1, 1));
});
}

// Here we will test that taking only 10 samples during the ending period works as expected.
#[test]
fn less_winning_samples_work() {
Expand Down
151 changes: 151 additions & 0 deletions runtime/common/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1092,3 +1092,154 @@ fn gap_bids_work() {
assert_eq!(Balances::reserved_balance(&20), 0);
});
}

// This test verifies that if a parachain already has won some lease periods, that it cannot bid for
// any of those same lease periods again.
#[test]
fn cant_bid_on_existing_lease_periods() {
new_test_ext().execute_with(|| {
assert!(System::block_number().is_one()); // So events are emitted
Balances::make_free_balance_be(&1, 1_000_000_000);
// First register 2 parathreads
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
assert_ok!(Registrar::reserve(Origin::signed(1)));
assert_ok!(Registrar::register(
Origin::signed(1),
ParaId::from(2000),
test_genesis_head(10),
test_validation_code(10),
));

// Start a new auction in the future
let starting_block = System::block_number();
let duration = 99u32;
let lease_period_index_start = 4u32;
assert_ok!(Auctions::new_auction(Origin::root(), duration, lease_period_index_start));

// 2 sessions later they are parathreads
run_to_session(2);

// Open a crowdloan for Para 1 for slots 0-3
assert_ok!(Crowdloan::create(
Origin::signed(1),
ParaId::from(2000),
1_000_000, // Cap
lease_period_index_start + 0, // First Slot
lease_period_index_start + 1, // Last Slot
400, // Long block end
None,
));
let crowdloan_account = Crowdloan::fund_account_id(ParaId::from(2000));

// Bunch of contributions
let mut total = 0;
for i in 10 .. 20 {
Balances::make_free_balance_be(&i, 1_000_000_000);
assert_ok!(Crowdloan::contribute(Origin::signed(i), ParaId::from(2000), 900 - i, None));
total += 900 - i;
}
assert!(total > 0);
assert_eq!(Balances::free_balance(&crowdloan_account), total);

// Finish the auction.
run_to_block(starting_block + 110);

// Appropriate Paras should have won slots
assert_eq!(
slots::Leases::<Test>::get(ParaId::from(2000)),
// -- 1 --- 2 --- 3 --- 4 --- 5 ------------- 6 ------------------------ 7 -------------
vec![None, None, None, Some((crowdloan_account, 8855)), Some((crowdloan_account, 8855))],
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
);

// Let's start another auction for the same range
let starting_block = System::block_number();
let duration = 99u32;
let lease_period_index_start = 4u32;
assert_ok!(Auctions::new_auction(Origin::root(), duration, lease_period_index_start));

// Poke the crowdloan into `NewRaise`
assert_ok!(Crowdloan::poke(Origin::signed(1), ParaId::from(2000)));
assert_eq!(Crowdloan::new_raise(), vec![ParaId::from(2000)]);

// Beginning of ending block.
run_to_block(starting_block + 100);

// Bids cannot be made which intersect
assert_noop!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start + 0,
lease_period_index_start + 1,
100,
), AuctionsError::<Test>::AlreadyLeasedOut,
);

assert_noop!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start + 1,
lease_period_index_start + 2,
100,
), AuctionsError::<Test>::AlreadyLeasedOut,
);

assert_noop!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start - 1,
lease_period_index_start + 0,
100,
), AuctionsError::<Test>::AlreadyLeasedOut,
);

assert_noop!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start + 0,
lease_period_index_start + 0,
100,
), AuctionsError::<Test>::AlreadyLeasedOut,
);

assert_noop!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start + 1,
lease_period_index_start + 1,
100,
), AuctionsError::<Test>::AlreadyLeasedOut,
);

assert_noop!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start - 1,
lease_period_index_start + 5,
100,
), AuctionsError::<Test>::AlreadyLeasedOut,
);

// Will work when not overlapping
assert_ok!(
Auctions::bid(
Origin::signed(crowdloan_account),
ParaId::from(2000),
2,
lease_period_index_start + 2,
lease_period_index_start + 3,
100,
)
);
});
}
39 changes: 38 additions & 1 deletion runtime/common/src/slots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
//! must handled by a separately, through the trait interface that this pallet provides or the root dispatchables.

use sp_std::prelude::*;
use sp_runtime::traits::{CheckedSub, Zero, CheckedConversion};
use sp_runtime::traits::{CheckedSub, Zero, CheckedConversion, Saturating};
use frame_support::{
decl_module, decl_storage, decl_event, decl_error, dispatch::DispatchResult,
traits::{Currency, ReservableCurrency, Get}, weights::Weight,
Expand Down Expand Up @@ -421,6 +421,43 @@ impl<T: Config> Leaser for Module<T> {
fn lease_period_index() -> Self::LeasePeriod {
<frame_system::Pallet<T>>::block_number() / T::LeasePeriod::get()
}

fn already_leased(
para_id: ParaId,
first_period: Self::LeasePeriod,
last_period: Self::LeasePeriod,
) -> bool {
let current_lease_period = Self::lease_period_index();

// Can't look in the past, so we pick whichever is the biggest.
let start_period = first_period.max(current_lease_period);
// Find the offset to look into the lease period list.
// Subtraction is safe because of max above.
let offset = match (start_period - current_lease_period).checked_into::<usize>() {
Some(offset) => offset,
None => return false,
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
};

// This calculates how deep we should look in the vec for a potential lease.
let period_count = match last_period.saturating_sub(start_period).checked_into::<usize>() {
Some(period_count) => period_count,
None => return false,
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
};

// Get the leases, and check each item in the vec which is part of the range we ar checking.
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
let leases = Leases::<T>::get(para_id);
for slot in offset ..= offset + period_count {
if let Some(maybe_lease) = leases.get(slot) {
// If there exists any lease period, we exit early and return true.
if maybe_lease.is_some() {
return true
}
}
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
}

// If we got here, then we did not find any overlapping leases.
false
}
}


Expand Down
8 changes: 8 additions & 0 deletions runtime/common/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ pub trait Leaser {

/// Returns the current lease period.
fn lease_period_index() -> Self::LeasePeriod;

/// Returns true if the parachain already has a lease in any of lease periods in the inclusive
/// range `[first_period, last_period]`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may be slightly incorrect: technically our implementation in slots only can check the current lease period and onwards. So if any periods in first_period..=last_period are in the past we will not be able to look at those. Additionally we will always return false if the entire range is in the past (I don't think this is an issue but just wanted to point it out)

shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
fn already_leased(
emostov marked this conversation as resolved.
Show resolved Hide resolved
para_id: ParaId,
first_period: Self::LeasePeriod,
last_period: Self::LeasePeriod
) -> bool;
}

pub trait Auctioneer {
Expand Down