Migrate generic-asset, identity and im-online to decl_error (#4473)

* Migrate generic-asset, identity and im-online to decl_error

* Update democracy tests

* Update nicks test

substrate/frame/balances/src/lib.rs

@@ -1002,13 +1002,13 @@ where
 				// ...yet is was alive before
 				&& old_balance >= T::ExistentialDeposit::get()
 			{
-				Err("payment would kill account")?
+				Err(Error::<T, I>::KeepAlive)?
 			}
 			Self::ensure_can_withdraw(who, value, reasons, new_balance)?;
 			Self::set_free_balance(who, new_balance);
 			Ok(NegativeImbalance::new(value))
 		} else {
-			Err("too few free funds in account")?
+			Err(Error::<T, I>::InsufficientBalance)?
 		}
 	}
 
@@ -1123,7 +1123,7 @@ where
 	fn reserve(who: &T::AccountId, value: Self::Balance) -> result::Result<(), DispatchError> {
 		let b = Self::free_balance(who);
 		if b < value {
-			Err("not enough free funds")?
+			Err(Error::<T, I>::InsufficientBalance)?
 		}
 		let new_balance = b - value;
 		Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve.into(), new_balance)?;
@@ -1268,7 +1268,7 @@ where
 		starting_block: T::BlockNumber
 	) -> DispatchResult {
 		if <Vesting<T, I>>::exists(who) {
-			Err("A vesting schedule already exists for this account.")?
+			Err(Error::<T, I>::ExistingVestingSchedule)?
 		}
 		let vesting_schedule = VestingSchedule {
 			locked,

substrate/frame/democracy/src/lib.rs

@@ -1152,7 +1152,7 @@ mod tests {
 		traits::{BlakeTwo256, IdentityLookup, Bounded, BadOrigin},
 		testing::Header, Perbill,
 	};
-	use pallet_balances::BalanceLock;
+	use pallet_balances::{BalanceLock, Error as BalancesError};
 	use frame_system::EnsureSignedBy;
 
 	const AYE: Vote = Vote{ aye: true, conviction: Conviction::None };
@@ -1356,7 +1356,7 @@ mod tests {
 			PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 100);
 			assert_noop!(
 				Democracy::note_preimage(Origin::signed(6), vec![0; 500]),
-				"not enough free funds",
+				BalancesError::<Test, _>::InsufficientBalance,
 			);
 			// fee of 1 is reasonable.
 			PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
@@ -2118,7 +2118,7 @@ mod tests {
 	fn poor_proposer_should_not_work() {
 		new_test_ext().execute_with(|| {
 			System::set_block_number(1);
-			assert_noop!(propose_set_balance(1, 2, 11), "not enough free funds");
+			assert_noop!(propose_set_balance(1, 2, 11), BalancesError::<Test, _>::InsufficientBalance);
 		});
 	}
 
@@ -2127,7 +2127,7 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			System::set_block_number(1);
 			assert_ok!(propose_set_balance_and_note(2, 2, 11));
-			assert_noop!(Democracy::second(Origin::signed(1), 0), "not enough free funds");
+			assert_noop!(Democracy::second(Origin::signed(1), 0), BalancesError::<Test, _>::InsufficientBalance);
 		});
 	}
 

substrate/frame/generic-asset/src/lib.rs

