Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 2 additions & 5 deletions bins/revme/src/cmd/statetest/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,7 @@ pub fn execute_test_suite(
let (e, exec_result) = if trace {
let mut evm = evm
.modify()
.reset_handler_with_external_context(TracerEip3155::new(
Box::new(stderr()),
false,
))
.reset_handler_with_external_context(TracerEip3155::new(Box::new(stderr())))
.append_handler_register(inspector_handle_register)
.build();

Expand Down Expand Up @@ -421,7 +418,7 @@ pub fn execute_test_suite(
let mut evm = Evm::builder()
.with_spec_id(spec_id)
.with_db(state)
.with_external_context(TracerEip3155::new(Box::new(stdout()), false))
.with_external_context(TracerEip3155::new(Box::new(stdout())))
.append_handler_register(inspector_handle_register)
.build();
let _ = evm.transact_commit();
Expand Down
29 changes: 25 additions & 4 deletions crates/revm/src/inspector/eip3155.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ pub struct TracerEip3155 {
refunded: i64,
mem_size: usize,
skip: bool,
include_memory: bool,
memory: Option<Vec<u8>>,
Copy link
Member

Choose a reason for hiding this comment

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

Onee more thing, it would be good to have String here so we don't copy memory multiple times.

Now we are doing clone in step and clone into String in step_end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it to String now, however my change did not improve the performance. I guess the self.memory.take() method also copies it internally. Without the .take() I've got the error "cannot move out of self.memory which is behind a mutable reference" and didn't manage to fix it in a better way.

Copy link
Member

Choose a reason for hiding this comment

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

take() does not copy full string, it is the right approach. Copying memory on every step is a heavy operation, not a lot we could do there. There are few optimisation that we could do but they require introspection of opcodes that change memory, definitely out of scope for this PR.

}

// # Output
Expand Down Expand Up @@ -58,7 +60,7 @@ struct Output {
error: Option<String>,
/// Array of all allocated values
#[serde(default, skip_serializing_if = "Option::is_none")]
memory: Option<Vec<String>>,
memory: Option<String>,
/// Array of all stored values
#[serde(default, skip_serializing_if = "Option::is_none")]
storage: Option<HashMap<String, String>>,
Expand Down Expand Up @@ -98,12 +100,14 @@ impl TracerEip3155 {
}

impl TracerEip3155 {
pub fn new(output: Box<dyn Write>, print_summary: bool) -> Self {
pub fn new(output: Box<dyn Write>) -> Self {
Self {
output,
gas_inspector: GasInspector::default(),
print_summary,
print_summary: true,
include_memory: false,
stack: Default::default(),
memory: Default::default(),
pc: 0,
opcode: 0,
gas: 0,
Expand All @@ -113,6 +117,18 @@ impl TracerEip3155 {
}
}

/// Don't include a summary at the end of the trace
pub fn without_summary(mut self) -> Self {
self.print_summary = false;
self
}

/// Include a memory field for each step. This significantly increases processing time and output size.
pub fn with_memory(mut self) -> Self {
self.include_memory = true;
self
}

fn write_value(&mut self, value: &impl serde::Serialize) -> std::io::Result<()> {
serde_json::to_writer(&mut *self.output, value)?;
self.output.write_all(b"\n")?;
Expand All @@ -128,6 +144,11 @@ impl<DB: Database> Inspector<DB> for TracerEip3155 {
fn step(&mut self, interp: &mut Interpreter, context: &mut EvmContext<DB>) {
self.gas_inspector.step(interp, context);
self.stack = interp.stack.data().clone();
self.memory = if self.include_memory {
Some(interp.shared_memory.context_memory().to_owned())
} else {
None
};
self.pc = interp.program_counter();
self.opcode = interp.current_opcode();
self.mem_size = interp.shared_memory.len();
Expand Down Expand Up @@ -159,7 +180,7 @@ impl<DB: Database> Inspector<DB> for TracerEip3155 {
} else {
None
},
memory: None,
memory: self.memory.as_ref().map(hex::encode_prefixed),
storage: None,
return_stack: None,
};
Expand Down
2 changes: 1 addition & 1 deletion examples/db_by_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn run_transaction_and_commit(db: &mut CacheDB<EmptyDB>) -> anyhow::Result<()> {
fn main() -> anyhow::Result<()> {
let mut cache_db = CacheDB::new(EmptyDB::default());

let mut tracer = TracerEip3155::new(Box::new(std::io::stdout()), true);
let mut tracer = TracerEip3155::new(Box::new(std::io::stdout()));

run_transaction_and_commit_with_ext(&mut cache_db, &mut tracer, inspector_handle_register)?;
run_transaction_and_commit(&mut cache_db)?;
Expand Down
2 changes: 1 addition & 1 deletion examples/generate_block_traces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ async fn main() -> anyhow::Result<()> {
let mut state = StateBuilder::new_with_database(cache_db).build();
let mut evm = Evm::builder()
.with_db(&mut state)
.with_external_context(TracerEip3155::new(Box::new(std::io::stdout()), true))
.with_external_context(TracerEip3155::new(Box::new(std::io::stdout())))
.modify_block_env(|b| {
if let Some(number) = block.number {
let nn = number.0[0];
Expand Down