From a0faf0e40974b7e705e7e71b7dc046265c4473dc Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Tue, 5 Nov 2024 01:58:36 +0000 Subject: [PATCH 01/17] feat(transaction-pool): chaining & static txs for best transactions trait --- crates/transaction-pool/src/pool/best.rs | 275 +++++++++++++++++++++++ crates/transaction-pool/src/pool/mod.rs | 39 ++-- 2 files changed, 296 insertions(+), 18 deletions(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 36a14edaa232..e4baddae8c02 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -341,6 +341,160 @@ where } } +/// An implementation of [`crate::traits::BestTransactions`] that yields +/// a pre-defined set of transactions. +/// +/// This is useful to put a sequencer-specified set of transactions into the block +/// and compose it with the rest of the transactions. +#[derive(Debug)] +pub struct BestTransactionsFixed { + transactions: Vec, + index: usize, +} + +impl BestTransactionsFixed { + /// Constructs a new [`BestTransactionsFixed`]. + pub fn new(transactions: Vec) -> Self { + Self { transactions, index: Default::default() } + } + + /// Constructs a new [`BestTransactionsFixed`] with a single transaction. + pub fn single(transaction: T) -> Self { + Self { transactions: vec![transaction], index: Default::default() } + } +} + +impl Iterator for BestTransactionsFixed { + type Item = T; + + fn next(&mut self) -> Option { + (self.index < self.transactions.len()).then(|| { + let tx = self.transactions[self.index].clone(); + self.index += 1; + tx + }) + } +} + +impl crate::traits::BestTransactions for BestTransactionsFixed { + fn mark_invalid(&mut self, _tx: &Self::Item) { + // TODO: Implement when it's supported on the main pool too + } + + fn no_updates(&mut self) {} + + fn set_skip_blobs(&mut self, _skip_blobs: bool) {} +} + +/// Wrapper over [`crate::traits::BestTransactions`] that combines transactions from multiple +/// `BestTransactions` iterators and keeps track of the gas for both of iterators. +/// +/// We can't use [`Iterator::chain`], because: +/// (a) we need to propagate the `mark_invalid` and `no_updates` +/// (b) we need to keep track of the gas +/// +/// Notes that [`BestTransactionsChain`] fully drains the first iterator +/// before moving to the second one. +/// +/// If the `before` iterator has transactions that are not fitting into the block, +/// the after iterator will get propagated a `mark_invalid` call for each of them. +#[derive(Debug)] +pub struct BestTransactionsChain { + /// Iterator that will be used first + before: B, + /// Allowed gas for the transactions from `before` iterator. If `None`, no gas limit is + /// enforced. + before_max_gas: Option, + /// Gas used by the transactions from `before` iterator + before_gas: u64, + /// Iterator that will be used after `before` iterator + after: A, + /// Allowed gas for the transactions from `after` iterator. If `None`, no gas limit is + /// enforced. + after_max_gas: Option, + /// Gas used by the transactions from `after` iterator + after_gas: u64, +} + +impl BestTransactionsChain { + /// Constructs a new [`BestTransactionsChain`]. + pub fn new( + before: B, + before_max_gas: Option, + after: A, + after_max_gas: Option, + ) -> Self { + Self { + before, + before_max_gas, + before_gas: Default::default(), + after, + after_max_gas, + after_gas: Default::default(), + } + } +} + +impl Iterator for BestTransactionsChain +where + B: crate::traits::BestTransactions>>, + A: crate::traits::BestTransactions>>, + T: PoolTransaction, +{ + type Item = Arc>; + + fn next(&mut self) -> Option { + while let Some(tx) = self.before.next() { + if let Some(before_max_gas) = self.before_max_gas { + if self.before_gas + tx.transaction.gas_limit() <= before_max_gas { + self.before_gas += tx.transaction.gas_limit(); + return Some(tx); + } + self.before.mark_invalid(&tx); + self.after.mark_invalid(&tx); + } else { + return Some(tx); + } + } + + while let Some(tx) = self.after.next() { + if let Some(after_max_gas) = self.after_max_gas { + if self.after_gas + tx.transaction.gas_limit() <= after_max_gas { + self.after_gas += tx.transaction.gas_limit(); + return Some(tx); + } + self.after.mark_invalid(&tx); + } else { + return Some(tx); + } + } + + None + } +} + +impl crate::traits::BestTransactions for BestTransactionsChain +where + B: crate::traits::BestTransactions>>, + A: crate::traits::BestTransactions>>, + T: PoolTransaction, +{ + fn mark_invalid(&mut self, tx: &Self::Item) { + self.before.mark_invalid(tx); + self.after.mark_invalid(tx); + } + + fn no_updates(&mut self) { + self.before.no_updates(); + self.after.no_updates(); + } + + fn set_skip_blobs(&mut self, skip_blobs: bool) { + self.before.set_skip_blobs(skip_blobs); + self.after.set_skip_blobs(skip_blobs); + } +} + #[cfg(test)] mod tests { use super::*; @@ -728,4 +882,125 @@ mod tests { assert_eq!(tx.nonce() % 2, 0); } } + + #[test] + fn test_best_transactions_prioritized_senders() { + let mut pool = PendingPool::new(MockOrdering::default()); + let mut f = MockTransactionFactory::default(); + + // Add 5 plain transactions from different senders with increasing gas price + for gas_price in 0..5 { + let tx = MockTransaction::eip1559().with_gas_price(gas_price); + let valid_tx = f.validated(tx); + pool.add_transaction(Arc::new(valid_tx), 0); + } + + // Add another transaction with 0 gas price that's going to be prioritized by sender + let prioritized_tx = MockTransaction::eip1559().with_gas_price(0); + let valid_prioritized_tx = f.validated(prioritized_tx.clone()); + pool.add_transaction(Arc::new(valid_prioritized_tx), 0); + + let prioritized_senders = HashSet::from([prioritized_tx.sender()]); + let best = + BestTransactionsWithPrioritizedSenders::new(prioritized_senders, 200, pool.best()); + + // Verify that the prioritized transaction is returned first + // and the rest are returned in the reverse order of gas price + let mut iter = best.into_iter(); + let top_of_block_tx = iter.next().unwrap(); + assert_eq!(top_of_block_tx.max_fee_per_gas(), 0); + assert_eq!(top_of_block_tx.sender(), prioritized_tx.sender()); + for gas_price in (0..5).rev() { + assert_eq!(iter.next().unwrap().max_fee_per_gas(), gas_price); + } + + // 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 syncronize 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 block = BestTransactionsChain::new( + // Segment 1 + BestTransactionsFixed::single(Arc::new( + f.validated(MockTransaction::eip1559().with_sender(address_top_of_block)), + )), + Some(100), + BestTransactionsChain::new( + // Segment 2 + priority_pool.best(), + Some(100), + // Segment 3 + BestTransactionsWithPrioritizedSenders::new( + HashSet::from([address_a]), + 200, + // Segment 4 + BestTransactionsWithPrioritizedSenders::new( + HashSet::from([address_b]), + 200, + // Segment 5 + pool.best(), + ), + ), + None, + ), + None, + ); + + let mut iter = block.into_iter(); + assert_eq!(iter.next().unwrap().sender(), address_top_of_block); + assert_eq!(iter.next().unwrap().sender(), address_in_priority_pool); + assert_eq!(iter.next().unwrap().sender(), address_a); + assert_eq!(iter.next().unwrap().sender(), address_b); + assert_eq!(iter.next().unwrap().sender(), address_regular); + } + + // TODO: Same nonce test } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 77446a523757..e9e2f5e1a3f0 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -106,7 +106,10 @@ use crate::{ traits::{GetPooledTransactionLimit, NewBlobSidecar, TransactionListenerKind}, validate::ValidTransaction, }; -pub use best::{BestTransactionFilter, BestTransactionsWithPrioritizedSenders}; +pub use best::{ + BestTransactionFilter, BestTransactionsChain, BestTransactionsFixed, + BestTransactionsWithPrioritizedSenders, +}; pub use blob::{blob_tx_priority, fee_delta}; pub use events::{FullTransactionEvent, TransactionEvent}; pub use listener::{AllTransactionsEvents, TransactionEvents}; @@ -310,7 +313,7 @@ where if let Ok(blob) = BlobTransaction::try_from_signed(transaction, Arc::unwrap_or_clone(sidecar)) { - return Some(blob) + return Some(blob); } } None @@ -339,7 +342,7 @@ where if let Some(blob) = self.get_blob_transaction(tx) { PooledTransactionsElement::BlobTransaction(blob) } else { - continue + continue; } } else { match PooledTransactionsElement::try_from(tx) { @@ -349,7 +352,7 @@ where target: "txpool", %err, "failed to convert transaction to pooled element; skipping", ); - continue + continue; } } }; @@ -358,7 +361,7 @@ where elements.push(pooled); if limit.exceeds(size) { - break + break; } } @@ -541,7 +544,7 @@ where if added.iter().any(Result::is_ok) { self.discard_worst() } else { Default::default() }; if discarded.is_empty() { - return added + return added; } { @@ -571,7 +574,7 @@ where if listener.kind.is_propagate_only() && !propagate_allowed { // only emit this hash to listeners that are only allowed to receive propagate only // transactions, such as network - return !listener.sender.is_closed() + return !listener.sender.is_closed(); } // broadcast all pending transactions to the listener @@ -586,7 +589,7 @@ where if listener.kind.is_propagate_only() && !event.transaction.propagate { // only emit this hash to listeners that are only allowed to receive propagate only // transactions, such as network - return !listener.sender.is_closed() + return !listener.sender.is_closed(); } listener.send(event.clone()) @@ -597,7 +600,7 @@ where fn on_new_blob_sidecar(&self, tx_hash: &TxHash, sidecar: &BlobTransactionSidecar) { let mut sidecar_listeners = self.blob_transaction_sidecar_listener.lock(); if sidecar_listeners.is_empty() { - return + return; } let sidecar = Arc::new(sidecar.clone()); sidecar_listeners.retain_mut(|listener| { @@ -706,7 +709,7 @@ where hashes: Vec, ) -> Vec>> { if hashes.is_empty() { - return Vec::new() + return Vec::new(); } let removed = self.pool.write().remove_transactions(hashes); @@ -724,7 +727,7 @@ where hashes: Vec, ) -> Vec>> { if hashes.is_empty() { - return Vec::new() + return Vec::new(); } let removed = self.pool.write().remove_transactions_and_descendants(hashes); @@ -755,7 +758,7 @@ where A: HandleMempoolData, { if announcement.is_empty() { - return + return; } let pool = self.get_pool_data(); announcement.retain_by_hash(|tx| !pool.contains(tx)) @@ -849,7 +852,7 @@ where txs: Vec, ) -> Vec>> { if txs.is_empty() { - return Vec::new() + return Vec::new(); } self.get_pool_data().get_all(txs).collect() } @@ -857,7 +860,7 @@ where /// Notify about propagated transactions. pub(crate) fn on_propagated(&self, txs: PropagatedTransactions) { if txs.0.is_empty() { - return + return; } let mut listener = self.event_listener.write(); @@ -1077,9 +1080,9 @@ where loop { let next = self.iter.next()?; if self.kind.is_propagate_only() && !next.propagate { - continue + continue; } - return Some(*next.hash()) + return Some(*next.hash()); } } } @@ -1101,12 +1104,12 @@ where loop { let next = self.iter.next()?; if self.kind.is_propagate_only() && !next.propagate { - continue + continue; } return Some(NewTransactionEvent { subpool: SubPool::Pending, transaction: next.clone(), - }) + }); } } } From 23ae85a7d550ea026421f75759dd60f3bece1408 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Tue, 5 Nov 2024 02:03:32 +0000 Subject: [PATCH 02/17] typo --- crates/transaction-pool/src/pool/best.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index e4baddae8c02..66cf54abe794 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -934,7 +934,7 @@ mod tests { // 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 syncronize between each other to make sure there are + // - 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. From 1206cf70dc42dd5914b1ed45b30038edc4573978 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Tue, 5 Nov 2024 18:30:13 +0000 Subject: [PATCH 03/17] clean up diff --- crates/transaction-pool/src/pool/mod.rs | 30 ++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index e9e2f5e1a3f0..172f975df96f 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -313,7 +313,7 @@ where if let Ok(blob) = BlobTransaction::try_from_signed(transaction, Arc::unwrap_or_clone(sidecar)) { - return Some(blob); + return Some(blob) } } None @@ -342,7 +342,7 @@ where if let Some(blob) = self.get_blob_transaction(tx) { PooledTransactionsElement::BlobTransaction(blob) } else { - continue; + continue } } else { match PooledTransactionsElement::try_from(tx) { @@ -352,7 +352,7 @@ where target: "txpool", %err, "failed to convert transaction to pooled element; skipping", ); - continue; + continue } } }; @@ -361,7 +361,7 @@ where elements.push(pooled); if limit.exceeds(size) { - break; + break } } @@ -544,7 +544,7 @@ where if added.iter().any(Result::is_ok) { self.discard_worst() } else { Default::default() }; if discarded.is_empty() { - return added; + return added } { @@ -574,7 +574,7 @@ where if listener.kind.is_propagate_only() && !propagate_allowed { // only emit this hash to listeners that are only allowed to receive propagate only // transactions, such as network - return !listener.sender.is_closed(); + return !listener.sender.is_closed() } // broadcast all pending transactions to the listener @@ -600,7 +600,7 @@ where fn on_new_blob_sidecar(&self, tx_hash: &TxHash, sidecar: &BlobTransactionSidecar) { let mut sidecar_listeners = self.blob_transaction_sidecar_listener.lock(); if sidecar_listeners.is_empty() { - return; + return } let sidecar = Arc::new(sidecar.clone()); sidecar_listeners.retain_mut(|listener| { @@ -709,7 +709,7 @@ where hashes: Vec, ) -> Vec>> { if hashes.is_empty() { - return Vec::new(); + return Vec::new() } let removed = self.pool.write().remove_transactions(hashes); @@ -758,7 +758,7 @@ where A: HandleMempoolData, { if announcement.is_empty() { - return; + return } let pool = self.get_pool_data(); announcement.retain_by_hash(|tx| !pool.contains(tx)) @@ -852,7 +852,7 @@ where txs: Vec, ) -> Vec>> { if txs.is_empty() { - return Vec::new(); + return Vec::new() } self.get_pool_data().get_all(txs).collect() } @@ -860,7 +860,7 @@ where /// Notify about propagated transactions. pub(crate) fn on_propagated(&self, txs: PropagatedTransactions) { if txs.0.is_empty() { - return; + return } let mut listener = self.event_listener.write(); @@ -1080,9 +1080,9 @@ where loop { let next = self.iter.next()?; if self.kind.is_propagate_only() && !next.propagate { - continue; + continue } - return Some(*next.hash()); + return Some(*next.hash()) } } } @@ -1104,12 +1104,12 @@ where loop { let next = self.iter.next()?; if self.kind.is_propagate_only() && !next.propagate { - continue; + continue } return Some(NewTransactionEvent { subpool: SubPool::Pending, transaction: next.clone(), - }); + }) } } } From 96b6a6d9052c29d13f0dd0c7a16120e39305327c Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Tue, 5 Nov 2024 18:31:03 +0000 Subject: [PATCH 04/17] clean up diff --- crates/transaction-pool/src/pool/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 172f975df96f..4641d357f909 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -589,7 +589,7 @@ where if listener.kind.is_propagate_only() && !event.transaction.propagate { // only emit this hash to listeners that are only allowed to receive propagate only // transactions, such as network - return !listener.sender.is_closed(); + return !listener.sender.is_closed() } listener.send(event.clone()) @@ -727,7 +727,7 @@ where hashes: Vec, ) -> Vec>> { if hashes.is_empty() { - return Vec::new(); + return Vec::new() } let removed = self.pool.write().remove_transactions_and_descendants(hashes); From b2d0f45275d8d197c35e81615c5500dc7924e65d Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Tue, 5 Nov 2024 23:13:33 +0000 Subject: [PATCH 05/17] add stub for priority example --- crates/optimism/node/tests/it/main.rs | 3 + crates/optimism/node/tests/it/priority.rs | 70 +++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 crates/optimism/node/tests/it/priority.rs diff --git a/crates/optimism/node/tests/it/main.rs b/crates/optimism/node/tests/it/main.rs index b84dd7426c24..d0533fc4541c 100644 --- a/crates/optimism/node/tests/it/main.rs +++ b/crates/optimism/node/tests/it/main.rs @@ -3,4 +3,7 @@ #[cfg(feature = "optimism")] mod builder; +#[cfg(feature = "optimism")] +mod priority; + const fn main() {} diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs new file mode 100644 index 000000000000..43f5b023abf8 --- /dev/null +++ b/crates/optimism/node/tests/it/priority.rs @@ -0,0 +1,70 @@ +//! Node builder test that customizes priority of transactions in the block. +use reth_db::test_utils::create_test_rw_db; +use reth_node_api::{FullNodeTypes, NodeTypesWithEngine}; +use reth_node_builder::{components::ComponentsBuilder, NodeBuilder, NodeConfig}; +use reth_optimism_chainspec::{OpChainSpec, OP_DEV}; +use reth_optimism_node::{ + args::RollupArgs, + node::{ + OpConsensusBuilder, OpExecutorBuilder, OpNetworkBuilder, OpPayloadBuilder, OpPoolBuilder, + }, + OpEngineTypes, OptimismNode, +}; +use reth_optimism_payload_builder::builder::OpPayloadTransactions; + +#[derive(Clone)] +struct CustomTxPriority {} + +impl OpPayloadTransactions for CustomTxPriority { + fn best_transactions( + &self, + pool: Pool, + attr: reth_transaction_pool::BestTransactionsAttributes, + // TODO(Seva): More arguments in this trait implementation for PayloadAttributes, etc. + ) -> reth_transaction_pool::BestTransactionsFor + where + Pool: reth_transaction_pool::TransactionPool, + { + // TODO(Seva): Build a custom implementation here + pool.best_transactions_with_attributes(attr) + } +} + +fn build_components() -> ComponentsBuilder< + Node, + OpPoolBuilder, + OpPayloadBuilder, + OpNetworkBuilder, + OpExecutorBuilder, + OpConsensusBuilder, +> +where + Node: + FullNodeTypes>, +{ + let RollupArgs { disable_txpool_gossip, compute_pending_block, discovery_v4, .. } = + RollupArgs::default(); + ComponentsBuilder::default() + .node_types::() + .pool(OpPoolBuilder::default()) + .payload( + OpPayloadBuilder::new(compute_pending_block).with_transactions(CustomTxPriority {}), + ) + .network(OpNetworkBuilder { disable_txpool_gossip, disable_discovery_v4: !discovery_v4 }) + .executor(OpExecutorBuilder::default()) + .consensus(OpConsensusBuilder::default()) +} + +#[tokio::test] +async fn test_custom_block_priority_config() { + let config = NodeConfig::new(OP_DEV.clone()); + let db = create_test_rw_db(); + + let _builder = NodeBuilder::new(config) + .with_database(db) + .with_types::() + .with_components(build_components()) + .check_launch(); + + // TODO(Seva): Launch it for real and test the custom priority +} From f5f5cb295630bfdd7005a1501ac33d8d407e2043 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Wed, 6 Nov 2024 19:01:39 +0000 Subject: [PATCH 06/17] add new `PayloadTransactions` trait --- crates/optimism/node/tests/it/priority.rs | 25 +++- crates/optimism/payload/src/builder.rs | 27 ++-- crates/transaction-pool/src/pool/best.rs | 171 +++++++++++----------- crates/transaction-pool/src/pool/mod.rs | 4 +- crates/transaction-pool/src/traits.rs | 18 +++ 5 files changed, 138 insertions(+), 107 deletions(-) diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 43f5b023abf8..77e3205e8f8f 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -11,6 +11,12 @@ use reth_optimism_node::{ OpEngineTypes, OptimismNode, }; use reth_optimism_payload_builder::builder::OpPayloadTransactions; +use reth_primitives::TransactionSignedEcRecovered; +use reth_transaction_pool::{ + pool::{PayloadTransactionsChain, PayloadTransactionsFixed}, + test_utils::MockTransaction, + PayloadTransactions, +}; #[derive(Clone)] struct CustomTxPriority {} @@ -20,13 +26,24 @@ impl OpPayloadTransactions for CustomTxPriority { &self, pool: Pool, attr: reth_transaction_pool::BestTransactionsAttributes, - // TODO(Seva): More arguments in this trait implementation for PayloadAttributes, etc. - ) -> reth_transaction_pool::BestTransactionsFor + ) -> impl PayloadTransactions where Pool: reth_transaction_pool::TransactionPool, { - // TODO(Seva): Build a custom implementation here - pool.best_transactions_with_attributes(attr) + // Block composition: + // 1. Top-of-block transaction created by the nod + // 2. Best transactions from the pool + + // TODO: Proper transaction generation. This transaction won't even validate. + let top_of_block_tx: TransactionSignedEcRecovered = MockTransaction::eip4844().into(); + + PayloadTransactionsChain::new( + PayloadTransactionsFixed::single(top_of_block_tx), + // Allow 100k gas for the top-of-block transaction + Some(100_000), + pool.best_transactions_with_attributes(attr), + None, + ) } } diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index f0c6c04ce73a..cd600777d9b1 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -2,7 +2,7 @@ use std::{fmt::Display, sync::Arc}; -use alloy_consensus::EMPTY_OMMER_ROOT_HASH; +use alloy_consensus::{Transaction, EMPTY_OMMER_ROOT_HASH}; use alloy_eips::merge::BEACON_NONCE; use alloy_primitives::{Address, Bytes, B64, U256}; use alloy_rpc_types_engine::PayloadId; @@ -23,8 +23,7 @@ use reth_primitives::{ use reth_provider::{ProviderError, StateProviderFactory, StateRootProvider}; use reth_revm::database::StateProviderDatabase; use reth_transaction_pool::{ - noop::NoopTransactionPool, BestTransactions, BestTransactionsAttributes, BestTransactionsFor, - TransactionPool, + noop::NoopTransactionPool, BestTransactionsAttributes, PayloadTransactions, TransactionPool, }; use reth_trie::HashedPostState; use revm::{ @@ -398,7 +397,7 @@ pub trait OpPayloadTransactions: Clone + Send + Sync + Unpin + 'static { &self, pool: Pool, attr: BestTransactionsAttributes, - ) -> BestTransactionsFor; + ) -> impl PayloadTransactions; } impl OpPayloadTransactions for () { @@ -406,7 +405,7 @@ impl OpPayloadTransactions for () { &self, pool: Pool, attr: BestTransactionsAttributes, - ) -> BestTransactionsFor { + ) -> impl PayloadTransactions { pool.best_transactions_with_attributes(attr) } } @@ -732,7 +731,7 @@ where &self, info: &mut ExecutionInfo, db: &mut State, - mut best_txs: BestTransactionsFor, + mut best_txs: impl PayloadTransactions, ) -> Result>, PayloadBuilderError> where DB: Database, @@ -740,19 +739,19 @@ where { let block_gas_limit = self.block_gas_limit(); let base_fee = self.base_fee(); - while let Some(pool_tx) = best_txs.next() { + while let Some(tx) = best_txs.next(()) { // ensure we still have capacity for this transaction - if info.cumulative_gas_used + pool_tx.gas_limit() > block_gas_limit { + if info.cumulative_gas_used + tx.gas_limit() > block_gas_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 // the iterator before we can continue - best_txs.mark_invalid(&pool_tx); + best_txs.mark_invalid(tx.signer(), tx.nonce()); continue } // A sequencer's block should never contain blob or deposit transactions from the pool. - if pool_tx.is_eip4844() || pool_tx.tx_type() == TxType::Deposit as u8 { - best_txs.mark_invalid(&pool_tx); + if tx.is_eip4844() || tx.tx_type() == TxType::Deposit as u8 { + best_txs.mark_invalid(tx.signer(), tx.nonce()); continue } @@ -762,7 +761,6 @@ where } // convert tx to a signed transaction - let tx = pool_tx.to_recovered_transaction(); let env = EnvWithHandlerCfg::new_with_cfg_env( self.initialized_cfg.clone(), self.initialized_block_env.clone(), @@ -784,7 +782,8 @@ where // if the transaction is invalid, we can skip it and all of its // descendants trace!(target: "payload_builder", %err, ?tx, "skipping invalid transaction and its descendants"); - best_txs.mark_invalid(&pool_tx); + // FIXME: This call is currently a NOOP. + best_txs.mark_invalid(tx.signer(), tx.nonce()); } continue @@ -819,7 +818,7 @@ where // update add to total fees let miner_fee = tx - .effective_tip_per_gas(Some(base_fee)) + .effective_tip_per_gas(base_fee) .expect("fee is always valid; execution succeeded"); info.total_fees += U256::from(miner_fee) * U256::from(gas_used); diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 66cf54abe794..cd7f9ed5520d 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -1,10 +1,12 @@ use crate::{ identifier::{SenderId, TransactionId}, pool::pending::PendingTransaction, - PoolTransaction, TransactionOrdering, ValidPoolTransaction, + PayloadTransactions, PoolTransaction, TransactionOrdering, ValidPoolTransaction, }; +use alloy_consensus::Transaction; use alloy_primitives::Address; use core::fmt; +use reth_primitives::TransactionSignedEcRecovered; use std::{ collections::{BTreeMap, BTreeSet, HashSet, VecDeque}, sync::Arc, @@ -48,7 +50,7 @@ impl Iterator for BestTransactionsWithFees { fn next(&mut self) -> Option { // find the next transaction that satisfies the base fee loop { - let best = self.best.next()?; + let best = Iterator::next(&mut self.best)?; // If both the base fee and blob fee (if applicable for EIP-4844) are satisfied, return // the transaction if best.transaction.max_fee_per_gas() >= self.base_fee as u128 && @@ -196,6 +198,30 @@ impl Iterator for BestTransactions { } } +impl>> PayloadTransactions + for Box>>> +{ + fn next(&mut self, _ctx: ()) -> Option { + Iterator::next(self).map(|tx| tx.to_recovered_transaction()) + } + + fn mark_invalid(&mut self, _sender: Address, _nonce: u64) { + // TODO: Implement this. Can't use BestTransactions::mark_invalid directly + // because it requires a sender ID. + } +} + +#[cfg(any(test, feature = "test-utils"))] +impl PayloadTransactions + for Box> +{ + fn next(&mut self, _ctx: ()) -> Option { + Iterator::next(self).map(|tx| tx.into()) + } + + fn mark_invalid(&mut self, _sender: Address, _nonce: u64) {} +} + /// A [`BestTransactions`](crate::traits::BestTransactions) implementation that filters the /// transactions of iter with predicate. /// @@ -341,53 +367,43 @@ where } } -/// An implementation of [`crate::traits::BestTransactions`] that yields +/// An implementation of [`crate::traits::PayloadTransactions`] that yields /// a pre-defined set of transactions. /// /// This is useful to put a sequencer-specified set of transactions into the block /// and compose it with the rest of the transactions. #[derive(Debug)] -pub struct BestTransactionsFixed { +pub struct PayloadTransactionsFixed { transactions: Vec, index: usize, } -impl BestTransactionsFixed { - /// Constructs a new [`BestTransactionsFixed`]. +impl PayloadTransactionsFixed { + /// Constructs a new [`PayloadTransactionsFixed`]. pub fn new(transactions: Vec) -> Self { Self { transactions, index: Default::default() } } - /// Constructs a new [`BestTransactionsFixed`] with a single transaction. + /// Constructs a new [`PayloadTransactionsFixed`] with a single transaction. pub fn single(transaction: T) -> Self { Self { transactions: vec![transaction], index: Default::default() } } } -impl Iterator for BestTransactionsFixed { - type Item = T; - - fn next(&mut self) -> Option { +impl PayloadTransactions for PayloadTransactionsFixed { + fn next(&mut self, _ctx: ()) -> Option { (self.index < self.transactions.len()).then(|| { let tx = self.transactions[self.index].clone(); self.index += 1; tx }) } -} - -impl crate::traits::BestTransactions for BestTransactionsFixed { - fn mark_invalid(&mut self, _tx: &Self::Item) { - // TODO: Implement when it's supported on the main pool too - } - - fn no_updates(&mut self) {} - fn set_skip_blobs(&mut self, _skip_blobs: bool) {} + fn mark_invalid(&mut self, _sender: Address, _nonce: u64) {} } -/// Wrapper over [`crate::traits::BestTransactions`] that combines transactions from multiple -/// `BestTransactions` iterators and keeps track of the gas for both of iterators. +/// Wrapper over [`crate::traits::PayloadTransactions`] that combines transactions from multiple +/// `PayloadTransactions` iterators and keeps track of the gas for both of iterators. /// /// We can't use [`Iterator::chain`], because: /// (a) we need to propagate the `mark_invalid` and `no_updates` @@ -399,7 +415,7 @@ impl crate::traits::BestTransactions for BestTransactionsFixed< /// If the `before` iterator has transactions that are not fitting into the block, /// the after iterator will get propagated a `mark_invalid` call for each of them. #[derive(Debug)] -pub struct BestTransactionsChain { +pub struct PayloadTransactionsChain { /// Iterator that will be used first before: B, /// Allowed gas for the transactions from `before` iterator. If `None`, no gas limit is @@ -416,8 +432,8 @@ pub struct BestTransactionsChain { after_gas: u64, } -impl BestTransactionsChain { - /// Constructs a new [`BestTransactionsChain`]. +impl PayloadTransactionsChain { + /// Constructs a new [`PayloadTransactionsChain`]. pub fn new( before: B, before_max_gas: Option, @@ -435,35 +451,32 @@ impl BestTransactionsChain { } } -impl Iterator for BestTransactionsChain +impl PayloadTransactions for PayloadTransactionsChain where - B: crate::traits::BestTransactions>>, - A: crate::traits::BestTransactions>>, - T: PoolTransaction, + B: PayloadTransactions, + A: PayloadTransactions, { - type Item = Arc>; - - fn next(&mut self) -> Option { - while let Some(tx) = self.before.next() { + 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.transaction.gas_limit() <= before_max_gas { self.before_gas += tx.transaction.gas_limit(); return Some(tx); } - self.before.mark_invalid(&tx); - self.after.mark_invalid(&tx); + self.before.mark_invalid(tx.signer(), tx.transaction.nonce()); + self.after.mark_invalid(tx.signer(), tx.transaction.nonce()); } else { return Some(tx); } } - while let Some(tx) = self.after.next() { + while let Some(tx) = self.after.next(ctx) { if let Some(after_max_gas) = self.after_max_gas { if self.after_gas + tx.transaction.gas_limit() <= after_max_gas { self.after_gas += tx.transaction.gas_limit(); return Some(tx); } - self.after.mark_invalid(&tx); + self.after.mark_invalid(tx.signer(), tx.transaction.nonce()); } else { return Some(tx); } @@ -471,27 +484,10 @@ where None } -} - -impl crate::traits::BestTransactions for BestTransactionsChain -where - B: crate::traits::BestTransactions>>, - A: crate::traits::BestTransactions>>, - T: PoolTransaction, -{ - fn mark_invalid(&mut self, tx: &Self::Item) { - self.before.mark_invalid(tx); - self.after.mark_invalid(tx); - } - - fn no_updates(&mut self) { - self.before.no_updates(); - self.after.no_updates(); - } - fn set_skip_blobs(&mut self, skip_blobs: bool) { - self.before.set_skip_blobs(skip_blobs); - self.after.set_skip_blobs(skip_blobs); + fn mark_invalid(&mut self, sender: Address, nonce: u64) { + self.before.mark_invalid(sender, nonce); + self.after.mark_invalid(sender, nonce); } } @@ -573,9 +569,9 @@ mod tests { dyn crate::traits::BestTransactions>>, > = Box::new(pool.best()); - let tx = best.next().unwrap(); - best.mark_invalid(&tx); - assert!(best.next().is_none()); + let tx = Iterator::next(&mut best).unwrap(); + crate::traits::BestTransactions::mark_invalid(&mut *best, &tx); + assert!(Iterator::next(&mut best).is_none()); } #[test] @@ -967,39 +963,40 @@ mod tests { priority_pool.add_transaction(Arc::new(valid_prioritized_tx), 0); } - let block = BestTransactionsChain::new( - // Segment 1 - BestTransactionsFixed::single(Arc::new( - f.validated(MockTransaction::eip1559().with_sender(address_top_of_block)), - )), - Some(100), - BestTransactionsChain::new( - // Segment 2 - priority_pool.best(), - Some(100), - // Segment 3 + type MockPoolBestTransactions = + Box>>>; + let priority_pool_best: MockPoolBestTransactions = Box::new(priority_pool.best()); + let main_pool_best: MockPoolBestTransactions = Box::new(pool.best()); + let prioritized_main_pool: MockPoolBestTransactions = + Box::new(BestTransactionsWithPrioritizedSenders::new( + HashSet::from([address_a]), + 200, BestTransactionsWithPrioritizedSenders::new( - HashSet::from([address_a]), + HashSet::from([address_b]), 200, - // Segment 4 - BestTransactionsWithPrioritizedSenders::new( - HashSet::from([address_b]), - 200, - // Segment 5 - pool.best(), - ), + main_pool_best, ), + )); + + let mut block: Box = Box::new(PayloadTransactionsChain::new( + PayloadTransactionsFixed::single( + MockTransaction::eip1559().with_sender(address_top_of_block).into(), + ), + Some(100), + PayloadTransactionsChain::new( + priority_pool_best, + Some(100), + prioritized_main_pool, None, ), None, - ); - - let mut iter = block.into_iter(); - assert_eq!(iter.next().unwrap().sender(), address_top_of_block); - assert_eq!(iter.next().unwrap().sender(), address_in_priority_pool); - assert_eq!(iter.next().unwrap().sender(), address_a); - assert_eq!(iter.next().unwrap().sender(), address_b); - assert_eq!(iter.next().unwrap().sender(), address_regular); + )); + + 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); } // TODO: Same nonce test diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index b71c3478ea21..a15867e2c3aa 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -107,8 +107,8 @@ use crate::{ validate::ValidTransaction, }; pub use best::{ - BestTransactionFilter, BestTransactionsChain, BestTransactionsFixed, - BestTransactionsWithPrioritizedSenders, + BestTransactionFilter, BestTransactionsWithPrioritizedSenders, PayloadTransactionsChain, + PayloadTransactionsFixed, }; pub use blob::{blob_tx_priority, fee_delta}; pub use events::{FullTransactionEvent, TransactionEvent}; diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index 6be25cb2ecc0..f21ce056fc9b 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1501,6 +1501,24 @@ impl Stream for NewSubpoolTransactionStream { } } +/// Iterator that returns transactions for the block building process in the order they should be +/// included in the block. +/// +/// Can include transactions from the pool and other sources (alternative pools, +/// sequencer-originated transactions, etc.). +pub trait PayloadTransactions { + /// Returns the next transaction to include in the block. + fn next( + &mut self, + // In the future, `ctx` can include access to state for block building purposes. + ctx: (), + ) -> Option; + + /// Exclude descendants of the transaction with given sender and nonce from the iterator, + /// because this transaction won't be included in the block. + fn mark_invalid(&mut self, sender: Address, nonce: u64); +} + #[cfg(test)] mod tests { use super::*; From 2ac46c3218d6c692cc454bb24e53ccc571f56814 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Wed, 6 Nov 2024 19:11:30 +0000 Subject: [PATCH 07/17] ci fixes --- crates/optimism/node/tests/it/priority.rs | 4 ++-- crates/transaction-pool/src/pool/best.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 77e3205e8f8f..829d90646c25 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -8,7 +8,7 @@ use reth_optimism_node::{ node::{ OpConsensusBuilder, OpExecutorBuilder, OpNetworkBuilder, OpPayloadBuilder, OpPoolBuilder, }, - OpEngineTypes, OptimismNode, + OpEngineTypes, OpNode, }; use reth_optimism_payload_builder::builder::OpPayloadTransactions; use reth_primitives::TransactionSignedEcRecovered; @@ -79,7 +79,7 @@ async fn test_custom_block_priority_config() { let _builder = NodeBuilder::new(config) .with_database(db) - .with_types::() + .with_types::() .with_components(build_components()) .check_launch(); diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index cd7f9ed5520d..2ac20e63a359 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -409,7 +409,7 @@ impl PayloadTransactions for PayloadTransactionsFixed Date: Wed, 6 Nov 2024 19:17:21 +0000 Subject: [PATCH 08/17] doc update --- crates/optimism/payload/src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index cd600777d9b1..57e199b2a00d 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -389,7 +389,7 @@ where } } -/// A type that returns a the [`BestTransactions`] that should be included in the pool. +/// A type that returns a the [`PayloadTransactions`] that should be included in the pool. 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. From f3804dae670c2cdc3bb3b5135daf384af01f61ed Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 19:27:49 +0000 Subject: [PATCH 09/17] move test utils to node library --- Cargo.lock | 4 ++++ crates/optimism/node/Cargo.toml | 16 ++++++++++++++-- crates/optimism/node/src/lib.rs | 4 ++++ .../optimism/node/{tests/e2e => src}/utils.rs | 17 +++++++++-------- crates/optimism/node/tests/e2e/main.rs | 3 --- crates/optimism/node/tests/e2e/p2p.rs | 2 +- 6 files changed, 32 insertions(+), 14 deletions(-) rename crates/optimism/node/{tests/e2e => src}/utils.rs (74%) diff --git a/Cargo.lock b/Cargo.lock index fba1e61e037a..708c863607e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8254,10 +8254,13 @@ dependencies = [ name = "reth-optimism-node" version = "1.1.1" dependencies = [ + "alloy-consensus", "alloy-eips", "alloy-genesis", + "alloy-network", "alloy-primitives", "alloy-rpc-types-engine", + "alloy-signer-local", "clap", "eyre", "op-alloy-consensus", @@ -8280,6 +8283,7 @@ dependencies = [ "reth-optimism-consensus", "reth-optimism-evm", "reth-optimism-forks", + "reth-optimism-node", "reth-optimism-payload-builder", "reth-optimism-rpc", "reth-payload-builder", diff --git a/crates/optimism/node/Cargo.toml b/crates/optimism/node/Cargo.toml index deabbac52499..4a4e95be1a90 100644 --- a/crates/optimism/node/Cargo.toml +++ b/crates/optimism/node/Cargo.toml @@ -56,17 +56,25 @@ parking_lot.workspace = true # rpc serde_json.workspace = true +# test-utils dependencies +reth = { workspace = true, optional = true } +reth-e2e-test-utils = { workspace = true, optional = true } +alloy-genesis = { workspace = true, optional = true } +tokio = { workspace = true, optional = true } + [dev-dependencies] -reth.workspace = true reth-db.workspace = true -reth-e2e-test-utils.workspace = true reth-node-builder = { workspace = true, features = ["test-utils"] } reth-provider = { workspace = true, features = ["test-utils"] } reth-revm = { workspace = true, features = ["test-utils"] } +reth-optimism-node = { workspace = true, features = ["test-utils"] } tokio.workspace = true alloy-primitives.workspace = true alloy-genesis.workspace = true op-alloy-consensus.workspace = true +alloy-signer-local.workspace = true +alloy-network.workspace = true +alloy-consensus.workspace = true [features] optimism = [ @@ -89,6 +97,10 @@ asm-keccak = [ "revm/asm-keccak" ] test-utils = [ + "reth", + "reth-e2e-test-utils", + "alloy-genesis", + "tokio", "reth-node-builder/test-utils", "reth-chainspec/test-utils", "reth-consensus/test-utils", diff --git a/crates/optimism/node/src/lib.rs b/crates/optimism/node/src/lib.rs index 6419611067e4..7af0f3b8a722 100644 --- a/crates/optimism/node/src/lib.rs +++ b/crates/optimism/node/src/lib.rs @@ -22,6 +22,10 @@ pub use node::OpNode; pub mod txpool; +/// Helpers for running test node instances. +#[cfg(feature = "test-utils")] +pub mod utils; + pub use reth_optimism_payload_builder::{ OpBuiltPayload, OpPayloadBuilder, OpPayloadBuilderAttributes, }; diff --git a/crates/optimism/node/tests/e2e/utils.rs b/crates/optimism/node/src/utils.rs similarity index 74% rename from crates/optimism/node/tests/e2e/utils.rs rename to crates/optimism/node/src/utils.rs index a8afab87ec26..e3ea3c59b411 100644 --- a/crates/optimism/node/tests/e2e/utils.rs +++ b/crates/optimism/node/src/utils.rs @@ -1,3 +1,4 @@ +use crate::{node::OptimismAddOns, OpNode as OtherOpNode}; use alloy_genesis::Genesis; use alloy_primitives::{Address, B256}; use reth::{rpc::types::engine::PayloadAttributes, tasks::TaskManager}; @@ -5,18 +6,18 @@ use reth_e2e_test_utils::{ transaction::TransactionTestContext, wallet::Wallet, Adapter, NodeHelperType, }; use reth_optimism_chainspec::OpChainSpecBuilder; -use reth_optimism_node::{ - node::OptimismAddOns, OpBuiltPayload, OpNode as OtherOpNode, OpPayloadBuilderAttributes, -}; +use reth_optimism_payload_builder::{OpBuiltPayload, OpPayloadBuilderAttributes}; use reth_payload_builder::EthPayloadBuilderAttributes; use std::sync::Arc; use tokio::sync::Mutex; /// Optimism Node Helper type -pub(crate) type OpNode = NodeHelperType>>; +pub type OpNode = NodeHelperType>>; -pub(crate) async fn setup(num_nodes: usize) -> eyre::Result<(Vec, TaskManager, Wallet)> { - let genesis: Genesis = serde_json::from_str(include_str!("../assets/genesis.json")).unwrap(); +/// Creates the initial setup with `num_nodes` of the node config, started and connected. +pub async fn setup(num_nodes: usize) -> eyre::Result<(Vec, TaskManager, Wallet)> { + let genesis: Genesis = + serde_json::from_str(include_str!("../tests/assets/genesis.json")).unwrap(); reth_e2e_test_utils::setup( num_nodes, Arc::new(OpChainSpecBuilder::base_mainnet().genesis(genesis).ecotone_activated().build()), @@ -27,7 +28,7 @@ pub(crate) async fn setup(num_nodes: usize) -> eyre::Result<(Vec, TaskMa } /// Advance the chain with sequential payloads returning them in the end. -pub(crate) async fn advance_chain( +pub async fn advance_chain( length: usize, node: &mut OpNode, wallet: Arc>, @@ -49,7 +50,7 @@ pub(crate) async fn advance_chain( } /// Helper function to create a new eth payload attributes -pub(crate) fn optimism_payload_attributes(timestamp: u64) -> OpPayloadBuilderAttributes { +pub fn optimism_payload_attributes(timestamp: u64) -> OpPayloadBuilderAttributes { let attributes = PayloadAttributes { timestamp, prev_randao: B256::ZERO, diff --git a/crates/optimism/node/tests/e2e/main.rs b/crates/optimism/node/tests/e2e/main.rs index 3438c766048b..7f4b22ba7e04 100644 --- a/crates/optimism/node/tests/e2e/main.rs +++ b/crates/optimism/node/tests/e2e/main.rs @@ -3,7 +3,4 @@ #[cfg(feature = "optimism")] mod p2p; -#[cfg(feature = "optimism")] -mod utils; - const fn main() {} diff --git a/crates/optimism/node/tests/e2e/p2p.rs b/crates/optimism/node/tests/e2e/p2p.rs index 30affa9bafb9..6cb37345aa1d 100644 --- a/crates/optimism/node/tests/e2e/p2p.rs +++ b/crates/optimism/node/tests/e2e/p2p.rs @@ -1,6 +1,6 @@ -use crate::utils::{advance_chain, setup}; use alloy_rpc_types_engine::PayloadStatusEnum; use reth::blockchain_tree::error::BlockchainTreeError; +use reth_optimism_node::utils::{advance_chain, setup}; use std::sync::Arc; use tokio::sync::Mutex; From db115b46afb416ddf47cee7cb6bcd9f4f194a08f Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 19:29:43 +0000 Subject: [PATCH 10/17] e2e priority test --- crates/optimism/node/tests/it/priority.rs | 151 ++++++++++++++++++---- 1 file changed, 127 insertions(+), 24 deletions(-) diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 829d90646c25..80b0bf9e4493 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -1,25 +1,43 @@ //! Node builder test that customizes priority of transactions in the block. -use reth_db::test_utils::create_test_rw_db; + +use alloy_consensus::TxEip1559; +use alloy_genesis::Genesis; +use alloy_network::TxSignerSync; +use alloy_primitives::{Address, ChainId, TxKind}; +use reth::{args::DatadirArgs, tasks::TaskManager}; +use reth_chainspec::EthChainSpec; +use reth_db::test_utils::create_test_rw_db_with_path; +use reth_e2e_test_utils::{ + node::NodeTestContext, transaction::TransactionTestContext, wallet::Wallet, +}; use reth_node_api::{FullNodeTypes, NodeTypesWithEngine}; -use reth_node_builder::{components::ComponentsBuilder, NodeBuilder, NodeConfig}; -use reth_optimism_chainspec::{OpChainSpec, OP_DEV}; +use reth_node_builder::{ + components::ComponentsBuilder, EngineNodeLauncher, NodeBuilder, NodeConfig, +}; +use reth_optimism_chainspec::{OpChainSpec, OpChainSpecBuilder}; use reth_optimism_node::{ args::RollupArgs, node::{ OpConsensusBuilder, OpExecutorBuilder, OpNetworkBuilder, OpPayloadBuilder, OpPoolBuilder, + OptimismAddOns, }, + utils::optimism_payload_attributes, OpEngineTypes, OpNode, }; use reth_optimism_payload_builder::builder::OpPayloadTransactions; -use reth_primitives::TransactionSignedEcRecovered; +use reth_primitives::{SealedBlock, Transaction, TransactionSigned, TransactionSignedEcRecovered}; +use reth_provider::providers::BlockchainProvider2; use reth_transaction_pool::{ pool::{PayloadTransactionsChain, PayloadTransactionsFixed}, - test_utils::MockTransaction, PayloadTransactions, }; +use std::sync::Arc; +use tokio::sync::Mutex; -#[derive(Clone)] -struct CustomTxPriority {} +#[derive(Clone, Debug)] +struct CustomTxPriority { + chain_id: ChainId, +} impl OpPayloadTransactions for CustomTxPriority { fn best_transactions( @@ -31,23 +49,44 @@ impl OpPayloadTransactions for CustomTxPriority { Pool: reth_transaction_pool::TransactionPool, { // Block composition: - // 1. Top-of-block transaction created by the nod - // 2. Best transactions from the pool + // 1. Best transactions from the pool (up to 250k gas) + // 2. End-of-block transaction created by the node (up to 100k gas) - // TODO: Proper transaction generation. This transaction won't even validate. - let top_of_block_tx: TransactionSignedEcRecovered = MockTransaction::eip4844().into(); + // End of block transaction should send a 0-value transfer to a random address. + let sender = Wallet::default().inner; + let mut end_of_block_tx = TxEip1559 { + chain_id: self.chain_id, + nonce: 1, // it will be 2nd tx after L1 block info tx that uses the same sender + gas_limit: 21000, + max_fee_per_gas: 20e9 as u128, + to: TxKind::Call(Address::random()), + value: 0.try_into().unwrap(), + ..Default::default() + }; + let signature = sender.sign_transaction_sync(&mut end_of_block_tx).unwrap(); + let end_of_block_tx = TransactionSignedEcRecovered::from_signed_transaction( + TransactionSigned::from_transaction_and_signature( + Transaction::Eip1559(end_of_block_tx), + signature, + ), + sender.address(), + ); PayloadTransactionsChain::new( - PayloadTransactionsFixed::single(top_of_block_tx), - // Allow 100k gas for the top-of-block transaction - Some(100_000), pool.best_transactions_with_attributes(attr), - None, + // Allow 250k gas for the transactions from the pool + Some(250_000), + PayloadTransactionsFixed::single(end_of_block_tx), + // Allow 100k gas for the end-of-block transaction + Some(100_000), ) } } -fn build_components() -> ComponentsBuilder< +/// Builds the node with custom transaction priority service within default payload builder. +fn build_components( + chain_id: ChainId, +) -> ComponentsBuilder< Node, OpPoolBuilder, OpPayloadBuilder, @@ -65,7 +104,8 @@ where .node_types::() .pool(OpPoolBuilder::default()) .payload( - OpPayloadBuilder::new(compute_pending_block).with_transactions(CustomTxPriority {}), + OpPayloadBuilder::new(compute_pending_block) + .with_transactions(CustomTxPriority { chain_id }), ) .network(OpNetworkBuilder { disable_txpool_gossip, disable_discovery_v4: !discovery_v4 }) .executor(OpExecutorBuilder::default()) @@ -74,14 +114,77 @@ where #[tokio::test] async fn test_custom_block_priority_config() { - let config = NodeConfig::new(OP_DEV.clone()); - let db = create_test_rw_db(); + reth_tracing::init_test_tracing(); + + let genesis: Genesis = serde_json::from_str(include_str!("../assets/genesis.json")).unwrap(); + let chain_spec = + Arc::new(OpChainSpecBuilder::base_mainnet().genesis(genesis).ecotone_activated().build()); - let _builder = NodeBuilder::new(config) + // This wallet is going to send: + // 1. L1 block info tx + // 2. End-of-block custom tx + let wallet = Arc::new(Mutex::new(Wallet::default().with_chain_id(chain_spec.chain().into()))); + + // Configure and launch the node. + let config = NodeConfig::new(chain_spec).with_datadir_args(DatadirArgs { + datadir: reth_db::test_utils::tempdir_path().into(), + ..Default::default() + }); + let db = create_test_rw_db_with_path( + config + .datadir + .datadir + .unwrap_or_chain_default(config.chain.chain(), config.datadir.clone()) + .db(), + ); + let tasks = TaskManager::current(); + let node_handle = NodeBuilder::new(config.clone()) .with_database(db) - .with_types::() - .with_components(build_components()) - .check_launch(); + .with_types_and_provider::>() + .with_components(build_components(config.chain.chain_id())) + .with_add_ons(OptimismAddOns::default()) + .launch_with_fn(|builder| { + let launcher = EngineNodeLauncher::new( + tasks.executor(), + builder.config.datadir(), + Default::default(), + ); + builder.launch_with(launcher) + }) + .await + .expect("Failed to launch node"); + + // Advance the chain with a single block. + let block_payloads = NodeTestContext::new(node_handle.node, optimism_payload_attributes) + .await + .unwrap() + .advance(1, |_| { + let wallet = wallet.clone(); + Box::pin(async move { + let mut wallet = wallet.lock().await; + let tx_fut = TransactionTestContext::optimism_l1_block_info_tx( + wallet.chain_id, + wallet.inner.clone(), + // This doesn't matter in the current test (because it's only one block), + // but make sure you're not reusing the nonce from end-of-block tx + // if they have the same signer. + wallet.inner_nonce * 2, + ); + wallet.inner_nonce += 1; + tx_fut.await + }) + }) + .await + .unwrap(); + assert_eq!(block_payloads.len(), 1); + let (block_payload, _) = block_payloads.first().unwrap(); + let block_payload: SealedBlock = block_payload.block().clone(); + assert_eq!(block_payload.body.transactions.len(), 2); // L1 block info tx + end-of-block custom tx - // TODO(Seva): Launch it for real and test the custom priority + // Check that last transaction in the block looks like a transfer to a random address. + let end_of_block_tx = block_payload.body.transactions.last().unwrap(); + let end_of_block_tx = end_of_block_tx.transaction.as_eip1559().unwrap(); + assert_eq!(end_of_block_tx.nonce, 1); + assert_eq!(end_of_block_tx.gas_limit, 21_000); + assert!(end_of_block_tx.input.is_empty()); } From 18a6e37629a1ea432b471b618d3de69b76d0a372 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 19:34:21 +0000 Subject: [PATCH 11/17] remove fixme --- crates/optimism/payload/src/builder.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 57e199b2a00d..0c4a7203b43b 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -782,7 +782,6 @@ where // if the transaction is invalid, we can skip it and all of its // descendants trace!(target: "payload_builder", %err, ?tx, "skipping invalid transaction and its descendants"); - // FIXME: This call is currently a NOOP. best_txs.mark_invalid(tx.signer(), tx.nonce()); } From c4ac870266a2d2d1a98f494100f2a24e783605d2 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 19:55:01 +0000 Subject: [PATCH 12/17] clean up after resolving conflicts --- crates/optimism/node/src/utils.rs | 5 +---- crates/optimism/node/tests/it/priority.rs | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/optimism/node/src/utils.rs b/crates/optimism/node/src/utils.rs index 38afda5b1c18..b54015fef0cc 100644 --- a/crates/optimism/node/src/utils.rs +++ b/crates/optimism/node/src/utils.rs @@ -1,4 +1,4 @@ -use crate::{OpNode as OtherOpNode}; +use crate::{node::OpAddOns, OpBuiltPayload, OpNode as OtherOpNode, OpPayloadBuilderAttributes}; use alloy_genesis::Genesis; use alloy_primitives::{Address, B256}; use reth::{rpc::types::engine::PayloadAttributes, tasks::TaskManager}; @@ -6,9 +6,6 @@ use reth_e2e_test_utils::{ transaction::TransactionTestContext, wallet::Wallet, Adapter, NodeHelperType, }; use reth_optimism_chainspec::OpChainSpecBuilder; -use reth_optimism_node::{ - node::OpAddOns, OpBuiltPayload, OpNode as OtherOpNode, OpPayloadBuilderAttributes, -}; use reth_payload_builder::EthPayloadBuilderAttributes; use std::sync::Arc; use tokio::sync::Mutex; diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 80b0bf9e4493..773dc2f1469b 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -18,8 +18,8 @@ use reth_optimism_chainspec::{OpChainSpec, OpChainSpecBuilder}; use reth_optimism_node::{ args::RollupArgs, node::{ - OpConsensusBuilder, OpExecutorBuilder, OpNetworkBuilder, OpPayloadBuilder, OpPoolBuilder, - OptimismAddOns, + OpAddOns, OpConsensusBuilder, OpExecutorBuilder, OpNetworkBuilder, OpPayloadBuilder, + OpPoolBuilder, }, utils::optimism_payload_attributes, OpEngineTypes, OpNode, @@ -142,7 +142,7 @@ async fn test_custom_block_priority_config() { .with_database(db) .with_types_and_provider::>() .with_components(build_components(config.chain.chain_id())) - .with_add_ons(OptimismAddOns::default()) + .with_add_ons(OpAddOns::default()) .launch_with_fn(|builder| { let launcher = EngineNodeLauncher::new( tasks.executor(), From 13c42ccf9e794ff07f241b1e118619c0b6ee81ef Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 20:04:22 +0000 Subject: [PATCH 13/17] propagate features --- crates/optimism/node/Cargo.toml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/optimism/node/Cargo.toml b/crates/optimism/node/Cargo.toml index 4a4e95be1a90..aa658a6dc719 100644 --- a/crates/optimism/node/Cargo.toml +++ b/crates/optimism/node/Cargo.toml @@ -63,11 +63,11 @@ alloy-genesis = { workspace = true, optional = true } tokio = { workspace = true, optional = true } [dev-dependencies] +reth-optimism-node = { workspace = true, features = ["test-utils"] } reth-db.workspace = true reth-node-builder = { workspace = true, features = ["test-utils"] } reth-provider = { workspace = true, features = ["test-utils"] } reth-revm = { workspace = true, features = ["test-utils"] } -reth-optimism-node = { workspace = true, features = ["test-utils"] } tokio.workspace = true alloy-primitives.workspace = true alloy-genesis.workspace = true @@ -88,13 +88,15 @@ optimism = [ "reth-optimism-rpc/optimism", "reth-engine-local/optimism", "reth-optimism-consensus/optimism", - "reth-db/optimism" + "reth-db/optimism", + "reth-optimism-node/optimism" ] asm-keccak = [ "reth-primitives/asm-keccak", "reth/asm-keccak", "alloy-primitives/asm-keccak", - "revm/asm-keccak" + "revm/asm-keccak", + "reth-optimism-node/asm-keccak" ] test-utils = [ "reth", @@ -113,5 +115,6 @@ test-utils = [ "reth-provider/test-utils", "reth-transaction-pool/test-utils", "reth-trie-db/test-utils", - "revm/test-utils" + "revm/test-utils", + "reth-optimism-node/test-utils" ] From 5d1bd9aa3dd71d1a73751d166c1a5096300de02b Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 20:06:26 +0000 Subject: [PATCH 14/17] remove duplicated deps --- crates/optimism/node/Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/optimism/node/Cargo.toml b/crates/optimism/node/Cargo.toml index aa658a6dc719..6084043404dd 100644 --- a/crates/optimism/node/Cargo.toml +++ b/crates/optimism/node/Cargo.toml @@ -68,9 +68,7 @@ reth-db.workspace = true reth-node-builder = { workspace = true, features = ["test-utils"] } reth-provider = { workspace = true, features = ["test-utils"] } reth-revm = { workspace = true, features = ["test-utils"] } -tokio.workspace = true alloy-primitives.workspace = true -alloy-genesis.workspace = true op-alloy-consensus.workspace = true alloy-signer-local.workspace = true alloy-network.workspace = true From deb4200b10ea00c966c632fa449484c76ab91cd2 Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 21:13:23 +0000 Subject: [PATCH 15/17] refactor to use a wrapper BestPayloadTransactions struct --- crates/optimism/node/tests/it/priority.rs | 4 +- crates/optimism/payload/src/builder.rs | 3 +- crates/transaction-pool/src/pool/best.rs | 76 +++++++++++++---------- crates/transaction-pool/src/pool/mod.rs | 4 +- 4 files changed, 50 insertions(+), 37 deletions(-) diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 773dc2f1469b..52e3bef3d918 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -28,7 +28,7 @@ use reth_optimism_payload_builder::builder::OpPayloadTransactions; use reth_primitives::{SealedBlock, Transaction, TransactionSigned, TransactionSignedEcRecovered}; use reth_provider::providers::BlockchainProvider2; use reth_transaction_pool::{ - pool::{PayloadTransactionsChain, PayloadTransactionsFixed}, + pool::{BestPayloadTransactions, PayloadTransactionsChain, PayloadTransactionsFixed}, PayloadTransactions, }; use std::sync::Arc; @@ -73,7 +73,7 @@ impl OpPayloadTransactions for CustomTxPriority { ); PayloadTransactionsChain::new( - pool.best_transactions_with_attributes(attr), + BestPayloadTransactions::new(pool.best_transactions_with_attributes(attr)), // Allow 250k gas for the transactions from the pool Some(250_000), PayloadTransactionsFixed::single(end_of_block_tx), diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 8d48c79fdff3..42326de6ea49 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -38,6 +38,7 @@ use crate::{ payload::{OpBuiltPayload, OpPayloadBuilderAttributes}, }; use op_alloy_consensus::DepositTransaction; +use reth_transaction_pool::pool::BestPayloadTransactions; /// Optimism's payload builder #[derive(Debug, Clone, PartialEq, Eq)] @@ -406,7 +407,7 @@ impl OpPayloadTransactions for () { pool: Pool, attr: BestTransactionsAttributes, ) -> impl PayloadTransactions { - pool.best_transactions_with_attributes(attr) + BestPayloadTransactions::new(pool.best_transactions_with_attributes(attr)) } } diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 2ac20e63a359..fee629027c83 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -198,28 +198,47 @@ impl Iterator for BestTransactions { } } -impl>> PayloadTransactions - for Box>>> +/// 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>>, { - fn next(&mut self, _ctx: ()) -> Option { - Iterator::next(self).map(|tx| tx.to_recovered_transaction()) - } + invalid: HashSet
, + best: I, +} - fn mark_invalid(&mut self, _sender: Address, _nonce: u64) { - // TODO: Implement this. Can't use BestTransactions::mark_invalid directly - // because it requires a sender ID. +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 } } } -#[cfg(any(test, feature = "test-utils"))] -impl PayloadTransactions - for Box> +impl PayloadTransactions for BestPayloadTransactions +where + T: PoolTransaction>, + I: Iterator>>, { fn next(&mut self, _ctx: ()) -> Option { - Iterator::next(self).map(|tx| tx.into()) + loop { + let tx = self.best.next()?; + if self.invalid.contains(&tx.sender()) { + continue + } + return Some(tx.to_recovered_transaction()) + } } - fn mark_invalid(&mut self, _sender: Address, _nonce: u64) {} + fn mark_invalid(&mut self, sender: Address, _nonce: u64) { + self.invalid.insert(sender); + } } /// A [`BestTransactions`](crate::traits::BestTransactions) implementation that filters the @@ -963,34 +982,27 @@ mod tests { priority_pool.add_transaction(Arc::new(valid_prioritized_tx), 0); } - type MockPoolBestTransactions = - Box>>>; - let priority_pool_best: MockPoolBestTransactions = Box::new(priority_pool.best()); - let main_pool_best: MockPoolBestTransactions = Box::new(pool.best()); - let prioritized_main_pool: MockPoolBestTransactions = - Box::new(BestTransactionsWithPrioritizedSenders::new( - HashSet::from([address_a]), - 200, - BestTransactionsWithPrioritizedSenders::new( - HashSet::from([address_b]), - 200, - main_pool_best, - ), - )); - - let mut block: Box = Box::new(PayloadTransactionsChain::new( + let mut block = PayloadTransactionsChain::new( PayloadTransactionsFixed::single( MockTransaction::eip1559().with_sender(address_top_of_block).into(), ), Some(100), PayloadTransactionsChain::new( - priority_pool_best, + BestPayloadTransactions::new(priority_pool.best()), Some(100), - prioritized_main_pool, + 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); diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index a15867e2c3aa..76b2490b12fa 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -107,8 +107,8 @@ use crate::{ validate::ValidTransaction, }; pub use best::{ - BestTransactionFilter, BestTransactionsWithPrioritizedSenders, PayloadTransactionsChain, - PayloadTransactionsFixed, + BestPayloadTransactions, BestTransactionFilter, BestTransactionsWithPrioritizedSenders, + PayloadTransactionsChain, PayloadTransactionsFixed, }; pub use blob::{blob_tx_priority, fee_delta}; pub use events::{FullTransactionEvent, TransactionEvent}; From 70e5dbcd37919d53f04bb102bd60ec56fadc25dc Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 21:15:21 +0000 Subject: [PATCH 16/17] doc lints --- crates/transaction-pool/src/pool/best.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index fee629027c83..3614a8b9a3ff 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -198,8 +198,8 @@ impl Iterator for BestTransactions { } } -/// Wrapper struct that allows to convert BestTransactions (used in tx pool) to PayloadTransactions -/// (used in block composition). +/// Wrapper struct that allows to convert [`BestTransactions`] (used in tx pool) to +/// [`PayloadTransactions`] (used in block composition). #[derive(Debug)] pub struct BestPayloadTransactions where @@ -215,7 +215,7 @@ where T: PoolTransaction>, I: Iterator>>, { - /// Create a new BestPayloadTransactions with the given iterator. + /// Create a new `BestPayloadTransactions` with the given iterator. pub fn new(best: I) -> Self { Self { invalid: Default::default(), best } } From 672859f12f70339275e9488910478d71d7d9b9dc Mon Sep 17 00:00:00 2001 From: sevazhidkov Date: Thu, 7 Nov 2024 21:21:55 +0000 Subject: [PATCH 17/17] tiny doc fix --- crates/transaction-pool/src/pool/best.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 3614a8b9a3ff..48afa5003b87 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -198,8 +198,8 @@ impl Iterator for BestTransactions { } } -/// Wrapper struct that allows to convert [`BestTransactions`] (used in tx pool) to -/// [`PayloadTransactions`] (used in block composition). +/// Wrapper struct that allows to convert `BestTransactions` (used in tx pool) to +/// `PayloadTransactions` (used in block composition). #[derive(Debug)] pub struct BestPayloadTransactions where