From 72b604b8e87145c96ace89918c6a54f0d63d44cc Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sun, 20 Mar 2016 19:20:37 +0100 Subject: [PATCH] Avoid tracing DELEGATECALL and CALLCODE. Plus tests for it. --- ethcore/res/null_homestead_morden.json | 35 +++++++++++ ethcore/res/null_morden.json | 2 +- ethcore/src/engine.rs | 10 ++++ ethcore/src/ethereum/ethash.rs | 20 +++---- ethcore/src/evm/interpreter.rs | 32 +++++----- ethcore/src/executive.rs | 11 +++- ethcore/src/null_engine.rs | 11 +++- ethcore/src/spec/spec.rs | 3 + ethcore/src/state.rs | 82 ++++++++++++++++++++++++++ 9 files changed, 177 insertions(+), 29 deletions(-) create mode 100644 ethcore/res/null_homestead_morden.json diff --git a/ethcore/res/null_homestead_morden.json b/ethcore/res/null_homestead_morden.json new file mode 100644 index 000000000..abd3f4de9 --- /dev/null +++ b/ethcore/res/null_homestead_morden.json @@ -0,0 +1,35 @@ +{ + "name": "Morden", + "engineName": "NullEngine", + "params": { + "accountStartNonce": "0x0100000", + "frontierCompatibilityModeLimit": "0x0", + "maximumExtraDataSize": "0x20", + "tieBreakingGas": false, + "minGasLimit": "0x1388", + "gasLimitBoundDivisor": "0x0400", + "minimumDifficulty": "0x020000", + "difficultyBoundDivisor": "0x0800", + "durationLimit": "0x0d", + "blockReward": "0x4563918244F40000", + "registrar": "", + "networkID" : "0x2" + }, + "genesis": { + "nonce": "0x00006d6f7264656e", + "difficulty": "0x20000", + "mixHash": "0x00000000000000000000000000000000000000647572616c65787365646c6578", + "author": "0x0000000000000000000000000000000000000000", + "timestamp": "0x00", + "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000", + "extraData": "0x", + "gasLimit": "0x2fefd8" + }, + "accounts": { + "0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, + "0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } }, + "0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } }, + "0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } }, + "102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" } + } +} diff --git a/ethcore/res/null_morden.json b/ethcore/res/null_morden.json index 70b48fbdb..86148d640 100644 --- a/ethcore/res/null_morden.json +++ b/ethcore/res/null_morden.json @@ -3,7 +3,7 @@ "engineName": "NullEngine", "params": { "accountStartNonce": "0x0100000", - "frontierCompatibilityModeLimit": "0xfffa2990", + "frontierCompatibilityModeLimit": "0x789b0", "maximumExtraDataSize": "0x20", "tieBreakingGas": false, "minGasLimit": "0x1388", diff --git a/ethcore/src/engine.rs b/ethcore/src/engine.rs index 0b2ce8ae2..7698af529 100644 --- a/ethcore/src/engine.rs +++ b/ethcore/src/engine.rs @@ -103,4 +103,14 @@ pub trait Engine : Sync + Send { fn execute_builtin(&self, a: &Address, input: &[u8], output: &mut [u8]) { self.spec().builtins.get(a).unwrap().execute(input, output); } // TODO: sealing stuff - though might want to leave this for later. + + /// Get a named parameter from the `spec()`'s `engine_params` item and convert to u64, or 0 if that fails. + fn u64_param(&self, name: &str) -> u64 { + self.spec().engine_params.get(name).map_or(0u64, |a| decode(&a)) + } + + /// Get a named parameter from the `spec()`'s `engine_params` item and convert to U256, or 0 if that fails. + fn u256_param(&self, name: &str) -> U256 { + self.spec().engine_params.get(name).map_or(x!(0), |a| decode(&a)) + } } diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 6f854921d..d2c56ebf1 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -57,16 +57,6 @@ impl Ethash { u256_params: RwLock::new(HashMap::new()) } } - - fn u64_param(&self, name: &str) -> u64 { - *self.u64_params.write().unwrap().entry(name.to_owned()).or_insert_with(|| - self.spec().engine_params.get(name).map_or(0u64, |a| decode(&a))) - } - - fn u256_param(&self, name: &str) -> U256 { - *self.u256_params.write().unwrap().entry(name.to_owned()).or_insert_with(|| - self.spec().engine_params.get(name).map_or(x!(0), |a| decode(&a))) - } } impl Engine for Ethash { @@ -199,6 +189,16 @@ impl Engine for Ethash { fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> { t.sender().map(|_|()) // Perform EC recovery and cache sender } + + fn u64_param(&self, name: &str) -> u64 { + *self.u64_params.write().unwrap().entry(name.to_owned()).or_insert_with(|| + self.spec().engine_params.get(name).map_or(0u64, |a| decode(&a))) + } + + fn u256_param(&self, name: &str) -> U256 { + *self.u256_params.write().unwrap().entry(name.to_owned()).or_insert_with(|| + self.spec().engine_params.get(name).map_or(x!(0), |a| decode(&a))) + } } #[cfg_attr(feature="dev", allow(wrong_self_convention))] // to_ethash should take self diff --git a/ethcore/src/evm/interpreter.rs b/ethcore/src/evm/interpreter.rs index b29fc0d41..eb29ef257 100644 --- a/ethcore/src/evm/interpreter.rs +++ b/ethcore/src/evm/interpreter.rs @@ -348,12 +348,13 @@ impl evm::Evm for Interpreter { impl Interpreter { #[cfg_attr(feature="dev", allow(cyclomatic_complexity))] - fn get_gas_cost_mem(&self, - ext: &evm::Ext, - instruction: Instruction, - mem: &mut Memory, - stack: &Stack - ) -> Result<(U256, usize), evm::Error> { + fn get_gas_cost_mem( + &self, + ext: &evm::Ext, + instruction: Instruction, + mem: &mut Memory, + stack: &Stack + ) -> Result<(U256, usize), evm::Error> { let schedule = ext.schedule(); let info = instructions::get_info(instruction); @@ -522,15 +523,16 @@ impl Interpreter { } #[cfg_attr(feature="dev", allow(too_many_arguments))] - fn exec_instruction(&self, - gas: Gas, - params: &ActionParams, - ext: &mut evm::Ext, - instruction: Instruction, - code: &mut CodeReader, - mem: &mut Memory, - stack: &mut Stack - ) -> Result { + fn exec_instruction( + &self, + gas: Gas, + params: &ActionParams, + ext: &mut evm::Ext, + instruction: Instruction, + code: &mut CodeReader, + mem: &mut Memory, + stack: &mut Stack + ) -> Result { match instruction { instructions::JUMP => { let jump = stack.pop_back(); diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 6ea39ec3b..c862545f4 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -196,6 +196,7 @@ impl<'a> Executive<'a> { if (self.depth + 1) % MAX_VM_DEPTH_FOR_THREAD != 0 { let mut ext = self.as_externalities(OriginInfo::from(¶ms), unconfirmed_substate, output_policy); let vm_factory = self.engine.vm_factory(); + trace!(target: "executive", "ext.schedule.have_delegate_call: {}", ext.schedule().have_delegate_call); return vm_factory.create().exec(params, &mut ext); } @@ -247,24 +248,30 @@ impl<'a> Executive<'a> { } } else if params.code.is_some() { // if destination is a contract, do normal message call + + // don't trace is it's DELEGATECALL or CALLCODE. + let should_trace = if let ActionValue::Transfer(_) = params.value { + params.code_address == params.address + } else { false }; // part of substate that may be reverted let mut unconfirmed_substate = Substate::new(substate.subtraces.is_some()); // transaction tracing stuff. None if there's no tracing. - let mut trace_info = substate.subtraces.as_ref().map(|_| (TraceAction::from_call(¶ms), self.depth)); + let mut trace_info = if should_trace { substate.subtraces.as_ref().map(|_| (TraceAction::from_call(¶ms), self.depth)) } else { None }; let mut trace_output = trace_info.as_ref().map(|_| vec![]); let res = { self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return(output, trace_output.as_mut())) }; + trace!(target: "executive", "res={:?}", res); + // if there's tracing, make up trace_info's result with trace_output and some arithmetic. if let Some((TraceAction::Call(ref mut c), _)) = trace_info { c.result = res.as_ref().ok().map(|gas_left| (c.gas - *gas_left, trace_output.expect("trace_info is Some: qed"))); } - trace!(target: "executive", "sstore-clears={}\n", unconfirmed_substate.sstore_clears_count); trace!(target: "executive", "substate={:?}; unconfirmed_substate={:?}\n", substate, unconfirmed_substate); self.enact_result(&res, substate, unconfirmed_substate, trace_info); diff --git a/ethcore/src/null_engine.rs b/ethcore/src/null_engine.rs index af7255617..99231add4 100644 --- a/ethcore/src/null_engine.rs +++ b/ethcore/src/null_engine.rs @@ -41,7 +41,16 @@ impl Engine for NullEngine { fn vm_factory(&self) -> &Factory { &self.factory } + fn name(&self) -> &str { "NullEngine" } + fn spec(&self) -> &Spec { &self.spec } - fn schedule(&self, _env_info: &EnvInfo) -> Schedule { Schedule::new_frontier() } + + fn schedule(&self, env_info: &EnvInfo) -> Schedule { + if env_info.number < self.u64_param("frontierCompatibilityModeLimit") { + Schedule::new_frontier() + } else { + Schedule::new_homestead() + } + } } diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index c7e2e4e9f..f97d8c8dc 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -333,6 +333,9 @@ impl Spec { /// Create a new Spec which conforms to the Morden chain except that it's a NullEngine consensus. pub fn new_test() -> Spec { Self::from_json_utf8(include_bytes!("../../res/null_morden.json")) } + + /// Create a new Spec which conforms to the Morden chain except that it's a NullEngine consensus. + pub fn new_homestead_test() -> Spec { Self::from_json_utf8(include_bytes!("../../res/null_homestead_morden.json")) } } #[cfg(test)] diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 78084e6db..c2924d1bb 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -532,6 +532,87 @@ fn should_not_trace_subcall_transaction_to_builtin() { assert_eq!(result.trace, expected_trace); } +#[test] +fn should_not_trace_callcode() { + init_log(); + + let temp = RandomTempPath::new(); + let mut state = get_temp_state_in(temp.as_path()); + + let mut info = EnvInfo::default(); + info.gas_limit = x!(1_000_000); + let engine = Spec::new_test().to_engine().unwrap(); + + let t = Transaction { + nonce: x!(0), + gas_price: x!(0), + gas: x!(100_000), + action: Action::Call(x!(0xa)), + value: x!(0), + data: vec![], + }.sign(&"".sha3()); + + state.init_code(&x!(0xa), FromHex::from_hex("60006000600060006000600b611000f2").unwrap()); + state.init_code(&x!(0xb), FromHex::from_hex("6000").unwrap()); + let result = state.apply(&info, engine.deref(), &t, true).unwrap(); + + let expected_trace = Some(Trace { + depth: 0, + action: TraceAction::Call(TraceCall { + from: x!("9cce34f7ab185c7aba1b7c8140d620b4bda941d6"), + to: x!(0xa), + value: x!(0), + gas: x!(79000), + input: vec![], + result: Some((x!(64), vec![])) + }), + subs: vec![] + }); + assert_eq!(result.trace, expected_trace); +} + +#[test] +fn should_not_trace_delegatecall() { + init_log(); + + let temp = RandomTempPath::new(); + let mut state = get_temp_state_in(temp.as_path()); + + let mut info = EnvInfo::default(); + info.gas_limit = x!(1_000_000); + info.number = 0x789b0; + let engine = Spec::new_test().to_engine().unwrap(); + + println!("schedule.have_delegate_call: {:?}", engine.schedule(&info).have_delegate_call); + + let t = Transaction { + nonce: x!(0), + gas_price: x!(0), + gas: x!(100_000), + action: Action::Call(x!(0xa)), + value: x!(0), + data: vec![], + }.sign(&"".sha3()); + + state.init_code(&x!(0xa), FromHex::from_hex("6000600060006000600b618000f4").unwrap()); + state.init_code(&x!(0xb), FromHex::from_hex("6000").unwrap()); + let result = state.apply(&info, engine.deref(), &t, true).unwrap(); + + let expected_trace = Some(Trace { + depth: 0, + action: TraceAction::Call(TraceCall { + from: x!("9cce34f7ab185c7aba1b7c8140d620b4bda941d6"), + to: x!(0xa), + value: x!(0), + gas: x!(79000), + input: vec![], + result: Some((x!(61), vec![])) + }), + subs: vec![] + }); + assert_eq!(result.trace, expected_trace); +} + #[test] fn should_trace_failed_call_transaction() { init_log(); @@ -572,6 +653,7 @@ fn should_trace_failed_call_transaction() { assert_eq!(result.trace, expected_trace); } + #[test] fn should_trace_call_with_subcall_transaction() { init_log();