From fd29926a2157d747d2b7cac2f74155aa61fcc567 Mon Sep 17 00:00:00 2001
From: Andronik Ordian <write@reusable.software>
Date: Fri, 13 Dec 2019 20:25:51 +0100
Subject: [PATCH] tx-q: enable basic verification of local transactions
 (#11332)

* tx-q: basic verification of local transactions

* miner: basic test for local import

* miner: info log when rejecting local txn

* Hernandofmt

Co-Authored-By: Hernando Castano <HCastano@users.noreply.github.com>

* miner: assert in a test with the concrete error type

* tx-q: info! -> warn! on local tx rejection
---
 ethcore/src/miner/miner.rs       | 24 ++++++++++++++++++++++++
 ethcore/src/miner/pool_client.rs |  7 ++++++-
 miner/src/pool/client.rs         |  9 +++++++++
 miner/src/pool/tests/client.rs   |  6 ++++++
 miner/src/pool/verifier.rs       |  8 +++++++-
 5 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs
index 997ceb1bc..e3d3e0442 100644
--- a/ethcore/src/miner/miner.rs
+++ b/ethcore/src/miner/miner.rs
@@ -1695,6 +1695,30 @@ mod tests {
 		assert_eq!(miner.prepare_pending_block(&client), BlockPreparationStatus::NotPrepared);
 	}
 
+	#[test]
+	fn should_reject_local_transaction_with_invalid_chain_id() {
+		let spec = spec::new_test();
+		let miner = Miner::new_for_tests(&spec, None);
+		let client = TestBlockChainClient::default();
+		let chain_id = spec.chain_id();
+
+		// chain_id + 100500 is invalid
+		let import = miner.import_claimed_local_transaction(
+			&client,
+			PendingTransaction::new(transaction_with_chain_id(chain_id + 10500), None),
+			false,
+		);
+		assert_eq!(import, Err(transaction::Error::InvalidChainId));
+
+		// chain_id is valid
+		let import = miner.import_claimed_local_transaction(
+			&client,
+			PendingTransaction::new(transaction_with_chain_id(chain_id), None),
+			false,
+		);
+		assert_eq!(import, Ok(()));
+	}
+
 	#[test]
 	fn should_prioritize_locals() {
 		let client = TestBlockChainClient::default();
diff --git a/ethcore/src/miner/pool_client.rs b/ethcore/src/miner/pool_client.rs
index 8b8740dff..61bdaad00 100644
--- a/ethcore/src/miner/pool_client.rs
+++ b/ethcore/src/miner/pool_client.rs
@@ -139,7 +139,12 @@ impl<'a, C: 'a> pool::client::Client for PoolClient<'a, C> where
 		self.chain.transaction_block(TransactionId::Hash(*hash)).is_some()
 	}
 
-	fn verify_transaction(&self, tx: UnverifiedTransaction)-> Result<SignedTransaction, transaction::Error> {
+	fn verify_transaction_basic(&self, tx: &UnverifiedTransaction) -> Result<(), transaction::Error> {
+		self.engine.verify_transaction_basic(tx, &self.best_block_header)?;
+		Ok(())
+	}
+
+	fn verify_transaction(&self, tx: UnverifiedTransaction) -> Result<SignedTransaction, transaction::Error> {
 		self.engine.verify_transaction_basic(&tx, &self.best_block_header)?;
 		let tx = tx.verify_unordered()?;
 
diff --git a/miner/src/pool/client.rs b/miner/src/pool/client.rs
index bcd2b9689..1579ba40d 100644
--- a/miner/src/pool/client.rs
+++ b/miner/src/pool/client.rs
@@ -50,6 +50,15 @@ pub trait Client: fmt::Debug + Sync {
 	/// Is transaction with given hash already in the blockchain?
 	fn transaction_already_included(&self, hash: &H256) -> bool;
 
+	/// Perform basic/cheap transaction verification.
+	///
+	/// This should include all cheap checks that can be done before
+	/// actually checking the signature, like chain-replay protection.
+	///
+	/// This method is currently used only for verifying local transactions.
+	fn verify_transaction_basic(&self, t: &transaction::UnverifiedTransaction)
+		-> Result<(), transaction::Error>;
+
 	/// Structurarily verify given transaction.
 	fn verify_transaction(&self, tx: transaction::UnverifiedTransaction)
 		-> Result<transaction::SignedTransaction, transaction::Error>;
diff --git a/miner/src/pool/tests/client.rs b/miner/src/pool/tests/client.rs
index 4735fbd64..ce927c8e9 100644
--- a/miner/src/pool/tests/client.rs
+++ b/miner/src/pool/tests/client.rs
@@ -103,6 +103,12 @@ impl pool::client::Client for TestClient {
 		false
 	}
 
+	fn verify_transaction_basic(&self, _tx: &UnverifiedTransaction)
+		-> Result<(), transaction::Error>
+	{
+		Ok(())
+	}
+
 	fn verify_transaction(&self, tx: UnverifiedTransaction)
 		-> Result<SignedTransaction, transaction::Error>
 	{
diff --git a/miner/src/pool/verifier.rs b/miner/src/pool/verifier.rs
index 79d50d713..ae64ac522 100644
--- a/miner/src/pool/verifier.rs
+++ b/miner/src/pool/verifier.rs
@@ -250,7 +250,13 @@ impl<C: Client> txpool::Verifier<Transaction> for Verifier<C, ::pool::scoring::N
 					return Err(err)
 				},
 			},
-			Transaction::Local(tx) => tx,
+			Transaction::Local(tx) => match self.client.verify_transaction_basic(&**tx) {
+				Ok(()) => tx,
+				Err(err) => {
+					warn!(target: "txqueue", "[{:?}] Rejected local tx {:?}", hash, err);
+					return Err(err)
+				}
+			},
 		};
 
 		// Verify RLP payload