refactor(txpool): use transaction error type (#1698)

This commit is contained in:
Matthias Seitz
2023-03-10 15:57:12 +01:00
committed by GitHub
parent 89514d70f2
commit fdfeeb42dc
9 changed files with 158 additions and 110 deletions

1
Cargo.lock generated
View File

@ -5149,7 +5149,6 @@ dependencies = [
"parking_lot 0.12.1",
"paste",
"rand 0.8.5",
"reth-interfaces",
"reth-metrics-derive",
"reth-primitives",
"reth-provider",

View File

@ -4,7 +4,7 @@ use crate::result::{internal_rpc_err, rpc_err};
use jsonrpsee::{core::Error as RpcError, types::error::INVALID_PARAMS_CODE};
use reth_primitives::{constants::SELECTOR_LEN, Address, U128, U256};
use reth_rpc_types::{error::EthRpcErrorCode, BlockError};
use reth_transaction_pool::error::PoolError;
use reth_transaction_pool::error::{InvalidPoolTransactionError, PoolError};
use revm::primitives::{EVMError, Halt};
/// Result alias
@ -22,7 +22,7 @@ pub(crate) enum EthApiError {
#[error("Invalid transaction signature")]
InvalidTransactionSignature,
#[error(transparent)]
PoolError(GethTxPoolError),
PoolError(RpcPoolError),
#[error("Unknown block number")]
UnknownBlockNumber,
#[error("Invalid block range")]
@ -266,9 +266,9 @@ impl std::fmt::Display for RevertError {
impl std::error::Error for RevertError {}
/// A helper error type that mirrors `geth` Txpool's error messages
/// A helper error type that's mainly used to mirror `geth` Txpool's error messages
#[derive(Debug, thiserror::Error)]
pub(crate) enum GethTxPoolError {
pub(crate) enum RpcPoolError {
#[error("already known")]
AlreadyKnown,
#[error("invalid sender")]
@ -285,26 +285,28 @@ pub(crate) enum GethTxPoolError {
NegativeValue,
#[error("oversized data")]
OversizedData,
#[error(transparent)]
Invalid(#[from] InvalidPoolTransactionError),
#[error(transparent)]
Other(Box<dyn std::error::Error + Send + Sync>),
}
impl From<PoolError> for GethTxPoolError {
fn from(err: PoolError) -> GethTxPoolError {
impl From<PoolError> for RpcPoolError {
fn from(err: PoolError) -> RpcPoolError {
match err {
PoolError::ReplacementUnderpriced(_) => GethTxPoolError::ReplaceUnderpriced,
PoolError::ProtocolFeeCapTooLow(_, _) => GethTxPoolError::Underpriced,
PoolError::SpammerExceededCapacity(_, _) => GethTxPoolError::TxPoolOverflow,
PoolError::DiscardedOnInsert(_) => GethTxPoolError::TxPoolOverflow,
PoolError::TxExceedsGasLimit(_, _, _) => GethTxPoolError::GasLimit,
PoolError::TxExceedsMaxInitCodeSize(_, _, _) => GethTxPoolError::OversizedData,
PoolError::AccountNotFound(_) => GethTxPoolError::InvalidSender,
PoolError::OversizedData(_, _, _) => GethTxPoolError::OversizedData,
PoolError::ReplacementUnderpriced(_) => RpcPoolError::ReplaceUnderpriced,
PoolError::ProtocolFeeCapTooLow(_, _) => RpcPoolError::Underpriced,
PoolError::SpammerExceededCapacity(_, _) => RpcPoolError::TxPoolOverflow,
PoolError::DiscardedOnInsert(_) => RpcPoolError::TxPoolOverflow,
PoolError::InvalidTransaction(_, err) => err.into(),
PoolError::Other(_, err) => RpcPoolError::Other(err),
}
}
}
impl From<PoolError> for EthApiError {
fn from(err: PoolError) -> Self {
EthApiError::PoolError(GethTxPoolError::from(err))
EthApiError::PoolError(RpcPoolError::from(err))
}
}

View File

@ -20,7 +20,6 @@ normal = [
# reth
reth-primitives = { path = "../primitives" }
reth-provider = { path = "../storage/provider" }
reth-interfaces = { path = "../interfaces" }
reth-rlp = { path = "../rlp" }
# async/futures

View File

@ -1,6 +1,6 @@
//! Transaction pool errors
use reth_primitives::{Address, TxHash};
use reth_primitives::{Address, InvalidTransactionError, TxHash};
/// Transaction pool result type.
pub type PoolResult<T> = Result<T, PoolError>;
@ -21,22 +21,13 @@ pub enum PoolError {
/// respect the size limits of the pool.
#[error("[{0:?}] Transaction discarded outright due to pool size constraints.")]
DiscardedOnInsert(TxHash),
/// Thrown when a new transaction is added to the pool, but then immediately discarded to
/// respect the size limits of the pool.
#[error("[{0:?}] Transaction's gas limit {1} exceeds block's gas limit {2}.")]
TxExceedsGasLimit(TxHash, u64, u64),
/// Thrown when a new transaction is added to the pool, but then immediately discarded to
/// respect the max_init_code_size.
#[error("[{0:?}] Transaction's size {1} exceeds max_init_code_size {2}.")]
TxExceedsMaxInitCodeSize(TxHash, usize, usize),
/// Thrown if the transaction contains an invalid signature
#[error("[{0:?}]: Invalid sender")]
AccountNotFound(TxHash),
/// Thrown if the input data of a transaction is greater
/// than some meaningful limit a user might use. This is not a consensus error
/// making the transaction invalid, rather a DOS protection.
#[error("[{0:?}]: Input data too large")]
OversizedData(TxHash, usize, usize),
/// Thrown when the transaction is considered invalid.
#[error("[{0:?}] {1:?}")]
InvalidTransaction(TxHash, InvalidPoolTransactionError),
/// Any other error that occurred while inserting/validating a transaction. e.g. IO database
/// error
#[error("[{0:?}] {1:?}")]
Other(TxHash, Box<dyn std::error::Error + Send + Sync>),
}
// === impl PoolError ===
@ -49,10 +40,34 @@ impl PoolError {
PoolError::ProtocolFeeCapTooLow(hash, _) => hash,
PoolError::SpammerExceededCapacity(_, hash) => hash,
PoolError::DiscardedOnInsert(hash) => hash,
PoolError::TxExceedsGasLimit(hash, _, _) => hash,
PoolError::TxExceedsMaxInitCodeSize(hash, _, _) => hash,
PoolError::AccountNotFound(hash) => hash,
PoolError::OversizedData(hash, _, _) => hash,
PoolError::InvalidTransaction(hash, _) => hash,
PoolError::Other(hash, _) => hash,
}
}
}
/// Represents errors that can happen when validating transactions for the pool
///
/// See [TransactionValidator](crate::TransactionValidator).
#[derive(Debug, Clone, thiserror::Error)]
pub enum InvalidPoolTransactionError {
/// Hard consensus errors
#[error(transparent)]
Consensus(#[from] InvalidTransactionError),
/// Thrown when a new transaction is added to the pool, but then immediately discarded to
/// respect the size limits of the pool.
#[error("Transaction's gas limit {0} exceeds block's gas limit {1}.")]
ExceedsGasLimit(u64, u64),
/// Thrown when a new transaction is added to the pool, but then immediately discarded to
/// respect the max_init_code_size.
#[error("Transaction's size {0} exceeds max_init_code_size {1}.")]
ExceedsMaxInitCodeSize(usize, usize),
/// Thrown if the transaction contains an invalid signature
#[error("Invalid sender")]
AccountNotFound,
/// Thrown if the input data of a transaction is greater
/// than some meaningful limit a user might use. This is not a consensus error
/// making the transaction invalid, rather a DOS protection.
#[error("Input data too large")]
OversizedData(usize, usize),
}

View File

@ -86,7 +86,10 @@ pub use crate::{
BestTransactions, OnNewBlockEvent, PoolTransaction, PooledTransaction, PropagateKind,
PropagatedTransactions, TransactionOrigin, TransactionPool,
},
validate::{TransactionValidationOutcome, TransactionValidator, ValidPoolTransaction},
validate::{
EthTransactionValidator, TransactionValidationOutcome, TransactionValidator,
ValidPoolTransaction,
},
};
use crate::{
error::PoolResult,
@ -94,7 +97,7 @@ use crate::{
traits::{NewTransactionEvent, PoolSize},
};
use reth_interfaces::consensus::ConsensusError;
use crate::error::PoolError;
use reth_primitives::{TxHash, U256};
use std::{collections::HashMap, sync::Arc};
use tokio::sync::mpsc::Receiver;
@ -164,9 +167,7 @@ where
&self,
origin: TransactionOrigin,
transactions: impl IntoIterator<Item = V::Transaction>,
) -> PoolResult<
HashMap<TxHash, Result<TransactionValidationOutcome<V::Transaction>, ConsensusError>>,
> {
) -> PoolResult<HashMap<TxHash, TransactionValidationOutcome<V::Transaction>>> {
let outcome = futures_util::future::join_all(
transactions.into_iter().map(|tx| self.validate(origin, tx)),
)
@ -182,7 +183,7 @@ where
&self,
origin: TransactionOrigin,
transaction: V::Transaction,
) -> (TxHash, Result<TransactionValidationOutcome<V::Transaction>, ConsensusError>) {
) -> (TxHash, TransactionValidationOutcome<V::Transaction>) {
let hash = *transaction.hash();
// TODO(mattsse): this is where additional validate checks would go, like banned senders
@ -229,18 +230,14 @@ where
let (_, tx) = self.validate(origin, transaction).await;
match tx {
Ok(TransactionValidationOutcome::Valid {
balance: _,
state_nonce: _,
transaction: _,
}) => self
.pool
.add_transactions(origin, std::iter::once(tx.unwrap()))
.pop()
.expect("exists; qed"),
Ok(TransactionValidationOutcome::Invalid(_transaction, error)) => Err(error),
Err(_err) => {
unimplemented!()
TransactionValidationOutcome::Valid { .. } => {
self.pool.add_transactions(origin, std::iter::once(tx)).pop().expect("exists; qed")
}
TransactionValidationOutcome::Invalid(transaction, error) => {
Err(PoolError::InvalidTransaction(*transaction.hash(), error))
}
TransactionValidationOutcome::Error(transaction, error) => {
Err(PoolError::Other(*transaction.hash(), error))
}
}
}
@ -252,8 +249,7 @@ where
) -> PoolResult<Vec<PoolResult<TxHash>>> {
let validated = self.validate_all(origin, transactions).await?;
let transactions =
self.pool.add_transactions(origin, validated.into_values().map(|x| x.unwrap()));
let transactions = self.pool.add_transactions(origin, validated.into_values());
Ok(transactions)
}

View File

@ -232,7 +232,12 @@ where
TransactionValidationOutcome::Invalid(tx, err) => {
let mut listener = self.event_listener.write();
listener.discarded(tx.hash());
Err(err)
Err(PoolError::InvalidTransaction(*tx.hash(), err))
}
TransactionValidationOutcome::Error(tx, err) => {
let mut listener = self.event_listener.write();
listener.discarded(tx.hash());
Err(PoolError::Other(*tx.hash(), err))
}
}
}

View File

@ -1,7 +1,7 @@
//! The internal transaction pool implementation.
use crate::{
config::MAX_ACCOUNT_SLOTS_PER_SENDER,
error::PoolError,
error::{InvalidPoolTransactionError, PoolError},
identifier::{SenderId, TransactionId},
metrics::TxPoolMetrics,
pool::{
@ -220,8 +220,6 @@ impl<T: TransactionOrdering> TxPool<T> {
.or_default()
.update(on_chain_nonce, on_chain_balance);
let _hash = *tx.hash();
match self.all_transactions.insert_tx(tx, on_chain_balance, on_chain_nonce) {
Ok(InsertOk { transaction, move_to, replaced_tx, updates, .. }) => {
self.add_new_transaction(transaction.clone(), replaced_tx, move_to);
@ -263,10 +261,9 @@ impl<T: TransactionOrdering> TxPool<T> {
transaction,
block_gas_limit,
tx_gas_limit,
} => Err(PoolError::TxExceedsGasLimit(
} => Err(PoolError::InvalidTransaction(
*transaction.hash(),
block_gas_limit,
tx_gas_limit,
InvalidPoolTransactionError::ExceedsGasLimit(block_gas_limit, tx_gas_limit),
)),
}
}

View File

@ -9,7 +9,6 @@ use crate::{
};
use async_trait::async_trait;
pub use mock::*;
use reth_interfaces::consensus::ConsensusError;
use std::{marker::PhantomData, sync::Arc};
/// A [Pool] used for testing
@ -37,12 +36,12 @@ impl<T: PoolTransaction> TransactionValidator for NoopTransactionValidator<T> {
&self,
origin: TransactionOrigin,
transaction: Self::Transaction,
) -> Result<TransactionValidationOutcome<Self::Transaction>, ConsensusError> {
Ok(TransactionValidationOutcome::Valid {
) -> TransactionValidationOutcome<Self::Transaction> {
TransactionValidationOutcome::Valid {
balance: Default::default(),
state_nonce: 0,
transaction,
})
}
}
}

View File

@ -1,12 +1,11 @@
//! Transaction validation abstractions.
use crate::{
error::PoolError,
error::InvalidPoolTransactionError,
identifier::{SenderId, TransactionId},
traits::{PoolTransaction, TransactionOrigin},
MAX_INIT_CODE_SIZE, TX_MAX_SIZE,
};
use reth_interfaces::consensus::ConsensusError;
use reth_primitives::{
Address, InvalidTransactionError, TransactionKind, TxHash, EIP1559_TX_TYPE_ID,
EIP2930_TX_TYPE_ID, LEGACY_TX_TYPE_ID, U256,
@ -28,7 +27,9 @@ pub enum TransactionValidationOutcome<T: PoolTransaction> {
},
/// The transaction is considered invalid indefinitely: It violates constraints that prevent
/// this transaction from ever becoming valid.
Invalid(T, PoolError),
Invalid(T, InvalidPoolTransactionError),
/// An error occurred while trying to validate the transaction
Error(T, Box<dyn std::error::Error + Send + Sync>),
}
/// Provides support for validating transaction at any given state of the chain
@ -59,7 +60,7 @@ pub trait TransactionValidator: Send + Sync {
&self,
origin: TransactionOrigin,
transaction: Self::Transaction,
) -> Result<TransactionValidationOutcome<Self::Transaction>, ConsensusError>;
) -> TransactionValidationOutcome<Self::Transaction>;
/// Ensure that the code size is not greater than `max_init_code_size`.
/// `max_init_code_size` should be configurable so this will take it as an argument.
@ -67,11 +68,10 @@ pub trait TransactionValidator: Send + Sync {
&self,
transaction: Self::Transaction,
max_init_code_size: usize,
) -> Result<(), PoolError> {
) -> Result<(), InvalidPoolTransactionError> {
if *transaction.kind() == TransactionKind::Create && transaction.size() > max_init_code_size
{
Err(PoolError::TxExceedsMaxInitCodeSize(
*transaction.hash(),
Err(InvalidPoolTransactionError::ExceedsMaxInitCodeSize(
transaction.size(),
max_init_code_size,
))
@ -81,8 +81,9 @@ pub trait TransactionValidator: Send + Sync {
}
}
/// TODO: Add docs and make this public
pub(crate) struct EthTransactionValidatorConfig<Client: AccountProvider> {
/// A [TransactionValidator] implementation that validates ethereum transaction.
#[derive(Debug, Clone)]
pub struct EthTransactionValidator<Client> {
/// Chain id
chain_id: u64,
/// This type fetches account info from the db
@ -101,7 +102,7 @@ pub(crate) struct EthTransactionValidatorConfig<Client: AccountProvider> {
#[async_trait::async_trait]
impl<T: PoolTransaction + AccountProvider + Clone> TransactionValidator
for EthTransactionValidatorConfig<T>
for EthTransactionValidator<T>
{
type Transaction = T;
@ -109,7 +110,7 @@ impl<T: PoolTransaction + AccountProvider + Clone> TransactionValidator
&self,
origin: TransactionOrigin,
transaction: Self::Transaction,
) -> Result<TransactionValidationOutcome<Self::Transaction>, ConsensusError> {
) -> TransactionValidationOutcome<Self::Transaction> {
// Checks for tx_type
match transaction.tx_type() {
LEGACY_TX_TYPE_ID => {
@ -119,101 +120,136 @@ impl<T: PoolTransaction + AccountProvider + Clone> TransactionValidator
EIP2930_TX_TYPE_ID => {
// Accept only legacy transactions until EIP-2718/2930 activates
if !self.eip2718 {
return Err(InvalidTransactionError::Eip2930Disabled.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::Eip1559Disabled.into(),
)
}
}
EIP1559_TX_TYPE_ID => {
// Reject dynamic fee transactions until EIP-1559 activates.
if !self.eip1559 {
return Err(InvalidTransactionError::Eip1559Disabled.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::Eip1559Disabled.into(),
)
}
}
_ => return Err(InvalidTransactionError::TxTypeNotSupported.into()),
_ => {
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::TxTypeNotSupported.into(),
)
}
};
// Reject transactions over defined size to prevent DOS attacks
if transaction.size() > TX_MAX_SIZE {
return Ok(TransactionValidationOutcome::Invalid(
return TransactionValidationOutcome::Invalid(
transaction.clone(),
PoolError::OversizedData(*transaction.hash(), transaction.size(), TX_MAX_SIZE),
))
InvalidPoolTransactionError::OversizedData(transaction.size(), TX_MAX_SIZE),
)
}
// Check whether the init code size has been exceeded.
if self.shanghai {
match self.ensure_max_init_code_size(transaction.clone(), MAX_INIT_CODE_SIZE) {
Ok(_) => {}
Err(e) => return Ok(TransactionValidationOutcome::Invalid(transaction, e)),
if let Err(err) =
self.ensure_max_init_code_size(transaction.clone(), MAX_INIT_CODE_SIZE)
{
return TransactionValidationOutcome::Invalid(transaction, err)
}
}
// Checks for gas limit
if transaction.gas_limit() > self.current_max_gas_limit {
return Ok(TransactionValidationOutcome::Invalid(
return TransactionValidationOutcome::Invalid(
transaction.clone(),
PoolError::TxExceedsGasLimit(
*transaction.hash(),
InvalidPoolTransactionError::ExceedsGasLimit(
transaction.gas_limit(),
self.current_max_gas_limit,
),
))
)
}
// Ensure max_fee_per_gas is greater than or equal to max_priority_fee_per_gas.
if transaction.max_fee_per_gas() <= transaction.max_priority_fee_per_gas() {
return Err(InvalidTransactionError::TipAboveFeeCap.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::TipAboveFeeCap.into(),
)
}
// Drop non-local transactions under our own minimal accepted gas price or tip
if !origin.is_local() && transaction.max_fee_per_gas() < self.gas_price {
return Err(InvalidTransactionError::MaxFeeLessThenBaseFee.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::MaxFeeLessThenBaseFee.into(),
)
}
// Checks for chainid
if transaction.chain_id() != Some(self.chain_id) {
return Err(InvalidTransactionError::ChainIdMismatch.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::ChainIdMismatch.into(),
)
}
let account = match self.client.basic_account(transaction.sender()).unwrap() {
let account = match self.client.basic_account(transaction.sender()) {
Ok(account) => account,
Err(err) => return TransactionValidationOutcome::Error(transaction, Box::new(err)),
};
let account = match account {
Some(account) => {
// Signer account shouldn't have bytecode. Presence of bytecode means this is a
// smartcontract.
if account.has_bytecode() {
return Err(InvalidTransactionError::SignerAccountHasBytecode.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::SignerAccountHasBytecode.into(),
)
} else {
account
}
}
None => {
return Ok(TransactionValidationOutcome::Invalid(
transaction.clone(),
PoolError::AccountNotFound(*transaction.hash()),
))
return TransactionValidationOutcome::Invalid(
transaction,
InvalidPoolTransactionError::AccountNotFound,
)
}
};
// Checks for nonce
if transaction.nonce() < account.nonce {
return Err(InvalidTransactionError::NonceNotConsistent.into())
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::NonceNotConsistent.into(),
)
}
// Checks for max cost
if transaction.cost() > account.balance {
return Err(InvalidTransactionError::InsufficientFunds {
max_fee: transaction.max_fee_per_gas().unwrap_or_default(),
available_funds: account.balance,
}
.into())
let max_fee = transaction.max_fee_per_gas().unwrap_or_default();
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::InsufficientFunds {
max_fee,
available_funds: account.balance,
}
.into(),
)
}
// Return the valid transaction
Ok(TransactionValidationOutcome::Valid {
TransactionValidationOutcome::Valid {
balance: account.balance,
state_nonce: account.nonce,
transaction,
})
}
}
}