Small fixes in para-scheduler pallet (#3524)

Fixes some typos, outdated comments and test asserts. Also uses safe
math and `defensive` for arithmetic operations.

polkadot/runtime/parachains/src/scheduler.rs

@@ -37,7 +37,7 @@
 //! availability cores over time.
 
 use crate::{configuration, initializer::SessionChangeNotification, paras};
-use frame_support::pallet_prelude::*;
+use frame_support::{pallet_prelude::*, traits::Defensive};
 use frame_system::pallet_prelude::BlockNumberFor;
 pub use polkadot_core_primitives::v2::BlockNumber;
 use primitives::{
@@ -59,6 +59,7 @@ pub use pallet::*;
 mod tests;
 
 const LOG_TARGET: &str = "runtime::parachains::scheduler";
+
 pub mod migration;
 
 #[frame_support::pallet]
@@ -88,10 +89,8 @@ pub mod pallet {
 	#[pallet::getter(fn validator_groups)]
 	pub(crate) type ValidatorGroups<T> = StorageValue<_, Vec<Vec<ValidatorIndex>>, ValueQuery>;
 
-	/// One entry for each availability core. Entries are `None` if the core is not currently
-	/// occupied. Can be temporarily `Some` if scheduled but not occupied.
-	/// The i'th parachain belongs to the i'th core, with the remaining cores all being
-	/// parathread-multiplexers.
+	/// One entry for each availability core. The i'th parachain belongs to the i'th core, with the
+	/// remaining cores all being on demand parachain multiplexers.
 	///
 	/// Bounded by the maximum of either of these two values:
 	///   * The number of parachains and parathread multiplexers
@@ -146,7 +145,7 @@ pub mod pallet {
 
 	/// One entry for each availability core. The `VecDeque` represents the assignments to be
 	/// scheduled on that core. The value contained here will not be valid after the end of
-	/// a block. Runtime APIs should be used to determine scheduled cores/ for the upcoming block.
+	/// a block. Runtime APIs should be used to determine scheduled cores for the upcoming block.
 	#[pallet::storage]
 	#[pallet::getter(fn claimqueue)]
 	pub(crate) type ClaimQueue<T: Config> =
@@ -236,8 +235,16 @@ impl<T: Config> Pallet<T> {
 		if n_cores == 0 || validators.is_empty() {
 			ValidatorGroups::<T>::set(Vec::new());
 		} else {
-			let group_base_size = validators.len() / n_cores as usize;
-			let n_larger_groups = validators.len() % n_cores as usize;
+			let group_base_size = validators
+				.len()
+				.checked_div(n_cores as usize)
+				.defensive_proof("n_cores should not be 0")
+				.unwrap_or(0);
+			let n_larger_groups = validators
+				.len()
+				.checked_rem(n_cores as usize)
+				.defensive_proof("n_cores should not be 0")
+				.unwrap_or(0);
 
 			// Groups contain indices into the validators from the session change notification,
 			// which are already shuffled.

polkadot/runtime/parachains/src/scheduler/tests.rs

@@ -213,6 +213,7 @@ fn claimqueue_ttl_drop_fn_works() {
 
 		// Drop expired claim.
 		Scheduler::drop_expired_claims_from_claimqueue();
+		assert!(!claimqueue_contains_para_ids::<Test>(vec![para_id]));
 
 		// Add a claim on core 0 with a ttl == now (17)
 		let paras_entry_non_expired = ParasEntry::new(Assignment::Bulk(para_id), now);
@@ -222,7 +223,7 @@ fn claimqueue_ttl_drop_fn_works() {
 		Scheduler::add_to_claimqueue(core_idx, paras_entry_expired.clone());
 		Scheduler::add_to_claimqueue(core_idx, paras_entry_non_expired.clone());
 		let cq = Scheduler::claimqueue();
-		assert!(cq.get(&core_idx).unwrap().len() == 3);
+		assert_eq!(cq.get(&core_idx).unwrap().len(), 3);
 
 		// Add a claim to the test assignment provider.
 		let assignment = Assignment::Bulk(para_id);
@@ -234,8 +235,9 @@ fn claimqueue_ttl_drop_fn_works() {
 
 		let cq = Scheduler::claimqueue();
 		let cqc = cq.get(&core_idx).unwrap();
-		// Same number of claims
-		assert!(cqc.len() == 3);
+		// Same number of claims, because a new claim is popped from `MockAssigner` instead of the
+		// expired one
+		assert_eq!(cqc.len(), 3);
 
 		// The first 2 claims in the queue should have a ttl of 17,
 		// being the ones set up prior in this test as claims 1 and 3.
@@ -355,14 +357,13 @@ fn fill_claimqueue_fills() {
 		schedule_blank_para(para_b);
 		schedule_blank_para(para_c);
 
-		// start a new session to activate, 3 validators for 3 cores.
+		// start a new session to activate, 2 validators for 2 cores.
 		run_to_block(1, |number| match number {
 			1 => Some(SessionChangeNotification {
 				new_config: default_config(),
 				validators: vec![
 					ValidatorId::from(Sr25519Keyring::Alice.public()),
 					ValidatorId::from(Sr25519Keyring::Bob.public()),
-					ValidatorId::from(Sr25519Keyring::Charlie.public()),
 				],
 				..Default::default()
 			}),