From 8dfdebc2a2b26329194ae4ee74ca5449ed3577c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Oct 2017 11:27:18 +0200 Subject: [PATCH] Refactor static context check in CREATE. (#6886) * Refactor static context check in CREATE. * Fix wasm. --- ethcore/evm/src/interpreter/mod.rs | 13 +++++++------ ethcore/evm/src/tests.rs | 22 +++++++++++++++++++++- ethcore/src/externalities.rs | 1 - ethcore/vm/src/ext.rs | 3 --- ethcore/vm/src/tests.rs | 3 ++- ethcore/wasm/src/runtime.rs | 20 ++++++++------------ 6 files changed, 38 insertions(+), 24 deletions(-) diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index 0d3a6495645..2042bbb1dae 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -322,20 +322,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) => { @@ -351,9 +355,6 @@ impl Interpreter { stack.push(U256::zero()); Ok(InstructionResult::Ok) }, - ContractCreateResult::FailedInStaticCall => { - Err(vm::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 3eb10bc6aa9..6e85e180397 100644 --- a/ethcore/evm/src/tests.rs +++ b/ethcore/evm/src/tests.rs @@ -724,7 +724,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(); @@ -769,6 +768,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 caf14967b9f..c7cf622760a 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -240,7 +240,6 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> Ok(FinalizationResult{ gas_left, apply_state: false, return_data }) => { ContractCreateResult::Reverted(gas_left, return_data) }, - Err(vm::Error::MutableCallInStaticContext) => ContractCreateResult::FailedInStaticCall, _ => ContractCreateResult::Failed, } } diff --git a/ethcore/vm/src/ext.rs b/ethcore/vm/src/ext.rs index 574af9e828a..3f5004586bc 100644 --- a/ethcore/vm/src/ext.rs +++ b/ethcore/vm/src/ext.rs @@ -35,9 +35,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/vm/src/tests.rs b/ethcore/vm/src/tests.rs index 0d7ca1f8263..222578afdf5 100644 --- a/ethcore/vm/src/tests.rs +++ b/ethcore/vm/src/tests.rs @@ -65,6 +65,7 @@ pub struct FakeExt { pub schedule: Schedule, pub balances: HashMap, pub tracing: bool, + pub is_static: bool, } // similar to the normal `finalize` function, but ignoring NeedsReturn. @@ -192,7 +193,7 @@ impl Ext for FakeExt { } fn is_static(&self) -> bool { - false + self.is_static } fn inc_sstore_clears(&mut self) { diff --git a/ethcore/wasm/src/runtime.rs b/ethcore/wasm/src/runtime.rs index 1f8bf428a98..d26346a3e78 100644 --- a/ethcore/wasm/src/runtime.rs +++ b/ethcore/wasm/src/runtime.rs @@ -221,8 +221,8 @@ impl<'a, 'b> Runtime<'a, 'b> { } /// Charge gas according to closure - pub fn charge(&mut self, f: F) -> Result<(), InterpreterError> - where F: FnOnce(&vm::Schedule) -> u64 + pub fn charge(&mut self, f: F) -> Result<(), InterpreterError> + where F: FnOnce(&vm::Schedule) -> u64 { let amount = f(self.ext.schedule()); if !self.charge_gas(amount as u64) { @@ -277,10 +277,6 @@ impl<'a, 'b> Runtime<'a, 'b> { self.gas_counter = self.gas_limit - gas_left.low_u64(); Ok(Some((-1i32).into())) }, - vm::ContractCreateResult::FailedInStaticCall => { - trace!(target: "wasm", "runtime: create contract called in static context"); - Err(interpreter::Error::Trap("CREATE in static context".to_owned())) - }, } } @@ -615,11 +611,11 @@ impl<'a, 'b> Runtime<'a, 'b> { } fn return_u256_ptr(&mut self, ptr: u32, val: U256) -> Result<(), InterpreterError> { - let value: H256 = val.into(); + let value: H256 = val.into(); self.charge(|schedule| schedule.wasm.static_u256 as u64)?; self.memory.set(ptr, &*value)?; Ok(()) - } + } fn coinbase(&mut self, context: InterpreterCallerContext) -> Result, InterpreterError> @@ -640,7 +636,7 @@ impl<'a, 'b> Runtime<'a, 'b> { context.value_stack.pop_as::()? as u32, sender, )?; - Ok(None) + Ok(None) } fn address(&mut self, context: InterpreterCallerContext) @@ -651,7 +647,7 @@ impl<'a, 'b> Runtime<'a, 'b> { context.value_stack.pop_as::()? as u32, addr, )?; - Ok(None) + Ok(None) } fn origin(&mut self, context: InterpreterCallerContext) @@ -662,7 +658,7 @@ impl<'a, 'b> Runtime<'a, 'b> { context.value_stack.pop_as::()? as u32, origin, )?; - Ok(None) + Ok(None) } fn value(&mut self, context: InterpreterCallerContext) @@ -709,7 +705,7 @@ impl<'a, 'b> Runtime<'a, 'b> { context.value_stack.pop_as::()? as u32, gas_limit, )?; - Ok(None) + Ok(None) } fn return_i64(&mut self, val: i64) -> Result, InterpreterError> {