Allow an Offset to Lease Periods (#3980)

* add slot offset for slots

* trying things out

* fix test

* improve api to return the first block of a new lease period

* add an integration test with offset

* de-duplicate test

* hide lease period_period_length from public api

* fix benchmarks

* Update runtime/common/src/slots.rs

* support the exact same range of crowdloans

* fix docs

* fix docs again

* introduce offset to runtimes

* fix and check edge case w/ offset and lease period first block

* remove newline

* turn into an option

* fix benchmarks

Co-authored-by: Robert Habermeier <rphmeier@gmail.com>

polkadot/runtime/common/src/crowdloan.rs

@@ -72,8 +72,11 @@ use sp_runtime::{
 };
 use sp_std::vec::Vec;
 
-type CurrencyOf<T> = <<T as Config>::Auctioneer as Auctioneer>::Currency;
-type LeasePeriodOf<T> = <<T as Config>::Auctioneer as Auctioneer>::LeasePeriod;
+type CurrencyOf<T> =
+	<<T as Config>::Auctioneer as Auctioneer<<T as frame_system::Config>::BlockNumber>>::Currency;
+type LeasePeriodOf<T> = <<T as Config>::Auctioneer as Auctioneer<
+	<T as frame_system::Config>::BlockNumber,
+>>::LeasePeriod;
 type BalanceOf<T> = <CurrencyOf<T> as Currency<<T as frame_system::Config>::AccountId>>::Balance;
 
 #[allow(dead_code)]
@@ -203,8 +206,8 @@ pub mod pallet {
 
 		/// The type representing the auctioning system.
 		type Auctioneer: Auctioneer<
+			Self::BlockNumber,
 			AccountId = Self::AccountId,
-			BlockNumber = Self::BlockNumber,
 			LeasePeriod = Self::BlockNumber,
 		>;
 
@@ -313,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]
@@ -365,20 +370,31 @@ pub mod pallet {
 			verifier: Option<MultiSigner>,
 		) -> DispatchResult {
 			let depositor = ensure_signed(origin)?;
+			let now = frame_system::Pallet::<T>::block_number();
 
 			ensure!(first_period <= last_period, Error::<T>::LastPeriodBeforeFirstPeriod);
 			let last_period_limit = first_period
 				.checked_add(&((SlotRange::LEASE_PERIODS_PER_SLOT as u32) - 1).into())
 				.ok_or(Error::<T>::FirstPeriodTooFarInFuture)?;
 			ensure!(last_period <= last_period_limit, Error::<T>::LastPeriodTooFarInFuture);
-			ensure!(end > <frame_system::Pallet<T>>::block_number(), Error::<T>::CannotEndInPast);
-			let last_possible_win_date = (first_period.saturating_add(One::one()))
-				.saturating_mul(T::Auctioneer::lease_period());
-			ensure!(end <= last_possible_win_date, Error::<T>::EndTooFarInFuture);
-			ensure!(
-				first_period >= T::Auctioneer::lease_period_index(),
-				Error::<T>::FirstPeriodInPast
-			);
+			ensure!(end > now, Error::<T>::CannotEndInPast);
+
+			// 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).ok_or(Error::<T>::NoLeasePeriod)?;
+			let adjusted_lease_period_at_end = if is_first_block {
+				lease_period_at_end.saturating_sub(One::one())
+			} else {
+				lease_period_at_end
+			};
+			ensure!(adjusted_lease_period_at_end <= first_period, Error::<T>::EndTooFarInFuture);
+
+			// Can't start a crowdloan for a lease period that already passed.
+			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);
@@ -439,7 +455,9 @@ pub mod pallet {
 			ensure!(now < fund.end, Error::<T>::ContributionPeriodOver);
 
 			// Make sure crowdloan is in a valid lease period
-			let current_lease_period = T::Auctioneer::lease_period_index();
+			let now = frame_system::Pallet::<T>::block_number();
+			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.
@@ -751,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();
+		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
@@ -931,14 +950,16 @@ mod tests {
 	}
 
 	pub struct TestAuctioneer;
-	impl Auctioneer for TestAuctioneer {
+	impl Auctioneer<u64> for TestAuctioneer {
 		type AccountId = u64;
-		type BlockNumber = BlockNumber;
 		type LeasePeriod = u64;
 		type Currency = Balances;
 
 		fn new_auction(duration: u64, lease_period_index: u64) -> DispatchResult {
-			assert!(lease_period_index >= Self::lease_period_index());
+			let now = System::block_number();
+			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)));
@@ -991,12 +1012,17 @@ mod tests {
 			Ok(())
 		}
 
-		fn lease_period_index() -> u64 {
-			System::block_number() / Self::lease_period()
+		fn lease_period_index(b: BlockNumber) -> Option<(u64, bool)> {
+			let (lease_period_length, offset) = Self::lease_period_length();
+			let b = b.checked_sub(offset)?;
+
+			let lease_period = b / lease_period_length;
+			let first_block = (b % lease_period_length).is_zero();
+			Some((lease_period, first_block))
 		}
 
-		fn lease_period() -> u64 {
-			20
+		fn lease_period_length() -> (u64, u64) {
+			(20, 0)
 		}
 
 		fn has_won_an_auction(para: ParaId, bidder: &u64) -> bool {
@@ -1367,7 +1393,8 @@ mod tests {
 			let para_3 = new_para();
 			assert_ok!(Crowdloan::create(Origin::signed(1), para_3, 1000, 1, 4, 40, None));
 			run_to_block(40);
-			assert_eq!(TestAuctioneer::lease_period_index(), 2);
+			let now = System::block_number();
+			assert_eq!(TestAuctioneer::lease_period_index(now).unwrap().0, 2);
 			assert_noop!(
 				Crowdloan::contribute(Origin::signed(1), para_3, 49, None),
 				Error::<Test>::ContributionPeriodOver
@@ -1842,7 +1869,11 @@ mod benchmarking {
 
 	fn create_fund<T: Config>(id: u32, end: T::BlockNumber) -> ParaId {
 		let cap = BalanceOf::<T>::max_value();
-		let lease_period_index = T::Auctioneer::lease_period_index();
+		let (_, offset) = T::Auctioneer::lease_period_length();
+		// Set to the very beginning of lease period index 0.
+		frame_system::Pallet::<T>::set_block_number(offset);
+		let now = frame_system::Pallet::<T>::block_number();
+		let (lease_period_index, _) = T::Auctioneer::lease_period_index(now).unwrap_or_default();
 		let first_period = lease_period_index;
 		let last_period =
 			lease_period_index + ((SlotRange::LEASE_PERIODS_PER_SLOT as u32) - 1).into();
@@ -1894,7 +1925,8 @@ mod benchmarking {
 			let cap = BalanceOf::<T>::max_value();
 			let first_period = 0u32.into();
 			let last_period = 3u32.into();
-			let end = T::Auctioneer::lease_period();
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
 
 			let caller: T::AccountId = whitelisted_caller();
 			let head_data = T::Registrar::worst_head_data();
@@ -1913,7 +1945,9 @@ mod benchmarking {
 
 		// Contribute has two arms: PreEnding and Ending, but both are equal complexity.
 		contribute {
-			let fund_index = create_fund::<T>(1, 100u32.into());
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
+			let fund_index = create_fund::<T>(1, end);
 			let caller: T::AccountId = whitelisted_caller();
 			let contribution = T::MinContribution::get();
 			CurrencyOf::<T>::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
@@ -1931,7 +1965,9 @@ mod benchmarking {
 		}
 
 		withdraw {
-			let fund_index = create_fund::<T>(1337, 100u32.into());
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
+			let fund_index = create_fund::<T>(1337, end);
 			let caller: T::AccountId = whitelisted_caller();
 			let contributor = account("contributor", 0, 0);
 			contribute_fund::<T>(&contributor, fund_index);
@@ -1945,7 +1981,9 @@ mod benchmarking {
 		#[skip_meta]
 		refund {
 			let k in 0 .. T::RemoveKeysLimit::get();
-			let fund_index = create_fund::<T>(1337, 100u32.into());
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
+			let fund_index = create_fund::<T>(1337, end);
 
 			// Dissolve will remove at most `RemoveKeysLimit` at once.
 			for i in 0 .. k {
@@ -1960,7 +1998,9 @@ mod benchmarking {
 		}
 
 		dissolve {
-			let fund_index = create_fund::<T>(1337, 100u32.into());
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
+			let fund_index = create_fund::<T>(1337, end);
 			let caller: T::AccountId = whitelisted_caller();
 			frame_system::Pallet::<T>::set_block_number(T::BlockNumber::max_value());
 		}: _(RawOrigin::Signed(caller.clone()), fund_index)
@@ -1973,7 +2013,8 @@ mod benchmarking {
 			let cap = BalanceOf::<T>::max_value();
 			let first_period = 0u32.into();
 			let last_period = 3u32.into();
-			let end = T::Auctioneer::lease_period();
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
 
 			let caller: T::AccountId = whitelisted_caller();
 			let head_data = T::Registrar::worst_head_data();
@@ -1997,7 +2038,9 @@ mod benchmarking {
 		}
 
 		add_memo {
-			let fund_index = create_fund::<T>(1, 100u32.into());
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
+			let fund_index = create_fund::<T>(1, end);
 			let caller: T::AccountId = whitelisted_caller();
 			contribute_fund::<T>(&caller, fund_index);
 			let worst_memo = vec![42; T::MaxMemoLength::get().into()];
@@ -2011,7 +2054,9 @@ mod benchmarking {
 		}
 
 		poke {
-			let fund_index = create_fund::<T>(1, 100u32.into());
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end = lpl + offset;
+			let fund_index = create_fund::<T>(1, end);
 			let caller: T::AccountId = whitelisted_caller();
 			contribute_fund::<T>(&caller, fund_index);
 			NewRaise::<T>::kill();
@@ -2028,7 +2073,8 @@ mod benchmarking {
 		on_initialize {
 			// We test the complexity over different number of new raise
 			let n in 2 .. 100;
-			let end_block: T::BlockNumber = 100u32.into();
+			let (lpl, offset) = T::Auctioneer::lease_period_length();
+			let end_block = lpl + offset - 1u32.into();
 
 			let pubkey = crypto::create_ed25519_pubkey(b"//verifier".to_vec());
 
@@ -2043,7 +2089,8 @@ mod benchmarking {
 				Crowdloan::<T>::contribute(RawOrigin::Signed(contributor).into(), fund_index, contribution, Some(sig))?;
 			}
 
-			let lease_period_index = T::Auctioneer::lease_period_index();
+			let now = frame_system::Pallet::<T>::block_number();
+			let (lease_period_index, _) = T::Auctioneer::lease_period_index(now).unwrap_or_default();
 			let duration = end_block
 				.checked_sub(&frame_system::Pallet::<T>::block_number())
 				.ok_or("duration of auction less than zero")?;
@@ -2062,7 +2109,7 @@ mod benchmarking {
 
 	impl_benchmark_test_suite!(
 		Crowdloan,
-		crate::integration_tests::new_test_ext(),
+		crate::integration_tests::new_test_ext_with_offset(10),
 		crate::integration_tests::Test,
 	);
 }