diff --git a/bin/reth/src/debug_cmd/build_block.rs b/bin/reth/src/debug_cmd/build_block.rs index c55b49ea2..fe8ff0bd3 100644 --- a/bin/reth/src/debug_cmd/build_block.rs +++ b/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?; } diff --git a/crates/net/eth-wire/tests/pooled_transactions.rs b/crates/net/eth-wire/tests/pooled_transactions.rs index c5dfe30ad..a5f20b62f 100644 --- a/crates/net/eth-wire/tests/pooled_transactions.rs +++ b/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] diff --git a/crates/net/network/src/transactions.rs b/crates/net/network/src/transactions.rs index b1894752e..4c7d4da45 100644 --- a/crates/net/network/src/transactions.rs +++ b/crates/net/network/src/transactions.rs @@ -845,7 +845,7 @@ impl PropagateTransaction { /// Create a new instance from a pooled transaction fn new(tx: Arc>) -> 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()); } } diff --git a/crates/primitives/src/transaction/eip1559.rs b/crates/primitives/src/transaction/eip1559.rs index bb2092be8..02ff1ec49 100644 --- a/crates/primitives/src/transaction/eip1559.rs +++ b/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 } diff --git a/crates/primitives/src/transaction/eip2930.rs b/crates/primitives/src/transaction/eip2930.rs index e490d7bff..5cd4d7803 100644 --- a/crates/primitives/src/transaction/eip2930.rs +++ b/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 } diff --git a/crates/primitives/src/transaction/eip4844.rs b/crates/primitives/src/transaction/eip4844.rs index 5b4dd0eb5..c01d1b049 100644 --- a/crates/primitives/src/transaction/eip4844.rs +++ b/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, commitments: Vec, proofs: Vec) -> 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: diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 65bedeb52..bf6ae7b02 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/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 for TransactionSigned { diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 8f32435ef..22b59637b 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/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 { diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index fd595ae04..a7aa525b9 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/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)?; diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index 9940d6927..255dd2a01 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/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, } } diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index d05e8b3d2..a3ace0b79 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/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 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) } } diff --git a/crates/transaction-pool/src/validate/mod.rs b/crates/transaction-pool/src/validate/mod.rs index 68fcbd886..1cfef0bbf 100644 --- a/crates/transaction-pool/src/validate/mod.rs +++ b/crates/transaction-pool/src/validate/mod.rs @@ -191,8 +191,6 @@ pub struct ValidPoolTransaction { 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 ValidPoolTransaction { &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 Clone for ValidPoolTransaction { propagate: self.propagate, timestamp: self.timestamp, origin: self.origin, - encoded_length: self.encoded_length, } } }