Repository: paritytech/substrate
File: frame/election-provider-multi-phase/src/unsigned.rs (Rust)

Code before:
	/// 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`.
	pub fn maximum_voter_for_weight<W: WeightInfo>(
		desired_winners: u32,

Review comment (suggestion):
Again, given that this is OCW, I won't annoy you now, but we can do this much better. We can _statically_ know the encoded size of each voter, based on how many votes it is casting (e.g. is it a votes1 or a votes5 field in `compact`), so there's not much need for any of this re-encoding at the end of the day. 

Code after:
				}

				log!(debug, "removed {} voter to meet the max weight limit.", to_remove);
				Ok(compact)
			}
			_ => {
				// nada, return as-is
				log!(debug, "didn't remove any voter for weight limits.");
				Ok(compact)
			}
		}
	}

	/// 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<T>,
		voter_index: impl Fn(&T::AccountId) -> Option<CompactVoterIndexOf<T>>,
	) -> Result<CompactOf<T>, 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::<u32>()) <= 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::<Vec<_>>();
		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)?;