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<TrieError> impl for Error, removed map_err calls

* fix test breakages

* fix doc tests

* update docs

[ci skip]

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.")
+	}
+}

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<Self, TrieError> {
+		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]);
 }

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<Self, TrieError> {
+		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]);
 }

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<Self, TrieError> {
 		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::<Vec<_>>(), TrieDB::new(&memdb, &root).iter().map(|x|x.0).collect::<Vec<_>>());
-	assert_eq!(d, TrieDB::new(&memdb, &root).iter().map(|x|x.1).collect::<Vec<_>>());
+	assert_eq!(d.iter().map(|i|i.to_vec()).collect::<Vec<_>>(), TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.0).collect::<Vec<_>>());
+	assert_eq!(d, TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.1).collect::<Vec<_>>());
 }

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<Self, TrieError>
-	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<Self, TrieError> {
 		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
+			})
 		}
 	}