BREAKING - Try-runtime: Use proper error types

frame/assets/src/migration.rs

@@ -18,6 +18,9 @@
 use super::*;
 use frame_support::{log, traits::OnRuntimeUpgrade};
 
+#[cfg(feature = "try-runtime")]
+use sp_runtime::TryRuntimeError;
+
 pub mod v1 {
  use frame_support::{pallet_prelude::*, weights::Weight};
 
@@ -92,7 +95,7 @@ pub mod v1 {
  }
 
  #[cfg(feature = "try-runtime")]
- fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+ fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
  frame_support::ensure!(
  Pallet::<T>::on_chain_storage_version() == 0,
  "must upgrade linearly"
@@ -102,31 +105,29 @@ pub mod v1 {
  }
 
  #[cfg(feature = "try-runtime")]
- fn post_upgrade(prev_count: Vec<u8>) -> Result<(), &'static str> {
+ fn post_upgrade(prev_count: Vec<u8>) -> Result<(), TryRuntimeError> {
  let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect(
  "the state parameter should be something that was generated by pre_upgrade",
  );
  let post_count = Asset::<T>::iter().count() as u32;
- assert_eq!(
- prev_count, post_count,
+ ensure!(
+ prev_count == post_count,
  "the asset count before and after the migration should be the same"
  );
 
  let current_version = Pallet::<T>::current_storage_version();
  let onchain_version = Pallet::<T>::on_chain_storage_version();
 
  frame_support::ensure!(current_version == 1, "must_upgrade");
- assert_eq!(
- current_version, onchain_version,
+ ensure!(
+ current_version == onchain_version,
  "after migration, the current_version and onchain_version should be the same"
  );
 
- Asset::<T>::iter().for_each(|(_id, asset)| {
- assert!(
- asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen,
- "assets should only be live or frozen. None should be in destroying status, or undefined state"
- )
- });
+ Asset::<T>::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> {
+ ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state");
+ Ok(())
+ })?;
  Ok(())
  }
  }

frame/bags-list/src/lib.rs

@@ -59,6 +59,9 @@ use frame_system::ensure_signed;
 use sp_runtime::traits::{AtLeast32BitUnsigned, Bounded, StaticLookup};
 use sp_std::prelude::*;
 
+#[cfg(any(test, feature = "try-runtime", feature = "fuzz"))]
+use sp_runtime::TryRuntimeError;
+
 #[cfg(any(feature = "runtime-benchmarks", test))]
 mod benchmarks;
 
@@ -267,15 +270,15 @@ pub mod pallet {
  }
 
  #[cfg(feature = "try-runtime")]
- fn try_state(_: BlockNumberFor<T>) -> Result<(), &'static str> {
+ fn try_state(_: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
  <Self as SortedListProvider<T::AccountId>>::try_state()
  }
  }
 }
 
 #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))]
 impl<T: Config<I>, I: 'static> Pallet<T, I> {
- pub fn do_try_state() -> Result<(), &'static str> {
+ pub fn do_try_state() -> Result<(), TryRuntimeError> {
  List::<T, I>::do_try_state()
  }
 }
@@ -355,7 +358,7 @@ impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I>
  }
 
  #[cfg(feature = "try-runtime")]
- fn try_state() -> Result<(), &'static str> {
+ fn try_state() -> Result<(), TryRuntimeError> {
  Self::do_try_state()
  }
 

frame/bags-list/src/list/mod.rs

@@ -42,6 +42,9 @@ use sp_std::{
  prelude::*,
 };
 
+#[cfg(any(test, feature = "try-runtime", feature = "fuzz"))]
+use sp_runtime::TryRuntimeError;
+
 #[derive(Debug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, PalletError)]
 pub enum ListError {
  /// A duplicate id has been detected.
@@ -512,11 +515,11 @@ impl<T: Config<I>, I: 'static> List<T, I> {
  /// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure
  /// all bags and nodes are checked per *any* update to `List`.
  #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))]
- pub(crate) fn do_try_state() -> Result<(), &'static str> {
+ pub(crate) fn do_try_state() -> Result<(), TryRuntimeError> {
  let mut seen_in_list = BTreeSet::new();
  ensure!(
  Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)),
- "duplicate identified",
+ "duplicate identified"
  );
 
  let iter_count = Self::iter().count() as u32;
@@ -750,7 +753,7 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
  /// * Ensures tail has no next.
  /// * Ensures there are no loops, traversal from head to tail is correct.
  #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))]
