diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 04f9d03363..60d8b7485d 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -157,6 +157,10 @@ impl system::Trait for Runtime { type AvailableBlockRatio = AvailableBlockRatio; /// Version of the runtime. type Version = Version; + /// Converts a module to the index of the module in `construct_runtime!`. + /// + /// This type is being generated by `construct_runtime!`. + type ModuleToIndex = ModuleToIndex; } impl aura::Trait for Runtime { diff --git a/substrate/bin/node-template/runtime/src/template.rs b/substrate/bin/node-template/runtime/src/template.rs index b800eae70c..a64a4c3216 100644 --- a/substrate/bin/node-template/runtime/src/template.rs +++ b/substrate/bin/node-template/runtime/src/template.rs @@ -40,7 +40,7 @@ decl_module! { // Just a dummy entry point. // function that can be called by the external world as an extrinsics call // takes a parameter of the type `AccountId`, stores it and emits an event - pub fn do_something(origin, something: u32) -> dispatch::Result { + pub fn do_something(origin, something: u32) -> dispatch::DispatchResult { // TODO: You only need this if you want to check it was signed. let who = ensure_signed(origin)?; @@ -106,6 +106,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl Trait for Test { type Event = (); diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index e3f3101b37..488b29aa2a 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -125,6 +125,7 @@ impl frame_system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = Version; + type ModuleToIndex = (); } impl pallet_utility::Trait for Runtime { diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index 3beb247170..94ec8f73df 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -92,7 +92,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn issue_token_airdrop(origin) -> dispatch::Result { +//! pub fn issue_token_airdrop(origin) -> dispatch::DispatchResult { //! let sender = ensure_signed(origin).map_err(|e| e.as_str())?; //! //! const ACCOUNT_ALICE: u64 = 1; @@ -133,7 +133,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use frame_support::{Parameter, decl_module, decl_event, decl_storage, decl_error, ensure}; -use sp_runtime::traits::{Member, SimpleArithmetic, Zero, StaticLookup, ModuleDispatchError}; +use sp_runtime::traits::{Member, SimpleArithmetic, Zero, StaticLookup}; use frame_system::{self as system, ensure_signed}; use sp_runtime::traits::One; @@ -151,14 +151,14 @@ pub trait Trait: frame_system::Trait { decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + 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).map_err(|e| e.as_str())?; + let origin = ensure_signed(origin)?; let id = Self::next_asset_id(); >::mutate(|id| *id += One::one()); @@ -175,12 +175,12 @@ decl_module! { target: ::Source, #[compact] amount: T::Balance ) { - let origin = ensure_signed(origin).map_err(|e| e.as_str())?; + let origin = ensure_signed(origin)?; let origin_account = (id, origin.clone()); let origin_balance = >::get(&origin_account); let target = T::Lookup::lookup(target)?; - ensure!(!amount.is_zero(), Error::AmountZero); - ensure!(origin_balance >= amount, Error::BalanceLow); + ensure!(!amount.is_zero(), Error::::AmountZero); + ensure!(origin_balance >= amount, Error::::BalanceLow); Self::deposit_event(RawEvent::Transferred(id, origin, target.clone(), amount)); >::insert(origin_account, origin_balance - amount); @@ -189,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).map_err(|e| e.as_str())?; + let origin = ensure_signed(origin)?; let balance = >::take((id, &origin)); - ensure!(!balance.is_zero(), Error::BalanceZero); + ensure!(!balance.is_zero(), Error::::BalanceZero); >::mutate(id, |total_supply| *total_supply -= balance); Self::deposit_event(RawEvent::Destroyed(id, origin, balance)); @@ -215,7 +215,7 @@ decl_event! { } decl_error! { - pub enum Error { + pub enum Error for Module { /// Transfer amount should be non-zero AmountZero, /// Account balance must be greater than or equal to the transfer amount @@ -293,6 +293,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } impl Trait for Test { type Event = (); @@ -353,7 +354,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), Error::BalanceLow); + assert_noop!(Assets::transfer(Origin::signed(1), 0, 1, 50), Error::::BalanceLow); }); } @@ -362,7 +363,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), Error::AmountZero); + assert_noop!(Assets::transfer(Origin::signed(1), 0, 2, 0), Error::::AmountZero); }); } @@ -371,7 +372,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), Error::BalanceLow); + assert_noop!(Assets::transfer(Origin::signed(1), 0, 2, 101), Error::::BalanceLow); }); } @@ -389,7 +390,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), Error::BalanceZero); + assert_noop!(Assets::destroy(Origin::signed(2), 0), Error::::BalanceZero); }); } } diff --git a/substrate/frame/aura/src/mock.rs b/substrate/frame/aura/src/mock.rs index 758bb72597..fbab5a421c 100644 --- a/substrate/frame/aura/src/mock.rs +++ b/substrate/frame/aura/src/mock.rs @@ -60,6 +60,7 @@ impl frame_system::Trait for Test { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } impl pallet_timestamp::Trait for Test { diff --git a/substrate/frame/authority-discovery/src/lib.rs b/substrate/frame/authority-discovery/src/lib.rs index b4abae88dd..065005373f 100644 --- a/substrate/frame/authority-discovery/src/lib.rs +++ b/substrate/frame/authority-discovery/src/lib.rs @@ -165,6 +165,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } impl_outer_origin! { diff --git a/substrate/frame/authorship/src/lib.rs b/substrate/frame/authorship/src/lib.rs index 882e8161da..5c2f964220 100644 --- a/substrate/frame/authorship/src/lib.rs +++ b/substrate/frame/authorship/src/lib.rs @@ -184,12 +184,12 @@ decl_module! { /// Provide a set of uncles. #[weight = SimpleDispatchInfo::FixedOperational(10_000)] - fn set_uncles(origin, new_uncles: Vec) -> dispatch::Result { + fn set_uncles(origin, new_uncles: Vec) -> dispatch::DispatchResult { ensure_none(origin)?; ensure!(new_uncles.len() <= MAX_UNCLES, "Too many uncles"); if ::DidSetUncles::get() { - return Err("Uncles already set in block."); + Err("Uncles already set in block.")? } ::DidSetUncles::put(true); @@ -219,7 +219,7 @@ impl Module { } } - fn verify_and_import_uncles(new_uncles: Vec) -> dispatch::Result { + fn verify_and_import_uncles(new_uncles: Vec) -> dispatch::DispatchResult { let now = >::block_number(); let mut uncles = ::Uncles::get(); @@ -408,6 +408,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } parameter_types! { @@ -565,7 +566,7 @@ mod tests { ); assert_eq!( Authorship::verify_and_import_uncles(vec![uncle_a.clone(), uncle_a.clone()]), - Err("uncle already included"), + Err("uncle already included".into()), ); } @@ -580,7 +581,7 @@ mod tests { assert_eq!( Authorship::verify_and_import_uncles(vec![uncle_a.clone()]), - Err("uncle already included"), + Err("uncle already included".into()), ); } @@ -590,7 +591,7 @@ mod tests { assert_eq!( Authorship::verify_and_import_uncles(vec![uncle_clone]), - Err("uncle already included"), + Err("uncle already included".into()), ); } @@ -599,7 +600,7 @@ mod tests { let unsealed = create_header(3, canon_chain.canon_hash(2), [2; 32].into()); assert_eq!( Authorship::verify_and_import_uncles(vec![unsealed]), - Err("no author"), + Err("no author".into()), ); } @@ -614,7 +615,7 @@ mod tests { assert_eq!( Authorship::verify_and_import_uncles(vec![gen_2]), - Err("uncle not recent enough to be included"), + Err("uncle not recent enough to be included".into()), ); } diff --git a/substrate/frame/babe/src/mock.rs b/substrate/frame/babe/src/mock.rs index a8acfee291..fb9804dfb7 100644 --- a/substrate/frame/babe/src/mock.rs +++ b/substrate/frame/babe/src/mock.rs @@ -65,6 +65,7 @@ impl frame_system::Trait for Test { type MaximumBlockWeight = MaximumBlockWeight; type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; + type ModuleToIndex = (); } impl_opaque_keys! { diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index e0d54c6020..5367d7413b 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/substrate/frame/balances/src/lib.rs @@ -170,10 +170,9 @@ use frame_support::{ Imbalance, SignedImbalance, ReservableCurrency, Get, VestingCurrency, }, weights::SimpleDispatchInfo, - dispatch::Result, }; use sp_runtime::{ - RuntimeDebug, + RuntimeDebug, DispatchResult, DispatchError, traits::{ Zero, SimpleArithmetic, StaticLookup, Member, CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Saturating, Bounded, @@ -821,6 +820,7 @@ impl, I: Instance> frame_system::Trait for ElevatedTrait { type MaximumBlockLength = T::MaximumBlockLength; type AvailableBlockRatio = T::AvailableBlockRatio; type Version = T::Version; + type ModuleToIndex = T::ModuleToIndex; } impl, I: Instance> Trait for ElevatedTrait { type Balance = T::Balance; @@ -891,11 +891,11 @@ where _amount: T::Balance, reasons: WithdrawReasons, new_balance: T::Balance, - ) -> Result { + ) -> DispatchResult { if reasons.intersects(WithdrawReason::Reserve | WithdrawReason::Transfer) && Self::vesting_balance(who) > new_balance { - return Err("vesting balance too high to send value"); + Err("vesting balance too high to send value")? } let locks = Self::locks(who); if locks.is_empty() { @@ -912,7 +912,7 @@ where { Ok(()) } else { - Err("account liquidity restrictions prevent withdrawal") + Err("account liquidity restrictions prevent withdrawal".into()) } } @@ -921,22 +921,22 @@ where dest: &T::AccountId, value: Self::Balance, existence_requirement: ExistenceRequirement, - ) -> Result { + ) -> DispatchResult { let from_balance = Self::free_balance(transactor); let to_balance = Self::free_balance(dest); let would_create = to_balance.is_zero(); let fee = if would_create { T::CreationFee::get() } else { T::TransferFee::get() }; let liability = match value.checked_add(&fee) { Some(l) => l, - None => return Err("got overflow after adding a fee to value"), + None => Err("got overflow after adding a fee to value")?, }; let new_from_balance = match from_balance.checked_sub(&liability) { - None => return Err("balance too low to send value"), + None => Err("balance too low to send value")?, Some(b) => b, }; if would_create && value < T::ExistentialDeposit::get() { - return Err("value too low to create account"); + Err("value too low to create account")? } Self::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; @@ -944,13 +944,13 @@ where // but better to be safe than sorry. let new_to_balance = match to_balance.checked_add(&value) { Some(b) => b, - None => return Err("destination balance too high to receive value"), + None => Err("destination balance too high to receive value")?, }; if transactor != dest { if existence_requirement == ExistenceRequirement::KeepAlive { if new_from_balance < Self::minimum_balance() { - return Err("transfer would kill account"); + Err("transfer would kill account")? } } @@ -976,7 +976,7 @@ where value: Self::Balance, reasons: WithdrawReasons, liveness: ExistenceRequirement, - ) -> result::Result { + ) -> result::Result { let old_balance = Self::free_balance(who); if let Some(new_balance) = old_balance.checked_sub(&value) { // if we need to keep the account alive... @@ -986,13 +986,13 @@ where // ...yet is was alive before && old_balance >= T::ExistentialDeposit::get() { - return Err("payment would kill account") + Err("payment would kill account")? } 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("too few free funds in account")? } } @@ -1022,9 +1022,9 @@ where fn deposit_into_existing( who: &T::AccountId, value: Self::Balance - ) -> result::Result { + ) -> result::Result { if Self::total_balance(who).is_zero() { - return Err("beneficiary account must pre-exist"); + Err("beneficiary account must pre-exist")? } Self::set_free_balance(who, Self::free_balance(who) + value); Ok(PositiveImbalance::new(value)) @@ -1104,10 +1104,10 @@ where >::get(who) } - fn reserve(who: &T::AccountId, value: Self::Balance) -> result::Result<(), &'static str> { + fn reserve(who: &T::AccountId, value: Self::Balance) -> result::Result<(), DispatchError> { let b = Self::free_balance(who); if b < value { - return Err("not enough free funds") + Err("not enough free funds")? } let new_balance = b - value; Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve.into(), new_balance)?; @@ -1139,9 +1139,9 @@ where slashed: &T::AccountId, beneficiary: &T::AccountId, value: Self::Balance, - ) -> result::Result { + ) -> result::Result { if Self::total_balance(beneficiary).is_zero() { - return Err("beneficiary account must pre-exist"); + Err("beneficiary account must pre-exist")? } let b = Self::reserved_balance(slashed); let slash = cmp::min(b, value); @@ -1250,9 +1250,9 @@ where locked: T::Balance, per_block: T::Balance, starting_block: T::BlockNumber - ) -> Result { + ) -> DispatchResult { if >::exists(who) { - return Err("A vesting schedule already exists for this account."); + Err("A vesting schedule already exists for this account.")? } let vesting_schedule = VestingSchedule { locked, diff --git a/substrate/frame/balances/src/mock.rs b/substrate/frame/balances/src/mock.rs index 2ae15d977a..c54165d6ec 100644 --- a/substrate/frame/balances/src/mock.rs +++ b/substrate/frame/balances/src/mock.rs @@ -76,6 +76,7 @@ impl frame_system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const TransactionBaseFee: u64 = 0; diff --git a/substrate/frame/balances/src/tests.rs b/substrate/frame/balances/src/tests.rs index a58426462d..fe0df11b3a 100644 --- a/substrate/frame/balances/src/tests.rs +++ b/substrate/frame/balances/src/tests.rs @@ -18,7 +18,7 @@ use super::*; use mock::{Balances, ExtBuilder, Runtime, System, info_from_weight, CALL}; -use sp_runtime::traits::SignedExtension; +use sp_runtime::traits::{SignedExtension, BadOrigin}; use frame_support::{ assert_noop, assert_ok, assert_err, traits::{LockableCurrency, LockIdentifier, WithdrawReason, WithdrawReasons, @@ -347,7 +347,7 @@ fn force_transfer_works() { let _ = Balances::deposit_creating(&1, 111); assert_noop!( Balances::force_transfer(Some(2).into(), 1, 2, 69), - "RequireRootOrigin", + BadOrigin, ); assert_ok!(Balances::force_transfer(RawOrigin::Root.into(), 1, 2, 69)); assert_eq!(Balances::total_balance(&1), 42); diff --git a/substrate/frame/collective/src/lib.rs b/substrate/frame/collective/src/lib.rs index 11134c87de..617fb32e4e 100644 --- a/substrate/frame/collective/src/lib.rs +++ b/substrate/frame/collective/src/lib.rs @@ -26,7 +26,7 @@ use sp_std::{prelude::*, result}; use sp_core::u32_trait::Value as U32; use sp_runtime::RuntimeDebug; -use sp_runtime::traits::{Hash, EnsureOrigin, ModuleDispatchError}; +use sp_runtime::traits::{Hash, EnsureOrigin}; use frame_support::weights::SimpleDispatchInfo; use frame_support::{ dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode}, @@ -125,7 +125,7 @@ decl_event! { } decl_error! { - pub enum Error { + pub enum Error for Module, I: Instance> { /// Account is not a member NotMember, /// Duplicate proposals not allowed @@ -146,7 +146,7 @@ decl_error! { // operational class. decl_module! { pub struct Module, I: Instance=DefaultInstance> for enum Call where origin: ::Origin { - type Error = Error; + type Error = Error; fn deposit_event() = default; @@ -156,7 +156,7 @@ decl_module! { /// Requires root origin. #[weight = SimpleDispatchInfo::FixedOperational(100_000)] fn set_members(origin, new_members: Vec) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; let mut new_members = new_members; new_members.sort(); >::mutate(|m| { @@ -170,8 +170,8 @@ decl_module! { /// Origin must be a member of the collective. #[weight = SimpleDispatchInfo::FixedOperational(100_000)] fn execute(origin, proposal: Box<>::Proposal>) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(Self::is_member(&who), Error::NotMember); + let who = ensure_signed(origin)?; + 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(); @@ -184,12 +184,12 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedOperational(5_000_000)] fn propose(origin, #[compact] threshold: MemberCount, proposal: Box<>::Proposal>) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(Self::is_member(&who), Error::NotMember); + let who = ensure_signed(origin)?; + ensure!(Self::is_member(&who), Error::::NotMember); let proposal_hash = T::Hashing::hash_of(&proposal); - ensure!(!>::exists(proposal_hash), Error::DuplicateProposal); + ensure!(!>::exists(proposal_hash), Error::::DuplicateProposal); if threshold < 2 { let seats = Self::members().len() as MemberCount; @@ -213,11 +213,11 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedOperational(200_000)] fn vote(origin, proposal: T::Hash, #[compact] index: ProposalIndex, approve: bool) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(Self::is_member(&who), Error::NotMember); + let who = ensure_signed(origin)?; + ensure!(Self::is_member(&who), Error::::NotMember); - let mut voting = Self::voting(&proposal).ok_or(Error::ProposalMissing)?; - ensure!(voting.index == index, Error::WrongIndex); + let mut voting = Self::voting(&proposal).ok_or(Error::::ProposalMissing)?; + 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); @@ -226,7 +226,7 @@ decl_module! { if position_yes.is_none() { voting.ayes.push(who.clone()); } else { - return Err(Error::DuplicateVote) + Err(Error::::DuplicateVote)? } if let Some(pos) = position_no { voting.nays.swap_remove(pos); @@ -235,7 +235,7 @@ decl_module! { if position_no.is_none() { voting.nays.push(who.clone()); } else { - return Err(Error::DuplicateVote) + Err(Error::::DuplicateVote)? } if let Some(pos) = position_yes { voting.ayes.swap_remove(pos); @@ -430,6 +430,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl Trait for Test { type Origin = Origin; @@ -584,7 +585,7 @@ mod tests { let proposal = make_proposal(42); assert_noop!( Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone())), - Error::NotMember + Error::::NotMember ); }); } @@ -598,7 +599,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), - Error::NotMember, + Error::::NotMember, ); }); } @@ -612,7 +613,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), - Error::WrongIndex, + Error::::WrongIndex, ); }); } @@ -630,7 +631,7 @@ mod tests { ); assert_noop!( Collective::vote(Origin::signed(1), hash.clone(), 0, true), - Error::DuplicateVote, + Error::::DuplicateVote, ); assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); assert_eq!( @@ -639,7 +640,7 @@ mod tests { ); assert_noop!( Collective::vote(Origin::signed(1), hash.clone(), 0, false), - Error::DuplicateVote, + Error::::DuplicateVote, ); assert_eq!(System::events(), vec![ diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index c2a20ec4a1..261e4297b7 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -23,7 +23,7 @@ use crate::rent; use sp_std::prelude::*; use sp_runtime::traits::{Bounded, CheckedAdd, CheckedSub, Zero}; use frame_support::{ - storage::unhashed, + storage::unhashed, dispatch::DispatchError, traits::{WithdrawReason, Currency, Time, Randomness}, }; @@ -66,7 +66,7 @@ impl ExecReturnValue { /// non-existent destination contract, etc.). #[cfg_attr(test, derive(sp_runtime::RuntimeDebug))] pub struct ExecError { - pub reason: &'static str, + pub reason: DispatchError, /// This is an allocated buffer that may be reused. The buffer must be cleared explicitly /// before reuse. pub buffer: Vec, @@ -83,7 +83,9 @@ macro_rules! try_or_exec_error { ($e:expr, $buffer:expr) => { match $e { Ok(val) => val, - Err(reason) => return Err($crate::exec::ExecError { reason, buffer: $buffer }), + Err(reason) => return Err( + $crate::exec::ExecError { reason: reason.into(), buffer: $buffer } + ), } } } @@ -336,7 +338,7 @@ where ) -> ExecResult { if self.depth == self.config.max_depth as usize { return Err(ExecError { - reason: "reached maximum depth, cannot make a call", + reason: "reached maximum depth, cannot make a call".into(), buffer: input_data, }); } @@ -346,7 +348,7 @@ where .is_out_of_gas() { return Err(ExecError { - reason: "not enough gas to pay base call fee", + reason: "not enough gas to pay base call fee".into(), buffer: input_data, }); } @@ -359,7 +361,7 @@ where // Calls to dead contracts always fail. if let Some(ContractInfo::Tombstone(_)) = contract_info { return Err(ExecError { - reason: "contract has been evicted", + reason: "contract has been evicted".into(), buffer: input_data, }); }; @@ -404,7 +406,7 @@ where .expect("a nested execution context must have a parent; qed"); if parent.is_live(&dest) { return Err(ExecError { - reason: "contract cannot be destroyed during recursive execution", + reason: "contract cannot be destroyed during recursive execution".into(), buffer: output.data, }); } @@ -428,7 +430,7 @@ where ) -> Result<(T::AccountId, ExecReturnValue), ExecError> { if self.depth == self.config.max_depth as usize { return Err(ExecError { - reason: "reached maximum depth, cannot instantiate", + reason: "reached maximum depth, cannot instantiate".into(), buffer: input_data, }); } @@ -438,7 +440,7 @@ where .is_out_of_gas() { return Err(ExecError { - reason: "not enough gas to pay base instantiate fee", + reason: "not enough gas to pay base instantiate fee".into(), buffer: input_data, }); } @@ -488,7 +490,7 @@ where // Error out if insufficient remaining balance. if nested.overlay.get_balance(&dest) < nested.config.existential_deposit { return Err(ExecError { - reason: "insufficient remaining balance", + reason: "insufficient remaining balance".into(), buffer: output.data, }); } @@ -603,7 +605,7 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( dest: &T::AccountId, value: BalanceOf, ctx: &mut ExecutionContext<'a, T, V, L>, -) -> Result<(), &'static str> { +) -> Result<(), DispatchError> { use self::TransferCause::*; use self::TransferFeeKind::*; @@ -637,23 +639,28 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( }; if gas_meter.charge(ctx.config, token).is_out_of_gas() { - return Err("not enough gas to pay transfer fee"); + Err("not enough gas to pay transfer fee")? } // We allow balance to go below the existential deposit here: let from_balance = ctx.overlay.get_balance(transactor); let new_from_balance = match from_balance.checked_sub(&value) { Some(b) => b, - None => return Err("balance too low to send value"), + None => Err("balance too low to send value")?, }; if would_create && value < ctx.config.existential_deposit { - return Err("value too low to create account"); + Err("value too low to create account")? } - T::Currency::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; + T::Currency::ensure_can_withdraw( + transactor, + value, + WithdrawReason::Transfer.into(), + new_from_balance, + )?; let new_to_balance = match to_balance.checked_add(&value) { Some(b) => b, - None => return Err("destination balance too high to receive value"), + None => Err("destination balance too high to receive value")?, }; if transactor != dest { @@ -821,6 +828,7 @@ mod tests { }; use std::{cell::RefCell, rc::Rc, collections::HashMap, marker::PhantomData}; use assert_matches::assert_matches; + use sp_runtime::DispatchError; const ALICE: u64 = 1; const BOB: u64 = 2; @@ -1176,7 +1184,10 @@ mod tests { assert_matches!( result, - Err(ExecError { reason: "balance too low to send value", buffer: _ }) + Err(ExecError { + reason: DispatchError::Other("balance too low to send value"), + buffer: _, + }) ); assert_eq!(ctx.overlay.get_balance(&origin), 0); assert_eq!(ctx.overlay.get_balance(&dest), 0); @@ -1313,7 +1324,10 @@ mod tests { // Verify that we've got proper error and set `reached_bottom`. assert_matches!( r, - Err(ExecError { reason: "reached maximum depth, cannot make a call", buffer: _ }) + Err(ExecError { + reason: DispatchError::Other("reached maximum depth, cannot make a call"), + buffer: _, + }) ); *reached_bottom = true; } else { @@ -1583,7 +1597,7 @@ mod tests { let mut loader = MockLoader::empty(); let dummy_ch = loader.insert( - |_| Err(ExecError { reason: "It's a trap!", buffer: Vec::new() }) + |_| Err(ExecError { reason: "It's a trap!".into(), buffer: Vec::new() }) ); let instantiator_ch = loader.insert({ let dummy_ch = dummy_ch.clone(); @@ -1596,7 +1610,7 @@ mod tests { ctx.gas_meter, vec![] ), - Err(ExecError { reason: "It's a trap!", buffer: _ }) + Err(ExecError { reason: DispatchError::Other("It's a trap!"), buffer: _ }) ); exec_success() diff --git a/substrate/frame/contracts/src/gas.rs b/substrate/frame/contracts/src/gas.rs index f914aaad1b..af9236e2e5 100644 --- a/substrate/frame/contracts/src/gas.rs +++ b/substrate/frame/contracts/src/gas.rs @@ -21,6 +21,7 @@ use sp_runtime::traits::{ }; use frame_support::{ traits::{Currency, ExistenceRequirement, Imbalance, OnUnbalanced, WithdrawReason}, StorageValue, + dispatch::DispatchError, }; #[cfg(test)] @@ -201,7 +202,7 @@ impl GasMeter { pub fn buy_gas( transactor: &T::AccountId, gas_limit: Gas, -) -> Result<(GasMeter, NegativeImbalanceOf), &'static str> { +) -> Result<(GasMeter, NegativeImbalanceOf), DispatchError> { // Buy the specified amount of gas. let gas_price = >::gas_price(); let cost = if gas_price.is_zero() { diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 83f11b1d9f..d1bea0ca98 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -119,7 +119,7 @@ use sp_runtime::{ }, RuntimeDebug, }; -use frame_support::dispatch::{Result, Dispatchable}; +use frame_support::dispatch::{DispatchResult, Dispatchable}; use frame_support::{ Parameter, decl_module, decl_event, decl_storage, storage::child, parameter_types, IsSubType, @@ -539,10 +539,10 @@ decl_module! { /// Updates the schedule for metering contracts. /// /// The schedule must have a greater version than the stored schedule. - pub fn update_schedule(origin, schedule: Schedule) -> Result { + pub fn update_schedule(origin, schedule: Schedule) -> DispatchResult { ensure_root(origin)?; if >::current_schedule().version >= schedule.version { - return Err("new schedule must have a greater version than current"); + Err("new schedule must have a greater version than current")? } Self::deposit_event(RawEvent::ScheduleUpdated(schedule.version)); @@ -557,7 +557,7 @@ decl_module! { origin, #[compact] gas_limit: Gas, code: Vec - ) -> Result { + ) -> DispatchResult { let origin = ensure_signed(origin)?; let (mut gas_meter, imbalance) = gas::buy_gas::(&origin, gas_limit)?; @@ -570,7 +570,7 @@ decl_module! { gas::refund_unused_gas::(&origin, gas_meter, imbalance); - result.map(|_| ()) + result.map(|_| ()).map_err(Into::into) } /// Makes a call to an account, optionally transferring some balance. @@ -586,13 +586,13 @@ decl_module! { #[compact] value: BalanceOf, #[compact] gas_limit: Gas, data: Vec - ) -> Result { + ) -> DispatchResult { let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(dest)?; Self::bare_call(origin, dest, value, gas_limit, data) .map(|_| ()) - .map_err(|e| e.reason) + .map_err(|e| e.reason.into()) } /// Instantiates a new contract from the `codehash` generated by `put_code`, optionally transferring some balance. @@ -611,7 +611,7 @@ decl_module! { #[compact] gas_limit: Gas, code_hash: CodeHash, data: Vec - ) -> Result { + ) -> DispatchResult { let origin = ensure_signed(origin)?; Self::execute_wasm(origin, gas_limit, |ctx, gas_meter| { @@ -619,7 +619,7 @@ decl_module! { .map(|(_address, output)| output) }) .map(|_| ()) - .map_err(|e| e.reason) + .map_err(|e| e.reason.into()) } /// Allows block producers to claim a small reward for evicting a contract. If a block producer @@ -636,10 +636,10 @@ decl_module! { (Ok(frame_system::RawOrigin::None), Some(aux_sender)) => { (false, aux_sender) }, - _ => return Err( + _ => Err( "Invalid surcharge claim: origin must be signed or \ inherent and auxiliary sender only provided on inherent" - ), + )?, }; // Add some advantage for block producers (who send unsigned extrinsics) by @@ -783,7 +783,7 @@ impl Module { code_hash: CodeHash, rent_allowance: BalanceOf, delta: Vec - ) -> Result { + ) -> DispatchResult { let mut origin_contract = >::get(&origin) .and_then(|c| c.get_alive()) .ok_or("Cannot restore from inexisting or tombstone contract")?; @@ -791,7 +791,7 @@ impl Module { let current_block = >::block_number(); if origin_contract.last_write == Some(current_block) { - return Err("Origin TrieId written in the current block"); + Err("Origin TrieId written in the current block")? } let dest_tombstone = >::get(&dest) @@ -816,7 +816,7 @@ impl Module { origin_contract.child_trie_unique_id(), &blake2_256(key), ); - + (key, value) }) }) @@ -841,7 +841,7 @@ impl Module { ); } - return Err("Tombstones don't match"); + return Err("Tombstones don't match".into()); } origin_contract.storage_size -= key_values_taken.iter() diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index 160b1d74dc..7cf86b31c7 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/substrate/frame/contracts/src/tests.rs @@ -117,6 +117,7 @@ impl frame_system::Trait for Test { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } impl pallet_balances::Trait for Test { type Balance = u64; diff --git a/substrate/frame/contracts/src/wasm/mod.rs b/substrate/frame/contracts/src/wasm/mod.rs index 472951efb5..28b05fcd1a 100644 --- a/substrate/frame/contracts/src/wasm/mod.rs +++ b/substrate/frame/contracts/src/wasm/mod.rs @@ -161,6 +161,7 @@ mod tests { use wabt; use hex_literal::hex; use assert_matches::assert_matches; + use sp_runtime::DispatchError; #[derive(Debug, PartialEq, Eq)] struct DispatchEntry(Call); @@ -1429,7 +1430,7 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { reason: "during execution", buffer: _ }) + Err(ExecError { reason: DispatchError::Other("during execution"), buffer: _ }) ); } @@ -1471,7 +1472,7 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { reason: "during execution", buffer: _ }) + Err(ExecError { reason: DispatchError::Other("during execution"), buffer: _ }) ); } diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index 98675aa0e4..cbf666dde2 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -98,7 +98,7 @@ pub(crate) fn to_execution_result( // validated by the code preparation process. However, because panics are really // undesirable in the runtime code, we treat this as a trap for now. Eventually, we might // want to revisit this. - Ok(_) => Err(ExecError { reason: "return type error", buffer: runtime.scratch_buf }), + Ok(_) => Err(ExecError { reason: "return type error".into(), buffer: runtime.scratch_buf }), // `Error::Module` is returned only if instantiation or linking failed (i.e. // wasm binary tried to import a function that is not provided by the host). // This shouldn't happen because validation process ought to reject such binaries. @@ -106,10 +106,10 @@ pub(crate) fn to_execution_result( // Because panics are really undesirable in the runtime code, we treat this as // a trap for now. Eventually, we might want to revisit this. Err(sp_sandbox::Error::Module) => - Err(ExecError { reason: "validation error", buffer: runtime.scratch_buf }), + Err(ExecError { reason: "validation error".into(), buffer: runtime.scratch_buf }), // Any other kind of a trap should result in a failure. Err(sp_sandbox::Error::Execution) | Err(sp_sandbox::Error::OutOfBounds) => - Err(ExecError { reason: "during execution", buffer: runtime.scratch_buf }), + Err(ExecError { reason: "during execution".into(), buffer: runtime.scratch_buf }), } } diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index eb9cf819c5..3bbe011497 100644 --- a/substrate/frame/democracy/src/lib.rs +++ b/substrate/frame/democracy/src/lib.rs @@ -21,16 +21,12 @@ use sp_std::prelude::*; use sp_std::{result, convert::TryFrom}; use sp_runtime::{ - RuntimeDebug, - traits::{ - Zero, Bounded, CheckedMul, CheckedDiv, EnsureOrigin, Hash, - Dispatchable, Saturating, ModuleDispatchError, - }, + RuntimeDebug, DispatchResult, + traits::{Zero, Bounded, CheckedMul, CheckedDiv, EnsureOrigin, Hash, Dispatchable, Saturating}, }; use codec::{Ref, Encode, Decode, Input, Output}; use frame_support::{ - decl_module, decl_storage, decl_event, decl_error, ensure, - Parameter, + decl_module, decl_storage, decl_event, decl_error, ensure, Parameter, weights::SimpleDispatchInfo, traits::{ Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get, @@ -365,7 +361,7 @@ decl_event! { } decl_error! { - pub enum Error { + pub enum Error for Module { /// Value too low ValueLow, /// Proposal does not exist @@ -415,7 +411,7 @@ decl_error! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + 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 @@ -454,8 +450,8 @@ decl_module! { proposal_hash: T::Hash, #[compact] value: BalanceOf ) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(value >= T::MinimumDeposit::get(), Error::ValueLow); + let who = ensure_signed(origin)?; + ensure!(value >= T::MinimumDeposit::get(), Error::::ValueLow); T::Currency::reserve(&who, value)?; let index = Self::public_prop_count(); @@ -476,9 +472,9 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(5_000_000)] fn second(origin, #[compact] proposal: PropIndex) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; + let who = ensure_signed(origin)?; let mut deposit = Self::deposit_of(proposal) - .ok_or(Error::ProposalMissing)?; + .ok_or(Error::::ProposalMissing)?; T::Currency::reserve(&who, deposit.0)?; deposit.1.push(who); >::insert(proposal, deposit); @@ -495,8 +491,8 @@ decl_module! { fn vote(origin, #[compact] ref_index: ReferendumIndex, vote: Vote - ) -> Result<(), Error> { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; + ) -> DispatchResult { + let who = ensure_signed(origin)?; Self::do_vote(who, ref_index, vote) } @@ -511,10 +507,8 @@ decl_module! { fn proxy_vote(origin, #[compact] ref_index: ReferendumIndex, vote: Vote - ) -> Result<(), Error> { - let who = Self::proxy( - ensure_signed(origin).map_err(|e| e.as_str())? - ).ok_or(Error::NotProxy)?; + ) -> DispatchResult { + let who = Self::proxy(ensure_signed(origin)?).ok_or(Error::::NotProxy)?; Self::do_vote(who, ref_index, vote) } @@ -522,11 +516,11 @@ decl_module! { /// referendum. #[weight = SimpleDispatchInfo::FixedOperational(500_000)] fn emergency_cancel(origin, ref_index: ReferendumIndex) { - T::CancellationOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?; + T::CancellationOrigin::ensure_origin(origin)?; - let info = Self::referendum_info(ref_index).ok_or(Error::BadIndex)?; + let info = Self::referendum_info(ref_index).ok_or(Error::::BadIndex)?; let h = info.proposal_hash; - ensure!(!>::exists(h), Error::AlreadyCanceled); + ensure!(!>::exists(h), Error::::AlreadyCanceled); >::insert(h, true); Self::clear_referendum(ref_index); @@ -536,10 +530,13 @@ decl_module! { /// referendum. #[weight = SimpleDispatchInfo::FixedNormal(5_000_000)] fn external_propose(origin, proposal_hash: T::Hash) { - T::ExternalOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?; - ensure!(!>::exists(), Error::DuplicateProposal); + T::ExternalOrigin::ensure_origin(origin)?; + ensure!(!>::exists(), Error::::DuplicateProposal); if let Some((until, _)) = >::get(proposal_hash) { - ensure!(>::block_number() >= until, Error::ProposalBlacklisted); + ensure!( + >::block_number() >= until, + Error::::ProposalBlacklisted, + ); } >::put((proposal_hash, VoteThreshold::SuperMajorityApprove)); } @@ -551,7 +548,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).map_err(|e| Into::<&str>::into(e))?; + T::ExternalMajorityOrigin::ensure_origin(origin)?; >::put((proposal_hash, VoteThreshold::SimpleMajority)); } @@ -562,7 +559,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).map_err(|e| Into::<&str>::into(e))?; + T::ExternalDefaultOrigin::ensure_origin(origin)?; >::put((proposal_hash, VoteThreshold::SuperMajorityAgainst)); } @@ -581,13 +578,13 @@ decl_module! { voting_period: T::BlockNumber, delay: T::BlockNumber ) { - T::FastTrackOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?; - let (e_proposal_hash, threshold) = >::get().ok_or(Error::ProposalMissing)?; + T::FastTrackOrigin::ensure_origin(origin)?; + let (e_proposal_hash, threshold) = >::get().ok_or(Error::::ProposalMissing)?; ensure!( threshold != VoteThreshold::SuperMajorityApprove, - Error::NotSimpleMajority + Error::::NotSimpleMajority, ); - ensure!(proposal_hash == e_proposal_hash, Error::InvalidHash); + ensure!(proposal_hash == e_proposal_hash, Error::::InvalidHash); >::kill(); let now = >::block_number(); @@ -599,19 +596,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).map_err(|e| Into::<&str>::into(e))?; + let who = T::VetoOrigin::ensure_origin(origin)?; if let Some((e_proposal_hash, _)) = >::get() { - ensure!(proposal_hash == e_proposal_hash, Error::ProposalMissing); + ensure!(proposal_hash == e_proposal_hash, Error::::ProposalMissing); } else { - Err(Error::NoProposal)?; + Err(Error::::NoProposal)?; } let mut existing_vetoers = >::get(&proposal_hash) .map(|pair| pair.1) .unwrap_or_else(Vec::new); let insert_position = existing_vetoers.binary_search(&who) - .err().ok_or(Error::AlreadyVetoed)?; + .err().ok_or(Error::::AlreadyVetoed)?; existing_vetoers.insert(insert_position, who.clone()); let until = >::block_number() + T::CooloffPeriod::get(); @@ -624,24 +621,24 @@ decl_module! { /// Remove a referendum. #[weight = SimpleDispatchInfo::FixedOperational(10_000)] fn cancel_referendum(origin, #[compact] ref_index: ReferendumIndex) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; 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).map_err(|e| e.as_str())?; + ensure_root(origin)?; let mut items = >::get(); let original_len = items.len(); items.retain(|i| i.2 != which); - ensure!(items.len() < original_len, Error::ProposalMissing); + ensure!(items.len() < original_len, Error::::ProposalMissing); >::put(items); } fn on_initialize(n: T::BlockNumber) { if let Err(e) = Self::begin_block(n) { - sp_runtime::print(e.as_str()); + sp_runtime::print(e); } } @@ -652,8 +649,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(100_000)] fn set_proxy(origin, proxy: T::AccountId) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(!>::exists(&proxy), Error::AlreadyProxy); + let who = ensure_signed(origin)?; + ensure!(!>::exists(&proxy), Error::::AlreadyProxy); >::insert(proxy, who) } @@ -664,7 +661,7 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(100_000)] fn resign_proxy(origin) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; + let who = ensure_signed(origin)?; >::remove(who); } @@ -675,8 +672,11 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(100_000)] fn remove_proxy(origin, proxy: T::AccountId) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(&Self::proxy(&proxy).ok_or(Error::NotProxy)? == &who, Error::WrongProxy); + let who = ensure_signed(origin)?; + ensure!( + &Self::proxy(&proxy).ok_or(Error::::NotProxy)? == &who, + Error::::WrongProxy, + ); >::remove(proxy); } @@ -687,7 +687,7 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(500_000)] pub fn delegate(origin, to: T::AccountId, conviction: Conviction) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; + let who = ensure_signed(origin)?; >::insert(&who, (&to, conviction)); // Currency is locked indefinitely as long as it's delegated. T::Currency::extend_lock( @@ -707,8 +707,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(500_000)] fn undelegate(origin) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(>::exists(&who), Error::NotDelegated); + let who = ensure_signed(origin)?; + ensure!(>::exists(&who), Error::::NotDelegated); let (_, conviction) = >::take(&who); // Indefinite lock is reduced to the maximum voting lock that could be possible. let now = >::block_number(); @@ -726,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).map_err(|e| e.as_str())?; + ensure_root(origin)?; >::kill(); } @@ -735,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) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; + let who = ensure_signed(origin)?; let proposal_hash = T::Hashing::hash(&encoded_proposal[..]); - ensure!(!>::exists(&proposal_hash), Error::DuplicatePreimage); + ensure!(!>::exists(&proposal_hash), Error::::DuplicatePreimage); let deposit = >::from(encoded_proposal.len() as u32) .saturating_mul(T::PreimageByteDeposit::get()); @@ -753,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) { - let who = ensure_signed(origin).map_err(|e| e.as_str())?; + let who = ensure_signed(origin)?; let proposal_hash = T::Hashing::hash(&encoded_proposal[..]); - ensure!(!>::exists(&proposal_hash), Error::DuplicatePreimage); + ensure!(!>::exists(&proposal_hash), Error::::DuplicatePreimage); let queue = >::get(); - ensure!(queue.iter().any(|item| &item.1 == &proposal_hash), Error::NotImminent); + ensure!(queue.iter().any(|item| &item.1 == &proposal_hash), Error::::NotImminent); let now = >::block_number(); let free = >::zero(); @@ -773,16 +773,17 @@ 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).map_err(|e| e.as_str())?; + let who = ensure_signed(origin)?; - let (_, old, deposit, then) = >::get(&proposal_hash).ok_or(Error::PreimageMissing)?; + let (_, old, deposit, then) = >::get(&proposal_hash) + .ok_or(Error::::PreimageMissing)?; let now = >::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, Error::Early); + ensure!(now >= then + voting + additional, Error::::Early); let queue = >::get(); - ensure!(!queue.iter().any(|item| &item.1 == &proposal_hash), Error::Imminent); + ensure!(!queue.iter().any(|item| &item.1 == &proposal_hash), Error::::Imminent); let _ = T::Currency::repatriate_reserved(&old, &who, deposit); >::remove(&proposal_hash); @@ -931,8 +932,8 @@ impl Module { // private. /// Actually enact a vote, if legit. - fn do_vote(who: T::AccountId, ref_index: ReferendumIndex, vote: Vote) -> Result<(), Error> { - ensure!(Self::is_active_referendum(ref_index), Error::ReferendumInvalid); + fn do_vote(who: T::AccountId, ref_index: ReferendumIndex, vote: Vote) -> DispatchResult { + ensure!(Self::is_active_referendum(ref_index), Error::::ReferendumInvalid); if !>::exists((ref_index, &who)) { >::append_or_insert(ref_index, &[&who][..]); } @@ -959,6 +960,7 @@ impl Module { fn clear_referendum(ref_index: ReferendumIndex) { >::remove(ref_index); + LowestUnbaked::mutate(|i| if *i == ref_index { *i += 1; let end = ReferendumCount::get(); @@ -973,7 +975,7 @@ impl Module { } /// Enact a proposal from a referendum. - fn enact_proposal(proposal_hash: T::Hash, index: ReferendumIndex) -> Result<(), Error> { + fn enact_proposal(proposal_hash: T::Hash, index: ReferendumIndex) -> DispatchResult { if let Some((encoded_proposal, who, amount, _)) = >::take(&proposal_hash) { if let Ok(proposal) = T::Proposal::decode(&mut &encoded_proposal[..]) { let _ = T::Currency::unreserve(&who, amount); @@ -986,25 +988,25 @@ impl Module { } else { T::Slash::on_unbalanced(T::Currency::slash_reserved(&who, amount).0); Self::deposit_event(RawEvent::PreimageInvalid(proposal_hash, index)); - Err(Error::PreimageInvalid) + Err(Error::::PreimageInvalid.into()) } } else { Self::deposit_event(RawEvent::PreimageMissing(proposal_hash, index)); - Err(Error::PreimageMissing) + Err(Error::::PreimageMissing.into()) } } /// Table the next waiting proposal for a vote. - fn launch_next(now: T::BlockNumber) -> Result<(), Error> { + fn launch_next(now: T::BlockNumber) -> DispatchResult { 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(|_| Error::NoneWaiting) + }.map_err(|_| Error::::NoneWaiting.into()) } /// Table the waiting external proposal for a vote, if there is one. - fn launch_external(now: T::BlockNumber) -> Result<(), Error> { + fn launch_external(now: T::BlockNumber) -> DispatchResult { if let Some((proposal, threshold)) = >::take() { LastTabledWasExternal::put(true); Self::deposit_event(RawEvent::ExternalTabled); @@ -1016,12 +1018,12 @@ impl Module { ); Ok(()) } else { - Err(Error::NoneWaiting) + Err(Error::::NoneWaiting)? } } /// Table the waiting public proposal with the highest backing for a vote. - fn launch_public(now: T::BlockNumber) -> Result<(), Error> { + fn launch_public(now: T::BlockNumber) -> DispatchResult { let mut public_props = Self::public_props(); if let Some((winner_index, _)) = public_props.iter() .enumerate() @@ -1046,7 +1048,7 @@ impl Module { } Ok(()) } else { - Err(Error::NoneWaiting) + Err(Error::::NoneWaiting)? } } @@ -1055,7 +1057,7 @@ impl Module { now: T::BlockNumber, index: ReferendumIndex, info: ReferendumInfo - ) -> Result<(), Error> { + ) -> DispatchResult { let (approve, against, capital) = Self::tally(index); let total_issuance = T::Currency::total_issuance(); let approved = info.threshold.approved(approve, against, capital, total_issuance); @@ -1103,7 +1105,7 @@ impl Module { } /// Current era is ending; we should finish up any proposals. - fn begin_block(now: T::BlockNumber) -> Result<(), Error> { + fn begin_block(now: T::BlockNumber) -> DispatchResult { // 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 @@ -1146,7 +1148,10 @@ mod tests { weights::Weight, }; use sp_core::H256; - use sp_runtime::{traits::{BlakeTwo256, IdentityLookup, Bounded}, testing::Header, Perbill}; + use sp_runtime::{ + traits::{BlakeTwo256, IdentityLookup, Bounded, BadOrigin}, + testing::Header, Perbill, + }; use pallet_balances::BalanceLock; use frame_system::EnsureSignedBy; @@ -1191,6 +1196,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 0; @@ -1289,13 +1295,14 @@ mod tests { let p = set_balance_proposal(value); let h = BlakeTwo256::hash(&p[..]); match Democracy::note_preimage(Origin::signed(6), p) { - Ok(_) | Err(Error::DuplicatePreimage) => (), + Ok(_) => (), + Err(x) if x == Error::::DuplicatePreimage.into() => (), Err(x) => panic!(x), } h } - fn propose_set_balance(who: u64, value: u64, delay: u64) -> Result<(), Error> { + fn propose_set_balance(who: u64, value: u64, delay: u64) -> DispatchResult { Democracy::propose( Origin::signed(who), set_balance_proposal_hash(value), @@ -1303,7 +1310,7 @@ mod tests { ) } - fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> Result<(), Error> { + fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> DispatchResult { Democracy::propose( Origin::signed(who), set_balance_proposal_hash_and_note(value), @@ -1349,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]), - Error::Other("not enough free funds") + "not enough free funds", ); // fee of 1 is reasonable. PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1); @@ -1384,7 +1391,7 @@ mod tests { next_block(); assert_noop!( Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2)), - Error::Early + Error::::Early ); next_block(); assert_ok!(Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2))); @@ -1400,7 +1407,7 @@ mod tests { System::set_block_number(1); assert_noop!( Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2)), - Error::PreimageMissing + Error::::PreimageMissing ); PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1); @@ -1412,7 +1419,7 @@ mod tests { next_block(); assert_noop!( Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2)), - Error::Early + Error::::Early ); next_block(); @@ -1439,7 +1446,7 @@ mod tests { assert_noop!( Democracy::note_imminent_preimage(Origin::signed(7), set_balance_proposal(2)), - Error::NotImminent + Error::::NotImminent ); next_block(); @@ -1463,7 +1470,7 @@ mod tests { next_block(); next_block(); // now imminent. - assert_noop!(Democracy::reap_preimage(Origin::signed(6), h), Error::Imminent); + assert_noop!(Democracy::reap_preimage(Origin::signed(6), h), Error::::Imminent); }); } @@ -1592,7 +1599,7 @@ mod tests { ); assert!(Democracy::referendum_info(r).is_some()); - assert_noop!(Democracy::emergency_cancel(Origin::signed(3), r), "Invalid origin".into()); + assert_noop!(Democracy::emergency_cancel(Origin::signed(3), r), BadOrigin); assert_ok!(Democracy::emergency_cancel(Origin::signed(4), r)); assert!(Democracy::referendum_info(r).is_none()); @@ -1605,7 +1612,7 @@ mod tests { 2 ); assert!(Democracy::referendum_info(r).is_some()); - assert_noop!(Democracy::emergency_cancel(Origin::signed(4), r), Error::AlreadyCanceled); + assert_noop!(Democracy::emergency_cancel(Origin::signed(4), r), Error::::AlreadyCanceled); }); } @@ -1627,14 +1634,14 @@ mod tests { assert_noop!(Democracy::external_propose( Origin::signed(2), set_balance_proposal_hash(2), - ), Error::ProposalBlacklisted); + ), 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), - ), Error::ProposalBlacklisted); + ), Error::::ProposalBlacklisted); fast_forward_to(2); // works; as we're out of the cooloff period. @@ -1647,7 +1654,7 @@ mod tests { // 3 can't veto the same thing twice. assert_noop!( Democracy::veto_external(Origin::signed(3), h.clone()), - Error::AlreadyVetoed + Error::::AlreadyVetoed ); // 4 vetoes. @@ -1660,7 +1667,7 @@ mod tests { assert_noop!(Democracy::external_propose( Origin::signed(2), set_balance_proposal_hash(2), - ), Error::ProposalBlacklisted); + ), Error::::ProposalBlacklisted); // different proposal works fine. assert_ok!(Democracy::external_propose( Origin::signed(2), @@ -1673,10 +1680,13 @@ mod tests { fn external_referendum_works() { new_test_ext().execute_with(|| { System::set_block_number(0); - assert_noop!(Democracy::external_propose( - Origin::signed(1), - set_balance_proposal_hash(2), - ), "Invalid origin".into()); + assert_noop!( + Democracy::external_propose( + Origin::signed(1), + set_balance_proposal_hash(2), + ), + BadOrigin, + ); assert_ok!(Democracy::external_propose( Origin::signed(2), set_balance_proposal_hash_and_note(2), @@ -1684,7 +1694,7 @@ mod tests { assert_noop!(Democracy::external_propose( Origin::signed(2), set_balance_proposal_hash(1), - ), Error::DuplicateProposal); + ), Error::::DuplicateProposal); fast_forward_to(2); assert_eq!( Democracy::referendum_info(0), @@ -1702,10 +1712,13 @@ mod tests { fn external_majority_referendum_works() { new_test_ext().execute_with(|| { System::set_block_number(0); - assert_noop!(Democracy::external_propose_majority( - Origin::signed(1), - set_balance_proposal_hash(2) - ), "Invalid origin".into()); + assert_noop!( + Democracy::external_propose_majority( + Origin::signed(1), + set_balance_proposal_hash(2) + ), + BadOrigin, + ); assert_ok!(Democracy::external_propose_majority( Origin::signed(3), set_balance_proposal_hash_and_note(2) @@ -1727,10 +1740,13 @@ mod tests { fn external_default_referendum_works() { new_test_ext().execute_with(|| { System::set_block_number(0); - assert_noop!(Democracy::external_propose_default( - Origin::signed(3), - set_balance_proposal_hash(2) - ), "Invalid origin".into()); + assert_noop!( + Democracy::external_propose_default( + Origin::signed(3), + set_balance_proposal_hash(2) + ), + BadOrigin, + ); assert_ok!(Democracy::external_propose_default( Origin::signed(1), set_balance_proposal_hash_and_note(2) @@ -1753,12 +1769,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), Error::ProposalMissing); + 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".into()); + assert_noop!(Democracy::fast_track(Origin::signed(1), h, 3, 2), BadOrigin); assert_ok!(Democracy::fast_track(Origin::signed(5), h, 0, 0)); assert_eq!( Democracy::referendum_info(0), @@ -1783,7 +1799,7 @@ mod tests { )); assert_noop!( Democracy::fast_track(Origin::signed(5), h, 3, 2), - Error::NotSimpleMajority + Error::::NotSimpleMajority ); }); } @@ -1865,7 +1881,7 @@ mod tests { (6, set_balance_proposal_hash_and_note(2), 0) ]); - assert_noop!(Democracy::cancel_queued(Origin::ROOT, 1), Error::ProposalMissing); + assert_noop!(Democracy::cancel_queued(Origin::ROOT, 1), Error::::ProposalMissing); assert_ok!(Democracy::cancel_queued(Origin::ROOT, 0)); assert_eq!(Democracy::dispatch_queue(), vec![]); }); @@ -1879,7 +1895,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), Error::AlreadyProxy); + 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)); @@ -1887,7 +1903,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), Error::WrongProxy); + assert_noop!(Democracy::remove_proxy(Origin::signed(2), 10), Error::::WrongProxy); // 1 fires his proxy: assert_ok!(Democracy::remove_proxy(Origin::signed(1), 10)); @@ -2094,7 +2110,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), Error::ValueLow); + assert_noop!(propose_set_balance(1, 2, 0), Error::::ValueLow); }); } @@ -2102,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), Error::Other("not enough free funds")); + assert_noop!(propose_set_balance(1, 2, 11), "not enough free funds"); }); } @@ -2111,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), Error::Other("not enough free funds")); + assert_noop!(Democracy::second(Origin::signed(1), 0), "not enough free funds"); }); } diff --git a/substrate/frame/elections-phragmen/src/lib.rs b/substrate/frame/elections-phragmen/src/lib.rs index 62bb4ba86d..8e93775649 100644 --- a/substrate/frame/elections-phragmen/src/lib.rs +++ b/substrate/frame/elections-phragmen/src/lib.rs @@ -83,9 +83,9 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_std::prelude::*; -use sp_runtime::{print, traits::{Zero, StaticLookup, Bounded, Convert}}; +use sp_runtime::{print, DispatchResult, traits::{Zero, StaticLookup, Bounded, Convert}}; use frame_support::{ - decl_storage, decl_event, ensure, decl_module, dispatch, weights::SimpleDispatchInfo, + decl_storage, decl_event, ensure, decl_module, weights::SimpleDispatchInfo, traits::{ Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons, ChangeMembers, OnUnbalanced, WithdrawReason @@ -373,7 +373,7 @@ decl_module! { return Ok(()); } - return Err("origin is not a candidate, member or a runner up."); + Err("origin is not a candidate, member or a runner up.")? } /// Remove a particular member from the set. This is effective immediately and the bond of @@ -390,7 +390,7 @@ decl_module! { /// Writes: O(do_phragmen) /// # #[weight = SimpleDispatchInfo::FixedOperational(2_000_000)] - fn remove_member(origin, who: ::Source) -> dispatch::Result { + fn remove_member(origin, who: ::Source) -> DispatchResult { ensure_root(origin)?; let who = T::Lookup::lookup(who)?; @@ -402,7 +402,7 @@ decl_module! { if !had_replacement { Self::do_phragmen(); } - }) + }).map_err(Into::into) } /// What to do at the end of each block. Checks if an election needs to happen or not. @@ -566,7 +566,7 @@ impl Module { /// /// Runs phragmen election and cleans all the previous candidate state. The voter state is NOT /// cleaned and voters must themselves submit a transaction to retract. - fn end_block(block_number: T::BlockNumber) -> dispatch::Result { + fn end_block(block_number: T::BlockNumber) -> DispatchResult { if !Self::term_duration().is_zero() { if (block_number % Self::term_duration()).is_zero() { Self::do_phragmen(); @@ -768,6 +768,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { diff --git a/substrate/frame/elections/src/lib.rs b/substrate/frame/elections/src/lib.rs index 5b2bee253c..1435c05951 100644 --- a/substrate/frame/elections/src/lib.rs +++ b/substrate/frame/elections/src/lib.rs @@ -25,12 +25,11 @@ use sp_std::prelude::*; use sp_runtime::{ - RuntimeDebug, - print, + RuntimeDebug, DispatchResult, print, traits::{Zero, One, StaticLookup, Bounded, Saturating}, }; use frame_support::{ - dispatch::Result, decl_storage, decl_event, ensure, decl_module, + decl_storage, decl_event, ensure, decl_module, weights::SimpleDispatchInfo, traits::{ Currency, ExistenceRequirement, Get, LockableCurrency, LockIdentifier, @@ -346,7 +345,7 @@ decl_module! { #[compact] index: VoteIndex, hint: SetIndex, #[compact] value: BalanceOf - ) -> Result { + ) -> DispatchResult { let who = ensure_signed(origin)?; Self::do_set_approvals(who, votes, index, hint, value) } @@ -363,7 +362,7 @@ decl_module! { #[compact] index: VoteIndex, hint: SetIndex, #[compact] value: BalanceOf - ) -> Result { + ) -> DispatchResult { let who = Self::proxy(ensure_signed(origin)?).ok_or("not a proxy")?; Self::do_set_approvals(who, votes, index, hint, value) } @@ -526,7 +525,7 @@ decl_module! { candidate: ::Source, #[compact] total: BalanceOf, #[compact] index: VoteIndex - ) -> Result { + ) -> DispatchResult { let who = ensure_signed(origin)?; ensure!( !total.is_zero(), @@ -587,7 +586,7 @@ decl_module! { // better safe than sorry. let imbalance = T::Currency::slash(&who, bad_presentation_punishment).0; T::BadPresentation::on_unbalanced(imbalance); - Err(if dupe { "duplicate presentation" } else { "incorrect total" }) + Err(if dupe { "duplicate presentation" } else { "incorrect total" })? } } @@ -714,7 +713,7 @@ impl Module { // Private /// Check there's nothing to do this block - fn end_block(block_number: T::BlockNumber) -> Result { + fn end_block(block_number: T::BlockNumber) -> DispatchResult { if (block_number % T::VotingPeriod::get()).is_zero() { if let Some(number) = Self::next_tally() { if block_number == number { @@ -750,7 +749,7 @@ impl Module { index: VoteIndex, hint: SetIndex, value: BalanceOf, - ) -> Result { + ) -> DispatchResult { let candidates_len = ::Candidates::decode_len().unwrap_or(0_usize); ensure!(!Self::presentation_active(), "no approval changes during presentation period"); @@ -873,7 +872,7 @@ impl Module { /// approved candidates in their place. If the total number of members is less than the desired /// membership a new vote is started. Clears all presented candidates, returning the bond of the /// elected ones. - fn finalize_tally() -> Result { + fn finalize_tally() -> DispatchResult { let (_, coming, expiring): (T::BlockNumber, u32, Vec) = >::take() .ok_or("finalize can only be called after a tally is started.")?; diff --git a/substrate/frame/elections/src/mock.rs b/substrate/frame/elections/src/mock.rs index de4f263f0e..c53789f7ad 100644 --- a/substrate/frame/elections/src/mock.rs +++ b/substrate/frame/elections/src/mock.rs @@ -53,6 +53,7 @@ impl frame_system::Trait for Test { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { diff --git a/substrate/frame/elections/src/tests.rs b/substrate/frame/elections/src/tests.rs index b502c52f8f..d7d7e8718b 100644 --- a/substrate/frame/elections/src/tests.rs +++ b/substrate/frame/elections/src/tests.rs @@ -1034,7 +1034,10 @@ fn election_double_presentations_should_be_punished() { System::set_block_number(6); assert_ok!(Elections::present_winner(Origin::signed(4), 2, 20, 0)); assert_ok!(Elections::present_winner(Origin::signed(4), 5, 50, 0)); - assert_eq!(Elections::present_winner(Origin::signed(4), 5, 50, 0), Err("duplicate presentation")); + assert_eq!( + Elections::present_winner(Origin::signed(4), 5, 50, 0), + Err("duplicate presentation".into()), + ); assert_ok!(Elections::end_block(System::block_number())); assert_eq!(Elections::members(), vec![(5, 11), (2, 11)]); diff --git a/substrate/frame/evm/src/lib.rs b/substrate/frame/evm/src/lib.rs index 9adafa3c92..891729e71a 100644 --- a/substrate/frame/evm/src/lib.rs +++ b/substrate/frame/evm/src/lib.rs @@ -30,8 +30,10 @@ 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, ModuleDispatchError}; use sp_core::{U256, H256, H160, Hasher}; +use sp_runtime::{ + DispatchResult, traits::{UniqueSaturatedInto, AccountIdConversion, SaturatedConversion}, +}; use evm::{ExitReason, ExitSucceed, ExitError}; use evm::executor::StackExecutor; use evm::backend::ApplyBackend; @@ -176,7 +178,7 @@ decl_event! { } decl_error! { - pub enum Error { + pub enum Error for Module { /// Not enough balance to perform action BalanceLow, /// Calculating total fee overflowed @@ -198,14 +200,14 @@ decl_error! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + type Error = Error; fn deposit_event() = default; /// Despoit balance from currency/balances module into EVM. #[weight = SimpleDispatchInfo::FixedNormal(10_000)] fn deposit_balance(origin, value: BalanceOf) { - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; + let sender = ensure_signed(origin)?; let imbalance = T::Currency::withdraw( &sender, @@ -225,13 +227,13 @@ decl_module! { /// Withdraw balance from EVM into currency/balances module. #[weight = SimpleDispatchInfo::FixedNormal(10_000)] fn withdraw_balance(origin, value: BalanceOf) { - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; + let sender = ensure_signed(origin)?; let address = T::ConvertAccountId::convert_account_id(&sender); let bvalue = U256::from(UniqueSaturatedInto::::unique_saturated_into(value)); let mut account = Accounts::get(&address); account.balance = account.balance.checked_sub(bvalue) - .ok_or(Error::BalanceLow)?; + .ok_or(Error::::BalanceLow)?; let imbalance = T::Currency::withdraw( &Self::account_id(), @@ -254,10 +256,9 @@ decl_module! { value: U256, gas_limit: u32, gas_price: U256, - ) { - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(gas_price >= T::FeeCalculator::min_gas_price(), Error::GasPriceTooLow); - + ) -> DispatchResult { + let sender = ensure_signed(origin)?; + ensure!(gas_price >= T::FeeCalculator::min_gas_price(), Error::::GasPriceTooLow); let source = T::ConvertAccountId::convert_account_id(&sender); let vicinity = Vicinity { @@ -274,13 +275,13 @@ decl_module! { ); let total_fee = gas_price.checked_mul(U256::from(gas_limit)) - .ok_or(Error::FeeOverflow)?; + .ok_or(Error::::FeeOverflow)?; if Accounts::get(&source).balance < - value.checked_add(total_fee).ok_or(Error::PaymentOverflow)? + value.checked_add(total_fee).ok_or(Error::::PaymentOverflow)? { - return Err(Error::BalanceLow) + Err(Error::::BalanceLow)? } - executor.withdraw(source, total_fee).map_err(|_| Error::WithdrawFailed)?; + executor.withdraw(source, total_fee).map_err(|_| Error::::WithdrawFailed)?; let reason = executor.transact_call( source, @@ -292,9 +293,9 @@ decl_module! { let ret = match reason { ExitReason::Succeed(_) => Ok(()), - ExitReason::Error(_) => Err(Error::ExitReasonFailed), - ExitReason::Revert(_) => Err(Error::ExitReasonRevert), - ExitReason::Fatal(_) => Err(Error::ExitReasonFatal), + ExitReason::Error(_) => Err(Error::::ExitReasonFailed), + ExitReason::Revert(_) => Err(Error::::ExitReasonRevert), + ExitReason::Fatal(_) => Err(Error::::ExitReasonFatal), }; let actual_fee = executor.fee(gas_price); executor.deposit(source, total_fee.saturating_sub(actual_fee)); @@ -302,7 +303,7 @@ decl_module! { let (values, logs) = executor.deconstruct(); backend.apply(values, logs, true); - return ret; + ret.map_err(Into::into) } /// Issue an EVM create operation. This is similar to a contract creation transaction in @@ -314,9 +315,9 @@ decl_module! { value: U256, gas_limit: u32, gas_price: U256, - ) { - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(gas_price >= T::FeeCalculator::min_gas_price(), Error::GasPriceTooLow); + ) -> DispatchResult { + let sender = ensure_signed(origin)?; + ensure!(gas_price >= T::FeeCalculator::min_gas_price(), Error::::GasPriceTooLow); let source = T::ConvertAccountId::convert_account_id(&sender); @@ -334,13 +335,13 @@ decl_module! { ); let total_fee = gas_price.checked_mul(U256::from(gas_limit)) - .ok_or(Error::FeeOverflow)?; + .ok_or(Error::::FeeOverflow)?; if Accounts::get(&source).balance < - value.checked_add(total_fee).ok_or(Error::PaymentOverflow)? + value.checked_add(total_fee).ok_or(Error::::PaymentOverflow)? { - return Err(Error::BalanceLow) + Err(Error::::BalanceLow)? } - executor.withdraw(source, total_fee).map_err(|_| Error::WithdrawFailed)?; + executor.withdraw(source, total_fee).map_err(|_| Error::::WithdrawFailed)?; let reason = executor.transact_create( source, @@ -351,9 +352,9 @@ decl_module! { let ret = match reason { ExitReason::Succeed(_) => Ok(()), - ExitReason::Error(_) => Err(Error::ExitReasonFailed), - ExitReason::Revert(_) => Err(Error::ExitReasonRevert), - ExitReason::Fatal(_) => Err(Error::ExitReasonFatal), + ExitReason::Error(_) => Err(Error::::ExitReasonFailed), + ExitReason::Revert(_) => Err(Error::::ExitReasonRevert), + ExitReason::Fatal(_) => Err(Error::::ExitReasonFatal), }; let actual_fee = executor.fee(gas_price); executor.deposit(source, total_fee.saturating_sub(actual_fee)); @@ -361,7 +362,7 @@ decl_module! { let (values, logs) = executor.deconstruct(); backend.apply(values, logs, true); - return ret; + ret.map_err(Into::into) } } } diff --git a/substrate/frame/example/src/lib.rs b/substrate/frame/example/src/lib.rs index d77998c85f..f13a78db56 100644 --- a/substrate/frame/example/src/lib.rs +++ b/substrate/frame/example/src/lib.rs @@ -255,7 +255,7 @@ use sp_std::marker::PhantomData; use frame_support::{ - dispatch::Result, decl_module, decl_storage, decl_event, + dispatch::DispatchResult, decl_module, decl_storage, decl_event, weights::{SimpleDispatchInfo, DispatchInfo, DispatchClass, ClassifyDispatch, WeighData, Weight, PaysFee}, }; use frame_system::{self as system, ensure_signed, ensure_root}; @@ -460,7 +460,7 @@ decl_module! { // transaction and the latter demonstrates the [`DispatchClass`] of the call. A higher // weight means a larger transaction (less of which can be placed in a single block). #[weight = SimpleDispatchInfo::FixedNormal(10_000)] - fn accumulate_dummy(origin, increase_by: T::Balance) -> Result { + fn accumulate_dummy(origin, increase_by: T::Balance) -> DispatchResult { // This is a public call, so we ensure that the origin is some signed account. let _sender = ensure_signed(origin)?; @@ -543,7 +543,7 @@ decl_module! { impl Module { // Add public immutables and private mutables. #[allow(dead_code)] - fn accumulate_foo(origin: T::Origin, increase_by: T::Balance) -> Result { + fn accumulate_foo(origin: T::Origin, increase_by: T::Balance) -> DispatchResult { let _sender = ensure_signed(origin)?; let prev = >::get(); @@ -685,6 +685,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 4dfa72f9c6..fd05c410d9 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -415,6 +415,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 0; @@ -660,14 +661,7 @@ mod tests { t.execute_with(|| { assert_eq!(Executive::validate_transaction(xt.clone()), Ok(Default::default())); - assert_eq!( - Executive::apply_extrinsic(xt), - Ok( - Err( - DispatchError { module: Some(1), error: 0, message: Some("RequireRootOrigin") } - ) - ) - ); + assert_eq!(Executive::apply_extrinsic(xt), Ok(Err(DispatchError::BadOrigin))); }); } diff --git a/substrate/frame/finality-tracker/src/lib.rs b/substrate/frame/finality-tracker/src/lib.rs index 7830a2b8f5..5fbf2b9531 100644 --- a/substrate/frame/finality-tracker/src/lib.rs +++ b/substrate/frame/finality-tracker/src/lib.rs @@ -19,7 +19,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_inherents::{InherentIdentifier, ProvideInherent, InherentData, MakeFatalError}; -use sp_runtime::traits::{One, Zero, SaturatedConversion, ModuleDispatchError}; +use sp_runtime::traits::{One, Zero, SaturatedConversion}; use sp_std::{prelude::*, result, cmp, vec}; use frame_support::{decl_module, decl_storage, decl_error, ensure}; use frame_support::traits::Get; @@ -57,7 +57,7 @@ decl_storage! { } decl_error! { - pub enum Error { + pub enum Error for Module { /// Final hint must be updated only once in the block AlreadyUpdated, /// Finalized height above block number @@ -67,7 +67,7 @@ decl_error! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + type Error = Error; /// The number of recent samples to keep from this chain. Default is 101. const WindowSize: T::BlockNumber = T::WindowSize::get(); @@ -77,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).map_err(|e| e.as_str())?; - ensure!(!::Update::exists(), Error::AlreadyUpdated); + ensure_none(origin)?; + ensure!(!::Update::exists(), Error::::AlreadyUpdated); ensure!( frame_system::Module::::block_number() >= hint, - Error::BadHint, + Error::::BadHint, ); ::Update::put(hint); } @@ -260,6 +260,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const WindowSize: u64 = 11; diff --git a/substrate/frame/generic-asset/src/lib.rs b/substrate/frame/generic-asset/src/lib.rs index 4ad61af4a7..de63ba2f12 100644 --- a/substrate/frame/generic-asset/src/lib.rs +++ b/substrate/frame/generic-asset/src/lib.rs @@ -123,7 +123,7 @@ //! # } //! type AssetOf = <::Currency as Currency<::AccountId>>::Balance; //! -//! fn charge_fee(transactor: &T::AccountId, amount: AssetOf) -> dispatch::Result { +//! fn charge_fee(transactor: &T::AccountId, amount: AssetOf) -> dispatch::DispatchResult { //! // ... //! T::Currency::withdraw( //! transactor, @@ -135,7 +135,7 @@ //! Ok(()) //! } //! -//! fn refund_fee(transactor: &T::AccountId, amount: AssetOf) -> dispatch::Result { +//! fn refund_fee(transactor: &T::AccountId, amount: AssetOf) -> dispatch::DispatchResult { //! // ... //! T::Currency::deposit_into_existing(transactor, amount)?; //! // ... @@ -153,7 +153,7 @@ use codec::{Decode, Encode, HasCompact, Input, Output, Error}; -use sp_runtime::RuntimeDebug; +use sp_runtime::{RuntimeDebug, DispatchResult, DispatchError}; use sp_runtime::traits::{ CheckedAdd, CheckedSub, MaybeSerializeDeserialize, Member, One, Saturating, SimpleArithmetic, Zero, Bounded, @@ -325,7 +325,7 @@ decl_module! { fn deposit_event() = default; /// Create a new kind of asset. - fn create(origin, options: AssetOptions) -> dispatch::Result { + fn create(origin, options: AssetOptions) -> dispatch::DispatchResult { let origin = ensure_signed(origin)?; let id = Self::next_asset_id(); @@ -358,7 +358,7 @@ decl_module! { origin, #[compact] asset_id: T::AssetId, new_permission: PermissionLatest - ) -> dispatch::Result { + ) -> dispatch::DispatchResult { let origin = ensure_signed(origin)?; let permissions: PermissionVersions = new_permission.into(); @@ -370,14 +370,14 @@ decl_module! { Ok(()) } else { - Err("Origin does not have enough permission to update permissions.") + Err("Origin does not have enough permission to update permissions.")? } } /// Mints an asset, increases its total issuance. /// The origin must have `mint` permissions. fn mint(origin, #[compact] asset_id: T::AssetId, to: T::AccountId, amount: T::Balance) - -> dispatch::Result + -> dispatch::DispatchResult { let origin = ensure_signed(origin)?; if Self::check_permission(&asset_id, &origin, &PermissionType::Mint) { @@ -395,7 +395,7 @@ decl_module! { Ok(()) } else { - Err("The origin does not have permission to mint an asset.") + Err("The origin does not have permission to mint an asset.")? } } @@ -403,7 +403,7 @@ decl_module! { /// /// The `origin` must have `burn` permissions. fn burn(origin, #[compact] asset_id: T::AssetId, to: T::AccountId, amount: T::Balance) - -> dispatch::Result + -> dispatch::DispatchResult { let origin = ensure_signed(origin)?; @@ -424,7 +424,7 @@ decl_module! { Ok(()) } else { - Err("The origin does not have permission to burn an asset.") + Err("The origin does not have permission to burn an asset.")? } } @@ -434,7 +434,7 @@ decl_module! { origin, asset_id: T::AssetId, options: AssetOptions - ) -> dispatch::Result { + ) -> dispatch::DispatchResult { ensure_root(origin)?; Self::create_asset(Some(asset_id), None, options) } @@ -543,7 +543,7 @@ impl Module { asset_id: Option, from_account: Option, options: AssetOptions, - ) -> dispatch::Result { + ) -> 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."); @@ -576,7 +576,7 @@ impl Module { from: &T::AccountId, to: &T::AccountId, amount: T::Balance - ) -> dispatch::Result { + ) -> dispatch::DispatchResult { let new_balance = Self::free_balance(asset_id, from) .checked_sub(&amount) .ok_or_else(|| "balance too low to send amount")?; @@ -597,7 +597,7 @@ impl Module { from: &T::AccountId, to: &T::AccountId, amount: T::Balance, - ) -> dispatch::Result { + ) -> dispatch::DispatchResult { Self::make_transfer(asset_id, from, to, amount)?; if from != to { @@ -612,13 +612,13 @@ impl Module { /// If the free balance is lower than `amount`, then no funds will be moved and an `Err` will /// be returned. This is different behavior than `unreserve`. pub fn reserve(asset_id: &T::AssetId, who: &T::AccountId, amount: T::Balance) - -> dispatch::Result + -> dispatch::DispatchResult { // Do we need to consider that this is an atomic transaction? 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 { - return Err("not enough free funds"); + Err("not enough free funds")? } let new_reserve_balance = original_reserve_balance + amount; Self::set_reserved_balance(asset_id, who, new_reserve_balance); @@ -751,7 +751,7 @@ impl Module { _amount: T::Balance, reasons: WithdrawReasons, new_balance: T::Balance, - ) -> dispatch::Result { + ) -> dispatch::DispatchResult { if asset_id != &Self::staking_asset_id() { return Ok(()); } @@ -767,7 +767,7 @@ impl Module { { Ok(()) } else { - Err("account liquidity restrictions prevent withdrawal") + Err("account liquidity restrictions prevent withdrawal")? } } @@ -1092,6 +1092,7 @@ impl frame_system::Trait for ElevatedTrait { type AvailableBlockRatio = T::AvailableBlockRatio; type BlockHashCount = T::BlockHashCount; type Version = T::Version; + type ModuleToIndex = (); } impl Trait for ElevatedTrait { type Balance = T::Balance; @@ -1133,7 +1134,7 @@ where dest: &T::AccountId, value: Self::Balance, _: ExistenceRequirement, // no existential deposit policy for generic asset - ) -> dispatch::Result { + ) -> DispatchResult { >::make_transfer(&U::asset_id(), transactor, dest, value) } @@ -1142,7 +1143,7 @@ where amount: Self::Balance, reasons: WithdrawReasons, new_balance: Self::Balance, - ) -> dispatch::Result { + ) -> DispatchResult { >::ensure_can_withdraw(&U::asset_id(), who, amount, reasons, new_balance) } @@ -1151,7 +1152,7 @@ where value: Self::Balance, reasons: WithdrawReasons, _: ExistenceRequirement, // no existential deposit policy for generic asset - ) -> result::Result { + ) -> result::Result { let new_balance = Self::free_balance(who) .checked_sub(&value) .ok_or_else(|| "account has too few funds")?; @@ -1163,7 +1164,7 @@ where fn deposit_into_existing( who: &T::AccountId, value: Self::Balance, - ) -> result::Result { + ) -> result::Result { // No existential deposit rule and creation fee in GA. `deposit_into_existing` is same with `deposit_creating`. Ok(Self::deposit_creating(who, value)) } @@ -1248,7 +1249,7 @@ where >::reserved_balance(&U::asset_id(), &who) } - fn reserve(who: &T::AccountId, value: Self::Balance) -> result::Result<(), &'static str> { + fn reserve(who: &T::AccountId, value: Self::Balance) -> DispatchResult { >::reserve(&U::asset_id(), who, value) } @@ -1268,7 +1269,7 @@ where slashed: &T::AccountId, beneficiary: &T::AccountId, value: Self::Balance, - ) -> result::Result { + ) -> result::Result { Ok(>::repatriate_reserved(&U::asset_id(), slashed, beneficiary, value)) } } diff --git a/substrate/frame/generic-asset/src/mock.rs b/substrate/frame/generic-asset/src/mock.rs index 90426516c1..09ec8f69fb 100644 --- a/substrate/frame/generic-asset/src/mock.rs +++ b/substrate/frame/generic-asset/src/mock.rs @@ -61,6 +61,7 @@ impl frame_system::Trait for Test { type AvailableBlockRatio = AvailableBlockRatio; type BlockHashCount = BlockHashCount; type Version = (); + type ModuleToIndex = (); } impl Trait for Test { diff --git a/substrate/frame/grandpa/src/lib.rs b/substrate/frame/grandpa/src/lib.rs index eae43affde..99777cb893 100644 --- a/substrate/frame/grandpa/src/lib.rs +++ b/substrate/frame/grandpa/src/lib.rs @@ -34,9 +34,7 @@ use sp_std::prelude::*; use codec::{self as codec, Encode, Decode}; use frame_support::{decl_event, decl_storage, decl_module, decl_error, storage}; use sp_runtime::{ - generic::{DigestItem, OpaqueDigestItemId}, - traits::{Zero, ModuleDispatchError}, - Perbill, + DispatchResult, generic::{DigestItem, OpaqueDigestItemId}, traits::Zero, Perbill, }; use sp_staking::{ SessionIndex, @@ -137,7 +135,7 @@ decl_event! { } decl_error! { - pub enum Error { + pub enum Error for Module { /// Attempt to signal GRANDPA pause when the authority set isn't live /// (either paused or already pending pause). PauseFailed, @@ -187,13 +185,13 @@ decl_storage! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + type Error = Error; fn deposit_event() = default; /// Report some misbehavior. fn report_misbehavior(origin, _report: Vec) { - ensure_signed(origin).map_err(|e| e.as_str())?; + ensure_signed(origin)?; // FIXME: https://github.com/paritytech/substrate/issues/1112 } @@ -283,7 +281,7 @@ impl Module { /// 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) -> Result<(), Error> { + pub fn schedule_pause(in_blocks: T::BlockNumber) -> DispatchResult { if let StoredState::Live = >::get() { let scheduled_at = >::block_number(); >::put(StoredState::PendingPause { @@ -293,12 +291,12 @@ impl Module { Ok(()) } else { - Err(Error::PauseFailed) + Err(Error::::PauseFailed)? } } /// Schedule a resume of GRANDPA after pausing. - pub fn schedule_resume(in_blocks: T::BlockNumber) -> Result<(), Error> { + pub fn schedule_resume(in_blocks: T::BlockNumber) -> DispatchResult { if let StoredState::Paused = >::get() { let scheduled_at = >::block_number(); >::put(StoredState::PendingResume { @@ -308,7 +306,7 @@ impl Module { Ok(()) } else { - Err(Error::ResumeFailed) + Err(Error::::ResumeFailed)? } } @@ -330,13 +328,13 @@ impl Module { next_authorities: AuthorityList, in_blocks: T::BlockNumber, forced: Option, - ) -> Result<(), Error> { + ) -> DispatchResult { if !>::exists() { let scheduled_at = >::block_number(); if let Some(_) = forced { if Self::next_forced().map_or(false, |next| next > scheduled_at) { - return Err(Error::TooSoon); + Err(Error::::TooSoon)? } // only allow the next forced change when twice the window has passed since @@ -353,7 +351,7 @@ impl Module { Ok(()) } else { - Err(Error::ChangePending) + Err(Error::::ChangePending)? } } diff --git a/substrate/frame/grandpa/src/mock.rs b/substrate/frame/grandpa/src/mock.rs index 20701b11aa..87eadea706 100644 --- a/substrate/frame/grandpa/src/mock.rs +++ b/substrate/frame/grandpa/src/mock.rs @@ -64,6 +64,7 @@ impl frame_system::Trait for Test { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } mod grandpa { diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs index 2813d6c83b..0957396658 100644 --- a/substrate/frame/identity/src/lib.rs +++ b/substrate/frame/identity/src/lib.rs @@ -69,9 +69,9 @@ use sp_std::prelude::*; use sp_std::{fmt::Debug, ops::Add, iter::once}; use enumflags2::BitFlags; use codec::{Encode, Decode}; -use sp_runtime::{traits::{StaticLookup, EnsureOrigin, Zero}, RuntimeDebug}; +use sp_runtime::{DispatchResult, traits::{StaticLookup, EnsureOrigin, Zero}, RuntimeDebug}; use frame_support::{ - decl_module, decl_event, decl_storage, ensure, dispatch::Result, + decl_module, decl_event, decl_storage, ensure, traits::{Currency, ReservableCurrency, OnUnbalanced, Get}, weights::SimpleDispatchInfo, }; @@ -423,8 +423,7 @@ decl_module! { fn add_registrar(origin, account: T::AccountId) { T::RegistrarOrigin::try_origin(origin) .map(|_| ()) - .or_else(ensure_root) - .map_err(|_| "bad origin")?; + .or_else(ensure_root)?; let i = >::mutate(|r| { r.push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() })); @@ -595,7 +594,7 @@ decl_module! { 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() { - return Err("sticky judgement") + Err("sticky judgement")? } else { id.judgements[i] = item }, @@ -636,7 +635,7 @@ decl_module! { let fee = if let Judgement::FeePaid(fee) = id.judgements.remove(pos).1 { fee } else { - return Err("judgement given") + Err("judgement given")? }; let _ = T::Currency::unreserve(&sender, fee); @@ -661,14 +660,14 @@ decl_module! { fn set_fee(origin, #[compact] index: RegistrarIndex, #[compact] fee: BalanceOf, - ) -> Result { + ) -> DispatchResult { let who = ensure_signed(origin)?; >::mutate(|rs| 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("invalid index") + .ok_or_else(|| "invalid index".into()) ) } @@ -688,14 +687,14 @@ decl_module! { fn set_account_id(origin, #[compact] index: RegistrarIndex, new: T::AccountId, - ) -> Result { + ) -> DispatchResult { let who = ensure_signed(origin)?; >::mutate(|rs| 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("invalid index") + .ok_or_else(|| "invalid index".into()) ) } @@ -715,14 +714,14 @@ decl_module! { fn set_fields(origin, #[compact] index: RegistrarIndex, fields: IdentityFields, - ) -> Result { + ) -> DispatchResult { let who = ensure_signed(origin)?; >::mutate(|rs| 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("invalid index") + .ok_or_else(|| "invalid index".into()) ) } @@ -798,8 +797,7 @@ decl_module! { fn kill_identity(origin, target: ::Source) { T::ForceOrigin::try_origin(origin) .map(|_| ()) - .or_else(ensure_root) - .map_err(|_| "bad origin")?; + .or_else(ensure_root)?; // Figure out who we're meant to be clearing. let target = T::Lookup::lookup(target)?; @@ -822,6 +820,7 @@ decl_module! { mod tests { use super::*; + use sp_runtime::traits::BadOrigin; use frame_support::{assert_ok, assert_noop, impl_outer_origin, parameter_types, weights::Weight}; use sp_core::H256; use frame_system::EnsureSignedBy; @@ -862,6 +861,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 0; @@ -999,7 +999,7 @@ mod tests { fn killing_slashing_should_work() { new_test_ext().execute_with(|| { assert_ok!(Identity::set_identity(Origin::signed(10), ten())); - assert_noop!(Identity::kill_identity(Origin::signed(1), 10), "bad origin"); + assert_noop!(Identity::kill_identity(Origin::signed(1), 10), BadOrigin); assert_ok!(Identity::kill_identity(Origin::signed(2), 10)); assert_eq!(Identity::identity(10), None); assert_eq!(Balances::free_balance(10), 90); diff --git a/substrate/frame/im-online/src/lib.rs b/substrate/frame/im-online/src/lib.rs index 64d23dbb5d..d7dfb1c673 100644 --- a/substrate/frame/im-online/src/lib.rs +++ b/substrate/frame/im-online/src/lib.rs @@ -50,7 +50,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn is_online(origin, authority_index: u32) -> dispatch::Result { +//! pub fn is_online(origin, authority_index: u32) -> dispatch::DispatchResult { //! let _sender = ensure_signed(origin)?; //! let _is_online = >::is_online(authority_index); //! Ok(()) diff --git a/substrate/frame/im-online/src/mock.rs b/substrate/frame/im-online/src/mock.rs index 94f91ddc2e..7feed1ecca 100644 --- a/substrate/frame/im-online/src/mock.rs +++ b/substrate/frame/im-online/src/mock.rs @@ -117,6 +117,7 @@ impl frame_system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { diff --git a/substrate/frame/im-online/src/tests.rs b/substrate/frame/im-online/src/tests.rs index 3fbd424421..1b4356aa49 100644 --- a/substrate/frame/im-online/src/tests.rs +++ b/substrate/frame/im-online/src/tests.rs @@ -111,7 +111,7 @@ fn heartbeat( session_index: u32, authority_index: u32, id: UintAuthorityId, -) -> dispatch::Result { +) -> dispatch::DispatchResult { #[allow(deprecated)] use frame_support::unsigned::ValidateUnsigned; @@ -127,7 +127,8 @@ fn heartbeat( let signature = id.sign(&heartbeat.encode()).unwrap(); #[allow(deprecated)] // Allow ValidateUnsigned - ImOnline::pre_dispatch(&crate::Call::heartbeat(heartbeat.clone(), signature.clone()))?; + ImOnline::pre_dispatch(&crate::Call::heartbeat(heartbeat.clone(), signature.clone())) + .map_err(|e| <&'static str>::from(e))?; ImOnline::heartbeat( Origin::system(frame_system::RawOrigin::None), heartbeat, diff --git a/substrate/frame/indices/src/mock.rs b/substrate/frame/indices/src/mock.rs index e4ff3d2d77..2a1cb0746f 100644 --- a/substrate/frame/indices/src/mock.rs +++ b/substrate/frame/indices/src/mock.rs @@ -86,6 +86,7 @@ impl frame_system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl Trait for Runtime { type AccountIndex = u64; diff --git a/substrate/frame/membership/src/lib.rs b/substrate/frame/membership/src/lib.rs index 93fcb479c7..0a7f8ec7fc 100644 --- a/substrate/frame/membership/src/lib.rs +++ b/substrate/frame/membership/src/lib.rs @@ -259,6 +259,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const One: u64 = 1; diff --git a/substrate/frame/nicks/src/lib.rs b/substrate/frame/nicks/src/lib.rs index 1c28146edb..d2f0b4d8c9 100644 --- a/substrate/frame/nicks/src/lib.rs +++ b/substrate/frame/nicks/src/lib.rs @@ -269,6 +269,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 0; diff --git a/substrate/frame/offences/src/mock.rs b/substrate/frame/offences/src/mock.rs index 1175ebaeee..343fdc88fa 100644 --- a/substrate/frame/offences/src/mock.rs +++ b/substrate/frame/offences/src/mock.rs @@ -88,6 +88,7 @@ impl frame_system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl Trait for Runtime { diff --git a/substrate/frame/randomness-collective-flip/src/lib.rs b/substrate/frame/randomness-collective-flip/src/lib.rs index ff75d6b9b8..8432a86198 100644 --- a/substrate/frame/randomness-collective-flip/src/lib.rs +++ b/substrate/frame/randomness-collective-flip/src/lib.rs @@ -41,7 +41,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn random_module_example(origin) -> dispatch::Result { +//! pub fn random_module_example(origin) -> dispatch::DispatchResult { //! let _random_seed = >::random_seed(); //! Ok(()) //! } @@ -188,6 +188,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } type System = frame_system::Module; diff --git a/substrate/frame/scored-pool/src/lib.rs b/substrate/frame/scored-pool/src/lib.rs index 9703d041d7..65a867df60 100644 --- a/substrate/frame/scored-pool/src/lib.rs +++ b/substrate/frame/scored-pool/src/lib.rs @@ -61,7 +61,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn candidate(origin) -> dispatch::Result { +//! pub fn candidate(origin) -> dispatch::DispatchResult { //! let who = ensure_signed(origin)?; //! //! let _ = >::submit_candidacy( @@ -261,7 +261,7 @@ decl_module! { // `None` are always sorted to the end. if let Err(e) = >::append(&[(who.clone(), None)]) { T::Currency::unreserve(&who, deposit); - return Err(e); + Err(e)? } >::insert(&who, true); diff --git a/substrate/frame/scored-pool/src/mock.rs b/substrate/frame/scored-pool/src/mock.rs index 097d7bc33f..fe873da26a 100644 --- a/substrate/frame/scored-pool/src/mock.rs +++ b/substrate/frame/scored-pool/src/mock.rs @@ -70,6 +70,7 @@ impl frame_system::Trait for Test { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl pallet_balances::Trait for Test { diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index aee01dc37f..5b3e4e2aee 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -483,14 +483,14 @@ decl_module! { /// - One extra DB entry. /// # #[weight = SimpleDispatchInfo::FixedNormal(150_000)] - fn set_keys(origin, keys: T::Keys, proof: Vec) -> dispatch::Result { + fn set_keys(origin, keys: T::Keys, proof: Vec) -> dispatch::DispatchResult { let who = ensure_signed(origin)?; ensure!(keys.ownership_proof_is_valid(&proof), "invalid ownership proof"); let who = match T::ValidatorIdOf::convert(who) { Some(val_id) => val_id, - None => return Err("no associated validator ID for account."), + None => Err("no associated validator ID for account.")?, }; Self::do_set_keys(&who, keys)?; @@ -631,7 +631,7 @@ impl Module { // perform the set_key operation, checking for duplicates. // does not set `Changed`. - fn do_set_keys(who: &T::ValidatorId, keys: T::Keys) -> dispatch::Result { + fn do_set_keys(who: &T::ValidatorId, keys: T::Keys) -> dispatch::DispatchResult { let old_keys = Self::load_keys(&who); for id in T::Keys::key_ids() { diff --git a/substrate/frame/session/src/mock.rs b/substrate/frame/session/src/mock.rs index 14fbc46c82..28c84d7374 100644 --- a/substrate/frame/session/src/mock.rs +++ b/substrate/frame/session/src/mock.rs @@ -174,6 +174,7 @@ impl frame_system::Trait for Test { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } impl pallet_timestamp::Trait for Test { diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index bc43b54e91..2acbe28a0a 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -147,7 +147,7 @@ //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { //! /// Reward a validator. -//! pub fn reward_myself(origin) -> dispatch::Result { +//! pub fn reward_myself(origin) -> dispatch::DispatchResult { //! let reported = ensure_signed(origin)?; //! >::reward_by_ids(vec![(reported, 10)]); //! Ok(()) @@ -272,7 +272,7 @@ use sp_runtime::{ curve::PiecewiseLinear, traits::{ Convert, Zero, One, StaticLookup, CheckedSub, Saturating, Bounded, SaturatedConversion, - SimpleArithmetic, EnsureOrigin, ModuleDispatchError, + SimpleArithmetic, EnsureOrigin, } }; use sp_staking::{ @@ -785,7 +785,7 @@ decl_event!( decl_error! { /// Error for the stacking module. - pub enum Error { + pub enum Error for Module { /// Not a controller account. NotController, /// Not a stash account. @@ -817,7 +817,7 @@ decl_module! { /// Number of eras that staked funds must remain bonded for. const BondingDuration: EraIndex = T::BondingDuration::get(); - type Error = Error; + type Error = Error; fn deposit_event() = default; @@ -853,21 +853,21 @@ decl_module! { #[compact] value: BalanceOf, payee: RewardDestination ) { - let stash = ensure_signed(origin).map_err(|e| e.as_str())?; + let stash = ensure_signed(origin)?; if >::exists(&stash) { - return Err(Error::AlreadyBonded) + Err(Error::::AlreadyBonded)? } let controller = T::Lookup::lookup(controller)?; if >::exists(&controller) { - return Err(Error::AlreadyPaired) + Err(Error::::AlreadyPaired)? } // reject a bond which is considered to be _dust_. if value < T::Currency::minimum_balance() { - return Err(Error::InsufficientValue) + Err(Error::::InsufficientValue)? } // You're auto-bonded forever, here. We might improve this by only bonding when @@ -897,10 +897,10 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(500_000)] fn bond_extra(origin, #[compact] max_additional: BalanceOf) { - let stash = ensure_signed(origin).map_err(|e| e.as_str())?; + let stash = ensure_signed(origin)?; - let controller = Self::bonded(&stash).ok_or(Error::NotStash)?; - let mut ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = Self::bonded(&stash).ok_or(Error::::NotStash)?; + let mut ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let stash_balance = T::Currency::free_balance(&stash); @@ -937,11 +937,11 @@ decl_module! { /// #[weight = SimpleDispatchInfo::FixedNormal(400_000)] fn unbond(origin, #[compact] value: BalanceOf) { - let controller = ensure_signed(origin).map_err(|e| e.as_str())?; - let mut ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = ensure_signed(origin)?; + let mut ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; ensure!( ledger.unlocking.len() < MAX_UNLOCKING_CHUNKS, - Error::NoMoreChunks + Error::::NoMoreChunks, ); let mut value = value.min(ledger.active); @@ -979,8 +979,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(400_000)] fn withdraw_unbonded(origin) { - let controller = ensure_signed(origin).map_err(|e| e.as_str())?; - let ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let ledger = ledger.consolidate_unlocked(Self::current_era()); if ledger.unlocking.is_empty() && ledger.active.is_zero() { @@ -1013,8 +1013,8 @@ decl_module! { fn validate(origin, prefs: ValidatorPrefs) { Self::ensure_storage_upgraded(); - let controller = ensure_signed(origin).map_err(|e| e.as_str())?; - let ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let stash = &ledger.stash; >::remove(stash); >::insert(stash, prefs); @@ -1035,10 +1035,10 @@ decl_module! { fn nominate(origin, targets: Vec<::Source>) { Self::ensure_storage_upgraded(); - let controller = ensure_signed(origin).map_err(|e| e.as_str())?; - let ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let stash = &ledger.stash; - ensure!(!targets.is_empty(), Error::EmptyTargets); + ensure!(!targets.is_empty(), Error::::EmptyTargets); let targets = targets.into_iter() .take(MAX_NOMINATIONS) .map(|t| T::Lookup::lookup(t)) @@ -1067,8 +1067,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(500_000)] fn chill(origin) { - let controller = ensure_signed(origin).map_err(|e| e.as_str())?; - let ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; Self::chill_stash(&ledger.stash); } @@ -1085,8 +1085,8 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(500_000)] fn set_payee(origin, payee: RewardDestination) { - let controller = ensure_signed(origin).map_err(|e| e.as_str())?; - let ledger = Self::ledger(&controller).ok_or(Error::NotController)?; + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let stash = &ledger.stash; >::insert(stash, payee); } @@ -1104,11 +1104,11 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(750_000)] fn set_controller(origin, controller: ::Source) { - let stash = ensure_signed(origin).map_err(|e| e.as_str())?; - let old_controller = Self::bonded(&stash).ok_or(Error::NotStash)?; + let stash = ensure_signed(origin)?; + let old_controller = Self::bonded(&stash).ok_or(Error::::NotStash)?; let controller = T::Lookup::lookup(controller)?; if >::exists(&controller) { - return Err(Error::AlreadyPaired) + Err(Error::::AlreadyPaired)? } if controller != old_controller { >::insert(&stash, &controller); @@ -1121,7 +1121,7 @@ decl_module! { /// The ideal number of validators. #[weight = SimpleDispatchInfo::FreeOperational] fn set_validator_count(origin, #[compact] new: u32) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; ValidatorCount::put(new); } @@ -1134,7 +1134,7 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FreeOperational] fn force_no_eras(origin) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; ForceEra::put(Forcing::ForceNone); } @@ -1146,21 +1146,21 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FreeOperational] fn force_new_era(origin) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; ForceEra::put(Forcing::ForceNew); } /// Set the validators who cannot be slashed (if any). #[weight = SimpleDispatchInfo::FreeOperational] fn set_invulnerables(origin, validators: Vec) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; >::put(validators); } /// Force a current staker to become completely unstaked, immediately. #[weight = SimpleDispatchInfo::FreeOperational] fn force_unstake(origin, stash: T::AccountId) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; // remove the lock. T::Currency::remove_lock(STAKING_ID, &stash); @@ -1175,7 +1175,7 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FreeOperational] fn force_new_era_always(origin) { - ensure_root(origin).map_err(|e| e.as_str())?; + ensure_root(origin)?; ForceEra::put(Forcing::ForceAlways); } @@ -1191,7 +1191,7 @@ decl_module! { T::SlashCancelOrigin::try_origin(origin) .map(|_| ()) .or_else(ensure_root) - .map_err(|_| Error::BadOrigin)?; + .map_err(|_| Error::::BadOrigin)?; let mut slash_indices = slash_indices; slash_indices.sort_unstable(); @@ -1201,12 +1201,12 @@ decl_module! { let index = index as usize; // if `index` is not duplicate, `removed` must be <= index. - ensure!(removed <= index, Error::DuplicateIndex); + ensure!(removed <= index, Error::::DuplicateIndex); // all prior removals were from before this index, since the // list is sorted. let index = index - removed; - ensure!(index < unapplied.len(), Error::InvalidSlashIndex); + ensure!(index < unapplied.len(), Error::::InvalidSlashIndex); unapplied.remove(index); } diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 16c587f9be..81066f9dd8 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -137,6 +137,7 @@ impl frame_system::Trait for Test { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const TransferFee: Balance = 0; diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 33bf860b2c..109f2e086f 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -18,7 +18,7 @@ use super::*; use mock::*; -use sp_runtime::{assert_eq_error_rate, traits::OnInitialize}; +use sp_runtime::{assert_eq_error_rate, traits::{OnInitialize, BadOrigin}}; use sp_staking::offence::OffenceDetails; use frame_support::{assert_ok, assert_noop, traits::{Currency, ReservableCurrency}}; use substrate_test_utils::assert_eq_uvec; @@ -35,7 +35,7 @@ fn force_unstake_works() { "account liquidity restrictions prevent withdrawal" ); // Force unstake requires root. - assert_noop!(Staking::force_unstake(Origin::signed(11), 11), "RequireRootOrigin".into()); + assert_noop!(Staking::force_unstake(Origin::signed(11), 11), BadOrigin); // We now force them to unstake assert_ok!(Staking::force_unstake(Origin::ROOT, 11)); // No longer bonded. @@ -142,7 +142,7 @@ fn change_controller_works() { assert_noop!( Staking::validate(Origin::signed(10), ValidatorPrefs::default()), - Error::NotController, + Error::::NotController, ); assert_ok!(Staking::validate(Origin::signed(5), ValidatorPrefs::default())); }) @@ -680,10 +680,10 @@ fn double_staking_should_fail() { // 4 = not used so far, 1 stashed => not allowed. assert_noop!( Staking::bond(Origin::signed(1), 4, arbitrary_value, - RewardDestination::default()), Error::AlreadyBonded, + RewardDestination::default()), Error::::AlreadyBonded, ); // 1 = stashed => attempting to nominate should fail. - assert_noop!(Staking::nominate(Origin::signed(1), vec![1]), Error::NotController); + assert_noop!(Staking::nominate(Origin::signed(1), vec![1]), Error::::NotController); // 2 = controller => nominating should work. assert_ok!(Staking::nominate(Origin::signed(2), vec![1])); }); @@ -705,7 +705,7 @@ fn double_controlling_should_fail() { // 2 = controller, 3 stashed (Note that 2 is reused.) => no-op assert_noop!( Staking::bond(Origin::signed(3), 2, arbitrary_value, RewardDestination::default()), - Error::AlreadyPaired, + Error::::AlreadyPaired, ); }); } @@ -1152,11 +1152,11 @@ fn too_many_unbond_calls_should_not_work() { // locked at era 1 until 4 assert_ok!(Staking::unbond(Origin::signed(10), 1)); // can't do more. - assert_noop!(Staking::unbond(Origin::signed(10), 1), Error::NoMoreChunks); + assert_noop!(Staking::unbond(Origin::signed(10), 1), Error::::NoMoreChunks); start_era(3); - assert_noop!(Staking::unbond(Origin::signed(10), 1), Error::NoMoreChunks); + assert_noop!(Staking::unbond(Origin::signed(10), 1), Error::::NoMoreChunks); // free up. assert_ok!(Staking::withdraw_unbonded(Origin::signed(10))); @@ -1422,7 +1422,7 @@ fn bond_with_no_staked_value() { // Can't bond with 1 assert_noop!( Staking::bond(Origin::signed(1), 2, 1, RewardDestination::Controller), - Error::InsufficientValue, + Error::::InsufficientValue, ); // bonded with absolute minimum value possible. assert_ok!(Staking::bond(Origin::signed(1), 2, 5, RewardDestination::Controller)); diff --git a/substrate/frame/sudo/src/lib.rs b/substrate/frame/sudo/src/lib.rs index b7486edf31..00a1b72a86 100644 --- a/substrate/frame/sudo/src/lib.rs +++ b/substrate/frame/sudo/src/lib.rs @@ -58,7 +58,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn privileged_function(origin) -> dispatch::Result { +//! pub fn privileged_function(origin) -> dispatch::DispatchResult { //! ensure_root(origin)?; //! //! // do something... @@ -87,9 +87,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_std::prelude::*; -use sp_runtime::{ - traits::{StaticLookup, Dispatchable, ModuleDispatchError}, DispatchError, -}; +use sp_runtime::{traits::{StaticLookup, Dispatchable}, DispatchError}; use frame_support::{ Parameter, decl_module, decl_event, decl_storage, decl_error, ensure, @@ -108,7 +106,7 @@ pub trait Trait: frame_system::Trait { 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; + type Error = Error; fn deposit_event() = default; @@ -125,8 +123,8 @@ decl_module! { #[weight = SimpleDispatchInfo::FreeOperational] fn sudo(origin, proposal: Box) { // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(sender == Self::key(), Error::RequireSudo); + let sender = ensure_signed(origin)?; + ensure!(sender == Self::key(), Error::::RequireSudo); let res = match proposal.dispatch(frame_system::RawOrigin::Root.into()) { Ok(_) => true, @@ -151,8 +149,8 @@ decl_module! { /// # fn set_key(origin, new: ::Source) { // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(sender == Self::key(), Error::RequireSudo); + let sender = ensure_signed(origin)?; + ensure!(sender == Self::key(), Error::::RequireSudo); let new = T::Lookup::lookup(new)?; Self::deposit_event(RawEvent::KeyChanged(Self::key())); @@ -173,8 +171,8 @@ decl_module! { #[weight = SimpleDispatchInfo::FixedOperational(0)] fn sudo_as(origin, who: ::Source, proposal: Box) { // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin).map_err(|e| e.as_str())?; - ensure!(sender == Self::key(), Error::RequireSudo); + let sender = ensure_signed(origin)?; + ensure!(sender == Self::key(), Error::::RequireSudo); let who = T::Lookup::lookup(who)?; @@ -212,7 +210,7 @@ decl_storage! { decl_error! { /// Error for the Sudo module - pub enum Error { + pub enum Error for Module { /// Sender must be the Sudo account RequireSudo, } diff --git a/substrate/frame/support/procedural/src/construct_runtime/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/mod.rs index 07b7f2cefe..8472542d16 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/mod.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/mod.rs @@ -24,6 +24,9 @@ use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::quote; use syn::{Ident, Result}; +/// The fixed name of the system module. +const SYSTEM_MODULE_NAME: &str = "System"; + pub fn construct_runtime(input: TokenStream) -> TokenStream { let definition = syn::parse_macro_input!(input as RuntimeDefinition); construct_runtime_parsed(definition) @@ -63,7 +66,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result Result Result( fn decl_all_modules<'a>( runtime: &'a Ident, - system_name: &'a Ident, module_declarations: impl Iterator, ) -> TokenStream2 { let mut types = TokenStream2::new(); @@ -330,22 +335,49 @@ fn decl_all_modules<'a>( names.push(&module_declaration.name); } // Make nested tuple structure like (((Babe, Consensus), Grandpa), ...) - let all_modules = names.iter().fold( - TokenStream2::default(), - |combined, name| quote!((#name, #combined)), - ); + // But ignore the system module. + let all_modules = names.iter() + .filter(|n| **n != SYSTEM_MODULE_NAME) + .fold(TokenStream2::default(), |combined, name| quote!((#name, #combined))); quote!( - pub type System = #system_name::Module<#runtime>; #types type AllModules = ( #all_modules ); ) } +fn decl_module_to_index<'a>( + module_declarations: impl Iterator, + num_modules: usize, + scrate: &TokenStream2, +) -> TokenStream2 { + let names = module_declarations.map(|d| &d.name); + let indices = 0..num_modules; + + quote!( + /// Provides an implementation of `ModuleToIndex` to map a module + /// to its index in the runtime. + pub struct ModuleToIndex; + + impl #scrate::traits::ModuleToIndex for ModuleToIndex { + fn module_to_index() -> Option { + let type_id = #scrate::sp_std::any::TypeId::of::(); + #( + if type_id == #scrate::sp_std::any::TypeId::of::<#names>() { + return Some(#indices) + } + )* + + None + } + } + ) +} + fn find_system_module<'a>( mut module_declarations: impl Iterator, ) -> Option<&'a Ident> { module_declarations - .find(|decl| decl.name == "System") + .find(|decl| decl.name == SYSTEM_MODULE_NAME) .map(|decl| &decl.module) } diff --git a/substrate/frame/support/procedural/tools/src/lib.rs b/substrate/frame/support/procedural/tools/src/lib.rs index 10d3c4b59b..e1e73af104 100644 --- a/substrate/frame/support/procedural/tools/src/lib.rs +++ b/substrate/frame/support/procedural/tools/src/lib.rs @@ -47,7 +47,7 @@ pub fn generate_crate_access(unique_id: &str, def_crate: &str) -> TokenStream { /// Generates the hidden includes that are required to make the macro independent from its scope. pub fn generate_hidden_includes(unique_id: &str, def_crate: &str) -> TokenStream { - if ::std::env::var("CARGO_PKG_NAME").unwrap() == def_crate { + if std::env::var("CARGO_PKG_NAME").unwrap() == def_crate { TokenStream::new() } else { let mod_name = generate_hidden_includes_mod_name(unique_id); diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index ca535a6a3b..6683aaea31 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -27,17 +27,11 @@ pub use crate::weights::{ SimpleDispatchInfo, GetDispatchInfo, DispatchInfo, WeighData, ClassifyDispatch, TransactionPriority, Weight, WeighBlock, PaysFee, }; -pub use sp_runtime::{ - traits::{Dispatchable, DispatchResult, ModuleDispatchError}, - DispatchError, -}; +pub use sp_runtime::{traits::Dispatchable, DispatchError, DispatchResult}; /// A type that cannot be instantiated. pub enum Never {} -/// Result with string error message. This exists for backward compatibility purpose. -pub type Result = DispatchResult<&'static str>; - /// Serializable version of Dispatchable. /// This value can be used as a "function" in an extrinsic. pub trait Callable { @@ -68,14 +62,14 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// /// // Private functions are dispatchable, but not available to other /// // SRML modules. -/// fn my_function(origin, var: u64) -> dispatch::Result { +/// fn my_function(origin, var: u64) -> dispatch::DispatchResult { /// // Your implementation /// Ok(()) /// } /// /// // Public functions are both dispatchable and available to other /// // SRML modules. -/// pub fn my_public_function(origin) -> dispatch::Result { +/// pub fn my_public_function(origin) -> dispatch::DispatchResult { /// // Your implementation /// Ok(()) /// } @@ -95,8 +89,8 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// /// ### Shorthand Example /// -/// The macro automatically expands a shorthand function declaration to return the `Result` type. -/// These functions are the same: +/// The macro automatically expands a shorthand function declaration to return the +/// [`DispatchResult`] type. These functions are the same: /// /// ``` /// # #[macro_use] @@ -106,7 +100,7 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// decl_module! { /// pub struct Module for enum Call where origin: T::Origin { /// -/// fn my_long_function(origin) -> dispatch::Result { +/// fn my_long_function(origin) -> dispatch::DispatchResult { /// // Your implementation /// Ok(()) /// } @@ -130,7 +124,7 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// # use frame_system::{self as system, Trait, ensure_signed, ensure_root}; /// decl_module! { /// pub struct Module for enum Call where origin: T::Origin { -/// fn my_privileged_function(origin) -> dispatch::Result { +/// fn my_privileged_function(origin) -> dispatch::DispatchResult { /// ensure_root(origin)?; /// // Your implementation /// Ok(()) @@ -1043,9 +1037,8 @@ macro_rules! decl_module { #[allow(unreachable_code)] $vis fn $name( $origin: $origin_ty $(, $param: $param_ty )* - ) -> $crate::dispatch::DispatchResult<$error_type> { - use $crate::sp_std::if_std; - if_std! { + ) -> $crate::dispatch::DispatchResult { + $crate::sp_std::if_std! { use $crate::tracing; let span = tracing::span!(tracing::Level::DEBUG, stringify!($name)); let _enter = span.enter(); @@ -1417,8 +1410,7 @@ macro_rules! decl_module { { type Trait = $trait_instance; type Origin = $origin_type; - type Error = $error_type; - fn dispatch(self, _origin: Self::Origin) -> $crate::dispatch::DispatchResult { + fn dispatch(self, _origin: Self::Origin) -> $crate::sp_runtime::DispatchResult { match self { $( $call_type::$fn_name( $( $param_name ),* ) => { @@ -1446,7 +1438,7 @@ macro_rules! decl_module { pub fn dispatch>( d: D, origin: D::Origin - ) -> $crate::dispatch::DispatchResult { + ) -> $crate::sp_runtime::DispatchResult { d.dispatch(origin) } } @@ -1514,11 +1506,10 @@ macro_rules! impl_outer_dispatch { impl $crate::dispatch::Dispatchable for $call_type { type Origin = $origin; type Trait = $call_type; - type Error = $crate::dispatch::DispatchError; fn dispatch( self, origin: $origin, - ) -> $crate::dispatch::DispatchResult<$crate::dispatch::DispatchError> { + ) -> $crate::sp_runtime::DispatchResult { $crate::impl_outer_dispatch! { @DISPATCH_MATCH self @@ -1565,11 +1556,7 @@ macro_rules! impl_outer_dispatch { $origin { $( $generated )* - $call_type::$name(call) => call.dispatch($origin).map_err(|e| { - let mut error: $crate::dispatch::DispatchError = e.into(); - error.module = Some($index); - error - }), + $call_type::$name(call) => call.dispatch($origin), } $index + 1; $( $rest ),* @@ -1895,13 +1882,13 @@ mod tests { } pub mod system { - use super::Result; + use super::*; pub trait Trait { type AccountId; } - pub fn ensure_root(_: R) -> Result { + pub fn ensure_root(_: R) -> DispatchResult { Ok(()) } } @@ -1917,13 +1904,13 @@ mod tests { decl_module! { pub struct Module for enum Call where origin: T::Origin, T::AccountId: From { /// Hi, this is a comment. - fn aux_0(_origin) -> Result { unreachable!() } - fn aux_1(_origin, #[compact] _data: u32,) -> Result { unreachable!() } - fn aux_2(_origin, _data: i32, _data2: String) -> Result { unreachable!() } + fn aux_0(_origin) -> DispatchResult { unreachable!() } + fn aux_1(_origin, #[compact] _data: u32,) -> DispatchResult { unreachable!() } + fn aux_2(_origin, _data: i32, _data2: String) -> DispatchResult { unreachable!() } #[weight = SimpleDispatchInfo::FixedNormal(3)] - fn aux_3(_origin) -> Result { unreachable!() } - fn aux_4(_origin, _data: i32) -> Result { unreachable!() } - fn aux_5(_origin, _data: i32, #[compact] _data2: u32,) -> Result { unreachable!() } + fn aux_3(_origin) -> DispatchResult { unreachable!() } + fn aux_4(_origin, _data: i32) -> DispatchResult { unreachable!() } + fn aux_5(_origin, _data: i32, #[compact] _data2: u32,) -> DispatchResult { unreachable!() } #[weight = SimpleDispatchInfo::FixedNormal(7)] fn on_initialize(n: T::BlockNumber,) { if n.into() == 42 { panic!("on_initialize") } } diff --git a/substrate/frame/support/src/error.rs b/substrate/frame/support/src/error.rs index d256f0d58b..0120b6da60 100644 --- a/substrate/frame/support/src/error.rs +++ b/substrate/frame/support/src/error.rs @@ -17,18 +17,19 @@ //! Macro for declaring a module error. #[doc(hidden)] -pub use sp_runtime::traits::LookupError; +pub use sp_runtime::traits::{LookupError, BadOrigin}; +#[doc(hidden)] pub use frame_metadata::{ModuleErrorMetadata, ErrorMetadata, DecodeDifferent}; /// Declare an error type for a runtime module. /// -/// The generated error type inherently has the variants `Other` and `CannotLookup`. `Other` can -/// hold any `&'static str` error message and is present for convenience/backward compatibility. -/// The `CannotLookup` variant indicates that some lookup could not be done. For both variants the -/// error type implements `From<&'static str>` and `From` to make them usable with the -/// try operator. +/// `decl_error!` supports only variants that do not hold any data. The dispatchable +/// functions return [`DispatchResult`](sp_runtime::DispatchResult). The error type +/// implements `From for DispatchResult` to make the error type usable as error +/// in the dispatchable functions. /// -/// `decl_error!` supports only variants that do not hold any data. +/// It is required that the error type is registed in `decl_module!` to make the error +/// exported in the metadata. /// /// # Usage /// @@ -36,7 +37,7 @@ pub use frame_metadata::{ModuleErrorMetadata, ErrorMetadata, DecodeDifferent}; /// # use frame_support::{decl_error, decl_module}; /// decl_error! { /// /// Errors that can occur in my module. -/// pub enum MyError { +/// pub enum MyError for Module { /// /// Hey this is an error message that indicates bla. /// MyCoolErrorMessage, /// /// You are just not cool enough for my module! @@ -44,27 +45,36 @@ pub use frame_metadata::{ModuleErrorMetadata, ErrorMetadata, DecodeDifferent}; /// } /// } /// -/// # use frame_system::{self as system, Trait, ensure_signed}; +/// # use frame_system::{self as system, Trait}; /// -/// // You need to register the error type in `decl_module!` as well. +/// // You need to register the error type in `decl_module!` as well to make the error +/// // exported in the metadata. /// /// decl_module! { /// pub struct Module for enum Call where origin: T::Origin { -/// type Error = MyError; +/// type Error = MyError; /// -/// fn do_something(origin) -> Result<(), MyError> { -/// Err(MyError::YouAreNotCoolEnough) +/// fn do_something(origin) -> frame_support::dispatch::DispatchResult { +/// Err(MyError::::YouAreNotCoolEnough.into()) /// } /// } /// } /// /// # fn main() {} /// ``` +/// +/// For instantiable modules you also need to give the instance generic type and bound to the +/// error declaration. #[macro_export] macro_rules! decl_error { ( $(#[$attr:meta])* - pub enum $error:ident { + pub enum $error:ident + for $module:ident< + $generic:ident: $trait:path + $(, $inst_generic:ident: $instance:path)? + > + { $( $( #[doc = $doc_attr:tt] )* $name:ident @@ -72,33 +82,42 @@ macro_rules! decl_error { $(,)? } ) => { - #[derive(Clone, PartialEq, Eq, $crate::RuntimeDebug)] $(#[$attr])* - pub enum $error { - Other(&'static str), - CannotLookup, + pub enum $error<$generic: $trait $(, $inst_generic: $instance)?> { + #[doc(hidden)] + __Ignore( + $crate::sp_std::marker::PhantomData<($generic $(, $inst_generic)?)>, + $crate::dispatch::Never, + ), $( $( #[doc = $doc_attr] )* $name ),* } - impl $crate::dispatch::ModuleDispatchError for $error { + impl<$generic: $trait $(, $inst_generic: $instance)?> $crate::sp_std::fmt::Debug + for $error<$generic $(, $inst_generic)?> + { + fn fmt(&self, f: &mut $crate::sp_std::fmt::Formatter<'_>) -> $crate::sp_std::fmt::Result { + f.write_str(self.as_str()) + } + } + + impl<$generic: $trait $(, $inst_generic: $instance)?> $error<$generic $(, $inst_generic)?> { fn as_u8(&self) -> u8 { $crate::decl_error! { @GENERATE_AS_U8 self $error {} - 2, + 0, $( $name ),* } } fn as_str(&self) -> &'static str { match self { - $error::Other(err) => err, - $error::CannotLookup => "Can not lookup", + Self::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"), $( $error::$name => stringify!($name), )* @@ -106,33 +125,33 @@ macro_rules! decl_error { } } - impl From<&'static str> for $error { - fn from(val: &'static str) -> $error { - $error::Other(val) - } - } - - impl From<$crate::error::LookupError> for $error { - fn from(_: $crate::error::LookupError) -> $error { - $error::CannotLookup - } - } - - impl From<$error> for &'static str { - fn from(err: $error) -> &'static str { - use $crate::dispatch::ModuleDispatchError; + impl<$generic: $trait $(, $inst_generic: $instance)?> From<$error<$generic $(, $inst_generic)?>> + for &'static str + { + fn from(err: $error<$generic $(, $inst_generic)?>) -> &'static str { err.as_str() } } - impl Into<$crate::dispatch::DispatchError> for $error { - fn into(self) -> $crate::dispatch::DispatchError { - use $crate::dispatch::ModuleDispatchError; - $crate::dispatch::DispatchError::new(None, self.as_u8(), Some(self.as_str())) + impl<$generic: $trait $(, $inst_generic: $instance)?> From<$error<$generic $(, $inst_generic)?>> + for $crate::sp_runtime::DispatchError + { + fn from(err: $error<$generic $(, $inst_generic)?>) -> Self { + let index = <$generic::ModuleToIndex as $crate::traits::ModuleToIndex> + ::module_to_index::<$module<$generic $(, $inst_generic)?>>() + .expect("Every active module has an index in the runtime; qed") as u8; + + $crate::sp_runtime::DispatchError::Module { + index, + error: err.as_u8(), + message: Some(err.as_str()), + } } } - impl $crate::error::ModuleErrorMetadata for $error { + impl<$generic: $trait $(, $inst_generic: $instance)?> $crate::error::ModuleErrorMetadata + for $error<$generic $(, $inst_generic)?> + { fn metadata() -> &'static [$crate::error::ErrorMetadata] { &[ $( @@ -174,8 +193,7 @@ macro_rules! decl_error { $index:expr, ) => { match $self { - $error::Other(_) => 0, - $error::CannotLookup => 1, + $error::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"), $( $generated )* } } diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index cf6b2472f0..f0357cff2f 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -124,7 +124,7 @@ pub use frame_support_procedural::{decl_storage, construct_runtime}; #[macro_export] macro_rules! fail { ( $y:expr ) => {{ - return Err($y); + return Err($y.into()); }} } @@ -168,7 +168,7 @@ macro_rules! assert_noop { #[cfg(feature = "std")] macro_rules! assert_err { ( $x:expr , $y:expr $(,)? ) => { - assert_eq!($x, Err($y)); + assert_eq!($x, Err($y.into())); } } diff --git a/substrate/frame/support/src/metadata.rs b/substrate/frame/support/src/metadata.rs index c27cc4ac73..ad6a5c797c 100644 --- a/substrate/frame/support/src/metadata.rs +++ b/substrate/frame/support/src/metadata.rs @@ -240,13 +240,14 @@ mod tests { mod system { use super::*; - pub trait Trait { + pub trait Trait: 'static { const ASSOCIATED_CONST: u64 = 500; type Origin: Into, Self::Origin>> + From>; type AccountId: From + Encode; type BlockNumber: From + Encode; type SomeValue: Get; + type ModuleToIndex: crate::traits::ModuleToIndex; } decl_module! { @@ -286,10 +287,8 @@ mod tests { mod event_module { use crate::dispatch::DispatchResult; - pub trait Trait { - type Origin; + pub trait Trait: super::system::Trait { type Balance; - type BlockNumber; } decl_event!( @@ -302,14 +301,14 @@ mod tests { decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + type Error = Error; - fn aux_0(_origin) -> DispatchResult { unreachable!() } + fn aux_0(_origin) -> DispatchResult { unreachable!() } } } crate::decl_error! { - pub enum Error { + pub enum Error for Module { /// Some user input error UserInputError, /// Something bad happened @@ -372,9 +371,7 @@ mod tests { } impl event_module::Trait for TestRuntime { - type Origin = Origin; type Balance = u32; - type BlockNumber = u32; } impl event_module2::Trait for TestRuntime { @@ -392,6 +389,7 @@ mod tests { type AccountId = u32; type BlockNumber = u32; type SomeValue = SystemValue; + type ModuleToIndex = (); } impl_runtime_metadata!( diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index afc4a4e4f4..379f964d27 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -22,7 +22,7 @@ use sp_std::{prelude::*, result, marker::PhantomData, ops::Div, fmt::Debug}; use codec::{FullCodec, Codec, Encode, Decode}; use sp_core::u32_trait::Value as U32; use sp_runtime::{ - ConsensusEngineId, + ConsensusEngineId, DispatchResult, DispatchError, traits::{MaybeSerializeDeserialize, SimpleArithmetic, Saturating}, }; @@ -386,7 +386,7 @@ pub trait Currency { _amount: Self::Balance, reasons: WithdrawReasons, new_balance: Self::Balance, - ) -> result::Result<(), &'static str>; + ) -> DispatchResult; // PUBLIC MUTABLES (DANGEROUS) @@ -399,7 +399,7 @@ pub trait Currency { dest: &AccountId, value: Self::Balance, existence_requirement: ExistenceRequirement, - ) -> result::Result<(), &'static str>; + ) -> DispatchResult; /// Deducts up to `value` from the combined balance of `who`, preferring to deduct from the /// free balance. This function cannot fail. @@ -419,7 +419,7 @@ pub trait Currency { fn deposit_into_existing( who: &AccountId, value: Self::Balance - ) -> result::Result; + ) -> result::Result; /// Similar to deposit_creating, only accepts a `NegativeImbalance` and returns nothing on /// success. @@ -465,7 +465,7 @@ pub trait Currency { value: Self::Balance, reasons: WithdrawReasons, liveness: ExistenceRequirement, - ) -> result::Result; + ) -> result::Result; /// Similar to withdraw, only accepts a `PositiveImbalance` and returns nothing on success. fn settle( @@ -528,7 +528,7 @@ pub trait ReservableCurrency: Currency { /// /// If the free balance is lower than `value`, then no funds will be moved and an `Err` will /// be returned to notify of this. This is different behavior than `unreserve`. - fn reserve(who: &AccountId, value: Self::Balance) -> result::Result<(), &'static str>; + fn reserve(who: &AccountId, value: Self::Balance) -> DispatchResult; /// Moves up to `value` from reserved balance to free balance. This function cannot fail. /// @@ -552,7 +552,7 @@ pub trait ReservableCurrency: Currency { slashed: &AccountId, beneficiary: &AccountId, value: Self::Balance - ) -> result::Result; + ) -> result::Result; } /// An identifier for a lock. Used for disambiguating different locks so that @@ -619,7 +619,7 @@ pub trait VestingCurrency: Currency { locked: Self::Balance, per_block: Self::Balance, starting_block: Self::Moment, - ) -> result::Result<(), &'static str>; + ) -> DispatchResult; /// Remove a vesting schedule for a given account. fn remove_vesting_schedule(who: &AccountId); @@ -777,3 +777,15 @@ pub trait ValidatorRegistration { /// module fn is_registered(id: &ValidatorId) -> bool; } + +/// Something that can convert a given module into the index of the module in the runtime. +/// +/// The index of a module is determined by the position it appears in `construct_runtime!`. +pub trait ModuleToIndex { + /// Convert the given module `M` into an index. + fn module_to_index() -> Option; +} + +impl ModuleToIndex for () { + fn module_to_index() -> Option { Some(0) } +} diff --git a/substrate/frame/support/test/tests/decl_error.rs b/substrate/frame/support/test/tests/decl_error.rs new file mode 100644 index 0000000000..42a799caad --- /dev/null +++ b/substrate/frame/support/test/tests/decl_error.rs @@ -0,0 +1,135 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +#![recursion_limit="128"] + +use sp_runtime::{generic, traits::{BlakeTwo256, Block as _, Verify}, DispatchError}; +use sp_core::{H256, sr25519}; + +mod system; + +pub trait Currency {} + +mod module1 { + use super::*; + + pub trait Trait: system::Trait {} + + frame_support::decl_module! { + pub struct Module, I: Instance = DefaultInstance> for enum Call + where origin: ::Origin + { + pub fn fail(_origin) -> frame_support::dispatch::DispatchResult { + Err(Error::::Something.into()) + } + } + } + + frame_support::decl_error! { + pub enum Error for Module, I: Instance> { + Something + } + } + + frame_support::decl_storage! { + trait Store for Module, I: Instance=DefaultInstance> as Module {} + } +} + +mod module2 { + use super::*; + + pub trait Trait: system::Trait {} + + frame_support::decl_module! { + pub struct Module for enum Call + where origin: ::Origin + { + pub fn fail(_origin) -> frame_support::dispatch::DispatchResult { + Err(Error::::Something.into()) + } + } + } + + frame_support::decl_error! { + pub enum Error for Module { + Something + } + } + + frame_support::decl_storage! { + trait Store for Module as Module {} + } +} + +impl module1::Trait for Runtime {} +impl module1::Trait for Runtime {} +impl module2::Trait for Runtime {} + +pub type Signature = sr25519::Signature; +pub type AccountId = ::Signer; +pub type BlockNumber = u64; +pub type Index = u64; + +impl system::Trait for Runtime { + type Hash = H256; + type Origin = Origin; + type BlockNumber = BlockNumber; + type AccountId = AccountId; + type Event = Event; + type ModuleToIndex = ModuleToIndex; +} + +frame_support::construct_runtime!( + pub enum Runtime where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Module, Call, Event}, + Module1_1: module1::::{Module, Call, Storage}, + Module2: module2::{Module, Call, Storage}, + Module1_2: module1::::{Module, Call, Storage}, + } +); + +pub type Header = generic::Header; +pub type Block = generic::Block; +pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; + +#[test] +fn check_module1_1_error_type() { + assert_eq!( + Module1_1::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 1, error: 0, message: Some("Something") }), + ); +} + +#[test] +fn check_module1_2_error_type() { + assert_eq!( + Module1_2::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 3, error: 0, message: Some("Something") }), + ); +} + +#[test] +fn check_module2_error_type() { + assert_eq!( + Module2::fail(system::Origin::::Root.into()), + Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), + ); +} diff --git a/substrate/frame/support/test/tests/instance.rs b/substrate/frame/support/test/tests/instance.rs index 835c082a66..cd14736266 100644 --- a/substrate/frame/support/test/tests/instance.rs +++ b/substrate/frame/support/test/tests/instance.rs @@ -238,6 +238,7 @@ impl system::Trait for Runtime { type BlockNumber = BlockNumber; type AccountId = AccountId; type Event = Event; + type ModuleToIndex = (); } frame_support::construct_runtime!( diff --git a/substrate/frame/support/test/tests/issue2219.rs b/substrate/frame/support/test/tests/issue2219.rs index 4c9731b498..f6679f3498 100644 --- a/substrate/frame/support/test/tests/issue2219.rs +++ b/substrate/frame/support/test/tests/issue2219.rs @@ -160,6 +160,7 @@ impl system::Trait for Runtime { type BlockNumber = BlockNumber; type AccountId = AccountId; type Event = Event; + type ModuleToIndex = (); } impl module::Trait for Runtime {} diff --git a/substrate/frame/support/test/tests/reserved_keyword/on_initialize.rs b/substrate/frame/support/test/tests/reserved_keyword/on_initialize.rs index 9a7ffcf067..e389529bca 100644 --- a/substrate/frame/support/test/tests/reserved_keyword/on_initialize.rs +++ b/substrate/frame/support/test/tests/reserved_keyword/on_initialize.rs @@ -12,14 +12,14 @@ macro_rules! reserved { pub mod system { use frame_support::dispatch; - pub fn ensure_root(_: R) -> dispatch::Result { + pub fn ensure_root(_: R) -> dispatch::DispatchResult { Ok(()) } } frame_support::decl_module! { pub struct Module for enum Call where origin: T::Origin { - fn $reserved(_origin) -> dispatch::Result { unreachable!() } + fn $reserved(_origin) -> dispatch::DispatchResult { unreachable!() } } } } diff --git a/substrate/frame/support/test/tests/system.rs b/substrate/frame/support/test/tests/system.rs index e7da24bbab..83786538e6 100644 --- a/substrate/frame/support/test/tests/system.rs +++ b/substrate/frame/support/test/tests/system.rs @@ -8,6 +8,7 @@ pub trait Trait: 'static + Eq + Clone { type Hash; type AccountId: Encode + EncodeLike + Decode; type Event: From; + type ModuleToIndex: frame_support::traits::ModuleToIndex; } frame_support::decl_module! { @@ -28,7 +29,7 @@ frame_support::decl_event!( ); frame_support::decl_error! { - pub enum Error { + pub enum Error for Module { /// Test error documentation TestError, /// Error documentation diff --git a/substrate/frame/system/benches/bench.rs b/substrate/frame/system/benches/bench.rs index 5102c56adf..21e9dbb0f2 100644 --- a/substrate/frame/system/benches/bench.rs +++ b/substrate/frame/system/benches/bench.rs @@ -74,6 +74,7 @@ impl system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl module::Trait for Runtime { diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 1acdc30570..f713811f21 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -75,7 +75,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn system_module_example(origin) -> dispatch::Result { +//! pub fn system_module_example(origin) -> dispatch::DispatchResult { //! let _sender = ensure_signed(origin)?; //! let _extrinsic_count = >::extrinsic_count(); //! let _parent_hash = >::parent_hash(); @@ -105,7 +105,7 @@ use sp_runtime::{ }, traits::{ self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, Lookup, LookupError, - SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, SaturatedConversion, + SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, BadOrigin, SaturatedConversion, MaybeSerialize, MaybeSerializeDeserialize, StaticLookup, One, Bounded, }, }; @@ -113,7 +113,7 @@ use sp_runtime::{ use sp_core::storage::well_known_keys; use frame_support::{ decl_module, decl_event, decl_storage, decl_error, storage, Parameter, - traits::{Contains, Get}, + traits::{Contains, Get, ModuleToIndex}, weights::{Weight, DispatchInfo, DispatchClass, SimpleDispatchInfo}, }; use codec::{Encode, Decode}; @@ -219,6 +219,12 @@ pub trait Trait: 'static + Eq + Clone { /// Get the chain's current version. type Version: Get; + + /// Convert a module to its index in the runtime. + /// + /// Expects the `ModuleToIndex` type that is being generated by `construct_runtime!` in the + /// runtime. For tests it is okay to use `()` as type (returns `0` for each input). + type ModuleToIndex: ModuleToIndex; } pub type DigestOf = generic::Digest<::Hash>; @@ -229,7 +235,7 @@ pub type KeyValue = (Vec, Vec); decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + type Error = Error; /// A big dispatch that will disallow any other transaction to be included. // TODO: this must be preferable available for testing really (not possible at the moment). @@ -319,11 +325,7 @@ decl_event!( decl_error! { /// Error for the System module - pub enum Error { - RequireSignedOrigin, - RequireRootOrigin, - RequireNoOrigin, - } + pub enum Error for Module {} } /// Origin for the System module. @@ -502,32 +504,32 @@ impl EnsureOrigin for EnsureNever { /// Ensure that the origin `o` represents a signed extrinsic (i.e. transaction). /// Returns `Ok` with the account that signed the extrinsic or an `Err` otherwise. -pub fn ensure_signed(o: OuterOrigin) -> Result +pub fn ensure_signed(o: OuterOrigin) -> Result where OuterOrigin: Into, OuterOrigin>> { match o.into() { Ok(RawOrigin::Signed(t)) => Ok(t), - _ => Err(Error::RequireSignedOrigin), + _ => Err(BadOrigin), } } /// Ensure that the origin `o` represents the root. Returns `Ok` or an `Err` otherwise. -pub fn ensure_root(o: OuterOrigin) -> Result<(), Error> +pub fn ensure_root(o: OuterOrigin) -> Result<(), BadOrigin> where OuterOrigin: Into, OuterOrigin>> { match o.into() { Ok(RawOrigin::Root) => Ok(()), - _ => Err(Error::RequireRootOrigin), + _ => Err(BadOrigin), } } /// Ensure that the origin `o` represents an unsigned extrinsic. Returns `Ok` or an `Err` otherwise. -pub fn ensure_none(o: OuterOrigin) -> Result<(), Error> +pub fn ensure_none(o: OuterOrigin) -> Result<(), BadOrigin> where OuterOrigin: Into, OuterOrigin>> { match o.into() { Ok(RawOrigin::None) => Ok(()), - _ => Err(Error::RequireNoOrigin), + _ => Err(BadOrigin), } } @@ -1175,6 +1177,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } impl From for u16 { @@ -1230,7 +1233,7 @@ mod tests { System::initialize(&2, &[0u8; 32].into(), &[0u8; 32].into(), &Default::default()); System::deposit_event(42u16); System::note_applied_extrinsic(&Ok(()), 0, Default::default()); - System::note_applied_extrinsic(&Err(DispatchError::new(Some(1), 2, None)), 0, Default::default()); + System::note_applied_extrinsic(&Err(DispatchError::BadOrigin), 0, Default::default()); System::note_finished_extrinsics(); System::deposit_event(3u16); System::finalize(); diff --git a/substrate/frame/timestamp/src/lib.rs b/substrate/frame/timestamp/src/lib.rs index f15c0ed627..449e509c23 100644 --- a/substrate/frame/timestamp/src/lib.rs +++ b/substrate/frame/timestamp/src/lib.rs @@ -69,7 +69,7 @@ //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { -//! pub fn get_time(origin) -> dispatch::Result { +//! pub fn get_time(origin) -> dispatch::DispatchResult { //! let _sender = ensure_signed(origin)?; //! let _now = >::get(); //! Ok(()) @@ -273,6 +273,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const MinimumPeriod: u64 = 5; diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs index 562e7fe21f..8feb3c6b29 100644 --- a/substrate/frame/transaction-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/src/lib.rs @@ -291,6 +291,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index 49de399b50..ee0e1adc5e 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/frame/treasury/src/lib.rs @@ -66,9 +66,7 @@ use frame_support::traits::{ ReservableCurrency, WithdrawReason }; use sp_runtime::{Permill, ModuleId}; -use sp_runtime::traits::{ - Zero, EnsureOrigin, StaticLookup, AccountIdConversion, Saturating, ModuleDispatchError, -}; +use sp_runtime::traits::{Zero, EnsureOrigin, StaticLookup, AccountIdConversion, Saturating}; use frame_support::weights::SimpleDispatchInfo; use codec::{Encode, Decode}; use frame_system::{self as system, ensure_signed}; @@ -126,7 +124,7 @@ decl_module! { /// Percentage of spare funds (if any) that are burnt per spend period. const Burn: Permill = T::Burn::get(); - type Error = Error; + type Error = Error; fn deposit_event() = default; @@ -145,12 +143,12 @@ decl_module! { #[compact] value: BalanceOf, beneficiary: ::Source ) { - let proposer = ensure_signed(origin).map_err(|e| e.as_str())?; + let proposer = ensure_signed(origin)?; let beneficiary = T::Lookup::lookup(beneficiary)?; let bond = Self::calculate_bond(value); T::Currency::reserve(&proposer, bond) - .map_err(|_| Error::InsufficientProposersBalance)?; + .map_err(|_| Error::::InsufficientProposersBalance)?; let c = Self::proposal_count(); ProposalCount::put(c + 1); @@ -169,7 +167,7 @@ decl_module! { #[weight = SimpleDispatchInfo::FixedOperational(100_000)] fn reject_proposal(origin, #[compact] proposal_id: ProposalIndex) { T::RejectOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?; - let proposal = >::take(proposal_id).ok_or(Error::InvalidProposalIndex)?; + let proposal = >::take(proposal_id).ok_or(Error::::InvalidProposalIndex)?; let value = proposal.bond; let imbalance = T::Currency::slash_reserved(&proposal.proposer, value).0; @@ -188,7 +186,7 @@ decl_module! { fn approve_proposal(origin, #[compact] proposal_id: ProposalIndex) { T::ApproveOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?; - ensure!(>::exists(proposal_id), Error::InvalidProposalIndex); + ensure!(>::exists(proposal_id), Error::::InvalidProposalIndex); Approvals::mutate(|v| v.push(proposal_id)); } @@ -257,7 +255,7 @@ decl_event!( decl_error! { /// Error for the treasury module. - pub enum Error { + pub enum Error for Module { /// Proposer's balance is too low. InsufficientProposersBalance, /// No proposal at that index. @@ -398,6 +396,7 @@ mod tests { type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 1; @@ -484,7 +483,10 @@ mod tests { #[test] fn spend_proposal_fails_when_proposer_poor() { new_test_ext().execute_with(|| { - assert_noop!(Treasury::propose_spend(Origin::signed(2), 100, 3), Error::InsufficientProposersBalance); + assert_noop!( + Treasury::propose_spend(Origin::signed(2), 100, 3), + Error::::InsufficientProposersBalance, + ); }); } @@ -536,21 +538,21 @@ mod tests { assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3)); assert_ok!(Treasury::reject_proposal(Origin::ROOT, 0)); - assert_noop!(Treasury::reject_proposal(Origin::ROOT, 0), Error::InvalidProposalIndex); + assert_noop!(Treasury::reject_proposal(Origin::ROOT, 0), Error::::InvalidProposalIndex); }); } #[test] fn reject_non_existant_spend_proposal_fails() { new_test_ext().execute_with(|| { - assert_noop!(Treasury::reject_proposal(Origin::ROOT, 0), Error::InvalidProposalIndex); + assert_noop!(Treasury::reject_proposal(Origin::ROOT, 0), Error::::InvalidProposalIndex); }); } #[test] fn accept_non_existant_spend_proposal_fails() { new_test_ext().execute_with(|| { - assert_noop!(Treasury::approve_proposal(Origin::ROOT, 0), Error::InvalidProposalIndex); + assert_noop!(Treasury::approve_proposal(Origin::ROOT, 0), Error::::InvalidProposalIndex); }); } @@ -561,7 +563,7 @@ mod tests { assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3)); assert_ok!(Treasury::reject_proposal(Origin::ROOT, 0)); - assert_noop!(Treasury::approve_proposal(Origin::ROOT, 0), Error::InvalidProposalIndex); + assert_noop!(Treasury::approve_proposal(Origin::ROOT, 0), Error::::InvalidProposalIndex); }); } diff --git a/substrate/frame/utility/src/lib.rs b/substrate/frame/utility/src/lib.rs index c5400b891e..7f8552c402 100644 --- a/substrate/frame/utility/src/lib.rs +++ b/substrate/frame/utility/src/lib.rs @@ -68,7 +68,7 @@ mod tests { weights::Weight }; use sp_core::H256; - use sp_runtime::{Perbill, traits::{BlakeTwo256, IdentityLookup}, testing::Header}; + use sp_runtime::{Perbill, traits::{BlakeTwo256, IdentityLookup, BadOrigin}, testing::Header}; impl_outer_origin! { pub enum Origin for Test where system = frame_system {} @@ -108,6 +108,7 @@ mod tests { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } parameter_types! { pub const ExistentialDeposit: u64 = 0; @@ -146,10 +147,13 @@ mod tests { new_test_ext().execute_with(|| { assert_eq!(Balances::free_balance(1), 10); assert_eq!(Balances::free_balance(2), 0); - assert_noop!(Utility::batch(Origin::signed(1), vec![ - Call::Balances(pallet_balances::Call::force_transfer(1, 2, 5)), - Call::Balances(pallet_balances::Call::force_transfer(1, 2, 5)) - ]), "RequireRootOrigin"); + assert_noop!( + Utility::batch(Origin::signed(1), vec![ + Call::Balances(pallet_balances::Call::force_transfer(1, 2, 5)), + Call::Balances(pallet_balances::Call::force_transfer(1, 2, 5)) + ]), + BadOrigin, + ); assert_ok!(Utility::batch(Origin::ROOT, vec![ Call::Balances(pallet_balances::Call::force_transfer(1, 2, 5)), Call::Balances(pallet_balances::Call::force_transfer(1, 2, 5)) diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index db039d5b75..986b4bf660 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/substrate/primitives/runtime/src/lib.rs @@ -358,26 +358,57 @@ impl From for DispatchOutcome { } } +/// Result of a module function call; either nothing (functions are only called for "side effects") +/// or an error message. +pub type DispatchResult = sp_std::result::Result<(), DispatchError>; + +/// Reason why a dispatch call failed #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Serialize))] -/// Reason why a dispatch call failed -pub struct DispatchError { - /// Module index, matching the metadata module index - pub module: Option, - /// Module specific error value - pub error: u8, - /// Optional error message. - #[codec(skip)] - pub message: Option<&'static str>, +pub enum DispatchError { + /// Some error occurred. + Other(#[codec(skip)] &'static str), + /// Failed to lookup some data. + CannotLookup, + /// A bad origin. + BadOrigin, + /// A custom error in a module + Module { + /// Module index, matching the metadata module index + index: u8, + /// Module specific error value + error: u8, + /// Optional error message. + #[codec(skip)] + message: Option<&'static str>, + }, } -impl DispatchError { - /// Create a new instance of `DispatchError`. - pub fn new(module: Option, error: u8, message: Option<&'static str>) -> Self { - Self { - module, - error, - message, +impl From for DispatchError { + fn from(_: crate::traits::LookupError) -> Self { + Self::CannotLookup + } +} + +impl From for DispatchError { + fn from(_: crate::traits::BadOrigin) -> Self { + Self::BadOrigin + } +} + +impl From<&'static str> for DispatchError { + fn from(err: &'static str) -> DispatchError { + DispatchError::Other(err) + } +} + +impl Into<&'static str> for DispatchError { + fn into(self) -> &'static str { + match self { + Self::Other(msg) => msg, + Self::CannotLookup => "Can not lookup", + Self::BadOrigin => "Bad origin", + Self::Module { message, .. } => message.unwrap_or("Unknown module error"), } } } @@ -385,29 +416,18 @@ impl DispatchError { impl traits::Printable for DispatchError { fn print(&self) { "DispatchError".print(); - if let Some(module) = self.module { - module.print(); + match self { + Self::Other(err) => err.print(), + Self::CannotLookup => "Can not lookup".print(), + Self::BadOrigin => "Bad origin".print(), + Self::Module { index, error, message } => { + index.print(); + error.print(); + if let Some(msg) = message { + msg.print(); + } + } } - self.error.print(); - if let Some(msg) = self.message { - msg.print(); - } - } -} - -impl traits::ModuleDispatchError for &'static str { - fn as_u8(&self) -> u8 { - 0 - } - - fn as_str(&self) -> &'static str { - self - } -} - -impl From<&'static str> for DispatchError { - fn from(err: &'static str) -> DispatchError { - DispatchError::new(None, 0, Some(err)) } } @@ -668,18 +688,18 @@ mod tests { #[test] fn dispatch_error_encoding() { - let error = DispatchError { - module: Some(1), + let error = DispatchError::Module { + index: 1, error: 2, message: Some("error message"), }; let encoded = error.encode(); let decoded = DispatchError::decode(&mut &encoded[..]).unwrap(); - assert_eq!(encoded, vec![1, 1, 2]); + assert_eq!(encoded, vec![3, 1, 2]); assert_eq!( decoded, - DispatchError { - module: Some(1), + DispatchError::Module { + index: 1, error: 2, message: None, }, diff --git a/substrate/primitives/runtime/src/traits.rs b/substrate/primitives/runtime/src/traits.rs index aee19ea0cc..22cd2814e7 100644 --- a/substrate/primitives/runtime/src/traits.rs +++ b/substrate/primitives/runtime/src/traits.rs @@ -136,11 +136,11 @@ impl< /// An error type that indicates that the origin is invalid. #[derive(Encode, Decode)] -pub struct InvalidOrigin; +pub struct BadOrigin; -impl From for &'static str { - fn from(_: InvalidOrigin) -> &'static str { - "Invalid origin" +impl From for &'static str { + fn from(_: BadOrigin) -> &'static str { + "Bad origin" } } @@ -149,8 +149,8 @@ pub trait EnsureOrigin { /// A return type. type Success; /// Perform the origin check. - fn ensure_origin(o: OuterOrigin) -> result::Result { - Self::try_origin(o).map_err(|_| InvalidOrigin) + fn ensure_origin(o: OuterOrigin) -> result::Result { + Self::try_origin(o).map_err(|_| BadOrigin) } /// Perform the origin check. fn try_origin(o: OuterOrigin) -> result::Result; @@ -668,10 +668,6 @@ impl Checkable for T { } } -/// Result of a module function call; either nothing (functions are only called for "side effects") -/// or an error message. -pub type DispatchResult = result::Result<(), Error>; - /// A lazy call (module function and argument values) that can be executed via its `dispatch` /// method. pub trait Dispatchable { @@ -681,10 +677,8 @@ pub trait Dispatchable { type Origin; /// ... type Trait; - /// The error type returned by this dispatchable. - type Error: Into; /// Actually dispatch this call and result the result of it. - fn dispatch(self, origin: Self::Origin) -> DispatchResult; + fn dispatch(self, origin: Self::Origin) -> crate::DispatchResult; } /// Means by which a transaction may be extended. This type embodies both the data and the logic @@ -789,17 +783,6 @@ pub trait SignedExtension: Codec + Debug + Sync + Send + Clone + Eq + PartialEq fn post_dispatch(_pre: Self::Pre, _info: Self::DispatchInfo, _len: usize) { } } -/// An error that is returned by a dispatchable function of a module. -pub trait ModuleDispatchError { - /// Convert this error to a `u8`. - /// - /// The `u8` corresponds to the index of the variant in the error enum. - fn as_u8(&self) -> u8; - - /// Convert the error to a `&'static str`. - fn as_str(&self) -> &'static str; -} - #[impl_for_tuples(1, 12)] impl SignedExtension for Tuple { for_tuples!( where #( Tuple: SignedExtension )* ); diff --git a/substrate/test-utils/runtime/src/lib.rs b/substrate/test-utils/runtime/src/lib.rs index 2a0cfe454f..e3b34881c3 100644 --- a/substrate/test-utils/runtime/src/lib.rs +++ b/substrate/test-utils/runtime/src/lib.rs @@ -374,6 +374,7 @@ impl frame_system::Trait for Runtime { type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; type Version = (); + type ModuleToIndex = (); } impl pallet_timestamp::Trait for Runtime {