diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index b2a59d587db90..aaf470ed33764 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -517,11 +517,6 @@ parameter_types! { .get(DispatchClass::Normal) .max_extrinsic.expect("Normal extrinsics have a weight limit configured; qed") .saturating_sub(BlockExecutionWeight::get()); - // Solution can occupy 90% of normal block size - pub MinerMaxLength: u32 = Perbill::from_rational(9u32, 10) * - *RuntimeBlockLength::get() - .max - .get(DispatchClass::Normal); } sp_npos_elections::generate_solution_type!( @@ -544,7 +539,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type SolutionImprovementThreshold = SolutionImprovementThreshold; type MinerMaxIterations = MinerMaxIterations; type MinerMaxWeight = MinerMaxWeight; - type MinerMaxLength = MinerMaxLength; type MinerTxPriority = MultiPhaseUnsignedPriority; type DataProvider = Staking; type OnChainAccuracy = Perbill; diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index c59d68a33adba..17978566a858e 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -534,19 +534,12 @@ pub mod pallet { /// Maximum number of iteration of balancing that will be executed in the embedded miner of /// the pallet. type MinerMaxIterations: Get; - /// Maximum weight that the miner should consume. /// /// The miner will ensure that the total weight of the unsigned solution will not exceed - /// this value, based on [`WeightInfo::submit_unsigned`]. + /// this values, based on [`WeightInfo::submit_unsigned`]. type MinerMaxWeight: Get; - /// Maximum length (bytes) that the mined solution should consume. - /// - /// The miner will ensure that the total length of the unsigned solution will not exceed - /// this value. - type MinerMaxLength: Get; - /// Something that will provide the election data. type DataProvider: ElectionDataProvider; diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 79e6e952bfec8..5a0a83354b266 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -205,7 +205,6 @@ parameter_types! { pub static MinerTxPriority: u64 = 100; pub static SolutionImprovementThreshold: Perbill = Perbill::zero(); pub static MinerMaxWeight: Weight = BlockWeights::get().max_block; - pub static MinerMaxLength: u32 = 256; pub static MockWeightInfo: bool = false; @@ -278,7 +277,6 @@ impl crate::Config for Runtime { type SolutionImprovementThreshold = SolutionImprovementThreshold; type MinerMaxIterations = MinerMaxIterations; type MinerMaxWeight = MinerMaxWeight; - type MinerMaxLength = MinerMaxLength; type MinerTxPriority = MinerTxPriority; type DataProvider = StakingMock; type WeightInfo = DualMockWeightInfo; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 26e51cf58b34b..6b0d237c6e0a2 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -46,8 +46,6 @@ pub enum MinerError { PreDispatchChecksFailed, /// The solution generated from the miner is not feasible. Feasibility(FeasibilityError), - /// There are no more voters to remove to trim the solution. - NoMoreVoters, } impl From for MinerError { @@ -179,13 +177,8 @@ impl Pallet { maximum_allowed_voters, ); - // trim length and weight - let compact = Self::trim_compact_weight(maximum_allowed_voters, compact, &voter_index)?; - let compact = Self::trim_compact_length( - T::MinerMaxLength::get(), - compact, - &voter_index, - )?; + // trim weight. + let compact = Self::trim_compact(maximum_allowed_voters, compact, &voter_index)?; // re-calc score. let winners = sp_npos_elections::to_without_backing(winners); @@ -228,7 +221,7 @@ impl Pallet { /// /// 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. - pub fn trim_compact_weight( + pub fn trim_compact( maximum_allowed_voters: u32, mut compact: CompactOf, voter_index: FN, @@ -274,50 +267,6 @@ impl Pallet { } } - /// Greedily reduce the size of the solution to fit into the block w.r.t length. - /// - /// The length of the solution is largely a function of the number of voters. The number of - /// winners cannot be changed. Thus, to reduce the solution size, we need to strip voters. - /// - /// Note that this solution is already computed, and winners are elected based on the merit of - /// the total stake in the system. Nevertheless, some of the voters may be removed here. - /// - /// Sometimes, removing a voter can cause a validator to also be implicitly removed, if - /// that voter was the only backer of that winner. In such cases, this solution is invalid, which - /// will be caught prior to submission. - /// - /// The score must be computed **after** this step. If this step reduces the score too much, - /// then the solution must be discarded. - pub fn trim_compact_length( - max_allowed_length: u32, - mut compact: CompactOf, - voter_index: impl Fn(&T::AccountId) -> Option>, - ) -> Result, MinerError> { - // short-circuit to avoid getting the voters if possible - // this involves a redundant encoding, but that should hopefully be relatively cheap - if (compact.encoded_size().saturated_into::()) <= max_allowed_length { - return Ok(compact); - } - - // grab all voters and sort them by least stake. - let RoundSnapshot { voters, .. } = - Self::snapshot().ok_or(MinerError::SnapshotUnAvailable)?; - let mut voters_sorted = voters - .into_iter() - .map(|(who, stake, _)| (who.clone(), stake)) - .collect::>(); - voters_sorted.sort_by_key(|(_, y)| *y); - voters_sorted.reverse(); - - while compact.encoded_size() > max_allowed_length.saturated_into() { - let (smallest_stake_voter, _) = voters_sorted.pop().ok_or(MinerError::NoMoreVoters)?; - let index = voter_index(&smallest_stake_voter).ok_or(MinerError::SnapshotUnAvailable)?; - compact.remove_voter(index); - } - - Ok(compact) - } - /// Find the maximum `len` that a compact can have in order to fit into the block weight. /// /// This only returns a value between zero and `size.nominators`. @@ -557,7 +506,6 @@ mod tests { Call, *, }; use frame_support::{dispatch::Dispatchable, traits::OffchainWorker}; - use helpers::voter_index_fn_linear; use mock::Call as OuterCall; use frame_election_provider_support::Assignment; use sp_runtime::{traits::ValidateUnsigned, PerU16}; @@ -941,116 +889,4 @@ mod tests { assert!(matches!(call, OuterCall::MultiPhase(Call::submit_unsigned(_, _)))); }) } - - #[test] - fn trim_compact_length_does_not_modify_when_short_enough() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { - roll_to(25); - - // given - let RoundSnapshot { voters, ..} = MultiPhase::snapshot().unwrap(); - let RawSolution { mut compact, .. } = raw_solution(); - let encoded_len = compact.encode().len() as u32; - let compact_clone = compact.clone(); - - // when - assert!(encoded_len < ::MinerMaxLength::get()); - - // then - compact = MultiPhase::trim_compact_length( - encoded_len, - compact, - voter_index_fn_linear::(&voters), - ).unwrap(); - assert_eq!(compact, compact_clone); - }); - } - - #[test] - fn trim_compact_length_modifies_when_too_long() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { - roll_to(25); - - let RoundSnapshot { voters, ..} = - MultiPhase::snapshot().unwrap(); - - let RawSolution { mut compact, .. } = raw_solution(); - let encoded_len = compact.encoded_size() as u32; - let compact_clone = compact.clone(); - - compact = MultiPhase::trim_compact_length( - encoded_len - 1, - compact, - voter_index_fn_linear::(&voters), - ).unwrap(); - - assert_ne!(compact, compact_clone); - assert!((compact.encoded_size() as u32) < encoded_len); - }); - } - - #[test] - fn trim_compact_length_trims_lowest_stake() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { - roll_to(25); - - let RoundSnapshot { voters, ..} = - MultiPhase::snapshot().unwrap(); - - let RawSolution { mut compact, .. } = raw_solution(); - let encoded_len = compact.encoded_size() as u32; - let voter_count = compact.voter_count(); - let min_stake_voter = voters.iter() - .map(|(id, weight, _)| (weight, id)) - .min() - .map(|(_, id)| id) - .unwrap(); - - - compact = MultiPhase::trim_compact_length( - encoded_len - 1, - compact, - voter_index_fn_linear::(&voters), - ).unwrap(); - - assert_eq!(compact.voter_count(), voter_count - 1, "we must have removed exactly 1 voter"); - - let assignments = compact.into_assignment( - |voter| Some(voter as AccountId), - |target| Some(target as AccountId), - ).unwrap(); - assert!( - assignments.iter() - .all(|Assignment{ who, ..}| who != min_stake_voter), - "min_stake_voter must no longer be in the set of voters", - ); - }); - } - - // all the other solution-generation functions end up delegating to `mine_solution`, so if we - // demonstrate that `mine_solution` solutions are all trimmed to an acceptable length, then - // we know that higher-level functions will all also have short-enough solutions. - #[test] - fn mine_solution_solutions_always_within_acceptable_length() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { - roll_to(25); - - // how long would the default solution be? - let solution = MultiPhase::mine_solution(0).unwrap(); - let max_length = ::MinerMaxLength::get(); - let solution_size = solution.0.compact.encoded_size(); - assert!(solution_size <= max_length as usize); - - // now set the max size to less than the actual size and regenerate - ::MinerMaxLength::set(solution_size as u32 - 1); - let solution = MultiPhase::mine_solution(0).unwrap(); - let max_length = ::MinerMaxLength::get(); - let solution_size = solution.0.compact.encoded_size(); - assert!(solution_size <= max_length as usize); - }); - } }