Test improvement ideas (#6487)

substrate/frame/election-provider-multi-block/src/mock/mod.rs

@@ -274,6 +274,7 @@ impl ElectionProvider for MockFallback {
 
 #[derive(Default)]
 pub struct ExtBuilder {
+	minimum_score: Option<ElectionScore>,
 	with_verifier: bool,
 }
 
@@ -344,8 +345,13 @@ impl ExtBuilder {
 		self
 	}
 
+	pub(crate) fn minimum_score(mut self, score: ElectionScore) -> Self {
+		self.minimum_score = Some(score);
+		self
+	}
+
 	pub(crate) fn verifier() -> Self {
-		ExtBuilder { with_verifier: true }
+		ExtBuilder { with_verifier: true, ..Default::default() }
 	}
 
 	pub(crate) fn build(self) -> sp_io::TestExternalities {
@@ -377,6 +383,12 @@ impl ExtBuilder {
 		}
 		.assimilate_storage(&mut storage);
 
+		let _ = verifier_pallet::GenesisConfig::<T> {
+			minimum_score: self.minimum_score,
+			..Default::default()
+		}
+		.assimilate_storage(&mut storage);
+
 		if self.with_verifier {
 			// nothing special for now
 		}
@@ -621,6 +633,16 @@ pub(crate) fn signed_events() -> Vec<crate::signed::Event<T>> {
 		.collect()
 }
 
+pub(crate) fn verifier_events() -> Vec<crate::verifier::Event<T>> {
+	System::events()
+		.into_iter()
+		.map(|r| r.event)
+		.filter_map(
+			|e| if let RuntimeEvent::VerifierPallet(inner) = e { Some(inner) } else { None },
+		)
+		.collect()
+}
+
 // TODO fix or use macro.
 pub(crate) fn filter_events(
 	types: Vec<RuntimeEvent>,

substrate/frame/election-provider-multi-block/src/signed/mod.rs

@@ -485,7 +485,9 @@ pub mod pallet {
 			who: &T::AccountId,
 			metadata: SubmissionMetadata<T>,
 		) {
-			debug_assert!(SortedScores::<T>::get(round).iter().any(|(account, _)| who == account));
+			// TODO: remove comment
+			//debug_assert!(SortedScores::<T>::get(round).iter().any(|(account, _)| who ==
+			// account));
 
 			Self::mutate_checked(round, || {
 				SubmissionMetadataStorage::<T>::insert(round, who, metadata);
@@ -606,6 +608,35 @@ pub mod pallet {
 		}
 	}
 
+	#[cfg(any(feature = "runtime-benchmarks", test))]
+	impl<T: Config> Submissions<T> {
+		pub(crate) fn submission_metadata_from(
+			claimed_score: ElectionScore,
+			pages: BoundedVec<bool, PagesOf<T>>,
+			deposit: BalanceOf<T>,
+			release_strategy: ReleaseStrategy,
+		) -> SubmissionMetadata<T> {
+			SubmissionMetadata { claimed_score, pages, deposit, release_strategy }
+		}
+
+		pub(crate) fn insert_score_and_metadata(
+			round: u32,
+			who: T::AccountId,
+			maybe_score: Option<ElectionScore>,
+			maybe_metadata: Option<SubmissionMetadata<T>>,
+		) {
+			if let Some(score) = maybe_score {
+				let mut scores = Submissions::<T>::scores_for(round);
+				scores.try_push((who.clone(), score)).unwrap();
+				SortedScores::<T>::insert(round, scores);
+			}
+
+			if let Some(metadata) = maybe_metadata {
+				SubmissionMetadataStorage::<T>::insert(round, who.clone(), metadata);
+			}
+		}
+	}
+
 	impl<T: Config> Pallet<T> {
 		pub(crate) fn do_register(
 			who: &T::AccountId,
@@ -831,8 +862,7 @@ impl<T: Config> SolutionDataProvider for Pallet<T> {
 				};
 				(leader, metadata)
 			} else {
-				// TODO(gpestana): turn into defensive.
-				sublog!(error, "signed", "unexpected: leader called without active submissions.");
+				defensive!("unexpected: selected leader without active submissions.");
 				return
 			};
 

substrate/frame/election-provider-multi-block/src/unsigned/tests.rs

@@ -20,7 +20,6 @@ use crate::{
 	mock::*, verifier::QueuedSolution, PagedVoterSnapshot, Phase, Snapshot, TargetSnapshot,
 	Verifier,
 };
-use sp_externalities::Extension;
 
 use frame_election_provider_support::ElectionProvider;
 use frame_support::{assert_err, assert_ok};

substrate/frame/election-provider-multi-block/src/verifier/impls.rs

@@ -27,7 +27,7 @@ use frame_support::{
 	BoundedVec,
 };
 use sp_runtime::{traits::Zero, Perbill};
-use sp_std::{collections::btree_map::BTreeMap, vec::Vec};
+use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, vec::Vec};
 
 #[frame_support::pallet]
 pub(crate) mod pallet {
@@ -98,7 +98,7 @@ pub(crate) mod pallet {
 	/// [`MininumScore`].
 	/// - The [`QueuedSolutionBackings`] are always the backings corresponding to the *invalid*
 	/// variant.
-	pub struct QueuedSolution<T: Config>(sp_std::marker::PhantomData<T>);
+	pub struct QueuedSolution<T: Config>(PhantomData<T>);
 
 	impl<T: Config> QueuedSolution<T> {
 		fn mutate_checked<R>(mutate: impl FnOnce() -> R) -> R {
@@ -284,6 +284,22 @@ pub(crate) mod pallet {
 	pub(crate) type RemainingUnsignedPages<T: Config> =
 		StorageValue<_, BoundedVec<PageIndex, T::Pages>, ValueQuery>;
 
+	#[pallet::genesis_config]
+	#[derive(frame_support::DefaultNoBound)]
+	pub struct GenesisConfig<T: Config> {
+		pub minimum_score: Option<ElectionScore>,
+		pub _phantom: PhantomData<T>,
+	}
+
+	#[pallet::genesis_build]
+	impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
+		fn build(&self) {
+			if let Some(min_score) = self.minimum_score {
+				Pallet::<T>::set_minimum_score(min_score);
+			}
+		}
+	}
+
 	#[pallet::pallet]
 	pub struct Pallet<T>(PhantomData<T>);
 
@@ -429,7 +445,7 @@ impl<T: impls::pallet::Config> AsyncVerifier for Pallet<T> {
 					FeasibilityError::ScoreTooLow,
 				));
 				// report to the solution data provider that the page verification failed.
-				T::SolutionDataProvider::report_result(VerificationResult::Rejected);
+				Self::SolutionDataProvider::report_result(VerificationResult::Rejected);
 				// despite the verification failed, this was a successful `start` operation.
 				Ok(())
 			} else {
@@ -650,6 +666,7 @@ impl<T: impls::pallet::Config> Pallet<T> {
 			.map_or(true, |min_score| score.strict_threshold_better(min_score, Perbill::zero()));
 
 		ensure!(is_greater_than_min_trusted, FeasibilityError::ScoreTooLow);
+
 		Ok(())
 	}
 

substrate/frame/election-provider-multi-block/src/verifier/mod.rs

@@ -80,7 +80,8 @@ use sp_runtime::RuntimeDebug;
 // public re-exports.
 pub use impls::pallet::{
 	Call, Config, Event, Pallet, QueuedSolution, __substrate_call_check, __substrate_event_check,
-	tt_default_parts, tt_default_parts_v2, tt_error_token,
+	__substrate_genesis_config_check, tt_default_parts, tt_default_parts_v2, tt_error_token,
+	GenesisConfig,
 };
 
 /// Errors related to the solution feasibility checks.

substrate/frame/election-provider-multi-block/src/verifier/tests.rs

@@ -17,10 +17,11 @@
 
 use crate::{
 	mock::*,
-	verifier::{impls::pallet::*, *},
+	signed,
+	verifier::{impls::pallet::*, SolutionDataProvider, *},
 	Phase,
 };
-use frame_support::{assert_err, assert_noop, assert_ok, StorageMap};
+use frame_support::{assert_err, assert_noop, assert_ok, testing_prelude::*, StorageMap};
 use sp_npos_elections::ElectionScore;
 use sp_runtime::Perbill;
 
@@ -412,7 +413,7 @@ mod async_verifier {
 
 	mod verification_start {
 		use super::*;
-		use crate::signed::pallet::Submissions;
+		use crate::signed::{pallet::Submissions, SubmissionMetadata};
 
 		#[test]
 		fn fails_if_verification_is_ongoing() {
@@ -437,6 +438,104 @@ mod async_verifier {
 		// 		let leader_metadata = Submissions::metadata_for(current_round, leader.unwrap().0);
 		// 	});
 		// }
+		//
+
+		#[test]
+		#[should_panic(expected = "unexpected: selected leader without active submissions.")]
+		fn reports_result_rejection_no_metadata_fails() {
+			ExtBuilder::default()
+				.minimum_score(ElectionScore {
+					minimal_stake: 100,
+					sum_stake: 100,
+					sum_stake_squared: 100,
+				})
+				.solution_improvements_threshold(Perbill::from_percent(10))
+				.build_and_execute(|| {
+					<VerifierPallet as AsyncVerifier>::set_status(Status::Nothing);
+
+					// no score in sorted scores yet.
+					assert!(<SignedPallet as SolutionDataProvider>::get_score().is_none());
+					assert!(Submissions::<T>::scores_for(current_round()).is_empty());
+
+					let low_score =
+						ElectionScore { minimal_stake: 10, sum_stake: 10, sum_stake_squared: 10 };
+
+					// force insert score and `None` metadata.
+					Submissions::<T>::insert_score_and_metadata(
+						current_round(),
+						1,
+						Some(low_score),
+						None,
+					);
+
+					// low_score has been added to the sorted scores.
+					assert_eq!(
+						<SignedPallet as SolutionDataProvider>::get_score(),
+						Some(low_score)
+					);
+					assert!(Submissions::<T>::scores_for(current_round()).len() == 1);
+					// metadata is None.
+					assert_eq!(
+						Submissions::<T>::take_leader_score(current_round()),
+						Some((1, None))
+					);
+					// will defensive panic since submission metadata does not exist.
+					let _ = <VerifierPallet as AsyncVerifier>::start();
+				})
+		}
+
+		#[test]
+		fn reports_result_rejection_works() {
+			ExtBuilder::default()
+				.minimum_score(ElectionScore {
+					minimal_stake: 100,
+					sum_stake: 100,
+					sum_stake_squared: 100,
+				})
+				.solution_improvements_threshold(Perbill::from_percent(10))
+				.build_and_execute(|| {
+					<VerifierPallet as AsyncVerifier>::set_status(Status::Nothing);
+
+					// no score in sorted scores or leader yet.
+					assert!(<SignedPallet as SolutionDataProvider>::get_score().is_none());
+					assert!(Submissions::<T>::scores_for(current_round()).is_empty());
+					assert_eq!(Submissions::<T>::take_leader_score(current_round()), None);
+
+					let low_score =
+						ElectionScore { minimal_stake: 10, sum_stake: 10, sum_stake_squared: 10 };
+
+					let metadata = Submissions::submission_metadata_from(
+						low_score,
+						Default::default(),
+						Default::default(),
+						Default::default(),
+					);
+
+					// force insert score and metadata.
+					Submissions::<T>::insert_score_and_metadata(
+						current_round(),
+						1,
+						Some(low_score),
+						Some(metadata),
+					);
+
+					// low_score has been added to the sorted scores.
+					assert_eq!(
+						<SignedPallet as SolutionDataProvider>::get_score(),
+						Some(low_score)
+					);
+					assert!(Submissions::<T>::scores_for(current_round()).len() == 1);
+
+					// insert a score lower than minimum score.
+					assert_ok!(<VerifierPallet as AsyncVerifier>::start());
+
+					// score too low event submitted.
+					assert_eq!(
+						verifier_events(),
+						vec![Event::<T>::VerificationFailed(2, FeasibilityError::ScoreTooLow,)]
+					);
+				})
+		}
 
 		#[test]
 		fn given_better_score_sets_verification_status_to_ongoing() {