feat: track encoded length with pooled txs (#4976)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

bin/reth/src/debug_cmd/build_block.rs

@@ -22,8 +22,9 @@ use reth_primitives::{
     fs,
     revm_primitives::KzgSettings,
     stage::StageId,
-    Address, BlobTransactionSidecar, Bytes, ChainSpec, SealedBlock, SealedBlockWithSenders,
-    Transaction, TransactionSigned, TxEip4844, B256, U256, U64,
+    Address, BlobTransaction, BlobTransactionSidecar, Bytes, ChainSpec, PooledTransactionsElement,
+    SealedBlock, SealedBlockWithSenders, Transaction, TransactionSigned, TxEip4844, B256, U256,
+    U64,
 };
 use reth_provider::{
     providers::BlockchainProvider, BlockHashReader, BlockReader, BlockWriter, ExecutorFactory,
@@ -195,24 +196,44 @@ impl Command {
                 .into_ecrecovered()
                 .ok_or(eyre::eyre!("failed to recover tx"))?;
 
-            if let Transaction::Eip4844(TxEip4844 { blob_versioned_hashes, .. }) =
-                &transaction.transaction
-            {
-                let blobs_bundle = blobs_bundle.as_mut().ok_or(eyre::eyre!(
-                    "encountered a blob tx. `--blobs-bundle-path` must be provided"
-                ))?;
-                let (commitments, proofs, blobs) = blobs_bundle.take(blob_versioned_hashes.len());
-                blob_store.insert(
-                    transaction.hash,
-                    BlobTransactionSidecar { blobs, commitments, proofs },
-                )?;
-            }
+            let encoded_length = match &transaction.transaction {
+                Transaction::Eip4844(TxEip4844 { blob_versioned_hashes, .. }) => {
+                    let blobs_bundle = blobs_bundle.as_mut().ok_or(eyre::eyre!(
+                        "encountered a blob tx. `--blobs-bundle-path` must be provided"
+                    ))?;
+
+                    let (commitments, proofs, blobs) =
+                        blobs_bundle.take(blob_versioned_hashes.len());
+
+                    // first construct the tx, calculating the length of the tx with sidecar before
+                    // insertion
+                    let sidecar = BlobTransactionSidecar::new(
+                        blobs.clone(),
+                        commitments.clone(),
+                        proofs.clone(),
+                    );
+                    let tx =
+                        BlobTransaction::try_from_signed(transaction.as_ref().clone(), sidecar)
+                            .expect("should not fail to convert blob tx if it is already eip4844");
+                    let pooled = PooledTransactionsElement::BlobTransaction(tx);
+                    let encoded_length = pooled.length_without_header();
+
+                    // insert the blob into the store
+                    blob_store.insert(
+                        transaction.hash,
+                        BlobTransactionSidecar { blobs, commitments, proofs },
+                    )?;
+
+                    encoded_length
+                }
+                _ => transaction.length_without_header(),
+            };
 
             debug!(target: "reth::cli", ?transaction, "Adding transaction to the pool");
             transaction_pool
                 .add_transaction(
                     TransactionOrigin::External,
-                    EthPooledTransaction::new(transaction),
+                    EthPooledTransaction::new(transaction, encoded_length),
                 )
                 .await?;
         }

crates/net/eth-wire/tests/pooled_transactions.rs

@@ -20,7 +20,13 @@ fn decode_pooled_transactions_data() {
     }
 
     // now do another decoding, on what we encoded - this should succeed
-    let _txs2 = PooledTransactions::decode(&mut &buf[..]).unwrap();
+    let txs2 = PooledTransactions::decode(&mut &buf[..]).unwrap();
+
+    // ensure that the payload length is the same
+    assert_eq!(txs.length(), txs2.length());
+
+    // ensure that the length is equal to the length of the encoded data
+    assert_eq!(txs.length(), buf.len());
 }
 
 #[test]

crates/net/network/src/transactions.rs

@@ -845,7 +845,7 @@ impl PropagateTransaction {
 
     /// Create a new instance from a pooled transaction
     fn new<T: PoolTransaction>(tx: Arc<ValidPoolTransaction<T>>) -> Self {
-        let size = tx.encoded_length;
+        let size = tx.encoded_length();
         let transaction = Arc::new(tx.transaction.to_recovered_transaction().into_signed());
         Self { size, transaction }
     }
@@ -900,7 +900,7 @@ impl PooledTransactionsHashesBuilder {
             PooledTransactionsHashesBuilder::Eth66(msg) => msg.0.push(*pooled_tx.hash()),
             PooledTransactionsHashesBuilder::Eth68(msg) => {
                 msg.hashes.push(*pooled_tx.hash());
-                msg.sizes.push(pooled_tx.encoded_length);
+                msg.sizes.push(pooled_tx.encoded_length());
                 msg.types.push(pooled_tx.transaction.tx_type());
             }
         }

crates/primitives/src/transaction/eip1559.rs

@@ -159,11 +159,16 @@ impl TxEip1559 {
         signature.encode(out);
     }
 
-    /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header.
-    pub(crate) fn payload_len_with_signature(&self, signature: &Signature) -> usize {
+    /// Output the length of the RLP signed transaction encoding, _without_ a RLP string header.
+    pub(crate) fn payload_len_with_signature_without_header(&self, signature: &Signature) -> usize {
         let payload_length = self.fields_len() + signature.payload_len();
         // 'transaction type byte length' + 'header length' + 'payload length'
-        let len = 1 + length_of_length(payload_length) + payload_length;
+        1 + length_of_length(payload_length) + payload_length
+    }
+
+    /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header.
+    pub(crate) fn payload_len_with_signature(&self, signature: &Signature) -> usize {
+        let len = self.payload_len_with_signature_without_header(signature);
         length_of_length(len) + len
     }
 

crates/primitives/src/transaction/eip2930.rs

@@ -138,11 +138,16 @@ impl TxEip2930 {
         signature.encode(out);
     }
 
-    /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header.
-    pub(crate) fn payload_len_with_signature(&self, signature: &Signature) -> usize {
+    /// Output the length of the RLP signed transaction encoding, _without_ a RLP string header.
+    pub(crate) fn payload_len_with_signature_without_header(&self, signature: &Signature) -> usize {
         let payload_length = self.fields_len() + signature.payload_len();
         // 'transaction type byte length' + 'header length' + 'payload length'
-        let len = 1 + length_of_length(payload_length) + payload_length;
+        1 + length_of_length(payload_length) + payload_length
+    }
+
+    /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header.
+    pub(crate) fn payload_len_with_signature(&self, signature: &Signature) -> usize {
+        let len = self.payload_len_with_signature_without_header(signature);
         length_of_length(len) + len
     }
 

crates/primitives/src/transaction/eip4844.rs

@@ -275,10 +275,15 @@ impl TxEip4844 {
 
     /// Output the length of the RLP signed transaction encoding. This encodes with a RLP header.
     pub(crate) fn payload_len_with_signature(&self, signature: &Signature) -> usize {
+        let len = self.payload_len_with_signature_without_header(signature);
+        length_of_length(len) + len
+    }
+
+    /// Output the length of the RLP signed transaction encoding, _without_ a RLP header.
+    pub(crate) fn payload_len_with_signature_without_header(&self, signature: &Signature) -> usize {
         let payload_length = self.fields_len() + signature.payload_len();
         // 'transaction type byte length' + 'header length' + 'payload length'
-        let len = 1 + length_of_length(payload_length) + payload_length;
-        length_of_length(len) + len
+        1 + length_of_length(payload_length) + payload_length
     }
 
     /// Get transaction type
@@ -592,6 +597,11 @@ pub struct BlobTransactionSidecar {
 }
 
 impl BlobTransactionSidecar {
+    /// Creates a new [BlobTransactionSidecar] using the given blobs, commitments, and proofs.
+    pub fn new(blobs: Vec<Blob>, commitments: Vec<Bytes48>, proofs: Vec<Bytes48>) -> Self {
+        Self { blobs, commitments, proofs }
+    }
+
     /// Encodes the inner [BlobTransactionSidecar] fields as RLP bytes, without a RLP header.
     ///
     /// This encodes the fields in the following order:

crates/primitives/src/transaction/mod.rs

@@ -1051,6 +1051,23 @@ impl TransactionSigned {
             TransactionSigned::decode_enveloped_typed_transaction(&mut data)
         }
     }
+
+    /// Returns the length without an RLP header - this is used for eth/68 sizes.
+    pub fn length_without_header(&self) -> usize {
+        // method computes the payload len without a RLP header
+        match &self.transaction {
+            Transaction::Legacy(legacy_tx) => legacy_tx.payload_len_with_signature(&self.signature),
+            Transaction::Eip2930(access_list_tx) => {
+                access_list_tx.payload_len_with_signature_without_header(&self.signature)
+            }
+            Transaction::Eip1559(dynamic_fee_tx) => {
+                dynamic_fee_tx.payload_len_with_signature_without_header(&self.signature)
+            }
+            Transaction::Eip4844(blob_tx) => {
+                blob_tx.payload_len_with_signature_without_header(&self.signature)
+            }
+        }
+    }
 }
 
 impl From<TransactionSignedEcRecovered> for TransactionSigned {

crates/primitives/src/transaction/pooled.rs

@@ -220,6 +220,28 @@ impl PooledTransactionsElement {
             Self::BlobTransaction(blob_tx) => blob_tx.into_parts().0,
         }
     }
+
+    /// Returns the length without an RLP header - this is used for eth/68 sizes.
+    pub fn length_without_header(&self) -> usize {
+        match self {
+            Self::Legacy { transaction, signature, .. } => {
+                // method computes the payload len with a RLP header
+                transaction.payload_len_with_signature(signature)
+            }
+            Self::Eip2930 { transaction, signature, .. } => {
+                // method computes the payload len without a RLP header
+                transaction.payload_len_with_signature_without_header(signature)
+            }
+            Self::Eip1559 { transaction, signature, .. } => {
+                // method computes the payload len without a RLP header
+                transaction.payload_len_with_signature_without_header(signature)
+            }
+            Self::BlobTransaction(blob_tx) => {
+                // the encoding does not use a header, so we set `with_header` to false
+                blob_tx.payload_len_with_type(false)
+            }
+        }
+    }
 }
 
 impl Encodable for PooledTransactionsElement {

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

@@ -392,7 +392,7 @@ where
             } => {
                 let sender_id = self.get_sender_id(transaction.sender());
                 let transaction_id = TransactionId::new(sender_id, transaction.nonce());
-                let encoded_length = transaction.encoded_length();
+                let _encoded_length = transaction.encoded_length();
 
                 // split the valid transaction and the blob sidecar if it has any
                 let (transaction, maybe_sidecar) = match transaction {
@@ -412,7 +412,6 @@ where
                     propagate,
                     timestamp: Instant::now(),
                     origin,
-                    encoded_length,
                 };
 
                 let added = self.pool.write().add_transaction(tx, balance, state_nonce)?;

crates/transaction-pool/src/test_utils/mock.rs

@@ -894,14 +894,12 @@ impl MockTransactionFactory {
         transaction: MockTransaction,
     ) -> MockValidTx {
         let transaction_id = self.tx_id(&transaction);
-        let encoded_length = transaction.encoded_length();
         MockValidTx {
             propagate: false,
             transaction_id,
             transaction,
             timestamp: Instant::now(),
             origin,
-            encoded_length,
         }
     }
 

crates/transaction-pool/src/traits.rs

@@ -4,7 +4,6 @@ use crate::{
     validate::ValidPoolTransaction,
     AllTransactionsEvents,
 };
-use alloy_rlp::Encodable;
 use futures_util::{ready, Stream};
 use reth_primitives::{
     AccessList, Address, BlobTransactionSidecar, BlobTransactionValidationError,
@@ -743,7 +742,9 @@ pub trait PoolTransaction:
         self.tx_type() == EIP4844_TX_TYPE_ID
     }
 
-    /// Returns the length of the rlp encoded object
+    /// Returns the length of the rlp encoded transaction object
+    ///
+    /// Note: Implementations should cache this value.
     fn encoded_length(&self) -> usize;
 
     /// Returns chain_id
@@ -787,6 +788,10 @@ pub struct EthPooledTransaction {
     /// max_blob_fee_per_gas * blob_gas_used`.
     pub(crate) cost: U256,
 
+    /// This is the RLP length of the transaction, computed when the transaction is added to the
+    /// pool.
+    pub(crate) encoded_length: usize,
+
     /// The blob side car for this transaction
     pub(crate) blob_sidecar: EthBlobTransactionSidecar,
 }
@@ -807,7 +812,7 @@ pub enum EthBlobTransactionSidecar {
 
 impl EthPooledTransaction {
     /// Create new instance of [Self].
-    pub fn new(transaction: TransactionSignedEcRecovered) -> Self {
+    pub fn new(transaction: TransactionSignedEcRecovered, encoded_length: usize) -> Self {
         let mut blob_sidecar = EthBlobTransactionSidecar::None;
         let gas_cost = match &transaction.transaction {
             Transaction::Legacy(t) => U256::from(t.gas_price) * U256::from(t.gas_limit),
@@ -826,7 +831,7 @@ impl EthPooledTransaction {
             cost += U256::from(blob_tx.max_fee_per_blob_gas * blob_tx.blob_gas() as u128);
         }
 
-        Self { transaction, cost, blob_sidecar }
+        Self { transaction, cost, encoded_length, blob_sidecar }
     }
 
     /// Return the reference to the underlying transaction.
@@ -838,19 +843,20 @@ impl EthPooledTransaction {
 /// Conversion from the network transaction type to the pool transaction type.
 impl From<PooledTransactionsElementEcRecovered> for EthPooledTransaction {
     fn from(tx: PooledTransactionsElementEcRecovered) -> Self {
+        let encoded_length = tx.length_without_header();
         let (tx, signer) = tx.into_components();
         match tx {
             PooledTransactionsElement::BlobTransaction(tx) => {
                 // include the blob sidecar
                 let (tx, blob) = tx.into_parts();
                 let tx = TransactionSignedEcRecovered::from_signed_transaction(tx, signer);
-                let mut pooled = EthPooledTransaction::new(tx);
+                let mut pooled = EthPooledTransaction::new(tx, encoded_length);
                 pooled.blob_sidecar = EthBlobTransactionSidecar::Present(blob);
                 pooled
             }
             tx => {
                 // no blob sidecar
-                EthPooledTransaction::new(tx.into_ecrecovered_transaction(signer))
+                EthPooledTransaction::new(tx.into_ecrecovered_transaction(signer), encoded_length)
             }
         }
     }
@@ -957,7 +963,7 @@ impl PoolTransaction for EthPooledTransaction {
 
     /// Returns the length of the rlp encoded object
     fn encoded_length(&self) -> usize {
-        self.transaction.length()
+        self.encoded_length
     }
 
     /// Returns chain_id
@@ -993,7 +999,10 @@ impl EthPoolTransaction for EthPooledTransaction {
 
 impl FromRecoveredTransaction for EthPooledTransaction {
     fn from_recovered_transaction(tx: TransactionSignedEcRecovered) -> Self {
-        EthPooledTransaction::new(tx)
+        // CAUTION: this should not be done for EIP-4844 transactions, as the blob sidecar is
+        // missing.
+        let encoded_length = tx.length_without_header();
+        EthPooledTransaction::new(tx, encoded_length)
     }
 }
 

crates/transaction-pool/src/validate/mod.rs

@@ -191,8 +191,6 @@ pub struct ValidPoolTransaction<T: PoolTransaction> {
     pub timestamp: Instant,
     /// Where this transaction originated from.
     pub origin: TransactionOrigin,
-    /// The length of the rlp encoded transaction (cached)
-    pub encoded_length: usize,
 }
 
 // === impl ValidPoolTransaction ===
@@ -223,6 +221,12 @@ impl<T: PoolTransaction> ValidPoolTransaction<T> {
         &self.transaction_id
     }
 
+    /// Returns the length of the rlp encoded transaction
+    #[inline]
+    pub fn encoded_length(&self) -> usize {
+        self.transaction.encoded_length()
+    }
+
     /// Returns the nonce set for this transaction.
     pub fn nonce(&self) -> u64 {
         self.transaction.nonce()
@@ -304,7 +308,6 @@ impl<T: PoolTransaction + Clone> Clone for ValidPoolTransaction<T> {
             propagate: self.propagate,
             timestamp: self.timestamp,
             origin: self.origin,
-            encoded_length: self.encoded_length,
         }
     }
 }