refactor(txpool): consider below proto fee cap an error (#71)

crates/transaction-pool/src/error.rs

@@ -1,6 +1,6 @@
 //! Transaction pool errors
 
-use reth_primitives::BlockID;
+use reth_primitives::{BlockID, TxHash, U256};
 
 /// Transaction pool result type.
 pub type PoolResult<T> = Result<T, PoolError>;
@@ -9,15 +9,9 @@ pub type PoolResult<T> = Result<T, PoolError>;
 #[derive(Debug, thiserror::Error)]
 pub enum PoolError {
     /// Thrown if a replacement transaction's gas price is below the already imported transaction
-    #[error("Tx: insufficient gas price to replace existing transaction")]
+    #[error("[{0:?}]: insufficient gas price to replace existing transaction.")]
-    ReplacementUnderpriced,
+    ReplacementUnderpriced(TxHash),
     /// Encountered a transaction that was already added into the poll
-    #[error("[{0:?}] Already added")]
+    #[error("[{0:?}] Transaction feeCap {1} below chain minimum.")]
-    AlreadyAdded(Box<dyn std::any::Any + Send + Sync>),
+    ProtocolFeeCapTooLow(TxHash, U256),
-    /// Encountered a cycle in the graph pool
-    #[error("Transaction with cyclic dependent transactions")]
-    CyclicTransaction,
-    /// Thrown if no number was found for the given block id
-    #[error("Invalid block id: {0:?}")]
-    BlockNumberNotFound(BlockID),
 }

crates/transaction-pool/src/pool/state.rs

@@ -4,10 +4,6 @@ bitflags::bitflags! {
     /// This mirrors [erigon's ephemeral state field](https://github.com/ledgerwatch/erigon/wiki/Transaction-Pool-Design#ordering-function).
     #[derive(Default)]
     pub(crate) struct TxState: u8 {
-        /// Set to `1` if the `feeCap` of the transaction meets the chain's minimum `feeCap` requirement.
-        ///
-        /// This is different from `ENOUGH_FEE_CAP_BLOCK` which tracks on a per-block basis.
-        const ENOUGH_FEE_CAP_PROTOCOL = 0b100000;
         /// Set to `1` of the transaction is either the next transaction of the sender (on chain nonce == tx.nonce) or all prior transactions are also present in the pool.
         const NO_NONCE_GAPS = 0b010000;
         /// Bit derived from the sender's balance.
@@ -23,9 +19,9 @@ bitflags::bitflags! {
         const ENOUGH_FEE_CAP_BLOCK = 0b000010;
         const IS_LOCAL = 0b000001;
 
-        const BASE_FEE_POOL_BITS = Self::ENOUGH_FEE_CAP_PROTOCOL.bits | Self::NO_NONCE_GAPS.bits | Self::ENOUGH_BALANCE.bits | Self::NOT_TOO_MUCH_GAS.bits;
+        const BASE_FEE_POOL_BITS = Self::NO_NONCE_GAPS.bits | Self::ENOUGH_BALANCE.bits | Self::NOT_TOO_MUCH_GAS.bits;
 
-        const QUEUED_POOL_BITS  = Self::ENOUGH_FEE_CAP_PROTOCOL.bits;
+        const QUEUED_POOL_BITS  = 0b100000;
     }
 }
 
@@ -74,4 +70,10 @@ mod tests {
         state |= TxState::NO_NONCE_GAPS;
         assert!(state.intersects(TxState::NO_NONCE_GAPS))
     }
+
+    #[test]
+    fn test_tx_queud() {
+        let mut state = TxState::default();
+        assert_eq!(SubPool::Queued, state.into());
+    }
 }

crates/transaction-pool/src/pool/txpool.rs

@@ -24,7 +24,7 @@ use std::{
 /// The minimal value the basefee can decrease to
 ///
 /// The `BASE_FEE_MAX_CHANGE_DENOMINATOR` (https://eips.ethereum.org/EIPS/eip-1559) is `8`, or 12.5%, once the base fee has dropped to `7` WEI it cannot decrease further because 12.5% of 7 is less than 1.
-const MIN_PROTOCOL_BASE_FEE: U256 = U256([7, 0, 0, 0]);
+pub(crate) const MIN_PROTOCOL_BASE_FEE: U256 = U256([7, 0, 0, 0]);
 
 /// A pool that manages transactions.
 ///
@@ -182,7 +182,10 @@ impl<T: TransactionOrdering> TxPool<T> {
                 Ok(res)
             }
             InsertResult::Underpriced { existing, .. } => {
-                Err(PoolError::AlreadyAdded(Box::new(existing)))
+                Err(PoolError::ReplacementUnderpriced(existing))
+            }
+            InsertResult::ProtocolFeeCapTooLow { transaction, fee_cap } => {
+                Err(PoolError::ProtocolFeeCapTooLow(*transaction.hash(), fee_cap))
             }
         }
     }
