Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

add an absolute measure of election score on-chain as a parameter #8903

Merged
6 commits merged into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type CompactSolution = NposCompactSolution16;
type Fallback = Fallback;
type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight<Runtime>;
type ForceOrigin = EnsureRootOrHalfCouncil;
type BenchmarkingConfig = ();
}

Expand Down
68 changes: 65 additions & 3 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,8 @@ pub enum FeasibilityError {
InvalidScore,
/// The provided round is incorrect.
InvalidRound,
/// Comparison against `MinimumUntrustedScore` failed.
UntrustedScoreTooLow,
}

impl From<sp_npos_elections::Error> for FeasibilityError {
Expand Down Expand Up @@ -579,6 +581,9 @@ pub mod pallet {
/// Configuration for the fallback
type Fallback: Get<FallbackStrategy>;

/// Origin that can set the minimum score.
type ForceOrigin: EnsureOrigin<Self::Origin>;

/// The configuration of benchmarking.
type BenchmarkingConfig: BenchmarkingConfig;

Expand Down Expand Up @@ -773,6 +778,21 @@ pub mod pallet {

Ok(None.into())
}

/// Set a new value for `MinimumUntrustedScore`.
///
/// Dispatch origin must be aligned with `T::ForceOrigin`.
///
/// This check can be turned off by setting the value to `None`.
#[pallet::weight(T::DbWeight::get().writes(1))]
fn set_minimum_untrusted_score(
origin: OriginFor<T>,
maybe_next_score: Option<ElectionScore>,
) -> DispatchResult {
T::ForceOrigin::ensure_origin(origin)?;
<MinimumUntrustedScore<T>>::set(maybe_next_score);
Ok(())
}
}

#[pallet::event]
Expand Down Expand Up @@ -909,6 +929,14 @@ pub mod pallet {
#[pallet::getter(fn snapshot_metadata)]
pub type SnapshotMetadata<T: Config> = StorageValue<_, SolutionOrSnapshotSize>;

/// The minimum score that each 'untrusted' solution must attain in order to be considered
/// feasible.
///
/// Can be set via `set_minimum_untrusted_score`.
#[pallet::storage]
#[pallet::getter(fn minimum_untrusted_score)]
pub type MinimumUntrustedScore<T: Config> = StorageValue<_, ElectionScore>;

#[pallet::pallet]
#[pallet::generate_store(pub(super) trait Store)]
pub struct Pallet<T>(PhantomData<T>);
Expand Down Expand Up @@ -959,7 +987,7 @@ impl<T: Config> Pallet<T> {
///
/// Returns `Ok(snapshot_weight)` if success, where `snapshot_weight` is the weight that
/// needs to recorded for the creation of snapshot.
pub(crate) fn on_initialize_open_signed() -> Result<Weight, ElectionError> {
pub fn on_initialize_open_signed() -> Result<Weight, ElectionError> {
let weight = Self::create_snapshot()?;
<CurrentPhase<T>>::put(Phase::Signed);
Self::deposit_event(Event::SignedPhaseStarted(Self::round()));
Expand All @@ -972,7 +1000,7 @@ impl<T: Config> Pallet<T> {
///
/// Returns `Ok(snapshot_weight)` if success, where `snapshot_weight` is the weight that
/// needs to recorded for the creation of snapshot.
pub(crate) fn on_initialize_open_unsigned(
pub fn on_initialize_open_unsigned(
need_snapshot: bool,
enabled: bool,
now: T::BlockNumber,
Expand All @@ -997,7 +1025,7 @@ impl<T: Config> Pallet<T> {
/// 3. [`DesiredTargets`]
///
/// Returns `Ok(consumed_weight)` if operation is okay.
pub(crate) fn create_snapshot() -> Result<Weight, ElectionError> {
pub fn create_snapshot() -> Result<Weight, ElectionError> {
let target_limit = <CompactTargetIndexOf<T>>::max_value().saturated_into::<usize>();
let voter_limit = <CompactVoterIndexOf<T>>::max_value().saturated_into::<usize>();

Expand Down Expand Up @@ -1052,6 +1080,15 @@ impl<T: Config> Pallet<T> {
// upon arrival, thus we would then remove it here. Given overlay it is cheap anyhow
ensure!(winners.len() as u32 == desired_targets, FeasibilityError::WrongWinnerCount);

// ensure that the solution's score can pass absolute min-score.
let submitted_score = solution.score.clone();
ensure!(
Self::minimum_untrusted_score().map_or(true, |min_score|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By having this check here, would it affect the weight of submit_unsigned? Or is this operation minuscule enough to not have any noticeable effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insignificant to reweigh.

sp_npos_elections::is_score_better(submitted_score, min_score, Perbill::zero())
),
FeasibilityError::UntrustedScoreTooLow
);

// read the entire snapshot.
let RoundSnapshot { voters: snapshot_voters, targets: snapshot_targets } =
Self::snapshot().ok_or(FeasibilityError::SnapshotUnavailable)?;
Expand Down Expand Up @@ -1596,6 +1633,31 @@ mod tests {
})
}

#[test]
fn untrusted_score_verification_is_respected() {
ExtBuilder::default().build_and_execute(|| {
roll_to(15);
assert_eq!(MultiPhase::current_phase(), Phase::Signed);


let (solution, _) = MultiPhase::mine_solution(2).unwrap();
// default solution has a score of [50, 100, 5000].
assert_eq!(solution.score, [50, 100, 5000]);

<MinimumUntrustedScore<Runtime>>::put([49, 0, 0]);
assert_ok!(MultiPhase::feasibility_check(solution.clone(), ElectionCompute::Signed));

<MinimumUntrustedScore<Runtime>>::put([51, 0, 0]);
assert_noop!(
MultiPhase::feasibility_check(
solution,
ElectionCompute::Signed
),
FeasibilityError::UntrustedScoreTooLow,
);
})
}

#[test]
fn number_of_voters_allowed_2sec_block() {
// Just a rough estimate with the substrate weights.
Expand Down
1 change: 1 addition & 0 deletions frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ impl crate::Config for Runtime {
type BenchmarkingConfig = ();
type OnChainAccuracy = Perbill;
type Fallback = Fallback;
type ForceOrigin = frame_system::EnsureRoot<AccountId>;
type CompactSolution = TestCompact;
}

Expand Down
12 changes: 6 additions & 6 deletions frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<T: Config> Pallet<T> {
}

/// Mine a new solution as a call. Performs all checks.
fn mine_checked_call() -> Result<Call<T>, MinerError> {
pub fn mine_checked_call() -> Result<Call<T>, MinerError> {
let iters = Self::get_balancing_iters();
// get the solution, with a load of checks to ensure if submitted, IT IS ABSOLUTELY VALID.
let (raw_solution, witness) = Self::mine_and_check(iters)?;
Expand Down Expand Up @@ -227,7 +227,7 @@ impl<T: Config> Pallet<T> {
// perform basic checks of a solution's validity
//
// Performance: note that it internally clones the provided solution.
fn basic_checks(
pub fn basic_checks(
raw_solution: &RawSolution<CompactOf<T>>,
solution_type: &str,
) -> Result<(), MinerError> {
Expand Down Expand Up @@ -404,7 +404,7 @@ impl<T: Config> Pallet<T> {
///
/// Indeed, the score must be computed **after** this step. If this step reduces the score too
/// much or remove a winner, then the solution must be discarded **after** this step.
fn trim_assignments_weight(
pub fn trim_assignments_weight(
desired_targets: u32,
size: SolutionOrSnapshotSize,
max_weight: Weight,
Expand Down Expand Up @@ -438,7 +438,7 @@ impl<T: Config> Pallet<T> {
///
/// The score must be computed **after** this step. If this step reduces the score too much,
/// then the solution must be discarded.
pub(crate) fn trim_assignments_length(
pub fn trim_assignments_length(
max_allowed_length: u32,
assignments: &mut Vec<IndexAssignmentOf<T>>,
encoded_size_of: impl Fn(&[IndexAssignmentOf<T>]) -> Result<usize, sp_npos_elections::Error>,
Expand Down Expand Up @@ -579,7 +579,7 @@ impl<T: Config> Pallet<T> {
///
/// Returns `Ok(())` if offchain worker limit is respected, `Err(reason)` otherwise. If `Ok()`
/// is returned, `now` is written in storage and will be used in further calls as the baseline.
pub(crate) fn ensure_offchain_repeat_frequency(now: T::BlockNumber) -> Result<(), MinerError> {
pub fn ensure_offchain_repeat_frequency(now: T::BlockNumber) -> Result<(), MinerError> {
let threshold = T::OffchainRepeat::get();
let last_block = StorageValueRef::persistent(&OFFCHAIN_LAST_BLOCK);

Expand Down Expand Up @@ -619,7 +619,7 @@ impl<T: Config> Pallet<T> {
///
/// NOTE: Ideally, these tests should move more and more outside of this and more to the miner's
/// code, so that we do less and less storage reads here.
pub(crate) fn unsigned_pre_dispatch_checks(
pub fn unsigned_pre_dispatch_checks(
solution: &RawSolution<CompactOf<T>>,
) -> DispatchResult {
// ensure solution is timely. Don't panic yet. This is a cheap check.
Expand Down
2 changes: 1 addition & 1 deletion frame/merkle-mountain-range/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ decl_storage! {
decl_module! {
/// A public part of the pallet.
pub struct Module<T: Config<I>, I: Instance = DefaultInstance> for enum Call where origin: T::Origin {
fn on_initialize(n: T::BlockNumber) -> Weight {
fn on_initialize(_n: T::BlockNumber) -> Weight {
use primitives::LeafDataProvider;
let leaves = Self::mmr_leaves();
let peaks_before = mmr::utils::NodesUtils::new(leaves).number_of_peaks();
Expand Down