From 13968aaa38aa494c243fa1058c4739311a2f4a24 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 7 Jun 2016 20:44:09 +0200 Subject: [PATCH] Refactor triedb constructors to error on invalid state root (#1230) * add TrieError, refactor Trie DB creation * remove Result type alias due to glob import conflicts * fix fallout in state.rs * add debug, display impl for TrieError * fix fallout in account.rs * ethcore::Error::TrieError variant * fix remaining fallout in ethcore crate * added From impl for Error, removed map_err calls * fix test breakages * fix doc tests * update docs [ci skip] --- ethcore/src/account.rs | 12 +++++++-- ethcore/src/basic_authority.rs | 2 +- ethcore/src/block.rs | 28 ++++++++++++++------ ethcore/src/client/client.rs | 13 ++++------ ethcore/src/error.rs | 9 +++++++ ethcore/src/ethereum/ethash.rs | 4 +-- ethcore/src/ethereum/mod.rs | 2 +- ethcore/src/state.rs | 47 +++++++++++++++++++--------------- util/src/trie/mod.rs | 27 ++++++++++++++----- util/src/trie/sectriedb.rs | 20 +++++++++------ util/src/trie/sectriedbmut.rs | 18 +++++++------ util/src/trie/triedb.rs | 30 ++++++++++++---------- util/src/trie/triedbmut.rs | 26 +++++++++---------- 13 files changed, 146 insertions(+), 92 deletions(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index 5e0dc335a..29922dda5 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -116,7 +116,12 @@ impl Account { /// Get (and cache) the contents of the trie's storage at `key`. pub fn storage_at(&self, db: &AccountDB, key: &H256) -> H256 { self.storage_overlay.borrow_mut().entry(key.clone()).or_insert_with(||{ - (Filth::Clean, H256::from(SecTrieDB::new(db, &self.storage_root).get(key.bytes()).map_or(U256::zero(), |v| -> U256 {decode(v)}))) + 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."); + + (Filth::Clean, H256::from(db.get(key.bytes()).map_or(U256::zero(), |v| -> U256 {decode(v)}))) }).1.clone() } @@ -204,7 +209,10 @@ impl Account { /// Commit the `storage_overlay` to the backing DB and update `storage_root`. pub fn commit_storage(&mut self, db: &mut AccountDBMut) { - let mut t = SecTrieDBMut::from_existing(db, &mut self.storage_root); + let mut t = SecTrieDBMut::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."); for (k, &mut (ref mut f, ref mut v)) in self.storage_overlay.borrow_mut().iter_mut() { if f == &Filth::Dirty { // cast key and value to trait type, diff --git a/ethcore/src/basic_authority.rs b/ethcore/src/basic_authority.rs index b4a938642..c732ce711 100644 --- a/ethcore/src/basic_authority.rs +++ b/ethcore/src/basic_authority.rs @@ -278,7 +278,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, addr.clone(), 3141562.into(), vec![]); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, addr.clone(), 3141562.into(), vec![]).unwrap(); let b = b.close_and_lock(); let seal = engine.generate_seal(b.block(), Some(&tap)).unwrap(); diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 0b75f5a7e..8f2dc263c 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -228,9 +228,20 @@ pub struct SealedBlock { impl<'x> OpenBlock<'x> { #[cfg_attr(feature="dev", allow(too_many_arguments))] /// Create a new `OpenBlock` ready for transaction pushing. - pub fn new(engine: &'x Engine, vm_factory: &'x EvmFactory, tracing: bool, db: Box, parent: &Header, last_hashes: LastHashes, author: Address, gas_floor_target: U256, extra_data: Bytes) -> Self { + pub fn new( + engine: &'x Engine, + vm_factory: &'x EvmFactory, + tracing: bool, + db: Box, + parent: &Header, + last_hashes: LastHashes, + author: Address, + gas_floor_target: U256, + extra_data: Bytes + ) -> Result { + let state = try!(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce())); let mut r = OpenBlock { - block: ExecutedBlock::new(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce()), tracing), + block: ExecutedBlock::new(state, tracing), engine: engine, vm_factory: vm_factory, last_hashes: last_hashes, @@ -245,7 +256,7 @@ impl<'x> OpenBlock<'x> { engine.populate_from_parent(&mut r.block.base.header, parent, gas_floor_target); engine.on_new_block(&mut r.block); - r + Ok(r) } /// Alter the author for the block. @@ -464,12 +475,12 @@ impl IsBlock for SealedBlock { pub fn enact(header: &Header, transactions: &[SignedTransaction], uncles: &[Header], engine: &Engine, tracing: bool, db: Box, parent: &Header, last_hashes: LastHashes, vm_factory: &EvmFactory) -> Result { { if ::log::max_log_level() >= ::log::LogLevel::Trace { - let s = State::from_existing(db.boxed_clone(), parent.state_root().clone(), engine.account_start_nonce()); + let s = try!(State::from_existing(db.boxed_clone(), parent.state_root().clone(), engine.account_start_nonce())); trace!("enact(): root={}, author={}, author_balance={}\n", s.root(), header.author(), s.balance(&header.author())); } } - let mut b = OpenBlock::new(engine, vm_factory, tracing, db, parent, last_hashes, header.author().clone(), 3141562.into(), header.extra_data().clone()); + let mut b = try!(OpenBlock::new(engine, vm_factory, tracing, db, parent, last_hashes, header.author().clone(), 3141562.into(), header.extra_data().clone())); b.set_difficulty(*header.difficulty()); b.set_gas_limit(*header.gas_limit()); b.set_timestamp(header.timestamp()); @@ -514,7 +525,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]).unwrap(); let b = b.close_and_lock(); let _ = b.seal(engine.deref(), vec![]); } @@ -530,7 +541,8 @@ mod tests { let mut db = db_result.take(); spec.ensure_db_good(db.as_hashdb_mut()); let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]).close_and_lock().seal(engine.deref(), vec![]).unwrap(); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]).unwrap() + .close_and_lock().seal(engine.deref(), vec![]).unwrap(); let orig_bytes = b.rlp_bytes(); let orig_db = b.drain(); @@ -557,7 +569,7 @@ mod tests { let mut db = db_result.take(); spec.ensure_db_good(db.as_hashdb_mut()); let vm_factory = Default::default(); - let mut open_block = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]); + let mut open_block = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]).unwrap(); let mut uncle1_header = Header::new(); uncle1_header.extra_data = b"uncle1".to_vec(); let mut uncle2_header = Header::new(); diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 03f38891a..f8eabb2a3 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -386,18 +386,14 @@ impl Client where V: Verifier { let root = HeaderView::new(&header).state_root(); - // TODO [rob]: refactor State::from_existing so we avoid doing redundant lookups. - if !db.contains(&root) { - return None; - } - - Some(State::from_existing(db, root, self.engine.account_start_nonce())) + State::from_existing(db, root, self.engine.account_start_nonce()).ok() }) } /// Get a copy of the best block's state. pub fn state(&self) -> State { State::from_existing(self.state_db.lock().unwrap().boxed_clone(), HeaderView::new(&self.best_block_header()).state_root(), self.engine.account_start_nonce()) + .expect("State root of best block header always valid.") } /// Get info on the cache. @@ -481,7 +477,7 @@ impl BlockChainClient for Client where V: Verifier { } 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.deref().deref(), &self.vm_factory).transact(t, options); - + // TODO gav move this into Executive. if analytics.state_diffing { if let Ok(ref mut x) = ret { @@ -776,7 +772,8 @@ impl MiningBlockChainClient for Client where V: Verifier { author, gas_floor_target, extra_data, - ); + ).expect("OpenBlock::new only fails if parent state root invalid. State root of best block's header is never invalid. \ + Therefore creating an OpenBlock with the best block's header will not fail."); // Add uncles self.chain diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 3d7c148ba..98aec917a 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -224,6 +224,8 @@ pub enum Error { PowHashInvalid, /// The value of the nonce or mishash is invalid. PowInvalid, + /// Error concerning TrieDBs + TrieError(TrieError), } impl fmt::Display for Error { @@ -239,6 +241,7 @@ impl fmt::Display for Error { f.write_fmt(format_args!("Unknown engine name ({})", name)), Error::PowHashInvalid => f.write_str("Invalid or out of date PoW hash."), Error::PowInvalid => f.write_str("Invalid nonce or mishash"), + Error::TrieError(ref err) => f.write_fmt(format_args!("{}", err)), } } } @@ -300,6 +303,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: TrieError) -> Error { + Error::TrieError(err) + } +} + // TODO: uncomment below once https://github.com/rust-lang/rust/issues/27336 sorted. /*#![feature(concat_idents)] macro_rules! assimilate { diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 1dadb8a65..746da9069 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -308,7 +308,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]).unwrap(); let b = b.close(); assert_eq!(b.state().balance(&Address::zero()), U256::from_str("4563918244f40000").unwrap()); } @@ -323,7 +323,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let mut b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]); + let mut b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]).unwrap(); let mut uncle = Header::new(); let uncle_author = address_from_hex("ef2d6d194084c2de36e0dabfce45d046b37d1106"); uncle.author = uncle_author.clone(); diff --git a/ethcore/src/ethereum/mod.rs b/ethcore/src/ethereum/mod.rs index 1f5712c54..9c9e48342 100644 --- a/ethcore/src/ethereum/mod.rs +++ b/ethcore/src/ethereum/mod.rs @@ -62,7 +62,7 @@ mod tests { let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); spec.ensure_db_good(db.as_hashdb_mut()); - let s = State::from_existing(db, genesis_header.state_root.clone(), engine.account_start_nonce()); + let s = State::from_existing(db, genesis_header.state_root.clone(), engine.account_start_nonce()).unwrap(); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000001")), U256::from(1u64)); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000002")), U256::from(1u64)); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000003")), U256::from(1u64)); diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 72bd6c8fd..204cea618 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -44,6 +44,9 @@ pub struct State { account_start_nonce: U256, } +const SEC_TRIE_DB_UNWRAP_STR: &'static str = "A state can only be created with valid root. Creating a SecTrieDB with a valid root will not fail. \ + Therefore creating a SecTrieDB with this state's root will not fail."; + impl State { /// Creates new state with empty state root #[cfg(test)] @@ -64,18 +67,17 @@ impl State { } /// Creates new state with existing state root - pub fn from_existing(db: Box, root: H256, account_start_nonce: U256) -> State { - { - // trie should panic! if root does not exist - let _ = SecTrieDB::new(db.as_hashdb(), &root); - } - - State { - db: db, - root: root, - cache: RefCell::new(HashMap::new()), - snapshots: RefCell::new(Vec::new()), - account_start_nonce: account_start_nonce, + pub fn from_existing(db: Box, root: H256, account_start_nonce: U256) -> Result { + if !db.as_hashdb().contains(&root) { + Err(TrieError::InvalidStateRoot) + } else { + Ok(State { + db: db, + root: root, + cache: RefCell::new(HashMap::new()), + snapshots: RefCell::new(Vec::new()), + account_start_nonce: account_start_nonce, + }) } } @@ -154,7 +156,8 @@ impl State { /// Determine whether an account exists. pub fn exists(&self, a: &Address) -> bool { - self.cache.borrow().get(&a).unwrap_or(&None).is_some() || SecTrieDB::new(self.db.as_hashdb(), &self.root).contains(&a) + let db = SecTrieDB::new(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); + self.cache.borrow().get(&a).unwrap_or(&None).is_some() || db.contains(&a) } /// Get the balance of account `a`. @@ -245,7 +248,7 @@ impl State { } { - let mut trie = SecTrieDBMut::from_existing(db, root); + let mut trie = SecTrieDBMut::from_existing(db, root).unwrap(); for (address, ref a) in accounts.iter() { match **a { Some(ref account) => trie.insert(address, &account.rlp()), @@ -308,7 +311,8 @@ impl State { fn get<'a>(&'a self, a: &Address, require_code: bool) -> &'a Option { let have_key = self.cache.borrow().contains_key(a); if !have_key { - self.insert_cache(a, SecTrieDB::new(self.db.as_hashdb(), &self.root).get(&a).map(Account::from_rlp)) + let db = SecTrieDB::new(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); + self.insert_cache(a, db.get(&a).map(Account::from_rlp)) } if require_code { if let Some(ref mut account) = self.cache.borrow_mut().get_mut(a).unwrap().as_mut() { @@ -328,7 +332,8 @@ impl State { fn require_or_from<'a, F: FnOnce() -> Account, G: FnOnce(&mut Account)>(&self, a: &Address, require_code: bool, default: F, not_default: G) -> &'a mut Account { let have_key = self.cache.borrow().contains_key(a); if !have_key { - self.insert_cache(a, SecTrieDB::new(self.db.as_hashdb(), &self.root).get(&a).map(Account::from_rlp)) + let db = SecTrieDB::new(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); + self.insert_cache(a, db.get(&a).map(Account::from_rlp)) } else { self.note_cache(a); } @@ -1145,7 +1150,7 @@ fn code_from_database() { state.drop() }; - let state = State::from_existing(db, root, U256::from(0u8)); + let state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.code(&a), Some([1u8, 2, 3].to_vec())); } @@ -1160,7 +1165,7 @@ fn storage_at_from_database() { state.drop() }; - let s = State::from_existing(db, root, U256::from(0u8)); + let s = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(s.storage_at(&a, &H256::from(&U256::from(01u64))), H256::from(&U256::from(69u64))); } @@ -1177,7 +1182,7 @@ fn get_from_database() { state.drop() }; - let state = State::from_existing(db, root, U256::from(0u8)); + let state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.balance(&a), U256::from(69u64)); assert_eq!(state.nonce(&a), U256::from(1u64)); } @@ -1210,7 +1215,7 @@ fn remove_from_database() { }; let (root, db) = { - let mut state = State::from_existing(db, root, U256::from(0u8)); + let mut state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.exists(&a), true); assert_eq!(state.nonce(&a), U256::from(1u64)); state.kill_account(&a); @@ -1220,7 +1225,7 @@ fn remove_from_database() { state.drop() }; - let state = State::from_existing(db, root, U256::from(0u8)); + let state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.exists(&a), false); assert_eq!(state.nonce(&a), U256::from(0u64)); } diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index b8b98880d..79135162c 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -16,6 +16,8 @@ //! Trie interface and implementation. +use std::fmt; + /// Export the trietraits module. pub mod trietraits; /// Export the standardmap module. @@ -33,9 +35,22 @@ pub mod sectriedb; /// Export the sectriedbmut module. pub mod sectriedbmut; -pub use self::trietraits::*; -pub use self::standardmap::*; -pub use self::triedbmut::*; -pub use self::triedb::*; -pub use self::sectriedbmut::*; -pub use self::sectriedb::*; +pub use self::trietraits::{Trie, TrieMut}; +pub use self::standardmap::{Alphabet, StandardMap, ValueMode}; +pub use self::triedbmut::TrieDBMut; +pub use self::triedb::TrieDB; +pub use self::sectriedbmut::SecTrieDBMut; +pub use self::sectriedb::SecTrieDB; + +/// Trie Errors +#[derive(Debug)] +pub enum TrieError { + /// Attempted to create a trie with a state root not in the DB. + InvalidStateRoot, +} + +impl fmt::Display for TrieError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Trie Error: Invalid state root.") + } +} \ No newline at end of file diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index 5558344fa..eda4ac58b 100644 --- a/util/src/trie/sectriedb.rs +++ b/util/src/trie/sectriedb.rs @@ -16,9 +16,10 @@ use hash::*; use sha3::*; -use hashdb::*; -use super::triedb::*; -use super::trietraits::*; +use hashdb::HashDB; +use super::triedb::TrieDB; +use super::trietraits::Trie; +use super::TrieError; /// A `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// @@ -29,10 +30,12 @@ pub struct SecTrieDB<'db> { impl<'db> SecTrieDB<'db> { /// Create a new trie with the backing database `db` and empty `root` + /// /// Initialise to the state entailed by the genesis block. /// This guarantees the trie is built correctly. - pub fn new(db: &'db HashDB, root: &'db H256) -> Self { - SecTrieDB { raw: TrieDB::new(db, root) } + /// Returns an error if root does not exist. + pub fn new(db: &'db HashDB, root: &'db H256) -> Result { + Ok(SecTrieDB { raw: try!(TrieDB::new(db, root)) }) } /// Get a reference to the underlying raw `TrieDB` struct. @@ -60,8 +63,9 @@ impl<'db> Trie for SecTrieDB<'db> { #[test] fn trie_to_sectrie() { - use memorydb::*; - use super::triedbmut::*; + use memorydb::MemoryDB; + use super::triedbmut::TrieDBMut; + use super::trietraits::TrieMut; let mut memdb = MemoryDB::new(); let mut root = H256::new(); @@ -69,6 +73,6 @@ fn trie_to_sectrie() { let mut t = TrieDBMut::new(&mut memdb, &mut root); t.insert(&(&[0x01u8, 0x23]).sha3(), &[0x01u8, 0x23]); } - let t = SecTrieDB::new(&memdb, &root); + let t = SecTrieDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&[0x01u8, 0x23]).unwrap(), &[0x01u8, 0x23]); } diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index 895a821c9..9c8231af4 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -16,9 +16,10 @@ use hash::*; use sha3::*; -use hashdb::*; -use super::triedbmut::*; -use super::trietraits::*; +use hashdb::HashDB; +use super::triedbmut::TrieDBMut; +use super::trietraits::{Trie, TrieMut}; +use super::TrieError; /// A mutable `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// @@ -35,10 +36,11 @@ impl<'db> SecTrieDBMut<'db> { SecTrieDBMut { raw: TrieDBMut::new(db, root) } } - /// Create a new trie with the backing database `db` and `root` - /// Panics, if `root` does not exist - pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Self { - SecTrieDBMut { raw: TrieDBMut::from_existing(db, root) } + /// Create a new trie with the backing database `db` and `root`. + /// + /// Returns an error if root does not exist. + pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Result { + Ok(SecTrieDBMut { raw: try!(TrieDBMut::from_existing(db, root)) }) } /// Get the backing database. @@ -81,6 +83,6 @@ fn sectrie_to_trie() { let mut t = SecTrieDBMut::new(&mut memdb, &mut root); t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); } - let t = TrieDB::new(&memdb, &root); + let t = TrieDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap(), &[0x01u8, 0x23]); } diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index 1c6d2236b..fccd5da90 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -18,8 +18,9 @@ use common::*; use hashdb::*; use nibbleslice::*; use rlp::*; -use super::trietraits::*; -use super::node::*; +use super::trietraits::Trie; +use super::node::Node; +use super::TrieError; /// A `Trie` implementation using a generic `HashDB` backing database. /// @@ -41,7 +42,7 @@ use super::node::*; /// let mut memdb = MemoryDB::new(); /// let mut root = H256::new(); /// TrieDBMut::new(&mut memdb, &mut root).insert(b"foo", b"bar"); -/// let t = TrieDB::new(&memdb, &root); +/// let t = TrieDB::new(&memdb, &root).unwrap(); /// assert!(t.contains(b"foo")); /// assert_eq!(t.get(b"foo").unwrap(), b"bar"); /// assert!(t.db_items_remaining().is_empty()); @@ -57,16 +58,16 @@ pub struct TrieDB<'db> { #[cfg_attr(feature="dev", allow(wrong_self_convention))] impl<'db> TrieDB<'db> { /// Create a new trie with the backing database `db` and `root` - /// Panics, if `root` does not exist - pub fn new(db: &'db HashDB, root: &'db H256) -> Self { + /// Returns an error if `root` does not exist + pub fn new(db: &'db HashDB, root: &'db H256) -> Result { if !db.contains(root) { - flushln!("TrieDB::new({}): Trie root not found!", root); - panic!("Trie root not found!"); - } - TrieDB { - db: db, - root: root, - hash_count: 0 + Err(TrieError::InvalidStateRoot) + } else { + Ok(TrieDB { + db: db, + root: root, + hash_count: 0 + }) } } @@ -356,6 +357,7 @@ impl<'db> fmt::Debug for TrieDB<'db> { #[test] fn iterator() { + use super::trietraits::TrieMut; use memorydb::*; use super::triedbmut::*; @@ -369,6 +371,6 @@ fn iterator() { t.insert(&x, &x); } } - assert_eq!(d.iter().map(|i|i.to_vec()).collect::>(), TrieDB::new(&memdb, &root).iter().map(|x|x.0).collect::>()); - assert_eq!(d, TrieDB::new(&memdb, &root).iter().map(|x|x.1).collect::>()); + assert_eq!(d.iter().map(|i|i.to_vec()).collect::>(), TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.0).collect::>()); + assert_eq!(d, TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.1).collect::>()); } diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index 3d75fa3e1..afabd6437 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -18,9 +18,10 @@ use common::*; use hashdb::*; use nibbleslice::*; use rlp::*; -use super::node::*; -use super::journal::*; -use super::trietraits::*; +use super::node::Node; +use super::journal::Journal; +use super::trietraits::{Trie, TrieMut}; +use super::TrieError; /// A `Trie` implementation using a generic `HashDB` backing database. /// @@ -84,17 +85,16 @@ impl<'db> TrieDBMut<'db> { } /// Create a new trie with the backing database `db` and `root`. - /// Panics, if `root` does not exist. - // TODO: return Result - pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Self { + /// Returns an error if `root` does not exist. + pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Result { if !db.exists(root) { - flushln!("Trie root not found {}", root); - panic!("Trie root not found!"); - } - TrieDBMut { - db: db, - root: root, - hash_count: 0 + Err(TrieError::InvalidStateRoot) + } else { + Ok(TrieDBMut { + db: db, + root: root, + hash_count: 0 + }) } }