diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index a5bc63922..bfdcb97f5 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1095,7 +1095,7 @@ impl MiningBlockChainClient for Client { engine, self.factories.clone(), false, // TODO: this will need to be parameterised once we want to do immediate mining insertion. - self.state_db.lock().boxed_clone(), + self.state_db.lock().boxed_clone_canon(&h), &chain.block_header(&h).expect("h is best block hash: so its header must exist: qed"), self.build_last_hashes(h.clone()), author, diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 0d02f8631..7348db566 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -194,7 +194,7 @@ impl Miner { true => None, false => Some(WorkPoster::new(&options.new_work_notify)) }; - let txq = Arc::new(Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, options.tx_gas_limit))); + let txq = Arc::new(Mutex::new(TransactionQueue::with_limits(options.tx_queue_size, !U256::zero(), options.tx_gas_limit))); Miner { transaction_queue: txq, next_allowed_reseal: Mutex::new(Instant::now()), @@ -443,6 +443,8 @@ impl Miner { let gas_limit = HeaderView::new(&chain.best_block_header()).gas_limit(); let mut queue = self.transaction_queue.lock(); queue.set_gas_limit(gas_limit); + // Set total qx queue gas limit to be 2x the block gas limit. + queue.set_total_gas_limit(gas_limit << 1); } /// Returns true if we had to prepare new pending block. diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index 02e569cf2..38923438c 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -130,6 +130,8 @@ struct TransactionOrder { /// (e.g. Tx(nonce:5), State(nonce:0) -> height: 5) /// High nonce_height = Low priority (processed later) nonce_height: U256, + /// Gas specified in the transaction. + gas: U256, /// Gas Price of the transaction. /// Low gas price = Low priority (processed later) gas_price: U256, @@ -146,6 +148,7 @@ impl TransactionOrder { fn for_transaction(tx: &VerifiedTransaction, base_nonce: U256) -> Self { TransactionOrder { nonce_height: tx.nonce() - base_nonce, + gas: tx.transaction.gas.clone(), gas_price: tx.transaction.gas_price, hash: tx.hash(), origin: tx.origin, @@ -287,6 +290,7 @@ struct TransactionSet { by_address: Table, by_gas_price: GasPriceQueue, limit: usize, + gas_limit: U256, } impl TransactionSet { @@ -317,15 +321,18 @@ impl TransactionSet { /// It drops transactions from this set but also removes associated `VerifiedTransaction`. /// Returns addresses and lowest nonces of transactions removed because of limit. fn enforce_limit(&mut self, by_hash: &mut HashMap) -> Option> { - let len = self.by_priority.len(); - if len <= self.limit { - return None; - } - + let mut count = 0; + let mut gas: U256 = 0.into(); let to_drop : Vec<(Address, U256)> = { self.by_priority .iter() - .skip(self.limit) + .skip_while(|order| { + count = count + 1; + let r = gas.overflowing_add(order.gas); + if r.1 { return false } + gas = r.0; + count <= self.limit && gas <= self.gas_limit + }) .map(|order| by_hash.get(&order.hash) .expect("All transactions in `self.by_priority` and `self.by_address` are kept in sync with `by_hash`.")) .map(|tx| (tx.sender(), tx.nonce())) @@ -432,16 +439,17 @@ impl Default for TransactionQueue { impl TransactionQueue { /// Creates new instance of this Queue pub fn new() -> Self { - Self::with_limits(1024, !U256::zero()) + Self::with_limits(1024, !U256::zero(), !U256::zero()) } /// Create new instance of this Queue with specified limits - pub fn with_limits(limit: usize, tx_gas_limit: U256) -> Self { + pub fn with_limits(limit: usize, gas_limit: U256, tx_gas_limit: U256) -> Self { let current = TransactionSet { by_priority: BTreeSet::new(), by_address: Table::new(), by_gas_price: Default::default(), limit: limit, + gas_limit: gas_limit, }; let future = TransactionSet { @@ -449,6 +457,7 @@ impl TransactionQueue { by_address: Table::new(), by_gas_price: Default::default(), limit: limit, + gas_limit: gas_limit, }; TransactionQueue { @@ -504,6 +513,13 @@ impl TransactionQueue { }; } + /// Sets new total gas limit. + pub fn set_total_gas_limit(&mut self, gas_limit: U256) { + self.future.gas_limit = gas_limit; + self.current.gas_limit = gas_limit; + self.future.enforce_limit(&mut self.by_hash); + } + /// Set the new limit for the amount of gas any individual transaction may have. /// Any transaction already imported to the queue is not affected. pub fn set_tx_gas_limit(&mut self, limit: U256) { @@ -827,6 +843,16 @@ impl TransactionQueue { let nonce = tx.nonce(); let hash = tx.hash(); + { + // Rough size sanity check + let gas = &tx.transaction.gas; + if U256::from(tx.transaction.data.len()) > *gas { + // Droping transaction + trace!(target: "txqueue", "Dropping oversized transaction: {:?} (gas: {} < size {})", hash, gas, tx.transaction.data.len()); + return Err(TransactionError::LimitReached); + } + } + // The transaction might be old, let's check that. // This has to be the first test, otherwise calculating // nonce height would result in overflow. @@ -979,6 +1005,7 @@ mod test { } fn default_nonce() -> U256 { 123.into() } + fn default_gas_val() -> U256 { 100_000.into() } fn default_gas_price() -> U256 { 1.into() } fn new_unsigned_tx(nonce: U256, gas_price: U256) -> Transaction { @@ -986,7 +1013,7 @@ mod test { action: Action::Create, value: U256::from(100), data: "3331600055".from_hex().unwrap(), - gas: U256::from(100_000), + gas: default_gas_val(), gas_price: gas_price, nonce: nonce } @@ -1051,7 +1078,7 @@ mod test { #[test] fn should_return_correct_nonces_when_dropped_because_of_limit() { // given - let mut txq = TransactionQueue::with_limits(2, !U256::zero()); + let mut txq = TransactionQueue::with_limits(2, !U256::zero(), !U256::zero()); let (tx1, tx2) = new_tx_pair(123.into(), 1.into(), 1.into(), 0.into()); let sender = tx1.sender().unwrap(); let nonce = tx1.nonce; @@ -1089,7 +1116,8 @@ mod test { by_priority: BTreeSet::new(), by_address: Table::new(), by_gas_price: Default::default(), - limit: 1 + limit: 1, + gas_limit: !U256::zero(), }; let (tx1, tx2) = new_tx_pair_default(1.into(), 0.into()); let tx1 = VerifiedTransaction::new(tx1, TransactionOrigin::External).unwrap(); @@ -1129,7 +1157,8 @@ mod test { by_priority: BTreeSet::new(), by_address: Table::new(), by_gas_price: Default::default(), - limit: 1 + limit: 1, + gas_limit: !U256::zero(), }; // Create two transactions with same nonce // (same hash) @@ -1177,7 +1206,8 @@ mod test { by_priority: BTreeSet::new(), by_address: Table::new(), by_gas_price: Default::default(), - limit: 2 + limit: 2, + gas_limit: !U256::zero(), }; let tx = new_tx_default(); let tx1 = VerifiedTransaction::new(tx.clone(), TransactionOrigin::External).unwrap(); @@ -1194,7 +1224,8 @@ mod test { by_priority: BTreeSet::new(), by_address: Table::new(), by_gas_price: Default::default(), - limit: 1 + limit: 1, + gas_limit: !U256::zero(), }; assert_eq!(set.gas_price_entry_limit(), 0.into()); @@ -1690,7 +1721,7 @@ mod test { #[test] fn should_drop_old_transactions_when_hitting_the_limit() { // given - let mut txq = TransactionQueue::with_limits(1, !U256::zero()); + let mut txq = TransactionQueue::with_limits(1, !U256::zero(), !U256::zero()); let (tx, tx2) = new_tx_pair_default(1.into(), 0.into()); let sender = tx.sender().unwrap(); let nonce = tx.nonce; @@ -1711,7 +1742,7 @@ mod test { #[test] fn should_limit_future_transactions() { - let mut txq = TransactionQueue::with_limits(1, !U256::zero()); + let mut txq = TransactionQueue::with_limits(1, !U256::zero(), !U256::zero()); txq.current.set_limit(10); let (tx1, tx2) = new_tx_pair_default(4.into(), 1.into()); let (tx3, tx4) = new_tx_pair_default(4.into(), 2.into()); @@ -1728,6 +1759,16 @@ mod test { assert_eq!(txq.status().future, 1); } + #[test] + fn should_limit_by_gas() { + let mut txq = TransactionQueue::with_limits(100, default_gas_val() * U256::from(2), !U256::zero()); + let (tx1, _) = new_tx_pair_default(U256::from(4), U256::from(1)); + let (tx3, _) = new_tx_pair_default(U256::from(4), U256::from(2)); + txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap(); + txq.add(tx3.clone(), &default_account_details, TransactionOrigin::External).unwrap(); + assert_eq!(txq.status().pending, 2); + } + #[test] fn should_drop_transactions_with_old_nonces() { let mut txq = TransactionQueue::new(); @@ -1971,7 +2012,7 @@ mod test { #[test] fn should_keep_right_order_in_future() { // given - let mut txq = TransactionQueue::with_limits(1, !U256::zero()); + let mut txq = TransactionQueue::with_limits(1, !U256::zero(), !U256::zero()); let (tx1, tx2) = new_tx_pair_default(1.into(), 0.into()); let prev_nonce = |a: &Address| AccountDetails { nonce: default_account_details(a).nonce - U256::one(), balance: default_account_details(a).balance }; diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 565c53827..d72ad400c 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -123,6 +123,7 @@ const MAX_ROUND_PARENTS: usize = 32; const MAX_NEW_HASHES: usize = 64; const MAX_TX_TO_IMPORT: usize = 512; const MAX_NEW_BLOCK_AGE: BlockNumber = 20; +const MAX_TRANSACTION_SIZE: usize = 300*1024; const STATUS_PACKET: u8 = 0x00; const NEW_BLOCK_HASHES_PACKET: u8 = 0x01; @@ -1261,7 +1262,12 @@ impl ChainSync { item_count = min(item_count, MAX_TX_TO_IMPORT); let mut transactions = Vec::with_capacity(item_count); for i in 0 .. item_count { - let tx = try!(r.at(i)).as_raw().to_vec(); + let rlp = try!(r.at(i)); + if rlp.as_raw().len() > MAX_TRANSACTION_SIZE { + debug!("Skipped oversized transaction of {} bytes", rlp.as_raw().len()); + continue; + } + let tx = rlp.as_raw().to_vec(); transactions.push(tx); } io.chain().queue_transactions(transactions);