Merge pull request #196 from ethcore/delegatecall

Fixing delegatecall

src/action_params.rs

@@ -3,8 +3,16 @@ use util::hash::*;
 use util::uint::*;
 use util::bytes::*;
 
-// TODO: should be a trait, possible to avoid cloning everything from a Transaction(/View).
+/// Transaction value
+#[derive(Clone, Debug)]
+pub enum ActionValue {
+	/// Value that should be transfered
+	Transfer(U256),
+	/// Apparent value for transaction (not transfered)
+	Apparent(U256)
+}
 
+// TODO: should be a trait, possible to avoid cloning everything from a Transaction(/View).
 /// Action (call/create) input params. Everything else should be specified in Externalities.
 #[derive(Clone, Debug)]
 pub struct ActionParams {
@@ -22,16 +30,16 @@ pub struct ActionParams {
 	/// Gas price.
 	pub gas_price: U256,
 	/// Transaction value.
-	pub value: U256,
+	pub value: ActionValue,
 	/// Code being executed.
 	pub code: Option<Bytes>,
 	/// Input data.
 	pub data: Option<Bytes>
 }
 
-impl ActionParams {
+impl Default for ActionParams {
-	/// TODO [Gav Wood] Please document me
+	/// Returns default ActionParams initialized with zeros
-	pub fn new() -> ActionParams {
+	fn default() -> ActionParams {
 		ActionParams {
 			code_address: Address::new(),
 			address: Address::new(),
@@ -39,7 +47,7 @@ impl ActionParams {
 			origin: Address::new(),
 			gas: U256::zero(),
 			gas_price: U256::zero(),
-			value: U256::zero(),
+			value: ActionValue::Transfer(U256::zero()),
 			code: None,
 			data: None
 		}

src/evm/ext.rs

@@ -26,7 +26,7 @@ pub enum MessageCallResult {
 	Failed
 }
 
-/// TODO [debris] Please document me
+/// Externalities interface for EVMs
 pub trait Ext {
 	/// Returns a value for given key.
 	fn storage_at(&self, key: &H256) -> H256;
@@ -55,8 +55,9 @@ pub trait Ext {
 	/// and true if subcall was successfull.
 	fn call(&mut self, 
 			gas: &U256, 
-			address: &Address, 
+			sender_address: &Address, 
-			value: &U256, 
+			receive_address: &Address, 
+			value: Option<U256>,
 			data: &[u8], 
 			code_address: &Address, 
 			output: &mut [u8]) -> MessageCallResult;

src/evm/interpreter.rs

@@ -571,16 +571,10 @@ impl Interpreter {
 				let call_gas = stack.pop_back();
 				let code_address = stack.pop_back();
 				let code_address = u256_to_address(&code_address);
-				let is_delegatecall = instruction == instructions::DELEGATECALL;
 
-				let value = match is_delegatecall {
+				let value = match instruction == instructions::DELEGATECALL {
-					true => params.value,
+					true => None,
-					false => stack.pop_back()
+					false => Some(stack.pop_back())
-				};
-
-				let address = match instruction == instructions::CALL {
-					true => &code_address,
-					false => &params.address
 				};
 
 				let in_off = stack.pop_back();
@@ -588,13 +582,27 @@ impl Interpreter {
 				let out_off = stack.pop_back();
 				let out_size = stack.pop_back();
 
-				let call_gas = call_gas + match !is_delegatecall && value > U256::zero() {
+				// Add stipend (only CALL|CALLCODE when value > 0)
+				let call_gas = call_gas + value.map_or_else(U256::zero, |val| match val > U256::zero() {
 					true => U256::from(ext.schedule().call_stipend),
 					false => U256::zero()
+				});
+
+				// Get sender & receive addresses, check if we have balance
+				let (sender_address, receive_address, has_balance) = match instruction {
+					instructions::CALL => {
+						let has_balance = ext.balance(&params.address) >= value.unwrap();
+						(&params.address, &code_address, has_balance)
+					},
+					instructions::CALLCODE => {
+						let has_balance = ext.balance(&params.address) >= value.unwrap();
+						(&params.address, &params.address, has_balance)
+					},
+					instructions::DELEGATECALL => (&params.sender, &params.address, true),
+					_ => panic!(format!("Unexpected instruction {} in CALL branch.", instruction))
 				};
 
-				let can_call = (is_delegatecall || ext.balance(&params.address) >= value) && ext.depth() < ext.schedule().max_depth;
+				let can_call = has_balance && ext.depth() < ext.schedule().max_depth;
-
 				if !can_call {
 					stack.push(U256::zero());
 					return Ok(InstructionResult::UnusedGas(call_gas));
@@ -605,7 +613,7 @@ impl Interpreter {
 					// and we don't want to copy
 					let input = unsafe { ::std::mem::transmute(mem.read_slice(in_off, in_size)) };
 					let output = mem.writeable_slice(out_off, out_size);
-					ext.call(&call_gas, address, &value, input, &code_address, output)
+					ext.call(&call_gas, sender_address, receive_address, value, input, &code_address, output)
 				};
 
 				return match call_result {
@@ -712,7 +720,10 @@ impl Interpreter {
 				stack.push(address_to_u256(params.sender.clone()));
 			},
 			instructions::CALLVALUE => {
-				stack.push(params.value.clone());
+				stack.push(match params.value {
+					ActionValue::Transfer(val) => val,
+					ActionValue::Apparent(val) => val,
+				});
 			},
 			instructions::CALLDATALOAD => {
 				let big_id = stack.pop_back();

src/evm/tests.rs

@@ -61,8 +61,9 @@ impl Ext for FakeExt {
 
 	fn call(&mut self, 
 			_gas: &U256, 
-			_address: &Address, 
+			_sender_address: &Address, 
-			_value: &U256, 
+			_receive_address: &Address, 
+			_value: Option<U256>,
 			_data: &[u8], 
 			_code_address: &Address, 
 			_output: &mut [u8]) -> MessageCallResult {
@@ -110,7 +111,7 @@ fn test_stack_underflow() {
 	let address = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 	let code = "01600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -137,7 +138,7 @@ fn test_add(factory: super::Factory) {
   let address = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 	let code = "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff01600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -157,7 +158,7 @@ fn test_sha3(factory: super::Factory) {
 	let address = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 	let code = "6000600020600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -177,7 +178,7 @@ fn test_address(factory: super::Factory) {
 	let address = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 	let code = "30600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -198,7 +199,7 @@ fn test_origin(factory: super::Factory) {
 	let origin = Address::from_str("cd1722f2947def4cf144679da39c4c32bdc35681").unwrap();
 	let code = "32600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.origin = origin.clone();
 	params.gas = U256::from(100_000);
@@ -220,7 +221,7 @@ fn test_sender(factory: super::Factory) {
 	let sender = Address::from_str("cd1722f2947def4cf144679da39c4c32bdc35681").unwrap();
 	let code = "33600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.sender = sender.clone();
 	params.gas = U256::from(100_000);
@@ -254,7 +255,7 @@ fn test_extcodecopy(factory: super::Factory) {
 	let code = "333b60006000333c600051600055".from_hex().unwrap();
 	let sender_code = "6005600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.sender = sender.clone();
 	params.gas = U256::from(100_000);
@@ -276,7 +277,7 @@ fn test_log_empty(factory: super::Factory) {
 	let address = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 	let code = "60006000a0".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -307,7 +308,7 @@ fn test_log_sender(factory: super::Factory) {
 	let sender = Address::from_str("cd1722f3947def4cf144679da39c4c32bdc35681").unwrap();
 	let code = "60ff6000533360206000a1".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.sender = sender.clone();
 	params.gas = U256::from(100_000);
@@ -332,7 +333,7 @@ fn test_blockhash(factory: super::Factory) {
 	let code = "600040600055".from_hex().unwrap();
 	let blockhash = H256::from_str("123400000000000000000000cd1722f2947def4cf144679da39c4c32bdc35681").unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -354,7 +355,7 @@ fn test_calldataload(factory: super::Factory) {
 	let code = "600135600055".from_hex().unwrap();
 	let data = "0123ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff23".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.address = address.clone();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
@@ -376,7 +377,7 @@ fn test_author(factory: super::Factory) {
 	let author = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 	let code = "41600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
 	let mut ext = FakeExt::new();
@@ -396,7 +397,7 @@ fn test_timestamp(factory: super::Factory) {
 	let timestamp = 0x1234; 
 	let code = "42600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
 	let mut ext = FakeExt::new();
@@ -416,7 +417,7 @@ fn test_number(factory: super::Factory) {
 	let number = 0x1234; 
 	let code = "43600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
 	let mut ext = FakeExt::new();
@@ -436,7 +437,7 @@ fn test_difficulty(factory: super::Factory) {
 	let difficulty = U256::from(0x1234);
 	let code = "44600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
 	let mut ext = FakeExt::new();
@@ -456,7 +457,7 @@ fn test_gas_limit(factory: super::Factory) {
 	let gas_limit = U256::from(0x1234);
 	let code = "45600055".from_hex().unwrap();
 
-	let mut params = ActionParams::new();
+	let mut params = ActionParams::default();
 	params.gas = U256::from(100_000);
 	params.code = Some(code);
 	let mut ext = FakeExt::new();

src/executive.rs

@@ -133,7 +133,7 @@ impl<'a> Executive<'a> {
 					origin: sender.clone(),
 					gas: init_gas,
 					gas_price: t.gas_price,
-					value: t.value,
+					value: ActionValue::Transfer(t.value),
 					code: Some(t.data.clone()),
 					data: None,
 				};
@@ -147,7 +147,7 @@ impl<'a> Executive<'a> {
 					origin: sender.clone(),
 					gas: init_gas,
 					gas_price: t.gas_price,
-					value: t.value,
+					value: ActionValue::Transfer(t.value),
 					code: self.state.code(address),
 					data: Some(t.data.clone()),
 				};
@@ -166,11 +166,14 @@ impl<'a> Executive<'a> {
 	/// Modifies the substate and the output.
 	/// Returns either gas_left or `evm::Error`.
 	pub fn call(&mut self, params: ActionParams, substate: &mut Substate, mut output: BytesRef) -> evm::Result {
+    println!("Calling executive. Sender: {}", params.sender);
 		// backup used in case of running out of gas
 		let backup = self.state.clone();
 
 		// at first, transfer value to destination
-		self.state.transfer_balance(&params.sender, &params.address, &params.value);
+		if let ActionValue::Transfer(val) = params.value {
+			self.state.transfer_balance(&params.sender, &params.address, &val);
+		}
 		trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info);
 
 		if self.engine.is_builtin(&params.code_address) {
@@ -227,7 +230,9 @@ impl<'a> Executive<'a> {
 		self.state.new_contract(&params.address);
 
 		// then transfer value to it
-		self.state.transfer_balance(&params.sender, &params.address, &params.value);
+		if let ActionValue::Transfer(val) = params.value {
+			self.state.transfer_balance(&params.sender, &params.address, &val);
+		}
 
 		let res = {
 			let mut ext = self.as_externalities(OriginInfo::from(&params), &mut unconfirmed_substate, OutputPolicy::InitContract);
@@ -363,12 +368,12 @@ mod tests {
 	fn test_sender_balance(factory: Factory) {
 		let sender = Address::from_str("0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6").unwrap();
 		let address = contract_address(&sender, &U256::zero());
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address.clone();
 		params.sender = sender.clone();
 		params.gas = U256::from(100_000);
 		params.code = Some("3331600055".from_hex().unwrap());
-		params.value = U256::from(0x7);
+		params.value = ActionValue::Transfer(U256::from(0x7));
 		let mut state = State::new_temp();
 		state.add_balance(&sender, &U256::from(0x100u64));
 		let info = EnvInfo::new();
@@ -420,13 +425,13 @@ mod tests {
 		let address = contract_address(&sender, &U256::zero());
 		// TODO: add tests for 'callcreate'
 		//let next_address = contract_address(&address, &U256::zero());
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address.clone();
 		params.sender = sender.clone();
 		params.origin = sender.clone();
 		params.gas = U256::from(100_000);
 		params.code = Some(code.clone());
-		params.value = U256::from(100);
+		params.value = ActionValue::Transfer(U256::from(100));
 		let mut state = State::new_temp();
 		state.add_balance(&sender, &U256::from(100));
 		let info = EnvInfo::new();
@@ -473,13 +478,13 @@ mod tests {
 		let address = contract_address(&sender, &U256::zero());
 		// TODO: add tests for 'callcreate'
 		//let next_address = contract_address(&address, &U256::zero());
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address.clone();
 		params.sender = sender.clone();
 		params.origin = sender.clone();
 		params.gas = U256::from(100_000);
 		params.code = Some(code.clone());
-		params.value = U256::from(100);
+		params.value = ActionValue::Transfer(U256::from(100));
 		let mut state = State::new_temp();
 		state.add_balance(&sender, &U256::from(100));
 		let info = EnvInfo::new();
@@ -524,13 +529,13 @@ mod tests {
 		let sender = Address::from_str("cd1722f3947def4cf144679da39c4c32bdc35681").unwrap();
 		let address = contract_address(&sender, &U256::zero());
 		let next_address = contract_address(&address, &U256::zero());
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address.clone();
 		params.sender = sender.clone();
 		params.origin = sender.clone();
 		params.gas = U256::from(100_000);
 		params.code = Some(code.clone());
-		params.value = U256::from(100);
+		params.value = ActionValue::Transfer(U256::from(100));
 		let mut state = State::new_temp();
 		state.add_balance(&sender, &U256::from(100));
 		let info = EnvInfo::new();
@@ -580,12 +585,12 @@ mod tests {
 		let address_b = Address::from_str("945304eb96065b2a98b57a48a06ae28d285a71b5" ).unwrap();
 		let sender = Address::from_str("cd1722f3947def4cf144679da39c4c32bdc35681").unwrap();
 
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address_a.clone();
 		params.sender = sender.clone();
 		params.gas = U256::from(100_000);
 		params.code = Some(code_a.clone());
-		params.value = U256::from(100_000);
+		params.value = ActionValue::Transfer(U256::from(100_000));
 
 		let mut state = State::new_temp();
 		state.init_code(&address_a, code_a.clone());
@@ -629,7 +634,7 @@ mod tests {
 		let sender = Address::from_str("cd1722f3947def4cf144679da39c4c32bdc35681").unwrap();
 		let code = "600160005401600055600060006000600060003060e05a03f1600155".from_hex().unwrap();
 		let address = contract_address(&sender, &U256::zero());
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address.clone();
 		params.gas = U256::from(100_000);
 		params.code = Some(code.clone());
@@ -785,13 +790,13 @@ mod tests {
 		let address = contract_address(&sender, &U256::zero());
 		// TODO: add tests for 'callcreate'
 		//let next_address = contract_address(&address, &U256::zero());
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		params.address = address.clone();
 		params.sender = sender.clone();
 		params.origin = sender.clone();
 		params.gas = U256::from(0x0186a0);
 		params.code = Some(code.clone());
-		params.value = U256::from_str("0de0b6b3a7640000").unwrap();
+		params.value = ActionValue::Transfer(U256::from_str("0de0b6b3a7640000").unwrap());
 		let mut state = State::new_temp();
 		state.add_balance(&sender, &U256::from_str("152d02c7e14af6800000").unwrap());
 		let info = EnvInfo::new();

src/externalities.rs

@@ -19,7 +19,8 @@ pub enum OutputPolicy<'a> {
 pub struct OriginInfo {
 	address: Address,
 	origin: Address,
-	gas_price: U256
+	gas_price: U256,
+	value: U256
 }
 
 impl OriginInfo {
@@ -28,7 +29,11 @@ impl OriginInfo {
 		OriginInfo {
 			address: params.address.clone(),
 			origin: params.origin.clone(),
-			gas_price: params.gas_price.clone()
+			gas_price: params.gas_price.clone(),
+			value: match params.value {
+				ActionValue::Transfer(val) => val,
+				ActionValue::Apparent(val) => val,
+			}
 		}
 	}
 }
@@ -111,7 +116,7 @@ impl<'a> Ext for Externalities<'a> {
 			origin: self.origin_info.origin.clone(),
 			gas: *gas,
 			gas_price: self.origin_info.gas_price.clone(),
-			value: value.clone(),
+			value: ActionValue::Transfer(value.clone()),
 			code: Some(code.to_vec()),
 			data: None,
 		};
@@ -131,24 +136,29 @@ impl<'a> Ext for Externalities<'a> {
 
 	fn call(&mut self, 
 			gas: &U256, 
-			address: &Address, 
+			sender_address: &Address, 
-			value: &U256, 
+			receive_address: &Address, 
+			value: Option<U256>,
 			data: &[u8], 
 			code_address: &Address, 
 			output: &mut [u8]) -> MessageCallResult {
 
-		let params = ActionParams {
+		let mut params = ActionParams {
+			sender: sender_address.clone(),
+			address: receive_address.clone(), 
+			value: ActionValue::Apparent(self.origin_info.value.clone()),
 			code_address: code_address.clone(),
-			address: address.clone(), 
-			sender: self.origin_info.address.clone(),
 			origin: self.origin_info.origin.clone(),
 			gas: *gas,
 			gas_price: self.origin_info.gas_price.clone(),
-			value: value.clone(),
 			code: self.state.code(code_address),
 			data: Some(data.to_vec()),
 		};
 
+		if let Some(value) = value {
+			params.value = ActionValue::Transfer(value);
+		}
+
 		let mut ex = Executive::from_parent(self.state, self.env_info, self.engine, self.depth);
 
 		match ex.call(params, self.substate, BytesRef::Fixed(output)) {

src/tests/executive.rs

@@ -101,8 +101,9 @@ impl<'a> Ext for TestExt<'a> {
 
 	fn call(&mut self, 
 			gas: &U256, 
+			_sender_address: &Address, 
 			receive_address: &Address, 
-			value: &U256, 
+			value: Option<U256>,
 			data: &[u8], 
 			_code_address: &Address, 
 			_output: &mut [u8]) -> MessageCallResult {
@@ -110,7 +111,7 @@ impl<'a> Ext for TestExt<'a> {
 			data: data.to_vec(),
 			destination: Some(receive_address.clone()),
 			gas_limit: *gas,
-			value: *value
+			value: value.unwrap()
 		});
 		MessageCallResult::Success(*gas)
 	}
@@ -194,7 +195,7 @@ fn do_json_test_for(vm: &VMType, json_data: &[u8]) -> Vec<String> {
 		let engine = TestEngine::new(1, vm.clone());
 
 		// params
-		let mut params = ActionParams::new();
+		let mut params = ActionParams::default();
 		test.find("exec").map(|exec| {
 			params.address = xjson!(&exec["address"]);
 			params.sender = xjson!(&exec["caller"]);
@@ -203,7 +204,7 @@ fn do_json_test_for(vm: &VMType, json_data: &[u8]) -> Vec<String> {
 			params.data = xjson!(&exec["data"]);
 			params.gas = xjson!(&exec["gas"]);
 			params.gas_price = xjson!(&exec["gasPrice"]);
-			params.value = xjson!(&exec["value"]);
+			params.value = ActionValue::Transfer(xjson!(&exec["value"]));
 		});
 
 		let out_of_gas = test.find("callcreates").map(|_calls| {