From 3190b1f9f6af289fb230892793402ff4fc62500a Mon Sep 17 00:00:00 2001 From: refcell Date: Thu, 2 Nov 2023 22:37:56 +0100 Subject: [PATCH 1/6] move optimism tx meta to new file --- crates/rpc/rpc/src/eth/api/block.rs | 45 ++-------------------- crates/rpc/rpc/src/eth/api/mod.rs | 10 +++-- crates/rpc/rpc/src/eth/api/optimism.rs | 31 +++++++++++++++ crates/rpc/rpc/src/eth/api/transactions.rs | 2 +- 4 files changed, 42 insertions(+), 46 deletions(-) create mode 100644 crates/rpc/rpc/src/eth/api/optimism.rs diff --git a/crates/rpc/rpc/src/eth/api/block.rs b/crates/rpc/rpc/src/eth/api/block.rs index dc8117f356ca..f99de8a78de5 100644 --- a/crates/rpc/rpc/src/eth/api/block.rs +++ b/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>, - /// The L1 fee for the block. - pub(crate) l1_fee: Option, - /// The L1 data gas for the block. - pub(crate) l1_data_gas: Option, -} - -#[cfg(feature = "optimism")] -impl OptimismTxMeta { - /// Creates a new [OptimismTxMeta]. - pub(crate) fn new( - l1_block_info: Option>, - l1_fee: Option, - l1_data_gas: Option, - ) -> Self { - Self { l1_block_info, l1_fee, l1_data_gas } - } -} - impl EthApi where Provider: @@ -150,7 +113,7 @@ where ) }) .collect::>>(); - return receipts.map(Some) + return receipts.map(Some); } Ok(None) @@ -167,7 +130,7 @@ where if block_id.is_pending() { // Pending block can be fetched directly without need for caching - return Ok(self.provider().pending_block()?.map(|block| block.body.len())) + return Ok(self.provider().pending_block()?.map(|block| block.body.len())); } let block_hash = match self.provider().block_hash_for_id(block_id)? { @@ -189,10 +152,10 @@ where // Pending block can be fetched directly without need for caching let maybe_pending = self.provider().pending_block()?; return if maybe_pending.is_some() { - return Ok(maybe_pending) + return Ok(maybe_pending); } else { self.local_pending_block().await - } + }; } let block_hash = match self.provider().block_hash_for_id(block_id)? { diff --git a/crates/rpc/rpc/src/eth/api/mod.rs b/crates/rpc/rpc/src/eth/api/mod.rs index 025cb5449ff6..cac5bf7328fb 100644 --- a/crates/rpc/rpc/src/eth/api/mod.rs +++ b/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; @@ -280,7 +282,7 @@ where pub(crate) async fn local_pending_block(&self) -> EthResult> { let pending = self.pending_block_env_and_cfg()?; if pending.origin.is_actual_pending() { - return Ok(pending.origin.into_actual_pending()) + return Ok(pending.origin.into_actual_pending()); } // no pending block from the CL yet, so we need to build it ourselves via txpool @@ -295,13 +297,13 @@ where pending.origin.header().hash == pending_block.block.parent_hash && now <= pending_block.expires_at { - return Ok(Some(pending_block.block.clone())) + return Ok(Some(pending_block.block.clone())); } } // if we're currently syncing, we're unable to build a pending block if this.network().is_syncing() { - return Ok(None) + return Ok(None); } // we rebuild the block @@ -309,7 +311,7 @@ where Ok(block) => block, Err(err) => { tracing::debug!(target: "rpc", "Failed to build pending block: {:?}", err); - return Ok(None) + return Ok(None); } }; diff --git a/crates/rpc/rpc/src/eth/api/optimism.rs b/crates/rpc/rpc/src/eth/api/optimism.rs new file mode 100644 index 000000000000..44f93c724328 --- /dev/null +++ b/crates/rpc/rpc/src/eth/api/optimism.rs @@ -0,0 +1,31 @@ +use reth_primitives::U256; +use revm::L1BlockInfo; +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. +#[derive(Debug, Default, Clone)] +pub(crate) struct OptimismTxMeta { + /// The L1 block info. + pub(crate) l1_block_info: Option>, + /// The L1 fee for the block. + pub(crate) l1_fee: Option, + /// The L1 data gas for the block. + pub(crate) l1_data_gas: Option, +} + +impl OptimismTxMeta { + /// Creates a new [OptimismTxMeta]. + pub(crate) fn new( + l1_block_info: Option>, + l1_fee: Option, + l1_data_gas: Option, + ) -> Self { + Self { l1_block_info, l1_fee, l1_data_gas } + } +} diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index fd0e7dfb6107..6a12d63c9812 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -42,7 +42,7 @@ 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")] From 63cd43d1e83856b1cf584ce4b033dde986b9cdc2 Mon Sep 17 00:00:00 2001 From: refcell Date: Fri, 3 Nov 2023 13:10:52 +0100 Subject: [PATCH 2/6] fix nits --- crates/rpc/rpc/src/eth/api/block.rs | 2 +- crates/rpc/rpc/src/eth/api/optimism.rs | 5 ++- crates/rpc/rpc/src/eth/api/transactions.rs | 38 +++++++++++++--------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/crates/rpc/rpc/src/eth/api/block.rs b/crates/rpc/rpc/src/eth/api/block.rs index f99de8a78de5..97e39c3b930c 100644 --- a/crates/rpc/rpc/src/eth/api/block.rs +++ b/crates/rpc/rpc/src/eth/api/block.rs @@ -81,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 diff --git a/crates/rpc/rpc/src/eth/api/optimism.rs b/crates/rpc/rpc/src/eth/api/optimism.rs index 44f93c724328..84881fccac73 100644 --- a/crates/rpc/rpc/src/eth/api/optimism.rs +++ b/crates/rpc/rpc/src/eth/api/optimism.rs @@ -1,6 +1,5 @@ use reth_primitives::U256; use revm::L1BlockInfo; -use std::rc::Rc; /// Optimism Transaction Metadata /// @@ -12,7 +11,7 @@ use std::rc::Rc; #[derive(Debug, Default, Clone)] pub(crate) struct OptimismTxMeta { /// The L1 block info. - pub(crate) l1_block_info: Option>, + pub(crate) l1_block_info: Option, /// The L1 fee for the block. pub(crate) l1_fee: Option, /// The L1 data gas for the block. @@ -22,7 +21,7 @@ pub(crate) struct OptimismTxMeta { impl OptimismTxMeta { /// Creates a new [OptimismTxMeta]. pub(crate) fn new( - l1_block_info: Option>, + l1_block_info: Option, l1_fee: Option, l1_data_gas: Option, ) -> Self { diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index 6a12d63c9812..74e7a3cccded 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -49,8 +49,6 @@ use reth_revm::optimism::RethL1BlockInfo; 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>>; @@ -902,7 +900,7 @@ where .block_by_number(meta.block_number)? .ok_or(EthApiError::UnknownBlockNumber)?; - 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( @@ -914,11 +912,15 @@ where ) } + /// 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>, + l1_block_info: Option, block_timestamp: u64, ) -> EthResult { if let Some(l1_block_info) = l1_block_info { @@ -928,27 +930,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( + 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( + ) { + 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 { From 0474763cf23442019dcf584e43e790423feb7372 Mon Sep 17 00:00:00 2001 From: refcell Date: Fri, 3 Nov 2023 13:39:50 +0100 Subject: [PATCH 3/6] joined future block and receipt fetching --- crates/rpc/rpc/src/eth/api/transactions.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index 74e7a3cccded..a25a549c9633 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -890,15 +890,14 @@ where meta: TransactionMeta, receipt: Receipt, ) -> EthResult { - // 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(); let optimism_tx_meta = self.build_op_tx_meta(&tx, l1_block_info, block.timestamp)?; @@ -907,7 +906,7 @@ where tx, meta, receipt, - &all_receipts, + &receipts, optimism_tx_meta, ) } From ac050a21a3845b18657ec5595248510c823d73c6 Mon Sep 17 00:00:00 2001 From: refcell Date: Fri, 3 Nov 2023 13:45:05 +0100 Subject: [PATCH 4/6] referenced chainspec instead of arcs --- crates/revm/src/optimism/mod.rs | 9 ++++----- crates/rpc/rpc/src/eth/api/transactions.rs | 4 ++-- crates/transaction-pool/src/validate/eth.rs | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/revm/src/optimism/mod.rs b/crates/revm/src/optimism/mod.rs index b2ae23e3db8c..715d43be7d2b 100644 --- a/crates/revm/src/optimism/mod.rs +++ b/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, + 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, + chain_spec: &ChainSpec, timestamp: u64, input: &Bytes, ) -> Result; @@ -110,7 +109,7 @@ pub trait RethL1BlockInfo { impl RethL1BlockInfo for L1BlockInfo { fn l1_tx_data_fee( &self, - chain_spec: Arc, + 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, + chain_spec: &ChainSpec, timestamp: u64, input: &Bytes, ) -> Result { diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index a25a549c9633..efe863488ffe 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -932,7 +932,7 @@ where let (l1_fee, l1_data_gas) = match (!tx.is_deposit()) .then(|| { let inner_l1_fee = match l1_block_info.l1_tx_data_fee( - self.inner.provider.chain_spec(), + &self.inner.provider.chain_spec(), block_timestamp, &envelope_buf, tx.is_deposit(), @@ -941,7 +941,7 @@ where Err(e) => return Err(e), }; let inner_l1_data_gas = match l1_block_info.l1_data_gas( - self.inner.provider.chain_spec(), + &self.inner.provider.chain_spec(), block_timestamp, &envelope_buf, ) { diff --git a/crates/transaction-pool/src/validate/eth.rs b/crates/transaction-pool/src/validate/eth.rs index 25507724601e..4d04ee739ec9 100644 --- a/crates/transaction-pool/src/validate/eth.rs +++ b/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(), From 018789f3e492f763ab5e481da33628293d7e1413 Mon Sep 17 00:00:00 2001 From: refcell Date: Fri, 3 Nov 2023 13:53:35 +0100 Subject: [PATCH 5/6] remove trailing semi-colons --- crates/rpc/rpc/src/eth/api/block.rs | 6 +++--- crates/rpc/rpc/src/eth/api/mod.rs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/rpc/rpc/src/eth/api/block.rs b/crates/rpc/rpc/src/eth/api/block.rs index 97e39c3b930c..d5e83999872a 100644 --- a/crates/rpc/rpc/src/eth/api/block.rs +++ b/crates/rpc/rpc/src/eth/api/block.rs @@ -113,7 +113,7 @@ where ) }) .collect::>>(); - return receipts.map(Some); + return receipts.map(Some) } Ok(None) @@ -152,10 +152,10 @@ where // Pending block can be fetched directly without need for caching let maybe_pending = self.provider().pending_block()?; return if maybe_pending.is_some() { - return Ok(maybe_pending); + return Ok(maybe_pending) } else { self.local_pending_block().await - }; + } } let block_hash = match self.provider().block_hash_for_id(block_id)? { diff --git a/crates/rpc/rpc/src/eth/api/mod.rs b/crates/rpc/rpc/src/eth/api/mod.rs index cac5bf7328fb..c081a9a49304 100644 --- a/crates/rpc/rpc/src/eth/api/mod.rs +++ b/crates/rpc/rpc/src/eth/api/mod.rs @@ -282,7 +282,7 @@ where pub(crate) async fn local_pending_block(&self) -> EthResult> { let pending = self.pending_block_env_and_cfg()?; if pending.origin.is_actual_pending() { - return Ok(pending.origin.into_actual_pending()); + return Ok(pending.origin.into_actual_pending()) } // no pending block from the CL yet, so we need to build it ourselves via txpool @@ -297,13 +297,13 @@ where pending.origin.header().hash == pending_block.block.parent_hash && now <= pending_block.expires_at { - return Ok(Some(pending_block.block.clone())); + return Ok(Some(pending_block.block.clone())) } } // if we're currently syncing, we're unable to build a pending block if this.network().is_syncing() { - return Ok(None); + return Ok(None) } // we rebuild the block @@ -311,7 +311,7 @@ where Ok(block) => block, Err(err) => { tracing::debug!(target: "rpc", "Failed to build pending block: {:?}", err); - return Ok(None); + return Ok(None) } }; From 9be321205bbbb613b5bc535f5359566ae57e658f Mon Sep 17 00:00:00 2001 From: refcell Date: Fri, 3 Nov 2023 13:54:50 +0100 Subject: [PATCH 6/6] missed one trailing semicolon --- crates/rpc/rpc/src/eth/api/block.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/rpc/src/eth/api/block.rs b/crates/rpc/rpc/src/eth/api/block.rs index d5e83999872a..3cd33be201f9 100644 --- a/crates/rpc/rpc/src/eth/api/block.rs +++ b/crates/rpc/rpc/src/eth/api/block.rs @@ -130,7 +130,7 @@ where if block_id.is_pending() { // Pending block can be fetched directly without need for caching - return Ok(self.provider().pending_block()?.map(|block| block.body.len())); + return Ok(self.provider().pending_block()?.map(|block| block.body.len())) } let block_hash = match self.provider().block_hash_for_id(block_id)? {