Skip to content
This repository has been archived by the owner on Feb 21, 2024. It is now read-only.

Commit

Permalink
add an absolute measure of election score on-chain as a parameter (pa…
Browse files Browse the repository at this point in the history
…ritytech#8903)

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

* make it storage item

* line width

* some nits

* Apply suggestions from code review

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

* make a few more things pub

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
  • Loading branch information
3 people authored and nazar-pc committed Aug 8, 2021
1 parent 203502f commit 8ac2ccb
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 10 deletions.
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|
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

0 comments on commit 8ac2ccb

Please sign in to comment.