From c825b26f2e75f09dcd1dd6f4843405c0b804488d Mon Sep 17 00:00:00 2001
From: Matthias Seitz <matthias.seitz@outlook.de>
Date: Tue, 21 Nov 2023 20:58:36 +0100
Subject: [PATCH] fix: dont use Stack::default (#5521)

---
 .../revm-inspectors/src/tracing/builder/parity.rs    | 10 ++++------
 crates/revm/revm-inspectors/src/tracing/mod.rs       | 12 +++++++-----
 crates/revm/revm-inspectors/src/tracing/types.rs     |  9 ++-------
 crates/rpc/rpc/src/trace.rs                          |  1 +
 4 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs
index d17ecc5311..9d984bcfb6 100644
--- a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs
+++ b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs
@@ -454,8 +454,10 @@ impl ParityTraceBuilder {
             };
             let mut push_stack = step.push_stack.clone().unwrap_or_default();
             for idx in (0..show_stack).rev() {
-                if step.stack.len() > idx {
-                    push_stack.push(step.stack.peek(idx).unwrap_or_default())
+                if let Some(stack) = step.stack.as_ref() {
+                    if stack.len() > idx {
+                        push_stack.push(stack.peek(idx).unwrap_or_default())
+                    }
                 }
             }
             push_stack
@@ -487,10 +489,6 @@ impl ParityTraceBuilder {
 }
 
 /// An iterator for [TransactionTrace]s
-///
-/// This iterator handles additional selfdestruct actions based on the last emitted
-/// [TransactionTrace], since selfdestructs are not recorded as individual call traces but are
-/// derived from recorded call
 struct TransactionTraceIter<Iter> {
     iter: Iter,
     next_selfdestruct: Option<TransactionTrace>,
diff --git a/crates/revm/revm-inspectors/src/tracing/mod.rs b/crates/revm/revm-inspectors/src/tracing/mod.rs
index a947c42538..dbd7101ed8 100644
--- a/crates/revm/revm-inspectors/src/tracing/mod.rs
+++ b/crates/revm/revm-inspectors/src/tracing/mod.rs
@@ -282,8 +282,7 @@ impl TracingInspector {
             .record_memory_snapshots
             .then(|| RecordedMemory::new(interp.shared_memory.context_memory().to_vec()))
             .unwrap_or_default();
-        let stack =
-            self.config.record_stack_snapshots.then(|| interp.stack.clone()).unwrap_or_default();
+        let stack = self.config.record_stack_snapshots.then(|| interp.stack.clone());
 
         let op = OpCode::new(interp.current_opcode())
             .or_else(|| {
@@ -326,9 +325,12 @@ impl TracingInspector {
             self.step_stack.pop().expect("can't fill step without starting a step first");
         let step = &mut self.traces.arena[trace_idx].trace.steps[step_idx];
 
-        if interp.stack.len() > step.stack.len() {
-            // if the stack grew, we need to record the new values
-            step.push_stack = Some(interp.stack.data()[step.stack.len()..].to_vec());
+        if let Some(stack) = step.stack.as_ref() {
+            // only check stack changes if record stack snapshots is enabled: if stack is Some
+            if interp.stack.len() > stack.len() {
+                // if the stack grew, we need to record the new values
+                step.push_stack = Some(interp.stack.data()[stack.len()..].to_vec());
+            }
         }
 
         if self.config.record_memory_snapshots {
diff --git a/crates/revm/revm-inspectors/src/tracing/types.rs b/crates/revm/revm-inspectors/src/tracing/types.rs
index ec0e7269a0..ecc2e261b2 100644
--- a/crates/revm/revm-inspectors/src/tracing/types.rs
+++ b/crates/revm/revm-inspectors/src/tracing/types.rs
@@ -429,8 +429,6 @@ impl CallTraceNode {
     }
 
     /// Converts this call trace into an _empty_ geth [CallFrame]
-    ///
-    /// Caution: this does not include any of the child calls
     pub(crate) fn geth_empty_call_frame(&self, include_logs: bool) -> CallFrame {
         let mut call_frame = CallFrame {
             typ: self.trace.kind.to_string(),
@@ -485,9 +483,6 @@ pub(crate) struct CallTraceStepStackItem<'a> {
 }
 
 /// Ordering enum for calls and logs
-///
-/// i.e. if Call 0 occurs before Log 0, it will be pushed into the `CallTraceNode`'s ordering before
-/// the log.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) enum LogCallOrder {
     Log(usize),
@@ -516,7 +511,7 @@ pub(crate) struct CallTraceStep {
     /// Current contract address
     pub(crate) contract: Address,
     /// Stack before step execution
-    pub(crate) stack: Stack,
+    pub(crate) stack: Option<Stack>,
     /// The new stack items placed by this step if any
     pub(crate) push_stack: Option<Vec<U256>>,
     /// All allocated memory in a step
@@ -568,7 +563,7 @@ impl CallTraceStep {
         };
 
         if opts.is_stack_enabled() {
-            log.stack = Some(self.stack.data().clone());
+            log.stack = self.stack.as_ref().map(|stack| stack.data().clone());
         }
 
         if opts.is_memory_enabled() {
diff --git a/crates/rpc/rpc/src/trace.rs b/crates/rpc/rpc/src/trace.rs
index 5399ec4907..7c21930868 100644
--- a/crates/rpc/rpc/src/trace.rs
+++ b/crates/rpc/rpc/src/trace.rs
@@ -558,6 +558,7 @@ fn tracing_config(trace_types: &HashSet<TraceType>) -> TracingInspectorConfig {
     let needs_vm_trace = trace_types.contains(&TraceType::VmTrace);
     TracingInspectorConfig::default_parity()
         .set_steps(needs_vm_trace)
+        .set_stack_snapshots(needs_vm_trace)
         .set_memory_snapshots(needs_vm_trace)
 }