Merge pull request #760 from ethcore/tx_queue_gas_limit

Avoid importing transactions with gas above 1.1*block_gas_limit to transaction queue

ethcore/src/client/test_client.rs

@@ -111,6 +111,7 @@ impl TestBlockChainClient {
 			header.difficulty = From::from(n);
 			header.parent_hash = self.last_hash.read().unwrap().clone();
 			header.number = n as BlockNumber;
+			header.gas_limit = U256::from(1_000_000);
 			let uncles = match with {
 				EachBlockWith::Uncle | EachBlockWith::UncleAndTransaction => {
 					let mut uncles = RlpStream::new_list(1);

ethcore/src/error.rs

@@ -79,6 +79,13 @@ pub enum TransactionError {
 		/// Transaction cost
 		cost: U256,
 	},
+	/// Transactions gas is higher then current gas limit
+	GasLimitExceeded {
+		/// Current gas limit
+		limit: U256,
+		/// Declared transaction gas
+		got: U256,
+	},
 	/// Transaction's gas limit (aka gas) is invalid.
 	InvalidGasLimit(OutOfBounds<U256>),
 }

miner/src/miner.rs

@@ -21,7 +21,7 @@ use std::sync::atomic::AtomicBool;
 use std::collections::HashSet;
 
 use util::{H256, U256, Address, Bytes, Uint};
-use ethcore::views::{BlockView};
+use ethcore::views::{BlockView, HeaderView};
 use ethcore::client::{BlockChainClient, BlockId};
 use ethcore::block::{ClosedBlock, IsBlock};
 use ethcore::error::{Error};
@@ -94,6 +94,12 @@ impl Miner {
 	pub fn set_minimal_gas_price(&self, min_gas_price: U256) {
 		self.transaction_queue.lock().unwrap().set_minimal_gas_price(min_gas_price);
 	}
+
+	fn update_gas_limit(&self, chain: &BlockChainClient) {
+		let gas_limit = HeaderView::new(&chain.best_block_header()).gas_limit();
+		let mut queue = self.transaction_queue.lock().unwrap();
+		queue.set_gas_limit(gas_limit);
+	}
 }
 
 impl MinerService for Miner {
@@ -185,6 +191,11 @@ impl MinerService for Miner {
 			let block = BlockView::new(&block);
 			block.transactions()
 		}
+
+		// First update gas limit in transaction queue
+		self.update_gas_limit(chain);
+
+		// Then import all transactions...
 		{
 			let out_of_chain = retracted
 				.par_iter()
@@ -201,7 +212,8 @@ impl MinerService for Miner {
 				});
 			});
 		}
-		// First import all transactions and after that remove old ones
+
+		// ...and after that remove old ones
 		{
 			let in_chain = {
 				let mut in_chain = HashSet::new();
@@ -227,6 +239,7 @@ impl MinerService for Miner {
 			});
 		}
 
+		// Update mined block
 		if self.sealing_enabled.load(atomic::Ordering::Relaxed) {
 			self.prepare_sealing(chain);
 		}

miner/src/transaction_queue.rs

@@ -252,10 +252,16 @@ pub struct AccountDetails {
 	pub balance: U256,
 }
 
+
+/// Transactions with `gas > (gas_limit + gas_limit * Factor(in percents))` are not imported to the queue.
+const GAS_LIMIT_HYSTERESIS: usize = 10; // %
+
 /// TransactionQueue implementation
 pub struct TransactionQueue {
 	/// Gas Price threshold for transactions that can be imported to this queue (defaults to 0)
 	minimal_gas_price: U256,
+	/// Current gas limit (block gas limit * factor). Transactions above the limit will not be accepted (default to !0)
+	gas_limit: U256,
 	/// Priority queue for transactions that can go to block
 	current: TransactionSet,
 	/// Priority queue for transactions that has been received but are not yet valid to go to block
@@ -293,6 +299,7 @@ impl TransactionQueue {
 
 		TransactionQueue {
 			minimal_gas_price: U256::zero(),
+			gas_limit: !U256::zero(),
 			current: current,
 			future: future,
 			by_hash: HashMap::new(),
@@ -301,11 +308,22 @@ impl TransactionQueue {
 	}
 
 	/// Sets new gas price threshold for incoming transactions.
-	/// Any transactions already imported to the queue are not affected.
+	/// Any transaction already imported to the queue is not affected.
 	pub fn set_minimal_gas_price(&mut self, min_gas_price: U256) {
 		self.minimal_gas_price = min_gas_price;
 	}
 
+	/// Sets new gas limit. Transactions with gas slightly (`GAS_LIMIT_HYSTERESIS`) above the limit won't be imported.
+	/// Any transaction already imported to the queue is not affected.
+	pub fn set_gas_limit(&mut self, gas_limit: U256) {
+		let extra = gas_limit / U256::from(GAS_LIMIT_HYSTERESIS);
+
+		self.gas_limit = match gas_limit.overflowing_add(extra) {
+			(_, true) => !U256::zero(),
+			(val, false) => val,
+		};
+	}
+
 	/// Returns current status for this queue
 	pub fn status(&self) -> TransactionQueueStatus {
 		TransactionQueueStatus {
@@ -341,6 +359,18 @@ impl TransactionQueue {
 			}));
 		}
 
+		if tx.gas > self.gas_limit {
+			trace!(target: "miner",
+				"Dropping transaction above gas limit: {:?} ({} > {})",
+				tx.hash(), tx.gas, self.gas_limit
+			);
+
+			return Err(Error::Transaction(TransactionError::GasLimitExceeded {
+				limit: self.gas_limit,
+				got: tx.gas,
+			}));
+		}
+
 
 		let vtx = try!(VerifiedTransaction::new(tx));
 		let account = fetch_account(&vtx.sender());
@@ -677,6 +707,37 @@ mod test {
 		assert_eq!(stats.pending, 1);
 	}
 
+	#[test]
+	fn gas_limit_should_never_overflow() {
+		// given
+		let mut txq = TransactionQueue::new();
+		txq.set_gas_limit(U256::zero());
+		assert_eq!(txq.gas_limit, U256::zero());
+
+		// when
+		txq.set_gas_limit(!U256::zero());
+
+		// then
+		assert_eq!(txq.gas_limit, !U256::zero());
+	}
+
+	#[test]
+	fn should_not_import_transaction_above_gas_limit() {
+		// given
+		let mut txq = TransactionQueue::new();
+		let tx = new_tx();
+		txq.set_gas_limit(tx.gas / U256::from(2));
+
+		// when
+		txq.add(tx, &default_nonce).unwrap_err();
+
+		// then
+		let stats = txq.status();
+		assert_eq!(stats.pending, 0);
+		assert_eq!(stats.future, 0);
+	}
+
+
 	#[test]
 	fn should_drop_transactions_from_senders_without_balance() {
 		// given