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

* add already leased

Co-Authored-By: parity-processbot <>

* add unit test

Co-Authored-By: parity-processbot <>

* add integration test and fix

Co-Authored-By: parity-processbot <>

* better doc

Co-Authored-By: parity-processbot <>

* Update runtime/common/src/slots.rs

* Update runtime/common/src/slots.rs

* Apply suggestions from code review

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update runtime/common/src/integration_tests.rs

* Update runtime/common/src/integration_tests.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_common::auctions --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_common_auctions.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_common::auctions --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_common_auctions.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_common::crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_common_crowdloan.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_common::auctions --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_common_auctions.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_common::crowdloan --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_common_crowdloan.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=kusama-dev --steps=50 --repeat=20 --pallet=runtime_common::slots --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/kusama/src/weights/runtime_common_slots.rs

* cargo run --release --features=runtime-benchmarks -- benchmark --chain=westend-dev --steps=50 --repeat=20 --pallet=runtime_common::slots --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=./runtime/westend/src/weights/runtime_common_slots.rs

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Parity Bot <admin@parity.io>

polkadot/runtime/common/src/auctions.rs

@@ -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.
@@ -413,6 +415,9 @@ impl<T: Config> Pallet<T> {
 			AuctionStatus::VrfDelay(_) => return Err(Error::<T>::AuctionEnded.into()),
 		};
 
+		// 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.
@@ -746,6 +751,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!{
@@ -1306,6 +1323,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() {

polkadot/runtime/common/src/integration_tests.rs

@@ -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 a parathread
+		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 -------------
+			vec![None, None, None, Some((crowdloan_account, 8855)), Some((crowdloan_account, 8855))],
+		);
+
+		// 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,
+			)
+		);
+	});
+}

polkadot/runtime/common/src/slots.rs

@@ -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,
@@ -421,6 +421,41 @@ 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 true,
+		};
+
+		// 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 true,
+		};
+
+		// Get the leases, and check each item in the vec which is part of the range we are checking.
+		let leases = Leases::<T>::get(para_id);
+		for slot in offset ..= offset + period_count {
+			if let Some(Some(_)) = leases.get(slot) {
+				// If there exists any lease period, we exit early and return true.
+				return true
+			}
+		}
+
+		// If we got here, then we did not find any overlapping leases.
+		false
+	}
 }
 
 

polkadot/runtime/common/src/traits.rs

@@ -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]`, intersected with the unbounded range [`current_lease_period`..] .
+	fn already_leased(
+		para_id: ParaId,
+		first_period: Self::LeasePeriod,
+		last_period: Self::LeasePeriod
+	) -> bool;
 }
 
 /// An enum which tracks the status of the auction system, and which phase it is in.