refactor: Remove standalone from_recovered functions and make part of TransactionCompat trait (#13653)

Author: tonypony220

crates/rpc/rpc-eth-api/src/helpers/transaction.rs

@@ -21,7 +21,7 @@ use reth_provider::{
     TransactionsProvider,
 };
 use reth_rpc_eth_types::{utils::binary_search, EthApiError, SignError, TransactionSource};
-use reth_rpc_types_compat::transaction::{from_recovered, from_recovered_with_block_context};
+use reth_rpc_types_compat::transaction::TransactionCompat;
 use reth_transaction_pool::{PoolTransaction, TransactionOrigin, TransactionPool};
 use std::sync::Arc;
 
@@ -221,11 +221,9 @@ pub trait EthTransactions: LoadTransaction<Provider: BlockReaderIdExt> {
                         index: Some(index as u64),
                     };
 
-                    return Ok(Some(from_recovered_with_block_context(
-                        tx.clone().with_signer(*signer),
-                        tx_info,
-                        self.tx_resp_builder(),
-                    )?))
+                    return Ok(Some(
+                        self.tx_resp_builder().fill(tx.clone().with_signer(*signer), tx_info)?,
+                    ))
                 }
             }
 
@@ -250,7 +248,7 @@ pub trait EthTransactions: LoadTransaction<Provider: BlockReaderIdExt> {
                     RpcNodeCore::pool(self).get_transaction_by_sender_and_nonce(sender, nonce)
                 {
                     let transaction = tx.transaction.clone_into_consensus();
-                    return Ok(Some(from_recovered(transaction, self.tx_resp_builder())?));
+                    return Ok(Some(self.tx_resp_builder().fill_pending(transaction)?));
                 }
             }
 
@@ -301,11 +299,7 @@ pub trait EthTransactions: LoadTransaction<Provider: BlockReaderIdExt> {
                                 base_fee: base_fee_per_gas.map(u128::from),
                                 index: Some(index as u64),
                             };
-                            from_recovered_with_block_context(
-                                tx.clone().with_signer(*signer),
-                                tx_info,
-                                self.tx_resp_builder(),
-                            )
+                            self.tx_resp_builder().fill(tx.clone().with_signer(*signer), tx_info)
                         })
                 })
                 .ok_or(EthApiError::HeaderNotFound(block_id))?

crates/rpc/rpc-eth-types/src/transaction.rs

@@ -6,10 +6,7 @@ use alloy_primitives::B256;
 use alloy_rpc_types_eth::TransactionInfo;
 use reth_primitives::{RecoveredTx, TransactionSigned};
 use reth_primitives_traits::SignedTransaction;
-use reth_rpc_types_compat::{
-    transaction::{from_recovered, from_recovered_with_block_context},
-    TransactionCompat,
-};
+use reth_rpc_types_compat::TransactionCompat;
 
 /// Represents from where a transaction was fetched.
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -47,7 +44,7 @@ impl<T: SignedTransaction> TransactionSource<T> {
         resp_builder: &Builder,
     ) -> Result<Builder::Transaction, Builder::Error> {
         match self {
-            Self::Pool(tx) => from_recovered(tx, resp_builder),
+            Self::Pool(tx) => resp_builder.fill_pending(tx),
             Self::Block { transaction, index, block_hash, block_number, base_fee } => {
                 let tx_info = TransactionInfo {
                     hash: Some(transaction.trie_hash()),
@@ -57,7 +54,7 @@ impl<T: SignedTransaction> TransactionSource<T> {
                     base_fee: base_fee.map(u128::from),
                 };
 
-                from_recovered_with_block_context(transaction, tx_info, resp_builder)
+                resp_builder.fill(transaction, tx_info)
             }
         }
     }

crates/rpc/rpc-types-compat/src/block.rs

@@ -9,7 +9,7 @@ use alloy_rpc_types_eth::{
 use reth_primitives::{transaction::SignedTransactionIntoRecoveredExt, BlockWithSenders};
 use reth_primitives_traits::{Block as BlockTrait, BlockBody, SignedTransaction};
 
-use crate::{transaction::from_recovered_with_block_context, TransactionCompat};
+use crate::transaction::TransactionCompat;
 
 /// Converts the given primitive block into a [`Block`] response with the given
 /// [`BlockTransactionsKind`]
@@ -94,11 +94,7 @@ where
                 index: Some(idx as u64),
             };
 
-            from_recovered_with_block_context::<_, T>(
-                signed_tx_ec_recovered,
-                tx_info,
-                tx_resp_builder,
-            )
+            tx_resp_builder.fill(signed_tx_ec_recovered, tx_info)
         })
         .collect::<Result<Vec<_>, T::Error>>()?;
 

crates/rpc/rpc-types-compat/src/transaction.rs

@@ -7,28 +7,6 @@ use alloy_rpc_types_eth::{request::TransactionRequest, TransactionInfo};
 use reth_primitives::{RecoveredTx, TransactionSigned};
 use serde::{Deserialize, Serialize};
 
