Allow configuring winner count in dry-run (#539)

* first pass: allow configuring winner count in dry-run

* cargo fmt

* fix merge

* move feasibility checking until after we've obtained and logged a solution

* replace metadata with modern, stripped down version with just EPM pallet in

* no feasibliity check if force_winner_count set

* revert; put original metadata from main back; stripping updates to v15 which is no good here

src/commands/dry_run.rs

@@ -37,15 +37,21 @@ pub struct DryRunConfig {
 	#[clap(long)]
 	pub force_snapshot: bool,
 
+	/// The number of winners to take, instead of the `desired_targets` in snapshot.
+	// Doing this would cause the dry-run to typically fail, but that's fine, the program should
+	// still print out some score, and that should be it.
+	#[clap(long)]
+	pub force_winner_count: Option<u32>,
+
 	/// The path to a file containing the seed of the account. If the file is not found, the seed is
-	/// used as-is.
+	/// used as-is. If this is not provided, we won't attempt to submit anything.
 	///
 	/// Can also be provided via the `SEED` environment variable.
 	///
 	/// WARNING: Don't use an account with a large stash for this. Based on how the bot is
 	/// configured, it might re-try and lose funds through transaction fees/deposits.
 	#[clap(long, short, env = "SEED")]
-	pub seed_or_path: String,
+	pub seed_or_path: Option<String>,
 }
 
 pub async fn dry_run_cmd<T>(api: SubxtClient, config: DryRunConfig) -> Result<(), Error>
@@ -56,27 +62,21 @@ where
 		+ 'static,
 	T::Solution: Send,
 {
-	let signer = Signer::new(&config.seed_or_path)?;
-
-	let account_info = api
-		.storage()
-		.at(None)
-		.await?
-		.fetch(&runtime::storage().system().account(signer.account_id()))
-		.await?
-		.ok_or(Error::AccountDoesNotExists)?;
-
-	log::info!(target: LOG_TARGET, "Loaded account {}, {:?}", signer, account_info);
-
 	let round = api
 		.storage()
 		.at(config.at)
 		.await?
 		.fetch_or_default(&runtime::storage().election_provider_multi_phase().round())
 		.await?;
 
-	let (solution, score, _size) =
-		epm::fetch_snapshot_and_mine_solution::<T>(&api, config.at, config.solver, round).await?;
+	let miner_solution = epm::fetch_snapshot_and_mine_solution::<T>(
+		&api,
+		config.at,
+		config.solver,
+		round,
+		config.force_winner_count,
+	)
+	.await?;
 
 	let round = api
 		.storage()
@@ -86,8 +86,9 @@ where
 		.await?
 		.unwrap_or(1);
 
+	let solution = miner_solution.solution();
+	let score = miner_solution.score();
 	let raw_solution = RawSolution { solution, score, round };
-	let nonce = api.rpc().system_account_next_index(signer.account_id()).await?;
 
 	log::info!(
 		target: LOG_TARGET,
@@ -96,17 +97,37 @@ where
 		raw_solution.encode().len(),
 	);
 
-	let tx = epm::signed_solution(raw_solution)?;
-	let xt = api
-		.tx()
-		.create_signed_with_nonce(&tx, &*signer, nonce, ExtrinsicParams::default())?;
-
-	let outcome = api.rpc().dry_run(xt.encoded(), config.at).await?;
-
-	log::info!(target: LOG_TARGET, "dry-run outcome is {:?}", outcome);
+	// Now we've logged the score, check whether the solution makes sense. No point doing this
+	// if force_winner_count is selected since it'll definitely fail in that case.
+	if config.force_winner_count.is_none() {
+		miner_solution.feasibility_check()?;
+	}
 
-	match outcome {
-		Ok(()) => Ok(()),
-		Err(e) => Err(Error::Other(format!("{e:?}"))),
+	// If an account seed or path is provided, then do a dry run to the node. Otherwise,
+	// we've logged the solution above and we do nothing else.
+	if let Some(seed_or_path) = &config.seed_or_path {
+		let signer = Signer::new(seed_or_path)?;
+		let account_info = api
+			.storage()
+			.at(None)
+			.await?
+			.fetch(&runtime::storage().system().account(signer.account_id()))
+			.await?
+			.ok_or(Error::AccountDoesNotExists)?;
+
+		log::info!(target: LOG_TARGET, "Loaded account {}, {:?}", signer, account_info);
+
+		let nonce = api.rpc().system_account_next_index(signer.account_id()).await?;
+		let tx = epm::signed_solution(raw_solution)?;
+		let xt =
+			api.tx()
+				.create_signed_with_nonce(&tx, &*signer, nonce, ExtrinsicParams::default())?;
+		let outcome = api.rpc().dry_run(xt.encoded(), config.at).await?;
+
+		log::info!(target: LOG_TARGET, "dry-run outcome is {:?}", outcome);
+
+		outcome.map_err(|e| Error::Other(format!("{e:?}")))?;
 	}
+
+	Ok(())
 }

src/commands/monitor.rs

@@ -303,23 +303,36 @@ where
 	tokio::time::sleep(std::time::Duration::from_secs(config.delay as u64)).await;
 	let _lock = submit_lock.lock().await;
 
-	let (solution, score) =
-		match epm::fetch_snapshot_and_mine_solution::<T>(&api, Some(hash), config.solver, round)
-			.timed()
-			.await
-		{
-			(Ok((solution, score, size)), elapsed) => {
-				let elapsed_ms = elapsed.as_millis();
-				let encoded_len = solution.encoded_size();
-				let active_voters = solution.voter_count() as u32;
-				let desired_targets = solution.unique_targets().len() as u32;
-
-				let final_weight = tokio::task::spawn_blocking(move || {
-					T::solution_weight(size.voters, size.targets, active_voters, desired_targets)
-				})
-				.await?;
-
-				log::info!(
+	let (solution, score) = match epm::fetch_snapshot_and_mine_solution::<T>(
+		&api,
+		Some(hash),
+		config.solver,
+		round,
+		None,
+	)
+	.timed()
+	.await
+	{
+		(Ok(miner_solution), elapsed) => {
+			// check that the solution looks valid:
+			miner_solution.feasibility_check()?;
+
+			// and then get the values we need from it:
+			let solution = miner_solution.solution();
+			let score = miner_solution.score();
+			let size = miner_solution.size();
+
+			let elapsed_ms = elapsed.as_millis();
+			let encoded_len = solution.encoded_size();
+			let active_voters = solution.voter_count() as u32;
+			let desired_targets = solution.unique_targets().len() as u32;
+
+			let final_weight = tokio::task::spawn_blocking(move || {
+				T::solution_weight(size.voters, size.targets, active_voters, desired_targets)
+			})
+			.await?;
+
+			log::info!(
 					target: LOG_TARGET,
 					"Mined solution with {:?} size: {:?} round: {:?} at: {}, took: {} ms, len: {:?}, weight = {:?}",
 					score,
@@ -331,15 +344,15 @@ where
 					final_weight,
 				);
 
-				prometheus::set_length(encoded_len);
-				prometheus::set_weight(final_weight);
-				prometheus::observe_mined_solution_duration(elapsed_ms as f64);
-				prometheus::set_score(score);
+			prometheus::set_length(encoded_len);
+			prometheus::set_weight(final_weight);
+			prometheus::observe_mined_solution_duration(elapsed_ms as f64);
+			prometheus::set_score(score);
 
-				(solution, score)
-			},
-			(Err(e), _) => return Err(e),
-		};
+			(solution, score)
+		},
+		(Err(e), _) => return Err(e),
+	};
 
 	let best_head = get_latest_head(&api, config.listen).await?;
 

src/epm.rs

@@ -26,7 +26,7 @@ use crate::{
 use codec::{Decode, Encode};
 use frame_election_provider_support::{NposSolution, PhragMMS, SequentialPhragmen};
 use frame_support::weights::Weight;
-use pallet_election_provider_multi_phase::{RawSolution, SolutionOf, SolutionOrSnapshotSize};
+use pallet_election_provider_multi_phase::{RawSolution, SolutionOrSnapshotSize};
 use scale_info::{PortableRegistry, TypeInfo};
 use scale_value::scale::{decode_as_type, TypeId};
 use sp_core::Bytes;
@@ -175,7 +175,8 @@ pub async fn fetch_snapshot_and_mine_solution<T>(
 	hash: Option<Hash>,
 	solver: Solver,
 	round: u32,
-) -> Result<(SolutionOf<T>, ElectionScore, SolutionOrSnapshotSize), Error>
+	forced_desired_targets: Option<u32>,
+) -> Result<MinedSolution<T>, Error>
 where
 	T: MinerConfig<AccountId = AccountId, MaxVotesPerVoter = static_types::MaxVotesPerVoter>
 		+ Send
@@ -184,13 +185,17 @@ where
 	T::Solution: Send,
 {
 	let snapshot = snapshot_at(hash, &api).await?;
-	let desired_targets = api
-		.storage()
-		.at(hash)
-		.await?
-		.fetch(&runtime::storage().election_provider_multi_phase().desired_targets())
-		.await?
-		.expect("Snapshot is non-empty; `desired_target` should exist; qed");
+
+	let desired_targets = match forced_desired_targets {
+		Some(x) => x,
+		None => api
+			.storage()
+			.at(hash)
+			.await?
+			.fetch(&runtime::storage().election_provider_multi_phase().desired_targets())
+			.await?
+			.expect("Snapshot is non-empty; `desired_target` should exist; qed"),
+	};
 
 	let minimum_untrusted_score = api
 		.storage()
@@ -221,27 +226,70 @@ where
 	.await;
 
 	match blocking_task {
-		Ok(Ok((solution, score, solution_or_snapshot))) => {
-			match Miner::<T>::feasibility_check(
-				RawSolution { solution: solution.clone(), score, round },
-				pallet_election_provider_multi_phase::ElectionCompute::Signed,
-				desired_targets,
-				snapshot,
-				round,
-				minimum_untrusted_score,
-			) {
-				Ok(_) => Ok((solution, score, solution_or_snapshot)),
-				Err(e) => {
-					log::error!(target: LOG_TARGET, "Solution feasibility error {:?}", e);
-					Err(Error::Feasibility(format!("{:?}", e)))
-				},
-			}
-		},
+		Ok(Ok((solution, score, solution_or_snapshot_size))) => Ok(MinedSolution {
+			round,
+			desired_targets,
+			snapshot,
+			minimum_untrusted_score,
+			solution,
+			score,
+			solution_or_snapshot_size,
+		}),
 		Ok(Err(err)) => Err(Error::Other(format!("{:?}", err))),
 		Err(err) => Err(err.into()),
 	}
 }
 
+/// The result of calling [`fetch_snapshot_and_mine_solution`].
+pub struct MinedSolution<T: MinerConfig> {
+	round: u32,
+	desired_targets: u32,
+	snapshot: RoundSnapshot,
+	minimum_untrusted_score: Option<ElectionScore>,
+	solution: T::Solution,
+	score: ElectionScore,
+	solution_or_snapshot_size: SolutionOrSnapshotSize,
+}
+
+impl<T> MinedSolution<T>
+where
+	T: MinerConfig<AccountId = AccountId, MaxVotesPerVoter = static_types::MaxVotesPerVoter>
+		+ Send
+		+ Sync
+		+ 'static,
+	T::Solution: Send,
+{
+	pub fn solution(&self) -> T::Solution {
+		self.solution.clone()
+	}
+
+	pub fn score(&self) -> ElectionScore {
+		self.score
+	}
+
+	pub fn size(&self) -> SolutionOrSnapshotSize {
+		self.solution_or_snapshot_size
+	}
+
+	/// Check that this solution is feasible
+	pub fn feasibility_check(&self) -> Result<(), Error> {
+		match Miner::<T>::feasibility_check(
+			RawSolution { solution: self.solution.clone(), score: self.score, round: self.round },
+			pallet_election_provider_multi_phase::ElectionCompute::Signed,
+			self.desired_targets,
+			self.snapshot.clone(),
+			self.round,
+			self.minimum_untrusted_score,
+		) {
+			Ok(_) => Ok(()),
+			Err(e) => {
+				log::error!(target: LOG_TARGET, "Solution feasibility error {:?}", e);
+				Err(Error::Feasibility(format!("{:?}", e)))
+			},
+		}
+	}
+}
+
 fn make_type<T: scale_info::TypeInfo + 'static>() -> (TypeId, PortableRegistry) {
 	let m = scale_info::MetaType::new::<T>();
 	let mut types = scale_info::Registry::new();

src/main.rs

@@ -317,7 +317,8 @@ mod tests {
 					at: None,
 					solver: opt::Solver::PhragMMS { iterations: 10 },
 					force_snapshot: false,
-					seed_or_path: "//Alice".to_string(),
+					force_winner_count: None,
+					seed_or_path: Some("//Alice".to_string()),
 				}),
 			}
 		);