From 5276093e71ff48bfcc8c1cab00ae7c946eb4418f Mon Sep 17 00:00:00 2001 From: Ayodeji Akinola Date: Sat, 16 Nov 2024 05:39:34 +0100 Subject: [PATCH] chore(util): Add reth payload util (#12590) --- Cargo.lock | 12 ++ Cargo.toml | 2 + crates/optimism/node/Cargo.toml | 1 + crates/optimism/node/tests/it/priority.rs | 6 +- crates/optimism/payload/Cargo.toml | 1 + crates/optimism/payload/src/builder.rs | 3 +- crates/payload/util/Cargo.toml | 20 ++++ crates/payload/util/src/lib.rs | 15 +++ crates/payload/util/src/traits.rs | 20 ++++ crates/payload/util/src/transaction.rs | 128 +++++++++++++++++++++ crates/transaction-pool/Cargo.toml | 1 + crates/transaction-pool/src/pool/best.rs | 131 +--------------------- crates/transaction-pool/src/pool/mod.rs | 1 - crates/transaction-pool/src/traits.rs | 18 --- 14 files changed, 207 insertions(+), 152 deletions(-) create mode 100644 crates/payload/util/Cargo.toml create mode 100644 crates/payload/util/src/lib.rs create mode 100644 crates/payload/util/src/traits.rs create mode 100644 crates/payload/util/src/transaction.rs diff --git a/Cargo.lock b/Cargo.lock index ac3dd4fa9882..9514d361fbfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8324,6 +8324,7 @@ dependencies = [ "reth-optimism-payload-builder", "reth-optimism-rpc", "reth-payload-builder", + "reth-payload-util", "reth-primitives", "reth-provider", "reth-revm", @@ -8359,6 +8360,7 @@ dependencies = [ "reth-optimism-forks", "reth-payload-builder", "reth-payload-primitives", + "reth-payload-util", "reth-primitives", "reth-provider", "reth-revm", @@ -8486,6 +8488,15 @@ dependencies = [ "tracing", ] +[[package]] +name = "reth-payload-util" +version = "1.1.1" +dependencies = [ + "alloy-consensus", + "alloy-primitives", + "reth-primitives", +] + [[package]] name = "reth-payload-validator" version = "1.1.1" @@ -9265,6 +9276,7 @@ dependencies = [ "reth-execution-types", "reth-fs-util", "reth-metrics", + "reth-payload-util", "reth-primitives", "reth-primitives-traits", "reth-provider", diff --git a/Cargo.toml b/Cargo.toml index 3f65bceb4bf8..398be3e5faf7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,6 +82,7 @@ members = [ "crates/payload/builder/", "crates/payload/primitives/", "crates/payload/validator/", + "crates/payload/util/", "crates/primitives-traits/", "crates/primitives/", "crates/prune/prune", @@ -381,6 +382,7 @@ reth-optimism-storage = { path = "crates/optimism/storage" } reth-payload-builder = { path = "crates/payload/builder" } reth-payload-primitives = { path = "crates/payload/primitives" } reth-payload-validator = { path = "crates/payload/validator" } +reth-payload-util = { path = "crates/payload/util" } reth-primitives = { path = "crates/primitives", default-features = false, features = [ "std", ] } diff --git a/crates/optimism/node/Cargo.toml b/crates/optimism/node/Cargo.toml index fb8cc27787e3..9a80c83deec1 100644 --- a/crates/optimism/node/Cargo.toml +++ b/crates/optimism/node/Cargo.toml @@ -16,6 +16,7 @@ reth-chainspec.workspace = true reth-engine-local.workspace = true reth-primitives.workspace = true reth-payload-builder.workspace = true +reth-payload-util.workspace = true reth-basic-payload-builder.workspace = true reth-consensus.workspace = true reth-node-api.workspace = true diff --git a/crates/optimism/node/tests/it/priority.rs b/crates/optimism/node/tests/it/priority.rs index 52e3bef3d918..f1260d2da01f 100644 --- a/crates/optimism/node/tests/it/priority.rs +++ b/crates/optimism/node/tests/it/priority.rs @@ -25,12 +25,10 @@ use reth_optimism_node::{ OpEngineTypes, OpNode, }; use reth_optimism_payload_builder::builder::OpPayloadTransactions; +use reth_payload_util::{PayloadTransactions, PayloadTransactionsChain, PayloadTransactionsFixed}; use reth_primitives::{SealedBlock, Transaction, TransactionSigned, TransactionSignedEcRecovered}; use reth_provider::providers::BlockchainProvider2; -use reth_transaction_pool::{ - pool::{BestPayloadTransactions, PayloadTransactionsChain, PayloadTransactionsFixed}, - PayloadTransactions, -}; +use reth_transaction_pool::pool::BestPayloadTransactions; use std::sync::Arc; use tokio::sync::Mutex; diff --git a/crates/optimism/payload/Cargo.toml b/crates/optimism/payload/Cargo.toml index 646df040e19f..19a47be4951a 100644 --- a/crates/optimism/payload/Cargo.toml +++ b/crates/optimism/payload/Cargo.toml @@ -22,6 +22,7 @@ reth-rpc-types-compat.workspace = true reth-evm.workspace = true reth-execution-types.workspace = true reth-payload-builder.workspace = true +reth-payload-util.workspace = true reth-payload-primitives = { workspace = true, features = ["op"] } reth-basic-payload-builder.workspace = true reth-trie.workspace = true diff --git a/crates/optimism/payload/src/builder.rs b/crates/optimism/payload/src/builder.rs index 8fb569f3dac7..d0eb464ae025 100644 --- a/crates/optimism/payload/src/builder.rs +++ b/crates/optimism/payload/src/builder.rs @@ -16,6 +16,7 @@ use reth_optimism_chainspec::OpChainSpec; use reth_optimism_consensus::calculate_receipt_root_no_memo_optimism; use reth_optimism_forks::OpHardforks; use reth_payload_primitives::{PayloadBuilderAttributes, PayloadBuilderError}; +use reth_payload_util::PayloadTransactions; use reth_primitives::{ proofs, revm_primitives::{BlockEnv, CfgEnvWithHandlerCfg}, @@ -24,7 +25,7 @@ use reth_primitives::{ use reth_provider::{ProviderError, StateProofProvider, StateProviderFactory, StateRootProvider}; use reth_revm::database::StateProviderDatabase; use reth_transaction_pool::{ - noop::NoopTransactionPool, BestTransactionsAttributes, PayloadTransactions, TransactionPool, + noop::NoopTransactionPool, BestTransactionsAttributes, TransactionPool, }; use reth_trie::HashedPostState; use revm::{ diff --git a/crates/payload/util/Cargo.toml b/crates/payload/util/Cargo.toml new file mode 100644 index 000000000000..2da8dc660280 --- /dev/null +++ b/crates/payload/util/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "reth-payload-util" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +homepage.workspace = true +repository.workspace = true +description = "reth payload utilities" + +[lints] +workspace = true + +[dependencies] +# reth +reth-primitives.workspace = true + +# alloy +alloy-primitives.workspace = true +alloy-consensus.workspace = true \ No newline at end of file diff --git a/crates/payload/util/src/lib.rs b/crates/payload/util/src/lib.rs new file mode 100644 index 000000000000..5ad0e83507b2 --- /dev/null +++ b/crates/payload/util/src/lib.rs @@ -0,0 +1,15 @@ +//! payload utils. + +#![doc( + html_logo_url = "https://raw.githubusercontent.com/paradigmxyz/reth/main/assets/reth-docs.png", + html_favicon_url = "https://avatars0.githubusercontent.com/u/97369466?s=256", + issue_tracker_base_url = "https://github.com/paradigmxyz/reth/issues/" +)] +#![cfg_attr(not(test), warn(unused_crate_dependencies))] +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] + +mod traits; +mod transaction; + +pub use traits::PayloadTransactions; +pub use transaction::{PayloadTransactionsChain, PayloadTransactionsFixed}; diff --git a/crates/payload/util/src/traits.rs b/crates/payload/util/src/traits.rs new file mode 100644 index 000000000000..52dad5111698 --- /dev/null +++ b/crates/payload/util/src/traits.rs @@ -0,0 +1,20 @@ +use alloy_primitives::Address; +use reth_primitives::TransactionSignedEcRecovered; + +/// 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); +} diff --git a/crates/payload/util/src/transaction.rs b/crates/payload/util/src/transaction.rs new file mode 100644 index 000000000000..a45e177d4d34 --- /dev/null +++ b/crates/payload/util/src/transaction.rs @@ -0,0 +1,128 @@ +use crate::PayloadTransactions; +use alloy_consensus::Transaction; +use alloy_primitives::Address; +use reth_primitives::TransactionSignedEcRecovered; + +/// 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 PayloadTransactionsFixed { + transactions: Vec, + index: usize, +} + +impl PayloadTransactionsFixed { + /// Constructs a new [`PayloadTransactionsFixed`]. + pub fn new(transactions: Vec) -> Self { + Self { transactions, index: Default::default() } + } + + /// Constructs a new [`PayloadTransactionsFixed`] with a single transaction. + pub fn single(transaction: T) -> Self { + Self { transactions: vec![transaction], index: Default::default() } + } +} + +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 + }) + } + + fn mark_invalid(&mut self, _sender: Address, _nonce: u64) {} +} + +/// 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` +/// (b) we need to keep track of the gas +/// +/// Notes that [`PayloadTransactionsChain`] 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 PayloadTransactionsChain { + /// 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 PayloadTransactionsChain { + /// Constructs a new [`PayloadTransactionsChain`]. + 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 PayloadTransactions for PayloadTransactionsChain +where + B: PayloadTransactions, + A: PayloadTransactions, +{ + 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.signer(), tx.transaction.nonce()); + self.after.mark_invalid(tx.signer(), tx.transaction.nonce()); + } else { + return Some(tx); + } + } + + 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.signer(), tx.transaction.nonce()); + } else { + return Some(tx); + } + } + + None + } + + fn mark_invalid(&mut self, sender: Address, nonce: u64) { + self.before.mark_invalid(sender, nonce); + self.after.mark_invalid(sender, nonce); + } +} diff --git a/crates/transaction-pool/Cargo.toml b/crates/transaction-pool/Cargo.toml index 7c760c81c540..22df82536826 100644 --- a/crates/transaction-pool/Cargo.toml +++ b/crates/transaction-pool/Cargo.toml @@ -18,6 +18,7 @@ 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 21bcc668b75c..7c2e5a025b7f 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -1,11 +1,11 @@ use crate::{ identifier::{SenderId, TransactionId}, pool::pending::PendingTransaction, - PayloadTransactions, PoolTransaction, TransactionOrdering, ValidPoolTransaction, + PoolTransaction, TransactionOrdering, ValidPoolTransaction, }; -use alloy_consensus::Transaction; use alloy_primitives::Address; use core::fmt; +use reth_payload_util::PayloadTransactions; use reth_primitives::TransactionSignedEcRecovered; use std::{ collections::{BTreeMap, BTreeSet, HashSet, VecDeque}, @@ -20,7 +20,6 @@ use tracing::debug; /// This is a wrapper around [`BestTransactions`] that also enforces a specific basefee. /// /// This iterator guarantees that all transaction it returns satisfy both the base fee and blob fee! -#[derive(Debug)] pub(crate) struct BestTransactionsWithFees { pub(crate) best: BestTransactions, pub(crate) base_fee: u64, @@ -73,7 +72,6 @@ impl Iterator for BestTransactionsWithFees { /// be executed on the current state, but only yields transactions that are ready to be executed /// now. While it contains all gapless transactions of a sender, it _always_ only returns the /// transaction with the current on chain nonce. -#[derive(Debug)] pub(crate) struct BestTransactions { /// Contains a copy of _all_ transactions of the pending pool at the point in time this /// iterator was created. @@ -397,130 +395,6 @@ where } } -/// 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 PayloadTransactionsFixed { - transactions: Vec, - index: usize, -} - -impl PayloadTransactionsFixed { - /// Constructs a new [`PayloadTransactionsFixed`]. - pub fn new(transactions: Vec) -> Self { - Self { transactions, index: Default::default() } - } - - /// Constructs a new [`PayloadTransactionsFixed`] with a single transaction. - pub fn single(transaction: T) -> Self { - Self { transactions: vec![transaction], index: Default::default() } - } -} - -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 - }) - } - - fn mark_invalid(&mut self, _sender: Address, _nonce: u64) {} -} - -/// 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` -/// (b) we need to keep track of the gas -/// -/// Notes that [`PayloadTransactionsChain`] 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 PayloadTransactionsChain { - /// 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 PayloadTransactionsChain { - /// Constructs a new [`PayloadTransactionsChain`]. - 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 PayloadTransactions for PayloadTransactionsChain -where - B: PayloadTransactions, - A: PayloadTransactions, -{ - 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.signer(), tx.transaction.nonce()); - self.after.mark_invalid(tx.signer(), tx.transaction.nonce()); - } else { - return Some(tx); - } - } - - 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.signer(), tx.transaction.nonce()); - } else { - return Some(tx); - } - } - - None - } - - fn mark_invalid(&mut self, sender: Address, nonce: u64) { - self.before.mark_invalid(sender, nonce); - self.after.mark_invalid(sender, nonce); - } -} - #[cfg(test)] mod tests { use super::*; @@ -530,6 +404,7 @@ mod tests { Priority, }; use alloy_primitives::U256; + use reth_payload_util::{PayloadTransactionsChain, PayloadTransactionsFixed}; #[test] fn test_best_iter() { diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 78cc790e9423..6441ed687f2a 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -108,7 +108,6 @@ use crate::{ }; pub use best::{ BestPayloadTransactions, 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 9b4cccfb9d17..6c247a84cdb4 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -1510,24 +1510,6 @@ 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::*;