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

Add essential EIP-7756 tracing fields #2023

Merged
merged 4 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions crates/handler/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ where

// ExtDelegateCall is not allowed to call non-EOF contracts.
if is_ext_delegate_call && !bytecode.bytes_slice().starts_with(&EOF_MAGIC_BYTES) {
context.journal().checkpoint_revert(checkpoint);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing the depth field to read incorrectly when this case triggered. Not sure if here was any way to leverage it. Also, EOF only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find! Yeah it is good fix and it is EOF only

return return_result(InstructionResult::InvalidExtDelegateCallTarget);
}

Expand Down
25 changes: 24 additions & 1 deletion crates/inspector/src/eip3155.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{inspectors::GasInspector, Inspector};
use revm::interpreter::interpreter_types::{RuntimeFlag, SubRoutineStack};
use revm::{
bytecode::opcode::OpCode,
context::Cfg,
Expand All @@ -21,6 +22,8 @@ pub struct TracerEip3155<CTX, INTR> {
print_summary: bool,
stack: Vec<U256>,
pc: usize,
section: Option<u64>,
function_depth: Option<u64>,
opcode: u8,
gas: u64,
refunded: i64,
Expand All @@ -39,6 +42,9 @@ struct Output {
// Required fields:
/// Program counter
pc: u64,
/// EOF code section
#[serde(default, skip_serializing_if = "Option::is_none")]
section: Option<u64>,
/// OpCode
op: u8,
/// Gas left before executing this operation
Expand All @@ -49,6 +55,9 @@ struct Output {
stack: Vec<String>,
/// Depth of the call stack
depth: u64,
/// Depth of the EOF function call stack
#[serde(default, skip_serializing_if = "Option::is_none")]
function_depth: Option<u64>,
/// Data returned by the function call
return_data: String,
/// Amount of **global** gas refunded
Expand Down Expand Up @@ -140,6 +149,8 @@ where
stack: Default::default(),
memory: Default::default(),
pc: 0,
section: None,
function_depth: None,
opcode: 0,
gas: 0,
refunded: 0,
Expand Down Expand Up @@ -213,7 +224,17 @@ where
} else {
None
};
self.pc = interp.bytecode.pc();
self.pc = interp.bytecode.trace_pc();
self.section = if interp.runtime_flag.is_eof() {
Some(interp.sub_routine.routine_idx() as u64)
} else {
None
};
self.function_depth = if interp.runtime_flag.is_eof() {
Some(interp.sub_routine.len() as u64 + 1)
} else {
None
};
self.opcode = interp.bytecode.opcode();
self.mem_size = interp.memory.size();
self.gas = interp.control.gas().remaining();
Expand All @@ -229,11 +250,13 @@ where

let value = Output {
pc: self.pc as u64,
section: self.section,
op: self.opcode,
gas: hex_number(self.gas),
gas_cost: hex_number(self.gas_inspector.last_gas_cost()),
stack: self.stack.iter().map(hex_number_u256).collect(),
depth: context.journal().depth() as u64,
function_depth: self.function_depth,
return_data: "0x".to_string(),
refund: hex_number(self.refunded as u64),
mem_size: self.mem_size.to_string(),
Expand Down
12 changes: 12 additions & 0 deletions crates/interpreter/src/interpreter/ext_bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ impl Jumps for ExtBytecode {
.offset_from(self.base.bytecode().as_ptr()) as usize
}
}
#[inline]
fn trace_pc(&self) -> usize {
match &self.base {
Bytecode::Eof(_) => unsafe {
// SAFETY: `instruction_pointer` should be at an offset from the start of the bytecode.
// In practice this is always true unless a caller modifies the `instruction_pointer` field manually.
self.instruction_pointer
.offset_from(self.base.eof().unwrap().raw.as_ptr()) as usize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already made a change so current pc is doing the same thing as trace_pc

fn pc(&self) -> usize {
// SAFETY: `instruction_pointer` should be at an offset from the start of the bytecode.
// In practice this is always true unless a caller modifies the `instruction_pointer` field manually.
unsafe {
self.instruction_pointer
.offset_from(self.base.bytecode().as_ptr()) as usize
}
}
}

and bytecode() gives self.base.eof().raw here:

pub fn bytecode(&self) -> &Bytes {
match self {
Self::LegacyAnalyzed(analyzed) => analyzed.bytecode(),
Self::Eof(eof) => &eof.body.code,
Self::Eip7702(code) => code.raw(),
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay my mistake, will see to change the fn pc to become trace_pc it is not used in any other place for EOF.

},
_ => self.pc(),
}
}
}

impl Immediates for ExtBytecode {
Expand Down
2 changes: 2 additions & 0 deletions crates/interpreter/src/interpreter_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ pub trait Jumps {
fn is_valid_legacy_jump(&mut self, offset: usize) -> bool;
/// Returns current program counter.
fn pc(&self) -> usize;
/// Returns the pc offset for tracing
fn trace_pc(&self) -> usize;
/// Returns instruction opcode.
fn opcode(&self) -> u8;
}
Expand Down
Loading