From 4709fcf16cdbebcef21e6fb2a1d10899cb31c934 Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Mon, 19 Jun 2023 15:35:25 +0200 Subject: [PATCH] feat(txpool): add price bump for `is_underpriced` transaction check (#3202) --- crates/transaction-pool/src/lib.rs | 3 ++ crates/transaction-pool/src/pool/txpool.rs | 45 +++++++++++++++++++++- crates/transaction-pool/src/validate.rs | 5 --- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/crates/transaction-pool/src/lib.rs b/crates/transaction-pool/src/lib.rs index d633a18f4..fa1c73b93 100644 --- a/crates/transaction-pool/src/lib.rs +++ b/crates/transaction-pool/src/lib.rs @@ -136,6 +136,9 @@ 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; +// Price bump (in %) for the transaction pool underpriced check +pub(crate) const PRICE_BUMP: u128 = 10; + /// A shareable, generic, customizable `TransactionPool` implementation. #[derive(Debug)] pub struct Pool { diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index c50181633..e9d370c0a 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -13,7 +13,8 @@ use crate::{ AddedPendingTransaction, AddedTransaction, OnNewCanonicalStateOutcome, }, traits::{BlockInfo, PoolSize}, - PoolConfig, PoolResult, PoolTransaction, TransactionOrdering, ValidPoolTransaction, U256, + PoolConfig, PoolResult, PoolTransaction, TransactionOrdering, ValidPoolTransaction, PRICE_BUMP, + U256, }; use fnv::FnvHashMap; use reth_primitives::{ @@ -971,6 +972,25 @@ impl AllTransactions { Ok(transaction) } + /// Returns true if `transaction_a` is underpriced compared to `transaction_B`. + fn is_underpriced( + transaction_a: &ValidPoolTransaction, + transaction_b: &ValidPoolTransaction, + price_bump: u128, + ) -> bool { + let tx_a_max_priority_fee_per_gas = + transaction_a.transaction.max_priority_fee_per_gas().unwrap_or(0); + let tx_b_max_priority_fee_per_gas = + transaction_b.transaction.max_priority_fee_per_gas().unwrap_or(0); + + transaction_a.max_fee_per_gas() <= + transaction_b.max_fee_per_gas() * (100 + price_bump) / 100 || + (tx_a_max_priority_fee_per_gas <= + tx_b_max_priority_fee_per_gas * (100 + price_bump) / 100 && + tx_a_max_priority_fee_per_gas != 0 && + tx_b_max_priority_fee_per_gas != 0) + } + /// Inserts a new transaction into the pool. /// /// If the transaction already exists, it will be replaced if not underpriced. @@ -1038,7 +1058,12 @@ impl AllTransactions { Entry::Occupied(mut entry) => { // Transaction already exists // Ensure the new transaction is not underpriced - if transaction.is_underpriced(entry.get().transaction.as_ref()) { + + if Self::is_underpriced( + transaction.as_ref(), + entry.get().transaction.as_ref(), + PRICE_BUMP, + ) { return Err(InsertErr::Underpriced { transaction: pool_tx.transaction, existing: *entry.get().transaction.hash(), @@ -1410,6 +1435,22 @@ mod tests { assert_eq!(pool.len(), 1); } + #[test] + fn insert_replace_underpriced() { + let on_chain_balance = U256::ZERO; + let on_chain_nonce = 0; + let mut f = MockTransactionFactory::default(); + let mut pool = AllTransactions::default(); + let tx = MockTransaction::eip1559().inc_price().inc_limit(); + let first = f.validated(tx.clone()); + let _res = pool.insert_tx(first.clone(), on_chain_balance, on_chain_nonce); + let mut replacement = f.validated(tx.rng_hash()); + replacement.transaction = replacement.transaction.decr_price(); + let err = + pool.insert_tx(replacement.clone(), on_chain_balance, on_chain_nonce).unwrap_err(); + assert!(matches!(err, InsertErr::Underpriced { .. })); + } + // insert nonce then nonce - 1 #[test] fn insert_previous() { diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index e7bd6f190..959d5958b 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -367,11 +367,6 @@ impl ValidPoolTransaction { self.transaction.gas_limit() } - /// Returns true if this transaction is underpriced compared to the other. - pub(crate) fn is_underpriced(&self, other: &Self) -> bool { - self.transaction.effective_gas_price() <= other.transaction.effective_gas_price() - } - /// Whether the transaction originated locally. pub fn is_local(&self) -> bool { self.origin.is_local()