Fix debug trace response (#1629)

bin/citrea/tests/evm/tracing.rs

@@ -5,7 +5,7 @@ use alloy_primitives::Address;
 use alloy_rpc_types_trace::geth::GethTrace::{self, CallTracer, FourByteTracer};
 use alloy_rpc_types_trace::geth::{
     CallConfig, CallFrame, FourByteFrame, GethDebugBuiltInTracerType, GethDebugTracerType,
-    GethDebugTracingOptions,
+    GethDebugTracingOptions, TraceResult,
 };
 use citrea_common::SequencerConfig;
 use citrea_evm::smart_contracts::{CallerContract, SimpleStorageContract};
@@ -222,7 +222,14 @@ async fn tracing_tests() -> Result<(), Box<dyn std::error::Error>> {
                 GethDebugTracerType::BuiltInTracer(GethDebugBuiltInTracerType::CallTracer),
             )),
         )
-        .await;
+        .await
+        .into_iter()
+        .map(|trace| match trace {
+            TraceResult::Success { result, .. } => Ok(result),
+            _ => anyhow::bail!("Unexpected trace result"),
+        })
+        .collect::<Result<Vec<_>, _>>()?;
+
     assert_eq!(traces.len(), 2);
     assert_eq!(traces[1], CallTracer(expected_send_eth_trace.clone()));
     assert_eq!(traces[0], CallTracer(expected_call_get_trace.clone()));
@@ -240,7 +247,13 @@ async fn tracing_tests() -> Result<(), Box<dyn std::error::Error>> {
                 GethDebugTracerType::BuiltInTracer(GethDebugBuiltInTracerType::CallTracer),
             )),
         )
-        .await;
+        .await
+        .into_iter()
+        .map(|trace| match trace {
+            TraceResult::Success { result, .. } => Ok(result),
+            _ => anyhow::bail!("Unexpected trace result"),
+        })
+        .collect::<Result<Vec<_>, _>>()?;
 
     assert_eq!(traces.len(), 2);
     assert_eq!(traces[1], CallTracer(expected_send_eth_trace.clone()));
@@ -253,7 +266,13 @@ async fn tracing_tests() -> Result<(), Box<dyn std::error::Error>> {
                 GethDebugTracerType::BuiltInTracer(GethDebugBuiltInTracerType::FourByteTracer),
             )),
         )
-        .await;
+        .await
+        .into_iter()
+        .map(|trace| match trace {
+            TraceResult::Success { result, .. } => Ok(result),
+            _ => anyhow::bail!("Unexpected trace result"),
+        })
+        .collect::<Result<Vec<_>, _>>()?;
 
     let expected_call_get_4byte_trace =
         serde_json::from_value::<FourByteFrame>(json![{"35c152bd-32": 1, "6d4ce63c-0": 1}])
@@ -285,7 +304,13 @@ async fn tracing_tests() -> Result<(), Box<dyn std::error::Error>> {
                     }),
             ),
         )
-        .await;
+        .await
+        .into_iter()
+        .map(|trace| match trace {
+            TraceResult::Success { result, .. } => Ok(result),
+            _ => anyhow::bail!("Unexpected trace result"),
+        })
+        .collect::<Result<Vec<_>, _>>()?;
 
     let expected_top_call_only_call_get_trace = serde_json::from_value::<CallFrame>(
         json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x1886","gasUsed":"0x6b64","to":"0xe7f1725e7734ce288f8367e1bb143e90bb3f0512",
@@ -313,7 +338,14 @@ async fn tracing_tests() -> Result<(), Box<dyn std::error::Error>> {
                 GethDebugTracerType::BuiltInTracer(GethDebugBuiltInTracerType::CallTracer),
             )),
         )
