perf: only record return value if required (#3040)

Co-authored-by: Bjerg <onbjerg@users.noreply.github.com>

crates/revm/revm-inspectors/src/tracing/builder/geth.rs

@@ -4,7 +4,7 @@ use crate::tracing::{
     types::{CallTraceNode, CallTraceStepStackItem},
     TracingInspectorConfig,
 };
-use reth_primitives::{Address, H256};
+use reth_primitives::{Address, Bytes, H256};
 use reth_rpc_types::trace::geth::*;
 use std::collections::{BTreeMap, HashMap, VecDeque};
 
@@ -58,7 +58,7 @@ impl GethTraceBuilder {
             }
 
             if opts.is_return_data_enabled() {
-                log.return_data = trace_node.trace.last_call_return_value.clone().map(Into::into);
+                log.return_data = Some(trace_node.trace.output.clone().into());
             }
 
             // Add step to geth trace
@@ -74,10 +74,13 @@ impl GethTraceBuilder {
     }
 
     /// Generate a geth-style trace e.g. for `debug_traceTransaction`
+    ///
+    /// This expects the gas used and return value for the
+    /// [ExecutionResult](revm::primitives::ExecutionResult) of the executed transaction.
     pub fn geth_traces(
         &self,
-        // TODO(mattsse): This should be the total gas used, or gas used by last CallTrace?
         receipt_gas_used: u64,
+        return_value: Bytes,
         opts: GethDefaultTracingOptions,
     ) -> DefaultFrame {
         if self.nodes.is_empty() {
@@ -95,7 +98,7 @@ impl GethTraceBuilder {
             // If the top-level trace succeeded, then it was a success
             failed: !main_trace.success,
             gas: receipt_gas_used,
-            return_value: main_trace.output.clone().into(),
+            return_value,
             struct_logs,
         }
     }

crates/revm/revm-inspectors/src/tracing/config.rs

@@ -16,6 +16,8 @@ pub struct TracingInspectorConfig {
     pub record_state_diff: bool,
     /// Whether to ignore precompile calls.
     pub exclude_precompile_calls: bool,
+    /// Whether to record individual return data
+    pub record_call_return_data: bool,
     /// Whether to record logs
     pub record_logs: bool,
 }
@@ -29,6 +31,7 @@ impl TracingInspectorConfig {
             record_stack_snapshots: true,
             record_state_diff: false,
             exclude_precompile_calls: false,
+            record_call_return_data: false,
             record_logs: true,
         }
     }
@@ -43,6 +46,7 @@ impl TracingInspectorConfig {
             record_stack_snapshots: false,
             record_state_diff: false,
             exclude_precompile_calls: true,
+            record_call_return_data: false,
             record_logs: false,
         }
     }
@@ -57,6 +61,7 @@ impl TracingInspectorConfig {
             record_stack_snapshots: true,
             record_state_diff: true,
             exclude_precompile_calls: false,
+            record_call_return_data: false,
             record_logs: false,
         }
     }

crates/revm/revm-inspectors/src/tracing/mod.rs

@@ -177,7 +177,6 @@ impl TracingInspector {
                 value,
                 status: InstructionResult::Continue,
                 caller,
-                last_call_return_value: self.last_call_return_data.clone(),
                 maybe_precompile,
                 gas_limit,
                 ..Default::default()
@@ -214,6 +213,7 @@ impl TracingInspector {
         trace.status = status;
         trace.success = matches!(status, return_ok!());
         trace.output = output.clone();
+
         self.last_call_return_data = Some(output);
 
         if let Some(address) = created_address {

crates/revm/revm-inspectors/src/tracing/types.rs

@@ -142,8 +142,6 @@ pub(crate) struct CallTrace {
     /// The return data of the call if this was not a contract creation, otherwise it is the
     /// runtime bytecode of the created contract
     pub(crate) output: Bytes,
-    /// The return data of the last call, if any
-    pub(crate) last_call_return_value: Option<Bytes>,
     /// The gas cost of the call
     pub(crate) gas_used: u64,
     /// The gas limit of the call
@@ -181,7 +179,6 @@ impl Default for CallTrace {
             data: Default::default(),
             maybe_precompile: None,
             output: Default::default(),
-            last_call_return_value: None,
             gas_used: Default::default(),
             gas_limit: Default::default(),
             status: InstructionResult::Continue,

crates/rpc/rpc/src/debug.rs

@@ -2,7 +2,8 @@ use crate::{
     eth::{
         error::{EthApiError, EthResult},
         revm_utils::{
-            clone_into_empty_db, inspect, prepare_call_env, replay_transactions_until, EvmOverrides,
+            clone_into_empty_db, inspect, prepare_call_env, replay_transactions_until,
+            result_output, EvmOverrides,
         },
         EthTransactions, TransactionSource,
     },
@@ -346,8 +347,8 @@ where
         let (res, _) =
             self.inner.eth_api.inspect_call_at(call, at, overrides, &mut inspector).await?;
         let gas_used = res.result.gas_used();
-
+        let return_value = result_output(&res.result).unwrap_or_default().into();
-        let frame = inspector.into_geth_builder().geth_traces(gas_used, config);
+        let frame = inspector.into_geth_builder().geth_traces(gas_used, return_value, config);
 
         Ok(frame.into())
     }
@@ -424,8 +425,8 @@ where
 
         let (res, _) = inspect(db, env, &mut inspector)?;
         let gas_used = res.result.gas_used();
-
+        let return_value = result_output(&res.result).unwrap_or_default().into();
-        let frame = inspector.into_geth_builder().geth_traces(gas_used, config);
+        let frame = inspector.into_geth_builder().geth_traces(gas_used, return_value, config);
 
         Ok((frame.into(), res.state))
     }

crates/rpc/rpc/src/eth/revm_utils.rs

@@ -18,7 +18,7 @@ use revm::{
 };
 use revm_primitives::{
     db::{DatabaseCommit, DatabaseRef},
-    Bytecode,
+    Bytecode, ExecutionResult,
 };
 use tracing::trace;
 
@@ -524,6 +524,18 @@ where
     }
 }
 
+/// Helper to get the output data from a result
+///
+/// TODO: Can be phased out when <https://github.com/bluealloy/revm/pull/509> is released
+#[inline]
+pub(crate) fn result_output(res: &ExecutionResult) -> Option<bytes::Bytes> {
+    match res {
+        ExecutionResult::Success { output, .. } => Some(output.clone().into_data()),
+        ExecutionResult::Revert { output, .. } => Some(output.clone()),
+        _ => None,
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;