More decl_error! migrations (#4427)

* Update assets to `decl_error`

* Update aura to `decl_error`

* Update authority discovery to `decl_error`

* Update collective to `decl_error`

* Update evm to `decl_error!`

* Fix error with replace

* Revert "Update authority discovery to `decl_error`"

This reverts commit 26e8f3c56656963d847e984c6f2c8e1f88014899.

* Revert "Update aura to `decl_error`"

This reverts commit 8f17c44ca8375a4a755710aaab7ad4d9522c4376.

* Update democracy to `decl_error`

* Update finality-tracker to `decl_error`

* Update grandpa to `decl_error`

* `assert` to `ensure` in dispatchable function

substrate/frame/assets/src/lib.rs

@@ -84,7 +84,8 @@
 //! ### Simple Code Snippet
 //!
 //! ```rust,ignore
-//! use frame_support::{decl_module, dispatch};
+//! use pallet_assets as assets;
+//! use frame_support::{decl_module, dispatch, ensure};
 //! use frame_system::{self as system, ensure_signed};
 //!
 //! pub trait Trait: assets::Trait { }
@@ -92,14 +93,15 @@
 //! decl_module! {
 //! 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
 //! 		pub fn issue_token_airdrop(origin) -> dispatch::Result {
+//! 			let sender = ensure_signed(origin).map_err(|e| e.as_str())?;
+//!
 //! 			const ACCOUNT_ALICE: u64 = 1;
 //! 			const ACCOUNT_BOB: u64 = 2;
-//! 			const COUNT_AIRDROP_RECIPIENTS = 2;
+//! 			const COUNT_AIRDROP_RECIPIENTS: u64 = 2;
 //! 			const TOKENS_FIXED_SUPPLY: u64 = 100;
 //!
 //! 			ensure!(!COUNT_AIRDROP_RECIPIENTS.is_zero(), "Divide by zero error.");
 //!
-//! 			let sender = ensure_signed(origin)?;
 //! 			let asset_id = Self::next_asset_id();
 //!
 //! 			<NextAssetId<T>>::mutate(|asset_id| *asset_id += 1);
@@ -130,8 +132,8 @@
 // Ensure we're `no_std` when compiling for Wasm.
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use frame_support::{Parameter, decl_module, decl_event, decl_storage, ensure};
+use frame_support::{Parameter, decl_module, decl_event, decl_storage, decl_error, ensure};
-use sp_runtime::traits::{Member, SimpleArithmetic, Zero, StaticLookup};
+use sp_runtime::traits::{Member, SimpleArithmetic, Zero, StaticLookup, ModuleDispatchError};
 use frame_system::{self as system, ensure_signed};
 use sp_runtime::traits::One;
 
@@ -149,12 +151,14 @@ pub trait Trait: frame_system::Trait {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error;
+
 		fn deposit_event() = default;
 		/// Issue a new class of fungible assets. There are, and will only ever be, `total`
 		/// such assets and they'll all belong to the `origin` initially. It will have an
 		/// identifier `AssetId` instance: this will be specified in the `Issued` event.
 		fn issue(origin, #[compact] total: T::Balance) {
-			let origin = ensure_signed(origin)?;
+			let origin = ensure_signed(origin).map_err(|e| e.as_str())?;
 
 			let id = Self::next_asset_id();
 			<NextAssetId<T>>::mutate(|id| *id += One::one());
@@ -171,12 +175,12 @@ decl_module! {
 			target: <T::Lookup as StaticLookup>::Source,
 			#[compact] amount: T::Balance
 		) {
-			let origin = ensure_signed(origin)?;
+			let origin = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let origin_account = (id, origin.clone());
 			let origin_balance = <Balances<T>>::get(&origin_account);
 			let target = T::Lookup::lookup(target)?;
-			ensure!(!amount.is_zero(), "transfer amount should be non-zero");
+			ensure!(!amount.is_zero(), Error::AmountZero);
-			ensure!(origin_balance >= amount, "origin account balance must be greater than or equal to the transfer amount");
+			ensure!(origin_balance >= amount, Error::BalanceLow);
 
 			Self::deposit_event(RawEvent::Transferred(id, origin, target.clone(), amount));
 			<Balances<T>>::insert(origin_account, origin_balance - amount);
@@ -185,9 +189,9 @@ decl_module! {
 
 		/// Destroy any assets of `id` owned by `origin`.
 		fn destroy(origin, #[compact] id: T::AssetId) {
-			let origin = ensure_signed(origin)?;
+			let origin = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let balance = <Balances<T>>::take((id, &origin));
-			ensure!(!balance.is_zero(), "origin balance should be non-zero");
+			ensure!(!balance.is_zero(), Error::BalanceZero);
 
 			<TotalSupply<T>>::mutate(id, |total_supply| *total_supply -= balance);
 			Self::deposit_event(RawEvent::Destroyed(id, origin, balance));
@@ -195,7 +199,7 @@ decl_module! {
 	}
 }
 
-decl_event!(
+decl_event! {
 	pub enum Event<T> where
 		<T as frame_system::Trait>::AccountId,
 		<T as Trait>::Balance,
@@ -208,7 +212,19 @@ decl_event!(
 		/// Some assets were destroyed.
 		Destroyed(AssetId, AccountId, Balance),
 	}
-);
+}
+
+decl_error! {
+	pub enum Error {
+		/// Transfer amount should be non-zero
+		AmountZero,
+		/// Account balance must be greater than or equal to the transfer amount
+		BalanceLow,
+		/// Balance should be non-zero
+		BalanceZero,
+	}
+
+}
 
 decl_storage! {
 	trait Store for Module<T: Trait> as Assets {
@@ -337,7 +353,7 @@ mod tests {
 			assert_eq!(Assets::balance(0, 2), 50);
 			assert_ok!(Assets::destroy(Origin::signed(1), 0));
 			assert_eq!(Assets::balance(0, 1), 0);
-			assert_noop!(Assets::transfer(Origin::signed(1), 0, 1, 50), "origin account balance must be greater than or equal to the transfer amount");
+			assert_noop!(Assets::transfer(Origin::signed(1), 0, 1, 50), Error::BalanceLow);
 		});
 	}
 
@@ -346,7 +362,7 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Assets::issue(Origin::signed(1), 100));
 			assert_eq!(Assets::balance(0, 1), 100);
-			assert_noop!(Assets::transfer(Origin::signed(1), 0, 2, 0), "transfer amount should be non-zero");
+			assert_noop!(Assets::transfer(Origin::signed(1), 0, 2, 0), Error::AmountZero);
 		});
 	}
 
@@ -355,7 +371,7 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Assets::issue(Origin::signed(1), 100));
 			assert_eq!(Assets::balance(0, 1), 100);
-			assert_noop!(Assets::transfer(Origin::signed(1), 0, 2, 101), "origin account balance must be greater than or equal to the transfer amount");
+			assert_noop!(Assets::transfer(Origin::signed(1), 0, 2, 101), Error::BalanceLow);
 		});
 	}
 
@@ -373,7 +389,7 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			assert_ok!(Assets::issue(Origin::signed(1), 100));
 			assert_eq!(Assets::balance(0, 2), 0);
-			assert_noop!(Assets::destroy(Origin::signed(2), 0), "origin balance should be non-zero");
+			assert_noop!(Assets::destroy(Origin::signed(2), 0), Error::BalanceZero);
 		});
 	}
 }

substrate/frame/collective/src/lib.rs

@@ -26,12 +26,12 @@
 use sp_std::{prelude::*, result};
 use sp_core::u32_trait::Value as U32;
 use sp_runtime::RuntimeDebug;
