diff --git a/core/basic-authorship/src/basic_authorship.rs b/core/basic-authorship/src/basic_authorship.rs index 2030f2be5a2f9..ffc4d766c9bd3 100644 --- a/core/basic-authorship/src/basic_authorship.rs +++ b/core/basic-authorship/src/basic_authorship.rs @@ -30,9 +30,10 @@ use inherents::InherentData; use log::{error, info, debug, trace}; use primitives::{H256, Blake2Hasher, ExecutionContext}; use sr_primitives::{ - traits::{Block as BlockT, Hash as HashT, Header as HeaderT, ProvideRuntimeApi, DigestFor, BlakeTwo256}, + traits::{ + Block as BlockT, Hash as HashT, Header as HeaderT, ProvideRuntimeApi, DigestFor, BlakeTwo256 + }, generic::BlockId, - ApplyError, }; use transaction_pool::txpool::{self, Pool as TransactionPool}; use substrate_telemetry::{telemetry, CONSENSUS_INFO}; @@ -170,7 +171,7 @@ impl Proposer, A> wh Ok(()) => { debug!("[{:?}] Pushed to the block.", pending.hash); } - Err(error::Error::ApplyExtrinsicFailed(ApplyError::FullBlock)) => { + Err(error::Error::ApplyExtrinsicFailed(e)) if e.exhausted_resources() => { if is_first { debug!("[{:?}] Invalid transaction: FullBlock on empty block", pending.hash); unqueue_invalid.push(pending.hash.clone()); @@ -178,7 +179,7 @@ impl Proposer, A> wh skipped += 1; debug!( "Block seems full, but will try {} more transactions before quitting.", - MAX_SKIPPED_TRANSACTIONS - skipped + MAX_SKIPPED_TRANSACTIONS - skipped, ); } else { debug!("Block is full, proceed with proposing."); diff --git a/core/client/src/block_builder/block_builder.rs b/core/client/src/block_builder/block_builder.rs index 22aac577e3342..b6fae5068a284 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 codec::Encode; -use sr_primitives::ApplyOutcome; use sr_primitives::generic::BlockId; use sr_primitives::traits::{ Header as HeaderT, Hash, Block as BlockT, One, HashFor, ProvideRuntimeApi, ApiRef, DigestFor, @@ -104,7 +103,7 @@ where ExecutionContext::BlockConstruction, xt.clone() )? { - Ok(ApplyOutcome::Success) | Ok(ApplyOutcome::Fail) => { + Ok(_) => { extrinsics.push(xt); Ok(()) } diff --git a/core/rpc/api/src/author/error.rs b/core/rpc/api/src/author/error.rs index 0a71de5cb9bea..727b58bd210f4 100644 --- a/core/rpc/api/src/author/error.rs +++ b/core/rpc/api/src/author/error.rs @@ -101,15 +101,15 @@ impl From for rpc::Error { message: format!("Verification Error: {}", e).into(), data: Some(format!("{:?}", e).into()), }, - Error::Pool(PoolError::InvalidTransaction(code)) => rpc::Error { + Error::Pool(PoolError::InvalidTransaction(e)) => rpc::Error { code: rpc::ErrorCode::ServerError(POOL_INVALID_TX), message: "Invalid Transaction".into(), - data: Some(code.into()), + data: serde_json::to_value(e).ok(), }, - Error::Pool(PoolError::UnknownTransactionValidity(code)) => rpc::Error { + Error::Pool(PoolError::UnknownTransaction(e)) => rpc::Error { code: rpc::ErrorCode::ServerError(POOL_UNKNOWN_VALIDITY), message: "Unknown Transaction Validity".into(), - data: Some(code.into()), + data: serde_json::to_value(e).ok(), }, Error::Pool(PoolError::TemporarilyBanned) => rpc::Error { code: rpc::ErrorCode::ServerError(POOL_TEMPORARILY_BANNED), @@ -133,7 +133,7 @@ impl From for rpc::Error { }, Error::Pool(PoolError::ImmediatelyDropped) => rpc::Error { code: rpc::ErrorCode::ServerError(POOL_IMMEDIATELY_DROPPED), - message: "Immediately Dropped" .into(), + message: "Immediately Dropped".into(), data: Some("The transaction couldn't enter the pool because of the limit".into()), }, Error::UnsupportedKeyType => rpc::Error { diff --git a/core/sr-io/src/lib.rs b/core/sr-io/src/lib.rs index b79c13fb12934..de5c48f5fedf9 100644 --- a/core/sr-io/src/lib.rs +++ b/core/sr-io/src/lib.rs @@ -55,7 +55,13 @@ pub mod offchain; /// Trait for things which can be printed. pub trait Printable { /// Print the object. - fn print(self); + fn print(&self); +} + +impl Printable for u8 { + fn print(&self) { + u64::from(*self).print() + } } /// Converts a public trait definition into a private trait and set of public functions diff --git a/core/sr-io/with_std.rs b/core/sr-io/with_std.rs index 8e6a160cf203a..acbeb8ce0e9bd 100644 --- a/core/sr-io/with_std.rs +++ b/core/sr-io/with_std.rs @@ -479,19 +479,19 @@ pub fn with_storage R>( } impl<'a> Printable for &'a [u8] { - fn print(self) { - println!("Runtime: {}", HexDisplay::from(&self)); + fn print(&self) { + println!("Runtime: {}", HexDisplay::from(self)); } } impl<'a> Printable for &'a str { - fn print(self) { + fn print(&self) { println!("Runtime: {}", self); } } impl Printable for u64 { - fn print(self) { + fn print(&self) { println!("Runtime: {}", self); } } diff --git a/core/sr-io/without_std.rs b/core/sr-io/without_std.rs index c5f2ac483fa81..58aff2444e368 100644 --- a/core/sr-io/without_std.rs +++ b/core/sr-io/without_std.rs @@ -1220,7 +1220,7 @@ unsafe fn from_raw_parts(ptr: *mut u8, len: u32) -> Option> { impl Api for () {} impl<'a> Printable for &'a [u8] { - fn print(self) { + fn print(&self) { unsafe { ext_print_hex.get()(self.as_ptr(), self.len() as u32); } @@ -1228,7 +1228,7 @@ impl<'a> Printable for &'a [u8] { } impl<'a> Printable for &'a str { - fn print(self) { + fn print(&self) { unsafe { ext_print_utf8.get()(self.as_ptr() as *const u8, self.len() as u32); } @@ -1236,7 +1236,8 @@ impl<'a> Printable for &'a str { } impl Printable for u64 { - fn print(self) { - unsafe { ext_print_num.get()(self); } + fn print(&self) { + unsafe { ext_print_num.get()(*self); } } } + diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 08d7b10386721..e8d41325c4324 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -17,10 +17,8 @@ //! Generic implementation of an extrinsic that has passed the verification //! stage. -use rstd::result::Result; use crate::traits::{ - self, Member, MaybeDisplay, SignedExtension, DispatchError, Dispatchable, DispatchResult, - ValidateUnsigned + self, Member, MaybeDisplay, SignedExtension, Dispatchable, ValidateUnsigned, }; use crate::weights::{GetDispatchInfo, DispatchInfo}; use crate::transaction_validity::TransactionValidity; @@ -55,28 +53,24 @@ where self.signed.as_ref().map(|x| &x.0) } - fn validate>(&self, + fn validate>( + &self, info: DispatchInfo, len: usize, ) -> TransactionValidity { if let Some((ref id, ref extra)) = self.signed { - Extra::validate(extra, id, &self.function, info, len).into() + Extra::validate(extra, id, &self.function, info, len) } else { - match Extra::validate_unsigned(&self.function, info, len) { - Ok(extra) => match U::validate_unsigned(&self.function) { - TransactionValidity::Valid(v) => - TransactionValidity::Valid(v.combine_with(extra)), - x => x, - }, - x => x.into(), - } + let valid = Extra::validate_unsigned(&self.function, info, len)?; + Ok(valid.combine_with(U::validate_unsigned(&self.function)?)) } } - fn dispatch(self, + fn apply( + self, info: DispatchInfo, len: usize, - ) -> Result { + ) -> crate::ApplyResult { let (maybe_who, pre) = if let Some((id, extra)) = self.signed { let pre = Extra::pre_dispatch(extra, &id, &self.function, info, len)?; (Some(id), pre) @@ -86,7 +80,7 @@ where }; let res = self.function.dispatch(Origin::from(maybe_who)); Extra::post_dispatch(pre, info, len); - Ok(res) + Ok(res.map_err(Into::into)) } } diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index aebf80edc63ec..48614946d6d13 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -21,9 +21,11 @@ use std::fmt; use rstd::prelude::*; use runtime_io::blake2_256; -use crate::codec::{Decode, Encode, Input, Error}; -use crate::traits::{self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic}; -use super::CheckedExtrinsic; +use codec::{Decode, Encode, Input, Error}; +use crate::{ + traits::{self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic}, + generic::CheckedExtrinsic, transaction_validity::{TransactionValidityError, InvalidTransaction}, +}; const TRANSACTION_VERSION: u8 = 3; @@ -101,11 +103,11 @@ where Signature: Member + traits::Verify, Extra: SignedExtension, AccountId: Member + MaybeDisplay, - Lookup: traits::Lookup + Lookup: traits::Lookup, { type Checked = CheckedExtrinsic; - fn check(self, lookup: &Lookup) -> Result { + fn check(self, lookup: &Lookup) -> Result { Ok(match self.signature { Some((signed, signature, extra)) => { let signed = lookup.lookup(signed)?; @@ -113,7 +115,7 @@ where if !raw_payload.using_encoded(|payload| { signature.verify(payload, &signed) }) { - return Err(crate::BAD_SIGNATURE) + return Err(InvalidTransaction::BadProof.into()) } let (function, extra, _) = raw_payload.deconstruct(); @@ -136,9 +138,9 @@ where /// is going to be different than the `SignaturePayload` - so the thing the extrinsic /// actually contains. pub struct SignedPayload(( - Call, - Extra, - Extra::AdditionalSigned, + Call, + Extra, + Extra::AdditionalSigned, )); impl SignedPayload where @@ -148,7 +150,7 @@ impl SignedPayload where /// Create new `SignedPayload`. /// /// This function may fail if `additional_signed` of `Extra` is not available. - pub fn new(call: Call, extra: Extra) -> Result { + pub fn new(call: Call, extra: Extra) -> Result { let additional_signed = extra.additional_signed()?; let raw_payload = (call, extra, additional_signed); Ok(Self(raw_payload)) @@ -256,7 +258,12 @@ where Extra: SignedExtension, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "UncheckedExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.0, &x.2)), self.function) + write!( + f, + "UncheckedExtrinsic({:?}, {:?})", + self.signature.as_ref().map(|x| (&x.0, &x.2)), + self.function, + ) } } @@ -265,15 +272,10 @@ mod tests { use super::*; use runtime_io::blake2_256; use crate::codec::{Encode, Decode}; - use crate::traits::{SignedExtension, Lookup}; + use crate::traits::{SignedExtension, IdentityLookup}; use serde::{Serialize, Deserialize}; - struct TestContext; - impl Lookup for TestContext { - type Source = u64; - type Target = u64; - fn lookup(&self, s: u64) -> Result { Ok(s) } - } + type TestContext = IdentityLookup; #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)] struct TestSig(u64, Vec); @@ -298,7 +300,7 @@ mod tests { type AdditionalSigned = (); type Pre = (); - fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } } type Ex = UncheckedExtrinsic; @@ -340,7 +342,7 @@ mod tests { fn unsigned_check_should_work() { let ux = Ex::new_unsigned(vec![0u8; 0]); assert!(!ux.is_signed().unwrap_or(false)); - assert!(>::check(ux, &TestContext).is_ok()); + assert!(>::check(ux, &Default::default()).is_ok()); } #[test] @@ -349,10 +351,13 @@ mod tests { vec![0u8; 0], TEST_ACCOUNT, TestSig(TEST_ACCOUNT, vec![0u8; 0]), - TestExtra + TestExtra, ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + assert_eq!( + >::check(ux, &Default::default()), + Err(InvalidTransaction::BadProof.into()), + ); } #[test] @@ -361,12 +366,12 @@ mod tests { vec![0u8; 0], TEST_ACCOUNT, TestSig(TEST_ACCOUNT, (vec![0u8; 0], TestExtra).encode()), - TestExtra + TestExtra, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }) + >::check(ux, &Default::default()), + Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }), ); } diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 454f1cdfb8106..bcd54c660b8a7 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -58,19 +58,6 @@ pub use generic::{DigestItem, Digest}; pub use primitives::crypto::{key_types, KeyTypeId, CryptoType}; pub use app_crypto::AppKey; -/// 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"; - /// Justification type. pub type Justification = Vec; @@ -636,53 +623,111 @@ impl From for AnySignature { } } -#[derive(Eq, PartialEq, Clone, Copy, Decode)] +#[derive(Eq, PartialEq, Clone, Copy, Decode, Encode)] #[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, +/// Reason why an extrinsic couldn't be applied (i.e. invalid extrinsic). +pub enum ApplyError { + /// General error to do with the permissions of the sender. + NoPermission, + + /// General error to do with the state of the system in general. + BadState, + + /// Any error to do with the transaction validity. + Validity(transaction_validity::TransactionValidityError), } -impl codec::Encode for ApplyOutcome { - fn using_encoded R>(&self, f: F) -> R { - f(&[*self as u8]) +impl ApplyError { + /// Returns if the reason for the error was block resource exhaustion. + pub fn exhausted_resources(&self) -> bool { + match self { + Self::Validity(e) => e.exhausted_resources(), + _ => false, + } } } -impl codec::EncodeLike for ApplyOutcome {} - -#[derive(Eq, PartialEq, Clone, Copy, Decode)] -#[cfg_attr(feature = "std", derive(Debug, Serialize))] -#[repr(u8)] -/// Reason why an extrinsic couldn't be applied (i.e. invalid extrinsic). -pub enum ApplyError { - /// Bad signature. - BadSignature = 0, - /// Nonce too low. - Stale = 1, - /// Nonce too high. - Future = 2, - /// Sending account had too low a balance. - CantPay = 3, - /// Block is full, no more extrinsics can be applied. - FullBlock = 255, +impl From for &'static str { + fn from(err: ApplyError) -> &'static str { + match err { + ApplyError::NoPermission => "Transaction does not have required permissions", + ApplyError::BadState => "System state currently prevents this transaction", + ApplyError::Validity(v) => v.into(), + } + } } -impl codec::Encode for ApplyError { - fn using_encoded R>(&self, f: F) -> R { - f(&[*self as u8]) +impl From for ApplyError { + fn from(err: transaction_validity::TransactionValidityError) -> Self { + ApplyError::Validity(err) } } -impl codec::EncodeLike for ApplyError {} +/// The outcome of applying a transaction. +pub type ApplyOutcome = Result<(), DispatchError>; + +impl From for ApplyOutcome { + fn from(err: DispatchError) -> Self { + Err(err) + } +} /// Result from attempt to apply an extrinsic. pub type ApplyResult = Result; +#[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: Option, + /// Module specific error value + pub error: u8, + /// Optional error message. + #[codec(skip)] + pub message: Option<&'static str>, +} + +impl DispatchError { + /// Create a new instance of `DispatchError`. + pub fn new(module: Option, error: u8, message: Option<&'static str>) -> Self { + Self { + module, + error, + message, + } + } +} + +impl runtime_io::Printable for DispatchError { + fn print(&self) { + "DispatchError".print(); + if let Some(module) = self.module { + module.print(); + } + self.error.print(); + if let Some(msg) = self.message { + msg.print(); + } + } +} + +impl traits::ModuleDispatchError for &'static str { + fn as_u8(&self) -> u8 { + 0 + } + + fn as_str(&self) -> &'static str { + self + } +} + +impl From<&'static str> for DispatchError { + fn from(err: &'static str) -> DispatchError { + DispatchError::new(None, 0, Some(err)) + } +} + /// 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. pub fn verify_encoded_lazy(sig: &V, item: &T, signer: &V::Signer) -> bool { @@ -852,6 +897,7 @@ impl traits::Extrinsic for OpaqueExtrinsic { #[cfg(test)] mod tests { + use super::DispatchError; use crate::codec::{Encode, Decode}; use super::{Perbill, Permill}; @@ -967,6 +1013,26 @@ mod tests { ); } + #[test] + fn dispatch_error_encoding() { + let error = DispatchError { + module: Some(1), + error: 2, + message: Some("error message"), + }; + let encoded = error.encode(); + let decoded = DispatchError::decode(&mut &encoded[..]).unwrap(); + assert_eq!(encoded, vec![1, 1, 2]); + assert_eq!( + decoded, + DispatchError { + module: Some(1), + error: 2, + message: None, + }, + ); + } + #[test] fn per_bill_square() { const FIXTURES: &[(u32, u32)] = &[ diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index ed5ac7fa45675..9c827d8a704bd 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -20,14 +20,14 @@ use serde::{Serialize, Serializer, Deserialize, de::Error as DeError, Deserializ use std::{fmt::Debug, ops::Deref, fmt}; use crate::codec::{Codec, Encode, Decode}; use crate::traits::{ - self, Checkable, Applyable, BlakeTwo256, OpaqueKeys, DispatchError, DispatchResult, - ValidateUnsigned, SignedExtension, Dispatchable, + self, Checkable, Applyable, BlakeTwo256, OpaqueKeys, ValidateUnsigned, + SignedExtension, Dispatchable, }; -use crate::{generic, KeyTypeId}; +use crate::{generic, KeyTypeId, ApplyResult}; use crate::weights::{GetDispatchInfo, DispatchInfo}; pub use primitives::H256; use primitives::{crypto::{CryptoType, Dummy, key_types, Public}, U256}; -use crate::transaction_validity::TransactionValidity; +use crate::transaction_validity::{TransactionValidity, TransactionValidityError}; /// Authority Id #[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize)] @@ -271,7 +271,7 @@ impl Debug for TestXt { impl Checkable for TestXt { type Checked = Self; - fn check(self, _: &Context) -> Result { Ok(self) } + fn check(self, _: &Context) -> Result { Ok(self) } } impl traits::Extrinsic for TestXt { type Call = Call; @@ -289,7 +289,7 @@ impl traits::Extrinsic for TestXt impl Applyable for TestXt where Call: 'static + Sized + Send + Sync + Clone + Eq + Codec + Debug + Dispatchable, Extra: SignedExtension, - Origin: From> + Origin: From>, { type AccountId = u64; type Call = Call; @@ -297,19 +297,21 @@ impl Applyable for TestXt where fn sender(&self) -> Option<&Self::AccountId> { self.0.as_ref().map(|x| &x.0) } /// Checks to see if this is a valid *transaction*. It returns information on it if so. - fn validate>(&self, + fn validate>( + &self, _info: DispatchInfo, _len: usize, ) -> TransactionValidity { - TransactionValidity::Valid(Default::default()) + Ok(Default::default()) } /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. - fn dispatch(self, + fn apply( + self, info: DispatchInfo, len: usize, - ) -> Result { + ) -> ApplyResult { let maybe_who = if let Some((who, extra)) = self.0 { Extra::pre_dispatch(extra, &who, &self.1, info, len)?; Some(who) @@ -317,7 +319,8 @@ impl Applyable for TestXt where Extra::pre_dispatch_unsigned(&self.1, info, len)?; None }; - Ok(self.1.dispatch(maybe_who.into())) + + Ok(self.1.dispatch(maybe_who.into()).map_err(Into::into)) } } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index fe3d67268f117..0ac40e9942412 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -23,7 +23,9 @@ use runtime_io; #[cfg(feature = "std")] use serde::{Serialize, Deserialize, de::DeserializeOwned}; use primitives::{self, Hasher, Blake2Hasher}; use crate::codec::{Codec, Encode, Decode, HasCompact}; -use crate::transaction_validity::{ValidTransaction, TransactionValidity}; +use crate::transaction_validity::{ + ValidTransaction, TransactionValidity, TransactionValidityError, UnknownTransaction, +}; use crate::generic::{Digest, DigestItem}; use crate::weights::DispatchInfo; pub use integer_sqrt::IntegerSquareRoot; @@ -93,18 +95,44 @@ impl< } } +/// An error type that indicates that the origin is invalid. +#[derive(Encode, Decode)] +pub struct InvalidOrigin; + +impl From for &'static str { + fn from(_: InvalidOrigin) -> &'static str { + "Invalid origin" + } +} + /// Some sort of check on the origin is performed by this object. pub trait EnsureOrigin { /// A return type. type Success; /// Perform the origin check. - fn ensure_origin(o: OuterOrigin) -> result::Result { - Self::try_origin(o).map_err(|_| "Invalid origin") + fn ensure_origin(o: OuterOrigin) -> result::Result { + Self::try_origin(o).map_err(|_| InvalidOrigin) } /// Perform the origin check. fn try_origin(o: OuterOrigin) -> result::Result; } +/// An error that indicates that a lookup failed. +#[derive(Encode, Decode)] +pub struct LookupError; + +impl From for &'static str { + fn from(_: LookupError) -> &'static str { + "Can not lookup" + } +} + +impl From for TransactionValidityError { + fn from(_: LookupError) -> Self { + UnknownTransaction::CannotLookup.into() + } +} + /// Means of changing one type into another in a manner dependent on the source type. pub trait Lookup { /// Type to lookup from. @@ -112,7 +140,7 @@ pub trait Lookup { /// Type to lookup into. type Target; /// Attempt a lookup. - fn lookup(&self, s: Self::Source) -> result::Result; + fn lookup(&self, s: Self::Source) -> Result; } /// Means of changing one type into another in a manner dependent on the source type. @@ -124,7 +152,7 @@ pub trait StaticLookup { /// Type to lookup into. type Target; /// Attempt a lookup. - fn lookup(s: Self::Source) -> result::Result; + fn lookup(s: Self::Source) -> Result; /// Convert from Target back to Source. fn unlookup(t: Self::Target) -> Self::Source; } @@ -135,13 +163,14 @@ pub struct IdentityLookup(PhantomData); impl StaticLookup for IdentityLookup { type Source = T; type Target = T; - fn lookup(x: T) -> result::Result { Ok(x) } + fn lookup(x: T) -> 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) } + fn lookup(&self, x: T) -> Result { Ok(x) } } /// Extensible conversion trait. Generic over both source and destination types. @@ -781,7 +810,7 @@ pub trait Checkable: Sized { type Checked; /// 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 @@ -793,61 +822,21 @@ pub trait BlindCheckable: Sized { type Checked; /// 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; - fn check(self, _c: &Context) -> Result { - BlindCheckable::check(self) - } -} - -/// An abstract error concerning an attempt to verify, check or dispatch the transaction. This -/// cannot be more concrete because it's designed to work reasonably well over a broad range of -/// possible transaction types. -#[cfg_attr(feature = "std", derive(PartialEq, Debug))] -pub enum DispatchError { - /// General error to do with the inability to pay some fees (e.g. account balance too low). - Payment, - - /// General error to do with the exhaustion of block resources. - Exhausted, - /// General error to do with the permissions of the sender. - NoPermission, - - /// General error to do with the state of the system in general. - BadState, - - /// General error to do with the transaction being outdated (e.g. nonce too low). - Stale, - - /// General error to do with the transaction not yet being valid (e.g. nonce too high). - Future, - - /// General error to do with the transaction's proofs (e.g. signature). - BadProof, -} - -impl From for i8 { - fn from(e: DispatchError) -> i8 { - match e { - DispatchError::Payment => -64, - DispatchError::Exhausted => -65, - DispatchError::NoPermission => -66, - DispatchError::BadState => -67, - DispatchError::Stale => -68, - DispatchError::Future => -69, - DispatchError::BadProof => -70, - } + fn check(self, _c: &Context) -> Result { + BlindCheckable::check(self) } } /// Result of a module function call; either nothing (functions are only called for "side effects") /// or an error message. -pub type DispatchResult = result::Result<(), &'static str>; +pub type DispatchResult = result::Result<(), Error>; /// A lazy call (module function and argument values) that can be executed via its `dispatch` /// method. @@ -858,15 +847,15 @@ pub trait Dispatchable { type Origin; /// ... type Trait; + /// The error type returned by this dispatchable. + type Error: Into; /// Actually dispatch this call and result the result of it. - fn dispatch(self, origin: Self::Origin) -> DispatchResult; + fn dispatch(self, origin: Self::Origin) -> DispatchResult; } /// Means by which a transaction may be extended. This type embodies both the data and the logic /// that should be additionally associated with the transaction. It should be plain old data. -pub trait SignedExtension: - Codec + MaybeDebug + Sync + Send + Clone + Eq + PartialEq -{ +pub trait SignedExtension: Codec + MaybeDebug + Sync + Send + Clone + Eq + PartialEq { /// The type which encodes the sender identity. type AccountId; @@ -882,7 +871,7 @@ pub trait SignedExtension: /// Construct any additional data that should be in the signed payload of the transaction. Can /// also perform any pre-signature-verification checks and return an error if needed. - fn additional_signed(&self) -> Result; + fn additional_signed(&self) -> Result; /// Validate a signed transaction for the transaction queue. /// @@ -899,8 +888,8 @@ pub trait SignedExtension: _call: &Self::Call, _info: DispatchInfo, _len: usize, - ) -> Result { - Ok(Default::default()) + ) -> TransactionValidity { + Ok(ValidTransaction::default()) } /// Do any pre-flight stuff for a signed transaction. @@ -917,8 +906,10 @@ pub trait SignedExtension: call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { - self.validate(who, call, info, len).map(|_| Self::Pre::default()) + ) -> Result { + self.validate(who, call, info, len) + .map(|_| Self::Pre::default()) + .map_err(Into::into) } /// Validate an unsigned transaction for the transaction queue. @@ -936,7 +927,9 @@ pub trait SignedExtension: _call: &Self::Call, _info: DispatchInfo, _len: usize, - ) -> Result { Ok(Default::default()) } + ) -> TransactionValidity { + Ok(ValidTransaction::default()) + } /// Do any pre-flight stuff for a unsigned transaction. /// @@ -950,16 +943,25 @@ pub trait SignedExtension: call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { - Self::validate_unsigned(call, info, len).map(|_| Self::Pre::default()) + ) -> Result { + Self::validate_unsigned(call, info, len) + .map(|_| Self::Pre::default()) + .map_err(Into::into) } /// Do any post-flight stuff for a transaction. - fn post_dispatch( - _pre: Self::Pre, - _info: DispatchInfo, - _len: usize, - ) { } + fn post_dispatch(_pre: Self::Pre, _info: DispatchInfo, _len: usize) { } +} + +/// An error that is returned by a dispatchable function of a module. +pub trait ModuleDispatchError { + /// Convert this error to an `u8`. + /// + /// The `u8` corresponds to the index of the variant in the error enum. + fn as_u8(&self) -> u8; + + /// Convert the error to a `&'static str`. + fn as_str(&self) -> &'static str; } macro_rules! tuple_impl_indexed { @@ -974,10 +976,10 @@ macro_rules! tuple_impl_indexed { > SignedExtension for ($($direct),+,) { type AccountId = AccountId; type Call = Call; - type AdditionalSigned = ($($direct::AdditionalSigned,)+); + type AdditionalSigned = ( $( $direct::AdditionalSigned, )+ ); type Pre = ($($direct::Pre,)+); - fn additional_signed(&self) -> Result { - Ok(( $(self.$index.additional_signed()?,)+ )) + fn additional_signed(&self) -> Result { + Ok(( $( self.$index.additional_signed()?, )+ )) } fn validate( &self, @@ -985,9 +987,16 @@ macro_rules! tuple_impl_indexed { call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { - let aggregator = vec![$(<$direct as SignedExtension>::validate(&self.$index, who, call, info, len)?),+]; - Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a))) + ) -> TransactionValidity { + let aggregator = vec![ + $( <$direct as SignedExtension>::validate(&self.$index, who, call, info, len)? ),+ + ]; + Ok( + aggregator.into_iter().fold( + ValidTransaction::default(), + |acc, a| acc.combine_with(a), + ) + ) } fn pre_dispatch( self, @@ -995,22 +1004,28 @@ macro_rules! tuple_impl_indexed { call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { + ) -> Result { Ok(($(self.$index.pre_dispatch(who, call, info, len)?,)+)) } fn validate_unsigned( call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { - let aggregator = vec![$($direct::validate_unsigned(call, info, len)?),+]; - Ok(aggregator.into_iter().fold(ValidTransaction::default(), |acc, a| acc.combine_with(a))) + ) -> TransactionValidity { + let aggregator = vec![ $( $direct::validate_unsigned(call, info, len)? ),+ ]; + + Ok( + aggregator.into_iter().fold( + ValidTransaction::default(), + |acc, a| acc.combine_with(a), + ) + ) } fn pre_dispatch_unsigned( call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { + ) -> Result { Ok(($($direct::pre_dispatch_unsigned(call, info, len)?,)+)) } fn post_dispatch( @@ -1047,7 +1062,7 @@ impl SignedExtension for () { type AdditionalSigned = (); type Call = (); type Pre = (); - fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } } /// An "executable" piece of information, used by the standard Substrate Executive in order to @@ -1067,17 +1082,19 @@ pub trait Applyable: Sized + Send + Sync { fn sender(&self) -> Option<&Self::AccountId>; /// Checks to see if this is a valid *transaction*. It returns information on it if so. - fn validate>(&self, + fn validate>( + &self, info: DispatchInfo, len: usize, ) -> TransactionValidity; /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. - fn dispatch(self, + fn apply( + self, info: DispatchInfo, len: usize, - ) -> Result; + ) -> crate::ApplyResult; } /// Auxiliary wrapper that holds an api instance and binds it to the given lifetime. diff --git a/core/sr-primitives/src/transaction_validity.rs b/core/sr-primitives/src/transaction_validity.rs index 4d5d53baf199f..eb6cf3bdbb00e 100644 --- a/core/sr-primitives/src/transaction_validity.rs +++ b/core/sr-primitives/src/transaction_validity.rs @@ -17,8 +17,7 @@ //! Transaction validity interface. use rstd::prelude::*; -use crate::codec::{Encode, Decode, Error}; -use crate::traits::DispatchError; +use crate::codec::{Encode, Decode}; /// Priority for a transaction. Additive. Higher is better. pub type TransactionPriority = u64; @@ -30,29 +29,151 @@ pub type TransactionLongevity = u64; /// Tag for a transaction. No two transactions with the same tag should be placed on-chain. pub type TransactionTag = Vec; -/// Information on a transaction's validity and, if valid, on how it relates to other transactions. -#[derive(Clone, PartialEq, Eq, Encode)] -#[cfg_attr(feature = "std", derive(Debug))] -pub enum TransactionValidity { - /// Transaction is invalid. Details are described by the error code. - Invalid(i8), - /// Transaction is valid. - Valid(ValidTransaction), +/// An invalid transaction validity. +#[derive(Clone, PartialEq, Eq, Encode, Decode, Copy)] +#[cfg_attr(feature = "std", derive(Debug, serde::Serialize))] +pub enum InvalidTransaction { + /// The call of the transaction is not expected. + Call, + /// General error to do with the inability to pay some fees (e.g. account balance too low). + Payment, + /// General error to do with the transaction not yet being valid (e.g. nonce too high). + Future, + /// General error to do with the transaction being outdated (e.g. nonce too low). + Stale, + /// General error to do with the transaction's proofs (e.g. signature). + BadProof, + /// The transaction birth block is ancient. + AncientBirthBlock, + /// The transaction would exhaust the resources of current block. + /// + /// The transaction might be valid, but there are not enough resources left in the current block. + ExhaustsResources, + /// Any other custom invalid validity that is not covered by this enum. + Custom(u8), +} + +impl InvalidTransaction { + /// Returns if the reason for the invalidity was block resource exhaustion. + pub fn exhausted_resources(&self) -> bool { + match self { + Self::ExhaustsResources => true, + _ => false, + } + } +} + +impl From for &'static str { + fn from(invalid: InvalidTransaction) -> &'static str { + match invalid { + InvalidTransaction::Call => "Transaction call is not expected", + InvalidTransaction::Future => "Transaction will be valid in the future", + InvalidTransaction::Stale => "Transaction is outdated", + InvalidTransaction::BadProof => "Transaction has a bad signature", + InvalidTransaction::AncientBirthBlock => "Transaction has an ancient birth block", + InvalidTransaction::ExhaustsResources => + "Transaction would exhausts the block limits", + InvalidTransaction::Payment => + "Inability to pay some fees (e.g. account balance too low)", + InvalidTransaction::Custom(_) => "InvalidTransaction custom error", + } + } +} + +/// An unknown transaction validity. +#[derive(Clone, PartialEq, Eq, Encode, Decode, Copy)] +#[cfg_attr(feature = "std", derive(Debug, serde::Serialize))] +pub enum UnknownTransaction { + /// Could not lookup some information that is required to validate the transaction. + CannotLookup, + /// No validator found for the given unsigned transaction. + NoUnsignedValidator, + /// Any other custom unknown validity that is not covered by this enum. + Custom(u8), +} + +impl From for &'static str { + fn from(unknown: UnknownTransaction) -> &'static str { + match unknown { + UnknownTransaction::CannotLookup => + "Could not lookup information required to validate the transaction", + UnknownTransaction::NoUnsignedValidator => + "Could not find an unsigned validator for the unsigned transaction", + UnknownTransaction::Custom(_) => "UnknownTransaction custom error", + } + } +} + +/// Errors that can occur while checking the validity of a transaction. +#[derive(Clone, PartialEq, Eq, Encode, Decode, Copy)] +#[cfg_attr(feature = "std", derive(Debug, serde::Serialize))] +pub enum TransactionValidityError { + /// The transaction is invalid. + Invalid(InvalidTransaction), /// Transaction validity can't be determined. - Unknown(i8), + Unknown(UnknownTransaction), +} + +impl TransactionValidityError { + /// Returns `true` if the reason for the error was block resource exhaustion. + pub fn exhausted_resources(&self) -> bool { + match self { + Self::Invalid(e) => e.exhausted_resources(), + Self::Unknown(_) => false, + } + } } -impl From> for TransactionValidity { - fn from(r: Result) -> Self { - match r { - Ok(v) => TransactionValidity::Valid(v), - Err(e) => TransactionValidity::Invalid(e.into()), +impl From for &'static str { + fn from(err: TransactionValidityError) -> &'static str { + match err { + TransactionValidityError::Invalid(invalid) => invalid.into(), + TransactionValidityError::Unknown(unknown) => unknown.into(), } } } +impl From for TransactionValidityError { + fn from(err: InvalidTransaction) -> Self { + TransactionValidityError::Invalid(err) + } +} + +impl From for TransactionValidityError { + fn from(err: UnknownTransaction) -> Self { + TransactionValidityError::Unknown(err) + } +} + +impl From for crate::ApplyError { + fn from(invalid: InvalidTransaction) -> crate::ApplyError { + TransactionValidityError::from(invalid).into() + } +} + +impl From for crate::ApplyError { + fn from(unknown: UnknownTransaction) -> crate::ApplyError { + TransactionValidityError::from(unknown).into() + } +} + +/// Information on a transaction's validity and, if valid, on how it relates to other transactions. +pub type TransactionValidity = Result; + +impl Into for InvalidTransaction { + fn into(self) -> TransactionValidity { + Err(self.into()) + } +} + +impl Into for UnknownTransaction { + fn into(self) -> TransactionValidity { + Err(self.into()) + } +} + /// Information concerning a valid transaction. -#[derive(Clone, PartialEq, Eq, Encode)] +#[derive(Clone, PartialEq, Eq, Encode, Decode)] #[cfg_attr(feature = "std", derive(Debug))] pub struct ValidTransaction { /// Priority of the transaction. @@ -112,49 +233,13 @@ impl ValidTransaction { } } -impl Decode for TransactionValidity { - fn decode(value: &mut I) -> Result { - match value.read_byte()? { - 0 => Ok(TransactionValidity::Invalid(i8::decode(value)?)), - 1 => { - let priority = TransactionPriority::decode(value)?; - let requires = Vec::decode(value)?; - let provides = Vec::decode(value)?; - let longevity = TransactionLongevity::decode(value)?; - let propagate = bool::decode(value).unwrap_or(true); - - Ok(TransactionValidity::Valid(ValidTransaction { - priority, requires, provides, longevity, propagate, - })) - }, - 2 => Ok(TransactionValidity::Unknown(i8::decode(value)?)), - _ => Err("Invalid transaction validity variant".into()), - } - } -} - #[cfg(test)] mod tests { use super::*; - #[test] - fn should_decode_with_backward_compat() { - let old_encoding = vec![ - 1, 5, 0, 0, 0, 0, 0, 0, 0, 4, 16, 1, 2, 3, 4, 4, 12, 4, 5, 6, 42, 0, 0, 0, 0, 0, 0, 0 - ]; - - assert_eq!(TransactionValidity::decode(&mut &*old_encoding), Ok(TransactionValidity::Valid(ValidTransaction { - priority: 5, - requires: vec![vec![1, 2, 3, 4]], - provides: vec![vec![4, 5, 6]], - longevity: 42, - propagate: true, - }))); - } - #[test] fn should_encode_and_decode() { - let v = TransactionValidity::Valid(ValidTransaction { + let v: TransactionValidity = Ok(ValidTransaction { priority: 5, requires: vec![vec![1, 2, 3, 4]], provides: vec![vec![4, 5, 6]], @@ -165,7 +250,7 @@ mod tests { let encoded = v.encode(); assert_eq!( encoded, - vec![1, 5, 0, 0, 0, 0, 0, 0, 0, 4, 16, 1, 2, 3, 4, 4, 12, 4, 5, 6, 42, 0, 0, 0, 0, 0, 0, 0, 0] + vec![0, 5, 0, 0, 0, 0, 0, 0, 0, 4, 16, 1, 2, 3, 4, 4, 12, 4, 5, 6, 42, 0, 0, 0, 0, 0, 0, 0, 0] ); // decode back diff --git a/core/test-runtime/src/lib.rs b/core/test-runtime/src/lib.rs index f023fd89e137d..ec7ed9670c330 100644 --- a/core/test-runtime/src/lib.rs +++ b/core/test-runtime/src/lib.rs @@ -38,7 +38,9 @@ use substrate_client::{ }; use sr_primitives::{ ApplyResult, create_runtime_str, Perbill, impl_opaque_keys, - transaction_validity::{TransactionValidity, ValidTransaction}, + transaction_validity::{ + TransactionValidity, ValidTransaction, TransactionValidityError, InvalidTransaction, + }, traits::{ BlindCheckable, BlakeTwo256, Block as BlockT, Extrinsic as ExtrinsicT, GetNodeBlockType, GetRuntimeBlockType, Verify, IdentityLookup, @@ -123,17 +125,17 @@ impl serde::Serialize for Extrinsic { impl BlindCheckable for Extrinsic { type Checked = Self; - fn check(self) -> Result { + fn check(self) -> Result { match self { Extrinsic::AuthoritiesChange(new_auth) => Ok(Extrinsic::AuthoritiesChange(new_auth)), Extrinsic::Transfer(transfer, signature) => { if sr_primitives::verify_encoded_lazy(&signature, &transfer, &transfer.from) { Ok(Extrinsic::Transfer(transfer, signature)) } else { - Err(sr_primitives::BAD_SIGNATURE) + Err(InvalidTransaction::BadProof.into()) } }, - Extrinsic::IncludeData(_) => Err(sr_primitives::BAD_SIGNATURE), + Extrinsic::IncludeData(_) => Err(InvalidTransaction::BadProof.into()), Extrinsic::StorageChange(key, value) => Ok(Extrinsic::StorageChange(key, value)), } } @@ -478,7 +480,7 @@ cfg_if! { impl client_api::TaggedTransactionQueue for Runtime { fn validate_transaction(utx: ::Extrinsic) -> TransactionValidity { if let Extrinsic::IncludeData(data) = utx { - return TransactionValidity::Valid(ValidTransaction { + return Ok(ValidTransaction { priority: data.len() as u64, requires: vec![], provides: vec![data], @@ -662,7 +664,7 @@ cfg_if! { impl client_api::TaggedTransactionQueue for Runtime { fn validate_transaction(utx: ::Extrinsic) -> TransactionValidity { if let Extrinsic::IncludeData(data) = utx { - return TransactionValidity::Valid(ValidTransaction{ + return Ok(ValidTransaction{ priority: data.len() as u64, requires: vec![], provides: vec![data], diff --git a/core/test-runtime/src/system.rs b/core/test-runtime/src/system.rs index 63f9c0050b73c..f1288f8b2806f 100644 --- a/core/test-runtime/src/system.rs +++ b/core/test-runtime/src/system.rs @@ -21,12 +21,12 @@ use rstd::prelude::*; use runtime_io::{storage_root, ordered_trie_root, storage_changes_root, twox_128, blake2_256}; use runtime_support::storage::{self, StorageValue, StorageMap}; use runtime_support::storage_items; -use sr_primitives::traits::{Hash as HashT, BlakeTwo256, Header as _}; -use sr_primitives::generic; -use sr_primitives::{ApplyError, ApplyOutcome, ApplyResult}; -use sr_primitives::transaction_validity::{TransactionValidity, ValidTransaction}; +use sr_primitives::{ + traits::{Hash as HashT, BlakeTwo256, Header as _}, generic, ApplyError, ApplyResult, + transaction_validity::{TransactionValidity, ValidTransaction, InvalidTransaction}, +}; use codec::{KeyedVec, Encode}; -use super::{ +use crate::{ AccountId, BlockNumber, Extrinsic, Transfer, H256 as Hash, Block, Header, Digest, AuthorityId }; use primitives::{Blake2Hasher, storage::well_known_keys}; @@ -119,7 +119,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)); - execute_transaction_backend(e).unwrap_or_else(|_| panic!("Invalid transaction")); + let _ = execute_transaction_backend(e).unwrap_or_else(|_| panic!("Invalid transaction")); storage::unhashed::kill(well_known_keys::EXTRINSIC_INDEX); }); @@ -158,17 +158,17 @@ impl executive::ExecuteBlock for BlockExecutor { /// This doesn't attempt to validate anything regarding the block. pub fn validate_transaction(utx: Extrinsic) -> TransactionValidity { if check_signature(&utx).is_err() { - return TransactionValidity::Invalid(ApplyError::BadSignature as i8); + return InvalidTransaction::BadProof.into(); } let tx = utx.transfer(); 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 TransactionValidity::Invalid(ApplyError::Stale as i8); + return InvalidTransaction::Stale.into(); } if tx.nonce > expected_nonce + 64 { - return TransactionValidity::Unknown(ApplyError::Future as i8); + return InvalidTransaction::Future.into(); } let hash = |from: &AccountId, nonce: u64| { @@ -188,7 +188,7 @@ pub fn validate_transaction(utx: Extrinsic) -> TransactionValidity { p }; - TransactionValidity::Valid(ValidTransaction { + Ok(ValidTransaction { priority: tx.amount, requires, provides, @@ -244,8 +244,7 @@ pub fn finalize_block() -> Header { #[inline(always)] fn check_signature(utx: &Extrinsic) -> Result<(), ApplyError> { use sr_primitives::traits::BlindCheckable; - utx.clone().check().map_err(|_| ApplyError::BadSignature)?; - Ok(()) + utx.clone().check().map_err(|_| InvalidTransaction::BadProof.into()).map(|_| ()) } fn execute_transaction_backend(utx: &Extrinsic) -> ApplyResult { @@ -253,7 +252,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(_) => Ok(Ok(())), Extrinsic::StorageChange(key, value) => execute_storage_change(key, value.as_ref().map(|v| &**v)), } } @@ -263,7 +262,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 Err(InvalidTransaction::Stale.into()); } // increment nonce in storage @@ -275,18 +274,18 @@ fn execute_transfer_backend(tx: &Transfer) -> ApplyResult { // enact transfer if !(tx.amount <= from_balance) { - return Err(ApplyError::CantPay) + return Err(InvalidTransaction::Payment.into()); } 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)); - Ok(ApplyOutcome::Success) + Ok(Ok(())) } fn execute_new_authorities_backend(new_authorities: &[AuthorityId]) -> ApplyResult { NewAuthorities::put(new_authorities.to_vec()); - Ok(ApplyOutcome::Success) + Ok(Ok(())) } fn execute_storage_change(key: &[u8], value: Option<&[u8]>) -> ApplyResult { @@ -294,7 +293,7 @@ fn execute_storage_change(key: &[u8], value: Option<&[u8]>) -> ApplyResult { Some(value) => storage::unhashed::put_raw(key, value), None => storage::unhashed::kill(key), } - Ok(ApplyOutcome::Success) + Ok(Ok(())) } #[cfg(feature = "std")] @@ -304,7 +303,7 @@ fn info_expect_equal_hash(given: &Hash, expected: &Hash) { println!( "Hash: given={}, expected={}", HexDisplay::from(given.as_fixed_bytes()), - HexDisplay::from(expected.as_fixed_bytes()) + HexDisplay::from(expected.as_fixed_bytes()), ); } } @@ -312,9 +311,9 @@ fn info_expect_equal_hash(given: &Hash, expected: &Hash) { #[cfg(not(feature = "std"))] fn info_expect_equal_hash(given: &Hash, expected: &Hash) { if given != expected { - ::runtime_io::print("Hash not equal"); - ::runtime_io::print(given.as_bytes()); - ::runtime_io::print(expected.as_bytes()); + runtime_io::print("Hash not equal"); + runtime_io::print(given.as_bytes()); + runtime_io::print(expected.as_bytes()); } } diff --git a/core/transaction-pool/graph/src/error.rs b/core/transaction-pool/graph/src/error.rs index ce457c01796b2..79006461c6e37 100644 --- a/core/transaction-pool/graph/src/error.rs +++ b/core/transaction-pool/graph/src/error.rs @@ -16,7 +16,9 @@ //! Transaction pool errors. -use sr_primitives::transaction_validity::TransactionPriority as Priority; +use sr_primitives::transaction_validity::{ + TransactionPriority as Priority, InvalidTransaction, UnknownTransaction, +}; /// Transaction pool result. pub type Result = std::result::Result; @@ -25,11 +27,11 @@ pub type Result = std::result::Result; #[derive(Debug, derive_more::Display, derive_more::From)] pub enum Error { /// Transaction is not verifiable yet, but might be in the future. - #[display(fmt="Unkown Transaction Validity. Error code: {}", _0)] - UnknownTransactionValidity(i8), + #[display(fmt="Unknown transaction validity: {:?}", _0)] + UnknownTransaction(UnknownTransaction), /// Transaction is invalid. - #[display(fmt="Invalid Transaction. Error Code: {}", _0)] - InvalidTransaction(i8), + #[display(fmt="Invalid transaction validity: {:?}", _0)] + InvalidTransaction(InvalidTransaction), /// The transaction validity returned no "provides" tag. /// /// Such transactions are not accepted to the pool, since we use those tags diff --git a/core/transaction-pool/graph/src/pool.rs b/core/transaction-pool/graph/src/pool.rs index fa09132d293fa..75a4bc4394f40 100644 --- a/core/transaction-pool/graph/src/pool.rs +++ b/core/transaction-pool/graph/src/pool.rs @@ -34,7 +34,7 @@ use parking_lot::{Mutex, RwLock}; use sr_primitives::{ generic::BlockId, traits::{self, SaturatedConversion}, - transaction_validity::{TransactionValidity, TransactionTag as Tag}, + transaction_validity::{TransactionValidity, TransactionTag as Tag, TransactionValidityError}, }; pub use crate::base_pool::Limit; @@ -129,7 +129,7 @@ impl Pool { } match self.api.validate_transaction(at, xt.clone())? { - TransactionValidity::Valid(validity) => if validity.provides.is_empty() { + Ok(validity) => if validity.provides.is_empty() { Err(error::Error::NoTagsProvided.into()) } else { Ok(base::Transaction { @@ -145,12 +145,12 @@ impl Pool { .saturating_add(validity.longevity), }) }, - TransactionValidity::Invalid(e) => { + Err(TransactionValidityError::Invalid(e)) => { Err(error::Error::InvalidTransaction(e).into()) }, - TransactionValidity::Unknown(e) => { + Err(TransactionValidityError::Unknown(e)) => { self.listener.write().invalid(&hash); - Err(error::Error::UnknownTransactionValidity(e).into()) + Err(error::Error::UnknownTransaction(e).into()) }, } }) @@ -244,7 +244,7 @@ impl Pool { None => { let validity = self.api.validate_transaction(parent, extrinsic.clone()); match validity { - Ok(TransactionValidity::Valid(mut validity)) => { + Ok(Ok(mut validity)) => { tags.append(&mut validity.provides); }, // silently ignore invalid extrinsics, @@ -453,7 +453,7 @@ fn fire_events( #[cfg(test)] mod tests { use super::*; - use sr_primitives::transaction_validity::ValidTransaction; + use sr_primitives::transaction_validity::{ValidTransaction, InvalidTransaction}; use codec::Encode; use test_runtime::{Block, Extrinsic, Transfer, H256, AccountId}; use assert_matches::assert_matches; @@ -472,8 +472,11 @@ mod tests { type Error = error::Error; /// Verify extrinsic at given block. - fn validate_transaction(&self, at: &BlockId, uxt: ExtrinsicFor) -> Result { - + fn validate_transaction( + &self, + at: &BlockId, + uxt: ExtrinsicFor, + ) -> Result { let block_number = self.block_id_to_number(at)?.unwrap(); let nonce = uxt.transfer().nonce; @@ -488,9 +491,9 @@ mod tests { } if nonce < block_number { - Ok(TransactionValidity::Invalid(0)) + Ok(InvalidTransaction::Stale.into()) } else { - Ok(TransactionValidity::Valid(ValidTransaction { + Ok(Ok(ValidTransaction { priority: 4, requires: if nonce > block_number { vec![vec![nonce as u8 - 1]] } else { vec![] }, provides: if nonce == INVALID_NONCE { vec![] } else { vec![vec![nonce as u8]] }, diff --git a/core/transaction-pool/src/tests.rs b/core/transaction-pool/src/tests.rs index 71ed988e5e7b3..1661b7108b9f8 100644 --- a/core/transaction-pool/src/tests.rs +++ b/core/transaction-pool/src/tests.rs @@ -39,7 +39,11 @@ impl txpool::ChainApi for TestApi { type Hash = Hash; type Error = error::Error; - fn validate_transaction(&self, at: &BlockId, uxt: txpool::ExtrinsicFor) -> error::Result { + fn validate_transaction( + &self, + at: &BlockId, + uxt: txpool::ExtrinsicFor, + ) -> error::Result { let expected = index(at); let requires = if expected == uxt.transfer().nonce { vec![] @@ -48,13 +52,15 @@ impl txpool::ChainApi for TestApi { }; let provides = vec![vec![uxt.transfer().nonce as u8]]; - Ok(TransactionValidity::Valid(ValidTransaction { - priority: 1, - requires, - provides, - longevity: 64, - propagate: true, - })) + Ok( + Ok(ValidTransaction { + priority: 1, + requires, + provides, + longevity: 64, + propagate: true, + }) + ) } fn block_id_to_number(&self, at: &BlockId) -> error::Result>> { diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 6a73a47d366e9..487937eab0f86 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -235,7 +235,6 @@ impl balances::Trait for Runtime { type OnNewAccount = Indices; /// The ubiquitous event type. type Event = Event; - type TransactionPayment = (); type DustRemoval = (); type TransferPayment = (); @@ -268,7 +267,7 @@ construct_runtime!( Babe: babe::{Module, Call, Storage, Config, Inherent(Timestamp)}, Grandpa: grandpa::{Module, Call, Storage, Config, Event}, 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/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index e371aef8e0842..c16a8af6f8f29 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -56,7 +56,8 @@ impl FactoryState { system::CheckEra::from(Era::mortal(256, phase)), system::CheckNonce::from(index), system::CheckWeight::new(), - balances::TakeFees::from(0) + balances::TakeFees::from(0), + Default::default(), ) } } @@ -147,7 +148,7 @@ impl RuntimeAdapter for FactoryState { (*amount).into() ) ) - }, key, (version, genesis_hash.clone(), prior_block_hash.clone(), (), (), ())) + }, key, (version, genesis_hash.clone(), prior_block_hash.clone(), (), (), (), ())) } fn inherent_extrinsics(&self) -> InherentData { diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index bb1d77b285ca6..509de7a943815 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -470,11 +470,19 @@ mod tests { let check_nonce = system::CheckNonce::from(index); let check_weight = system::CheckWeight::new(); let take_fees = balances::TakeFees::from(0); - let extra = (check_version, check_genesis, check_era, check_nonce, check_weight, take_fees); + let extra = ( + check_version, + check_genesis, + check_era, + check_nonce, + check_weight, + take_fees, + Default::default(), + ); let raw_payload = SignedPayload::from_raw( function, extra, - (version, genesis_hash, genesis_hash, (), (), ()) + (version, genesis_hash, genesis_hash, (), (), (), ()) ); let signature = raw_payload.using_encoded(|payload| { signer.sign(payload) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 27b94df8b3fc5..fc0ca390497e4 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -43,16 +43,16 @@ mod tests { use runtime_support::{Hashable, StorageValue, StorageMap, assert_eq_error_rate, traits::Currency}; use state_machine::{CodeExecutor, Externalities, TestExternalities as CoreTestExternalities}; use primitives::{Blake2Hasher, NeverNativeValue, NativeOrEncoded, map}; - use sr_primitives::traits::{Header as HeaderT, Hash as HashT, Convert}; - use sr_primitives::{ApplyOutcome, ApplyError, ApplyResult}; - use sr_primitives::weights::{WeightMultiplier, GetDispatchInfo}; + use node_primitives::{Hash, BlockNumber, Balance}; + use sr_primitives::{ + traits::{Header as HeaderT, Hash as HashT, Convert}, ApplyOutcome, ApplyResult, + transaction_validity::InvalidTransaction, weights::{WeightMultiplier, GetDispatchInfo}, + }; use contracts::ContractAddressFor; use system::{EventRecord, Phase}; - use node_primitives::{Hash, BlockNumber, Balance}; use node_runtime::{ Header, Block, UncheckedExtrinsic, CheckedExtrinsic, Call, Runtime, Balances, BuildStorage, - System, Event, - TransferFee, TransactionBaseFee, TransactionByteFee, + System, Event, TransferFee, TransactionBaseFee, TransactionByteFee, }; use node_runtime::constants::currency::*; use node_runtime::impls::WeightToFee; @@ -150,7 +150,7 @@ mod tests { None, ).0.unwrap(); let r = ApplyResult::decode(&mut &v.as_encoded()[..]).unwrap(); - assert_eq!(r, Err(ApplyError::CantPay)); + assert_eq!(r, Err(InvalidTransaction::Payment.into())); } #[test] @@ -186,7 +186,7 @@ mod tests { None, ).0.unwrap(); let r = ApplyResult::decode(&mut &v.as_encoded()[..]).unwrap(); - assert_eq!(r, Err(ApplyError::CantPay)); + assert_eq!(r, Err(InvalidTransaction::Payment.into())); } #[test] @@ -763,7 +763,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, Err(ApplyError::CantPay)); + assert_eq!(r, Err(InvalidTransaction::Payment.into())); } #[test] @@ -784,8 +784,10 @@ mod tests { assert!(r.is_ok()); 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)); + ApplyResult::decode(&mut &r[..]) + .unwrap() + .expect("Extrinsic could be applied") + .expect("Extrinsic did not fail"); runtime_io::with_externalities(&mut t, || { assert_eq!(Balances::total_balance(&alice()), 42 * DOLLARS - 1 * transfer_fee(&xt())); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 03d11111f1939..46667db36fe2c 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -80,8 +80,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 154, - impl_version: 159, + spec_version: 155, + impl_version: 155, apis: RUNTIME_API_VERSIONS, }; @@ -445,6 +445,7 @@ impl system::offchain::CreateTransaction for Runtim system::CheckNonce::::from(index), system::CheckWeight::::new(), balances::TakeFees::::from(tip), + Default::default(), ); let raw_payload = SignedPayload::new(call, extra).ok()?; let signature = F::sign(account.clone(), &raw_payload)?; @@ -465,7 +466,7 @@ construct_runtime!( Timestamp: timestamp::{Module, Call, Storage, Inherent}, Authorship: authorship::{Module, Call, Storage, Inherent}, Indices: indices, - Balances: balances, + Balances: balances::{default, Error}, Staking: staking::{default, OfflineWorker}, Session: session::{Module, Call, Storage, Event, Config}, Democracy: democracy::{Module, Call, Storage, Config, Event}, @@ -501,7 +502,8 @@ pub type SignedExtra = ( system::CheckEra, system::CheckNonce, system::CheckWeight, - balances::TakeFees + balances::TakeFees, + contracts::CheckBlockGasLimit, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; diff --git a/node/testing/src/keyring.rs b/node/testing/src/keyring.rs index 12a3e7c2d4d43..0c6eb478cc5a0 100644 --- a/node/testing/src/keyring.rs +++ b/node/testing/src/keyring.rs @@ -72,7 +72,8 @@ pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra { system::CheckEra::from(Era::mortal(256, 0)), system::CheckNonce::from(nonce), system::CheckWeight::new(), - balances::TakeFees::from(extra_fee) + balances::TakeFees::from(extra_fee), + Default::default(), ) } diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 80caa59f21cf5..b865efa92ad98 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -161,19 +161,26 @@ use rstd::prelude::*; use rstd::{cmp, result, mem}; use codec::{Codec, Encode, Decode}; -use support::{StorageValue, StorageMap, Parameter, decl_event, decl_storage, decl_module}; -use support::traits::{ - UpdateBalanceOutcome, Currency, OnFreeBalanceZero, OnUnbalanced, - WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, - Imbalance, SignedImbalance, ReservableCurrency, Get, +use support::{ + StorageValue, StorageMap, Parameter, decl_event, decl_storage, decl_module, + traits::{ + UpdateBalanceOutcome, Currency, OnFreeBalanceZero, OnUnbalanced, + WithdrawReason, WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, + Imbalance, SignedImbalance, ReservableCurrency, Get, + }, + dispatch::Result, }; -use support::dispatch::Result; -use sr_primitives::traits::{ - Zero, SimpleArithmetic, StaticLookup, Member, CheckedAdd, CheckedSub, MaybeSerializeDebug, - Saturating, Bounded, SignedExtension, SaturatedConversion, DispatchError, Convert, +use sr_primitives::{ + transaction_validity::{ + TransactionPriority, ValidTransaction, InvalidTransaction, TransactionValidityError, + TransactionValidity, + }, + traits::{ + Zero, SimpleArithmetic, StaticLookup, Member, CheckedAdd, CheckedSub, MaybeSerializeDebug, + Saturating, Bounded, SignedExtension, SaturatedConversion, Convert, + }, + weights::{DispatchInfo, SimpleDispatchInfo, Weight}, }; -use sr_primitives::transaction_validity::{TransactionPriority, ValidTransaction}; -use sr_primitives::weights::{DispatchInfo, SimpleDispatchInfo, Weight}; use system::{IsDeadAccount, OnNewAccount, ensure_signed, ensure_root}; mod mock; @@ -1235,7 +1242,7 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { type Call = T::Call; type AdditionalSigned = (); type Pre = (); - fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } fn validate( &self, @@ -1243,15 +1250,18 @@ impl, I: Instance + Clone + Eq> SignedExtension for TakeFees { _call: &Self::Call, info: DispatchInfo, len: usize, - ) -> rstd::result::Result { + ) -> TransactionValidity { // pay any fees. let fee = Self::compute_fee(len, info, self.0); - let imbalance = >::withdraw( + let imbalance = match >::withdraw( who, fee, WithdrawReason::TransactionPayment, ExistenceRequirement::KeepAlive, - ).map_err(|_| DispatchError::Payment)?; + ) { + Ok(imbalance) => imbalance, + Err(_) => return InvalidTransaction::Payment.into(), + }; T::TransactionPayment::on_unbalanced(imbalance); let mut r = ValidTransaction::default(); diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 777206910da10..8e9f6acdd850b 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -232,7 +232,7 @@ fn default_indexing_on_new_accounts_should_not_work2() { // 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" + "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); @@ -359,7 +359,7 @@ fn force_transfer_works() { let _ = Balances::deposit_creating(&1, 111); assert_noop!( Balances::force_transfer(Some(2).into(), 1, 2, 69), - "bad origin: expected to be a root origin" + "RequireRootOrigin", ); assert_ok!(Balances::force_transfer(RawOrigin::Root.into(), 1, 2, 69)); assert_eq!(Balances::total_balance(&1), 42); @@ -389,7 +389,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), + "balance too low to send value", + ); }); } @@ -517,7 +520,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" + "destination balance too high to receive value", ); assert_eq!(Balances::free_balance(&1), u64::max_value()); @@ -595,7 +598,7 @@ fn transfer_overflow_isnt_exploitable() { assert_err!( Balances::transfer(Some(1).into(), 5, evil_value), - "got overflow after adding a fee to value" + "got overflow after adding a fee to value", ); } ); @@ -680,7 +683,7 @@ fn unvested_balance_should_not_transfer() { assert_eq!(Balances::vesting_balance(&1), 45); assert_noop!( Balances::transfer(Some(1).into(), 2, 56), - "vesting balance too high to send value" + "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 8e1ffbca0bd50..08070916fbf29 100644 --- a/srml/contracts/src/gas.rs +++ b/srml/contracts/src/gas.rs @@ -16,10 +16,12 @@ use crate::{GasSpent, Module, Trait, BalanceOf, NegativeImbalanceOf}; use rstd::convert::TryFrom; -use sr_primitives::BLOCK_FULL; -use sr_primitives::traits::{CheckedMul, Zero, SaturatedConversion, SimpleArithmetic, UniqueSaturatedInto}; -use support::StorageValue; -use support::traits::{Currency, ExistenceRequirement, Get, Imbalance, OnUnbalanced, WithdrawReason}; +use sr_primitives::traits::{ + CheckedMul, Zero, SaturatedConversion, SimpleArithmetic, UniqueSaturatedInto, +}; +use support::{ + traits::{Currency, ExistenceRequirement, Imbalance, OnUnbalanced, WithdrawReason}, StorageValue, +}; #[cfg(test)] use std::{any::Any, fmt::Debug}; @@ -200,14 +202,6 @@ pub fn buy_gas( transactor: &T::AccountId, gas_limit: Gas, ) -> Result<(GasMeter, NegativeImbalanceOf), &'static str> { - // Check if the specified amount of gas is available in the current block. - // This cannot underflow since `gas_spent` is never greater than `T::BlockGasLimit`. - let gas_available = T::BlockGasLimit::get() - >::gas_spent(); - if gas_limit > gas_available { - // gas limit reached, revert the transaction and retry again in the future - return Err(BLOCK_FULL); - } - // Buy the specified amount of gas. let gas_price = >::gas_price(); let cost = if gas_price.is_zero() { diff --git a/srml/contracts/src/lib.rs b/srml/contracts/src/lib.rs index 37299d44885a6..74ad5867af3ca 100644 --- a/srml/contracts/src/lib.rs +++ b/srml/contracts/src/lib.rs @@ -63,6 +63,15 @@ //! This creates a new smart contract account and calls its contract deploy handler to initialize the contract. //! * `call` - Makes a call to an account, optionally transferring some balance. //! +//! ### Signed Extensions +//! +//! The contracts module defines the following extension: +//! +//! - [`CheckBlockGasLimit`]: Ensures that the transaction does not exceeds the block gas limit. +//! +//! The signed extension needs to be added as signed extra to the transaction type to be used in the +//! runtime. +//! //! ## Usage //! //! The Contract module is a work in progress. The following examples show how this Contract module can be @@ -100,15 +109,19 @@ use primitives::crypto::UncheckedFrom; use rstd::{prelude::*, marker::PhantomData}; use codec::{Codec, Encode, Decode}; use runtime_io::blake2_256; -use sr_primitives::traits::{ - Hash, StaticLookup, Zero, MaybeSerializeDebug, Member +use sr_primitives::{ + traits::{Hash, StaticLookup, Zero, MaybeSerializeDebug, Member, SignedExtension}, + weights::DispatchInfo, + transaction_validity::{ + ValidTransaction, InvalidTransaction, TransactionValidity, TransactionValidityError, + }, }; use support::dispatch::{Result, Dispatchable}; use support::{ Parameter, StorageMap, StorageValue, decl_module, decl_event, decl_storage, storage::child, parameter_types, }; -use support::traits::{OnFreeBalanceZero, OnUnbalanced, Currency, Get}; +use support::{traits::{OnFreeBalanceZero, OnUnbalanced, Currency, Get}, IsSubType}; use system::{ensure_signed, RawOrigin, ensure_root}; use primitives::storage::well_known_keys::CHILD_STORAGE_KEY_PREFIX; use timestamp; @@ -321,7 +334,7 @@ pub trait Trait: timestamp::Trait { type Currency: Currency; /// The outer call dispatch type. - type Call: Parameter + Dispatchable::Origin>; + type Call: Parameter + Dispatchable::Origin> + IsSubType, Self>; /// The overarching event type. type Event: From> + Into<::Event>; @@ -597,15 +610,17 @@ decl_module! { /// the sender is not eligible for the reward. fn claim_surcharge(origin, dest: T::AccountId, aux_sender: Option) { let origin = origin.into(); - let (signed, rewarded) = match origin { - Ok(system::RawOrigin::Signed(ref account)) if aux_sender.is_none() => { + let (signed, rewarded) = match (origin, aux_sender) { + (Ok(system::RawOrigin::Signed(account)), None) => { (true, account) }, - Ok(system::RawOrigin::None) if aux_sender.is_some() => { - (false, aux_sender.as_ref().expect("checked above")) + (Ok(system::RawOrigin::None), Some(aux_sender)) => { + (false, aux_sender) }, - _ => return Err("Invalid surcharge claim: origin must be signed or \ - inherent and auxiliary sender only provided on inherent") + _ => return Err( + "Invalid surcharge claim: origin must be signed or \ + inherent and auxiliary sender only provided on inherent" + ), }; // Add some advantage for block producers (who send unsigned extrinsics) by @@ -619,7 +634,7 @@ decl_module! { // If poking the contract has lead to eviction of the contract, give out the rewards. if rent::try_evict::(&dest, handicap) == rent::RentOutcome::Evicted { - T::Currency::deposit_into_existing(rewarded, T::SurchargeReward::get())?; + T::Currency::deposit_into_existing(&rewarded, T::SurchargeReward::get())?; } } @@ -937,3 +952,62 @@ impl Default for Schedule { } } } + +/// `SignedExtension` that checks if a transaction would exhausts the block gas limit. +#[derive(Encode, Decode, Clone, Eq, PartialEq)] +pub struct CheckBlockGasLimit(PhantomData); + +impl Default for CheckBlockGasLimit { + fn default() -> Self { + Self(PhantomData) + } +} + +#[cfg(feature = "std")] +impl std::fmt::Debug for CheckBlockGasLimit { + fn fmt(&self, _: &mut std::fmt::Formatter) -> std::fmt::Result { + Ok(()) + } +} + +impl SignedExtension for CheckBlockGasLimit { + type AccountId = T::AccountId; + type Call = ::Call; + type AdditionalSigned = (); + type Pre = (); + + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } + + fn validate( + &self, + _: &Self::AccountId, + call: &Self::Call, + _: DispatchInfo, + _: usize, + ) -> TransactionValidity { + let call = match call.is_sub_type() { + Some(call) => call, + None => return Ok(ValidTransaction::default()), + }; + + match call { + Call::claim_surcharge(_, _) | Call::update_schedule(_) => + Ok(ValidTransaction::default()), + Call::put_code(gas_limit, _) + | Call::call(_, _, gas_limit, _) + | Call::create(_, gas_limit, _, _) + => { + // Check if the specified amount of gas is available in the current block. + // This cannot underflow since `gas_spent` is never greater than `T::BlockGasLimit`. + let gas_available = T::BlockGasLimit::get() - >::gas_spent(); + if *gas_limit > gas_available { + // gas limit reached, revert the transaction and retry again in the future + InvalidTransaction::ExhaustsResources.into() + } else { + Ok(ValidTransaction::default()) + } + }, + Call::__PhantomItem(_, _) => unreachable!("Variant is never constructed"), + } + } +} diff --git a/srml/contracts/src/tests.rs b/srml/contracts/src/tests.rs index 91c3e4f712d64..f585eddb596e2 100644 --- a/srml/contracts/src/tests.rs +++ b/srml/contracts/src/tests.rs @@ -22,27 +22,27 @@ use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; use crate::{ BalanceOf, ComputeDispatchFee, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, - Module, RawAliveContractInfo, RawEvent, Trait, TrieId, TrieIdFromParentCounter, - TrieIdGenerator, Schedule, + Module, RawAliveContractInfo, RawEvent, Trait, TrieId, TrieIdFromParentCounter, Schedule, + TrieIdGenerator, CheckBlockGasLimit, }; use assert_matches::assert_matches; use hex_literal::*; use codec::{Decode, Encode, KeyedVec}; use runtime_io; use runtime_io::with_externalities; -use sr_primitives::testing::{Digest, DigestItem, Header, UintAuthorityId, H256}; -use sr_primitives::traits::{BlakeTwo256, Hash, IdentityLookup}; -use sr_primitives::{Perbill, BuildStorage}; +use sr_primitives::{ + Perbill, BuildStorage, transaction_validity::{InvalidTransaction, ValidTransaction}, + traits::{BlakeTwo256, Hash, IdentityLookup, SignedExtension}, + weights::{DispatchInfo, DispatchClass}, + testing::{Digest, DigestItem, Header, UintAuthorityId, H256}, +}; use support::{ assert_ok, assert_err, impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types, storage::child, StorageMap, StorageValue, traits::{Currency, Get}, }; -use std::cell::RefCell; -use std::sync::atomic::{AtomicUsize, Ordering}; -use primitives::storage::well_known_keys; -use primitives::Blake2Hasher; +use std::{cell::RefCell, sync::atomic::{AtomicUsize, Ordering}}; +use primitives::{storage::well_known_keys, Blake2Hasher}; use system::{self, EventRecord, Phase}; -use {balances, wabt}; mod contract { // Re-export contents of the root. This basically @@ -2390,3 +2390,22 @@ fn cannot_self_destruct_in_constructor() { } ); } + +#[test] +fn check_block_gas_limit_works() { + with_externalities( + &mut ExtBuilder::default().block_gas_limit(50).build(), + || { + let info = DispatchInfo { weight: 100, class: DispatchClass::Normal }; + let check = CheckBlockGasLimit::(Default::default()); + let call: Call = crate::Call::put_code(1000, vec![]).into(); + + assert_eq!( + check.validate(&0, &call, info, 0), InvalidTransaction::ExhaustsResources.into(), + ); + + let call: Call = crate::Call::update_schedule(Default::default()).into(); + assert_eq!(check.validate(&0, &call, info, 0), Ok(Default::default())); + } + ); +} diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index 3ec76a15e1567..aa27bcde21f6a 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -62,6 +62,8 @@ mod tests { impl_outer_dispatch! { pub enum Call for Test where origin: Origin { + type Error = Error; + balances::Balances, democracy::Democracy, } @@ -115,6 +117,7 @@ mod tests { type Header = Header; type WeightMultiplierUpdate = (); type Event = Event; + type Error = Error; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; type MaximumBlockLength = MaximumBlockLength; @@ -136,6 +139,7 @@ mod tests { type TransactionPayment = (); type TransferPayment = (); type DustRemoval = (); + type Error = Error; type ExistentialDeposit = ExistentialDeposit; type TransferFee = TransferFee; type CreationFee = CreationFee; diff --git a/srml/democracy/src/lib.rs b/srml/democracy/src/lib.rs index 1fc694beced14..2b74c44598b2a 100644 --- a/srml/democracy/src/lib.rs +++ b/srml/democracy/src/lib.rs @@ -20,12 +20,14 @@ use rstd::prelude::*; use rstd::{result, convert::TryFrom}; -use sr_primitives::traits::{Zero, Bounded, CheckedMul, CheckedDiv, EnsureOrigin, Hash}; -use sr_primitives::weights::SimpleDispatchInfo; +use sr_primitives::{ + traits::{Zero, Bounded, CheckedMul, CheckedDiv, EnsureOrigin, Hash, Dispatchable}, + weights::SimpleDispatchInfo, +}; use codec::{Encode, Decode, Input, Output, Error}; use support::{ decl_module, decl_storage, decl_event, ensure, StorageValue, StorageMap, StorageLinkedMap, - Parameter, Dispatchable, + Parameter, traits::{ Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get, OnFreeBalanceZero diff --git a/srml/elections/src/lib.rs b/srml/elections/src/lib.rs index b14ffa9463d17..64b1e9e1dbe3e 100644 --- a/srml/elections/src/lib.rs +++ b/srml/elections/src/lib.rs @@ -504,7 +504,7 @@ decl_module! { let who = ensure_signed(origin)?; ensure!( !total.is_zero(), - "stake deposited to present winner and be added to leaderboard should be non-zero" + "stake deposited to present winner and be added to leaderboard should be non-zero", ); let candidate = T::Lookup::lookup(candidate)?; @@ -1234,7 +1234,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/example/src/lib.rs b/srml/example/src/lib.rs index 596defae0493c..cfa3636e4d03a 100644 --- a/srml/example/src/lib.rs +++ b/srml/example/src/lib.rs @@ -258,9 +258,10 @@ use support::{StorageValue, dispatch::Result, decl_module, decl_storage, decl_ev use system::{ensure_signed, ensure_root}; use codec::{Encode, Decode}; use sr_primitives::{ - traits::{SignedExtension, DispatchError, Bounded}, - transaction_validity::ValidTransaction, - weights::{SimpleDispatchInfo, DispatchInfo}, + traits::{SignedExtension, Bounded}, weights::{SimpleDispatchInfo, DispatchInfo}, + transaction_validity::{ + ValidTransaction, TransactionValidityError, InvalidTransaction, TransactionValidity, + }, }; /// Our module's configuration trait. All our types and consts go in here. If the @@ -561,7 +562,7 @@ impl SignedExtension for WatchDummy { type AdditionalSigned = (); type Pre = (); - fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } fn validate( &self, @@ -569,9 +570,11 @@ impl SignedExtension for WatchDummy { call: &Self::Call, _info: DispatchInfo, len: usize, - ) -> rstd::result::Result { + ) -> TransactionValidity { // if the transaction is too big, just drop it. - if len > 200 { return Err(DispatchError::Exhausted) } + if len > 200 { + return InvalidTransaction::ExhaustsResources.into() + } // check for `set_dummy` match call { @@ -582,7 +585,7 @@ impl SignedExtension for WatchDummy { valid_tx.priority = Bounded::max_value(); Ok(valid_tx) } - _ => Ok(Default::default()) + _ => Ok(Default::default()), } } } @@ -712,12 +715,14 @@ mod tests { let info = DispatchInfo::default(); assert_eq!( - WatchDummy::(PhantomData).validate(&1, &call, info, 150).unwrap().priority, - Bounded::max_value() + WatchDummy::(PhantomData).validate(&1, &call, info, 150) + .unwrap() + .priority, + Bounded::max_value(), ); assert_eq!( WatchDummy::(PhantomData).validate(&1, &call, info, 250), - Err(DispatchError::Exhausted) + InvalidTransaction::ExhaustsResources.into(), ); }) } diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index c6931af0a1b0b..f3e8b63a28316 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -59,13 +59,13 @@ //! # pub type Balances = u64; //! # pub type AllModules = u64; //! # pub enum Runtime {}; -//! # use sr_primitives::transaction_validity::TransactionValidity; +//! # use sr_primitives::transaction_validity::{TransactionValidity, UnknownTransaction}; //! # use sr_primitives::traits::ValidateUnsigned; //! # impl ValidateUnsigned for Runtime { //! # type Call = (); //! # //! # fn validate_unsigned(_call: &Self::Call) -> TransactionValidity { -//! # TransactionValidity::Invalid(0) +//! # UnknownTransaction::NoUnsignedValidator.into() //! # } //! # } //! /// Executive: handles dispatch to the various modules. @@ -74,50 +74,17 @@ #![cfg_attr(not(feature = "std"), no_std)] -use rstd::prelude::*; -use rstd::marker::PhantomData; -use rstd::result; -use sr_primitives::{generic::Digest, traits::{ - self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, - OnInitialize, NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned -}}; -use support::Dispatchable; +use rstd::{prelude::*, marker::PhantomData}; +use sr_primitives::{ + generic::Digest, ApplyResult, weights::GetDispatchInfo, + traits::{ + self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, + NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, Dispatchable + }, + transaction_validity::TransactionValidity, +}; use codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; -use sr_primitives::{ApplyOutcome, ApplyError}; -use sr_primitives::transaction_validity::TransactionValidity; -use sr_primitives::weights::GetDispatchInfo; - -mod internal { - use sr_primitives::traits::DispatchError; - - pub enum ApplyError { - BadSignature(&'static str), - Stale, - Future, - CantPay, - FullBlock, - } - - pub enum ApplyOutcome { - Success, - Fail(&'static str), - } - - impl From for ApplyError { - fn from(d: DispatchError) -> Self { - match d { - DispatchError::Payment => ApplyError::CantPay, - DispatchError::Exhausted => ApplyError::FullBlock, - DispatchError::NoPermission => ApplyError::CantPay, - DispatchError::BadState => ApplyError::CantPay, - DispatchError::Stale => ApplyError::Stale, - DispatchError::Future => ApplyError::Future, - DispatchError::BadProof => ApplyError::BadSignature(""), - } - } - } -} /// Trait that can be used to execute a block. pub trait ExecuteBlock { @@ -239,30 +206,19 @@ 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"), + Ok(Ok(())) => (), + Ok(Err(e)) => runtime_io::print(e), + Err(e) => { let err: &'static str = e.into(); panic!(err) }, } } @@ -271,9 +227,9 @@ 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)?; + let xt = uxt.check(&Default::default())?; // We don't need to make sure to `note_extrinsic` only after we know it's going to be // executed to prevent it from leaking in storage since at this point, it will either @@ -286,15 +242,11 @@ where // Decode parameters and dispatch let dispatch_info = xt.get_dispatch_info(); - let r = Applyable::dispatch(xt, dispatch_info, encoded_len) - .map_err(internal::ApplyError::from)?; + let r = Applyable::apply(xt, dispatch_info, encoded_len)?; >::note_applied_extrinsic(&r, encoded_len as u32); - r.map(|_| internal::ApplyOutcome::Success).or_else(|e| match e { - sr_primitives::BLOCK_FULL => Err(internal::ApplyError::FullBlock), - e => Ok(internal::ApplyOutcome::Fail(e)) - }) + Ok(r) } fn final_checks(header: &System::Header) { @@ -324,21 +276,8 @@ where /// /// Changes made to storage should be discarded. pub fn validate_transaction(uxt: Block::Extrinsic) -> TransactionValidity { - // Note errors > 0 are from ApplyError - const UNKNOWN_ERROR: i8 = -127; - const INVALID_INDEX: i8 = -10; - let encoded_len = uxt.using_encoded(|d| d.len()); - 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(sr_primitives::BAD_SIGNATURE) => return TransactionValidity::Invalid(ApplyError::BadSignature as i8), - Err(_) => return TransactionValidity::Invalid(UNKNOWN_ERROR), - }; + let xt = uxt.check(&Default::default())?; let dispatch_info = xt.get_dispatch_info(); xt.validate::(dispatch_info, encoded_len) @@ -357,13 +296,15 @@ mod tests { use balances::Call; use runtime_io::with_externalities; use primitives::{H256, Blake2Hasher}; - use sr_primitives::generic::Era; - use sr_primitives::Perbill; - use sr_primitives::weights::Weight; - use sr_primitives::traits::{Header as HeaderT, BlakeTwo256, IdentityLookup, ConvertInto}; - use sr_primitives::testing::{Digest, Header, Block}; - use support::{impl_outer_event, impl_outer_origin, parameter_types}; - use support::traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons, WithdrawReason}; + use sr_primitives::{ + generic::Era, Perbill, DispatchError, weights::Weight, testing::{Digest, Header, Block}, + traits::{Header as HeaderT, BlakeTwo256, IdentityLookup, ConvertInto}, + transaction_validity::{InvalidTransaction, UnknownTransaction}, ApplyError, + }; + use support::{ + impl_outer_event, impl_outer_origin, parameter_types, + traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons, WithdrawReason}, + }; use system; use hex_literal::hex; @@ -432,8 +373,8 @@ mod tests { fn validate_unsigned(call: &Self::Call) -> TransactionValidity { match call { - Call::set_balance(_, _, _) => TransactionValidity::Valid(Default::default()), - _ => TransactionValidity::Invalid(0), + Call::set_balance(_, _, _) => Ok(Default::default()), + _ => UnknownTransaction::NoUnsignedValidator.into(), } } } @@ -479,7 +420,7 @@ mod tests { Digest::default(), )); let r = Executive::apply_extrinsic(xt); - assert_eq!(r, Ok(ApplyOutcome::Success)); + assert!(r.is_ok()); assert_eq!(>::total_balance(&1), 142 - 10 - weight); assert_eq!(>::total_balance(&2), 69); }); @@ -582,14 +523,19 @@ mod tests { assert_eq!(>::all_extrinsics_weight(), 0); for nonce in 0..=num_to_exhaust_block { - let xt = sr_primitives::testing::TestXt(sign_extra(1, nonce.into(), 0), Call::transfer::(33, 0)); + let xt = sr_primitives::testing::TestXt( + sign_extra(1, nonce.into(), 0), Call::transfer::(33, 0), + ); let res = Executive::apply_extrinsic(xt); if nonce != num_to_exhaust_block { - assert_eq!(res.unwrap(), ApplyOutcome::Success); - assert_eq!(>::all_extrinsics_weight(), encoded_len * (nonce + 1)); + assert!(res.is_ok()); + assert_eq!( + >::all_extrinsics_weight(), + encoded_len * (nonce + 1), + ); assert_eq!(>::extrinsic_index(), Some(nonce as u32 + 1)); } else { - assert_eq!(res, Err(ApplyError::FullBlock)); + assert_eq!(res, Err(InvalidTransaction::ExhaustsResources.into())); } } }); @@ -606,9 +552,9 @@ mod tests { assert_eq!(>::all_extrinsics_weight(), 0); assert_eq!(>::all_extrinsics_weight(), 0); - assert_eq!(Executive::apply_extrinsic(xt.clone()).unwrap(), ApplyOutcome::Success); - assert_eq!(Executive::apply_extrinsic(x1.clone()).unwrap(), ApplyOutcome::Success); - assert_eq!(Executive::apply_extrinsic(x2.clone()).unwrap(), ApplyOutcome::Success); + assert!(Executive::apply_extrinsic(xt.clone()).unwrap().is_ok()); + assert!(Executive::apply_extrinsic(x1.clone()).unwrap().is_ok()); + assert!(Executive::apply_extrinsic(x2.clone()).unwrap().is_ok()); // default weight for `TestXt` == encoded length. assert_eq!(>::all_extrinsics_weight(), (3 * len).into()); @@ -624,12 +570,18 @@ mod tests { #[test] fn validate_unsigned() { let xt = sr_primitives::testing::TestXt(None, Call::set_balance(33, 69, 69)); - let valid = TransactionValidity::Valid(Default::default()); let mut t = new_test_ext(1); with_externalities(&mut t, || { - assert_eq!(Executive::validate_transaction(xt.clone()), valid); - assert_eq!(Executive::apply_extrinsic(xt), Ok(ApplyOutcome::Fail)); + assert_eq!(Executive::validate_transaction(xt.clone()), Ok(Default::default())); + assert_eq!( + Executive::apply_extrinsic(xt), + Ok( + Err( + DispatchError { module: None, error: 0, message: Some("RequireRootOrigin") } + ) + ) + ); }); } @@ -657,11 +609,21 @@ mod tests { )); if lock == WithdrawReasons::except(WithdrawReason::TransactionPayment) { - assert_eq!(Executive::apply_extrinsic(xt).unwrap(), ApplyOutcome::Fail); + assert_eq!( + Executive::apply_extrinsic(xt).unwrap(), + Err(DispatchError { + module: None, + 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 - weight); } else { - assert_eq!(Executive::apply_extrinsic(xt), Err(ApplyError::CantPay)); + assert_eq!( + Executive::apply_extrinsic(xt), + Err(ApplyError::Validity(InvalidTransaction::Payment.into())), + ); assert_eq!(>::total_balance(&1), 111); } }); diff --git a/srml/grandpa/src/mock.rs b/srml/grandpa/src/mock.rs index 126aa0569af79..af2fedf42f8f9 100644 --- a/srml/grandpa/src/mock.rs +++ b/srml/grandpa/src/mock.rs @@ -37,9 +37,9 @@ pub fn grandpa_log(log: ConsensusLog) -> 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; - } parameter_types! { pub const BlockHashCount: u64 = 250; @@ -53,7 +53,7 @@ impl system::Trait for Test { type BlockNumber = u64; type Call = (); type Hash = H256; - type Hashing = ::sr_primitives::traits::BlakeTwo256; + type Hashing = sr_primitives::traits::BlakeTwo256; type AccountId = u64; type Lookup = IdentityLookup; type Header = Header; diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index d6ef6c2e9cb13..6106b1e45c153 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -74,9 +74,10 @@ use rstd::prelude::*; use session::historical::IdentificationTuple; use runtime_io::Printable; use sr_primitives::{ - Perbill, ApplyError, - traits::{Convert, Member}, - transaction_validity::{TransactionValidity, TransactionLongevity, ValidTransaction}, + traits::{Convert, Member}, Perbill, + transaction_validity::{ + TransactionValidity, TransactionLongevity, ValidTransaction, InvalidTransaction, + }, }; use sr_staking_primitives::{ SessionIndex, CurrentElectedSet, @@ -160,7 +161,7 @@ enum OffchainErr { } impl Printable for OffchainErr { - fn print(self) { + fn print(&self) { match self { OffchainErr::DecodeWorkerStatus => print("Offchain error: decoding WorkerStatus failed!"), OffchainErr::FailedSigning => print("Offchain error: signing failed!"), @@ -489,24 +490,24 @@ impl session::OneSessionHandler for Module { impl support::unsigned::ValidateUnsigned for Module { type Call = Call; - fn validate_unsigned(call: &Self::Call) -> support::unsigned::TransactionValidity { + fn validate_unsigned(call: &Self::Call) -> TransactionValidity { if let Call::heartbeat(heartbeat, signature) = call { if >::is_online_in_current_session(heartbeat.authority_index) { // we already received a heartbeat for this authority - return TransactionValidity::Invalid(ApplyError::Stale as i8); + return InvalidTransaction::Stale.into(); } // check if session index from heartbeat is recent let current_session = >::current_index(); if heartbeat.session_index != current_session { - return TransactionValidity::Invalid(ApplyError::Stale as i8); + return InvalidTransaction::Stale.into(); } // verify that the incoming (unverified) pubkey is actually an authority id let keys = Keys::::get(); let authority_id = match keys.get(heartbeat.authority_index as usize) { Some(id) => id, - None => return TransactionValidity::Invalid(ApplyError::BadSignature as i8), + None => return InvalidTransaction::BadProof.into(), }; // check signature (this is expensive so we do it last). @@ -515,19 +516,19 @@ impl support::unsigned::ValidateUnsigned for Module { }); if !signature_valid { - return TransactionValidity::Invalid(ApplyError::BadSignature as i8); + return InvalidTransaction::BadProof.into(); } - return TransactionValidity::Valid(ValidTransaction { + Ok(ValidTransaction { priority: 0, requires: vec![], provides: vec![(current_session, authority_id).encode()], longevity: TransactionLongevity::max_value(), propagate: true, }) + } else { + InvalidTransaction::Call.into() } - - TransactionValidity::Invalid(0) } } diff --git a/srml/indices/src/lib.rs b/srml/indices/src/lib.rs index 15921ffdf4c44..7b8e1c0461110 100644 --- a/srml/indices/src/lib.rs +++ b/srml/indices/src/lib.rs @@ -19,10 +19,10 @@ #![cfg_attr(not(feature = "std"), no_std)] -use rstd::{prelude::*, result, marker::PhantomData, convert::TryInto}; +use rstd::{prelude::*, marker::PhantomData, convert::TryInto}; use codec::{Encode, Codec}; use support::{StorageValue, StorageMap, Parameter, decl_module, decl_event, decl_storage}; -use sr_primitives::traits::{One, SimpleArithmetic, StaticLookup, Member}; +use sr_primitives::traits::{One, SimpleArithmetic, StaticLookup, Member, LookupError}; use system::{IsDeadAccount, OnNewAccount}; use self::address::Address as RawAddress; @@ -224,9 +224,11 @@ impl OnNewAccount for Module { impl StaticLookup for Module { type Source = address::Address; type Target = T::AccountId; - fn lookup(a: Self::Source) -> result::Result { - Self::lookup_address(a).ok_or("invalid account index") + + fn lookup(a: Self::Source) -> Result { + Self::lookup_address(a).ok_or(LookupError) } + fn unlookup(a: Self::Target) -> Self::Source { address::Address::Id(a) } diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs index 12cdd2f4e91f0..0f8bc5c09a16c 100644 --- a/srml/staking/src/lib.rs +++ b/srml/staking/src/lib.rs @@ -882,8 +882,8 @@ decl_module! { ensure!(!targets.is_empty(), "targets cannot be empty"); let targets = targets.into_iter() .take(MAX_NOMINATIONS) - .map(T::Lookup::lookup) - .collect::, &'static str>>()?; + .map(|t| T::Lookup::lookup(t)) + .collect::, _>>()?; >::remove(stash); >::insert(stash, targets); diff --git a/srml/staking/src/tests.rs b/srml/staking/src/tests.rs index c3f39222994d6..34d95b60f2c55 100644 --- a/srml/staking/src/tests.rs +++ b/srml/staking/src/tests.rs @@ -748,7 +748,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), + "account liquidity restrictions prevent withdrawal", + ); // Give account 11 extra free balance let _ = Balances::make_free_balance_be(&11, 10000); @@ -774,7 +777,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), + "account liquidity restrictions prevent withdrawal", + ); assert_ok!(Balances::transfer(Origin::signed(21), 20, 1000)); }); } diff --git a/srml/sudo/src/lib.rs b/srml/sudo/src/lib.rs index e933810237dd3..845f1e4104f49 100644 --- a/srml/sudo/src/lib.rs +++ b/srml/sudo/src/lib.rs @@ -87,12 +87,10 @@ #![cfg_attr(not(feature = "std"), no_std)] use rstd::prelude::*; -use sr_primitives::traits::StaticLookup; -use sr_primitives::weights::SimpleDispatchInfo; -use support::{ - StorageValue, Parameter, Dispatchable, decl_module, decl_event, - decl_storage, ensure +use sr_primitives::{ + traits::{StaticLookup, Dispatchable}, weights::SimpleDispatchInfo, DispatchError, }; +use support::{StorageValue, Parameter, decl_module, decl_event, decl_storage, ensure}; use system::ensure_signed; pub trait Trait: system::Trait { @@ -126,6 +124,7 @@ decl_module! { let res = match proposal.dispatch(system::RawOrigin::Root.into()) { Ok(_) => true, Err(e) => { + let e: DispatchError = e.into(); runtime_io::print(e); false } diff --git a/srml/support/src/dispatch.rs b/srml/support/src/dispatch.rs index 86c21da67bcab..916cffdc345d1 100644 --- a/srml/support/src/dispatch.rs +++ b/srml/support/src/dispatch.rs @@ -25,18 +25,18 @@ pub use srml_metadata::{ FunctionMetadata, DecodeDifferent, DecodeDifferentArray, FunctionArgumentMetadata, ModuleConstantMetadata, DefaultByte, DefaultByteGetter, }; -pub use sr_primitives::weights::{SimpleDispatchInfo, GetDispatchInfo, DispatchInfo, WeighData, - ClassifyDispatch, - TransactionPriority +pub use sr_primitives::{ + weights::{ + SimpleDispatchInfo, GetDispatchInfo, DispatchInfo, WeighData, ClassifyDispatch, + TransactionPriority + }, traits::{Dispatchable, DispatchResult, ModuleDispatchError}, DispatchError }; -pub use sr_primitives::traits::{Dispatchable, DispatchResult}; /// A type that cannot be instantiated. 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 = DispatchResult; +/// Result with string error message. This exists for backward compatibility purpose. +pub type Result = DispatchResult<&'static str>; /// Serializable version of Dispatchable. /// This value can be used as a "function" in an extrinsic. @@ -239,6 +239,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -270,6 +271,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -286,6 +288,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event() = default; @@ -301,6 +304,7 @@ macro_rules! decl_module { { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } + { $( $error_type )* } [ $( $dispatchables )* ] $($rest)* ); @@ -315,6 +319,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event @@ -334,6 +339,7 @@ macro_rules! decl_module { {} { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_finalize($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -349,6 +355,7 @@ macro_rules! decl_module { { fn on_finalize( $( $param_name : $param ),* ) { $( $impl )* } } { $( $offchain )* } { $( $constants )* } + { $( $error_type )* } [ $( $dispatchables )* ] $($rest)* ); @@ -363,6 +370,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_initialize($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -378,6 +386,7 @@ macro_rules! decl_module { { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } + { $( $error_type )* } [ $( $dispatchables )* ] $($rest)* ); @@ -395,6 +404,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn offchain_worker($($param_name:ident : $param:ty),* ) { $( $impl:tt )* } @@ -412,6 +422,7 @@ macro_rules! decl_module { { $( $on_finalize )* } { fn offchain_worker( $( $param_name : $param ),* ) { $( $impl )* } } { $( $constants )* } + { $( $error_type )* } [ $( $dispatchables )* ] $($rest)* ); @@ -431,6 +442,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $( #[doc = $doc_attr:tt] )* const $name:ident: $ty:ty = $value:expr; @@ -453,11 +465,85 @@ macro_rules! decl_module { $( #[doc = $doc_attr ] )* $name: $ty = $value; } + { $( $error_type )* } [ $( $dispatchables )* ] $($rest)* ); }; + // Parse error type + (@normalize + $(#[$attr:meta])* + 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 + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { } + [ $( $dispatchables: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$(, $instance: $instantiable $(= $module_default_instance)?)? + > + for enum $call_type where origin: $origin_type, system = $system + { $( $other_where_bounds )* } + { $( $deposit_event )* } + { $( $on_initialize )* } + { $( $on_finalize )* } + { $( $offchain )* } + { $( $constants )* } + { $error_type } + [ $( $dispatchables )* ] + $($rest)* + ); + }; + // Add default Error if none supplied + (@normalize + $(#[$attr:meta])* + 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 + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { } + [ $($t:tt)* ] + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type< + $trait_instance: $trait_name$(, $instance: $instantiable $(= $module_default_instance)?)? + > + for enum $call_type where origin: $origin_type, system = $system + { $( $other_where_bounds )* } + { $( $deposit_event )* } + { $( $on_initialize )* } + { $( $on_finalize )* } + { $( $offchain )* } + { $( $constants )* } + { &'static str } + [ $($t)* ] + $($rest)* + ); + }; + // This puts the function statement into the [], decreasing `$rest` and moving toward finishing the parse. (@normalize $(#[$attr:meta])* @@ -472,6 +558,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $error_type:ty } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -492,6 +579,7 @@ macro_rules! decl_module { { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } + { $error_type } [ $( $dispatchables )* $(#[doc = $doc_attr])* @@ -518,6 +606,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $fn_vis:vis fn $fn_name:ident( @@ -537,6 +626,7 @@ macro_rules! decl_module { { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } + { $( $error_type )* } [ $( $dispatchables )* ] $(#[doc = $doc_attr])* #[weight = $crate::dispatch::SimpleDispatchInfo::default()] @@ -557,6 +647,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -581,6 +672,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -605,6 +697,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -630,6 +723,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $( $error_type:tt )* } [ $( $dispatchables:tt )* ] ) => { $crate::decl_module!(@imp @@ -644,6 +738,7 @@ macro_rules! decl_module { { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } + { $( $error_type )* } ); }; @@ -794,6 +889,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 ( @@ -804,7 +900,7 @@ macro_rules! decl_module { #[allow(unreachable_code)] $vis fn $name( $origin: $origin_ty $(, $param: $param_ty )* - ) -> $crate::dispatch::Result { + ) -> $crate::dispatch::DispatchResult<$error_type> { { $( $impl )* } // May be unreachable. Ok(()) @@ -815,6 +911,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 ( @@ -969,6 +1066,7 @@ macro_rules! decl_module { { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } + { $error_type:ty } ) => { $crate::__check_reserved_fn_name! { $( $fn_name )* } @@ -1020,6 +1118,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 ( @@ -1150,7 +1249,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::DispatchResult { match self { $( $call_type::$fn_name( $( $param_name ),* ) => { @@ -1177,8 +1277,8 @@ macro_rules! decl_module { #[doc(hidden)] pub fn dispatch>( d: D, - origin: D::Origin, - ) -> $crate::dispatch::Result { + origin: D::Origin + ) -> $crate::dispatch::DispatchResult { d.dispatch(origin) } } @@ -1234,9 +1334,19 @@ 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 { - match self { - $( $call_type::$camelcase(call) => call.dispatch(origin), )* + type Error = $crate::dispatch::DispatchError; + fn dispatch( + self, + origin: $origin, + ) -> $crate::dispatch::DispatchResult<$crate::dispatch::DispatchError> { + $crate::impl_outer_dispatch! { + @DISPATCH_MATCH + self + $call_type + origin + {} + 0; + $( $camelcase ),* } } } @@ -1258,6 +1368,43 @@ macro_rules! impl_outer_dispatch { } } )* + }; + (@DISPATCH_MATCH + $self:ident + $call_type:ident + $origin:ident + { $( $generated:tt )* } + $index:expr; + $name:ident + $( , $rest:ident )* + ) => { + $crate::impl_outer_dispatch! { + @DISPATCH_MATCH + $self + $call_type + $origin + { + $( $generated )* + $call_type::$name(call) => call.dispatch($origin).map_err(|e| { + let mut error: $crate::dispatch::DispatchError = e.into(); + error.module = Some($index); + error + }), + } + $index + 1; + $( $rest ),* + } + }; + (@DISPATCH_MATCH + $self:ident + $call_type:ident + $origin:ident + { $( $generated:tt )* } + $index:expr; + ) => { + match $self { + $( $generated )* + } } } diff --git a/srml/support/src/error.rs b/srml/support/src/error.rs new file mode 100644 index 0000000000000..2f9d9379bfd04 --- /dev/null +++ b/srml/support/src/error.rs @@ -0,0 +1,151 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Macro for declaring a module error. + +#[doc(hidden)] +pub use sr_primitives::traits::LookupError; + +/// Declare an error type for a runtime module. +/// +/// The generated error type inherently has the variants `Other` and `CannotLookup`. `Other` can +/// hold any `&'static str` error message and is present for convenience/backward compatibility. +/// The `CannotLookup` variant indicates that some lookup could not be done. For both variants the +/// error type implements `From<&'static str>` and `From` to make them usable with the +/// try operator. +/// +/// # Usage +/// +/// ``` +/// # use srml_support::decl_error; +/// decl_error! { +/// /// Errors that can occur in my module. +/// pub enum MyError { +/// /// Hey this is an error message that indicates bla. +/// MyCoolErrorMessage, +/// /// You are just not cool enough for my module! +/// YouAreNotCoolEnough, +/// } +/// } +/// ``` +/// +/// `decl_error!` supports only variants that do not hold any data. +#[macro_export] +macro_rules! decl_error { + ( + $(#[$attr:meta])* + pub enum $error:ident { + $( + $( #[$variant_attr:meta] )* + $name:ident + ),* + $(,)? + } + ) => { + #[derive(Clone, PartialEq, Eq)] + #[cfg_attr(feature = "std", derive(Debug))] + $(#[$attr])* + pub enum $error { + Other(&'static str), + CannotLookup, + $( + $(#[$variant_attr])* + $name + ),* + } + + impl $crate::dispatch::ModuleDispatchError for $error { + fn as_u8(&self) -> u8 { + $crate::decl_error! { + @GENERATE_AS_U8 + self + $error + {} + 2, + $( $name ),* + } + } + + fn as_str(&self) -> &'static str { + match self { + $error::Other(err) => err, + $error::CannotLookup => "Can not lookup", + $( + $error::$name => stringify!($name), + )* + } + } + } + + impl From<&'static str> for $error { + fn from(val: &'static str) -> $error { + $error::Other(val) + } + } + + impl From<$crate::error::LookupError> for $error { + fn from(_: $crate::error::LookupError) -> $error { + $error::CannotLookup + } + } + + impl From<$error> for &'static str { + fn from(err: $error) -> &'static str { + use $crate::dispatch::ModuleDispatchError; + err.as_str() + } + } + + impl Into<$crate::dispatch::DispatchError> for $error { + fn into(self) -> $crate::dispatch::DispatchError { + use $crate::dispatch::ModuleDispatchError; + $crate::dispatch::DispatchError::new(None, self.as_u8(), Some(self.as_str())) + } + } + }; + (@GENERATE_AS_U8 + $self:ident + $error:ident + { $( $generated:tt )* } + $index:expr, + $name:ident + $( , $rest:ident )* + ) => { + $crate::decl_error! { + @GENERATE_AS_U8 + $self + $error + { + $( $generated )* + $error::$name => $index, + } + $index + 1, + $( $rest ),* + } + }; + (@GENERATE_AS_U8 + $self:ident + $error:ident + { $( $generated:tt )* } + $index:expr, + ) => { + match $self { + $error::Other(_) => 0, + $error::CannotLookup => 1, + $( $generated )* + } + } +} diff --git a/srml/support/src/lib.rs b/srml/support/src/lib.rs index e7386930b9108..b815e00b9df23 100644 --- a/srml/support/src/lib.rs +++ b/srml/support/src/lib.rs @@ -58,14 +58,16 @@ mod runtime; pub mod inherent; #[macro_use] pub mod unsigned; +#[macro_use] +pub mod error; mod double_map; pub mod traits; pub use self::storage::{StorageValue, StorageMap, StorageLinkedMap, StorageDoubleMap}; pub use self::hashable::Hashable; -pub use self::dispatch::{Parameter, Dispatchable, Callable, IsSubType}; +pub use self::dispatch::{Parameter, Callable, IsSubType}; pub use self::double_map::StorageDoubleMapWithHasher; -pub use runtime_io::{print, storage_root}; +pub use runtime_io::{print, storage_root, Printable}; pub use sr_primitives::{self, ConsensusEngineId}; /// Macro for easily creating a new implementation of the `Get` trait. Use similarly to @@ -128,7 +130,7 @@ macro_rules! fail { /// Used as `ensure!(expression_to_ensure, expression_to_return_on_false)`. #[macro_export] macro_rules! ensure { - ( $x:expr, $y:expr ) => {{ + ( $x:expr, $y:expr $(,)? ) => {{ if !$x { $crate::fail!($y); } @@ -142,7 +144,10 @@ macro_rules! ensure { #[macro_export] #[cfg(feature = "std")] macro_rules! assert_noop { - ( $x:expr , $y:expr ) => { + ( + $x:expr, + $y:expr $(,)? + ) => { let h = $crate::storage_root(); $crate::assert_err!($x, $y); assert_eq!(h, $crate::storage_root()); @@ -159,7 +164,7 @@ macro_rules! assert_noop { #[macro_export] #[cfg(feature = "std")] macro_rules! assert_err { - ( $x:expr , $y:expr ) => { + ( $x:expr , $y:expr $(,)? ) => { assert_eq!($x, Err($y)); } } diff --git a/srml/support/src/unsigned.rs b/srml/support/src/unsigned.rs index 1c62dd0c58435..4d2ceddd79f4a 100644 --- a/srml/support/src/unsigned.rs +++ b/srml/support/src/unsigned.rs @@ -17,7 +17,7 @@ #[doc(hidden)] pub use crate::sr_primitives::traits::ValidateUnsigned; #[doc(hidden)] -pub use crate::sr_primitives::transaction_validity::TransactionValidity; +pub use crate::sr_primitives::transaction_validity::{TransactionValidity, UnknownTransaction}; #[doc(hidden)] pub use crate::sr_primitives::ApplyError; @@ -72,7 +72,7 @@ macro_rules! impl_outer_validate_unsigned { #[allow(unreachable_patterns)] match call { $( Call::$module(inner_call) => $module::validate_unsigned(inner_call), )* - _ => $crate::unsigned::TransactionValidity::Invalid($crate::unsigned::ApplyError::BadSignature as i8), + _ => $crate::unsigned::UnknownTransaction::NoUnsignedValidator.into(), } } } @@ -81,8 +81,7 @@ macro_rules! impl_outer_validate_unsigned { #[cfg(test)] mod test_empty_call { - pub enum Call { - } + pub enum Call {} #[allow(unused)] pub struct Runtime; diff --git a/srml/support/test/tests/system.rs b/srml/support/test/tests/system.rs index afeb0e0195b17..344a2bec730f1 100644 --- a/srml/support/test/tests/system.rs +++ b/srml/support/test/tests/system.rs @@ -27,6 +27,13 @@ support::decl_event!( } ); +support::decl_error! { + pub enum Error { + TestError, + AnotherError + } +} + /// Origin for the system module. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index dab0c11fa5e1c..2f1d0ab9e0e20 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -95,22 +95,24 @@ use rstd::prelude::*; use rstd::map; use rstd::marker::PhantomData; use sr_version::RuntimeVersion; -use sr_primitives::generic::{self, Era}; -use sr_primitives::Perbill; -use sr_primitives::weights::{ - Weight, DispatchInfo, DispatchClass, WeightMultiplier, SimpleDispatchInfo -}; -use sr_primitives::transaction_validity::{ - ValidTransaction, TransactionPriority, TransactionLongevity -}; -use sr_primitives::traits::{self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, Convert, - SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, DispatchError, SaturatedConversion, - MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, Lookup, +use sr_primitives::{ + generic::{self, Era}, Perbill, ApplyError, ApplyOutcome, DispatchError, + weights::{Weight, DispatchInfo, DispatchClass, WeightMultiplier, SimpleDispatchInfo}, + transaction_validity::{ + ValidTransaction, TransactionPriority, TransactionLongevity, TransactionValidityError, + InvalidTransaction, TransactionValidity, + }, + traits::{ + self, CheckEqual, SimpleArithmetic, Zero, SignedExtension, Convert, Lookup, LookupError, + SimpleBitOps, Hash, Member, MaybeDisplay, EnsureOrigin, SaturatedConversion, + MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup, One, Bounded, + }, }; + use primitives::storage::well_known_keys; use support::{ storage, decl_module, decl_event, decl_storage, StorageDoubleMap, StorageValue, StorageMap, - Parameter, for_each_tuple, traits::{Contains, Get} + Parameter, for_each_tuple, traits::{Contains, Get}, decl_error, }; use safe_mix::TripletMix; use codec::{Encode, Decode}; @@ -248,6 +250,8 @@ pub type KeyValue = (Vec, Vec); decl_module! { pub struct Module for enum Call where origin: T::Origin { + type Error = Error; + /// A big dispatch that will disallow any other transaction to be included. // TODO: this must be preferable available for testing really (not possible at the moment). #[weight = SimpleDispatchInfo::MaxOperational] @@ -323,10 +327,21 @@ decl_event!( /// An extrinsic completed successfully. ExtrinsicSuccess, /// An extrinsic failed. - ExtrinsicFailed, + ExtrinsicFailed(DispatchError), } ); +decl_error! { + /// Error for the System module + pub enum Error { + BadSignature, + BlockFull, + RequireSignedOrigin, + RequireRootOrigin, + RequireNoOrigin, + } +} + /// Origin for the System module. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] @@ -510,32 +525,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), } } @@ -811,11 +826,13 @@ 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(r: &ApplyOutcome, _encoded_len: u32) { + Self::deposit_event( + match r { + Ok(()) => Event::ExtrinsicSuccess, + Err(err) => Event::ExtrinsicFailed(err.clone()), + } + ); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; @@ -844,7 +861,6 @@ impl Module { pub struct CheckWeight(PhantomData); impl CheckWeight { - /// Get the quota ratio of each dispatch class type. This indicates that all operational /// dispatches can use the full capacity of any resource, while user-triggered ones can consume /// a portion. @@ -859,31 +875,33 @@ impl CheckWeight { /// Checks if the current extrinsic can fit into the block with respect to block weight limits. /// /// Upon successes, it returns the new block weight as a `Result`. - fn check_weight(info: DispatchInfo) -> Result { + fn check_weight(info: DispatchInfo) -> Result { let current_weight = Module::::all_extrinsics_weight(); let maximum_weight = T::MaximumBlockWeight::get(); let limit = Self::get_dispatch_limit_ratio(info.class) * maximum_weight; let added_weight = info.weight.min(limit); let next_weight = current_weight.saturating_add(added_weight); if next_weight > limit { - return Err(DispatchError::Exhausted) + Err(InvalidTransaction::ExhaustsResources.into()) + } else { + Ok(next_weight) } - Ok(next_weight) } /// Checks if the current extrinsic can fit into the block with respect to block length limits. /// /// Upon successes, it returns the new block length as a `Result`. - fn check_block_length(info: DispatchInfo, len: usize) -> Result { + fn check_block_length(info: DispatchInfo, len: usize) -> Result { let current_len = Module::::all_extrinsics_len(); let maximum_len = T::MaximumBlockLength::get(); let limit = Self::get_dispatch_limit_ratio(info.class) * maximum_len; let added_len = len as u32; let next_len = current_len.saturating_add(added_len); if next_len > limit { - return Err(DispatchError::Exhausted) + Err(InvalidTransaction::ExhaustsResources.into()) + } else { + Ok(next_len) } - Ok(next_len) } /// get the priority of an extrinsic denoted by `info`. @@ -906,7 +924,7 @@ impl SignedExtension for CheckWeight { type AdditionalSigned = (); type Pre = (); - fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } fn pre_dispatch( self, @@ -914,7 +932,7 @@ impl SignedExtension for CheckWeight { _call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result<(), DispatchError> { + ) -> Result<(), ApplyError> { let next_len = Self::check_block_length(info, len)?; AllExtrinsicsLen::put(next_len); let next_weight = Self::check_weight(info)?; @@ -928,12 +946,18 @@ impl SignedExtension for CheckWeight { _call: &Self::Call, info: DispatchInfo, len: usize, - ) -> Result { + ) -> TransactionValidity { // There is no point in writing to storage here since changes are discarded. This basically - // discards any transaction which is bigger than the length or weight limit **alone**,which + // discards any transaction which is bigger than the length or weight limit **alone**, which // is a guarantee that it will fail in the pre-dispatch phase. - let _ = Self::check_block_length(info, len)?; - let _ = Self::check_weight(info)?; + if let Err(e) = Self::check_block_length(info, len) { + return Err(e); + } + + if let Err(e) = Self::check_weight(info) { + return Err(e); + } + Ok(ValidTransaction { priority: Self::get_priority(info), ..Default::default() }) } } @@ -969,7 +993,7 @@ impl SignedExtension for CheckNonce { type AdditionalSigned = (); type Pre = (); - fn additional_signed(&self) -> rstd::result::Result<(), &'static str> { Ok(()) } + fn additional_signed(&self) -> rstd::result::Result<(), TransactionValidityError> { Ok(()) } fn pre_dispatch( self, @@ -977,13 +1001,18 @@ impl SignedExtension for CheckNonce { _call: &Self::Call, _info: DispatchInfo, _len: usize, - ) -> Result<(), DispatchError> { + ) -> Result<(), ApplyError> { let expected = >::get(who); if self.0 != expected { return Err( - if self.0 < expected { DispatchError::Stale } else { DispatchError::Future } + if self.0 < expected { + InvalidTransaction::Stale + } else { + InvalidTransaction::Future + }.into() ) } + >::insert(who, expected + T::Index::one()); Ok(()) } @@ -994,11 +1023,11 @@ impl SignedExtension for CheckNonce { _call: &Self::Call, info: DispatchInfo, _len: usize, - ) -> Result { + ) -> TransactionValidity { // check index let expected = >::get(who); if self.0 < expected { - return Err(DispatchError::Stale) + return InvalidTransaction::Stale.into() } let provides = vec![Encode::encode(&(who, self.0))]; @@ -1048,7 +1077,7 @@ impl SignedExtension for CheckEra { _call: &Self::Call, _info: DispatchInfo, _len: usize, - ) -> Result { + ) -> TransactionValidity { let current_u64 = >::block_number().saturated_into::(); let valid_till = (self.0).0.death(current_u64); Ok(ValidTransaction { @@ -1057,11 +1086,14 @@ impl SignedExtension for CheckEra { }) } - fn additional_signed(&self) -> Result { + fn additional_signed(&self) -> Result { let current_u64 = >::block_number().saturated_into::(); let n = (self.0).0.birth(current_u64).saturated_into::(); - if !>::exists(n) { Err("transaction birth block ancient")? } - Ok(>::block_hash(n)) + if !>::exists(n) { + Err(InvalidTransaction::AncientBirthBlock.into()) + } else { + Ok(>::block_hash(n)) + } } } @@ -1089,7 +1121,7 @@ impl SignedExtension for CheckGenesis { type AdditionalSigned = T::Hash; type Pre = (); - fn additional_signed(&self) -> Result { + fn additional_signed(&self) -> Result { Ok(>::block_hash(T::BlockNumber::zero())) } } @@ -1118,7 +1150,7 @@ impl SignedExtension for CheckVersion { type AdditionalSigned = u32; type Pre = (); - fn additional_signed(&self) -> Result { + fn additional_signed(&self) -> Result { Ok(>::runtime_version().spec_version) } } @@ -1133,7 +1165,8 @@ impl Default for ChainContext { impl Lookup for ChainContext { type Source = ::Source; type Target = ::Target; - fn lookup(&self, s: Self::Source) -> rstd::result::Result { + + fn lookup(&self, s: Self::Source) -> Result { ::lookup(s) } } @@ -1143,10 +1176,10 @@ mod tests { use super::*; use runtime_io::with_externalities; use primitives::H256; - use sr_primitives::{traits::{BlakeTwo256, IdentityLookup}, testing::Header}; + use sr_primitives::{traits::{BlakeTwo256, IdentityLookup}, testing::Header, DispatchError}; use support::{impl_outer_origin, parameter_types}; - impl_outer_origin!{ + impl_outer_origin! { pub enum Origin for Test where system = super {} } @@ -1183,7 +1216,7 @@ mod tests { fn from(e: Event) -> u16 { match e { Event::ExtrinsicSuccess => 100, - Event::ExtrinsicFailed => 101, + Event::ExtrinsicFailed(_) => 101, } } } @@ -1232,16 +1265,19 @@ 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(&Err(DispatchError::new(Some(1), 2, None)), 0); System::note_finished_extrinsics(); System::deposit_event(3u16); System::finalize(); - 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::Finalization, event: 3u16, topics: vec![] } - ]); + 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::Finalization, event: 3u16, topics: vec![] } + ] + ); }); } @@ -1440,14 +1476,17 @@ mod tests { let op = DispatchInfo { weight: 100, class: DispatchClass::Operational }; let len = 0_usize; - assert_eq!( - CheckWeight::(PhantomData).validate(&1, CALL, normal, len).unwrap().priority, - 100, - ); - assert_eq!( - CheckWeight::(PhantomData).validate(&1, CALL, op, len).unwrap().priority, - Bounded::max_value(), - ); + let priority = CheckWeight::(PhantomData) + .validate(&1, CALL, normal, len) + .unwrap() + .priority; + assert_eq!(priority, 100); + + let priority = CheckWeight::(PhantomData) + .validate(&1, CALL, op, len) + .unwrap() + .priority; + assert_eq!(priority, Bounded::max_value()); }) } @@ -1481,7 +1520,7 @@ mod tests { // future assert_eq!( CheckEra::::from(Era::mortal(4, 2)).additional_signed().err().unwrap(), - "transaction birth block ancient" + InvalidTransaction::AncientBirthBlock.into(), ); // correct @@ -1503,10 +1542,7 @@ mod tests { System::set_block_number(17); >::insert(16, H256::repeat_byte(1)); - assert_eq!( - ext.validate(&1, CALL, normal, len).unwrap().longevity, - 15, - ); + assert_eq!(ext.validate(&1, CALL, normal, len).unwrap().longevity, 15); }) } } diff --git a/subkey/src/main.rs b/subkey/src/main.rs index e0ac2cb8b6b0f..a68674513ad4d 100644 --- a/subkey/src/main.rs +++ b/subkey/src/main.rs @@ -103,6 +103,7 @@ fn execute(matches: clap::ArgMatches) where system::CheckNonce::::from(i), system::CheckWeight::::new(), balances::TakeFees::::from(f), + Default::default(), ) }; let password = matches.value_of("password"); @@ -185,7 +186,7 @@ fn execute(matches: clap::ArgMatches) where let raw_payload = SignedPayload::from_raw( function, extra(index, 0), - (VERSION.spec_version as u32, genesis_hash, genesis_hash, (), (), ()), + (VERSION.spec_version as u32, genesis_hash, genesis_hash, (), (), (), ()), ); let signature = raw_payload.using_encoded(|payload| { println!("Signing {}", HexDisplay::from(&payload));