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

Avoid tracing DELEGATECALL and CALLCODE. Plus tests for it. #794

Merged
merged 6 commits into from
Mar 22, 2016
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
35 changes: 35 additions & 0 deletions ethcore/res/null_homestead_morden.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"name": "Morden",
"engineName": "NullEngine",
"params": {
"accountStartNonce": "0x0100000",
"frontierCompatibilityModeLimit": "0x0",
"maximumExtraDataSize": "0x20",
"tieBreakingGas": false,
"minGasLimit": "0x1388",
"gasLimitBoundDivisor": "0x0400",
"minimumDifficulty": "0x020000",
"difficultyBoundDivisor": "0x0800",
"durationLimit": "0x0d",
"blockReward": "0x4563918244F40000",
"registrar": "",
"networkID" : "0x2"
},
"genesis": {
"nonce": "0x00006d6f7264656e",
"difficulty": "0x20000",
"mixHash": "0x00000000000000000000000000000000000000647572616c65787365646c6578",
"author": "0x0000000000000000000000000000000000000000",
"timestamp": "0x00",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"extraData": "0x",
"gasLimit": "0x2fefd8"
},
"accounts": {
"0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },
"0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } },
"0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } },
"0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } },
"102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" }
}
}
2 changes: 1 addition & 1 deletion ethcore/res/null_morden.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engineName": "NullEngine",
"params": {
"accountStartNonce": "0x0100000",
"frontierCompatibilityModeLimit": "0xfffa2990",
"frontierCompatibilityModeLimit": "0x789b0",
"maximumExtraDataSize": "0x20",
"tieBreakingGas": false,
"minGasLimit": "0x1388",
Expand Down
10 changes: 10 additions & 0 deletions ethcore/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,14 @@ pub trait Engine : Sync + Send {
fn execute_builtin(&self, a: &Address, input: &[u8], output: &mut [u8]) { self.spec().builtins.get(a).unwrap().execute(input, output); }

// TODO: sealing stuff - though might want to leave this for later.

/// Get a named parameter from the `spec()`'s `engine_params` item and convert to u64, or 0 if that fails.
fn u64_param(&self, name: &str) -> u64 {
self.spec().engine_params.get(name).map_or(0u64, |a| decode(&a))
}

/// Get a named parameter from the `spec()`'s `engine_params` item and convert to U256, or 0 if that fails.
fn u256_param(&self, name: &str) -> U256 {
self.spec().engine_params.get(name).map_or(x!(0), |a| decode(&a))
}
}
20 changes: 10 additions & 10 deletions ethcore/src/ethereum/ethash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,6 @@ impl Ethash {
u256_params: RwLock::new(HashMap::new())
}
}

fn u64_param(&self, name: &str) -> u64 {
*self.u64_params.write().unwrap().entry(name.to_owned()).or_insert_with(||
self.spec().engine_params.get(name).map_or(0u64, |a| decode(&a)))
}

fn u256_param(&self, name: &str) -> U256 {
*self.u256_params.write().unwrap().entry(name.to_owned()).or_insert_with(||
self.spec().engine_params.get(name).map_or(x!(0), |a| decode(&a)))
}
}

impl Engine for Ethash {
Expand Down Expand Up @@ -199,6 +189,16 @@ impl Engine for Ethash {
fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
t.sender().map(|_|()) // Perform EC recovery and cache sender
}

fn u64_param(&self, name: &str) -> u64 {
*self.u64_params.write().unwrap().entry(name.to_owned()).or_insert_with(||
self.spec().engine_params.get(name).map_or(0u64, |a| decode(&a)))
}

fn u256_param(&self, name: &str) -> U256 {
*self.u256_params.write().unwrap().entry(name.to_owned()).or_insert_with(||
self.spec().engine_params.get(name).map_or(x!(0), |a| decode(&a)))
}
}

