feat: refactor PooledTransactionsElement into typed variants (#4241)

crates/primitives/src/transaction/eip1559.rs

@@ -1,7 +1,7 @@
 use super::access_list::AccessList;
-use crate::{Bytes, ChainId, TransactionKind};
+use crate::{Bytes, ChainId, Signature, TransactionKind, TxType};
 use reth_codecs::{main_codec, Compact};
-use reth_rlp::{Decodable, DecodeError};
+use reth_rlp::{length_of_length, Decodable, DecodeError, Encodable, Header};
 use std::mem;
 
 /// A transaction with a priority fee ([EIP-1559](https://eips.ethereum.org/EIPS/eip-1559)).
@@ -111,6 +111,62 @@ impl TxEip1559 {
         })
     }
 
+    /// Encodes only the transaction's fields into the desired buffer, without a RLP header.
+    pub(crate) fn fields_len(&self) -> usize {
+        let mut len = 0;
+        len += self.chain_id.length();
+        len += self.nonce.length();
+        len += self.max_priority_fee_per_gas.length();
+        len += self.max_fee_per_gas.length();
+        len += self.gas_limit.length();
+        len += self.to.length();
+        len += self.value.length();
+        len += self.input.0.length();
+        len += self.access_list.length();
+        len
+    }
+
+    /// Encodes only the transaction's fields into the desired buffer, without a RLP header.
+    pub(crate) fn encode_fields(&self, out: &mut dyn bytes::BufMut) {
+        self.chain_id.encode(out);
+        self.nonce.encode(out);
+        self.max_priority_fee_per_gas.encode(out);
+        self.max_fee_per_gas.encode(out);
+        self.gas_limit.encode(out);
+        self.to.encode(out);
+        self.value.encode(out);
+        self.input.0.encode(out);
+        self.access_list.encode(out);
+    }
+
+    /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating
+    /// hash that for eip2718 does not require rlp header
+    pub(crate) fn encode_with_signature(
+        &self,
+        signature: &Signature,
+        out: &mut dyn bytes::BufMut,
+        with_header: bool,
+    ) {
+        let payload_length = self.fields_len() + signature.payload_len();
+        if with_header {
+            Header {
+                list: false,
+                payload_length: 1 + length_of_length(payload_length) + payload_length,
+            }
+            .encode(out);
+        }
+        out.put_u8(self.tx_type() as u8);
+        let header = Header { list: true, payload_length };
+        header.encode(out);
+        self.encode_fields(out);
+        signature.encode(out);
+    }
+
+    /// Get transaction type
+    pub(crate) fn tx_type(&self) -> TxType {
+        TxType::EIP1559
+    }
+
     /// Calculates a heuristic for the in-memory size of the [TxEip1559] transaction.
     #[inline]
     pub fn size(&self) -> usize {

crates/primitives/src/transaction/eip2930.rs

@@ -1,7 +1,7 @@
 use super::access_list::AccessList;
-use crate::{Bytes, ChainId, TransactionKind};
+use crate::{Bytes, ChainId, Signature, TransactionKind, TxType};
 use reth_codecs::{main_codec, Compact};
-use reth_rlp::{Decodable, DecodeError};
+use reth_rlp::{length_of_length, Decodable, DecodeError, Encodable, Header};
 use std::mem;
 
 /// Transaction with an [`AccessList`] ([EIP-2930](https://eips.ethereum.org/EIPS/eip-2930)).
@@ -91,6 +91,60 @@ impl TxEip2930 {
             access_list: Decodable::decode(buf)?,
         })
     }
+
+    /// Outputs the length of the transaction's fields, without a RLP header.
+    pub(crate) fn fields_len(&self) -> usize {
+        let mut len = 0;
+        len += self.chain_id.length();
+        len += self.nonce.length();
+        len += self.gas_price.length();
+        len += self.gas_limit.length();
+        len += self.to.length();
+        len += self.value.length();
+        len += self.input.0.length();
+        len += self.access_list.length();
+        len
+    }
+
+    /// Encodes only the transaction's fields into the desired buffer, without a RLP header.
+    pub(crate) fn encode_fields(&self, out: &mut dyn bytes::BufMut) {
+        self.chain_id.encode(out);
+        self.nonce.encode(out);
+        self.gas_price.encode(out);
+        self.gas_limit.encode(out);
+        self.to.encode(out);
+        self.value.encode(out);
+        self.input.0.encode(out);
+        self.access_list.encode(out);
+    }
+
+    /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating
+    /// hash that for eip2718 does not require rlp header
+    pub(crate) fn encode_with_signature(
+        &self,
+        signature: &Signature,
+        out: &mut dyn bytes::BufMut,
+        with_header: bool,
+    ) {
+        let payload_length = self.fields_len() + signature.payload_len();
+        if with_header {
+            Header {
+                list: false,
+                payload_length: 1 + length_of_length(payload_length) + payload_length,
+            }
+            .encode(out);
+        }
+        out.put_u8(self.tx_type() as u8);
+        let header = Header { list: true, payload_length };
+        header.encode(out);
+        self.encode_fields(out);
+        signature.encode(out);
+    }
+
+    /// Get transaction type
+    pub(crate) fn tx_type(&self) -> TxType {
+        TxType::EIP2930
+    }
 }
 
 #[cfg(test)]