-/// Create a new rpc transaction result for a mined transaction, using the given block hash,
-/// number, and tx index fields to populate the corresponding fields in the rpc result.
-///
-/// The block hash, number, and tx index fields should be from the original block where the
-/// transaction was mined.
-pub fn from_recovered_with_block_context<Tx, T: TransactionCompat<Tx>>(
-    tx: RecoveredTx<Tx>,
-    tx_info: TransactionInfo,
-    resp_builder: &T,
-) -> Result<T::Transaction, T::Error> {
-    resp_builder.fill(tx, tx_info)
-}
-
-/// Create a new rpc transaction result for a _pending_ signed transaction, setting block
-/// environment related fields to `None`.
-pub fn from_recovered<Tx, T: TransactionCompat<Tx>>(
-    tx: RecoveredTx<Tx>,
-    resp_builder: &T,
-) -> Result<T::Transaction, T::Error> {
-    resp_builder.fill(tx, TransactionInfo::default())
-}
-
 /// Builds RPC transaction w.r.t. network.
 pub trait TransactionCompat<T = TransactionSigned>:
     Send + Sync + Unpin + Clone + fmt::Debug
@@ -45,8 +23,18 @@ pub trait TransactionCompat<T = TransactionSigned>:
     /// RPC transaction error type.
     type Error: error::Error + Into<jsonrpsee_types::ErrorObject<'static>>;
 
+    /// Wrapper for `fill()` with default `TransactionInfo`
     /// Create a new rpc transaction result for a _pending_ signed transaction, setting block
     /// environment related fields to `None`.
+    fn fill_pending(&self, tx: RecoveredTx<T>) -> Result<Self::Transaction, Self::Error> {
+        self.fill(tx, TransactionInfo::default())
+    }
+
+    /// Create a new rpc transaction result for a mined transaction, using the given block hash,
+    /// number, and tx index fields to populate the corresponding fields in the rpc result.
+    ///
+    /// The block hash, number, and tx index fields should be from the original block where the
+    /// transaction was mined.
     fn fill(
         &self,
         tx: RecoveredTx<T>,

crates/rpc/rpc/src/eth/filter.rs

@@ -23,7 +23,6 @@ use reth_rpc_eth_types::{
     EthApiError, EthFilterConfig, EthStateCache, EthSubscriptionIdProvider,
 };
 use reth_rpc_server_types::{result::rpc_error_with_code, ToRpcResult};
-use reth_rpc_types_compat::transaction::from_recovered;
 use reth_tasks::TaskSpawner;
 use reth_transaction_pool::{NewSubpoolTransactionStream, PoolTransaction, TransactionPool};
 use std::{
@@ -637,7 +636,7 @@ where
         let mut prepared_stream = self.txs_stream.lock().await;
 
         while let Ok(tx) = prepared_stream.try_recv() {
-            match from_recovered(tx.transaction.to_consensus(), &self.tx_resp_builder) {
+            match self.tx_resp_builder.fill_pending(tx.transaction.to_consensus()) {
                 Ok(tx) => pending_txs.push(tx),
                 Err(err) => {
                     error!(target: "rpc",

crates/rpc/rpc/src/eth/pubsub.rs

@@ -19,7 +19,6 @@ use reth_rpc_eth_api::{
 };
 use reth_rpc_eth_types::logs_utils;
 use reth_rpc_server_types::result::{internal_rpc_err, invalid_params_rpc_err};
-use reth_rpc_types_compat::transaction::from_recovered;
 use reth_tasks::{TaskSpawner, TokioTaskExecutor};
 use reth_transaction_pool::{NewTransactionEvent, PoolConsensusTx, TransactionPool};
 use serde::Serialize;
@@ -119,10 +118,11 @@ where
                     Params::Bool(true) => {
                         // full transaction objects requested
                         let stream = pubsub.full_pending_transaction_stream().filter_map(|tx| {
-                            let tx_value = match from_recovered(
-                                tx.transaction.to_consensus(),
-                                pubsub.eth_api.tx_resp_builder(),
-                            ) {
+                            let tx_value = match pubsub
+                                .eth_api
+                                .tx_resp_builder()
+                                .fill_pending(tx.transaction.to_consensus())
+                            {
                                 Ok(tx) => Some(tx),
                                 Err(err) => {
                                     error!(target = "rpc",

crates/rpc/rpc/src/txpool.rs

@@ -9,7 +9,7 @@ use alloy_rpc_types_txpool::{
 use async_trait::async_trait;
 use jsonrpsee::core::RpcResult;
 use reth_rpc_api::TxPoolApiServer;
-use reth_rpc_types_compat::{transaction::from_recovered, TransactionCompat};
+use reth_rpc_types_compat::TransactionCompat;
 use reth_transaction_pool::{
     AllPoolTransactions, PoolConsensusTx, PoolTransaction, TransactionPool,
 };
@@ -50,7 +50,7 @@ where
         {
             content.entry(tx.sender()).or_default().insert(
                 tx.nonce().to_string(),
-                from_recovered(tx.clone_into_consensus(), resp_builder)?,
+                resp_builder.fill_pending(tx.clone_into_consensus())?,
             );
 
             Ok(())