- fn do_try_state(&self) -> Result<(), &'static str> {
+ fn do_try_state(&self) -> Result<(), TryRuntimeError> {
  frame_support::ensure!(
  self.head()
  .map(|head| head.prev().is_none())
@@ -895,15 +898,12 @@ impl<T: Config<I>, I: 'static> Node<T, I> {
  }
 
  #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))]
- fn do_try_state(&self) -> Result<(), &'static str> {
+ fn do_try_state(&self) -> Result<(), TryRuntimeError> {
  let expected_bag = Bag::<T, I>::get(self.bag_upper).ok_or("bag not found for node")?;
 
  let id = self.id();
 
- frame_support::ensure!(
- expected_bag.contains(id),
- "node does not exist in the expected bag"
- );
+ frame_support::ensure!(expected_bag.contains(id), "node does not exist in the bag");
 
  let non_terminal_check = !self.is_terminal() &&
  expected_bag.head.as_ref() != Some(id) &&

frame/bags-list/src/list/tests.rs

@@ -22,6 +22,7 @@ use crate::{
 };
 use frame_election_provider_support::{SortedListProvider, VoteWeight};
 use frame_support::{assert_ok, assert_storage_noop};
+use sp_runtime::TryRuntimeError;
 
 fn node(
  id: AccountId,
@@ -359,7 +360,10 @@ mod list {
  // make sure there are no duplicates.
  ExtBuilder::default().build_and_execute_no_post_check(|| {
  Bag::<Runtime>::get(10).unwrap().insert_unchecked(2, 10);
- assert_eq!(List::<Runtime>::do_try_state(), Err("duplicate identified"));
+ assert_eq!(
+ List::<Runtime>::do_try_state(),
+ TryRuntimeError::Other("duplicate identified").into()
+ );
  });
 
  // ensure count is in sync with `ListNodes::count()`.
@@ -373,7 +377,10 @@ mod list {
  CounterForListNodes::<Runtime>::mutate(|counter| *counter += 1);
  assert_eq!(crate::ListNodes::<Runtime>::count(), 5);
 
- assert_eq!(List::<Runtime>::do_try_state(), Err("iter_count != stored_count"));
+ assert_eq!(
+ List::<Runtime>::do_try_state(),
+ TryRuntimeError::Other("iter_count != stored_count").into()
+ );
  });
  }
 

frame/bags-list/src/migrations.rs

@@ -24,6 +24,9 @@ use frame_support::traits::OnRuntimeUpgrade;
 
 #[cfg(feature = "try-runtime")]
 use frame_support::ensure;
+#[cfg(feature = "try-runtime")]
+use sp_runtime::TryRuntimeError;
+
 #[cfg(feature = "try-runtime")]
 use sp_std::vec::Vec;
 
@@ -35,7 +38,7 @@ impl<T: crate::Config<I>, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix<T,
  }
 
  #[cfg(feature = "try-runtime")]
- fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+ fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
  // The old explicit storage item.
  #[frame_support::storage_alias]
  type CounterForListNodes<T: crate::Config<I>, I: 'static> =
@@ -88,7 +91,7 @@ mod old {
 pub struct AddScore<T: crate::Config<I>, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>);
 impl<T: crate::Config<I>, I: 'static> OnRuntimeUpgrade for AddScore<T, I> {
  #[cfg(feature = "try-runtime")]
- fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+ fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
  // The list node data should be corrupt at this point, so this is zero.
  ensure!(crate::ListNodes::<T, I>::iter().count() == 0, "list node data is not corrupt");
  // We can use the helper `old::ListNode` to get the existing data.
@@ -119,7 +122,7 @@ impl<T: crate::Config<I>, I: 'static> OnRuntimeUpgrade for AddScore<T, I> {
  }
 
  #[cfg(feature = "try-runtime")]