#[cfg_attr(feature="dev", allow(wrong_self_convention))] // to_ethash should take self
Expand Down
32 changes: 17 additions & 15 deletions ethcore/src/evm/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,13 @@ impl evm::Evm for Interpreter {

impl Interpreter {
#[cfg_attr(feature="dev", allow(cyclomatic_complexity))]
fn get_gas_cost_mem(&self,
ext: &evm::Ext,
instruction: Instruction,
mem: &mut Memory,
stack: &Stack<U256>
) -> Result<(U256, usize), evm::Error> {
fn get_gas_cost_mem(
&self,
ext: &evm::Ext,
instruction: Instruction,
mem: &mut Memory,
stack: &Stack<U256>
) -> Result<(U256, usize), evm::Error> {
let schedule = ext.schedule();
let info = instructions::get_info(instruction);

Expand Down Expand Up @@ -522,15 +523,16 @@ impl Interpreter {
}

#[cfg_attr(feature="dev", allow(too_many_arguments))]
fn exec_instruction(&self,
gas: Gas,
params: &ActionParams,
ext: &mut evm::Ext,
instruction: Instruction,
code: &mut CodeReader,
mem: &mut Memory,
stack: &mut Stack<U256>
) -> Result<InstructionResult, evm::Error> {
fn exec_instruction(
&self,
gas: Gas,
params: &ActionParams,
ext: &mut evm::Ext,
instruction: Instruction,
code: &mut CodeReader,
mem: &mut Memory,
stack: &mut Stack<U256>
) -> Result<InstructionResult, evm::Error> {
match instruction {
instructions::JUMP => {
let jump = stack.pop_back();
Expand Down
61 changes: 39 additions & 22 deletions ethcore/src/executive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ impl<'a> Executive<'a> {
if (self.depth + 1) % MAX_VM_DEPTH_FOR_THREAD != 0 {
let mut ext = self.as_externalities(OriginInfo::from(&params), unconfirmed_substate, output_policy);
let vm_factory = self.engine.vm_factory();
trace!(target: "executive", "ext.schedule.have_delegate_call: {}", ext.schedule().have_delegate_call);
return vm_factory.create().exec(params, &mut ext);
}

Expand Down Expand Up @@ -245,35 +246,49 @@ impl<'a> Executive<'a> {
Err(evm::Error::OutOfGas)
}
}
} else if params.code.is_some() {
} else {
// if destination is a contract, do normal message call

// part of substate that may be reverted
let mut unconfirmed_substate = Substate::new(substate.subtraces.is_some());

// don't trace if it's DELEGATECALL or CALLCODE.
let should_trace = if let ActionValue::Transfer(_) = params.value {
params.code_address == params.address && substate.subtraces.is_some()
} else { false };

// transaction tracing stuff. None if there's no tracing.
let mut trace_info = substate.subtraces.as_ref().map(|_| (TraceAction::from_call(&params), self.depth));
let mut trace_output = trace_info.as_ref().map(|_| vec![]);
let (mut trace_info, mut trace_output) = if should_trace {
(Some((TraceAction::from_call(&params), self.depth)), Some(vec![]))
} else { (None, None) };

let res = {
self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return(output, trace_output.as_mut()))
};
if params.code.is_some() {
// part of substate that may be reverted
let mut unconfirmed_substate = Substate::new(should_trace);

// if there's tracing, make up trace_info's result with trace_output and some arithmetic.
if let Some((TraceAction::Call(ref mut c), _)) = trace_info {
c.result = res.as_ref().ok().map(|gas_left| (c.gas - *gas_left, trace_output.expect("trace_info is Some: qed")));
}
let res = {
self.exec_vm(params, &mut unconfirmed_substate, OutputPolicy::Return(output, trace_output.as_mut()))
};

trace!(target: "executive", "sstore-clears={}\n", unconfirmed_substate.sstore_clears_count);
trace!(target: "executive", "substate={:?}; unconfirmed_substate={:?}\n", substate, unconfirmed_substate);
trace!(target: "executive", "res={:?}", res);

self.enact_result(&res, substate, unconfirmed_substate, trace_info);
trace!(target: "executive", "enacted: substate={:?}\n", substate);
res
} else {
// otherwise, nothing
self.state.clear_snapshot();
Ok(params.gas)
// if there's tracing, make up trace_info's result with trace_output and some arithmetic.
if let Some((TraceAction::Call(ref mut c), _)) = trace_info {
c.result = res.as_ref().ok().map(|gas_left| (c.gas - *gas_left, trace_output.expect("trace_info is Some: so should_trace: qed")));
}

trace!(target: "executive", "substate={:?}; unconfirmed_substate={:?}\n", substate, unconfirmed_substate);

self.enact_result(&res, substate, unconfirmed_substate, trace_info);
trace!(target: "executive", "enacted: substate={:?}\n", substate);
res
} else {
// otherwise it's just a basic transaction, only do tracing, if necessary.
trace!(target: "executive", "Basic message (send funds) should_trace={}", should_trace);
self.state.clear_snapshot();
if let Some((TraceAction::Call(ref mut c), _)) = trace_info {
c.result = Some((x!(0), vec![]));
}
substate.accrue_trace(if should_trace {Some(vec![])} else {None}, trace_info);
Ok(params.gas)
}
}
}

Expand Down Expand Up @@ -530,6 +545,7 @@ mod tests {
//let next_address = contract_address(&address, &U256::zero());
let mut params = ActionParams::default();
params.address = address.clone();
params.code_address = address.clone();
params.sender = sender.clone();
params.origin = sender.clone();
params.gas = U256::from(100_000);
Expand Down Expand Up @@ -627,6 +643,7 @@ mod tests {
assert_eq!(substate.subtraces, expected_trace);
assert_eq!(gas_left, U256::from(96_776));
}

evm_test!{test_create_contract_value_too_high: test_create_contract_value_too_high_jit, test_create_contract_value_too_high_int}
fn test_create_contract_value_too_high(factory: Factory) {
// code:
Expand Down
11 changes: 10 additions & 1 deletion ethcore/src/null_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,16 @@ impl Engine for NullEngine {
fn vm_factory(&self) -> &Factory {
&self.factory
}

fn name(&self) -> &str { "NullEngine" }

fn spec(&self) -> &Spec { &self.spec }
fn schedule(&self, _env_info: &EnvInfo) -> Schedule { Schedule::new_frontier() }

fn schedule(&self, env_info: &EnvInfo) -> Schedule {
if env_info.number < self.u64_param("frontierCompatibilityModeLimit") {
Schedule::new_frontier()
} else {
Schedule::new_homestead()
}
}
}
3 changes: 3 additions & 0 deletions ethcore/src/spec/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ impl Spec {

/// Create a new Spec which conforms to the Morden chain except that it's a NullEngine consensus.
pub fn new_test() -> Spec { Self::from_json_utf8(include_bytes!("../../res/null_morden.json")) }

/// Create a new Spec which conforms to the Morden chain except that it's a NullEngine consensus.
pub fn new_homestead_test() -> Spec { Self::from_json_utf8(include_bytes!("../../res/null_homestead_morden.json")) }
}

#[cfg(test)]
Expand Down
Loading