From 5c0ec6feebc824cedfdad400d651ea2775593acd Mon Sep 17 00:00:00 2001
From: Matthias Seitz <matthias.seitz@outlook.de>
Date: Thu, 10 Aug 2023 20:01:23 +0200
Subject: [PATCH] fix: exclude single stop vm trace instruction (#4149)

---
 .../src/tracing/builder/parity.rs             | 46 ++++++++++++++-----
 .../revm/revm-inspectors/src/tracing/types.rs |  6 +++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs
index 073e1707f73f..38f81e17fd83 100644
--- a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs
+++ b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs
@@ -273,6 +273,18 @@ impl ParityTraceBuilder {
         self.into_transaction_traces_iter().collect()
     }
 
+    /// Returns the last recorded step
+    #[inline]
+    fn last_step(&self) -> Option<&CallTraceStep> {
+        self.nodes.last().and_then(|node| node.trace.steps.last())
+    }
+
+    /// Returns true if the last recorded step is a STOP
+    #[inline]
+    fn is_last_step_stop_op(&self) -> bool {
+        self.last_step().map(|step| step.is_stop()).unwrap_or(false)
+    }
+
     /// Creates a VM trace by walking over `CallTraceNode`s
     ///
     /// does not have the code fields filled in
@@ -283,18 +295,18 @@ impl ParityTraceBuilder {
         }
     }
 
-    /// returns a VM trace without the code filled in
+    /// Returns a VM trace without the code filled in
     ///
-    /// iteratively creaters a VM trace by traversing an arena
+    /// Iteratively creates a VM trace by traversing the recorded nodes in the arena
     fn make_vm_trace(&self, start: &CallTraceNode) -> VmTrace {
-        let mut child_idx_stack: Vec<usize> = Vec::with_capacity(self.nodes.len());
-        let mut sub_stack: VecDeque<Option<VmTrace>> = VecDeque::with_capacity(self.nodes.len());
+        let mut child_idx_stack = Vec::with_capacity(self.nodes.len());
+        let mut sub_stack = VecDeque::with_capacity(self.nodes.len());
 
         let mut current = start;
         let mut child_idx: usize = 0;
 
         // finds the deepest nested calls of each call frame and fills them up bottom to top
-        let instructions = loop {
+        let instructions = 'outer: loop {
             match current.children.get(child_idx) {
                 Some(child) => {
                     child_idx_stack.push(child_idx + 1);
@@ -303,17 +315,23 @@ impl ParityTraceBuilder {
                     current = self.nodes.get(*child).expect("there should be a child");
                 }
                 None => {
-                    let mut instructions: Vec<VmInstruction> =
-                        Vec::with_capacity(current.trace.steps.len());
+                    let mut instructions = Vec::with_capacity(current.trace.steps.len());
 
                     for step in &current.trace.steps {
-                        let maybe_sub = if step.is_calllike_op() {
-                            sub_stack.pop_front().expect("there should be a sub trace")
+                        let maybe_sub_call = if step.is_calllike_op() {
+                            sub_stack.pop_front().flatten()
                         } else {
                             None
                         };
 
-                        instructions.push(self.make_instruction(step, maybe_sub));
+                        if step.is_stop() && instructions.is_empty() && self.is_last_step_stop_op()
+                        {
+                            // This is a special case where there's a single STOP which is
+                            // "optimised away", transfers for example
+                            break 'outer instructions
+                        }
+
+                        instructions.push(self.make_instruction(step, maybe_sub_call));
                     }
 
                     match current.parent {
@@ -338,7 +356,11 @@ impl ParityTraceBuilder {
 
     /// Creates a VM instruction from a [CallTraceStep] and a [VmTrace] for the subcall if there is
     /// one
-    fn make_instruction(&self, step: &CallTraceStep, maybe_sub: Option<VmTrace>) -> VmInstruction {
+    fn make_instruction(
+        &self,
+        step: &CallTraceStep,
+        maybe_sub_call: Option<VmTrace>,
+    ) -> VmInstruction {
         let maybe_storage = step.storage_change.map(|storage_change| StorageDelta {
             key: storage_change.key,
             val: storage_change.value,
@@ -369,7 +391,7 @@ impl ParityTraceBuilder {
             pc: step.pc,
             cost: cost as u64,
             ex: maybe_execution,
-            sub: maybe_sub,
+            sub: maybe_sub_call,
             op: Some(step.op.to_string()),
             idx: None,
         }
diff --git a/crates/revm/revm-inspectors/src/tracing/types.rs b/crates/revm/revm-inspectors/src/tracing/types.rs
index 83a4586a4f3e..552e7d33a0c7 100644
--- a/crates/revm/revm-inspectors/src/tracing/types.rs
+++ b/crates/revm/revm-inspectors/src/tracing/types.rs
@@ -599,6 +599,12 @@ impl CallTraceStep {
         log
     }
 
+    /// Returns true if the step is a STOP opcode
+    #[inline]
+    pub(crate) fn is_stop(&self) -> bool {
+        matches!(self.op.u8(), opcode::STOP)
+    }
+
     /// Returns true if the step is a call operation, any of
     /// CALL, CALLCODE, DELEGATECALL, STATICCALL, CREATE, CREATE2
     #[inline]