Skip to content

Commit 8005345

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 311c3c1 commit 8005345

File tree

13 files changed

+3172
-2967
lines changed

13 files changed

+3172
-2967
lines changed

‎Cargo.lock‎

Lines changed: 3043 additions & 2849 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 = weights::pallet_election_provider_multi_block_verifier::WeightInfo<Runtime>;
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: 44 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -382,63 +382,56 @@ mod validate_unsigned {
382382

383383
#[test]
384384
fn retracts_weak_score_accepts_threshold_better() {
385-
ExtBuilder::unsigned()
386-
.solution_improvement_threshold(sp_runtime::Perbill::from_percent(10))
387-
.build_and_execute(|| {
388-
roll_to_snapshot_created();
385+
ExtBuilder::unsigned().build_and_execute(|| {
386+
roll_to_snapshot_created();
389387

390-
let solution = mine_full_solution().unwrap();
391-
load_mock_signed_and_start(solution.clone());
392-
roll_to_full_verification();
388+
let solution = mine_full_solution().unwrap();
389+
load_mock_signed_and_start(solution.clone());
390+
roll_to_full_verification();
393391

394-
// Some good solution is queued now.
395-
assert_eq!(
396-
<VerifierPallet as Verifier>::queued_score(),
397-
Some(ElectionScore {
398-
minimal_stake: 55,
399-
sum_stake: 130,
400-
sum_stake_squared: 8650
401-
})
402-
);
392+
// Some good solution is queued now.
393+
assert_eq!(
394+
<VerifierPallet as Verifier>::queued_score(),
395+
Some(ElectionScore { minimal_stake: 55, sum_stake: 130, sum_stake_squared: 8650 })
396+
);
403397

404-
roll_to_unsigned_open();
398+
roll_to_unsigned_open();
405399

406-
// this is just worse
407-
let attempt =
408-
fake_solution(ElectionScore { minimal_stake: 20, ..Default::default() });
409-
let call = Call::submit_unsigned { paged_solution: Box::new(attempt) };
410-
assert_eq!(
411-
UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).unwrap_err(),
412-
TransactionValidityError::Invalid(InvalidTransaction::Custom(2)),
413-
);
400+
// this is just worse
401+
let attempt = fake_solution(ElectionScore { minimal_stake: 20, ..Default::default() });
402+
let call = Call::submit_unsigned { paged_solution: Box::new(attempt) };
403+
assert_eq!(
404+
UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).unwrap_err(),
405+
TransactionValidityError::Invalid(InvalidTransaction::Custom(2)),
406+
);
414407

415-
// this is better, but not enough better.
416-
let insufficient_improvement = 55 * 105 / 100;
417-
let attempt = fake_solution(ElectionScore {
418-
minimal_stake: insufficient_improvement,
419-
..Default::default()
420-
});
421-
let call = Call::submit_unsigned { paged_solution: Box::new(attempt) };
422-
assert_eq!(
423-
UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).unwrap_err(),
424-
TransactionValidityError::Invalid(InvalidTransaction::Custom(2)),
425-
);
408+
// this is better, but the number of winners is incorrect.
409+
let insufficient_improvement = 55 * 105 / 100;
410+
let attempt = fake_solution(ElectionScore {
411+
minimal_stake: insufficient_improvement,
412+
..Default::default()
413+
});
414+
let call = Call::submit_unsigned { paged_solution: Box::new(attempt) };
415+
assert_eq!(
416+
UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).unwrap_err(),
417+
TransactionValidityError::Invalid(InvalidTransaction::Custom(4)),
418+
);
426419

427-
// note that we now have to use a solution with 2 winners, just to pass all of the
428-
// snapshot independent checks.
429-
let mut paged = raw_paged_from_supports(
430-
vec![vec![
431-
(40, Support { total: 10, voters: vec![(3, 5)] }),
432-
(30, Support { total: 10, voters: vec![(3, 5)] }),
433-
]],
434-
0,
435-
);
436-
let sufficient_improvement = 55 * 115 / 100;
437-
paged.score =
438-
ElectionScore { minimal_stake: sufficient_improvement, ..Default::default() };
439-
let call = Call::submit_unsigned { paged_solution: Box::new(paged) };
440-
assert!(UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).is_ok());
441-
})
420+
// note that we now have to use a solution with 2 winners, just to pass all of the
421+
// snapshot independent checks.
422+
let mut paged = raw_paged_from_supports(
423+
vec![vec![
424+
(40, Support { total: 10, voters: vec![(3, 5)] }),
425+
(30, Support { total: 10, voters: vec![(3, 5)] }),
426+
]],
427+
0,
428+
);
429+
let sufficient_improvement = 55 * 115 / 100;
430+
paged.score =
431+
ElectionScore { minimal_stake: sufficient_improvement, ..Default::default() };
432+
let call = Call::submit_unsigned { paged_solution: Box::new(paged) };
433+
assert!(UnsignedPallet::validate_unsigned(TransactionSource::Local, &call).is_ok());
434+
})
442435
}
443436