@@ -151,7 +151,7 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use codec::{Decode, Encode, HasCompact, Input, Output, Error};
+use codec::{Decode, Encode, HasCompact, Input, Output, Error as CodecError};
 
 use sp_runtime::{RuntimeDebug, DispatchResult, DispatchError};
 use sp_runtime::traits::{
@@ -162,7 +162,7 @@ use sp_runtime::traits::{
 use sp_std::prelude::*;
 use sp_std::{cmp, result, fmt::Debug};
 use frame_support::{
-	decl_event, decl_module, decl_storage, ensure, dispatch,
+	decl_event, decl_module, decl_storage, ensure, dispatch, decl_error,
 	traits::{
 		Currency, ExistenceRequirement, Imbalance, LockIdentifier, LockableCurrency, ReservableCurrency,
 		SignedImbalance, UpdateBalanceOutcome, WithdrawReason, WithdrawReasons, TryDrop,
@@ -285,7 +285,7 @@ impl<AccountId: Encode> Encode for PermissionVersions<AccountId> {
 impl<AccountId: Encode> codec::EncodeLike for PermissionVersions<AccountId> {}
 
 impl<AccountId: Decode> Decode for PermissionVersions<AccountId> {
-	fn decode<I: Input>(input: &mut I) -> core::result::Result<Self, Error> {
+	fn decode<I: Input>(input: &mut I) -> core::result::Result<Self, CodecError> {
 		let version = PermissionVersionNumber::decode(input)?;
 		Ok(
 			match version {
@@ -320,8 +320,42 @@ impl<AccountId> Into<PermissionVersions<AccountId>> for PermissionLatest<Account
 	}
 }
 
+decl_error! {
+	/// Error for the identity module.
+	pub enum Error for Module<T: Trait> {
+		/// No new assets id available.
+		NoIdAvailable,
+		/// Cannot transfer zero amount.
+		ZeroAmount,
+		/// The origin does not have enough permission to update permissions.
+		NoUpdatePermission,
+		/// The origin does not have permission to mint an asset.
+		NoMintPermission,
+		/// The origin does not have permission to burn an asset.
+		NoBurnPermission,
+		/// Total issuance got overflowed after minting.
+		TotalMintingOverflow,
+		/// Free balance got overflowed after minting.
+		FreeMintingOverflow,
+		/// Total issuance got underflowed after burning.
+		TotalBurningUnderflow,
+		/// Free balance got underflowed after burning.
+		FreeBurningUnderflow,
+		/// Asset id is already taken.
+		IdAlreadyTaken,
+		/// Asset id not available.
+		IdUnavailable,
+		/// The balance is too low to send amount.
+		InsufficientBalance,
+		/// The account liquidity restrictions prevent withdrawal.
+		LiquidityRestrictions,
+	}
+}
+
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error<T>;
+
 		fn deposit_event() = default;
 
 		/// Create a new kind of asset.
@@ -332,7 +366,7 @@ decl_module! {
 			let permissions: PermissionVersions<T::AccountId> = options.permissions.clone().into();
 
 			// The last available id serves as the overflow mark and won't be used.
-			let next_id = id.checked_add(&One::one()).ok_or_else(|| "No new assets id available.")?;
+			let next_id = id.checked_add(&One::one()).ok_or_else(|| Error::<T>::NoIdAvailable)?;
 
 			<NextAssetId<T>>::put(next_id);
 			<TotalIssuance<T>>::insert(id, &options.initial_issuance);
@@ -347,7 +381,7 @@ decl_module! {
 		/// Transfer some liquid free balance to another account.
 		pub fn transfer(origin, #[compact] asset_id: T::AssetId, to: T::AccountId, #[compact] amount: T::Balance) {
 			let origin = ensure_signed(origin)?;
-			ensure!(!amount.is_zero(), "cannot transfer zero amount");
+			ensure!(!amount.is_zero(), Error::<T>::ZeroAmount);
 			Self::make_transfer_with_event(&asset_id, &origin, &to, amount)?;
 		}
 
@@ -370,7 +404,7 @@ decl_module! {
 
 				Ok(())
 			} else {
-				Err("Origin does not have enough permission to update permissions.")?
+				Err(Error::<T>::NoUpdatePermission)?
 			}
 		}
 
@@ -384,9 +418,9 @@ decl_module! {
 				let original_free_balance = Self::free_balance(&asset_id, &to);
 				let current_total_issuance = <TotalIssuance<T>>::get(asset_id);
 				let new_total_issuance = current_total_issuance.checked_add(&amount)
-					.ok_or_else(|| "total_issuance got overflow after minting.")?;
+					.ok_or(Error::<T>::TotalMintingOverflow)?;
 				let value = original_free_balance.checked_add(&amount)
-					.ok_or_else(|| "free balance got overflow after minting.")?;
+					.ok_or(Error::<T>::FreeMintingOverflow)?;
 
 				<TotalIssuance<T>>::insert(asset_id, new_total_issuance);
 				Self::set_free_balance(&asset_id, &to, value);
@@ -395,7 +429,7 @@ decl_module! {
 
 				Ok(())
 			} else {
-				Err("The origin does not have permission to mint an asset.")?
+				Err(Error::<T>::NoMintPermission)?
 			}
 		}
 
@@ -412,9 +446,9 @@ decl_module! {
 
 				let current_total_issuance = <TotalIssuance<T>>::get(asset_id);
 				let new_total_issuance = current_total_issuance.checked_sub(&amount)
-					.ok_or_else(|| "total_issuance got underflow after burning")?;
+					.ok_or(Error::<T>::TotalBurningUnderflow)?;
 				let value = original_free_balance.checked_sub(&amount)
-					.ok_or_else(|| "free_balance got underflow after burning")?;
+					.ok_or(Error::<T>::FreeBurningUnderflow)?;
 
 				<TotalIssuance<T>>::insert(asset_id, new_total_issuance);
 
@@ -424,7 +458,7 @@ decl_module! {
 
 				Ok(())
 			} else {
-				Err("The origin does not have permission to burn an asset.")?
+				Err(Error::<T>::NoBurnPermission)?
 			}
 		}
 
@@ -545,14 +579,14 @@ impl<T: Trait> Module<T> {
 		options: AssetOptions<T::Balance, T::AccountId>,
 	) -> dispatch::DispatchResult {
 		let asset_id = if let Some(asset_id) = asset_id {
-			ensure!(!<TotalIssuance<T>>::exists(&asset_id), "Asset id already taken.");
+			ensure!(!<TotalIssuance<T>>::exists(&asset_id), Error::<T>::IdAlreadyTaken);
-			ensure!(asset_id < Self::next_asset_id(), "Asset id not available.");
+			ensure!(asset_id < Self::next_asset_id(), Error::<T>::IdUnavailable);
 			asset_id
 		} else {
 			let asset_id = Self::next_asset_id();
 			let next_id = asset_id
 				.checked_add(&One::one())
-				.ok_or_else(|| "No new user asset id available.")?;
+				.ok_or(Error::<T>::NoIdAvailable)?;
 			<NextAssetId<T>>::put(next_id);
 			asset_id
 		};
@@ -579,7 +613,7 @@ impl<T: Trait> Module<T> {
 	) -> dispatch::DispatchResult {
 		let new_balance = Self::free_balance(asset_id, from)
 			.checked_sub(&amount)
-			.ok_or_else(|| "balance too low to send amount")?;
+			.ok_or(Error::<T>::InsufficientBalance)?;
 		Self::ensure_can_withdraw(asset_id, from, amount, WithdrawReason::Transfer.into(), new_balance)?;
 
 		if from != to {
@@ -618,7 +652,7 @@ impl<T: Trait> Module<T> {
 		let original_reserve_balance = Self::reserved_balance(asset_id, who);
 		let original_free_balance = Self::free_balance(asset_id, who);
 		if original_free_balance < amount {
-			Err("not enough free funds")?
+			Err(Error::<T>::InsufficientBalance)?
 		}
 		let new_reserve_balance = original_reserve_balance + amount;
 		Self::set_reserved_balance(asset_id, who, new_reserve_balance);
@@ -767,7 +801,7 @@ impl<T: Trait> Module<T> {
 		{
 			Ok(())
 		} else {
-			Err("account liquidity restrictions prevent withdrawal")?
+			Err(Error::<T>::LiquidityRestrictions)?
 		}
 	}
 
@@ -1155,7 +1189,7 @@ where
 	) -> result::Result<Self::NegativeImbalance, DispatchError> {
 		let new_balance = Self::free_balance(who)
 			.checked_sub(&value)
-			.ok_or_else(|| "account has too few funds")?;
+			.ok_or(Error::<T>::InsufficientBalance)?;
 		Self::ensure_can_withdraw(who, value, reasons, new_balance)?;
 		<Module<T>>::set_free_balance(&U::asset_id(), who, new_balance);
 		Ok(NegativeImbalance::new(value))

substrate/frame/generic-asset/src/tests.rs

@@ -65,7 +65,7 @@ fn issuing_with_next_asset_id_overflow_should_not_work() {
 					permissions: default_permission
 				}
 			),
-			"No new assets id available."
+			Error::<Test>::NoIdAvailable
 		);
 		assert_eq!(GenericAsset::next_asset_id(), u32::max_value());
 	});
@@ -173,7 +173,7 @@ fn transferring_amount_should_fail_when_transferring_more_than_free_balance() {
 		));
 		assert_noop!(
 			GenericAsset::transfer(Origin::signed(1), asset_id, 2, 2000),
-			"balance too low to send amount"
+			Error::<Test>::InsufficientBalance
 		);
 	});
 }
@@ -198,7 +198,7 @@ fn transferring_less_than_one_unit_should_not_work() {
 		assert_eq!(GenericAsset::free_balance(&asset_id, &1), 100);
 		assert_noop!(
 			GenericAsset::transfer(Origin::signed(1), asset_id, 2, 0),
-			"cannot transfer zero amount"
+			Error::<Test>::ZeroAmount
 		);
 	});
 }
@@ -256,7 +256,7 @@ fn transferring_more_units_than_total_supply_should_not_work() {
 		assert_eq!(GenericAsset::free_balance(&asset_id, &1), 100);
 		assert_noop!(
 			GenericAsset::transfer(Origin::signed(1), asset_id, 2, 101),
-			"balance too low to send amount"
+			Error::<Test>::InsufficientBalance
 		);
 	});
 }
@@ -424,7 +424,7 @@ fn reserve_should_moves_amount_from_balance_to_reserved_balance() {
 #[test]
 fn reserve_should_not_moves_amount_from_balance_to_reserved_balance() {
 	ExtBuilder::default().free_balance((1, 0, 100)).build().execute_with(|| {
-		assert_noop!(GenericAsset::reserve(&1, &0, 120), "not enough free funds");
+		assert_noop!(GenericAsset::reserve(&1, &0, 120), Error::<Test>::InsufficientBalance);
 		assert_eq!(GenericAsset::free_balance(&1, &0), 100);
 		assert_eq!(GenericAsset::reserved_balance(&1, &0), 0);
 	});
@@ -626,7 +626,7 @@ fn mint_should_throw_permission_error() {
 
 		assert_noop!(
 			GenericAsset::mint(Origin::signed(origin), asset_id, to_account, amount),
-			"The origin does not have permission to mint an asset."
+			Error::<Test>::NoMintPermission,
 		);
 	});
 }
@@ -687,7 +687,7 @@ fn burn_should_throw_permission_error() {
 
 		assert_noop!(
 			GenericAsset::burn(Origin::signed(origin), asset_id, to_account, amount),
-			"The origin does not have permission to burn an asset."
+			Error::<Test>::NoBurnPermission,
 		);
 	});
 }
