From 4b0f4ec67e06b955e5a9bb9cf38a9ae8ee9eed56 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 1 Aug 2023 14:19:08 +0200 Subject: [PATCH] fix: record selfdstructs properly (#3921) --- .../src/tracing/builder/parity.rs | 66 +++++++++++++++++-- .../revm/revm-inspectors/src/tracing/types.rs | 44 ++++++++++--- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs index 7c3720621b..fb18366424 100644 --- a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs +++ b/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 { 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, + next_selfdestruct: Option, +} + +impl Iterator for TransactionTraceIter +where + Iter: Iterator, +{ + type Item = TransactionTrace; + + fn next(&mut self) -> Option { + 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] /// diff --git a/crates/revm/revm-inspectors/src/tracing/types.rs b/crates/revm/revm-inspectors/src/tracing/types.rs index 83c720dd28..2739517dd4 100644 --- a/crates/revm/revm-inspectors/src/tracing/types.rs +++ b/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) -> 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 { + 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, + ) -> Option { + 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 {