use result for TransactionCompact::fill. (#12170)

Co-authored-by: Emilia Hane <elsaemiliaevahane@gmail.com>
Co-authored-by: dkathiriya <lakshya-sky@users.noreply.github.com>

crates/optimism/rpc/src/error.rs

@@ -1,6 +1,6 @@
 //! RPC errors specific to OP.
 
-use alloy_rpc_types_eth::error::EthRpcErrorCode;
+use alloy_rpc_types_eth::{error::EthRpcErrorCode, BlockError};
 use jsonrpsee_types::error::INTERNAL_ERROR_CODE;
 use reth_optimism_evm::OpBlockExecutionError;
 use reth_primitives::revm_primitives::{InvalidTransaction, OptimismInvalidTransaction};
@@ -113,3 +113,9 @@ impl From<SequencerClientError> for jsonrpsee_types::error::ErrorObject<'static>
         )
     }
 }
+
+impl From<BlockError> for OpEthApiError {
+    fn from(error: BlockError) -> Self {
+        Self::Eth(error.into())
+    }
+}

crates/optimism/rpc/src/eth/transaction.rs

@@ -15,7 +15,7 @@ use reth_rpc_eth_api::{
 use reth_rpc_eth_types::utils::recover_raw_transaction;
 use reth_transaction_pool::{PoolTransaction, TransactionOrigin, TransactionPool};
 
-use crate::{OpEthApi, SequencerClient};
+use crate::{OpEthApi, OpEthApiError, SequencerClient};
 
 impl<N> EthTransactions for OpEthApi<N>
 where
@@ -76,12 +76,13 @@ where
     N: FullNodeComponents,
 {
     type Transaction = Transaction;
+    type Error = OpEthApiError;
 
     fn fill(
         &self,
         tx: TransactionSignedEcRecovered,
         tx_info: TransactionInfo,
-    ) -> Self::Transaction {
+    ) -> Result<Self::Transaction, Self::Error> {
         let from = tx.signer();
         let TransactionSigned { transaction, signature, hash } = tx.into_signed();
 
@@ -106,8 +107,7 @@ where
             .inner
             .provider()
             .receipt_by_hash(hash)
-            .ok() // todo: change sig to return result
-            .flatten()
+            .map_err(Self::Error::from_eth_err)?
             .and_then(|receipt| receipt.deposit_receipt_version);
 
         let TransactionInfo {
@@ -120,7 +120,7 @@ where
             })
             .unwrap_or_else(|| inner.max_fee_per_gas());
 
-        Transaction {
+        Ok(Transaction {
             inner: alloy_rpc_types_eth::Transaction {
                 inner,
                 block_hash,
@@ -130,7 +130,7 @@ where
                 effective_gas_price: Some(effective_gas_price),
             },
             deposit_receipt_version,
-        }
+        })
     }
 
     fn otterscan_api_truncate_input(tx: &mut Self::Transaction) {

crates/rpc/rpc-eth-api/src/core.rs

@@ -501,7 +501,8 @@ where
         trace!(target: "rpc::eth", ?hash, "Serving eth_getTransactionByHash");
         Ok(EthTransactions::transaction_by_hash(self, hash)
             .await?
-            .map(|tx| tx.into_transaction(self.tx_resp_builder())))
+            .map(|tx| tx.into_transaction(self.tx_resp_builder()))
+            .transpose()?)
     }
 
     /// Handler for: `eth_getRawTransactionByBlockHashAndIndex`

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

@@ -64,8 +64,7 @@ pub trait EthBlocks: LoadBlock {
                 full.into(),
                 Some(block_hash),
                 self.tx_resp_builder(),
-            )
-            .map_err(Self::Error::from_eth_err)?;
+            )?;
             Ok(Some(block))
         }
     }

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

@@ -17,7 +17,6 @@
 pub mod block;
 pub mod blocking_task;
 pub mod call;
-pub mod error;
 pub mod fee;
 pub mod pending_block;
 pub mod receipt;

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

@@ -208,7 +208,7 @@ pub trait EthTransactions: LoadTransaction<Provider: BlockReaderIdExt> {
                         tx.clone().with_signer(*signer),
                         tx_info,
                         self.tx_resp_builder(),
-                    )))
+                    )?))
                 }
             }
 
@@ -233,7 +233,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.into(), self.tx_resp_builder())));
+                    return Ok(Some(from_recovered(transaction.into(), self.tx_resp_builder())?));
                 }
             }
 
