From 1ba4df08f9c5dbadbbd287688fe6c709b1c4a7a0 Mon Sep 17 00:00:00 2001 From: Seun LanLege Date: Thu, 15 Aug 2019 15:48:41 +0100 Subject: [PATCH] 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 | 16 ++++++++++++---- miner/src/pool/queue.rs | 2 +- miner/src/pool/tests/mod.rs | 16 ++++++++-------- miner/src/pool/verifier.rs | 6 +++--- rpc/src/v1/helpers/errors.rs | 7 +++++-- 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/ethcore/types/src/transaction/error.rs b/ethcore/types/src/transaction/error.rs index a563347ce..1ad9d5358 100644 --- a/ethcore/types/src/transaction/error.rs +++ b/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, + /// new transaction's gas price + new: Option, + }, /// 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), diff --git a/miner/src/pool/queue.rs b/miner/src/pool/queue.rs index bc79d3424..a6dbe3450 100644 --- a/miner/src/pool/queue.rs +++ b/miner/src/pool/queue.rs @@ -594,7 +594,7 @@ fn convert_error(err: txpool::Error) -> 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 } } } diff --git a/miner/src/pool/tests/mod.rs b/miner/src/pool/tests/mod.rs index 0eb223dba..1df1be4ce 100644 --- a/miner/src/pool/tests/mod.rs +++ b/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 - // and minimal effective gas price taken into account. - Err(transaction::Error::InsufficientGasPrice { minimal: 2.into(), got: 1.into() }), - Ok(()) + // The error here indicates reaching the limit + // and minimal effective gas price taken into account. + Err(transaction::Error::TooCheapToReplace { prev: Some(2.into()), new: Some(1.into()) }), + 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 { - minimal: 2.into(), - got: 1.into(), + assert_eq!(res, vec![Err(transaction::Error::TooCheapToReplace { + prev: Some(2.into()), + new: Some(1.into()), })]); assert!(!client.was_verification_triggered()); diff --git a/miner/src/pool/verifier.rs b/miner/src/pool/verifier.rs index 5c8273dd5..79d50d713 100644 --- a/miner/src/pool/verifier.rs +++ b/miner/src/pool/verifier.rs @@ -231,9 +231,9 @@ impl txpool::Verifier for Verifier 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 => { - "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() + TooCheapToReplace { prev, new } => { + 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()