Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[stable] Refactor static context check in CREATE #6889

Merged
merged 2 commits into from
Oct 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions ethcore/evm/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
19 changes: 10 additions & 9 deletions ethcore/evm/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,20 +313,24 @@ impl<Cost: CostType> Interpreter<Cost> {
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(&params.address)? >= endowment && ext.depth() < ext.schedule().max_depth;
if ext.is_static() {
return Err(evm::Error::MutableCallInStaticContext);
}

// clear return data buffer before creating new call frame.
self.return_data = ReturnData::empty();

let can_create = ext.balance(&params.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) => {
Expand All @@ -342,9 +346,6 @@ impl<Cost: CostType> Interpreter<Cost> {
stack.push(U256::zero());
Ok(InstructionResult::Ok)
},
ContractCreateResult::FailedInStaticCall => {
Err(evm::Error::MutableCallInStaticContext)
},
};
},
instructions::CALL | instructions::CALLCODE | instructions::DELEGATECALL | instructions::STATICCALL => {
Expand Down Expand Up @@ -906,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() {
Expand Down Expand Up @@ -950,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);
}
}
54 changes: 43 additions & 11 deletions ethcore/evm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,20 @@ pub struct FakeExt {
pub store: HashMap<H256, H256>,
pub suicides: HashSet<Address>,
pub calls: HashSet<FakeCall>,
sstore_clears: usize,
depth: usize,
blockhashes: HashMap<U256, H256>,
codes: HashMap<Address, Arc<Bytes>>,
logs: Vec<FakeLogEntry>,
info: EnvInfo,
schedule: Schedule,
balances: HashMap<Address, U256>,
pub sstore_clears: usize,
pub depth: usize,
pub blockhashes: HashMap<U256, H256>,
pub codes: HashMap<Address, Arc<Bytes>>,
pub logs: Vec<FakeLogEntry>,
pub info: EnvInfo,
pub schedule: Schedule,
pub balances: HashMap<Address, U256>,
pub is_static: bool,
pub tracing: bool,
}

// similar to the normal `finalize` function, but ignoring NeedsReturn.
fn test_finalize(res: Result<GasLeft, evm::Error>) -> Result<U256, evm::Error> {
pub fn test_finalize(res: Result<GasLeft, evm::Error>) -> Result<U256, evm::Error> {
match res {
Ok(GasLeft::Known(gas)) => Ok(gas),
Ok(GasLeft::NeedsReturn{..}) => unimplemented!(), // since ret is unimplemented.
Expand All @@ -78,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 {
Expand Down Expand Up @@ -168,7 +176,7 @@ impl Ext for FakeExt {
Ok(())
}

fn ret(self, _gas: &U256, _data: &ReturnData) -> evm::Result<U256> {
fn ret(self, _gas: &U256, _data: &ReturnData, _apply_state: bool) -> evm::Result<U256> {
unimplemented!();
}

Expand All @@ -192,6 +200,10 @@ impl Ext for FakeExt {
fn inc_sstore_clears(&mut self) {
self.sstore_clears += 1;
}

fn is_static(&self) -> bool {
self.is_static
}
}

#[test]
Expand Down Expand Up @@ -917,7 +929,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();
Expand Down Expand Up @@ -962,6 +973,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, ::Error::MutableCallInStaticContext);
assert_eq!(ext.calls.len(), 0);
}

fn assert_set_contains<T : Debug + Eq + PartialEq + Hash>(set: &HashSet<T>, val: &T) {
let contains = set.contains(val);
if !contains {
Expand Down
4 changes: 0 additions & 4 deletions ethcore/evm/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
},
}
}

Expand Down
2 changes: 0 additions & 2 deletions ethcore/src/externalities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
1 change: 1 addition & 0 deletions scripts/targets.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ export TARGETS="
-p ethcore-ipc-tests \
-p ethcore-ipc-nano \
-p ethcore-light \
-p evm \
-p parity"