fix(txpool): pendind pool reordering (paradigmxyz#3955)

crates/transaction-pool/src/lib.rs

@@ -157,7 +157,7 @@ pub use crate::{
         TXPOOL_SUBPOOL_MAX_SIZE_MB_DEFAULT, TXPOOL_SUBPOOL_MAX_TXS_DEFAULT,
     },
     error::PoolResult,
-    ordering::{GasCostOrdering, TransactionOrdering},
+    ordering::{CoinbaseTipOrdering, Priority, TransactionOrdering},
     pool::{
         state::SubPool, AllTransactionsEvents, FullTransactionEvent, TransactionEvent,
         TransactionEvents,
@@ -280,12 +280,12 @@ where
 }
 
 impl<Client>
-    Pool<EthTransactionValidator<Client, PooledTransaction>, GasCostOrdering<PooledTransaction>>
+    Pool<EthTransactionValidator<Client, PooledTransaction>, CoinbaseTipOrdering<PooledTransaction>>
 where
     Client: StateProviderFactory + Clone + 'static,
 {
     /// Returns a new [Pool] that uses the default [EthTransactionValidator] when validating
-    /// [PooledTransaction]s and ords via [GasCostOrdering]
+    /// [PooledTransaction]s and ords via [CoinbaseTipOrdering]
     ///
     /// # Example
     ///
@@ -305,7 +305,7 @@ where
         validator: EthTransactionValidator<Client, PooledTransaction>,
         config: PoolConfig,
     ) -> Self {
-        Self::new(validator, GasCostOrdering::default(), config)
+        Self::new(validator, CoinbaseTipOrdering::default(), config)
     }
 }
 

crates/transaction-pool/src/ordering.rs

@@ -2,6 +2,26 @@ use crate::traits::PoolTransaction;
 use reth_primitives::U256;
 use std::{fmt, marker::PhantomData};
 
+/// Priority of the transaction that can be missing.
+///
+/// Transactions with missing priorities are ranked lower.
+#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug)]
+pub enum Priority<T: Ord + Clone> {
+    /// The value of the priority of the transaction.
+    Value(T),
+    /// Missing priority due to ordering internals.
+    None,
+}
+
+impl<T: Ord + Clone> From<Option<T>> for Priority<T> {
+    fn from(value: Option<T>) -> Self {
+        match value {
+            Some(val) => Priority::Value(val),
+            None => Priority::None,
+        }
+    }
+}
+
 /// Transaction ordering trait to determine the order of transactions.
 ///
 /// Decides how transactions should be ordered within the pool, depending on a `Priority` value.
@@ -11,42 +31,53 @@ pub trait TransactionOrdering: Send + Sync + 'static {
     /// Priority of a transaction.
     ///
     /// Higher is better.
-    type Priority: Ord + Clone + Default + fmt::Debug + Send + Sync;
+    type PriorityValue: Ord + Clone + Default + fmt::Debug + Send + Sync;
 
     /// The transaction type to determine the priority of.
     type Transaction: PoolTransaction;
 
     /// Returns the priority score for the given transaction.
-    fn priority(&self, transaction: &Self::Transaction) -> Self::Priority;
+    fn priority(
+        &self,
+        transaction: &Self::Transaction,
+        base_fee: u64,
+    ) -> Priority<Self::PriorityValue>;
 }
 
 /// Default ordering for the pool.
 ///
-/// The transactions are ordered by their gas cost. The higher the gas cost,
-/// the higher the priority of this transaction is.
+/// The transactions are ordered by their coinbase tip.
+/// The higher the coinbase tip is, the higher the priority of the transaction.
 #[derive(Debug)]
 #[non_exhaustive]
-pub struct GasCostOrdering<T>(PhantomData<T>);
+pub struct CoinbaseTipOrdering<T>(PhantomData<T>);
 
