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

Code before:
	/// record the weight of the given `supports`.
	fn weigh_supports(supports: &Supports<T::AccountId>) {
		let active_voters = supports
			.iter()
			.map(|(_, x)| x)
			.fold(Zero::zero(), |acc, next| acc + next.voters.len() as u32);
		let desired_targets = supports.len() as u32;
		Self::register_weight(T::WeightInfo::elect_queued(active_voters, desired_targets));
	}
}

#[cfg(any(feature = "try-runtime", test))]
impl<T: Config> Pallet<T> {
	fn do_try_state() -> Result<(), &'static str> {
		Self::try_state_snapshot()?;
		Self::try_state_signed_submissions_map()?;
		Self::try_state_phase_off()
	}

	// [`Snapshot`] state check. Invariants:
	// - [`DesiredTargets`] exist IFF [`Snapshot`] is present.
	// - [`SnapshotMetadata`] exist IFF [`Snapshot`] is present.
	fn try_state_snapshot() -> Result<(), &'static str> {
		let set = <DesiredTargets<T>>::get().is_some() && <SnapshotMetadata<T>>::get().is_some();

		match <Snapshot<T>>::take() {
            Some(_) if !set => Err("If the snapshot exists, the desired targets and snapshot metadata must also exist."),
            None if set => Err("If the snapshot does not exists, the desired targets and snapshot metadata should also not exists"),
            _ => Ok(()),
		}
	}

	// [`SignedSubmissionsMap`] state check. Invariants:
	// - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more;
	// - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`];
	// - [`SignedSubmissionIndices`] is sorted by election score.
	fn try_state_signed_submissions_map() -> Result<(), &'static str> {
		let mut best_score: ElectionScore = Default::default();
		let indices = <SignedSubmissionIndices<T>>::get();

		for (i, indice) in indices.iter().enumerate() {
			let submission = <SignedSubmissionsMap<T>>::get(indice.2);
			if submission.is_none() {
				return Err("All signed submissions indices must be part of the submissions map")
			}

			if i == 0 {
				best_score = indice.0
			} else {
				if best_score.strict_threshold_better(indice.0, Perbill::zero()) {
					return Err("Signed submission indices vector must be ordered by election score")

Review comment (suggestion):
`take` would remove item from storage (externalities) which might be a problem for other try state tests following this right? In general, `try-state` checks should not have side effects I think..

Code after:
	/// record the weight of the given `supports`.
	fn weigh_supports(supports: &Supports<T::AccountId>) {
		let active_voters = supports
			.iter()
			.map(|(_, x)| x)
			.fold(Zero::zero(), |acc, next| acc + next.voters.len() as u32);
		let desired_targets = supports.len() as u32;
		Self::register_weight(T::WeightInfo::elect_queued(active_voters, desired_targets));
	}
}

#[cfg(feature = "try-runtime")]
impl<T: Config> Pallet<T> {
	fn do_try_state() -> Result<(), &'static str> {
		Self::try_state_snapshot()?;
		Self::try_state_signed_submissions_map()?;
		Self::try_state_phase_off()
	}

	// [`Snapshot`] state check. Invariants:
	// - [`DesiredTargets`] exists if and only if [`Snapshot`] is present.
	// - [`SnapshotMetadata`] exist if and only if [`Snapshot`] is present.
	fn try_state_snapshot() -> Result<(), &'static str> {
		if <Snapshot<T>>::exists() &&
			<SnapshotMetadata<T>>::exists() &&
			<DesiredTargets<T>>::exists()
		{
			Ok(())
		} else if !<Snapshot<T>>::exists() &&
			!<SnapshotMetadata<T>>::exists() &&
			!<DesiredTargets<T>>::exists()
		{
			Ok(())
		} else {
			Err("If snapshot exists, metadata and desired targets should be set too. Otherwise, none should be set.")
		}
	}

	// [`SignedSubmissionsMap`] state check. Invariants:
	// - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more;
	// - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`];
	// - [`SignedSubmissionIndices`] is sorted by election score.
	fn try_state_signed_submissions_map() -> Result<(), &'static str> {
		let mut last_score: ElectionScore = Default::default();
		let indices = <SignedSubmissionIndices<T>>::get();

		for (i, indice) in indices.iter().enumerate() {
			let submission = <SignedSubmissionsMap<T>>::get(indice.2);
			if submission.is_none() {
				return Err("All signed submissions indices must be part of the submissions map")
			}