crates/primitives/src/transaction/eip4844.rs

@@ -1,15 +1,16 @@
 use super::access_list::AccessList;
 use crate::{
     constants::eip4844::DATA_GAS_PER_BLOB,
+    keccak256,
     kzg::{
         self, Blob, Bytes48, KzgCommitment, KzgProof, KzgSettings, BYTES_PER_BLOB,
         BYTES_PER_COMMITMENT, BYTES_PER_PROOF,
     },
     kzg_to_versioned_hash, Bytes, ChainId, Signature, Transaction, TransactionKind,
-    TransactionSigned, TransactionSignedNoHash, TxType, EIP4844_TX_TYPE_ID, H256,
+    TransactionSigned, TxHash, TxType, EIP4844_TX_TYPE_ID, H256,
 };
 use reth_codecs::{main_codec, Compact};
-use reth_rlp::{Decodable, DecodeError, Encodable, Header};
+use reth_rlp::{length_of_length, Decodable, DecodeError, Encodable, Header};
 use serde::{Deserialize, Serialize};
 use std::{mem, ops::Deref};
 
@@ -142,6 +143,38 @@ impl TxEip4844 {
         })
     }
 
+    /// Outputs the length of the transaction's fields, without a RLP header.
+    pub(crate) fn fields_len(&self) -> usize {
+        let mut len = 0;
+        len += self.chain_id.length();
+        len += self.nonce.length();
+        len += self.gas_limit.length();
+        len += self.max_fee_per_gas.length();
+        len += self.max_priority_fee_per_gas.length();
+        len += self.to.length();
+        len += self.value.length();
+        len += self.access_list.length();
+        len += self.blob_versioned_hashes.length();
+        len += self.max_fee_per_blob_gas.length();
+        len += self.input.0.length();
+        len
+    }
+
+    /// Encodes only the transaction's fields into the desired buffer, without a RLP header.
+    pub(crate) fn encode_fields(&self, out: &mut dyn bytes::BufMut) {
+        self.chain_id.encode(out);
+        self.nonce.encode(out);
+        self.max_priority_fee_per_gas.encode(out);
+        self.max_fee_per_gas.encode(out);
+        self.gas_limit.encode(out);
+        self.to.encode(out);
+        self.value.encode(out);
+        self.input.0.encode(out);
+        self.access_list.encode(out);
+        self.max_fee_per_blob_gas.encode(out);
+        self.blob_versioned_hashes.encode(out);
+    }
+
     /// Calculates a heuristic for the in-memory size of the [TxEip4844] transaction.
     #[inline]
     pub fn size(&self) -> usize {
@@ -157,6 +190,34 @@ impl TxEip4844 {
         self.blob_versioned_hashes.capacity() * mem::size_of::<H256>() + // blob hashes size
         mem::size_of::<u128>() // max_fee_per_data_gas
     }
+
+    /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating
+    /// hash that for eip2718 does not require rlp header
+    pub(crate) fn encode_with_signature(
+        &self,
+        signature: &Signature,
+        out: &mut dyn bytes::BufMut,
+        with_header: bool,
+    ) {
+        let payload_length = self.fields_len() + signature.payload_len();
+        if with_header {
+            Header {
+                list: false,
+                payload_length: 1 + length_of_length(payload_length) + payload_length,
+            }
+            .encode(out);
+        }
+        out.put_u8(self.tx_type() as u8);
+        let header = Header { list: true, payload_length };
+        header.encode(out);
+        self.encode_fields(out);
+        signature.encode(out);
+    }
+
+    /// Get transaction type
+    pub(crate) fn tx_type(&self) -> TxType {
+        TxType::EIP4844
+    }
 }
 
 /// An error that can occur when validating a [BlobTransaction].
@@ -185,8 +246,12 @@ impl From<kzg::Error> for BlobTransactionValidationError {
 /// which should always construct the [TransactionSigned] with an EIP-4844 transaction.
 #[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct BlobTransaction {
+    /// The transaction hash.
+    pub hash: TxHash,
     /// The transaction payload.
-    pub transaction: TransactionSigned,
+    pub transaction: TxEip4844,
+    /// The transaction signature.
+    pub signature: Signature,
     /// The transaction's blob sidecar.
     pub sidecar: BlobTransactionSidecar,
 }
