Remove (almost all) panickers from trie module (#1776)

* memorydb ub patch and other cleanup

* fix denote invocations

* move trie traits into trie module

* replace "denote" with shim

* triedb returns results and no longer panics

* fix warnings

* get ethcore compiling

* warn on trie errors in ethcore

* remove unsafety from node decoder

* restore broken denote behavior for this branch

* fix overlayrecent fallout

* fix triedb tests

* remove unwrap in state

* alter Trie::get to return Result<Option<_>>

* fix refcell error in require

* fix test warnings

* fix json tests

* whitespace

[ci:skip]

* Avoid unneeded match/indentation

* whitespace

* prettify map_or_else

* remove test warning

util/src/trie/mod.rs

@@ -20,8 +20,6 @@ use std::fmt;
 use hash::H256;
 use hashdb::HashDB;
 
-/// Export the trietraits module.
-pub mod trietraits;
 /// Export the standardmap module.
 pub mod standardmap;
 /// Export the journal module.
@@ -40,7 +38,6 @@ pub mod sectriedbmut;
 mod fatdb;
 mod fatdbmut;
 
-pub use self::trietraits::{Trie, TrieMut};
 pub use self::standardmap::{Alphabet, StandardMap, ValueMode};
 pub use self::triedbmut::TrieDBMut;
 pub use self::triedb::{TrieDB, TrieDBIterator};
@@ -49,19 +46,80 @@ pub use self::sectriedb::SecTrieDB;
 pub use self::fatdb::{FatDB, FatDBIterator};
 pub use self::fatdbmut::FatDBMut;
 
-/// Trie Errors
-#[derive(Debug)]
+/// Trie Errors.
+///
+/// These borrow the data within them to avoid excessive copying on every
+/// trie operation.
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub enum TrieError {
 	/// Attempted to create a trie with a state root not in the DB.
-	InvalidStateRoot,
+	InvalidStateRoot(H256),
+	/// Trie item not found in the database,
+	IncompleteDatabase(H256),
 }
 
 impl fmt::Display for TrieError {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		write!(f, "Trie Error: Invalid state root.")
+		match *self {
+			TrieError::InvalidStateRoot(ref root) => write!(f, "Invalid state root: {}", root),
+			TrieError::IncompleteDatabase(ref missing) =>
+				write!(f, "Database missing expected key: {}", missing),
+		}
 	}
 }
 
+/// Trie-Item type.
+pub type TrieItem<'a> = (Vec<u8>, &'a [u8]);
+
+/// Trie result type. Boxed to avoid copying around extra space for `H256`s on successful queries.
+pub type Result<T> = ::std::result::Result<T, Box<TrieError>>;
+
+/// A key-value datastore implemented as a database-backed modified Merkle tree.
+pub trait Trie {
+	/// Return the root of the trie.
+	fn root(&self) -> &H256;
+
+	/// Is the trie empty?
+	fn is_empty(&self) -> bool { *self.root() == ::rlp::SHA3_NULL_RLP }
+
+	/// Does the trie contain a given key?
+	fn contains(&self, key: &[u8]) -> Result<bool> {
+		self.get(key).map(|x| x.is_some())
+	}
+
+	/// What is the value of the given key in this trie?
+	fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result<Option<&'a [u8]>> where 'a: 'key;
+
+	/// Returns an iterator over elements of trie.
+	fn iter<'a>(&'a self) -> Box<Iterator<Item = TrieItem> + 'a>;
+}
+
+/// A key-value datastore implemented as a database-backed modified Merkle tree.
+pub trait TrieMut {
+	/// Return the root of the trie.
+	fn root(&mut self) -> &H256;
+
+	/// Is the trie empty?
+	fn is_empty(&self) -> bool;
+
+	/// Does the trie contain a given key?
+	fn contains(&self, key: &[u8]) -> Result<bool> {
+		self.get(key).map(|x| x.is_some())
+	}
+
+	/// What is the value of the given key in this trie?
+	fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result<Option<&'a [u8]>> where 'a: 'key;
+
+	/// Insert a `key`/`value` pair into the trie. An `empty` value is equivalent to removing
+	/// `key` from the trie.
+	fn insert(&mut self, key: &[u8], value: &[u8]) -> Result<()>;
+
+	/// Remove a `key` from the trie. Equivalent to making it equal to the empty
+	/// value.
+	fn remove(&mut self, key: &[u8]) -> Result<()>;
+}
+
+
 /// Trie types
 #[derive(Debug, PartialEq, Clone)]
 pub enum TrieSpec {
@@ -95,7 +153,7 @@ impl TrieFactory {
 	}
 
 	/// Create new immutable instance of Trie.
-	pub fn readonly<'db>(&self, db: &'db HashDB, root: &'db H256) -> Result<Box<Trie + 'db>, TrieError> {
+	pub fn readonly<'db>(&self, db: &'db HashDB, root: &'db H256) -> Result<Box<Trie + 'db>> {
 		match self.spec {
 			TrieSpec::Generic => Ok(Box::new(try!(TrieDB::new(db, root)))),
 			TrieSpec::Secure => Ok(Box::new(try!(SecTrieDB::new(db, root)))),
@@ -113,7 +171,7 @@ impl TrieFactory {
 	}
 
 	/// Create new mutable instance of trie and check for errors.
-	pub fn from_existing<'db>(&self, db: &'db mut HashDB, root: &'db mut H256) -> Result<Box<TrieMut + 'db>, TrieError> {
+	pub fn from_existing<'db>(&self, db: &'db mut HashDB, root: &'db mut H256) -> Result<Box<TrieMut + 'db>> {
 		match self.spec {
 			TrieSpec::Generic => Ok(Box::new(try!(TrieDBMut::from_existing(db, root)))),
 			TrieSpec::Secure => Ok(Box::new(try!(SecTrieDBMut::from_existing(db, root)))),