Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: exclude single stop vm trace instruction #4149

Merged
merged 1 commit into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 34 additions & 12 deletions crates/revm/revm-inspectors/src/tracing/builder/parity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
}
Expand Down
6 changes: 6 additions & 0 deletions crates/revm/revm-inspectors/src/tracing/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading