From ba48a0a4c27dd4a1a89faf3fef60597b632930a0 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Mon, 17 Jun 2019 21:46:13 +1200 Subject: [PATCH 01/40] srml-system checks --- .../client/src/block_builder/block_builder.rs | 6 +- .../src/generic/unchecked_extrinsic.rs | 8 +- .../unchecked_mortal_compact_extrinsic.rs | 10 +- .../src/generic/unchecked_mortal_extrinsic.rs | 10 +- core/sr-primitives/src/lib.rs | 63 ++++----- core/sr-primitives/src/testing.rs | 1 + core/sr-primitives/src/traits.rs | 36 ++++- core/test-runtime/src/system.rs | 8 +- node/executor/src/lib.rs | 4 +- srml/balances/src/lib.rs | 1 + srml/executive/src/lib.rs | 6 +- srml/support/src/dispatch.rs | 123 +++++++++++++++++- srml/support/src/error.rs | 62 +++++++++ srml/support/src/lib.rs | 2 + srml/support/src/runtime.rs | 8 ++ srml/system/src/lib.rs | 60 +++++++-- 16 files changed, 329 insertions(+), 79 deletions(-) create mode 100644 srml/support/src/error.rs diff --git a/core/client/src/block_builder/block_builder.rs b/core/client/src/block_builder/block_builder.rs index 4564c29aae419..3deaebfc3720c 100644 --- a/core/client/src/block_builder/block_builder.rs +++ b/core/client/src/block_builder/block_builder.rs @@ -17,7 +17,7 @@ use super::api::BlockBuilder as BlockBuilderApi; use std::vec::Vec; use parity_codec::Encode; -use runtime_primitives::ApplyOutcome; +use runtime_primitives::ApplyResult; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{ Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef, DigestFor, @@ -104,11 +104,11 @@ where ExecutionContext::BlockConstruction, xt.clone() )? { - Ok(ApplyOutcome::Success) | Ok(ApplyOutcome::Fail) => { + ApplyResult::Success | ApplyResult::ModuleError { .. } => { extrinsics.push(xt); Ok(()) } - Err(e) => { + ApplyResult::ApplyError(e) => { Err(error::Error::ApplyExtrinsicFailed(e)) } } diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index d6e0d60e2c218..6db08297a5caa 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -22,6 +22,7 @@ use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; +use crate::Error; use super::CheckedExtrinsic; #[derive(PartialEq, Eq, Clone, Encode, Decode)] @@ -83,17 +84,18 @@ where Call: Encode + Member, Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, - Context: Lookup, + Context: Lookup, { type Checked = CheckedExtrinsic; + type Error = Error; - fn check(self, context: &Context) -> Result { + fn check(self, context: &Context) -> Result { Ok(match self.signature { Some(SignatureContent{signed, signature, index}) => { let payload = (index, self.function); let signed = context.lookup(signed)?; if !crate::verify_encoded_lazy(&signature, &payload, &signed) { - return Err(crate::BAD_SIGNATURE) + return Err(Error::BadSignature) } CheckedExtrinsic { signed: Some((signed, payload.0)), diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 36e17fc277cde..7d9beec941a66 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -24,6 +24,7 @@ use runtime_io::blake2_256; use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; +use crate::Error; use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -75,18 +76,19 @@ where AccountId: Member + MaybeDisplay, BlockNumber: SimpleArithmetic, Hash: Encode, - Context: Lookup + Context: Lookup + CurrentHeight + BlockNumberToHash, { type Checked = CheckedExtrinsic; + type Error = Error; - fn check(self, context: &Context) -> Result { + fn check(self, context: &Context) -> Result { Ok(match self.signature { Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) - .ok_or("transaction birth block ancient")?; + .ok_or(Error::Unknown("transaction birth block ancient"))?; let signed = context.lookup(signed)?; let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { @@ -96,7 +98,7 @@ where signature.verify(payload, &signed) } }) { - return Err(crate::BAD_SIGNATURE) + return Err(Error::BadSignature) } CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 7f92b20edd0c3..dfe7a44b2493e 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -26,6 +26,7 @@ use crate::traits::{ self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion }; +use crate::Error; use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -76,18 +77,19 @@ where AccountId: Member + MaybeDisplay, BlockNumber: SimpleArithmetic, Hash: Encode, - Context: Lookup + Context: Lookup + CurrentHeight + BlockNumberToHash, { type Checked = CheckedExtrinsic; + type Error = Error; - fn check(self, context: &Context) -> Result { + fn check(self, context: &Context) -> Result { Ok(match self.signature { Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) - .ok_or("transaction birth block ancient")?; + .ok_or(Error::Unknown("transaction birth block ancient"))?; let signed = context.lookup(signed)?; let raw_payload = (index, self.function, era, h); @@ -98,7 +100,7 @@ where signature.verify(payload, &signed) } }) { - return Err(crate::BAD_SIGNATURE) + return Err(Error::BadSignature) } CheckedExtrinsic { signed: Some((signed, raw_payload.0)), diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 1ef8bc227578c..986a94c006a84 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -48,18 +48,23 @@ pub mod transaction_validity; /// Re-export these since they're only "kind of" generic. pub use generic::{DigestItem, Digest}; -/// A message indicating an invalid signature in extrinsic. -pub const BAD_SIGNATURE: &str = "bad signature in extrinsic"; - -/// Full block error message. -/// -/// This allows modules to indicate that given transaction is potentially valid -/// in the future, but can't be executed in the current state. -/// Note this error should be returned early in the execution to prevent DoS, -/// cause the fees are not being paid if this error is returned. -/// -/// Example: block gas limit is reached (the transaction can be retried in the next block though). -pub const BLOCK_FULL: &str = "block size limit is reached"; +/// Error type +pub enum Error { + /// Unknown error + /// This exists only to make implementation easier. Should be avoid as much as possible. + Unknown(&'static str), + /// Indicating an invalid signature in extrinsic. + BadSignature, + /// Full block error. + /// + /// This allows modules to indicate that given transaction is potentially valid + /// in the future, but can't be executed in the current state. + /// Note this error should be returned early in the execution to prevent DoS, + /// cause the fees are not being paid if this error is returned. + /// + /// Example: block gas limit is reached (the transaction can be retried in the next block though). + BlockFull +} /// Justification type. pub type Justification = Vec; @@ -486,23 +491,6 @@ impl From for AnySignature { } } -#[derive(Eq, PartialEq, Clone, Copy, Decode)] -#[cfg_attr(feature = "std", derive(Debug, Serialize))] -#[repr(u8)] -/// Outcome of a valid extrinsic application. Capable of being sliced. -pub enum ApplyOutcome { - /// Successful application (extrinsic reported no issue). - Success = 0, - /// Failed application (extrinsic was probably a no-op other than fees). - Fail = 1, -} - -impl codec::Encode for ApplyOutcome { - fn using_encoded R>(&self, f: F) -> R { - f(&[*self as u8]) - } -} - #[derive(Eq, PartialEq, Clone, Copy, Decode)] #[cfg_attr(feature = "std", derive(Debug, Serialize))] #[repr(u8)] @@ -526,8 +514,23 @@ impl codec::Encode for ApplyError { } } +// TODO: custom implement Encode & Decode to make it a two byte value +#[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] +#[cfg_attr(feature = "std", derive(Debug, Serialize))] /// Result from attempt to apply an extrinsic. -pub type ApplyResult = Result; +pub enum ApplyResult { + /// Successful application (extrinsic reported no issue). + Success, + /// Failed application (extrinsic was probably a no-op other than fees). + ModuleError { + /// Module index, matching the metadata module index + module: i8, // use i8 instead of u8 because u8 is not supported by parity-codec + /// Module specific error value + error: i8, + }, + /// Invalid extrinsic application. + ApplyError(ApplyError), +} /// Verify a signature on an encoded value in a lazy manner. This can be /// an optimization if the signature scheme has an "unsigned" escape hash. diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 35f3ec476f6d5..af2e2735b5bb4 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -200,6 +200,7 @@ impl Debug for TestXt { impl Checkable for TestXt { type Checked = Self; + type Error = &'static str; fn check(self, _: &Context) -> Result { Ok(self) } } impl traits::Extrinsic for TestXt { diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index b2bb7ab80511d..30db09461dfd4 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -69,26 +69,40 @@ impl Verify for substrate_primitives::sr25519::Signature { } } +/// EnsureOrigin Error type +pub trait EnsureOriginError { + /// Indicates invalid origin + fn invalid_origin() -> Self; +} + /// Some sort of check on the origin is performed by this object. pub trait EnsureOrigin { /// A return type. type Success; + /// Error type + type Error: EnsureOriginError; /// Perform the origin check. - fn ensure_origin(o: OuterOrigin) -> result::Result { - Self::try_origin(o).map_err(|_| "Invalid origin") + fn ensure_origin(o: OuterOrigin) -> result::Result { + Self::try_origin(o).map_err(|_| Self::Error::invalid_origin()) } /// Perform the origin check. fn try_origin(o: OuterOrigin) -> result::Result; } +impl EnsureOriginError for () { + fn invalid_origin() -> () { } +} + /// Means of changing one type into another in a manner dependent on the source type. pub trait Lookup { /// Type to lookup from. type Source; /// Type to lookup into. type Target; + /// Error type + type Error; /// Attempt a lookup. - fn lookup(&self, s: Self::Source) -> result::Result; + fn lookup(&self, s: Self::Source) -> result::Result; } /// Means of changing one type into another in a manner dependent on the source type. @@ -99,8 +113,10 @@ pub trait StaticLookup { type Source: Codec + Clone + PartialEq + MaybeDebug; /// Type to lookup into. type Target; + /// Error type + type Error; /// Attempt a lookup. - fn lookup(s: Self::Source) -> result::Result; + fn lookup(s: Self::Source) -> result::Result; /// Convert from Target back to Source. fn unlookup(t: Self::Target) -> Self::Source; } @@ -111,13 +127,15 @@ pub struct IdentityLookup(PhantomData); impl StaticLookup for IdentityLookup { type Source = T; type Target = T; - fn lookup(x: T) -> result::Result { Ok(x) } + type Error = (); + fn lookup(x: T) -> result::Result { Ok(x) } fn unlookup(x: T) -> T { x } } impl Lookup for IdentityLookup { type Source = T; type Target = T; - fn lookup(&self, x: T) -> result::Result { Ok(x) } + type Error = (); + fn lookup(&self, x: T) -> result::Result { Ok(x) } } /// Get the "current" block number. @@ -701,9 +719,11 @@ pub type DigestItemFor = DigestItem<<::Header as Header>::Hash>; pub trait Checkable: Sized { /// Returned if `check` succeeds. type Checked; + /// Indicates why `check` failed + type Error; /// Check self, given an instance of Context. - fn check(self, c: &Context) -> Result; + fn check(self, c: &Context) -> Result; } /// A "checkable" piece of information, used by the standard Substrate Executive in order to @@ -721,6 +741,8 @@ pub trait BlindCheckable: Sized { // Every `BlindCheckable` is also a `StaticCheckable` for arbitrary `Context`. impl Checkable for T { type Checked = ::Checked; + type Error = &'static str; + fn check(self, _c: &Context) -> Result { BlindCheckable::check(self) } diff --git a/core/test-runtime/src/system.rs b/core/test-runtime/src/system.rs index d150a573e86fe..7ebc7fa16775f 100644 --- a/core/test-runtime/src/system.rs +++ b/core/test-runtime/src/system.rs @@ -23,7 +23,7 @@ use runtime_support::storage::{self, StorageValue, StorageMap}; use runtime_support::storage_items; use runtime_primitives::traits::{Hash as HashT, BlakeTwo256, Header as _}; use runtime_primitives::generic; -use runtime_primitives::{ApplyError, ApplyOutcome, ApplyResult, transaction_validity::TransactionValidity}; +use runtime_primitives::{ApplyError, ApplyResult, transaction_validity::TransactionValidity}; use parity_codec::{KeyedVec, Encode}; use super::{ AccountId, BlockNumber, Extrinsic, Transfer, H256 as Hash, Block, Header, Digest, AuthorityId @@ -237,7 +237,7 @@ fn execute_transaction_backend(utx: &Extrinsic) -> ApplyResult { match utx { Extrinsic::Transfer(ref transfer, _) => execute_transfer_backend(transfer), Extrinsic::AuthoritiesChange(ref new_auth) => execute_new_authorities_backend(new_auth), - Extrinsic::IncludeData(_) => Ok(ApplyOutcome::Success), + Extrinsic::IncludeData(_) => ApplyResult::Success, } } @@ -264,13 +264,13 @@ fn execute_transfer_backend(tx: &Transfer) -> ApplyResult { let to_balance: u64 = storage::hashed::get_or(&blake2_256, &to_balance_key, 0); storage::hashed::put(&blake2_256, &from_balance_key, &(from_balance - tx.amount)); storage::hashed::put(&blake2_256, &to_balance_key, &(to_balance + tx.amount)); - Ok(ApplyOutcome::Success) + ApplyResult::Success } fn execute_new_authorities_backend(new_authorities: &[AuthorityId]) -> ApplyResult { let new_authorities: Vec = new_authorities.iter().cloned().collect(); ::put(new_authorities); - Ok(ApplyOutcome::Success) + ApplyResult::Success } #[cfg(feature = "std")] diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 4f0c38d88482a..46527e665882d 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -38,7 +38,7 @@ mod tests { NativeOrEncoded}; use node_primitives::{Hash, BlockNumber, AccountId}; use runtime_primitives::traits::{Header as HeaderT, Hash as HashT}; - use runtime_primitives::{generic::Era, ApplyOutcome, ApplyError, ApplyResult, Perbill}; + use runtime_primitives::{generic::Era, ApplyError, ApplyResult, Perbill}; use {balances, indices, system, staking, timestamp, treasury, contract}; use contract::ContractAddressFor; use system::{EventRecord, Phase}; @@ -912,7 +912,7 @@ mod tests { let r = WasmExecutor::new() .call(&mut t, 8, COMPACT_CODE, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())).unwrap(); let r = ApplyResult::decode(&mut &r[..]).unwrap(); - assert_eq!(r, Ok(ApplyOutcome::Success)); + assert_eq!(r, ApplyResult::Success); runtime_io::with_externalities(&mut t, || { assert_eq!(Balances::total_balance(&alice()), 42); diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 72ec997206550..3095d92527612 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -697,6 +697,7 @@ impl, I: Instance> system::Trait for ElevatedTrait { type Lookup = T::Lookup; type Header = T::Header; type Event = (); + type Error = (); } impl, I: Instance> Trait for ElevatedTrait { type Balance = T::Balance; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index f4299abe47630..eecaa2447a6dd 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -85,7 +85,7 @@ use primitives::{generic::Digest, traits::{ use srml_support::{Dispatchable, traits::MakePayment}; use parity_codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; -use primitives::{ApplyOutcome, ApplyError}; +use primitives::{ApplyResult, ApplyError}; use primitives::transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}; use primitives::weights::Weighable; @@ -100,9 +100,9 @@ mod internal { FullBlock, } - pub enum ApplyOutcome { + pub enum ApplyOutcome { Success, - Fail(&'static str), + Fail(Error), } } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 37e40058252dd..ecbfc88178647 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -30,7 +30,7 @@ pub enum Never {} /// Result of a module function call; either nothing (functions are only called for "side effects") /// or an error message. -pub type Result = result::Result<(), &'static str>; +pub type Result = result::Result<(), Error>; /// A lazy call (module function and argument values) that can be executed via its `dispatch` /// method. @@ -40,7 +40,8 @@ pub trait Dispatchable { /// identifier for the caller. The origin can be empty in the case of an inherent extrinsic. type Origin; type Trait; - fn dispatch(self, origin: Self::Origin) -> Result; + type Error; + fn dispatch(self, origin: Self::Origin) -> Result; } /// Serializable version of Dispatchable. @@ -217,6 +218,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -237,6 +239,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -251,6 +254,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event $(<$dpeg:ident $(, $dpeg_instance:ident)?>)* () = default; @@ -264,6 +268,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } + { $( $error_type )* } [ $($t)* ] $($rest)* ); @@ -276,6 +281,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event $(<$dpeg:ident $(, $dpeg_instance:ident)?>)* ( @@ -291,6 +297,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } + { $( $error_type )* } [ $($t)* ] $($rest)* ); @@ -303,6 +310,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } {} { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* fn on_finalize($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -316,6 +324,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { fn on_finalize( $( $param_name : $param ),* ) { $( $impl )* } } { $( $offchain )* } + { $( $error_type )* } [ $($t)* ] $($rest)* ); @@ -328,6 +337,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } {} { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* fn on_finalise($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -345,6 +355,7 @@ macro_rules! decl_module { {} { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* fn on_initialize($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -358,6 +369,7 @@ macro_rules! decl_module { { fn on_initialize( $( $param_name : $param ),* ) { $( $impl )* } } { $( $on_finalize )* } { $( $offchain )* } + { $( $error_type )* } [ $($t)* ] $($rest)* ); @@ -370,6 +382,7 @@ macro_rules! decl_module { {} { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* fn on_initialise($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -387,6 +400,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* fn offchain_worker($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -399,11 +413,39 @@ macro_rules! decl_module { { $( $deposit_event )* } { $( $on_initialize )* } { $( $on_finalize )* } + { $( $error_type )* } { fn offchain_worker( $( $param_name : $param ),* ) { $( $impl )* } } [ $($t)* ] $($rest)* ); }; + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident> + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { } + [ $($t:tt)* ] + $(#[doc = $doc_attr:tt])* + type Error = $error_type:ty; + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type<$trait_instance: $trait_name> + for enum $call_type where origin: $origin_type, system = $system + { $( $deposit_event )* } + { $( $on_initialize )* } + { $( $on_finalize )* } + { $( $offchain )* } + { $error_type } + [ $($t)* ] + $($rest)* + ); + }; // This puts the function statement into the [], decreasing `$rest` and moving toward finishing the parse. (@normalize $(#[$attr:meta])* @@ -416,6 +458,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $error_type:ty } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -434,6 +477,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } + { $error_type } [ $($t)* $(#[doc = $doc_attr])* @@ -458,6 +502,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* $fn_vis:vis fn $fn_name:ident( @@ -475,6 +520,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } + { $( $error_type )* } [ $($t)* ] $(#[doc = $doc_attr])* #[weight = $crate::dispatch::TransactionWeight::default()] @@ -484,6 +530,36 @@ macro_rules! decl_module { $($rest)* ); }; + // Add default Error if none supplied + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident> + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { } + [ $($t:tt)* ] + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type<$trait_instance: $trait_name> + for enum $call_type where origin: $origin_type, system = $system + { $( $deposit_event )* } + { $( $on_initialize )* } + { $( $on_finalize )* } + { $( $offchain )* } + { Error } + [ $($t)* ] + $($rest)* + ); + $crate::decl_error! { + pub enum Error { + } + } + }; // Ignore any ident which is not `origin` with type `T::Origin`. (@normalize $(#[$attr:meta])* @@ -493,6 +569,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -516,6 +593,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -539,6 +617,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -555,6 +634,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } + { $( $error_type )* } [ $($t)* ] $(#[doc = $doc_attr])* @@ -575,6 +655,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $( $error_type:tt )* } [ $($t:tt)* ] ) => { $crate::decl_module!(@imp @@ -587,6 +668,7 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } + { $( $error_type )* } ); }; @@ -745,13 +827,14 @@ macro_rules! decl_module { (@impl_function $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; $origin_ty:ty; + $error_type:ty; root; $(#[doc = $doc_attr:tt])* $vis:vis fn $name:ident ( root $(, $param:ident : $param_ty:ty )* ) { $( $impl:tt )* } ) => { $(#[doc = $doc_attr])* #[allow(unreachable_code)] - $vis fn $name($( $param: $param_ty ),* ) -> $crate::dispatch::Result { + $vis fn $name($( $param: $param_ty ),* ) -> $crate::dispatch::Result<$error_type> { { $( $impl )* } Ok(()) } @@ -761,6 +844,7 @@ macro_rules! decl_module { (@impl_function $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; $origin_ty:ty; + $error_type:ty; root; $(#[doc = $doc_attr:tt])* $vis:vis fn $name:ident ( @@ -777,6 +861,7 @@ macro_rules! decl_module { (@impl_function $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; $origin_ty:ty; + $error_type:ty; $ignore:ident; $(#[doc = $doc_attr:tt])* $vis:vis fn $name:ident ( @@ -786,7 +871,7 @@ macro_rules! decl_module { $(#[doc = $doc_attr])* $vis fn $name( $origin: $origin_ty $(, $param: $param_ty )* - ) -> $crate::dispatch::Result { + ) -> $crate::dispatch::Result<$error_type> { { $( $impl )* } Ok(()) } @@ -796,6 +881,7 @@ macro_rules! decl_module { (@impl_function $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; $origin_ty:ty; + $error_type:ty; $ignore:ident; $(#[doc = $doc_attr:tt])* $vis:vis fn $name:ident ( @@ -939,6 +1025,7 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } + { $error_type:ty } ) => { $crate::__check_reserved_fn_name! { $($fn_name)* @@ -985,6 +1072,7 @@ macro_rules! decl_module { @impl_function $mod_type<$trait_instance: $trait_name $(, $fn_instance: $fn_instantiable)?>; $origin_type; + $error_type; $from; $(#[doc = $doc_attr])* $fn_vis fn $fn_name ( @@ -1087,7 +1175,8 @@ macro_rules! decl_module { { type Trait = $trait_instance; type Origin = $origin_type; - fn dispatch(self, _origin: Self::Origin) -> $crate::dispatch::Result { + type Error = $error_type; + fn dispatch(self, _origin: Self::Origin) -> $crate::dispatch::Result { match self { $( $call_type::$fn_name( $( $param_name ),* ) => { @@ -1110,7 +1199,7 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $mod_type<$trait_instance $(, $instance)?> { #[doc(hidden)] - pub fn dispatch>(d: D, origin: D::Origin) -> $crate::dispatch::Result { + pub fn dispatch>(d: D, origin: D::Origin) -> $crate::dispatch::Result { d.dispatch(origin) } } @@ -1135,6 +1224,25 @@ macro_rules! impl_outer_dispatch { $module:ident::$camelcase:ident, )* } + ) => { + $crate::impl_outer_dispatch! { + $(#[$attr])* + pub enum $call_type for $runtime where origin: $origin { + type Error = &'static str; + $( + $module::$camelcase, + )* + } + } + }; + ( + $(#[$attr:meta])* + pub enum $call_type:ident for $runtime:ident where origin: $origin:ty { + type Error = $error_type:ty; + $( + $module:ident::$camelcase:ident, + )* + } ) => { $(#[$attr])* #[derive(Clone, PartialEq, Eq, $crate::codec::Encode, $crate::codec::Decode)] @@ -1154,7 +1262,8 @@ macro_rules! impl_outer_dispatch { impl $crate::dispatch::Dispatchable for $call_type { type Origin = $origin; type Trait = $call_type; - fn dispatch(self, origin: $origin) -> $crate::dispatch::Result { + type Error = $error_type; + fn dispatch(self, origin: $origin) -> $crate::dispatch::Result { match self { $( $call_type::$camelcase(call) => call.dispatch(origin), )* } diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs new file mode 100644 index 0000000000000..5f16571589c64 --- /dev/null +++ b/srml/support/src/error.rs @@ -0,0 +1,62 @@ +#[macro_export] +macro_rules! impl_outer_error { + ( + $(#[$attr:meta])* + pub enum $name:ident { + $( $modules:tt , )* + } + ) => { + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. + #[derive(Clone, PartialEq, Eq, $crate::codec::Encode, $crate::codec::Decode)] + #[cfg_attr(feature = "std", derive(Debug, $crate::Serialize, $crate::Deserialize))] + $(#[$attr])* + #[allow(non_camel_case_types)] + pub enum $name { + $( + $modules( $modules::Error ), + )* + } + } +} + + +#[macro_export] +macro_rules! decl_error { + ( + $(#[$attr:meta])* + pub enum Error { + $( + $errors:tt + )* + } + ) => { + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. + #[derive(Clone, PartialEq, Eq, $crate::codec::Encode)] + #[cfg_attr(feature = "std", derive(Debug))] + $(#[$attr])* + #[allow(non_camel_case_types)] + pub enum Error { + Unknown(&'static str), + $( + $errors + )* + } + impl From for () { + fn from(_: Error) -> () { () } + } + impl Into for Error { + fn into(self) -> u8 { + match self { + Error::Unknown(_) => 0, + _ => $crate::codec::Encode::using_encoded(&self, |s| s[0]), + } + } + } + + impl From<&'static str> for Error { + fn from(val: &'static str) -> Error { + Error::Unknown(val) + } + } + } +} diff --git a/srml/support/src/lib.rs b/srml/support/src/lib.rs index ae825397a6ae4..cf09f0f634adc 100644 --- a/srml/support/src/lib.rs +++ b/srml/support/src/lib.rs @@ -54,6 +54,8 @@ mod runtime; pub mod inherent; #[macro_use] pub mod unsigned; +#[macro_use] +pub mod error; mod double_map; pub mod traits; diff --git a/srml/support/src/runtime.rs b/srml/support/src/runtime.rs index 6bccac0d4eff0..11085b2a55f47 100644 --- a/srml/support/src/runtime.rs +++ b/srml/support/src/runtime.rs @@ -238,6 +238,12 @@ macro_rules! construct_runtime { $name: $module:: $( < $module_instance >:: )? { $( $modules $( <$modules_generic $(, $modules_instance)?> )* ),* } ),* ); + $crate::__decl_outer_error!( + $runtime; + $( + $name: $module:: $( < $module_instance >:: )? { $( $modules $( <$modules_generic $(, $modules_instance)?> )* ),* } + ),* + ); $crate::__decl_all_modules!( $runtime; ; @@ -444,6 +450,7 @@ macro_rules! __create_decl_macro { __create_decl_macro!(__decl_outer_event, impl_outer_event, Event, $); __create_decl_macro!(__decl_outer_origin, impl_outer_origin, Origin, $); +__create_decl_macro!(__decl_outer_error, impl_outer_error, Error, $); /// A macro that defines all modules as an associated types of the Runtime type. #[macro_export] @@ -593,6 +600,7 @@ macro_rules! __decl_outer_dispatch { ) => { $crate::impl_outer_dispatch!( pub enum Call for $runtime where origin: Origin { + type Error = Error; $( $parsed_modules::$parsed_name, )* } ); diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 73f8c942091e3..1256808d50498 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -76,15 +76,19 @@ use serde::Serialize; use rstd::prelude::*; #[cfg(any(feature = "std", test))] use rstd::map; -use primitives::{generic, traits::{self, CheckEqual, SimpleArithmetic, - SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, - MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, Lookup, -}}; +use primitives::{ + generic, Error as PrimitiveError, + traits::{ + self, CheckEqual, SimpleArithmetic, + SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, + MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, Lookup, + } +}; #[cfg(any(feature = "std", test))] use primitives::traits::Zero; use substrate_primitives::storage::well_known_keys; use srml_support::{ - storage, decl_module, decl_event, decl_storage, StorageDoubleMap, StorageValue, + storage, decl_module, decl_event, decl_storage, decl_error, StorageDoubleMap, StorageValue, StorageMap, Parameter, for_each_tuple, traits::Contains }; use safe_mix::TripletMix; @@ -184,6 +188,9 @@ pub trait Trait: 'static + Eq + Clone { /// The aggregated event type of the runtime. type Event: Parameter + Member + From; + + /// The aggregated error type of the runtime. + type Error: Parameter + Member + From; } pub type DigestOf = generic::Digest<::Hash>; @@ -194,6 +201,8 @@ pub type KeyValue = (Vec, Vec); decl_module! { pub struct Module for enum Call where origin: T::Origin { + type Error = Error; + /// Deposits an event into this block's event record. pub fn deposit_event(event: T::Event) { Self::deposit_event_indexed(&[], event); @@ -262,6 +271,27 @@ decl_event!( } ); +decl_error! { + /// Error for the System module + pub enum Error { + BadSignature, + BlockFull, + RequireSignedOrigin, + RequireRootOrigin, + RequireNoOrigin, + } +} + +impl From for Error { + fn from(err: PrimitiveError) -> Error { + match err { + PrimitiveError::Unknown(err) => Error::Unknown(err), + PrimitiveError::BadSignature => Error::BadSignature, + PrimitiveError::BlockFull => Error::BlockFull, + } + } +} + /// Origin for the System module. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] @@ -378,6 +408,7 @@ impl< AccountId, > EnsureOrigin for EnsureRoot { type Success = (); + type Error = (); fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Root => Ok(()), @@ -392,6 +423,7 @@ impl< AccountId, > EnsureOrigin for EnsureSigned { type Success = AccountId; + type Error = (); fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Signed(who) => Ok(who), @@ -407,6 +439,7 @@ impl< AccountId: PartialEq + Clone, > EnsureOrigin for EnsureSignedBy { type Success = AccountId; + type Error = (); fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Signed(ref who) if Who::contains(who) => Ok(who.clone()), @@ -421,6 +454,7 @@ impl< AccountId, > EnsureOrigin for EnsureNone { type Success = (); + type Error = (); fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::None => Ok(()), @@ -432,6 +466,7 @@ impl< pub struct EnsureNever(::rstd::marker::PhantomData); impl EnsureOrigin for EnsureNever { type Success = T; + type Error = (); fn try_origin(o: O) -> Result { Err(o) } @@ -439,32 +474,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("bad origin: expected to be a signed origin"), + _ => Err(Error::RequireSignedOrigin), } } /// Ensure that the origin `o` represents the root. Returns `Ok` or an `Err` otherwise. -pub fn ensure_root(o: OuterOrigin) -> Result<(), &'static str> +pub fn ensure_root(o: OuterOrigin) -> Result<(), Error> where OuterOrigin: Into, OuterOrigin>> { match o.into() { Ok(RawOrigin::Root) => Ok(()), - _ => Err("bad origin: expected to be a root origin"), + _ => Err(Error::RequireRootOrigin), } } /// Ensure that the origin `o` represents an unsigned extrinsic. Returns `Ok` or an `Err` otherwise. -pub fn ensure_none(o: OuterOrigin) -> Result<(), &'static str> +pub fn ensure_none(o: OuterOrigin) -> Result<(), Error> where OuterOrigin: Into, OuterOrigin>> { match o.into() { Ok(RawOrigin::None) => Ok(()), - _ => Err("bad origin: expected to be no origin"), + _ => Err(Error::RequireNoOrigin), } } @@ -741,7 +776,8 @@ impl Default for ChainContext { impl Lookup for ChainContext { type Source = ::Source; type Target = ::Target; - fn lookup(&self, s: Self::Source) -> rstd::result::Result { + type Error = ::Error; + fn lookup(&self, s: Self::Source) -> rstd::result::Result { ::lookup(s) } } From 52c6b90a86c911d705e900bb87137ae0f772056e Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Tue, 18 Jun 2019 20:24:15 +1200 Subject: [PATCH 02/40] wip --- .../client/src/block_builder/block_builder.rs | 2 +- core/sr-primitives/src/lib.rs | 31 +++- core/sr-primitives/src/traits.rs | 12 +- srml/balances/src/lib.rs | 4 +- srml/contract/src/gas.rs | 4 +- srml/executive/src/lib.rs | 173 +++++++++--------- srml/indices/src/lib.rs | 2 + srml/support/src/dispatch.rs | 33 ++-- srml/support/src/error.rs | 134 +++++++++++++- srml/system/src/lib.rs | 26 ++- 10 files changed, 297 insertions(+), 124 deletions(-) diff --git a/core/client/src/block_builder/block_builder.rs b/core/client/src/block_builder/block_builder.rs index 3deaebfc3720c..48ef76c480660 100644 --- a/core/client/src/block_builder/block_builder.rs +++ b/core/client/src/block_builder/block_builder.rs @@ -104,7 +104,7 @@ where ExecutionContext::BlockConstruction, xt.clone() )? { - ApplyResult::Success | ApplyResult::ModuleError { .. } => { + ApplyResult::Success | ApplyResult::DispatchError(_) => { extrinsics.push(xt); Ok(()) } diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 986a94c006a84..ca095aaa01494 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -66,6 +66,17 @@ pub enum Error { BlockFull } +// Exists for for backward compatibility purpose. +impl From for &str { + fn from(err: Error) -> &'static str { + match err { + Error::Unknown(val) => val, + Error::BadSignature => "bad signature in extrinsic", + Error::BlockFull => "block size limit is reached", + } + } +} + /// Justification type. pub type Justification = Vec; @@ -514,6 +525,19 @@ impl codec::Encode for ApplyError { } } +#[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] +#[cfg_attr(feature = "std", derive(Debug, Serialize))] +/// Reason why a dispatch call failed +pub struct DispatchError { + /// Module index, matching the metadata module index + pub module: i8, // use i8 instead of u8 because u8 is not supported by parity-codec + /// Module specific error value + pub error: i8, + /// Optional error message + #[codec(skip)] + pub message: Option<&'static str>, +} + // TODO: custom implement Encode & Decode to make it a two byte value #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] #[cfg_attr(feature = "std", derive(Debug, Serialize))] @@ -522,12 +546,7 @@ pub enum ApplyResult { /// Successful application (extrinsic reported no issue). Success, /// Failed application (extrinsic was probably a no-op other than fees). - ModuleError { - /// Module index, matching the metadata module index - module: i8, // use i8 instead of u8 because u8 is not supported by parity-codec - /// Module specific error value - error: i8, - }, + DispatchError(DispatchError), /// Invalid extrinsic application. ApplyError(ApplyError), } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 30db09461dfd4..2885b7ce51f90 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -113,8 +113,8 @@ pub trait StaticLookup { type Source: Codec + Clone + PartialEq + MaybeDebug; /// Type to lookup into. type Target; - /// Error type - type Error; + /// Error type. + type Error: Into<&'static str>; // Into<&'static str> for backward compatibility purpose. /// Attempt a lookup. fn lookup(s: Self::Source) -> result::Result; /// Convert from Target back to Source. @@ -127,15 +127,15 @@ pub struct IdentityLookup(PhantomData); impl StaticLookup for IdentityLookup { type Source = T; type Target = T; - type Error = (); - fn lookup(x: T) -> result::Result { Ok(x) } + type Error = &'static str; + fn lookup(x: T) -> result::Result { Ok(x) } fn unlookup(x: T) -> T { x } } impl Lookup for IdentityLookup { type Source = T; type Target = T; - type Error = (); - fn lookup(&self, x: T) -> result::Result { Ok(x) } + type Error = &'static str; + fn lookup(&self, x: T) -> result::Result { Ok(x) } } /// Get the "current" block number. diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 3095d92527612..97bc4976b1d9f 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -371,7 +371,7 @@ decl_module! { #[compact] value: T::Balance ) { let transactor = ensure_signed(origin)?; - let dest = T::Lookup::lookup(dest)?; + let dest = T::Lookup::lookup(dest).map_err(Into::into)?; >::transfer(&transactor, &dest, value)?; } @@ -393,7 +393,7 @@ decl_module! { #[compact] new_free: T::Balance, #[compact] new_reserved: T::Balance ) { - let who = T::Lookup::lookup(who)?; + let who = T::Lookup::lookup(who).map_err(Into::into)?; let current_free = >::get(&who); if new_free > current_free { diff --git a/srml/contract/src/gas.rs b/srml/contract/src/gas.rs index 1ea519634463c..c8519cdc4d558 100644 --- a/srml/contract/src/gas.rs +++ b/srml/contract/src/gas.rs @@ -15,7 +15,7 @@ // along with Substrate. If not, see . use crate::{GasSpent, Module, Trait, BalanceOf, NegativeImbalanceOf}; -use runtime_primitives::BLOCK_FULL; +use runtime_primitives::Error as PrimitiveError; use runtime_primitives::traits::{CheckedMul, CheckedSub, Zero, SaturatedConversion}; use srml_support::{StorageValue, traits::{OnUnbalanced, ExistenceRequirement, WithdrawReason, Currency, Imbalance}}; @@ -207,7 +207,7 @@ pub fn buy_gas( let gas_available = >::block_gas_limit() - >::gas_spent(); if gas_limit > gas_available { // gas limit reached, revert the transaction and retry again in the future - return Err(BLOCK_FULL); + return Err(PrimitiveError::BlockFull.into()); } // Buy the specified amount of gas. diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index eecaa2447a6dd..4bfb115880edb 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -76,34 +76,23 @@ use rstd::prelude::*; use rstd::marker::PhantomData; -use rstd::result; -use primitives::{generic::Digest, traits::{ - self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, - OnInitialize, NumberFor, Block as BlockT, OffchainWorker, - ValidateUnsigned, -}}; +use rstd::convert::TryInto; +use primitives::{ + generic::Digest, ApplyResult, ApplyError, DispatchError, + traits::{ + self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, + OnInitialize, NumberFor, Block as BlockT, OffchainWorker, + ValidateUnsigned, + } +}; use srml_support::{Dispatchable, traits::MakePayment}; use parity_codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; -use primitives::{ApplyResult, ApplyError}; use primitives::transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}; use primitives::weights::Weighable; mod internal { pub const MAX_TRANSACTIONS_WEIGHT: u32 = 4 * 1024 * 1024; - - pub enum ApplyError { - BadSignature(&'static str), - Stale, - Future, - CantPay, - FullBlock, - } - - pub enum ApplyOutcome { - Success, - Fail(Error), - } } /// Trait that can be used to execute a block. @@ -127,11 +116,12 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, + Error: Into + TryInto, > ExecuteBlock for Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Codec, CheckedOf: Applyable + Weighable, - CallOf: Dispatchable, + CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { @@ -147,11 +137,12 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, + Error: Into + TryInto, > Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Codec, CheckedOf: Applyable + Weighable, - CallOf: Dispatchable, + CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { @@ -228,30 +219,30 @@ where /// Apply extrinsic outside of the block execution function. /// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt /// hashes. - pub fn apply_extrinsic(uxt: Block::Extrinsic) -> result::Result { + pub fn apply_extrinsic(uxt: Block::Extrinsic) -> ApplyResult { let encoded = uxt.encode(); let encoded_len = encoded.len(); - match Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded)) { - Ok(internal::ApplyOutcome::Success) => Ok(ApplyOutcome::Success), - Ok(internal::ApplyOutcome::Fail(_)) => Ok(ApplyOutcome::Fail), - Err(internal::ApplyError::CantPay) => Err(ApplyError::CantPay), - Err(internal::ApplyError::BadSignature(_)) => Err(ApplyError::BadSignature), - Err(internal::ApplyError::Stale) => Err(ApplyError::Stale), - Err(internal::ApplyError::Future) => Err(ApplyError::Future), - Err(internal::ApplyError::FullBlock) => Err(ApplyError::FullBlock), - } + Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded)) } /// Apply an extrinsic inside the block execution function. fn apply_extrinsic_no_note(uxt: Block::Extrinsic) { let l = uxt.encode().len(); match Self::apply_extrinsic_with_len(uxt, l, None) { - Ok(internal::ApplyOutcome::Success) => (), - Ok(internal::ApplyOutcome::Fail(e)) => runtime_io::print(e), - Err(internal::ApplyError::CantPay) => panic!("All extrinsics should have sender able to pay their fees"), - Err(internal::ApplyError::BadSignature(_)) => panic!("All extrinsics should be properly signed"), - Err(internal::ApplyError::Stale) | Err(internal::ApplyError::Future) => panic!("All extrinsics should have the correct nonce"), - Err(internal::ApplyError::FullBlock) => panic!("Extrinsics should not exceed block limit"), + ApplyResult::Success => (), + ApplyResult::DispatchError(e) => { + runtime_io::print("Error:"); + // as u8 first to ensure not using sign-extend + runtime_io::print(e.module as u8 as u64); + runtime_io::print(e.error as u8 as u64); + if let Some(msg) = e.message { + runtime_io::print(msg); + } + }, + ApplyResult::ApplyError(ApplyError::CantPay) => panic!("All extrinsics should have sender able to pay their fees"), + ApplyResult::ApplyError(ApplyError::BadSignature) => panic!("All extrinsics should be properly signed"), + ApplyResult::ApplyError(ApplyError::Stale) | ApplyResult::ApplyError(ApplyError::Future) => panic!("All extrinsics should have the correct nonce"), + ApplyResult::ApplyError(ApplyError::FullBlock) => panic!("Extrinsics should not exceed block limit"), } } @@ -260,47 +251,58 @@ where uxt: Block::Extrinsic, encoded_len: usize, to_note: Option>, - ) -> result::Result { + ) -> ApplyResult { // Verify that the signature is good. - let xt = uxt.check(&Default::default()).map_err(internal::ApplyError::BadSignature)?; - - // Check the weight of the block if that extrinsic is applied. - let weight = xt.weight(encoded_len); - if >::all_extrinsics_weight() + weight > internal::MAX_TRANSACTIONS_WEIGHT { - return Err(internal::ApplyError::FullBlock); - } + match uxt.check(&Default::default()) { + Err(_) => ApplyResult::ApplyError(ApplyError::BadSignature), + Ok(xt) => { + // Check the weight of the block if that extrinsic is applied. + let weight = xt.weight(encoded_len); + if >::all_extrinsics_weight() + weight > internal::MAX_TRANSACTIONS_WEIGHT { + return ApplyResult::ApplyError(ApplyError::FullBlock); + } - if let (Some(sender), Some(index)) = (xt.sender(), xt.index()) { - // check index - let expected_index = >::account_nonce(sender); - if index != &expected_index { return Err( - if index < &expected_index { internal::ApplyError::Stale } else { internal::ApplyError::Future } - ) } - // pay any fees - Payment::make_payment(sender, encoded_len).map_err(|_| internal::ApplyError::CantPay)?; - - // AUDIT: Under no circumstances may this function panic from here onwards. - // FIXME: ensure this at compile-time (such as by not defining a panic function, forcing - // a linker error unless the compiler can prove it cannot be called). - // increment nonce in storage - >::inc_account_nonce(sender); - } + if let (Some(sender), Some(index)) = (xt.sender(), xt.index()) { + // check index + let expected_index = >::account_nonce(sender); + if index != &expected_index { + return if index < &expected_index { + ApplyResult::ApplyError(ApplyError::Stale) + } else { + ApplyResult::ApplyError(ApplyError::Future) + } + } + // pay any fees + // TODO: propagate why can't pay + match Payment::make_payment(sender, encoded_len) { + Err(_) => return ApplyResult::ApplyError(ApplyError::CantPay), + Ok(_) => () + }; + + // AUDIT: Under no circumstances may this function panic from here onwards. + // FIXME: ensure this at compile-time (such as by not defining a panic function, forcing + // a linker error unless the compiler can prove it cannot be called). + // increment nonce in storage + >::inc_account_nonce(sender); + } - // Make sure to `note_extrinsic` only after we know it's going to be executed - // to prevent it from leaking in storage. - if let Some(encoded) = to_note { - >::note_extrinsic(encoded); - } + // Make sure to `note_extrinsic` only after we know it's going to be executed + // to prevent it from leaking in storage. + if let Some(encoded) = to_note { + >::note_extrinsic(encoded); + } - // Decode parameters and dispatch - let (f, s) = xt.deconstruct(); - let r = f.dispatch(s.into()); - >::note_applied_extrinsic(&r, encoded_len as u32); + // Decode parameters and dispatch + let (f, s) = xt.deconstruct(); + let r = f.dispatch(s.into()); + >::note_applied_extrinsic(r.is_ok(), encoded_len as u32); - r.map(|_| internal::ApplyOutcome::Success).or_else(|e| match e { - primitives::BLOCK_FULL => Err(internal::ApplyError::FullBlock), - e => Ok(internal::ApplyOutcome::Fail(e)) - }) + match r { + Ok(_) => ApplyResult::Success, + Err(e) => ApplyResult::DispatchError(e.into()), + } + } + } } fn final_checks(header: &System::Header) { @@ -340,12 +342,19 @@ where let xt = match uxt.check(&Default::default()) { // Checks out. Carry on. Ok(xt) => xt, - // An unknown account index implies that the transaction may yet become valid. - Err("invalid account index") => return TransactionValidity::Unknown(INVALID_INDEX), - // Technically a bad signature could also imply an out-of-date account index, but - // that's more of an edge case. - Err(primitives::BAD_SIGNATURE) => return TransactionValidity::Invalid(ApplyError::BadSignature as i8), - Err(_) => return TransactionValidity::Invalid(UNKNOWN_ERROR), + Err(err) => { + match err.try_into() { + // An unknown account index implies that the transaction may yet become valid. + // TODO: avoid hardcoded error string here + Ok(system::Error::Unknown("invalid account index")) => + return TransactionValidity::Unknown(INVALID_INDEX), + // Technically a bad signature could also imply an out-of-date account index, but + // that's more of an edge case. + Ok(system::Error::BadSignature) => + return TransactionValidity::Invalid(ApplyError::BadSignature as i8), + _ => return TransactionValidity::Invalid(UNKNOWN_ERROR), + } + } }; match (xt.sender(), xt.index()) { diff --git a/srml/indices/src/lib.rs b/srml/indices/src/lib.rs index 45487e3b51cd1..cbb2079fc824f 100644 --- a/srml/indices/src/lib.rs +++ b/srml/indices/src/lib.rs @@ -224,6 +224,8 @@ impl OnNewAccount for Module { impl StaticLookup for Module { type Source = address::Address; type Target = T::AccountId; + type Error = &'static str; + fn lookup(a: Self::Source) -> result::Result { Self::lookup_address(a).ok_or("invalid account index") } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index ecbfc88178647..2ab241b250006 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -29,8 +29,11 @@ pub use sr_primitives::weights::{TransactionWeight, Weighable, Weight}; pub enum Never {} /// Result of a module function call; either nothing (functions are only called for "side effects") -/// or an error message. -pub type Result = result::Result<(), Error>; +/// or an error. +pub type DispatchResult = result::Result<(), Error>; + +/// Result with string error message. This exists for backward compatibility purpose. +pub type Result = DispatchResult<&'static str>; /// A lazy call (module function and argument values) that can be executed via its `dispatch` /// method. @@ -41,7 +44,7 @@ pub trait Dispatchable { type Origin; type Trait; type Error; - fn dispatch(self, origin: Self::Origin) -> Result; + fn dispatch(self, origin: Self::Origin) -> DispatchResult; } /// Serializable version of Dispatchable. @@ -533,7 +536,10 @@ macro_rules! decl_module { // Add default Error if none supplied (@normalize $(#[$attr:meta])* - pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident> + pub struct $mod_type:ident< + $trait_instance:ident: + $trait_name:ident$(, $instance:ident: $instantiable:path $(= $module_default_instance:path)?)? + > for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } @@ -545,7 +551,9 @@ macro_rules! decl_module { ) => { $crate::decl_module!(@normalize $(#[$attr])* - pub struct $mod_type<$trait_instance: $trait_name> + pub struct $mod_type< + $trait_instance: $trait_name$(, $instance: $instantiable $(= $module_default_instance)?)? + > for enum $call_type where origin: $origin_type, system = $system { $( $deposit_event )* } { $( $on_initialize )* } @@ -555,10 +563,7 @@ macro_rules! decl_module { [ $($t)* ] $($rest)* ); - $crate::decl_error! { - pub enum Error { - } - } + pub type Error = &'static str; }; // Ignore any ident which is not `origin` with type `T::Origin`. (@normalize @@ -834,7 +839,7 @@ macro_rules! decl_module { ) => { $(#[doc = $doc_attr])* #[allow(unreachable_code)] - $vis fn $name($( $param: $param_ty ),* ) -> $crate::dispatch::Result<$error_type> { + $vis fn $name($( $param: $param_ty ),* ) -> $crate::dispatch::DispatchResult<$error_type> { { $( $impl )* } Ok(()) } @@ -871,7 +876,7 @@ macro_rules! decl_module { $(#[doc = $doc_attr])* $vis fn $name( $origin: $origin_ty $(, $param: $param_ty )* - ) -> $crate::dispatch::Result<$error_type> { + ) -> $crate::dispatch::DispatchResult<$error_type> { { $( $impl )* } Ok(()) } @@ -1176,7 +1181,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::Result { + fn dispatch(self, _origin: Self::Origin) -> $crate::dispatch::DispatchResult { match self { $( $call_type::$fn_name( $( $param_name ),* ) => { @@ -1199,7 +1204,7 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $mod_type<$trait_instance $(, $instance)?> { #[doc(hidden)] - pub fn dispatch>(d: D, origin: D::Origin) -> $crate::dispatch::Result { + pub fn dispatch>(d: D, origin: D::Origin) -> $crate::dispatch::DispatchResult { d.dispatch(origin) } } @@ -1263,7 +1268,7 @@ macro_rules! impl_outer_dispatch { type Origin = $origin; type Trait = $call_type; type Error = $error_type; - fn dispatch(self, origin: $origin) -> $crate::dispatch::Result { + fn dispatch(self, origin: $origin) -> $crate::dispatch::DispatchResult { match self { $( $call_type::$camelcase(call) => call.dispatch(origin), )* } diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index 5f16571589c64..3b943b953cb2a 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -2,20 +2,144 @@ macro_rules! impl_outer_error { ( $(#[$attr:meta])* - pub enum $name:ident { - $( $modules:tt , )* + pub enum $name:ident for $runtime:ident { + $( $module:ident $( <$generic:ident $(, $instance:path )? > )? ),* $(,)? } + ) => { + $crate::impl_outer_error! { + $(#[$attr])* + pub enum $name for $runtime where system = system { + $( $module $( <$generic $(, $instance )? > )?, )* + } + } + }; + ( + $(#[$attr:meta])* + pub enum $name:ident for $runtime:ident where system = $system:ident { + $( $module:ident $( <$generic:ident $(, $instance:path )?> )? ),* $(,)? + } + ) => { + $crate::impl_outer_error!( + $( #[$attr] )*; + $name; + $runtime; + $system; + Modules { $( $module $( <$generic $(, $instance )? > )*, )* }; + ); + }; + ( + $(#[$attr:meta])*; + $name:ident; + $runtime:ident; + $system:ident; + Modules { + $module:ident $( )?, + $( $rest_module:tt )* + }; + $( $parsed:tt )* + ) => { + $crate::impl_outer_error!( + $( #[$attr] )*; + $name; + $runtime; + $system; + Modules { $( $rest_module )* }; + $( $parsed )* $module $( <$runtime $(, $instance )? > )?, + ); + }; + + // The main macro expansion that actually renders the Error enum. + + ( + $(#[$attr:meta])*; + $name:ident; + $runtime:ident; + $system:ident; + Modules { }; + $( $module:ident $( <$generic_param:ident $(, $generic_instance:path )? > )* ,)* ) => { // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. - #[derive(Clone, PartialEq, Eq, $crate::codec::Encode, $crate::codec::Decode)] - #[cfg_attr(feature = "std", derive(Debug, $crate::Serialize, $crate::Deserialize))] + #[derive(Clone, PartialEq, Eq, $crate::codec::Encode)] + #[cfg_attr(feature = "std", derive(Debug))] $(#[$attr])* #[allow(non_camel_case_types)] pub enum $name { + system($system::Error), $( - $modules( $modules::Error ), + $module($module::Error), )* } + + impl From<$system::Error> for $name { + fn from(err: $system::Error) -> Self { + $name::system(err) + } + } + + impl TryInto<$system::Error> for $name { + type Error = Self; + fn try_into(self) -> $crate::dispatch::result::Result<$modules::Error, Self::Error> { + if let $name::system(err) = self { + Ok(err) + } else { + Err(self) + } + } + } + + impl Into<$crate::runtime_primitives::DispatchError> for $name { + fn into(self) -> $crate::runtime_primitives::DispatchError { + match self { + system(err) => match err { + $system::Error::Unknown(msg) => + $crate::runtime_primitives::DispatchError { + module: 0, + error: 0, + message: msg, + } + _ => $crate::runtime_primitives::DispatchError { + module: 0, + error: err.into(), + message: None, + } + }, + $( + $modules(err) => match err { + $modules::Error::Unknown(msg) => + $crate::runtime_primitives::DispatchError { + module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), + error: 0, + message: msg, + } + _ => $crate::runtime_primitives::DispatchError { + module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), + error: err.into(), + message: None, + } + } + )* + } + } + } + + $( + impl From<$modules::Error> for $name { + fn from(err: $system::Error) -> Self { + $name::$modules(err) + } + } + + impl TryInto<$modules::Error> for $name { + type Error = Self; + fn try_into(self) -> $crate::dispatch::result::Result<$modules::Error, Self::Error> { + if let $name::$modules(err) = self { + Ok(err) + } else { + Err(self) + } + } + } + )* } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 1256808d50498..2f5ece98efe55 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -292,6 +292,20 @@ impl From for Error { } } +// Exists for for backward compatibility purpose. +impl From for &str { + fn from(err: Error) -> &'static str { + match err { + Error::Unknown(val) => val, + Error::BadSignature => "bad signature in extrinsic", + Error::BlockFull => "block size limit is reached", + Error::RequireSignedOrigin => "bad origin: expected to be a signed origin", + Error::RequireRootOrigin => "bad origin: expected to be a root origin", + Error::RequireNoOrigin => "bad origin: expected to be no origin", + } + } +} + /// Origin for the System module. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] @@ -738,10 +752,10 @@ impl Module { } /// To be called immediately after an extrinsic has been applied. - pub fn note_applied_extrinsic(r: &Result<(), &'static str>, encoded_len: u32) { - Self::deposit_event(match r { - Ok(_) => Event::ExtrinsicSuccess, - Err(_) => Event::ExtrinsicFailed, + pub fn note_applied_extrinsic(success: bool, encoded_len: u32) { + Self::deposit_event(match success { + true => Event::ExtrinsicSuccess, + false => Event::ExtrinsicFailed, }.into()); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; @@ -867,8 +881,8 @@ mod tests { System::initialize(&2, &[0u8; 32].into(), &[0u8; 32].into(), &Default::default()); System::deposit_event(42u16); - System::note_applied_extrinsic(&Ok(()), 0); - System::note_applied_extrinsic(&Err(""), 0); + System::note_applied_extrinsic(true, 0); + System::note_applied_extrinsic(false, 0); System::note_finished_extrinsics(); System::deposit_event(3u16); System::finalize(); From bd7228fd06388229bcceac16ab6df1b3c5c69d81 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Tue, 18 Jun 2019 23:51:55 +1200 Subject: [PATCH 03/40] more modules compiles --- core/sr-primitives/src/traits.rs | 4 ++++ srml/contract/src/lib.rs | 2 +- srml/council/src/motions.rs | 14 +++++++++----- srml/council/src/seats.rs | 6 +++--- srml/democracy/src/lib.rs | 10 +++++----- srml/staking/src/lib.rs | 7 ++++--- srml/sudo/src/lib.rs | 7 ++++--- srml/support/src/error.rs | 30 +++++++++++++++--------------- srml/treasury/src/lib.rs | 6 +++--- 9 files changed, 48 insertions(+), 38 deletions(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 2885b7ce51f90..67b05cfe3102c 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -93,6 +93,10 @@ impl EnsureOriginError for () { fn invalid_origin() -> () { } } +impl EnsureOriginError for &'static str { + fn invalid_origin() -> &'static str { "Invalid origin" } +} + /// Means of changing one type into another in a manner dependent on the source type. pub trait Lookup { /// Type to lookup from. diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index b9c8976cc04be..04c63fb4ee027 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -397,7 +397,7 @@ decl_module! { data: Vec ) -> Result { let origin = ensure_signed(origin)?; - let dest = T::Lookup::lookup(dest)?; + let dest = T::Lookup::lookup(dest).map_err(Into::into)?; // Pay for the gas upfront. // diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index a53752d71f905..c7b0eb51c2439 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -18,7 +18,7 @@ use rstd::{prelude::*, result}; use substrate_primitives::u32_trait::Value as U32; -use primitives::traits::{Hash, EnsureOrigin}; +use primitives::traits::{Hash, EnsureOrigin, EnsureOriginError}; use srml_support::{ dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode}, StorageValue, StorageMap, decl_module, decl_event, decl_storage, ensure @@ -259,10 +259,11 @@ pub fn ensure_council_members(o: OuterOrigin, n: MemberC pub struct EnsureMember(::rstd::marker::PhantomData); impl< - O: Into, O>> + From>, + O: Into, O>> + From> + EnsureOriginError, AccountId > EnsureOrigin for EnsureMember { type Success = AccountId; + type Error = O; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Member(id) => Ok(id), @@ -273,11 +274,12 @@ impl< pub struct EnsureMembers(::rstd::marker::PhantomData<(N, AccountId)>); impl< - O: Into, O>> + From>, + O: Into, O>> + From> + EnsureOriginError, N: U32, AccountId, > EnsureOrigin for EnsureMembers { type Success = (MemberCount, MemberCount); + type Error = O; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n >= N::VALUE => Ok((n, m)), @@ -290,12 +292,13 @@ pub struct EnsureProportionMoreThan( ::rstd::marker::PhantomData<(N, D, AccountId)> ); impl< - O: Into, O>> + From>, + O: Into, O>> + From> + EnsureOriginError, N: U32, D: U32, AccountId, > EnsureOrigin for EnsureProportionMoreThan { type Success = (); + type Error = O; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n * D::VALUE > N::VALUE * m => Ok(()), @@ -308,12 +311,13 @@ pub struct EnsureProportionAtLeast( ::rstd::marker::PhantomData<(N, D, AccountId)> ); impl< - O: Into, O>> + From>, + O: Into, O>> + From> + EnsureOriginError, N: U32, D: U32, AccountId, > EnsureOrigin for EnsureProportionAtLeast { type Success = (); + type Error = O; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n * D::VALUE >= N::VALUE * m => Ok(()), diff --git a/srml/council/src/seats.rs b/srml/council/src/seats.rs index 84b6f388f29fc..7d9903868bc84 100644 --- a/srml/council/src/seats.rs +++ b/srml/council/src/seats.rs @@ -213,7 +213,7 @@ decl_module! { #[compact] assumed_vote_index: VoteIndex ) { let reporter = ensure_signed(origin)?; - let who = T::Lookup::lookup(who)?; + let who = T::Lookup::lookup(who).map_err(Into::into)?; ensure!(!Self::presentation_active(), "cannot reap during presentation period"); ensure!(Self::voter_info(&reporter).is_some(), "reporter must be a voter"); @@ -352,7 +352,7 @@ decl_module! { "stake deposited to present winner and be added to leaderboard should be non-zero" ); - let candidate = T::Lookup::lookup(candidate)?; + let candidate = T::Lookup::lookup(candidate).map_err(Into::into)?; ensure!(index == Self::vote_index(), "index not current"); let (_, _, expiring) = Self::next_finalize().ok_or("cannot present outside of presentation period")?; let bad_presentation_punishment = @@ -417,7 +417,7 @@ decl_module! { /// Note: A tally should happen instantly (if not already in a presentation /// period) to fill the seat if removal means that the desired members are not met. fn remove_member(who: ::Source) { - let who = T::Lookup::lookup(who)?; + let who = T::Lookup::lookup(who).map_err(Into::into)?; let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council() .into_iter() .filter(|i| i.0 != who) diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index 0fead411c8796..afefa480ed615 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -191,23 +191,23 @@ pub trait Trait: system::Trait + Sized { /// Origin from which the next tabled referendum may be forced. This is a normal /// "super-majority-required" referendum. - type ExternalOrigin: EnsureOrigin; + type ExternalOrigin: EnsureOrigin; /// Origin from which the next tabled referendum may be forced; this allows for the tabling of /// a majority-carries referendum. - type ExternalMajorityOrigin: EnsureOrigin; + type ExternalMajorityOrigin: EnsureOrigin; /// Origin from which emergency referenda may be scheduled. - type EmergencyOrigin: EnsureOrigin; + type EmergencyOrigin: EnsureOrigin; /// Minimum voting period allowed for an emergency referendum. type EmergencyVotingPeriod: Get; /// Origin from which any referenda may be cancelled in an emergency. - type CancellationOrigin: EnsureOrigin; + type CancellationOrigin: EnsureOrigin; /// Origin for anyone able to veto proposals. - type VetoOrigin: EnsureOrigin; + type VetoOrigin: EnsureOrigin; /// Period in blocks where an external proposal may not be re-submitted after being vetoed. type CooloffPeriod: Get; diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 086f78bd53379..2241d3c02f7ab 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -637,7 +637,7 @@ decl_module! { return Err("stash already bonded") } - let controller = T::Lookup::lookup(controller)?; + let controller = T::Lookup::lookup(controller).map_err(Into::into)?; if >::exists(&controller) { return Err("controller already paired") @@ -795,7 +795,8 @@ decl_module! { let targets = targets.into_iter() .take(MAX_NOMINATIONS) .map(T::Lookup::lookup) - .collect::, &'static str>>()?; + .collect::, _>>() + .map_err(Into::into)?; >::remove(stash); >::insert(stash, targets); @@ -852,7 +853,7 @@ decl_module! { fn set_controller(origin, controller: ::Source) { let stash = ensure_signed(origin)?; let old_controller = Self::bonded(&stash).ok_or("not a stash")?; - let controller = T::Lookup::lookup(controller)?; + let controller = T::Lookup::lookup(controller).map_err(Into::into)?; if >::exists(&controller) { return Err("controller already paired") } diff --git a/srml/sudo/src/lib.rs b/srml/sudo/src/lib.rs index a421bdae68a52..fc29547228301 100644 --- a/srml/sudo/src/lib.rs +++ b/srml/sudo/src/lib.rs @@ -123,8 +123,9 @@ decl_module! { let res = match proposal.dispatch(system::RawOrigin::Root.into()) { Ok(_) => true, - Err(e) => { - sr_io::print(e); + Err(_e) => { + // TODO: not sure how to deal with this + // sr_io::print(e); false } }; @@ -145,7 +146,7 @@ decl_module! { // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), "only the current sudo key can change the sudo key"); - let new = T::Lookup::lookup(new)?; + let new = T::Lookup::lookup(new).map_err(Into::into)?; Self::deposit_event(RawEvent::KeyChanged(Self::key())); >::put(new); diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index 3b943b953cb2a..8528497c5f490 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -76,9 +76,9 @@ macro_rules! impl_outer_error { } } - impl TryInto<$system::Error> for $name { + impl $crate::rstd::convert::TryInto<$system::Error> for $name { type Error = Self; - fn try_into(self) -> $crate::dispatch::result::Result<$modules::Error, Self::Error> { + fn try_into(self) -> $crate::dispatch::result::Result<$system::Error, Self::Error> { if let $name::system(err) = self { Ok(err) } else { @@ -90,49 +90,49 @@ macro_rules! impl_outer_error { impl Into<$crate::runtime_primitives::DispatchError> for $name { fn into(self) -> $crate::runtime_primitives::DispatchError { match self { - system(err) => match err { + $name::system(err) => match err { $system::Error::Unknown(msg) => $crate::runtime_primitives::DispatchError { module: 0, error: 0, message: msg, - } + }, _ => $crate::runtime_primitives::DispatchError { module: 0, error: err.into(), message: None, - } + }, }, $( - $modules(err) => match err { - $modules::Error::Unknown(msg) => + $name::$module(err) => match err { + $module::Error::Unknown(msg) => $crate::runtime_primitives::DispatchError { module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), error: 0, message: msg, - } + }, _ => $crate::runtime_primitives::DispatchError { module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), error: err.into(), message: None, - } - } + }, + }, )* } } } $( - impl From<$modules::Error> for $name { + impl From<$module::Error> for $name { fn from(err: $system::Error) -> Self { - $name::$modules(err) + $name::$module(err) } } - impl TryInto<$modules::Error> for $name { + impl $crate::rstd::convert::TryInto<$module::Error> for $name { type Error = Self; - fn try_into(self) -> $crate::dispatch::result::Result<$modules::Error, Self::Error> { - if let $name::$modules(err) = self { + fn try_into(self) -> $crate::dispatch::result::Result<$module::Error, Self::Error> { + if let $name::$module(err) = self { Ok(err) } else { Err(self) diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index 95fa90f88ff5f..cae0d77d8f138 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -87,10 +87,10 @@ pub trait Trait: system::Trait { type Currency: Currency + ReservableCurrency; /// Origin from which approvals must come. - type ApproveOrigin: EnsureOrigin; + type ApproveOrigin: EnsureOrigin; /// Origin from which rejections must come. - type RejectOrigin: EnsureOrigin; + type RejectOrigin: EnsureOrigin; /// The overarching event type. type Event: From> + Into<::Event>; @@ -122,7 +122,7 @@ decl_module! { beneficiary: ::Source ) { let proposer = ensure_signed(origin)?; - let beneficiary = T::Lookup::lookup(beneficiary)?; + let beneficiary = T::Lookup::lookup(beneficiary).map_err(Into::into)?; let bond = Self::calculate_bond(value); T::Currency::reserve(&proposer, bond) From c0cece1c76f0230dd5695834d4ba4e330a765b40 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 19 Jun 2019 11:41:31 +1200 Subject: [PATCH 04/40] node-runtime checks --- .../src/generic/unchecked_extrinsic.rs | 4 ++-- .../unchecked_mortal_compact_extrinsic.rs | 4 ++-- .../src/generic/unchecked_mortal_extrinsic.rs | 4 ++-- core/sr-primitives/src/lib.rs | 6 ++++++ node/runtime/src/lib.rs | 1 + srml/council/src/motions.rs | 18 +++++++++--------- srml/executive/src/lib.rs | 14 ++++++++------ srml/support/src/dispatch.rs | 2 +- srml/support/src/error.rs | 14 ++++++++++---- srml/system/src/lib.rs | 2 +- 10 files changed, 42 insertions(+), 27 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 6db08297a5caa..fcee4242c5e8a 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -84,7 +84,7 @@ where Call: Encode + Member, Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, - Context: Lookup, + Context: Lookup { type Checked = CheckedExtrinsic; type Error = Error; @@ -93,7 +93,7 @@ where Ok(match self.signature { Some(SignatureContent{signed, signature, index}) => { let payload = (index, self.function); - let signed = context.lookup(signed)?; + let signed = context.lookup(signed).map_err(Into::into)?; if !crate::verify_encoded_lazy(&signature, &payload, &signed) { return Err(Error::BadSignature) } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 7d9beec941a66..6fa642d8e28ff 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -76,7 +76,7 @@ where AccountId: Member + MaybeDisplay, BlockNumber: SimpleArithmetic, Hash: Encode, - Context: Lookup + Context: Lookup + CurrentHeight + BlockNumberToHash, { @@ -89,7 +89,7 @@ where let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or(Error::Unknown("transaction birth block ancient"))?; - let signed = context.lookup(signed)?; + let signed = context.lookup(signed).map_err(Into::into)?; let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index dfe7a44b2493e..20d5df30bb795 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -77,7 +77,7 @@ where AccountId: Member + MaybeDisplay, BlockNumber: SimpleArithmetic, Hash: Encode, - Context: Lookup + Context: Lookup + CurrentHeight + BlockNumberToHash, { @@ -90,7 +90,7 @@ where let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or(Error::Unknown("transaction birth block ancient"))?; - let signed = context.lookup(signed)?; + let signed = context.lookup(signed).map_err(Into::into)?; let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index ca095aaa01494..89013117e4111 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -77,6 +77,12 @@ impl From for &str { } } +impl Into for &'static str { + fn into(self) -> Error { + Error::Unknown(self) + } +} + /// Justification type. pub type Justification = Vec; diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 67a5b450a95bb..6f39946f9115b 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -96,6 +96,7 @@ impl system::Trait for Runtime { type Lookup = Indices; type Header = generic::Header; type Event = Event; + type Error = Error; } impl aura::Trait for Runtime { diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index c7b0eb51c2439..d3e69c935225e 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -18,7 +18,7 @@ use rstd::{prelude::*, result}; use substrate_primitives::u32_trait::Value as U32; -use primitives::traits::{Hash, EnsureOrigin, EnsureOriginError}; +use primitives::traits::{Hash, EnsureOrigin}; use srml_support::{ dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode}, StorageValue, StorageMap, decl_module, decl_event, decl_storage, ensure @@ -259,11 +259,11 @@ pub fn ensure_council_members(o: OuterOrigin, n: MemberC pub struct EnsureMember(::rstd::marker::PhantomData); impl< - O: Into, O>> + From> + EnsureOriginError, + O: Into, O>> + From>, AccountId > EnsureOrigin for EnsureMember { type Success = AccountId; - type Error = O; + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Member(id) => Ok(id), @@ -274,12 +274,12 @@ impl< pub struct EnsureMembers(::rstd::marker::PhantomData<(N, AccountId)>); impl< - O: Into, O>> + From> + EnsureOriginError, + O: Into, O>> + From>, N: U32, AccountId, > EnsureOrigin for EnsureMembers { type Success = (MemberCount, MemberCount); - type Error = O; + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n >= N::VALUE => Ok((n, m)), @@ -292,13 +292,13 @@ pub struct EnsureProportionMoreThan( ::rstd::marker::PhantomData<(N, D, AccountId)> ); impl< - O: Into, O>> + From> + EnsureOriginError, + O: Into, O>> + From>, N: U32, D: U32, AccountId, > EnsureOrigin for EnsureProportionMoreThan { type Success = (); - type Error = O; + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n * D::VALUE > N::VALUE * m => Ok(()), @@ -311,13 +311,13 @@ pub struct EnsureProportionAtLeast( ::rstd::marker::PhantomData<(N, D, AccountId)> ); impl< - O: Into, O>> + From> + EnsureOriginError, + O: Into, O>> + From>, N: U32, D: U32, AccountId, > EnsureOrigin for EnsureProportionAtLeast { type Success = (); - type Error = O; + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n * D::VALUE >= N::VALUE * m => Ok(()), diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 4bfb115880edb..5cdcf1c7441cc 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -78,7 +78,7 @@ use rstd::prelude::*; use rstd::marker::PhantomData; use rstd::convert::TryInto; use primitives::{ - generic::Digest, ApplyResult, ApplyError, DispatchError, + generic::Digest, ApplyResult, ApplyError, DispatchError, Error as PrimitiveError, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, @@ -116,10 +116,11 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, + CheckableError: Into, Error: Into + TryInto, > ExecuteBlock for Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Codec, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, OriginOf: From>, @@ -137,10 +138,11 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, + CheckableError: Into, Error: Into + TryInto, > Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Codec, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, OriginOf: From>, @@ -343,14 +345,14 @@ where // Checks out. Carry on. Ok(xt) => xt, Err(err) => { - match err.try_into() { + match err.into() { // An unknown account index implies that the transaction may yet become valid. // TODO: avoid hardcoded error string here - Ok(system::Error::Unknown("invalid account index")) => + PrimitiveError::Unknown("invalid account index") => return TransactionValidity::Unknown(INVALID_INDEX), // Technically a bad signature could also imply an out-of-date account index, but // that's more of an edge case. - Ok(system::Error::BadSignature) => + PrimitiveError::BadSignature => return TransactionValidity::Invalid(ApplyError::BadSignature as i8), _ => return TransactionValidity::Invalid(UNKNOWN_ERROR), } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 2ab241b250006..cd31f70f66c77 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -1270,7 +1270,7 @@ macro_rules! impl_outer_dispatch { type Error = $error_type; fn dispatch(self, origin: $origin) -> $crate::dispatch::DispatchResult { match self { - $( $call_type::$camelcase(call) => call.dispatch(origin), )* + $( $call_type::$camelcase(call) => call.dispatch(origin).map_err(Into::into), )* } } } diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index 8528497c5f490..4cb02e594c5cb 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -76,6 +76,12 @@ macro_rules! impl_outer_error { } } + impl From<&'static str> for $name { + fn from(err: &'static str) -> Self { + $name::system($system::Error::Unknown(err)) + } + } + impl $crate::rstd::convert::TryInto<$system::Error> for $name { type Error = Self; fn try_into(self) -> $crate::dispatch::result::Result<$system::Error, Self::Error> { @@ -95,11 +101,11 @@ macro_rules! impl_outer_error { $crate::runtime_primitives::DispatchError { module: 0, error: 0, - message: msg, + message: Some(msg), }, _ => $crate::runtime_primitives::DispatchError { module: 0, - error: err.into(), + error: Into::::into(err) as i8, message: None, }, }, @@ -109,11 +115,11 @@ macro_rules! impl_outer_error { $crate::runtime_primitives::DispatchError { module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), error: 0, - message: msg, + message: Some(msg), }, _ => $crate::runtime_primitives::DispatchError { module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), - error: err.into(), + error: Into::::into(err) as i8, message: None, }, }, diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 2f5ece98efe55..02e4a8c44bfa8 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -190,7 +190,7 @@ pub trait Trait: 'static + Eq + Clone { type Event: Parameter + Member + From; /// The aggregated error type of the runtime. - type Error: Parameter + Member + From; + type Error: Member + From; } pub type DigestOf = generic::Digest<::Hash>; From a93fedd419e1b16b24cb1c42def6a155a99c0d42 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 19 Jun 2019 13:27:17 +1200 Subject: [PATCH 05/40] build.sh passes --- core/sr-primitives/src/traits.rs | 8 +++++--- core/test-runtime/src/lib.rs | 8 +++++--- core/test-runtime/src/system.rs | 21 +++++++++++++-------- node-template/runtime/src/lib.rs | 2 ++ 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 67b05cfe3102c..e238cc7887d21 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -737,17 +737,19 @@ pub trait Checkable: Sized { pub trait BlindCheckable: Sized { /// Returned if `check` succeeds. type Checked; + /// Returned if `check` failed. + type Error; /// Check self. - fn check(self) -> Result; + fn check(self) -> Result; } // Every `BlindCheckable` is also a `StaticCheckable` for arbitrary `Context`. impl Checkable for T { type Checked = ::Checked; - type Error = &'static str; + type Error = ::Error; - fn check(self, _c: &Context) -> Result { + fn check(self, _c: &Context) -> Result { BlindCheckable::check(self) } } diff --git a/core/test-runtime/src/lib.rs b/core/test-runtime/src/lib.rs index ce9758da4e9ae..e54af5c7a4d49 100644 --- a/core/test-runtime/src/lib.rs +++ b/core/test-runtime/src/lib.rs @@ -37,6 +37,7 @@ use runtime_primitives::{ ApplyResult, create_runtime_str, transaction_validity::TransactionValidity, + Error, traits::{ BlindCheckable, BlakeTwo256, Block as BlockT, Extrinsic as ExtrinsicT, GetNodeBlockType, GetRuntimeBlockType, Verify @@ -117,18 +118,19 @@ impl serde::Serialize for Extrinsic { impl BlindCheckable for Extrinsic { type Checked = Self; + type Error = Error; - fn check(self) -> Result { + fn check(self) -> Result { match self { Extrinsic::AuthoritiesChange(new_auth) => Ok(Extrinsic::AuthoritiesChange(new_auth)), Extrinsic::Transfer(transfer, signature) => { if runtime_primitives::verify_encoded_lazy(&signature, &transfer, &transfer.from) { Ok(Extrinsic::Transfer(transfer, signature)) } else { - Err(runtime_primitives::BAD_SIGNATURE) + Err(Error::BadSignature) } }, - Extrinsic::IncludeData(_) => Err(runtime_primitives::BAD_SIGNATURE), + Extrinsic::IncludeData(_) => Err(Error::BadSignature), } } } diff --git a/core/test-runtime/src/system.rs b/core/test-runtime/src/system.rs index 7ebc7fa16775f..51529336eb42f 100644 --- a/core/test-runtime/src/system.rs +++ b/core/test-runtime/src/system.rs @@ -106,7 +106,10 @@ fn execute_block_with_state_root_handler( // execute transactions block.extrinsics.iter().enumerate().for_each(|(i, e)| { storage::unhashed::put(well_known_keys::EXTRINSIC_INDEX, &(i as u32)); - execute_transaction_backend(e).unwrap_or_else(|_| panic!("Invalid transaction")); + match execute_transaction_backend(e) { + ApplyResult::Success => (), + _ => panic!("Invalid transaction"), + }; storage::unhashed::kill(well_known_keys::EXTRINSIC_INDEX); }); @@ -233,11 +236,13 @@ fn check_signature(utx: &Extrinsic) -> Result<(), ApplyError> { } fn execute_transaction_backend(utx: &Extrinsic) -> ApplyResult { - check_signature(utx)?; - match utx { - Extrinsic::Transfer(ref transfer, _) => execute_transfer_backend(transfer), - Extrinsic::AuthoritiesChange(ref new_auth) => execute_new_authorities_backend(new_auth), - Extrinsic::IncludeData(_) => ApplyResult::Success, + match check_signature(utx) { + Ok(_) => match utx { + Extrinsic::Transfer(ref transfer, _) => execute_transfer_backend(transfer), + Extrinsic::AuthoritiesChange(ref new_auth) => execute_new_authorities_backend(new_auth), + Extrinsic::IncludeData(_) => ApplyResult::Success, + }, + Err(err) => ApplyResult::ApplyError(err) } } @@ -246,7 +251,7 @@ fn execute_transfer_backend(tx: &Transfer) -> ApplyResult { let nonce_key = tx.from.to_keyed_vec(NONCE_OF); let expected_nonce: u64 = storage::hashed::get_or(&blake2_256, &nonce_key, 0); if !(tx.nonce == expected_nonce) { - return Err(ApplyError::Stale) + return ApplyResult::ApplyError(ApplyError::Stale) } // increment nonce in storage @@ -258,7 +263,7 @@ fn execute_transfer_backend(tx: &Transfer) -> ApplyResult { // enact transfer if !(tx.amount <= from_balance) { - return Err(ApplyError::CantPay) + return ApplyResult::ApplyError(ApplyError::CantPay) } let to_balance_key = tx.to.to_keyed_vec(BALANCE_OF); let to_balance: u64 = storage::hashed::get_or(&blake2_256, &to_balance_key, 0); diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 9b99e7f08f495..26c046b2787a5 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -126,6 +126,8 @@ impl system::Trait for Runtime { type Event = Event; /// The ubiquitous origin type. type Origin = Origin; + /// The ubiquitous error type. + type Error = Error; } impl aura::Trait for Runtime { From 3039e7ce1e04536ccfa5365d589f899ffb868339 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 19 Jun 2019 16:25:51 +1200 Subject: [PATCH 06/40] include dispatch error in failed event --- srml/executive/src/lib.rs | 6 +++--- srml/system/src/lib.rs | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 5cdcf1c7441cc..e010b0db7da47 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -296,12 +296,12 @@ where // Decode parameters and dispatch let (f, s) = xt.deconstruct(); - let r = f.dispatch(s.into()); - >::note_applied_extrinsic(r.is_ok(), encoded_len as u32); + let r = f.dispatch(s.into()).map_err(Into::::into); + >::note_applied_extrinsic(&r, encoded_len as u32); match r { Ok(_) => ApplyResult::Success, - Err(e) => ApplyResult::DispatchError(e.into()), + Err(e) => ApplyResult::DispatchError(e), } } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 02e4a8c44bfa8..883e4e8a517a3 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -77,7 +77,7 @@ use rstd::prelude::*; #[cfg(any(feature = "std", test))] use rstd::map; use primitives::{ - generic, Error as PrimitiveError, + generic, Error as PrimitiveError, DispatchError, traits::{ self, CheckEqual, SimpleArithmetic, SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, @@ -267,7 +267,7 @@ decl_event!( /// An extrinsic completed successfully. ExtrinsicSuccess, /// An extrinsic failed. - ExtrinsicFailed, + ExtrinsicFailed(DispatchError), } ); @@ -752,10 +752,10 @@ impl Module { } /// To be called immediately after an extrinsic has been applied. - pub fn note_applied_extrinsic(success: bool, encoded_len: u32) { - Self::deposit_event(match success { - true => Event::ExtrinsicSuccess, - false => Event::ExtrinsicFailed, + pub fn note_applied_extrinsic(r: &Result<(), DispatchError>, encoded_len: u32) { + Self::deposit_event(match r { + Ok(_) => Event::ExtrinsicSuccess, + Err(err) => Event::ExtrinsicFailed(err.clone()), }.into()); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; From c5c2e5afa44c546964c6f5c894022b910bdedd92 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 19 Jun 2019 23:30:37 +1200 Subject: [PATCH 07/40] revert some unnecessary changes --- .../client/src/block_builder/block_builder.rs | 5 +- core/sr-primitives/src/lib.rs | 20 ++-- core/test-runtime/src/system.rs | 27 +++--- node/executor/src/lib.rs | 4 +- srml/executive/src/lib.rs | 97 +++++++++---------- 5 files changed, 70 insertions(+), 83 deletions(-) diff --git a/core/client/src/block_builder/block_builder.rs b/core/client/src/block_builder/block_builder.rs index 48ef76c480660..72b0d5f5838db 100644 --- a/core/client/src/block_builder/block_builder.rs +++ b/core/client/src/block_builder/block_builder.rs @@ -17,7 +17,6 @@ use super::api::BlockBuilder as BlockBuilderApi; use std::vec::Vec; use parity_codec::Encode; -use runtime_primitives::ApplyResult; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{ Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef, DigestFor, @@ -104,11 +103,11 @@ where ExecutionContext::BlockConstruction, xt.clone() )? { - ApplyResult::Success | ApplyResult::DispatchError(_) => { + Ok(_) => { extrinsics.push(xt); Ok(()) } - ApplyResult::ApplyError(e) => { + Err(e) => { Err(error::Error::ApplyExtrinsicFailed(e)) } } diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 89013117e4111..a25b94c30cb69 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -544,18 +544,18 @@ pub struct DispatchError { pub message: Option<&'static str>, } -// TODO: custom implement Encode & Decode to make it a two byte value #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] -#[cfg_attr(feature = "std", derive(Debug, Serialize))] + #[cfg_attr(feature = "std", derive(Debug, Serialize))] + /// Outcome of a valid extrinsic application. Capable of being sliced. + pub enum ApplyOutcome { + /// Successful application (extrinsic reported no issue). + Success, + /// Failed application (extrinsic was probably a no-op other than fees). + Fail(DispatchError), + } + /// Result from attempt to apply an extrinsic. -pub enum ApplyResult { - /// Successful application (extrinsic reported no issue). - Success, - /// Failed application (extrinsic was probably a no-op other than fees). - DispatchError(DispatchError), - /// Invalid extrinsic application. - ApplyError(ApplyError), -} +pub type ApplyResult = Result; /// Verify a signature on an encoded value in a lazy manner. This can be /// an optimization if the signature scheme has an "unsigned" escape hash. diff --git a/core/test-runtime/src/system.rs b/core/test-runtime/src/system.rs index 51529336eb42f..d150a573e86fe 100644 --- a/core/test-runtime/src/system.rs +++ b/core/test-runtime/src/system.rs @@ -23,7 +23,7 @@ use runtime_support::storage::{self, StorageValue, StorageMap}; use runtime_support::storage_items; use runtime_primitives::traits::{Hash as HashT, BlakeTwo256, Header as _}; use runtime_primitives::generic; -use runtime_primitives::{ApplyError, ApplyResult, transaction_validity::TransactionValidity}; +use runtime_primitives::{ApplyError, ApplyOutcome, ApplyResult, transaction_validity::TransactionValidity}; use parity_codec::{KeyedVec, Encode}; use super::{ AccountId, BlockNumber, Extrinsic, Transfer, H256 as Hash, Block, Header, Digest, AuthorityId @@ -106,10 +106,7 @@ fn execute_block_with_state_root_handler( // execute transactions block.extrinsics.iter().enumerate().for_each(|(i, e)| { storage::unhashed::put(well_known_keys::EXTRINSIC_INDEX, &(i as u32)); - match execute_transaction_backend(e) { - ApplyResult::Success => (), - _ => panic!("Invalid transaction"), - }; + execute_transaction_backend(e).unwrap_or_else(|_| panic!("Invalid transaction")); storage::unhashed::kill(well_known_keys::EXTRINSIC_INDEX); }); @@ -236,13 +233,11 @@ fn check_signature(utx: &Extrinsic) -> Result<(), ApplyError> { } fn execute_transaction_backend(utx: &Extrinsic) -> ApplyResult { - match check_signature(utx) { - Ok(_) => match utx { - Extrinsic::Transfer(ref transfer, _) => execute_transfer_backend(transfer), - Extrinsic::AuthoritiesChange(ref new_auth) => execute_new_authorities_backend(new_auth), - Extrinsic::IncludeData(_) => ApplyResult::Success, - }, - Err(err) => ApplyResult::ApplyError(err) + check_signature(utx)?; + match utx { + Extrinsic::Transfer(ref transfer, _) => execute_transfer_backend(transfer), + Extrinsic::AuthoritiesChange(ref new_auth) => execute_new_authorities_backend(new_auth), + Extrinsic::IncludeData(_) => Ok(ApplyOutcome::Success), } } @@ -251,7 +246,7 @@ fn execute_transfer_backend(tx: &Transfer) -> ApplyResult { let nonce_key = tx.from.to_keyed_vec(NONCE_OF); let expected_nonce: u64 = storage::hashed::get_or(&blake2_256, &nonce_key, 0); if !(tx.nonce == expected_nonce) { - return ApplyResult::ApplyError(ApplyError::Stale) + return Err(ApplyError::Stale) } // increment nonce in storage @@ -263,19 +258,19 @@ fn execute_transfer_backend(tx: &Transfer) -> ApplyResult { // enact transfer if !(tx.amount <= from_balance) { - return ApplyResult::ApplyError(ApplyError::CantPay) + return Err(ApplyError::CantPay) } let to_balance_key = tx.to.to_keyed_vec(BALANCE_OF); let to_balance: u64 = storage::hashed::get_or(&blake2_256, &to_balance_key, 0); storage::hashed::put(&blake2_256, &from_balance_key, &(from_balance - tx.amount)); storage::hashed::put(&blake2_256, &to_balance_key, &(to_balance + tx.amount)); - ApplyResult::Success + Ok(ApplyOutcome::Success) } fn execute_new_authorities_backend(new_authorities: &[AuthorityId]) -> ApplyResult { let new_authorities: Vec = new_authorities.iter().cloned().collect(); ::put(new_authorities); - ApplyResult::Success + Ok(ApplyOutcome::Success) } #[cfg(feature = "std")] diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 46527e665882d..4f0c38d88482a 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -38,7 +38,7 @@ mod tests { NativeOrEncoded}; use node_primitives::{Hash, BlockNumber, AccountId}; use runtime_primitives::traits::{Header as HeaderT, Hash as HashT}; - use runtime_primitives::{generic::Era, ApplyError, ApplyResult, Perbill}; + use runtime_primitives::{generic::Era, ApplyOutcome, ApplyError, ApplyResult, Perbill}; use {balances, indices, system, staking, timestamp, treasury, contract}; use contract::ContractAddressFor; use system::{EventRecord, Phase}; @@ -912,7 +912,7 @@ mod tests { let r = WasmExecutor::new() .call(&mut t, 8, COMPACT_CODE, "BlockBuilder_apply_extrinsic", &vec![].and(&xt())).unwrap(); let r = ApplyResult::decode(&mut &r[..]).unwrap(); - assert_eq!(r, ApplyResult::Success); + assert_eq!(r, Ok(ApplyOutcome::Success)); runtime_io::with_externalities(&mut t, || { assert_eq!(Balances::total_balance(&alice()), 42); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index e010b0db7da47..af9756e554ac7 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -78,7 +78,7 @@ use rstd::prelude::*; use rstd::marker::PhantomData; use rstd::convert::TryInto; use primitives::{ - generic::Digest, ApplyResult, ApplyError, DispatchError, Error as PrimitiveError, + generic::Digest, ApplyResult, ApplyOutcome, ApplyError, DispatchError, Error as PrimitiveError, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, @@ -231,8 +231,8 @@ where fn apply_extrinsic_no_note(uxt: Block::Extrinsic) { let l = uxt.encode().len(); match Self::apply_extrinsic_with_len(uxt, l, None) { - ApplyResult::Success => (), - ApplyResult::DispatchError(e) => { + Ok(ApplyOutcome::Success) => (), + Ok(ApplyOutcome::Fail(e)) => { runtime_io::print("Error:"); // as u8 first to ensure not using sign-extend runtime_io::print(e.module as u8 as u64); @@ -241,10 +241,10 @@ where runtime_io::print(msg); } }, - ApplyResult::ApplyError(ApplyError::CantPay) => panic!("All extrinsics should have sender able to pay their fees"), - ApplyResult::ApplyError(ApplyError::BadSignature) => panic!("All extrinsics should be properly signed"), - ApplyResult::ApplyError(ApplyError::Stale) | ApplyResult::ApplyError(ApplyError::Future) => panic!("All extrinsics should have the correct nonce"), - ApplyResult::ApplyError(ApplyError::FullBlock) => panic!("Extrinsics should not exceed block limit"), + Err(ApplyError::CantPay) => panic!("All extrinsics should have sender able to pay their fees"), + Err(ApplyError::BadSignature) => panic!("All extrinsics should be properly signed"), + Err(ApplyError::Stale) | Err(ApplyError::Future) => panic!("All extrinsics should have the correct nonce"), + Err(ApplyError::FullBlock) => panic!("Extrinsics should not exceed block limit"), } } @@ -255,56 +255,49 @@ where to_note: Option>, ) -> ApplyResult { // Verify that the signature is good. - match uxt.check(&Default::default()) { - Err(_) => ApplyResult::ApplyError(ApplyError::BadSignature), - Ok(xt) => { - // Check the weight of the block if that extrinsic is applied. - let weight = xt.weight(encoded_len); - if >::all_extrinsics_weight() + weight > internal::MAX_TRANSACTIONS_WEIGHT { - return ApplyResult::ApplyError(ApplyError::FullBlock); - } + let xt = uxt.check(&Default::default()).map_err(|_| ApplyError::BadSignature)?; + // Check the weight of the block if that extrinsic is applied. + let weight = xt.weight(encoded_len); + if >::all_extrinsics_weight() + weight > internal::MAX_TRANSACTIONS_WEIGHT { + return Err(ApplyError::FullBlock); + } - if let (Some(sender), Some(index)) = (xt.sender(), xt.index()) { - // check index - let expected_index = >::account_nonce(sender); - if index != &expected_index { - return if index < &expected_index { - ApplyResult::ApplyError(ApplyError::Stale) - } else { - ApplyResult::ApplyError(ApplyError::Future) - } - } - // pay any fees - // TODO: propagate why can't pay - match Payment::make_payment(sender, encoded_len) { - Err(_) => return ApplyResult::ApplyError(ApplyError::CantPay), - Ok(_) => () - }; - - // AUDIT: Under no circumstances may this function panic from here onwards. - // FIXME: ensure this at compile-time (such as by not defining a panic function, forcing - // a linker error unless the compiler can prove it cannot be called). - // increment nonce in storage - >::inc_account_nonce(sender); + if let (Some(sender), Some(index)) = (xt.sender(), xt.index()) { + // check index + let expected_index = >::account_nonce(sender); + if index != &expected_index { + return if index < &expected_index { + Err(ApplyError::Stale) + } else { + Err(ApplyError::Future) } + } + // pay any fees + // TODO: propagate why can't pay + Payment::make_payment(sender, encoded_len).map_err(|_| ApplyError::CantPay)?; + + // AUDIT: Under no circumstances may this function panic from here onwards. + // FIXME: ensure this at compile-time (such as by not defining a panic function, forcing + // a linker error unless the compiler can prove it cannot be called). + // increment nonce in storage + >::inc_account_nonce(sender); + } - // Make sure to `note_extrinsic` only after we know it's going to be executed - // to prevent it from leaking in storage. - if let Some(encoded) = to_note { - >::note_extrinsic(encoded); - } + // Make sure to `note_extrinsic` only after we know it's going to be executed + // to prevent it from leaking in storage. + if let Some(encoded) = to_note { + >::note_extrinsic(encoded); + } - // Decode parameters and dispatch - let (f, s) = xt.deconstruct(); - let r = f.dispatch(s.into()).map_err(Into::::into); - >::note_applied_extrinsic(&r, encoded_len as u32); + // Decode parameters and dispatch + let (f, s) = xt.deconstruct(); + let r = f.dispatch(s.into()).map_err(Into::::into); + >::note_applied_extrinsic(&r, encoded_len as u32); - match r { - Ok(_) => ApplyResult::Success, - Err(e) => ApplyResult::DispatchError(e), - } - } - } + Ok(match r { + Ok(_) => ApplyOutcome::Success, + Err(e) => ApplyOutcome::Fail(e), + }) } fn final_checks(header: &System::Header) { From 1198fb75af677f17ac1a7288802f5f05a0514059 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Thu, 20 Jun 2019 11:34:48 +1200 Subject: [PATCH 08/40] refactor based on comments --- .../src/generic/unchecked_mortal_compact_extrinsic.rs | 2 +- .../sr-primitives/src/generic/unchecked_mortal_extrinsic.rs | 2 +- core/sr-primitives/src/lib.rs | 6 +++--- srml/executive/src/lib.rs | 6 +++--- srml/system/src/lib.rs | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 6fa642d8e28ff..2193b7b06f2f9 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -88,7 +88,7 @@ where Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) - .ok_or(Error::Unknown("transaction birth block ancient"))?; + .ok_or("transaction birth block ancient".into())?; let signed = context.lookup(signed).map_err(Into::into)?; let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 20d5df30bb795..c84fde6c45810 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -89,7 +89,7 @@ where Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) - .ok_or(Error::Unknown("transaction birth block ancient"))?; + .ok_or("transaction birth block ancient".into())?; let signed = context.lookup(signed).map_err(Into::into)?; let raw_payload = (index, self.function, era, h); diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index a25b94c30cb69..a37eb43cbbde7 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -67,9 +67,9 @@ pub enum Error { } // Exists for for backward compatibility purpose. -impl From for &str { - fn from(err: Error) -> &'static str { - match err { +impl Into<&'static str> for Error{ + fn into(self) -> &'static str { + match self { Error::Unknown(val) => val, Error::BadSignature => "bad signature in extrinsic", Error::BlockFull => "block size limit is reached", diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index af9756e554ac7..0d493986c2c79 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -116,11 +116,11 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, - CheckableError: Into, Error: Into + TryInto, > ExecuteBlock for Executive where Block::Extrinsic: Checkable + Codec, + >::Error: Into, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, OriginOf: From>, @@ -138,11 +138,11 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, - CheckableError: Into, Error: Into + TryInto, > Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Codec, + >::Error: Into, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, OriginOf: From>, diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 883e4e8a517a3..6b0811d011e8a 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -293,9 +293,9 @@ impl From for Error { } // Exists for for backward compatibility purpose. -impl From for &str { - fn from(err: Error) -> &'static str { - match err { +impl Into<&'static str> for Error { + fn into(self) -> &'static str { + match self { Error::Unknown(val) => val, Error::BadSignature => "bad signature in extrinsic", Error::BlockFull => "block size limit is reached", From 1f964d1e402df28037d23d023ed7ddcfd6735d0a Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Thu, 20 Jun 2019 16:49:01 +1200 Subject: [PATCH 09/40] more compile error fixes --- .../unchecked_mortal_compact_extrinsic.rs | 7 ++++--- .../src/generic/unchecked_mortal_extrinsic.rs | 7 ++++--- core/sr-primitives/src/lib.rs | 1 + srml/executive/src/lib.rs | 2 +- srml/system/src/lib.rs | 17 +++++++++++------ 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 2193b7b06f2f9..4b01cf51b2b0a 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -200,6 +200,7 @@ mod tests { impl Lookup for TestContext { type Source = u64; type Target = u64; + type Error = &'static str; fn lookup(&self, s: u64) -> Result { Ok(s) } } impl CurrentHeight for TestContext { @@ -258,7 +259,7 @@ mod tests { fn badly_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); } #[test] @@ -286,14 +287,14 @@ mod tests { fn too_late_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); } #[test] fn too_early_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); } #[test] diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index c84fde6c45810..d0dacbe98585c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -201,6 +201,7 @@ mod tests { impl Lookup for TestContext { type Source = u64; type Target = u64; + type Error = &'static str; fn lookup(&self, s: u64) -> Result { Ok(s) } } impl CurrentHeight for TestContext { @@ -259,7 +260,7 @@ mod tests { fn badly_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); } #[test] @@ -287,14 +288,14 @@ mod tests { fn too_late_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); } #[test] fn too_early_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); } #[test] diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index a37eb43cbbde7..824612719b279 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -48,6 +48,7 @@ pub mod transaction_validity; /// Re-export these since they're only "kind of" generic. pub use generic::{DigestItem, Digest}; +#[cfg_attr(test, derive(PartialEq, Debug))] /// Error type pub enum Error { /// Unknown error diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 0d493986c2c79..72853ddc165bb 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -119,7 +119,7 @@ impl< Error: Into + TryInto, > ExecuteBlock for Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Codec, >::Error: Into, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 6b0811d011e8a..dce2b7b36d317 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -190,7 +190,7 @@ pub trait Trait: 'static + Eq + Clone { type Event: Parameter + Member + From; /// The aggregated error type of the runtime. - type Error: Member + From; + type Error: Member + From + From<&'static str>; } pub type DigestOf = generic::Digest<::Hash>; @@ -819,12 +819,16 @@ mod tests { use primitives::BuildStorage; use primitives::traits::{BlakeTwo256, IdentityLookup}; use primitives::testing::Header; - use srml_support::impl_outer_origin; + use srml_support::{impl_outer_origin, impl_outer_error}; - impl_outer_origin!{ + impl_outer_origin! { pub enum Origin for Test where system = super {} } + impl_outer_error! { + pub enum Error for Test where system = super {} + } + #[derive(Clone, Eq, PartialEq)] pub struct Test; impl Trait for Test { @@ -837,13 +841,14 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = u16; + type Error = Error; } impl From for u16 { fn from(e: Event) -> u16 { match e { Event::ExtrinsicSuccess => 100, - Event::ExtrinsicFailed => 101, + Event::ExtrinsicFailed(err) => Encode::using_encoded(&err, |s| (s[0] as u16) | ((s[1] as u16) << 8)), } } } @@ -881,8 +886,8 @@ mod tests { System::initialize(&2, &[0u8; 32].into(), &[0u8; 32].into(), &Default::default()); System::deposit_event(42u16); - System::note_applied_extrinsic(true, 0); - System::note_applied_extrinsic(false, 0); + System::note_applied_extrinsic(&Ok(()), 0); + System::note_applied_extrinsic(&Err(DispatchError { module: 1, error: 2, message: None }), 0); System::note_finished_extrinsics(); System::deposit_event(3u16); System::finalize(); From 83975c007061b6b2789c93b70c45c86ce8c2904d Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 20 Jun 2019 22:58:01 +1200 Subject: [PATCH 10/40] avoid unnecessary into --- core/sr-primitives/src/generic/unchecked_extrinsic.rs | 2 +- .../src/generic/unchecked_mortal_compact_extrinsic.rs | 4 ++-- .../src/generic/unchecked_mortal_extrinsic.rs | 4 ++-- core/sr-primitives/src/lib.rs | 6 +++--- core/sr-primitives/src/traits.rs | 2 +- srml/balances/src/lib.rs | 6 +++--- srml/contracts/src/lib.rs | 2 +- srml/council/src/seats.rs | 6 +++--- srml/staking/src/lib.rs | 6 +++--- srml/sudo/src/lib.rs | 2 +- srml/system/src/lib.rs | 9 +++++---- srml/treasury/src/lib.rs | 2 +- 12 files changed, 26 insertions(+), 25 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index fcee4242c5e8a..0b8cc42bd137a 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -93,7 +93,7 @@ where Ok(match self.signature { Some(SignatureContent{signed, signature, index}) => { let payload = (index, self.function); - let signed = context.lookup(signed).map_err(Into::into)?; + let signed = context.lookup(signed)?; if !crate::verify_encoded_lazy(&signature, &payload, &signed) { return Err(Error::BadSignature) } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 4b01cf51b2b0a..44003fc0d302c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -88,8 +88,8 @@ where Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) - .ok_or("transaction birth block ancient".into())?; - let signed = context.lookup(signed).map_err(Into::into)?; + .ok_or("transaction birth block ancient")?; + let signed = context.lookup(signed)?; let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index d0dacbe98585c..080bfa0d0531a 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -89,8 +89,8 @@ where Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) - .ok_or("transaction birth block ancient".into())?; - let signed = context.lookup(signed).map_err(Into::into)?; + .ok_or("transaction birth block ancient")?; + let signed = context.lookup(signed)?; let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 824612719b279..53f4743a78c81 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -78,9 +78,9 @@ impl Into<&'static str> for Error{ } } -impl Into for &'static str { - fn into(self) -> Error { - Error::Unknown(self) +impl From<&'static str> for Error { + fn from(val: &'static str) -> Error { + Error::Unknown(val) } } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index e238cc7887d21..03f6dc4dc65cd 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -118,7 +118,7 @@ pub trait StaticLookup { /// Type to lookup into. type Target; /// Error type. - type Error: Into<&'static str>; // Into<&'static str> for backward compatibility purpose. + type Error; /// Attempt a lookup. fn lookup(s: Self::Source) -> result::Result; /// Convert from Target back to Source. diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 97bc4976b1d9f..811e3d7cdc92a 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -371,7 +371,7 @@ decl_module! { #[compact] value: T::Balance ) { let transactor = ensure_signed(origin)?; - let dest = T::Lookup::lookup(dest).map_err(Into::into)?; + let dest = T::Lookup::lookup(dest)?; >::transfer(&transactor, &dest, value)?; } @@ -393,7 +393,7 @@ decl_module! { #[compact] new_free: T::Balance, #[compact] new_reserved: T::Balance ) { - let who = T::Lookup::lookup(who).map_err(Into::into)?; + let who = T::Lookup::lookup(who)?; let current_free = >::get(&who); if new_free > current_free { @@ -697,7 +697,7 @@ impl, I: Instance> system::Trait for ElevatedTrait { type Lookup = T::Lookup; type Header = T::Header; type Event = (); - type Error = (); + type Error = &'static str; } impl, I: Instance> Trait for ElevatedTrait { type Balance = T::Balance; diff --git a/srml/contracts/src/lib.rs b/srml/contracts/src/lib.rs index 04c63fb4ee027..b9c8976cc04be 100644 --- a/srml/contracts/src/lib.rs +++ b/srml/contracts/src/lib.rs @@ -397,7 +397,7 @@ decl_module! { data: Vec ) -> Result { let origin = ensure_signed(origin)?; - let dest = T::Lookup::lookup(dest).map_err(Into::into)?; + let dest = T::Lookup::lookup(dest)?; // Pay for the gas upfront. // diff --git a/srml/council/src/seats.rs b/srml/council/src/seats.rs index 7d9903868bc84..84b6f388f29fc 100644 --- a/srml/council/src/seats.rs +++ b/srml/council/src/seats.rs @@ -213,7 +213,7 @@ decl_module! { #[compact] assumed_vote_index: VoteIndex ) { let reporter = ensure_signed(origin)?; - let who = T::Lookup::lookup(who).map_err(Into::into)?; + let who = T::Lookup::lookup(who)?; ensure!(!Self::presentation_active(), "cannot reap during presentation period"); ensure!(Self::voter_info(&reporter).is_some(), "reporter must be a voter"); @@ -352,7 +352,7 @@ decl_module! { "stake deposited to present winner and be added to leaderboard should be non-zero" ); - let candidate = T::Lookup::lookup(candidate).map_err(Into::into)?; + let candidate = T::Lookup::lookup(candidate)?; ensure!(index == Self::vote_index(), "index not current"); let (_, _, expiring) = Self::next_finalize().ok_or("cannot present outside of presentation period")?; let bad_presentation_punishment = @@ -417,7 +417,7 @@ decl_module! { /// Note: A tally should happen instantly (if not already in a presentation /// period) to fill the seat if removal means that the desired members are not met. fn remove_member(who: ::Source) { - let who = T::Lookup::lookup(who).map_err(Into::into)?; + let who = T::Lookup::lookup(who)?; let new_council: Vec<(T::AccountId, T::BlockNumber)> = Self::active_council() .into_iter() .filter(|i| i.0 != who) diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 2241d3c02f7ab..d0cd98d307105 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -637,7 +637,7 @@ decl_module! { return Err("stash already bonded") } - let controller = T::Lookup::lookup(controller).map_err(Into::into)?; + let controller = T::Lookup::lookup(controller)?; if >::exists(&controller) { return Err("controller already paired") @@ -796,7 +796,7 @@ decl_module! { .take(MAX_NOMINATIONS) .map(T::Lookup::lookup) .collect::, _>>() - .map_err(Into::into)?; + ?; >::remove(stash); >::insert(stash, targets); @@ -853,7 +853,7 @@ decl_module! { fn set_controller(origin, controller: ::Source) { let stash = ensure_signed(origin)?; let old_controller = Self::bonded(&stash).ok_or("not a stash")?; - let controller = T::Lookup::lookup(controller).map_err(Into::into)?; + let controller = T::Lookup::lookup(controller)?; if >::exists(&controller) { return Err("controller already paired") } diff --git a/srml/sudo/src/lib.rs b/srml/sudo/src/lib.rs index fc29547228301..73b311b1d77cd 100644 --- a/srml/sudo/src/lib.rs +++ b/srml/sudo/src/lib.rs @@ -146,7 +146,7 @@ decl_module! { // This is a public call, so we ensure that the origin is some signed account. let sender = ensure_signed(origin)?; ensure!(sender == Self::key(), "only the current sudo key can change the sudo key"); - let new = T::Lookup::lookup(new).map_err(Into::into)?; + let new = T::Lookup::lookup(new)?; Self::deposit_event(RawEvent::KeyChanged(Self::key())); >::put(new); diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index dce2b7b36d317..f0b9a5ada9f10 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -178,7 +178,8 @@ pub trait Trait: 'static + Eq + Clone { /// Used to define the type and conversion mechanism for referencing accounts in transactions. It's perfectly /// reasonable for this to be an identity conversion (with the source type being `AccountId`), but other modules /// (e.g. Indices module) may provide more functional/efficient alternatives. - type Lookup: StaticLookup; + // TODO: avoid &'static str error type + type Lookup: StaticLookup; /// The block header. type Header: Parameter + traits::Header< @@ -293,9 +294,9 @@ impl From for Error { } // Exists for for backward compatibility purpose. -impl Into<&'static str> for Error { - fn into(self) -> &'static str { - match self { +impl From for &'static str { + fn from(err: Error) -> &'static str { + match err { Error::Unknown(val) => val, Error::BadSignature => "bad signature in extrinsic", Error::BlockFull => "block size limit is reached", diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index cae0d77d8f138..6d7eecf7d1f60 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -122,7 +122,7 @@ decl_module! { beneficiary: ::Source ) { let proposer = ensure_signed(origin)?; - let beneficiary = T::Lookup::lookup(beneficiary).map_err(Into::into)?; + let beneficiary = T::Lookup::lookup(beneficiary)?; let bond = Self::calculate_bond(value); T::Currency::reserve(&proposer, bond) From e03db0c1148e846a345e3db43fb19f9169b41837 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 20 Jun 2019 22:59:53 +1200 Subject: [PATCH 11/40] reorder code --- core/sr-primitives/src/lib.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 53f4743a78c81..ac798fd44bdb5 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -509,6 +509,16 @@ impl From for AnySignature { } } +#[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] + #[cfg_attr(feature = "std", derive(Debug, Serialize))] + /// Outcome of a valid extrinsic application. Capable of being sliced. + pub enum ApplyOutcome { + /// Successful application (extrinsic reported no issue). + Success, + /// Failed application (extrinsic was probably a no-op other than fees). + Fail(DispatchError), + } + #[derive(Eq, PartialEq, Clone, Copy, Decode)] #[cfg_attr(feature = "std", derive(Debug, Serialize))] #[repr(u8)] @@ -545,16 +555,6 @@ pub struct DispatchError { pub message: Option<&'static str>, } -#[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] - #[cfg_attr(feature = "std", derive(Debug, Serialize))] - /// Outcome of a valid extrinsic application. Capable of being sliced. - pub enum ApplyOutcome { - /// Successful application (extrinsic reported no issue). - Success, - /// Failed application (extrinsic was probably a no-op other than fees). - Fail(DispatchError), - } - /// Result from attempt to apply an extrinsic. pub type ApplyResult = Result; From 7e01497074cc9d7ac8e63a92b4ee472d57028e51 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Fri, 21 Jun 2019 12:04:44 +1200 Subject: [PATCH 12/40] fixes some tests --- core/sr-primitives/src/testing.rs | 5 +++-- node-template/runtime/src/template.rs | 6 ++++++ srml/assets/src/lib.rs | 7 ++++++- srml/aura/src/mock.rs | 6 ++++++ srml/balances/src/mock.rs | 7 ++++++- srml/contracts/src/tests.rs | 7 ++++++- srml/council/src/lib.rs | 7 ++++++- srml/democracy/src/lib.rs | 7 ++++++- srml/example/src/lib.rs | 7 ++++++- srml/executive/src/lib.rs | 15 ++++++++++----- srml/finality-tracker/src/lib.rs | 7 ++++++- srml/grandpa/src/mock.rs | 10 ++++++++-- srml/indices/src/mock.rs | 7 ++++++- srml/session/src/lib.rs | 7 ++++++- srml/staking/src/mock.rs | 7 ++++++- srml/support/src/dispatch.rs | 5 ++--- srml/support/test/tests/instance.rs | 6 ++++++ srml/system/benches/bench.rs | 6 ++++++ srml/system/src/lib.rs | 10 +++++----- srml/timestamp/src/lib.rs | 7 ++++++- srml/treasury/src/lib.rs | 7 ++++++- 21 files changed, 124 insertions(+), 29 deletions(-) diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index af2e2735b5bb4..3fb276632097c 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -22,6 +22,7 @@ use crate::codec::{Codec, Encode, Decode}; use crate::traits::{self, Checkable, Applyable, BlakeTwo256, OpaqueKeys}; use crate::generic; use crate::weights::{Weighable, Weight}; +use crate::Error; pub use substrate_primitives::H256; use substrate_primitives::U256; use substrate_primitives::ed25519::{Public as AuthorityId}; @@ -200,8 +201,8 @@ impl Debug for TestXt { impl Checkable for TestXt { type Checked = Self; - type Error = &'static str; - fn check(self, _: &Context) -> Result { Ok(self) } + type Error = Error; + fn check(self, _: &Context) -> Result { Ok(self) } } impl traits::Extrinsic for TestXt { fn is_signed(&self) -> Option { diff --git a/node-template/runtime/src/template.rs b/node-template/runtime/src/template.rs index c8b75f43f5f5b..dcd50f5c1a709 100644 --- a/node-template/runtime/src/template.rs +++ b/node-template/runtime/src/template.rs @@ -82,6 +82,11 @@ mod tests { pub enum Origin for Test {} } + #[allow(non_camel_case_types)] + pub enum Error { + system(system::Error) + } + // For testing the module, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the // configuration traits of modules we want to use. @@ -97,6 +102,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl Trait for Test { type Event = (); diff --git a/srml/assets/src/lib.rs b/srml/assets/src/lib.rs index d563db3b14146..a65a9296cd0a9 100644 --- a/srml/assets/src/lib.rs +++ b/srml/assets/src/lib.rs @@ -240,7 +240,7 @@ mod tests { use super::*; use runtime_io::with_externalities; - use srml_support::{impl_outer_origin, assert_ok, assert_noop}; + use srml_support::{impl_outer_origin, impl_outer_error, assert_ok, assert_noop}; use substrate_primitives::{H256, Blake2Hasher}; // The testing primitives are very useful for avoiding having to work with signatures // or public keys. `u64` is used as the `AccountId` and no `Signature`s are required. @@ -254,6 +254,10 @@ mod tests { pub enum Origin for Test {} } + impl_outer_error! { + pub enum Error for Test {} + } + // For testing the module, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the // configuration traits of modules we want to use. @@ -269,6 +273,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl Trait for Test { type Event = (); diff --git a/srml/aura/src/mock.rs b/srml/aura/src/mock.rs index e9c43850f6e01..386938f92069d 100644 --- a/srml/aura/src/mock.rs +++ b/srml/aura/src/mock.rs @@ -32,6 +32,11 @@ impl_outer_origin!{ #[derive(Clone, PartialEq, Eq, Debug)] pub struct Test; +#[allow(non_camel_case_types)] +pub enum Error { + system(system::Error) +} + impl system::Trait for Test { type Origin = Origin; type Index = u64; @@ -42,6 +47,7 @@ impl system::Trait for Test { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl timestamp::Trait for Test { diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index ac5208ab90c2a..d97ae38dcc042 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -22,13 +22,17 @@ use primitives::BuildStorage; use primitives::{traits::{IdentityLookup}, testing::Header}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_io; -use srml_support::impl_outer_origin; +use srml_support::{impl_outer_origin, impl_outer_error}; use crate::{GenesisConfig, Module, Trait}; impl_outer_origin!{ pub enum Origin for Runtime {} } +impl_outer_error! { + pub enum Error for Runtime {} +} + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, PartialEq, Eq, Debug)] pub struct Runtime; @@ -42,6 +46,7 @@ impl system::Trait for Runtime { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl Trait for Runtime { type Balance = u64; diff --git a/srml/contracts/src/tests.rs b/srml/contracts/src/tests.rs index 61a81a4eb683b..84097c728a1eb 100644 --- a/srml/contracts/src/tests.rs +++ b/srml/contracts/src/tests.rs @@ -33,7 +33,7 @@ use runtime_primitives::testing::{Digest, DigestItem, Header, UintAuthorityId, H use runtime_primitives::traits::{BlakeTwo256, IdentityLookup}; use runtime_primitives::BuildStorage; use srml_support::{ - assert_ok, impl_outer_dispatch, impl_outer_event, impl_outer_origin, storage::child, + assert_ok, impl_outer_dispatch, impl_outer_event, impl_outer_origin, impl_outer_error, storage::child, traits::Currency, StorageMap, StorageValue }; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -64,6 +64,10 @@ impl_outer_dispatch! { } } +impl_outer_error! { + pub enum Error for Test {} +} + #[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; impl system::Trait for Test { @@ -76,6 +80,7 @@ impl system::Trait for Test { type Lookup = IdentityLookup; type Header = Header; type Event = MetaEvent; + type Error = Error; } impl balances::Trait for Test { type Balance = u64; diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index 681bc731be895..3b61ef89fc223 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -38,7 +38,7 @@ mod tests { // These re-exports are here for a reason, edit with care pub use super::*; pub use runtime_io::with_externalities; - use srml_support::{impl_outer_origin, impl_outer_event, impl_outer_dispatch, parameter_types}; + use srml_support::{impl_outer_origin, impl_outer_error, impl_outer_event, impl_outer_dispatch, parameter_types}; pub use substrate_primitives::{H256, Blake2Hasher, u32_trait::{_1, _2, _3, _4}}; pub use primitives::{ BuildStorage, traits::{BlakeTwo256, IdentityLookup}, testing::{Digest, DigestItem, Header} @@ -64,6 +64,10 @@ mod tests { } } + impl_outer_error! { + pub enum Error for Test {} + } + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; @@ -77,6 +81,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = Event; + type Error = Error; } impl balances::Trait for Test { type Balance = u64; diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index afefa480ed615..695c16fadf4d5 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -913,7 +913,7 @@ mod tests { use super::*; use runtime_io::with_externalities; use srml_support::{ - impl_outer_origin, impl_outer_dispatch, assert_noop, assert_ok, parameter_types, + impl_outer_origin, impl_outer_error, impl_outer_dispatch, assert_noop, assert_ok, parameter_types, traits::Contains }; use substrate_primitives::{H256, Blake2Hasher}; @@ -939,6 +939,10 @@ mod tests { } } + impl_outer_error! { + pub enum Error for Test {} + } + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; @@ -952,6 +956,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl balances::Trait for Test { type Balance = u64; diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index dd14b7198acd3..a15e9d5f2bf5d 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -504,7 +504,7 @@ impl Module { mod tests { use super::*; - use srml_support::{impl_outer_origin, assert_ok}; + use srml_support::{impl_outer_origin, impl_outer_error, assert_ok}; use sr_io::with_externalities; use substrate_primitives::{H256, Blake2Hasher}; // The testing primitives are very useful for avoiding having to work with signatures @@ -518,6 +518,10 @@ mod tests { pub enum Origin for Test {} } + impl_outer_error! { + pub enum Error for Test {} + } + // For testing the module, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the // configuration traits of modules we want to use. @@ -533,6 +537,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl balances::Trait for Test { type Balance = u64; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 72853ddc165bb..d335e7c5dd029 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -116,13 +116,13 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, - Error: Into + TryInto, > ExecuteBlock for Executive where Block::Extrinsic: Checkable + Codec, >::Error: Into, CheckedOf: Applyable + Weighable, - CallOf: Dispatchable, + CallOf: Dispatchable, + as Dispatchable>::Error: Into + TryInto, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { @@ -138,13 +138,13 @@ impl< Payment: MakePayment, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, - Error: Into + TryInto, > Executive where Block::Extrinsic: Checkable + Codec, >::Error: Into, CheckedOf: Applyable + Weighable, - CallOf: Dispatchable, + CallOf: Dispatchable, + as Dispatchable>::Error: Into + TryInto, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { @@ -402,7 +402,7 @@ mod tests { use primitives::BuildStorage; use primitives::traits::{Header as HeaderT, BlakeTwo256, IdentityLookup}; use primitives::testing::{Digest, Header, Block}; - use srml_support::{traits::Currency, impl_outer_origin, impl_outer_event}; + use srml_support::{traits::Currency, impl_outer_origin, impl_outer_event, impl_outer_error}; use system; use hex_literal::hex; @@ -417,6 +417,10 @@ mod tests { } } + impl_outer_error! { + pub enum Error for Runtime {} + } + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, Eq, PartialEq)] pub struct Runtime; @@ -430,6 +434,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = MetaEvent; + type Error = Error; } impl balances::Trait for Runtime { type Balance = u64; diff --git a/srml/finality-tracker/src/lib.rs b/srml/finality-tracker/src/lib.rs index 6442fee543ab4..a10954ead17d5 100644 --- a/srml/finality-tracker/src/lib.rs +++ b/srml/finality-tracker/src/lib.rs @@ -264,7 +264,7 @@ mod tests { use primitives::BuildStorage; use primitives::traits::{BlakeTwo256, IdentityLookup, OnFinalize, Header as HeaderT}; use primitives::testing::Header; - use srml_support::impl_outer_origin; + use srml_support::{impl_outer_origin, impl_outer_error}; use srml_system as system; use lazy_static::lazy_static; use parking_lot::Mutex; @@ -284,6 +284,10 @@ mod tests { pub enum Origin for Test {} } + impl_outer_error! { + pub enum Error for Test {} + } + impl system::Trait for Test { type Origin = Origin; type Index = u64; @@ -294,6 +298,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } type System = system::Module; diff --git a/srml/grandpa/src/mock.rs b/srml/grandpa/src/mock.rs index 80c99b9a3cfaa..06ce9a3bb6e65 100644 --- a/srml/grandpa/src/mock.rs +++ b/srml/grandpa/src/mock.rs @@ -22,7 +22,7 @@ use primitives::{ BuildStorage, DigestItem, traits::IdentityLookup, testing::{Header, UintAuthorityId} }; use runtime_io; -use srml_support::{impl_outer_origin, impl_outer_event}; +use srml_support::{impl_outer_origin, impl_outer_error, impl_outer_event}; use substrate_primitives::{H256, Blake2Hasher}; use parity_codec::{Encode, Decode}; use crate::{AuthorityId, GenesisConfig, Trait, Module, Signal}; @@ -32,6 +32,10 @@ impl_outer_origin!{ pub enum Origin for Test {} } +impl_outer_error! { + pub enum Error for Test {} +} + impl From> for DigestItem { fn from(log: Signal) -> DigestItem { DigestItem::Consensus(GRANDPA_ENGINE_ID, log.encode()) @@ -41,10 +45,11 @@ impl From> for DigestItem { // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, PartialEq, Eq, Debug, Decode, Encode)] pub struct Test; + impl Trait for Test { type Event = TestEvent; - } + impl system::Trait for Test { type Origin = Origin; type Index = u64; @@ -55,6 +60,7 @@ impl system::Trait for Test { type Lookup = IdentityLookup; type Header = Header; type Event = TestEvent; + type Error = Error; } mod grandpa { diff --git a/srml/indices/src/mock.rs b/srml/indices/src/mock.rs index e2ea51e89d958..8159a2f4bffcc 100644 --- a/srml/indices/src/mock.rs +++ b/srml/indices/src/mock.rs @@ -23,7 +23,7 @@ use ref_thread_local::{ref_thread_local, RefThreadLocal}; use primitives::BuildStorage; use primitives::testing::Header; use substrate_primitives::{H256, Blake2Hasher}; -use srml_support::impl_outer_origin; +use srml_support::{impl_outer_origin, impl_outer_error}; use {runtime_io, system}; use crate::{GenesisConfig, Module, Trait, IsDeadAccount, OnNewAccount, ResolveHint}; @@ -31,6 +31,10 @@ impl_outer_origin!{ pub enum Origin for Runtime {} } +impl_outer_error! { + pub enum Error for Runtime {} +} + ref_thread_local! { static managed ALIVE: HashSet = HashSet::new(); } @@ -75,6 +79,7 @@ impl system::Trait for Runtime { type Lookup = Indices; type Header = Header; type Event = (); + type Error = Error; } impl Trait for Runtime { type AccountIndex = u64; diff --git a/srml/session/src/lib.rs b/srml/session/src/lib.rs index 3ae7c9801299b..f39d1f2f38d6b 100644 --- a/srml/session/src/lib.rs +++ b/srml/session/src/lib.rs @@ -381,7 +381,7 @@ impl OnFreeBalanceZero for Module { mod tests { use super::*; use std::cell::RefCell; - use srml_support::{impl_outer_origin, assert_ok}; + use srml_support::{impl_outer_origin, impl_outer_error, assert_ok}; use runtime_io::with_externalities; use substrate_primitives::{H256, Blake2Hasher}; use primitives::BuildStorage; @@ -392,6 +392,10 @@ mod tests { pub enum Origin for Test {} } + impl_outer_error! { + pub enum Error for Test {} + } + thread_local!{ static NEXT_VALIDATORS: RefCell> = RefCell::new(vec![1, 2, 3]); static AUTHORITIES: RefCell> = @@ -449,6 +453,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl timestamp::Trait for Test { type Moment = u64; diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index ea4fcbd8f9f01..2e6a8731d051a 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -22,7 +22,7 @@ use primitives::traits::{IdentityLookup, Convert, OpaqueKeys, OnInitialize}; use primitives::testing::{Header, UintAuthorityId}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_io; -use srml_support::{impl_outer_origin, parameter_types, assert_ok, traits::Currency}; +use srml_support::{impl_outer_origin, impl_outer_error, parameter_types, assert_ok, traits::Currency}; use crate::{EraIndex, GenesisConfig, Module, Trait, StakerStatus, ValidatorPrefs, RewardDestination}; /// The AccountId alias in this test module. @@ -70,6 +70,10 @@ impl_outer_origin!{ pub enum Origin for Test {} } +impl_outer_error! { + pub enum Error for Test {} +} + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, PartialEq, Eq, Debug)] pub struct Test; @@ -83,6 +87,7 @@ impl system::Trait for Test { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl balances::Trait for Test { type Balance = u64; diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index cd31f70f66c77..d8bb4ea030f74 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -416,8 +416,8 @@ macro_rules! decl_module { { $( $deposit_event )* } { $( $on_initialize )* } { $( $on_finalize )* } - { $( $error_type )* } { fn offchain_worker( $( $param_name : $param ),* ) { $( $impl )* } } + { $( $error_type )* } [ $($t)* ] $($rest)* ); @@ -559,11 +559,10 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } - { Error } + { &'static str } [ $($t)* ] $($rest)* ); - pub type Error = &'static str; }; // Ignore any ident which is not `origin` with type `T::Origin`. (@normalize diff --git a/srml/support/test/tests/instance.rs b/srml/support/test/tests/instance.rs index f7b4a4bd3a251..2201a92573b0c 100644 --- a/srml/support/test/tests/instance.rs +++ b/srml/support/test/tests/instance.rs @@ -48,6 +48,7 @@ mod system { srml_support::decl_module! { pub struct Module for enum Call where origin: T::Origin { + type Error = Error; pub fn deposit_event(_event: T::Event) { } } @@ -90,6 +91,11 @@ mod system { { o.into().map(|_| ()).map_err(|_| "bad origin: expected to be a root origin") } + + srml_support::decl_error! { + pub enum Error { + } + } } // Test for: diff --git a/srml/system/benches/bench.rs b/srml/system/benches/bench.rs index ee4ebf711ab31..a3068463e7fde 100644 --- a/srml/system/benches/bench.rs +++ b/srml/system/benches/bench.rs @@ -54,6 +54,11 @@ impl_outer_event! { } } +#[allow(non_camel_case_types)] +pub enum Error { + system(system::Error) +} + #[derive(Clone, Eq, PartialEq)] pub struct Runtime; impl system::Trait for Runtime { @@ -66,6 +71,7 @@ impl system::Trait for Runtime { type Lookup = IdentityLookup; type Header = Header; type Event = Event; + type Error = Error; } impl module::Trait for Runtime { diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index f0b9a5ada9f10..a487da0238d90 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -423,7 +423,7 @@ impl< AccountId, > EnsureOrigin for EnsureRoot { type Success = (); - type Error = (); + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Root => Ok(()), @@ -438,7 +438,7 @@ impl< AccountId, > EnsureOrigin for EnsureSigned { type Success = AccountId; - type Error = (); + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Signed(who) => Ok(who), @@ -454,7 +454,7 @@ impl< AccountId: PartialEq + Clone, > EnsureOrigin for EnsureSignedBy { type Success = AccountId; - type Error = (); + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Signed(ref who) if Who::contains(who) => Ok(who.clone()), @@ -469,7 +469,7 @@ impl< AccountId, > EnsureOrigin for EnsureNone { type Success = (); - type Error = (); + type Error = &'static str; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::None => Ok(()), @@ -481,7 +481,7 @@ impl< pub struct EnsureNever(::rstd::marker::PhantomData); impl EnsureOrigin for EnsureNever { type Success = T; - type Error = (); + type Error = &'static str; fn try_origin(o: O) -> Result { Err(o) } diff --git a/srml/timestamp/src/lib.rs b/srml/timestamp/src/lib.rs index e9c0d85a20a05..34e6024f2a685 100644 --- a/srml/timestamp/src/lib.rs +++ b/srml/timestamp/src/lib.rs @@ -330,7 +330,7 @@ impl ProvideInherent for Module { mod tests { use super::*; - use srml_support::{impl_outer_origin, assert_ok}; + use srml_support::{impl_outer_origin, impl_outer_error, assert_ok}; use runtime_io::{with_externalities, TestExternalities}; use substrate_primitives::H256; use runtime_primitives::BuildStorage; @@ -341,6 +341,10 @@ mod tests { pub enum Origin for Test {} } + impl_outer_error! { + pub enum Error for Runtime {} + } + #[derive(Clone, Eq, PartialEq)] pub struct Test; impl system::Trait for Test { @@ -353,6 +357,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl Trait for Test { type Moment = u64; diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index 6d7eecf7d1f60..972d51231a1c9 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -334,7 +334,7 @@ mod tests { use super::*; use runtime_io::with_externalities; - use srml_support::{impl_outer_origin, assert_ok, assert_noop}; + use srml_support::{impl_outer_origin, impl_outer_error, assert_ok, assert_noop}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_primitives::BuildStorage; use runtime_primitives::traits::{BlakeTwo256, OnFinalize, IdentityLookup}; @@ -344,6 +344,10 @@ mod tests { pub enum Origin for Test {} } + impl_outer_error! { + pub enum Error for Test where system = system {} + } + #[derive(Clone, Eq, PartialEq)] pub struct Test; impl system::Trait for Test { @@ -356,6 +360,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl balances::Trait for Test { type Balance = u64; From a6c989bb3090817af5a1a73828c9bb1e0b12896a Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Fri, 21 Jun 2019 12:33:47 +1200 Subject: [PATCH 13/40] manually implement encode & decode to avoid i8 workaround --- core/sr-primitives/src/lib.rs | 27 +++++++++++++++++++++------ srml/executive/src/lib.rs | 5 ++--- srml/support/src/error.rs | 4 ++-- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index ac798fd44bdb5..c9754b861de13 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -536,25 +536,40 @@ pub enum ApplyError { FullBlock = 255, } -impl codec::Encode for ApplyError { +impl Encode for ApplyError { fn using_encoded R>(&self, f: F) -> R { f(&[*self as u8]) } } -#[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] +#[derive(Eq, PartialEq, Clone, Copy)] #[cfg_attr(feature = "std", derive(Debug, Serialize))] /// Reason why a dispatch call failed pub struct DispatchError { /// Module index, matching the metadata module index - pub module: i8, // use i8 instead of u8 because u8 is not supported by parity-codec + pub module: u8, /// Module specific error value - pub error: i8, - /// Optional error message - #[codec(skip)] + pub error: u8, + /// Optional error message. pub message: Option<&'static str>, } +impl Encode for DispatchError { + fn using_encoded R>(&self, f: F) -> R { + f(&[self.module, self.error]) + } +} + +impl Decode for DispatchError { + fn decode(input: &mut R) -> Option { + Some(DispatchError { + module: input.read_byte()?, + error: input.read_byte()?, + message: None, + }) + } +} + /// Result from attempt to apply an extrinsic. pub type ApplyResult = Result; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index d335e7c5dd029..049b99f1a4b78 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -234,9 +234,8 @@ where Ok(ApplyOutcome::Success) => (), Ok(ApplyOutcome::Fail(e)) => { runtime_io::print("Error:"); - // as u8 first to ensure not using sign-extend - runtime_io::print(e.module as u8 as u64); - runtime_io::print(e.error as u8 as u64); + runtime_io::print(e.module as u64); + runtime_io::print(e.error as u64); if let Some(msg) = e.message { runtime_io::print(msg); } diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index 4cb02e594c5cb..d65965f11de63 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -105,7 +105,7 @@ macro_rules! impl_outer_error { }, _ => $crate::runtime_primitives::DispatchError { module: 0, - error: Into::::into(err) as i8, + error: Into::::into(err), message: None, }, }, @@ -119,7 +119,7 @@ macro_rules! impl_outer_error { }, _ => $crate::runtime_primitives::DispatchError { module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), - error: Into::::into(err) as i8, + error: Into::::into(err), message: None, }, }, From 9d2dbbb63fe6a2f559c6de528f7e24f84d545d54 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Fri, 21 Jun 2019 20:53:40 +1200 Subject: [PATCH 14/40] more test fixes --- srml/aura/src/mock.rs | 11 +++--- srml/balances/src/lib.rs | 23 +++++++++++- srml/balances/src/mock.rs | 9 ++++- srml/balances/src/tests.rs | 13 ++++--- srml/support/src/dispatch.rs | 69 +++++++++++++++++++----------------- srml/support/src/error.rs | 19 +++++----- srml/system/src/lib.rs | 2 +- 7 files changed, 91 insertions(+), 55 deletions(-) diff --git a/srml/aura/src/mock.rs b/srml/aura/src/mock.rs index 386938f92069d..9b6e3e6573fec 100644 --- a/srml/aura/src/mock.rs +++ b/srml/aura/src/mock.rs @@ -19,7 +19,7 @@ #![cfg(test)] use primitives::{BuildStorage, traits::IdentityLookup, testing::{Header, UintAuthorityId}}; -use srml_support::impl_outer_origin; +use srml_support::{impl_outer_origin, impl_outer_error}; use runtime_io; use substrate_primitives::{H256, Blake2Hasher}; use crate::{Trait, Module, GenesisConfig}; @@ -28,15 +28,14 @@ impl_outer_origin!{ pub enum Origin for Test {} } +impl_outer_error! { + pub enum Error for Runtime {} +} + // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, PartialEq, Eq, Debug)] pub struct Test; -#[allow(non_camel_case_types)] -pub enum Error { - system(system::Error) -} - impl system::Trait for Test { type Origin = Origin; type Index = u64; diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 811e3d7cdc92a..88f746f9ffaf3 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -152,7 +152,7 @@ use rstd::prelude::*; use rstd::{cmp, result, mem}; use parity_codec::{Codec, Encode, Decode}; -use srml_support::{StorageValue, StorageMap, Parameter, decl_event, decl_storage, decl_module}; +use srml_support::{StorageValue, StorageMap, Parameter, decl_event, decl_storage, decl_module, decl_error}; use srml_support::traits::{ UpdateBalanceOutcome, Currency, OnFreeBalanceZero, MakePayment, OnUnbalanced, WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, @@ -170,6 +170,8 @@ mod tests; pub use self::imbalances::{PositiveImbalance, NegativeImbalance}; +pub type DispatchResult = srml_support::dispatch::DispatchResult<>::Error>; + pub trait Subtrait: system::Trait { /// The balance of an account. type Balance: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + @@ -211,6 +213,9 @@ pub trait Trait: system::Trait { /// The overarching event type. type Event: From> + Into<::Event>; + + /// The overarching error type. + type Error: From + From + From<&'static str>; } impl, I: Instance> Subtrait for T { @@ -233,6 +238,19 @@ decl_event!( } ); +decl_error! { + pub enum Error { + } +} + +impl From for &'static str { + fn from(err: Error) -> &'static str { + match err { + Error::Unknown(msg) => msg, + } + } +} + /// Struct to encode the vesting schedule of an individual account. #[derive(Encode, Decode, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "std", derive(Debug))] @@ -340,6 +358,8 @@ decl_storage! { decl_module! { pub struct Module, I: Instance = DefaultInstance> for enum Call where origin: T::Origin { + type Error = >::Error; + fn deposit_event() = default; /// Transfer some liquid free balance to another account. @@ -707,6 +727,7 @@ impl, I: Instance> Trait for ElevatedTrait { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = &'static str; } impl, I: Instance> Currency for Module diff --git a/srml/balances/src/mock.rs b/srml/balances/src/mock.rs index d97ae38dcc042..b5681f31c3c40 100644 --- a/srml/balances/src/mock.rs +++ b/srml/balances/src/mock.rs @@ -29,8 +29,14 @@ impl_outer_origin!{ pub enum Origin for Runtime {} } +mod balances { + pub use crate::Error; +} + impl_outer_error! { - pub enum Error for Runtime {} + pub enum Error for Runtime { + balances, + } } // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. @@ -56,6 +62,7 @@ impl Trait for Runtime { type TransactionPayment = (); type DustRemoval = (); type TransferPayment = (); + type Error = Error; } pub struct ExtBuilder { diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 0a5a4b5bb70a6..2d85af274e143 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -208,7 +208,7 @@ fn default_indexing_on_new_accounts_should_not_work2() { // account 1 has 256 * 10 = 2560, account 5 is not exist, ext_deposit is 10, value is 9, not satisfies for ext_deposit assert_noop!( Balances::transfer(Some(1).into(), 5, 9), - "value too low to create account" + mock::Error::balances(Error::Unknown("value too low to create account")) ); assert_eq!(Balances::is_dead_account(&5), true); // account 5 should not exist assert_eq!(Balances::free_balance(&1), 100); @@ -347,7 +347,10 @@ fn balance_transfer_when_reserved_should_not_work() { with_externalities(&mut ExtBuilder::default().build(), || { let _ = Balances::deposit_creating(&1, 111); assert_ok!(Balances::reserve(&1, 69)); - assert_noop!(Balances::transfer(Some(1).into(), 2, 69), "balance too low to send value"); + assert_noop!( + Balances::transfer(Some(1).into(), 2, 69), + mock::Error::balances(Error::Unknown("balance too low to send value")) + ); }); } @@ -475,7 +478,7 @@ fn transferring_too_high_value_should_not_panic() { assert_err!( Balances::transfer(Some(1).into(), 2, u64::max_value()), - "destination balance too high to receive value" + mock::Error::balances(Error::Unknown("destination balance too high to receive value")) ); assert_eq!(Balances::free_balance(&1), u64::max_value()); @@ -553,7 +556,7 @@ fn transfer_overflow_isnt_exploitable() { assert_err!( Balances::transfer(Some(1).into(), 5, evil_value), - "got overflow after adding a fee to value" + mock::Error::balances(Error::Unknown("got overflow after adding a fee to value")) ); } ); @@ -617,7 +620,7 @@ fn unvested_balance_should_not_transfer() { assert_eq!(Balances::vesting_balance(&1), 90); // Account 1 has only 10 units vested at block 1 assert_noop!( Balances::transfer(Some(1).into(), 2, 11), - "vesting balance too high to send value" + mock::Error::balances(Error::Unknown("vesting balance too high to send value")) ); // Account 1 cannot send more than vested amount } ); diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index d8bb4ea030f74..c01180c94f693 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -424,7 +424,10 @@ macro_rules! decl_module { }; (@normalize $(#[$attr:meta])* - pub struct $mod_type:ident<$trait_instance:ident: $trait_name:ident> + pub struct $mod_type:ident< + $trait_instance:ident: + $trait_name:ident$(, $instance:ident: $instantiable:path $(= $module_default_instance:path)?)? + > for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } @@ -438,7 +441,9 @@ macro_rules! decl_module { ) => { $crate::decl_module!(@normalize $(#[$attr])* - pub struct $mod_type<$trait_instance: $trait_name> + pub struct $mod_type< + $trait_instance: $trait_name$(, $instance: $instantiable $(= $module_default_instance)?)? + > for enum $call_type where origin: $origin_type, system = $system { $( $deposit_event )* } { $( $on_initialize )* } @@ -449,7 +454,7 @@ macro_rules! decl_module { $($rest)* ); }; - // This puts the function statement into the [], decreasing `$rest` and moving toward finishing the parse. + // Add default Error if none supplied (@normalize $(#[$attr:meta])* pub struct $mod_type:ident< @@ -461,13 +466,8 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } - { $error_type:ty } + { } [ $($t:tt)* ] - $(#[doc = $doc_attr:tt])* - #[weight = $weight:expr] - $fn_vis:vis fn $fn_name:ident( - $origin:ident $(, $(#[$codec_attr:ident])* $param_name:ident : $param:ty)* - ) $( -> $result:ty )* { $( $impl:tt )* } $($rest:tt)* ) => { $crate::decl_module!(@normalize @@ -480,20 +480,12 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } - { $error_type } - [ - $($t)* - $(#[doc = $doc_attr])* - #[weight = $weight] - $fn_vis fn $fn_name( - $origin $( , $(#[$codec_attr])* $param_name : $param )* - ) $( -> $result )* { $( $impl )* } - { $($instance: $instantiable)? } - ] + { &'static str } + [ $($t)* ] $($rest)* ); }; - // Add #[weight] if none is defined. + // This puts the function statement into the [], decreasing `$rest` and moving toward finishing the parse. (@normalize $(#[$attr:meta])* pub struct $mod_type:ident< @@ -505,11 +497,12 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } - { $( $error_type:tt )* } + { $error_type:ty } [ $($t:tt)* ] $(#[doc = $doc_attr:tt])* + #[weight = $weight:expr] $fn_vis:vis fn $fn_name:ident( - $from:ident $(, $(#[$codec_attr:ident])* $param_name:ident : $param:ty)* + $origin:ident $(, $(#[$codec_attr:ident])* $param_name:ident : $param:ty)* ) $( -> $result:ty )* { $( $impl:tt )* } $($rest:tt)* ) => { @@ -523,17 +516,20 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } - { $( $error_type )* } - [ $($t)* ] - $(#[doc = $doc_attr])* - #[weight = $crate::dispatch::TransactionWeight::default()] - $fn_vis fn $fn_name( - $from $(, $(#[$codec_attr])* $param_name : $param )* - ) $( -> $result )* { $( $impl )* } + { $error_type } + [ + $($t)* + $(#[doc = $doc_attr])* + #[weight = $weight] + $fn_vis fn $fn_name( + $origin $( , $(#[$codec_attr])* $param_name : $param )* + ) $( -> $result )* { $( $impl )* } + { $($instance: $instantiable)? } + ] $($rest)* ); }; - // Add default Error if none supplied + // Add #[weight] if none is defined. (@normalize $(#[$attr:meta])* pub struct $mod_type:ident< @@ -545,8 +541,12 @@ macro_rules! decl_module { { $( $on_initialize:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } - { } + { $( $error_type:tt )* } [ $($t:tt)* ] + $(#[doc = $doc_attr:tt])* + $fn_vis:vis fn $fn_name:ident( + $from:ident $(, $(#[$codec_attr:ident])* $param_name:ident : $param:ty)* + ) $( -> $result:ty )* { $( $impl:tt )* } $($rest:tt)* ) => { $crate::decl_module!(@normalize @@ -559,8 +559,13 @@ macro_rules! decl_module { { $( $on_initialize )* } { $( $on_finalize )* } { $( $offchain )* } - { &'static str } + { $( $error_type )* } [ $($t)* ] + $(#[doc = $doc_attr])* + #[weight = $crate::dispatch::TransactionWeight::default()] + $fn_vis fn $fn_name( + $from $(, $(#[$codec_attr])* $param_name : $param )* + ) $( -> $result )* { $( $impl )* } $($rest)* ); }; diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index d65965f11de63..c4b83e796b8f1 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -96,7 +96,7 @@ macro_rules! impl_outer_error { impl Into<$crate::runtime_primitives::DispatchError> for $name { fn into(self) -> $crate::runtime_primitives::DispatchError { match self { - $name::system(err) => match err { + $name::system(ref err) => match err { $system::Error::Unknown(msg) => $crate::runtime_primitives::DispatchError { module: 0, @@ -110,15 +110,15 @@ macro_rules! impl_outer_error { }, }, $( - $name::$module(err) => match err { + $name::$module(ref err) => match err { $module::Error::Unknown(msg) => $crate::runtime_primitives::DispatchError { - module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), + module: $crate::codec::Encode::using_encoded(&self, |s| s[0]), error: 0, message: Some(msg), }, _ => $crate::runtime_primitives::DispatchError { - module: $crate::codec::Encode.using_encoded(&self, |s| s[0]), + module: $crate::codec::Encode::using_encoded(&self, |s| s[0]), error: Into::::into(err), message: None, }, @@ -130,7 +130,7 @@ macro_rules! impl_outer_error { $( impl From<$module::Error> for $name { - fn from(err: $system::Error) -> Self { + fn from(err: $module::Error) -> Self { $name::$module(err) } } @@ -174,11 +174,12 @@ macro_rules! decl_error { impl From for () { fn from(_: Error) -> () { () } } - impl Into for Error { - fn into(self) -> u8 { - match self { + + impl From<&Error> for u8 { + fn from(err: &Error) -> u8 { + match err { Error::Unknown(_) => 0, - _ => $crate::codec::Encode::using_encoded(&self, |s| s[0]), + _ => $crate::codec::Encode::using_encoded(err, |s| s[0]), } } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index a487da0238d90..ab9c82a249a39 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -202,7 +202,7 @@ pub type KeyValue = (Vec, Vec); decl_module! { pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + type Error = T::Error; /// Deposits an event into this block's event record. pub fn deposit_event(event: T::Event) { From 31c9c845f0928f33f2681f6b7d536462b080f664 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Fri, 21 Jun 2019 22:01:51 +1200 Subject: [PATCH 15/40] more fixes --- node-template/runtime/src/lib.rs | 2 ++ node-template/runtime/src/template.rs | 5 ++--- node/runtime/src/lib.rs | 1 + srml/contracts/src/tests.rs | 5 ++++- srml/council/src/lib.rs | 5 ++++- srml/democracy/src/lib.rs | 5 ++++- srml/example/src/lib.rs | 5 ++++- srml/executive/src/lib.rs | 7 +++++-- srml/staking/src/mock.rs | 5 ++++- srml/treasury/src/lib.rs | 5 ++++- 10 files changed, 34 insertions(+), 11 deletions(-) diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 26c046b2787a5..0d783d69f6136 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -162,6 +162,8 @@ impl balances::Trait for Runtime { type OnNewAccount = Indices; /// The ubiquitous event type. type Event = Event; + /// The ubiquitous event type. + type Error = Error; type TransactionPayment = (); type DustRemoval = (); diff --git a/node-template/runtime/src/template.rs b/node-template/runtime/src/template.rs index dcd50f5c1a709..3124b67f7deb2 100644 --- a/node-template/runtime/src/template.rs +++ b/node-template/runtime/src/template.rs @@ -82,9 +82,8 @@ mod tests { pub enum Origin for Test {} } - #[allow(non_camel_case_types)] - pub enum Error { - system(system::Error) + impl_outer_error! { + pub enum Error for Test {} } // For testing the module, we construct most of a mock runtime. This means diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index f43b27b92e6fc..ff4632690cedc 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -119,6 +119,7 @@ impl balances::Trait for Runtime { type TransactionPayment = (); type DustRemoval = (); type TransferPayment = (); + type Error = Error; } impl timestamp::Trait for Runtime { diff --git a/srml/contracts/src/tests.rs b/srml/contracts/src/tests.rs index 84097c728a1eb..c416379c60847 100644 --- a/srml/contracts/src/tests.rs +++ b/srml/contracts/src/tests.rs @@ -65,7 +65,9 @@ impl_outer_dispatch! { } impl_outer_error! { - pub enum Error for Test {} + pub enum Error for Test { + balances + } } #[derive(Clone, Eq, PartialEq, Debug)] @@ -90,6 +92,7 @@ impl balances::Trait for Test { type TransactionPayment = (); type DustRemoval = (); type TransferPayment = (); + type Error = Error; } impl timestamp::Trait for Test { type Moment = u64; diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index 3b61ef89fc223..76599d5f100dd 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -65,7 +65,9 @@ mod tests { } impl_outer_error! { - pub enum Error for Test {} + pub enum Error for Test { + balances + } } // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. @@ -91,6 +93,7 @@ mod tests { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = Error; } parameter_types! { pub const LaunchPeriod: u64 = 1; diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index 695c16fadf4d5..ca60292452ae7 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -940,7 +940,9 @@ mod tests { } impl_outer_error! { - pub enum Error for Test {} + pub enum Error for Test { + balances + } } // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. @@ -966,6 +968,7 @@ mod tests { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = Error; } parameter_types! { pub const LaunchPeriod: u64 = 2; diff --git a/srml/example/src/lib.rs b/srml/example/src/lib.rs index a15e9d5f2bf5d..74eef11fcdfe3 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -519,7 +519,9 @@ mod tests { } impl_outer_error! { - pub enum Error for Test {} + pub enum Error for Test { + balances + } } // For testing the module, we construct most of a mock runtime. This means @@ -547,6 +549,7 @@ mod tests { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = Error; } impl Trait for Test { type Event = (); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 049b99f1a4b78..91948a2ecd40b 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -417,7 +417,9 @@ mod tests { } impl_outer_error! { - pub enum Error for Runtime {} + pub enum Error for Runtime { + balances + } } // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. @@ -443,6 +445,7 @@ mod tests { type TransactionPayment = (); type DustRemoval = (); type TransferPayment = (); + type Error = Error; } impl ValidateUnsigned for Runtime { @@ -639,7 +642,7 @@ mod tests { with_externalities(&mut t, || { assert_eq!(Executive::validate_transaction(xt.clone()), valid); - assert_eq!(Executive::apply_extrinsic(xt), Ok(ApplyOutcome::Fail)); + assert_eq!(Executive::apply_extrinsic(xt), Ok(ApplyOutcome::Fail(DispatchError { module: 0, error: 0, message: None }))); }); } } diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index 2e6a8731d051a..7dc368cc52319 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -71,7 +71,9 @@ impl_outer_origin!{ } impl_outer_error! { - pub enum Error for Test {} + pub enum Error for Test { + balances + } } // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. @@ -97,6 +99,7 @@ impl balances::Trait for Test { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = Error; } parameter_types! { pub const Period: BlockNumber = 1; diff --git a/srml/treasury/src/lib.rs b/srml/treasury/src/lib.rs index 972d51231a1c9..690c24f3f5716 100644 --- a/srml/treasury/src/lib.rs +++ b/srml/treasury/src/lib.rs @@ -345,7 +345,9 @@ mod tests { } impl_outer_error! { - pub enum Error for Test where system = system {} + pub enum Error for Test { + balances + } } #[derive(Clone, Eq, PartialEq)] @@ -370,6 +372,7 @@ mod tests { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = Error; } impl Trait for Test { type Currency = balances::Module; From 6cad33a73b76aaabed1c004a21a8b6d30ae47f95 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 16:30:16 +1200 Subject: [PATCH 16/40] more error fixes --- node-template/runtime/src/lib.rs | 2 +- node-template/runtime/src/template.rs | 2 +- node/runtime/src/lib.rs | 2 +- srml/contracts/src/tests.rs | 2 ++ srml/council/src/lib.rs | 2 ++ srml/democracy/src/lib.rs | 4 +++- srml/staking/src/tests.rs | 10 ++++++++-- 7 files changed, 18 insertions(+), 6 deletions(-) diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 0d783d69f6136..b685201800126 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -191,7 +191,7 @@ construct_runtime!( Timestamp: timestamp::{Module, Call, Storage, Config, Inherent}, Aura: aura::{Module, Config, Inherent(Timestamp)}, Indices: indices::{default, Config}, - Balances: balances, + Balances: balances::{default, Error}, Sudo: sudo, // Used for the module template in `./template.rs` TemplateModule: template::{Module, Call, Storage, Event}, diff --git a/node-template/runtime/src/template.rs b/node-template/runtime/src/template.rs index 3124b67f7deb2..eb138b032c7fd 100644 --- a/node-template/runtime/src/template.rs +++ b/node-template/runtime/src/template.rs @@ -71,7 +71,7 @@ mod tests { use runtime_io::with_externalities; use primitives::{H256, Blake2Hasher}; - use support::{impl_outer_origin, assert_ok}; + use support::{impl_outer_origin, impl_outer_error, assert_ok}; use runtime_primitives::{ BuildStorage, traits::{BlakeTwo256, IdentityLookup}, diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index ff4632690cedc..e4677aeb49f8e 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -253,7 +253,7 @@ construct_runtime!( Aura: aura::{Module, Config, Inherent(Timestamp)}, Timestamp: timestamp::{Module, Call, Storage, Config, Inherent}, Indices: indices, - Balances: balances, + Balances: balances::{default, Error}, Session: session::{Module, Call, Storage, Event, Config}, Staking: staking::{default, OfflineWorker}, Democracy: democracy, diff --git a/srml/contracts/src/tests.rs b/srml/contracts/src/tests.rs index c416379c60847..b18b9bf26163d 100644 --- a/srml/contracts/src/tests.rs +++ b/srml/contracts/src/tests.rs @@ -59,6 +59,8 @@ impl_outer_origin! { } impl_outer_dispatch! { pub enum Call for Test where origin: Origin { + type Error = Error; + balances::Balances, contract::Contract, } diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index 76599d5f100dd..b21cb145adc24 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -59,6 +59,8 @@ mod tests { impl_outer_dispatch! { pub enum Call for Test where origin: Origin { + type Error = Error; + balances::Balances, democracy::Democracy, } diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index ca60292452ae7..4823d650a0885 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -934,6 +934,8 @@ mod tests { impl_outer_dispatch! { pub enum Call for Test where origin: Origin { + type Error = Error; + balances::Balances, democracy::Democracy, } @@ -941,7 +943,7 @@ mod tests { impl_outer_error! { pub enum Error for Test { - balances + balances, } } diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index 180ce130ae414..0bdd4d5abadca 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -818,7 +818,10 @@ fn cannot_transfer_staked_balance() { // Confirm account 11 (via controller 10) is totally staked assert_eq!(Staking::stakers(&11).total, 1000); // Confirm account 11 cannot transfer as a result - assert_noop!(Balances::transfer(Origin::signed(11), 20, 1), "account liquidity restrictions prevent withdrawal"); + assert_noop!( + Balances::transfer(Origin::signed(11), 20, 1), + mock::Error::system(system::Error::Unknown("account liquidity restrictions prevent withdrawal")) + ); // Give account 11 extra free balance let _ = Balances::make_free_balance_be(&11, 10000); @@ -844,7 +847,10 @@ fn cannot_transfer_staked_balance_2() { // Confirm account 21 (via controller 20) is totally staked assert_eq!(Staking::stakers(&21).total, 1000); // Confirm account 21 can transfer at most 1000 - assert_noop!(Balances::transfer(Origin::signed(21), 20, 1001), "account liquidity restrictions prevent withdrawal"); + assert_noop!( + Balances::transfer(Origin::signed(21), 20, 1001), + mock::Error::system(system::Error::Unknown("account liquidity restrictions prevent withdrawal")) + ); assert_ok!(Balances::transfer(Origin::signed(21), 20, 1000)); }); } From ba42e6a0f8417194e54ddc468f44753a287c62c4 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Sat, 22 Jun 2019 17:59:47 +1200 Subject: [PATCH 17/40] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Tomasz Drwięga --- core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 080bfa0d0531a..bdf96e15298d6 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -84,7 +84,7 @@ where type Checked = CheckedExtrinsic; type Error = Error; - fn check(self, context: &Context) -> Result { + fn check(self, context: &Context) -> Result { Ok(match self.signature { Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); @@ -202,7 +202,7 @@ mod tests { type Source = u64; type Target = u64; type Error = &'static str; - fn lookup(&self, s: u64) -> Result { Ok(s) } + fn lookup(&self, s: u64) -> Result { Ok(s) } } impl CurrentHeight for TestContext { type BlockNumber = u64; From 0f01cc48625c0bfeaad848e2c3a89c17a4d196bf Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 18:00:11 +1200 Subject: [PATCH 18/40] address comments --- core/client/src/block_builder/block_builder.rs | 3 ++- srml/executive/src/lib.rs | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/client/src/block_builder/block_builder.rs b/core/client/src/block_builder/block_builder.rs index 72b0d5f5838db..882791f95a79a 100644 --- a/core/client/src/block_builder/block_builder.rs +++ b/core/client/src/block_builder/block_builder.rs @@ -17,6 +17,7 @@ use super::api::BlockBuilder as BlockBuilderApi; use std::vec::Vec; use parity_codec::Encode; +use runtime_primitives::ApplyOutcome; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{ Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef, DigestFor, @@ -103,7 +104,7 @@ where ExecutionContext::BlockConstruction, xt.clone() )? { - Ok(_) => { + Ok(ApplyOutcome::Success) | Ok(ApplyOutcome::Fail(_)) => { extrinsics.push(xt); Ok(()) } diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 91948a2ecd40b..2e2e50fa27522 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -337,16 +337,16 @@ where // Checks out. Carry on. Ok(xt) => xt, Err(err) => { - match err.into() { + return match err.into() { // An unknown account index implies that the transaction may yet become valid. // TODO: avoid hardcoded error string here PrimitiveError::Unknown("invalid account index") => - return TransactionValidity::Unknown(INVALID_INDEX), + TransactionValidity::Unknown(INVALID_INDEX), // Technically a bad signature could also imply an out-of-date account index, but // that's more of an edge case. PrimitiveError::BadSignature => - return TransactionValidity::Invalid(ApplyError::BadSignature as i8), - _ => return TransactionValidity::Invalid(UNKNOWN_ERROR), + TransactionValidity::Invalid(ApplyError::BadSignature as i8), + _ => TransactionValidity::Invalid(UNKNOWN_ERROR), } } }; From c7ae1b177f57e11ddc43b74ce3f998f1de4f8bd8 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 18:17:15 +1200 Subject: [PATCH 19/40] test for DispatchError encoding --- core/sr-primitives/src/lib.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index c9754b861de13..aee62937f2331 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -688,6 +688,7 @@ impl traits::Extrinsic for OpaqueExtrinsic { #[cfg(test)] mod tests { + use super::DispatchError; use crate::codec::{Encode, Decode}; macro_rules! per_thing_mul_upper_test { @@ -786,4 +787,21 @@ mod tests { ((Into::::into(std::u128::MAX) * 999_999u32) / 1_000_000u32).as_u128() ); } + + #[test] + fn dispatch_error_encoding() { + let error = DispatchError { + module: 1, + error: 2, + message: Some("error message"), + }; + let encoded = error.encode(); + let decoded = DispatchError::decode(&mut &*encoded).unwrap(); + assert_eq!(encoded, vec![1, 2]); + assert_eq!(decoded, DispatchError { + module: 1, + error: 2, + message: None, + }); + } } From 0e424c1b450895dce08480a0e2cd3201df4f95c8 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 18:18:57 +1200 Subject: [PATCH 20/40] tyep alias for democracy --- srml/democracy/src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index 4823d650a0885..f43e5d7c993fb 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -164,6 +164,7 @@ impl Decode for Vote { } type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +type Error = &'static str; pub trait Trait: system::Trait + Sized { type Proposal: Parameter + Dispatchable + IsSubType>; @@ -191,23 +192,23 @@ pub trait Trait: system::Trait + Sized { /// Origin from which the next tabled referendum may be forced. This is a normal /// "super-majority-required" referendum. - type ExternalOrigin: EnsureOrigin; + type ExternalOrigin: EnsureOrigin; /// Origin from which the next tabled referendum may be forced; this allows for the tabling of /// a majority-carries referendum. - type ExternalMajorityOrigin: EnsureOrigin; + type ExternalMajorityOrigin: EnsureOrigin; /// Origin from which emergency referenda may be scheduled. - type EmergencyOrigin: EnsureOrigin; + type EmergencyOrigin: EnsureOrigin; /// Minimum voting period allowed for an emergency referendum. type EmergencyVotingPeriod: Get; /// Origin from which any referenda may be cancelled in an emergency. - type CancellationOrigin: EnsureOrigin; + type CancellationOrigin: EnsureOrigin; /// Origin for anyone able to veto proposals. - type VetoOrigin: EnsureOrigin; + type VetoOrigin: EnsureOrigin; /// Period in blocks where an external proposal may not be re-submitted after being vetoed. type CooloffPeriod: Get; From a145f271c44281e7c3c9e2765c1535c9a8095b13 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 18:30:45 +1200 Subject: [PATCH 21/40] make error printable --- srml/sudo/src/lib.rs | 7 +++---- srml/support/src/error.rs | 12 ++++++++++++ srml/support/src/lib.rs | 2 +- srml/system/src/lib.rs | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/srml/sudo/src/lib.rs b/srml/sudo/src/lib.rs index 73b311b1d77cd..90fedc2fe6f89 100644 --- a/srml/sudo/src/lib.rs +++ b/srml/sudo/src/lib.rs @@ -99,7 +99,7 @@ pub trait Trait: system::Trait { type Event: From> + Into<::Event>; /// A sudo-able call. - type Proposal: Parameter + Dispatchable; + type Proposal: Parameter + Dispatchable::Error>; } decl_module! { @@ -123,9 +123,8 @@ decl_module! { let res = match proposal.dispatch(system::RawOrigin::Root.into()) { Ok(_) => true, - Err(_e) => { - // TODO: not sure how to deal with this - // sr_io::print(e); + Err(e) => { + sr_io::print(e); false } }; diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index c4b83e796b8f1..fa990880bb8cc 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -82,6 +82,18 @@ macro_rules! impl_outer_error { } } + impl $crate::Printable for $name { + fn print(self) { + $crate::print("Error:"); + let err = Into::<$crate::runtime_primitives::DispatchError>::into(self); + $crate::print(err.module as u64); + $crate::print(err.error as u64); + if let Some(msg) = err.message { + $crate::print(msg); + } + } + } + impl $crate::rstd::convert::TryInto<$system::Error> for $name { type Error = Self; fn try_into(self) -> $crate::dispatch::result::Result<$system::Error, Self::Error> { diff --git a/srml/support/src/lib.rs b/srml/support/src/lib.rs index cf09f0f634adc..a6cef4b0bdfb7 100644 --- a/srml/support/src/lib.rs +++ b/srml/support/src/lib.rs @@ -65,7 +65,7 @@ pub use self::storage::{ pub use self::hashable::Hashable; pub use self::dispatch::{Parameter, Dispatchable, Callable, IsSubType}; pub use self::double_map::StorageDoubleMapWithHasher; -pub use runtime_io::{print, storage_root}; +pub use runtime_io::{print, storage_root, Printable}; /// Macro for easily creating a new implementation of the `Get` trait. Use similarly to /// how you would declare a `const`: diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index ab9c82a249a39..2b2289262a85f 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -191,7 +191,7 @@ pub trait Trait: 'static + Eq + Clone { type Event: Parameter + Member + From; /// The aggregated error type of the runtime. - type Error: Member + From + From<&'static str>; + type Error: Member + From + From<&'static str> + runtime_io::Printable; } pub type DigestOf = generic::Digest<::Hash>; From ef4eecdc3fc855407ab64efc59a02539a578bc87 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 18:35:53 +1200 Subject: [PATCH 22/40] line width --- srml/executive/src/lib.rs | 5 ++++- srml/support/src/dispatch.rs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 2e2e50fa27522..6670a6dfae3a5 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -642,7 +642,10 @@ mod tests { with_externalities(&mut t, || { assert_eq!(Executive::validate_transaction(xt.clone()), valid); - assert_eq!(Executive::apply_extrinsic(xt), Ok(ApplyOutcome::Fail(DispatchError { module: 0, error: 0, message: None }))); + assert_eq!( + Executive::apply_extrinsic(xt), + Ok(ApplyOutcome::Fail(DispatchError { module: 0, error: 0, message: None })) + ); }); } } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index c01180c94f693..e6018579537dc 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -1208,7 +1208,10 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $mod_type<$trait_instance $(, $instance)?> { #[doc(hidden)] - pub fn dispatch>(d: D, origin: D::Origin) -> $crate::dispatch::DispatchResult { + pub fn dispatch>( + d: D, + origin: D::Origin + ) -> $crate::dispatch::DispatchResult { d.dispatch(origin) } } From 33668d85856ba2a4af678183c893cfafcb5623e7 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 19:04:00 +1200 Subject: [PATCH 23/40] fix balances tests --- srml/balances/src/tests.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 2d85af274e143..39fdc4ae26198 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -208,7 +208,7 @@ fn default_indexing_on_new_accounts_should_not_work2() { // account 1 has 256 * 10 = 2560, account 5 is not exist, ext_deposit is 10, value is 9, not satisfies for ext_deposit assert_noop!( Balances::transfer(Some(1).into(), 5, 9), - mock::Error::balances(Error::Unknown("value too low to create account")) + mock::Error::system(system::Error::Unknown("value too low to create account")) ); assert_eq!(Balances::is_dead_account(&5), true); // account 5 should not exist assert_eq!(Balances::free_balance(&1), 100); @@ -349,7 +349,7 @@ fn balance_transfer_when_reserved_should_not_work() { assert_ok!(Balances::reserve(&1, 69)); assert_noop!( Balances::transfer(Some(1).into(), 2, 69), - mock::Error::balances(Error::Unknown("balance too low to send value")) + mock::Error::system(system::Error::Unknown("balance too low to send value")) ); }); } @@ -478,7 +478,7 @@ fn transferring_too_high_value_should_not_panic() { assert_err!( Balances::transfer(Some(1).into(), 2, u64::max_value()), - mock::Error::balances(Error::Unknown("destination balance too high to receive value")) + mock::Error::system(system::Error::Unknown("destination balance too high to receive value")) ); assert_eq!(Balances::free_balance(&1), u64::max_value()); @@ -556,7 +556,7 @@ fn transfer_overflow_isnt_exploitable() { assert_err!( Balances::transfer(Some(1).into(), 5, evil_value), - mock::Error::balances(Error::Unknown("got overflow after adding a fee to value")) + mock::Error::system(system::Error::Unknown("got overflow after adding a fee to value")) ); } ); @@ -620,7 +620,7 @@ fn unvested_balance_should_not_transfer() { assert_eq!(Balances::vesting_balance(&1), 90); // Account 1 has only 10 units vested at block 1 assert_noop!( Balances::transfer(Some(1).into(), 2, 11), - mock::Error::balances(Error::Unknown("vesting balance too high to send value")) + mock::Error::system(system::Error::Unknown("vesting balance too high to send value")) ); // Account 1 cannot send more than vested amount } ); From d128a1af462f06e02519f4bca212f792179ee0be Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 22:15:34 +1200 Subject: [PATCH 24/40] fix executive test --- srml/executive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 6670a6dfae3a5..41e239fe2a462 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -644,7 +644,7 @@ mod tests { assert_eq!(Executive::validate_transaction(xt.clone()), valid); assert_eq!( Executive::apply_extrinsic(xt), - Ok(ApplyOutcome::Fail(DispatchError { module: 0, error: 0, message: None })) + Ok(ApplyOutcome::Fail(DispatchError { module: 0, error: 4 /*RequireNoOrigin*/ , message: None })) ); }); } From a4a8cdc8588b1e8b3829e79f5c27092b62dc794f Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 22:58:51 +1200 Subject: [PATCH 25/40] fix system tests --- srml/system/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 2b2289262a85f..ad13a311e3a2d 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -895,7 +895,7 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), event: 42u16, topics: vec![] }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: 100u16, topics: vec![] }, - EventRecord { phase: Phase::ApplyExtrinsic(1), event: 101u16, topics: vec![] }, + EventRecord { phase: Phase::ApplyExtrinsic(1), event: 0x0201u16, topics: vec![] }, EventRecord { phase: Phase::Finalization, event: 3u16, topics: vec![] } ]); }); From f4387fbb8623bc763fc02dcc40578009001fa693 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 23:29:42 +1200 Subject: [PATCH 26/40] bump version --- node/runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 70a03ab5faacb..6b74f8499389c 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -58,8 +58,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("node"), impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, - spec_version: 97, - impl_version: 98, + spec_version: 99, + impl_version: 99, apis: RUNTIME_API_VERSIONS, }; From ef2eaa6fb0633709a9852b5d46eafba9b396986c Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 22 Jun 2019 23:36:51 +1200 Subject: [PATCH 27/40] ensure consistent method signature --- core/sr-primitives/src/generic/unchecked_extrinsic.rs | 2 +- .../src/generic/unchecked_mortal_compact_extrinsic.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 0b8cc42bd137a..5706e8d428b3f 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -89,7 +89,7 @@ where type Checked = CheckedExtrinsic; type Error = Error; - fn check(self, context: &Context) -> Result { + fn check(self, context: &Context) -> Result { Ok(match self.signature { Some(SignatureContent{signed, signature, index}) => { let payload = (index, self.function); diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 44003fc0d302c..80e07da1ea0b2 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -83,7 +83,7 @@ where type Checked = CheckedExtrinsic; type Error = Error; - fn check(self, context: &Context) -> Result { + fn check(self, context: &Context) -> Result { Ok(match self.signature { Some((signed, signature, index, era)) => { let current_u64 = context.current_height().saturated_into::(); From 31c998f736bd904f60164727f30f06be98e9a47e Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 26 Jun 2019 13:33:43 +1200 Subject: [PATCH 28/40] Apply suggestions from code review Co-Authored-By: Gavin Wood --- core/sr-primitives/src/lib.rs | 4 ++-- core/sr-primitives/src/traits.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index aee62937f2331..43eea029b18c2 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -64,11 +64,11 @@ pub enum Error { /// cause the fees are not being paid if this error is returned. /// /// Example: block gas limit is reached (the transaction can be retried in the next block though). - BlockFull + BlockFull, } // Exists for for backward compatibility purpose. -impl Into<&'static str> for Error{ +impl Into<&'static str> for Error { fn into(self) -> &'static str { match self { Error::Unknown(val) => val, diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 03f6dc4dc65cd..270d9fe84a0b4 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -90,7 +90,7 @@ pub trait EnsureOrigin { } impl EnsureOriginError for () { - fn invalid_origin() -> () { } + fn invalid_origin() -> () {} } impl EnsureOriginError for &'static str { From 51ec6283ff1fb3a1fd9e1844d83229e40e387a63 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 26 Jun 2019 14:03:41 +1200 Subject: [PATCH 29/40] changes based on review --- node/runtime/src/lib.rs | 14 +++++++------- srml/council/src/lib.rs | 10 +++++----- srml/council/src/motions.rs | 38 +++++++++++++++++++++---------------- srml/executive/src/lib.rs | 22 ++++++++++----------- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 6b74f8499389c..5906b4c4cc0a2 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -187,11 +187,11 @@ impl democracy::Trait for Runtime { type VotingPeriod = VotingPeriod; type EmergencyVotingPeriod = EmergencyVotingPeriod; type MinimumDeposit = MinimumDeposit; - type ExternalOrigin = council_motions::EnsureProportionAtLeast<_1, _2, AccountId>; - type ExternalMajorityOrigin = council_motions::EnsureProportionAtLeast<_2, _3, AccountId>; - type EmergencyOrigin = council_motions::EnsureProportionAtLeast<_1, _1, AccountId>; - type CancellationOrigin = council_motions::EnsureProportionAtLeast<_2, _3, AccountId>; - type VetoOrigin = council_motions::EnsureMember; + type ExternalOrigin = council_motions::EnsureProportionAtLeast<_1, _2, AccountId, &'static str>; + type ExternalMajorityOrigin = council_motions::EnsureProportionAtLeast<_2, _3, AccountId, &'static str>; + type EmergencyOrigin = council_motions::EnsureProportionAtLeast<_1, _1, AccountId, &'static str>; + type CancellationOrigin = council_motions::EnsureProportionAtLeast<_2, _3, AccountId, &'static str>; + type VetoOrigin = council_motions::EnsureMember; type CooloffPeriod = CooloffPeriod; } @@ -212,8 +212,8 @@ impl council::motions::Trait for Runtime { impl treasury::Trait for Runtime { type Currency = Balances; - type ApproveOrigin = council_motions::EnsureMembers<_4, AccountId>; - type RejectOrigin = council_motions::EnsureMembers<_2, AccountId>; + type ApproveOrigin = council_motions::EnsureMembers<_4, AccountId, &'static str>; + type RejectOrigin = council_motions::EnsureMembers<_2, AccountId, &'static str>; type Event = Event; type MintedForSpending = (); type ProposalRejection = (); diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index b21cb145adc24..7bead543a9049 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -113,11 +113,11 @@ mod tests { type EmergencyVotingPeriod = VotingPeriod; type VotingPeriod = VotingPeriod; type MinimumDeposit = MinimumDeposit; - type ExternalOrigin = motions::EnsureProportionAtLeast<_1, _2, u64>; - type ExternalMajorityOrigin = motions::EnsureProportionAtLeast<_2, _3, u64>; - type EmergencyOrigin = motions::EnsureProportionAtLeast<_1, _1, u64>; - type CancellationOrigin = motions::EnsureProportionAtLeast<_2, _3, u64>; - type VetoOrigin = motions::EnsureMember; + type ExternalOrigin = motions::EnsureProportionAtLeast<_1, _2, u64, &'static str>; + type ExternalMajorityOrigin = motions::EnsureProportionAtLeast<_2, _3, u64, &'static str>; + type EmergencyOrigin = motions::EnsureProportionAtLeast<_1, _1, u64, &'static str>; + type CancellationOrigin = motions::EnsureProportionAtLeast<_2, _3, u64, &'static str>; + type VetoOrigin = motions::EnsureMember; type CooloffPeriod = CooloffPeriod; } impl seats::Trait for Test { diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index d3e69c935225e..3d2ee1506d5ab 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -18,7 +18,7 @@ use rstd::{prelude::*, result}; use substrate_primitives::u32_trait::Value as U32; -use primitives::traits::{Hash, EnsureOrigin}; +use primitives::traits::{Hash, EnsureOrigin, EnsureOriginError}; use srml_support::{ dispatch::{Dispatchable, Parameter}, codec::{Encode, Decode}, StorageValue, StorageMap, decl_module, decl_event, decl_storage, ensure @@ -257,13 +257,14 @@ pub fn ensure_council_members(o: OuterOrigin, n: MemberC } } -pub struct EnsureMember(::rstd::marker::PhantomData); +pub struct EnsureMember(::rstd::marker::PhantomData<(AccountId, Error)>); impl< O: Into, O>> + From>, - AccountId -> EnsureOrigin for EnsureMember { + AccountId, + Error: EnsureOriginError, +> EnsureOrigin for EnsureMember { type Success = AccountId; - type Error = &'static str; + type Error = Error; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Member(id) => Ok(id), @@ -272,14 +273,17 @@ impl< } } -pub struct EnsureMembers(::rstd::marker::PhantomData<(N, AccountId)>); +pub struct EnsureMembers( + ::rstd::marker::PhantomData<(N, AccountId, Error)> +); impl< O: Into, O>> + From>, N: U32, AccountId, -> EnsureOrigin for EnsureMembers { + Error: EnsureOriginError, +> EnsureOrigin for EnsureMembers { type Success = (MemberCount, MemberCount); - type Error = &'static str; + type Error = Error; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n >= N::VALUE => Ok((n, m)), @@ -288,17 +292,18 @@ impl< } } -pub struct EnsureProportionMoreThan( - ::rstd::marker::PhantomData<(N, D, AccountId)> +pub struct EnsureProportionMoreThan( + ::rstd::marker::PhantomData<(N, D, AccountId, Error)> ); impl< O: Into, O>> + From>, N: U32, D: U32, AccountId, -> EnsureOrigin for EnsureProportionMoreThan { + Error: EnsureOriginError, +> EnsureOrigin for EnsureProportionMoreThan { type Success = (); - type Error = &'static str; + type Error = Error; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n * D::VALUE > N::VALUE * m => Ok(()), @@ -307,17 +312,18 @@ impl< } } -pub struct EnsureProportionAtLeast( - ::rstd::marker::PhantomData<(N, D, AccountId)> +pub struct EnsureProportionAtLeast( + ::rstd::marker::PhantomData<(N, D, AccountId, Error)> ); impl< O: Into, O>> + From>, N: U32, D: U32, AccountId, -> EnsureOrigin for EnsureProportionAtLeast { + Error: EnsureOriginError, +> EnsureOrigin for EnsureProportionAtLeast { type Success = (); - type Error = &'static str; + type Error = Error; fn try_origin(o: O) -> Result { o.into().and_then(|o| match o { RawOrigin::Members(n, m) if n * D::VALUE >= N::VALUE * m => Ok(()), diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 41e239fe2a462..43e62b8d302f6 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -336,18 +336,16 @@ where let xt = match uxt.check(&Default::default()) { // Checks out. Carry on. Ok(xt) => xt, - Err(err) => { - return match err.into() { - // An unknown account index implies that the transaction may yet become valid. - // TODO: avoid hardcoded error string here - PrimitiveError::Unknown("invalid account index") => - TransactionValidity::Unknown(INVALID_INDEX), - // Technically a bad signature could also imply an out-of-date account index, but - // that's more of an edge case. - PrimitiveError::BadSignature => - TransactionValidity::Invalid(ApplyError::BadSignature as i8), - _ => TransactionValidity::Invalid(UNKNOWN_ERROR), - } + Err(err) => return match err.into() { + // An unknown account index implies that the transaction may yet become valid. + // TODO: avoid hardcoded error string here + PrimitiveError::Unknown("invalid account index") => + TransactionValidity::Unknown(INVALID_INDEX), + // Technically a bad signature could also imply an out-of-date account index, but + // that's more of an edge case. + PrimitiveError::BadSignature => + TransactionValidity::Invalid(ApplyError::BadSignature as i8), + _ => TransactionValidity::Invalid(UNKNOWN_ERROR), } }; From 88887ee77fc791e6cc156ca159ab2fcc4eea4521 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 26 Jun 2019 14:13:31 +1200 Subject: [PATCH 30/40] Add issue number for TODOs --- srml/executive/src/lib.rs | 4 ++-- srml/system/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 43e62b8d302f6..2c057bc976d4d 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -272,7 +272,7 @@ where } } // pay any fees - // TODO: propagate why can't pay + // TODO: propagate why can't pay #2952 Payment::make_payment(sender, encoded_len).map_err(|_| ApplyError::CantPay)?; // AUDIT: Under no circumstances may this function panic from here onwards. @@ -338,7 +338,7 @@ where Ok(xt) => xt, Err(err) => return match err.into() { // An unknown account index implies that the transaction may yet become valid. - // TODO: avoid hardcoded error string here + // TODO: avoid hardcoded error string here #2953 PrimitiveError::Unknown("invalid account index") => TransactionValidity::Unknown(INVALID_INDEX), // Technically a bad signature could also imply an out-of-date account index, but diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index ad13a311e3a2d..f8dcb4407f0fe 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -178,7 +178,7 @@ pub trait Trait: 'static + Eq + Clone { /// Used to define the type and conversion mechanism for referencing accounts in transactions. It's perfectly /// reasonable for this to be an identity conversion (with the source type being `AccountId`), but other modules /// (e.g. Indices module) may provide more functional/efficient alternatives. - // TODO: avoid &'static str error type + // TODO: avoid &'static str error type #2953 type Lookup: StaticLookup; /// The block header. From c147a8e7ad29e177f005f39030233990f1934e8a Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 29 Jun 2019 15:16:54 +1200 Subject: [PATCH 31/40] fix --- srml/support/src/runtime.rs | 4 +++- srml/support/test/tests/instance.rs | 1 + srml/support/test/tests/issue2219.rs | 3 ++- srml/support/test/tests/system.rs | 8 +++++++- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/srml/support/src/runtime.rs b/srml/support/src/runtime.rs index c162704a2f664..b5006714ca577 100644 --- a/srml/support/src/runtime.rs +++ b/srml/support/src/runtime.rs @@ -219,7 +219,9 @@ macro_rules! construct_runtime { $crate::__decl_outer_error!( $runtime; $( - $name: $module:: $( < $module_instance >:: )? { $( $modules $( <$modules_generic $(, $modules_instance)?> )* ),* } + $name: $module:: $( < $module_instance >:: )? { + $( $modules $( <$modules_generic> )* ),* + } ),* ); $crate::__decl_all_modules!( diff --git a/srml/support/test/tests/instance.rs b/srml/support/test/tests/instance.rs index 28636573d3b55..00cace355a16e 100644 --- a/srml/support/test/tests/instance.rs +++ b/srml/support/test/tests/instance.rs @@ -209,6 +209,7 @@ impl system::Trait for Runtime { type BlockNumber = BlockNumber; type AccountId = AccountId; type Event = Event; + type Error = Error; } srml_support::construct_runtime!( diff --git a/srml/support/test/tests/issue2219.rs b/srml/support/test/tests/issue2219.rs index 185b5e24807a9..0fba65bd750a7 100644 --- a/srml/support/test/tests/issue2219.rs +++ b/srml/support/test/tests/issue2219.rs @@ -160,6 +160,7 @@ impl system::Trait for Runtime { type BlockNumber = BlockNumber; type AccountId = AccountId; type Event = Event; + type Error = Error; } impl module::Trait for Runtime {} @@ -183,4 +184,4 @@ fn create_genesis_config() { enable_storage_role: true, }) }; -} \ No newline at end of file +} diff --git a/srml/support/test/tests/system.rs b/srml/support/test/tests/system.rs index 6483161211afc..5f04021106a59 100644 --- a/srml/support/test/tests/system.rs +++ b/srml/support/test/tests/system.rs @@ -8,6 +8,7 @@ pub trait Trait: 'static + Eq + Clone { type Hash; type AccountId: Encode + Decode; type Event: From; + type Error: From + From<&'static str>; } srml_support::decl_module! { @@ -24,6 +25,11 @@ srml_support::decl_event!( } ); +srml_support::decl_error! { + pub enum Error { + } +} + /// Origin for the system module. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] @@ -49,4 +55,4 @@ pub fn ensure_root(o: OuterOrigin) -> Result<(), &'stati where OuterOrigin: Into, OuterOrigin>> { o.into().map(|_| ()).map_err(|_| "bad origin: expected to be a root origin") -} \ No newline at end of file +} From 633a482723dae72bdc003bd2ce32fb6fc2aaaee3 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 29 Jun 2019 15:20:51 +1200 Subject: [PATCH 32/40] line width --- srml/staking/src/mock.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/srml/staking/src/mock.rs b/srml/staking/src/mock.rs index ee87012e8035b..d9afd1659affa 100644 --- a/srml/staking/src/mock.rs +++ b/srml/staking/src/mock.rs @@ -22,7 +22,9 @@ use primitives::traits::{IdentityLookup, Convert, OpaqueKeys, OnInitialize}; use primitives::testing::{Header, UintAuthorityId}; use substrate_primitives::{H256, Blake2Hasher}; use runtime_io; -use srml_support::{impl_outer_origin, impl_outer_error, parameter_types, assert_ok, traits::Currency, EnumerableStorageMap}; +use srml_support::{ + impl_outer_origin, impl_outer_error, parameter_types, assert_ok, traits::Currency, EnumerableStorageMap +}; use crate::{EraIndex, GenesisConfig, Module, Trait, StakerStatus, ValidatorPrefs, RewardDestination, Nominators }; From b61e13758c95e7d21de2e56b376252d09eb80245 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Sat, 29 Jun 2019 15:53:37 +1200 Subject: [PATCH 33/40] fix test --- srml/authorship/src/lib.rs | 7 ++++++- srml/finality-tracker/src/lib.rs | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/srml/authorship/src/lib.rs b/srml/authorship/src/lib.rs index 8add05bb5930d..ffe7f974492f7 100644 --- a/srml/authorship/src/lib.rs +++ b/srml/authorship/src/lib.rs @@ -315,12 +315,16 @@ mod tests { use primitives::traits::{BlakeTwo256, IdentityLookup}; use primitives::testing::Header; use primitives::generic::DigestItem; - use srml_support::{parameter_types, impl_outer_origin, ConsensusEngineId}; + use srml_support::{parameter_types, impl_outer_origin, impl_outer_error, ConsensusEngineId}; impl_outer_origin!{ pub enum Origin for Test {} } + impl_outer_error!{ + pub enum Error for Test {} + } + #[derive(Clone, Eq, PartialEq)] pub struct Test; @@ -334,6 +338,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } impl Trait for Test { diff --git a/srml/finality-tracker/src/lib.rs b/srml/finality-tracker/src/lib.rs index 46e4492fd16b1..ed7509b2244fb 100644 --- a/srml/finality-tracker/src/lib.rs +++ b/srml/finality-tracker/src/lib.rs @@ -294,6 +294,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = (); + type Error = Error; } type System = system::Module; From 3d2c850a7074b31499b0a4bc8d70f6f9874d5f5c Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 10 Jul 2019 15:35:59 +1200 Subject: [PATCH 34/40] Update core/sr-primitives/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bastian Köcher --- core/sr-primitives/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 2f08bdc479395..f5c4abd53be40 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -56,7 +56,7 @@ pub use generic::{DigestItem, Digest}; pub enum Error { /// Unknown error /// This exists only to make implementation easier. Should be avoid as much as possible. - Unknown(&'static str), + Other(&'static str), /// Indicating an invalid signature in extrinsic. BadSignature, /// Full block error. From 8330acfb97b0892d3c62d2ccedb1254942645d25 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 10 Jul 2019 15:36:22 +1200 Subject: [PATCH 35/40] Update core/sr-primitives/src/traits.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bastian Köcher --- core/sr-primitives/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 40b2e1ebb483e..bfe68d22088ed 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -131,7 +131,7 @@ pub struct IdentityLookup(PhantomData); impl StaticLookup for IdentityLookup { type Source = T; type Target = T; - type Error = &'static str; + type Error = (); fn lookup(x: T) -> result::Result { Ok(x) } fn unlookup(x: T) -> T { x } } From d41955a7facc0a78906e647c40d28d3006c71480 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 10 Jul 2019 15:40:55 +1200 Subject: [PATCH 36/40] Update srml/council/src/motions.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bastian Köcher --- srml/council/src/motions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index 91fdbd1565b66..fe9da639538f5 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -313,7 +313,7 @@ impl< } pub struct EnsureProportionAtLeast( - ::rstd::marker::PhantomData<(N, D, AccountId, Error)> + rstd::marker::PhantomData<(N, D, AccountId, Error)> ); impl< O: Into, O>> + From>, From 66187b3c00e489b79cdd6600a6aaf0145046c178 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 10 Jul 2019 15:41:10 +1200 Subject: [PATCH 37/40] Update srml/council/src/motions.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bastian Köcher --- srml/council/src/motions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index fe9da639538f5..a44377a10833e 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -274,7 +274,7 @@ impl< } pub struct EnsureMembers( - ::rstd::marker::PhantomData<(N, AccountId, Error)> + rstd::marker::PhantomData<(N, AccountId, Error)> ); impl< O: Into, O>> + From>, From 7489d3a7defe7878229709cbc89073c1ae06ae9e Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 10 Jul 2019 17:02:22 +1200 Subject: [PATCH 38/40] update based on review --- .../src/generic/unchecked_extrinsic.rs | 6 ++-- .../unchecked_mortal_compact_extrinsic.rs | 12 +++---- .../src/generic/unchecked_mortal_extrinsic.rs | 12 +++---- core/sr-primitives/src/lib.rs | 34 +++++++++---------- core/sr-primitives/src/testing.rs | 4 +-- core/sr-primitives/src/traits.rs | 2 +- core/test-runtime/src/lib.rs | 10 +++--- srml/balances/src/lib.rs | 2 +- srml/balances/src/tests.rs | 10 +++--- srml/contracts/src/gas.rs | 2 +- srml/executive/src/lib.rs | 14 ++++---- srml/staking/src/tests.rs | 4 +-- srml/support/src/error.rs | 15 ++++---- srml/system/src/lib.rs | 6 ++-- 14 files changed, 64 insertions(+), 69 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 5706e8d428b3f..d11561a415836 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -22,7 +22,7 @@ use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; -use crate::Error; +use crate::PrimitiveError; use super::CheckedExtrinsic; #[derive(PartialEq, Eq, Clone, Encode, Decode)] @@ -87,7 +87,7 @@ where Context: Lookup { type Checked = CheckedExtrinsic; - type Error = Error; + type Error = PrimitiveError; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -95,7 +95,7 @@ where let payload = (index, self.function); let signed = context.lookup(signed)?; if !crate::verify_encoded_lazy(&signature, &payload, &signed) { - return Err(Error::BadSignature) + return Err(PrimitiveError::BadSignature) } CheckedExtrinsic { signed: Some((signed, payload.0)), diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 80e07da1ea0b2..2cd52b386662a 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -24,7 +24,7 @@ use runtime_io::blake2_256; use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; -use crate::Error; +use crate::PrimitiveError; use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -81,7 +81,7 @@ where + BlockNumberToHash, { type Checked = CheckedExtrinsic; - type Error = Error; + type Error = PrimitiveError; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -98,7 +98,7 @@ where signature.verify(payload, &signed) } }) { - return Err(Error::BadSignature) + return Err(PrimitiveError::BadSignature) } CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), @@ -259,7 +259,7 @@ mod tests { fn badly_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); + assert_eq!(>::check(ux, &TestContext), Err(PrimitiveError::BadSignature)); } #[test] @@ -287,14 +287,14 @@ mod tests { fn too_late_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); + assert_eq!(>::check(ux, &TestContext), Err(PrimitiveError::BadSignature)); } #[test] fn too_early_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); + assert_eq!(>::check(ux, &TestContext), Err(PrimitiveError::BadSignature)); } #[test] diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index bdf96e15298d6..38f88b65a1cfe 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -26,7 +26,7 @@ use crate::traits::{ self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion }; -use crate::Error; +use crate::PrimitiveError; use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -82,7 +82,7 @@ where + BlockNumberToHash, { type Checked = CheckedExtrinsic; - type Error = Error; + type Error = PrimitiveError; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -100,7 +100,7 @@ where signature.verify(payload, &signed) } }) { - return Err(Error::BadSignature) + return Err(PrimitiveError::BadSignature) } CheckedExtrinsic { signed: Some((signed, raw_payload.0)), @@ -260,7 +260,7 @@ mod tests { fn badly_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); + assert_eq!(>::check(ux, &TestContext), Err(PrimitiveError::BadSignature)); } #[test] @@ -288,14 +288,14 @@ mod tests { fn too_late_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); + assert_eq!(>::check(ux, &TestContext), Err(PrimitiveError::BadSignature)); } #[test] fn too_early_mortal_signed_check_should_fail() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(Error::BadSignature)); + assert_eq!(>::check(ux, &TestContext), Err(PrimitiveError::BadSignature)); } #[test] diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index f5c4abd53be40..e197885ca82f9 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -52,8 +52,8 @@ pub mod transaction_validity; pub use generic::{DigestItem, Digest}; #[cfg_attr(test, derive(PartialEq, Debug))] -/// Error type -pub enum Error { +/// Primitive Error type +pub enum PrimitiveError { /// Unknown error /// This exists only to make implementation easier. Should be avoid as much as possible. Other(&'static str), @@ -71,19 +71,19 @@ pub enum Error { } // Exists for for backward compatibility purpose. -impl Into<&'static str> for Error { +impl Into<&'static str> for PrimitiveError { fn into(self) -> &'static str { match self { - Error::Unknown(val) => val, - Error::BadSignature => "bad signature in extrinsic", - Error::BlockFull => "block size limit is reached", + PrimitiveError::Other(val) => val, + PrimitiveError::BadSignature => "bad signature in extrinsic", + PrimitiveError::BlockFull => "block size limit is reached", } } } -impl From<&'static str> for Error { - fn from(val: &'static str) -> Error { - Error::Unknown(val) +impl From<&'static str> for PrimitiveError { + fn from(val: &'static str) -> PrimitiveError { + PrimitiveError::Other(val) } } @@ -565,14 +565,14 @@ impl From for AnySignature { } #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode)] - #[cfg_attr(feature = "std", derive(Debug, Serialize))] - /// Outcome of a valid extrinsic application. Capable of being sliced. - pub enum ApplyOutcome { - /// Successful application (extrinsic reported no issue). - Success, - /// Failed application (extrinsic was probably a no-op other than fees). - Fail(DispatchError), - } +#[cfg_attr(feature = "std", derive(Debug, Serialize))] +/// Outcome of a valid extrinsic application. Capable of being sliced. +pub enum ApplyOutcome { + /// Successful application (extrinsic reported no issue). + Success, + /// Failed application (extrinsic was probably a no-op other than fees). + Fail(DispatchError), +} #[derive(Eq, PartialEq, Clone, Copy, Decode)] #[cfg_attr(feature = "std", derive(Debug, Serialize))] diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 3fb276632097c..ed779156dd21d 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -22,7 +22,7 @@ use crate::codec::{Codec, Encode, Decode}; use crate::traits::{self, Checkable, Applyable, BlakeTwo256, OpaqueKeys}; use crate::generic; use crate::weights::{Weighable, Weight}; -use crate::Error; +use crate::PrimitiveError; pub use substrate_primitives::H256; use substrate_primitives::U256; use substrate_primitives::ed25519::{Public as AuthorityId}; @@ -201,7 +201,7 @@ impl Debug for TestXt { impl Checkable for TestXt { type Checked = Self; - type Error = Error; + type Error = PrimitiveError; fn check(self, _: &Context) -> Result { Ok(self) } } impl traits::Extrinsic for TestXt { diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index bfe68d22088ed..40b2e1ebb483e 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -131,7 +131,7 @@ pub struct IdentityLookup(PhantomData); impl StaticLookup for IdentityLookup { type Source = T; type Target = T; - type Error = (); + type Error = &'static str; fn lookup(x: T) -> result::Result { Ok(x) } fn unlookup(x: T) -> T { x } } diff --git a/core/test-runtime/src/lib.rs b/core/test-runtime/src/lib.rs index a47be8fb2e715..edd192407eb5f 100644 --- a/core/test-runtime/src/lib.rs +++ b/core/test-runtime/src/lib.rs @@ -37,7 +37,7 @@ use runtime_primitives::{ ApplyResult, create_runtime_str, transaction_validity::TransactionValidity, - Error, + PrimitiveError, traits::{ BlindCheckable, BlakeTwo256, Block as BlockT, Extrinsic as ExtrinsicT, GetNodeBlockType, GetRuntimeBlockType, Verify @@ -123,19 +123,19 @@ impl serde::Serialize for Extrinsic { impl BlindCheckable for Extrinsic { type Checked = Self; - type Error = Error; + type Error = PrimitiveError; - fn check(self) -> Result { + fn check(self) -> Result { match self { Extrinsic::AuthoritiesChange(new_auth) => Ok(Extrinsic::AuthoritiesChange(new_auth)), Extrinsic::Transfer(transfer, signature) => { if runtime_primitives::verify_encoded_lazy(&signature, &transfer, &transfer.from) { Ok(Extrinsic::Transfer(transfer, signature)) } else { - Err(Error::BadSignature) + Err(PrimitiveError::BadSignature) } }, - Extrinsic::IncludeData(_) => Err(Error::BadSignature), + Extrinsic::IncludeData(_) => Err(PrimitiveError::BadSignature), Extrinsic::StorageChange(key, value) => Ok(Extrinsic::StorageChange(key, value)), } } diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index e479bbfecb69b..99273f0618d5a 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -287,7 +287,7 @@ decl_error! { impl From for &'static str { fn from(err: Error) -> &'static str { match err { - Error::Unknown(msg) => msg, + Error::Other(msg) => msg, } } } diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 39fdc4ae26198..8989a4bab99fb 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -208,7 +208,7 @@ fn default_indexing_on_new_accounts_should_not_work2() { // account 1 has 256 * 10 = 2560, account 5 is not exist, ext_deposit is 10, value is 9, not satisfies for ext_deposit assert_noop!( Balances::transfer(Some(1).into(), 5, 9), - mock::Error::system(system::Error::Unknown("value too low to create account")) + mock::Error::system(system::Error::Other("value too low to create account")) ); assert_eq!(Balances::is_dead_account(&5), true); // account 5 should not exist assert_eq!(Balances::free_balance(&1), 100); @@ -349,7 +349,7 @@ fn balance_transfer_when_reserved_should_not_work() { assert_ok!(Balances::reserve(&1, 69)); assert_noop!( Balances::transfer(Some(1).into(), 2, 69), - mock::Error::system(system::Error::Unknown("balance too low to send value")) + mock::Error::system(system::Error::Other("balance too low to send value")) ); }); } @@ -478,7 +478,7 @@ fn transferring_too_high_value_should_not_panic() { assert_err!( Balances::transfer(Some(1).into(), 2, u64::max_value()), - mock::Error::system(system::Error::Unknown("destination balance too high to receive value")) + mock::Error::system(system::Error::Other("destination balance too high to receive value")) ); assert_eq!(Balances::free_balance(&1), u64::max_value()); @@ -556,7 +556,7 @@ fn transfer_overflow_isnt_exploitable() { assert_err!( Balances::transfer(Some(1).into(), 5, evil_value), - mock::Error::system(system::Error::Unknown("got overflow after adding a fee to value")) + mock::Error::system(system::Error::Other("got overflow after adding a fee to value")) ); } ); @@ -620,7 +620,7 @@ fn unvested_balance_should_not_transfer() { assert_eq!(Balances::vesting_balance(&1), 90); // Account 1 has only 10 units vested at block 1 assert_noop!( Balances::transfer(Some(1).into(), 2, 11), - mock::Error::system(system::Error::Unknown("vesting balance too high to send value")) + mock::Error::system(system::Error::Other("vesting balance too high to send value")) ); // Account 1 cannot send more than vested amount } ); diff --git a/srml/contracts/src/gas.rs b/srml/contracts/src/gas.rs index da13c0fea2f53..cf0aa2ebd7ebf 100644 --- a/srml/contracts/src/gas.rs +++ b/srml/contracts/src/gas.rs @@ -16,7 +16,7 @@ use crate::{GasSpent, Module, Trait, BalanceOf, NegativeImbalanceOf}; use rstd::convert::TryFrom; -use runtime_primitives::Error as PrimitiveError; +use runtime_primitives::PrimitiveError; use runtime_primitives::traits::{CheckedMul, Zero, SaturatedConversion, SimpleArithmetic, UniqueSaturatedInto}; use srml_support::StorageValue; use srml_support::traits::{Currency, ExistenceRequirement, Get, Imbalance, OnUnbalanced, WithdrawReason}; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 8aafeac448fe9..90ea8448a768b 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -78,7 +78,7 @@ use rstd::prelude::*; use rstd::marker::PhantomData; use rstd::convert::TryInto; use primitives::{ - generic::Digest, ApplyResult, ApplyOutcome, ApplyError, DispatchError, Error as PrimitiveError, + generic::Digest, ApplyResult, ApplyOutcome, ApplyError, DispatchError, PrimitiveError, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, @@ -264,12 +264,10 @@ where if let (Some(sender), Some(index)) = (xt.sender(), xt.index()) { // check index let expected_index = >::account_nonce(sender); - if index != &expected_index { - return if index < &expected_index { - Err(ApplyError::Stale) - } else { - Err(ApplyError::Future) - } + if index < &expected_index { + return Err(ApplyError::Stale) + } else if index > &expected_index { + return Err(ApplyError::Future) } // pay any fees // TODO: propagate why can't pay #2952 @@ -339,7 +337,7 @@ where Err(err) => return match err.into() { // An unknown account index implies that the transaction may yet become valid. // TODO: avoid hardcoded error string here #2953 - PrimitiveError::Unknown("invalid account index") => + PrimitiveError::Other("invalid account index") => TransactionValidity::Unknown(INVALID_INDEX), // Technically a bad signature could also imply an out-of-date account index, but // that's more of an edge case. diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index f70813d7bc661..b3863a3a2ca07 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -887,7 +887,7 @@ fn cannot_transfer_staked_balance() { // Confirm account 11 cannot transfer as a result assert_noop!( Balances::transfer(Origin::signed(11), 20, 1), - mock::Error::system(system::Error::Unknown("account liquidity restrictions prevent withdrawal")) + mock::Error::system(system::Error::Other("account liquidity restrictions prevent withdrawal")) ); // Give account 11 extra free balance @@ -916,7 +916,7 @@ fn cannot_transfer_staked_balance_2() { // Confirm account 21 can transfer at most 1000 assert_noop!( Balances::transfer(Origin::signed(21), 20, 1001), - mock::Error::system(system::Error::Unknown("account liquidity restrictions prevent withdrawal")) + mock::Error::system(system::Error::Other("account liquidity restrictions prevent withdrawal")) ); assert_ok!(Balances::transfer(Origin::signed(21), 20, 1000)); }); diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index fa990880bb8cc..d39e43271768f 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -78,7 +78,7 @@ macro_rules! impl_outer_error { impl From<&'static str> for $name { fn from(err: &'static str) -> Self { - $name::system($system::Error::Unknown(err)) + $name::system($system::Error::Other(err)) } } @@ -109,7 +109,7 @@ macro_rules! impl_outer_error { fn into(self) -> $crate::runtime_primitives::DispatchError { match self { $name::system(ref err) => match err { - $system::Error::Unknown(msg) => + $system::Error::Other(msg) => $crate::runtime_primitives::DispatchError { module: 0, error: 0, @@ -123,7 +123,7 @@ macro_rules! impl_outer_error { }, $( $name::$module(ref err) => match err { - $module::Error::Unknown(msg) => + $module::Error::Other(msg) => $crate::runtime_primitives::DispatchError { module: $crate::codec::Encode::using_encoded(&self, |s| s[0]), error: 0, @@ -178,19 +178,16 @@ macro_rules! decl_error { $(#[$attr])* #[allow(non_camel_case_types)] pub enum Error { - Unknown(&'static str), + Other(&'static str), $( $errors )* } - impl From for () { - fn from(_: Error) -> () { () } - } impl From<&Error> for u8 { fn from(err: &Error) -> u8 { match err { - Error::Unknown(_) => 0, + Error::Other(_) => 0, _ => $crate::codec::Encode::using_encoded(err, |s| s[0]), } } @@ -198,7 +195,7 @@ macro_rules! decl_error { impl From<&'static str> for Error { fn from(val: &'static str) -> Error { - Error::Unknown(val) + Error::Other(val) } } } diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 005c3975ed53f..b4668d2cdfeb6 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -77,7 +77,7 @@ use rstd::prelude::*; #[cfg(any(feature = "std", test))] use rstd::map; use primitives::{ - generic, Error as PrimitiveError, DispatchError, + generic, PrimitiveError, DispatchError, traits::{ self, CheckEqual, SimpleArithmetic, SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, CurrentHeight, BlockNumberToHash, @@ -286,7 +286,7 @@ decl_error! { impl From for Error { fn from(err: PrimitiveError) -> Error { match err { - PrimitiveError::Unknown(err) => Error::Unknown(err), + PrimitiveError::Other(err) => Error::Other(err), PrimitiveError::BadSignature => Error::BadSignature, PrimitiveError::BlockFull => Error::BlockFull, } @@ -297,7 +297,7 @@ impl From for Error { impl From for &'static str { fn from(err: Error) -> &'static str { match err { - Error::Unknown(val) => val, + Error::Other(val) => val, Error::BadSignature => "bad signature in extrinsic", Error::BlockFull => "block size limit is reached", Error::RequireSignedOrigin => "bad origin: expected to be a signed origin", From 6e5c3ce89c5ff9d11b6fbf687937f7bf5d8fbb13 Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 10 Jul 2019 18:16:50 +1200 Subject: [PATCH 39/40] More concrete macro matching --- srml/support/src/error.rs | 11 +++++++---- srml/support/test/tests/system.rs | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs index d39e43271768f..e0041dd41f2ef 100644 --- a/srml/support/src/error.rs +++ b/srml/support/src/error.rs @@ -168,8 +168,10 @@ macro_rules! decl_error { $(#[$attr:meta])* pub enum Error { $( - $errors:tt - )* + $(#[$variant_attr:meta])* + $name:ident + ),* + $(,)? } ) => { // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. @@ -180,8 +182,9 @@ macro_rules! decl_error { pub enum Error { Other(&'static str), $( - $errors - )* + $(#[$variant_attr])* + $name + ),* } impl From<&Error> for u8 { diff --git a/srml/support/test/tests/system.rs b/srml/support/test/tests/system.rs index 5f04021106a59..9ae9c4a4bf060 100644 --- a/srml/support/test/tests/system.rs +++ b/srml/support/test/tests/system.rs @@ -27,6 +27,8 @@ srml_support::decl_event!( srml_support::decl_error! { pub enum Error { + TestError, + AnotherError } } From efe953a3f105da29a7bf9f5c587e22b65fa998df Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Thu, 18 Jul 2019 14:16:16 +1200 Subject: [PATCH 40/40] fix test build issue --- srml/balances/src/tests.rs | 2 +- srml/collective/src/lib.rs | 1 + srml/elections/src/lib.rs | 4 +++- srml/executive/src/lib.rs | 6 +++++- srml/generic-asset/src/lib.rs | 1 + srml/generic-asset/src/mock.rs | 7 ++++++- 6 files changed, 17 insertions(+), 4 deletions(-) diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 9dacdfd595140..cf2b5831055ab 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -652,7 +652,7 @@ fn unvested_balance_should_not_transfer() { assert_eq!(Balances::vesting_balance(&1), 45); assert_noop!( Balances::transfer(Some(1).into(), 2, 56), - mock::Error::system(system::Error::Other("vesting balance too high to send value") + mock::Error::system(system::Error::Other("vesting balance too high to send value")) ); // Account 1 cannot send more than vested amount } ); diff --git a/srml/collective/src/lib.rs b/srml/collective/src/lib.rs index 60652f6af07f6..914bdbb253062 100644 --- a/srml/collective/src/lib.rs +++ b/srml/collective/src/lib.rs @@ -414,6 +414,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = Event; + type Error = Error; type BlockHashCount = BlockHashCount; } impl Trait for Test { diff --git a/srml/elections/src/lib.rs b/srml/elections/src/lib.rs index 80324ecefbdaf..92ccc3c3374b8 100644 --- a/srml/elections/src/lib.rs +++ b/srml/elections/src/lib.rs @@ -1119,6 +1119,7 @@ mod tests { type Lookup = IdentityLookup; type Header = Header; type Event = Event; + type Error = Error; type BlockHashCount = BlockHashCount; } parameter_types! { @@ -1133,6 +1134,7 @@ mod tests { type OnNewAccount = (); type OnFreeBalanceZero = (); type Event = Event; + type Error = Error; type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); @@ -1220,7 +1222,7 @@ mod tests { UncheckedExtrinsic = UncheckedExtrinsic { System: system::{Module, Call, Event}, - Balances: balances::{Module, Call, Event, Config}, + Balances: balances::{Module, Call, Event, Config, Error}, Elections: elections::{Module, Call, Event, Config}, } ); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index d98e1e1d378a8..970d65b6dfdd5 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -682,7 +682,11 @@ mod tests { )); if lock == WithdrawReasons::except(WithdrawReason::TransactionPayment) { - assert_eq!(Executive::apply_extrinsic(xt).unwrap(), ApplyOutcome::Fail); + assert_eq!(Executive::apply_extrinsic(xt).unwrap(), ApplyOutcome::Fail(DispatchError { + module: 0, + error: 0, + message: Some("account liquidity restrictions prevent withdrawal") + })); // but tx fee has been deducted. the transaction failed on transfer, not on fee. assert_eq!(>::total_balance(&1), 111 - 10); } else { diff --git a/srml/generic-asset/src/lib.rs b/srml/generic-asset/src/lib.rs index 60370600a69a6..6cab7c4935033 100644 --- a/srml/generic-asset/src/lib.rs +++ b/srml/generic-asset/src/lib.rs @@ -1056,6 +1056,7 @@ impl system::Trait for ElevatedTrait { type Lookup = T::Lookup; type Header = T::Header; type Event = (); + type Error = &'static str; type BlockHashCount = T::BlockHashCount; } impl Trait for ElevatedTrait { diff --git a/srml/generic-asset/src/mock.rs b/srml/generic-asset/src/mock.rs index 02e18fc335839..70d6273fddfa8 100644 --- a/srml/generic-asset/src/mock.rs +++ b/srml/generic-asset/src/mock.rs @@ -25,7 +25,7 @@ use primitives::{ traits::{BlakeTwo256, IdentityLookup}, }; use substrate_primitives::{Blake2Hasher, H256}; -use support::{parameter_types, impl_outer_event, impl_outer_origin}; +use support::{parameter_types, impl_outer_event, impl_outer_origin, impl_outer_error}; use super::*; @@ -33,6 +33,10 @@ impl_outer_origin! { pub enum Origin for Test {} } +impl_outer_error! { + pub enum Error for Test {} +} + // For testing the module, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the // configuration traits of modules we want to use. @@ -51,6 +55,7 @@ impl system::Trait for Test { type Lookup = IdentityLookup; type Header = Header; type Event = TestEvent; + type Error = Error; type BlockHashCount = BlockHashCount; }