Validating minimal required gas for a transaction (#2937)

* Validating minimal required gas for a transaction

* Adding RPC case and note

* Fixing whitespace [ci skip]

ethcore/src/client/client.rs

@@ -60,7 +60,7 @@ use receipt::LocalizedReceipt;
 use trace::{TraceDB, ImportRequest as TraceImportRequest, LocalizedTrace, Database as TraceDatabase};
 use trace;
 use trace::FlatTransactionTraces;
-use evm::Factory as EvmFactory;
+use evm::{Factory as EvmFactory, Schedule};
 use miner::{Miner, MinerService};
 use snapshot::{self, io as snapshot_io};
 use factory::Factories;
@@ -1156,6 +1156,23 @@ impl BlockChainClient for Client {
 }
 
 impl MiningBlockChainClient for Client {
+
+	fn latest_schedule(&self) -> Schedule {
+		let header_data = self.best_block_header();
+		let view = HeaderView::new(&header_data);
+
+		let env_info = EnvInfo {
+			number: view.number(),
+			author: view.author(),
+			timestamp: view.timestamp(),
+			difficulty: view.difficulty(),
+			last_hashes: self.build_last_hashes(view.hash()),
+			gas_used: U256::default(),
+			gas_limit: view.gas_limit(),
+		};
+		self.engine.schedule(&env_info)
+	}
+
 	fn prepare_open_block(&self, author: Address, gas_range_target: (U256, U256), extra_data: Bytes) -> OpenBlock {
 		let engine = &*self.engine;
 		let chain = self.chain.read();

ethcore/src/client/test_client.rs

@@ -34,7 +34,7 @@ use log_entry::LocalizedLogEntry;
 use receipt::{Receipt, LocalizedReceipt};
 use blockchain::extras::BlockReceipts;
 use error::{ImportResult};
-use evm::{Factory as EvmFactory, VMType};
+use evm::{Factory as EvmFactory, VMType, Schedule};
 use miner::{Miner, MinerService, TransactionImportResult};
 use spec::Spec;
 
@@ -137,7 +137,7 @@ impl TestBlockChainClient {
 		client.genesis_hash = client.last_hash.read().clone();
 		client
 	}
-	
+
 	/// Set the transaction receipt result
 	pub fn set_transaction_receipt(&self, id: TransactionID, receipt: LocalizedReceipt) {
 		self.receipts.write().insert(id, receipt);
@@ -306,6 +306,10 @@ pub fn get_temp_state_db() -> GuardedTempResult<StateDB> {
 }
 
 impl MiningBlockChainClient for TestBlockChainClient {
+	fn latest_schedule(&self) -> Schedule {
+		Schedule::new_homestead_gas_fix()
+	}
+
 	fn prepare_open_block(&self, author: Address, gas_range_target: (U256, U256), extra_data: Bytes) -> OpenBlock {
 		let engine = &*self.spec.engine;
 		let genesis_header = self.spec.genesis_header();

ethcore/src/client/traits.rs

@@ -27,7 +27,7 @@ use views::{BlockView};
 use error::{ImportResult, CallError};
 use receipt::LocalizedReceipt;
 use trace::LocalizedTrace;
-use evm::Factory as EvmFactory;
+use evm::{Factory as EvmFactory, Schedule};
 use types::ids::*;
 use types::trace_filter::Filter as TraceFilter;
 use executive::Executed;
@@ -236,6 +236,9 @@ pub trait MiningBlockChainClient : BlockChainClient {
 
 	/// Import sealed block. Skips all verifications.
 	fn import_sealed_block(&self, block: SealedBlock) -> ImportResult;
+
+	/// Returns latest schedule.
+	fn latest_schedule(&self) -> Schedule;
 }
 
 impl IpcConfig for BlockChainClient { }

ethcore/src/error.rs

@@ -47,6 +47,13 @@ pub enum TransactionError {
 		/// Transaction gas price
 		got: U256,
 	},
+	/// Transaction's gas is below currently set minimal gas requirement.
+	InsufficientGas {
+		/// Minimal expected gas
+		minimal: U256,
+		/// Transaction gas
+		got: U256,
+	},
 	/// Sender doesn't have enough funds to pay for this transaction
 	InsufficientBalance {
 		/// Senders balance
@@ -81,6 +88,8 @@ impl fmt::Display for TransactionError {
 			LimitReached => "Transaction limit reached".into(),
 			InsufficientGasPrice { minimal, got } =>
 				format!("Insufficient gas price. Min={}, Given={}", minimal, got),
+			InsufficientGas { minimal, got } =>
+				format!("Insufficient gas. Min={}, Given={}", minimal, got),
 			InsufficientBalance { balance, cost } =>
 				format!("Insufficient balance for transaction. Balance={}, Cost={}",
 					balance, cost),

ethcore/src/miner/banning_queue.rs

@@ -75,11 +75,15 @@ impl BanningTransactionQueue {
 
 	/// Add to the queue taking bans into consideration.
 	/// May reject transaction because of the banlist.
-	pub fn add_with_banlist<F>(
+	pub fn add_with_banlist<F, G>(
 		&mut self,
 		transaction: SignedTransaction,
 		account_details: &F,
-	) -> Result<TransactionImportResult, Error> where F: Fn(&Address) -> AccountDetails {
+		gas_estimator: &G,
+	) -> Result<TransactionImportResult, Error> where
+		F: Fn(&Address) -> AccountDetails,
+		G: Fn(&SignedTransaction) -> U256,
+	{
 		if let Threshold::BanAfter(threshold) = self.ban_threshold {
 			// NOTE In all checks use direct query to avoid increasing ban timeout.
 
@@ -111,7 +115,7 @@ impl BanningTransactionQueue {
 				}
 			}
 		}
-		self.queue.add(transaction, account_details, TransactionOrigin::External)
+		self.queue.add(transaction, TransactionOrigin::External, account_details, gas_estimator)
 	}
 
 	/// Ban transaction with given hash.
@@ -228,6 +232,10 @@ mod tests {
 		}
 	}
 
+	fn gas_required(_tx: &SignedTransaction) -> U256 {
+		0.into()
+	}
+
 	fn transaction(action: Action) -> SignedTransaction {
 		let keypair = Random.generate().unwrap();
 		Transaction {
@@ -255,7 +263,7 @@ mod tests {
 		let mut txq = queue();
 
 		// when
-		txq.queue().add(tx, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.queue().add(tx, TransactionOrigin::External, &default_account_details, &gas_required).unwrap();
 
 		// then
 		// should also deref to queue
@@ -271,12 +279,12 @@ mod tests {
 		let banlist1 = txq.ban_sender(tx.sender().unwrap());
 		assert!(!banlist1, "Threshold not reached yet.");
 		// Insert once
-		let import1 = txq.add_with_banlist(tx.clone(), &default_account_details).unwrap();
+		let import1 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required).unwrap();
 		assert_eq!(import1, TransactionImportResult::Current);
 
 		// when
 		let banlist2 = txq.ban_sender(tx.sender().unwrap());
-		let import2 = txq.add_with_banlist(tx.clone(), &default_account_details);
+		let import2 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required);
 
 		// then
 		assert!(banlist2, "Threshold should be reached - banned.");
@@ -295,12 +303,12 @@ mod tests {
 		let banlist1 = txq.ban_recipient(recipient);
 		assert!(!banlist1, "Threshold not reached yet.");
 		// Insert once
-		let import1 = txq.add_with_banlist(tx.clone(), &default_account_details).unwrap();
+		let import1 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required).unwrap();
 		assert_eq!(import1, TransactionImportResult::Current);
 
 		// when
 		let banlist2 = txq.ban_recipient(recipient);
-		let import2 = txq.add_with_banlist(tx.clone(), &default_account_details);
+		let import2 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required);
 
 		// then
 		assert!(banlist2, "Threshold should be reached - banned.");
@@ -317,12 +325,12 @@ mod tests {
 		let banlist1 = txq.ban_codehash(codehash);
 		assert!(!banlist1, "Threshold not reached yet.");
 		// Insert once
-		let import1 = txq.add_with_banlist(tx.clone(), &default_account_details).unwrap();
+		let import1 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required).unwrap();
 		assert_eq!(import1, TransactionImportResult::Current);
 
 		// when
 		let banlist2 = txq.ban_codehash(codehash);
-		let import2 = txq.add_with_banlist(tx.clone(), &default_account_details);
+		let import2 = txq.add_with_banlist(tx.clone(), &default_account_details, &gas_required);
 
 		// then
 		assert!(banlist2, "Threshold should be reached - banned.");

ethcore/src/miner/miner.rs

@@ -562,13 +562,15 @@ impl Miner {
 			balance: chain.latest_balance(a),
 		};
 
+		let schedule = chain.latest_schedule();
+		let gas_required = |tx: &SignedTransaction| tx.gas_required(&schedule).into();
 		transactions.into_iter()
 			.map(|tx| match origin {
 				TransactionOrigin::Local | TransactionOrigin::RetractedBlock => {
-					transaction_queue.add(tx, &fetch_account, origin)
+					transaction_queue.add(tx, origin, &fetch_account, &gas_required)
 				},
 				TransactionOrigin::External => {
-					transaction_queue.add_with_banlist(tx, &fetch_account)
+					transaction_queue.add_with_banlist(tx, &fetch_account, &gas_required)
 				}
 			})
 			.collect()

ethcore/src/miner/transaction_queue.rs

@@ -48,10 +48,11 @@
 //!			nonce: U256::from(10),
 //!			balance: U256::from(1_000_000),
 //!		};
+//!		let gas_estimator = |_tx: &SignedTransaction| 2.into();
 //!
 //!		let mut txq = TransactionQueue::default();
-//!		txq.add(st2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-//!		txq.add(st1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+//!		txq.add(st2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+//!		txq.add(st1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 //!
 //!		// Check status
 //!		assert_eq!(txq.status().pending, 2);
@@ -593,9 +594,20 @@ impl TransactionQueue {
 		}
 	}
 
-	/// Add signed transaction to queue to be verified and imported
-	pub fn add<T>(&mut self, tx: SignedTransaction, fetch_account: &T, origin: TransactionOrigin) -> Result<TransactionImportResult, Error>
-	where T: Fn(&Address) -> AccountDetails {
+	/// Add signed transaction to queue to be verified and imported.
+	///
+	/// NOTE fetch_account and gas_estimator should be cheap to compute
+	/// otherwise it might open up an attack vector.
+	pub fn add<F, G>(
+		&mut self,
+		tx: SignedTransaction,
+		origin: TransactionOrigin,
+		fetch_account: &F,
+		gas_estimator: &G,
+	) -> Result<TransactionImportResult, Error> where
+		F: Fn(&Address) -> AccountDetails,
+		G: Fn(&SignedTransaction) -> U256,
+	{
 
 		if tx.gas_price < self.minimal_gas_price && origin != TransactionOrigin::Local {
 			trace!(target: "txqueue",
@@ -626,8 +638,6 @@ impl TransactionQueue {
 			}));
 		}
 
-		try!(tx.check_low_s());
-
 		if tx.gas > self.gas_limit || tx.gas > self.tx_gas_limit {
 			trace!(target: "txqueue",
 				"Dropping transaction above gas limit: {:?} ({} > min({}, {}))",
@@ -643,6 +653,24 @@ impl TransactionQueue {
 			}));
 		}
 
+		let minimal_gas = gas_estimator(&tx);
+		if tx.gas < minimal_gas {
+			trace!(target: "txqueue",
+				"Dropping transaction with insufficient gas: {:?} ({} > {})",
+				tx.hash(),
+				tx.gas,
+				minimal_gas,
+			);
+
+			return Err(Error::Transaction(TransactionError::InsufficientGas {
+				minimal: minimal_gas,
+				got: tx.gas,
+			}));
+		}
+
+		// Verify signature
+		try!(tx.check_low_s());
+
 		let vtx = try!(VerifiedTransaction::new(tx, origin));
 		let client_account = fetch_account(&vtx.sender());
 
@@ -905,16 +933,6 @@ 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.
@@ -1104,6 +1122,10 @@ mod test {
 		}
 	}
 
+	fn gas_estimator(_tx: &SignedTransaction) -> U256 {
+		U256::zero()
+	}
+
 	fn new_tx_pair(nonce: U256, gas_price: U256, nonce_increment: U256, gas_price_increment: U256) -> (SignedTransaction, SignedTransaction) {
 		let tx1 = new_unsigned_tx(nonce, default_gas_val(), gas_price);
 		let tx2 = new_unsigned_tx(nonce + nonce_increment, default_gas_val(), gas_price + gas_price_increment);
@@ -1155,14 +1177,14 @@ mod test {
 		let (tx1, tx2) = new_tx_pair(123.into(), 1.into(), 1.into(), 0.into());
 		let sender = tx1.sender().unwrap();
 		let nonce = tx1.nonce;
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 2);
 		assert_eq!(txq.last_nonce(&sender), Some(nonce + 1.into()));
 
 		// when
 		let tx = new_tx(123.into(), 1.into());
-		let res = txq.add(tx.clone(), &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		// No longer the case as we don't even consider a transaction that isn't above a full
@@ -1318,12 +1340,12 @@ mod test {
 			!U256::zero() };
 
 		// First insert one transaction to future
-		let res = txq.add(tx, &prev_nonce, TransactionOrigin::External);
+		let res = txq.add(tx, TransactionOrigin::External, &prev_nonce, &gas_estimator);
 		assert_eq!(res.unwrap(), TransactionImportResult::Future);
 		assert_eq!(txq.status().future, 1);
 
 		// now import second transaction to current
-		let res = txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// and then there should be only one transaction in current (the one with higher gas_price)
 		assert_eq!(res.unwrap(), TransactionImportResult::Current);
@@ -1343,12 +1365,12 @@ mod test {
 			!U256::zero() };
 
 		// First insert one transaction to future
-		let res = txq.add(tx.clone(), &prev_nonce, TransactionOrigin::External);
+		let res = txq.add(tx.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator);
 		assert_eq!(res.unwrap(), TransactionImportResult::Future);
 		assert_eq!(txq.status().future, 1);
 
 		// now import second transaction to current
-		let res = txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(res.unwrap(), TransactionImportResult::Current);
@@ -1367,7 +1389,7 @@ mod test {
 		let tx = new_tx_default();
 
 		// when
-		let res = txq.add(tx, &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx, TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(res.unwrap(), TransactionImportResult::Current);
@@ -1386,10 +1408,10 @@ mod test {
 		txq.set_minimal_gas_price(15.into());
 
 		// when
-		let res1 = txq.add(tx1, &default_account_details, TransactionOrigin::External);
-		let res2 = txq.add(tx2, &default_account_details, TransactionOrigin::External);
-		let res3 = txq.add(tx3, &default_account_details, TransactionOrigin::External);
-		let res4 = txq.add(tx4, &default_account_details, TransactionOrigin::External);
+		let res1 = txq.add(tx1, TransactionOrigin::External, &default_account_details, &gas_estimator);
+		let res2 = txq.add(tx2, TransactionOrigin::External, &default_account_details, &gas_estimator);
+		let res3 = txq.add(tx3, TransactionOrigin::External, &default_account_details, &gas_estimator);
+		let res4 = txq.add(tx4, TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(res1.unwrap(), TransactionImportResult::Current);
@@ -1420,10 +1442,10 @@ mod test {
 		txq.set_minimal_gas_price(15.into());
 
 		// when
-		let res1 = txq.add(tx1, &default_account_details, TransactionOrigin::External);
-		let res2 = txq.add(tx2, &default_account_details, TransactionOrigin::External);
-		let res3 = txq.add(tx3, &default_account_details, TransactionOrigin::External);
-		let res4 = txq.add(tx4, &default_account_details, TransactionOrigin::External);
+		let res1 = txq.add(tx1, TransactionOrigin::External, &default_account_details, &gas_estimator);
+		let res2 = txq.add(tx2, TransactionOrigin::External, &default_account_details, &gas_estimator);
+		let res3 = txq.add(tx3, TransactionOrigin::External, &default_account_details, &gas_estimator);
+		let res4 = txq.add(tx4, TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(res1.unwrap(), TransactionImportResult::Current);
@@ -1466,7 +1488,7 @@ mod test {
 		txq.set_gas_limit(limit);
 
 		// when
-		let res = txq.add(tx, &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx, TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(unwrap_tx_err(res), TransactionError::GasLimitExceeded {
@@ -1490,7 +1512,7 @@ mod test {
 		};
 
 		// when
-		let res = txq.add(tx, &account, TransactionOrigin::External);
+		let res = txq.add(tx, TransactionOrigin::External, &account, &gas_estimator);
 
 		// then
 		assert_eq!(unwrap_tx_err(res), TransactionError::InsufficientBalance {
@@ -1510,7 +1532,7 @@ mod test {
 		txq.set_minimal_gas_price(tx.gas_price + U256::one());
 
 		// when
-		let res = txq.add(tx, &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx, TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(unwrap_tx_err(res), TransactionError::InsufficientGasPrice {
@@ -1530,7 +1552,7 @@ mod test {
 		txq.set_minimal_gas_price(tx.gas_price + U256::one());
 
 		// when
-		let res = txq.add(tx, &default_account_details, TransactionOrigin::Local);
+		let res = txq.add(tx, TransactionOrigin::Local, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(res.unwrap(), TransactionImportResult::Current);
@@ -1560,7 +1582,7 @@ mod test {
 			rlp::decode(s.as_raw())
 		};
 		// when
-		let res = txq.add(stx, &default_account_details, TransactionOrigin::External);
+		let res = txq.add(stx, TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		assert!(res.is_err());
@@ -1574,8 +1596,8 @@ mod test {
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 
 		// when
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let top = txq.top_transactions();
@@ -1594,9 +1616,9 @@ mod test {
 
 		// when
 		// first insert the one with higher gas price
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		// then the one with lower gas price, but local
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let top = txq.top_transactions();
@@ -1615,9 +1637,9 @@ mod test {
 
 		// when
 		// first insert local one with higher gas price
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
 		// then the one with lower gas price, but from retracted block
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::RetractedBlock).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::RetractedBlock, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let top = txq.top_transactions();
@@ -1633,8 +1655,8 @@ mod test {
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 
 		// when
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let top = txq.top_transactions();
@@ -1653,10 +1675,10 @@ mod test {
 		let (tx1, tx2) = new_tx_pair_with_gas_price_increment(3.into());
 
 		// insert everything
-		txq.add(txa.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
-		txq.add(txb.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
-		txq.add(tx1.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
+		txq.add(txa.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap();
+		txq.add(txb.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap();
 
 		assert_eq!(txq.status().future, 4);
 
@@ -1682,10 +1704,10 @@ mod test {
 		let (tx1, tx2) = new_tx_pair_with_gas_price_increment(3.into());
 
 		// insert everything
-		txq.add(txa.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(txb.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(txa.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(txb.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		let top = txq.top_transactions();
 		assert_eq!(top[0], tx1);
@@ -1714,8 +1736,8 @@ mod test {
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 
 		// when
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let top = txq.pending_hashes();
@@ -1732,8 +1754,8 @@ mod test {
 		let (tx, tx2) = new_tx_pair_default(2.into(), 0.into());
 
 		// when
-		let res1 = txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		let res2 = txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		let res1 = txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		let res2 = txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		assert_eq!(res1, TransactionImportResult::Current);
@@ -1756,8 +1778,8 @@ mod test {
 		let mut txq = TransactionQueue::default();
 
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
-		txq.add(tx.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &prev_nonce, TransactionOrigin::External).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 2);
 
 		// when
@@ -1779,13 +1801,13 @@ mod test {
 		let tx1 = new_unsigned_tx(124.into(), default_gas_val(), 1.into()).sign(secret);
 		let tx2 = new_unsigned_tx(125.into(), default_gas_val(), 1.into()).sign(secret);
 
-		txq.add(tx, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 1);
-		txq.add(tx2, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 1);
 
 		// when
-		txq.add(tx1, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let stats = txq.status();
@@ -1801,8 +1823,8 @@ mod test {
 		// given
 		let mut txq2 = TransactionQueue::default();
 		let (tx, tx2) = new_tx_pair_default(3.into(), 0.into());
-		txq2.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq2.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq2.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq2.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq2.status().pending, 1);
 		assert_eq!(txq2.status().future, 1);
 
@@ -1823,10 +1845,10 @@ mod test {
 		let mut txq = TransactionQueue::default();
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 		let tx3 = new_tx_default();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 1);
-		txq.add(tx3.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx3.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 3);
 
 		// when
@@ -1845,8 +1867,8 @@ mod test {
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 
 		// add
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		let stats = txq.status();
 		assert_eq!(stats.pending, 2);
 
@@ -1865,11 +1887,11 @@ mod test {
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 		let sender = tx.sender().unwrap();
 		let nonce = tx.nonce;
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 1);
 
 		// when
-		let res = txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External);
+		let res = txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator);
 
 		// then
 		let t = txq.top_transactions();
@@ -1886,14 +1908,14 @@ mod test {
 		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());
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx3.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx3.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 2);
 
 		// when
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 1);
-		txq.add(tx4.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx4.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		assert_eq!(txq.status().future, 1);
@@ -1904,11 +1926,11 @@ mod test {
 		let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 100, default_gas_val() * U256::from(2), !U256::zero());
 		let (tx1, tx2) = new_tx_pair_default(U256::from(1), U256::from(1));
 		let (tx3, tx4) = new_tx_pair_default(U256::from(1), U256::from(2));
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx3.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx3.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		// limited by gas
-		txq.add(tx4.clone(), &default_account_details, TransactionOrigin::External).unwrap_err();
+		txq.add(tx4.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap_err();
 		assert_eq!(txq.status().pending, 2);
 	}
 
@@ -1918,13 +1940,13 @@ mod test {
 		let (tx1, tx2) = new_tx_pair_default(U256::from(1), U256::from(1));
 		let (tx3, tx4) = new_tx_pair_default(U256::from(1), U256::from(2));
 		let (tx5, tx6) = new_tx_pair_default(U256::from(1), U256::from(2));
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
-		txq.add(tx5.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx5.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		// Not accepted because of limit
-		txq.add(tx6.clone(), &default_account_details, TransactionOrigin::External).unwrap_err();
-		txq.add(tx3.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
-		txq.add(tx4.clone(), &default_account_details, TransactionOrigin::Local).unwrap();
+		txq.add(tx6.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap_err();
+		txq.add(tx3.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx4.clone(), TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 4);
 	}
 
@@ -1936,7 +1958,7 @@ mod test {
 		let fetch_last_nonce = |_a: &Address| AccountDetails { nonce: last_nonce, balance: !U256::zero() };
 
 		// when
-		let res = txq.add(tx, &fetch_last_nonce, TransactionOrigin::External);
+		let res = txq.add(tx, TransactionOrigin::External, &fetch_last_nonce, &gas_estimator);
 
 		// then
 		assert_eq!(unwrap_tx_err(res), TransactionError::Old);
@@ -1952,12 +1974,12 @@ mod test {
 			balance: !U256::zero() };
 		let mut txq = TransactionQueue::default();
 		let (_tx1, tx2) = new_tx_pair_default(1.into(), 0.into());
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 1);
 		assert_eq!(txq.status().pending, 0);
 
 		// when
-		let res = txq.add(tx2.clone(), &nonce, TransactionOrigin::External);
+		let res = txq.add(tx2.clone(), TransactionOrigin::External, &nonce, &gas_estimator);
 
 		// then
 		assert_eq!(unwrap_tx_err(res), TransactionError::AlreadyImported);
@@ -1971,15 +1993,15 @@ mod test {
 		// given
 		let mut txq = TransactionQueue::default();
 		let (tx1, tx2) = new_tx_pair_default(1.into(), 0.into());
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 2);
 
 		// when
 		txq.remove_invalid(&tx1.hash(), &default_account_details);
 		assert_eq!(txq.status().pending, 0);
 		assert_eq!(txq.status().future, 1);
-		txq.add(tx1.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let stats = txq.status();
@@ -1993,10 +2015,10 @@ mod test {
 		let mut txq = TransactionQueue::default();
 		let (tx, tx2) = new_tx_pair_default(1.into(), 0.into());
 		let tx3 = new_tx_default();
-		txq.add(tx2.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx2.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 1);
-		txq.add(tx3.clone(), &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx.clone(), &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx3.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx.clone(), TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().pending, 3);
 
 		// when
@@ -2023,8 +2045,8 @@ mod test {
 		};
 
 		// when
-		txq.add(tx, &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let stats = txq.status();
@@ -2051,10 +2073,10 @@ mod test {
 		};
 
 		// when
-		txq.add(tx1, &default_account_details, TransactionOrigin::External).unwrap();
-		txq.add(tx2, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx1, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 1);
-		txq.add(tx0, &default_account_details, TransactionOrigin::External).unwrap();
+		txq.add(tx0, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap();
 
 		// then
 		let stats = txq.status();
@@ -2072,8 +2094,8 @@ mod test {
 			!U256::zero() };
 		let mut txq = TransactionQueue::default();
 		let (tx1, tx2) = new_tx_pair_default(1.into(), 0.into());
-		txq.add(tx1.clone(), &previous_nonce, TransactionOrigin::External).unwrap();
-		txq.add(tx2, &previous_nonce, TransactionOrigin::External).unwrap();
+		txq.add(tx1.clone(), TransactionOrigin::External, &previous_nonce, &gas_estimator).unwrap();
+		txq.add(tx2, TransactionOrigin::External, &previous_nonce, &gas_estimator).unwrap();
 		assert_eq!(txq.status().future, 2);
 
 		// when
@@ -2104,7 +2126,7 @@ mod test {
 		let details = |_a: &Address| AccountDetails { nonce: nonce, balance: !U256::zero() };
 
 		// when
-		txq.add(tx, &details, TransactionOrigin::External).unwrap();
+		txq.add(tx, TransactionOrigin::External, &details, &gas_estimator).unwrap();
 
 		// then
 		assert_eq!(txq.last_nonce(&from), Some(nonce));
@@ -2119,7 +2141,7 @@ mod test {
 		let details1 = |_a: &Address| AccountDetails { nonce: nonce1, balance: !U256::zero() };
 
 		// Insert first transaction
-		txq.add(tx1, &details1, TransactionOrigin::External).unwrap();
+		txq.add(tx1, TransactionOrigin::External, &details1, &gas_estimator).unwrap();
 
 		// when
 		txq.remove_all(tx2.sender().unwrap(), nonce2 + U256::one());
@@ -2139,9 +2161,9 @@ mod test {
 
 		// when
 		// Insert first transaction
-		assert_eq!(txq.add(tx1, &details1, TransactionOrigin::External).unwrap(), TransactionImportResult::Current);
+		assert_eq!(txq.add(tx1, TransactionOrigin::External, &details1, &gas_estimator).unwrap(), TransactionImportResult::Current);
 		// Second should go to future
-		assert_eq!(txq.add(tx2, &details1, TransactionOrigin::External).unwrap(), TransactionImportResult::Future);
+		assert_eq!(txq.add(tx2, TransactionOrigin::External, &details1, &gas_estimator).unwrap(), TransactionImportResult::Future);
 		// Now block is imported
 		txq.remove_all(sender, nonce2 - U256::from(1));
 		// tx2 should be not be promoted to current
@@ -2160,9 +2182,9 @@ mod test {
 		assert_eq!(txq.has_local_pending_transactions(), false);
 
 		// when
-		assert_eq!(txq.add(tx1, &default_account_details, TransactionOrigin::External).unwrap(), TransactionImportResult::Current);
+		assert_eq!(txq.add(tx1, TransactionOrigin::External, &default_account_details, &gas_estimator).unwrap(), TransactionImportResult::Current);
 		assert_eq!(txq.has_local_pending_transactions(), false);
-		assert_eq!(txq.add(tx2, &default_account_details, TransactionOrigin::Local).unwrap(), TransactionImportResult::Current);
+		assert_eq!(txq.add(tx2, TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap(), TransactionImportResult::Current);
 
 		// then
 		assert_eq!(txq.has_local_pending_transactions(), true);
@@ -2177,8 +2199,8 @@ mod test {
 			default_account_details(a).balance };
 
 		// when
-		assert_eq!(txq.add(tx2, &prev_nonce, TransactionOrigin::External).unwrap(), TransactionImportResult::Future);
-		assert_eq!(txq.add(tx1.clone(), &prev_nonce, TransactionOrigin::External).unwrap(), TransactionImportResult::Future);
+		assert_eq!(txq.add(tx2, TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap(), TransactionImportResult::Future);
+		assert_eq!(txq.add(tx1.clone(), TransactionOrigin::External, &prev_nonce, &gas_estimator).unwrap(), TransactionImportResult::Future);
 
 		// then
 		assert_eq!(txq.future.by_priority.len(), 1);
@@ -2203,14 +2225,14 @@ mod test {
 			(tx.sign(secret), tx2.sign(secret), tx2_2.sign(secret), tx3.sign(secret))
 		};
 		let sender = tx1.sender().unwrap();
-		txq.add(tx1, &default_account_details, TransactionOrigin::Local).unwrap();
-		txq.add(tx2, &default_account_details, TransactionOrigin::Local).unwrap();
-		txq.add(tx3, &default_account_details, TransactionOrigin::Local).unwrap();
+		txq.add(tx1, TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx2, TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
+		txq.add(tx3, TransactionOrigin::Local, &default_account_details, &gas_estimator).unwrap();
 		assert_eq!(txq.future.by_priority.len(), 0);
 		assert_eq!(txq.current.by_priority.len(), 3);
 
 		// when
-		let res = txq.add(tx2_2, &default_account_details, TransactionOrigin::Local);
+		let res = txq.add(tx2_2, TransactionOrigin::Local, &default_account_details, &gas_estimator);
 
 		// then
 		assert_eq!(txq.last_nonce(&sender).unwrap(), 125.into());
@@ -2218,4 +2240,24 @@ mod test {
 		assert_eq!(txq.current.by_priority.len(), 3);
 	}
 
+	#[test]
+	fn should_reject_transactions_below_bas_gas() {
+		// given
+		let mut txq = TransactionQueue::default();
+		let (tx1, tx2) = new_tx_pair_default(1.into(), 0.into());
+		let high_gas = |_: &SignedTransaction| 100_001.into();
+
+		// when
+		let res1 = txq.add(tx1, TransactionOrigin::Local, &default_account_details, &gas_estimator);
+		let res2 = txq.add(tx2, TransactionOrigin::Local, &default_account_details, &high_gas);
+
+		// then
+		assert_eq!(res1.unwrap(), TransactionImportResult::Current);
+		assert_eq!(unwrap_tx_err(res2), TransactionError::InsufficientGas {
+			minimal: 100_001.into(),
+			got: 100_000.into(),
+		});
+
+	}
+
 }

rpc/src/v1/helpers/errors.rs

@@ -221,6 +221,9 @@ pub fn from_transaction_error(error: EthcoreError) -> Error {
 			LimitReached => {
 				"There are too many transactions in the queue. Your transaction was dropped due to limit. Try increasing the fee.".into()
 			},
+			InsufficientGas { minimal, got } => {
+				format!("Transaction gas is too low. There is not enough gas to cover minimal cost of the transaction (minimal: {}, got: {}). Try increasing supplied gas.", minimal, got)
+			},
 			InsufficientGasPrice { minimal, got } => {
 				format!("Transaction gas price is too low. It does not satisfy your node's minimal gas price (minimal: {}, got: {}). Try increasing the gas price.", minimal, got)
 			},