Better error message for rpc gas price errors (#10931)

* Better error message for rpc gas price errors

* correct tests

* dedupe error variants

* fixed tests, removed spacing

ethcore/types/src/transaction/error.rs

@@ -30,9 +30,6 @@ pub enum Error {
 	AlreadyImported,
 	/// Transaction is not valid anymore (state already has higher nonce)
 	Old,
-	/// Transaction has too low fee
-	/// (there is already a transaction with the same sender-nonce but higher gas price)
-	TooCheapToReplace,
 	/// Transaction was not imported to the queue because limit has been reached.
 	LimitReached,
 	/// Transaction's gas price is below threshold.
@@ -42,6 +39,14 @@ pub enum Error {
 		/// Transaction gas price
 		got: U256,
 	},
+	/// Transaction has too low fee
+	/// (there is already a transaction with the same sender-nonce but higher gas price)
+	TooCheapToReplace {
+		/// previous transaction's gas price
+		prev: Option<U256>,
+		/// new transaction's gas price
+		new: Option<U256>,
+	},
 	/// Transaction's gas is below currently set minimal gas requirement.
 	InsufficientGas {
 		/// Minimal expected gas
@@ -101,7 +106,10 @@ impl fmt::Display for Error {
 		let msg = match *self {
 			AlreadyImported => "Already imported".into(),
 			Old => "No longer valid".into(),
-			TooCheapToReplace => "Gas price too low to replace".into(),
+			TooCheapToReplace { prev, new } =>
+				format!("Gas price too low to replace, previous tx gas: {:?}, new tx gas: {:?}",
+						prev, new
+				),
 			LimitReached => "Transaction limit reached".into(),
 			InsufficientGasPrice { minimal, got } =>
 				format!("Insufficient gas price. Min={}, Given={}", minimal, got),

miner/src/pool/queue.rs

@@ -594,7 +594,7 @@ fn convert_error<H: fmt::Debug + fmt::LowerHex>(err: txpool::Error<H>) -> transa
 	match err {
 		Error::AlreadyImported(..) => transaction::Error::AlreadyImported,
 		Error::TooCheapToEnter(..) => transaction::Error::LimitReached,
-		Error::TooCheapToReplace(..) => transaction::Error::TooCheapToReplace
+		Error::TooCheapToReplace(..) => transaction::Error::TooCheapToReplace { prev: None, new: None }
 	}
 }
 

miner/src/pool/tests/mod.rs

@@ -91,10 +91,10 @@ fn should_return_correct_nonces_when_dropped_because_of_limit() {
 	// then
 	assert_eq!(res, vec![Ok(()), Ok(())]);
 	assert_eq!(res2, vec![
-			   // The error here indicates reaching the limit
+		// The error here indicates reaching the limit
-			   // and minimal effective gas price taken into account.
+		// and minimal effective gas price taken into account.
-			   Err(transaction::Error::InsufficientGasPrice { minimal: 2.into(), got: 1.into() }),
+		Err(transaction::Error::TooCheapToReplace { prev: Some(2.into()), new: Some(1.into()) }),
-			   Ok(())
+		Ok(())
 	]);
 	assert_eq!(txq.status().status.transaction_count, 3);
 	// tx2 transaction got dropped because of limit
@@ -585,7 +585,7 @@ fn should_not_replace_same_transaction_if_the_fee_is_less_than_minimal_bump() {
 	let res = txq.import(client.clone(), vec![tx2, tx4].local());
 
 	// then
-	assert_eq!(res, vec![Err(transaction::Error::TooCheapToReplace), Ok(())]);
+	assert_eq!(res, vec![Err(transaction::Error::TooCheapToReplace { prev: None, new: None }), Ok(())]);
 	assert_eq!(txq.status().status.transaction_count, 2);
 	assert_eq!(txq.pending(client.clone(), PendingSettings::all_prioritized(0, 0))[0].signed().gas_price, U256::from(20));
 	assert_eq!(txq.pending(client.clone(), PendingSettings::all_prioritized(0, 0))[1].signed().gas_price, U256::from(2));
@@ -1027,9 +1027,9 @@ fn should_reject_early_in_case_gas_price_is_less_than_min_effective() {
 	let client = TestClient::new();
 	let tx1 = Tx::default().signed().unverified();
 	let res = txq.import(client.clone(), vec![tx1]);
-	assert_eq!(res, vec![Err(transaction::Error::InsufficientGasPrice {
+	assert_eq!(res, vec![Err(transaction::Error::TooCheapToReplace {
-		minimal: 2.into(),
+		prev: Some(2.into()),
-		got: 1.into(),
+		new: Some(1.into()),
 	})]);
 	assert!(!client.was_verification_triggered());
 

miner/src/pool/verifier.rs

@@ -231,9 +231,9 @@ impl<C: Client> txpool::Verifier<Transaction> for Verifier<C, ::pool::scoring::N
 						tx.gas_price(),
 						vtx.transaction.gas_price,
 					);
-					return Err(transaction::Error::InsufficientGasPrice {
+					return Err(transaction::Error::TooCheapToReplace {
-						minimal: vtx.transaction.gas_price,
+						prev: Some(vtx.transaction.gas_price),
-						got: *tx.gas_price(),
+						new: Some(*tx.gas_price()),
 					});
 				}
 			}

rpc/src/v1/helpers/errors.rs

@@ -420,8 +420,11 @@ pub fn transaction_message(error: &TransactionError) -> String {
 	match *error {
 		AlreadyImported => "Transaction with the same hash was already imported.".into(),
 		Old => "Transaction nonce is too low. Try incrementing the nonce.".into(),
-		TooCheapToReplace => {
+		TooCheapToReplace { prev, new } => {
-			"Transaction gas price is too low. There is another transaction with same nonce in the queue. Try increasing the gas price or incrementing the nonce.".into()
+			format!("Transaction gas price {} is too low. There is another transaction with same nonce in the queue{}. Try increasing the gas price or incrementing the nonce.",
+					new.map(|gas| format!("{}wei", gas)).unwrap_or("supplied".into()),
+					prev.map(|gas| format!(" with gas price: {}wei", gas)).unwrap_or("".into())
+			)
 		}
 		LimitReached => {
 			"There are too many transactions in the queue. Your transaction was dropped due to limit. Try increasing the fee.".into()