444437
#[test]

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

Lines changed: 7 additions & 16 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
///
@@ -473,8 +466,7 @@ pub(crate) mod pallet {
473466
ensure!(
474467
Pallet::<T>::minimum_score()
475468
.zip(Self::queued_score())
476-
.map_or(true, |(min_score, score)| score
477-
.strict_threshold_better(min_score, Perbill::zero())),
469+
.map_or(true, |(min_score, score)| score.strict_better(min_score)),
478470
"queued solution has weak score (min-score)"
479471
);
480472

@@ -705,7 +697,7 @@ impl<T: Config> Pallet<T> {
705697
) -> Result<(), (PageIndex, FeasibilityError)> {
706698
let first_page = solution_pages.first().cloned().unwrap_or_default();
707699
let last_page = solution_pages.last().cloned().unwrap_or_default();
708-
// first, ensure this score will be good enough, even if valid..
700+
// first, ensure this score will be good enough, even if valid.
709701
let _ = Self::ensure_score_quality(claimed_score).map_err(|fe| (first_page, fe))?;
710702
ensure!(
711703
partial_solutions.len() == solution_pages.len(),
@@ -817,13 +809,12 @@ impl<T: Config> Pallet<T> {
817809
/// - better than the queued solution, if one exists.
818810
/// - greater than the minimum untrusted score.
819811
pub(crate) fn ensure_score_quality(score: ElectionScore) -> Result<(), FeasibilityError> {
820-
let is_improvement = <Self as Verifier>::queued_score().map_or(true, |best_score| {
821-
score.strict_threshold_better(best_score, T::SolutionImprovementThreshold::get())
822-
});
812+
let is_improvement = <Self as Verifier>::queued_score()
813+
.map_or(true, |best_score| score.strict_better(best_score));
823814
ensure!(is_improvement, FeasibilityError::ScoreTooLow);
824815

825-
let is_greater_than_min_untrusted = Self::minimum_score()
826-
.map_or(true, |min_score| score.strict_threshold_better(min_score, Perbill::zero()));
816+
let is_greater_than_min_untrusted =
817+
Self::minimum_score().map_or(true, |min_score| score.strict_better(min_score));
827818
ensure!(is_greater_than_min_untrusted, FeasibilityError::ScoreTooLow);
828819

829820
Ok(())

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

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,38 +1420,76 @@ mod single_page_sync_verification {
14201420
}
14211421

14221422
#[test]
1423-
fn solution_improvement_threshold_respected() {
1424-
ExtBuilder::verifier()
1425-
.solution_improvement_threshold(Perbill::from_percent(10))
1426-
.build_and_execute(|| {
1427-
roll_to_snapshot_created();
1423+
fn solution_improvement_respected() {
1424+
ExtBuilder::verifier().build_and_execute(|| {
1425+
roll_to_snapshot_created();
14281426

1429-
// submit something good.
1430-
let single_page = mine_solution(1).unwrap();
1431-
let _ = <VerifierPallet as Verifier>::verify_synchronous(
1432-
single_page.solution_pages.first().cloned().unwrap(),
1433-
single_page.score,
1427+
// submit something good.
1428+
let single_page = mine_solution(1).unwrap();
1429+
let _ = <VerifierPallet as Verifier>::verify_synchronous(
1430+
single_page.solution_pages.first().cloned().unwrap(),
1431+
single_page.score,
1432+
MultiBlock::msp(),
1433+
)
1434+
.unwrap();
1435+
1436+
// The slightly better solution is incorrect in the number of winners.
1437+
let mut better_score = single_page.score;
1438+
let improvement = Perbill::from_percent(5) * better_score.minimal_stake;
1439+
better_score.minimal_stake += improvement;
1440+
let slightly_better = fake_solution(better_score);
1441+
1442+
// The score is checked first; we therefore expect verification to
1443+
// succeed in score but subsequently fail in winner count.
1444+
assert_eq!(
1445+
<VerifierPallet as Verifier>::verify_synchronous(
1446+
slightly_better.solution_pages.first().cloned().unwrap(),
1447+
slightly_better.score,
14341448
MultiBlock::msp(),
14351449
)
1436-
.unwrap();
1450+
.unwrap_err(),
1451+
FeasibilityError::WrongWinnerCount
1452+
);
1453+
});
1454+
}
14371455

1438-
// the slightly better solution need not even be correct. We improve it by 5%, but
1439-
// we need 10%.
1440-
let mut better_score = single_page.score;
1441-
let improvement = Perbill::from_percent(5) * better_score.minimal_stake;
1442-
better_score.minimal_stake += improvement;
1443-
let slightly_better = fake_solution(better_score);
1456+
#[test]
1457+
fn exact_same_score_is_rejected() {
1458+
ExtBuilder::verifier().build_and_execute(|| {
1459+
roll_to_snapshot_created();
14441460

1445-
assert_eq!(
1446-
<VerifierPallet as Verifier>::verify_synchronous(
1447-
slightly_better.solution_pages.first().cloned().unwrap(),
1448-
slightly_better.score,
1449-
MultiBlock::msp(),
1450-
)
1451-
.unwrap_err(),
1452-
FeasibilityError::ScoreTooLow
1453-
);
1454-
});
1461+
// Queue something useful.
1462+
let single_page = mine_solution(1).unwrap();
1463+
let _ = <VerifierPallet as Verifier>::verify_synchronous(
1464+
single_page.solution_pages.first().cloned().unwrap(),
1465+
single_page.score,
1466+
MultiBlock::msp(),
1467+
)
1468+
.unwrap();
1469+
assert_eq!(<VerifierPallet as Verifier>::queued_score(), Some(single_page.score));
1470+
1471+
// Now try and submit the exact same score again.
1472+
let same = fake_solution(single_page.score);
1473+
1474+
assert_eq!(
1475+
<VerifierPallet as Verifier>::verify_synchronous(
1476+
same.solution_pages.first().cloned().unwrap(),
1477+
same.score,
1478+
MultiBlock::msp(),
1479+
)
1480+
.unwrap_err(),
1481+
FeasibilityError::ScoreTooLow
1482+
);
1483+
1484+
assert_eq!(
1485+
verifier_events(),
1486+
vec![
1487+
Event::<Runtime>::Verified(2, 2),
1488+
Event::<Runtime>::Queued(single_page.score, None),
1489+
Event::<Runtime>::VerificationFailed(2, FeasibilityError::ScoreTooLow),
1490+
]
1491+
);
1492+
});
14551493
}
14561494

14571495
#[test]

‎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: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -838,9 +838,8 @@ impl<T: MinerConfig> Miner<T> {
838838
// Ensure that the solution's score can pass absolute min-score.
839839
let submitted_score = raw_solution.score;
840840
ensure!(
841-
minimum_untrusted_score.map_or(true, |min_score| {
842-
submitted_score.strict_threshold_better(min_score, sp_runtime::Perbill::zero())
843-
}),
841+
minimum_untrusted_score
842+
.map_or(true, |min_score| { submitted_score.strict_better(min_score) }),
844843
FeasibilityError::UntrustedScoreTooLow
845844
);
846845

‎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)