@@ -873,8 +873,6 @@ fn update_permission_should_throw_error_when_lack_of_permissions() {
 			burn: Owner::None,
 		};
 
-		let expected_error_message = "Origin does not have enough permission to update permissions.";
-
 		assert_ok!(GenericAsset::create(
 			Origin::signed(origin),
 			AssetOptions {
@@ -885,7 +883,7 @@ fn update_permission_should_throw_error_when_lack_of_permissions() {
 
 		assert_noop!(
 			GenericAsset::update_permission(Origin::signed(origin), asset_id, new_permission),
-			expected_error_message,
+			Error::<Test>::NoUpdatePermission,
 		);
 	});
 }
@@ -963,7 +961,7 @@ fn create_asset_with_non_reserved_asset_id_should_not_work() {
 					permissions: default_permission.clone()
 				}
 			),
-			"Asset id not available."
+			Error::<Test>::IdUnavailable,
 		);
 	});
 }
@@ -1005,7 +1003,7 @@ fn create_asset_with_a_taken_asset_id_should_not_work() {
 					permissions: default_permission.clone()
 				}
 			),
-			"Asset id already taken."
+			Error::<Test>::IdAlreadyTaken,
 		);
 	});
 }

substrate/frame/identity/src/lib.rs

@@ -71,7 +71,7 @@ use enumflags2::BitFlags;
 use codec::{Encode, Decode};
 use sp_runtime::{DispatchResult, traits::{StaticLookup, EnsureOrigin, Zero}, RuntimeDebug};
 use frame_support::{
-	decl_module, decl_event, decl_storage, ensure,
+	decl_module, decl_event, decl_storage, ensure, decl_error,
 	traits::{Currency, ReservableCurrency, OnUnbalanced, Get},
 	weights::SimpleDispatchInfo,
 };