-        .await;
+        .await
+        .into_iter()
+        .map(|trace| match trace {
+            TraceResult::Success { result, .. } => Ok(result),
+            _ => anyhow::bail!("Unexpected trace result"),
+        })
+        .collect::<Result<Vec<_>, _>>()?;
+
     assert_eq!(traces.len(), 8);
     assert_eq!(traces[5], CallTracer(reth_json));
     assert_eq!(traces[6], CallTracer(expected_call_get_trace));

bin/citrea/tests/test_client/mod.rs

@@ -11,7 +11,7 @@ use alloy::transports::http::{Http, HyperClient};
 use alloy_primitives::{Address, Bytes, TxHash, TxKind, B256, U256, U64};
 // use reth_rpc_types::TransactionReceipt;
 use alloy_rpc_types::AnyNetworkBlock;
-use alloy_rpc_types_trace::geth::{GethDebugTracingOptions, GethTrace};
+use alloy_rpc_types_trace::geth::{GethDebugTracingOptions, GethTrace, TraceResult};
 use citrea_batch_prover::GroupCommitments;
 use citrea_evm::{Filter, LogResponse};
 use ethereum_rpc::SyncStatus;
@@ -601,7 +601,7 @@ impl TestClient {
         &self,
         block_number: BlockNumberOrTag,
         opts: Option<GethDebugTracingOptions>,
-    ) -> Vec<GethTrace> {
+    ) -> Vec<TraceResult> {
         self.http_client
             .request("debug_traceBlockByNumber", rpc_params![block_number, opts])
             .await
@@ -612,7 +612,7 @@ impl TestClient {
         &self,
         block_hash: B256,
         opts: Option<GethDebugTracingOptions>,
-    ) -> Vec<GethTrace> {
+    ) -> Vec<TraceResult> {
         self.http_client
             .request("debug_traceBlockByHash", rpc_params![block_hash, opts])
             .await
@@ -624,7 +624,7 @@ impl TestClient {
         start_block: BlockNumberOrTag,
         end_block: BlockNumberOrTag,
         opts: Option<GethDebugTracingOptions>,
-    ) -> Vec<GethTrace> {
+    ) -> Vec<TraceResult> {
         let mut subscription = self
             .ws_client
             .subscribe(
@@ -643,7 +643,7 @@ impl TestClient {
             BlockNumberOrTag::Latest => self.eth_block_number().await,
             _ => panic!("Only number and latest"),
         };
-        let mut traces: Vec<Vec<GethTrace>> = vec![];
+        let mut traces: Vec<Vec<TraceResult>> = vec![];
         for _ in start_block..end_block {
             let block_traces = subscription.next().await.unwrap().unwrap();
             traces.push(block_traces);

crates/ethereum-rpc/src/ethereum.rs

@@ -1,7 +1,7 @@
 use std::sync::{Arc, Mutex};
 
 use alloy_primitives::U256;
-use alloy_rpc_types_trace::geth::GethTrace;
+use alloy_rpc_types_trace::geth::TraceResult;
 use citrea_evm::Evm;
 use jsonrpsee::http_client::HttpClient;
 use rustc_version_runtime::version;
@@ -34,7 +34,7 @@ pub struct Ethereum<C: sov_modules_api::Context, Da: DaService> {
     pub(crate) ledger_db: LedgerDB,
     pub(crate) sequencer_client: Option<HttpClient>,
     pub(crate) web3_client_version: String,
-    pub(crate) trace_cache: Mutex<LruMap<u64, Vec<GethTrace>, ByLength>>,
+    pub(crate) trace_cache: Mutex<LruMap<u64, Vec<TraceResult>, ByLength>>,
     pub(crate) subscription_manager: Option<SubscriptionManager>,
 }
 

crates/ethereum-rpc/src/lib.rs

@@ -8,7 +8,7 @@ use std::sync::Arc;
 use alloy_network::AnyNetwork;
 use alloy_primitives::{keccak256, Bytes, B256, U256};
 use alloy_rpc_types::{FeeHistory, Index};
-use alloy_rpc_types_trace::geth::{GethDebugTracingOptions, GethTrace};
+use alloy_rpc_types_trace::geth::{GethDebugTracingOptions, GethTrace, TraceResult};
 use citrea_evm::{Evm, Filter};
 use citrea_sequencer::SequencerRpcClient;
 pub use ethereum::{EthRpcConfig, Ethereum};
@@ -94,7 +94,7 @@ pub trait EthereumRpc {
         &self,
         block_hash: B256,
         opts: Option<GethDebugTracingOptions>,
-    ) -> RpcResult<Vec<GethTrace>>;
+    ) -> RpcResult<Vec<TraceResult>>;
 
     /// Returns traces for a block by number.
     #[method(name = "debug_traceBlockByNumber")]
@@ -103,7 +103,7 @@ pub trait EthereumRpc {
         &self,
         block_number: BlockNumberOrTag,
         opts: Option<GethDebugTracingOptions>,
-    ) -> RpcResult<Vec<GethTrace>>;
+    ) -> RpcResult<Vec<TraceResult>>;
 
     /// Returns trace for a transaction.
     #[method(name = "debug_traceTransaction")]
@@ -237,7 +237,7 @@ where
         &self,
         block_hash: B256,
         opts: Option<GethDebugTracingOptions>,
-    ) -> RpcResult<Vec<GethTrace>> {
+    ) -> RpcResult<Vec<TraceResult>> {
         let evm = Evm::<C>::default();
         let mut working_set = WorkingSet::new(self.ethereum.storage.clone());
 
@@ -263,7 +263,7 @@ where
         &self,
         block_number: BlockNumberOrTag,
         opts: Option<GethDebugTracingOptions>,
-    ) -> RpcResult<Vec<GethTrace>> {
+    ) -> RpcResult<Vec<TraceResult>> {
         let mut working_set = WorkingSet::new(self.ethereum.storage.clone());
         let evm = Evm::<C>::default();
         let latest_block_number: u64 = evm.block_number(&mut working_set)?.saturating_to();
@@ -324,7 +324,13 @@ where
         )
         .map_err(to_eth_rpc_error)?;
 
-        Ok(traces[0].clone())
+        match &traces[0] {
+            TraceResult::Success { result, .. } => Ok(result.clone()),
+            // this should never happen since we propagate any tracing error
+            TraceResult::Error { error, tx_hash: _ } => {
+                Err(EthApiError::EvmCustom(error.clone()).into())
+            }
+        }
     }
 
     fn txpool_content(&self) -> RpcResult<Value> {

crates/ethereum-rpc/src/trace.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use alloy_rpc_types_trace::geth::{
     CallConfig, CallFrame, FourByteFrame, GethDebugBuiltInTracerType, GethDebugTracerConfig,
-    GethDebugTracerType, GethDebugTracingOptions, GethTrace, NoopFrame,
+    GethDebugTracerType, GethDebugTracingOptions, GethTrace, NoopFrame, TraceResult,
 };
 use citrea_evm::Evm;
 use jsonrpsee::types::ErrorObjectOwned;
@@ -118,7 +118,7 @@ pub fn debug_trace_by_block_number<C: sov_modules_api::Context, Da: DaService>(
     evm: &Evm<C>,
     working_set: &mut WorkingSet<C::Storage>,
     opts: Option<GethDebugTracingOptions>,
-) -> Result<Vec<GethTrace>, ErrorObjectOwned> {
+) -> Result<Vec<TraceResult>, ErrorObjectOwned> {
     // If opts is None or if opts.tracer is None, then do not check cache or insert cache, just perform the operation
     if opts.as_ref().map_or(true, |o| o.tracer.is_none()) {
         let traces =
@@ -187,10 +187,10 @@ fn remove_logs_from_call_frame(call_frame: &mut Vec<CallFrame>) {
 }
 
 fn get_traces_with_requested_tracer_and_config(
-    traces: Vec<GethTrace>,
+    traces: Vec<TraceResult>,
     tracer: GethDebugTracerType,
     tracer_config: GethDebugTracerConfig,
-) -> Result<Vec<GethTrace>, EthApiError> {
+) -> Result<Vec<TraceResult>, EthApiError> {
     // This can be only CallConfig or PreStateConfig if it is not CallConfig return Error for now
 
     let mut new_traces = vec![];
@@ -215,10 +215,17 @@ fn get_traces_with_requested_tracer_and_config(
                         }
                         _ => {
                             traces.into_iter().for_each(|trace| {
-                                if let GethTrace::CallTracer(call_frame) = trace {
+                                if let TraceResult::Success {
+                                    result: GethTrace::CallTracer(call_frame),
+                                    tx_hash,
+                                } = trace
+                                {
                                     let new_call_frame =
                                         apply_call_config(call_frame.clone(), call_config);
-                                    new_traces.push(GethTrace::CallTracer(new_call_frame));
+                                    new_traces.push(TraceResult::new_success(
+                                        GethTrace::CallTracer(new_call_frame),
+                                        tx_hash,
+                                    ));
                                 }
                             });
                         }
@@ -227,17 +234,25 @@ fn get_traces_with_requested_tracer_and_config(
                 }
                 GethDebugBuiltInTracerType::FourByteTracer => {
                     traces.into_iter().for_each(|trace| {
-                        if let GethTrace::CallTracer(call_frame) = trace {
+                        if let TraceResult::Success {
+                            result: GethTrace::CallTracer(call_frame),
+                            tx_hash,
+                        } = trace
+                        {
                             let four_byte_frame =
                                 convert_call_trace_into_4byte_frame(vec![call_frame]);
-                            new_traces.push(GethTrace::FourByteTracer(four_byte_frame));
+                            new_traces.push(TraceResult::new_success(
+                                GethTrace::FourByteTracer(four_byte_frame),
+                                tx_hash,
+                            ));
                         }
                     });
                     Ok(new_traces)
                 }
-                GethDebugBuiltInTracerType::NoopTracer => {
-                    Ok(vec![GethTrace::NoopTracer(NoopFrame::default())])
-                }
+                GethDebugBuiltInTracerType::NoopTracer => Ok(vec![TraceResult::new_success(
+                    GethTrace::NoopTracer(NoopFrame::default()),
+                    None,
+                )]),
                 _ => Err(EthApiError::Unsupported("This tracer is not supported")),
             }
         }

crates/evm/src/query.rs

@@ -14,7 +14,7 @@ use alloy_rpc_types::{
 };
 use alloy_rpc_types_eth::transaction::TransactionRequest;
 use alloy_rpc_types_eth::Block as AlloyRpcBlock;
-use alloy_rpc_types_trace::geth::{GethDebugTracingOptions, GethTrace};
+use alloy_rpc_types_trace::geth::{GethDebugTracingOptions, TraceResult};
 use alloy_serde::OtherFields;
 use citrea_primitives::basefee::calculate_next_block_base_fee;
 use citrea_primitives::forks::FORKS;
@@ -1207,7 +1207,7 @@ impl<C: sov_modules_api::Context> Evm<C> {
         opts: Option<GethDebugTracingOptions>,
         stop_at: Option<usize>,
         working_set: &mut WorkingSet<C::Storage>,
-    ) -> RpcResult<Vec<GethTrace>> {
+    ) -> RpcResult<Vec<TraceResult>> {
         let sealed_block = self
             .get_sealed_block_by_number(Some(BlockNumberOrTag::Number(block_number)), working_set)?
             .ok_or_else(|| EthApiError::HeaderNotFound(block_number.into()))?;
@@ -1260,7 +1260,7 @@ impl<C: sov_modules_api::Context> Evm<C> {
                 &mut evm_db,
                 l1_fee_rate,
             )?;
-            traces.push(trace);
+            traces.push(TraceResult::new_success(trace, Some(tx.hash())));
 
             if limit == index {
                 break;