Merge pull request #794 from ethcore/tracing

Avoid tracing DELEGATECALL and CALLCODE. Plus tests for it.

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" }
+	}
+}

ethcore/res/null_morden.json

@@ -3,7 +3,7 @@
 	"engineName": "NullEngine",
 	"params": {
 		"accountStartNonce": "0x0100000",
-		"frontierCompatibilityModeLimit": "0xfffa2990",
+		"frontierCompatibilityModeLimit": "0x789b0",
 		"maximumExtraDataSize": "0x20",
 		"tieBreakingGas": false,
 		"minGasLimit": "0x1388",

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))
+	}
 }

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

ethcore/src/evm/interpreter.rs

@@ -348,7 +348,8 @@ impl evm::Evm for Interpreter {
 
 impl Interpreter {
 	#[cfg_attr(feature="dev", allow(cyclomatic_complexity))]
-	fn get_gas_cost_mem(&self,
+	fn get_gas_cost_mem(
+		&self,
 		ext: &evm::Ext,
 		instruction: Instruction,
 		mem: &mut Memory,
@@ -522,7 +523,8 @@ impl Interpreter {
 	}
 
 	#[cfg_attr(feature="dev", allow(too_many_arguments))]
-	fn exec_instruction(&self,
+	fn exec_instruction(
+		&self,
 		gas: Gas,
 		params: &ActionParams,
 		ext: &mut evm::Ext,

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(&params), 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);
 		}
 
@@ -245,37 +246,51 @@ impl<'a> Executive<'a> {
 					Err(evm::Error::OutOfGas)
 				}
 			}
-		} else if params.code.is_some() {
+		} else {
 			// if destination is a contract, do normal message call
 			
-			// part of substate that may be reverted
+			// don't trace if it's DELEGATECALL or CALLCODE.
-			let mut unconfirmed_substate = Substate::new(substate.subtraces.is_some());
+			let should_trace = if let ActionValue::Transfer(_) = params.value {
+				params.code_address == params.address && substate.subtraces.is_some()
+			} else { false };
 
 			// transaction tracing stuff. None if there's no tracing.
-			let mut trace_info = substate.subtraces.as_ref().map(|_| (TraceAction::from_call(&params), self.depth));
+			let (mut trace_info, mut trace_output) = if should_trace {
-			let mut trace_output = trace_info.as_ref().map(|_| vec![]);
+				(Some((TraceAction::from_call(&params), self.depth)), Some(vec![]))
+			} else { (None, None) };
+
+			if params.code.is_some() {
+				// part of substate that may be reverted
+				let mut unconfirmed_substate = Substate::new(should_trace);
 
 				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")));
+					c.result = res.as_ref().ok().map(|gas_left| (c.gas - *gas_left, trace_output.expect("trace_info is Some: so should_trace: 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);
 				trace!(target: "executive", "enacted: substate={:?}\n", substate);
 				res
 			} else {
-			// otherwise, nothing
+				// otherwise it's just a basic transaction, only do tracing, if necessary.
+				trace!(target: "executive", "Basic message (send funds) should_trace={}", should_trace);
 				self.state.clear_snapshot();
+				if let Some((TraceAction::Call(ref mut c), _)) = trace_info {
+					c.result = Some((x!(0), vec![]));
+				}
+				substate.accrue_trace(if should_trace {Some(vec![])} else {None}, trace_info);
 				Ok(params.gas)
 			}
 		}
+	}
 
 	/// Creates contract with given contract params.
 	/// NOTE. It does not finalize the transaction (doesn't do refunds, nor suicides).
@@ -530,6 +545,7 @@ mod tests {
 		//let next_address = contract_address(&address, &U256::zero());
 		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);
@@ -627,6 +643,7 @@ mod tests {
 		assert_eq!(substate.subtraces, expected_trace);
 		assert_eq!(gas_left, U256::from(96_776));
 	}
+
 	evm_test!{test_create_contract_value_too_high: test_create_contract_value_too_high_jit, test_create_contract_value_too_high_int}
 	fn test_create_contract_value_too_high(factory: Factory) {
 		// code:

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()
+		}
+	}
 }

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)]

ethcore/src/state.rs

@@ -469,6 +469,44 @@ fn should_trace_call_transaction() {
 	assert_eq!(result.trace, expected_trace);
 }
 
