fix: record selfdstructs properly (#3921)

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

@@ -217,6 +217,25 @@ impl ParityTraceBuilder {
             if with_traces {
                 let trace = node.parity_transaction_trace(trace_address);
                 traces.push(trace);
+
+                // check if the trace node is a selfdestruct
+                if node.is_selfdestruct() {
+                    // selfdestructs are not recorded as individual call traces but are derived from
+                    // the call trace and are added as additional `TransactionTrace` objects in the
+                    // trace array
+                    let addr = {
+                        let last = traces.last_mut().expect("exists");
+                        let mut addr = last.trace_address.clone();
+                        addr.push(last.subtraces);
+                        // need to account for the additional selfdestruct trace
+                        last.subtraces += 1;
+                        addr
+                    };
+
+                    if let Some(trace) = node.parity_selfdestruct_trace(addr) {
+                        traces.push(trace);
+                    }
+                }
             }
             if with_diff {
                 node.parity_update_state_diff(&mut diff);
@@ -232,11 +251,15 @@ impl ParityTraceBuilder {
     /// Returns an iterator over all recorded traces  for `trace_transaction`
     pub fn into_transaction_traces_iter(self) -> impl Iterator<Item = TransactionTrace> {
         let trace_addresses = self.trace_addresses();
-        self.nodes
-            .into_iter()
-            .zip(trace_addresses)
-            .filter(|(node, _)| !node.is_precompile())
-            .map(|(node, trace_address)| node.parity_transaction_trace(trace_address))
+        TransactionTraceIter {
+            next_selfdestruct: None,
+            iter: self
+                .nodes
+                .into_iter()
+                .zip(trace_addresses)
+                .filter(|(node, _)| !node.is_precompile())
+                .map(|(node, trace_address)| (node.parity_transaction_trace(trace_address), node)),
+        }
     }
 
     /// Returns the raw traces of the transaction
@@ -344,6 +367,39 @@ 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>,
+}
+
+impl<Iter> Iterator for TransactionTraceIter<Iter>
+where
+    Iter: Iterator<Item = (TransactionTrace, CallTraceNode)>,
+{
+    type Item = TransactionTrace;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if let Some(selfdestruct) = self.next_selfdestruct.take() {
+            return Some(selfdestruct)
+        }
+        let (mut trace, node) = self.iter.next()?;
+        if node.is_selfdestruct() {
+            // since selfdestructs are emitted as additional trace, increase the trace count
+            let mut addr = trace.trace_address.clone();
+            addr.push(trace.subtraces);
+            // need to account for the additional selfdestruct trace
+            trace.subtraces += 1;
+            self.next_selfdestruct = node.parity_selfdestruct_trace(addr);
+        }
+        Some(trace)
+    }
+}
+
 /// addresses are presorted via breadth first walk thru [CallTraceNode]s, this  can be done by a
 /// walker in [crate::tracing::builder::walker]
 ///

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

@@ -297,6 +297,12 @@ impl CallTraceNode {
         self.trace.status
     }
 
+    /// Returns true if the call was a selfdestruct
+    #[inline]
+    pub(crate) fn is_selfdestruct(&self) -> bool {
+        self.status() == InstructionResult::SelfDestruct
+    }
+
     /// Updates the values of the state diff
     pub(crate) fn parity_update_state_diff(&self, diff: &mut StateDiff) {
         let addr = self.trace.address;
@@ -348,9 +354,7 @@ impl CallTraceNode {
     /// Converts this node into a parity `TransactionTrace`
     pub(crate) fn parity_transaction_trace(&self, trace_address: Vec<usize>) -> TransactionTrace {
         let action = self.parity_action();
-        let result = if action.is_selfdestruct() ||
-            (self.trace.is_error() && !self.trace.is_revert())
-        {
+        let result = if self.trace.is_error() && !self.trace.is_revert() {
             // if the trace is a selfdestruct or an error that is not a revert, the result is None
             None
         } else {
@@ -377,15 +381,39 @@ impl CallTraceNode {
         }
     }
 
-    /// Returns the `Action` for a parity trace
-    pub(crate) fn parity_action(&self) -> Action {
-        if self.status() == InstructionResult::SelfDestruct {
-            return Action::Selfdestruct(SelfdestructAction {
+    /// If the trace is a selfdestruct, returns the `Action` for a parity trace.
+    pub(crate) fn parity_selfdestruct_action(&self) -> Option<Action> {
+        if self.is_selfdestruct() {
+            Some(Action::Selfdestruct(SelfdestructAction {
                 address: self.trace.address,
                 refund_address: self.trace.selfdestruct_refund_target.unwrap_or_default(),
                 balance: self.trace.value,
-            })
+            }))
+        } else {
+            None
         }
+    }
+
+    /// If the trace is a selfdestruct, returns the `TransactionTrace` for a parity trace.
+    pub(crate) fn parity_selfdestruct_trace(
+        &self,
+        trace_address: Vec<usize>,
+    ) -> Option<TransactionTrace> {
+        let trace = self.parity_selfdestruct_action()?;
+        Some(TransactionTrace {
+            action: trace,
+            error: None,
+            result: None,
+            trace_address,
+            subtraces: 0,
+        })
+    }
+
+    /// Returns the `Action` for a parity trace.
+    ///
+    /// Caution: This does not include the selfdestruct action, if the trace is a selfdestruct,
+    /// since those are handled in addition to the call action.
+    pub(crate) fn parity_action(&self) -> Action {
         match self.kind() {
             CallKind::Call | CallKind::StaticCall | CallKind::CallCode | CallKind::DelegateCall => {
                 Action::Call(CallAction {