@@ -401,9 +401,39 @@ decl_event!(
 	}
 );
 
+decl_error! {
+	/// Error for the identity module.
+	pub enum Error for Module<T: Trait> {
+		/// Too many subs-accounts.
+		TooManySubAccounts,
+		/// Account isn't found.
+		NotFound,
+		/// Account isn't named.
+		NotNamed,
+		/// Empty index.
+		EmptyIndex,
+		/// Fee is changed.
+		FeeChanged,
+		/// No identity found.
+		NoIdentity,
+		/// Sticky judgement.
+		StickyJudgement,
+		/// Judgement given.
+		JudgementGiven,
+		/// Invalid judgement.
+		InvalidJudgement,
+		/// The index is invalid.
+		InvalidIndex,
+		/// The target is invalid.
+		InvalidTarget,
+	}
+}
+
 decl_module! {
 	// Simple declaration of the `Module` type. Lets the macro know what it's working on.
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error<T>;
+
 		fn deposit_event() = default;
 
 		/// Add a registrar to the system.
@@ -497,8 +527,8 @@ decl_module! {
 		/// # </weight>
 		fn set_subs(origin, subs: Vec<(T::AccountId, Data)>) {
 			let sender = ensure_signed(origin)?;
-			ensure!(<IdentityOf<T>>::exists(&sender), "not found");
+			ensure!(<IdentityOf<T>>::exists(&sender), Error::<T>::NotFound);
-			ensure!(subs.len() <= T::MaximumSubAccounts::get() as usize, "too many subs");
+			ensure!(subs.len() <= T::MaximumSubAccounts::get() as usize, Error::<T>::TooManySubAccounts);
 
 			let (old_deposit, old_ids) = <SubsOf<T>>::get(&sender);
 			let new_deposit = T::SubAccountDeposit::get() * <BalanceOf<T>>::from(subs.len() as u32);
@@ -545,7 +575,7 @@ decl_module! {
 			let sender = ensure_signed(origin)?;
 
 			let (subs_deposit, sub_ids) = <SubsOf<T>>::take(&sender);
-			let deposit = <IdentityOf<T>>::take(&sender).ok_or("not named")?.total_deposit()
+			let deposit = <IdentityOf<T>>::take(&sender).ok_or(Error::<T>::NotNamed)?.total_deposit()
 				+ subs_deposit;
 			for sub in sub_ids.iter() {
 				<SuperOf<T>>::remove(sub);
@@ -587,14 +617,14 @@ decl_module! {
 			let sender = ensure_signed(origin)?;
 			let registrars = <Registrars<T>>::get();
 			let registrar = registrars.get(reg_index as usize).and_then(Option::as_ref)
-				.ok_or("empty index")?;
+				.ok_or(Error::<T>::EmptyIndex)?;
-			ensure!(max_fee >= registrar.fee, "fee changed");
+			ensure!(max_fee >= registrar.fee, Error::<T>::FeeChanged);
-			let mut id = <IdentityOf<T>>::get(&sender).ok_or("no identity")?;
+			let mut id = <IdentityOf<T>>::get(&sender).ok_or(Error::<T>::NoIdentity)?;
 
 			let item = (reg_index, Judgement::FeePaid(registrar.fee));
 			match id.judgements.binary_search_by_key(&reg_index, |x| x.0) {
 				Ok(i) => if id.judgements[i].1.is_sticky() {
-					Err("sticky judgement")?
+					Err(Error::<T>::StickyJudgement)?
 				} else {
 					id.judgements[i] = item
 				},
@@ -628,14 +658,14 @@ decl_module! {
 		#[weight = SimpleDispatchInfo::FixedNormal(50_000)]
 		fn cancel_request(origin, reg_index: RegistrarIndex) {
 			let sender = ensure_signed(origin)?;
-			let mut id = <IdentityOf<T>>::get(&sender).ok_or("no identity")?;
+			let mut id = <IdentityOf<T>>::get(&sender).ok_or(Error::<T>::NoIdentity)?;
 
 			let pos = id.judgements.binary_search_by_key(&reg_index, |x| x.0)
-				.map_err(|_| "not found")?;
+				.map_err(|_| Error::<T>::NotFound)?;
 			let fee = if let Judgement::FeePaid(fee) = id.judgements.remove(pos).1 {
 				fee
 			} else {
-				Err("judgement given")?
+				Err(Error::<T>::JudgementGiven)?
 			};
 
 			let _ = T::Currency::unreserve(&sender, fee);
@@ -667,7 +697,7 @@ decl_module! {
 				rs.get_mut(index as usize)
 					.and_then(|x| x.as_mut())
 					.and_then(|r| if r.account == who { r.fee = fee; Some(()) } else { None })
-					.ok_or_else(|| "invalid index".into())
+					.ok_or_else(|| Error::<T>::InvalidIndex.into())
 			)
 		}
 
@@ -694,7 +724,7 @@ decl_module! {
 				rs.get_mut(index as usize)
 					.and_then(|x| x.as_mut())
 					.and_then(|r| if r.account == who { r.account = new; Some(()) } else { None })
-					.ok_or_else(|| "invalid index".into())
+					.ok_or_else(|| Error::<T>::InvalidIndex.into())
 			)
 		}
 
@@ -721,7 +751,7 @@ decl_module! {
 				rs.get_mut(index as usize)
 					.and_then(|x| x.as_mut())
 					.and_then(|r| if r.account == who { r.fields = fields; Some(()) } else { None })
-					.ok_or_else(|| "invalid index".into())
+					.ok_or_else(|| Error::<T>::InvalidIndex.into())
 			)
 		}
 
@@ -752,13 +782,13 @@ decl_module! {
 		) {
 			let sender = ensure_signed(origin)?;
 			let target = T::Lookup::lookup(target)?;
-			ensure!(!judgement.has_deposit(), "invalid judgement");
+			ensure!(!judgement.has_deposit(), Error::<T>::InvalidJudgement);
 			<Registrars<T>>::get()
 				.get(reg_index as usize)
 				.and_then(Option::as_ref)
 				.and_then(|r| if r.account == sender { Some(r) } else { None })
-				.ok_or("invalid index")?;
+				.ok_or(Error::<T>::InvalidIndex)?;
-			let mut id = <IdentityOf<T>>::get(&target).ok_or("invalid target")?;
+			let mut id = <IdentityOf<T>>::get(&target).ok_or(Error::<T>::InvalidTarget)?;
 
 			let item = (reg_index, judgement);
 			match id.judgements.binary_search_by_key(&reg_index, |x| x.0) {
@@ -803,7 +833,7 @@ decl_module! {
 			let target = T::Lookup::lookup(target)?;
 			// Grab their deposit (and check that they have one).
 			let (subs_deposit, sub_ids) = <SubsOf<T>>::take(&target);
-			let deposit = <IdentityOf<T>>::take(&target).ok_or("not named")?.total_deposit()
+			let deposit = <IdentityOf<T>>::take(&target).ok_or(Error::<T>::NotNamed)?.total_deposit()
 				+ subs_deposit;
 			for sub in sub_ids.iter() {
 				<SuperOf<T>>::remove(sub);
@@ -951,7 +981,7 @@ mod tests {
 			assert_eq!(Balances::free_balance(10), 90);
 			assert_ok!(Identity::clear_identity(Origin::signed(10)));
 			assert_eq!(Balances::free_balance(10), 100);
-			assert_noop!(Identity::clear_identity(Origin::signed(10)), "not named");
+			assert_noop!(Identity::clear_identity(Origin::signed(10)), Error::<Test>::NotNamed);
 		});
 	}
 
@@ -960,23 +990,23 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_noop!(
 				Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable),
-				"invalid index"
+				Error::<Test>::InvalidIndex
 			);
 
 			assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 			assert_noop!(
 				Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable),
-				"invalid target"
+				Error::<Test>::InvalidTarget
 			);
 
 			assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
 			assert_noop!(
 				Identity::provide_judgement(Origin::signed(10), 0, 10, Judgement::Reasonable),
-				"invalid index"
+				Error::<Test>::InvalidIndex
 			);
 			assert_noop!(
 				Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::FeePaid(1)),
-				"invalid judgement"
+				Error::<Test>::InvalidJudgement
 			);
 
 			assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable));
@@ -1003,7 +1033,7 @@ mod tests {
 			assert_ok!(Identity::kill_identity(Origin::signed(2), 10));
 			assert_eq!(Identity::identity(10), None);
 			assert_eq!(Balances::free_balance(10), 90);
-			assert_noop!(Identity::kill_identity(Origin::signed(2), 10), "not named");
+			assert_noop!(Identity::kill_identity(Origin::signed(2), 10), Error::<Test>::NotNamed);
 		});
 	}
 
@@ -1011,7 +1041,7 @@ mod tests {
 	fn setting_subaccounts_should_work() {
 		new_test_ext().execute_with(|| {
 			let mut subs = vec![(20, Data::Raw(vec![40; 1]))];
-			assert_noop!(Identity::set_subs(Origin::signed(10), subs.clone()), "not found");
+			assert_noop!(Identity::set_subs(Origin::signed(10), subs.clone()), Error::<Test>::NotFound);
 
 			assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
 			assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone()));
@@ -1044,7 +1074,7 @@ mod tests {
 			assert_eq!(Identity::super_of(40), None);
 
 			subs.push((20, Data::Raw(vec![40; 1])));
-			assert_noop!(Identity::set_subs(Origin::signed(10), subs.clone()), "too many subs");
+			assert_noop!(Identity::set_subs(Origin::signed(10), subs.clone()), Error::<Test>::TooManySubAccounts);
 		});
 	}
 
@@ -1075,15 +1105,15 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 			assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
-			assert_noop!(Identity::cancel_request(Origin::signed(10), 0), "no identity");
+			assert_noop!(Identity::cancel_request(Origin::signed(10), 0), Error::<Test>::NoIdentity);
 			assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
 			assert_ok!(Identity::request_judgement(Origin::signed(10), 0, 10));
 			assert_ok!(Identity::cancel_request(Origin::signed(10), 0));
 			assert_eq!(Balances::free_balance(10), 90);
-			assert_noop!(Identity::cancel_request(Origin::signed(10), 0), "not found");
+			assert_noop!(Identity::cancel_request(Origin::signed(10), 0), Error::<Test>::NotFound);
 
 			assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable));
