Last round of fix ups

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,15 +171,15 @@ impl<Block, B, E, RA, A> Proposer<Block, SubstrateClient<B, E, Block, RA>, A>	wh
 				Ok(()) => {
 					debug!("[{:?}] Pushed to the block.", pending.hash);
 				}
-				Err(error::Error::ApplyExtrinsicFailed(ApplyError::FullBlock)) => {
+				Err(error::Error::ApplyExtrinsicFailed(e)) if e.exhaust_resources() => {
 					if is_first {
 						debug!("[{:?}] Invalid transaction: FullBlock on empty block", pending.hash);
 						unqueue_invalid.push(pending.hash.clone());
 					} else if skipped < MAX_SKIPPED_TRANSACTIONS {
 						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.");

core/rpc/src/author/error.rs

@@ -99,15 +99,15 @@ impl From<Error> 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::UnknownTransactionValidity(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),
@@ -131,7 +131,7 @@ impl From<Error> 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()),
 			},
 			e => errors::internal(e),

core/sr-primitives/src/generic/unchecked_extrinsic.rs

@@ -25,7 +25,9 @@ use codec::{Decode, Encode, Input, Error};
 use crate::{
 	traits::{self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic},
 	generic::CheckedExtrinsic,
-	transaction_validity::{TransactionValidityError, InvalidTransactionValidity},
+	transaction_validity::{
+		TransactionValidityError, InvalidTransactionValidity, UnknownTransactionValidity,
+	},
 };
 
 const TRANSACTION_VERSION: u8 = 3;
@@ -95,7 +97,6 @@ where
 	Extra: SignedExtension<AccountId=AccountId>,
 	AccountId: Member + MaybeDisplay,
 	Lookup: traits::Lookup<Source=Address, Target=AccountId>,
-	TransactionValidityError: From<Lookup::Error>,
 {
 	type Checked = CheckedExtrinsic<AccountId, Call, Extra>;
 
@@ -104,7 +105,7 @@ where
 			Some((signed, signature, extra)) => {
 				let additional_signed = extra.additional_signed()?;
 				let raw_payload = (self.function, extra, additional_signed);
-				let signed = lookup.lookup(signed)?;
+				let signed = lookup.lookup(signed).ok_or(UnknownTransactionValidity::InvalidIndex)?;
 				if !raw_payload.using_encoded(|payload| {
 					if payload.len() > 256 {
 						signature.verify(&blake2_256(payload)[..], &signed)
@@ -214,16 +215,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;
-		type Error = TransactionValidityError;
-		fn lookup(&self, s: u64) -> Result<u64, TransactionValidityError> { Ok(s) }
-	}
+	type TestContext = IdentityLookup<u64>;
 
 	#[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)]
 	struct TestSig(u64, Vec<u8>);
@@ -290,7 +285,7 @@ mod tests {
 	fn unsigned_check_should_work() {
 		let ux = Ex::new_unsigned(vec![0u8; 0]);
 		assert!(!ux.is_signed().unwrap_or(false));
-		assert!(<Ex as Checkable<TestContext>>::check(ux, &TestContext).is_ok());
+		assert!(<Ex as Checkable<TestContext>>::check(ux, &Default::default()).is_ok());
 	}
 
 	#[test]
@@ -303,7 +298,7 @@ mod tests {
 		);
 		assert!(ux.is_signed().unwrap_or(false));
 		assert_eq!(
-			<Ex as Checkable<TestContext>>::check(ux, &TestContext),
+			<Ex as Checkable<TestContext>>::check(ux, &Default::default()),
 			Err(InvalidTransactionValidity::BadProof.into()),
 		);
 	}
@@ -318,7 +313,7 @@ mod tests {
 		);
 		assert!(ux.is_signed().unwrap_or(false));
 		assert_eq!(
-			<Ex as Checkable<TestContext>>::check(ux, &TestContext),
+			<Ex as Checkable<TestContext>>::check(ux, &Default::default()),
 			Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }),
 		);
 	}

core/sr-primitives/src/lib.rs

@@ -621,19 +621,25 @@ pub enum ApplyError {
 	/// General error to do with the state of the system in general.
 	BadState,
 
-	/// General error to do with the exhaustion of block resources.
-	Exhausted,
-
 	/// Any error to do with the transaction validity.
 	Validity(transaction_validity::TransactionValidityError),
 }
 
+impl ApplyError {
+	/// Returns if the reason for the error was block resource exhaustion.
+	pub fn exhaust_resources(&self) -> bool {
+		match self {
+			Self::Validity(e) => e.exhaust_resources(),
+			_ => false,
+		}
+	}
+}
+
 impl Into<&'static str> for ApplyError {
 	fn into(self) -> &'static str {
 		match self {
 			ApplyError::NoPermission => "Transaction does not have required permissions",
 			ApplyError::BadState => "System in bad state",
-			ApplyError::Exhausted => "Transaction exhausted block limits",
 			ApplyError::Validity(v) => v.into(),
 		}
 	}

core/sr-primitives/src/traits.rs

@@ -93,44 +93,36 @@ impl<
 	}
 }
 