+#[test]
+fn should_trace_basic_call_transaction() {
+	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 = TestEngine::new(5, Factory::default());
+
+	let t = Transaction {
+		nonce: x!(0),
+		gas_price: x!(0),
+		gas: x!(100_000),
+		action: Action::Call(x!(0xa)),
+		value: x!(100),
+		data: vec![],
+	}.sign(&"".sha3());
+
+	state.add_balance(t.sender().as_ref().unwrap(), &x!(100));
+	let result = state.apply(&info, &engine, &t, true).unwrap();
+	let expected_trace = Some(Trace {
+		depth: 0,
+		action: TraceAction::Call(TraceCall {
+			from: x!("9cce34f7ab185c7aba1b7c8140d620b4bda941d6"),
+			to: x!(0xa),
+			value: x!(100),
+			gas: x!(79000),
+			input: vec![],
+			result: Some((x!(0), vec![]))
+		}),
+		subs: vec![]
+	});
+
+	assert_eq!(result.trace, expected_trace);
+}
+
 #[test]
 fn should_not_trace_call_transaction_to_builtin() {
 	init_log();
@@ -532,6 +570,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 +691,7 @@ fn should_trace_failed_call_transaction() {
 
 	assert_eq!(result.trace, expected_trace);
 }
+
 #[test]
 fn should_trace_call_with_subcall_transaction() {
 	init_log();
@@ -623,6 +743,95 @@ fn should_trace_call_with_subcall_transaction() {
 	assert_eq!(result.trace, expected_trace);
 }
 
+#[test]
+fn should_trace_call_with_basic_subcall_transaction() {
+	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 = TestEngine::new(5, Factory::default());
+
+	let t = Transaction {
+		nonce: x!(0),
+		gas_price: x!(0),
+		gas: x!(100_000),
+		action: Action::Call(x!(0xa)),
+		value: x!(100),
+		data: vec![],
+	}.sign(&"".sha3());
+
+	state.init_code(&x!(0xa), FromHex::from_hex("60006000600060006045600b6000f1").unwrap());
+	state.add_balance(t.sender().as_ref().unwrap(), &x!(100));
+	let result = state.apply(&info, &engine, &t, true).unwrap();
+	let expected_trace = Some(Trace {
+		depth: 0,
+		action: TraceAction::Call(TraceCall {
+			from: x!("9cce34f7ab185c7aba1b7c8140d620b4bda941d6"),
+			to: x!(0xa),
+			value: x!(100),
+			gas: x!(79000),
+			input: vec![],
+			result: Some((x!(31761), vec![]))
+		}),
+		subs: vec![Trace {
+			depth: 1,
+			action: TraceAction::Call(TraceCall {
+				from: x!(0xa),
+				to: x!(0xb),
+				value: x!(69),
+				gas: x!(2300),
+				input: vec![],
+				result: Some((x!(0), vec![]))
+			}),
+			subs: vec![]
+		}]
+	});
+
+	assert_eq!(result.trace, expected_trace);
+}
+
+#[test]
+fn should_not_trace_call_with_invalid_basic_subcall_transaction() {
+	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 = TestEngine::new(5, Factory::default());
+
+	let t = Transaction {
+		nonce: x!(0),
+		gas_price: x!(0),
+		gas: x!(100_000),
+		action: Action::Call(x!(0xa)),
+		value: x!(100),
+		data: vec![],
+	}.sign(&"".sha3());
+
+	state.init_code(&x!(0xa), FromHex::from_hex("600060006000600060ff600b6000f1").unwrap());	// not enough funds.
+	state.add_balance(t.sender().as_ref().unwrap(), &x!(100));
+	let result = state.apply(&info, &engine, &t, true).unwrap();
+	let expected_trace = Some(Trace {
+		depth: 0,
+		action: TraceAction::Call(TraceCall {
+			from: x!("9cce34f7ab185c7aba1b7c8140d620b4bda941d6"),
+			to: x!(0xa),
+			value: x!(100),
+			gas: x!(79000),
+			input: vec![],
+			result: Some((x!(31761), vec![]))
+		}),
+		subs: vec![]
+	});
+
+	assert_eq!(result.trace, expected_trace);
+}
+
 #[test]
 fn should_trace_failed_subcall_transaction() {
 	init_log();