-			assert_noop!(Identity::cancel_request(Origin::signed(10), 0), "judgement given");
+			assert_noop!(Identity::cancel_request(Origin::signed(10), 0), Error::<Test>::JudgementGiven);
 		});
 	}
 
@@ -1093,19 +1123,19 @@ mod tests {
 			assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 			assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
 			assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
-			assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 9), "fee changed");
+			assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 9), Error::<Test>::FeeChanged);
 			assert_ok!(Identity::request_judgement(Origin::signed(10), 0, 10));
 			// 10 for the judgement request, 10 for the identity.
 			assert_eq!(Balances::free_balance(10), 80);
 
 			// Re-requesting won't work as we already paid.
-			assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 10), "sticky judgement");
+			assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 10), Error::<Test>::StickyJudgement);
 			assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Erroneous));
 			// Registrar got their payment now.
 			assert_eq!(Balances::free_balance(3), 20);
 
 			// Re-requesting still won't work as it's erroneous.
-			assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 10), "sticky judgement");
+			assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 10), Error::<Test>::StickyJudgement);
 
 			// Requesting from a second registrar still works.
 			assert_ok!(Identity::add_registrar(Origin::signed(1), 4));
@@ -1137,7 +1167,7 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 			// account 4 cannot change the first registrar's identity since it's owned by 3.
-			assert_noop!(Identity::set_account_id(Origin::signed(4), 0, 3), "invalid index");
+			assert_noop!(Identity::set_account_id(Origin::signed(4), 0, 3), Error::<Test>::InvalidIndex);
 			// account 3 can, because that's the registrar's current account.
 			assert_ok!(Identity::set_account_id(Origin::signed(3), 0, 4));
 			// account 4 can now, because that's their new ID.

