Safe desired targets call (#12826)

* checked call for desired targets

* fix compile

* fmt

* fix tests

* cleaner with and_then

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

@@ -1409,12 +1409,12 @@ impl<T: Config> Pallet<T> {
 			return Err(ElectionError::DataProvider("Snapshot too big for submission."))
 		}
 
-		let mut desired_targets =
+		let mut desired_targets = <Pallet<T> as ElectionProviderBase>::desired_targets_checked()
-			T::DataProvider::desired_targets().map_err(ElectionError::DataProvider)?;
+			.map_err(|e| ElectionError::DataProvider(e))?;
 
-		// If `desired_targets` > `targets.len()`, cap `desired_targets` to that
+		// If `desired_targets` > `targets.len()`, cap `desired_targets` to that level and emit a
-		// level and emit a warning
+		// warning
-		let max_desired_targets: u32 = (targets.len() as u32).min(T::MaxWinners::get());
+		let max_desired_targets: u32 = targets.len() as u32;
 		if desired_targets > max_desired_targets {
 			log!(
 				warn,

substrate/frame/election-provider-multi-phase/src/signed.rs

@@ -594,10 +594,12 @@ mod tests {
 			DesiredTargets::set(4);
 			MaxWinners::set(3);
 
-			let (_, _, actual_desired_targets) = MultiPhase::create_snapshot_external().unwrap();
+			// snapshot not created because data provider returned an unexpected number of
-
+			// desired_targets
-			// snapshot is created with min of desired_targets and MaxWinners
+			assert_noop!(
-			assert_eq!(actual_desired_targets, 3);
+				MultiPhase::create_snapshot_external(),
+				ElectionError::DataProvider("desired_targets must not be greater than MaxWinners."),
+			);
 		})
 	}
 

substrate/frame/election-provider-support/src/lib.rs

@@ -386,15 +386,13 @@ pub trait ElectionProviderBase {
 	/// checked call to `Self::DataProvider::desired_targets()` ensuring the value never exceeds
 	/// [`Self::MaxWinners`].
 	fn desired_targets_checked() -> data_provider::Result<u32> {
-		match Self::DataProvider::desired_targets() {
+		Self::DataProvider::desired_targets().and_then(|desired_targets| {
-			Ok(desired_targets) =>
+			if desired_targets <= Self::MaxWinners::get() {
-				if desired_targets <= Self::MaxWinners::get() {
+				Ok(desired_targets)
-					Ok(desired_targets)
+			} else {
-				} else {
+				Err("desired_targets must not be greater than MaxWinners.")
-					Err("desired_targets should not be greater than MaxWinners")
+			}
-				},
+		})
-			Err(e) => Err(e),
-		}
 	}
 }