Allow runtime to return transaction validation error codes (#1534)

* Allow runtime to return more detailed transaction validation errors.

* Re-use ApplyError codes and update test-runtime.

* Fix pool tests.

* Revert using Compact for validity.

substrate/core/client/db/src/lib.rs

@@ -49,10 +49,10 @@ use runtime_primitives::BuildStorage;
 use state_machine::backend::Backend as StateBackend;
 use executor::RuntimeInfo;
 use state_machine::{CodeExecutor, DBValue, ExecutionStrategy};
-use utils::{Meta, db_err, meta_keys, open_database, read_db, block_id_to_lookup_key, read_meta};
+use crate::utils::{Meta, db_err, meta_keys, open_database, read_db, block_id_to_lookup_key, read_meta};
 use client::LeafSet;
 use state_db::StateDb;
-use storage_cache::{CachingState, SharedCache, new_shared_cache};
+use crate::storage_cache::{CachingState, SharedCache, new_shared_cache};
 use log::{trace, debug, warn};
 pub use state_db::PruningMode;
 

substrate/core/sr-primitives/src/transaction_validity.rs

@@ -33,7 +33,7 @@ pub type TransactionTag = Vec<u8>;
 #[cfg_attr(feature = "std", derive(Debug))]
 pub enum TransactionValidity {
 	/// Transaction is invalid. Details are described by the error code.
-	Invalid,
+	Invalid(i8),
 	/// Transaction is valid.
 	Valid {
 		/// Priority of the transaction.
@@ -60,5 +60,5 @@ pub enum TransactionValidity {
 		longevity: TransactionLongevity,
 	},
 	/// Transaction validity can't be determined.
-	Unknown,
+	Unknown(i8),
 }

substrate/core/test-runtime/src/system.rs

@@ -107,17 +107,17 @@ pub fn execute_block(block: Block) {
 /// 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;
+		return TransactionValidity::Invalid(ApplyError::BadSignature as i8);
 	}
 
 	let tx = utx.transfer();
 	let nonce_key = tx.from.to_keyed_vec(NONCE_OF);
 	let expected_nonce: u64 = storage::get_or(&nonce_key, 0);
 	if tx.nonce < expected_nonce {
-		return TransactionValidity::Invalid;
+		return TransactionValidity::Invalid(ApplyError::Stale as i8);
 	}
 	if tx.nonce > expected_nonce + 64 {
-		return TransactionValidity::Unknown;
+		return TransactionValidity::Unknown(ApplyError::Future as i8);
 	}
 
 	let hash = |from: &AccountId, nonce: u64| {
@@ -139,7 +139,7 @@ pub fn validate_transaction(utx: Extrinsic) -> TransactionValidity {
 		priority: tx.amount,
 		requires,
 		provides,
-		longevity: 64
+		longevity: 64,
 	}
 }
 
@@ -393,7 +393,7 @@ mod tests {
 			});
 		});
 	}
