Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Chain-selection subsystem data revert (#5350)
Browse files Browse the repository at this point in the history
* Chain-selection subsystem data revert

* Cargo fmt

* Better code comments

* Remove unwraps

* Document public method

* Remove duplicated 'ChainSelectionSubsystem' impl block

* Fix typos

* Nitpicks

* Revert returns a service Error

* Removed superflous error handling

* Apply suggestions from code review

* Rename tree 'revert' to 'revert_to'

Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
  • Loading branch information
davxy and skunert authored Apr 28, 2022
1 parent 6566adc commit 6503d68
Show file tree
Hide file tree
Showing 6 changed files with 243 additions and 45 deletions.
14 changes: 13 additions & 1 deletion cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,19 @@ pub fn run() -> Result<()> {

Ok(runner.async_run(|mut config| {
let (client, backend, _, task_manager) = service::new_chain_ops(&mut config, None)?;
Ok((cmd.run(client, backend, None).map_err(Error::SubstrateCli), task_manager))
let aux_revert = Box::new(|client, backend, blocks| {
service::revert_backend(client, backend, blocks, config).map_err(|err| {
match err {
service::Error::Blockchain(err) => err.into(),
// Generic application-specific error.
err => sc_cli::Error::Application(err.into()),
}
})
});
Ok((
cmd.run(client, backend, Some(aux_revert)).map_err(Error::SubstrateCli),
task_manager,
))
})?)
},
Some(Subcommand::PvfPrepareWorker(cmd)) => {
Expand Down
6 changes: 4 additions & 2 deletions node/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use polkadot_primitives::{
use sc_client_api::{AuxStore, Backend as BackendT, BlockchainEvents, KeyIterator, UsageProvider};
use sc_executor::NativeElseWasmExecutor;
use sp_api::{CallApiAt, Encode, NumberFor, ProvideRuntimeApi};
use sp_blockchain::HeaderBackend;
use sp_blockchain::{HeaderBackend, HeaderMetadata};
use sp_consensus::BlockStatus;
use sp_core::Pair;
use sp_keyring::Sr25519Keyring;
Expand Down Expand Up @@ -173,6 +173,7 @@ pub trait AbstractClient<Block, Backend>:
+ CallApiAt<Block, StateBackend = Backend::State>
+ AuxStore
+ UsageProvider<Block>
+ HeaderMetadata<Block, Error = sp_blockchain::Error>
where
Block: BlockT,
Backend: BackendT<Block>,
Expand All @@ -194,7 +195,8 @@ where
+ Sized
+ Send
+ Sync
+ CallApiAt<Block, StateBackend = Backend::State>,
+ CallApiAt<Block, StateBackend = Backend::State>
+ HeaderMetadata<Block, Error = sp_blockchain::Error>,
Client::Api: RuntimeApiCollection<StateBackend = Backend::State>,
{
}
Expand Down
26 changes: 18 additions & 8 deletions node/core/chain-selection/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,17 @@ impl ChainSelectionSubsystem {
pub fn new(config: Config, db: Arc<dyn Database>) -> Self {
ChainSelectionSubsystem { config, db }
}

/// Revert to the block corresponding to the specified `hash`.
/// The revert is not allowed for blocks older than the last finalized one.
pub fn revert(&self, hash: Hash) -> Result<(), Error> {
let backend_config = db_backend::v1::Config { col_data: self.config.col_data };
let mut backend = db_backend::v1::DbBackend::new(self.db.clone(), backend_config);

let ops = tree::revert_to(&backend, hash)?.into_write_ops();

backend.write(ops)
}
}

impl<Context> overseer::Subsystem<Context, SubsystemError> for ChainSelectionSubsystem
Expand All @@ -323,9 +334,9 @@ where
Context: overseer::SubsystemContext<Message = ChainSelectionMessage>,
{
fn start(self, ctx: Context) -> SpawnedSubsystem {
let backend = crate::db_backend::v1::DbBackend::new(
let backend = db_backend::v1::DbBackend::new(
self.db,
crate::db_backend::v1::Config { col_data: self.config.col_data },
db_backend::v1::Config { col_data: self.config.col_data },
);

SpawnedSubsystem {
Expand Down Expand Up @@ -412,7 +423,7 @@ where
let _ = tx.send(leaves);
}
ChainSelectionMessage::BestLeafContaining(required, tx) => {
let best_containing = crate::backend::find_best_leaf_containing(
let best_containing = backend::find_best_leaf_containing(
&*backend,
required,
)?;
Expand Down Expand Up @@ -549,7 +560,7 @@ async fn handle_active_leaf(
};

let reversion_logs = extract_reversion_logs(&header);
crate::tree::import_block(
tree::import_block(
&mut overlay,
hash,
header.number,
Expand Down Expand Up @@ -612,8 +623,7 @@ fn handle_finalized_block(
finalized_hash: Hash,
finalized_number: BlockNumber,
) -> Result<(), Error> {
let ops =
crate::tree::finalize_block(&*backend, finalized_hash, finalized_number)?.into_write_ops();
let ops = tree::finalize_block(&*backend, finalized_hash, finalized_number)?.into_write_ops();

backend.write(ops)
}
Expand All @@ -623,7 +633,7 @@ fn handle_approved_block(backend: &mut impl Backend, approved_block: Hash) -> Re
let ops = {
let mut overlay = OverlayedBackend::new(&*backend);

crate::tree::approve_block(&mut overlay, approved_block)?;
tree::approve_block(&mut overlay, approved_block)?;

overlay.into_write_ops()
};
Expand All @@ -633,7 +643,7 @@ fn handle_approved_block(backend: &mut impl Backend, approved_block: Hash) -> Re

fn detect_stagnant(backend: &mut impl Backend, now: Timestamp) -> Result<(), Error> {
let ops = {
let overlay = crate::tree::detect_stagnant(&*backend, now)?;
let overlay = tree::detect_stagnant(&*backend, now)?;

overlay.into_write_ops()
};
Expand Down
102 changes: 101 additions & 1 deletion node/core/chain-selection/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
//! and as the finalized block advances, orphaned sub-trees are entirely pruned.
use polkadot_node_primitives::BlockWeight;
use polkadot_node_subsystem::ChainApiError;
use polkadot_primitives::v2::{BlockNumber, Hash};

use std::collections::HashMap;
Expand Down Expand Up @@ -86,7 +87,7 @@ impl ViabilityUpdate {

// Propagate viability update to descendants of the given block. This writes
// the `base` entry as well as all descendants. If the parent of the block
// entry is not viable, this wlil not affect any descendants.
// entry is not viable, this will not affect any descendants.
//
// If the block entry provided is self-unviable, then it's assumed that an
// unviability update needs to be propagated to descendants.
Expand Down Expand Up @@ -561,3 +562,102 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>(

Ok(backend)
}

/// Revert the tree to the block relative to `hash`.
///
/// This accepts a fresh backend and returns an overlay on top of it representing
/// all changes made.
pub(super) fn revert_to<'a, B: Backend + 'a>(
backend: &'a B,
hash: Hash,
) -> Result<OverlayedBackend<'a, B>, Error> {
let first_number = backend.load_first_block_number()?.unwrap_or_default();

let mut backend = OverlayedBackend::new(backend);

let mut entry = match backend.load_block_entry(&hash)? {
Some(entry) => entry,
None => {
// May be a revert to the last finalized block. If this is the case,
// then revert to this block should be handled specially since no
// information about finalized blocks is persisted within the tree.
//
// We use part of the information contained in the finalized block
// children (that are expected to be in the tree) to construct a
// dummy block entry for the last finalized block. This will be
// wiped as soon as the next block is finalized.

let blocks = backend.load_blocks_by_number(first_number)?;

let block = blocks
.first()
.and_then(|hash| backend.load_block_entry(hash).ok())
.flatten()
.ok_or_else(|| {
ChainApiError::from(format!(
"Lookup failure for block at height {}",
first_number
))
})?;

// The parent is expected to be the last finalized block.
if block.parent_hash != hash {
return Err(ChainApiError::from("Can't revert below last finalized block").into())
}

// The weight is set to the one of the first child. Even though this is
// not accurate, it does the job. The reason is that the revert point is
// the last finalized block, i.e. this is the best and only choice.
let block_number = first_number.saturating_sub(1);
let viability = ViabilityCriteria {
explicitly_reverted: false,
approval: Approval::Approved,
earliest_unviable_ancestor: None,
};
let entry = BlockEntry {
block_hash: hash,
block_number,
parent_hash: Hash::default(),
children: blocks,
viability,
weight: block.weight,
};
// This becomes the first entry according to the block number.
backend.write_blocks_by_number(block_number, vec![hash]);
entry
},
};

let mut stack: Vec<_> = std::mem::take(&mut entry.children)
.into_iter()
.map(|h| (h, entry.block_number + 1))
.collect();

// Write revert point block entry without the children.
backend.write_block_entry(entry.clone());

let mut viable_leaves = backend.load_leaves()?;

viable_leaves.insert(LeafEntry {
block_hash: hash,
block_number: entry.block_number,
weight: entry.weight,
});

while let Some((hash, number)) = stack.pop() {
let entry = backend.load_block_entry(&hash)?;
backend.delete_block_entry(&hash);

viable_leaves.remove(&hash);

let mut blocks_at_height = backend.load_blocks_by_number(number)?;
blocks_at_height.retain(|h| h != &hash);
backend.write_blocks_by_number(number, blocks_at_height);

stack.extend(entry.into_iter().flat_map(|e| e.children).map(|h| (h, number + 1)));
}

backend.write_leaves(viable_leaves);

Ok(backend)
}
Loading

0 comments on commit 6503d68

Please sign in to comment.