clear statedb panics (#797)

* state-db: remove the assertion and replace it with Result<>

* state-db: unit test fixes

* comment fixes

* typo fix

substrate/core/client/db/src/lib.rs

@@ -603,7 +603,8 @@ impl<Block> client::backend::Backend<Block, Blake2Hasher> for Backend<Block> whe
 				}
 			}
 			let number_u64 = number.as_();
-			let commit = self.storage.state_db.insert_block(&hash, number_u64, &pending_block.header.parent_hash(), changeset);
+			let commit = self.storage.state_db.insert_block(&hash, number_u64, &pending_block.header.parent_hash(), changeset)
+				.map_err(|e: state_db::Error<io::Error>| client::error::Error::from(format!("State database error: {:?}", e)))?;
 			apply_state_commit(&mut transaction, commit);
 			apply_changes_trie_commit(&mut transaction, operation.changes_trie_updates);
 

substrate/core/client/src/error.rs

@@ -58,7 +58,7 @@ error_chain! {
 			display("Current state of blockchain has invalid authorities set"),
 		}
 
-		/// Cound not get runtime version.
+		/// Could not get runtime version.
 		VersionInvalid {
 			description("Runtime version error"),
 			display("On-chain runtime does not specify version"),

substrate/core/state-db/src/lib.rs

@@ -80,6 +80,8 @@ pub enum Error<E: fmt::Debug> {
 	Db(E),
 	/// `Codec` decoding error.
 	Decoding,
+	/// NonCanonical error.
+	NonCanonical,
 }
 
 impl<E: fmt::Debug> fmt::Debug for Error<E> {
@@ -87,6 +89,7 @@ impl<E: fmt::Debug> fmt::Debug for Error<E> {
 		match self {
 			Error::Db(e) => e.fmt(f),
 			Error::Decoding => write!(f, "Error decoding slicable value"),
+			Error::NonCanonical => write!(f, "Error processing non-canonical data"),
 		}
 	}
 }
@@ -179,21 +182,21 @@ impl<BlockHash: Hash, Key: Hash> StateDbSync<BlockHash, Key> {
 		})
 	}
 