- fn post_upgrade(node_count_before: Vec<u8>) -> Result<(), &'static str> {
+ fn post_upgrade(node_count_before: Vec<u8>) -> Result<(), TryRuntimeError> {
  let node_count_before: u32 = Decode::decode(&mut node_count_before.as_slice())
  .expect("the state parameter should be something that was generated by pre_upgrade");
  // Now the list node data is not corrupt anymore.

frame/collective/src/lib.rs

@@ -60,6 +60,9 @@ use frame_support::{
  weights::Weight,
 };
 
+#[cfg(any(feature = "try-runtime", test))]
+use sp_runtime::TryRuntimeError;
+
 #[cfg(test)]
 mod tests;
 
@@ -352,9 +355,8 @@ pub mod pallet {
  #[pallet::hooks]
  impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
  #[cfg(feature = "try-runtime")]
- fn try_state(_n: BlockNumberFor<T>) -> Result<(), &'static str> {
- Self::do_try_state()?;
- Ok(())
+ fn try_state(_n: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
+ Self::do_try_state()
  }
  }
 
@@ -973,77 +975,78 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
  /// Looking at prime account:
  /// * The prime account must be a member of the collective.
  #[cfg(any(feature = "try-runtime", test))]
- fn do_try_state() -> DispatchResult {
- Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult {
- ensure!(
- Self::proposal_of(proposal).is_some(),
- DispatchError::Other(
+ fn do_try_state() -> Result<(), TryRuntimeError> {
+ Self::proposals()
+ .into_iter()
+ .try_for_each(|proposal| -> Result<(), TryRuntimeError> {
+ ensure!(
+ Self::proposal_of(proposal).is_some(),
  "Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping."
- )
- );
- Ok(())
- })?;
+ );
+ Ok(())
+ })?;
 
  ensure!(
  Self::proposals().into_iter().count() <= Self::proposal_count() as usize,
- DispatchError::Other("The actual number of proposals is greater than `ProposalCount`")
+ "The actual number of proposals is greater than `ProposalCount`"
  );
  ensure!(
  Self::proposals().into_iter().count() == <ProposalOf<T, I>>::iter_keys().count(),
- DispatchError::Other("Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`")
+ "Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`"
  );
 
- Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult {
- if let Some(votes) = Self::voting(proposal) {
- let ayes = votes.ayes.len();
- let nays = votes.nays.len();
-
- ensure!(
- ayes.saturating_add(nays) <= T::MaxMembers::get() as usize,
- DispatchError::Other("The sum of ayes and nays is greater than `MaxMembers`")
- );
- }
- Ok(())
- })?;
+ Self::proposals()
+ .into_iter()
+ .try_for_each(|proposal| -> Result<(), TryRuntimeError> {
+ if let Some(votes) = Self::voting(proposal) {
+ let ayes = votes.ayes.len();
+ let nays = votes.nays.len();
+
+ ensure!(
+ ayes.saturating_add(nays) <= T::MaxMembers::get() as usize,
+ "The sum of ayes and nays is greater than `MaxMembers`"
+ );
+ }
+ Ok(())
+ })?;
 
  let mut proposal_indices = vec![];
- Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult {
- if let Some(votes) = Self::voting(proposal) {
- let proposal_index = votes.index;
- ensure!(
- !proposal_indices.contains(&proposal_index),
- DispatchError::Other("The proposal index is not unique.")
- );
- proposal_indices.push(proposal_index);
- }
- Ok(())
- })?;
+ Self::proposals()
+ .into_iter()
+ .try_for_each(|proposal| -> Result<(), TryRuntimeError> {
+ if let Some(votes) = Self::voting(proposal) {
+ let proposal_index = votes.index;
+ ensure!(
+ !proposal_indices.contains(&proposal_index),
+ "The proposal index is not unique."
+ );
+ proposal_indices.push(proposal_index);
+ }
+ Ok(())
+ })?;
 
- <Voting<T, I>>::iter_keys().try_for_each(|proposal_hash| -> DispatchResult {
- ensure!(
- Self::proposals().contains(&proposal_hash),
- DispatchError::Other(
+ <Voting<T, I>>::iter_keys().try_for_each(
+ |proposal_hash| -> Result<(), TryRuntimeError> {
+ ensure!(
+  Self::proposals().contains(&proposal_hash),
  "`Proposals` doesn't contain the proposal hash from the `Voting` storage map."
- )
- );
- Ok(())
- })?;
+ );
+  Ok(())
+ },
+ )?;
 
  ensure!(
  Self::members().len() <= T::MaxMembers::get() as usize,
- DispatchError::Other("The member count is greater than `MaxMembers`.")
+ "The member count is greater than `MaxMembers`."
  );
 
  ensure!(
  Self::members().windows(2).all(|members| members[0] <= members[1]),
- DispatchError::Other("The members are not sorted by value.")
+ "The members are not sorted by value."
  );
 
  if let Some(prime) = Self::prime() {
- ensure!(
- Self::members().contains(&prime),
- DispatchError::Other("Prime account is not a member.")
- );
+ ensure!(Self::members().contains(&prime), "Prime account is not a member.");
  }
 
  Ok(())