Merge pull request #3081 from ChenxingLi/fix_trace

Fix filter logic for new trace matching implementation

Cargo.toml

@@ -58,6 +58,7 @@ members = [
     "crates/rpc/rpc-eth-api",
     "crates/rpc/parity-trace-types",
     "crates/rpc/rpc-eth-impl",
+    "crates/rpc/rpc-common-impl",
     "crates/util/util-macros",
     "crates/rpc/rpc-utils",
     "crates/rpc/rpc-builder",
@@ -191,6 +192,7 @@ cfx-rpc = { path = "./crates/rpc/rpc-eth-impl" }
 cfx-rpc-utils = { path = "./crates/rpc/rpc-utils" }
 cfx-rpc-builder = { path = "./crates/rpc/rpc-builder" }
 cfx-rpc-cfx-impl = { path = "./crates/rpc/rpc-cfx-impl" }
+cfx-rpc-common-impl = { path = "./crates/rpc/rpc-common-impl" }
 cfx-rpc-middlewares = { path = "./crates/rpc/rpc-middlewares" }
 bounded-executor = { path = "./crates/pos/common/bounded-executor" }
 #diem-channel = { path = "./crates/pos/common/channel", package = "channel" }
@@ -384,6 +386,7 @@ winapi = "0.3.7"
 synstructure = "0.12"
 lru-cache = "0.1"
 lru_time_cache = "0.9.0"
+slice-group-by = "0.3.1"
 
 # num misc
 bigdecimal = "0.1.0"

crates/cfxcore/execute-helper/src/observer/exec_tracer/mod.rs

@@ -12,7 +12,6 @@ pub use cfx_parity_trace_types::{
         self, Action, ActionType, Call, CallResult, Create, CreateResult,
         InternalTransferAction, Outcome,
     },
-    conversion::{construct_parity_trace, TraceWithPosition},
     filter::{self, TraceFilter},
     trace_types::{
         self, BlockExecTraces, ExecTrace, LocalizedTrace, TransactionExecTraces,

crates/client/Cargo.toml

@@ -79,6 +79,8 @@ cfx-rpc = { workspace = true }
 cfx-rpc-utils = { workspace = true }
 cfx-rpc-builder = { workspace = true }
 jsonrpsee = { workspace = true }
+cfx-rpc-common-impl = { workspace = true }
+cfx-parity-trace-types = { workspace = true }
 
 [dev-dependencies]
 criterion = { workspace = true }

crates/client/src/rpc/impls/cfx/trace.rs

@@ -7,16 +7,15 @@ use crate::{
     rpc::{
         traits::trace::Trace,
         types::{
-            eth::LocalizedTrace as EthLocalizedTrace,
-            EpochNumber as RpcEpochNumber, LocalizedBlockTrace,
-            LocalizedTrace as RpcLocalizedTrace, LocalizedTrace,
+            EpochNumber as RpcEpochNumber, LocalizedBlockTrace, LocalizedTrace,
             TraceFilter as RpcTraceFilter,
         },
         CoreResult,
     },
 };
 use cfx_addr::Network;
 use cfx_execute_helper::exec_tracer::TraceFilter as PrimitiveTraceFilter;
+use cfx_parity_trace_types::LocalizedTrace as PrimitiveLocalizedTrace;
 use cfx_rpc_cfx_impl::TraceHandler as CfxTraceHandler;
 use cfx_rpc_cfx_types::trace::EpochTrace;
 use cfx_types::H256;
@@ -40,16 +39,10 @@ impl TraceHandler {
         self.inner.consensus_graph()
     }
 
-    pub(crate) fn filter_traces_impl(
+    pub(crate) fn filter_primitives_traces_impl(
         &self, filter: PrimitiveTraceFilter,
-    ) -> CoreResult<Option<Vec<RpcLocalizedTrace>>> {
-        self.inner.filter_traces_impl(filter)
-    }
-
-    pub fn to_eth_traces(
-        traces: Vec<LocalizedTrace>,
-    ) -> JsonRpcResult<Vec<EthLocalizedTrace>> {
-        CfxTraceHandler::to_eth_traces(traces)
+    ) -> CoreResult<Option<Vec<PrimitiveLocalizedTrace>>> {
+        self.inner.filter_primitives_traces_impl(filter)
     }
 }
 

crates/client/src/rpc/impls/eth/eth_trace.rs

@@ -1,16 +1,13 @@
 use crate::rpc::{
     impls::cfx::TraceHandler,
     traits::eth_space::trace::Trace as EthTrace,
-    types::{
-        eth::{
-            BlockNumber, LocalizedTrace as EthLocalizedTrace, Res as EthRes,
-            TraceFilter as EthTraceFilter,
-        },
-        Action as RpcAction,
+    types::eth::{
+        BlockNumber, LocalizedTrace as EthLocalizedTrace,
+        TraceFilter as EthTraceFilter,
     },
 };
-use cfx_execute_helper::exec_tracer::{
-    construct_parity_trace, TraceWithPosition,
+use cfx_rpc_common_impl::trace::{
+    into_eth_localized_traces, primitive_traces_to_eth_localized_traces,
 };
 use cfx_types::H256;
 use cfx_util_macros::unwrap_option_or_return_result_none as unwrap_or_return;
@@ -51,47 +48,23 @@ impl EthTrace for EthTraceHandler {
         let block_number = phantom_block.pivot_header.height();
         let block_hash = phantom_block.pivot_header.hash();
 
+        let network = self.trace_handler.inner.network;
+
         for (idx, tx_traces) in phantom_block.traces.into_iter().enumerate() {
             let tx_hash = phantom_block.transactions[idx].hash();
-
-            for TraceWithPosition {
-                action,
-                result,
-                child_count,
-                trace_path,
-            } in construct_parity_trace(&tx_traces).map_err(|e| {
+            let tx_eth_traces = into_eth_localized_traces(
+                &tx_traces.0,
+                block_number,
+                block_hash,
+                tx_hash,
+                idx,
+                network,
+            )
+            .map_err(|e| {
                 warn!("Internal error on trace reconstruction: {}", e);
                 JsonRpcError::internal_error()
-            })? {
-                let mut eth_trace = EthLocalizedTrace {
-                    action: RpcAction::try_from(
-                        action.action.clone(),
-                        self.trace_handler.inner.network,
-                    )
-                    .map_err(|_| JsonRpcError::internal_error())?
-                    .try_into()
-                    .map_err(|_| JsonRpcError::internal_error())?,
-                    result: EthRes::None,
-                    trace_address: trace_path,
-                    subtraces: child_count,
-                    transaction_position: Some(idx),
-                    transaction_hash: Some(tx_hash),
-                    block_number,
-                    block_hash,
-                    // action and its result should have the same `valid`.
-                    valid: action.valid,
-                };
-
-                eth_trace.set_result(
-                    RpcAction::try_from(
-                        result.action.clone(),
-                        self.trace_handler.inner.network,
-                    )
-                    .map_err(|_| JsonRpcError::internal_error())?,
-                )?;
-
-                eth_traces.push(eth_trace);
-            }
+            })?;
+            eth_traces.extend(tx_eth_traces);
         }
 
         Ok(Some(eth_traces))
@@ -100,17 +73,24 @@ impl EthTrace for EthTraceHandler {
     fn filter_traces(
         &self, filter: EthTraceFilter,
     ) -> JsonRpcResult<Option<Vec<EthLocalizedTrace>>> {
-        // TODO(lpl): Use `TransactionExecTraces::filter_trace_pairs` to avoid
-        // pairing twice.
         let primitive_filter = filter.into_primitive()?;
 
-        let traces =
-            match self.trace_handler.filter_traces_impl(primitive_filter)? {
-                None => return Ok(None),
-                Some(traces) => traces,
-            };
+        let Some(primitive_traces) = self
+            .trace_handler
+            .filter_primitives_traces_impl(primitive_filter)?
+        else {
+            return Ok(None);
+        };
 
-        Ok(Some(TraceHandler::to_eth_traces(traces)?))
+        let traces = primitive_traces_to_eth_localized_traces(
+            &primitive_traces,
+            self.trace_handler.inner.network,
+        )
+        .map_err(|e| {
+            warn!("Internal error on trace reconstruction: {}", e);
+            JsonRpcError::internal_error()
+        })?;
+        Ok(Some(traces))
     }
 
     fn transaction_traces(
@@ -154,51 +134,21 @@ impl EthTrace for EthTraceHandler {
         let tx = &phantom_block.transactions[id];
         let tx_traces = phantom_block.traces[id].clone();
 
-        // convert traces
-        let trace_pairs = construct_parity_trace(&tx_traces).map_err(|e| {
+        let network = self.trace_handler.inner.network;
+
+        let eth_traces = into_eth_localized_traces(
+            &tx_traces.0,
+            epoch_num,
+            phantom_block.pivot_header.hash(),
+            tx.hash,
+            id,
+            network,
+        )
+        .map_err(|e| {
             warn!("Internal error on trace reconstruction: {}", e);
             JsonRpcError::internal_error()
         })?;
 
-        let mut eth_traces = Vec::new();
-
-        for TraceWithPosition {
-            action,
-            result,
-            child_count,
-            trace_path,
-        } in trace_pairs
-        {
-            let mut eth_trace = EthLocalizedTrace {
-                action: RpcAction::try_from(
-                    action.action.clone(),
-                    self.trace_handler.inner.network,
-                )
-                .map_err(|_| JsonRpcError::internal_error())?
-                .try_into()
-                .map_err(|_| JsonRpcError::internal_error())?,
-                result: EthRes::None,
-                trace_address: trace_path,
-                subtraces: child_count,
-                transaction_position: Some(id),
-                transaction_hash: Some(tx.hash()),
-                block_number: epoch_num,
-                block_hash: phantom_block.pivot_header.hash(),
-                // action and its result should have the same `valid`.
-                valid: action.valid,
-            };
-
-            eth_trace.set_result(
-                RpcAction::try_from(
-                    result.action.clone(),
-                    self.trace_handler.inner.network,
-                )
-                .map_err(|_| JsonRpcError::internal_error())?,
-            )?;
-
-            eth_traces.push(eth_trace);
-        }
-
         Ok(Some(eth_traces))
     }
 }

crates/rpc/parity-trace-types/src/lib.rs

@@ -1,6 +1,5 @@
 pub mod action_types;
 pub mod address_pocket;
-pub mod conversion;
 pub mod filter;
 pub mod trace_types;
 
@@ -9,6 +8,5 @@ mod tests;
 
 pub use action_types::*;
 pub use address_pocket::AddressPocket;
-pub use conversion::{construct_parity_trace, TraceWithPosition};
 pub use filter::*;
 pub use trace_types::*;

crates/rpc/rpc-cfx-impl/Cargo.toml

@@ -13,3 +13,5 @@ cfxcore = { workspace = true }
 primitives = { workspace = true }
 cfx-rpc-cfx-types = { workspace = true }
 cfx-util-macros = { workspace = true }
+cfx-parity-trace-types = { workspace = true }
+cfx-rpc-common-impl = { workspace = true }