Addressing PR feedback

frame/scored-pool/src/lib.rs

@@ -263,7 +263,7 @@ pub mod pallet {
 	#[pallet::genesis_build]
 	impl<T: Config<I>, I: 'static> GenesisBuild<T, I> for GenesisConfig<T, I> {
 		fn build(&self) {
-			let mut pool = self.pool.clone().into_inner();
+			let mut pool = self.pool.clone();
 
 			// reserve balance for each candidate in the pool.
 			// panicking here is ok, since this just happens one time, pre-genesis.
@@ -274,12 +274,12 @@ pub mod pallet {
 			});
 
 			// Sorts the `Pool` by score in a descending order. Entities which
-			// have a score of `None` are sorted to the beginning of the vec.
+			// have a score of `None` are sorted at the end of the bounded vec.
 			pool.sort_by_key(|(_, maybe_score)| Reverse(maybe_score.unwrap_or_default()));
-			let pool_bounded: PoolT<T, I> = BoundedVec::truncate_from(pool);
-			<MemberCount<T, I>>::put(self.member_count);
-			<Pool<T, I>>::put(&pool_bounded);
-			<Pallet<T, I>>::refresh_members(pool_bounded, ChangeReceiver::MembershipInitialized);
+			<Pallet<T, I>>::update_member_count(self.member_count)
+				.expect("Number of allowed memebers exceeded");
+			<Pool<T, I>>::put(&pool);
+			<Pallet<T, I>>::refresh_members(pool, ChangeReceiver::MembershipInitialized);
 		}
 	}
 
@@ -422,7 +422,7 @@ pub mod pallet {
 		#[pallet::weight(0)]
 		pub fn change_member_count(origin: OriginFor<T>, count: u32) -> DispatchResult {
 			ensure_root(origin)?;
-			MemberCount::<T, I>::put(&count);
+			Self::update_member_count(count).map_err(|_| Error::<T, I>::TooManyMembers)?;
 			Ok(())
 		}
 	}
@@ -437,7 +437,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	fn refresh_members(pool: PoolT<T, I>, notify: ChangeReceiver) {
 		let count = MemberCount::<T, I>::get();
 		let old_members = <Members<T, I>>::get();
-		let pool = pool.into_inner();
 
 		let new_members: Vec<T::AccountId> = pool
 			.into_iter()
@@ -446,6 +445,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			.map(|(account_id, _)| account_id)
 			.collect();
 
+		// It's safe to truncate_from at this point since MemberCount
+		// is verified that it does not exceed the MaximumMembers value
 		let mut new_members_bounded: MembersT<T, I> = BoundedVec::truncate_from(new_members);
 
 		new_members_bounded.sort();
@@ -502,4 +503,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		Ok(())
 	}
+
+	/// Make sure the new member count value does not exceed the MaximumMembers
+	fn update_member_count(new_member_count: u32) -> Result<(), Error<T, I>> {
+		ensure!(new_member_count < T::MaximumMembers::get(), Error::<T, I>::TooManyMembers);
+		<MemberCount<T, I>>::put(new_member_count);
+		Ok(())
+	}
 }