-use sp_runtime::traits::{Hash, EnsureOrigin};
+use sp_runtime::traits::{Hash, EnsureOrigin, ModuleDispatchError};
 use frame_support::weights::SimpleDispatchInfo;
 use frame_support::{
 	dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode},
 	traits::{ChangeMembers, InitializeMembers}, decl_module, decl_event,
-	decl_storage, ensure,
+	decl_storage, decl_error, ensure,
 };
 use frame_system::{self as system, ensure_signed, ensure_root};
 
@@ -102,7 +102,7 @@ decl_storage! {
 	}
 }
 
-decl_event!(
+decl_event! {
 	pub enum Event<T, I=DefaultInstance> where
 		<T as frame_system::Trait>::Hash,
 		<T as frame_system::Trait>::AccountId,
@@ -122,13 +122,32 @@ decl_event!(
 		/// A single member did some action; `bool` is true if returned without error.
 		MemberExecuted(Hash, bool),
 	}
-);
+}
+
+decl_error! {
+	pub enum Error {
+		/// Account is not a member
+		NotMember,
+		/// Duplicate proposals not allowed
+		DuplicateProposal,
+		/// Proposal must exist
+		ProposalMissing,
+		/// Mismatched index
+		WrongIndex,
+		/// Duplicate vote ignored
+		DuplicateVote,
+		/// Members are already initialized!
+		AlreadyInitialized,
+	}
+}
 
 // Note: this module is not benchmarked. The weights are obtained based on the similarity of the
 // executed logic with other democracy function. Note that councillor operations are assigned to the
 // operational class.
 decl_module! {
 	pub struct Module<T: Trait<I>, I: Instance=DefaultInstance> for enum Call where origin: <T as frame_system::Trait>::Origin {
+		type Error = Error;
+
 		fn deposit_event() = default;
 
 		/// Set the collective's membership manually to `new_members`. Be nice to the chain and
@@ -137,7 +156,7 @@ decl_module! {
 		/// Requires root origin.
 		#[weight = SimpleDispatchInfo::FixedOperational(100_000)]
 		fn set_members(origin, new_members: Vec<T::AccountId>) {
-			ensure_root(origin)?;
+			ensure_root(origin).map_err(|e| e.as_str())?;
 			let mut new_members = new_members;
 			new_members.sort();
 			<Members<T, I>>::mutate(|m| {
@@ -151,8 +170,8 @@ decl_module! {
 		/// Origin must be a member of the collective.
 		#[weight = SimpleDispatchInfo::FixedOperational(100_000)]
 		fn execute(origin, proposal: Box<<T as Trait<I>>::Proposal>) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(Self::is_member(&who), "proposer not a member");
+			ensure!(Self::is_member(&who), Error::NotMember);
 
 			let proposal_hash = T::Hashing::hash_of(&proposal);
 			let ok = proposal.dispatch(RawOrigin::Member(who).into()).is_ok();
@@ -165,12 +184,12 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedOperational(5_000_000)]
 		fn propose(origin, #[compact] threshold: MemberCount, proposal: Box<<T as Trait<I>>::Proposal>) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(Self::is_member(&who), "proposer not a member");
+			ensure!(Self::is_member(&who), Error::NotMember);
 
 			let proposal_hash = T::Hashing::hash_of(&proposal);
 
-			ensure!(!<ProposalOf<T, I>>::exists(proposal_hash), "duplicate proposals not allowed");
+			ensure!(!<ProposalOf<T, I>>::exists(proposal_hash), Error::DuplicateProposal);
 
 			if threshold < 2 {
 				let seats = Self::members().len() as MemberCount;
@@ -194,11 +213,11 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedOperational(200_000)]
 		fn vote(origin, proposal: T::Hash, #[compact] index: ProposalIndex, approve: bool) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(Self::is_member(&who), "voter not a member");
+			ensure!(Self::is_member(&who), Error::NotMember);
 
-			let mut voting = Self::voting(&proposal).ok_or("proposal must exist")?;
+			let mut voting = Self::voting(&proposal).ok_or(Error::ProposalMissing)?;
-			ensure!(voting.index == index, "mismatched index");
+			ensure!(voting.index == index, Error::WrongIndex);
 
 			let position_yes = voting.ayes.iter().position(|a| a == &who);
 			let position_no = voting.nays.iter().position(|a| a == &who);
@@ -207,7 +226,7 @@ decl_module! {
 				if position_yes.is_none() {
 					voting.ayes.push(who.clone());
 				} else {
-					return Err("duplicate vote ignored")
+					return Err(Error::DuplicateVote)
 				}
 				if let Some(pos) = position_no {
 					voting.nays.swap_remove(pos);
@@ -216,7 +235,7 @@ decl_module! {
 				if position_no.is_none() {
 					voting.nays.push(who.clone());
 				} else {
-					return Err("duplicate vote ignored")
+					return Err(Error::DuplicateVote)
 				}
 				if let Some(pos) = position_yes {
 					voting.ayes.swap_remove(pos);
@@ -565,7 +584,7 @@ mod tests {
 			let proposal = make_proposal(42);
 			assert_noop!(
 				Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone())),
-				"proposer not a member"
+				Error::NotMember
 			);
 		});
 	}
@@ -579,7 +598,7 @@ mod tests {
 			assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone())));
 			assert_noop!(
 				Collective::vote(Origin::signed(42), hash.clone(), 0, true),
-				"voter not a member",
+				Error::NotMember,
 			);
 		});
 	}
@@ -593,7 +612,7 @@ mod tests {
 			assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone())));
 			assert_noop!(
 				Collective::vote(Origin::signed(2), hash.clone(), 1, true),
-				"mismatched index",
+				Error::WrongIndex,
 			);
 		});
 	}
