Skip to content

Commit b6af2a9

Browse files
andreitrandandreitrand
committed
Remove "SolutionImprovementThreshold" logic
Fixes #9119 The threshold mechanism used by the "election-provider-multi-block" verifier pallet is no longer relevant because there is no more queued solution to compare against during the initial verification and subseuquently, solutions are processed in the order of decreasing score, with the first one being chosen in all cases. --------- Signed-off-by: Andrei Trandafir <[email protected]>
1 parent 0ae4eef commit b6af2a9

File tree

13 files changed

+3129
-2892
lines changed

13 files changed

+3129
-2892
lines changed

‎Cargo.lock‎

Lines changed: 3066 additions & 2850 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cumulus/parachains/runtimes/assets/asset-hub-westend/src/staking.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,6 @@ parameter_types! {
6767

6868
/// Size of the exposures. This should be small enough to make the reward payouts feasible.
6969
pub MaxExposurePageSize: u32 = 64;
70-
71-
/// Each solution is considered "better" if it is 0.01% better.
72-
pub SolutionImprovementThreshold: Perbill = Perbill::from_rational(1u32, 10_000);
7370
}
7471

7572
frame_election_provider_support::generate_solution_type!(
@@ -143,7 +140,6 @@ impl multi_block::verifier::Config for Runtime {
143140
type MaxBackersPerWinner = MaxBackersPerWinner;
144141
type MaxBackersPerWinnerFinal = MaxBackersPerWinnerFinal;
145142
type SolutionDataProvider = MultiBlockElectionSigned;
146-
type SolutionImprovementThreshold = SolutionImprovementThreshold;
147143
type WeightInfo = multi_block::weights::westend::MultiBlockVerifierWeightInfo<Self>;
148144
}
149145

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@ parameter_types! {
149149

150150
pub static FallbackMode: FallbackModes = FallbackModes::Emergency;
151151
pub static MinerTxPriority: u64 = 100;
152-
pub static SolutionImprovementThreshold: Perbill = Perbill::zero();
153152
pub static OffchainRepeat: BlockNumber = 5;
154153
pub static OffchainStorage: bool = true;
155154
pub static MinerMaxLength: u32 = 256;
@@ -187,7 +186,6 @@ impl Get<Phase<Runtime>> for AreWeDone {
187186
}
188187

189188
impl crate::verifier::Config for Runtime {
190-
type SolutionImprovementThreshold = SolutionImprovementThreshold;
191189
type MaxBackersPerWinnerFinal = MaxBackersPerWinnerFinal;
192190
type MaxBackersPerWinner = MaxBackersPerWinner;
193191
type MaxWinnersPerPage = MaxWinnersPerPage;
@@ -370,10 +368,6 @@ impl ExtBuilder {
370368
MinerTxPriority::set(p);
371369
self
372370
}
373-
pub(crate) fn solution_improvement_threshold(self, p: Perbill) -> Self {
374-
SolutionImprovementThreshold::set(p);
375-
self
376-
}
377371
pub(crate) fn election_start(self, at: BlockNumber) -> Self {
378372
ElectionStart::set(at);
379373
self

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,6 @@ mod validate_unsigned {
383383
#[test]
384384
fn retracts_weak_score_accepts_threshold_better() {
385385
ExtBuilder::unsigned()
386-
.solution_improvement_threshold(sp_runtime::Perbill::from_percent(10))
387386
.build_and_execute(|| {
388387
roll_to_snapshot_created();
389388

@@ -412,7 +411,7 @@ mod validate_unsigned {
412411
TransactionValidityError::Invalid(InvalidTransaction::Custom(2)),
413412
);
414413

415-
// this is better, but not enough better.
414+
// this is better, but the number of winners is incorrect.
416415
let insufficient_improvement = 55 * 105 / 100;
417416
let attempt = fake_solution(ElectionScore {
418417
minimal_stake: insufficient_improvement,
@@ -421,7 +420,7 @@ mod validate_unsigned {
421420
let call = Call::submit_unsigned { paged_solution: Box::new(attempt) };
422421
assert_eq!(
423422
UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).unwrap_err(),
424-
TransactionValidityError::Invalid(InvalidTransaction::Custom(2)),
423+
TransactionValidityError::Invalid(InvalidTransaction::Custom(4)),
425424
);
426425

427426
// note that we now have to use a solution with 2 winners, just to pass all of the

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ use frame_support::{
3838
use frame_system::pallet_prelude::*;
3939
use pallet::*;
4040
use sp_npos_elections::{evaluate_support, ElectionScore};
41-
use sp_runtime::Perbill;
4241
use sp_std::{collections::btree_map::BTreeMap, prelude::*};
4342

4443
pub(crate) type SupportsOfVerifier<V> = frame_election_provider_support::BoundedSupports<
@@ -115,11 +114,6 @@ pub(crate) mod pallet {
115114
#[pallet::config]
116115
#[pallet::disable_frame_system_supertrait_check]
117116
pub trait Config: crate::Config {
118-
/// The minimum amount of improvement to the solution score that defines a solution as
119-
/// "better".
120-
#[pallet::constant]
121-
type SolutionImprovementThreshold: Get<Perbill>;
122-
123117
/// Maximum number of backers, per winner, among all pages of an election.
124118
///
125119
/// This can only be checked at the very final step of verification.
@@ -193,8 +187,7 @@ pub(crate) mod pallet {
193187
///
194188
/// - `QueuedSolutionScore` must always be correct. In other words, it should correctly be the
195189
/// score of `QueuedValidVariant`.
196-
/// - `QueuedSolutionScore` must always be [`Config::SolutionImprovementThreshold`] better than
197-
/// `MinimumScore`.
190+
/// - `QueuedSolutionScore` must always be better than `MinimumScore`.
198191
/// - The number of existing keys in `QueuedSolutionBackings` must always match that of the
199192
/// INVALID variant.
200193
///
@@ -474,7 +467,7 @@ pub(crate) mod pallet {
474467
Pallet::<T>::minimum_score()
475468
.zip(Self::queued_score())
476469
.map_or(true, |(min_score, score)| score
477-
.strict_threshold_better(min_score, Perbill::zero())),
470+
.strict_better(min_score)),
478471
"queued solution has weak score (min-score)"
479472
);
480473

@@ -705,7 +698,7 @@ impl<T: Config> Pallet<T> {
705698
) -> Result<(), (PageIndex, FeasibilityError)> {
706699
let first_page = solution_pages.first().cloned().unwrap_or_default();
707700
let last_page = solution_pages.last().cloned().unwrap_or_default();
708-
// first, ensure this score will be good enough, even if valid..
701+
// first, ensure this score will be good enough, even if valid.
709702
let _ = Self::ensure_score_quality(claimed_score).map_err(|fe| (first_page, fe))?;
710703
ensure!(
711704
partial_solutions.len() == solution_pages.len(),
@@ -818,12 +811,12 @@ impl<T: Config> Pallet<T> {
818811
/// - greater than the minimum untrusted score.
819812
pub(crate) fn ensure_score_quality(score: ElectionScore) -> Result<(), FeasibilityError> {
820813
let is_improvement = <Self as Verifier>::queued_score().map_or(true, |best_score| {
821-
score.strict_threshold_better(best_score, T::SolutionImprovementThreshold::get())
814+
score.strict_better(best_score)
822815
});
823816
ensure!(is_improvement, FeasibilityError::ScoreTooLow);
824817

825818
let is_greater_than_min_untrusted = Self::minimum_score()
826-
.map_or(true, |min_score| score.strict_threshold_better(min_score, Perbill::zero()));
819+
.map_or(true, |min_score| score.strict_better(min_score));
827820
ensure!(is_greater_than_min_untrusted, FeasibilityError::ScoreTooLow);
828821

829822
Ok(())

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

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,9 +1420,8 @@ mod single_page_sync_verification {
14201420
}
14211421

14221422
#[test]
1423-
fn solution_improvement_threshold_respected() {
1423+
fn solution_improvement_respected() {
14241424
ExtBuilder::verifier()
1425-
.solution_improvement_threshold(Perbill::from_percent(10))
14261425
.build_and_execute(|| {
14271426
roll_to_snapshot_created();
14281427

@@ -1435,25 +1434,65 @@ mod single_page_sync_verification {
14351434
)
14361435
.unwrap();
14371436

1438-
// the slightly better solution need not even be correct. We improve it by 5%, but
1439-
// we need 10%.
1437+
// The slightly better solution is incorrect in the number of winners.
14401438
let mut better_score = single_page.score;
14411439
let improvement = Perbill::from_percent(5) * better_score.minimal_stake;
14421440
better_score.minimal_stake += improvement;
14431441
let slightly_better = fake_solution(better_score);
14441442

1443+
// The score is checked first; we therefore expect verification to
1444+
// succeed in score but subsequently fail in winner count.
14451445
assert_eq!(
14461446
<VerifierPallet as Verifier>::verify_synchronous(
14471447
slightly_better.solution_pages.first().cloned().unwrap(),
14481448
slightly_better.score,
14491449
MultiBlock::msp(),
14501450
)
14511451
.unwrap_err(),
1452-
FeasibilityError::ScoreTooLow
1452+
FeasibilityError::WrongWinnerCount
14531453
);
14541454
});
14551455
}
14561456

1457+
#[test]
1458+
fn exact_same_score_is_rejected() {
1459+
ExtBuilder::verifier().build_and_execute(|| {
1460+
roll_to_snapshot_created();
1461+
1462+
// Queue something useful.
1463+
let single_page = mine_solution(1).unwrap();
1464+
let _ = <VerifierPallet as Verifier>::verify_synchronous(
1465+
single_page.solution_pages.first().cloned().unwrap(),
1466+
single_page.score,
1467+
MultiBlock::msp(),
1468+
)
1469+
.unwrap();
1470+
assert_eq!(<VerifierPallet as Verifier>::queued_score(), Some(single_page.score));
1471+
1472+
// Now try and submit the exact same score again.
1473+
let same = fake_solution(single_page.score);
1474+
1475+
assert_eq!(
1476+
<VerifierPallet as Verifier>::verify_synchronous(
1477+
same.solution_pages.first().cloned().unwrap(),
1478+
same.score,
1479+
MultiBlock::msp(),
1480+
)
1481+
.unwrap_err(),
1482+
FeasibilityError::ScoreTooLow
1483+
);
1484+
1485+
assert_eq!(
1486+
verifier_events(),
1487+
vec![
1488+
Event::<Runtime>::Verified(2, 2),
1489+
Event::<Runtime>::Queued(single_page.score, None),
1490+
Event::<Runtime>::VerificationFailed(2, FeasibilityError::ScoreTooLow),
1491+
]
1492+
);
1493+
});
1494+
}
1495+
14571496
#[test]
14581497
fn weak_score_is_insta_rejected() {
14591498
ExtBuilder::verifier().build_and_execute(|| {

‎substrate/frame/election-provider-multi-phase/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,7 @@ impl<T: Config> Pallet<T> {
17511751
if i == 0 {
17521752
last_score = indice.0
17531753
} else {
1754-
if last_score.strict_threshold_better(indice.0, Perbill::zero()) {
1754+
if last_score.strict_better(indice.0) {
17551755
return Err(
17561756
"Signed submission indices vector must be ordered by election score".into()
17571757
)

‎substrate/frame/election-provider-multi-phase/src/unsigned.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ impl<T: MinerConfig> Miner<T> {
839839
let submitted_score = raw_solution.score;
840840
ensure!(
841841
minimum_untrusted_score.map_or(true, |min_score| {
842-
submitted_score.strict_threshold_better(min_score, sp_runtime::Perbill::zero())
842+
submitted_score.strict_better(min_score)
843843
}),
844844
FeasibilityError::UntrustedScoreTooLow
845845
);

‎substrate/frame/staking-async/ahm-test/src/ah/mock.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ impl multi_block::verifier::Config for Runtime {
283283
type MaxWinnersPerPage = MaxWinnersPerPage;
284284

285285
type SolutionDataProvider = MultiBlockSigned;
286-
type SolutionImprovementThreshold = ();
287286
type WeightInfo = ();
288287
}
289288

‎substrate/frame/staking-async/runtimes/parachain/src/staking.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,6 @@ parameter_types! {
180180
/// lightweight per-page.
181181
// TODO: this is currently 512 in all networks, but 64 might yield better PoV, need to check logs.
182182
pub const MaxExposurePageSize: u32 = 512;
183-
184-
/// Each solution is considered "better" if it is an epsilon better than the previous one.
185-
pub SolutionImprovementThreshold: Perbill = Perbill::from_rational(1u32, 10_000);
186183
}
187184

188185
// Signed phase parameters.
@@ -304,7 +301,6 @@ impl multi_block::verifier::Config for Runtime {
304301
type MaxBackersPerWinner = MaxBackersPerWinner;
305302
type MaxBackersPerWinnerFinal = MaxBackersPerWinnerFinal;
306303
type SolutionDataProvider = MultiBlockElectionSigned;
307-
type SolutionImprovementThreshold = SolutionImprovementThreshold;
308304
type WeightInfo = multi_block::weights::polkadot::MultiBlockVerifierWeightInfo<Self>;
309305
}
310306

0 commit comments

Comments
 (0)