diff --git a/substrate/frame/election-provider-multi-block/src/mock/mod.rs b/substrate/frame/election-provider-multi-block/src/mock/mod.rs index bafaf9e8209b..ea20679e6690 100644 --- a/substrate/frame/election-provider-multi-block/src/mock/mod.rs +++ b/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, 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:: { + 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> { .collect() } +pub(crate) fn verifier_events() -> Vec> { + 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, diff --git a/substrate/frame/election-provider-multi-block/src/signed/mod.rs b/substrate/frame/election-provider-multi-block/src/signed/mod.rs index 33fcc9f15a4f..07dae2d5f684 100644 --- a/substrate/frame/election-provider-multi-block/src/signed/mod.rs +++ b/substrate/frame/election-provider-multi-block/src/signed/mod.rs @@ -485,7 +485,9 @@ pub mod pallet { who: &T::AccountId, metadata: SubmissionMetadata, ) { - debug_assert!(SortedScores::::get(round).iter().any(|(account, _)| who == account)); + // TODO: remove comment + //debug_assert!(SortedScores::::get(round).iter().any(|(account, _)| who == + // account)); Self::mutate_checked(round, || { SubmissionMetadataStorage::::insert(round, who, metadata); @@ -606,6 +608,35 @@ pub mod pallet { } } + #[cfg(any(feature = "runtime-benchmarks", test))] + impl Submissions { + pub(crate) fn submission_metadata_from( + claimed_score: ElectionScore, + pages: BoundedVec>, + deposit: BalanceOf, + release_strategy: ReleaseStrategy, + ) -> SubmissionMetadata { + SubmissionMetadata { claimed_score, pages, deposit, release_strategy } + } + + pub(crate) fn insert_score_and_metadata( + round: u32, + who: T::AccountId, + maybe_score: Option, + maybe_metadata: Option>, + ) { + if let Some(score) = maybe_score { + let mut scores = Submissions::::scores_for(round); + scores.try_push((who.clone(), score)).unwrap(); + SortedScores::::insert(round, scores); + } + + if let Some(metadata) = maybe_metadata { + SubmissionMetadataStorage::::insert(round, who.clone(), metadata); + } + } + } + impl Pallet { pub(crate) fn do_register( who: &T::AccountId, @@ -831,8 +862,7 @@ impl SolutionDataProvider for Pallet { }; (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 }; diff --git a/substrate/frame/election-provider-multi-block/src/unsigned/tests.rs b/substrate/frame/election-provider-multi-block/src/unsigned/tests.rs index 980c2e6183d2..6138dc7af8a9 100644 --- a/substrate/frame/election-provider-multi-block/src/unsigned/tests.rs +++ b/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}; diff --git a/substrate/frame/election-provider-multi-block/src/verifier/impls.rs b/substrate/frame/election-provider-multi-block/src/verifier/impls.rs index 0631230af0e9..2a1b179469a7 100644 --- a/substrate/frame/election-provider-multi-block/src/verifier/impls.rs +++ b/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(sp_std::marker::PhantomData); + pub struct QueuedSolution(PhantomData); impl QueuedSolution { fn mutate_checked(mutate: impl FnOnce() -> R) -> R { @@ -284,6 +284,22 @@ pub(crate) mod pallet { pub(crate) type RemainingUnsignedPages = StorageValue<_, BoundedVec, ValueQuery>; + #[pallet::genesis_config] + #[derive(frame_support::DefaultNoBound)] + pub struct GenesisConfig { + pub minimum_score: Option, + pub _phantom: PhantomData, + } + + #[pallet::genesis_build] + impl BuildGenesisConfig for GenesisConfig { + fn build(&self) { + if let Some(min_score) = self.minimum_score { + Pallet::::set_minimum_score(min_score); + } + } + } + #[pallet::pallet] pub struct Pallet(PhantomData); @@ -429,7 +445,7 @@ impl AsyncVerifier for Pallet { 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 Pallet { .map_or(true, |min_score| score.strict_threshold_better(min_score, Perbill::zero())); ensure!(is_greater_than_min_trusted, FeasibilityError::ScoreTooLow); + Ok(()) } diff --git a/substrate/frame/election-provider-multi-block/src/verifier/mod.rs b/substrate/frame/election-provider-multi-block/src/verifier/mod.rs index 3de52d479a1e..d1ba0fd2cefa 100644 --- a/substrate/frame/election-provider-multi-block/src/verifier/mod.rs +++ b/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. diff --git a/substrate/frame/election-provider-multi-block/src/verifier/tests.rs b/substrate/frame/election-provider-multi-block/src/verifier/tests.rs index 8eb49a4b7478..25181a8a61c4 100644 --- a/substrate/frame/election-provider-multi-block/src/verifier/tests.rs +++ b/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(|| { + ::set_status(Status::Nothing); + + // no score in sorted scores yet. + assert!(::get_score().is_none()); + assert!(Submissions::::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::::insert_score_and_metadata( + current_round(), + 1, + Some(low_score), + None, + ); + + // low_score has been added to the sorted scores. + assert_eq!( + ::get_score(), + Some(low_score) + ); + assert!(Submissions::::scores_for(current_round()).len() == 1); + // metadata is None. + assert_eq!( + Submissions::::take_leader_score(current_round()), + Some((1, None)) + ); + // will defensive panic since submission metadata does not exist. + let _ = ::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(|| { + ::set_status(Status::Nothing); + + // no score in sorted scores or leader yet. + assert!(::get_score().is_none()); + assert!(Submissions::::scores_for(current_round()).is_empty()); + assert_eq!(Submissions::::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::::insert_score_and_metadata( + current_round(), + 1, + Some(low_score), + Some(metadata), + ); + + // low_score has been added to the sorted scores. + assert_eq!( + ::get_score(), + Some(low_score) + ); + assert!(Submissions::::scores_for(current_round()).len() == 1); + + // insert a score lower than minimum score. + assert_ok!(::start()); + + // score too low event submitted. + assert_eq!( + verifier_events(), + vec![Event::::VerificationFailed(2, FeasibilityError::ScoreTooLow,)] + ); + }) + } #[test] fn given_better_score_sets_verification_status_to_ongoing() {