Merge pull request #3762 from ethcore/tx-check

Reject existing transactions

ethcore/src/client/client.rs

@@ -1060,6 +1060,10 @@ impl BlockChainClient for Client {
 		self.transaction_address(id).and_then(|address| self.chain.read().transaction(&address))
 	}
 
+	fn transaction_block(&self, id: TransactionID) -> Option<H256> {
+		self.transaction_address(id).map(|addr| addr.block_hash)
+	}
+
 	fn uncle(&self, id: UncleID) -> Option<Bytes> {
 		let index = id.position;
 		self.block_body(id.block).and_then(|body| BodyView::new(&body).uncle_rlp_at(index))

ethcore/src/client/test_client.rs

@@ -432,6 +432,10 @@ impl BlockChainClient for TestBlockChainClient {
 		None	// Simple default.
 	}
 
+	fn transaction_block(&self, _id: TransactionID) -> Option<H256> {
+		None	// Simple default.
+	}
+
 	fn uncle(&self, _id: UncleID) -> Option<Bytes> {
 		None	// Simple default.
 	}

ethcore/src/client/traits.rs

@@ -129,6 +129,9 @@ pub trait BlockChainClient : Sync + Send {
 	/// Get transaction with given hash.
 	fn transaction(&self, id: TransactionID) -> Option<LocalizedTransaction>;
 
+	/// Get the hash of block that contains the transaction, if any.
+	fn transaction_block(&self, id: TransactionID) -> Option<H256>;
+
 	/// Get uncle with given id.
 	fn uncle(&self, id: UncleID) -> Option<Bytes>;
 

ethcore/src/miner/miner.rs

@@ -23,7 +23,7 @@ use account_provider::{AccountProvider, Error as AccountError};
 use views::{BlockView, HeaderView};
 use header::Header;
 use state::{State, CleanupMode};
-use client::{MiningBlockChainClient, Executive, Executed, EnvInfo, TransactOptions, BlockID, CallAnalytics};
+use client::{MiningBlockChainClient, Executive, Executed, EnvInfo, TransactOptions, BlockID, CallAnalytics, TransactionID};
 use client::TransactionImportResult;
 use executive::contract_address;
 use block::{ClosedBlock, SealedBlock, IsBlock, Block};
@@ -357,6 +357,8 @@ impl Miner {
 		let block_number = open_block.block().fields().header.number();
 
 		// TODO Push new uncles too.
+		let mut tx_count: usize = 0;
+		let tx_total = transactions.len();
 		for tx in transactions {
 			let hash = tx.hash();
 			let start = Instant::now();
@@ -378,7 +380,7 @@ impl Miner {
 				},
 				_ => {},
 			}
-
+			trace!(target: "miner", "Adding tx {:?} took {:?}", hash, took);
 			match result {
 				Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, gas })) => {
 					debug!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?} (limit: {:?}, used: {:?}, gas: {:?})", hash, gas_limit, gas_used, gas);
@@ -407,9 +409,12 @@ impl Miner {
 						   "Error adding transaction to block: number={}. transaction_hash={:?}, Error: {:?}",
 						   block_number, hash, e);
 				},
-				_ => {}	// imported ok
+				_ => {
+					tx_count += 1;
+				}	// imported ok
 			}
 		}
+		trace!(target: "miner", "Pushed {}/{} transactions", tx_count, tx_total);
 
 		let block = open_block.close();
 
@@ -580,6 +585,10 @@ impl Miner {
 		let best_block_header: Header = ::rlp::decode(&chain.best_block_header());
 		transactions.into_iter()
 			.map(|tx| {
+				if chain.transaction_block(TransactionID::Hash(tx.hash())).is_some() {
+					debug!(target: "miner", "Rejected tx {:?}: already in the blockchain", tx.hash());
+					return Err(Error::Transaction(TransactionError::AlreadyImported));
+				}
 				match self.engine.verify_transaction_basic(&tx, &best_block_header) {
 					Err(e) => {
 						debug!(target: "miner", "Rejected tx {:?} with invalid signature: {:?}", tx.hash(), e);