From 08eddce1cf106afe873fe5290382d6323d74e105 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Wed, 25 Apr 2018 19:17:38 +0800 Subject: [PATCH 1/5] Trace precompiled contracts when the transfer value is not zero --- ethcore/src/executive.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index cded6358e8f..68f35554cc9 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -428,8 +428,12 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { self.state.discard_checkpoint(); output.write(0, &builtin_out_buffer); - // trace only top level calls to builtins to avoid DDoS attacks - if self.depth == 0 { + // trace only top level calls and calls with balance transfer to builtins to avoid DDoS attacks + let transferred = match params.value { + ActionValue::Transfer(value) => value, + ActionValue::Apparent(_) => U256::zero(), + }; + if self.depth == 0 || transferred != U256::zero() { let mut trace_output = tracer.prepare_trace_output(); if let Some(out) = trace_output.as_mut() { *out = output.to_owned(); From 6084acb1194bf5a8bb3da09860b58f75de12b368 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 27 Apr 2018 17:21:23 +0800 Subject: [PATCH 2/5] Add tests for precompiled CALL tracing --- ethcore/src/executive.rs | 70 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 68f35554cc9..e7cb802322c 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -817,6 +817,76 @@ mod tests { assert_eq!(substate.contracts_created.len(), 0); } + #[test] + fn test_call_to_precompiled_tracing() { + // code: + // + // 60 00 - push 00 out size + // 60 00 - push 00 out offset + // 60 00 - push 00 in size + // 60 00 - push 00 in offset + // 60 01 - push 01 value + // 60 03 - push 03 to + // 61 ffff - push fff gas + // f1 - CALL + + let code = "60006000600060006001600361fffff1".from_hex().unwrap(); + let sender = Address::from_str("4444444444444444444444444444444444444444").unwrap(); + let address = Address::from_str("5555555555555555555555555555555555555555").unwrap(); + + let mut params = ActionParams::default(); + params.address = address.clone(); + params.code_address = address.clone(); + params.sender = sender.clone(); + params.origin = sender.clone(); + params.gas = U256::from(100_000); + params.code = Some(Arc::new(code)); + params.value = ActionValue::Transfer(U256::from(100)); + params.call_type = CallType::Call; + let mut state = get_temp_state(); + state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); + let info = EnvInfo::default(); + let machine = make_frontier_machine(5); + let mut substate = Substate::new(); + let mut tracer = ExecutiveTracer::default(); + let mut vm_tracer = ExecutiveVMTracer::toplevel(); + + let mut ex = Executive::new(&mut state, &info, &machine); + let output = BytesRef::Fixed(&mut[0u8;0]); + ex.call(params, &mut substate, output, &mut tracer, &mut vm_tracer).unwrap(); + + assert_eq!(tracer.drain(), vec![FlatTrace { + action: trace::Action::Call(trace::Call { + from: "4444444444444444444444444444444444444444".into(), + to: "5555555555555555555555555555555555555555".into(), + value: 100.into(), + gas: 100_000.into(), + input: vec![], + call_type: CallType::Call + }), + result: trace::Res::Call(trace::CallResult { + gas_used: 32361.into(), + output: vec![] + }), + subtraces: 1, + trace_address: Default::default() + }, FlatTrace { + action: trace::Action::Call(trace::Call { + from: "5555555555555555555555555555555555555555".into(), + to: "0000000000000000000000000000000000000003".into(), + value: 1.into(), + gas: 67835.into(), + input: vec![], + call_type: CallType::Call + }), result: trace::Res::Call(trace::CallResult { + gas_used: 600.into(), + output: vec![] + }), + subtraces: 0, + trace_address: vec![0].into_iter().collect(), + }]); + } + #[test] // Tracing is not suported in JIT fn test_call_to_create() { From e3d7ae332522f3b899398d41eafbb3d04aa90c26 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 27 Apr 2018 17:42:51 +0800 Subject: [PATCH 3/5] Use byzantium test machine for the new test --- ethcore/src/executive.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index e7cb802322c..0c7cbeae074 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -726,6 +726,12 @@ mod tests { machine } + fn make_byzantium_machine(max_depth: usize) -> EthereumMachine { + let mut machine = ::ethereum::new_byzantium_test_machine(); + machine.set_schedule_creation_rules(Box::new(move |s, _| s.max_depth = max_depth)); + machine + } + #[test] fn test_contract_address() { let address = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap(); @@ -846,7 +852,7 @@ mod tests { let mut state = get_temp_state(); state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); - let machine = make_frontier_machine(5); + let machine = make_byzantium_machine(5); let mut substate = Substate::new(); let mut tracer = ExecutiveTracer::default(); let mut vm_tracer = ExecutiveVMTracer::toplevel(); @@ -865,7 +871,7 @@ mod tests { call_type: CallType::Call }), result: trace::Res::Call(trace::CallResult { - gas_used: 32361.into(), + gas_used: 33021.into(), output: vec![] }), subtraces: 1, @@ -875,7 +881,7 @@ mod tests { from: "5555555555555555555555555555555555555555".into(), to: "0000000000000000000000000000000000000003".into(), value: 1.into(), - gas: 67835.into(), + gas: 66560.into(), input: vec![], call_type: CallType::Call }), result: trace::Res::Call(trace::CallResult { From 4e63a9adba58c6dec1beb73888d64ccf81ed31c5 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 8 May 2018 21:05:57 +0800 Subject: [PATCH 4/5] Add notes in comments on why we don't trace all precompileds --- ethcore/src/executive.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 0c7cbeae074..1a25e899cd8 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -428,7 +428,9 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { self.state.discard_checkpoint(); output.write(0, &builtin_out_buffer); - // trace only top level calls and calls with balance transfer to builtins to avoid DDoS attacks + // Trace only top level calls and calls with balance transfer to builtins. The reason why we don't + // trace all internal calls to builtin contracts is that memcpy (IDENTITY) is a heavily used + // function. let transferred = match params.value { ActionValue::Transfer(value) => value, ActionValue::Apparent(_) => U256::zero(), From 585d307c60c635cd0a52de45c995d5b54e620b09 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 8 May 2018 21:08:19 +0800 Subject: [PATCH 5/5] Use is_transferred instead of transferred --- ethcore/src/executive.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 1a25e899cd8..e29da093c7d 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -431,11 +431,11 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // Trace only top level calls and calls with balance transfer to builtins. The reason why we don't // trace all internal calls to builtin contracts is that memcpy (IDENTITY) is a heavily used // function. - let transferred = match params.value { - ActionValue::Transfer(value) => value, - ActionValue::Apparent(_) => U256::zero(), + let is_transferred = match params.value { + ActionValue::Transfer(value) => value != U256::zero(), + ActionValue::Apparent(_) => false, }; - if self.depth == 0 || transferred != U256::zero() { + if self.depth == 0 || is_transferred { let mut trace_output = tracer.prepare_trace_output(); if let Some(out) = trace_output.as_mut() { *out = output.to_owned();