Fix leaf block removal in the backend (#12005)

* Fix leaf block removal in the backend

The fix introduced the new 'removal' method for the backend leaves set
and the improvement of the undo features.

* Update docs

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Fix docs typo

* On block block removal the new children list should be persisted.

* Align leaves set removal tests to the new interface

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
This commit is contained in:
Davide Galassi
2022-08-17 17:36:18 +02:00
committed by GitHub
parent ba2c89e6b5
commit 61eeba81f0
2 changed files with 251 additions and 94 deletions
+81 -11
View File
@@ -59,7 +59,7 @@ use codec::{Decode, Encode};
use hash_db::Prefix;
use sc_client_api::{
backend::NewBlockState,
leaves::{FinalizationDisplaced, LeafSet},
leaves::{FinalizationOutcome, LeafSet},
utils::is_descendent_of,
IoInfo, MemoryInfo, MemorySize, UsageInfo,
};
@@ -1251,7 +1251,7 @@ impl<Block: BlockT> Backend<Block> {
header: &Block::Header,
last_finalized: Option<Block::Hash>,
justification: Option<Justification>,
finalization_displaced: &mut Option<FinalizationDisplaced<Block::Hash, NumberFor<Block>>>,
finalization_displaced: &mut Option<FinalizationOutcome<Block::Hash, NumberFor<Block>>>,
) -> ClientResult<MetaUpdate<Block>> {
// TODO: ensure best chain contains this block.
let number = *header.number();
@@ -1657,7 +1657,7 @@ impl<Block: BlockT> Backend<Block> {
transaction: &mut Transaction<DbHash>,
f_header: &Block::Header,
f_hash: Block::Hash,
displaced: &mut Option<FinalizationDisplaced<Block::Hash, NumberFor<Block>>>,
displaced: &mut Option<FinalizationOutcome<Block::Hash, NumberFor<Block>>>,
with_state: bool,
) -> ClientResult<()> {
let f_num = *f_header.number();
@@ -1696,7 +1696,7 @@ impl<Block: BlockT> Backend<Block> {
&self,
transaction: &mut Transaction<DbHash>,
finalized: NumberFor<Block>,
displaced: &FinalizationDisplaced<Block::Hash, NumberFor<Block>>,
displaced: &FinalizationOutcome<Block::Hash, NumberFor<Block>>,
) -> ClientResult<()> {
if let BlocksPruning::Some(blocks_pruning) = self.blocks_pruning {
// Always keep the last finalized block
@@ -2226,9 +2226,40 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
apply_state_commit(&mut transaction, commit);
}
transaction.remove(columns::KEY_LOOKUP, hash.as_ref());
leaves.revert(*hash, hdr.number);
let children: Vec<_> = self
.blockchain()
.children(hdr.parent)?
.into_iter()
.filter(|child_hash| child_hash != hash)
.collect();
let parent_leaf = if children.is_empty() {
children::remove_children(
&mut transaction,
columns::META,
meta_keys::CHILDREN_PREFIX,
hdr.parent,
);
Some(hdr.parent)
} else {
children::write_children(
&mut transaction,
columns::META,
meta_keys::CHILDREN_PREFIX,
hdr.parent,
children,
);
None
};
let remove_outcome = leaves.remove(*hash, hdr.number, parent_leaf);
leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX);
self.storage.db.commit(transaction)?;
if let Err(e) = self.storage.db.commit(transaction) {
if let Some(outcome) = remove_outcome {
leaves.undo().undo_remove(outcome);
}
return Err(e.into())
}
self.blockchain().remove_header_metadata(*hash);
Ok(())
}
@@ -3376,7 +3407,21 @@ pub(crate) mod tests {
prev_hash = hash;
}
// insert a fork at block 2, which becomes best block
for i in 0..2 {
let hash = insert_block(
&backend,
2,
blocks[1],
None,
sp_core::H256::random(),
vec![i.into()],
None,
)
.unwrap();
blocks.push(hash);
}
// insert a fork at block 1, which becomes best block
let best_hash = insert_block(
&backend,
1,
@@ -3387,11 +3432,36 @@ pub(crate) mod tests {
None,
)
.unwrap();
assert_eq!(backend.blockchain().info().best_hash, best_hash);
assert!(backend.remove_leaf_block(&best_hash).is_err());
assert!(backend.have_state_at(&prev_hash, 1));
backend.remove_leaf_block(&prev_hash).unwrap();
assert_eq!(None, backend.blockchain().header(BlockId::hash(prev_hash)).unwrap());
assert!(!backend.have_state_at(&prev_hash, 1));
assert_eq!(backend.blockchain().leaves().unwrap(), vec![blocks[2], blocks[3], best_hash]);
assert_eq!(backend.blockchain().children(blocks[1]).unwrap(), vec![blocks[2], blocks[3]]);
assert!(backend.have_state_at(&blocks[3], 2));
assert!(backend.blockchain().header(BlockId::hash(blocks[3])).unwrap().is_some());
backend.remove_leaf_block(&blocks[3]).unwrap();
assert!(!backend.have_state_at(&blocks[3], 2));
assert!(backend.blockchain().header(BlockId::hash(blocks[3])).unwrap().is_none());
assert_eq!(backend.blockchain().leaves().unwrap(), vec![blocks[2], best_hash]);
assert_eq!(backend.blockchain().children(blocks[1]).unwrap(), vec![blocks[2]]);
assert!(backend.have_state_at(&blocks[2], 2));
assert!(backend.blockchain().header(BlockId::hash(blocks[2])).unwrap().is_some());
backend.remove_leaf_block(&blocks[2]).unwrap();
assert!(!backend.have_state_at(&blocks[2], 2));
assert!(backend.blockchain().header(BlockId::hash(blocks[2])).unwrap().is_none());
assert_eq!(backend.blockchain().leaves().unwrap(), vec![best_hash, blocks[1]]);
assert_eq!(backend.blockchain().children(blocks[1]).unwrap(), vec![]);
assert!(backend.have_state_at(&blocks[1], 1));
assert!(backend.blockchain().header(BlockId::hash(blocks[1])).unwrap().is_some());
backend.remove_leaf_block(&blocks[1]).unwrap();
assert!(!backend.have_state_at(&blocks[1], 1));
assert!(backend.blockchain().header(BlockId::hash(blocks[1])).unwrap().is_none());
assert_eq!(backend.blockchain().leaves().unwrap(), vec![best_hash]);
assert_eq!(backend.blockchain().children(blocks[0]).unwrap(), vec![best_hash]);
}
#[test]