From aa0760597b1d33615731aa06192595dd3fcf09a7 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Wed, 20 Jan 2016 16:52:22 +0100 Subject: [PATCH 1/7] Fixing delegatecall --- src/action_params.rs | 11 ++++++---- src/evm/ext.rs | 14 +++++++++++- src/evm/interpreter.rs | 49 +++++++++++++++++++++++++++++++++++------- src/evm/tests.rs | 41 +++++++++++++++++++++-------------- src/executive.rs | 25 +++++++++++++-------- src/externalities.rs | 32 +++++++++++++++++++++++++++ src/tests/executive.rs | 18 +++++++++++++++- 7 files changed, 151 insertions(+), 39 deletions(-) diff --git a/src/action_params.rs b/src/action_params.rs index da1ae0ce0..6f876874b 100644 --- a/src/action_params.rs +++ b/src/action_params.rs @@ -23,15 +23,17 @@ pub struct ActionParams { pub gas_price: U256, /// Transaction value. pub value: U256, + /// Should transfer value from sender to origin + pub is_value_transfer: bool, /// Code being executed. pub code: Option, /// Input data. pub data: Option } -impl ActionParams { - /// TODO [Gav Wood] Please document me - pub fn new() -> ActionParams { +impl Default for ActionParams { + /// Returns default ActionParams initialized with zeros + fn default() -> ActionParams { ActionParams { code_address: Address::new(), address: Address::new(), @@ -41,7 +43,8 @@ impl ActionParams { gas_price: U256::zero(), value: U256::zero(), code: None, - data: None + data: None, + is_value_transfer: true } } } diff --git a/src/evm/ext.rs b/src/evm/ext.rs index 4d2471593..b7bf609ca 100644 --- a/src/evm/ext.rs +++ b/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; @@ -61,6 +61,18 @@ pub trait Ext { code_address: &Address, output: &mut [u8]) -> MessageCallResult; + /// Delegate Message call. + /// + /// Returns Err, if we run out of gas. + /// Otherwise returns call_result which contains gas left + /// and true if subcall was successfull. + fn delegatecall(&mut self, + gas: &U256, + value: &U256, + data: &[u8], + code_address: &Address, + output: &mut [u8]) -> MessageCallResult; + /// Returns code at given address fn extcode(&self, address: &Address) -> Bytes; diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index 88823cbea..c7d2bfda8 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -564,17 +564,50 @@ impl Interpreter { } }; }, - instructions::CALL | instructions::CALLCODE | instructions::DELEGATECALL => { + instructions::DELEGATECALL => { + let call_gas = stack.pop_back(); + let code_address = stack.pop_back(); + let code_address = u256_to_address(&code_address); + + let value = params.value; + + let in_off = stack.pop_back(); + let in_size = stack.pop_back(); + let out_off = stack.pop_back(); + let out_size = stack.pop_back(); + + let can_call = ext.depth() < ext.schedule().max_depth; + if !can_call { + stack.push(U256::zero()); + return Ok(InstructionResult::UnusedGas(call_gas)); + } + + let call_result = { + // we need to write and read from memory in the same time + // 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.delegatecall(&call_gas, &value, input, &code_address, output) + }; + + return match call_result { + MessageCallResult::Success(gas_left) => { + stack.push(U256::one()); + Ok(InstructionResult::UnusedGas(gas_left)) + }, + MessageCallResult::Failed => { + stack.push(U256::zero()); + Ok(InstructionResult::Ok) + } + }; + }, + instructions::CALL | instructions::CALLCODE => { assert!(ext.schedule().call_value_transfer_gas > ext.schedule().call_stipend, "overflow possible"); 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 { - true => params.value, - false => stack.pop_back() - }; + let value = stack.pop_back(); let address = match instruction == instructions::CALL { true => &code_address, @@ -586,12 +619,12 @@ 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() { + let call_gas = call_gas + match value > U256::zero() { true => U256::from(ext.schedule().call_stipend), false => U256::zero() }; - let can_call = (is_delegatecall || ext.balance(¶ms.address) >= value) && ext.depth() < ext.schedule().max_depth; + let can_call = ext.balance(¶ms.address) >= value && ext.depth() < ext.schedule().max_depth; if !can_call { stack.push(U256::zero()); diff --git a/src/evm/tests.rs b/src/evm/tests.rs index 8e1b5eff4..7eb9d484c 100644 --- a/src/evm/tests.rs +++ b/src/evm/tests.rs @@ -69,6 +69,15 @@ impl Ext for FakeExt { unimplemented!(); } + fn delegatecall(&mut self, + _gas: &U256, + _value: &U256, + _data: &[u8], + _address: &Address, + _output: &mut [u8]) -> MessageCallResult { + unimplemented!(); + } + fn extcode(&self, address: &Address) -> Bytes { self.codes.get(address).unwrap_or(&Bytes::new()).clone() } @@ -110,7 +119,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 +146,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 +166,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 +186,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 +207,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 +229,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 +263,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 +285,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 +316,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 +341,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 +363,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 +385,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 +405,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 +425,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 +445,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 +465,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(); diff --git a/src/executive.rs b/src/executive.rs index 6c2b29e3f..ea1a8f6de 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -134,6 +134,7 @@ impl<'a> Executive<'a> { gas: init_gas, gas_price: t.gas_price, value: t.value, + is_value_transfer: true, code: Some(t.data.clone()), data: None, }; @@ -148,6 +149,7 @@ impl<'a> Executive<'a> { gas: init_gas, gas_price: t.gas_price, value: t.value, + is_value_transfer: true, code: self.state.code(address), data: Some(t.data.clone()), }; @@ -166,11 +168,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(¶ms.sender, ¶ms.address, ¶ms.value); + if params.is_value_transfer { + self.state.transfer_balance(¶ms.sender, ¶ms.address, ¶ms.value); + } trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); if self.engine.is_builtin(¶ms.code_address) { @@ -227,7 +232,9 @@ impl<'a> Executive<'a> { self.state.new_contract(¶ms.address); // then transfer value to it - self.state.transfer_balance(¶ms.sender, ¶ms.address, ¶ms.value); + if params.is_value_transfer { + self.state.transfer_balance(¶ms.sender, ¶ms.address, ¶ms.value); + } let res = { let mut ext = self.to_externalities(OriginInfo::from(¶ms), &mut unconfirmed_substate, OutputPolicy::InitContract); @@ -367,7 +374,7 @@ 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); @@ -424,7 +431,7 @@ 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(); @@ -477,7 +484,7 @@ 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(); @@ -528,7 +535,7 @@ 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(); @@ -584,7 +591,7 @@ 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); @@ -633,7 +640,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()); @@ -789,7 +796,7 @@ 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(); diff --git a/src/externalities.rs b/src/externalities.rs index 8b16cc72b..110b965d7 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -17,6 +17,7 @@ pub enum OutputPolicy<'a> { /// Transaction properties that externalities need to know about. pub struct OriginInfo { + sender: Address, address: Address, origin: Address, gas_price: U256 @@ -26,6 +27,7 @@ impl OriginInfo { /// Populates origin info from action params. pub fn from(params: &ActionParams) -> Self { OriginInfo { + sender: params.sender.clone(), address: params.address.clone(), origin: params.origin.clone(), gas_price: params.gas_price.clone() @@ -112,6 +114,7 @@ impl<'a> Ext for Externalities<'a> { gas: *gas, gas_price: self.origin_info.gas_price.clone(), value: value.clone(), + is_value_transfer: true, code: Some(code.to_vec()), data: None, }; @@ -129,6 +132,34 @@ impl<'a> Ext for Externalities<'a> { } } + fn delegatecall(&mut self, + gas: &U256, + value: &U256, + data: &[u8], + code_address: &Address, + output: &mut [u8]) -> MessageCallResult { + + let params = ActionParams { + code_address: code_address.clone(), + address: self.origin_info.address.clone(), + sender: self.origin_info.sender.clone(), + origin: self.origin_info.origin.clone(), + gas: *gas, + gas_price: self.origin_info.gas_price.clone(), + value: value.clone(), + is_value_transfer: false, + code: self.state.code(code_address), + data: Some(data.to_vec()), + }; + + let mut ex = Executive::from_parent(self.state, self.env_info, self.engine, self.depth); + + match ex.call(params, self.substate, BytesRef::Fixed(output)) { + Ok(gas_left) => MessageCallResult::Success(gas_left), + _ => MessageCallResult::Failed + } + } + fn call(&mut self, gas: &U256, address: &Address, @@ -145,6 +176,7 @@ impl<'a> Ext for Externalities<'a> { gas: *gas, gas_price: self.origin_info.gas_price.clone(), value: value.clone(), + is_value_transfer: true, code: self.state.code(code_address), data: Some(data.to_vec()), }; diff --git a/src/tests/executive.rs b/src/tests/executive.rs index fe428e199..ca18850c4 100644 --- a/src/tests/executive.rs +++ b/src/tests/executive.rs @@ -115,6 +115,22 @@ impl<'a> Ext for TestExt<'a> { MessageCallResult::Success(*gas) } + fn delegatecall(&mut self, + gas: &U256, + value: &U256, + data: &[u8], + _code_address: &Address, + _output: &mut [u8]) -> MessageCallResult { + + self.callcreates.push(CallCreate { + data: data.to_vec(), + destination: None, + gas_limit: *gas, + value: *value + }); + MessageCallResult::Success(*gas) + } + fn extcode(&self, address: &Address) -> Bytes { self.ext.extcode(address) } @@ -200,7 +216,7 @@ fn do_json_test_for(vm: &VMType, json_data: &[u8]) -> Vec { 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"]); From 8084e1b6d7288db1c6e6f91d8f1ffdb138217ab2 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Wed, 20 Jan 2016 17:01:58 +0100 Subject: [PATCH 2/7] Removing value from delegatecall function --- src/evm/ext.rs | 1 - src/evm/interpreter.rs | 4 +--- src/evm/tests.rs | 1 - src/externalities.rs | 5 +++-- src/tests/executive.rs | 3 +-- 5 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/evm/ext.rs b/src/evm/ext.rs index b7bf609ca..7e2f0f47f 100644 --- a/src/evm/ext.rs +++ b/src/evm/ext.rs @@ -68,7 +68,6 @@ pub trait Ext { /// and true if subcall was successfull. fn delegatecall(&mut self, gas: &U256, - value: &U256, data: &[u8], code_address: &Address, output: &mut [u8]) -> MessageCallResult; diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index c7d2bfda8..a5e8efc91 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -569,8 +569,6 @@ impl Interpreter { let code_address = stack.pop_back(); let code_address = u256_to_address(&code_address); - let value = params.value; - let in_off = stack.pop_back(); let in_size = stack.pop_back(); let out_off = stack.pop_back(); @@ -587,7 +585,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.delegatecall(&call_gas, &value, input, &code_address, output) + ext.delegatecall(&call_gas, input, &code_address, output) }; return match call_result { diff --git a/src/evm/tests.rs b/src/evm/tests.rs index 7eb9d484c..83c58bbf8 100644 --- a/src/evm/tests.rs +++ b/src/evm/tests.rs @@ -71,7 +71,6 @@ impl Ext for FakeExt { fn delegatecall(&mut self, _gas: &U256, - _value: &U256, _data: &[u8], _address: &Address, _output: &mut [u8]) -> MessageCallResult { diff --git a/src/externalities.rs b/src/externalities.rs index 110b965d7..20326fe03 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -18,6 +18,7 @@ pub enum OutputPolicy<'a> { /// Transaction properties that externalities need to know about. pub struct OriginInfo { sender: Address, + value: U256, address: Address, origin: Address, gas_price: U256 @@ -28,6 +29,7 @@ impl OriginInfo { pub fn from(params: &ActionParams) -> Self { OriginInfo { sender: params.sender.clone(), + value: params.value.clone(), address: params.address.clone(), origin: params.origin.clone(), gas_price: params.gas_price.clone() @@ -134,7 +136,6 @@ impl<'a> Ext for Externalities<'a> { fn delegatecall(&mut self, gas: &U256, - value: &U256, data: &[u8], code_address: &Address, output: &mut [u8]) -> MessageCallResult { @@ -146,7 +147,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: self.origin_info.value.clone(), is_value_transfer: false, code: self.state.code(code_address), data: Some(data.to_vec()), diff --git a/src/tests/executive.rs b/src/tests/executive.rs index ca18850c4..17cfb7df5 100644 --- a/src/tests/executive.rs +++ b/src/tests/executive.rs @@ -117,7 +117,6 @@ impl<'a> Ext for TestExt<'a> { fn delegatecall(&mut self, gas: &U256, - value: &U256, data: &[u8], _code_address: &Address, _output: &mut [u8]) -> MessageCallResult { @@ -126,7 +125,7 @@ impl<'a> Ext for TestExt<'a> { data: data.to_vec(), destination: None, gas_limit: *gas, - value: *value + value: U256::zero() }); MessageCallResult::Success(*gas) } From cd9a0e4e588215b6e2839c9e8475b0ddecab3c38 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Wed, 20 Jan 2016 17:27:33 +0100 Subject: [PATCH 3/7] Changing is_value_transfer to enum --- src/action_params.rs | 19 ++++++++++++------- src/evm/interpreter.rs | 5 ++++- src/executive.rs | 14 ++++++-------- src/externalities.rs | 16 ++++++++-------- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/action_params.rs b/src/action_params.rs index 6f876874b..a56e64f4d 100644 --- a/src/action_params.rs +++ b/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,9 +30,7 @@ pub struct ActionParams { /// Gas price. pub gas_price: U256, /// Transaction value. - pub value: U256, - /// Should transfer value from sender to origin - pub is_value_transfer: bool, + pub value: ActionValue, /// Code being executed. pub code: Option, /// Input data. @@ -41,10 +47,9 @@ impl Default for ActionParams { origin: Address::new(), gas: U256::zero(), gas_price: U256::zero(), - value: U256::zero(), + value: ActionValue::Transfer(U256::zero()), code: None, - data: None, - is_value_transfer: true + data: None } } } diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index a5e8efc91..52509a2c0 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -741,7 +741,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(); diff --git a/src/executive.rs b/src/executive.rs index ea1a8f6de..dd61a5b97 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -133,8 +133,7 @@ impl<'a> Executive<'a> { origin: sender.clone(), gas: init_gas, gas_price: t.gas_price, - value: t.value, - is_value_transfer: true, + value: ActionValue::Transfer(t.value), code: Some(t.data.clone()), data: None, }; @@ -148,8 +147,7 @@ impl<'a> Executive<'a> { origin: sender.clone(), gas: init_gas, gas_price: t.gas_price, - value: t.value, - is_value_transfer: true, + value: ActionValue::Transfer(t.value), code: self.state.code(address), data: Some(t.data.clone()), }; @@ -173,8 +171,8 @@ impl<'a> Executive<'a> { let backup = self.state.clone(); // at first, transfer value to destination - if params.is_value_transfer { - self.state.transfer_balance(¶ms.sender, ¶ms.address, ¶ms.value); + if let ActionValue::Transfer(val) = params.value { + self.state.transfer_balance(¶ms.sender, ¶ms.address, &val); } trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); @@ -232,8 +230,8 @@ impl<'a> Executive<'a> { self.state.new_contract(¶ms.address); // then transfer value to it - if params.is_value_transfer { - self.state.transfer_balance(¶ms.sender, ¶ms.address, ¶ms.value); + if let ActionValue::Transfer(val) = params.value { + self.state.transfer_balance(¶ms.sender, ¶ms.address, &val); } let res = { diff --git a/src/externalities.rs b/src/externalities.rs index 20326fe03..a9a14c5c2 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -29,10 +29,13 @@ impl OriginInfo { pub fn from(params: &ActionParams) -> Self { OriginInfo { sender: params.sender.clone(), - value: params.value.clone(), 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, + } } } } @@ -115,8 +118,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(), - is_value_transfer: true, + value: ActionValue::Transfer(value.clone()), code: Some(code.to_vec()), data: None, }; @@ -147,8 +149,7 @@ impl<'a> Ext for Externalities<'a> { origin: self.origin_info.origin.clone(), gas: *gas, gas_price: self.origin_info.gas_price.clone(), - value: self.origin_info.value.clone(), - is_value_transfer: false, + value: ActionValue::Apparent(self.origin_info.value.clone()), code: self.state.code(code_address), data: Some(data.to_vec()), }; @@ -176,8 +177,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(), - is_value_transfer: true, + value: ActionValue::Transfer(value.clone()), code: self.state.code(code_address), data: Some(data.to_vec()), }; From 651d2d66e04240862df7b00ff0bd765c9a354ac6 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Wed, 20 Jan 2016 17:31:37 +0100 Subject: [PATCH 4/7] Fixing tests --- src/executive.rs | 12 ++++++------ src/tests/executive.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/executive.rs b/src/executive.rs index dd61a5b97..f5fa9cbc7 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -377,7 +377,7 @@ mod tests { 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(); @@ -435,7 +435,7 @@ mod tests { 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(); @@ -488,7 +488,7 @@ mod tests { 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(); @@ -539,7 +539,7 @@ mod tests { 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(); @@ -594,7 +594,7 @@ mod tests { 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()); @@ -800,7 +800,7 @@ mod tests { 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(); diff --git a/src/tests/executive.rs b/src/tests/executive.rs index 17cfb7df5..bc88a4697 100644 --- a/src/tests/executive.rs +++ b/src/tests/executive.rs @@ -224,7 +224,7 @@ fn do_json_test_for(vm: &VMType, json_data: &[u8]) -> Vec { 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| { From e27d628e753a94924cb32e698fc9e2ef022fe0a7 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Sat, 23 Jan 2016 10:41:13 +0100 Subject: [PATCH 5/7] Changing delegatecall logic --- Cargo.toml | 2 +- src/evm/ext.rs | 16 +++----------- src/evm/interpreter.rs | 14 ++++++------ src/evm/tests.rs | 13 +++-------- src/externalities.rs | 49 +++++++++++------------------------------- src/tests/executive.rs | 20 +++-------------- 6 files changed, 30 insertions(+), 84 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 714af5693..d8c92e0aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ evmjit = { path = "rust-evmjit", optional = true } ethash = { path = "ethash" } num_cpus = "0.2" ctrlc = "1.0" -clippy = "*" # Always newest, since we use nightly +clippy = "0.0.37" # Always newest, since we use nightly [features] jit = ["evmjit"] diff --git a/src/evm/ext.rs b/src/evm/ext.rs index 7e2f0f47f..748bc89da 100644 --- a/src/evm/ext.rs +++ b/src/evm/ext.rs @@ -55,19 +55,9 @@ pub trait Ext { /// and true if subcall was successfull. fn call(&mut self, gas: &U256, - address: &Address, - value: &U256, - data: &[u8], - code_address: &Address, - output: &mut [u8]) -> MessageCallResult; - - /// Delegate Message call. - /// - /// Returns Err, if we run out of gas. - /// Otherwise returns call_result which contains gas left - /// and true if subcall was successfull. - fn delegatecall(&mut self, - gas: &U256, + sender_address: &Address, + receive_address: &Address, + value: Option<&U256>, data: &[u8], code_address: &Address, output: &mut [u8]) -> MessageCallResult; diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index 7f0db421d..276f2873b 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -587,7 +587,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.delegatecall(&call_gas, input, &code_address, output) + ext.call(&call_gas, ¶ms.sender, ¶ms.address, None, input, &code_address, output) }; return match call_result { @@ -609,11 +609,6 @@ impl Interpreter { let value = stack.pop_back(); - let address = match instruction == instructions::CALL { - true => &code_address, - false => ¶ms.address - }; - let in_off = stack.pop_back(); let in_size = stack.pop_back(); let out_off = stack.pop_back(); @@ -624,6 +619,11 @@ impl Interpreter { false => U256::zero() }; + let (sender_address, receive_address) = match instruction == instructions::CALL { + true => (¶ms.address, &code_address), + false => (¶ms.address, ¶ms.address) + }; + let can_call = ext.balance(¶ms.address) >= value && ext.depth() < ext.schedule().max_depth; if !can_call { @@ -636,7 +636,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, Some(&value), input, &code_address, output) }; return match call_result { diff --git a/src/evm/tests.rs b/src/evm/tests.rs index aaf082093..cf4262914 100644 --- a/src/evm/tests.rs +++ b/src/evm/tests.rs @@ -61,22 +61,15 @@ impl Ext for FakeExt { fn call(&mut self, _gas: &U256, - _address: &Address, - _value: &U256, + _sender_address: &Address, + _receive_address: &Address, + _value: Option<&U256>, _data: &[u8], _code_address: &Address, _output: &mut [u8]) -> MessageCallResult { unimplemented!(); } - fn delegatecall(&mut self, - _gas: &U256, - _data: &[u8], - _address: &Address, - _output: &mut [u8]) -> MessageCallResult { - unimplemented!(); - } - fn extcode(&self, address: &Address) -> Bytes { self.codes.get(address).unwrap_or(&Bytes::new()).clone() } diff --git a/src/externalities.rs b/src/externalities.rs index 293a60999..d8b5d6110 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -17,18 +17,16 @@ pub enum OutputPolicy<'a> { /// Transaction properties that externalities need to know about. pub struct OriginInfo { - sender: Address, - value: U256, address: Address, origin: Address, - gas_price: U256 + gas_price: U256, + value: U256 } impl OriginInfo { /// Populates origin info from action params. pub fn from(params: &ActionParams) -> Self { OriginInfo { - sender: params.sender.clone(), address: params.address.clone(), origin: params.origin.clone(), gas_price: params.gas_price.clone(), @@ -136,52 +134,31 @@ impl<'a> Ext for Externalities<'a> { } } - fn delegatecall(&mut self, - gas: &U256, - data: &[u8], - code_address: &Address, - output: &mut [u8]) -> MessageCallResult { - - let params = ActionParams { - code_address: code_address.clone(), - address: self.origin_info.address.clone(), - sender: self.origin_info.sender.clone(), - origin: self.origin_info.origin.clone(), - gas: *gas, - gas_price: self.origin_info.gas_price.clone(), - value: ActionValue::Apparent(self.origin_info.value.clone()), - code: self.state.code(code_address), - data: Some(data.to_vec()), - }; - - let mut ex = Executive::from_parent(self.state, self.env_info, self.engine, self.depth); - - match ex.call(params, self.substate, BytesRef::Fixed(output)) { - Ok(gas_left) => MessageCallResult::Success(gas_left), - _ => MessageCallResult::Failed - } - } - fn call(&mut self, gas: &U256, - address: &Address, - value: &U256, + sender_address: &Address, + 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: ActionValue::Transfer(value.clone()), code: self.state.code(code_address), data: Some(data.to_vec()), }; + if let Some(value) = value { + params.value = ActionValue::Transfer(value.clone()); + } + let mut ex = Executive::from_parent(self.state, self.env_info, self.engine, self.depth); match ex.call(params, self.substate, BytesRef::Fixed(output)) { diff --git a/src/tests/executive.rs b/src/tests/executive.rs index cfca30740..0604c9992 100644 --- a/src/tests/executive.rs +++ b/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,22 +111,7 @@ impl<'a> Ext for TestExt<'a> { data: data.to_vec(), destination: Some(receive_address.clone()), gas_limit: *gas, - value: *value - }); - MessageCallResult::Success(*gas) - } - - fn delegatecall(&mut self, - gas: &U256, - data: &[u8], - _code_address: &Address, - _output: &mut [u8]) -> MessageCallResult { - - self.callcreates.push(CallCreate { - data: data.to_vec(), - destination: None, - gas_limit: *gas, - value: U256::zero() + value: *value.unwrap() }); MessageCallResult::Success(*gas) } From 58bda2209c70783de13d5eee0c66078959bdbef5 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Sat, 23 Jan 2016 10:42:25 +0100 Subject: [PATCH 6/7] Reverting clippy wildcard --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d8c92e0aa..714af5693 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ evmjit = { path = "rust-evmjit", optional = true } ethash = { path = "ethash" } num_cpus = "0.2" ctrlc = "1.0" -clippy = "0.0.37" # Always newest, since we use nightly +clippy = "*" # Always newest, since we use nightly [features] jit = ["evmjit"] From 16f2fa33ee9142ae8633b1a54998cf4a290bacc8 Mon Sep 17 00:00:00 2001 From: Tomusdrw Date: Mon, 25 Jan 2016 23:59:50 +0100 Subject: [PATCH 7/7] Merging CALL & DELEGATECALL branches --- src/evm/ext.rs | 2 +- src/evm/interpreter.rs | 69 ++++++++++++++---------------------------- src/evm/tests.rs | 2 +- src/externalities.rs | 4 +-- src/tests/executive.rs | 4 +-- 5 files changed, 29 insertions(+), 52 deletions(-) diff --git a/src/evm/ext.rs b/src/evm/ext.rs index 748bc89da..83f093bcf 100644 --- a/src/evm/ext.rs +++ b/src/evm/ext.rs @@ -57,7 +57,7 @@ pub trait Ext { gas: &U256, sender_address: &Address, receive_address: &Address, - value: Option<&U256>, + value: Option, data: &[u8], code_address: &Address, output: &mut [u8]) -> MessageCallResult; diff --git a/src/evm/interpreter.rs b/src/evm/interpreter.rs index 276f2873b..7657b1bbe 100644 --- a/src/evm/interpreter.rs +++ b/src/evm/interpreter.rs @@ -566,66 +566,43 @@ impl Interpreter { } }; }, - instructions::DELEGATECALL => { - let call_gas = stack.pop_back(); - let code_address = stack.pop_back(); - let code_address = u256_to_address(&code_address); - - let in_off = stack.pop_back(); - let in_size = stack.pop_back(); - let out_off = stack.pop_back(); - let out_size = stack.pop_back(); - - let can_call = ext.depth() < ext.schedule().max_depth; - if !can_call { - stack.push(U256::zero()); - return Ok(InstructionResult::UnusedGas(call_gas)); - } - - let call_result = { - // we need to write and read from memory in the same time - // 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, ¶ms.sender, ¶ms.address, None, input, &code_address, output) - }; - - return match call_result { - MessageCallResult::Success(gas_left) => { - stack.push(U256::one()); - Ok(InstructionResult::UnusedGas(gas_left)) - }, - MessageCallResult::Failed => { - stack.push(U256::zero()); - Ok(InstructionResult::Ok) - } - }; - }, - instructions::CALL | instructions::CALLCODE => { + instructions::CALL | instructions::CALLCODE | instructions::DELEGATECALL => { assert!(ext.schedule().call_value_transfer_gas > ext.schedule().call_stipend, "overflow possible"); let call_gas = stack.pop_back(); let code_address = stack.pop_back(); let code_address = u256_to_address(&code_address); - let value = stack.pop_back(); + let value = match instruction == instructions::DELEGATECALL { + true => None, + false => Some(stack.pop_back()) + }; let in_off = stack.pop_back(); let in_size = stack.pop_back(); let out_off = stack.pop_back(); let out_size = stack.pop_back(); - let call_gas = call_gas + match 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(¶ms.address) >= value.unwrap(); + (¶ms.address, &code_address, has_balance) + }, + instructions::CALLCODE => { + let has_balance = ext.balance(¶ms.address) >= value.unwrap(); + (¶ms.address, ¶ms.address, has_balance) + }, + instructions::DELEGATECALL => (¶ms.sender, ¶ms.address, true), + _ => panic!(format!("Unexpected instruction {} in CALL branch.", instruction)) }; - let (sender_address, receive_address) = match instruction == instructions::CALL { - true => (¶ms.address, &code_address), - false => (¶ms.address, ¶ms.address) - }; - - let can_call = ext.balance(¶ms.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)); @@ -636,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, sender_address, receive_address, Some(&value), input, &code_address, output) + ext.call(&call_gas, sender_address, receive_address, value, input, &code_address, output) }; return match call_result { diff --git a/src/evm/tests.rs b/src/evm/tests.rs index cf4262914..ad81cf877 100644 --- a/src/evm/tests.rs +++ b/src/evm/tests.rs @@ -63,7 +63,7 @@ impl Ext for FakeExt { _gas: &U256, _sender_address: &Address, _receive_address: &Address, - _value: Option<&U256>, + _value: Option, _data: &[u8], _code_address: &Address, _output: &mut [u8]) -> MessageCallResult { diff --git a/src/externalities.rs b/src/externalities.rs index d8b5d6110..f9a79c3c0 100644 --- a/src/externalities.rs +++ b/src/externalities.rs @@ -138,7 +138,7 @@ impl<'a> Ext for Externalities<'a> { gas: &U256, sender_address: &Address, receive_address: &Address, - value: Option<&U256>, + value: Option, data: &[u8], code_address: &Address, output: &mut [u8]) -> MessageCallResult { @@ -156,7 +156,7 @@ impl<'a> Ext for Externalities<'a> { }; if let Some(value) = value { - params.value = ActionValue::Transfer(value.clone()); + params.value = ActionValue::Transfer(value); } let mut ex = Executive::from_parent(self.state, self.env_info, self.engine, self.depth); diff --git a/src/tests/executive.rs b/src/tests/executive.rs index 0604c9992..a94fd1605 100644 --- a/src/tests/executive.rs +++ b/src/tests/executive.rs @@ -103,7 +103,7 @@ impl<'a> Ext for TestExt<'a> { gas: &U256, _sender_address: &Address, receive_address: &Address, - value: Option<&U256>, + value: Option, data: &[u8], _code_address: &Address, _output: &mut [u8]) -> MessageCallResult { @@ -111,7 +111,7 @@ impl<'a> Ext for TestExt<'a> { data: data.to_vec(), destination: Some(receive_address.clone()), gas_limit: *gas, - value: *value.unwrap() + value: value.unwrap() }); MessageCallResult::Success(*gas) }