From 2c2e0d772dfbc0563586a9b93bf2a7348e06e06b Mon Sep 17 00:00:00 2001 From: Stanislav Tkach Date: Sat, 21 Dec 2019 16:10:29 +0200 Subject: [PATCH] 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 | 8 +- substrate/frame/democracy/src/lib.rs | 8 +- substrate/frame/generic-asset/src/lib.rs | 72 +++++++++++----- substrate/frame/generic-asset/src/tests.rs | 22 +++-- substrate/frame/identity/src/lib.rs | 96 ++++++++++++++-------- substrate/frame/im-online/src/lib.rs | 17 +++- substrate/frame/nicks/src/lib.rs | 5 +- substrate/frame/scored-pool/src/tests.rs | 2 +- 8 files changed, 153 insertions(+), 77 deletions(-) diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 1b89ae2b16..8d37ce0f5a 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::::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::::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::::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 >::exists(who) { - Err("A vesting schedule already exists for this account.")? + Err(Error::::ExistingVestingSchedule)? } let vesting_schedule = VestingSchedule { locked, diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index 3bbe011497..61d1dc285e 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::::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::::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::::InsufficientBalance); }); } diff --git a/substrate/frame/generic-asset/src/lib.rs b/substrate/frame/generic-asset/src/lib.rs index de63ba2f12..d332d63c4e 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 Encode for PermissionVersions { impl codec::EncodeLike for PermissionVersions {} impl Decode for PermissionVersions { - fn decode(input: &mut I) -> core::result::Result { + fn decode(input: &mut I) -> core::result::Result { let version = PermissionVersionNumber::decode(input)?; Ok( match version { @@ -320,8 +320,42 @@ impl Into> for PermissionLatest { + /// 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 for enum Call where origin: T::Origin { + type Error = Error; + fn deposit_event() = default; /// Create a new kind of asset. @@ -332,7 +366,7 @@ decl_module! { let permissions: PermissionVersions = 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::::NoIdAvailable)?; >::put(next_id); >::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::::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::::NoUpdatePermission)? } } @@ -384,9 +418,9 @@ decl_module! { let original_free_balance = Self::free_balance(&asset_id, &to); let current_total_issuance = >::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::::TotalMintingOverflow)?; let value = original_free_balance.checked_add(&amount) - .ok_or_else(|| "free balance got overflow after minting.")?; + .ok_or(Error::::FreeMintingOverflow)?; >::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::::NoMintPermission)? } } @@ -412,9 +446,9 @@ decl_module! { let current_total_issuance = >::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::::TotalBurningUnderflow)?; let value = original_free_balance.checked_sub(&amount) - .ok_or_else(|| "free_balance got underflow after burning")?; + .ok_or(Error::::FreeBurningUnderflow)?; >::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::::NoBurnPermission)? } } @@ -545,14 +579,14 @@ impl Module { options: AssetOptions, ) -> dispatch::DispatchResult { let asset_id = if let Some(asset_id) = asset_id { - ensure!(!>::exists(&asset_id), "Asset id already taken."); - ensure!(asset_id < Self::next_asset_id(), "Asset id not available."); + ensure!(!>::exists(&asset_id), Error::::IdAlreadyTaken); + ensure!(asset_id < Self::next_asset_id(), Error::::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::::NoIdAvailable)?; >::put(next_id); asset_id }; @@ -579,7 +613,7 @@ impl Module { ) -> 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::::InsufficientBalance)?; Self::ensure_can_withdraw(asset_id, from, amount, WithdrawReason::Transfer.into(), new_balance)?; if from != to { @@ -618,7 +652,7 @@ impl Module { 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::::InsufficientBalance)? } let new_reserve_balance = original_reserve_balance + amount; Self::set_reserved_balance(asset_id, who, new_reserve_balance); @@ -767,7 +801,7 @@ impl Module { { Ok(()) } else { - Err("account liquidity restrictions prevent withdrawal")? + Err(Error::::LiquidityRestrictions)? } } @@ -1155,7 +1189,7 @@ where ) -> result::Result { let new_balance = Self::free_balance(who) .checked_sub(&value) - .ok_or_else(|| "account has too few funds")?; + .ok_or(Error::::InsufficientBalance)?; Self::ensure_can_withdraw(who, value, reasons, new_balance)?; >::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 1f9f458b2c..20647cc6f2 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::::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::::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::::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::::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::::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::::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::::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::::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::::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::::IdAlreadyTaken, ); }); } diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs index 0957396658..3fda978b13 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 { + /// 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 for enum Call where origin: T::Origin { + type Error = Error; + fn deposit_event() = default; /// Add a registrar to the system. @@ -497,8 +527,8 @@ decl_module! { /// # fn set_subs(origin, subs: Vec<(T::AccountId, Data)>) { let sender = ensure_signed(origin)?; - ensure!(>::exists(&sender), "not found"); - ensure!(subs.len() <= T::MaximumSubAccounts::get() as usize, "too many subs"); + ensure!(>::exists(&sender), Error::::NotFound); + ensure!(subs.len() <= T::MaximumSubAccounts::get() as usize, Error::::TooManySubAccounts); let (old_deposit, old_ids) = >::get(&sender); let new_deposit = T::SubAccountDeposit::get() * >::from(subs.len() as u32); @@ -545,7 +575,7 @@ decl_module! { let sender = ensure_signed(origin)?; let (subs_deposit, sub_ids) = >::take(&sender); - let deposit = >::take(&sender).ok_or("not named")?.total_deposit() + let deposit = >::take(&sender).ok_or(Error::::NotNamed)?.total_deposit() + subs_deposit; for sub in sub_ids.iter() { >::remove(sub); @@ -587,14 +617,14 @@ decl_module! { let sender = ensure_signed(origin)?; let registrars = >::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 = >::get(&sender).ok_or("no identity")?; + .ok_or(Error::::EmptyIndex)?; + ensure!(max_fee >= registrar.fee, Error::::FeeChanged); + let mut id = >::get(&sender).ok_or(Error::::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::::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 = >::get(&sender).ok_or("no identity")?; + let mut id = >::get(&sender).ok_or(Error::::NoIdentity)?; let pos = id.judgements.binary_search_by_key(®_index, |x| x.0) - .map_err(|_| "not found")?; + .map_err(|_| Error::::NotFound)?; let fee = if let Judgement::FeePaid(fee) = id.judgements.remove(pos).1 { fee } else { - Err("judgement given")? + Err(Error::::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::::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::::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::::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::::InvalidJudgement); >::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 = >::get(&target).ok_or("invalid target")?; + .ok_or(Error::::InvalidIndex)?; + let mut id = >::get(&target).ok_or(Error::::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) = >::take(&target); - let deposit = >::take(&target).ok_or("not named")?.total_deposit() + let deposit = >::take(&target).ok_or(Error::::NotNamed)?.total_deposit() + subs_deposit; for sub in sub_ids.iter() { >::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::::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::::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::::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::::InvalidIndex ); assert_noop!( Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::FeePaid(1)), - "invalid judgement" + Error::::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::::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::::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::::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::::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::::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::::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::::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::::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::::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::::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 d7dfb1c673..b681f7e4bd 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 { + /// Non existent public key. + InvalidKey, + /// Duplicated heartbeat. + DuplicatedHeartbeat, + } +} decl_module! { pub struct Module for enum Call where origin: T::Origin { + type Error = Error; + fn deposit_event() = default; fn heartbeat( @@ -274,9 +285,9 @@ decl_module! { &network_state ); } else if exists { - Err("Duplicated heartbeat.")? + Err(Error::::DuplicatedHeartbeat)? } else { - Err("Non existent public key.")? + Err(Error::::InvalidKey)? } } diff --git a/substrate/frame/nicks/src/lib.rs b/substrate/frame/nicks/src/lib.rs index f407c4aad0..997bf74392 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::::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::::InsufficientBalance + ); assert_noop!(Nicks::set_name(Origin::signed(1), b"Ga".to_vec()), Error::::TooShort); assert_noop!( diff --git a/substrate/frame/scored-pool/src/tests.rs b/substrate/frame/scored-pool/src/tests.rs index 6b6649f73c..1cecc7b309 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::::InsufficientBalance, ); assert_noop!( ScoredPool::submit_candidacy(Origin::signed(40)),