diff --git a/.github/assets/check_wasm.sh b/.github/assets/check_wasm.sh index 18f3a2833..827d30dc7 100755 --- a/.github/assets/check_wasm.sh +++ b/.github/assets/check_wasm.sh @@ -64,6 +64,7 @@ exclude_crates=( reth-stages-api # reth-provider, reth-prune reth-static-file # tokio reth-transaction-pool # c-kzg + reth-payload-util # reth-transaction-pool reth-trie-parallel # tokio reth-testing-utils ) diff --git a/Cargo.lock b/Cargo.lock index 869089387..97d937233 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8548,7 +8548,7 @@ version = "1.1.5" dependencies = [ "alloy-consensus", "alloy-primitives", - "reth-primitives", + "reth-transaction-pool", ] [[package]] @@ -9330,7 +9330,6 @@ dependencies = [ "reth-execution-types", "reth-fs-util", "reth-metrics", - "reth-payload-util", "reth-primitives", "reth-primitives-traits", "reth-provider", diff --git a/crates/optimism/node/src/node.rs b/crates/optimism/node/src/node.rs index 53976cf24..40339a492 100644 --- a/crates/optimism/node/src/node.rs +++ b/crates/optimism/node/src/node.rs @@ -473,16 +473,10 @@ impl OpPayloadBuilder { } } -impl OpPayloadBuilder -where - Txs: OpPayloadTransactions, -{ +impl OpPayloadBuilder { /// Configures the type responsible for yielding the transactions that should be included in the /// payload. - pub fn with_transactions( - self, - best_transactions: T, - ) -> OpPayloadBuilder { + pub fn with_transactions(self, best_transactions: T) -> OpPayloadBuilder { let Self { compute_pending_block, da_config, .. } = self; OpPayloadBuilder { compute_pending_block, best_transactions, da_config } } @@ -506,7 +500,7 @@ where + Unpin + 'static, Evm: ConfigureEvmFor>, - Txs: OpPayloadTransactions>, + Txs: OpPayloadTransactions, { let payload_builder = reth_optimism_payload_builder::OpPayloadBuilder::with_builder_config( pool, @@ -551,7 +545,7 @@ where Pool: TransactionPool>> + Unpin + 'static, - Txs: OpPayloadTransactions>, + Txs: OpPayloadTransactions, { async fn spawn_payload_service( self, diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index f110b44ee..f3899dabd 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -1,6 +1,6 @@ //! Node builder test that customizes priority of transactions in the block. -use alloy_consensus::TxEip1559; +use alloy_consensus::{SignableTransaction, TxEip1559}; use alloy_genesis::Genesis; use alloy_network::TxSignerSync; use alloy_primitives::{Address, ChainId, TxKind}; @@ -22,16 +22,19 @@ use reth_optimism_node::{ OpAddOns, OpConsensusBuilder, OpExecutorBuilder, OpNetworkBuilder, OpPayloadBuilder, OpPoolBuilder, }, + txpool::OpPooledTransaction, utils::optimism_payload_attributes, OpEngineTypes, OpNode, }; use reth_optimism_payload_builder::builder::OpPayloadTransactions; -use reth_optimism_primitives::{OpPrimitives, OpTransactionSigned}; -use reth_payload_util::{PayloadTransactions, PayloadTransactionsChain, PayloadTransactionsFixed}; +use reth_optimism_primitives::OpPrimitives; +use reth_payload_util::{ + BestPayloadTransactions, PayloadTransactions, PayloadTransactionsChain, + PayloadTransactionsFixed, +}; use reth_primitives::Recovered; use reth_provider::providers::BlockchainProvider; use reth_tasks::TaskManager; -use reth_transaction_pool::{pool::BestPayloadTransactions, PoolTransaction}; use std::sync::Arc; use tokio::sync::Mutex; @@ -40,16 +43,14 @@ struct CustomTxPriority { chain_id: ChainId, } -impl OpPayloadTransactions for CustomTxPriority { +impl OpPayloadTransactions for CustomTxPriority { fn best_transactions( &self, pool: Pool, attr: reth_transaction_pool::BestTransactionsAttributes, - ) -> impl PayloadTransactions + ) -> impl PayloadTransactions where - Pool: reth_transaction_pool::TransactionPool< - Transaction: PoolTransaction, - >, + Pool: reth_transaction_pool::TransactionPool, { // Block composition: // 1. Best transactions from the pool (up to 250k gas) @@ -68,12 +69,12 @@ impl OpPayloadTransactions for CustomTxPriority { }; let signature = sender.sign_transaction_sync(&mut end_of_block_tx).unwrap(); let end_of_block_tx = Recovered::new_unchecked( - OpTransactionSigned::new_unhashed( - OpTypedTransaction::Eip1559(end_of_block_tx), - signature, + op_alloy_consensus::OpPooledTransaction::Eip1559( + end_of_block_tx.into_signed(signature), ), sender.address(), - ); + ) + .into(); PayloadTransactionsChain::new( BestPayloadTransactions::new(pool.best_transactions_with_attributes(attr)), diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index eb3b71a41..df0f52d85 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -30,11 +30,11 @@ use reth_optimism_consensus::calculate_receipt_root_no_memo_optimism; use reth_optimism_evm::{OpReceiptBuilder, ReceiptBuilderCtx}; use reth_optimism_forks::OpHardforks; use reth_optimism_primitives::{ - transaction::signed::OpTransaction, OpTransactionSigned, ADDRESS_L2_TO_L1_MESSAGE_PASSER, + transaction::signed::OpTransaction, ADDRESS_L2_TO_L1_MESSAGE_PASSER, }; use reth_payload_builder_primitives::PayloadBuilderError; use reth_payload_primitives::PayloadBuilderAttributes; -use reth_payload_util::{NoopPayloadTransactions, PayloadTransactions}; +use reth_payload_util::{BestPayloadTransactions, NoopPayloadTransactions, PayloadTransactions}; use reth_primitives::{ transaction::SignedTransactionIntoRecoveredExt, BlockBody, NodePrimitives, SealedHeader, }; @@ -46,9 +46,7 @@ use reth_provider::{ use reth_revm::{ cancelled::CancelOnDrop, database::StateProviderDatabase, witness::ExecutionWitnessRecord, }; -use reth_transaction_pool::{ - pool::BestPayloadTransactions, BestTransactionsAttributes, PoolTransaction, TransactionPool, -}; +use reth_transaction_pool::{BestTransactionsAttributes, PoolTransaction, TransactionPool}; use revm::{ db::{states::bundle_state::BundleRetention, State}, primitives::{ExecutionResult, ResultAndState}, @@ -122,7 +120,7 @@ impl /// Configures the type responsible for yielding the transactions that should be included in the /// payload. - pub fn with_transactions( + pub fn with_transactions( self, best_transactions: T, ) -> OpPayloadBuilder { @@ -150,8 +148,10 @@ impl self.compute_pending_block } } + impl OpPayloadBuilder where + Pool: TransactionPool>, Client: StateProviderFactory + ChainSpecProvider, N: OpPayloadPrimitives, EvmConfig: ConfigureEvmFor, @@ -170,7 +170,7 @@ where best: impl FnOnce(BestTransactionsAttributes) -> Txs + Send + Sync + 'a, ) -> Result>, PayloadBuilderError> where - Txs: PayloadTransactions, + Txs: PayloadTransactions>, { let evm_env = self .evm_env(&args.config.attributes, &args.config.parent_header) @@ -251,7 +251,7 @@ where let state = StateProviderDatabase::new(state_provider); let mut state = State::builder().with_database(state).with_bundle_update().build(); - let builder = OpBuilder::new(|_| NoopPayloadTransactions::default()); + let builder = OpBuilder::new(|_| NoopPayloadTransactions::::default()); builder.witness(&mut state, &ctx) } } @@ -264,7 +264,7 @@ where N: OpPayloadPrimitives, Pool: TransactionPool>, EvmConfig: ConfigureEvmFor, - Txs: OpPayloadTransactions, + Txs: OpPayloadTransactions, { type Attributes = OpPayloadBuilderAttributes; type BuiltPayload = OpBuiltPayload; @@ -298,7 +298,7 @@ where cancel: Default::default(), best_payload: None, }; - self.build_payload(args, |_| NoopPayloadTransactions::default())? + self.build_payload(args, |_| NoopPayloadTransactions::::default())? .into_payload() .ok_or_else(|| PayloadBuilderError::MissingPayload) } @@ -332,10 +332,7 @@ impl<'a, Txs> OpBuilder<'a, Txs> { } } -impl OpBuilder<'_, Txs> -where - Txs: PayloadTransactions, -{ +impl OpBuilder<'_, Txs> { /// Executes the payload and returns the outcome. pub fn execute( self, @@ -344,7 +341,7 @@ where ) -> Result>, PayloadBuilderError> where N: OpPayloadPrimitives, - Txs: PayloadTransactions, + Txs: PayloadTransactions>, EvmConfig: ConfigureEvmFor, DB: Database + AsRef

, P: StorageRootProvider, @@ -408,7 +405,7 @@ where where EvmConfig: ConfigureEvmFor, N: OpPayloadPrimitives, - Txs: PayloadTransactions, + Txs: PayloadTransactions>, DB: Database + AsRef

, P: StateRootProvider + HashedPostStateProvider + StorageRootProvider, { @@ -533,7 +530,7 @@ where where EvmConfig: ConfigureEvmFor, N: OpPayloadPrimitives, - Txs: PayloadTransactions, + Txs: PayloadTransactions>, DB: Database + AsRef

, P: StateProofProvider + StorageRootProvider, { @@ -546,22 +543,18 @@ where } /// A type that returns a the [`PayloadTransactions`] that should be included in the pool. -pub trait OpPayloadTransactions: - Clone + Send + Sync + Unpin + 'static -{ +pub trait OpPayloadTransactions: Clone + Send + Sync + Unpin + 'static { /// Returns an iterator that yields the transaction in the order they should get included in the /// new payload. - fn best_transactions< - Pool: TransactionPool>, - >( + fn best_transactions>( &self, pool: Pool, attr: BestTransactionsAttributes, ) -> impl PayloadTransactions; } -impl OpPayloadTransactions for () { - fn best_transactions>>( +impl OpPayloadTransactions for () { + fn best_transactions>( &self, pool: Pool, attr: BestTransactionsAttributes, @@ -948,7 +941,9 @@ where &self, info: &mut ExecutionInfo, db: &mut State, - mut best_txs: impl PayloadTransactions, + mut best_txs: impl PayloadTransactions< + Transaction: PoolTransaction, + >, ) -> Result, PayloadBuilderError> where DB: Database, @@ -961,6 +956,7 @@ where let mut evm = self.evm_config.evm_with_env(&mut *db, self.evm_env.clone()); while let Some(tx) = best_txs.next(()) { + let tx = tx.into_consensus(); if info.is_tx_over_limits(tx.tx(), block_gas_limit, tx_da_limit, block_da_limit) { // we can't fit this transaction into the block, so we need to mark it as // invalid which also removes all dependent transaction from diff --git a/crates/optimism/rpc/src/witness.rs b/crates/optimism/rpc/src/witness.rs index ea6cf602c..3bf9c30cc 100644 --- a/crates/optimism/rpc/src/witness.rs +++ b/crates/optimism/rpc/src/witness.rs @@ -6,6 +6,7 @@ use jsonrpsee_core::{async_trait, RpcResult}; use op_alloy_rpc_types_engine::OpPayloadAttributes; use reth_chainspec::ChainSpecProvider; use reth_evm::ConfigureEvmFor; +use reth_node_api::NodePrimitives; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_payload_builder::{OpPayloadBuilder, OpPayloadPrimitives}; use reth_primitives::SealedHeader; @@ -15,6 +16,7 @@ use reth_provider::{ pub use reth_rpc_api::DebugExecutionWitnessApiServer; use reth_rpc_server_types::{result::internal_rpc_err, ToRpcResult}; use reth_tasks::TaskSpawner; +use reth_transaction_pool::{PoolTransaction, TransactionPool}; use std::{fmt::Debug, sync::Arc}; use tokio::sync::{oneshot, Semaphore}; @@ -55,7 +57,11 @@ where impl DebugExecutionWitnessApiServer for OpDebugWitnessApi where - Pool: Send + Sync + 'static, + Pool: TransactionPool< + Transaction: PoolTransaction< + Consensus = ::SignedTx, + >, + > + 'static, Provider: BlockReaderIdExt

+ NodePrimitivesProvider + StateProviderFactory diff --git a/crates/payload/util/Cargo.toml b/crates/payload/util/Cargo.toml index 4eed45331..f52484f92 100644 --- a/crates/payload/util/Cargo.toml +++ b/crates/payload/util/Cargo.toml @@ -13,7 +13,7 @@ workspace = true [dependencies] # reth -reth-primitives.workspace = true +reth-transaction-pool.workspace = true # alloy alloy-primitives.workspace = true diff --git a/crates/payload/util/src/lib.rs b/crates/payload/util/src/lib.rs index 7cf0f0a6e..ffffc936f 100644 --- a/crates/payload/util/src/lib.rs +++ b/crates/payload/util/src/lib.rs @@ -11,5 +11,5 @@ mod traits; mod transaction; -pub use traits::{NoopPayloadTransactions, PayloadTransactions}; +pub use traits::{BestPayloadTransactions, NoopPayloadTransactions, PayloadTransactions}; pub use transaction::{PayloadTransactionsChain, PayloadTransactionsFixed}; diff --git a/crates/payload/util/src/traits.rs b/crates/payload/util/src/traits.rs index 43e38312f..c88950a7b 100644 --- a/crates/payload/util/src/traits.rs +++ b/crates/payload/util/src/traits.rs @@ -1,5 +1,7 @@ -use alloy_primitives::Address; -use reth_primitives::Recovered; +use std::sync::Arc; + +use alloy_primitives::{map::HashSet, Address}; +use reth_transaction_pool::{PoolTransaction, ValidPoolTransaction}; /// Iterator that returns transactions for the block building process in the order they should be /// included in the block. @@ -15,7 +17,7 @@ pub trait PayloadTransactions { &mut self, // In the future, `ctx` can include access to state for block building purposes. ctx: (), - ) -> Option>; + ) -> Option; /// Exclude descendants of the transaction with given sender and nonce from the iterator, /// because this transaction won't be included in the block. @@ -35,9 +37,149 @@ impl Default for NoopPayloadTransactions { impl PayloadTransactions for NoopPayloadTransactions { type Transaction = T; - fn next(&mut self, _ctx: ()) -> Option> { + fn next(&mut self, _ctx: ()) -> Option { None } fn mark_invalid(&mut self, _sender: Address, _nonce: u64) {} } + +/// Wrapper struct that allows to convert `BestTransactions` (used in tx pool) to +/// `PayloadTransactions` (used in block composition). +#[derive(Debug)] +pub struct BestPayloadTransactions +where + T: PoolTransaction, + I: Iterator>>, +{ + invalid: HashSet
, + best: I, +} + +impl BestPayloadTransactions +where + T: PoolTransaction, + I: Iterator>>, +{ + /// Create a new `BestPayloadTransactions` with the given iterator. + pub fn new(best: I) -> Self { + Self { invalid: Default::default(), best } + } +} + +impl PayloadTransactions for BestPayloadTransactions +where + T: PoolTransaction, + I: Iterator>>, +{ + type Transaction = T; + + fn next(&mut self, _ctx: ()) -> Option { + loop { + let tx = self.best.next()?; + if self.invalid.contains(&tx.sender()) { + continue + } + return Some(tx.transaction.clone()) + } + } + + fn mark_invalid(&mut self, sender: Address, _nonce: u64) { + self.invalid.insert(sender); + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use crate::{ + BestPayloadTransactions, PayloadTransactions, PayloadTransactionsChain, + PayloadTransactionsFixed, + }; + use alloy_primitives::{map::HashSet, Address}; + use reth_transaction_pool::{ + pool::{BestTransactionsWithPrioritizedSenders, PendingPool}, + test_utils::{MockOrdering, MockTransaction, MockTransactionFactory}, + PoolTransaction, + }; + + #[test] + fn test_best_transactions_chained_iterators() { + let mut priority_pool = PendingPool::new(MockOrdering::default()); + let mut pool = PendingPool::new(MockOrdering::default()); + let mut f = MockTransactionFactory::default(); + + // Block composition + // === + // (1) up to 100 gas: custom top-of-block transaction + // (2) up to 100 gas: transactions from the priority pool + // (3) up to 200 gas: only transactions from address A + // (4) up to 200 gas: only transactions from address B + // (5) until block gas limit: all transactions from the main pool + + // Notes: + // - If prioritized addresses overlap, a single transaction will be prioritized twice and + // therefore use the per-segment gas limit twice. + // - Priority pool and main pool must synchronize between each other to make sure there are + // no conflicts for the same nonce. For example, in this scenario, pools can't reject + // transactions with seemingly incorrect nonces, because previous transactions might be in + // the other pool. + + let address_top_of_block = Address::random(); + let address_in_priority_pool = Address::random(); + let address_a = Address::random(); + let address_b = Address::random(); + let address_regular = Address::random(); + + // Add transactions to the main pool + { + let prioritized_tx_a = + MockTransaction::eip1559().with_gas_price(5).with_sender(address_a); + // without our custom logic, B would be prioritized over A due to gas price: + let prioritized_tx_b = + MockTransaction::eip1559().with_gas_price(10).with_sender(address_b); + let regular_tx = + MockTransaction::eip1559().with_gas_price(15).with_sender(address_regular); + pool.add_transaction(Arc::new(f.validated(prioritized_tx_a)), 0); + pool.add_transaction(Arc::new(f.validated(prioritized_tx_b)), 0); + pool.add_transaction(Arc::new(f.validated(regular_tx)), 0); + } + + // Add transactions to the priority pool + { + let prioritized_tx = + MockTransaction::eip1559().with_gas_price(0).with_sender(address_in_priority_pool); + let valid_prioritized_tx = f.validated(prioritized_tx); + priority_pool.add_transaction(Arc::new(valid_prioritized_tx), 0); + } + + let mut block = PayloadTransactionsChain::new( + PayloadTransactionsFixed::single( + MockTransaction::eip1559().with_sender(address_top_of_block), + ), + Some(100), + PayloadTransactionsChain::new( + BestPayloadTransactions::new(priority_pool.best()), + Some(100), + BestPayloadTransactions::new(BestTransactionsWithPrioritizedSenders::new( + HashSet::from([address_a]), + 200, + BestTransactionsWithPrioritizedSenders::new( + HashSet::from([address_b]), + 200, + pool.best(), + ), + )), + None, + ), + None, + ); + + assert_eq!(block.next(()).unwrap().sender(), address_top_of_block); + assert_eq!(block.next(()).unwrap().sender(), address_in_priority_pool); + assert_eq!(block.next(()).unwrap().sender(), address_a); + assert_eq!(block.next(()).unwrap().sender(), address_b); + assert_eq!(block.next(()).unwrap().sender(), address_regular); + } +} diff --git a/crates/payload/util/src/transaction.rs b/crates/payload/util/src/transaction.rs index 0893e8d10..818b06f0a 100644 --- a/crates/payload/util/src/transaction.rs +++ b/crates/payload/util/src/transaction.rs @@ -1,7 +1,7 @@ use crate::PayloadTransactions; use alloy_consensus::Transaction; use alloy_primitives::Address; -use reth_primitives::Recovered; +use reth_transaction_pool::PoolTransaction; /// An implementation of [`crate::traits::PayloadTransactions`] that yields /// a pre-defined set of transactions. @@ -26,10 +26,10 @@ impl PayloadTransactionsFixed { } } -impl PayloadTransactions for PayloadTransactionsFixed> { +impl PayloadTransactions for PayloadTransactionsFixed { type Transaction = T; - fn next(&mut self, _ctx: ()) -> Option> { + fn next(&mut self, _ctx: ()) -> Option { (self.index < self.transactions.len()).then(|| { let tx = self.transactions[self.index].clone(); self.index += 1; @@ -91,20 +91,20 @@ impl PayloadTransactionsChain PayloadTransactions for PayloadTransactionsChain where - A: PayloadTransactions, + A: PayloadTransactions, B: PayloadTransactions, { type Transaction = A::Transaction; - fn next(&mut self, ctx: ()) -> Option> { + fn next(&mut self, ctx: ()) -> Option { while let Some(tx) = self.before.next(ctx) { if let Some(before_max_gas) = self.before_max_gas { - if self.before_gas + tx.tx().gas_limit() <= before_max_gas { - self.before_gas += tx.tx().gas_limit(); + if self.before_gas + tx.gas_limit() <= before_max_gas { + self.before_gas += tx.gas_limit(); return Some(tx); } - self.before.mark_invalid(tx.signer(), tx.tx().nonce()); - self.after.mark_invalid(tx.signer(), tx.tx().nonce()); + self.before.mark_invalid(tx.sender(), tx.nonce()); + self.after.mark_invalid(tx.sender(), tx.nonce()); } else { return Some(tx); } @@ -112,11 +112,11 @@ where while let Some(tx) = self.after.next(ctx) { if let Some(after_max_gas) = self.after_max_gas { - if self.after_gas + tx.tx().gas_limit() <= after_max_gas { - self.after_gas += tx.tx().gas_limit(); + if self.after_gas + tx.gas_limit() <= after_max_gas { + self.after_gas += tx.gas_limit(); return Some(tx); } - self.after.mark_invalid(tx.signer(), tx.tx().nonce()); + self.after.mark_invalid(tx.sender(), tx.nonce()); } else { return Some(tx); } diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index 238a0e1d1..08435c900 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -18,7 +18,6 @@ reth-chainspec.workspace = true reth-eth-wire-types.workspace = true reth-primitives = { workspace = true, features = ["c-kzg", "secp256k1"] } reth-primitives-traits.workspace = true -reth-payload-util.workspace = true reth-execution-types.workspace = true reth-fs-util.workspace = true reth-storage-api.workspace = true diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 47b19e42c..681fa479d 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -8,8 +8,7 @@ use alloy_consensus::Transaction; use alloy_eips::Typed2718; use alloy_primitives::Address; use core::fmt; -use reth_payload_util::PayloadTransactions; -use reth_primitives::{InvalidTransactionError, Recovered}; +use reth_primitives::InvalidTransactionError; use std::{ collections::{BTreeMap, BTreeSet, HashSet, VecDeque}, sync::Arc, @@ -223,51 +222,6 @@ impl Iterator for BestTransactions { } } -/// Wrapper struct that allows to convert `BestTransactions` (used in tx pool) to -/// `PayloadTransactions` (used in block composition). -#[derive(Debug)] -pub struct BestPayloadTransactions -where - T: PoolTransaction, - I: Iterator>>, -{ - invalid: HashSet
, - best: I, -} - -impl BestPayloadTransactions -where - T: PoolTransaction, - I: Iterator>>, -{ - /// Create a new `BestPayloadTransactions` with the given iterator. - pub fn new(best: I) -> Self { - Self { invalid: Default::default(), best } - } -} - -impl PayloadTransactions for BestPayloadTransactions -where - T: PoolTransaction, - I: Iterator>>, -{ - type Transaction = T::Consensus; - - fn next(&mut self, _ctx: ()) -> Option> { - loop { - let tx = self.best.next()?; - if self.invalid.contains(&tx.sender()) { - continue - } - return Some(tx.to_consensus()) - } - } - - fn mark_invalid(&mut self, sender: Address, _nonce: u64) { - self.invalid.insert(sender); - } -} - /// A [`BestTransactions`](crate::traits::BestTransactions) implementation that filters the /// transactions of iter with predicate. /// @@ -425,7 +379,6 @@ mod tests { BestTransactions, Priority, }; use alloy_primitives::U256; - use reth_payload_util::{PayloadTransactionsChain, PayloadTransactionsFixed}; #[test] fn test_best_iter() { @@ -846,85 +799,6 @@ mod tests { // TODO: Test that gas limits for prioritized transactions are respected } - #[test] - fn test_best_transactions_chained_iterators() { - let mut priority_pool = PendingPool::new(MockOrdering::default()); - let mut pool = PendingPool::new(MockOrdering::default()); - let mut f = MockTransactionFactory::default(); - - // Block composition - // === - // (1) up to 100 gas: custom top-of-block transaction - // (2) up to 100 gas: transactions from the priority pool - // (3) up to 200 gas: only transactions from address A - // (4) up to 200 gas: only transactions from address B - // (5) until block gas limit: all transactions from the main pool - - // Notes: - // - If prioritized addresses overlap, a single transaction will be prioritized twice and - // therefore use the per-segment gas limit twice. - // - Priority pool and main pool must synchronize between each other to make sure there are - // no conflicts for the same nonce. For example, in this scenario, pools can't reject - // transactions with seemingly incorrect nonces, because previous transactions might be in - // the other pool. - - let address_top_of_block = Address::random(); - let address_in_priority_pool = Address::random(); - let address_a = Address::random(); - let address_b = Address::random(); - let address_regular = Address::random(); - - // Add transactions to the main pool - { - let prioritized_tx_a = - MockTransaction::eip1559().with_gas_price(5).with_sender(address_a); - // without our custom logic, B would be prioritized over A due to gas price: - let prioritized_tx_b = - MockTransaction::eip1559().with_gas_price(10).with_sender(address_b); - let regular_tx = - MockTransaction::eip1559().with_gas_price(15).with_sender(address_regular); - pool.add_transaction(Arc::new(f.validated(prioritized_tx_a)), 0); - pool.add_transaction(Arc::new(f.validated(prioritized_tx_b)), 0); - pool.add_transaction(Arc::new(f.validated(regular_tx)), 0); - } - - // Add transactions to the priority pool - { - let prioritized_tx = - MockTransaction::eip1559().with_gas_price(0).with_sender(address_in_priority_pool); - let valid_prioritized_tx = f.validated(prioritized_tx); - priority_pool.add_transaction(Arc::new(valid_prioritized_tx), 0); - } - - let mut block = PayloadTransactionsChain::new( - PayloadTransactionsFixed::single( - MockTransaction::eip1559().with_sender(address_top_of_block).into(), - ), - Some(100), - PayloadTransactionsChain::new( - BestPayloadTransactions::new(priority_pool.best()), - Some(100), - BestPayloadTransactions::new(BestTransactionsWithPrioritizedSenders::new( - HashSet::from([address_a]), - 200, - BestTransactionsWithPrioritizedSenders::new( - HashSet::from([address_b]), - 200, - pool.best(), - ), - )), - None, - ), - None, - ); - - assert_eq!(block.next(()).unwrap().signer(), address_top_of_block); - assert_eq!(block.next(()).unwrap().signer(), address_in_priority_pool); - assert_eq!(block.next(()).unwrap().signer(), address_a); - assert_eq!(block.next(()).unwrap().signer(), address_b); - assert_eq!(block.next(()).unwrap().signer(), address_regular); - } - #[test] fn test_best_with_fees_iter_no_blob_fee_required() { // Tests transactions without blob fees where base fees are checked. diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 954ef66f3..abcd9017f 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -101,9 +101,7 @@ use crate::{ traits::{GetPooledTransactionLimit, NewBlobSidecar, TransactionListenerKind}, validate::ValidTransaction, }; -pub use best::{ - BestPayloadTransactions, BestTransactionFilter, BestTransactionsWithPrioritizedSenders, -}; +pub use best::{BestTransactionFilter, BestTransactionsWithPrioritizedSenders}; pub use blob::{blob_tx_priority, fee_delta}; pub use events::{FullTransactionEvent, TransactionEvent}; pub use listener::{AllTransactionsEvents, TransactionEvents}; diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 78506549d..3f8629768 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -98,7 +98,7 @@ impl PendingPool { /// provides a way to mark transactions that the consumer of this iterator considers invalid. In /// which case the transaction's subgraph is also automatically marked invalid, See (1.). /// Invalid transactions are skipped. - pub(crate) fn best(&self) -> BestTransactions { + pub fn best(&self) -> BestTransactions { BestTransactions { all: self.by_id.clone(), independent: self.independent_transactions.values().cloned().collect(),