From 952efb93ac10bf65eaab7adae97a645697364562 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 15 Jan 2016 11:39:10 +0100 Subject: [PATCH 1/4] fixed builtin outofgas --- src/executive.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/executive.rs b/src/executive.rs index 75195dc06..d229f9894 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -207,7 +207,10 @@ impl<'a> Executive<'a> { Ok(params.gas - cost) }, // just drain the whole gas - false => Ok(U256::zero()) + false => { + substate.excepted = true; + Ok(params.gas) + } } } else if params.code.len() > 0 { // if destination is a contract, do normal message call From 87539234e31e6067be21561b1ce1452c7caeaaf6 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 15 Jan 2016 11:59:05 +0100 Subject: [PATCH 2/4] builin fail should return 0 gas left --- src/executive.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/executive.rs b/src/executive.rs index de4d3d10f..04f22bd74 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -209,7 +209,7 @@ impl<'a> Executive<'a> { // just drain the whole gas false => { substate.excepted = true; - Ok(params.gas) + Ok(U256::zero()) } } } else if params.code.len() > 0 { From 80f2bfd8a5bc3bcf3813f4b6f3f5a212d6d211b3 Mon Sep 17 00:00:00 2001 From: debris Date: Fri, 15 Jan 2016 13:07:44 +0100 Subject: [PATCH 3/4] executive error on not enoguh base gas --- src/error.rs | 7 +++++-- src/executive.rs | 24 ++++++++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/error.rs b/src/error.rs index 01618c66c..e3d880103 100644 --- a/src/error.rs +++ b/src/error.rs @@ -19,16 +19,19 @@ pub struct OutOfBounds { /// Result of executing the transaction. #[derive(PartialEq, Debug)] pub enum ExecutionError { + /// Returned when there gas paid for transaction execution is + /// lower than base gas required. + NotEnoughBaseGas { required: U256, got: U256 }, /// Returned when block (gas_used + gas) > gas_limit. /// /// If gas =< gas_limit, upstream may try to execute the transaction /// in next block. BlockGasLimitReached { gas_limit: U256, gas_used: U256, gas: U256 }, /// Returned when transaction nonce does not match state nonce. - InvalidNonce { expected: U256, is: U256 }, + InvalidNonce { expected: U256, got: U256 }, /// Returned when cost of transaction (value + gas_price * gas) exceeds /// current sender balance. - NotEnoughCash { required: U512, is: U512 }, + NotEnoughCash { required: U512, got: U512 }, /// Returned when internal evm error occurs. Internal } diff --git a/src/executive.rs b/src/executive.rs index 04f22bd74..29445dae4 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -117,11 +117,18 @@ impl<'a> Executive<'a> { let sender = try!(t.sender()); let nonce = self.state.nonce(&sender); - // TODO: error on base gas required + let schedule = self.engine.schedule(self.info); + let base_gas_required = U256::from(t.gas_required(&schedule)); + + if t.gas < base_gas_required { + return Err(From::from(ExecutionError::NotEnoughBaseGas { required: base_gas_required, got: t.gas })); + } + + let init_gas = t.gas - base_gas_required; // validate transaction nonce if t.nonce != nonce { - return Err(From::from(ExecutionError::InvalidNonce { expected: nonce, is: t.nonce })); + return Err(From::from(ExecutionError::InvalidNonce { expected: nonce, got: t.nonce })); } // validate if transaction fits into given block @@ -140,7 +147,7 @@ impl<'a> Executive<'a> { // avoid unaffordable transactions if U512::from(balance) < total_cost { - return Err(From::from(ExecutionError::NotEnoughCash { required: total_cost, is: U512::from(balance) })); + return Err(From::from(ExecutionError::NotEnoughCash { required: total_cost, got: U512::from(balance) })); } // NOTE: there can be no invalid transactions from this point. @@ -149,9 +156,6 @@ impl<'a> Executive<'a> { let mut substate = Substate::new(); - let schedule = self.engine.schedule(self.info); - let init_gas = t.gas - U256::from(t.gas_required(&schedule)); - let res = match t.action() { &Action::Create => { let params = ActionParams { @@ -949,8 +953,8 @@ mod tests { }; match res { - Err(Error::Execution(ExecutionError::InvalidNonce { expected, is })) - if expected == U256::zero() && is == U256::one() => (), + Err(Error::Execution(ExecutionError::InvalidNonce { expected, got })) + if expected == U256::zero() && got == U256::one() => (), _ => assert!(false, "Expected invalid nonce error.") } } @@ -1000,8 +1004,8 @@ mod tests { }; match res { - Err(Error::Execution(ExecutionError::NotEnoughCash { required , is })) - if required == U512::from(100_018) && is == U512::from(100_017) => (), + Err(Error::Execution(ExecutionError::NotEnoughCash { required , got })) + if required == U512::from(100_018) && got == U512::from(100_017) => (), _ => assert!(false, "Expected not enough cash error. {:?}", res) } } From 0c39a83365984b6ba78f711da6c28fa8fa9a42a5 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 15 Jan 2016 13:08:37 +0100 Subject: [PATCH 4/4] Fix executive for out of gas builtins. --- src/executive.rs | 18 +++--------------- src/pod_account.rs | 3 +-- src/tests/state.rs | 46 ++++++++++++++++++++-------------------------- 3 files changed, 24 insertions(+), 43 deletions(-) diff --git a/src/executive.rs b/src/executive.rs index 04f22bd74..3fc942afe 100644 --- a/src/executive.rs +++ b/src/executive.rs @@ -21,8 +21,6 @@ pub struct Substate { logs: Vec, /// Refund counter of SSTORE nonzero->zero. refunds_count: U256, - /// True if transaction, or one of its subcalls runs out of gas. - excepted: bool, /// Created contracts. contracts_created: Vec
} @@ -34,7 +32,6 @@ impl Substate { suicides: HashSet::new(), logs: vec![], refunds_count: U256::zero(), - excepted: false, contracts_created: vec![] } } @@ -43,11 +40,8 @@ impl Substate { self.suicides.extend(s.suicides.into_iter()); self.logs.extend(s.logs.into_iter()); self.refunds_count = self.refunds_count + s.refunds_count; - self.excepted |= s.excepted; self.contracts_created.extend(s.contracts_created.into_iter()); } - - pub fn excepted(&self) -> bool { self.excepted } } /// Transaction execution receipt. @@ -68,8 +62,6 @@ pub struct Executed { pub cumulative_gas_used: U256, /// Vector of logs generated by transaction. pub logs: Vec, - /// Execution ended running out of gas. - pub excepted: bool, /// Addresses of contracts created during execution of transaction. /// Ordered from earliest creation. /// @@ -208,8 +200,8 @@ impl<'a> Executive<'a> { }, // just drain the whole gas false => { - substate.excepted = true; - Ok(U256::zero()) + self.state.revert(backup); + Err(evm::Error::OutOfGas) } } } else if params.code.len() > 0 { @@ -296,7 +288,6 @@ impl<'a> Executive<'a> { refunded: refund, cumulative_gas_used: self.info.gas_used + gas_used, logs: substate.logs, - excepted: substate.excepted, contracts_created: substate.contracts_created }) }, @@ -307,7 +298,6 @@ impl<'a> Executive<'a> { refunded: U256::zero(), cumulative_gas_used: self.info.gas_used + t.gas, logs: vec![], - excepted: true, contracts_created: vec![] }) } @@ -318,7 +308,6 @@ impl<'a> Executive<'a> { // TODO: handle other evm::Errors same as OutOfGas once they are implemented match result { &Err(evm::Error::OutOfGas) => { - substate.excepted = true; self.state.revert(backup); }, &Ok(_) | &Err(evm::Error::Internal) => substate.accrue(un_substate) @@ -464,7 +453,7 @@ impl<'a> Ext for Externalities<'a> { let gas = *gas - gas_cost; - // if balance is insufficient or we are to deep, return + // if balance is insufficient or we are too deep, return if self.state.balance(&self.params.address) < *value || self.depth >= self.schedule.max_depth { return Ok((gas + call_gas, true)); } @@ -902,7 +891,6 @@ mod tests { assert_eq!(executed.refunded, U256::from(58_699)); assert_eq!(executed.cumulative_gas_used, U256::from(41_301)); assert_eq!(executed.logs.len(), 0); - assert_eq!(executed.excepted, false); assert_eq!(executed.contracts_created.len(), 0); assert_eq!(state.balance(&sender), U256::from(1)); assert_eq!(state.balance(&contract), U256::from(17)); diff --git a/src/pod_account.rs b/src/pod_account.rs index a39bf1fa3..29b43c0bf 100644 --- a/src/pod_account.rs +++ b/src/pod_account.rs @@ -31,8 +31,7 @@ impl PodAccount { let mut stream = RlpStream::new_list(4); stream.append(&self.nonce); stream.append(&self.balance); - // TODO. - stream.append(&SHA3_NULL_RLP); + stream.append(&sec_trie_root(self.storage.iter().map(|(k, v)| (k.to_vec(), encode(&U256::from(v.as_slice())))).collect())); stream.append(&self.code.sha3()); stream.out() } diff --git a/src/tests/state.rs b/src/tests/state.rs index fdedd7a83..d4a921add 100644 --- a/src/tests/state.rs +++ b/src/tests/state.rs @@ -4,11 +4,6 @@ use pod_state::*; use state_diff::*; use ethereum; -fn flush(s: String) { - ::std::io::stdout().write(s.as_bytes()).unwrap(); - ::std::io::stdout().flush().unwrap(); -} - fn do_json_test(json_data: &[u8]) -> Vec { let json = Json::from_str(::std::str::from_utf8(json_data).unwrap()).expect("Json is invalid"); let mut failed = Vec::new(); @@ -39,32 +34,31 @@ fn do_json_test(json_data: &[u8]) -> Vec { //println!("Transaction: {:?}", t); //println!("Env: {:?}", env); + let calc_post = sec_trie_root(post.get().iter().map(|(k, v)| (k.to_vec(), v.rlp())).collect()); - { + if fail_unless(post_state_root == calc_post) { + println!("!!! {}: Trie root mismatch (got: {}, expect: {}):", name, calc_post, post_state_root); + println!("!!! Post:\n{}", post); + } else { let mut s = State::new_temp(); - s.populate_from(post.clone()); + s.populate_from(pre); s.commit(); - assert_eq!(&post_state_root, s.root()); - } + let res = s.apply(&env, engine.deref(), &t); - let mut s = State::new_temp(); - s.populate_from(pre); - s.commit(); - let res = s.apply(&env, engine.deref(), &t); + if fail_unless(s.root() == &post_state_root) { + println!("!!! {}: State mismatch (got: {}, expect: {}):", name, s.root(), post_state_root); + let our_post = s.to_pod(); + println!("Got:\n{}", our_post); + println!("Expect:\n{}", post); + println!("Diff ---expect -> +++got:\n{}", StateDiff::diff_pod(&post, &our_post)); + } - if fail_unless(s.root() == &post_state_root) { - println!("!!! {}: State mismatch (got: {}, expect: {}):", name, s.root(), post_state_root); - let our_post = s.to_pod(); - println!("Got:\n{}", our_post); - println!("Expect:\n{}", post); - println!("Diff ---expect -> +++got:\n{}", StateDiff::diff_pod(&post, &our_post)); - } - - if let Ok(r) = res { - if fail_unless(logs == r.logs) { - println!("!!! {}: Logs mismatch:", name); - println!("Got:\n{:?}", r.logs); - println!("Expect:\n{:?}", logs); + if let Ok(r) = res { + if fail_unless(logs == r.logs) { + println!("!!! {}: Logs mismatch:", name); + println!("Got:\n{:?}", r.logs); + println!("Expect:\n{:?}", logs); + } } } }