From 1438e15925c892a3fa5b61e987852615f91daf1f Mon Sep 17 00:00:00 2001 From: Guanqun Lu Date: Wed, 26 Sep 2018 18:23:33 +0800 Subject: [PATCH] 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 | 3 +- substrate/core/client/src/error.rs | 2 +- substrate/core/state-db/src/lib.rs | 26 ++++--- substrate/core/state-db/src/noncanonical.rs | 75 ++++++++++++--------- 4 files changed, 60 insertions(+), 46 deletions(-) diff --git a/substrate/core/client/db/src/lib.rs b/substrate/core/client/db/src/lib.rs index 18c8d06e87..3b121757b6 100644 --- a/substrate/core/client/db/src/lib.rs +++ b/substrate/core/client/db/src/lib.rs @@ -603,7 +603,8 @@ impl client::backend::Backend for Backend 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| 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); diff --git a/substrate/core/client/src/error.rs b/substrate/core/client/src/error.rs index 3dd79d79e6..328bc48260 100644 --- a/substrate/core/client/src/error.rs +++ b/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"), diff --git a/substrate/core/state-db/src/lib.rs b/substrate/core/state-db/src/lib.rs index d26fdf0ee3..f0c031d960 100644 --- a/substrate/core/state-db/src/lib.rs +++ b/substrate/core/state-db/src/lib.rs @@ -80,6 +80,8 @@ pub enum Error { Db(E), /// `Codec` decoding error. Decoding, + /// NonCanonical error. + NonCanonical, } impl fmt::Debug for Error { @@ -87,6 +89,7 @@ impl fmt::Debug for Error { 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 StateDbSync { }) } - pub fn insert_block(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, mut changeset: ChangeSet) -> CommitSet { + pub fn insert_block(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, mut changeset: ChangeSet) -> Result, Error> { 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 StateDb { } /// Add a new non-canonical block. - pub fn insert_block(&self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet) -> CommitSet { + pub fn insert_block(&self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet) -> Result, Error> { self.db.write().insert_block(hash, number, parent_hash, changeset) } @@ -341,6 +344,7 @@ impl StateDb { #[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::(&H256::from(1), 1, &H256::from(0), make_changeset(&[1], &[91])).unwrap()); + db.commit(&state_db.insert_block::(&H256::from(21), 2, &H256::from(1), make_changeset(&[21], &[921, 1])).unwrap()); + db.commit(&state_db.insert_block::(&H256::from(22), 2, &H256::from(1), make_changeset(&[22], &[922])).unwrap()); + db.commit(&state_db.insert_block::(&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::(&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))); diff --git a/substrate/core/state-db/src/noncanonical.rs b/substrate/core/state-db/src/noncanonical.rs index 7efa2b3b6b..a0ab23f541 100644 --- a/substrate/core/state-db/src/noncanonical.rs +++ b/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 NonCanonicalOverlay { } /// 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) -> CommitSet { + pub fn insert(&mut self, hash: &BlockHash, number: u64, parent_hash: &BlockHash, changeset: ChangeSet) -> Result, Error> { 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 NonCanonicalOverlay { 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 NonCanonicalOverlay { #[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::::new(&db).unwrap(); - overlay.insert(&h1, 2, &H256::default(), ChangeSet::default()); - overlay.insert(&h2, 1, &h1, ChangeSet::default()); + overlay.insert::(&h1, 2, &H256::default(), ChangeSet::default()).unwrap(); + overlay.insert::(&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::::new(&db).unwrap(); - overlay.insert(&h1, 1, &H256::default(), ChangeSet::default()); - overlay.insert(&h2, 3, &h1, ChangeSet::default()); + overlay.insert::(&h1, 1, &H256::default(), ChangeSet::default()).unwrap(); + overlay.insert::(&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::::new(&db).unwrap(); - overlay.insert(&h1, 1, &H256::default(), ChangeSet::default()); - overlay.insert(&h2, 2, &H256::default(), ChangeSet::default()); + overlay.insert::(&h1, 1, &H256::default(), ChangeSet::default()).unwrap(); + overlay.insert::(&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::::new(&db).unwrap(); - overlay.insert(&h1, 1, &H256::default(), ChangeSet::default()); + overlay.insert::(&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::::new(&db).unwrap(); let changeset = make_changeset(&[3, 4], &[2]); - let insertion = overlay.insert(&h1, 1, &H256::default(), changeset.clone()); + let insertion = overlay.insert::(&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::::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::(&h1, 10, &H256::default(), make_changeset(&[3, 4], &[2])).unwrap()); + db.commit(&overlay.insert::(&h2, 11, &h1, make_changeset(&[5], &[3])).unwrap()); assert_eq!(db.meta.len(), 3); let overlay2 = NonCanonicalOverlay::::new(&db).unwrap(); @@ -373,8 +382,8 @@ mod tests { let h2 = H256::random(); let mut db = make_db(&[1, 2]); let mut overlay = NonCanonicalOverlay::::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::(&h1, 10, &H256::default(), make_changeset(&[3, 4], &[2])).unwrap()); + db.commit(&overlay.insert::(&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::::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::(&h1, 1, &H256::default(), changeset1).unwrap()); assert!(contains(&overlay, 5)); - db.commit(&overlay.insert(&h2, 2, &h1, changeset2)); + db.commit(&overlay.insert::(&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::::new(&db).unwrap(); - db.commit(&overlay.insert(&h_1, 1, &H256::default(), c_1)); + db.commit(&overlay.insert::(&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::(&h_1_1, 2, &h_1, c_1_1).unwrap()); + db.commit(&overlay.insert::(&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::(&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::(&h_2_1, 2, &h_2, c_2_1).unwrap()); + db.commit(&overlay.insert::(&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::(&h_1_1_1, 3, &h_1_1, c_1_1_1).unwrap()); + db.commit(&overlay.insert::(&h_1_2_1, 3, &h_1_2, c_1_2_1).unwrap()); + db.commit(&overlay.insert::(&h_1_2_2, 3, &h_1_2, c_1_2_2).unwrap()); + db.commit(&overlay.insert::(&h_1_2_3, 3, &h_1_2, c_1_2_3).unwrap()); + db.commit(&overlay.insert::(&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::(&h1, 1, &H256::default(), changeset1).unwrap()); + db.commit(&overlay.insert::(&h2, 2, &h1, changeset2).unwrap()); assert!(contains(&overlay, 7)); db.commit(&overlay.revert_one().unwrap()); assert_eq!(overlay.parents.len(), 1);