Merge pull request #155 from anton-rs/refcell/hoist-optimism-tx-meta

fix: addresses review comments

crates/revm/src/optimism/mod.rs

@@ -4,7 +4,6 @@ use revm::{
     primitives::{BedrockSpec, RegolithSpec},
     L1BlockInfo,
 };
-use std::sync::Arc;
 
 /// Optimism-specific processor implementation for the `EVMProcessor`
 pub mod processor;
@@ -87,7 +86,7 @@ pub trait RethL1BlockInfo {
     /// - `is_deposit`: Whether or not the transaction is a deposit.
     fn l1_tx_data_fee(
         &self,
-        chain_spec: Arc<ChainSpec>,
+        chain_spec: &ChainSpec,
         timestamp: u64,
         input: &Bytes,
         is_deposit: bool,
@@ -101,7 +100,7 @@ pub trait RethL1BlockInfo {
     /// - `input`: The calldata of the transaction.
     fn l1_data_gas(
         &self,
-        chain_spec: Arc<ChainSpec>,
+        chain_spec: &ChainSpec,
         timestamp: u64,
         input: &Bytes,
     ) -> Result<U256, BlockExecutionError>;
@@ -110,7 +109,7 @@ pub trait RethL1BlockInfo {
 impl RethL1BlockInfo for L1BlockInfo {
     fn l1_tx_data_fee(
         &self,
-        chain_spec: Arc<ChainSpec>,
+        chain_spec: &ChainSpec,
         timestamp: u64,
         input: &Bytes,
         is_deposit: bool,
@@ -134,7 +133,7 @@ impl RethL1BlockInfo for L1BlockInfo {
 
     fn l1_data_gas(
         &self,
-        chain_spec: Arc<ChainSpec>,
+        chain_spec: &ChainSpec,
         timestamp: u64,
         input: &Bytes,
     ) -> Result<U256, BlockExecutionError> {

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

@@ -16,43 +16,6 @@ use reth_rpc_types::{Index, RichBlock, TransactionReceipt};
 use reth_rpc_types_compat::block::{from_block, uncle_block_from_header};
 use reth_transaction_pool::TransactionPool;
 
-#[cfg(feature = "optimism")]
-use reth_primitives::U256;
-#[cfg(feature = "optimism")]
-use revm::L1BlockInfo;
-#[cfg(feature = "optimism")]
-use std::rc::Rc;
-
-/// Optimism Transaction Metadata
-///
-/// Includes the L1 fee and data gas for the tx along with the L1
-/// block info. In order to pass the [OptimismTxMeta] into the
-/// async colored `build_transaction_receipt_with_block_receipts`
-/// function, a reference counter for the L1 block info is
-/// used so the L1 block info can be shared between receipts.
-#[cfg(feature = "optimism")]
-#[derive(Debug, Default, Clone)]
-pub(crate) struct OptimismTxMeta {
-    /// The L1 block info.
-    pub(crate) l1_block_info: Option<Rc<L1BlockInfo>>,
-    /// The L1 fee for the block.
-    pub(crate) l1_fee: Option<U256>,
-    /// The L1 data gas for the block.
-    pub(crate) l1_data_gas: Option<U256>,
-}
-
-#[cfg(feature = "optimism")]
-impl OptimismTxMeta {
-    /// Creates a new [OptimismTxMeta].
-    pub(crate) fn new(
-        l1_block_info: Option<Rc<L1BlockInfo>>,
-        l1_fee: Option<U256>,
-        l1_data_gas: Option<U256>,
-    ) -> Self {
-        Self { l1_block_info, l1_fee, l1_data_gas }
-    }
-}
-
 impl<Provider, Pool, Network> EthApi<Provider, Pool, Network>
 where
     Provider:
@@ -118,7 +81,7 @@ where
                 let body = reth_revm::optimism::parse_l1_info_tx(
                     &block.body.first().ok_or(EthApiError::InternalEthError)?.input()[4..],
                 );
-                (block.timestamp, body.ok().map(Rc::new))
+                (block.timestamp, body.ok())
             };
 
             let receipts = block

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

@@ -33,6 +33,8 @@ use tokio::sync::{oneshot, Mutex};
 mod block;
 mod call;
 mod fees;
+#[cfg(feature = "optimism")]
+mod optimism;
 mod pending_block;
 mod server;
 mod sign;

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

@@ -0,0 +1,30 @@
+use reth_primitives::U256;
+use revm::L1BlockInfo;
+
+/// Optimism Transaction Metadata
+///
+/// Includes the L1 fee and data gas for the tx along with the L1
+/// block info. In order to pass the [OptimismTxMeta] into the
+/// async colored `build_transaction_receipt_with_block_receipts`
+/// function, a reference counter for the L1 block info is
+/// used so the L1 block info can be shared between receipts.
+#[derive(Debug, Default, Clone)]
+pub(crate) struct OptimismTxMeta {
+    /// The L1 block info.
+    pub(crate) l1_block_info: Option<L1BlockInfo>,
+    /// The L1 fee for the block.
+    pub(crate) l1_fee: Option<U256>,
+    /// The L1 data gas for the block.
+    pub(crate) l1_data_gas: Option<U256>,
+}
+
+impl OptimismTxMeta {
+    /// Creates a new [OptimismTxMeta].
+    pub(crate) fn new(
+        l1_block_info: Option<L1BlockInfo>,
+        l1_fee: Option<U256>,
+        l1_data_gas: Option<U256>,
+    ) -> Self {
+        Self { l1_block_info, l1_fee, l1_data_gas }
+    }
+}

crates/rpc/rpc/src/eth/api/transactions.rs

@@ -42,15 +42,13 @@ use revm::{
 use revm_primitives::{db::DatabaseCommit, Env, ExecutionResult, ResultAndState, SpecId, State};
 
 #[cfg(feature = "optimism")]
-use crate::eth::api::block::OptimismTxMeta;
+use crate::eth::api::optimism::OptimismTxMeta;
 #[cfg(feature = "optimism")]
 use reth_revm::optimism::RethL1BlockInfo;
 #[cfg(feature = "optimism")]
 use revm::L1BlockInfo;
 #[cfg(feature = "optimism")]
 use std::ops::Div;
-#[cfg(feature = "optimism")]
-use std::rc::Rc;
 
 /// Helper alias type for the state's [CacheDB]
 pub(crate) type StateCacheDB<'r> = CacheDB<StateProviderDatabase<StateProviderBox<'r>>>;
@@ -892,33 +890,36 @@ where
         meta: TransactionMeta,
         receipt: Receipt,
     ) -> EthResult<TransactionReceipt> {
-        // get all receipts for the block
-        let all_receipts = match self.cache().get_receipts(meta.block_hash).await? {
-            Some(recpts) => recpts,
-            None => return Err(EthApiError::UnknownBlockNumber),
-        };
-        let block = self
-            .provider()
-            .block_by_number(meta.block_number)?
-            .ok_or(EthApiError::UnknownBlockNumber)?;
+        let receipt_fut = self.cache().get_receipts(meta.block_hash);
+        let block_fut = self.cache().get_block(meta.block_hash);
+
+        let (receipts, block) = futures::try_join!(receipt_fut, block_fut)?;
+        let (receipts, block) = (
+            receipts.ok_or(EthApiError::InternalEthError)?,
+            block.ok_or(EthApiError::InternalEthError)?,
+        );
 
-        let l1_block_info = reth_revm::optimism::extract_l1_info(&block).ok().map(Rc::new);
+        let l1_block_info = reth_revm::optimism::extract_l1_info(&block).ok();
         let optimism_tx_meta = self.build_op_tx_meta(&tx, l1_block_info, block.timestamp)?;
 
         build_transaction_receipt_with_block_receipts(
             tx,
             meta,
             receipt,
-            &all_receipts,
+            &receipts,
             optimism_tx_meta,
         )
     }
 
+    /// Builds [OptimismTxMeta] object using the provided [TransactionSigned],
+    /// [L1BlockInfo] and `block_timestamp`. The [L1BlockInfo] is used to calculate
+    /// the l1 fee and l1 data gas for the transaction.
+    /// If the [L1BlockInfo] is not provided, the [OptimismTxMeta] will be empty.
     #[cfg(feature = "optimism")]
     pub(crate) fn build_op_tx_meta(
         &self,
         tx: &TransactionSigned,
-        l1_block_info: Option<Rc<L1BlockInfo>>,
+        l1_block_info: Option<L1BlockInfo>,
         block_timestamp: u64,
     ) -> EthResult<OptimismTxMeta> {
         if let Some(l1_block_info) = l1_block_info {
@@ -928,27 +929,33 @@ where
                 envelope_buf.freeze().into()
             };
 
-            let l1_fee = (!tx.is_deposit())
+            let (l1_fee, l1_data_gas) = match (!tx.is_deposit())
                 .then(|| {
-                    l1_block_info.l1_tx_data_fee(
-                        self.inner.provider.chain_spec(),
+                    let inner_l1_fee = match l1_block_info.l1_tx_data_fee(
+                        &self.inner.provider.chain_spec(),
                         block_timestamp,
                         &envelope_buf,
                         tx.is_deposit(),
-                    )
-                })
-                .transpose()
-                .map_err(|_| EthApiError::InternalEthError)?;
-            let l1_data_gas = (!tx.is_deposit())
-                .then(|| {
-                    l1_block_info.l1_data_gas(
-                        self.inner.provider.chain_spec(),
+                    ) {
+                        Ok(inner_l1_fee) => inner_l1_fee,
+                        Err(e) => return Err(e),
+                    };
+                    let inner_l1_data_gas = match l1_block_info.l1_data_gas(
+                        &self.inner.provider.chain_spec(),
                         block_timestamp,
                         &envelope_buf,
-                    )
+                    ) {
+                        Ok(inner_l1_data_gas) => inner_l1_data_gas,
+                        Err(e) => return Err(e),
+                    };
+                    Ok((inner_l1_fee, inner_l1_data_gas))
                 })
                 .transpose()
-                .map_err(|_| EthApiError::InternalEthError)?;
+                .map_err(|_| EthApiError::InternalEthError)?
+            {
+                Some((l1_fee, l1_data_gas)) => (Some(l1_fee), Some(l1_data_gas)),
+                None => (None, None),
+            };
 
             Ok(OptimismTxMeta::new(Some(l1_block_info), l1_fee, l1_data_gas))
         } else {

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

@@ -395,7 +395,7 @@ where
             transaction.to_recovered_transaction().encode_enveloped(&mut encoded);
             let cost_addition = match reth_revm::optimism::extract_l1_info(&block).map(|info| {
                 info.l1_tx_data_fee(
-                    Arc::clone(&self.chain_spec),
+                    &self.chain_spec,
                     block.timestamp,
                     &encoded.freeze().into(),
                     transaction.is_deposit(),