From 1bf2b277082f41cd49812e15cac75160228a29f8 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 26 Feb 2017 13:10:50 +0100 Subject: [PATCH] Propagate trie errors upwards from State (#4655) * state backend trait mirroring state_db API * minimal state backend trait make state module public * fix json tests * return errors on database corruption * fix tests, json tests * fix remainder of build * add Backend bound on state --- ethcore/src/block.rs | 3 +- ethcore/src/client/client.rs | 60 ++-- ethcore/src/engines/authority_round.rs | 8 +- ethcore/src/engines/tendermint/mod.rs | 8 +- ethcore/src/ethereum/ethash.rs | 38 ++- ethcore/src/ethereum/mod.rs | 12 +- ethcore/src/evm/evm.rs | 15 +- ethcore/src/evm/ext.rs | 19 +- ethcore/src/evm/interpreter/gasometer.rs | 12 +- ethcore/src/evm/interpreter/mod.rs | 20 +- ethcore/src/evm/tests.rs | 31 +- ethcore/src/executive.rs | 90 ++--- ethcore/src/externalities.rs | 67 ++-- ethcore/src/json_tests/executive.rs | 60 ++-- ethcore/src/miner/miner.rs | 35 +- ethcore/src/miner/mod.rs | 8 +- ethcore/src/spec/spec.rs | 2 +- ethcore/src/state/account.rs | 47 +-- ethcore/src/state/mod.rs | 388 +++++++++++----------- ethcore/src/tests/client.rs | 4 +- ethcore/src/types/executed.rs | 23 +- ethcore/src/types/trace_types/error.rs | 14 +- evmbin/src/ext.rs | 23 +- rpc/src/v1/helpers/errors.rs | 5 + rpc/src/v1/impls/eth.rs | 32 +- rpc/src/v1/tests/eth.rs | 1 - rpc/src/v1/tests/helpers/miner_service.rs | 29 +- 27 files changed, 598 insertions(+), 456 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index f2eff0d04..3626fdd3a 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -540,7 +540,8 @@ pub fn enact( { if ::log::max_log_level() >= ::log::LogLevel::Trace { let s = State::from_existing(db.boxed_clone(), parent.state_root().clone(), engine.account_start_nonce(), factories.clone())?; - trace!(target: "enact", "num={}, root={}, author={}, author_balance={}\n", header.number(), s.root(), header.author(), s.balance(&header.author())); + trace!(target: "enact", "num={}, root={}, author={}, author_balance={}\n", + header.number(), s.root(), header.author(), s.balance(&header.author())?); } } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 96b25b351..7f209bad1 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -890,17 +890,20 @@ impl BlockChainClient for Client { let original_state = if analytics.state_diffing { Some(state.clone()) } else { None }; let sender = t.sender(); - let balance = state.balance(&sender); + let balance = state.balance(&sender).map_err(|_| CallError::StateCorrupt)?; let needed_balance = t.value + t.gas * t.gas_price; if balance < needed_balance { // give the sender a sufficient balance - state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty); + state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty) + .map_err(|_| CallError::StateCorrupt)?; } let options = TransactOptions { tracing: analytics.transaction_tracing, vm_tracing: analytics.vm_tracing, check_nonce: false }; let mut ret = Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(t, options)?; // TODO gav move this into Executive. - ret.state_diff = original_state.map(|original| state.diff_from(original)); + if let Some(original) = original_state { + ret.state_diff = Some(state.diff_from(original).map_err(ExecutionError::from)?); + } Ok(ret) } @@ -921,7 +924,7 @@ impl BlockChainClient for Client { // that's just a copy of the state. let original_state = self.state_at(block).ok_or(CallError::StatePruned)?; let sender = t.sender(); - let balance = original_state.balance(&sender); + let balance = original_state.balance(&sender).map_err(ExecutionError::from)?; let options = TransactOptions { tracing: true, vm_tracing: false, check_nonce: false }; let cond = |gas| { @@ -933,27 +936,29 @@ impl BlockChainClient for Client { let needed_balance = tx.value + tx.gas * tx.gas_price; if balance < needed_balance { // give the sender a sufficient balance - state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty); + state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty) + .map_err(ExecutionError::from)?; } - Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm) + Ok(Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm) .transact(&tx, options.clone()) .map(|r| r.exception.is_none()) - .unwrap_or(false) + .unwrap_or(false)) }; let mut upper = header.gas_limit(); - if !cond(upper) { + if !cond(upper)? { // impossible at block gas limit - try `UPPER_CEILING` instead. // TODO: consider raising limit by powers of two. upper = UPPER_CEILING.into(); - if !cond(upper) { + if !cond(upper)? { trace!(target: "estimate_gas", "estimate_gas failed with {}", upper); - return Err(CallError::Execution(ExecutionError::Internal)) + let err = ExecutionError::Internal(format!("Requires higher than upper limit of {}", upper)); + return Err(err.into()) } } let lower = t.gas_required(&self.engine.schedule(&env_info)).into(); - if cond(lower) { + if cond(lower)? { trace!(target: "estimate_gas", "estimate_gas succeeded with {}", lower); return Ok(lower) } @@ -961,23 +966,25 @@ impl BlockChainClient for Client { /// Find transition point between `lower` and `upper` where `cond` changes from `false` to `true`. /// Returns the lowest value between `lower` and `upper` for which `cond` returns true. /// We assert: `cond(lower) = false`, `cond(upper) = true` - fn binary_chop(mut lower: U256, mut upper: U256, mut cond: F) -> U256 where F: FnMut(U256) -> bool { + fn binary_chop(mut lower: U256, mut upper: U256, mut cond: F) -> Result + where F: FnMut(U256) -> Result + { while upper - lower > 1.into() { let mid = (lower + upper) / 2.into(); trace!(target: "estimate_gas", "{} .. {} .. {}", lower, mid, upper); - let c = cond(mid); + let c = cond(mid)?; match c { true => upper = mid, false => lower = mid, }; trace!(target: "estimate_gas", "{} => {} .. {}", c, lower, upper); } - upper + Ok(upper) } // binary chop to non-excepting call with gas somewhere between 21000 and block gas limit trace!(target: "estimate_gas", "estimate_gas chopping {} .. {}", lower, upper); - Ok(binary_chop(lower, upper, cond)) + binary_chop(lower, upper, cond) } fn replay(&self, id: TransactionId, analytics: CallAnalytics) -> Result { @@ -1006,17 +1013,16 @@ impl BlockChainClient for Client { let rest = txs.split_off(address.index); for t in txs { let t = SignedTransaction::new(t).expect(PROOF); - match Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(&t, Default::default()) { - Ok(x) => { env_info.gas_used = env_info.gas_used + x.gas_used; } - Err(ee) => { return Err(CallError::Execution(ee)) } - } + let x = Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(&t, Default::default())?; + env_info.gas_used = env_info.gas_used + x.gas_used; } let first = rest.into_iter().next().expect("We split off < `address.index`; Length is checked earlier; qed"); let t = SignedTransaction::new(first).expect(PROOF); let original_state = if analytics.state_diffing { Some(state.clone()) } else { None }; let mut ret = Executive::new(&mut state, &env_info, &*self.engine, &self.factories.vm).transact(&t, options)?; - ret.state_diff = original_state.map(|original| state.diff_from(original)); - + if let Some(original) = original_state { + ret.state_diff = Some(state.diff_from(original).map_err(ExecutionError::from)?) + } Ok(ret) } @@ -1108,11 +1114,11 @@ impl BlockChainClient for Client { } fn nonce(&self, address: &Address, id: BlockId) -> Option { - self.state_at(id).map(|s| s.nonce(address)) + self.state_at(id).and_then(|s| s.nonce(address).ok()) } fn storage_root(&self, address: &Address, id: BlockId) -> Option { - self.state_at(id).and_then(|s| s.storage_root(address)) + self.state_at(id).and_then(|s| s.storage_root(address).ok()).and_then(|x| x) } fn block_hash(&self, id: BlockId) -> Option { @@ -1121,15 +1127,15 @@ impl BlockChainClient for Client { } fn code(&self, address: &Address, id: BlockId) -> Option> { - self.state_at(id).map(|s| s.code(address).map(|c| (*c).clone())) + self.state_at(id).and_then(|s| s.code(address).ok()).map(|c| c.map(|c| (&*c).clone())) } fn balance(&self, address: &Address, id: BlockId) -> Option { - self.state_at(id).map(|s| s.balance(address)) + self.state_at(id).and_then(|s| s.balance(address).ok()) } fn storage_at(&self, address: &Address, position: &H256, id: BlockId) -> Option { - self.state_at(id).map(|s| s.storage_at(address, position)) + self.state_at(id).and_then(|s| s.storage_at(address, position).ok()) } fn list_accounts(&self, id: BlockId, after: Option<&Address>, count: u64) -> Option> { @@ -1182,7 +1188,7 @@ impl BlockChainClient for Client { }; let root = match state.storage_root(account) { - Some(root) => root, + Ok(Some(root)) => root, _ => return None, }; diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index 4f99a644e..e4efdaea1 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -250,10 +250,12 @@ impl Engine for AuthorityRound { fn on_close_block(&self, block: &mut ExecutedBlock) { let fields = block.fields_mut(); // Bestow block reward - fields.state.add_balance(fields.header.author(), &self.block_reward, CleanupMode::NoEmpty); + let res = fields.state.add_balance(fields.header.author(), &self.block_reward, CleanupMode::NoEmpty) + .map_err(::error::Error::from) + .and_then(|_| fields.state.commit()); // Commit state so that we can actually figure out the state root. - if let Err(e) = fields.state.commit() { - warn!("Encountered error on state commit: {}", e); + if let Err(e) = res { + warn!("Encountered error on closing block: {}", e); } } diff --git a/ethcore/src/engines/tendermint/mod.rs b/ethcore/src/engines/tendermint/mod.rs index 21301898a..1c9edd33b 100644 --- a/ethcore/src/engines/tendermint/mod.rs +++ b/ethcore/src/engines/tendermint/mod.rs @@ -467,10 +467,12 @@ impl Engine for Tendermint { fn on_close_block(&self, block: &mut ExecutedBlock) { let fields = block.fields_mut(); // Bestow block reward - fields.state.add_balance(fields.header.author(), &self.block_reward, CleanupMode::NoEmpty); + let res = fields.state.add_balance(fields.header.author(), &self.block_reward, CleanupMode::NoEmpty) + .map_err(::error::Error::from) + .and_then(|_| fields.state.commit()); // Commit state so that we can actually figure out the state root. - if let Err(e) = fields.state.commit() { - warn!("Encountered error on state commit: {}", e); + if let Err(e) = res { + warn!("Encountered error on closing block: {}", e); } } diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index e02667d90..fbb9c8a5c 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -229,10 +229,16 @@ impl Engine for Ethash { if block.fields().header.number() == self.ethash_params.dao_hardfork_transition { // TODO: enable trigger function maybe? // if block.fields().header.gas_limit() <= 4_000_000.into() { - let mut state = block.fields_mut().state; + let state = block.fields_mut().state; for child in &self.ethash_params.dao_hardfork_accounts { - let b = state.balance(child); - state.transfer_balance(child, &self.ethash_params.dao_hardfork_beneficiary, &b, CleanupMode::NoEmpty); + let beneficiary = &self.ethash_params.dao_hardfork_beneficiary; + let res = state.balance(child) + .and_then(|b| state.transfer_balance(child, beneficiary, &b, CleanupMode::NoEmpty)); + + if let Err(_) = res { + warn!("Unable to apply DAO hardfork due to database corruption."); + warn!("Your node is now likely out of consensus."); + } } // } } @@ -245,12 +251,28 @@ impl Engine for Ethash { let fields = block.fields_mut(); // Bestow block reward - fields.state.add_balance(fields.header.author(), &(reward + reward / U256::from(32) * U256::from(fields.uncles.len())), CleanupMode::NoEmpty); + let res = fields.state.add_balance( + fields.header.author(), + &(reward + reward / U256::from(32) * U256::from(fields.uncles.len())), + CleanupMode::NoEmpty + ); + + if let Err(e) = res { + warn!("Failed to give block reward: {}", e); + } // Bestow uncle rewards let current_number = fields.header.number(); for u in fields.uncles.iter() { - fields.state.add_balance(u.author(), &(reward * U256::from(8 + u.number() - current_number) / U256::from(8)), CleanupMode::NoEmpty); + let res = fields.state.add_balance( + u.author(), + &(reward * U256::from(8 + u.number() - current_number) / U256::from(8)), + CleanupMode::NoEmpty + ); + + if let Err(e) = res { + warn!("Failed to give uncle reward: {}", e); + } } // Commit state so that we can actually figure out the state root. @@ -491,7 +513,7 @@ mod tests { let last_hashes = Arc::new(vec![genesis_header.hash()]); let b = OpenBlock::new(engine, Default::default(), false, db, &genesis_header, last_hashes, Address::zero(), (3141562.into(), 31415620.into()), vec![]).unwrap(); let b = b.close(); - assert_eq!(b.state().balance(&Address::zero()), U256::from_str("4563918244f40000").unwrap()); + assert_eq!(b.state().balance(&Address::zero()).unwrap(), U256::from_str("4563918244f40000").unwrap()); } #[test] @@ -509,8 +531,8 @@ mod tests { b.push_uncle(uncle).unwrap(); let b = b.close(); - assert_eq!(b.state().balance(&Address::zero()), "478eae0e571ba000".into()); - assert_eq!(b.state().balance(&uncle_author), "3cb71f51fc558000".into()); + assert_eq!(b.state().balance(&Address::zero()).unwrap(), "478eae0e571ba000".into()); + assert_eq!(b.state().balance(&uncle_author).unwrap(), "3cb71f51fc558000".into()); } #[test] diff --git a/ethcore/src/ethereum/mod.rs b/ethcore/src/ethereum/mod.rs index c8eb44911..b15c9e4de 100644 --- a/ethcore/src/ethereum/mod.rs +++ b/ethcore/src/ethereum/mod.rs @@ -91,12 +91,12 @@ mod tests { let mut db_result = get_temp_state_db(); let db = spec.ensure_db_good(db_result.take(), &Default::default()).unwrap(); let s = State::from_existing(db, genesis_header.state_root().clone(), engine.account_start_nonce(), Default::default()).unwrap(); - assert_eq!(s.balance(&"0000000000000000000000000000000000000001".into()), 1u64.into()); - assert_eq!(s.balance(&"0000000000000000000000000000000000000002".into()), 1u64.into()); - assert_eq!(s.balance(&"0000000000000000000000000000000000000003".into()), 1u64.into()); - assert_eq!(s.balance(&"0000000000000000000000000000000000000004".into()), 1u64.into()); - assert_eq!(s.balance(&"102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c".into()), U256::from(1u64) << 200); - assert_eq!(s.balance(&"0000000000000000000000000000000000000000".into()), 0u64.into()); + assert_eq!(s.balance(&"0000000000000000000000000000000000000001".into()).unwrap(), 1u64.into()); + assert_eq!(s.balance(&"0000000000000000000000000000000000000002".into()).unwrap(), 1u64.into()); + assert_eq!(s.balance(&"0000000000000000000000000000000000000003".into()).unwrap(), 1u64.into()); + assert_eq!(s.balance(&"0000000000000000000000000000000000000004".into()).unwrap(), 1u64.into()); + assert_eq!(s.balance(&"102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c".into()).unwrap(), U256::from(1u64) << 200); + assert_eq!(s.balance(&"0000000000000000000000000000000000000000".into()).unwrap(), 0u64.into()); } #[test] diff --git a/ethcore/src/evm/evm.rs b/ethcore/src/evm/evm.rs index 420ebb6a0..09a93f087 100644 --- a/ethcore/src/evm/evm.rs +++ b/ethcore/src/evm/evm.rs @@ -17,12 +17,12 @@ //! Evm interface. use std::{ops, cmp, fmt}; -use util::{U128, U256, U512, Uint}; +use util::{U128, U256, U512, Uint, trie}; use action_params::ActionParams; use evm::Ext; /// Evm errors. -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub enum Error { /// `OutOfGas` is returned when transaction execution runs out of gas. /// The state should be reverted to the state from before the @@ -61,8 +61,13 @@ pub enum Error { }, /// Returned on evm internal error. Should never be ignored during development. /// Likely to cause consensus issues. - #[allow(dead_code)] // created only by jit - Internal, + Internal(String), +} + +impl From> for Error { + fn from(err: Box) -> Self { + Error::Internal(format!("Internal error: {}", err)) + } } impl fmt::Display for Error { @@ -74,7 +79,7 @@ impl fmt::Display for Error { BadInstruction { .. } => "Bad instruction", StackUnderflow { .. } => "Stack underflow", OutOfStack { .. } => "Out of stack", - Internal => "Internal error", + Internal(ref msg) => msg, }; message.fmt(f) } diff --git a/ethcore/src/evm/ext.rs b/ethcore/src/evm/ext.rs index e2578cc68..352ffb7d9 100644 --- a/ethcore/src/evm/ext.rs +++ b/ethcore/src/evm/ext.rs @@ -42,24 +42,25 @@ pub enum MessageCallResult { } /// Externalities interface for EVMs +// TODO: [rob] associated error type instead of `trie::Result`. Not all EVMs are trie powered. pub trait Ext { /// Returns a value for given key. - fn storage_at(&self, key: &H256) -> H256; + fn storage_at(&self, key: &H256) -> trie::Result; /// Stores a value for given key. - fn set_storage(&mut self, key: H256, value: H256); + fn set_storage(&mut self, key: H256, value: H256) -> trie::Result<()>; /// Determine whether an account exists. - fn exists(&self, address: &Address) -> bool; + fn exists(&self, address: &Address) -> trie::Result; /// Determine whether an account exists and is not null (zero balance/nonce, no code). - fn exists_and_not_null(&self, address: &Address) -> bool; + fn exists_and_not_null(&self, address: &Address) -> trie::Result; /// Balance of the origin account. - fn origin_balance(&self) -> U256; + fn origin_balance(&self) -> trie::Result; /// Returns address balance. - fn balance(&self, address: &Address) -> U256; + fn balance(&self, address: &Address) -> trie::Result; /// Returns the hash of one of the 256 most recent complete blocks. fn blockhash(&self, number: &U256) -> H256; @@ -87,10 +88,10 @@ pub trait Ext { ) -> MessageCallResult; /// Returns code at given address - fn extcode(&self, address: &Address) -> Arc; + fn extcode(&self, address: &Address) -> trie::Result>; /// Returns code size at given address - fn extcodesize(&self, address: &Address) -> usize; + fn extcodesize(&self, address: &Address) -> trie::Result; /// Creates log entry with given topics and data fn log(&mut self, topics: Vec, data: &[u8]); @@ -101,7 +102,7 @@ pub trait Ext { /// Should be called when contract commits suicide. /// Address to which funds should be refunded. - fn suicide(&mut self, refund_address: &Address); + fn suicide(&mut self, refund_address: &Address) -> trie::Result<()> ; /// Returns schedule. fn schedule(&self) -> &Schedule; diff --git a/ethcore/src/evm/interpreter/gasometer.rs b/ethcore/src/evm/interpreter/gasometer.rs index 5c96c3c05..9086200fa 100644 --- a/ethcore/src/evm/interpreter/gasometer.rs +++ b/ethcore/src/evm/interpreter/gasometer.rs @@ -123,7 +123,7 @@ impl Gasometer { instructions::SSTORE => { let address = H256::from(stack.peek(0)); let newval = stack.peek(1); - let val = U256::from(&*ext.storage_at(&address)); + let val = U256::from(&*ext.storage_at(&address)?); let gas = if val.is_zero() && !newval.is_zero() { schedule.sstore_set_gas @@ -146,12 +146,12 @@ impl Gasometer { instructions::SUICIDE => { let mut gas = Gas::from(schedule.suicide_gas); - let is_value_transfer = !ext.origin_balance().is_zero(); + let is_value_transfer = !ext.origin_balance()?.is_zero(); let address = u256_to_address(stack.peek(0)); if ( - !schedule.no_empty && !ext.exists(&address) + !schedule.no_empty && !ext.exists(&address)? ) || ( - schedule.no_empty && is_value_transfer && !ext.exists_and_not_null(&address) + schedule.no_empty && is_value_transfer && !ext.exists_and_not_null(&address)? ) { gas = overflowing!(gas.overflow_add(schedule.suicide_to_new_account_cost.into())); } @@ -198,9 +198,9 @@ impl Gasometer { let is_value_transfer = !stack.peek(2).is_zero(); if instruction == instructions::CALL && ( - (!schedule.no_empty && !ext.exists(&address)) + (!schedule.no_empty && !ext.exists(&address)?) || - (schedule.no_empty && is_value_transfer && !ext.exists_and_not_null(&address)) + (schedule.no_empty && is_value_transfer && !ext.exists_and_not_null(&address)?) ) { gas = overflowing!(gas.overflow_add(schedule.call_new_account_gas.into())); } diff --git a/ethcore/src/evm/interpreter/mod.rs b/ethcore/src/evm/interpreter/mod.rs index bc3caa084..79304793e 100644 --- a/ethcore/src/evm/interpreter/mod.rs +++ b/ethcore/src/evm/interpreter/mod.rs @@ -273,7 +273,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"); 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; + let can_create = ext.balance(¶ms.address)? >= endowment && ext.depth() < ext.schedule().max_depth; if !can_create { stack.push(U256::zero()); @@ -319,11 +319,11 @@ impl Interpreter { // Get sender & receive addresses, check if we have balance let (sender_address, receive_address, has_balance, call_type) = match instruction { instructions::CALL => { - let has_balance = ext.balance(¶ms.address) >= value.expect("value set for all but delegate call; qed"); + let has_balance = ext.balance(¶ms.address)? >= value.expect("value set for all but delegate call; qed"); (¶ms.address, &code_address, has_balance, CallType::Call) }, instructions::CALLCODE => { - let has_balance = ext.balance(¶ms.address) >= value.expect("value set for all but delegate call; qed"); + let has_balance = ext.balance(¶ms.address)? >= value.expect("value set for all but delegate call; qed"); (¶ms.address, ¶ms.address, has_balance, CallType::CallCode) }, instructions::DELEGATECALL => (¶ms.sender, ¶ms.address, true, CallType::DelegateCall), @@ -366,7 +366,7 @@ impl Interpreter { }, instructions::SUICIDE => { let address = stack.pop_back(); - ext.suicide(&u256_to_address(&address)); + ext.suicide(&u256_to_address(&address))?; return Ok(InstructionResult::StopExecution); }, instructions::LOG0...instructions::LOG4 => { @@ -410,19 +410,19 @@ impl Interpreter { }, instructions::SLOAD => { let key = H256::from(&stack.pop_back()); - let word = U256::from(&*ext.storage_at(&key)); + let word = U256::from(&*ext.storage_at(&key)?); stack.push(word); }, instructions::SSTORE => { let address = H256::from(&stack.pop_back()); let val = stack.pop_back(); - let current_val = U256::from(&*ext.storage_at(&address)); + let current_val = U256::from(&*ext.storage_at(&address)?); // Increase refund for clear if !self.is_zero(¤t_val) && self.is_zero(&val) { ext.inc_sstore_clears(); } - ext.set_storage(address, H256::from(&val)); + ext.set_storage(address, H256::from(&val))?; }, instructions::PC => { stack.push(U256::from(code.position - 1)); @@ -438,7 +438,7 @@ impl Interpreter { }, instructions::BALANCE => { let address = u256_to_address(&stack.pop_back()); - let balance = ext.balance(&address); + let balance = ext.balance(&address)?; stack.push(balance); }, instructions::CALLER => { @@ -474,7 +474,7 @@ impl Interpreter { }, instructions::EXTCODESIZE => { let address = u256_to_address(&stack.pop_back()); - let len = ext.extcodesize(&address); + let len = ext.extcodesize(&address)?; stack.push(U256::from(len)); }, instructions::CALLDATACOPY => { @@ -485,7 +485,7 @@ impl Interpreter { }, instructions::EXTCODECOPY => { let address = u256_to_address(&stack.pop_back()); - let code = ext.extcode(&address); + let code = ext.extcode(&address)?; self.copy_data_to_memory(stack, &code); }, instructions::GASPRICE => { diff --git a/ethcore/src/evm/tests.rs b/ethcore/src/evm/tests.rs index ccf711a40..3002c170c 100644 --- a/ethcore/src/evm/tests.rs +++ b/ethcore/src/evm/tests.rs @@ -82,28 +82,29 @@ impl Default for Schedule { } impl Ext for FakeExt { - fn storage_at(&self, key: &H256) -> H256 { - self.store.get(key).unwrap_or(&H256::new()).clone() + fn storage_at(&self, key: &H256) -> trie::Result { + Ok(self.store.get(key).unwrap_or(&H256::new()).clone()) } - fn set_storage(&mut self, key: H256, value: H256) { + fn set_storage(&mut self, key: H256, value: H256) -> trie::Result<()> { self.store.insert(key, value); + Ok(()) } - fn exists(&self, address: &Address) -> bool { - self.balances.contains_key(address) + fn exists(&self, address: &Address) -> trie::Result { + Ok(self.balances.contains_key(address)) } - fn exists_and_not_null(&self, address: &Address) -> bool { - self.balances.get(address).map_or(false, |b| !b.is_zero()) + fn exists_and_not_null(&self, address: &Address) -> trie::Result { + Ok(self.balances.get(address).map_or(false, |b| !b.is_zero())) } - fn origin_balance(&self) -> U256 { + fn origin_balance(&self) -> trie::Result { unimplemented!() } - fn balance(&self, address: &Address) -> U256 { - self.balances[address] + fn balance(&self, address: &Address) -> trie::Result { + Ok(self.balances[address]) } fn blockhash(&self, number: &U256) -> H256 { @@ -146,12 +147,12 @@ impl Ext for FakeExt { MessageCallResult::Success(*gas) } - fn extcode(&self, address: &Address) -> Arc { - self.codes.get(address).unwrap_or(&Arc::new(Bytes::new())).clone() + fn extcode(&self, address: &Address) -> trie::Result> { + Ok(self.codes.get(address).unwrap_or(&Arc::new(Bytes::new())).clone()) } - fn extcodesize(&self, address: &Address) -> usize { - self.codes.get(address).map_or(0, |c| c.len()) + fn extcodesize(&self, address: &Address) -> trie::Result { + Ok(self.codes.get(address).map_or(0, |c| c.len())) } fn log(&mut self, topics: Vec, data: &[u8]) { @@ -165,7 +166,7 @@ impl Ext for FakeExt { unimplemented!(); } - fn suicide(&mut self, _refund_address: &Address) { + fn suicide(&mut self, _refund_address: &Address) -> trie::Result<()> { unimplemented!(); } diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 37b53202a..d9f1b7413 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -123,7 +123,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { mut vm_tracer: V ) -> Result where T: Tracer, V: VMTracer { let sender = t.sender(); - let nonce = self.state.nonce(&sender); + let nonce = self.state.nonce(&sender)?; let schedule = self.engine.schedule(self.info); let base_gas_required = U256::from(t.gas_required(&schedule)); @@ -149,7 +149,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { } // TODO: we might need bigints here, or at least check overflows. - let balance = self.state.balance(&sender); + let balance = self.state.balance(&sender)?; let gas_cost = t.gas.full_mul(t.gas_price); let total_cost = U512::from(t.value) + gas_cost; @@ -160,8 +160,8 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { } // NOTE: there can be no invalid transactions from this point. - self.state.inc_nonce(&sender); - self.state.sub_balance(&sender, &U256::from(gas_cost)); + self.state.inc_nonce(&sender)?; + self.state.sub_balance(&sender, &U256::from(gas_cost))?; let mut substate = Substate::new(); @@ -192,8 +192,8 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { gas: init_gas, gas_price: t.gas_price, value: ActionValue::Transfer(t.value), - code: self.state.code(address), - code_hash: self.state.code_hash(address), + code: self.state.code(address)?, + code_hash: self.state.code_hash(address)?, data: Some(t.data.clone()), call_type: CallType::Call, }; @@ -257,7 +257,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // at first, transfer value to destination if let ActionValue::Transfer(val) = params.value { - self.state.transfer_balance(¶ms.sender, ¶ms.address, &val, substate.to_cleanup_mode(&schedule)); + self.state.transfer_balance(¶ms.sender, ¶ms.address, &val, substate.to_cleanup_mode(&schedule))?; } trace!("Executive::call(params={:?}) self.env_info={:?}", params, self.info); @@ -322,13 +322,13 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let traces = subtracer.traces(); match res { - Ok(gas_left) => tracer.trace_call( + Ok(ref gas_left) => tracer.trace_call( trace_info, - gas - gas_left, + gas - *gas_left, trace_output, traces ), - Err(e) => tracer.trace_failed_call(trace_info, traces, e.into()), + Err(ref e) => tracer.trace_failed_call(trace_info, traces, e.into()), }; trace!(target: "executive", "substate={:?}; unconfirmed_substate={:?}\n", substate, unconfirmed_substate); @@ -365,9 +365,9 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // create contract and transfer value to it if necessary let schedule = self.engine.schedule(self.info); let nonce_offset = if schedule.no_empty {1} else {0}.into(); - let prev_bal = self.state.balance(¶ms.address); + let prev_bal = self.state.balance(¶ms.address)?; if let ActionValue::Transfer(val) = params.value { - self.state.sub_balance(¶ms.sender, &val); + self.state.sub_balance(¶ms.sender, &val)?; self.state.new_contract(¶ms.address, val + prev_bal, nonce_offset); } else { self.state.new_contract(¶ms.address, prev_bal, nonce_offset); @@ -388,14 +388,14 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { vm_tracer.done_subtrace(subvmtracer); match res { - Ok(gas_left) => tracer.trace_create( + Ok(ref gas_left) => tracer.trace_create( trace_info, - gas - gas_left, + gas - *gas_left, trace_output, created, subtracer.traces() ), - Err(e) => tracer.trace_failed_create(trace_info, subtracer.traces(), e.into()) + Err(ref e) => tracer.trace_failed_create(trace_info, subtracer.traces(), e.into()) }; self.enact_result(&res, substate, unconfirmed_substate); @@ -435,9 +435,9 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { let sender = t.sender(); trace!("exec::finalize: Refunding refund_value={}, sender={}\n", refund_value, sender); // Below: NoEmpty is safe since the sender must already be non-null to have sent this transaction - self.state.add_balance(&sender, &refund_value, CleanupMode::NoEmpty); + self.state.add_balance(&sender, &refund_value, CleanupMode::NoEmpty)?; trace!("exec::finalize: Compensating author: fees_value={}, author={}\n", fees_value, &self.info.author); - self.state.add_balance(&self.info.author, &fees_value, substate.to_cleanup_mode(&schedule)); + self.state.add_balance(&self.info.author, &fees_value, substate.to_cleanup_mode(&schedule))?; // perform suicides for address in &substate.suicides { @@ -446,13 +446,13 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { // perform garbage-collection for address in &substate.garbage { - if self.state.exists(address) && !self.state.exists_and_not_null(address) { + if self.state.exists(address)? && !self.state.exists_and_not_null(address)? { self.state.kill_account(address); } } match result { - Err(evm::Error::Internal) => Err(ExecutionError::Internal), + Err(evm::Error::Internal(msg)) => Err(ExecutionError::Internal(msg)), Err(exception) => { Ok(Executed { exception: Some(exception), @@ -495,7 +495,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> { | Err(evm::Error::OutOfStack {..}) => { self.state.revert_to_checkpoint(); }, - Ok(_) | Err(evm::Error::Internal) => { + Ok(_) | Err(evm::Error::Internal(_)) => { self.state.discard_checkpoint(); substate.accrue(un_substate); } @@ -544,7 +544,7 @@ mod tests { params.value = ActionValue::Transfer(U256::from(0x7)); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(0x100u64), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(0x100u64), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(0); let mut substate = Substate::new(); @@ -555,9 +555,9 @@ mod tests { }; assert_eq!(gas_left, U256::from(79_975)); - assert_eq!(state.storage_at(&address, &H256::new()), H256::from(&U256::from(0xf9u64))); - assert_eq!(state.balance(&sender), U256::from(0xf9)); - assert_eq!(state.balance(&address), U256::from(0x7)); + assert_eq!(state.storage_at(&address, &H256::new()).unwrap(), H256::from(&U256::from(0xf9u64))); + assert_eq!(state.balance(&sender).unwrap(), U256::from(0xf9)); + assert_eq!(state.balance(&address).unwrap(), U256::from(0x7)); // 0 cause contract hasn't returned assert_eq!(substate.contracts_created.len(), 0); @@ -603,7 +603,7 @@ mod tests { params.value = ActionValue::Transfer(U256::from(100)); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(0); let mut substate = Substate::new(); @@ -662,7 +662,7 @@ mod tests { params.call_type = CallType::Call; let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(5); let mut substate = Substate::new(); @@ -773,7 +773,7 @@ mod tests { params.value = ActionValue::Transfer(100.into()); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(5); let mut substate = Substate::new(); @@ -861,7 +861,7 @@ mod tests { params.value = ActionValue::Transfer(U256::from(100)); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(0); let mut substate = Substate::new(); @@ -913,7 +913,7 @@ mod tests { params.value = ActionValue::Transfer(U256::from(100)); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(100), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(1024); let mut substate = Substate::new(); @@ -971,9 +971,9 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.init_code(&address_a, code_a.clone()); - state.init_code(&address_b, code_b.clone()); - state.add_balance(&sender, &U256::from(100_000), CleanupMode::NoEmpty); + state.init_code(&address_a, code_a.clone()).unwrap(); + state.init_code(&address_b, code_b.clone()).unwrap(); + state.add_balance(&sender, &U256::from(100_000), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(0); @@ -985,7 +985,7 @@ mod tests { }; assert_eq!(gas_left, U256::from(73_237)); - assert_eq!(state.storage_at(&address_a, &H256::from(&U256::from(0x23))), H256::from(&U256::from(1))); + assert_eq!(state.storage_at(&address_a, &H256::from(&U256::from(0x23))).unwrap(), H256::from(&U256::from(1))); } // test is incorrect, mk @@ -1019,7 +1019,7 @@ mod tests { params.code = Some(Arc::new(code.clone())); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.init_code(&address, code); + state.init_code(&address, code).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(0); let mut substate = Substate::new(); @@ -1030,8 +1030,8 @@ mod tests { }; assert_eq!(gas_left, U256::from(59_870)); - assert_eq!(state.storage_at(&address, &H256::from(&U256::zero())), H256::from(&U256::from(1))); - assert_eq!(state.storage_at(&address, &H256::from(&U256::one())), H256::from(&U256::from(1))); + assert_eq!(state.storage_at(&address, &H256::from(&U256::zero())).unwrap(), H256::from(&U256::from(1))); + assert_eq!(state.storage_at(&address, &H256::from(&U256::one())).unwrap(), H256::from(&U256::from(1))); } // test is incorrect, mk @@ -1052,7 +1052,7 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(18), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(18), CleanupMode::NoEmpty).unwrap(); let mut info = EnvInfo::default(); info.gas_limit = U256::from(100_000); let engine = TestEngine::new(0); @@ -1069,10 +1069,10 @@ mod tests { assert_eq!(executed.cumulative_gas_used, U256::from(41_301)); assert_eq!(executed.logs.len(), 0); assert_eq!(executed.contracts_created.len(), 0); - assert_eq!(state.balance(&sender), U256::from(1)); - assert_eq!(state.balance(&contract), U256::from(17)); - assert_eq!(state.nonce(&sender), U256::from(1)); - assert_eq!(state.storage_at(&contract, &H256::new()), H256::from(&U256::from(1))); + assert_eq!(state.balance(&sender).unwrap(), U256::from(1)); + assert_eq!(state.balance(&contract).unwrap(), U256::from(17)); + assert_eq!(state.nonce(&sender).unwrap(), U256::from(1)); + assert_eq!(state.storage_at(&contract, &H256::new()).unwrap(), H256::from(&U256::from(1))); } evm_test!{test_transact_invalid_nonce: test_transact_invalid_nonce_jit, test_transact_invalid_nonce_int} @@ -1090,7 +1090,7 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(17), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(17), CleanupMode::NoEmpty).unwrap(); let mut info = EnvInfo::default(); info.gas_limit = U256::from(100_000); let engine = TestEngine::new(0); @@ -1123,7 +1123,7 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(17), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(17), CleanupMode::NoEmpty).unwrap(); let mut info = EnvInfo::default(); info.gas_used = U256::from(20_000); info.gas_limit = U256::from(100_000); @@ -1158,7 +1158,7 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from(100_017), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from(100_017), CleanupMode::NoEmpty).unwrap(); let mut info = EnvInfo::default(); info.gas_limit = U256::from(100_000); let engine = TestEngine::new(0); @@ -1193,7 +1193,7 @@ mod tests { params.value = ActionValue::Transfer(U256::from_str("0de0b6b3a7640000").unwrap()); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.add_balance(&sender, &U256::from_str("152d02c7e14af6800000").unwrap(), CleanupMode::NoEmpty); + state.add_balance(&sender, &U256::from_str("152d02c7e14af6800000").unwrap(), CleanupMode::NoEmpty).unwrap(); let info = EnvInfo::default(); let engine = TestEngine::new(0); let mut substate = Substate::new(); diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index 49ed2261e..893ba03be 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -108,25 +108,25 @@ impl<'a, T: 'a, V: 'a, B: 'a> Externalities<'a, T, V, B> impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> where T: Tracer, V: VMTracer, B: StateBackend { - fn storage_at(&self, key: &H256) -> H256 { + fn storage_at(&self, key: &H256) -> trie::Result { self.state.storage_at(&self.origin_info.address, key) } - fn set_storage(&mut self, key: H256, value: H256) { + fn set_storage(&mut self, key: H256, value: H256) -> trie::Result<()> { self.state.set_storage(&self.origin_info.address, key, value) } - fn exists(&self, address: &Address) -> bool { + fn exists(&self, address: &Address) -> trie::Result { self.state.exists(address) } - fn exists_and_not_null(&self, address: &Address) -> bool { + fn exists_and_not_null(&self, address: &Address) -> trie::Result { self.state.exists_and_not_null(address) } - fn origin_balance(&self) -> U256 { self.balance(&self.origin_info.address) } + fn origin_balance(&self) -> trie::Result { self.balance(&self.origin_info.address) } - fn balance(&self, address: &Address) -> U256 { + fn balance(&self, address: &Address) -> trie::Result { self.state.balance(address) } @@ -149,7 +149,13 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> fn create(&mut self, gas: &U256, value: &U256, code: &[u8]) -> ContractCreateResult { // create new contract address - let address = contract_address(&self.origin_info.address, &self.state.nonce(&self.origin_info.address)); + let address = match self.state.nonce(&self.origin_info.address) { + Ok(nonce) => contract_address(&self.origin_info.address, &nonce), + Err(e) => { + debug!(target: "ext", "Database corruption encountered: {:?}", e); + return ContractCreateResult::Failed + } + }; // prepare the params let params = ActionParams { @@ -166,7 +172,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> call_type: CallType::None, }; - self.state.inc_nonce(&self.origin_info.address); + if let Err(e) = self.state.inc_nonce(&self.origin_info.address) { + debug!(target: "ext", "Database corruption encountered: {:?}", e); + return ContractCreateResult::Failed + } let mut ex = Executive::from_parent(self.state, self.env_info, self.engine, self.vm_factory, self.depth); // TODO: handle internal error separately @@ -191,6 +200,14 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> ) -> MessageCallResult { trace!(target: "externalities", "call"); + let code_res = self.state.code(code_address) + .and_then(|code| self.state.code_hash(code_address).map(|hash| (code, hash))); + + let (code, code_hash) = match code_res { + Ok((code, hash)) => (code, hash), + Err(_) => return MessageCallResult::Failed, + }; + let mut params = ActionParams { sender: sender_address.clone(), address: receive_address.clone(), @@ -199,8 +216,8 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> origin: self.origin_info.origin.clone(), gas: *gas, gas_price: self.origin_info.gas_price, - code: self.state.code(code_address), - code_hash: self.state.code_hash(code_address), + code: code, + code_hash: code_hash, data: Some(data.to_vec()), call_type: call_type, }; @@ -217,12 +234,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> } } - fn extcode(&self, address: &Address) -> Arc { - self.state.code(address).unwrap_or_else(|| Arc::new(vec![])) + fn extcode(&self, address: &Address) -> trie::Result> { + Ok(self.state.code(address)?.unwrap_or_else(|| Arc::new(vec![]))) } - fn extcodesize(&self, address: &Address) -> usize { - self.state.code_size(address).unwrap_or(0) + fn extcodesize(&self, address: &Address) -> trie::Result { + Ok(self.state.code_size(address)?.unwrap_or(0)) } #[cfg_attr(feature="dev", allow(match_ref_pats))] @@ -257,10 +274,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> handle_copy(copy); - let mut code = vec![]; - code.extend_from_slice(data); - - self.state.init_code(&self.origin_info.address, code); + self.state.init_code(&self.origin_info.address, data.to_vec())?; Ok(*gas - return_cost) } } @@ -277,19 +291,26 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> }); } - fn suicide(&mut self, refund_address: &Address) { + fn suicide(&mut self, refund_address: &Address) -> trie::Result<()> { let address = self.origin_info.address.clone(); - let balance = self.balance(&address); + let balance = self.balance(&address)?; if &address == refund_address { // TODO [todr] To be consistent with CPP client we set balance to 0 in that case. - self.state.sub_balance(&address, &balance); + self.state.sub_balance(&address, &balance)?; } else { trace!(target: "ext", "Suiciding {} -> {} (xfer: {})", address, refund_address, balance); - self.state.transfer_balance(&address, refund_address, &balance, self.substate.to_cleanup_mode(&self.schedule)); + self.state.transfer_balance( + &address, + refund_address, + &balance, + self.substate.to_cleanup_mode(&self.schedule) + )?; } self.tracer.trace_suicide(address, balance, refund_address.clone()); self.substate.suicides.insert(address); + + Ok(()) } fn schedule(&self) -> &Schedule { @@ -485,7 +506,7 @@ mod tests { { let vm_factory = Default::default(); let mut ext = Externalities::new(state, &setup.env_info, &*setup.engine, &vm_factory, 0, get_test_origin(), &mut setup.sub_state, OutputPolicy::InitContract(None), &mut tracer, &mut vm_tracer); - ext.suicide(refund_account); + ext.suicide(refund_account).unwrap(); } assert_eq!(setup.sub_state.suicides.len(), 1); diff --git a/ethcore/src/json_tests/executive.rs b/ethcore/src/json_tests/executive.rs index f2e73ba97..844fa08f5 100644 --- a/ethcore/src/json_tests/executive.rs +++ b/ethcore/src/json_tests/executive.rs @@ -74,39 +74,39 @@ impl<'a, T: 'a, V: 'a, B: 'a> TestExt<'a, T, V, B> address: Address, tracer: &'a mut T, vm_tracer: &'a mut V, - ) -> Self { - TestExt { - contract_address: contract_address(&address, &state.nonce(&address)), + ) -> trie::Result { + Ok(TestExt { + contract_address: contract_address(&address, &state.nonce(&address)?), ext: Externalities::new(state, info, engine, vm_factory, depth, origin_info, substate, output, tracer, vm_tracer), callcreates: vec![] - } + }) } } impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B> where T: Tracer, V: VMTracer, B: StateBackend { - fn storage_at(&self, key: &H256) -> H256 { + fn storage_at(&self, key: &H256) -> trie::Result { self.ext.storage_at(key) } - fn set_storage(&mut self, key: H256, value: H256) { + fn set_storage(&mut self, key: H256, value: H256) -> trie::Result<()> { self.ext.set_storage(key, value) } - fn exists(&self, address: &Address) -> bool { + fn exists(&self, address: &Address) -> trie::Result { self.ext.exists(address) } - fn exists_and_not_null(&self, address: &Address) -> bool { + fn exists_and_not_null(&self, address: &Address) -> trie::Result { self.ext.exists_and_not_null(address) } - fn balance(&self, address: &Address) -> U256 { + fn balance(&self, address: &Address) -> trie::Result { self.ext.balance(address) } - fn origin_balance(&self) -> U256 { + fn origin_balance(&self) -> trie::Result { self.ext.origin_balance() } @@ -143,11 +143,11 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B> MessageCallResult::Success(*gas) } - fn extcode(&self, address: &Address) -> Arc { + fn extcode(&self, address: &Address) -> trie::Result> { self.ext.extcode(address) } - fn extcodesize(&self, address: &Address) -> usize { + fn extcodesize(&self, address: &Address) -> trie::Result { self.ext.extcodesize(address) } @@ -159,7 +159,7 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B> self.ext.ret(gas, data) } - fn suicide(&mut self, refund_address: &Address) { + fn suicide(&mut self, refund_address: &Address) -> trie::Result<()> { self.ext.suicide(refund_address) } @@ -201,6 +201,19 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8]) -> Vec { fail = true }; + macro_rules! try_fail { + ($e: expr) => { + match $e { + Ok(x) => x, + Err(e) => { + let msg = format!("Internal error: {}", e); + fail_unless(false, &msg); + continue + } + } + } + } + let out_of_gas = vm.out_of_gas(); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); @@ -217,7 +230,7 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8]) -> Vec { // execute let (res, callcreates) = { - let mut ex = TestExt::new( + let mut ex = try_fail!(TestExt::new( &mut state, &info, &engine, @@ -229,7 +242,7 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8]) -> Vec { params.address.clone(), &mut tracer, &mut vm_tracer, - ); + )); let mut evm = vm_factory.create(params.gas); let res = evm.exec(params, &mut ex); // a return in finalize will not alter callcreates @@ -248,14 +261,19 @@ fn do_json_test_for(vm_type: &VMType, json_data: &[u8]) -> Vec { for (address, account) in vm.post_state.unwrap().into_iter() { let address = address.into(); let code: Vec = account.code.into(); - fail_unless(state.code(&address).as_ref().map_or_else(|| code.is_empty(), |c| &**c == &code), "code is incorrect"); - fail_unless(state.balance(&address) == account.balance.into(), "balance is incorrect"); - fail_unless(state.nonce(&address) == account.nonce.into(), "nonce is incorrect"); - account.storage.into_iter().foreach(|(k, v)| { + let found_code = try_fail!(state.code(&address)); + let found_balance = try_fail!(state.balance(&address)); + let found_nonce = try_fail!(state.nonce(&address)); + + fail_unless(found_code.as_ref().map_or_else(|| code.is_empty(), |c| &**c == &code), "code is incorrect"); + fail_unless(found_balance == account.balance.into(), "balance is incorrect"); + fail_unless(found_nonce == account.nonce.into(), "nonce is incorrect"); + for (k, v) in account.storage { let key: U256 = k.into(); let value: U256 = v.into(); - fail_unless(state.storage_at(&address, &From::from(key)) == From::from(value), "storage is incorrect"); - }); + let found_storage = try_fail!(state.storage_at(&address, &From::from(key))); + fail_unless(found_storage == From::from(value), "storage is incorrect"); + } } let calls: Option> = vm.calls.map(|c| c.into_iter().map(From::from).collect()); diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 9cfd4a4a0..99c22f88e 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -711,17 +711,20 @@ impl MinerService for Miner { let original_state = if analytics.state_diffing { Some(state.clone()) } else { None }; let sender = t.sender(); - let balance = state.balance(&sender); + let balance = state.balance(&sender).map_err(ExecutionError::from)?; let needed_balance = t.value + t.gas * t.gas_price; if balance < needed_balance { // give the sender a sufficient balance - state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty); + state.add_balance(&sender, &(needed_balance - balance), CleanupMode::NoEmpty) + .map_err(ExecutionError::from)?; } let options = TransactOptions { tracing: analytics.transaction_tracing, vm_tracing: analytics.vm_tracing, check_nonce: false }; let mut ret = Executive::new(&mut state, &env_info, &*self.engine, client.vm_factory()).transact(t, options)?; // TODO gav move this into Executive. - ret.state_diff = original_state.map(|original| state.diff_from(original)); + if let Some(original) = original_state { + ret.state_diff = Some(state.diff_from(original).map_err(ExecutionError::from)?); + } Ok(ret) }, @@ -729,35 +732,37 @@ impl MinerService for Miner { } } - fn balance(&self, chain: &MiningBlockChainClient, address: &Address) -> U256 { + // TODO: The `chain.latest_x` actually aren't infallible, they just panic on corruption. + // TODO: return trie::Result here, or other. + fn balance(&self, chain: &MiningBlockChainClient, address: &Address) -> Option { self.from_pending_block( chain.chain_info().best_block_number, - || chain.latest_balance(address), - |b| b.block().fields().state.balance(address) + || Some(chain.latest_balance(address)), + |b| b.block().fields().state.balance(address).ok(), ) } - fn storage_at(&self, chain: &MiningBlockChainClient, address: &Address, position: &H256) -> H256 { + fn storage_at(&self, chain: &MiningBlockChainClient, address: &Address, position: &H256) -> Option { self.from_pending_block( chain.chain_info().best_block_number, - || chain.latest_storage_at(address, position), - |b| b.block().fields().state.storage_at(address, position) + || Some(chain.latest_storage_at(address, position)), + |b| b.block().fields().state.storage_at(address, position).ok(), ) } - fn nonce(&self, chain: &MiningBlockChainClient, address: &Address) -> U256 { + fn nonce(&self, chain: &MiningBlockChainClient, address: &Address) -> Option { self.from_pending_block( chain.chain_info().best_block_number, - || chain.latest_nonce(address), - |b| b.block().fields().state.nonce(address) + || Some(chain.latest_nonce(address)), + |b| b.block().fields().state.nonce(address).ok(), ) } - fn code(&self, chain: &MiningBlockChainClient, address: &Address) -> Option { + fn code(&self, chain: &MiningBlockChainClient, address: &Address) -> Option> { self.from_pending_block( chain.chain_info().best_block_number, - || chain.latest_code(address), - |b| b.block().fields().state.code(address).map(|c| (*c).clone()) + || Some(chain.latest_code(address)), + |b| b.block().fields().state.code(address).ok().map(|c| c.map(|c| (&*c).clone())) ) } diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index eee9d1c5c..74e1cb598 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -181,19 +181,19 @@ pub trait MinerService : Send + Sync { fn sensible_gas_limit(&self) -> U256 { 21000.into() } /// Latest account balance in pending state. - fn balance(&self, chain: &MiningBlockChainClient, address: &Address) -> U256; + fn balance(&self, chain: &MiningBlockChainClient, address: &Address) -> Option; /// Call into contract code using pending state. fn call(&self, chain: &MiningBlockChainClient, t: &SignedTransaction, analytics: CallAnalytics) -> Result; /// Get storage value in pending state. - fn storage_at(&self, chain: &MiningBlockChainClient, address: &Address, position: &H256) -> H256; + fn storage_at(&self, chain: &MiningBlockChainClient, address: &Address, position: &H256) -> Option; /// Get account nonce in pending state. - fn nonce(&self, chain: &MiningBlockChainClient, address: &Address) -> U256; + fn nonce(&self, chain: &MiningBlockChainClient, address: &Address) -> Option; /// Get contract code in pending state. - fn code(&self, chain: &MiningBlockChainClient, address: &Address) -> Option; + fn code(&self, chain: &MiningBlockChainClient, address: &Address) -> Option>; } /// Mining status diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 85996d24b..67e21208b 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -389,6 +389,6 @@ mod tests { let db = spec.ensure_db_good(db_result.take(), &Default::default()).unwrap(); let state = State::from_existing(db.boxed_clone(), spec.state_root(), spec.engine.account_start_nonce(), Default::default()).unwrap(); let expected = H256::from_str("0000000000000000000000000000000000000000000000000000000000000001").unwrap(); - assert_eq!(state.storage_at(&Address::from_str("0000000000000000000000000000000000000005").unwrap(), &H256::zero()), expected); + assert_eq!(state.storage_at(&Address::from_str("0000000000000000000000000000000000000005").unwrap(), &H256::zero()).unwrap(), expected); } } diff --git a/ethcore/src/state/account.rs b/ethcore/src/state/account.rs index 6e5297bc4..ebdf36d89 100644 --- a/ethcore/src/state/account.rs +++ b/ethcore/src/state/account.rs @@ -169,22 +169,16 @@ impl Account { /// Get (and cache) the contents of the trie's storage at `key`. /// Takes modifed storage into account. - pub fn storage_at(&self, db: &HashDB, key: &H256) -> H256 { + pub fn storage_at(&self, db: &HashDB, key: &H256) -> trie::Result { if let Some(value) = self.cached_storage_at(key) { - return value; + return Ok(value); } - let db = SecTrieDB::new(db, &self.storage_root) - .expect("Account storage_root initially set to zero (valid) and only altered by SecTrieDBMut. \ - SecTrieDBMut would not set it to an invalid state root. Therefore the root is valid and DB creation \ - using it will not fail."); + let db = SecTrieDB::new(db, &self.storage_root)?; - let item: U256 = match db.get_with(key, ::rlp::decode) { - Ok(x) => x.unwrap_or_else(U256::zero), - Err(e) => panic!("Encountered potential DB corruption: {}", e), - }; + let item: U256 = db.get_with(key, ::rlp::decode)?.unwrap_or_else(U256::zero); let value: H256 = item.into(); self.storage_cache.borrow_mut().insert(key.clone(), value.clone()); - value + Ok(value) } /// Get cached storage value if any. Returns `None` if the @@ -345,24 +339,19 @@ impl Account { } /// Commit the `storage_changes` to the backing DB and update `storage_root`. - pub fn commit_storage(&mut self, trie_factory: &TrieFactory, db: &mut HashDB) { - let mut t = trie_factory.from_existing(db, &mut self.storage_root) - .expect("Account storage_root initially set to zero (valid) and only altered by SecTrieDBMut. \ - SecTrieDBMut would not set it to an invalid state root. Therefore the root is valid and DB creation \ - using it will not fail."); + pub fn commit_storage(&mut self, trie_factory: &TrieFactory, db: &mut HashDB) -> trie::Result<()> { + let mut t = trie_factory.from_existing(db, &mut self.storage_root)?; for (k, v) in self.storage_changes.drain() { // cast key and value to trait type, // so we can call overloaded `to_bytes` method - let res = match v.is_zero() { - true => t.remove(&k), - false => t.insert(&k, &encode(&U256::from(&*v))), + match v.is_zero() { + true => t.remove(&k)?, + false => t.insert(&k, &encode(&U256::from(&*v)))?, }; - if let Err(e) = res { - warn!("Encountered potential DB corruption: {}", e); - } self.storage_cache.borrow_mut().insert(k, v); } + Ok(()) } /// Commit any unsaved code. `code_hash` will always return the hash of the `code_cache` after this. @@ -494,7 +483,7 @@ mod tests { let rlp = { let mut a = Account::new_contract(69.into(), 0.into()); a.set_storage(H256::from(&U256::from(0x00u64)), H256::from(&U256::from(0x1234u64))); - a.commit_storage(&Default::default(), &mut db); + a.commit_storage(&Default::default(), &mut db).unwrap(); a.init_code(vec![]); a.commit_code(&mut db); a.rlp() @@ -502,8 +491,8 @@ mod tests { let a = Account::from_rlp(&rlp); assert_eq!(a.storage_root().unwrap().hex(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2"); - assert_eq!(a.storage_at(&db.immutable(), &H256::from(&U256::from(0x00u64))), H256::from(&U256::from(0x1234u64))); - assert_eq!(a.storage_at(&db.immutable(), &H256::from(&U256::from(0x01u64))), H256::new()); + assert_eq!(a.storage_at(&db.immutable(), &H256::from(&U256::from(0x00u64))).unwrap(), H256::from(&U256::from(0x1234u64))); + assert_eq!(a.storage_at(&db.immutable(), &H256::from(&U256::from(0x01u64))).unwrap(), H256::new()); } #[test] @@ -532,7 +521,7 @@ mod tests { let mut db = AccountDBMut::new(&mut db, &Address::new()); a.set_storage(0.into(), 0x1234.into()); assert_eq!(a.storage_root(), None); - a.commit_storage(&Default::default(), &mut db); + a.commit_storage(&Default::default(), &mut db).unwrap(); assert_eq!(a.storage_root().unwrap().hex(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2"); } @@ -542,11 +531,11 @@ mod tests { let mut db = MemoryDB::new(); let mut db = AccountDBMut::new(&mut db, &Address::new()); a.set_storage(0.into(), 0x1234.into()); - a.commit_storage(&Default::default(), &mut db); + a.commit_storage(&Default::default(), &mut db).unwrap(); a.set_storage(1.into(), 0x1234.into()); - a.commit_storage(&Default::default(), &mut db); + a.commit_storage(&Default::default(), &mut db).unwrap(); a.set_storage(1.into(), 0.into()); - a.commit_storage(&Default::default(), &mut db); + a.commit_storage(&Default::default(), &mut db).unwrap(); assert_eq!(a.storage_root().unwrap().hex(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2"); } diff --git a/ethcore/src/state/mod.rs b/ethcore/src/state/mod.rs index d404f7b73..ebac907aa 100644 --- a/ethcore/src/state/mod.rs +++ b/ethcore/src/state/mod.rs @@ -37,6 +37,7 @@ use state_db::StateDB; use util::*; +use util::trie; use util::trie::recorder::Recorder; mod account; @@ -362,37 +363,37 @@ impl State { } /// Determine whether an account exists. - pub fn exists(&self, a: &Address) -> bool { + pub fn exists(&self, a: &Address) -> trie::Result { // Bloom filter does not contain empty accounts, so it is important here to // check if account exists in the database directly before EIP-161 is in effect. self.ensure_cached(a, RequireCache::None, false, |a| a.is_some()) } /// Determine whether an account exists and if not empty. - pub fn exists_and_not_null(&self, a: &Address) -> bool { + pub fn exists_and_not_null(&self, a: &Address) -> trie::Result { self.ensure_cached(a, RequireCache::None, false, |a| a.map_or(false, |a| !a.is_null())) } /// Get the balance of account `a`. - pub fn balance(&self, a: &Address) -> U256 { + pub fn balance(&self, a: &Address) -> trie::Result { self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().map_or(U256::zero(), |account| *account.balance())) } /// Get the nonce of account `a`. - pub fn nonce(&self, a: &Address) -> U256 { + pub fn nonce(&self, a: &Address) -> trie::Result { self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().map_or(self.account_start_nonce, |account| *account.nonce())) } /// Get the storage root of account `a`. - pub fn storage_root(&self, a: &Address) -> Option { + pub fn storage_root(&self, a: &Address) -> trie::Result> { self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().and_then(|account| account.storage_root().cloned())) } /// Mutate storage of account `address` so that it is `value` for `key`. - pub fn storage_at(&self, address: &Address, key: &H256) -> H256 { + pub fn storage_at(&self, address: &Address, key: &H256) -> trie::Result { // Storage key search and update works like this: // 1. If there's an entry for the account in the local cache check for the key and return it if found. // 2. If there's an entry for the account in the global cache check for the key or load it into that account. @@ -406,42 +407,46 @@ impl State { match maybe_acc.account { Some(ref account) => { if let Some(value) = account.cached_storage_at(key) { - return value; + return Ok(value); } else { local_account = Some(maybe_acc); } }, - _ => return H256::new(), + _ => return Ok(H256::new()), } } // check the global cache and and cache storage key there if found, - // otherwise cache the account localy and cache storage key there. - if let Some(result) = self.db.get_cached(address, |acc| acc.map_or(H256::new(), |a| { + let trie_res = self.db.get_cached(address, |acc| match acc { + None => Ok(H256::new()), + Some(a) => { let account_db = self.factories.accountdb.readonly(self.db.as_hashdb(), a.address_hash(address)); a.storage_at(account_db.as_hashdb(), key) - })) { - return result; + } + }); + + match trie_res { + None => {} + Some(res) => return res, } + + // otherwise cache the account localy and cache storage key there. if let Some(ref mut acc) = local_account { if let Some(ref account) = acc.account { let account_db = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(address)); return account.storage_at(account_db.as_hashdb(), key) } else { - return H256::new() + return Ok(H256::new()) } } } // check if the account could exist before any requests to trie - if self.db.is_known_null(address) { return H256::zero() } + if self.db.is_known_null(address) { return Ok(H256::zero()) } // account is not found in the global cache, get from the DB and insert into local let db = self.factories.trie.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - let maybe_acc = match db.get_with(address, Account::from_rlp) { - Ok(acc) => acc, - Err(e) => panic!("Potential DB corruption encountered: {}", e), - }; - let r = maybe_acc.as_ref().map_or(H256::new(), |a| { + let maybe_acc = db.get_with(address, Account::from_rlp)?; + let r = maybe_acc.as_ref().map_or(Ok(H256::new()), |a| { let account_db = self.factories.accountdb.readonly(self.db.as_hashdb(), a.address_hash(address)); a.storage_at(account_db.as_hashdb(), key) }); @@ -450,75 +455,84 @@ impl State { } /// Get accounts' code. - pub fn code(&self, a: &Address) -> Option> { + pub fn code(&self, a: &Address) -> trie::Result>> { self.ensure_cached(a, RequireCache::Code, true, |a| a.as_ref().map_or(None, |a| a.code().clone())) } /// Get an account's code hash. - pub fn code_hash(&self, a: &Address) -> H256 { + pub fn code_hash(&self, a: &Address) -> trie::Result { self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().map_or(SHA3_EMPTY, |a| a.code_hash())) } /// Get accounts' code size. - pub fn code_size(&self, a: &Address) -> Option { + pub fn code_size(&self, a: &Address) -> trie::Result> { self.ensure_cached(a, RequireCache::CodeSize, true, |a| a.as_ref().and_then(|a| a.code_size())) } /// Add `incr` to the balance of account `a`. #[cfg_attr(feature="dev", allow(single_match))] - pub fn add_balance(&mut self, a: &Address, incr: &U256, cleanup_mode: CleanupMode) { - trace!(target: "state", "add_balance({}, {}): {}", a, incr, self.balance(a)); + pub fn add_balance(&mut self, a: &Address, incr: &U256, cleanup_mode: CleanupMode) -> trie::Result<()> { + trace!(target: "state", "add_balance({}, {}): {}", a, incr, self.balance(a)?); let is_value_transfer = !incr.is_zero(); - if is_value_transfer || (cleanup_mode == CleanupMode::ForceCreate && !self.exists(a)) { - self.require(a, false).add_balance(incr); + if is_value_transfer || (cleanup_mode == CleanupMode::ForceCreate && !self.exists(a)?) { + self.require(a, false)?.add_balance(incr); } else { match cleanup_mode { - CleanupMode::KillEmpty(set) => if !is_value_transfer && self.exists(a) && !self.exists_and_not_null(a) { + CleanupMode::KillEmpty(set) => if !is_value_transfer && self.exists(a)? && !self.exists_and_not_null(a)? { set.insert(a.clone()); }, _ => {} } } + + Ok(()) } /// Subtract `decr` from the balance of account `a`. - pub fn sub_balance(&mut self, a: &Address, decr: &U256) { - trace!(target: "state", "sub_balance({}, {}): {}", a, decr, self.balance(a)); - if !decr.is_zero() || !self.exists(a) { - self.require(a, false).sub_balance(decr); + pub fn sub_balance(&mut self, a: &Address, decr: &U256) -> trie::Result<()> { + trace!(target: "state", "sub_balance({}, {}): {}", a, decr, self.balance(a)?); + if !decr.is_zero() || !self.exists(a)? { + self.require(a, false)?.sub_balance(decr); } + + Ok(()) } /// Subtracts `by` from the balance of `from` and adds it to that of `to`. - pub fn transfer_balance(&mut self, from: &Address, to: &Address, by: &U256, cleanup_mode: CleanupMode) { - self.sub_balance(from, by); - self.add_balance(to, by, cleanup_mode); + pub fn transfer_balance(&mut self, from: &Address, to: &Address, by: &U256, cleanup_mode: CleanupMode) -> trie::Result<()> { + self.sub_balance(from, by)?; + self.add_balance(to, by, cleanup_mode)?; + Ok(()) } /// Increment the nonce of account `a` by 1. - pub fn inc_nonce(&mut self, a: &Address) { - self.require(a, false).inc_nonce() + pub fn inc_nonce(&mut self, a: &Address) -> trie::Result<()> { + self.require(a, false).map(|mut x| x.inc_nonce()) } /// Mutate storage of account `a` so that it is `value` for `key`. - pub fn set_storage(&mut self, a: &Address, key: H256, value: H256) { - if self.storage_at(a, &key) != value { - self.require(a, false).set_storage(key, value) + pub fn set_storage(&mut self, a: &Address, key: H256, value: H256) -> trie::Result<()> { + if self.storage_at(a, &key)? != value { + self.require(a, false)?.set_storage(key, value) } + + Ok(()) } /// Initialise the code of account `a` so that it is `code`. /// NOTE: Account should have been created with `new_contract`. - pub fn init_code(&mut self, a: &Address, code: Bytes) { - self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce), |_|{}).init_code(code); + pub fn init_code(&mut self, a: &Address, code: Bytes) -> trie::Result<()> { + self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce), |_|{})?.init_code(code); + Ok(()) } /// Reset the code of account `a` so that it is `code`. - pub fn reset_code(&mut self, a: &Address, code: Bytes) { - self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce), |_|{}).reset_code(code); + pub fn reset_code(&mut self, a: &Address, code: Bytes) -> trie::Result<()> { + self.require_or_from(a, true, || Account::new_contract(0.into(), self.account_start_nonce), |_|{})?.reset_code(code); + Ok(()) } /// Execute a given transaction. @@ -559,7 +573,7 @@ impl State { let addr_hash = account.address_hash(address); { let mut account_db = factories.accountdb.create(db.as_hashdb_mut(), addr_hash); - account.commit_storage(&factories.trie, account_db.as_hashdb_mut()); + account.commit_storage(&factories.trie, account_db.as_hashdb_mut())?; account.commit_code(account_db.as_hashdb_mut()); } if !account.is_empty() { @@ -629,25 +643,29 @@ impl State { })) } - fn query_pod(&mut self, query: &PodState) { - for (address, pod_account) in query.get().into_iter() - .filter(|&(a, _)| self.ensure_cached(a, RequireCache::Code, true, |a| a.is_some())) - { + fn query_pod(&mut self, query: &PodState) -> trie::Result<()> { + for (address, pod_account) in query.get() { + if !self.ensure_cached(address, RequireCache::Code, true, |a| a.is_some())? { + continue + } + // needs to be split into two parts for the refcell code here // to work. for key in pod_account.storage.keys() { - self.storage_at(address, key); + self.storage_at(address, key)?; } } + + Ok(()) } /// Returns a `StateDiff` describing the difference from `orig` to `self`. /// Consumes self. - pub fn diff_from(&self, orig: State) -> StateDiff { + pub fn diff_from(&self, orig: State) -> trie::Result { let pod_state_post = self.to_pod(); let mut state_pre = orig; - state_pre.query_pod(&pod_state_post); - pod_state::diff_pod(&state_pre.to_pod(), &pod_state_post) + state_pre.query_pod(&pod_state_post)?; + Ok(pod_state::diff_pod(&state_pre.to_pod(), &pod_state_post)) } // load required account data from the databases. @@ -681,16 +699,16 @@ impl State { /// Check caches for required data /// First searches for account in the local, then the shared cache. /// Populates local cache if nothing found. - fn ensure_cached(&self, a: &Address, require: RequireCache, check_null: bool, f: F) -> U + fn ensure_cached(&self, a: &Address, require: RequireCache, check_null: bool, f: F) -> trie::Result where F: Fn(Option<&Account>) -> U { // check local cache first if let Some(ref mut maybe_acc) = self.cache.borrow_mut().get_mut(a) { if let Some(ref mut account) = maybe_acc.account { let accountdb = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(a)); Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb()); - return f(Some(account)); + return Ok(f(Some(account))); } - return f(None); + return Ok(f(None)); } // check global cache let result = self.db.get_cached(a, |mut acc| { @@ -701,37 +719,34 @@ impl State { f(acc.map(|a| &*a)) }); match result { - Some(r) => r, + Some(r) => Ok(r), None => { // first check if it is not in database for sure - if check_null && self.db.is_known_null(a) { return f(None); } + if check_null && self.db.is_known_null(a) { return Ok(f(None)); } // not found in the global cache, get from the DB and insert into local - let db = self.factories.trie.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - let mut maybe_acc = match db.get_with(a, Account::from_rlp) { - Ok(acc) => acc, - Err(e) => panic!("Potential DB corruption encountered: {}", e), - }; + let db = self.factories.trie.readonly(self.db.as_hashdb(), &self.root)?; + let mut maybe_acc = db.get_with(a, Account::from_rlp)?; if let Some(ref mut account) = maybe_acc.as_mut() { let accountdb = self.factories.accountdb.readonly(self.db.as_hashdb(), account.address_hash(a)); Self::update_account_cache(require, account, &self.db, accountdb.as_hashdb()); } let r = f(maybe_acc.as_ref()); self.insert_cache(a, AccountEntry::new_clean(maybe_acc)); - r + Ok(r) } } } /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. - fn require<'a>(&'a self, a: &Address, require_code: bool) -> RefMut<'a, Account> { + fn require<'a>(&'a self, a: &Address, require_code: bool) -> trie::Result> { self.require_or_from(a, require_code, || Account::new_basic(U256::from(0u8), self.account_start_nonce), |_|{}) } /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. /// If it doesn't exist, make account equal the evaluation of `default`. - fn require_or_from<'a, F: FnOnce() -> Account, G: FnOnce(&mut Account)>(&'a self, a: &Address, require_code: bool, default: F, not_default: G) - -> RefMut<'a, Account> + fn require_or_from<'a, F, G>(&'a self, a: &Address, require_code: bool, default: F, not_default: G) -> trie::Result> + where F: FnOnce() -> Account, G: FnOnce(&mut Account), { let contains_key = self.cache.borrow().contains_key(a); if !contains_key { @@ -739,11 +754,8 @@ impl State { Some(acc) => self.insert_cache(a, AccountEntry::new_clean_cached(acc)), None => { let maybe_acc = if !self.db.is_known_null(a) { - let db = self.factories.trie.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - match db.get_with(a, Account::from_rlp) { - Ok(acc) => AccountEntry::new_clean(acc), - Err(e) => panic!("Potential DB corruption encountered: {}", e), - } + let db = self.factories.trie.readonly(self.db.as_hashdb(), &self.root)?; + AccountEntry::new_clean(db.get_with(a, Account::from_rlp)?) } else { AccountEntry::new_clean(None) }; @@ -754,7 +766,7 @@ impl State { self.note_cache(a); // at this point the entry is guaranteed to be in the cache. - RefMut::map(self.cache.borrow_mut(), |c| { + Ok(RefMut::map(self.cache.borrow_mut(), |c| { let mut entry = c.get_mut(a).expect("entry known to exist in the cache; qed"); match &mut entry.account { @@ -775,18 +787,18 @@ impl State { }, _ => panic!("Required account must always exist; qed"), } - }) + })) } } -// LES state proof implementations. +// State proof implementations; useful for light client protocols. impl State { /// Prove an account's existence or nonexistence in the state trie. /// Returns a merkle proof of the account's trie node with all nodes before `from_level` /// omitted or an encountered trie error. /// Requires a secure trie to be used for accurate results. /// `account_key` == sha3(address) - pub fn prove_account(&self, account_key: H256, from_level: u32) -> Result, Box> { + pub fn prove_account(&self, account_key: H256, from_level: u32) -> trie::Result> { let mut recorder = Recorder::with_depth(from_level); let trie = TrieDB::new(self.db.as_hashdb(), &self.root)?; trie.get_with(&account_key, &mut recorder)?; @@ -799,7 +811,7 @@ impl State { /// `from_level` omitted. Requires a secure trie to be used for correctness. /// `account_key` == sha3(address) /// `storage_key` == sha3(key) - pub fn prove_storage(&self, account_key: H256, storage_key: H256, from_level: u32) -> Result, Box> { + pub fn prove_storage(&self, account_key: H256, storage_key: H256, from_level: u32) -> trie::Result> { // TODO: probably could look into cache somehow but it's keyed by // address, not sha3(address). let trie = TrieDB::new(self.db.as_hashdb(), &self.root)?; @@ -814,7 +826,7 @@ impl State { /// Get code by address hash. /// Only works when backed by a secure trie. - pub fn code_by_address_hash(&self, account_key: H256) -> Result, Box> { + pub fn code_by_address_hash(&self, account_key: H256) -> trie::Result> { let trie = TrieDB::new(self.db.as_hashdb(), &self.root)?; let mut acc = match trie.get_with(&account_key, Account::from_rlp)? { Some(acc) => acc, @@ -899,7 +911,7 @@ mod tests { data: FromHex::from_hex("601080600c6000396000f3006000355415600957005b60203560003555").unwrap(), }.sign(&secret(), None); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -929,13 +941,13 @@ mod tests { let temp = RandomTempPath::new(); let mut state = { let mut state = get_temp_state_in(temp.as_path()); - assert_eq!(state.exists(&a), false); - state.inc_nonce(&a); + assert_eq!(state.exists(&a).unwrap(), false); + state.inc_nonce(&a).unwrap(); state.commit().unwrap(); state.clone() }; - state.inc_nonce(&a); + state.inc_nonce(&a).unwrap(); state.commit().unwrap(); } @@ -959,7 +971,7 @@ mod tests { data: FromHex::from_hex("5b600056").unwrap(), }.sign(&secret(), None); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -996,8 +1008,8 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("6000").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("6000").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1039,7 +1051,7 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1123,7 +1135,7 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060006001610be0f1").unwrap()); + state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060006001610be0f1").unwrap()).unwrap(); let result = state.apply(&info, engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { @@ -1166,8 +1178,8 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b611000f2").unwrap()); - state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()); + state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b611000f2").unwrap()).unwrap(); + state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()).unwrap(); let result = state.apply(&info, engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { @@ -1228,8 +1240,8 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("6000600060006000600b618000f4").unwrap()); - state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()); + state.init_code(&0xa.into(), FromHex::from_hex("6000600060006000600b618000f4").unwrap()).unwrap(); + state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()).unwrap(); let result = state.apply(&info, engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { @@ -1287,8 +1299,8 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("5b600056").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("5b600056").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1327,9 +1339,9 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()); - state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()).unwrap(); + state.init_code(&0xb.into(), FromHex::from_hex("6000").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { @@ -1387,8 +1399,8 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006045600b6000f1").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006045600b6000f1").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1442,8 +1454,8 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060ff600b6000f1").unwrap()); // not enough funds. - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("600060006000600060ff600b6000f1").unwrap()).unwrap(); // not enough funds. + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1485,9 +1497,9 @@ mod tests { data: vec![],//600480600b6000396000f35b600056 }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()); - state.init_code(&0xb.into(), FromHex::from_hex("5b600056").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()).unwrap(); + state.init_code(&0xb.into(), FromHex::from_hex("5b600056").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1541,10 +1553,10 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()); - state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1").unwrap()); - state.init_code(&0xc.into(), FromHex::from_hex("6000").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()).unwrap(); + state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1").unwrap()).unwrap(); + state.init_code(&0xc.into(), FromHex::from_hex("6000").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1616,10 +1628,10 @@ mod tests { data: vec![],//600480600b6000396000f35b600056 }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()); - state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1505b601256").unwrap()); - state.init_code(&0xc.into(), FromHex::from_hex("6000").unwrap()); - state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("60006000600060006000600b602b5a03f1").unwrap()).unwrap(); + state.init_code(&0xb.into(), FromHex::from_hex("60006000600060006000600c602b5a03f1505b601256").unwrap()).unwrap(); + state.init_code(&0xc.into(), FromHex::from_hex("6000").unwrap()).unwrap(); + state.add_balance(&t.sender(), &(100.into()), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { @@ -1689,9 +1701,9 @@ mod tests { data: vec![], }.sign(&secret(), None); - state.init_code(&0xa.into(), FromHex::from_hex("73000000000000000000000000000000000000000bff").unwrap()); - state.add_balance(&0xa.into(), &50.into(), CleanupMode::NoEmpty); - state.add_balance(&t.sender(), &100.into(), CleanupMode::NoEmpty); + state.init_code(&0xa.into(), FromHex::from_hex("73000000000000000000000000000000000000000bff").unwrap()).unwrap(); + state.add_balance(&0xa.into(), &50.into(), CleanupMode::NoEmpty).unwrap(); + state.add_balance(&t.sender(), &100.into(), CleanupMode::NoEmpty).unwrap(); let result = state.apply(&info, &engine, &t, true).unwrap(); let expected_trace = vec![FlatTrace { trace_address: Default::default(), @@ -1728,16 +1740,16 @@ mod tests { let temp = RandomTempPath::new(); let (root, db) = { let mut state = get_temp_state_in(temp.as_path()); - state.require_or_from(&a, false, ||Account::new_contract(42.into(), 0.into()), |_|{}); - state.init_code(&a, vec![1, 2, 3]); - assert_eq!(state.code(&a), Some(Arc::new([1u8, 2, 3].to_vec()))); + state.require_or_from(&a, false, ||Account::new_contract(42.into(), 0.into()), |_|{}).unwrap(); + state.init_code(&a, vec![1, 2, 3]).unwrap(); + assert_eq!(state.code(&a).unwrap(), Some(Arc::new([1u8, 2, 3].to_vec()))); state.commit().unwrap(); - assert_eq!(state.code(&a), Some(Arc::new([1u8, 2, 3].to_vec()))); + assert_eq!(state.code(&a).unwrap(), Some(Arc::new([1u8, 2, 3].to_vec()))); state.drop() }; let state = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert_eq!(state.code(&a), Some(Arc::new([1u8, 2, 3].to_vec()))); + assert_eq!(state.code(&a).unwrap(), Some(Arc::new([1u8, 2, 3].to_vec()))); } #[test] @@ -1746,13 +1758,13 @@ mod tests { let temp = RandomTempPath::new(); let (root, db) = { let mut state = get_temp_state_in(temp.as_path()); - state.set_storage(&a, H256::from(&U256::from(1u64)), H256::from(&U256::from(69u64))); + state.set_storage(&a, H256::from(&U256::from(1u64)), H256::from(&U256::from(69u64))).unwrap(); state.commit().unwrap(); state.drop() }; let s = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert_eq!(s.storage_at(&a, &H256::from(&U256::from(1u64))), H256::from(&U256::from(69u64))); + assert_eq!(s.storage_at(&a, &H256::from(&U256::from(1u64))).unwrap(), H256::from(&U256::from(69u64))); } #[test] @@ -1761,16 +1773,16 @@ mod tests { let temp = RandomTempPath::new(); let (root, db) = { let mut state = get_temp_state_in(temp.as_path()); - state.inc_nonce(&a); - state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty); + state.inc_nonce(&a).unwrap(); + state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty).unwrap(); state.commit().unwrap(); - assert_eq!(state.balance(&a), U256::from(69u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); state.drop() }; let state = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert_eq!(state.balance(&a), U256::from(69u64)); - assert_eq!(state.nonce(&a), U256::from(1u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); + assert_eq!(state.nonce(&a).unwrap(), U256::from(1u64)); } #[test] @@ -1778,16 +1790,16 @@ mod tests { let a = Address::zero(); let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - assert_eq!(state.exists(&a), false); - assert_eq!(state.exists_and_not_null(&a), false); - state.inc_nonce(&a); - assert_eq!(state.exists(&a), true); - assert_eq!(state.exists_and_not_null(&a), true); - assert_eq!(state.nonce(&a), U256::from(1u64)); + assert_eq!(state.exists(&a).unwrap(), false); + assert_eq!(state.exists_and_not_null(&a).unwrap(), false); + state.inc_nonce(&a).unwrap(); + assert_eq!(state.exists(&a).unwrap(), true); + assert_eq!(state.exists_and_not_null(&a).unwrap(), true); + assert_eq!(state.nonce(&a).unwrap(), U256::from(1u64)); state.kill_account(&a); - assert_eq!(state.exists(&a), false); - assert_eq!(state.exists_and_not_null(&a), false); - assert_eq!(state.nonce(&a), U256::from(0u64)); + assert_eq!(state.exists(&a).unwrap(), false); + assert_eq!(state.exists_and_not_null(&a).unwrap(), false); + assert_eq!(state.nonce(&a).unwrap(), U256::from(0u64)); } #[test] @@ -1797,13 +1809,13 @@ mod tests { let db = get_temp_state_db_in(path.as_path()); let (root, db) = { let mut state = State::new(db, U256::from(0), Default::default()); - state.add_balance(&a, &U256::default(), CleanupMode::NoEmpty); // create an empty account + state.add_balance(&a, &U256::default(), CleanupMode::NoEmpty).unwrap(); // create an empty account state.commit().unwrap(); state.drop() }; let state = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert!(!state.exists(&a)); - assert!(!state.exists_and_not_null(&a)); + assert!(!state.exists(&a).unwrap()); + assert!(!state.exists_and_not_null(&a).unwrap()); } #[test] @@ -1813,13 +1825,13 @@ mod tests { let db = get_temp_state_db_in(path.as_path()); let (root, db) = { let mut state = State::new(db, U256::from(0), Default::default()); - state.add_balance(&a, &U256::default(), CleanupMode::ForceCreate); // create an empty account + state.add_balance(&a, &U256::default(), CleanupMode::ForceCreate).unwrap(); // create an empty account state.commit().unwrap(); state.drop() }; let state = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert!(state.exists(&a)); - assert!(!state.exists_and_not_null(&a)); + assert!(state.exists(&a).unwrap()); + assert!(!state.exists_and_not_null(&a).unwrap()); } #[test] @@ -1828,27 +1840,27 @@ mod tests { let temp = RandomTempPath::new(); let (root, db) = { let mut state = get_temp_state_in(temp.as_path()); - state.inc_nonce(&a); + state.inc_nonce(&a).unwrap(); state.commit().unwrap(); - assert_eq!(state.exists(&a), true); - assert_eq!(state.nonce(&a), U256::from(1u64)); + assert_eq!(state.exists(&a).unwrap(), true); + assert_eq!(state.nonce(&a).unwrap(), U256::from(1u64)); state.drop() }; let (root, db) = { let mut state = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert_eq!(state.exists(&a), true); - assert_eq!(state.nonce(&a), U256::from(1u64)); + assert_eq!(state.exists(&a).unwrap(), true); + assert_eq!(state.nonce(&a).unwrap(), U256::from(1u64)); state.kill_account(&a); state.commit().unwrap(); - assert_eq!(state.exists(&a), false); - assert_eq!(state.nonce(&a), U256::from(0u64)); + assert_eq!(state.exists(&a).unwrap(), false); + assert_eq!(state.nonce(&a).unwrap(), U256::from(0u64)); state.drop() }; let state = State::from_existing(db, root, U256::from(0u8), Default::default()).unwrap(); - assert_eq!(state.exists(&a), false); - assert_eq!(state.nonce(&a), U256::from(0u64)); + assert_eq!(state.exists(&a).unwrap(), false); + assert_eq!(state.nonce(&a).unwrap(), U256::from(0u64)); } #[test] @@ -1857,20 +1869,20 @@ mod tests { let mut state = state_result.reference_mut(); let a = Address::zero(); let b = 1u64.into(); - state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty); - assert_eq!(state.balance(&a), U256::from(69u64)); + state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty).unwrap(); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); state.commit().unwrap(); - assert_eq!(state.balance(&a), U256::from(69u64)); - state.sub_balance(&a, &U256::from(42u64)); - assert_eq!(state.balance(&a), U256::from(27u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); + state.sub_balance(&a, &U256::from(42u64)).unwrap(); + assert_eq!(state.balance(&a).unwrap(), U256::from(27u64)); state.commit().unwrap(); - assert_eq!(state.balance(&a), U256::from(27u64)); - state.transfer_balance(&a, &b, &U256::from(18u64), CleanupMode::NoEmpty); - assert_eq!(state.balance(&a), U256::from(9u64)); - assert_eq!(state.balance(&b), U256::from(18u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(27u64)); + state.transfer_balance(&a, &b, &U256::from(18u64), CleanupMode::NoEmpty).unwrap(); + assert_eq!(state.balance(&a).unwrap(), U256::from(9u64)); + assert_eq!(state.balance(&b).unwrap(), U256::from(18u64)); state.commit().unwrap(); - assert_eq!(state.balance(&a), U256::from(9u64)); - assert_eq!(state.balance(&b), U256::from(18u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(9u64)); + assert_eq!(state.balance(&b).unwrap(), U256::from(18u64)); } #[test] @@ -1878,16 +1890,16 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); let a = Address::zero(); - state.inc_nonce(&a); - assert_eq!(state.nonce(&a), U256::from(1u64)); - state.inc_nonce(&a); - assert_eq!(state.nonce(&a), U256::from(2u64)); + state.inc_nonce(&a).unwrap(); + assert_eq!(state.nonce(&a).unwrap(), U256::from(1u64)); + state.inc_nonce(&a).unwrap(); + assert_eq!(state.nonce(&a).unwrap(), U256::from(2u64)); state.commit().unwrap(); - assert_eq!(state.nonce(&a), U256::from(2u64)); - state.inc_nonce(&a); - assert_eq!(state.nonce(&a), U256::from(3u64)); + assert_eq!(state.nonce(&a).unwrap(), U256::from(2u64)); + state.inc_nonce(&a).unwrap(); + assert_eq!(state.nonce(&a).unwrap(), U256::from(3u64)); state.commit().unwrap(); - assert_eq!(state.nonce(&a), U256::from(3u64)); + assert_eq!(state.nonce(&a).unwrap(), U256::from(3u64)); } #[test] @@ -1895,11 +1907,11 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); let a = Address::zero(); - assert_eq!(state.balance(&a), U256::from(0u64)); - assert_eq!(state.nonce(&a), U256::from(0u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(0u64)); + assert_eq!(state.nonce(&a).unwrap(), U256::from(0u64)); state.commit().unwrap(); - assert_eq!(state.balance(&a), U256::from(0u64)); - assert_eq!(state.nonce(&a), U256::from(0u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(0u64)); + assert_eq!(state.nonce(&a).unwrap(), U256::from(0u64)); } #[test] @@ -1907,7 +1919,7 @@ mod tests { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); let a = Address::zero(); - state.require(&a, false); + state.require(&a, false).unwrap(); state.commit().unwrap(); assert_eq!(state.root().hex(), "0ce23f3c809de377b008a4a3ee94a0834aac8bec1f86e28ffe4fdb5a15b0c785"); } @@ -1918,15 +1930,15 @@ mod tests { let mut state = state_result.reference_mut(); let a = Address::zero(); state.checkpoint(); - state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty); - assert_eq!(state.balance(&a), U256::from(69u64)); + state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty).unwrap(); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); state.discard_checkpoint(); - assert_eq!(state.balance(&a), U256::from(69u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); state.checkpoint(); - state.add_balance(&a, &U256::from(1u64), CleanupMode::NoEmpty); - assert_eq!(state.balance(&a), U256::from(70u64)); + state.add_balance(&a, &U256::from(1u64), CleanupMode::NoEmpty).unwrap(); + assert_eq!(state.balance(&a).unwrap(), U256::from(70u64)); state.revert_to_checkpoint(); - assert_eq!(state.balance(&a), U256::from(69u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); } #[test] @@ -1936,12 +1948,12 @@ mod tests { let a = Address::zero(); state.checkpoint(); state.checkpoint(); - state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty); - assert_eq!(state.balance(&a), U256::from(69u64)); + state.add_balance(&a, &U256::from(69u64), CleanupMode::NoEmpty).unwrap(); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); state.discard_checkpoint(); - assert_eq!(state.balance(&a), U256::from(69u64)); + assert_eq!(state.balance(&a).unwrap(), U256::from(69u64)); state.revert_to_checkpoint(); - assert_eq!(state.balance(&a), U256::from(0)); + assert_eq!(state.balance(&a).unwrap(), U256::from(0)); } #[test] @@ -1958,14 +1970,14 @@ mod tests { let mut state = state.reference().clone(); let a: Address = 0xa.into(); - state.init_code(&a, b"abcdefg".to_vec()); - state.add_balance(&a, &256.into(), CleanupMode::NoEmpty); - state.set_storage(&a, 0xb.into(), 0xc.into()); + state.init_code(&a, b"abcdefg".to_vec()).unwrap();; + state.add_balance(&a, &256.into(), CleanupMode::NoEmpty).unwrap(); + state.set_storage(&a, 0xb.into(), 0xc.into()).unwrap(); let mut new_state = state.clone(); - new_state.set_storage(&a, 0xb.into(), 0xd.into()); + new_state.set_storage(&a, 0xb.into(), 0xd.into()).unwrap(); - new_state.diff_from(state); + new_state.diff_from(state).unwrap(); } } diff --git a/ethcore/src/tests/client.rs b/ethcore/src/tests/client.rs index d439d7057..526738586 100644 --- a/ethcore/src/tests/client.rs +++ b/ethcore/src/tests/client.rs @@ -291,7 +291,7 @@ fn change_history_size() { for _ in 0..20 { let mut b = client.prepare_open_block(Address::default(), (3141562.into(), 31415620.into()), vec![]); - b.block_mut().fields_mut().state.add_balance(&address, &5.into(), CleanupMode::NoEmpty); + b.block_mut().fields_mut().state.add_balance(&address, &5.into(), CleanupMode::NoEmpty).unwrap(); b.block_mut().fields_mut().state.commit().unwrap(); let b = b.close_and_lock().seal(&*test_spec.engine, vec![]).unwrap(); client.import_sealed_block(b).unwrap(); // account change is in the journal overlay @@ -306,7 +306,7 @@ fn change_history_size() { Arc::new(Miner::with_spec(&test_spec)), IoChannel::disconnected(), ).unwrap(); - assert_eq!(client.state().balance(&address), 100.into()); + assert_eq!(client.state().balance(&address).unwrap(), 100.into()); } #[test] diff --git a/ethcore/src/types/executed.rs b/ethcore/src/types/executed.rs index 21858c194..4301044ce 100644 --- a/ethcore/src/types/executed.rs +++ b/ethcore/src/types/executed.rs @@ -16,7 +16,7 @@ //! Transaction execution format module. -use util::{Bytes, U256, Address, U512}; +use util::{Bytes, U256, Address, U512, trie}; use rlp::*; use evm; use trace::{VMTrace, FlatTrace}; @@ -146,27 +146,33 @@ pub enum ExecutionError { got: U512 }, /// Returned when internal evm error occurs. - Internal, + Internal(String), /// Returned when generic transaction occurs TransactionMalformed(String), } +impl From> for ExecutionError { + fn from(err: Box) -> Self { + ExecutionError::Internal(format!("{}", err)) + } +} + impl fmt::Display for ExecutionError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use self::ExecutionError::*; let msg = match *self { - NotEnoughBaseGas { required, got } => + NotEnoughBaseGas { ref required, ref got } => format!("Not enough base gas. {} is required, but only {} paid", required, got), - BlockGasLimitReached { gas_limit, gas_used, gas } => + BlockGasLimitReached { ref gas_limit, ref gas_used, ref gas } => format!("Block gas limit reached. The limit is {}, {} has \ already been used, and {} more is required", gas_limit, gas_used, gas), - InvalidNonce { expected, got } => + InvalidNonce { ref expected, ref got } => format!("Invalid transaction nonce: expected {}, found {}", expected, got), - NotEnoughCash { required, got } => + NotEnoughCash { ref required, ref got } => format!("Cost of transaction exceeds sender balance. {} is required \ but the sender only has {}", required, got), - Internal => "Internal evm error".into(), + Internal(ref msg) => msg.clone(), TransactionMalformed(ref err) => format!("Malformed transaction: {}", err), }; @@ -184,6 +190,8 @@ pub enum CallError { StatePruned, /// Couldn't find an amount of gas that didn't result in an exception. Exceptional, + /// Corrupt state. + StateCorrupt, /// Error executing. Execution(ExecutionError), } @@ -202,6 +210,7 @@ impl fmt::Display for CallError { TransactionNotFound => "Transaction couldn't be found in the chain".into(), StatePruned => "Couldn't find the transaction block's state in the chain".into(), Exceptional => "An exception happened in the execution".into(), + StateCorrupt => "Stored state found to be corrupted.".into(), Execution(ref e) => format!("{}", e), }; diff --git a/ethcore/src/types/trace_types/error.rs b/ethcore/src/types/trace_types/error.rs index 7eb16570c..ea3d32679 100644 --- a/ethcore/src/types/trace_types/error.rs +++ b/ethcore/src/types/trace_types/error.rs @@ -40,19 +40,25 @@ pub enum Error { Internal, } -impl From for Error { - fn from(e: EvmError) -> Self { - match e { +impl<'a> From<&'a EvmError> for Error { + fn from(e: &'a EvmError) -> Self { + match *e { EvmError::OutOfGas => Error::OutOfGas, EvmError::BadJumpDestination { .. } => Error::BadJumpDestination, EvmError::BadInstruction { .. } => Error::BadInstruction, EvmError::StackUnderflow { .. } => Error::StackUnderflow, EvmError::OutOfStack { .. } => Error::OutOfStack, - EvmError::Internal => Error::Internal, + EvmError::Internal(_) => Error::Internal, } } } +impl From for Error { + fn from(e: EvmError) -> Self { + Error::from(&e) + } +} + impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use self::Error::*; diff --git a/evmbin/src/ext.rs b/evmbin/src/ext.rs index 6492f4fdc..bcce9adc1 100644 --- a/evmbin/src/ext.rs +++ b/evmbin/src/ext.rs @@ -18,7 +18,7 @@ use std::sync::Arc; use std::collections::HashMap; -use util::{U256, H256, Address, Bytes, FixedHash}; +use util::{U256, H256, Address, Bytes, FixedHash, trie}; use ethcore::client::EnvInfo; use ethcore::evm::{self, Ext, ContractCreateResult, MessageCallResult, Schedule, CallType}; @@ -39,27 +39,28 @@ impl Default for FakeExt { } impl Ext for FakeExt { - fn storage_at(&self, key: &H256) -> H256 { - self.store.get(key).unwrap_or(&H256::new()).clone() + fn storage_at(&self, key: &H256) -> trie::Result { + Ok(self.store.get(key).unwrap_or(&H256::new()).clone()) } - fn set_storage(&mut self, key: H256, value: H256) { + fn set_storage(&mut self, key: H256, value: H256) -> trie::Result<()> { self.store.insert(key, value); + Ok(()) } - fn exists(&self, _address: &Address) -> bool { + fn exists(&self, _address: &Address) -> trie::Result { unimplemented!(); } - fn exists_and_not_null(&self, _address: &Address) -> bool { + fn exists_and_not_null(&self, _address: &Address) -> trie::Result { unimplemented!(); } - fn origin_balance(&self) -> U256 { + fn origin_balance(&self) -> trie::Result { unimplemented!(); } - fn balance(&self, _address: &Address) -> U256 { + fn balance(&self, _address: &Address) -> trie::Result { unimplemented!(); } @@ -83,11 +84,11 @@ impl Ext for FakeExt { unimplemented!(); } - fn extcode(&self, _address: &Address) -> Arc { + fn extcode(&self, _address: &Address) -> trie::Result> { unimplemented!(); } - fn extcodesize(&self, _address: &Address) -> usize { + fn extcodesize(&self, _address: &Address) -> trie::Result { unimplemented!(); } @@ -99,7 +100,7 @@ impl Ext for FakeExt { Ok(*gas) } - fn suicide(&mut self, _refund_address: &Address) { + fn suicide(&mut self, _refund_address: &Address) -> trie::Result<()> { unimplemented!(); } diff --git a/rpc/src/v1/helpers/errors.rs b/rpc/src/v1/helpers/errors.rs index 8c8cf617e..0be3f7240 100644 --- a/rpc/src/v1/helpers/errors.rs +++ b/rpc/src/v1/helpers/errors.rs @@ -132,6 +132,10 @@ pub fn state_pruned() -> Error { } } +pub fn state_corrupt() -> Error { + internal("State corrupt", "") +} + pub fn exceptional() -> Error { Error { code: ErrorCode::ServerError(codes::EXCEPTION_ERROR), @@ -296,6 +300,7 @@ pub fn from_rlp_error(error: DecoderError) -> Error { pub fn from_call_error(error: CallError) -> Error { match error { CallError::StatePruned => state_pruned(), + CallError::StateCorrupt => state_corrupt(), CallError::Exceptional => exceptional(), CallError::Execution(e) => execution(e), CallError::TransactionNotFound => internal("{}, this should not be the case with eth_call, most likely a bug.", CallError::TransactionNotFound), diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 01627ba28..f47ab2055 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -349,7 +349,13 @@ impl Eth for EthClient where let address = address.into(); let res = match num.0.clone() { - BlockNumber::Pending => Ok(take_weakf!(self.miner).balance(&*take_weakf!(self.client), &address).into()), + BlockNumber::Pending => { + let client = take_weakf!(self.client); + match take_weakf!(self.miner).balance(&*client, &address) { + Some(balance) => Ok(balance.into()), + None => Err(errors::internal("Unable to load balance from database", "")) + } + } id => { let client = take_weakf!(self.client); @@ -369,7 +375,13 @@ impl Eth for EthClient where let position: U256 = RpcU256::into(pos); let res = match num.0.clone() { - BlockNumber::Pending => Ok(take_weakf!(self.miner).storage_at(&*take_weakf!(self.client), &address, &H256::from(position)).into()), + BlockNumber::Pending => { + let client = take_weakf!(self.client); + match take_weakf!(self.miner).storage_at(&*client, &address, &H256::from(position)) { + Some(s) => Ok(s.into()), + None => Err(errors::internal("Unable to load storage from database", "")) + } + } id => { let client = take_weakf!(self.client); @@ -387,7 +399,13 @@ impl Eth for EthClient where fn transaction_count(&self, address: RpcH160, num: Trailing) -> BoxFuture { let address: Address = RpcH160::into(address); let res = match num.0.clone() { - BlockNumber::Pending => Ok(take_weakf!(self.miner).nonce(&*take_weakf!(self.client), &address).into()), + BlockNumber::Pending => { + let client = take_weakf!(self.client); + match take_weakf!(self.miner).nonce(&*client, &address) { + Some(nonce) => Ok(nonce.into()), + None => Err(errors::internal("Unable to load nonce from database", "")) + } + } id => { let client = take_weakf!(self.client); @@ -437,7 +455,13 @@ impl Eth for EthClient where let address: Address = RpcH160::into(address); let res = match num.0.clone() { - BlockNumber::Pending => Ok(take_weakf!(self.miner).code(&*take_weakf!(self.client), &address).map_or_else(Bytes::default, Bytes::new)), + BlockNumber::Pending => { + let client = take_weakf!(self.client); + match take_weakf!(self.miner).code(&*client, &address) { + Some(code) => Ok(code.map_or_else(Bytes::default, Bytes::new)), + None => Err(errors::internal("Unable to load code from database", "")) + } + } id => { let client = take_weakf!(self.client); diff --git a/rpc/src/v1/tests/eth.rs b/rpc/src/v1/tests/eth.rs index 6b937d733..c505a2d5d 100644 --- a/rpc/src/v1/tests/eth.rs +++ b/rpc/src/v1/tests/eth.rs @@ -18,7 +18,6 @@ use std::sync::Arc; use std::time::Duration; -use devtools::RandomTempPath; use ethcore::client::{BlockChainClient, Client, ClientConfig}; use ethcore::ids::BlockId; use ethcore::spec::{Genesis, Spec}; diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 75ca928b4..01dd9edc7 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -254,26 +254,39 @@ impl MinerService for TestMinerService { unimplemented!(); } - fn balance(&self, _chain: &MiningBlockChainClient, address: &Address) -> U256 { - self.latest_closed_block.lock().as_ref().map_or_else(U256::zero, |b| b.block().fields().state.balance(address).clone()) + fn balance(&self, _chain: &MiningBlockChainClient, address: &Address) -> Option { + self.latest_closed_block.lock() + .as_ref() + .map(|b| b.block().fields().state.balance(address)) + .map(|b| b.ok()) + .unwrap_or(Some(U256::default())) } fn call(&self, _chain: &MiningBlockChainClient, _t: &SignedTransaction, _analytics: CallAnalytics) -> Result { unimplemented!(); } - fn storage_at(&self, _chain: &MiningBlockChainClient, address: &Address, position: &H256) -> H256 { - self.latest_closed_block.lock().as_ref().map_or_else(H256::default, |b| b.block().fields().state.storage_at(address, position).clone()) + fn storage_at(&self, _chain: &MiningBlockChainClient, address: &Address, position: &H256) -> Option { + self.latest_closed_block.lock() + .as_ref() + .map(|b| b.block().fields().state.storage_at(address, position)) + .map(|s| s.ok()) + .unwrap_or(Some(H256::default())) } - fn nonce(&self, _chain: &MiningBlockChainClient, address: &Address) -> U256 { + fn nonce(&self, _chain: &MiningBlockChainClient, address: &Address) -> Option { // we assume all transactions are in a pending block, ignoring the // reality of gas limits. - self.last_nonce(address).unwrap_or(U256::zero()) + Some(self.last_nonce(address).unwrap_or(U256::zero())) } - fn code(&self, _chain: &MiningBlockChainClient, address: &Address) -> Option { - self.latest_closed_block.lock().as_ref().map_or(None, |b| b.block().fields().state.code(address).map(|c| (*c).clone())) + fn code(&self, _chain: &MiningBlockChainClient, address: &Address) -> Option> { + self.latest_closed_block.lock() + .as_ref() + .map(|b| b.block().fields().state.code(address)) + .map(|c| c.ok()) + .unwrap_or(None) + .map(|c| c.map(|c| (&*c).clone())) } fn sensible_gas_price(&self) -> U256 {