@@ -291,7 +291,7 @@ pub trait EthTransactions: LoadTransaction<Provider: BlockReaderIdExt> {
                             )
                         })
                 })
-                .ok_or(EthApiError::HeaderNotFound(block_id).into())
+                .ok_or(EthApiError::HeaderNotFound(block_id))?
                 .map(Some)
         }
     }

crates/rpc/rpc-eth-api/src/lib.rs

@@ -20,12 +20,14 @@ pub mod node;
 pub mod pubsub;
 pub mod types;
 
+pub use reth_rpc_eth_types::error::{
+    AsEthApiError, FromEthApiError, FromEvmError, IntoEthApiError,
+};
 pub use reth_rpc_types_compat::TransactionCompat;
 
 pub use bundle::{EthBundleApiServer, EthCallBundleApiServer};
 pub use core::{EthApiServer, FullEthApiServer};
 pub use filter::EthFilterApiServer;
-pub use helpers::error::{AsEthApiError, FromEthApiError, FromEvmError, IntoEthApiError};
 pub use node::{RpcNodeCore, RpcNodeCoreExt};
 pub use pubsub::EthPubSubApiServer;
 pub use types::{EthApiTypes, FullEthApiTypes, RpcBlock, RpcReceipt, RpcTransaction};

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

@@ -39,15 +39,26 @@ pub type RpcBlock<T> = Block<RpcTransaction<T>, <T as Network>::HeaderResponse>;
 /// Adapter for network specific receipt type.
 pub type RpcReceipt<T> = <T as Network>::ReceiptResponse;
 
+/// Adapter for network specific error type.
+pub type RpcError<T> = <T as EthApiTypes>::Error;
+
 /// Helper trait holds necessary trait bounds on [`EthApiTypes`] to implement `eth` API.
 pub trait FullEthApiTypes:
-    EthApiTypes<TransactionCompat: TransactionCompat<Transaction = RpcTransaction<Self::NetworkTypes>>>
+    EthApiTypes<
+    TransactionCompat: TransactionCompat<
+        Transaction = RpcTransaction<Self::NetworkTypes>,
+        Error = RpcError<Self>,
+    >,
+>
 {
 }
 
 impl<T> FullEthApiTypes for T where
     T: EthApiTypes<
-        TransactionCompat: TransactionCompat<Transaction = RpcTransaction<T::NetworkTypes>>,
+        TransactionCompat: TransactionCompat<
+            Transaction = RpcTransaction<T::NetworkTypes>,
+            Error = RpcError<T>,
+        >,
     >
 {
 }

crates/rpc/rpc-eth-api/src/helpers/error.rs → crates/rpc/rpc-eth-types/src/error/api.rs

@@ -1,9 +1,10 @@
 //! Helper traits to wrap generic l1 errors, in network specific error type configured in
-//! [`EthApiTypes`](crate::EthApiTypes).
+//! `reth_rpc_eth_api::EthApiTypes`.
 
-use reth_rpc_eth_types::EthApiError;
 use revm_primitives::EVMError;
 
