diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 1b89ae2b16a64dd2704f9cac05abfd9413312b73..8d37ce0f5a5bddc422bcb66b0d7770a823d48614 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/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, diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index 3bbe01149750474ea4906dc83fd5c466911e37bc..61d1dc285e4abb2a5b3926baf3afb2e0c9248e53 100644 --- a/substrate/frame/democracy/src/lib.rs +++ b/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); }); } diff --git a/substrate/frame/generic-asset/src/lib.rs b/substrate/frame/generic-asset/src/lib.rs index de63ba2f1244c404ee88b9c1b488e39c153395d8..d332d63c4ec857a6f0cc6d53efa1033e0d98c83a 100644 --- a/substrate/frame/generic-asset/src/lib.rs +++ b/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!(asset_id < Self::next_asset_id(), "Asset id not available."); + ensure!(!<TotalIssuance<T>>::exists(&asset_id), Error::<T>::IdAlreadyTaken); + 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)) diff --git a/substrate/frame/generic-asset/src/tests.rs b/substrate/frame/generic-asset/src/tests.rs index 1f9f458b2ccece72db954f4d89ceb9d53433c028..20647cc6f22fccb8c560a1f2b69ec067a9524335 100644 --- a/substrate/frame/generic-asset/src/tests.rs +++ b/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, ); }); } diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs index 095739665823cb7b60c0a2f307efbe13888976fa..3fda978b135a39750541e370ac561dfc35874487 100644 --- a/substrate/frame/identity/src/lib.rs +++ b/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!(subs.len() <= T::MaximumSubAccounts::get() as usize, "too many subs"); + ensure!(<IdentityOf<T>>::exists(&sender), Error::<T>::NotFound); + 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")?; - ensure!(max_fee >= registrar.fee, "fee changed"); - let mut id = <IdentityOf<T>>::get(&sender).ok_or("no identity")?; + .ok_or(Error::<T>::EmptyIndex)?; + ensure!(max_fee >= registrar.fee, Error::<T>::FeeChanged); + 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(®_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(®_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")?; - let mut id = <IdentityOf<T>>::get(&target).ok_or("invalid target")?; + .ok_or(Error::<T>::InvalidIndex)?; + let mut id = <IdentityOf<T>>::get(&target).ok_or(Error::<T>::InvalidTarget)?; let item = (reg_index, judgement); match id.judgements.binary_search_by_key(®_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. diff --git a/substrate/frame/im-online/src/lib.rs b/substrate/frame/im-online/src/lib.rs index d7dfb1c67351988dfc6f08dabea1248e65363be5..b681f7e4bdd222b69ae6eff4d7fb803a58548ec2 100644 --- a/substrate/frame/im-online/src/lib.rs +++ b/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)? } } diff --git a/substrate/frame/nicks/src/lib.rs b/substrate/frame/nicks/src/lib.rs index f407c4aad0d75fc4e3bd6ddf702b89ec8ba50eab..997bf743928493b7b1fc81b4a0a1d987319b4bef 100644 --- a/substrate/frame/nicks/src/lib.rs +++ b/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!( diff --git a/substrate/frame/scored-pool/src/tests.rs b/substrate/frame/scored-pool/src/tests.rs index 6b6649f73ccd786a76ac1c8dd7f48e8af802382a..1cecc7b3098e46cc84d797851275aaf9b8337617 100644 --- a/substrate/frame/scored-pool/src/tests.rs +++ b/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)),