From 96e60b7962f137be54a7371ce3807e6ccb35c594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Oct 2017 10:12:40 +0200 Subject: [PATCH 1/2] Refactor static context check in CREATE. --- ethcore/evm/src/ext.rs | 3 --- ethcore/evm/src/interpreter/mod.rs | 13 +++++++------ ethcore/evm/src/tests.rs | 27 ++++++++++++++++++++++++++- ethcore/src/externalities.rs | 2 -- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/ethcore/evm/src/ext.rs b/ethcore/evm/src/ext.rs index 47731248115..a9ec40f5789 100644 --- a/ethcore/evm/src/ext.rs +++ b/ethcore/evm/src/ext.rs @@ -30,9 +30,6 @@ pub enum ContractCreateResult { /// Returned when contract creation failed. /// VM doesn't have to know the reason. Failed, - /// Returned when contract creation failed. - /// VM doesn't have to know the reason. - FailedInStaticCall, /// Reverted with REVERT. Reverted(U256, ReturnData), } diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index 24ef0245f83..a1483eabfae 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -313,20 +313,24 @@ impl Interpreter { let init_off = stack.pop_back(); let init_size = stack.pop_back(); - let address_scheme = if instruction == instructions::CREATE { CreateContractAddress::FromSenderAndNonce } else { CreateContractAddress::FromSenderAndCodeHash }; let create_gas = provided.expect("`provided` comes through Self::exec from `Gasometer::get_gas_cost_mem`; `gas_gas_mem_cost` guarantees `Some` when instruction is `CALL`/`CALLCODE`/`DELEGATECALL`/`CREATE`; this is `CREATE`; qed"); - let contract_code = self.mem.read_slice(init_off, init_size); - let can_create = ext.balance(¶ms.address)? >= endowment && ext.depth() < ext.schedule().max_depth; + if ext.is_static() { + return Err(vm::Error::MutableCallInStaticContext); + } // clear return data buffer before creating new call frame. self.return_data = ReturnData::empty(); + let can_create = ext.balance(¶ms.address)? >= endowment && ext.depth() < ext.schedule().max_depth; if !can_create { stack.push(U256::zero()); return Ok(InstructionResult::UnusedGas(create_gas)); } + let contract_code = self.mem.read_slice(init_off, init_size); + let address_scheme = if instruction == instructions::CREATE { CreateContractAddress::FromSenderAndNonce } else { CreateContractAddress::FromSenderAndCodeHash }; + let create_result = ext.create(&create_gas.as_u256(), &endowment, contract_code, address_scheme); return match create_result { ContractCreateResult::Created(address, gas_left) => { @@ -342,9 +346,6 @@ impl Interpreter { stack.push(U256::zero()); Ok(InstructionResult::Ok) }, - ContractCreateResult::FailedInStaticCall => { - Err(evm::Error::MutableCallInStaticContext) - }, }; }, instructions::CALL | instructions::CALLCODE | instructions::DELEGATECALL | instructions::STATICCALL => { diff --git a/ethcore/evm/src/tests.rs b/ethcore/evm/src/tests.rs index d0502f79adf..881b202260b 100644 --- a/ethcore/evm/src/tests.rs +++ b/ethcore/evm/src/tests.rs @@ -63,6 +63,7 @@ pub struct FakeExt { info: EnvInfo, schedule: Schedule, balances: HashMap, + pub is_static: bool, } // similar to the normal `finalize` function, but ignoring NeedsReturn. @@ -192,6 +193,10 @@ impl Ext for FakeExt { fn inc_sstore_clears(&mut self) { self.sstore_clears += 1; } + + fn is_static(&self) -> bool { + self.is_static + } } #[test] @@ -917,7 +922,6 @@ fn test_jumps(factory: super::Factory) { assert_eq!(gas_left, U256::from(54_117)); } - evm_test!{test_calls: test_calls_jit, test_calls_int} fn test_calls(factory: super::Factory) { let code = "600054602d57600160005560006000600060006050610998610100f160006000600060006050610998610100f25b".from_hex().unwrap(); @@ -962,6 +966,27 @@ fn test_calls(factory: super::Factory) { assert_eq!(ext.calls.len(), 2); } +evm_test!{test_create_in_staticcall: test_create_in_staticcall_jit, test_create_in_staticcall_int} +fn test_create_in_staticcall(factory: super::Factory) { + let code = "600060006064f000".from_hex().unwrap(); + + let address = Address::from(0x155); + let mut params = ActionParams::default(); + params.gas = U256::from(100_000); + params.code = Some(Arc::new(code)); + params.address = address.clone(); + let mut ext = FakeExt::new_byzantium(); + ext.is_static = true; + + let err = { + let mut vm = factory.create(params.gas); + test_finalize(vm.exec(params, &mut ext)).unwrap_err() + }; + + assert_eq!(err, vm::Error::MutableCallInStaticContext); + assert_eq!(ext.calls.len(), 0); +} + fn assert_set_contains(set: &HashSet, val: &T) { let contains = set.contains(val); if !contains { diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index e071de1bc36..a46897af341 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -234,8 +234,6 @@ impl<'a, T: 'a, V: 'a, B: 'a, E: 'a> Ext for Externalities<'a, T, V, B, E> Ok(FinalizationResult{ gas_left, apply_state: false, return_data }) => { ContractCreateResult::Reverted(gas_left, return_data) }, - Err(evm::Error::MutableCallInStaticContext) => ContractCreateResult::FailedInStaticCall, - _ => ContractCreateResult::Failed, } } From fa220eba7452aee7d2d94329c7da952bbf929ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Oct 2017 10:38:51 +0200 Subject: [PATCH 2/2] Fix evm tests. --- ethcore/evm/src/interpreter/mod.rs | 8 ++++---- ethcore/evm/src/tests.rs | 29 ++++++++++++++++++----------- ethcore/evm/src/wasm/runtime.rs | 4 ---- scripts/targets.sh | 1 + 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index a1483eabfae..ccba41c9876 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -316,7 +316,7 @@ impl Interpreter { let create_gas = provided.expect("`provided` comes through Self::exec from `Gasometer::get_gas_cost_mem`; `gas_gas_mem_cost` guarantees `Some` when instruction is `CALL`/`CALLCODE`/`DELEGATECALL`/`CREATE`; this is `CREATE`; qed"); if ext.is_static() { - return Err(vm::Error::MutableCallInStaticContext); + return Err(evm::Error::MutableCallInStaticContext); } // clear return data buffer before creating new call frame. @@ -907,8 +907,8 @@ mod tests { use rustc_hex::FromHex; use vmtype::VMType; use factory::Factory; - use vm::{self, ActionParams, ActionValue}; - use vm::tests::{FakeExt, test_finalize}; + use action_params::{ActionParams, ActionValue}; + use ::tests::{FakeExt, test_finalize}; #[test] fn should_not_fail_on_tracing_mem() { @@ -951,6 +951,6 @@ mod tests { test_finalize(vm.exec(params, &mut ext)).err().unwrap() }; - assert_eq!(err, ::vm::Error::OutOfBounds); + assert_eq!(err, ::Error::OutOfBounds); } } diff --git a/ethcore/evm/src/tests.rs b/ethcore/evm/src/tests.rs index 881b202260b..52640428a43 100644 --- a/ethcore/evm/src/tests.rs +++ b/ethcore/evm/src/tests.rs @@ -55,19 +55,20 @@ pub struct FakeExt { pub store: HashMap, pub suicides: HashSet
, pub calls: HashSet, - sstore_clears: usize, - depth: usize, - blockhashes: HashMap, - codes: HashMap>, - logs: Vec, - info: EnvInfo, - schedule: Schedule, - balances: HashMap, + pub sstore_clears: usize, + pub depth: usize, + pub blockhashes: HashMap, + pub codes: HashMap>, + pub logs: Vec, + pub info: EnvInfo, + pub schedule: Schedule, + pub balances: HashMap, pub is_static: bool, + pub tracing: bool, } // similar to the normal `finalize` function, but ignoring NeedsReturn. -fn test_finalize(res: Result) -> Result { +pub fn test_finalize(res: Result) -> Result { match res { Ok(GasLeft::Known(gas)) => Ok(gas), Ok(GasLeft::NeedsReturn{..}) => unimplemented!(), // since ret is unimplemented. @@ -79,6 +80,12 @@ impl FakeExt { pub fn new() -> Self { FakeExt::default() } + + pub fn new_byzantium() -> Self { + let mut ext = FakeExt::new(); + ext.schedule = Schedule::new_byzantium(); + ext + } } impl Default for Schedule { @@ -169,7 +176,7 @@ impl Ext for FakeExt { Ok(()) } - fn ret(self, _gas: &U256, _data: &ReturnData) -> evm::Result { + fn ret(self, _gas: &U256, _data: &ReturnData, _apply_state: bool) -> evm::Result { unimplemented!(); } @@ -983,7 +990,7 @@ fn test_create_in_staticcall(factory: super::Factory) { test_finalize(vm.exec(params, &mut ext)).unwrap_err() }; - assert_eq!(err, vm::Error::MutableCallInStaticContext); + assert_eq!(err, ::Error::MutableCallInStaticContext); assert_eq!(ext.calls.len(), 0); } diff --git a/ethcore/evm/src/wasm/runtime.rs b/ethcore/evm/src/wasm/runtime.rs index 46841392b42..e17249df5e3 100644 --- a/ethcore/evm/src/wasm/runtime.rs +++ b/ethcore/evm/src/wasm/runtime.rs @@ -169,10 +169,6 @@ impl<'a> Runtime<'a> { self.gas_counter = self.gas_limit - gas_left.low_u64(); Ok(Some((-1i32).into())) }, - ext::ContractCreateResult::FailedInStaticCall => { - trace!(target: "wasm", "runtime: create contract called in static context"); - Err(interpreter::Error::Trap("CREATE in static context".to_owned())) - }, } } diff --git a/scripts/targets.sh b/scripts/targets.sh index fb10c43f200..568fec273f6 100644 --- a/scripts/targets.sh +++ b/scripts/targets.sh @@ -19,4 +19,5 @@ export TARGETS=" -p ethcore-ipc-tests \ -p ethcore-ipc-nano \ -p ethcore-light \ + -p evm \ -p parity"