@@ -611,7 +630,7 @@ mod tests {
 			);
 			assert_noop!(
 				Collective::vote(Origin::signed(1), hash.clone(), 0, true),
-				"duplicate vote ignored",
+				Error::DuplicateVote,
 			);
 			assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false));
 			assert_eq!(
@@ -620,7 +639,7 @@ mod tests {
 			);
 			assert_noop!(
 				Collective::vote(Origin::signed(1), hash.clone(), 0, false),
-				"duplicate vote ignored",
+				Error::DuplicateVote,
 			);
 
 			assert_eq!(System::events(), vec![

substrate/frame/democracy/src/lib.rs

@@ -22,12 +22,14 @@ use sp_std::prelude::*;
 use sp_std::{result, convert::TryFrom};
 use sp_runtime::{
 	RuntimeDebug,
-	traits::{Zero, Bounded, CheckedMul, CheckedDiv, EnsureOrigin, Hash, Dispatchable, Saturating},
+	traits::{
+		Zero, Bounded, CheckedMul, CheckedDiv, EnsureOrigin, Hash,
+		Dispatchable, Saturating, ModuleDispatchError,
+	},
 };
-use codec::{Ref, Encode, Decode, Input, Output, Error};
+use codec::{Ref, Encode, Decode, Input, Output};
 use frame_support::{
-	decl_module, decl_storage, decl_event, ensure,
+	decl_module, decl_storage, decl_event, decl_error, ensure,
-	dispatch,
 	Parameter,
 	weights::SimpleDispatchInfo,
 	traits::{
@@ -163,12 +165,12 @@ impl Encode for Vote {
 impl codec::EncodeLike for Vote {}
 
 impl Decode for Vote {
-	fn decode<I: Input>(input: &mut I) -> core::result::Result<Self, Error> {
+	fn decode<I: Input>(input: &mut I) -> core::result::Result<Self, codec::Error> {
 		let b = input.read_byte()?;
 		Ok(Vote {
 			aye: (b & 0b1000_0000) == 0b1000_0000,
 			conviction: Conviction::try_from(b & 0b0111_1111)
-				.map_err(|_| Error::from("Invalid conviction"))?,
+				.map_err(|_| codec::Error::from("Invalid conviction"))?,
 		})
 	}
 }
@@ -320,7 +322,7 @@ decl_storage! {
 	}
 }
 
-decl_event!(
+decl_event! {
 	pub enum Event<T> where
 		Balance = BalanceOf<T>,
 		<T as frame_system::Trait>::AccountId,
@@ -360,10 +362,60 @@ decl_event!(
 		/// A registered preimage was removed and the deposit collected by the reaper (last item).
 		PreimageReaped(Hash, AccountId, Balance, AccountId),
 	}
-);
+}
+
+decl_error! {
+	pub enum Error {
+		/// Value too low
+		ValueLow,
+		/// Proposal does not exist
+		ProposalMissing,
+		/// Not a proxy
+		NotProxy,
+		/// Unknown index
+		BadIndex,
+		/// Cannot cancel the same proposal twice
+		AlreadyCanceled,
+		/// Proposal already made
+		DuplicateProposal,
+		/// Proposal still blacklisted
+		ProposalBlacklisted,
+		/// Next external proposal not simple majority
+		NotSimpleMajority,
+		/// Invalid hash
+		InvalidHash,
+		/// No external proposal
+		NoProposal,
+		/// Identity may not veto a proposal twice
+		AlreadyVetoed,
+		/// Already a proxy
+		AlreadyProxy,
+		/// Wrong proxy
+		WrongProxy,
+		/// Not delegated
+		NotDelegated,
+		/// Preimage already noted
+		DuplicatePreimage,
+		/// Not imminent
+		NotImminent,
+		/// Too early
+		Early,
+		/// Imminent
+		Imminent,
+		/// Preimage not found
+		PreimageMissing,
+		/// Vote given for invalid referendum
+		ReferendumInvalid,
+		/// Invalid preimage
+		PreimageInvalid,
+		/// No proposals waiting
+		NoneWaiting,
+	}
+}
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error;
 		/// The minimum period of locking and the period between a proposal being approved and enacted.
 		///
 		/// It should generally be a little more than the unstake period to ensure that
@@ -402,10 +454,9 @@ decl_module! {
 			proposal_hash: T::Hash,
 			#[compact] value: BalanceOf<T>
 		) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(value >= T::MinimumDeposit::get(), "value too low");
+			ensure!(value >= T::MinimumDeposit::get(), Error::ValueLow);
-			T::Currency::reserve(&who, value)
+			T::Currency::reserve(&who, value)?;
-				.map_err(|_| "proposer's balance too low")?;
 
 			let index = Self::public_prop_count();
 			PublicPropCount::put(index + 1);
@@ -425,11 +476,10 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(5_000_000)]
 		fn second(origin, #[compact] proposal: PropIndex) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let mut deposit = Self::deposit_of(proposal)
-				.ok_or("can only second an existing proposal")?;
+				.ok_or(Error::ProposalMissing)?;
-			T::Currency::reserve(&who, deposit.0)
+			T::Currency::reserve(&who, deposit.0)?;
-				.map_err(|_| "seconder's balance too low")?;
 			deposit.1.push(who);
 			<DepositOf<T>>::insert(proposal, deposit);
 		}
@@ -445,8 +495,8 @@ decl_module! {
 		fn vote(origin,
 			#[compact] ref_index: ReferendumIndex,
 			vote: Vote
-		) -> dispatch::Result {
+		) -> Result<(), Error> {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 			Self::do_vote(who, ref_index, vote)
 		}
 
@@ -461,8 +511,10 @@ decl_module! {
 		fn proxy_vote(origin,
 			#[compact] ref_index: ReferendumIndex,
 			vote: Vote
-		) -> dispatch::Result {
+		) -> Result<(), Error> {
-			let who = Self::proxy(ensure_signed(origin)?).ok_or("not a proxy")?;
+			let who = Self::proxy(
+				ensure_signed(origin).map_err(|e| e.as_str())?
+			).ok_or(Error::NotProxy)?;
 			Self::do_vote(who, ref_index, vote)
 		}
 
@@ -470,11 +522,11 @@ decl_module! {
 		/// referendum.
 		#[weight = SimpleDispatchInfo::FixedOperational(500_000)]
 		fn emergency_cancel(origin, ref_index: ReferendumIndex) {
-			T::CancellationOrigin::ensure_origin(origin)?;
+			T::CancellationOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
 
-			let info = Self::referendum_info(ref_index).ok_or("unknown index")?;
+			let info = Self::referendum_info(ref_index).ok_or(Error::BadIndex)?;
 			let h = info.proposal_hash;
-			ensure!(!<Cancellations<T>>::exists(h), "cannot cancel the same proposal twice");
+			ensure!(!<Cancellations<T>>::exists(h), Error::AlreadyCanceled);
 
 			<Cancellations<T>>::insert(h, true);
 			Self::clear_referendum(ref_index);
@@ -484,10 +536,10 @@ decl_module! {
 		/// referendum.
 		#[weight = SimpleDispatchInfo::FixedNormal(5_000_000)]
 		fn external_propose(origin, proposal_hash: T::Hash) {
-			T::ExternalOrigin::ensure_origin(origin)?;
+			T::ExternalOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
-			ensure!(!<NextExternal<T>>::exists(), "proposal already made");
+			ensure!(!<NextExternal<T>>::exists(), Error::DuplicateProposal);
 			if let Some((until, _)) = <Blacklist<T>>::get(proposal_hash) {
-				ensure!(<frame_system::Module<T>>::block_number() >= until, "proposal still blacklisted");
+				ensure!(<frame_system::Module<T>>::block_number() >= until, Error::ProposalBlacklisted);
 			}
 			<NextExternal<T>>::put((proposal_hash, VoteThreshold::SuperMajorityApprove));
 		}
@@ -499,7 +551,7 @@ decl_module! {
 		/// pre-scheduled `external_propose` call.
 		#[weight = SimpleDispatchInfo::FixedNormal(5_000_000)]
 		fn external_propose_majority(origin, proposal_hash: T::Hash) {
-			T::ExternalMajorityOrigin::ensure_origin(origin)?;
+			T::ExternalMajorityOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
 			<NextExternal<T>>::put((proposal_hash, VoteThreshold::SimpleMajority));
 		}
 
@@ -510,7 +562,7 @@ decl_module! {
 		/// pre-scheduled `external_propose` call.
 		#[weight = SimpleDispatchInfo::FixedNormal(5_000_000)]
 		fn external_propose_default(origin, proposal_hash: T::Hash) {
-			T::ExternalDefaultOrigin::ensure_origin(origin)?;
+			T::ExternalDefaultOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
 			<NextExternal<T>>::put((proposal_hash, VoteThreshold::SuperMajorityAgainst));
 		}
 
@@ -529,13 +581,13 @@ decl_module! {
 			voting_period: T::BlockNumber,
 			delay: T::BlockNumber
 		) {
-			T::FastTrackOrigin::ensure_origin(origin)?;
+			T::FastTrackOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
-			let (e_proposal_hash, threshold) = <NextExternal<T>>::get().ok_or("no proposal made")?;
+			let (e_proposal_hash, threshold) = <NextExternal<T>>::get().ok_or(Error::ProposalMissing)?;
 			ensure!(
 				threshold != VoteThreshold::SuperMajorityApprove,
-				"next external proposal not simple majority"
+				Error::NotSimpleMajority
 			);
-			ensure!(proposal_hash == e_proposal_hash, "invalid hash");
+			ensure!(proposal_hash == e_proposal_hash, Error::InvalidHash);
 
 			<NextExternal<T>>::kill();
 			let now = <frame_system::Module<T>>::block_number();
@@ -547,19 +599,19 @@ decl_module! {
 		/// Veto and blacklist the external proposal hash.
 		#[weight = SimpleDispatchInfo::FixedNormal(200_000)]
 		fn veto_external(origin, proposal_hash: T::Hash) {
-			let who = T::VetoOrigin::ensure_origin(origin)?;
+			let who = T::VetoOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
 
 			if let Some((e_proposal_hash, _)) = <NextExternal<T>>::get() {
-				ensure!(proposal_hash == e_proposal_hash, "unknown proposal");
+				ensure!(proposal_hash == e_proposal_hash, Error::ProposalMissing);
 			} else {
-				Err("no external proposal")?;
+				Err(Error::NoProposal)?;
 			}
 
 			let mut existing_vetoers = <Blacklist<T>>::get(&proposal_hash)
 				.map(|pair| pair.1)
 				.unwrap_or_else(Vec::new);
 			let insert_position = existing_vetoers.binary_search(&who)
-				.err().ok_or("identity may not veto a proposal twice")?;
+				.err().ok_or(Error::AlreadyVetoed)?;
 
 			existing_vetoers.insert(insert_position, who.clone());
 			let until = <frame_system::Module<T>>::block_number() + T::CooloffPeriod::get();
@@ -572,24 +624,24 @@ decl_module! {
 		/// Remove a referendum.
 		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
 		fn cancel_referendum(origin, #[compact] ref_index: ReferendumIndex) {
-			ensure_root(origin)?;
+			ensure_root(origin).map_err(|e| e.as_str())?;
 			Self::clear_referendum(ref_index);
 		}
 
 		/// Cancel a proposal queued for enactment.
 		#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
 		fn cancel_queued(origin, which: ReferendumIndex) {
-			ensure_root(origin)?;
+			ensure_root(origin).map_err(|e| e.as_str())?;
 			let mut items = <DispatchQueue<T>>::get();
 			let original_len = items.len();
 			items.retain(|i| i.2 != which);
-			ensure!(items.len() < original_len, "proposal not found");
+			ensure!(items.len() < original_len, Error::ProposalMissing);
 			<DispatchQueue<T>>::put(items);
 		}
 
 		fn on_initialize(n: T::BlockNumber) {
 			if let Err(e) = Self::begin_block(n) {
-				sp_runtime::print(e);
+				sp_runtime::print(e.as_str());
 			}
 		}
 
@@ -600,8 +652,8 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(100_000)]
 		fn set_proxy(origin, proxy: T::AccountId) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(!<Proxy<T>>::exists(&proxy), "already a proxy");
+			ensure!(!<Proxy<T>>::exists(&proxy), Error::AlreadyProxy);
 			<Proxy<T>>::insert(proxy, who)
 		}
 
@@ -612,7 +664,7 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(100_000)]
 		fn resign_proxy(origin) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 			<Proxy<T>>::remove(who);
 		}
 
@@ -623,8 +675,8 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(100_000)]
 		fn remove_proxy(origin, proxy: T::AccountId) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(&Self::proxy(&proxy).ok_or("not a proxy")? == &who, "wrong proxy");
+			ensure!(&Self::proxy(&proxy).ok_or(Error::NotProxy)? == &who, Error::WrongProxy);
 			<Proxy<T>>::remove(proxy);
 		}
 
@@ -635,7 +687,7 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(500_000)]
 		pub fn delegate(origin, to: T::AccountId, conviction: Conviction) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 			<Delegations<T>>::insert(&who, (&to, conviction));
 			// Currency is locked indefinitely as long as it's delegated.
 			T::Currency::extend_lock(
@@ -655,8 +707,8 @@ decl_module! {
 		/// # </weight>
 		#[weight = SimpleDispatchInfo::FixedNormal(500_000)]
 		fn undelegate(origin) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
-			ensure!(<Delegations<T>>::exists(&who), "not delegated");
+			ensure!(<Delegations<T>>::exists(&who), Error::NotDelegated);
 			let (_, conviction) = <Delegations<T>>::take(&who);
 			// Indefinite lock is reduced to the maximum voting lock that could be possible.
 			let now = <frame_system::Module<T>>::block_number();
@@ -674,7 +726,7 @@ decl_module! {
 		/// Veto and blacklist the proposal hash. Must be from Root origin.
 		#[weight = SimpleDispatchInfo::FixedNormal(10_000)]
 		fn clear_public_proposals(origin) {
-			ensure_root(origin)?;
+			ensure_root(origin).map_err(|e| e.as_str())?;
 
 			<PublicProps<T>>::kill();
 		}
@@ -683,9 +735,9 @@ decl_module! {
 		/// in the dispatch queue but does require a deposit, returned once enacted.
 		#[weight = SimpleDispatchInfo::FixedNormal(100_000)]
 		fn note_preimage(origin, encoded_proposal: Vec<u8>) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
-			ensure!(!<Preimages<T>>::exists(&proposal_hash), "preimage already noted");
+			ensure!(!<Preimages<T>>::exists(&proposal_hash), Error::DuplicatePreimage);
 
 			let deposit = <BalanceOf<T>>::from(encoded_proposal.len() as u32)
 				.saturating_mul(T::PreimageByteDeposit::get());
@@ -701,11 +753,11 @@ decl_module! {
 		/// in the dispatch queue. No deposit is needed.
 		#[weight = SimpleDispatchInfo::FixedNormal(100_000)]
 		fn note_imminent_preimage(origin, encoded_proposal: Vec<u8>) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
-			ensure!(!<Preimages<T>>::exists(&proposal_hash), "preimage already noted");
+			ensure!(!<Preimages<T>>::exists(&proposal_hash), Error::DuplicatePreimage);
 			let queue = <DispatchQueue<T>>::get();
-			ensure!(queue.iter().any(|item| &item.1 == &proposal_hash), "not imminent");
+			ensure!(queue.iter().any(|item| &item.1 == &proposal_hash), Error::NotImminent);
 
 			let now = <frame_system::Module<T>>::block_number();
 			let free = <BalanceOf<T>>::zero();
@@ -721,16 +773,16 @@ decl_module! {
 		/// work an additional `EnactmentPeriod` later.
 		#[weight = SimpleDispatchInfo::FixedNormal(10_000)]
 		fn reap_preimage(origin, proposal_hash: T::Hash) {
-			let who = ensure_signed(origin)?;
+			let who = ensure_signed(origin).map_err(|e| e.as_str())?;
 
-			let (_, old, deposit, then) = <Preimages<T>>::get(&proposal_hash).ok_or("not found")?;
+			let (_, old, deposit, then) = <Preimages<T>>::get(&proposal_hash).ok_or(Error::PreimageMissing)?;
 			let now = <frame_system::Module<T>>::block_number();
 			let (voting, enactment) = (T::VotingPeriod::get(), T::EnactmentPeriod::get());
 			let additional = if who == old { Zero::zero() } else { enactment };
-			ensure!(now >= then + voting + additional, "too early");
+			ensure!(now >= then + voting + additional, Error::Early);
 
 			let queue = <DispatchQueue<T>>::get();
-			ensure!(!queue.iter().any(|item| &item.1 == &proposal_hash), "imminent");
+			ensure!(!queue.iter().any(|item| &item.1 == &proposal_hash), Error::Imminent);
 
 			let _ = T::Currency::repatriate_reserved(&old, &who, deposit);
 			<Preimages<T>>::remove(&proposal_hash);
@@ -879,8 +931,8 @@ impl<T: Trait> Module<T> {
 	// private.
 
 	/// Actually enact a vote, if legit.
-	fn do_vote(who: T::AccountId, ref_index: ReferendumIndex, vote: Vote) -> dispatch::Result {
+	fn do_vote(who: T::AccountId, ref_index: ReferendumIndex, vote: Vote) -> Result<(), Error> {
-		ensure!(Self::is_active_referendum(ref_index), "vote given for invalid referendum.");
+		ensure!(Self::is_active_referendum(ref_index), Error::ReferendumInvalid);
 		if !<VoteOf<T>>::exists((ref_index, &who)) {
 			<VotersFor<T>>::append_or_insert(ref_index, &[&who][..]);
 		}
@@ -921,7 +973,7 @@ impl<T: Trait> Module<T> {
 	}
 
 	/// Enact a proposal from a referendum.
-	fn enact_proposal(proposal_hash: T::Hash, index: ReferendumIndex) -> dispatch::Result {
+	fn enact_proposal(proposal_hash: T::Hash, index: ReferendumIndex) -> Result<(), Error> {
 		if let Some((encoded_proposal, who, amount, _)) = <Preimages<T>>::take(&proposal_hash) {
 			if let Ok(proposal) = T::Proposal::decode(&mut &encoded_proposal[..]) {
 				let _ = T::Currency::unreserve(&who, amount);
@@ -934,25 +986,25 @@ impl<T: Trait> Module<T> {
 			} else {
 				T::Slash::on_unbalanced(T::Currency::slash_reserved(&who, amount).0);
 				Self::deposit_event(RawEvent::PreimageInvalid(proposal_hash, index));
-				Err("invalid preimage")
+				Err(Error::PreimageInvalid)
 			}
 		} else {
 			Self::deposit_event(RawEvent::PreimageMissing(proposal_hash, index));
-			Err("missing preimage")
+			Err(Error::PreimageMissing)
 		}
 	}
 
 	/// Table the next waiting proposal for a vote.
-	fn launch_next(now: T::BlockNumber) -> dispatch::Result {
+	fn launch_next(now: T::BlockNumber) -> Result<(), Error> {
 		if LastTabledWasExternal::take() {
 			Self::launch_public(now).or_else(|_| Self::launch_external(now))
 		} else {
 			Self::launch_external(now).or_else(|_| Self::launch_public(now))
-		}.map_err(|_| "No proposals waiting")
+		}.map_err(|_| Error::NoneWaiting)
 	}
 
 	/// Table the waiting external proposal for a vote, if there is one.
-	fn launch_external(now: T::BlockNumber) -> dispatch::Result {
+	fn launch_external(now: T::BlockNumber) -> Result<(), Error> {
 		if let Some((proposal, threshold)) = <NextExternal<T>>::take() {
 			LastTabledWasExternal::put(true);
 			Self::deposit_event(RawEvent::ExternalTabled);
@@ -964,12 +1016,12 @@ impl<T: Trait> Module<T> {
 			);
 			Ok(())
 		} else {
-			Err("No external proposal waiting")
+			Err(Error::NoneWaiting)
 		}
 	}
 
 	/// Table the waiting public proposal with the highest backing for a vote.
-	fn launch_public(now: T::BlockNumber) -> dispatch::Result {
+	fn launch_public(now: T::BlockNumber) -> Result<(), Error> {
 		let mut public_props = Self::public_props();
 		if let Some((winner_index, _)) = public_props.iter()
 			.enumerate()
@@ -994,7 +1046,7 @@ impl<T: Trait> Module<T> {
 			}
 			Ok(())
 		} else {
-			Err("No public proposals waiting")
+			Err(Error::NoneWaiting)
 		}
 
 	}
@@ -1003,7 +1055,7 @@ impl<T: Trait> Module<T> {
 		now: T::BlockNumber,
 		index: ReferendumIndex,
 		info: ReferendumInfo<T::BlockNumber, T::Hash>
-	) -> dispatch::Result {
+	) -> Result<(), Error> {
 		let (approve, against, capital) = Self::tally(index);
 		let total_issuance = T::Currency::total_issuance();
 		let approved = info.threshold.approved(approve, against, capital, total_issuance);
@@ -1051,7 +1103,7 @@ impl<T: Trait> Module<T> {
 	}
 
 	/// Current era is ending; we should finish up any proposals.
-	fn begin_block(now: T::BlockNumber) -> dispatch::Result {
+	fn begin_block(now: T::BlockNumber) -> Result<(), Error> {
 		// pick out another public referendum if it's time.
 		if (now % T::LaunchPeriod::get()).is_zero() {
 			// Errors come from the queue being empty. we don't really care about that, and even if
@@ -1237,13 +1289,13 @@ mod tests {
 		let p = set_balance_proposal(value);
 		let h = BlakeTwo256::hash(&p[..]);
 		match Democracy::note_preimage(Origin::signed(6), p) {
-			Ok(_) | Err("preimage already noted") => (),
+			Ok(_) | Err(Error::DuplicatePreimage) => (),
 			Err(x) => panic!(x),
 		}
 		h
 	}
 
-	fn propose_set_balance(who: u64, value: u64, delay: u64) -> dispatch::Result {
+	fn propose_set_balance(who: u64, value: u64, delay: u64) -> Result<(), Error> {
 		Democracy::propose(
 			Origin::signed(who),
 			set_balance_proposal_hash(value),
@@ -1251,7 +1303,7 @@ mod tests {
 		)
 	}
 
-	fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> dispatch::Result {
+	fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> Result<(), Error> {
 		Democracy::propose(
 			Origin::signed(who),
 			set_balance_proposal_hash_and_note(value),
@@ -1297,7 +1349,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"
+				Error::Other("not enough free funds")
 			);
 			// fee of 1 is reasonable.
 			PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
@@ -1332,7 +1384,7 @@ mod tests {
 			next_block();
 			assert_noop!(
 				Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2)),
-				"too early"
+				Error::Early
 			);
 			next_block();
 			assert_ok!(Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2)));
@@ -1348,7 +1400,7 @@ mod tests {
 			System::set_block_number(1);
 			assert_noop!(
 				Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2)),
-				"not found"
+				Error::PreimageMissing
 			);
 
 			PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
@@ -1360,7 +1412,7 @@ mod tests {
 			next_block();
 			assert_noop!(
 				Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2)),
-				"too early"
+				Error::Early
 			);
 
 			next_block();
@@ -1387,7 +1439,7 @@ mod tests {
 
 			assert_noop!(
 				Democracy::note_imminent_preimage(Origin::signed(7), set_balance_proposal(2)),
-				"not imminent"
+				Error::NotImminent
 			);
 
 			next_block();
@@ -1411,7 +1463,7 @@ mod tests {
 			next_block();
 			next_block();
 			// now imminent.
-			assert_noop!(Democracy::reap_preimage(Origin::signed(6), h), "imminent");
+			assert_noop!(Democracy::reap_preimage(Origin::signed(6), h), Error::Imminent);
 		});
 	}
 
@@ -1540,7 +1592,7 @@ mod tests {
 			);
 			assert!(Democracy::referendum_info(r).is_some());
 
-			assert_noop!(Democracy::emergency_cancel(Origin::signed(3), r), "Invalid origin");
+			assert_noop!(Democracy::emergency_cancel(Origin::signed(3), r), "Invalid origin".into());
 			assert_ok!(Democracy::emergency_cancel(Origin::signed(4), r));
 			assert!(Democracy::referendum_info(r).is_none());
 
@@ -1553,7 +1605,7 @@ mod tests {
 				2
 			);
 			assert!(Democracy::referendum_info(r).is_some());
-			assert_noop!(Democracy::emergency_cancel(Origin::signed(4), r), "cannot cancel the same proposal twice");
+			assert_noop!(Democracy::emergency_cancel(Origin::signed(4), r), Error::AlreadyCanceled);
 		});
 	}
 
@@ -1575,14 +1627,14 @@ mod tests {
 			assert_noop!(Democracy::external_propose(
 				Origin::signed(2),
 				set_balance_proposal_hash(2),
-			), "proposal still blacklisted");
+			), Error::ProposalBlacklisted);
 
 			fast_forward_to(1);
 			// fails as we're still in cooloff period.
 			assert_noop!(Democracy::external_propose(
 				Origin::signed(2),
 				set_balance_proposal_hash(2),
-			), "proposal still blacklisted");
+			), Error::ProposalBlacklisted);
 
 			fast_forward_to(2);
 			// works; as we're out of the cooloff period.
@@ -1595,7 +1647,7 @@ mod tests {
 			// 3 can't veto the same thing twice.
 			assert_noop!(
 				Democracy::veto_external(Origin::signed(3), h.clone()),
-				"identity may not veto a proposal twice"
+				Error::AlreadyVetoed
 			);
 
 			// 4 vetoes.
@@ -1608,7 +1660,7 @@ mod tests {
 			assert_noop!(Democracy::external_propose(
 				Origin::signed(2),
 				set_balance_proposal_hash(2),
-			), "proposal still blacklisted");
+			), Error::ProposalBlacklisted);
 			// different proposal works fine.
 			assert_ok!(Democracy::external_propose(
 				Origin::signed(2),
@@ -1624,7 +1676,7 @@ mod tests {
 			assert_noop!(Democracy::external_propose(
 				Origin::signed(1),
 				set_balance_proposal_hash(2),
-			), "Invalid origin");
+			), "Invalid origin".into());
 			assert_ok!(Democracy::external_propose(
 				Origin::signed(2),
 				set_balance_proposal_hash_and_note(2),
@@ -1632,7 +1684,7 @@ mod tests {
 			assert_noop!(Democracy::external_propose(
 				Origin::signed(2),
 				set_balance_proposal_hash(1),
-			), "proposal already made");
+			), Error::DuplicateProposal);
 			fast_forward_to(2);
 			assert_eq!(
 				Democracy::referendum_info(0),
@@ -1653,7 +1705,7 @@ mod tests {
 			assert_noop!(Democracy::external_propose_majority(
 				Origin::signed(1),
 				set_balance_proposal_hash(2)
-			), "Invalid origin");
+			), "Invalid origin".into());
 			assert_ok!(Democracy::external_propose_majority(
 				Origin::signed(3),
 				set_balance_proposal_hash_and_note(2)
@@ -1678,7 +1730,7 @@ mod tests {
 			assert_noop!(Democracy::external_propose_default(
 				Origin::signed(3),
 				set_balance_proposal_hash(2)
-			), "Invalid origin");
+			), "Invalid origin".into());
 			assert_ok!(Democracy::external_propose_default(
 				Origin::signed(1),
 				set_balance_proposal_hash_and_note(2)
@@ -1701,12 +1753,12 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			System::set_block_number(0);
 			let h = set_balance_proposal_hash_and_note(2);
-			assert_noop!(Democracy::fast_track(Origin::signed(5), h, 3, 2), "no proposal made");
+			assert_noop!(Democracy::fast_track(Origin::signed(5), h, 3, 2), Error::ProposalMissing);
 			assert_ok!(Democracy::external_propose_majority(
 				Origin::signed(3),
 				set_balance_proposal_hash_and_note(2)
 			));
-			assert_noop!(Democracy::fast_track(Origin::signed(1), h, 3, 2), "Invalid origin");
+			assert_noop!(Democracy::fast_track(Origin::signed(1), h, 3, 2), "Invalid origin".into());
 			assert_ok!(Democracy::fast_track(Origin::signed(5), h, 0, 0));
 			assert_eq!(
 				Democracy::referendum_info(0),
@@ -1731,7 +1783,7 @@ mod tests {
 			));
 			assert_noop!(
 				Democracy::fast_track(Origin::signed(5), h, 3, 2),
-				"next external proposal not simple majority"
+				Error::NotSimpleMajority
 			);
 		});
 	}
@@ -1813,7 +1865,7 @@ mod tests {
 				(6, set_balance_proposal_hash_and_note(2), 0)
 			]);
 
-			assert_noop!(Democracy::cancel_queued(Origin::ROOT, 1), "proposal not found");
+			assert_noop!(Democracy::cancel_queued(Origin::ROOT, 1), Error::ProposalMissing);
 			assert_ok!(Democracy::cancel_queued(Origin::ROOT, 0));
 			assert_eq!(Democracy::dispatch_queue(), vec![]);
 		});
@@ -1827,7 +1879,7 @@ mod tests {
 			assert_eq!(Democracy::proxy(10), Some(1));
 
 			// Can't set when already set.
-			assert_noop!(Democracy::set_proxy(Origin::signed(2), 10), "already a proxy");
+			assert_noop!(Democracy::set_proxy(Origin::signed(2), 10), Error::AlreadyProxy);
 
 			// But this works because 11 isn't proxying.
 			assert_ok!(Democracy::set_proxy(Origin::signed(2), 11));
@@ -1835,7 +1887,7 @@ mod tests {
 			assert_eq!(Democracy::proxy(11), Some(2));
 
 			// 2 cannot fire 1's proxy:
-			assert_noop!(Democracy::remove_proxy(Origin::signed(2), 10), "wrong proxy");
+			assert_noop!(Democracy::remove_proxy(Origin::signed(2), 10), Error::WrongProxy);
 
 			// 1 fires his proxy:
 			assert_ok!(Democracy::remove_proxy(Origin::signed(1), 10));
@@ -2042,7 +2094,7 @@ mod tests {
 	fn proposal_with_deposit_below_minimum_should_not_work() {
 		new_test_ext().execute_with(|| {
 			System::set_block_number(1);
-			assert_noop!(propose_set_balance(1, 2, 0), "value too low");
+			assert_noop!(propose_set_balance(1, 2, 0), Error::ValueLow);
 		});
 	}
 
@@ -2050,7 +2102,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), "proposer\'s balance too low");
+			assert_noop!(propose_set_balance(1, 2, 11), Error::Other("not enough free funds"));
 		});
 	}
 
@@ -2059,7 +2111,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), "seconder\'s balance too low");
+			assert_noop!(Democracy::second(Origin::signed(1), 0), Error::Other("not enough free funds"));
 		});
 	}
 

substrate/frame/evm/src/lib.rs

@@ -24,13 +24,13 @@ mod backend;
 pub use crate::backend::{Account, Log, Vicinity, Backend};
 
 use sp_std::{vec::Vec, marker::PhantomData};
-use frame_support::{dispatch, decl_module, decl_storage, decl_event};
+use frame_support::{decl_module, decl_storage, decl_event, decl_error};
 use frame_support::weights::{Weight, WeighData, ClassifyDispatch, DispatchClass, PaysFee};
 use frame_support::traits::{Currency, WithdrawReason, ExistenceRequirement};
 use frame_system::{self as system, ensure_signed};
 use sp_runtime::ModuleId;
 use frame_support::weights::SimpleDispatchInfo;
-use sp_runtime::traits::{UniqueSaturatedInto, AccountIdConversion, SaturatedConversion};
+use sp_runtime::traits::{UniqueSaturatedInto, AccountIdConversion, SaturatedConversion, ModuleDispatchError};
 use sp_core::{U256, H256, H160};
 use evm::{ExitReason, ExitSucceed, ExitError};
 use evm::executor::StackExecutor;
@@ -137,21 +137,42 @@ decl_storage! {
 	}
 }
 
-decl_event!(
+decl_event! {
 	/// EVM events
 	pub enum Event {
 		/// Ethereum events from contracts.
 		Log(Log),
 	}
-);
+}
+
+decl_error! {
+	pub enum Error {
+		/// Not enough balance to perform action
+		BalanceLow,
+		/// Calculating total fee overflowed
+		FeeOverflow,
+		/// Calculating total payment overflowed
+		PaymentOverflow,
+		/// Withdraw fee failed
+		WithdrawFailed,
+		/// Call failed
+		ExitReasonFailed,
+		/// Call reverted
+		ExitReasonRevert,
+		/// Call returned VM fatal error
+		ExitReasonFatal,
+	}
+}
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error;
+
 		fn deposit_event() = default;
 
 		#[weight = SimpleDispatchInfo::FixedNormal(10_000)]
-		fn deposit_balance(origin, value: BalanceOf<T>) -> dispatch::Result {
+		fn deposit_balance(origin, value: BalanceOf<T>) {
-			let sender = ensure_signed(origin)?;
+			let sender = ensure_signed(origin).map_err(|e| e.as_str())?;
 
 			let imbalance = T::Currency::withdraw(
 				&sender,
@@ -166,19 +187,17 @@ decl_module! {
 			Accounts::mutate(&address, |account| {
 				account.balance += bvalue;
 			});
-
-			Ok(())
 		}
 
 		#[weight = SimpleDispatchInfo::FixedNormal(10_000)]
-		fn withdraw_balance(origin, value: BalanceOf<T>) -> dispatch::Result {
+		fn withdraw_balance(origin, value: BalanceOf<T>) {
-			let sender = ensure_signed(origin)?;
+			let sender = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let address = T::ConvertAccountId::convert_account_id(&sender);
 			let bvalue = U256::from(UniqueSaturatedInto::<u128>::unique_saturated_into(value));
 
 			let mut account = Accounts::get(&address);
 			account.balance = account.balance.checked_sub(bvalue)
-				.ok_or("Not enough balance to withdraw")?;
+				.ok_or(Error::BalanceLow)?;
 
 			let imbalance = T::Currency::withdraw(
 				&Self::account_id(),
@@ -190,15 +209,11 @@ decl_module! {
 			Accounts::insert(&address, account);
 
 			T::Currency::resolve_creating(&sender, imbalance);
-
-			Ok(())
 		}
 
 		#[weight = WeightForCallCreate::<T::FeeCalculator>::default()]
-		fn call(origin, target: H160, input: Vec<u8>, value: U256, gas_limit: u32)
+		fn call(origin, target: H160, input: Vec<u8>, value: U256, gas_limit: u32) {
-			-> dispatch::Result
+			let sender = ensure_signed(origin).map_err(|e| e.as_str())?;
-		{
-			let sender = ensure_signed(origin)?;
 			let source = T::ConvertAccountId::convert_account_id(&sender);
 			let gas_price = T::FeeCalculator::gas_price();
 
@@ -216,13 +231,13 @@ decl_module! {
 			);
 
 			let total_fee = gas_price.checked_mul(U256::from(gas_limit))
-				.ok_or("Calculating total fee overflowed")?;
+				.ok_or(Error::FeeOverflow)?;
 			if Accounts::get(&source).balance <
-				value.checked_add(total_fee).ok_or("Calculating total payment overflowed")?
+				value.checked_add(total_fee).ok_or(Error::PaymentOverflow)?
 			{
-				return Err("Not enough balance to pay transaction fee")
+				return Err(Error::BalanceLow)
 			}
-			executor.withdraw(source, total_fee).map_err(|_| "Withdraw fee failed")?;
+			executor.withdraw(source, total_fee).map_err(|_| Error::WithdrawFailed)?;
 
 			let reason = executor.transact_call(
 				source,
@@ -234,9 +249,9 @@ decl_module! {
 
 			let ret = match reason {
 				ExitReason::Succeed(_) => Ok(()),
-				ExitReason::Error(_) => Err("Execute message call failed"),
+				ExitReason::Error(_) => Err(Error::ExitReasonFailed),
-				ExitReason::Revert(_) => Err("Execute message call reverted"),
+				ExitReason::Revert(_) => Err(Error::ExitReasonRevert),
-				ExitReason::Fatal(_) => Err("Execute message call returned VM fatal error"),
+				ExitReason::Fatal(_) => Err(Error::ExitReasonFatal),
 			};
 			let actual_fee = executor.fee(gas_price);
 			executor.deposit(source, total_fee.saturating_sub(actual_fee));
@@ -244,12 +259,12 @@ decl_module! {
 			let (values, logs) = executor.deconstruct();
 			backend.apply(values, logs, true);
 
-			ret
+			return ret;
 		}
 
 		#[weight = WeightForCallCreate::<T::FeeCalculator>::default()]
-		fn create(origin, init: Vec<u8>, value: U256, gas_limit: u32) -> dispatch::Result {
+		fn create(origin, init: Vec<u8>, value: U256, gas_limit: u32) {
-			let sender = ensure_signed(origin)?;
+			let sender = ensure_signed(origin).map_err(|e| e.as_str())?;
 			let source = T::ConvertAccountId::convert_account_id(&sender);
 			let gas_price = T::FeeCalculator::gas_price();
 
@@ -267,13 +282,13 @@ decl_module! {
 			);
 
 			let total_fee = gas_price.checked_mul(U256::from(gas_limit))
-				.ok_or("Calculating total fee overflowed")?;
+				.ok_or(Error::FeeOverflow)?;
 			if Accounts::get(&source).balance <
-				value.checked_add(total_fee).ok_or("Calculating total payment overflowed")?
+				value.checked_add(total_fee).ok_or(Error::PaymentOverflow)?
 			{
-				return Err("Not enough balance to pay transaction fee")
+				return Err(Error::BalanceLow)
 			}
-			executor.withdraw(source, total_fee).map_err(|_| "Withdraw fee failed")?;
+			executor.withdraw(source, total_fee).map_err(|_| Error::WithdrawFailed)?;
 
 			let reason = executor.transact_create(
 				source,
@@ -284,9 +299,9 @@ decl_module! {
 
 			let ret = match reason {
 				ExitReason::Succeed(_) => Ok(()),
-				ExitReason::Error(_) => Err("Execute contract creation failed"),
+				ExitReason::Error(_) => Err(Error::ExitReasonFailed),
-				ExitReason::Revert(_) => Err("Execute contract creation reverted"),
+				ExitReason::Revert(_) => Err(Error::ExitReasonRevert),
-				ExitReason::Fatal(_) => Err("Execute contract creation returned VM fatal error"),
+				ExitReason::Fatal(_) => Err(Error::ExitReasonFatal),
 			};
 			let actual_fee = executor.fee(gas_price);
 			executor.deposit(source, total_fee.saturating_sub(actual_fee));
@@ -294,7 +309,7 @@ decl_module! {
 			let (values, logs) = executor.deconstruct();
 			backend.apply(values, logs, true);
 
-			ret
+			return ret;
 		}
 	}
 }

substrate/frame/finality-tracker/src/lib.rs

@@ -19,9 +19,9 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 use sp_inherents::{InherentIdentifier, ProvideInherent, InherentData, MakeFatalError};
-use sp_runtime::traits::{One, Zero, SaturatedConversion};
+use sp_runtime::traits::{One, Zero, SaturatedConversion, ModuleDispatchError};
 use sp_std::{prelude::*, result, cmp, vec};
-use frame_support::{decl_module, decl_storage};
+use frame_support::{decl_module, decl_storage, decl_error, ensure};
 use frame_support::traits::Get;
 use frame_system::{ensure_none, Trait as SystemTrait};
 use sp_finality_tracker::{INHERENT_IDENTIFIER, FinalizedInherentData};
@@ -56,8 +56,18 @@ decl_storage! {
 	}
 }
 
+decl_error! {
+	pub enum Error {
+		/// Final hint must be updated only once in the block
+		AlreadyUpdated,
+		/// Finalized height above block number
+		BadHint,
+	}
+}
+
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error;
 		/// The number of recent samples to keep from this chain. Default is 101.
 		const WindowSize: T::BlockNumber = T::WindowSize::get();
 
@@ -67,11 +77,11 @@ decl_module! {
 		/// Hint that the author of this block thinks the best finalized
 		/// block is the given number.
 		fn final_hint(origin, #[compact] hint: T::BlockNumber) {
-			ensure_none(origin)?;
+			ensure_none(origin).map_err(|e| e.as_str())?;
-			assert!(!<Self as Store>::Update::exists(), "Final hint must be updated only once in the block");
+			ensure!(!<Self as Store>::Update::exists(), Error::AlreadyUpdated);
-			assert!(
+			ensure!(
 				frame_system::Module::<T>::block_number() >= hint,
-				"Finalized height above block number",
+				Error::BadHint,
 			);
 			<Self as Store>::Update::put(hint);
 		}

substrate/frame/grandpa/src/lib.rs

@@ -31,10 +31,12 @@
 pub use sp_finality_grandpa as fg_primitives;
 
 use sp_std::prelude::*;
-use codec::{self as codec, Encode, Decode, Error};
+use codec::{self as codec, Encode, Decode};
-use frame_support::{decl_event, decl_storage, decl_module, dispatch, storage};
+use frame_support::{decl_event, decl_storage, decl_module, decl_error, storage};
 use sp_runtime::{
-	generic::{DigestItem, OpaqueDigestItemId}, traits::Zero, Perbill,
+	generic::{DigestItem, OpaqueDigestItemId},
+	traits::{Zero, ModuleDispatchError},
+	Perbill,
 };
 use sp_staking::{
 	SessionIndex,
@@ -82,7 +84,7 @@ pub struct StoredPendingChange<N> {
 }
 
 impl<N: Decode> Decode for StoredPendingChange<N> {
-	fn decode<I: codec::Input>(value: &mut I) -> core::result::Result<Self, Error> {
+	fn decode<I: codec::Input>(value: &mut I) -> core::result::Result<Self, codec::Error> {
 		let old = OldStoredPendingChange::decode(value)?;
 		let forced = <Option<N>>::decode(value).unwrap_or(None);
 
@@ -123,7 +125,7 @@ pub enum StoredState<N> {
 	},
 }
 
-decl_event!(
+decl_event! {
 	pub enum Event {
 		/// New authority set has been applied.
 		NewAuthorities(AuthorityList),
@@ -132,7 +134,22 @@ decl_event!(
 		/// Current authority set has been resumed.
 		Resumed,
 	}
-);
+}
+
+decl_error! {
+	pub enum Error {
+		/// Attempt to signal GRANDPA pause when the authority set isn't live
+		/// (either paused or already pending pause).
+		PauseFailed,
+		/// Attempt to signal GRANDPA resume when the authority set isn't paused
+		/// (either live or already pending resume).
+		ResumeFailed,
+		/// Attempt to signal GRANDPA change with one already pending.
+		ChangePending,
+		/// Cannot signal forced change so soon after last.
+		TooSoon,
+	}
+}
 
 decl_storage! {
 	trait Store for Module<T: Trait> as GrandpaFinality {
@@ -170,11 +187,13 @@ decl_storage! {
 
 decl_module! {
 	pub struct Module<T: Trait> for enum Call where origin: T::Origin {
+		type Error = Error;
+
 		fn deposit_event() = default;
 
 		/// Report some misbehavior.
 		fn report_misbehavior(origin, _report: Vec<u8>) {
-			ensure_signed(origin)?;
+			ensure_signed(origin).map_err(|e| e.as_str())?;
 			// FIXME: https://github.com/paritytech/substrate/issues/1112
 		}
 
@@ -264,7 +283,7 @@ impl<T: Trait> Module<T> {
 
 	/// Schedule GRANDPA to pause starting in the given number of blocks.
 	/// Cannot be done when already paused.
-	pub fn schedule_pause(in_blocks: T::BlockNumber) -> dispatch::Result {
+	pub fn schedule_pause(in_blocks: T::BlockNumber) -> Result<(), Error> {
 		if let StoredState::Live = <State<T>>::get() {
 			let scheduled_at = <frame_system::Module<T>>::block_number();
 			<State<T>>::put(StoredState::PendingPause {
@@ -274,13 +293,12 @@ impl<T: Trait> Module<T> {
 
 			Ok(())
 		} else {
-			Err("Attempt to signal GRANDPA pause when the authority set isn't live \
+			Err(Error::PauseFailed)
-				(either paused or already pending pause).")
 		}
 	}
 
 	/// Schedule a resume of GRANDPA after pausing.
-	pub fn schedule_resume(in_blocks: T::BlockNumber) -> dispatch::Result {
+	pub fn schedule_resume(in_blocks: T::BlockNumber) -> Result<(), Error> {
 		if let StoredState::Paused = <State<T>>::get() {
 			let scheduled_at = <frame_system::Module<T>>::block_number();
 			<State<T>>::put(StoredState::PendingResume {
@@ -290,8 +308,7 @@ impl<T: Trait> Module<T> {
 
 			Ok(())
 		} else {
-			Err("Attempt to signal GRANDPA resume when the authority set isn't paused \
+			Err(Error::ResumeFailed)
-				(either live or already pending resume).")
 		}
 	}
 
@@ -313,13 +330,13 @@ impl<T: Trait> Module<T> {
 		next_authorities: AuthorityList,
 		in_blocks: T::BlockNumber,
 		forced: Option<T::BlockNumber>,
-	) -> dispatch::Result {
+	) -> Result<(), Error> {
 		if !<PendingChange<T>>::exists() {
 			let scheduled_at = <frame_system::Module<T>>::block_number();
 
 			if let Some(_) = forced {
 				if Self::next_forced().map_or(false, |next| next > scheduled_at) {
-					return Err("Cannot signal forced change so soon after last.");
+					return Err(Error::TooSoon);
 				}
 
 				// only allow the next forced change when twice the window has passed since
@@ -336,7 +353,7 @@ impl<T: Trait> Module<T> {
 
 			Ok(())
 		} else {
-			Err("Attempt to signal GRANDPA change with one already pending.")
+			Err(Error::ChangePending)
 		}
 	}