Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NPoS] Remove better solution threshold for unsigned submissions #2694

Merged
merged 9 commits into from
Dec 15, 2023
2 changes: 0 additions & 2 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ parameter_types! {
pub const SignedDepositByte: Balance = deposit(0, 10) / 1024;
// Each good submission will get 1 WND as reward
pub SignedRewardBase: Balance = 1 * UNITS;
pub BetterUnsignedThreshold: Perbill = Perbill::from_rational(5u32, 10_000);

// 1 hour session, 15 minutes unsigned phase, 4 offchain executions.
pub OffchainRepeat: BlockNumber = UnsignedPhase::get() / 4;
Expand Down Expand Up @@ -608,7 +607,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type MinerConfig = Self;
type SlashHandler = (); // burn slashes
type RewardHandler = (); // nothing to do upon rewards
type BetterUnsignedThreshold = BetterUnsignedThreshold;
type BetterSignedThreshold = ();
type OffchainRepeat = OffchainRepeat;
type MinerTxPriority = NposSolutionPriority;
Expand Down
14 changes: 14 additions & 0 deletions prdoc/pr_2694.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "pallet-election-provider-multi-phase: Removes BetterUnsignedThreshold from pallet config"
Ank4n marked this conversation as resolved.
Show resolved Hide resolved

doc:
- audience: Runtime Dev
description: |
Removes thresholding for accepting solutions better than the last queued for unsigned phase. This is unnecessary
as even without thresholding, the number of solutions that can be submitted to on-chain which is better than the
previous one is limited.

crates:
- name: "pallet-election-provider-multi-phase"
3 changes: 0 additions & 3 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,8 +704,6 @@ parameter_types! {
pub const SignedDepositIncreaseFactor: Percent = Percent::from_percent(10);
pub const SignedDepositByte: Balance = 1 * CENTS;

pub BetterUnsignedThreshold: Perbill = Perbill::from_rational(1u32, 10_000);

// miner configs
pub const MultiPhaseUnsignedPriority: TransactionPriority = StakingUnsignedPriority::get() - 1u64;
pub MinerMaxWeight: Weight = RuntimeBlockWeights::get()
Expand Down Expand Up @@ -822,7 +820,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type EstimateCallFee = TransactionPayment;
type SignedPhase = SignedPhase;
type UnsignedPhase = UnsignedPhase;
type BetterUnsignedThreshold = BetterUnsignedThreshold;
type BetterSignedThreshold = ();
type OffchainRepeat = OffchainRepeat;
type MinerTxPriority = MultiPhaseUnsignedPriority;
Expand Down
10 changes: 2 additions & 8 deletions substrate/frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@
//! unsigned transaction, thus the name _unsigned_ phase. This unsigned transaction can never be
//! valid if propagated, and it acts similar to an inherent.
//!
//! Validators will only submit solutions if the one that they have computed is sufficiently better
//! than the best queued one (see [`pallet::Config::BetterUnsignedThreshold`]) and will limit the
//! weight of the solution to [`MinerConfig::MaxWeight`].
//! Validators will only submit solutions if the one that they have computed is strictly better than
//! the best queued one and will limit the weight of the solution to [`MinerConfig::MaxWeight`].
//!
//! The unsigned phase can be made passive depending on how the previous signed phase went, by
//! setting the first inner value of [`Phase`] to `false`. For now, the signed phase is always
Expand Down Expand Up @@ -598,11 +597,6 @@ pub mod pallet {
#[pallet::constant]
type BetterSignedThreshold: Get<Perbill>;

/// The minimum amount of improvement to the solution score that defines a solution as
/// "better" in the Unsigned phase.
#[pallet::constant]
type BetterUnsignedThreshold: Get<Perbill>;

/// The repeat threshold of the offchain worker.
///
/// For example, if it is 5, that means that at least 5 blocks will elapse between attempts
Expand Down
7 changes: 1 addition & 6 deletions substrate/frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ parameter_types! {
pub static SignedMaxWeight: Weight = BlockWeights::get().max_block;
pub static MinerTxPriority: u64 = 100;
pub static BetterSignedThreshold: Perbill = Perbill::zero();
pub static BetterUnsignedThreshold: Perbill = Perbill::zero();
pub static OffchainRepeat: BlockNumber = 5;
pub static MinerMaxWeight: Weight = BlockWeights::get().max_block;
pub static MinerMaxLength: u32 = 256;
Expand Down Expand Up @@ -399,7 +398,6 @@ impl crate::Config for Runtime {
type EstimateCallFee = frame_support::traits::ConstU32<8>;
type SignedPhase = SignedPhase;
type UnsignedPhase = UnsignedPhase;
type BetterUnsignedThreshold = BetterUnsignedThreshold;
type BetterSignedThreshold = BetterSignedThreshold;
type OffchainRepeat = OffchainRepeat;
type MinerTxPriority = MinerTxPriority;
Expand Down Expand Up @@ -538,10 +536,7 @@ impl ExtBuilder {
<BetterSignedThreshold>::set(p);
self
}
pub fn better_unsigned_threshold(self, p: Perbill) -> Self {
<BetterUnsignedThreshold>::set(p);
self
}

pub fn phases(self, signed: BlockNumber, unsigned: BlockNumber) -> Self {
<SignedPhase>::set(signed);
<UnsignedPhase>::set(unsigned);
Expand Down
46 changes: 23 additions & 23 deletions substrate/frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ impl<T: Config> Pallet<T> {
ensure!(
Self::queued_solution().map_or(true, |q: ReadySolution<_, _>| raw_solution
.score
.strict_threshold_better(q.score, T::BetterUnsignedThreshold::get())),
.strict_threshold_better(q.score, sp_runtime::Perbill::zero())),
Ank4n marked this conversation as resolved.
Show resolved Hide resolved
Error::<T>::PreDispatchWeakSubmission,
);

Expand Down Expand Up @@ -1025,7 +1025,7 @@ mod tests {
bounded_vec,
offchain::storage_lock::{BlockAndTime, StorageLock},
traits::{Dispatchable, ValidateUnsigned, Zero},
ModuleError, PerU16, Perbill,
ModuleError, PerU16,
};

type Assignment = crate::unsigned::Assignment<Runtime>;
Expand Down Expand Up @@ -1360,20 +1360,22 @@ mod tests {
.desired_targets(1)
.add_voter(7, 2, bounded_vec![10])
.add_voter(8, 5, bounded_vec![10])
.better_unsigned_threshold(Perbill::from_percent(50))
.build_and_execute(|| {
roll_to_unsigned();
assert!(MultiPhase::current_phase().is_unsigned());
assert_eq!(MultiPhase::desired_targets().unwrap(), 1);

// an initial solution
let result = ElectionResult {
// note: This second element of backing stake is not important here.
winners: vec![(10, 10)],
assignments: vec![Assignment {
who: 10,
distribution: vec![(10, PerU16::one())],
}],
winners: vec![(10, 12)],
assignments: vec![
Assignment { who: 10, distribution: vec![(10, PerU16::one())] },
Assignment {
who: 7,
// note: this percent doesn't even matter, in solution it is 100%.
distribution: vec![(10, PerU16::one())],
},
],
};

let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap();
Expand All @@ -1394,19 +1396,16 @@ mod tests {
Box::new(solution),
witness
));
assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 10);
assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 12);

// trial 1: a solution who's score is only 2, i.e. 20% better in the first element.
// trial 1: a solution who's minimal stake is 10, i.e. 16% worse than the first
// solution.
let result = ElectionResult {
winners: vec![(10, 12)],
assignments: vec![
Assignment { who: 10, distribution: vec![(10, PerU16::one())] },
Assignment {
who: 7,
// note: this percent doesn't even matter, in solution it is 100%.
distribution: vec![(10, PerU16::one())],
},
],
winners: vec![(10, 10)],
assignments: vec![Assignment {
who: 10,
distribution: vec![(10, PerU16::one())],
}],
};
let (raw, score, _, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
Expand All @@ -1416,15 +1415,16 @@ mod tests {
)
.unwrap();
let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
// 12 is not 50% more than 10
assert_eq!(solution.score.minimal_stake, 12);
// 10 is not better than 12
assert_eq!(solution.score.minimal_stake, 10);
assert_noop!(
MultiPhase::unsigned_pre_dispatch_checks(&solution),
Error::<Runtime>::PreDispatchWeakSubmission,
);
// submitting this will actually panic.

// trial 2: a solution who's score is only 7, i.e. 70% better in the first element.
// trial 2: a solution who's minimal stake is 17, i.e. 5 better than the first
// solution.
let result = ElectionResult {
winners: vec![(10, 12)],
assignments: vec![
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type SignedPhase = SignedPhase;
type UnsignedPhase = UnsignedPhase;
type BetterSignedThreshold = ();
type BetterUnsignedThreshold = ();
type OffchainRepeat = OffchainRepeat;
type MinerTxPriority = TransactionPriority;
type MinerConfig = Self;
Expand Down