From ba96b9d1656a4460824af94ed459926be47d531d Mon Sep 17 00:00:00 2001 From: chirag-bgh <76247491+chirag-bgh@users.noreply.github.com> Date: Fri, 10 Mar 2023 15:05:59 +0530 Subject: [PATCH] feat: Add Latest State TransactionValidator implementation (#1498) --- Cargo.lock | 2 + crates/interfaces/src/consensus.rs | 37 ++++- crates/rpc/rpc/src/eth/error.rs | 2 + crates/transaction-pool/Cargo.toml | 2 + crates/transaction-pool/src/error.rs | 10 ++ crates/transaction-pool/src/lib.rs | 48 +++++- crates/transaction-pool/src/pool/mod.rs | 4 +- .../transaction-pool/src/test_utils/mock.rs | 4 + crates/transaction-pool/src/test_utils/mod.rs | 7 +- crates/transaction-pool/src/traits.rs | 8 + crates/transaction-pool/src/validate.rs | 150 +++++++++++++++++- 11 files changed, 257 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3abe264cc1..d66d9ac35c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5149,8 +5149,10 @@ dependencies = [ "parking_lot 0.12.1", "paste", "rand 0.8.5", + "reth-interfaces", "reth-metrics-derive", "reth-primitives", + "reth-provider", "reth-rlp", "ruint", "serde", diff --git a/crates/interfaces/src/consensus.rs b/crates/interfaces/src/consensus.rs index c940733b64..2c926c89eb 100644 --- a/crates/interfaces/src/consensus.rs +++ b/crates/interfaces/src/consensus.rs @@ -97,7 +97,7 @@ pub enum Error { #[error("Transaction nonce is not consistent.")] TransactionNonceNotConsistent, #[error("Account does not have enough funds ({available_funds:?}) to cover transaction max fee: {max_fee:?}.")] - InsufficientFunds { max_fee: u128, available_funds: u128 }, + InsufficientFunds { max_fee: u128, available_funds: U256 }, #[error("Eip2930 transaction is enabled after berlin hardfork.")] TransactionEip2930Disabled, #[error("Old legacy transaction before Spurious Dragon should not have chain_id.")] @@ -130,4 +130,39 @@ pub enum Error { WithdrawalIndexInvalid { got: u64, expected: u64 }, #[error("Missing withdrawals")] BodyWithdrawalsMissing, + /// Thrown when calculating gas usage + #[error("gas uint64 overflow")] + GasUintOverflow, + /// returned if the transaction is specified to use less gas than required to start the + /// invocation. + #[error("intrinsic gas too low")] + GasTooLow, + /// returned if the transaction gas exceeds the limit + #[error("intrinsic gas too high")] + GasTooHigh, + /// thrown if a transaction is not supported in the current network configuration. + #[error("transaction type not supported")] + TxTypeNotSupported, + /// Thrown to ensure no one is able to specify a transaction with a tip higher than the total + /// fee cap. + #[error("max priority fee per gas higher than max fee per gas")] + TipAboveFeeCap, + /// A sanity error to avoid huge numbers specified in the tip field. + #[error("max priority fee per gas higher than 2^256-1")] + TipVeryHigh, + /// A sanity error to avoid huge numbers specified in the fee cap field. + #[error("max fee per gas higher than 2^256-1")] + FeeCapVeryHigh, + /// Thrown post London if the transaction's fee is less than the base fee of the block + #[error("max fee per gas less than block base fee")] + FeeCapTooLow, + /// Thrown if the sender of a transaction is a contract. + #[error("sender not an eoa")] + SenderNoEOA, +} + +impl From for Error { + fn from(_: crate::error::Error) -> Self { + Error::TransactionSignerRecoveryError + } } diff --git a/crates/rpc/rpc/src/eth/error.rs b/crates/rpc/rpc/src/eth/error.rs index cca0dd39eb..8fe9eff1f4 100644 --- a/crates/rpc/rpc/src/eth/error.rs +++ b/crates/rpc/rpc/src/eth/error.rs @@ -296,6 +296,8 @@ impl From for GethTxPoolError { PoolError::DiscardedOnInsert(_) => GethTxPoolError::TxPoolOverflow, PoolError::TxExceedsGasLimit(_, _, _) => GethTxPoolError::GasLimit, PoolError::TxExceedsMaxInitCodeSize(_, _, _) => GethTxPoolError::OversizedData, + PoolError::AccountNotFound(_) => GethTxPoolError::InvalidSender, + PoolError::OversizedData(_, _, _) => GethTxPoolError::OversizedData, } } } diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index 34a09589b3..bde4c1da72 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -19,6 +19,8 @@ normal = [ # reth reth-primitives = { path = "../primitives" } +reth-provider = { path = "../storage/provider" } +reth-interfaces = { path = "../interfaces" } reth-rlp = { path = "../rlp" } # async/futures diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index ee8133787a..4048a37c41 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -29,6 +29,14 @@ pub enum PoolError { /// 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), } // === impl PoolError === @@ -43,6 +51,8 @@ impl PoolError { PoolError::DiscardedOnInsert(hash) => hash, PoolError::TxExceedsGasLimit(hash, _, _) => hash, PoolError::TxExceedsMaxInitCodeSize(hash, _, _) => hash, + PoolError::AccountNotFound(hash) => hash, + PoolError::OversizedData(hash, _, _) => hash, } } } diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index 3dd3ee2928..11ef1812f6 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -93,6 +93,8 @@ use crate::{ pool::PoolInner, traits::{NewTransactionEvent, PoolSize}, }; + +use reth_interfaces::consensus::Error; use reth_primitives::{TxHash, U256}; use std::{collections::HashMap, sync::Arc}; use tokio::sync::mpsc::Receiver; @@ -110,6 +112,24 @@ mod validate; /// Common test helpers for mocking A pool pub mod test_utils; +// TX_SLOT_SIZE is used to calculate how many data slots a single transaction +// takes up based on its size. The slots are used as DoS protection, ensuring +// that validating a new transaction remains a constant operation (in reality +// O(maxslots), where max slots are 4 currently). +pub(crate) const TX_SLOT_SIZE: usize = 32 * 1024; + +// TX_MAX_SIZE is the maximum size a single transaction can have. This field has +// non-trivial consequences: larger transactions are significantly harder and +// more expensive to propagate; larger transactions also take more resources +// to validate whether they fit into the pool or not. +pub(crate) const TX_MAX_SIZE: usize = 4 * TX_SLOT_SIZE; //128KB + +// Maximum bytecode to permit for a contract +pub(crate) const MAX_CODE_SIZE: usize = 24576; + +// Maximum initcode to permit in a creation transaction and create instructions +pub(crate) const MAX_INIT_CODE_SIZE: usize = 2 * MAX_CODE_SIZE; + /// A shareable, generic, customizable `TransactionPool` implementation. #[derive(Debug)] pub struct Pool { @@ -144,7 +164,8 @@ where &self, origin: TransactionOrigin, transactions: impl IntoIterator, - ) -> PoolResult>> { + ) -> PoolResult, Error>>> + { let outcome = futures_util::future::join_all( transactions.into_iter().map(|tx| self.validate(origin, tx)), ) @@ -160,10 +181,12 @@ where &self, origin: TransactionOrigin, transaction: V::Transaction, - ) -> (TxHash, TransactionValidationOutcome) { + ) -> (TxHash, Result, Error>) { let hash = *transaction.hash(); + // TODO(mattsse): this is where additional validate checks would go, like banned senders // etc... + let outcome = self.pool.validator().validate_transaction(origin, transaction).await; (hash, outcome) @@ -203,7 +226,22 @@ where transaction: Self::Transaction, ) -> PoolResult { let (_, tx) = self.validate(origin, transaction).await; - self.pool.add_transactions(origin, std::iter::once(tx)).pop().expect("exists; qed") + + 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!() + } + } } async fn add_transactions( @@ -212,7 +250,9 @@ where transactions: Vec, ) -> PoolResult>> { let validated = self.validate_all(origin, transactions).await?; - let transactions = self.pool.add_transactions(origin, validated.into_values()); + + let transactions = + self.pool.add_transactions(origin, validated.into_values().map(|x| x.unwrap())); Ok(transactions) } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 334dec5eb2..6ef8fc5312 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -1,7 +1,7 @@ //! Transaction Pool internals. //! -//! Incoming transactions are before they enter the pool first. The validation outcome can have 3 -//! states: +//! Incoming transactions validated are before they enter the pool first. The validation outcome can +//! have 3 states: //! //! 1. Transaction can _never_ be valid //! 2. Transaction is _currently_ valid diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index da03552b80..c9d1e5a239 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -356,6 +356,10 @@ impl PoolTransaction for MockTransaction { fn encoded_length(&self) -> usize { 0 } + + fn chain_id(&self) -> Option { + Some(1) + } } impl FromRecoveredTransaction for MockTransaction { diff --git a/crates/transaction-pool/src/test_utils/mod.rs b/crates/transaction-pool/src/test_utils/mod.rs index 7fd1da50bb..52e6ff2c21 100644 --- a/crates/transaction-pool/src/test_utils/mod.rs +++ b/crates/transaction-pool/src/test_utils/mod.rs @@ -9,6 +9,7 @@ use crate::{ }; use async_trait::async_trait; pub use mock::*; +use reth_interfaces::consensus::Error; use std::{marker::PhantomData, sync::Arc}; /// A [Pool] used for testing @@ -36,12 +37,12 @@ impl TransactionValidator for NoopTransactionValidator { &self, origin: TransactionOrigin, transaction: Self::Transaction, - ) -> TransactionValidationOutcome { - TransactionValidationOutcome::Valid { + ) -> Result, Error> { + Ok(TransactionValidationOutcome::Valid { balance: Default::default(), state_nonce: 0, transaction, - } + }) } } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 300699e861..697dbafc71 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -295,6 +295,9 @@ pub trait PoolTransaction: fmt::Debug + Send + Sync + FromRecoveredTransaction { /// Returns the length of the rlp encoded object fn encoded_length(&self) -> usize; + + /// Returns chain_id + fn chain_id(&self) -> Option; } /// The default [PoolTransaction] for the [Pool](crate::Pool). @@ -397,6 +400,11 @@ impl PoolTransaction for PooledTransaction { fn encoded_length(&self) -> usize { self.transaction.length() } + + /// Returns chain_id + fn chain_id(&self) -> Option { + self.transaction.chain_id() + } } impl FromRecoveredTransaction for PooledTransaction { diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index 7200d4f850..b2ebf339da 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -4,8 +4,14 @@ use crate::{ error::PoolError, identifier::{SenderId, TransactionId}, traits::{PoolTransaction, TransactionOrigin}, + MAX_INIT_CODE_SIZE, TX_MAX_SIZE, }; -use reth_primitives::{Address, TransactionKind, TxHash, U256}; +use reth_interfaces::consensus::Error; +use reth_primitives::{ + Address, TransactionKind, TxHash, EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID, LEGACY_TX_TYPE_ID, + U256, +}; +use reth_provider::AccountProvider; use std::{fmt, time::Instant}; /// A Result type returned after checking a transaction's validity. @@ -53,7 +59,7 @@ pub trait TransactionValidator: Send + Sync { &self, origin: TransactionOrigin, transaction: Self::Transaction, - ) -> TransactionValidationOutcome; + ) -> Result, Error>; /// 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. @@ -62,11 +68,6 @@ pub trait TransactionValidator: Send + Sync { transaction: Self::Transaction, max_init_code_size: usize, ) -> Result<(), PoolError> { - // TODO check whether we are in the Shanghai stage. - // if !self.shanghai { - // return Ok(()) - // } - if *transaction.kind() == TransactionKind::Create && transaction.size() > max_init_code_size { Err(PoolError::TxExceedsMaxInitCodeSize( @@ -80,6 +81,141 @@ pub trait TransactionValidator: Send + Sync { } } +/// TODO: Add docs and make this public +pub(crate) struct EthTransactionValidatorConfig { + /// Chain id + chain_id: u64, + /// This type fetches account info from the db + client: Client, + /// Fork indicator whether we are in the Shanghai stage. + shanghai: bool, + /// Fork indicator whether we are using EIP-2718 type transactions. + eip2718: bool, + /// Fork indicator whether we are using EIP-1559 type transactions. + eip1559: bool, + /// The current max gas limit + current_max_gas_limit: u64, + /// gasprice + gas_price: Option, +} + +#[async_trait::async_trait] +impl TransactionValidator + for EthTransactionValidatorConfig +{ + type Transaction = T; + + async fn validate_transaction( + &self, + origin: TransactionOrigin, + transaction: Self::Transaction, + ) -> Result, Error> { + // Checks for tx_type + match transaction.tx_type() { + LEGACY_TX_TYPE_ID => { + // Accept legacy transactions + } + + EIP2930_TX_TYPE_ID => { + // Accept only legacy transactions until EIP-2718/2930 activates + if !self.eip2718 { + return Err(Error::TransactionEip2930Disabled) + } + } + + EIP1559_TX_TYPE_ID => { + // Reject dynamic fee transactions until EIP-1559 activates. + if !self.eip1559 { + return Err(Error::TransactionEip1559Disabled) + } + } + + _ => return Err(Error::TxTypeNotSupported), + }; + + // Reject transactions over defined size to prevent DOS attacks + if transaction.size() > TX_MAX_SIZE { + return Ok(TransactionValidationOutcome::Invalid( + transaction.clone(), + PoolError::OversizedData(*transaction.hash(), 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)), + } + } + + // Checks for gas limit + if transaction.gas_limit() > self.current_max_gas_limit { + return Ok(TransactionValidationOutcome::Invalid( + transaction.clone(), + PoolError::TxExceedsGasLimit( + *transaction.hash(), + 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(Error::TipAboveFeeCap) + } + + // 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(Error::TransactionMaxFeeLessThenBaseFee) + } + + // Checks for chainid + if transaction.chain_id() != Some(self.chain_id) { + return Err(Error::TransactionChainId) + } + + let account = match self.client.basic_account(transaction.sender())? { + Some(account) => { + // Signer account shouldn't have bytecode. Presence of bytecode means this is a + // smartcontract. + if account.has_bytecode() { + return Err(Error::SignerAccountHasBytecode) + } else { + account + } + } + None => { + return Ok(TransactionValidationOutcome::Invalid( + transaction.clone(), + PoolError::AccountNotFound(*transaction.hash()), + )) + } + }; + + // Checks for nonce + if transaction.nonce() < account.nonce { + return Err(Error::TransactionNonceNotConsistent) + } + + // Checks for max cost + if transaction.cost() > account.balance { + return Err(Error::InsufficientFunds { + max_fee: transaction.max_fee_per_gas().unwrap_or_default(), + available_funds: account.balance, + }) + } + + // Return the valid transaction + Ok(TransactionValidationOutcome::Valid { + balance: account.balance, + state_nonce: account.nonce, + transaction, + }) + } +} + /// A valid transaction in the pool. pub struct ValidPoolTransaction { /// The transaction