From a100b9d09e4c9c401b0bba112c64faae2bc585d9 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 21 Sep 2016 12:56:13 +0200 Subject: [PATCH] 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 | 4 +++- ethcore/src/snapshot/account.rs | 3 ++- ethcore/src/snapshot/mod.rs | 3 ++- ethcore/src/snapshot/service.rs | 14 +++++++++-- ethcore/src/snapshot/tests/helpers.rs | 3 ++- util/src/trie/fatdb.rs | 24 ++++++++++--------- util/src/trie/mod.rs | 10 ++++---- util/src/trie/sectriedb.rs | 4 ++-- util/src/trie/triedb.rs | 34 +++++++++++++++++---------- 9 files changed, 62 insertions(+), 37 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index b765adc37..a60cce01d 100644 --- a/ethcore/src/client/client.rs +++ b/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

(path: P, item: &str) -> String where P: AsRef { diff --git a/ethcore/src/snapshot/account.rs b/ethcore/src/snapshot/account.rs index 16c59db2e..8cfc4c96b 100644 --- a/ethcore/src/snapshot/account.rs +++ b/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)); } diff --git a/ethcore/src/snapshot/mod.rs b/ethcore/src/snapshot/mod.rs index 43622fc51..5e8f3bd7a 100644 --- a/ethcore/src/snapshot/mod.rs +++ b/ethcore/src/snapshot/mod.rs @@ -358,7 +358,8 @@ pub fn chunk_state<'a>(db: &HashDB, root: &H256, writer: &Mutex= 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); diff --git a/ethcore/src/snapshot/tests/helpers.rs b/ethcore/src/snapshot/tests/helpers.rs index 42fb68220..cb928346e 100644 --- a/ethcore/src/snapshot/tests/helpers.rs +++ b/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::() < ACCOUNT_CHURN) + .map(Result::unwrap) .map(|(k, v)| (H256::from_slice(&k), v.to_owned())) .collect(); diff --git a/util/src/trie/fatdb.rs b/util/src/trie/fatdb.rs index bb35bd467..f4c65a84b 100644 --- a/util/src/trie/fatdb.rs +++ b/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 + 'a> { - Box::new(FatDBIterator::new(&self.raw)) + fn iter<'a>(&'a self) -> super::Result + 'a>> { + 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 { - FatDBIterator { - trie_iterator: TrieDBIterator::new(trie), + pub fn new(trie: &'db TrieDB) -> super::Result { + Ok(FatDBIterator { + trie_iterator: try!(TrieDBIterator::new(trie)), trie: trie, - } + }) } } impl<'db> Iterator for FatDBIterator<'db> { - type Item = (Vec, &'db [u8]); + type Item = TrieItem<'db>; fn next(&mut self) -> Option { self.trie_iterator.next() - .map(|(hash, value)| { - (self.trie.db().get_aux(&hash).expect("Missing fatdb hash"), value) - }) + .map(|res| + 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![0x01u8, 0x23], &[0x01u8, 0x23] as &[u8])]); + assert_eq!(t.iter().unwrap().map(Result::unwrap).collect::>(), vec![(vec![0x01u8, 0x23], &[0x01u8, 0x23] as &[u8])]); } diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index b71f6a5e2..6eebd8f5d 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -72,12 +72,12 @@ impl fmt::Display for TrieError { } } -/// Trie-Item type. -pub type TrieItem<'a> = (Vec, &'a [u8]); - /// Trie result type. Boxed to avoid copying around extra space for `H256`s on successful queries. pub type Result = ::std::result::Result>; +/// Trie-Item type. +pub type TrieItem<'a> = Result<(Vec, &'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 + 'a>; + fn iter<'a>(&'a self) -> Result + '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 + 'a> { + fn iter<'a>(&'a self) -> Result + 'a>> { wrapper!(self, iter,) } } diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index 9e807884c..d7108dc3e 100644 --- a/util/src/trie/sectriedb.rs +++ b/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 + 'a> { - Box::new(TrieDB::iter(&self.raw)) + fn iter<'a>(&'a self) -> super::Result + 'a>> { + TrieDB::iter(&self.raw) } fn root(&self) -> &H256 { self.raw.root() } diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index 99ef1f118..f5de26f8e 100644 --- a/util/src/trie/triedb.rs +++ b/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> { 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> { + 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 { 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 + 'a> { - Box::new(TrieDBIterator::new(self)) + fn iter<'a>(&'a self) -> super::Result + 'a>> { + 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::>(), t.iter().map(|x|x.0).collect::>()); - assert_eq!(d, t.iter().map(|x|x.1).collect::>()); + assert_eq!(d.iter().map(|i|i.to_vec()).collect::>(), t.iter().unwrap().map(|x| x.unwrap().0).collect::>()); + assert_eq!(d, t.iter().unwrap().map(|x| x.unwrap().1).collect::>()); }