From 65f0c5f3afce5621ec0fb835f0e053a183c2b098 Mon Sep 17 00:00:00 2001 From: Arkadiy Paronyan Date: Tue, 24 Jul 2018 15:54:34 +0200 Subject: [PATCH] Revert chain command (#393) * Revert chain command * BLOCKS -> NUM * Fixed warning --- substrate/polkadot/cli/src/cli.yml | 17 ++++++ substrate/polkadot/cli/src/lib.rs | 25 +++++++- substrate/substrate/client/db/src/lib.rs | 41 ++++++++++++- substrate/substrate/client/src/backend.rs | 5 +- substrate/substrate/client/src/client.rs | 8 ++- substrate/substrate/client/src/in_mem.rs | 6 +- .../substrate/client/src/light/backend.rs | 6 +- .../substrate/network/src/import_queue.rs | 28 +++------ substrate/substrate/service/src/config.rs | 2 +- substrate/substrate/state-db/src/lib.rs | 37 ++++++++++++ .../substrate/state-db/src/unfinalized.rs | 57 +++++++++++++++++++ 11 files changed, 203 insertions(+), 29 deletions(-) diff --git a/substrate/polkadot/cli/src/cli.yml b/substrate/polkadot/cli/src/cli.yml index be0d0e501b..d8e2797970 100644 --- a/substrate/polkadot/cli/src/cli.yml +++ b/substrate/polkadot/cli/src/cli.yml @@ -178,3 +178,20 @@ subcommands: long: max-heap-pages value_name: COUNT help: The maximum number of 64KB pages to ever allocate for Wasm execution. Don't alter this unless you know what you're doing. + - revert: + about: Revert chain to the previous state + args: + - NUM: + index: 1 + help: Number of blocks to revert. Default is 256. + - chain: + long: chain + value_name: CHAIN_SPEC + help: Specify the chain specification. + takes_value: true + - base-path: + long: base-path + short: d + value_name: PATH + help: Specify custom base path. + takes_value: true diff --git a/substrate/polkadot/cli/src/lib.rs b/substrate/polkadot/cli/src/lib.rs index 9e84660c3f..de8872472f 100644 --- a/substrate/polkadot/cli/src/lib.rs +++ b/substrate/polkadot/cli/src/lib.rs @@ -223,6 +223,10 @@ pub fn run(args: I, worker: W) -> error::Result<()> where return import_blocks(matches, worker.exit_only()); } + if let Some(matches) = matches.subcommand_matches("revert") { + return revert_chain(matches); + } + let (spec, is_global) = load_spec(&matches)?; let mut config = service::Configuration::default_with_spec(spec); @@ -248,7 +252,7 @@ pub fn run(args: I, worker: W) -> error::Result<()> where config.pruning = match matches.value_of("pruning") { Some("archive") => PruningMode::ArchiveAll, - None => PruningMode::keep_blocks(256), + None => PruningMode::default(), Some(s) => PruningMode::keep_blocks(s.parse() .map_err(|_| error::ErrorKind::Input("Invalid pruning mode specified".to_owned()))?), }; @@ -495,6 +499,25 @@ fn import_blocks(matches: &clap::ArgMatches, exit: E) -> error::Result<()> Ok(()) } +fn revert_chain(matches: &clap::ArgMatches) -> error::Result<()> { + let (spec, _) = load_spec(&matches)?; + let base_path = base_path(matches); + let mut config = service::Configuration::default_with_spec(spec); + config.database_path = db_path(&base_path, config.chain_spec.id()).to_string_lossy().into(); + + let client = service::new_client(config)?; + + let blocks = match matches.value_of("NUM") { + Some(v) => v.parse().map_err(|_| "Invalid block count specified")?, + None => 256, + }; + + let reverted = client.revert(blocks)?; + let info = client.info()?.chain; + info!("Reverted {} blocks. Best: #{} ({})", reverted, info.best_number, info.best_hash); + Ok(()) +} + fn run_until_exit( runtime: &mut Runtime, service: service::Service, diff --git a/substrate/substrate/client/db/src/lib.rs b/substrate/substrate/client/db/src/lib.rs index bdfd5ce522..741cd5cc94 100644 --- a/substrate/substrate/client/db/src/lib.rs +++ b/substrate/substrate/client/db/src/lib.rs @@ -50,7 +50,7 @@ use parking_lot::RwLock; use primitives::H256; use runtime_primitives::generic::BlockId; use runtime_primitives::bft::Justification; -use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, As, Hash, HashFor, Zero}; +use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, As, Hash, HashFor, NumberFor, Zero}; use runtime_primitives::BuildStorage; use state_machine::backend::Backend as StateBackend; use executor::RuntimeInfo; @@ -372,7 +372,12 @@ impl client::backend::Backend for Backend { let finalizing_hash = if self.finalization_window == 0 { Some(hash) } else { - self.blockchain.hash(As::sa((number_u64 - self.finalization_window) as u64))? + let finalizing = number_u64 - self.finalization_window; + if finalizing > self.storage.state_db.best_finalized() { + self.blockchain.hash(As::sa(finalizing))? + } else { + None + } }; if let Some(finalizing_hash) = finalizing_hash { trace!("Finalizing block #{} ({:?})", number_u64 - self.finalization_window, finalizing_hash); @@ -381,13 +386,43 @@ impl client::backend::Backend for Backend { } } - debug!("DB Commit {:?} ({})", hash, number); + debug!("DB Commit {:?} ({}), best = {}", hash, number, pending_block.is_best); self.storage.db.write(transaction).map_err(db_err)?; self.blockchain.update_meta(hash, number, pending_block.is_best); } Ok(()) } + fn revert(&self, n: NumberFor) -> Result, client::error::Error> { + use client::blockchain::HeaderBackend; + let mut best = self.blockchain.info()?.best_number; + for c in 0 .. n.as_() { + if best == As::sa(0) { + return Ok(As::sa(c)) + } + let mut transaction = DBTransaction::new(); + match self.storage.state_db.revert_one() { + Some(commit) => { + apply_state_commit(&mut transaction, commit); + let removed = self.blockchain.hash(best)?.ok_or_else( + || client::error::ErrorKind::UnknownBlock( + format!("Error reverting to {}. Block hash not found.", best)))?; + best -= As::sa(1); + let key = number_to_db_key(best.clone()); + let hash = self.blockchain.hash(best)?.ok_or_else( + || client::error::ErrorKind::UnknownBlock( + format!("Error reverting to {}. Block hash not found.", best)))?; + transaction.put(columns::META, meta_keys::BEST_BLOCK, &key); + transaction.delete(columns::BLOCK_INDEX, removed.as_ref()); + self.storage.db.write(transaction).map_err(db_err)?; + self.blockchain.update_meta(hash, best, true); + } + None => return Ok(As::sa(c)) + } + } + Ok(n) + } + fn blockchain(&self) -> &BlockchainDb { &self.blockchain } diff --git a/substrate/substrate/client/src/backend.rs b/substrate/substrate/client/src/backend.rs index 64f4a1d57f..39146fc9fe 100644 --- a/substrate/substrate/client/src/backend.rs +++ b/substrate/substrate/client/src/backend.rs @@ -19,7 +19,7 @@ use state_machine::backend::Backend as StateBackend; use error; use runtime_primitives::bft::Justification; -use runtime_primitives::traits::Block as BlockT; +use runtime_primitives::traits::{Block as BlockT, NumberFor}; use runtime_primitives::generic::BlockId; /// Block insertion operation. Keeps hold if the inserted block state and data. @@ -69,6 +69,9 @@ pub trait Backend: Send + Sync { fn blockchain(&self) -> &Self::Blockchain; /// Returns state backend with post-state of given block. fn state_at(&self, block: BlockId) -> error::Result; + /// Attempts to revert the chain by `n` blocks. Returns the number of blocks that were + /// successfully reverted. + fn revert(&self, n: NumberFor) -> error::Result>; } /// Mark for all Backend implementations, that are making use of state data, stored locally. diff --git a/substrate/substrate/client/src/client.rs b/substrate/substrate/client/src/client.rs index 8103128008..ef755b13ae 100644 --- a/substrate/substrate/client/src/client.rs +++ b/substrate/substrate/client/src/client.rs @@ -21,7 +21,7 @@ use futures::sync::mpsc; use parking_lot::{Mutex, RwLock}; use primitives::AuthorityId; use runtime_primitives::{bft::Justification, generic::{BlockId, SignedBlock, Block as RuntimeBlock}}; -use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, Zero, One, As}; +use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, Zero, One, As, NumberFor}; use runtime_primitives::BuildStorage; use primitives::storage::{StorageKey, StorageData}; use codec::Decode; @@ -379,6 +379,12 @@ impl Client where Ok(ImportResult::Queued) } + /// Attempts to revert the chain by `n` blocks. Returns the number of blocks that were + /// successfully reverted. + pub fn revert(&self, n: NumberFor) -> error::Result> { + Ok(self.backend.revert(n)?) + } + /// Get blockchain info. pub fn info(&self) -> error::Result> { let info = self.backend.blockchain().info().map_err(|e| error::Error::from_blockchain(Box::new(e)))?; diff --git a/substrate/substrate/client/src/in_mem.rs b/substrate/substrate/client/src/in_mem.rs index 0473c1fa6a..2da0f4e6d0 100644 --- a/substrate/substrate/client/src/in_mem.rs +++ b/substrate/substrate/client/src/in_mem.rs @@ -23,7 +23,7 @@ use error; use backend; use light; use runtime_primitives::generic::BlockId; -use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, Zero}; +use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, Zero, NumberFor, As}; use runtime_primitives::bft::Justification; use blockchain::{self, BlockStatus}; use state_machine::backend::{Backend as StateBackend, InMemory}; @@ -309,6 +309,10 @@ impl backend::Backend for Backend where None => Err(error::ErrorKind::UnknownBlock(format!("{}", block)).into()), } } + + fn revert(&self, _n: NumberFor) -> error::Result> { + Ok(As::sa(0)) + } } impl backend::LocalBackend for Backend {} diff --git a/substrate/substrate/client/src/light/backend.rs b/substrate/substrate/client/src/light/backend.rs index 1311618e99..a9eb4eb09b 100644 --- a/substrate/substrate/client/src/light/backend.rs +++ b/substrate/substrate/client/src/light/backend.rs @@ -20,7 +20,7 @@ use std::sync::{Arc, Weak}; use runtime_primitives::{bft::Justification, generic::BlockId}; -use runtime_primitives::traits::Block as BlockT; +use runtime_primitives::traits::{Block as BlockT, NumberFor}; use state_machine::{Backend as StateBackend, TrieBackend as StateTrieBackend, TryIntoTrieBackend as TryIntoStateTrieBackend}; @@ -93,6 +93,10 @@ impl ClientBackend for Backend where Block: BlockT, S: fetcher: self.blockchain.fetcher(), }) } + + fn revert(&self, _n: NumberFor) -> ClientResult> { + unimplemented!() + } } impl RemoteBackend for Backend where Block: BlockT, S: BlockchainStorage, F: Fetcher {} diff --git a/substrate/substrate/network/src/import_queue.rs b/substrate/substrate/network/src/import_queue.rs index 08679ed9f5..78cf7c2a28 100644 --- a/substrate/substrate/network/src/import_queue.rs +++ b/substrate/substrate/network/src/import_queue.rs @@ -21,9 +21,9 @@ use std::sync::{Arc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; use parking_lot::{Condvar, Mutex, RwLock}; -use client::{BlockOrigin, BlockStatus, ImportResult}; +use client::{BlockOrigin, ImportResult}; use network_libp2p::{NodeIndex, Severity}; -use runtime_primitives::generic::BlockId; + use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, NumberFor, Zero}; use blocks::BlockData; @@ -221,8 +221,6 @@ enum SyncLink<'a, B: 'a + BlockT, E: 'a + ExecuteInContext> { /// Block import successful result. #[derive(Debug, PartialEq)] enum BlockImportResult { - /// Block is not imported. - NotImported(H, N), /// Imported known block. ImportedKnown(H, N), /// Imported unknown block. @@ -286,16 +284,6 @@ fn import_single_block( let hash = header.hash(); let parent = header.parent_hash().clone(); - // check whether the block is known before importing. - match chain.block_status(&BlockId::Hash(hash)) { - Ok(BlockStatus::InChain) => return Ok(BlockImportResult::NotImported(hash, number)), - Ok(_) => {}, - Err(e) => { - debug!(target: "sync", "Error importing block {}: {:?}: {:?}", number, hash, e); - return Err(BlockImportError::Restart); - } - } - let result = chain.import( block_origin, header, @@ -347,10 +335,9 @@ fn process_import_result<'a, B: BlockT>( ) -> usize { match result { - Ok(BlockImportResult::NotImported(_, _)) => 0, Ok(BlockImportResult::ImportedKnown(hash, number)) => { link.block_imported(&hash, number); - 0 + 1 }, Ok(BlockImportResult::ImportedUnknown(hash, number)) => { link.block_imported(&hash, number); @@ -431,6 +418,7 @@ pub mod tests { use test_client::{self, TestClient}; use test_client::runtime::{Block, Hash}; use on_demand::tests::DummyExecutor; + use runtime_primitives::generic::BlockId; use super::*; /// Blocks import queue that is importing blocks in the same thread. @@ -522,7 +510,7 @@ pub mod tests { #[test] fn import_single_good_known_block_is_ignored() { let (client, hash, number, block) = prepare_good_block(); - assert_eq!(import_single_block(&client, BlockOrigin::File, block), Ok(BlockImportResult::NotImported(hash, number))); + assert_eq!(import_single_block(&client, BlockOrigin::File, block), Ok(BlockImportResult::ImportedKnown(hash, number))); } #[test] @@ -542,11 +530,11 @@ pub mod tests { #[test] fn process_import_result_works() { let mut link = TestLink::new(); - assert_eq!(process_import_result::(&mut link, Ok(BlockImportResult::NotImported(Default::default(), 0))), 0); - assert_eq!(link.total(), 0); + assert_eq!(process_import_result::(&mut link, Ok(BlockImportResult::ImportedKnown(Default::default(), 0))), 1); + assert_eq!(link.total(), 1); let mut link = TestLink::new(); - assert_eq!(process_import_result::(&mut link, Ok(BlockImportResult::ImportedKnown(Default::default(), 0))), 0); + assert_eq!(process_import_result::(&mut link, Ok(BlockImportResult::ImportedKnown(Default::default(), 0))), 1); assert_eq!(link.total(), 1); assert_eq!(link.imported, 1); diff --git a/substrate/substrate/service/src/config.rs b/substrate/substrate/service/src/config.rs index 8a5f324100..5ad1d51f78 100644 --- a/substrate/substrate/service/src/config.rs +++ b/substrate/substrate/service/src/config.rs @@ -71,7 +71,7 @@ impl Configuration Self { + PruningMode::keep_blocks(256) + } +} + fn to_meta_key(suffix: &[u8], data: &S) -> Vec { let mut buffer = data.encode(); buffer.extend(suffix); @@ -216,6 +222,10 @@ impl StateDbSync { commit } + pub fn best_finalized(&self) -> u64 { + return self.unfinalized.last_finalized_block_number() + } + fn prune(&mut self, commit: &mut CommitSet) { if let (&mut Some(ref mut pruning), &PruningMode::Constrained(ref constraints)) = (&mut self.pruning, &self.mode) { loop { @@ -237,6 +247,20 @@ impl StateDbSync { } } + /// Revert all unfinalized blocks with the best block number. + /// Returns a database commit or `None` if not possible. + /// For archive an empty commit set is returned. + pub fn revert_one(&mut self) -> Option> { + match self.mode { + PruningMode::ArchiveAll => { + Some(CommitSet::default()) + }, + PruningMode::ArchiveCanonical | PruningMode::Constrained(_) => { + self.unfinalized.revert_one() + }, + } + } + pub fn pin(&mut self, hash: &BlockHash) { self.pinned.insert(hash.clone()); } @@ -291,6 +315,19 @@ impl StateDb { pub fn get>(&self, key: &Key, db: &D) -> Result, Error> { self.db.read().get(key, db) } + + /// Revert all unfinalized blocks with the best block number. + /// Returns a database commit or `None` if not possible. + /// For archive an empty commit set is returned. + pub fn revert_one(&self) -> Option> { + self.db.write().revert_one() + } + + /// Returns last finalized block number. + pub fn best_finalized(&self) -> u64 { + return self.db.read().best_finalized() + } + } #[cfg(test)] diff --git a/substrate/substrate/state-db/src/unfinalized.rs b/substrate/substrate/state-db/src/unfinalized.rs index df3b98c8ca..ed122b4dbc 100644 --- a/substrate/substrate/state-db/src/unfinalized.rs +++ b/substrate/substrate/state-db/src/unfinalized.rs @@ -204,6 +204,10 @@ impl UnfinalizedOverlay { self.last_finalized.as_ref().map(|&(_, n)| n + 1).unwrap_or(0) } + pub fn last_finalized_block_number(&self) -> u64 { + self.last_finalized.as_ref().map(|&(_, n)| n).unwrap_or(0) + } + /// This may be called when the last finalization commit was applied to the database. pub fn clear_overlay(&mut self) { self.last_finalized_overlay.clear(); @@ -259,6 +263,18 @@ impl UnfinalizedOverlay { } None } + + /// Revert a single level. Returns commit set that deletes the journal or `None` if not possible. + pub fn revert_one(&mut self) -> Option> { + self.levels.pop_back().map(|level| { + let mut commit = CommitSet::default(); + for overlay in level.into_iter() { + commit.meta.deleted.push(overlay.journal_key); + self.parents.remove(&overlay.hash); + } + commit + }) + } } #[cfg(test)] @@ -370,6 +386,23 @@ mod tests { assert_eq!(overlay.last_finalized, overlay2.last_finalized); } + #[test] + fn restore_from_journal_after_finalize() { + let h1 = H256::random(); + let h2 = H256::random(); + let mut db = make_db(&[1, 2]); + let mut overlay = UnfinalizedOverlay::::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.finalize(&h1)); + assert_eq!(overlay.levels.len(), 1); + + let overlay2 = UnfinalizedOverlay::::new(&db).unwrap(); + assert_eq!(overlay.levels, overlay2.levels); + assert_eq!(overlay.parents, overlay2.parents); + assert_eq!(overlay.last_finalized, overlay2.last_finalized); + } + #[test] fn insert_finalize_two() { let h1 = H256::random(); @@ -492,4 +525,28 @@ mod tests { assert!(db.data_eq(&make_db(&[1, 12, 122]))); assert_eq!(overlay.last_finalized, Some((h_1_2_2, 3))); } + + #[test] + fn insert_revert() { + let h1 = H256::random(); + let h2 = H256::random(); + let mut db = make_db(&[1, 2, 3, 4]); + let mut overlay = UnfinalizedOverlay::::new(&db).unwrap(); + 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)); + assert!(contains(&overlay, 7)); + db.commit(&overlay.revert_one().unwrap()); + assert_eq!(overlay.parents.len(), 1); + assert!(contains(&overlay, 5)); + assert!(!contains(&overlay, 7)); + db.commit(&overlay.revert_one().unwrap()); + assert_eq!(overlay.levels.len(), 0); + assert_eq!(overlay.parents.len(), 0); + assert!(overlay.revert_one().is_none()); + } + } +