-	
+
 	#[test]
 	fn block_import_with_transaction_works_wasm() {
 		block_import_with_transaction_works(|b, ext| {

substrate/core/transaction-pool/graph/src/error.rs

@@ -24,14 +24,14 @@ use error_chain::{
 error_chain! {
 	errors {
 		/// Transaction is not verifiable yet, but might be in the future.
-		UnknownTransactionValidity {
+		UnknownTransactionValidity(e: i8) {
 			description("Runtime cannot determine validity of the transaction yet."),
-			display("Unkown Transaction Validity"),
+			display("Unkown Transaction Validity. Error code: {}", e),
 		}
 		/// Transaction is invalid
-		InvalidTransaction {
+		InvalidTransaction(e: i8) {
 			description("Runtime check for the transaction failed."),
-			display("Invalid Transaction"),
+			display("Invalid Transaction. Error Code: {}", e),
 		}
 		/// The transaction is temporarily baned
 		TemporarilyBanned {

substrate/core/transaction-pool/graph/src/pool.rs

@@ -118,12 +118,12 @@ impl<B: ChainApi> Pool<B> {
 							valid_till: block_number.as_().saturating_add(longevity),
 						})
 					},
-					TransactionValidity::Invalid => {
-						bail!(error::Error::from(error::ErrorKind::InvalidTransaction))
+					TransactionValidity::Invalid(e) => {
+						bail!(error::Error::from(error::ErrorKind::InvalidTransaction(e)))
 					},
-					TransactionValidity::Unknown => {
+					TransactionValidity::Unknown(e) => {
 						self.listener.write().invalid(&hash);
-						bail!(error::Error::from(error::ErrorKind::UnknownTransactionValidity))
+						bail!(error::Error::from(error::ErrorKind::UnknownTransactionValidity(e)))
 					},
 				}
 			})
@@ -247,7 +247,7 @@ impl<B: ChainApi> Pool<B> {
 		// Collect the hashes of transactions that now became invalid (meaning that they are succesfuly pruned).
 		let hashes = results.into_iter().enumerate().filter_map(|(idx, r)| match r.map_err(error::IntoPoolError::into_pool_error) {
 			Err(Ok(err)) => match err.kind() {
-				error::ErrorKind::InvalidTransaction => Some(hashes[idx].clone()),
+				error::ErrorKind::InvalidTransaction(_) => Some(hashes[idx].clone()),
 				_ => None,
 			},
 			_ => None,
@@ -413,7 +413,7 @@ mod tests {
 			let nonce = uxt.transfer().nonce;
 
 			if nonce < block_number {
-				Ok(TransactionValidity::Invalid)
+				Ok(TransactionValidity::Invalid(0))
 			} else {
 				Ok(TransactionValidity::Valid {
 					priority: 4,

substrate/core/transaction-pool/src/tests.rs

@@ -53,7 +53,7 @@ impl txpool::ChainApi for TestApi {
 			priority: 1,
 			requires,
 			provides,
-			longevity: 64
+			longevity: 64,
 		})
 	}
 

substrate/srml/executive/src/lib.rs

@@ -223,31 +223,37 @@ impl<
 	///
 	/// Changes made to the storage should be discarded.
 	pub fn validate_transaction(uxt: Block::Extrinsic) -> TransactionValidity {
+		// Note errors > 0 are from ApplyError
+		const UNKNOWN_ERROR: i8 = -127;
+		const MISSING_SENDER: i8 = -20;
+		const INVALID_INDEX: i8 = -10;
+
 		let encoded_len = uxt.encode().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,
+			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(_) => return TransactionValidity::Invalid,
+			Err("bad signature") => return TransactionValidity::Invalid(ApplyError::BadSignature as i8),
+			Err(_) => return TransactionValidity::Invalid(UNKNOWN_ERROR),
 		};
 
 		if let (Some(sender), Some(index)) = (xt.sender(), xt.index()) {
 			// pay any fees.
 			if Payment::make_payment(sender, encoded_len).is_err() {
-				return TransactionValidity::Invalid
+				return TransactionValidity::Invalid(ApplyError::CantPay as i8)
 			}
 
 			// check index
 			let mut expected_index = <system::Module<System>>::account_nonce(sender);
 			if index < &expected_index {
-				return TransactionValidity::Invalid
+				return TransactionValidity::Invalid(ApplyError::Stale as i8)
 			}
 			if *index > expected_index + As::sa(256) {
-				return TransactionValidity::Unknown
+				return TransactionValidity::Unknown(ApplyError::Future as i8)
 			}
 
 			let mut deps = Vec::new();
@@ -263,7 +269,11 @@ impl<
 				longevity: TransactionLongevity::max_value(),
 			}
 		} else {
-			return TransactionValidity::Invalid
+			return TransactionValidity::Invalid(if xt.sender().is_none() {
+				MISSING_SENDER
+			} else {
+				INVALID_INDEX
+			})
 		}
 	}
 }