-	pub fn insert_block(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, mut changeset: ChangeSet<Key>) -> CommitSet<Key> {
+	pub fn insert_block<E: fmt::Debug>(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, mut changeset: ChangeSet<Key>) -> Result<CommitSet<Key>, Error<E>> {
 		if number == 0 {
-			return CommitSet {
+			return Ok(CommitSet {
 				data: changeset,
 				meta: Default::default(),
-			}
+			})
 		}
 		match self.mode {
 			PruningMode::ArchiveAll => {
 				changeset.deleted.clear();
 				// write changes immediately
-				CommitSet {
+				Ok(CommitSet {
 					data: changeset,
 					meta: Default::default(),
-				}
+				})
 			},
 			PruningMode::Constrained(_) | PruningMode::ArchiveCanonical => {
 				self.non_canonical.insert(hash, number, parent_hash, changeset)
@@ -297,7 +300,7 @@ impl<BlockHash: Hash, Key: Hash> StateDb<BlockHash, Key> {
 	}
 
 	/// Add a new non-canonical block.
-	pub fn insert_block(&self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet<Key>) -> CommitSet<Key> {
+	pub fn insert_block<E: fmt::Debug>(&self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet<Key>) -> Result<CommitSet<Key>, Error<E>> {
 		self.db.write().insert_block(hash, number, parent_hash, changeset)
 	}
 
@@ -341,6 +344,7 @@ impl<BlockHash: Hash, Key: Hash> StateDb<BlockHash, Key> {
 
 #[cfg(test)]
 mod tests {
+	use std::io;
 	use primitives::H256;
 	use {StateDb, PruningMode, Constraints};
 	use test::{make_db, make_changeset, TestDb};
@@ -349,12 +353,12 @@ mod tests {
 		let mut db = make_db(&[91, 921, 922, 93, 94]);
 		let state_db = StateDb::new(settings, &db).unwrap();
 
-		db.commit(&state_db.insert_block(&H256::from(1), 1, &H256::from(0), make_changeset(&[1], &[91])));
-		db.commit(&state_db.insert_block(&H256::from(21), 2, &H256::from(1), make_changeset(&[21], &[921, 1])));
-		db.commit(&state_db.insert_block(&H256::from(22), 2, &H256::from(1), make_changeset(&[22], &[922])));
-		db.commit(&state_db.insert_block(&H256::from(3), 3, &H256::from(21), make_changeset(&[3], &[93])));
+		db.commit(&state_db.insert_block::<io::Error>(&H256::from(1), 1, &H256::from(0), make_changeset(&[1], &[91])).unwrap());
+		db.commit(&state_db.insert_block::<io::Error>(&H256::from(21), 2, &H256::from(1), make_changeset(&[21], &[921, 1])).unwrap());
+		db.commit(&state_db.insert_block::<io::Error>(&H256::from(22), 2, &H256::from(1), make_changeset(&[22], &[922])).unwrap());
+		db.commit(&state_db.insert_block::<io::Error>(&H256::from(3), 3, &H256::from(21), make_changeset(&[3], &[93])).unwrap());
 		db.commit(&state_db.canonicalize_block(&H256::from(1)));
-		db.commit(&state_db.insert_block(&H256::from(4), 4, &H256::from(3), make_changeset(&[4], &[94])));
+		db.commit(&state_db.insert_block::<io::Error>(&H256::from(4), 4, &H256::from(3), make_changeset(&[4], &[94])).unwrap());
 		db.commit(&state_db.canonicalize_block(&H256::from(21)));
 		db.commit(&state_db.canonicalize_block(&H256::from(3)));
 

substrate/core/state-db/src/noncanonical.rs

@@ -20,6 +20,7 @@
 //! Last canonicalized overlay is kept in memory until next call to `canonicalize` or
 //! `clear_overlay`
 
+use std::fmt;
 use std::collections::{HashMap, VecDeque};
 use super::{Error, DBValue, ChangeSet, CommitSet, MetaDb, Hash, to_meta_key};
 use codec::{Decode, Encode};
@@ -111,20 +112,27 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 	}
 
 	/// Insert a new block into the overlay. If inserted on the second level or lover expects parent to be present in the window.
-	pub fn insert(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet<Key>) -> CommitSet<Key> {
+	pub fn insert<E: fmt::Debug>(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet<Key>) -> Result<CommitSet<Key>, Error<E>> {
 		let mut commit = CommitSet::default();
 		if self.levels.is_empty() && self.last_canonicalized.is_none() {
+			if number < 1 {
+				return Err(Error::NonCanonical);
+			}
 			// assume that parent was canonicalized
 			let last_canonicalized = (parent_hash.clone(), number - 1);
 			commit.meta.inserted.push((to_meta_key(LAST_CANONICAL, &()), last_canonicalized.encode()));
 			self.last_canonicalized = Some(last_canonicalized);
 		} else if self.last_canonicalized.is_some() {
-			assert!(number >= self.front_block_number() && number < (self.front_block_number() + self.levels.len() as u64 + 1));
+			if number < self.front_block_number() || number >= self.front_block_number() + self.levels.len() as u64 + 1 {
+				return Err(Error::NonCanonical);
+			}
 			// check for valid parent if inserting on second level or higher
 			if number == self.front_block_number() {
-				assert!(self.last_canonicalized.as_ref().map_or(false, |&(ref h, n)| h == parent_hash && n == number - 1));
-			} else {
-				assert!(self.parents.contains_key(&parent_hash));
+				if !self.last_canonicalized.as_ref().map_or(false, |&(ref h, n)| h == parent_hash && n == number - 1) {
+					return Err(Error::NonCanonical);
+				}
+			} else if !self.parents.contains_key(&parent_hash) {
+				return Err(Error::NonCanonical);
 			}
 		}
 		let level = if self.levels.is_empty() || number == self.front_block_number() + self.levels.len() as u64 {
@@ -156,7 +164,7 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 		trace!(target: "state-db", "Inserted uncanonicalized changeset {}.{} ({} inserted, {} deleted)", number, index, journal_record.inserted.len(), journal_record.deleted.len());
 		let journal_record = journal_record.encode();
 		commit.meta.inserted.push((journal_key, journal_record));
-		commit
+		Ok(commit)
 	}
 
 	fn discard(
@@ -260,6 +268,7 @@ impl<BlockHash: Hash, Key: Hash> NonCanonicalOverlay<BlockHash, Key> {
 
 #[cfg(test)]
 mod tests {
+	use std::io;
 	use super::NonCanonicalOverlay;
 	use {ChangeSet};
 	use primitives::H256;
@@ -293,8 +302,8 @@ mod tests {
 		let h1 = H256::random();
 		let h2 = H256::random();
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		overlay.insert(&h1, 2, &H256::default(), ChangeSet::default());
-		overlay.insert(&h2, 1, &h1, ChangeSet::default());
+		overlay.insert::<io::Error>(&h1, 2, &H256::default(), ChangeSet::default()).unwrap();
+		overlay.insert::<io::Error>(&h2, 1, &h1, ChangeSet::default()).unwrap();
 	}
 
 	#[test]
@@ -304,8 +313,8 @@ mod tests {
 		let h2 = H256::random();
 		let db = make_db(&[]);
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		overlay.insert(&h1, 1, &H256::default(), ChangeSet::default());
-		overlay.insert(&h2, 3, &h1, ChangeSet::default());
+		overlay.insert::<io::Error>(&h1, 1, &H256::default(), ChangeSet::default()).unwrap();
+		overlay.insert::<io::Error>(&h2, 3, &h1, ChangeSet::default()).unwrap();
 	}
 
 	#[test]
@@ -315,8 +324,8 @@ mod tests {
 		let h1 = H256::random();
 		let h2 = H256::random();
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		overlay.insert(&h1, 1, &H256::default(), ChangeSet::default());
-		overlay.insert(&h2, 2, &H256::default(), ChangeSet::default());
+		overlay.insert::<io::Error>(&h1, 1, &H256::default(), ChangeSet::default()).unwrap();
+		overlay.insert::<io::Error>(&h2, 2, &H256::default(), ChangeSet::default()).unwrap();
 	}
 
 	#[test]
@@ -326,7 +335,7 @@ mod tests {
 		let h2 = H256::random();
 		let db = make_db(&[]);
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		overlay.insert(&h1, 1, &H256::default(), ChangeSet::default());
+		overlay.insert::<io::Error>(&h1, 1, &H256::default(), ChangeSet::default()).unwrap();
 		overlay.canonicalize(&h2);
 	}
 
@@ -336,7 +345,7 @@ mod tests {
 		let mut db = make_db(&[1, 2]);
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
 		let changeset = make_changeset(&[3, 4], &[2]);
-		let insertion = overlay.insert(&h1, 1, &H256::default(), changeset.clone());
+		let insertion = overlay.insert::<io::Error>(&h1, 1, &H256::default(), changeset.clone()).unwrap();
 		assert_eq!(insertion.data.inserted.len(), 0);
 		assert_eq!(insertion.data.deleted.len(), 0);
 		assert_eq!(insertion.meta.inserted.len(), 2);
@@ -357,8 +366,8 @@ mod tests {
 		let h2 = H256::random();
 		let mut db = make_db(&[1, 2]);
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		db.commit(&overlay.insert(&h1, 10, &H256::default(), make_changeset(&[3, 4], &[2])));
-		db.commit(&overlay.insert(&h2, 11, &h1, make_changeset(&[5], &[3])));
+		db.commit(&overlay.insert::<io::Error>(&h1, 10, &H256::default(), make_changeset(&[3, 4], &[2])).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h2, 11, &h1, make_changeset(&[5], &[3])).unwrap());
 		assert_eq!(db.meta.len(), 3);
 
 		let overlay2 = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
@@ -373,8 +382,8 @@ mod tests {
 		let h2 = H256::random();
 		let mut db = make_db(&[1, 2]);
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		db.commit(&overlay.insert(&h1, 10, &H256::default(), make_changeset(&[3, 4], &[2])));
-		db.commit(&overlay.insert(&h2, 11, &h1, make_changeset(&[5], &[3])));
+		db.commit(&overlay.insert::<io::Error>(&h1, 10, &H256::default(), make_changeset(&[3, 4], &[2])).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h2, 11, &h1, make_changeset(&[5], &[3])).unwrap());
 		db.commit(&overlay.canonicalize(&h1));
 		assert_eq!(overlay.levels.len(), 1);
 
@@ -392,9 +401,9 @@ mod tests {
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
 		let changeset1 = make_changeset(&[5, 6], &[2]);
 		let changeset2 = make_changeset(&[7, 8], &[5, 3]);
-		db.commit(&overlay.insert(&h1, 1, &H256::default(), changeset1));
+		db.commit(&overlay.insert::<io::Error>(&h1, 1, &H256::default(), changeset1).unwrap());
 		assert!(contains(&overlay, 5));
-		db.commit(&overlay.insert(&h2, 2, &h1, changeset2));
+		db.commit(&overlay.insert::<io::Error>(&h2, 2, &h1, changeset2).unwrap());
 		assert!(contains(&overlay, 7));
 		assert!(contains(&overlay, 5));
 		assert_eq!(overlay.levels.len(), 2);
@@ -443,21 +452,21 @@ mod tests {
 		let (h_2_1_1, c_2_1_1) = (H256::random(), make_changeset(&[211], &[]));
 
 		let mut overlay = NonCanonicalOverlay::<H256, H256>::new(&db).unwrap();
-		db.commit(&overlay.insert(&h_1, 1, &H256::default(), c_1));
+		db.commit(&overlay.insert::<io::Error>(&h_1, 1, &H256::default(), c_1).unwrap());
 
-		db.commit(&overlay.insert(&h_1_1, 2, &h_1, c_1_1));
-		db.commit(&overlay.insert(&h_1_2, 2, &h_1, c_1_2));
+		db.commit(&overlay.insert::<io::Error>(&h_1_1, 2, &h_1, c_1_1).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h_1_2, 2, &h_1, c_1_2).unwrap());
 
-		db.commit(&overlay.insert(&h_2, 1, &H256::default(), c_2));
+		db.commit(&overlay.insert::<io::Error>(&h_2, 1, &H256::default(), c_2).unwrap());
 
-		db.commit(&overlay.insert(&h_2_1, 2, &h_2, c_2_1));
-		db.commit(&overlay.insert(&h_2_2, 2, &h_2, c_2_2));
+		db.commit(&overlay.insert::<io::Error>(&h_2_1, 2, &h_2, c_2_1).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h_2_2, 2, &h_2, c_2_2).unwrap());
 
-		db.commit(&overlay.insert(&h_1_1_1, 3, &h_1_1, c_1_1_1));
-		db.commit(&overlay.insert(&h_1_2_1, 3, &h_1_2, c_1_2_1));
-		db.commit(&overlay.insert(&h_1_2_2, 3, &h_1_2, c_1_2_2));
-		db.commit(&overlay.insert(&h_1_2_3, 3, &h_1_2, c_1_2_3));
-		db.commit(&overlay.insert(&h_2_1_1, 3, &h_2_1, c_2_1_1));
+		db.commit(&overlay.insert::<io::Error>(&h_1_1_1, 3, &h_1_1, c_1_1_1).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h_1_2_1, 3, &h_1_2, c_1_2_1).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h_1_2_2, 3, &h_1_2, c_1_2_2).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h_1_2_3, 3, &h_1_2, c_1_2_3).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h_2_1_1, 3, &h_2_1, c_2_1_1).unwrap());
 
 		assert!(contains(&overlay, 2));
 		assert!(contains(&overlay, 11));
@@ -516,8 +525,8 @@ mod tests {
 		assert!(overlay.revert_one().is_none());
 		let changeset1 = make_changeset(&[5, 6], &[2]);
 		let changeset2 = make_changeset(&[7, 8], &[5, 3]);
-		db.commit(&overlay.insert(&h1, 1, &H256::default(), changeset1));
-		db.commit(&overlay.insert(&h2, 2, &h1, changeset2));
+		db.commit(&overlay.insert::<io::Error>(&h1, 1, &H256::default(), changeset1).unwrap());
+		db.commit(&overlay.insert::<io::Error>(&h2, 2, &h1, changeset2).unwrap());
 		assert!(contains(&overlay, 7));
 		db.commit(&overlay.revert_one().unwrap());
 		assert_eq!(overlay.parents.len(), 1);