-/// EnsureOrigin Error type
-pub trait EnsureOriginError {
-	/// Indicates invalid origin
-	fn invalid_origin() -> Self;
+/// An error type that indicates that the origin is invalid.
+#[derive(Encode, Decode)]
+pub struct InvalidOrigin;
+
+impl From<InvalidOrigin> 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<OuterOrigin> {
 	/// A return type.
 	type Success;
-	/// Error type
-	type Error: EnsureOriginError;
 	/// Perform the origin check.
-	fn ensure_origin(o: OuterOrigin) -> result::Result<Self::Success, Self::Error> {
-		Self::try_origin(o).map_err(|_| Self::Error::invalid_origin())
+	fn ensure_origin(o: OuterOrigin) -> result::Result<Self::Success, InvalidOrigin> {
+		Self::try_origin(o).map_err(|_| InvalidOrigin)
 	}
 	/// Perform the origin check.
 	fn try_origin(o: OuterOrigin) -> result::Result<Self::Success, OuterOrigin>;
 }
 
-impl EnsureOriginError for () {
-	fn invalid_origin() -> () {}
-}
-
-impl EnsureOriginError for &'static str {
-	fn invalid_origin() -> &'static str { "Invalid origin" }
-}
-
 /// Means of changing one type into another in a manner dependent on the source type.
 pub trait Lookup {
 	/// Type to lookup from.
 	type Source;
 	/// Type to lookup into.
 	type Target;
-	/// Error type
-	type Error;
 	/// Attempt a lookup.
-	fn lookup(&self, s: Self::Source) -> result::Result<Self::Target, Self::Error>;
+	fn lookup(&self, s: Self::Source) -> Option<Self::Target>;
 }
 
 /// Means of changing one type into another in a manner dependent on the source type.
@@ -141,10 +133,8 @@ pub trait StaticLookup {
 	type Source: Codec + Clone + PartialEq + MaybeDebug;
 	/// Type to lookup into.
 	type Target;
-	/// Error type.
-	type Error;
 	/// Attempt a lookup.
-	fn lookup(s: Self::Source) -> result::Result<Self::Target, Self::Error>;
+	fn lookup(s: Self::Source) -> Option<Self::Target>;
 	/// Convert from Target back to Source.
 	fn unlookup(t: Self::Target) -> Self::Source;
 }
@@ -155,15 +145,14 @@ pub struct IdentityLookup<T>(PhantomData<T>);
 impl<T: Codec + Clone + PartialEq + MaybeDebug> StaticLookup for IdentityLookup<T> {
 	type Source = T;
 	type Target = T;
-	type Error = &'static str;
-	fn lookup(x: T) -> result::Result<T, Self::Error> { Ok(x) }
+	fn lookup(x: T) -> Option<T> { Some(x) }
 	fn unlookup(x: T) -> T { x }
 }
+
 impl<T> Lookup for IdentityLookup<T> {
 	type Source = T;
 	type Target = T;
-	type Error = &'static str;
-	fn lookup(&self, x: T) -> result::Result<T, Self::Error> { Ok(x) }
+	fn lookup(&self, x: T) -> Option<T> { Some(x) }
 }
 
 /// Extensible conversion trait. Generic over both source and destination types.

core/sr-primitives/src/transaction_validity.rs

@@ -33,6 +33,8 @@ pub type TransactionTag = Vec<u8>;
 #[derive(Clone, PartialEq, Eq, Encode, Decode, Copy)]
 #[cfg_attr(feature = "std", derive(Debug, serde::Serialize))]
 pub enum InvalidTransactionValidity {
+	/// 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).
@@ -49,9 +51,20 @@ pub enum InvalidTransactionValidity {
 	Custom(u8),
 }
 
+impl InvalidTransactionValidity {
+	/// Returns if the reason for the invalidity was block resource exhaustion.
+	pub fn exhaust_resources(&self) -> bool {
+		match self {
+			Self::ExhaustResources => true,
+			_ => false,
+		}
+	}
+}
+
 impl Into<&'static str> for InvalidTransactionValidity {
 	fn into(self) -> &'static str {
 		match self {
+			InvalidTransactionValidity::Call => "Transaction call is not expected",
 			InvalidTransactionValidity::Future => "Transaction will be valid in the future",
 			InvalidTransactionValidity::Stale => "Transaction is outdated",
 			InvalidTransactionValidity::BadProof => "Transaction has a bad signature",
@@ -99,6 +112,16 @@ pub enum TransactionValidityError {
 	Unknown(UnknownTransactionValidity),
 }
 
+impl TransactionValidityError {
+	/// Returns if the reason for the error was block resource exhaustion.
+	pub fn exhaust_resources(&self) -> bool {
+		match self {
+			Self::Invalid(e) => e.exhaust_resources(),
+			Self::Unknown(_) => false,
+		}
+	}
+}
+
 impl Into<&'static str> for TransactionValidityError {
 	fn into(self) -> &'static str {
 		match self {
@@ -108,27 +131,27 @@ impl Into<&'static str> for TransactionValidityError {
 	}
 }
 
-impl Into<TransactionValidityError> for InvalidTransactionValidity {
-	fn into(self) -> TransactionValidityError {
-		TransactionValidityError::Invalid(self)
+impl From<InvalidTransactionValidity> for TransactionValidityError {
+	fn from(err: InvalidTransactionValidity) -> Self {
+		TransactionValidityError::Invalid(err)
 	}
 }
 
-impl Into<TransactionValidityError> for UnknownTransactionValidity {
-	fn into(self) -> TransactionValidityError {
-		TransactionValidityError::Unknown(self)
+impl From<UnknownTransactionValidity> for TransactionValidityError {
+	fn from(err: UnknownTransactionValidity) -> Self {
+		TransactionValidityError::Unknown(err)
 	}
 }
 
 impl Into<crate::ApplyError> for InvalidTransactionValidity {
 	fn into(self) -> crate::ApplyError {
-		TransactionValidityError::from(self.into()).into()
+		TransactionValidityError::from(self).into()
 	}
 }
 
 impl Into<crate::ApplyError> for UnknownTransactionValidity {
 	fn into(self) -> crate::ApplyError {
-		TransactionValidityError::from(self.into()).into()
+		TransactionValidityError::from(self).into()
 	}
 }