From a0cdf5c420f45129461608b2e3821171c7aa6db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 15:20:33 +0100 Subject: [PATCH] Ignoring transactions slightly above gas_limit --- ethcore/src/error.rs | 7 ++++ miner/src/miner.rs | 17 ++++++++- miner/src/transaction_queue.rs | 67 +++++++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 482a8de01..a3a379463 100644 --- a/ethcore/src/error.rs +++ b/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), } diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 4448b721b..0307c56fc 100644 --- a/miner/src/miner.rs +++ b/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); } diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 0f2ec6ec7..107904114 100644 --- a/miner/src/transaction_queue.rs +++ b/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,24 @@ 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, + }; + } + + // Will be used when rpc merged + #[allow(dead_code)] /// Returns current status for this queue pub fn status(&self) -> TransactionQueueStatus { TransactionQueueStatus { @@ -335,12 +355,24 @@ impl TransactionQueue { tx.hash(), tx.gas_price, self.minimal_gas_price ); - return Err(Error::Transaction(TransactionError::InsufficientGasPrice{ + return Err(Error::Transaction(TransactionError::InsufficientGasPrice { minimal: self.minimal_gas_price, got: tx.gas_price, })); } + 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 +709,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