Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rpc): add fee/value and balance to insufficient funds RPC error #10872

Merged
merged 6 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
28 changes: 21 additions & 7 deletions crates/rpc/rpc-eth-types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use std::time::Duration;

use alloy_primitives::{Address, Bytes};
use alloy_primitives::{Address, Bytes, U256};
use alloy_sol_types::decode_revert_reason;
use reth_errors::RethError;
use reth_primitives::{revm_primitives::InvalidHeader, BlockId};
Expand Down Expand Up @@ -305,8 +305,15 @@ pub enum RpcInvalidTransactionError {
#[error("max initcode size exceeded")]
MaxInitCodeSizeExceeded,
/// Represents the inability to cover max cost + value (account balance too low).
#[error("insufficient funds for gas * price + value")]
InsufficientFunds,
#[error(
"insufficient funds for gas * price + value: value or fee {value_or_fee}, balance {balance}"
emhane marked this conversation as resolved.
Show resolved Hide resolved
)]
InsufficientFunds {
/// Value or max fee.
value_or_fee: U256,
/// Current balance of transaction sender.
balance: U256,
},
/// Thrown when calculating gas usage
#[error("gas uint64 overflow")]
GasUintOverflow,
Expand Down Expand Up @@ -476,7 +483,9 @@ impl From<revm::primitives::InvalidTransaction> for RpcInvalidTransactionError {
InvalidTransaction::CallerGasLimitMoreThanBlock |
InvalidTransaction::CallGasCostMoreThanGasLimit => Self::GasTooHigh,
InvalidTransaction::RejectCallerWithCode => Self::SenderNoEOA,
InvalidTransaction::LackOfFundForMaxFee { .. } => Self::InsufficientFunds,
InvalidTransaction::LackOfFundForMaxFee { fee, balance } => {
Self::InsufficientFunds { value_or_fee: *fee, balance: *balance }
}
InvalidTransaction::OverflowPaymentInTransaction => Self::GasUintOverflow,
InvalidTransaction::NonceOverflowInTransaction => Self::NonceMaxValue,
InvalidTransaction::CreateInitCodeSizeLimit => Self::MaxInitCodeSizeExceeded,
Expand Down Expand Up @@ -518,7 +527,9 @@ impl From<reth_primitives::InvalidTransactionError> for RpcInvalidTransactionErr
// This conversion is used to convert any transaction errors that could occur inside the
// txpool (e.g. `eth_sendRawTransaction`) to their corresponding RPC
match err {
InvalidTransactionError::InsufficientFunds { .. } => Self::InsufficientFunds,
InvalidTransactionError::InsufficientFunds(res) => {
Self::InsufficientFunds { value_or_fee: res.expected, balance: res.got }
}
InvalidTransactionError::NonceNotConsistent { tx, state } => {
Self::NonceTooLow { tx, state }
}
Expand Down Expand Up @@ -678,8 +689,11 @@ impl From<InvalidPoolTransactionError> for RpcPoolError {
InvalidPoolTransactionError::Other(err) => Self::PoolTransactionError(err),
InvalidPoolTransactionError::Eip4844(err) => Self::Eip4844(err),
InvalidPoolTransactionError::Eip7702(err) => Self::Eip7702(err),
InvalidPoolTransactionError::Overdraft => {
Self::Invalid(RpcInvalidTransactionError::InsufficientFunds)
InvalidPoolTransactionError::Overdraft { value_or_fee, balance } => {
Self::Invalid(RpcInvalidTransactionError::InsufficientFunds {
value_or_fee,
balance,
})
}
}
}
Expand Down
23 changes: 13 additions & 10 deletions crates/rpc/rpc-eth-types/src/revm_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,19 @@ where
DB: Database,
EthApiError: From<<DB as Database>::Error>,
{
Ok(db
// Get the caller account.
.basic(env.caller)?
// Get the caller balance.
.map(|acc| acc.balance)
.unwrap_or_default()
// Subtract transferred value from the caller balance.
.checked_sub(env.value)
// Return error if the caller has insufficient funds.
.ok_or_else(|| RpcInvalidTransactionError::InsufficientFunds)?
// Get the caller account.
let caller = db.basic(env.caller)?;
// Get the caller balance.
let balance = caller.map(|acc| acc.balance).unwrap_or_default();
// Get transaction value.
let value = env.value;
// Subtract transferred value from the caller balance. Return error if the caller has
// insufficient funds.
let balance = balance.checked_sub(env.value).ok_or_else(|| {
RpcInvalidTransactionError::InsufficientFunds { value_or_fee: value, balance }
})?;

Ok(balance
// Calculate the amount of gas the caller can afford with the specified gas price.
.checked_div(env.gas_price)
// This will be 0 if gas price is 0. It is fine, because we check it before.
Expand Down
15 changes: 11 additions & 4 deletions crates/transaction-pool/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Transaction pool errors

use alloy_primitives::{Address, TxHash};
use alloy_primitives::{Address, TxHash, U256};
use reth_primitives::{BlobTransactionValidationError, InvalidTransactionError};

/// Transaction pool result type.
Expand Down Expand Up @@ -203,8 +203,15 @@ pub enum InvalidPoolTransactionError {
#[error("transaction underpriced")]
Underpriced,
/// Thrown if the transaction's would require an account to be overdrawn
#[error("transaction overdraws from account")]
Overdraft,
#[error(
"transaction overdraws from account, value or fee: {value_or_fee}, balance: {balance}"
)]
Overdraft {
/// Cost transaction is allowed to consume. See `reth_transaction_pool::PoolTransaction`.
value_or_fee: U256,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this isn't quite accurate because this is the cost this tx requires.

imo we can keep overdraft as is, this provides sufficient context

Copy link
Member Author

Choose a reason for hiding this comment

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

this isn't possible if we do the conversion from overdraft error to rpc error

/// Balance of account.
balance: U256,
},
/// EIP-4844 related errors
#[error(transparent)]
Eip4844(#[from] Eip4844PoolTransactionError),
Expand Down Expand Up @@ -274,7 +281,7 @@ impl InvalidPoolTransactionError {
false
}
Self::IntrinsicGasTooLow => true,
Self::Overdraft => false,
Self::Overdraft { .. } => false,
Self::Other(err) => err.is_bad_transaction(),
Self::Eip4844(eip4844_err) => {
match eip4844_err {
Expand Down
5 changes: 4 additions & 1 deletion crates/transaction-pool/src/pool/txpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,10 @@ impl<T: TransactionOrdering> TxPool<T> {
)),
InsertErr::Overdraft { transaction } => Err(PoolError::new(
*transaction.hash(),
PoolErrorKind::InvalidTransaction(InvalidPoolTransactionError::Overdraft),
PoolErrorKind::InvalidTransaction(InvalidPoolTransactionError::Overdraft {
value_or_fee: transaction.cost(),
balance: on_chain_balance,
}),
)),
InsertErr::TxTypeConflict { transaction } => Err(PoolError::new(
*transaction.hash(),
Expand Down
Loading