@@ -477,17 +480,16 @@ impl<T: PoolTransaction> AllTransactions<T> {
         }
 
         // Check dynamic fee
-        if let Some(fee) = transaction.max_fee_per_gas() {
+        if let Some(fee_cap) = transaction.max_fee_per_gas() {
-            if fee >= self.pending_basefee {
+            if fee_cap < self.minimal_protocol_basefee {
-                state.insert(TxState::ENOUGH_FEE_CAP_BLOCK);
+                return InsertResult::ProtocolFeeCapTooLow { transaction, fee_cap }
             }
-            if fee > self.minimal_protocol_basefee {
+            if fee_cap >= self.pending_basefee {
-                state.insert(TxState::ENOUGH_FEE_CAP_PROTOCOL);
+                state.insert(TxState::ENOUGH_FEE_CAP_BLOCK);
             }
         } else {
             // legacy transactions always satisfy the condition
             state.insert(TxState::ENOUGH_FEE_CAP_BLOCK);
-            state.insert(TxState::ENOUGH_FEE_CAP_PROTOCOL);
         }
 
         // Ensure tx does not exceed block gas limit
@@ -654,6 +656,10 @@ pub(crate) enum InsertResult<T: PoolTransaction> {
     },
     /// Attempted to replace existing transaction, but was underpriced
     Underpriced { transaction: Arc<ValidPoolTransaction<T>>, existing: TxHash },
+    /// The transactions feeCap is lower than the chain's minimum fee requirement.
+    ///
+    /// See also [`MIN_PROTOCOL_BASE_FEE`]
+    ProtocolFeeCapTooLow { transaction: Arc<ValidPoolTransaction<T>>, fee_cap: U256 },
 }
 
 // === impl InsertResult ===
@@ -785,7 +791,7 @@ mod tests {
                 assert!(!state.contains(TxState::ENOUGH_BALANCE));
                 assert_eq!(move_to, SubPool::Queued);
             }
-            InsertResult::Underpriced { .. } => {
+            _ => {
                 panic!("not underpriced")
             }
         };
@@ -810,7 +816,7 @@ mod tests {
                 assert!(!state.contains(TxState::ENOUGH_BALANCE));
                 assert_eq!(move_to, SubPool::Queued);
             }
-            InsertResult::Underpriced { .. } => {
+            _ => {
                 panic!("not underpriced")
             }
         };
@@ -837,8 +843,8 @@ mod tests {
                 let replaced = replaced_tx.unwrap();
                 assert_eq!(replaced.0.hash(), first.hash());
             }
-            InsertResult::Underpriced { .. } => {
+            _ => {
-                panic!("not underpriced")
+                panic!("is inserted")
             }
         };
         assert!(!pool.contains(first.hash()));
@@ -872,8 +878,8 @@ mod tests {
                 assert!(state.contains(TxState::NO_NONCE_GAPS));
                 assert_eq!(move_to, SubPool::Queued);
             }
-            InsertResult::Underpriced { .. } => {
+            _ => {
-                panic!("not underpriced")
+                panic!("is inserted")
             }
         };
 
@@ -909,8 +915,8 @@ mod tests {
                 assert!(state.contains(TxState::NO_NONCE_GAPS));
                 assert_eq!(move_to, SubPool::Pending);
             }
-            InsertResult::Underpriced { .. } => {
+            _ => {
-                panic!("not underpriced")
+                panic!("is inserted")
             }
         };
 

crates/transaction-pool/src/test_util/mock.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     identifier::{SenderIdentifiers, TransactionId},
-    pool::txpool::TxPool,
+    pool::txpool::{TxPool, MIN_PROTOCOL_BASE_FEE},
     PoolTransaction, TransactionOrdering, ValidPoolTransaction,
 };
 use paste::paste;
@@ -123,8 +123,8 @@ impl MockTransaction {
             hash: H256::random(),
             sender: Address::random(),
             nonce: 0,
-            max_fee_per_gas: U256::zero(),
+            max_fee_per_gas: MIN_PROTOCOL_BASE_FEE,
-            max_priority_fee_per_gas: U256::zero(),
+            max_priority_fee_per_gas: MIN_PROTOCOL_BASE_FEE,
             gas_limit: 0,
             value: Default::default(),
         }