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

Commit

Permalink
turn into an option
Browse files Browse the repository at this point in the history
  • Loading branch information
shawntabrizi committed Oct 5, 2021
1 parent e38c91a commit 3668966
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 63 deletions.
24 changes: 14 additions & 10 deletions runtime/common/src/auctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl<T: Config> Auctioneer<T::BlockNumber> for Pallet<T> {
Self::handle_bid(bidder, para, AuctionCounter::<T>::get(), first_slot, last_slot, amount)
}

fn lease_period_index(b: T::BlockNumber) -> (Self::LeasePeriod, bool) {
fn lease_period_index(b: T::BlockNumber) -> Option<(Self::LeasePeriod, bool)> {
T::Leaser::lease_period_index(b)
}

Expand Down Expand Up @@ -382,10 +382,10 @@ impl<T: Config> Pallet<T> {
let maybe_auction = AuctionInfo::<T>::get();
ensure!(maybe_auction.is_none(), Error::<T>::AuctionInProgress);
let now = frame_system::Pallet::<T>::block_number();
ensure!(
lease_period_index >= T::Leaser::lease_period_index(now).0,
Error::<T>::LeasePeriodInPast
);
if let Some((current_lease_period, _)) = T::Leaser::lease_period_index(now) {
// If there is no active lease period, then we don't need to make this check.
ensure!(lease_period_index >= current_lease_period, Error::<T>::LeasePeriodInPast);
}

// Bump the counter.
let n = AuctionCounter::<T>::mutate(|n| {
Expand Down Expand Up @@ -575,7 +575,9 @@ impl<T: Config> Pallet<T> {
let period_count = LeasePeriodOf::<T>::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::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.
},
Expand Down Expand Up @@ -758,7 +760,9 @@ mod tests {
LEASES.with(|l| {
let mut leases = l.borrow_mut();
let now = System::block_number();
if period_begin < Self::lease_period_index(now).0 {
let (current_lease_period, _) =
Self::lease_period_index(now).ok_or(LeaseError::NoLeasePeriod)?;
if period_begin < current_lease_period {
return Err(LeaseError::AlreadyEnded)
}
for period in period_begin..(period_begin + period_count) {
Expand Down Expand Up @@ -792,14 +796,14 @@ mod tests {
(10, 0)
}

fn lease_period_index(b: BlockNumber) -> (Self::LeasePeriod, bool) {
fn lease_period_index(b: BlockNumber) -> Option<(Self::LeasePeriod, bool)> {
let (lease_period_length, offset) = Self::lease_period_length();
let b = b.saturating_sub(offset);
let b = b.checked_sub(offset)?;

let lease_period = b / lease_period_length;
let first_block = (b % lease_period_length).is_zero();

(lease_period, first_block)
Some((lease_period, first_block))
}

fn already_leased(
Expand Down
28 changes: 17 additions & 11 deletions runtime/common/src/crowdloan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ pub mod pallet {
AlreadyInNewRaise,
/// No contributions allowed during the VRF delay
VrfDelayInProgress,
/// A lease period has not started yet, due to an offset in the starting block.
NoLeasePeriod,
}

#[pallet::hooks]
Expand Down Expand Up @@ -380,7 +382,8 @@ pub mod pallet {
// Here we check the lease period on the ending block is at most the first block of the
// period after `first_period`. If it would be larger, there is no way we could win an
// active auction, thus it would make no sense to have a crowdloan this long.
let (lease_period_at_end, is_first_block) = T::Auctioneer::lease_period_index(end);
let (lease_period_at_end, is_first_block) =
T::Auctioneer::lease_period_index(end).ok_or(Error::<T>::NoLeasePeriod)?;
let adjusted_lease_period_at_end = if is_first_block {
lease_period_at_end.saturating_sub(One::one())
} else {
Expand All @@ -389,10 +392,9 @@ pub mod pallet {
ensure!(adjusted_lease_period_at_end <= first_period, Error::<T>::EndTooFarInFuture);

// Can't start a crowdloan for a lease period that already passed.
ensure!(
first_period >= T::Auctioneer::lease_period_index(now).0,
Error::<T>::FirstPeriodInPast
);
if let Some((current_lease_period, _)) = T::Auctioneer::lease_period_index(now) {
ensure!(first_period >= current_lease_period, Error::<T>::FirstPeriodInPast);
}

// There should not be an existing fund.
ensure!(!Funds::<T>::contains_key(index), Error::<T>::FundNotEnded);
Expand Down Expand Up @@ -454,7 +456,8 @@ pub mod pallet {

// Make sure crowdloan is in a valid lease period
let now = frame_system::Pallet::<T>::block_number();
let current_lease_period = T::Auctioneer::lease_period_index(now).0;
let (current_lease_period, _) =
T::Auctioneer::lease_period_index(now).ok_or(Error::<T>::NoLeasePeriod)?;
ensure!(current_lease_period <= fund.first_period, Error::<T>::ContributionPeriodOver);

// Make sure crowdloan has not already won.
Expand Down Expand Up @@ -766,7 +769,8 @@ impl<T: Config> Pallet<T> {
// `fund.end` can represent the end of a failed crowdloan or the beginning of retirement
// If the current lease period is past the first period they are trying to bid for, then
// it is already too late to win the bid.
let current_lease_period = T::Auctioneer::lease_period_index(now).0;
let (current_lease_period, _) =
T::Auctioneer::lease_period_index(now).ok_or(Error::<T>::NoLeasePeriod)?;
ensure!(
now >= fund.end || current_lease_period > fund.first_period,
Error::<T>::FundNotEnded
Expand Down Expand Up @@ -953,7 +957,9 @@ mod tests {

fn new_auction(duration: u64, lease_period_index: u64) -> DispatchResult {
let now = System::block_number();
assert!(lease_period_index >= Self::lease_period_index(now).0);
let (current_lease_period, _) =
Self::lease_period_index(now).ok_or("no lease period yet")?;
assert!(lease_period_index >= current_lease_period);

let ending = System::block_number().saturating_add(duration);
AUCTION.with(|p| *p.borrow_mut() = Some((lease_period_index, ending)));
Expand Down Expand Up @@ -1006,13 +1012,13 @@ mod tests {
Ok(())
}

fn lease_period_index(b: BlockNumber) -> (u64, bool) {
fn lease_period_index(b: BlockNumber) -> Option<(u64, bool)> {
let (lease_period_length, offset) = Self::lease_period_length();
let b = b.saturating_sub(offset);

let lease_period = b / lease_period_length;
let first_block = (b % lease_period_length).is_zero();
(lease_period, first_block)
Some((lease_period, first_block))
}

fn lease_period_length() -> (u64, u64) {
Expand Down Expand Up @@ -1388,7 +1394,7 @@ mod tests {
assert_ok!(Crowdloan::create(Origin::signed(1), para_3, 1000, 1, 4, 40, None));
run_to_block(40);
let now = System::block_number();
assert_eq!(TestAuctioneer::lease_period_index(now).0, 2);
assert_eq!(TestAuctioneer::lease_period_index(now).unwrap().0, 2);
assert_noop!(
Crowdloan::contribute(Origin::signed(1), para_3, 49, None),
Error::<Test>::ContributionPeriodOver
Expand Down
80 changes: 40 additions & 40 deletions runtime/common/src/slots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,15 @@ pub mod pallet {
#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_initialize(n: T::BlockNumber) -> Weight {
let (lease_period, first_block) = Self::lease_period_index(n);
// If we're beginning a new lease period then handle that.
if first_block {
Self::manage_lease_period_start(lease_period)
} else {
0
if let Some((lease_period, first_block)) = Self::lease_period_index(n) {
// If we're beginning a new lease period then handle that.
if first_block {
return Self::manage_lease_period_start(lease_period)
}
}

// We didn't return early above, so we didn't do anything.
0
}
}

Expand Down Expand Up @@ -337,7 +339,8 @@ impl<T: Config> Leaser<T::BlockNumber> for Pallet<T> {
period_count: Self::LeasePeriod,
) -> Result<(), LeaseError> {
let now = frame_system::Pallet::<T>::block_number();
let current_lease_period = Self::lease_period_index(now).0;
let (current_lease_period, _) =
Self::lease_period_index(now).ok_or(LeaseError::NoLeasePeriod)?;
// Finally, we update the deposit held so it is `amount` for the new lease period
// indices that were won in the auction.
let offset = period_begin
Expand Down Expand Up @@ -436,20 +439,13 @@ impl<T: Config> Leaser<T::BlockNumber> for Pallet<T> {
(T::LeasePeriod::get(), T::LeaseOffset::get())
}

fn lease_period_index(b: T::BlockNumber) -> (Self::LeasePeriod, bool) {
// Note that lease lease period 0 is artificially extended by the lease offset.
let offset_block_now = b.saturating_sub(T::LeaseOffset::get());
fn lease_period_index(b: T::BlockNumber) -> Option<(Self::LeasePeriod, bool)> {
// Note that blocks before `LeaseOffset` do not count as any lease period.
let offset_block_now = b.checked_sub(&T::LeaseOffset::get())?;
let lease_period = offset_block_now / T::LeasePeriod::get();
let first_block = (offset_block_now % T::LeasePeriod::get()).is_zero();

// Special logic to handle lease period 0 being extended by the offset.
let first_block = if lease_period.is_zero() {
// Lease period 0 has their first block on only block 0.
b.is_zero()
} else {
(offset_block_now % T::LeasePeriod::get()).is_zero()
};

(lease_period, first_block)
Some((lease_period, first_block))
}

fn already_leased(
Expand All @@ -458,7 +454,10 @@ impl<T: Config> Leaser<T::BlockNumber> for Pallet<T> {
last_period: Self::LeasePeriod,
) -> bool {
let now = frame_system::Pallet::<T>::block_number();
let current_lease_period = Self::lease_period_index(now).0;
let (current_lease_period, _) = match Self::lease_period_index(now) {
Some(clp) => clp,
None => return true,
};

// Can't look in the past, so we pick whichever is the biggest.
let start_period = first_period.max(current_lease_period);
Expand Down Expand Up @@ -606,12 +605,12 @@ mod tests {
run_to_block(1);
assert_eq!(Slots::lease_period_length(), (10, 0));
let now = System::block_number();
assert_eq!(Slots::lease_period_index(now).0, 0);
assert_eq!(Slots::lease_period_index(now).unwrap().0, 0);
assert_eq!(Slots::deposit_held(1.into(), &1), 0);

run_to_block(10);
let now = System::block_number();
assert_eq!(Slots::lease_period_index(now).0, 1);
assert_eq!(Slots::lease_period_index(now).unwrap().0, 1);
});
}

Expand Down Expand Up @@ -873,7 +872,7 @@ mod tests {

run_to_block(20);
let now = System::block_number();
assert_eq!(Slots::lease_period_index(now).0, 2);
assert_eq!(Slots::lease_period_index(now).unwrap().0, 2);
// Can't lease from the past
assert!(Slots::lease_out(1.into(), &1, 1, 1, 1).is_err());
// Lease in the current period triggers onboarding
Expand Down Expand Up @@ -942,28 +941,29 @@ mod tests {
new_test_ext().execute_with(|| {
let (lpl, offset) = Slots::lease_period_length();
assert_eq!(offset, 0);
assert_eq!(Slots::lease_period_index(0), (0, true));
assert_eq!(Slots::lease_period_index(1), (0, false));
assert_eq!(Slots::lease_period_index(lpl - 1), (0, false));
assert_eq!(Slots::lease_period_index(lpl), (1, true));
assert_eq!(Slots::lease_period_index(lpl + 1), (1, false));
assert_eq!(Slots::lease_period_index(2 * lpl - 1), (1, false));
assert_eq!(Slots::lease_period_index(2 * lpl), (2, true));
assert_eq!(Slots::lease_period_index(2 * lpl + 1), (2, false));
assert_eq!(Slots::lease_period_index(0), Some((0, true)));
assert_eq!(Slots::lease_period_index(1), Some((0, false)));
assert_eq!(Slots::lease_period_index(lpl - 1), Some((0, false)));
assert_eq!(Slots::lease_period_index(lpl), Some((1, true)));
assert_eq!(Slots::lease_period_index(lpl + 1), Some((1, false)));
assert_eq!(Slots::lease_period_index(2 * lpl - 1), Some((1, false)));
assert_eq!(Slots::lease_period_index(2 * lpl), Some((2, true)));
assert_eq!(Slots::lease_period_index(2 * lpl + 1), Some((2, false)));

// Lease period is 10, and we add an offset of 5.
LeaseOffset::set(5);
let (lpl, offset) = Slots::lease_period_length();
assert_eq!(offset, 5);
assert_eq!(Slots::lease_period_index(0), (0, true));
assert_eq!(Slots::lease_period_index(1), (0, false));
assert_eq!(Slots::lease_period_index(lpl), (0, false));
assert_eq!(Slots::lease_period_index(lpl - 1 + offset), (0, false));
assert_eq!(Slots::lease_period_index(lpl + offset), (1, true));
assert_eq!(Slots::lease_period_index(lpl + offset + 1), (1, false));
assert_eq!(Slots::lease_period_index(2 * lpl - 1 + offset), (1, false));
assert_eq!(Slots::lease_period_index(2 * lpl + offset), (2, true));
assert_eq!(Slots::lease_period_index(2 * lpl + offset + 1), (2, false));
assert_eq!(Slots::lease_period_index(0), None);
assert_eq!(Slots::lease_period_index(1), None);
assert_eq!(Slots::lease_period_index(offset), Some((0, true)));
assert_eq!(Slots::lease_period_index(lpl), Some((0, false)));
assert_eq!(Slots::lease_period_index(lpl - 1 + offset), Some((0, false)));
assert_eq!(Slots::lease_period_index(lpl + offset), Some((1, true)));
assert_eq!(Slots::lease_period_index(lpl + offset + 1), Some((1, false)));
assert_eq!(Slots::lease_period_index(2 * lpl - 1 + offset), Some((1, false)));
assert_eq!(Slots::lease_period_index(2 * lpl + offset), Some((2, true)));
assert_eq!(Slots::lease_period_index(2 * lpl + offset + 1), Some((2, false)));
});
}
}
Expand Down
12 changes: 10 additions & 2 deletions runtime/common/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ pub enum LeaseError {
AlreadyLeased,
/// The period to be leased has already ended.
AlreadyEnded,
/// A lease period has not started yet, due to an offset in the starting block.
NoLeasePeriod,
}

/// Lease manager. Used by the auction module to handle parachain slot leases.
Expand Down Expand Up @@ -139,7 +141,10 @@ pub trait Leaser<BlockNumber> {
fn lease_period_length() -> (BlockNumber, BlockNumber);

/// Returns the lease period at `block`, and if this is the first block of a new lease period.
fn lease_period_index(block: BlockNumber) -> (Self::LeasePeriod, bool);
///
/// Will return `None` if the first lease period has not started yet, for example when an offset
/// is placed.
fn lease_period_index(block: BlockNumber) -> Option<(Self::LeasePeriod, bool)>;

/// Returns true if the parachain already has a lease in any of lease periods in the inclusive
/// range `[first_period, last_period]`, intersected with the unbounded range [`current_lease_period`..] .
Expand Down Expand Up @@ -236,7 +241,10 @@ pub trait Auctioneer<BlockNumber> {
fn lease_period_length() -> (BlockNumber, BlockNumber);

/// Returns the lease period at `block`, and if this is the first block of a new lease period.
fn lease_period_index(block: BlockNumber) -> (Self::LeasePeriod, bool);
///
/// Will return `None` if the first lease period has not started yet, for example when an offset
/// is placed.
fn lease_period_index(block: BlockNumber) -> Option<(Self::LeasePeriod, bool)>;

/// Check if the para and user combination has won an auction in the past.
fn has_won_an_auction(para: ParaId, bidder: &Self::AccountId) -> bool;
Expand Down

0 comments on commit 3668966

Please sign in to comment.