Trim compact solution for length during preparation

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

@@ -519,12 +519,19 @@ pub mod pallet {
 		/// Maximum number of iteration of balancing that will be executed in the embedded miner of
 		/// the pallet.
 		type MinerMaxIterations: Get<u32>;
+
 		/// Maximum weight that the miner should consume.
 		///
 		/// The miner will ensure that the total weight of the unsigned solution will not exceed
-		/// this values, based on [`WeightInfo::submit_unsigned`].
+		/// this value, based on [`WeightInfo::submit_unsigned`].
 		type MinerMaxWeight: Get<Weight>;
 
+		/// 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<u32>;
+
 		/// Something that will provide the election data.
 		type DataProvider: ElectionDataProvider<Self::AccountId, Self::BlockNumber>;
 

frame/election-provider-multi-phase/src/mock.rs

@@ -202,6 +202,7 @@ 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;
 
 
@@ -267,6 +268,7 @@ 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;

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

@@ -46,6 +46,8 @@ 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<sp_npos_elections::Error> for MinerError {
@@ -166,6 +168,11 @@ impl<T: Config> Pallet<T> {
 			maximum_allowed_voters,
 		);
 		let compact = Self::trim_compact(maximum_allowed_voters, compact, &voter_index)?;
+		let compact = Self::trim_compact_for_length(
+			T::MinerMaxLength::get(),
+			compact,
+			&voter_index,
+		)?;
 
 		// re-calc score.
 		let winners = sp_npos_elections::to_without_backing(winners);
@@ -252,6 +259,54 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
+	/// 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, and the `ElectionSize` is a proxy for the total number of stakers.
+	///
+	/// 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.
+	///
+	/// 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_for_length(
+		max_allowed_length: u32,
+		mut compact: CompactOf<T>,
+		voter_index: impl Fn(&T::AccountId) -> Option<CompactVoterIndexOf<T>>,
+	) -> Result<CompactOf<T>, MinerError> {
+		// 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::<Vec<_>>();
+		voters_sorted.sort_by_key(|(_, y)| *y);
+		voters_sorted.reverse();
+
+		// we run through the reduction process twice: once using the size_hint function, and once
+		// by actually encoding the data. Given a well-behaved size_hint implementation, this will
+		// only actually encode once. Given a misbehaving size_hint implementation, this still ensures
+		// that we accurately trim the solution.
+		let size_fns: [Box<dyn Fn(&CompactOf<T>) -> u32>; 2] = [
+			Box::new(|compact: &CompactOf<T>| compact.size_hint().saturated_into()),
+			Box::new(|compact: &CompactOf<T>| {
+				let encoded = compact.encode();
+				encoded.len().saturated_into()
+			}),
+		];
+		for size_fn in size_fns.iter() {
+			while size_fn(&compact) > max_allowed_length {
+				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`.
@@ -486,6 +541,7 @@ mod tests {
 		Call, *,
 	};
 	use frame_support::{dispatch::Dispatchable, traits::OffchainWorker};
+	use helpers::voter_index_fn_linear;
 	use mock::Call as OuterCall;
 	use sp_election_providers::Assignment;
 	use sp_runtime::{traits::ValidateUnsigned, PerU16};
@@ -870,4 +926,51 @@ mod tests {
 			assert!(matches!(call, OuterCall::MultiPhase(Call::submit_unsigned(_, _))));
 		})
 	}
+
+	#[test]
+	fn trim_compact_for_length_does_not_modify_when_short_enough() {
+		let (mut ext, _) = ExtBuilder::default().build_offchainify(0);
+		ext.execute_with(|| {
+			roll_to(25);
+
+			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();
+
+			compact = MultiPhase::trim_compact_for_length(
+				encoded_len,
+				compact,
+				voter_index_fn_linear::<Runtime>(&voters),
+			).unwrap();
+
+			assert_eq!(compact, compact_clone);
+		});
+	}
+
+	#[test]
+	fn trim_compact_for_length_modifies_when_too_long() {
+		let (mut ext, _) = ExtBuilder::default().build_offchainify(0);
+		ext.execute_with(|| {
+			roll_to(25);
+
+			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();
+
+			compact = MultiPhase::trim_compact_for_length(
+				encoded_len - 1,
+				compact,
+				voter_index_fn_linear::<Runtime>(&voters),
+			).unwrap();
+
+			assert_ne!(compact, compact_clone);
+			assert!((compact.encode().len() as u32) < encoded_len);
+		});
+	}
 }