From 36556016933acce95c63624535461a7ebefd0f5f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 23 Feb 2017 17:40:03 +0100 Subject: [PATCH] return errors on database corruption --- ethcore/src/block.rs | 3 +- ethcore/src/client/client.rs | 60 +++++---- ethcore/src/ethereum/ethash.rs | 10 +- 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/executive.rs | 40 +++--- ethcore/src/externalities.rs | 65 ++++++--- ethcore/src/miner/miner.rs | 32 +++-- ethcore/src/miner/mod.rs | 10 +- ethcore/src/state/account.rs | 33 ++--- ethcore/src/state/mod.rs | 164 ++++++++++++----------- ethcore/src/types/executed.rs | 23 +++- ethcore/src/types/trace_types/error.rs | 14 +- 15 files changed, 290 insertions(+), 230 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/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 6c7b91d39..3c9196a85 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -215,8 +215,14 @@ impl Engine for Ethash { // if block.fields().header.gas_limit() <= 4_000_000.into() { let mut 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(e) = res { + warn!("Unable to apply DAO hardfork due to database corruption."); + warn!("Your node is now likely out of consensus."); + } } // } } 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/executive.rs b/ethcore/src/executive.rs index 37b53202a..d287857a0 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); } diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index 49ed2261e..db4a587d1 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 { diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 9cfd4a4a0..432c58025 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -711,7 +711,7 @@ 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 @@ -721,7 +721,9 @@ impl MinerService for Miner { 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 +731,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..e7dc52055 100644 --- a/ethcore/src/miner/mod.rs +++ b/ethcore/src/miner/mod.rs @@ -62,7 +62,7 @@ pub use self::work_notify::NotifyWork; pub use self::stratum::{Stratum, Error as StratumError, Options as StratumOptions}; use std::collections::BTreeMap; -use util::{H256, U256, Address, Bytes}; +use util::{H256, U256, Address, Bytes, trie}; use client::{MiningBlockChainClient, Executed, CallAnalytics}; use block::ClosedBlock; use header::BlockNumber; @@ -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/state/account.rs b/ethcore/src/state/account.rs index 6e5297bc4..acd1591f7 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. diff --git a/ethcore/src/state/mod.rs b/ethcore/src/state/mod.rs index 1d26ea30f..b88a6a9f1 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. @@ -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, 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::*;