+use crate::EthApiError;
+
 /// Helper trait to wrap core [`EthApiError`].
 pub trait FromEthApiError: From<EthApiError> {
     /// Converts from error via [`EthApiError`].
@@ -51,7 +52,7 @@ pub trait AsEthApiError {
     fn as_err(&self) -> Option<&EthApiError>;
 
     /// Returns `true` if error is
-    /// [`RpcInvalidTransactionError::GasTooHigh`](reth_rpc_eth_types::RpcInvalidTransactionError::GasTooHigh).
+    /// [`RpcInvalidTransactionError::GasTooHigh`](crate::RpcInvalidTransactionError::GasTooHigh).
     fn is_gas_too_high(&self) -> bool {
         if let Some(err) = self.as_err() {
             return err.is_gas_too_high()
@@ -61,7 +62,7 @@ pub trait AsEthApiError {
     }
 
     /// Returns `true` if error is
-    /// [`RpcInvalidTransactionError::GasTooLow`](reth_rpc_eth_types::RpcInvalidTransactionError::GasTooLow).
+    /// [`RpcInvalidTransactionError::GasTooLow`](crate::RpcInvalidTransactionError::GasTooLow).
     fn is_gas_too_low(&self) -> bool {
         if let Some(err) = self.as_err() {
             return err.is_gas_too_low()

crates/rpc/rpc-eth-types/src/error.rs → crates/rpc/rpc-eth-types/src/error/mod.rs

@@ -1,6 +1,9 @@
 //! Implementation specific Errors for the `eth_` namespace.
 
-use std::time::Duration;
+pub mod api;
+pub use api::{AsEthApiError, FromEthApiError, FromEvmError, IntoEthApiError};
+
+use core::time::Duration;
 
 use alloy_eips::BlockId;
 use alloy_primitives::{Address, Bytes, U256};

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

@@ -21,8 +21,9 @@ use revm::{db::CacheDB, Database};
 use revm_primitives::{keccak256, Address, BlockEnv, Bytes, ExecutionResult, TxKind, B256, U256};
 
 use crate::{
-    cache::db::StateProviderTraitObjWrapper, error::ToRpcError, EthApiError, RevertError,
-    RpcInvalidTransactionError,
+    cache::db::StateProviderTraitObjWrapper,
+    error::{api::FromEthApiError, ToRpcError},
+    EthApiError, RevertError, RpcInvalidTransactionError,
 };
 
 /// Errors which may occur during `eth_simulateV1` execution.
@@ -170,7 +171,7 @@ where
 
 /// Handles outputs of the calls execution and builds a [`SimulatedBlock`].
 #[expect(clippy::complexity)]
-pub fn build_block<T: TransactionCompat>(
+pub fn build_block<T: TransactionCompat<Error: FromEthApiError>>(
     results: Vec<(Address, ExecutionResult)>,
     transactions: Vec<TransactionSigned>,
     block_env: &BlockEnv,
@@ -179,7 +180,7 @@ pub fn build_block<T: TransactionCompat>(
     full_transactions: bool,
     db: &CacheDB<StateProviderDatabase<StateProviderTraitObjWrapper<'_>>>,
     tx_resp_builder: &T,
-) -> Result<SimulatedBlock<Block<T::Transaction>>, EthApiError> {
+) -> Result<SimulatedBlock<Block<T::Transaction>>, T::Error> {
     let mut calls: Vec<SimCallResult> = Vec::with_capacity(results.len());
     let mut senders = Vec::with_capacity(results.len());
     let mut receipts = Vec::with_capacity(results.len());
@@ -272,7 +273,7 @@ pub fn build_block<T: TransactionCompat>(
         }
     }
 
-    let state_root = db.db.state_root(hashed_state)?;
+    let state_root = db.db.state_root(hashed_state).map_err(T::Error::from_eth_err)?;
 
     let header = reth_primitives::Header {
         beneficiary: block_env.coinbase,

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

@@ -41,7 +41,10 @@ impl TransactionSource {
     }
 
     /// Conversion into network specific transaction type.
-    pub fn into_transaction<T: TransactionCompat>(self, resp_builder: &T) -> T::Transaction {
+    pub fn into_transaction<T: TransactionCompat>(
+        self,
+        resp_builder: &T,
+    ) -> Result<T::Transaction, T::Error> {
         match self {
             Self::Pool(tx) => from_recovered(tx, resp_builder),
             Self::Block { transaction, index, block_hash, block_number, base_fee } => {

crates/rpc/rpc-types-compat/Cargo.toml

@@ -27,6 +27,7 @@ alloy-consensus.workspace = true
 
 # io
 serde.workspace = true
+jsonrpsee-types.workspace = true
 
 [dev-dependencies]
 serde_json.workspace = true

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

@@ -1,15 +1,16 @@
 //! Compatibility functions for rpc `Block` type.
 
-use crate::{transaction::from_recovered_with_block_context, TransactionCompat};
 use alloy_consensus::Sealed;
 use alloy_eips::eip4895::Withdrawals;
 use alloy_primitives::{B256, U256};
 use alloy_rlp::Encodable;
 use alloy_rpc_types_eth::{
-    Block, BlockError, BlockTransactions, BlockTransactionsKind, Header, TransactionInfo,
+    Block, BlockTransactions, BlockTransactionsKind, Header, TransactionInfo,
 };
 use reth_primitives::{Block as PrimitiveBlock, BlockWithSenders};
 
+use crate::{transaction::from_recovered_with_block_context, TransactionCompat};
+
 /// Converts the given primitive block into a [`Block`] response with the given
 /// [`BlockTransactionsKind`]
 ///
@@ -20,7 +21,7 @@ pub fn from_block<T: TransactionCompat>(
     kind: BlockTransactionsKind,
     block_hash: Option<B256>,
     tx_resp_builder: &T,
-) -> Result<Block<T::Transaction>, BlockError> {
+) -> Result<Block<T::Transaction>, T::Error> {
     match kind {
         BlockTransactionsKind::Hashes => {
             Ok(from_block_with_tx_hashes::<T::Transaction>(block, total_difficulty, block_hash))
@@ -63,7 +64,7 @@ pub fn from_block_full<T: TransactionCompat>(
     total_difficulty: U256,
     block_hash: Option<B256>,
     tx_resp_builder: &T,
-) -> Result<Block<T::Transaction>, BlockError> {
+) -> Result<Block<T::Transaction>, T::Error> {
     let block_hash = block_hash.unwrap_or_else(|| block.block.header.hash_slow());
     let block_number = block.block.number;
     let base_fee_per_gas = block.block.base_fee_per_gas;
@@ -88,7 +89,7 @@ pub fn from_block_full<T: TransactionCompat>(
 
             from_recovered_with_block_context::<T>(signed_tx_ec_recovered, tx_info, tx_resp_builder)
         })
-        .collect::<Vec<_>>();
+        .collect::<Result<Vec<_>, T::Error>>()?;
 
     Ok(from_block_with_transactions(
         block_length,

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

@@ -1,5 +1,6 @@
 //! Compatibility functions for rpc `Transaction` type.
 
+use core::error;
 use std::fmt;
 
 use alloy_consensus::Transaction as _;
@@ -19,7 +20,7 @@ pub fn from_recovered_with_block_context<T: TransactionCompat>(
     tx: TransactionSignedEcRecovered,
     tx_info: TransactionInfo,
     resp_builder: &T,
-) -> T::Transaction {
+) -> Result<T::Transaction, T::Error> {
     resp_builder.fill(tx, tx_info)
 }
 
@@ -28,7 +29,7 @@ pub fn from_recovered_with_block_context<T: TransactionCompat>(
 pub fn from_recovered<T: TransactionCompat>(
     tx: TransactionSignedEcRecovered,
     resp_builder: &T,
-) -> T::Transaction {
+) -> Result<T::Transaction, T::Error> {
     resp_builder.fill(tx, TransactionInfo::default())
 }
 
@@ -43,9 +44,16 @@ pub trait TransactionCompat: Send + Sync + Unpin + Clone + fmt::Debug {
         + Clone
         + fmt::Debug;
 
+    /// RPC transaction error type.
+    type Error: error::Error + Into<jsonrpsee_types::ErrorObject<'static>>;
+
     /// Create a new rpc transaction result for a _pending_ signed transaction, setting block
     /// environment related fields to `None`.
-    fn fill(&self, tx: TransactionSignedEcRecovered, tx_inf: TransactionInfo) -> Self::Transaction;
+    fn fill(
+        &self,
+        tx: TransactionSignedEcRecovered,
+        tx_inf: TransactionInfo,
+    ) -> Result<Self::Transaction, Self::Error>;
 
     /// Truncates the input of a transaction to only the first 4 bytes.
     // todo: remove in favour of using constructor on `TransactionResponse` or similar

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

@@ -34,7 +34,7 @@ use tokio::{
     sync::{mpsc::Receiver, Mutex},
     time::MissedTickBehavior,
 };
-use tracing::trace;
+use tracing::{error, trace};
 
 /// The maximum number of headers we read at once when handling a range filter.
 const MAX_HEADERS_RANGE: u64 = 1_000; // with ~530bytes per header this is ~500kb
@@ -625,10 +625,15 @@ where
         let mut prepared_stream = self.txs_stream.lock().await;
 
         while let Ok(tx) = prepared_stream.try_recv() {
-            pending_txs.push(from_recovered(
-                tx.transaction.to_recovered_transaction(),
-                &self.tx_resp_builder,
-            ))
+            match from_recovered(tx.transaction.to_recovered_transaction(), &self.tx_resp_builder) {
+                Ok(tx) => pending_txs.push(tx),
+                Err(err) => {
+                    error!(target: "rpc",
+                        %err,
+                        "Failed to fill txn with block context"
+                    );
+                }
+            }
         }
         FilterChanges::Transactions(pending_txs)
     }