diff --git a/substrate/frame/election-provider-multi-block/src/lib.rs b/substrate/frame/election-provider-multi-block/src/lib.rs index 4e01da18c7b15..df3b70d9b3cea 100644 --- a/substrate/frame/election-provider-multi-block/src/lib.rs +++ b/substrate/frame/election-provider-multi-block/src/lib.rs @@ -46,11 +46,11 @@ //! - pallet-unsigned //! ``` //! -//! Each sub-pallet has a specific set of tasks and implement one or more interfaces to expose their -//! functionality to the core pallet: +//! Each sub-pallet has a specific set of tasks and implements one or more interfaces to expose +//! their functionality to the core pallet: //! - The [`verifier`] pallet provides an implementation of the [`verifier::Verifier`] trait, which //! exposes the functionality to verify NPoS solutions in a multi-block context. In addition, it -//! implements [`verifier::AsyncVerifier`] which verifies multi-paged solution asynchronously. +//! implements [`verifier::AsyncVerifier`] which verifies multi-paged solutions asynchronously. //! - The [`signed`] pallet implements the signed phase, where off-chain entities commit to and //! submit their election solutions. This pallet implements the //! [`verifier::SolutionDataProvider`], which is used by the [`verifier`] pallet to fetch solution @@ -62,7 +62,7 @@ //! ### Pallet ordering //! //! Due to the assumptions of when the `on_initialize` hook must be called by the executor for the -//! core pallet and each sub-pallets, it is crucial the the pallets are ordered correctly in the +//! core pallet and each sub-pallets, it is crucial that the pallets are ordered correctly in the //! construct runtime. The ordering must be the following: //! //! ```text @@ -128,10 +128,6 @@ pub use types::*; pub use crate::{unsigned::miner, verifier::Verifier, weights::WeightInfo}; -/// Internal crate re-exports to use across benchmarking and tests. -#[cfg(any(test, feature = "runtime-benchmarks"))] -use crate::verifier::Pallet as PalletVerifier; - /// Log target for this the core EPM-MB pallet. const LOG_TARGET: &'static str = "runtime::multiblock-election"; 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 5fb9c213bff91..636f3acae1aae 100644 --- a/substrate/frame/election-provider-multi-block/src/mock/mod.rs +++ b/substrate/frame/election-provider-multi-block/src/mock/mod.rs @@ -35,6 +35,7 @@ use crate::{ }; use frame_support::{ derive_impl, pallet_prelude::*, parameter_types, traits::fungible::InspectHold, + weights::RuntimeDbWeight, }; use parking_lot::RwLock; use sp_runtime::{ @@ -66,6 +67,14 @@ pub type T = Runtime; pub type Block = frame_system::mocking::MockBlock; pub(crate) type Solver = SequentialPhragmen; +pub struct Weighter; + +impl Get for Weighter { + fn get() -> RuntimeDbWeight { + return RuntimeDbWeight { read: 12, write: 12 } + } +} + frame_election_provider_support::generate_solution_type!( #[compact] pub struct TestNposSolution::< @@ -80,6 +89,7 @@ frame_election_provider_support::generate_solution_type!( impl frame_system::Config for Runtime { type Block = Block; type AccountData = pallet_balances::AccountData; + type DbWeight = Weighter; } parameter_types! { @@ -274,13 +284,14 @@ impl ElectionProvider for MockFallback { #[derive(Copy, Clone)] pub struct ExtBuilder { + minimum_score: Option, core_try_state: bool, signed_try_state: bool, } impl Default for ExtBuilder { fn default() -> Self { - Self { core_try_state: true, signed_try_state: true } + Self { core_try_state: true, signed_try_state: true, minimum_score: None } } } @@ -337,7 +348,12 @@ impl ExtBuilder { } pub(crate) fn desired_targets(self, desired: u32) -> Self { - DesiredTargets::set(desired); + DesiredTargets::set(Ok(desired)); + self + } + + pub(crate) fn no_desired_targets(self) -> Self { + DesiredTargets::set(Err("none")); self } @@ -351,6 +367,11 @@ impl ExtBuilder { self } + pub(crate) fn minimum_score(mut self, score: ElectionScore) -> Self { + self.minimum_score = Some(score); + self + } + pub(crate) fn core_try_state(mut self, enable: bool) -> Self { self.core_try_state = enable; self @@ -362,8 +383,6 @@ impl ExtBuilder { } pub(crate) fn build(self) -> sp_io::TestExternalities { - sp_tracing::try_init_simple(); - let mut storage = frame_system::GenesisConfig::::default().build_storage().unwrap(); let _ = pallet_balances::GenesisConfig:: { balances: vec![ @@ -390,6 +409,11 @@ impl ExtBuilder { } .assimilate_storage(&mut storage); + let _ = verifier_pallet::GenesisConfig:: { + minimum_score: self.minimum_score, + ..Default::default() + } + .assimilate_storage(&mut storage); sp_io::TestExternalities::from(storage) } @@ -414,6 +438,7 @@ impl ExtBuilder { pub(crate) fn build_and_execute(self, test: impl FnOnce() -> ()) { let mut ext = self.build(); + ext.execute_with(|| roll_one()); ext.execute_with(test); ext.execute_with(|| { @@ -520,6 +545,10 @@ pub fn roll_to_phase(phase: Phase) { } } +pub fn set_phase_to(phase: Phase) { + CurrentPhase::::set(phase); +} + pub fn roll_to_export() { while !MultiPhase::current_phase().is_export() { roll_to(System::block_number() + 1); @@ -674,6 +703,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/mock/staking.rs b/substrate/frame/election-provider-multi-block/src/mock/staking.rs index ee44a639d864b..b0f5fb83c26c3 100644 --- a/substrate/frame/election-provider-multi-block/src/mock/staking.rs +++ b/substrate/frame/election-provider-multi-block/src/mock/staking.rs @@ -48,7 +48,7 @@ parameter_types! { (80, 80, bounded_vec![80]), ]; pub static EpochLength: u64 = 30; - pub static DesiredTargets: u32 = 5; + pub static DesiredTargets: data_provider::Result = Ok(5); pub static LastIteratedTargetIndex: Option = None; pub static LastIteratedVoterIndex: Option = None; @@ -146,7 +146,7 @@ impl ElectionDataProvider for MockStaking { fn desired_targets() -> data_provider::Result { match DataProviderErrors::get() { true => Err("MockDataProviderError"), - false => Ok(DesiredTargets::get()), + false => Ok(DesiredTargets::get()?), } } 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 4b818b2a1d14b..eabcd8e4a4ec5 100644 --- a/substrate/frame/election-provider-multi-block/src/signed/mod.rs +++ b/substrate/frame/election-provider-multi-block/src/signed/mod.rs @@ -146,7 +146,7 @@ type BalanceOf = <::Currency as FnInspect>>::Bala pub type CreditOf = Credit, ::Currency>; /// Release strategy for currency held by this pallet. -#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, PartialEq)] +#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebugNoBound, PartialEq, Clone)] pub(crate) enum ReleaseStrategy { /// Releases all held deposit. All, @@ -165,7 +165,7 @@ impl Default for ReleaseStrategy { } /// Metadata of a registered submission. -#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, Default, RuntimeDebugNoBound)] +#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, Default, RuntimeDebugNoBound, Clone)] #[cfg_attr(test, derive(frame_support::PartialEqNoBound, frame_support::EqNoBound))] #[codec(mel_bound(T: Config))] #[scale_info(skip_type_params(T))] @@ -321,7 +321,7 @@ pub mod pallet { /// Wrapper for signed submissions. /// - /// It handle 3 storage items: + /// It handles 3 storage items: /// /// 1. [`SortedScores`]: A flat, striclty sorted, vector with all the submission's scores. The /// vector contains a tuple of `submitter_id` and `claimed_score`. @@ -495,7 +495,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); @@ -580,7 +582,7 @@ pub mod pallet { Submissions::::scores_for(round).last().cloned() } - /// Returns the metadata of a submitter for a given account. + /// Returns the metadata of a submitter for a given round. pub(crate) fn metadata_for( round: u32, who: &T::AccountId, @@ -723,6 +725,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, @@ -839,7 +870,7 @@ pub mod pallet { Ok(()) } - /// Force clean submissions storage for a given (`sumitter`, `round`) tuple. + /// Force clean submissions storage for a given (`submitter`, `round`) tuple. /// /// This pallet expects that submitted pages for `round` may exist IFF a corresponding /// metadata exists. @@ -937,7 +968,7 @@ impl SolutionDataProvider for Pallet { }) } - /// Returns the score of the *best* solution in the queueu. + /// Returns the score of the *best* solution in the queue. fn get_score() -> Option { let round = CorePallet::::current_round(); Submissions::::leader(round).map(|(_who, score)| score) diff --git a/substrate/frame/election-provider-multi-block/src/signed/tests.rs b/substrate/frame/election-provider-multi-block/src/signed/tests.rs index 2f10d5fc1e20b..00702462cfd6b 100644 --- a/substrate/frame/election-provider-multi-block/src/signed/tests.rs +++ b/substrate/frame/election-provider-multi-block/src/signed/tests.rs @@ -16,7 +16,7 @@ // limitations under the License. use super::*; -use crate::{mock::*, verifier::SolutionDataProvider, CurrentPhase, Phase, Verifier}; +use crate::{mock::*, verifier::SolutionDataProvider, Phase, Verifier}; use frame_support::{assert_noop, assert_ok, testing_prelude::*}; use sp_npos_elections::ElectionScore; use sp_runtime::traits::Convert; @@ -256,6 +256,151 @@ mod calls { }) } + #[test] + fn calls_in_phase_other_than_signed() { + ExtBuilder::default().build_and_execute(|| { + let account_id = 99; + + let phases = vec![ + Phase::Halted, + Phase::Off, + Phase::SignedValidation(1), + Phase::Unsigned(1), + Phase::Snapshot(0), + Phase::Export(1), + Phase::Emergency, + ]; + + for phase in phases { + set_phase_to(phase); + + assert_err!( + SignedPallet::register(RuntimeOrigin::signed(account_id), Default::default()), + Error::::NotAcceptingSubmissions + ); + + assert_err!( + SignedPallet::submit_page( + RuntimeOrigin::signed(account_id), + 0, + Some(Default::default()) + ), + Error::::NotAcceptingSubmissions + ); + + assert_err!( + SignedPallet::bail(RuntimeOrigin::signed(account_id)), + Error::::CannotBail + ); + } + }) + } + + #[test] + fn bail_while_having_no_submissions_does_not_modify_balances() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + + // expected base deposit with 0 submissions in the queue. + let base_deposit = ::DepositBase::convert(0); + let page_deposit = ::DepositPerPage::get(); + assert!(base_deposit != 0 && page_deposit != 0 && base_deposit != page_deposit); + + let account_id = 99; + + // account_id has 100 free balance and 0 held balance for elections. + assert_eq!(balances(account_id), (100, 0)); + + assert_ok!(SignedPallet::register( + RuntimeOrigin::signed(account_id), + Default::default() + )); + + // free balance and held deposit updated as expected. + assert_eq!(balances(account_id), (100 - base_deposit, base_deposit)); + + // submit page + assert_ok!(SignedPallet::submit_page( + RuntimeOrigin::signed(account_id), + 0, + Some(Default::default()) + )); + + // free balance and held deposit updated as expected + assert_eq!( + balances(account_id), + (100 - base_deposit - page_deposit, base_deposit + page_deposit) + ); + + let bailing_account_id = 91; + + // bailing_account_id has 100 free balance and 0 held balance for elections. + assert_eq!(balances(bailing_account_id), (100, 0)); + + // account 1 submitted nothing, so bail should have no effect and return error + assert_noop!( + SignedPallet::bail(RuntimeOrigin::signed(bailing_account_id)), + Error::::SubmissionNotRegistered + ); + }) + } + + #[test] + fn force_clear_submission_fails_if_called_in_phase_other_than_off() { + ExtBuilder::default().build_and_execute(|| { + let phases = vec![ + Phase::Signed, + Phase::Snapshot(0), + Phase::SignedValidation(0), + Phase::Unsigned(0), + Phase::Export(0), + Phase::Emergency, + ]; + + let account_id = 99; + for phase in phases { + set_phase_to(phase); + + assert_err!( + SignedPallet::force_clear_submission(RuntimeOrigin::root(), 0, account_id), + Error::::CannotClear, + ); + } + }) + } + + #[test] + fn force_clear_submission_fails_if_submitter_done_no_submissions_at_all() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Off); + let account_id = 99; + + assert_err!( + SignedPallet::force_clear_submission(RuntimeOrigin::root(), 0, account_id), + Error::::NoSubmission, + ); + }) + } + + #[test] + fn force_clear_submission_fails_if_submitter_if_different_round() { + ExtBuilder::default().build_and_execute(|| { + let account_id = 99; + let current_round = MultiPhase::current_round(); + + roll_to_phase(Phase::Off); + + assert_noop!( + SignedPallet::force_clear_submission( + RuntimeOrigin::root(), + current_round + 1, + account_id + ), + Error::::NoSubmission, + ); + }) + } + #[test] fn bail_after_submission_works() { ExtBuilder::default().core_try_state(false).build_and_execute(|| { @@ -314,7 +459,7 @@ mod calls { ); // force phase Off. - CurrentPhase::::set(Phase::Off); + set_phase_to(Phase::Off); assert_ok!(SignedPallet::force_clear_submission(RuntimeOrigin::signed(99), round, 99)); // 3 pages submitted have been cleared from storage. @@ -344,14 +489,14 @@ mod calls { Phase::Export(0), Phase::Emergency, ] { - CurrentPhase::::set(disabled_phase); + set_phase_to(disabled_phase); assert_noop!( SignedPallet::force_clear_submission(RuntimeOrigin::signed(99), round, 99), Error::::CannotClear ); } - CurrentPhase::::set(Phase::Off); + set_phase_to(Phase::Off); // request force clear of a non existing submission. assert_noop!( @@ -431,29 +576,206 @@ mod deposit { mod solution_data_provider { use super::*; - #[test] - fn higher_score_works() { - ExtBuilder::default().build_and_execute(|| { - roll_to_phase(Phase::Signed); + mod get_score { + use super::*; - assert_eq!(::get_score(), None); + #[test] + fn returns_entry_with_highest_minimal_stake() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); - let higher_score = ElectionScore { minimal_stake: 40, ..Default::default() }; - assert_ok!(SignedPallet::register(RuntimeOrigin::signed(40), higher_score)); + assert_eq!(::get_score(), None); - let score = ElectionScore { minimal_stake: 30, ..Default::default() }; - assert_ok!(SignedPallet::register(RuntimeOrigin::signed(30), score)); + let higher_score = ElectionScore { minimal_stake: 40, ..Default::default() }; + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(40), higher_score)); - assert_eq!(::get_score(), Some(higher_score)); - }) + let score = ElectionScore { minimal_stake: 30, ..Default::default() }; + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(30), score)); + + assert_eq!(::get_score(), Some(higher_score)); + }) + } + + #[test] + fn returns_entry_with_highest_sum_stake() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + + assert_eq!(::get_score(), None); + + let higher_score = + ElectionScore { minimal_stake: 40, sum_stake: 10, sum_stake_squared: 0 }; + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(40), higher_score)); + + let score = ElectionScore { minimal_stake: 40, sum_stake: 5, sum_stake_squared: 0 }; + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(30), score)); + + assert_eq!(::get_score(), Some(higher_score)); + }) + } + + #[test] + fn returns_entry_with_lowest_sum_stake_squared() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + + assert_eq!(::get_score(), None); + + let higher_score = + ElectionScore { minimal_stake: 40, sum_stake: 10, sum_stake_squared: 2 }; + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(40), higher_score)); + + let score = + ElectionScore { minimal_stake: 40, sum_stake: 10, sum_stake_squared: 5 }; + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(30), score)); + + assert_eq!(::get_score(), Some(higher_score)); + }) + } } - #[test] - fn get_page_works() { - ExtBuilder::default().build_and_execute(|| { - roll_to_phase(Phase::Signed); - assert_eq!(::get_score(), None); - }) + mod get_paged_solution { + use super::*; + + #[test] + fn returns_previously_submitted_page() { + ExtBuilder::default().build_and_execute(|| { + let origin = RuntimeOrigin::signed(99); + roll_to_phase(Phase::Signed); + + assert_ok!(SignedPallet::register(origin.clone(), Default::default())); + assert_ok!(SignedPallet::submit_page(origin, 0, Some(Default::default()))); + + assert_ne!(::get_paged_solution(0), None) + }) + } + + #[test] + fn returns_none_if_there_are_no_submissions() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + assert_eq!(::get_paged_solution(12345), None) + }) + } + } + + mod report_result { + use super::*; + + #[test] + fn rewards_submitter_of_the_best_solution_given_queued_result() { + ExtBuilder::default().build_and_execute(|| { + let account_id = 99; + let origin = RuntimeOrigin::signed(account_id); + roll_to_phase(Phase::Signed); + + let base_deposit = ::DepositBase::convert(0); + let page_deposit = ::DepositPerPage::get(); + assert!(base_deposit != 0 && page_deposit != 0 && base_deposit != page_deposit); + + // account_id has 100 free balance and 0 held balance for elections. + assert_eq!(balances(account_id), (100, 0)); + + assert_ok!(SignedPallet::register(origin.clone(), Default::default())); + assert_ok!(SignedPallet::submit_page(origin, 0, Some(Default::default()))); + + assert_eq!( + balances(account_id), + (100 - base_deposit - page_deposit, base_deposit + page_deposit) + ); + + SignedPallet::report_result(VerificationResult::Queued); + + // the submitter should receive a reward but his funds are still blocked + assert_eq!( + balances(account_id), + ( + 100 - base_deposit - page_deposit + Reward::get(), + base_deposit + page_deposit + ) + ); + }) + } + + #[test] + fn burns_the_stake_of_the_best_submitter_given_rejected_result() { + ExtBuilder::default().build_and_execute(|| { + let account_id = 99; + let origin = RuntimeOrigin::signed(account_id); + roll_to_phase(Phase::Signed); + + let current_round = MultiPhase::current_round(); + + assert_ok!(SignedPallet::register(origin.clone(), Default::default())); + assert_ok!(SignedPallet::submit_page(origin, 0, Some(Default::default()))); + + assert_eq!( + Submissions::::metadata_for(current_round, &account_id).unwrap(), + SubmissionMetadata { + claimed_score: Default::default(), + deposit: 10, + pages: bounded_vec![false, false, false], + release_strategy: Default::default(), + } + ); + + SignedPallet::report_result(VerificationResult::Rejected); + + assert_eq!( + Submissions::::metadata_for(current_round, &account_id).unwrap(), + SubmissionMetadata { + claimed_score: Default::default(), + deposit: 10, + pages: bounded_vec![false, false, false], + release_strategy: ReleaseStrategy::BurnAll, + } + ); + }) + } + + #[test] + fn burns_the_stake_of_the_best_submitter_given_data_unavailable_result() { + ExtBuilder::default().build_and_execute(|| { + let account_id = 99; + let origin = RuntimeOrigin::signed(account_id); + roll_to_phase(Phase::Signed); + + let current_round = MultiPhase::current_round(); + + assert_ok!(SignedPallet::register(origin.clone(), Default::default())); + assert_ok!(SignedPallet::submit_page(origin, 0, Some(Default::default()))); + + assert_eq!( + Submissions::::metadata_for(current_round, &account_id).unwrap(), + SubmissionMetadata { + claimed_score: Default::default(), + deposit: 10, + pages: bounded_vec![false, false, false], + release_strategy: Default::default(), + } + ); + + SignedPallet::report_result(VerificationResult::DataUnavailable); + + assert_eq!( + Submissions::::metadata_for(current_round, &account_id).unwrap(), + SubmissionMetadata { + claimed_score: Default::default(), + deposit: 10, + pages: bounded_vec![false, false, false], + release_strategy: ReleaseStrategy::BurnAll, + } + ); + }) + } + + #[test] + fn does_nothing_if_no_submissions_where_sent() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + SignedPallet::report_result(VerificationResult::Queued); + }) + } } } @@ -476,7 +798,7 @@ mod e2e { let claimed_score = ElectionScore { minimal_stake: 100, ..Default::default() }; // register submission - assert_ok!(SignedPallet::register(RuntimeOrigin::signed(10), claimed_score,)); + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(10), claimed_score)); // metadata and claimed scores have been stored as expected. assert_eq!( diff --git a/substrate/frame/election-provider-multi-block/src/tests.rs b/substrate/frame/election-provider-multi-block/src/tests.rs index 0bda8d1ae712c..cf365d29a2df4 100644 --- a/substrate/frame/election-provider-multi-block/src/tests.rs +++ b/substrate/frame/election-provider-multi-block/src/tests.rs @@ -466,10 +466,10 @@ mod election_provider { .unwrap(); let supports_page_zero = - PalletVerifier::::feasibility_check(results.solution_pages[0].clone(), 0) + VerifierPallet::feasibility_check(results.solution_pages[0].clone(), 0) .unwrap(); let supports_page_one = - PalletVerifier::::feasibility_check(results.solution_pages[1].clone(), 1) + VerifierPallet::feasibility_check(results.solution_pages[1].clone(), 1) .unwrap(); let s0: Supports = vec![ @@ -520,10 +520,10 @@ mod election_provider { .unwrap(); let supports_page_zero = - PalletVerifier::::feasibility_check(results.solution_pages[0].clone(), 0) + VerifierPallet::feasibility_check(results.solution_pages[0].clone(), 0) .unwrap(); let supports_page_one = - PalletVerifier::::feasibility_check(results.solution_pages[1].clone(), 1) + VerifierPallet::feasibility_check(results.solution_pages[1].clone(), 1) .unwrap(); // consume supports and checks they fit within the max backers per winner bounds. diff --git a/substrate/frame/election-provider-multi-block/src/unsigned/miner.rs b/substrate/frame/election-provider-multi-block/src/unsigned/miner.rs index cc9d85f08f499..e9e5890f7e88a 100644 --- a/substrate/frame/election-provider-multi-block/src/unsigned/miner.rs +++ b/substrate/frame/election-provider-multi-block/src/unsigned/miner.rs @@ -159,7 +159,7 @@ impl Miner { let all_voters: Vec> = all_voter_pages.iter().cloned().flatten().collect::>(); - // these closures generate an efficient index mapping of each tvoter -> the snaphot + // these closures generate an efficient index mapping of each voter -> the snaphot // that they are part of. this needs to be the same indexing fn in the verifier side to // sync when reconstructing the assingments page from a solution. //let binding_targets = all_targets.clone(); 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 3464eb0be4536..ff23469a4cf70 100644 --- a/substrate/frame/election-provider-multi-block/src/unsigned/tests.rs +++ b/substrate/frame/election-provider-multi-block/src/unsigned/tests.rs @@ -16,10 +16,13 @@ // limitations under the License. use super::*; -use crate::{mock::*, PagedVoterSnapshot, Phase, Snapshot, TargetSnapshot, Verifier}; +use crate::{ + mock::*, verifier::QueuedSolution, PagedVoterSnapshot, Phase, Snapshot, TargetSnapshot, + Verifier, +}; use frame_election_provider_support::ElectionProvider; -use frame_support::assert_ok; +use frame_support::{assert_err, assert_ok}; mod calls { use super::*; @@ -134,6 +137,12 @@ mod calls { } mod miner { + use crate::{ + miner::{Miner, MinerError, SnapshotType}, + mock, MinerVoterOf, + }; + use frame_support::BoundedVec; + use super::*; #[test] @@ -207,4 +216,303 @@ mod miner { ); }) } + + #[test] + fn mine_fails_given_less_targets_than_desired() { + ExtBuilder::default().build_and_execute(|| { + let all_voter_pages = Default::default(); + let round = Default::default(); + let pages = Pages::get(); + + // only one target + let mut all_targets: BoundedVec = + Default::default(); + let _ = all_targets.try_push(0); + + // but two desired targets + let desired_targets = 2; + + let solution = Miner::::mine_paged_solution_with_snapshot( + &all_voter_pages, + &all_targets, + pages, + round, + desired_targets, + false, + ); + + assert_err!(solution, MinerError::NotEnoughTargets) + }); + } + + #[test] + fn mine_fails_due_to_unavailable_snapshot() { + ExtBuilder::default().build_and_execute(|| { + let round = Default::default(); + let desired_targets = Default::default(); + let pages = Pages::get(); + + // snapshot of voters for page 0 does not exist + let all_voter_pages = Default::default(); + + // but there is one target in targets snapshot + let mut all_targets: BoundedVec = + Default::default(); + let _ = all_targets.try_push(0); + + let solution = Miner::::mine_paged_solution_with_snapshot( + &all_voter_pages, + &all_targets, + pages, + round, + desired_targets, + false, + ); + + assert_err!(solution, MinerError::SnapshotUnAvailable(SnapshotType::Voters(0))) + }); + } + + #[test] + fn mining_done_solution_calculated() { + ExtBuilder::default() + .pages(1) + .desired_targets(1) + .snapshot_targets_page(1) + .snapshot_voters_page(1) + .build_and_execute(|| { + let round = Default::default(); + let pages = Pages::get(); + + let mut all_voter_pages: BoundedVec< + BoundedVec, mock::VoterSnapshotPerBlock>, + mock::Pages, + > = BoundedVec::with_bounded_capacity(pages.try_into().unwrap()); + + let mut voters_page = BoundedVec::new(); + + // one voter with accountId 12 that votes for validator 0 + let mut voters_votes: BoundedVec = + BoundedVec::new(); + assert_ok!(voters_votes.try_push(0)); + let voter = (12, 1, voters_votes); + + // one voters page with the voter 12 + assert_ok!(voters_page.try_push(voter)); + assert_ok!(all_voter_pages.try_push(voters_page)); + + // one election target with accountId 0 + let mut all_targets: BoundedVec = + Default::default(); + assert_ok!(all_targets.try_push(0)); + + // the election should result with one target chosen + let desired_targets = 1; + + let solution = Miner::::mine_paged_solution_with_snapshot( + &all_voter_pages, + &all_targets, + pages, + round, + desired_targets, + false, + ); + + assert_ok!(solution.clone()); + assert_eq!(solution.unwrap().0.solution_pages.len(), 1); + }); + } +} + +mod pallet { + use super::*; + mod pre_dispatch_checks { + use super::*; + + #[test] + fn pre_dispatch_checks_fails_if_phase_is_not_usnigned() { + ExtBuilder::default().build_and_execute(|| { + let phases = vec![ + Phase::Signed, + Phase::Snapshot(0), + Phase::SignedValidation(0), + Phase::Export(0), + Phase::Emergency, + Phase::Off, + ]; + + for phase in phases { + set_phase_to(phase); + let claimed_score = + ElectionScore { minimal_stake: 1, sum_stake: 1, sum_stake_squared: 1 }; + assert_err!(UnsignedPallet::pre_dispatch_checks(0, &claimed_score), ()); + } + }); + } + + #[test] + fn pre_dispatch_checks_fails_if_page_is_higher_than_msp() { + ExtBuilder::default().build_and_execute(|| { + set_phase_to(Phase::Unsigned(0)); + let claimed_score = + ElectionScore { minimal_stake: 1, sum_stake: 1, sum_stake_squared: 1 }; + assert_err!( + UnsignedPallet::pre_dispatch_checks(MultiPhase::msp() + 1, &claimed_score), + () + ); + }); + } + + #[test] + fn pre_dispatch_checks_fails_if_score_quality_is_insufficient() { + ExtBuilder::default().pages(1).build_and_execute(|| { + roll_to_phase(Phase::Signed); + + // a solution already stored + let score = + ElectionScore { minimal_stake: u128::max_value(), ..Default::default() }; + QueuedSolution::::finalize_solution(score); + + set_phase_to(Phase::Unsigned(0)); + let claimed_score = + ElectionScore { minimal_stake: 1, sum_stake: 1, sum_stake_squared: 1 }; + assert_err!(UnsignedPallet::pre_dispatch_checks(0, &claimed_score), ()); + }); + } + + #[test] + fn pre_dispatch_checks_succeeds_for_correct_page_and_better_score() { + ExtBuilder::default().build_and_execute(|| { + set_phase_to(Phase::Unsigned(0)); + let claimed_score = + ElectionScore { minimal_stake: 1, sum_stake: 1, sum_stake_squared: 1 }; + assert_ok!(UnsignedPallet::pre_dispatch_checks(0, &claimed_score)); + }); + } + } + + mod do_sync_offchain_worker { + use sp_runtime::offchain::storage::StorageValueRef; + + use super::*; + + #[test] + fn cached_results_clean_up_at_export_phase() { + let (mut ext, _) = ExtBuilder::default().build_offchainify(0); + ext.execute_with(|| { + set_phase_to(Phase::Export(0)); + + let score_storage = StorageValueRef::persistent( + &OffchainWorkerMiner::::OFFCHAIN_CACHED_SCORE, + ); + + // add some score to cache. + assert_ok!(score_storage.mutate::<_, (), _>(|_| Ok(ElectionScore::default()))); + + // there's something in the cache before worker run + assert_eq!( + StorageValueRef::persistent( + &OffchainWorkerMiner::::OFFCHAIN_CACHED_SCORE, + ) + .get::() + .unwrap(), + Some(ElectionScore::default()) + ); + + // call sync offchain workers in Export phase will clear up the cache. + assert_ok!(UnsignedPallet::do_sync_offchain_worker(0)); + + assert_eq!( + StorageValueRef::persistent( + &OffchainWorkerMiner::::OFFCHAIN_CACHED_SCORE, + ) + .get::() + .unwrap(), + None + ); + }); + } + + #[test] + fn worker_fails_to_mine_solution() { + let (mut ext, _) = ExtBuilder::default().no_desired_targets().build_offchainify(0); + ext.execute_with(|| { + roll_to_phase(Phase::Snapshot(crate::Pallet::::lsp())); + set_phase_to(Phase::Unsigned(0)); + assert!(UnsignedPallet::do_sync_offchain_worker(0).is_err()); + }); + } + + #[test] + fn mined_solution_score_not_high_enough() { + let (mut ext, pool) = ExtBuilder::default().build_offchainify(0); + ext.execute_with(|| { + // a solution already stored + let score = + ElectionScore { minimal_stake: u128::max_value(), ..Default::default() }; + QueuedSolution::::finalize_solution(score); + + set_phase_to(Phase::Unsigned(0)); + assert!(UnsignedPallet::do_sync_offchain_worker(0).is_ok()); + + // no transaction was sent + assert_eq!(pool.read().transactions.iter().count(), 0); + }); + } + + #[test] + fn solution_page_submitted() { + let (mut ext, pool) = ExtBuilder::default().pages(1).build_offchainify(0); + ext.execute_with(|| { + assert_eq!(pool.read().transactions.iter().count(), 0); + + roll_to_phase(Phase::Signed); + let _ = mine_full(0).unwrap(); + assert!(::next_missing_solution_page().is_some()); + + set_phase_to(Phase::Unsigned(0)); + assert!(UnsignedPallet::do_sync_offchain_worker(0).is_ok()); + + assert_eq!(pool.read().transactions.iter().count(), 1); + // TODO: zebedeusz - check that the sent transaction is an inherent + // TODO: zebedeusz - check the inherent content + }); + } + } +} + +mod hooks { + use frame_support::traits::Hooks; + + use crate::mock; + + use super::*; + + #[test] + fn on_initialize_returns_default_weight_in_non_off_phases() { + ExtBuilder::default().build_and_execute(|| { + let phases = vec![ + Phase::Signed, + Phase::Snapshot(0), + Phase::SignedValidation(0), + Phase::Unsigned(0), + Phase::Export(0), + Phase::Emergency, + ]; + + for phase in phases { + set_phase_to(phase); + assert_eq!(UnsignedPallet::on_initialize(0), Default::default()); + } + }); + } + + #[test] + fn on_initialize_returns_specific_weight_in_off_phase() { + ExtBuilder::default().build_and_execute(|| { + set_phase_to(Phase::Off); + assert_ne!(UnsignedPallet::on_initialize(0), Default::default()); + assert_eq!(UnsignedPallet::on_initialize(0), mock::Weighter::get().reads_writes(1, 1)); + }); + } } 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 1fb54aa213e53..c05f0d543c895 100644 --- a/substrate/frame/election-provider-multi-block/src/verifier/impls.rs +++ b/substrate/frame/election-provider-multi-block/src/verifier/impls.rs @@ -30,7 +30,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 { @@ -62,34 +62,34 @@ pub(crate) mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] - pub enum Event { + pub enum Event { /// A verificaction failed at the given page. - VerificationFailed(PageIndex, FeasibilityError), + VerificationFailed { page: PageIndex, error: FeasibilityError }, /// The final verifications of the `finalize_verification` failed. If this error happened, /// all the single pages passed the feasibility checks. - FinalVerificationFailed(FeasibilityError), + FinalVerificationFailed { error: FeasibilityError }, /// The given page has been correctly verified, with the number of backers that are part of /// the page. - Verified(PageIndex, u32), + Verified { page: PageIndex, backers: u32 }, /// A new solution with the given score has replaced the previous best solution, if any. - Queued(ElectionScore, Option), + Queued { score: ElectionScore, old_score: Option }, /// The solution data was not available for a specific page. - SolutionDataUnavailable(PageIndex), + SolutionDataUnavailable { page: PageIndex }, } /// A wrapper type of the storage items related to the queued solution. /// /// It manages the following storage types: - /// + ///∂ /// - [`QueuedSolutionX`]: variant X of the queued solution. /// - [`QueuedSolutionY`]: variant Y of the queued solution. /// - [`QueuedValidVariant`]: pointer to which variant is the currently valid. - /// - [`QueuedSolutionScore`]: the soltution score of the current valid variant. + /// - [`QueuedSolutionScore`]: the solution score of the current valid variant. /// - [`QueuedSolutionBackings`]. /// /// Note that, as an async verification is progressing, the paged solution is kept in the /// invalid variant storage. A solution is considered valid only when all the single page and - /// full solution checks have been perform based on the stored [`QueuedSolutionBackings`]. for + /// full solution checks have been performed based on the stored [`QueuedSolutionBackings`]. for /// the corresponding in-verification solution. After the solution verification is successful, /// the election score can be calculated and stored. /// @@ -101,7 +101,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 { @@ -287,6 +287,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); @@ -361,8 +377,11 @@ impl Verifier for Pallet { partial_score, page ); - Self::deposit_event(Event::::Verified(page, supports.len() as u32)); - Self::deposit_event(Event::::Queued(partial_score, maybe_current_score)); + Self::deposit_event(Event::::Verified { page, backers: supports.len() as u32 }); + Self::deposit_event(Event::::Queued { + score: partial_score, + old_score: maybe_current_score, + }); Ok(supports) }, Err(err) => { @@ -373,7 +392,7 @@ impl Verifier for Pallet { err, page ); - Self::deposit_event(Event::::VerificationFailed(page, err.clone())); + Self::deposit_event(Event::::VerificationFailed { page, error: err.clone() }); Err(err) }, } @@ -432,12 +451,12 @@ impl AsyncVerifier for Pallet { let claimed_score = Self::SolutionDataProvider::get_score().unwrap_or_default(); if Self::ensure_score_quality(claimed_score).is_err() { - Self::deposit_event(Event::::VerificationFailed( - CorePallet::::msp(), - FeasibilityError::ScoreTooLow, - )); + Self::deposit_event(Event::::VerificationFailed { + page: CorePallet::::msp(), + error: 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 { @@ -512,7 +531,7 @@ impl Pallet { VerificationStatus::::put(Status::Nothing); T::SolutionDataProvider::report_result(VerificationResult::DataUnavailable); - Self::deposit_event(Event::::SolutionDataUnavailable(current_page)); + Self::deposit_event(Event::::SolutionDataUnavailable { page: current_page }); return ::WeightInfo::on_initialize_ongoing_failed( max_backers_winner, @@ -526,7 +545,10 @@ impl Pallet { // TODO: can refator out some of these code blocks to clean up the code. let weight_consumed = match maybe_supports { Ok(supports) => { - Self::deposit_event(Event::::Verified(current_page, supports.len() as u32)); + Self::deposit_event(Event::::Verified { + page: current_page, + backers: supports.len() as u32, + }); QueuedSolution::::set_page(current_page, supports); if current_page > CorePallet::::lsp() { @@ -570,9 +592,12 @@ impl Pallet { } } }, - Err(err) => { + Err(error) => { // the paged solution is invalid. - Self::deposit_event(Event::::VerificationFailed(current_page, err)); + Self::deposit_event(Event::::VerificationFailed { + page: current_page, + error, + }); VerificationStatus::::put(Status::Nothing); QueuedSolution::::clear_invalid_and_backings(); T::SolutionDataProvider::report_result(VerificationResult::Rejected); @@ -623,10 +648,10 @@ impl Pallet { match (final_score == claimed_score, winner_count <= desired_targets) { (true, true) => { QueuedSolution::::finalize_solution(final_score); - Self::deposit_event(Event::::Queued( - final_score, - QueuedSolution::::queued_score(), - )); + Self::deposit_event(Event::::Queued { + score: final_score, + old_score: QueuedSolution::::queued_score(), + }); Ok(()) }, @@ -637,7 +662,10 @@ impl Pallet { }) .map_err(|err| { sublog!(warn, "verifier", "finalizing the solution was invalid due to {:?}", err); - Self::deposit_event(Event::::VerificationFailed(Zero::zero(), err.clone())); + Self::deposit_event(Event::::VerificationFailed { + page: Zero::zero(), + error: err.clone(), + }); err }); @@ -657,6 +685,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 2082b34b023e4..250de64b42616 100644 --- a/substrate/frame/election-provider-multi-block/src/verifier/mod.rs +++ b/substrate/frame/election-provider-multi-block/src/verifier/mod.rs @@ -79,8 +79,9 @@ use sp_runtime::RuntimeDebug; // public re-exports. pub use impls::pallet::{ - Call, Config, Event, Pallet, __substrate_call_check, __substrate_event_check, tt_default_parts, - tt_default_parts_v2, tt_error_token, + Call, Config, Event, Pallet, QueuedSolution, __substrate_call_check, __substrate_event_check, + __substrate_genesis_config_check, tt_default_parts, tt_default_parts_v2, tt_error_token, + GenesisConfig, }; /// Errors related to the solution feasibility checks. @@ -117,7 +118,9 @@ impl From for FeasibilityError { /// /// This pallet is either processing an async verification or doing nothing. A single page /// verification can only be done while the pallet has status [`Status::Nothing`]. -#[derive(Encode, Decode, scale_info::TypeInfo, Clone, Copy, MaxEncodedLen, RuntimeDebug)] +#[derive( + Encode, Decode, scale_info::TypeInfo, Clone, Copy, MaxEncodedLen, RuntimeDebug, PartialEq, +)] pub enum Status { /// A paged solution is ongoing and the next page to be verified is indicated in the inner /// value. 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 b15397155cd20..983fefb79a167 100644 --- a/substrate/frame/election-provider-multi-block/src/verifier/tests.rs +++ b/substrate/frame/election-provider-multi-block/src/verifier/tests.rs @@ -1,4 +1,4 @@ -// This file is part of Substrate. +// Threports_result_rejection_workilpart of Substrate. // Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 @@ -15,12 +15,13 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::*; use crate::{ mock::*, - verifier::{impls::pallet::*, *}, + verifier::{impls::pallet::*, SolutionDataProvider}, Phase, }; -use frame_support::{assert_err, assert_noop, assert_ok}; +use frame_support::testing_prelude::*; use sp_npos_elections::ElectionScore; use sp_runtime::Perbill; @@ -112,14 +113,144 @@ mod feasibility_check { ); }) } + + #[test] + fn targets_not_in_snapshot() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Off); + + crate::Snapshot::::kill(); + assert_eq!(crate::Snapshot::::targets(), None); + + assert_noop!( + VerifierPallet::feasibility_check(TestNposSolution::default(), 0), + FeasibilityError::SnapshotUnavailable, + ); + }) + } + + #[test] + fn voters_not_in_snapshot() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + + let _ = crate::PagedVoterSnapshot::::clear(u32::MAX, None); + + assert_eq!(crate::Snapshot::::targets().unwrap().len(), 8); + assert_eq!(crate::Snapshot::::voters(0), None); + + assert_noop!( + VerifierPallet::feasibility_check(TestNposSolution::default(), 0), + FeasibilityError::SnapshotUnavailable, + ); + }) + } + + #[test] + fn desired_targets_not_in_snapshot() { + ExtBuilder::default().no_desired_targets().build_and_execute(|| { + set_phase_to(Phase::Signed); + assert_err!( + VerifierPallet::feasibility_check(TestNposSolution::default(), 0), + FeasibilityError::SnapshotUnavailable, + ); + }) + } } mod sync_verifier { use super::*; - #[test] - fn sync_verifier_simple_works() { - ExtBuilder::default().build_and_execute(|| {}) + mod verify_synchronous { + use super::*; + + #[test] + fn given_better_solution_stores_provided_page_as_valid_solution() { + ExtBuilder::default().pages(1).build_and_execute(|| { + roll_to_phase(Phase::Signed); + let solution = mine_full(0).unwrap(); + + // empty solution storage items before verification + assert!(::next_missing_solution_page().is_some()); + assert!(QueuedSolutionBackings::::get(0).is_none()); + assert!(match QueuedSolution::::invalid() { + SolutionPointer::X => QueuedSolutionX::::get(0), + SolutionPointer::Y => QueuedSolutionY::::get(0), + } + .is_none()); + + assert_ok!(::verify_synchronous( + solution.solution_pages[0].clone(), + solution.score, + 0, + )); + + // solution storage items filled after verification + assert!(QueuedSolutionBackings::::get(0).is_some()); + assert_eq!(::next_missing_solution_page(), None); + assert!(match QueuedSolution::::invalid() { + SolutionPointer::X => QueuedSolutionX::::get(0), + SolutionPointer::Y => QueuedSolutionY::::get(0), + } + .is_some()); + }) + } + + #[test] + fn returns_error_if_score_quality_is_lower_than_expected() { + ExtBuilder::default().pages(1).build_and_execute(|| { + roll_to_phase(Phase::Signed); + + // a solution already stored + let score = + ElectionScore { minimal_stake: u128::max_value(), ..Default::default() }; + QueuedSolution::::finalize_solution(score); + + let solution = mine_full(0).unwrap(); + assert_err!( + ::verify_synchronous( + solution.solution_pages[0].clone(), + solution.score, + 0, + ), + FeasibilityError::ScoreTooLow + ); + }) + } + + #[test] + fn returns_error_if_solution_fails_feasibility_check() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + + let solution = mine_full(0).unwrap(); + let _ = crate::PagedVoterSnapshot::::clear(u32::MAX, None); + assert_err!( + ::verify_synchronous( + solution.solution_pages[0].clone(), + solution.score, + 0, + ), + FeasibilityError::SnapshotUnavailable + ); + }) + } + + #[test] + fn returns_error_if_computed_score_is_different_than_provided() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + let solution = mine_full(0).unwrap(); + assert_err!( + ::verify_synchronous( + solution.solution_pages[0].clone(), + solution.score, + 0, + ), + FeasibilityError::InvalidScore + ); + }) + } } #[test] @@ -159,4 +290,357 @@ mod async_verifier { fn async_verifier_simple_works() { ExtBuilder::default().build_and_execute(|| {}) } + + mod force_finalize_verification { + use frame_support::assert_err; + use sp_npos_elections::ElectionScore; + + use super::{AsyncVerifier, VerifierPallet, *}; + + #[test] + fn failed_score_computation_returns_underlying_error() { + ExtBuilder::default().build_and_execute(|| { + let claimed_score: ElectionScore = Default::default(); + assert_err!( + ::force_finalize_verification(claimed_score), + FeasibilityError::Incomplete + ); + }); + } + + #[test] + fn final_score_differs_from_claimed_score() { + ExtBuilder::default().pages(1).build_and_execute(|| { + roll_to_phase(Phase::Signed); + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(99), Default::default())); + QueuedSolution::::set_page(0, Default::default()); + + let claimed_score: ElectionScore = Default::default(); + assert_err!( + ::force_finalize_verification(claimed_score), + FeasibilityError::InvalidScore + ); + }); + } + + #[test] + fn winner_count_differs_from_desired_targets() { + ExtBuilder::default().pages(1).desired_targets(2).build_and_execute(|| { + roll_to_phase(Phase::Signed); + let solution = mine_full(0).unwrap(); + let supports = + VerifierPallet::feasibility_check(solution.solution_pages[0].clone(), 0); + assert!(supports.is_ok()); + let supports = supports.unwrap(); + + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(99), solution.score)); + QueuedSolution::::set_page(0, supports); + + // setting desired targets value to a lower one, to make the solution verification + // fail + self::DesiredTargets::set(Ok(1)); + assert_eq!(crate::Snapshot::::desired_targets(), Some(1)); + + // just to make sure there's more winners in the current solution than desired + // targets + let winner_count = QueuedSolution::::compute_current_score() + .map(|(_, winner_count)| winner_count) + .unwrap(); + assert_eq!(winner_count, 2); + + assert_err!( + ::force_finalize_verification(solution.score), + FeasibilityError::WrongWinnerCount + ); + }); + } + + #[test] + fn valid_score_results_with_solution_finalized() { + ExtBuilder::default().pages(1).desired_targets(2).build_and_execute(|| { + roll_to_phase(Phase::Signed); + let solution = mine_full(0).unwrap(); + let supports = + VerifierPallet::feasibility_check(solution.solution_pages[0].clone(), 0); + assert!(supports.is_ok()); + let supports = supports.unwrap(); + + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(99), solution.score)); + QueuedSolution::::set_page(0, supports); + + // no stored score so far + assert!(QueuedSolution::::queued_score().is_none()); + + assert_ok!(::force_finalize_verification( + solution.score + )); + + // stored score is the submitted one + assert_eq!(QueuedSolution::::queued_score(), Some(solution.score)); + }); + } + } + + #[test] + fn stopping_the_verification_cleans_storage_items() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(99), Default::default())); + QueuedSolution::::set_page(0, Default::default()); + + assert_ne!( + QueuedSolutionX::::iter().count() + + QueuedSolutionY::::iter().count(), + 0 + ); + assert_ne!(QueuedSolutionBackings::::iter().count(), 0); + + ::stop(); + + assert_eq!(::status(), Status::Nothing); + assert_eq!(QueuedSolutionX::::iter().count(), 0); + assert_eq!(QueuedSolutionY::::iter().count(), 0); + assert_eq!(QueuedSolutionBackings::::iter().count(), 0); + }); + } + + mod verification_start { + use super::*; + use crate::signed::pallet::Submissions; + + #[test] + fn fails_if_verification_is_ongoing() { + ExtBuilder::default().build_and_execute(|| { + ::set_status(Status::Ongoing(0)); + assert_err!(::start(), "verification ongoing"); + }); + } + + #[test] + 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 { + page: 2, + error: FeasibilityError::ScoreTooLow, + }] + ); + }) + } + + #[test] + fn given_better_score_sets_verification_status_to_ongoing() { + ExtBuilder::default().build_and_execute(|| { + roll_to_phase(Phase::Signed); + let msp = crate::Pallet::::msp(); + + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(99), Default::default())); + assert_eq!(::status(), Status::Nothing); + + assert_ok!(::start()); + + assert_eq!(::status(), Status::Ongoing(msp)); + }); + } + } +} + +mod hooks { + use super::*; + use crate::signed::pallet::Submissions; + use frame_support::traits::Hooks; + + #[test] + fn on_initialize_ongoing_fails() { + ExtBuilder::default().pages(1).build_and_execute(|| { + let round = current_round(); + let score = ElectionScore { minimal_stake: 10, sum_stake: 10, sum_stake_squared: 10 }; + let metadata = Submissions::submission_metadata_from( + score, + Default::default(), + Default::default(), + Default::default(), + ); + Submissions::::insert_score_and_metadata( + round, + 1, + Some(score), + Some(metadata.clone()), + ); + + // force ongoing status. + ::set_status(Status::Ongoing(0)); + assert_eq!(::status(), Status::Ongoing(0)); + + // no events yet. + assert!(verifier_events().is_empty()); + + // progress the block. + roll_one(); + + assert_eq!( + verifier_events(), + vec![Event::::VerificationFailed { + page: 0, + error: FeasibilityError::SnapshotUnavailable + }] + ); + }); + } + + #[test] + fn on_initialize_ongoing_invalid_score() { + ExtBuilder::default().pages(1).desired_targets(2).build_and_execute(|| { + // solution insertion + let round = current_round(); + let score = ElectionScore { minimal_stake: 10, sum_stake: 10, sum_stake_squared: 10 }; + let metadata = Submissions::submission_metadata_from( + score, + Default::default(), + Default::default(), + Default::default(), + ); + Submissions::::insert_score_and_metadata(round, 1, Some(score), Some(metadata)); + + // needed for targets to exist in the snapshot + roll_to_phase(Phase::Signed); + let _ = mine_full(0).unwrap(); + + ::set_status(Status::Ongoing(0)); + + assert_eq!(VerifierPallet::on_initialize(0), Default::default()); + + assert_eq!( + verifier_events(), + vec![ + Event::::Verified { page: 0, backers: 0 }, + Event::::VerificationFailed { + page: 0, + error: FeasibilityError::InvalidScore + } + ] + ); + assert!(QueuedSolution::::queued_score().is_none()); + assert_eq!(VerificationStatus::::get(), Status::Nothing); + }); + } + + #[test] + fn on_initialize_ongoing_works() { + ExtBuilder::default().pages(1).desired_targets(2).build_and_execute(|| { + roll_to_phase(Phase::Signed); + let solution = mine_full(0).unwrap(); + let supports = VerifierPallet::feasibility_check(solution.solution_pages[0].clone(), 0); + assert!(supports.is_ok()); + + assert_ok!(SignedPallet::register(RuntimeOrigin::signed(99), solution.score)); + QueuedSolution::::set_page(0, supports.unwrap()); + + assert_ok!(::force_finalize_verification( + solution.score + )); + + ::set_status(Status::Ongoing(0)); + + assert_eq!(VerifierPallet::on_initialize(0), Default::default()); + + assert_eq!( + verifier_events(), + vec![ + Event::::Queued { score: solution.score, old_score: Some(solution.score) }, + Event::Verified { page: 0, backers: 0 }, + Event::VerificationFailed { page: 0, error: FeasibilityError::InvalidScore }, + ] + ); + + assert_eq!(VerificationStatus::::get(), Status::Nothing); + }); + } } diff --git a/substrate/frame/election-provider-multi-phase/src/lib.rs b/substrate/frame/election-provider-multi-phase/src/lib.rs index 8676be438300f..fea15f1d243c7 100644 --- a/substrate/frame/election-provider-multi-phase/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/src/lib.rs @@ -172,7 +172,7 @@ //! //! ## Error types //! -//! This pallet provides a verbose error system to ease future debugging and debugging. The overall +//! This pallet provides a verbose error system to ease debugging. The overall //! hierarchy of errors is as follows: //! //! 1. [`pallet::Error`]: These are the errors that can be returned in the dispatchables of the