-impl<T> TransactionOrdering for GasCostOrdering<T>
+impl<T> TransactionOrdering for CoinbaseTipOrdering<T>
 where
     T: PoolTransaction + 'static,
 {
-    type Priority = U256;
+    type PriorityValue = U256;
     type Transaction = T;
 
-    fn priority(&self, transaction: &Self::Transaction) -> Self::Priority {
-        transaction.gas_cost()
+    /// Source: <https://github.com/ethereum/go-ethereum/blob/7f756dc1185d7f1eeeacb1d12341606b7135f9ea/core/txpool/legacypool/list.go#L469-L482>.
+    ///
+    /// NOTE: The implementation is incomplete for missing base fee.
+    fn priority(
+        &self,
+        transaction: &Self::Transaction,
+        base_fee: u64,
+    ) -> Priority<Self::PriorityValue> {
+        transaction.effective_tip_per_gas(base_fee).map(U256::from).into()
     }
 }
 
-impl<T> Default for GasCostOrdering<T> {
+impl<T> Default for CoinbaseTipOrdering<T> {
     fn default() -> Self {
         Self(Default::default())
     }
 }
 
-impl<T> Clone for GasCostOrdering<T> {
+impl<T> Clone for CoinbaseTipOrdering<T> {
     fn clone(&self) -> Self {
         Self::default()
     }

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

@@ -1,7 +1,6 @@
 use crate::{
-    identifier::TransactionId,
-    pool::pending::{PendingTransaction, PendingTransactionRef},
-    PoolTransaction, TransactionOrdering, ValidPoolTransaction,
+    identifier::TransactionId, pool::pending::PendingTransaction, PoolTransaction,
+    TransactionOrdering, ValidPoolTransaction,
 };
 use reth_primitives::H256 as TxHash;
 use std::{
@@ -54,12 +53,12 @@ impl<T: TransactionOrdering> Iterator for BestTransactionsWithBasefee<T> {
 pub(crate) struct BestTransactions<T: TransactionOrdering> {
     /// Contains a copy of _all_ transactions of the pending pool at the point in time this
     /// iterator was created.
-    pub(crate) all: BTreeMap<TransactionId, Arc<PendingTransaction<T>>>,
+    pub(crate) all: BTreeMap<TransactionId, PendingTransaction<T>>,
     /// Transactions that can be executed right away: these have the expected nonce.
     ///
     /// Once an `independent` transaction with the nonce `N` is returned, it unlocks `N+1`, which
     /// then can be moved from the `all` set to the `independent` set.
-    pub(crate) independent: BTreeSet<PendingTransactionRef<T>>,
+    pub(crate) independent: BTreeSet<PendingTransaction<T>>,
     /// There might be the case where a yielded transactions is invalid, this will track it.
     pub(crate) invalid: HashSet<TxHash>,
 }
@@ -74,7 +73,7 @@ impl<T: TransactionOrdering> BestTransactions<T> {
     ///
     /// Note: for a transaction with nonce higher than the current on chain nonce this will always
     /// return an ancestor since all transaction in this pool are gapless.
-    pub(crate) fn ancestor(&self, id: &TransactionId) -> Option<&Arc<PendingTransaction<T>>> {
+    pub(crate) fn ancestor(&self, id: &TransactionId) -> Option<&PendingTransaction<T>> {
         self.all.get(&id.unchecked_ancestor()?)
     }
 }
@@ -106,7 +105,7 @@ impl<T: TransactionOrdering> Iterator for BestTransactions<T> {
 
             // Insert transactions that just got unlocked.
             if let Some(unlocked) = self.all.get(&best.unlocks()) {
-                self.independent.insert(unlocked.transaction.clone());
+                self.independent.insert(unlocked.clone());
             }
 
             return Some(best.transaction)
@@ -133,7 +132,7 @@ mod tests {
         for nonce in 0..num_tx {
             let tx = tx.clone().rng_hash().with_nonce(nonce);
             let valid_tx = f.validated(tx);
-            pool.add_transaction(Arc::new(valid_tx));
+            pool.add_transaction(Arc::new(valid_tx), 0);
         }
 
         let mut best = pool.best();
@@ -159,7 +158,7 @@ mod tests {
         for nonce in 0..num_tx {
             let tx = tx.clone().rng_hash().with_nonce(nonce);
             let valid_tx = f.validated(tx);
-            pool.add_transaction(Arc::new(valid_tx));
+            pool.add_transaction(Arc::new(valid_tx), 0);
         }
 
         let mut best = pool.best();

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

@@ -306,10 +306,11 @@ pub(crate) struct QueuedOrd<T: PoolTransaction>(Arc<ValidPoolTransaction<T>>);
 
 impl_ord_wrapper!(QueuedOrd);
 
+// TODO: temporary solution for ordering the queued pool.
 impl<T: PoolTransaction> Ord for QueuedOrd<T> {
     fn cmp(&self, other: &Self) -> Ordering {
-        // Higher cost is better
-        self.gas_cost().cmp(&other.gas_cost()).then_with(||
+        // Higher price is better
+        self.max_fee_per_gas().cmp(&self.max_fee_per_gas()).then_with(||
             // Lower timestamp is better
             other.timestamp.cmp(&self.timestamp))
     }