@@ -207,14 +272,7 @@ impl BlobTransaction {
         &self,
         proof_settings: &KzgSettings,
     ) -> Result<bool, BlobTransactionValidationError> {
-        let inner_tx = match &self.transaction.transaction {
-            Transaction::Eip4844(blob_tx) => blob_tx,
-            non_blob_tx => {
-                return Err(BlobTransactionValidationError::NotBlobTransaction(
-                    non_blob_tx.tx_type(),
-                ))
-            }
-        };
+        let inner_tx = &self.transaction;
 
         // Ensure the versioned hashes and commitments have the same length
         if inner_tx.blob_versioned_hashes.len() != self.sidecar.commitments.len() {
@@ -257,7 +315,13 @@ impl BlobTransaction {
     /// Splits the [BlobTransaction] into its [TransactionSigned] and [BlobTransactionSidecar]
     /// components.
     pub fn into_parts(self) -> (TransactionSigned, BlobTransactionSidecar) {
-        (self.transaction, self.sidecar)
+        let transaction = TransactionSigned {
+            transaction: Transaction::Eip4844(self.transaction),
+            hash: self.hash,
+            signature: self.signature,
+        };
+
+        (transaction, self.sidecar)
     }
 
     /// Encodes the [BlobTransaction] fields as RLP, with a tx type. If `with_header` is `false`,
@@ -303,8 +367,7 @@ impl BlobTransaction {
         // its list header.
         let tx_header = Header {
             list: true,
-            payload_length: self.transaction.fields_len() +
-                self.transaction.signature.payload_len(),
+            payload_length: self.transaction.fields_len() + self.signature.payload_len(),
         };
 
         let tx_length = tx_header.length() + tx_header.payload_length;
@@ -365,8 +428,7 @@ impl BlobTransaction {
         // its list header.
         let tx_header = Header {
             list: true,
-            payload_length: self.transaction.fields_len() +
-                self.transaction.signature.payload_len(),
+            payload_length: self.transaction.fields_len() + self.signature.payload_len(),
         };
 
         let tx_length = tx_header.length() + tx_header.payload_length;
@@ -402,14 +464,11 @@ impl BlobTransaction {
         }
 
         // inner transaction
-        let transaction = Transaction::Eip4844(TxEip4844::decode_inner(data)?);
+        let transaction = TxEip4844::decode_inner(data)?;
 
         // signature
         let signature = Signature::decode(data)?;
 
-        // construct the tx now that we've decoded the fields in order
-        let tx_no_hash = TransactionSignedNoHash { transaction, signature };
-
         // All that's left are the blobs, commitments, and proofs
         let sidecar = BlobTransactionSidecar::decode_inner(data)?;
 
@@ -427,10 +486,13 @@ impl BlobTransaction {
         // Because the pooled transaction encoding is different than the hash encoding for
         // EIP-4844 transactions, we do not use the original buffer to calculate the hash.
         //
-        // Instead, we use `TransactionSignedNoHash` which will encode the transaction internally.
-        let signed_tx = tx_no_hash.with_hash();
+        // Instead, we use `encode_with_signature`, which RLP encodes the transaction with a
+        // signature for hashing without a header. We then hash the result.
+        let mut buf = Vec::new();
+        transaction.encode_with_signature(&signature, &mut buf, false);
+        let hash = keccak256(&buf);
 
-        Ok(Self { transaction: signed_tx, sidecar })
+        Ok(Self { transaction, hash, signature, sidecar })
     }
 }
 

crates/primitives/src/transaction/legacy.rs

@@ -1,5 +1,6 @@
-use crate::{Bytes, ChainId, TransactionKind};
+use crate::{Bytes, ChainId, Signature, TransactionKind, TxType};
 use reth_codecs::{main_codec, Compact};
+use reth_rlp::{Encodable, Header};
 use std::mem;
 
 /// Legacy transaction.
@@ -56,6 +57,46 @@ impl TxLegacy {
         mem::size_of::<u128>() + // value
         self.input.len() // input
     }
+
+    /// Outputs the length of the transaction's fields, without a RLP header or length of the
+    /// eip155 fields.
+    pub(crate) fn fields_len(&self) -> usize {
+        let mut len = 0;
+        len += self.nonce.length();
+        len += self.gas_price.length();
+        len += self.gas_limit.length();
+        len += self.to.length();
+        len += self.value.length();
+        len += self.input.0.length();
+        len
+    }
+
+    /// Encodes only the transaction's fields into the desired buffer, without a RLP header or
+    /// eip155 fields.
+    pub(crate) fn encode_fields(&self, out: &mut dyn bytes::BufMut) {
+        self.nonce.encode(out);
+        self.gas_price.encode(out);
+        self.gas_limit.encode(out);
+        self.to.encode(out);
+        self.value.encode(out);
+        self.input.0.encode(out);
+    }
+
+    /// Inner encoding function that is used for both rlp [`Encodable`] trait and for calculating
+    /// hash.
+    pub(crate) fn encode_with_signature(&self, signature: &Signature, out: &mut dyn bytes::BufMut) {
+        let payload_length =
+            self.fields_len() + signature.payload_len_with_eip155_chain_id(self.chain_id);
+        let header = Header { list: true, payload_length };
+        header.encode(out);
+        self.encode_fields(out);
+        signature.encode_with_eip155_chain_id(out, self.chain_id);
+    }
+
+    /// Get transaction type
+    pub(crate) fn tx_type(&self) -> TxType {
+        TxType::Legacy
+    }
 }
 
 #[cfg(test)]

crates/primitives/src/transaction/mod.rs

@@ -144,10 +144,10 @@ impl Transaction {
     /// Get transaction type
     pub fn tx_type(&self) -> TxType {
         match self {
-            Transaction::Legacy { .. } => TxType::Legacy,
-            Transaction::Eip2930 { .. } => TxType::EIP2930,
-            Transaction::Eip1559 { .. } => TxType::EIP1559,
-            Transaction::Eip4844 { .. } => TxType::EIP4844,
+            Transaction::Legacy(legacy_tx) => legacy_tx.tx_type(),
+            Transaction::Eip2930(access_list_tx) => access_list_tx.tx_type(),
+            Transaction::Eip1559(dynamic_fee_tx) => dynamic_fee_tx.tx_type(),
+            Transaction::Eip4844(blob_tx) => blob_tx.tx_type(),
         }
     }
 
@@ -345,184 +345,22 @@ impl Transaction {
 
     /// Outputs the length of the transaction's fields, without a RLP header or length of the
     /// eip155 fields.
-    pub fn fields_len(&self) -> usize {
+    pub(crate) fn fields_len(&self) -> usize {
         match self {
-            Transaction::Legacy(TxLegacy {
-                chain_id: _,
-                nonce,
-                gas_price,
-                gas_limit,
-                to,
-                value,
-                input,
-            }) => {
-                let mut len = 0;
-                len += nonce.length();
-                len += gas_price.length();
-                len += gas_limit.length();
-                len += to.length();
-                len += value.length();
-                len += input.0.length();
-                len
-            }
-            Transaction::Eip2930(TxEip2930 {
-                chain_id,
-                nonce,
-                gas_price,
-                gas_limit,
-                to,
-                value,
-                input,
-                access_list,
-            }) => {
-                let mut len = 0;
-                len += chain_id.length();
-                len += nonce.length();
-                len += gas_price.length();
-                len += gas_limit.length();
-                len += to.length();
-                len += value.length();
-                len += input.0.length();
-                len += access_list.length();
-                len
-            }
-            Transaction::Eip1559(TxEip1559 {
-                chain_id,
-                nonce,
-                gas_limit,
-                max_fee_per_gas,
-                max_priority_fee_per_gas,
-                to,
-                value,
-                input,
-                access_list,
-            }) => {
-                let mut len = 0;
-                len += chain_id.length();
-                len += nonce.length();
-                len += max_priority_fee_per_gas.length();
-                len += max_fee_per_gas.length();
-                len += gas_limit.length();
-                len += to.length();
-                len += value.length();
-                len += input.0.length();
-                len += access_list.length();
-                len
-            }
-            Transaction::Eip4844(TxEip4844 {
-                chain_id,
-                nonce,
-                gas_limit,
-                max_fee_per_gas,
-                max_priority_fee_per_gas,
-                to,
-                value,
-                access_list,
-                blob_versioned_hashes,
-                max_fee_per_blob_gas,
-                input,
-            }) => {
-                let mut len = 0;
-                len += chain_id.length();
-                len += nonce.length();
-                len += gas_limit.length();
-                len += max_fee_per_gas.length();
-                len += max_priority_fee_per_gas.length();
-                len += to.length();
-                len += value.length();
-                len += access_list.length();
-                len += blob_versioned_hashes.length();
-                len += max_fee_per_blob_gas.length();
-                len += input.0.length();
-                len
-            }
+            Transaction::Legacy(legacy_tx) => legacy_tx.fields_len(),
+            Transaction::Eip2930(access_list_tx) => access_list_tx.fields_len(),
+            Transaction::Eip1559(dynamic_fee_tx) => dynamic_fee_tx.fields_len(),
+            Transaction::Eip4844(blob_tx) => blob_tx.fields_len(),
         }
     }
 
     /// Encodes only the transaction's fields into the desired buffer, without a RLP header.
-    pub fn encode_fields(&self, out: &mut dyn bytes::BufMut) {
+    pub(crate) fn encode_fields(&self, out: &mut dyn bytes::BufMut) {
         match self {
-            Transaction::Legacy(TxLegacy {
-                chain_id: _,
-                nonce,
-                gas_price,
-                gas_limit,
-                to,
-                value,
-                input,
-            }) => {
-                nonce.encode(out);
-                gas_price.encode(out);
-                gas_limit.encode(out);
-                to.encode(out);
-                value.encode(out);
-                input.0.encode(out);
-            }
-            Transaction::Eip2930(TxEip2930 {
-                chain_id,
-                nonce,
-                gas_price,
-                gas_limit,
-                to,
-                value,
-                input,
-                access_list,
-            }) => {
-                chain_id.encode(out);
-                nonce.encode(out);
-                gas_price.encode(out);
-                gas_limit.encode(out);
-                to.encode(out);
-                value.encode(out);
-                input.0.encode(out);
-                access_list.encode(out);
-            }
-            Transaction::Eip1559(TxEip1559 {
-                chain_id,
-                nonce,
-                gas_limit,
-                max_fee_per_gas,
-                max_priority_fee_per_gas,
-                to,
-                value,
-                input,
-                access_list,
-            }) => {
-                chain_id.encode(out);
-                nonce.encode(out);
-                max_priority_fee_per_gas.encode(out);
-                max_fee_per_gas.encode(out);
-                gas_limit.encode(out);
-                to.encode(out);
-                value.encode(out);
-                input.0.encode(out);
-                access_list.encode(out);
-            }
-            Transaction::Eip4844(TxEip4844 {
-                chain_id,
-                nonce,
-                gas_limit,
-                max_fee_per_gas,
-                max_priority_fee_per_gas,
-                to,
-                value,
-                access_list,
-                blob_versioned_hashes,
-                max_fee_per_blob_gas,
-                input,
-            }) => {
-                chain_id.encode(out);
-                nonce.encode(out);
-                max_priority_fee_per_gas.encode(out);
-                max_fee_per_gas.encode(out);
-                gas_limit.encode(out);
-                to.encode(out);
-                value.encode(out);
-                input.0.encode(out);
-                access_list.encode(out);
-                max_fee_per_blob_gas.encode(out);
-                blob_versioned_hashes.encode(out);
-            }
+            Transaction::Legacy(legacy_tx) => legacy_tx.encode_fields(out),
+            Transaction::Eip2930(access_list_tx) => access_list_tx.encode_fields(out),
+            Transaction::Eip1559(dynamic_fee_tx) => dynamic_fee_tx.encode_fields(out),
+            Transaction::Eip4844(blob_tx) => blob_tx.encode_fields(out),
         }
     }
 
@@ -541,29 +379,18 @@ impl Transaction {
         with_header: bool,
     ) {
         match self {
-            Transaction::Legacy(TxLegacy { chain_id, .. }) => {
+            Transaction::Legacy(legacy_tx) => {
                 // do nothing w/ with_header
-                let payload_length =
-                    self.fields_len() + signature.payload_len_with_eip155_chain_id(*chain_id);
-                let header = Header { list: true, payload_length };
-                header.encode(out);
-                self.encode_fields(out);
-                signature.encode_with_eip155_chain_id(out, *chain_id);
+                legacy_tx.encode_with_signature(signature, out)
             }
-            _ => {
-                let payload_length = self.fields_len() + signature.payload_len();
-                if with_header {
-                    Header {
-                        list: false,
-                        payload_length: 1 + length_of_length(payload_length) + payload_length,
-                    }
-                    .encode(out);
-                }
-                out.put_u8(self.tx_type() as u8);
-                let header = Header { list: true, payload_length };
-                header.encode(out);
-                self.encode_fields(out);
-                signature.encode(out);
+            Transaction::Eip2930(access_list_tx) => {
+                access_list_tx.encode_with_signature(signature, out, with_header)
+            }
+            Transaction::Eip1559(dynamic_fee_tx) => {
+                dynamic_fee_tx.encode_with_signature(signature, out, with_header)
+            }
+            Transaction::Eip4844(blob_tx) => {
+                blob_tx.encode_with_signature(signature, out, with_header)
             }
         }
     }
@@ -1058,6 +885,36 @@ impl TransactionSigned {
         mem::size_of::<TxHash>() + self.transaction.size() + self.signature.size()
     }
 
+    /// Decodes legacy transaction from the data buffer into a tuple.
+    ///
+    /// This expects `rlp(legacy_tx)`
+    // TODO: make buf advancement semantics consistent with `decode_enveloped_typed_transaction`,
+    // so decoding methods do not need to manually advance the buffer
+    pub(crate) fn decode_rlp_legacy_transaction_tuple(
+        data: &mut &[u8],
+    ) -> Result<(TxLegacy, TxHash, Signature), DecodeError> {
+        // keep this around, so we can use it to calculate the hash
+        let original_encoding = *data;
+
+        let header = Header::decode(data)?;
+
+        let mut transaction = TxLegacy {
+            nonce: Decodable::decode(data)?,
+            gas_price: Decodable::decode(data)?,
+            gas_limit: Decodable::decode(data)?,
+            to: Decodable::decode(data)?,
+            value: Decodable::decode(data)?,
+            input: Bytes(Decodable::decode(data)?),
+            chain_id: None,
+        };
+        let (signature, extracted_id) = Signature::decode_with_eip155_chain_id(data)?;
+        transaction.chain_id = extracted_id;
+
+        let tx_length = header.payload_length + header.length();
+        let hash = keccak256(&original_encoding[..tx_length]);
+        Ok((transaction, hash, signature))
+    }
+
     /// Decodes legacy transaction from the data buffer.
     ///
     /// This expects `rlp(legacy_tx)`
@@ -1066,28 +923,10 @@ impl TransactionSigned {
     pub fn decode_rlp_legacy_transaction(
         data: &mut &[u8],
     ) -> Result<TransactionSigned, DecodeError> {
-        // keep this around, so we can use it to calculate the hash
-        let original_encoding = *data;
-
-        let header = Header::decode(data)?;
-
-        let mut transaction = Transaction::Legacy(TxLegacy {
-            nonce: Decodable::decode(data)?,
-            gas_price: Decodable::decode(data)?,
-            gas_limit: Decodable::decode(data)?,
-            to: Decodable::decode(data)?,
-            value: Decodable::decode(data)?,
-            input: Bytes(Decodable::decode(data)?),
-            chain_id: None,
-        });
-        let (signature, extracted_id) = Signature::decode_with_eip155_chain_id(data)?;
-        if let Some(id) = extracted_id {
-            transaction.set_chain_id(id);
-        }
-
-        let tx_length = header.payload_length + header.length();
-        let hash = keccak256(&original_encoding[..tx_length]);
-        let signed = TransactionSigned { transaction, hash, signature };
+        let (transaction, hash, signature) =
+            TransactionSigned::decode_rlp_legacy_transaction_tuple(data)?;
+        let signed =
+            TransactionSigned { transaction: Transaction::Legacy(transaction), hash, signature };
         Ok(signed)
     }
 

crates/primitives/src/transaction/pooled.rs

@@ -1,5 +1,9 @@
-//! Includes the
-use crate::{BlobTransaction, Bytes, TransactionSigned, EIP4844_TX_TYPE_ID};
+//! Defines the types for blob transactions, legacy, and other EIP-2718 transactions included in a
+//! response to `GetPooledTransactions`.
+use crate::{
+    BlobTransaction, Bytes, Signature, Transaction, TransactionSigned, TxEip1559, TxEip2930,
+    TxHash, TxLegacy, EIP4844_TX_TYPE_ID,
+};
 use bytes::Buf;
 use reth_rlp::{Decodable, DecodeError, Encodable, Header, EMPTY_LIST_CODE};
 use serde::{Deserialize, Serialize};
@@ -9,10 +13,35 @@ use serde::{Deserialize, Serialize};
 // TODO: redo arbitrary for this encoding - the previous encoding was incorrect
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 pub enum PooledTransactionsElement {
+    /// A legacy transaction
+    Legacy {
+        /// The inner transaction
+        transaction: TxLegacy,
+        /// The signature
+        signature: Signature,
+        /// The hash of the transaction
+        hash: TxHash,
+    },
+    /// An EIP-2930 typed transaction
+    Eip2930 {
+        /// The inner transaction
+        transaction: TxEip2930,
+        /// The signature
+        signature: Signature,
+        /// The hash of the transaction
+        hash: TxHash,
+    },
+    /// An EIP-1559 typed transaction
+    Eip1559 {
+        /// The inner transaction
+        transaction: TxEip1559,
+        /// The signature
+        signature: Signature,
+        /// The hash of the transaction
+        hash: TxHash,
+    },
     /// A blob transaction, which includes the transaction, blob data, commitments, and proofs.
     BlobTransaction(BlobTransaction),
-    /// A non-4844 signed transaction.
-    Transaction(TransactionSigned),
 }
 
 impl PooledTransactionsElement {
@@ -34,7 +63,10 @@ impl PooledTransactionsElement {
         // Check if the tx is a list - tx types are less than EMPTY_LIST_CODE (0xc0)
         if data[0] >= EMPTY_LIST_CODE {
             // decode as legacy transaction
-            Ok(Self::Transaction(TransactionSigned::decode_rlp_legacy_transaction(&mut data)?))
+            let (transaction, hash, signature) =
+                TransactionSigned::decode_rlp_legacy_transaction_tuple(&mut data)?;
+
+            Ok(Self::Legacy { transaction, signature, hash })
         } else {
             // decode the type byte, only decode BlobTransaction if it is a 4844 transaction
             let tx_type = *data.first().ok_or(DecodeError::InputTooShort)?;
@@ -60,7 +92,27 @@ impl PooledTransactionsElement {
                 // DO NOT advance the buffer for the type, since we want the enveloped decoding to
                 // decode it again and advance the buffer on its own.
                 let typed_tx = TransactionSigned::decode_enveloped_typed_transaction(&mut data)?;
-                Ok(PooledTransactionsElement::Transaction(typed_tx))
+
+                // because we checked the tx type, we can be sure that the transaction is not a
+                // blob transaction or legacy
+                match typed_tx.transaction {
+                    Transaction::Legacy(_) => Err(DecodeError::Custom(
+                        "legacy transactions should not be a result of EIP-2718 decoding",
+                    )),
+                    Transaction::Eip4844(_) => Err(DecodeError::Custom(
+                        "EIP-4844 transactions can only be decoded with transaction type 0x03",
+                    )),
+                    Transaction::Eip2930(tx) => Ok(PooledTransactionsElement::Eip2930 {
+                        transaction: tx,
+                        signature: typed_tx.signature,
+                        hash: typed_tx.hash,
+                    }),
+                    Transaction::Eip1559(tx) => Ok(PooledTransactionsElement::Eip1559 {
+                        transaction: tx,
+                        signature: typed_tx.signature,
+                        hash: typed_tx.hash,
+                    }),
+                }
             }
         }
     }
@@ -68,8 +120,20 @@ impl PooledTransactionsElement {
     /// Returns the inner [TransactionSigned].
     pub fn into_transaction(self) -> TransactionSigned {
         match self {
-            Self::Transaction(tx) => tx,
-            Self::BlobTransaction(blob_tx) => blob_tx.transaction,
+            Self::Legacy { transaction, signature, hash } => {
+                TransactionSigned { transaction: Transaction::Legacy(transaction), signature, hash }
+            }
+            Self::Eip2930 { transaction, signature, hash } => TransactionSigned {
+                transaction: Transaction::Eip2930(transaction),
+                signature,
+                hash,
+            },
+            Self::Eip1559 { transaction, signature, hash } => TransactionSigned {
+                transaction: Transaction::Eip1559(transaction),
+                signature,
+                hash,
+            },
+            Self::BlobTransaction(blob_tx) => blob_tx.into_parts().0,
         }
     }
 }
@@ -78,7 +142,39 @@ impl Encodable for PooledTransactionsElement {
     /// Encodes an enveloped post EIP-4844 [PooledTransactionsElement].
     fn encode(&self, out: &mut dyn bytes::BufMut) {
         match self {
-            Self::Transaction(tx) => tx.encode(out),
+            Self::Legacy { transaction, signature, hash } => {
+                // construct signed transaction
+                let signed_tx = TransactionSigned {
+                    transaction: Transaction::Legacy(transaction.clone()),
+                    signature: *signature,
+                    hash: *hash,
+                };
+
+                // encode signed transaction
+                signed_tx.encode(out);
+            }
+            Self::Eip2930 { transaction, signature, hash } => {
+                // construct signed transaction
+                let signed_tx = TransactionSigned {
+                    transaction: Transaction::Eip2930(transaction.clone()),
+                    signature: *signature,
+                    hash: *hash,
+                };
+
+                // encode signed transaction
+                signed_tx.encode(out);
+            }
+            Self::Eip1559 { transaction, signature, hash } => {
+                // construct signed transaction
+                let signed_tx = TransactionSigned {
+                    transaction: Transaction::Eip1559(transaction.clone()),
+                    signature: *signature,
+                    hash: *hash,
+                };
+
+                // encode signed transaction
+                signed_tx.encode(out);
+            }
             Self::BlobTransaction(blob_tx) => {
                 // The inner encoding is used with `with_header` set to true, making the final
                 // encoding:
@@ -90,7 +186,36 @@ impl Encodable for PooledTransactionsElement {
 
     fn length(&self) -> usize {
         match self {
-            Self::Transaction(tx) => tx.length(),
+            Self::Legacy { transaction, signature, hash } => {
+                // construct signed transaction
+                let signed_tx = TransactionSigned {
+                    transaction: Transaction::Legacy(transaction.clone()),
+                    signature: *signature,
+                    hash: *hash,
+                };
+
+                signed_tx.length()
+            }
+            Self::Eip2930 { transaction, signature, hash } => {
+                // construct signed transaction
+                let signed_tx = TransactionSigned {
+                    transaction: Transaction::Eip2930(transaction.clone()),
+                    signature: *signature,
+                    hash: *hash,
+                };
+
+                signed_tx.length()
+            }
+            Self::Eip1559 { transaction, signature, hash } => {
+                // construct signed transaction
+                let signed_tx = TransactionSigned {
+                    transaction: Transaction::Eip1559(transaction.clone()),
+                    signature: *signature,
+                    hash: *hash,
+                };
+
+                signed_tx.length()
+            }
             Self::BlobTransaction(blob_tx) => {
                 // the encoding uses a header, so we set `with_header` to true
                 blob_tx.payload_len_with_type(true)
@@ -129,14 +254,14 @@ impl Decodable for PooledTransactionsElement {
         // Check if the tx is a list
         if header.list {
             // decode as legacy transaction
-            let legacy_tx =
-                TransactionSigned::decode_rlp_legacy_transaction(&mut original_encoding)?;
+            let (transaction, hash, signature) =
+                TransactionSigned::decode_rlp_legacy_transaction_tuple(&mut original_encoding)?;
 
             // advance the buffer based on how far `decode_rlp_legacy_transaction` advanced the
             // buffer
             *buf = original_encoding;
 
-            Ok(PooledTransactionsElement::Transaction(legacy_tx))
+            Ok(Self::Legacy { transaction, signature, hash })
         } else {
             // decode the type byte, only decode BlobTransaction if it is a 4844 transaction
             let tx_type = *buf.first().ok_or(DecodeError::InputTooShort)?;
@@ -162,7 +287,27 @@ impl Decodable for PooledTransactionsElement {
                 // DO NOT advance the buffer for the type, since we want the enveloped decoding to
                 // decode it again and advance the buffer on its own.
                 let typed_tx = TransactionSigned::decode_enveloped_typed_transaction(buf)?;
-                Ok(PooledTransactionsElement::Transaction(typed_tx))
+
+                // because we checked the tx type, we can be sure that the transaction is not a
+                // blob transaction or legacy
+                match typed_tx.transaction {
+                    Transaction::Legacy(_) => Err(DecodeError::Custom(
+                        "legacy transactions should not be a result of EIP-2718 decoding",
+                    )),
+                    Transaction::Eip4844(_) => Err(DecodeError::Custom(
+                        "EIP-4844 transactions can only be decoded with transaction type 0x03",
+                    )),
+                    Transaction::Eip2930(tx) => Ok(PooledTransactionsElement::Eip2930 {
+                        transaction: tx,
+                        signature: typed_tx.signature,
+                        hash: typed_tx.hash,
+                    }),
+                    Transaction::Eip1559(tx) => Ok(PooledTransactionsElement::Eip1559 {
+                        transaction: tx,
+                        signature: typed_tx.signature,
+                        hash: typed_tx.hash,
+                    }),
+                }
             }
         }
     }
@@ -171,8 +316,28 @@ impl Decodable for PooledTransactionsElement {
 impl From<TransactionSigned> for PooledTransactionsElement {
     /// Converts from a [TransactionSigned] to a [PooledTransactionsElement].
     ///
-    /// NOTE: This will always return a [PooledTransactionsElement::Transaction] variant.
+    /// NOTE: For EIP-4844 transactions, this will return an empty sidecar.
     fn from(tx: TransactionSigned) -> Self {
-        Self::Transaction(tx)
+        let TransactionSigned { transaction, signature, hash } = tx;
+        match transaction {
+            Transaction::Legacy(tx) => {
+                PooledTransactionsElement::Legacy { transaction: tx, signature, hash }
+            }
+            Transaction::Eip2930(tx) => {
+                PooledTransactionsElement::Eip2930 { transaction: tx, signature, hash }
+            }
+            Transaction::Eip1559(tx) => {
+                PooledTransactionsElement::Eip1559 { transaction: tx, signature, hash }
+            }
+            Transaction::Eip4844(tx) => {
+                PooledTransactionsElement::BlobTransaction(BlobTransaction {
+                    transaction: tx,
+                    signature,
+                    hash,
+                    // This is empty - just for the conversion!
+                    sidecar: Default::default(),
+                })
+            }
+        }
     }
 }