substrate/frame/im-online/src/lib.rs

@@ -89,7 +89,7 @@ use sp_staking::{
 	offence::{ReportOffence, Offence, Kind},
 };
 use frame_support::{
-	decl_module, decl_event, decl_storage, print, Parameter, debug,
+	decl_module, decl_event, decl_storage, print, Parameter, debug, decl_error,
 	traits::Get,
 };
 use frame_system::{self as system, ensure_none};
@@ -243,9 +243,20 @@ decl_storage! {
 	}
 }
 
+decl_error! {
+	/// Error for the im-online module.
+	pub enum Error for Module<T: Trait> {
+		/// Non existent public key.
+		InvalidKey,
+		/// Duplicated heartbeat.
+		DuplicatedHeartbeat,
+	}
+}
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error<T>;
+
 		fn deposit_event() = default;
 
 		fn heartbeat(
@@ -274,9 +285,9 @@ decl_module! {
 					&network_state
 				);
 			} else if exists {
-				Err("Duplicated heartbeat.")?
+				Err(Error::<T>::DuplicatedHeartbeat)?
 			} else {
-				Err("Non existent public key.")?
+				Err(Error::<T>::InvalidKey)?
 			}
 		}
 

substrate/frame/nicks/src/lib.rs

@@ -383,7 +383,10 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_noop!(Nicks::clear_name(Origin::signed(1)), Error::<Test>::Unnamed);
 
-			assert_noop!(Nicks::set_name(Origin::signed(3), b"Dave".to_vec()), "not enough free funds");
+			assert_noop!(
+				Nicks::set_name(Origin::signed(3), b"Dave".to_vec()),
+				pallet_balances::Error::<Test, _>::InsufficientBalance
+			);
 
 			assert_noop!(Nicks::set_name(Origin::signed(1), b"Ga".to_vec()), Error::<Test>::TooShort);
 			assert_noop!(

substrate/frame/scored-pool/src/tests.rs

@@ -41,7 +41,7 @@ fn submit_candidacy_must_not_work() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
 			ScoredPool::submit_candidacy(Origin::signed(99)),
-			"not enough free funds"
+			pallet_balances::Error::<Test, _>::InsufficientBalance,
 		);
 		assert_noop!(
 			ScoredPool::submit_candidacy(Origin::signed(40)),