Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Better error message for rpc gas price errors #10931

Merged
merged 5 commits into from
Aug 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions ethcore/types/src/transaction/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to document this better: what is the difference between TooCheapToReplace and TxTooCheapToReplace – it is not clear to when we return the one or the other.

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
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion miner/src/pool/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand Down
16 changes: 8 additions & 8 deletions miner/src/pool/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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());

Expand Down
6 changes: 3 additions & 3 deletions miner/src/pool/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
minimal: vtx.transaction.gas_price,
got: *tx.gas_price(),
return Err(transaction::Error::TooCheapToReplace {
prev: Some(vtx.transaction.gas_price),
new: Some(*tx.gas_price()),
});
}
}
Expand Down
7 changes: 5 additions & 2 deletions rpc/src/v1/helpers/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
"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()
Expand Down