From 1ac4676f4bbf3024d53e7a80900c14d3a1129c91 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 14 Aug 2018 22:34:46 +0800 Subject: [PATCH] Remove prepare_trace_output and make sure prepare_trace_call and trace*call are balanced (#9353) This refactors `prepare_trace_output` to instead directly take the reference of return values, so that it's simpler and we save a stack item. This should also fixes [the issue](https://github.com/paritytech/parity-ethereum/pull/9236#issuecomment-408444995) @udoprog is facing. Replaces #9236 --- ethcore/src/executive.rs | 34 +++++++++++++-------------- ethcore/src/trace/executive_tracer.rs | 13 ++++------ ethcore/src/trace/mod.rs | 8 ++----- ethcore/src/trace/noop_tracer.rs | 11 ++------- 4 files changed, 25 insertions(+), 41 deletions(-) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 17d870fcb..5be54f724 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -423,8 +423,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let default = []; let data = if let Some(ref d) = params.data { d as &[u8] } else { &default as &[u8] }; - let trace_info = tracer.prepare_trace_call(¶ms); - let cost = builtin.cost(data); if cost <= params.gas { let mut builtin_out_buffer = Vec::new(); @@ -435,7 +433,12 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { if let Err(e) = result { self.state.revert_to_checkpoint(); let evm_err: vm::Error = e.into(); - tracer.trace_failed_call(trace_info, vec![], evm_err.clone().into()); + let trace_info = tracer.prepare_trace_call(¶ms); + tracer.trace_failed_call( + trace_info, + vec![], + evm_err.clone().into() + ); Err(evm_err) } else { self.state.discard_checkpoint(); @@ -448,15 +451,11 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { ActionValue::Apparent(_) => false, }; if self.depth == 0 || is_transferred { - let mut trace_output = tracer.prepare_trace_output(); - if let Some(out) = trace_output.as_mut() { - *out = builtin_out_buffer.clone(); - } - + let trace_info = tracer.prepare_trace_call(¶ms); tracer.trace_call( trace_info, cost, - trace_output, + &builtin_out_buffer, vec![] ); } @@ -472,13 +471,17 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // just drain the whole gas self.state.revert_to_checkpoint(); - tracer.trace_failed_call(trace_info, vec![], vm::Error::OutOfGas.into()); + let trace_info = tracer.prepare_trace_call(¶ms); + tracer.trace_failed_call( + trace_info, + vec![], + vm::Error::OutOfGas.into() + ); Err(vm::Error::OutOfGas) } } else { let trace_info = tracer.prepare_trace_call(¶ms); - let mut trace_output = tracer.prepare_trace_output(); let mut subtracer = tracer.subtracer(); let gas = params.gas; @@ -501,11 +504,10 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let traces = subtracer.drain(); match res { Ok(ref res) if res.apply_state => { - trace_output.as_mut().map(|d| *d = res.return_data.to_vec()); tracer.trace_call( trace_info, gas - res.gas_left, - trace_output, + &res.return_data, traces ); }, @@ -522,7 +524,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // otherwise it's just a basic transaction, only do tracing, if necessary. self.state.discard_checkpoint(); - tracer.trace_call(trace_info, U256::zero(), trace_output, vec![]); + tracer.trace_call(trace_info, U256::zero(), &[], vec![]); Ok(FinalizationResult { gas_left: params.gas, return_data: ReturnData::empty(), @@ -577,7 +579,6 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { } let trace_info = tracer.prepare_trace_create(¶ms); - let mut trace_output = tracer.prepare_trace_output(); let mut subtracer = tracer.subtracer(); let gas = params.gas; let created = params.address.clone(); @@ -596,11 +597,10 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { match res { Ok(ref res) if res.apply_state => { - trace_output.as_mut().map(|trace| *trace = res.return_data.to_vec()); tracer.trace_create( trace_info, gas - res.gas_left, - trace_output, + &res.return_data, created, subtracer.drain() ); diff --git a/ethcore/src/trace/executive_tracer.rs b/ethcore/src/trace/executive_tracer.rs index 1bae15d59..227b3a39f 100644 --- a/ethcore/src/trace/executive_tracer.rs +++ b/ethcore/src/trace/executive_tracer.rs @@ -17,7 +17,6 @@ //! Simple executive tracer. use ethereum_types::{U256, Address}; -use bytes::Bytes; use vm::ActionParams; use trace::trace::{Call, Create, Action, Res, CreateResult, CallResult, VMTrace, VMOperation, VMExecutedOperation, MemoryDiff, StorageDiff, Suicide, Reward, RewardType}; use trace::{Tracer, VMTracer, FlatTrace, TraceError}; @@ -92,18 +91,14 @@ impl Tracer for ExecutiveTracer { Some(Create::from(params.clone())) } - fn prepare_trace_output(&self) -> Option { - Some(vec![]) - } - - fn trace_call(&mut self, call: Option, gas_used: U256, output: Option, subs: Vec) { + fn trace_call(&mut self, call: Option, gas_used: U256, output: &[u8], subs: Vec) { let trace = FlatTrace { trace_address: Default::default(), subtraces: top_level_subtraces(&subs), action: Action::Call(call.expect("self.prepare_trace_call().is_some(): so we must be tracing: qed")), result: Res::Call(CallResult { gas_used: gas_used, - output: output.expect("self.prepare_trace_output().is_some(): so we must be tracing: qed") + output: output.into() }), }; debug!(target: "trace", "Traced call {:?}", trace); @@ -111,13 +106,13 @@ impl Tracer for ExecutiveTracer { self.traces.extend(prefix_subtrace_addresses(subs)); } - fn trace_create(&mut self, create: Option, gas_used: U256, code: Option, address: Address, subs: Vec) { + fn trace_create(&mut self, create: Option, gas_used: U256, code: &[u8], address: Address, subs: Vec) { let trace = FlatTrace { subtraces: top_level_subtraces(&subs), action: Action::Create(create.expect("self.prepare_trace_create().is_some(): so we must be tracing: qed")), result: Res::Create(CreateResult { gas_used: gas_used, - code: code.expect("self.prepare_trace_output.is_some(): so we must be tracing: qed"), + code: code.into(), address: address }), trace_address: Default::default(), diff --git a/ethcore/src/trace/mod.rs b/ethcore/src/trace/mod.rs index 90ea64b5d..670cc755f 100644 --- a/ethcore/src/trace/mod.rs +++ b/ethcore/src/trace/mod.rs @@ -38,7 +38,6 @@ pub use self::types::filter::{Filter, AddressesFilter}; use ethereum_types::{H256, U256, Address}; use kvdb::DBTransaction; -use bytes::Bytes; use self::trace::{Call, Create}; use vm::ActionParams; use header::BlockNumber; @@ -58,9 +57,6 @@ pub trait Tracer: Send { /// This is called before a create has been executed. fn prepare_trace_create(&self, params: &ActionParams) -> Option; - /// Prepare trace output. Noop tracer should return None. - fn prepare_trace_output(&self) -> Option; - /// Stores trace call info. /// /// This is called after a call has completed successfully. @@ -68,7 +64,7 @@ pub trait Tracer: Send { &mut self, call: Option, gas_used: U256, - output: Option, + output: &[u8], subs: Vec, ); @@ -79,7 +75,7 @@ pub trait Tracer: Send { &mut self, create: Option, gas_used: U256, - code: Option, + code: &[u8], address: Address, subs: Vec ); diff --git a/ethcore/src/trace/noop_tracer.rs b/ethcore/src/trace/noop_tracer.rs index 8312de58f..fdde9a6e3 100644 --- a/ethcore/src/trace/noop_tracer.rs +++ b/ethcore/src/trace/noop_tracer.rs @@ -17,7 +17,6 @@ //! Nonoperative tracer. use ethereum_types::{U256, Address}; -use bytes::Bytes; use vm::ActionParams; use trace::{Tracer, VMTracer, FlatTrace, TraceError}; use trace::trace::{Call, Create, VMTrace, RewardType}; @@ -36,18 +35,12 @@ impl Tracer for NoopTracer { None } - fn prepare_trace_output(&self) -> Option { - None - } - - fn trace_call(&mut self, call: Option, _: U256, output: Option, _: Vec) { + fn trace_call(&mut self, call: Option, _: U256, _: &[u8], _: Vec) { assert!(call.is_none(), "self.prepare_trace_call().is_none(): so we can't be tracing: qed"); - assert!(output.is_none(), "self.prepare_trace_output().is_none(): so we can't be tracing: qed"); } - fn trace_create(&mut self, create: Option, _: U256, code: Option, _: Address, _: Vec) { + fn trace_create(&mut self, create: Option, _: U256, _: &[u8], _: Address, _: Vec) { assert!(create.is_none(), "self.prepare_trace_create().is_none(): so we can't be tracing: qed"); - assert!(code.is_none(), "self.prepare_trace_output().is_none(): so we can't be tracing: qed"); } fn trace_failed_call(&mut self, call: Option, _: Vec, _: TraceError) {