Remove panickers from trie iterators (#2209)

* port trie iterators to use error handling

* use new trie iterators in snapshot

allows proper recovery from a premature periodic snapshot

ethcore/src/client/client.rs

@@ -144,7 +144,9 @@ pub struct Client {
 	factories: Factories,
 }
 
-const HISTORY: u64 = 1200;
+/// The pruning constant -- how old blocks must be before we
+/// assume finality of a given candidate.
+pub const HISTORY: u64 = 1200;
 
 /// Append a path element to the given path and return the string.
 pub fn append_path<P>(path: P, item: &str) -> String where P: AsRef<Path> {

ethcore/src/snapshot/account.rs

@@ -92,7 +92,8 @@ impl Account {
 
 		let mut pairs = Vec::new();
 
-		for (k, v) in db.iter() {
+		for item in try!(db.iter()) {
+			let (k, v) = try!(item);
 			pairs.push((k, v));
 		}
 

ethcore/src/snapshot/mod.rs

@@ -358,7 +358,8 @@ pub fn chunk_state<'a>(db: &HashDB, root: &H256, writer: &Mutex<SnapshotWriter +
 	let mut used_code = HashSet::new();
 
 	// account_key here is the address' hash.
-	for (account_key, account_data) in account_trie.iter() {
+	for item in try!(account_trie.iter()) {
+		let (account_key, account_data) = try!(item);
 		let account = Account::from_thin_rlp(account_data);
 		let account_key_hash = H256::from_slice(&account_key);
 

ethcore/src/snapshot/service.rs

@@ -27,7 +27,7 @@ use super::{ManifestData, StateRebuilder, BlockRebuilder, RestorationStatus, Sna
 use super::io::{SnapshotReader, LooseReader, SnapshotWriter, LooseWriter};
 
 use blockchain::BlockChain;
-use client::Client;
+use client::{BlockChainClient, Client};
 use engines::Engine;
 use error::Error;
 use ids::BlockID;
@@ -345,7 +345,17 @@ impl Service {
 		let res = client.take_snapshot(writer, BlockID::Number(num), &self.progress);
 
 		self.taking_snapshot.store(false, Ordering::SeqCst);
-		try!(res);
+		if let Err(e) = res {
+			if client.chain_info().best_block_number >= num + ::client::HISTORY {
+				// "Cancelled" is mincing words a bit -- what really happened
+				// is that the state we were snapshotting got pruned out
+				// before we could finish.
+				info!("Cancelled prematurely-started periodic snapshot.");
+				return Ok(())
+			} else {
+				return Err(e);
+			}
+		}
 
 		info!("Finished taking snapshot at #{}", num);
 

ethcore/src/snapshot/tests/helpers.rs

@@ -52,8 +52,9 @@ impl StateProducer {
 		// modify existing accounts.
 		let mut accounts_to_modify: Vec<_> = {
 			let trie = TrieDB::new(&*db, &self.state_root).unwrap();
-			let temp = trie.iter() // binding required due to complicated lifetime stuff
+			let temp = trie.iter().unwrap() // binding required due to complicated lifetime stuff
 				.filter(|_| rng.gen::<f32>() < ACCOUNT_CHURN)
+				.map(Result::unwrap)
 				.map(|(k, v)| (H256::from_slice(&k), v.to_owned()))
 				.collect();
 

util/src/trie/fatdb.rs

@@ -46,8 +46,8 @@ impl<'db> FatDB<'db> {
 }
 
 impl<'db> Trie for FatDB<'db> {
-	fn iter<'a>(&'a self) -> Box<Iterator<Item = TrieItem> + 'a> {
+	fn iter<'a>(&'a self) -> super::Result<Box<Iterator<Item = TrieItem> + 'a>> {
-		Box::new(FatDBIterator::new(&self.raw))
+		FatDBIterator::new(&self.raw).map(|iter| Box::new(iter) as Box<_>)
 	}
 
 	fn root(&self) -> &H256 {
@@ -73,22 +73,24 @@ pub struct FatDBIterator<'db> {
 
 impl<'db> FatDBIterator<'db> {
 	/// Creates new iterator.
-	pub fn new(trie: &'db TrieDB) -> Self {
+	pub fn new(trie: &'db TrieDB) -> super::Result<Self> {
-		FatDBIterator {
+		Ok(FatDBIterator {
-			trie_iterator: TrieDBIterator::new(trie),
+			trie_iterator: try!(TrieDBIterator::new(trie)),
 			trie: trie,
-		}
+		})
 	}
 }
 
 impl<'db> Iterator for FatDBIterator<'db> {
-	type Item = (Vec<u8>, &'db [u8]);
+	type Item = TrieItem<'db>;
 
 	fn next(&mut self) -> Option<Self::Item> {
 		self.trie_iterator.next()
-			.map(|(hash, value)| {
+			.map(|res|
-				(self.trie.db().get_aux(&hash).expect("Missing fatdb hash"), value)
+				res.map(|(hash, value)| {
-			})
+					(self.trie.db().get_aux(&hash).expect("Missing fatdb hash"), value)
+				})
+			)
 	}
 }
 
@@ -105,5 +107,5 @@ fn fatdb_to_trie() {
 	}
 	let t = FatDB::new(&memdb, &root).unwrap();
 	assert_eq!(t.get(&[0x01u8, 0x23]).unwrap().unwrap(), &[0x01u8, 0x23]);
-	assert_eq!(t.iter().collect::<Vec<_>>(), vec![(vec![0x01u8, 0x23], &[0x01u8, 0x23] as &[u8])]);
+	assert_eq!(t.iter().unwrap().map(Result::unwrap).collect::<Vec<_>>(), vec![(vec![0x01u8, 0x23], &[0x01u8, 0x23] as &[u8])]);
 }

util/src/trie/mod.rs

@@ -72,12 +72,12 @@ impl fmt::Display for TrieError {
 	}
 }
 
-/// 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>>;
 
+/// Trie-Item type.
+pub type TrieItem<'a> = Result<(Vec<u8>, &'a [u8])>;
+
 /// A key-value datastore implemented as a database-backed modified Merkle tree.
 pub trait Trie {
 	/// Return the root of the trie.
@@ -102,7 +102,7 @@ pub trait Trie {
 		where 'a: 'b, R: Recorder;
 
 	/// Returns an iterator over elements of trie.
-	fn iter<'a>(&'a self) -> Box<Iterator<Item = TrieItem> + 'a>;
+	fn iter<'a>(&'a self) -> Result<Box<Iterator<Item = TrieItem> + 'a>>;
 }
 
 /// A key-value datastore implemented as a database-backed modified Merkle tree.
@@ -193,7 +193,7 @@ impl<'db> Trie for TrieKinds<'db> {
 		wrapper!(self, get_recorded, key, r)
 	}
 
-	fn iter<'a>(&'a self) -> Box<Iterator<Item = TrieItem> + 'a> {
+	fn iter<'a>(&'a self) -> Result<Box<Iterator<Item = TrieItem> + 'a>> {
 		wrapper!(self, iter,)
 	}
 }

util/src/trie/sectriedb.rs

@@ -49,8 +49,8 @@ impl<'db> SecTrieDB<'db> {
 }
 
 impl<'db> Trie for SecTrieDB<'db> {
-	fn iter<'a>(&'a self) -> Box<Iterator<Item = TrieItem> + 'a> {
+	fn iter<'a>(&'a self) -> super::Result<Box<Iterator<Item = TrieItem> + 'a>> {
-		Box::new(TrieDB::iter(&self.raw))
+		TrieDB::iter(&self.raw)
 	}
 
 	fn root(&self) -> &H256 { self.raw.root() }

util/src/trie/triedb.rs

@@ -279,30 +279,38 @@ pub struct TrieDBIterator<'a> {
 
 impl<'a> TrieDBIterator<'a> {
 	/// Create a new iterator.
-	pub fn new(db: &'a TrieDB) -> TrieDBIterator<'a> {
+	pub fn new(db: &'a TrieDB) -> super::Result<TrieDBIterator<'a>> {
 		let mut r = TrieDBIterator {
 			db: db,
 			trail: vec![],
 			key_nibbles: Vec::new(),
 		};
-		r.descend(db.root_data(&mut NoOp).unwrap());
+
-		r
+		try!(db.root_data(&mut NoOp).and_then(|root| r.descend(root)));
+		Ok(r)
 	}
 
 	/// Descend into a payload.
-	fn descend(&mut self, d: &'a [u8]) {
+	fn descend(&mut self, d: &'a [u8]) -> super::Result<()> {
 		self.trail.push(Crumb {
 			status: Status::Entering,
-			node: self.db.get_node(d, &mut NoOp, 0).unwrap(),
+			node: try!(self.db.get_node(d, &mut NoOp, 0)),
 		});
 		match self.trail.last().unwrap().node {
 			Node::Leaf(n, _) | Node::Extension(n, _) => { self.key_nibbles.extend(n.iter()); },
 			_ => {}
 		}
+
+		Ok(())
 	}
 
 	/// Descend into a payload and get the next item.
-	fn descend_next(&mut self, d: &'a [u8]) -> Option<(Bytes, &'a [u8])> { self.descend(d); self.next() }
+	fn descend_next(&mut self, d: &'a [u8]) -> Option<TrieItem<'a>> {
+		match self.descend(d) {
+			Ok(()) => self.next(),
+			Err(e) => Some(Err(e)),
+		}
+	}
 
 	/// The present key.
 	fn key(&self) -> Bytes {
@@ -312,12 +320,12 @@ impl<'a> TrieDBIterator<'a> {
 }
 
 impl<'a> Iterator for TrieDBIterator<'a> {
-	type Item = (Bytes, &'a [u8]);
+	type Item = TrieItem<'a>;
 
 	fn next(&mut self) -> Option<Self::Item> {
 		let b = match self.trail.last_mut() {
 			Some(mut b) => { b.increment(); b.clone() },
-			None => return None
+			None => return None,
 		};
 		match (b.status, b.node) {
 			(Status::Exiting, n) => {
@@ -332,7 +340,7 @@ impl<'a> Iterator for TrieDBIterator<'a> {
 				self.trail.pop();
 				self.next()
 			},
-			(Status::At, Node::Leaf(_, v)) | (Status::At, Node::Branch(_, Some(v))) => Some((self.key(), v)),
+			(Status::At, Node::Leaf(_, v)) | (Status::At, Node::Branch(_, Some(v))) => Some(Ok((self.key(), v))),
 			(Status::At, Node::Extension(_, d)) => self.descend_next(d),
 			(Status::At, Node::Branch(_, _)) => self.next(),
 			(Status::AtChild(i), Node::Branch(children, _)) if children[i].len() > 0 => {
@@ -352,8 +360,8 @@ impl<'a> Iterator for TrieDBIterator<'a> {
 }
 
 impl<'db> Trie for TrieDB<'db> {
-	fn iter<'a>(&'a self) -> Box<Iterator<Item = TrieItem> + 'a> {
+	fn iter<'a>(&'a self) -> super::Result<Box<Iterator<Item = TrieItem> + 'a>> {
-		Box::new(TrieDBIterator::new(self))
+		TrieDBIterator::new(self).map(|iter| Box::new(iter) as Box<_>)
 	}
 
 	fn root(&self) -> &H256 { self.root }
@@ -392,6 +400,6 @@ fn iterator() {
 	}
 
 	let t = TrieDB::new(&memdb, &root).unwrap();
-	assert_eq!(d.iter().map(|i|i.to_vec()).collect::<Vec<_>>(), t.iter().map(|x|x.0).collect::<Vec<_>>());
+	assert_eq!(d.iter().map(|i|i.to_vec()).collect::<Vec<_>>(), t.iter().unwrap().map(|x| x.unwrap().0).collect::<Vec<_>>());
-	assert_eq!(d, t.iter().map(|x|x.1).collect::<Vec<_>>());
+	assert_eq!(d, t.iter().unwrap().map(|x| x.unwrap().1).collect::<Vec<_>>());
 }