diff --git a/Cargo.lock b/Cargo.lock
index f3808b7eaa53d1ab6013ebcf3f5819dc033481a4..ff6e3c06313f16c098628c4e8caf590ce41acc1c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -20018,6 +20018,7 @@ dependencies = [
"schnellru",
"sp-api",
"sp-consensus",
+ "sp-core",
"sp-database",
"sp-runtime",
"sp-state-machine",
diff --git a/prdoc/pr_4922.prdoc b/prdoc/pr_4922.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..2e2dd26947c0d486fa92023c3021cdcb52128938
--- /dev/null
+++ b/prdoc/pr_4922.prdoc
@@ -0,0 +1,15 @@
+title: Optimize finalization performance
+
+doc:
+ - audience: Node Dev
+ description: |
+ Finalization algorithm was replaced with a more efficient version, data structures refactored to be faster and do
+ fewer memory allocations. As the result some APIs have changed in a minor, but incompatible way.
+
+crates:
+- name: sc-client-api
+ bump: major
+- name: sc-client-db
+ bump: major
+- name: sp-blockchain
+ bump: major
diff --git a/substrate/client/api/src/backend.rs b/substrate/client/api/src/backend.rs
index 31b100433c7086719737275bfaa6b3931bffc88a..0b2a349524018d33b8003b6832363286b77d8503 100644
--- a/substrate/client/api/src/backend.rs
+++ b/substrate/client/api/src/backend.rs
@@ -217,7 +217,8 @@ pub trait BlockImportOperation<Block: BlockT> {
where
I: IntoIterator<Item = (Vec<u8>, Option<Vec<u8>>)>;
- /// Mark a block as finalized.
+ /// Mark a block as finalized, if multiple blocks are finalized in the same operation then they
+ /// must be marked in ascending order.
fn mark_finalized(
&mut self,
hash: Block::Hash,
diff --git a/substrate/client/api/src/leaves.rs b/substrate/client/api/src/leaves.rs
index e129de8bf3fad30a0bb76569957c93138c631db8..70efe8b19c627d03741a3ea11b8889332b5a567c 100644
--- a/substrate/client/api/src/leaves.rs
+++ b/substrate/client/api/src/leaves.rs
@@ -45,33 +45,20 @@ pub struct RemoveOutcome<H, N> {
}
/// Removed leaves after a finalization action.
-pub struct FinalizationOutcome<H, N> {
- removed: BTreeMap<Reverse<N>, Vec<H>>,
+pub struct FinalizationOutcome<I, H, N>
+where
+ I: Iterator<Item = (N, H)>,
+{
+ removed: I,
}
-impl<H: Copy, N: Ord> FinalizationOutcome<H, N> {
- /// Merge with another. This should only be used for displaced items that
- /// are produced within one transaction of each other.
- pub fn merge(&mut self, mut other: Self) {
- // this will ignore keys that are in duplicate, however
- // if these are actually produced correctly via the leaf-set within
- // one transaction, then there will be no overlap in the keys.
- self.removed.append(&mut other.removed);
- }
-
- /// Iterate over all displaced leaves.
- pub fn leaves(&self) -> impl Iterator<Item = &H> {
- self.removed.values().flatten()
- }
-
+impl<I, H: Ord, N: Ord> FinalizationOutcome<I, H, N>
+where
+ I: Iterator<Item = (N, H)>,
+{
/// Constructor
- pub fn new(new_displaced: impl Iterator<Item = (H, N)>) -> Self {
- let mut removed = BTreeMap::<Reverse<N>, Vec<H>>::new();
- for (hash, number) in new_displaced {
- removed.entry(Reverse(number)).or_default().push(hash);
- }
-
- FinalizationOutcome { removed }
+ pub fn new(new_displaced: I) -> Self {
+ FinalizationOutcome { removed: new_displaced }
}
}
@@ -86,7 +73,7 @@ pub struct LeafSet<H, N> {
impl<H, N> LeafSet<H, N>
where
H: Clone + PartialEq + Decode + Encode,
- N: std::fmt::Debug + Clone + AtLeast32Bit + Decode + Encode,
+ N: std::fmt::Debug + Copy + AtLeast32Bit + Decode + Encode,
{
/// Construct a new, blank leaf set.
pub fn new() -> Self {
@@ -117,13 +104,13 @@ where
let number = Reverse(number);
let removed = if number.0 != N::zero() {
- let parent_number = Reverse(number.0.clone() - N::one());
+ let parent_number = Reverse(number.0 - N::one());
self.remove_leaf(&parent_number, &parent_hash).then(|| parent_hash)
} else {
None
};
- self.insert_leaf(number.clone(), hash.clone());
+ self.insert_leaf(number, hash.clone());
ImportOutcome { inserted: LeafSetItem { hash, number }, removed }
}
@@ -150,7 +137,7 @@ where
let inserted = parent_hash.and_then(|parent_hash| {
if number.0 != N::zero() {
- let parent_number = Reverse(number.0.clone() - N::one());
+ let parent_number = Reverse(number.0 - N::one());
self.insert_leaf(parent_number, parent_hash.clone());
Some(parent_hash)
} else {
@@ -162,11 +149,12 @@ where
}
/// Remove all leaves displaced by the last block finalization.
- pub fn remove_displaced_leaves(&mut self, displaced_leaves: &FinalizationOutcome<H, N>) {
- for (number, hashes) in &displaced_leaves.removed {
- for hash in hashes.iter() {
- self.remove_leaf(number, hash);
- }
+ pub fn remove_displaced_leaves<I>(&mut self, displaced_leaves: FinalizationOutcome<I, H, N>)
+ where
+ I: Iterator<Item = (N, H)>,
+ {
+ for (number, hash) in displaced_leaves.removed {
+ self.remove_leaf(&Reverse(number), &hash);
}
}
@@ -186,13 +174,13 @@ where
let items = self
.storage
.iter()
- .flat_map(|(number, hashes)| hashes.iter().map(move |h| (h.clone(), number.clone())))
+ .flat_map(|(number, hashes)| hashes.iter().map(move |h| (h.clone(), *number)))
.collect::<Vec<_>>();
- for (hash, number) in &items {
+ for (hash, number) in items {
if number.0 > best_number {
assert!(
- self.remove_leaf(number, hash),
+ self.remove_leaf(&number, &hash),
"item comes from an iterator over storage; qed",
);
}
@@ -207,7 +195,7 @@ where
// we need to make sure that the best block exists in the leaf set as
// this is an invariant of regular block import.
if !leaves_contains_best {
- self.insert_leaf(best_number.clone(), best_hash.clone());
+ self.insert_leaf(best_number, best_hash.clone());
}
}
@@ -229,7 +217,7 @@ where
column: u32,
prefix: &[u8],
) {
- let leaves: Vec<_> = self.storage.iter().map(|(n, h)| (n.0.clone(), h.clone())).collect();
+ let leaves: Vec<_> = self.storage.iter().map(|(n, h)| (n.0, h.clone())).collect();
tx.set_from_vec(column, prefix, leaves.encode());
}
@@ -274,7 +262,7 @@ where
/// Returns the highest leaf and all hashes associated to it.
pub fn highest_leaf(&self) -> Option<(N, &[H])> {
- self.storage.iter().next().map(|(k, v)| (k.0.clone(), &v[..]))
+ self.storage.iter().next().map(|(k, v)| (k.0, &v[..]))
}
}
@@ -286,13 +274,13 @@ pub struct Undo<'a, H: 'a, N: 'a> {
impl<'a, H: 'a, N: 'a> Undo<'a, H, N>
where
H: Clone + PartialEq + Decode + Encode,
- N: std::fmt::Debug + Clone + AtLeast32Bit + Decode + Encode,
+ N: std::fmt::Debug + Copy + AtLeast32Bit + Decode + Encode,
{
/// Undo an imported block by providing the import operation outcome.
/// No additional operations should be performed between import and undo.
pub fn undo_import(&mut self, outcome: ImportOutcome<H, N>) {
if let Some(removed_hash) = outcome.removed {
- let removed_number = Reverse(outcome.inserted.number.0.clone() - N::one());
+ let removed_number = Reverse(outcome.inserted.number.0 - N::one());
self.inner.insert_leaf(removed_number, removed_hash);
}
self.inner.remove_leaf(&outcome.inserted.number, &outcome.inserted.hash);
@@ -302,7 +290,7 @@ where
/// No additional operations should be performed between remove and undo.
pub fn undo_remove(&mut self, outcome: RemoveOutcome<H, N>) {
if let Some(inserted_hash) = outcome.inserted {
- let inserted_number = Reverse(outcome.removed.number.0.clone() - N::one());
+ let inserted_number = Reverse(outcome.removed.number.0 - N::one());
self.inner.remove_leaf(&inserted_number, &inserted_hash);
}
self.inner.insert_leaf(outcome.removed.number, outcome.removed.hash);
@@ -310,8 +298,13 @@ where
/// Undo a finalization operation by providing the displaced leaves.
/// No additional operations should be performed between finalization and undo.
- pub fn undo_finalization(&mut self, mut outcome: FinalizationOutcome<H, N>) {
- self.inner.storage.append(&mut outcome.removed);
+ pub fn undo_finalization<I>(&mut self, outcome: FinalizationOutcome<I, H, N>)
+ where
+ I: Iterator<Item = (N, H)>,
+ {
+ for (number, hash) in outcome.removed {
+ self.inner.storage.entry(Reverse(number)).or_default().push(hash);
+ }
}
}
diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs
index 8d8b7a2aff88f88a3f034845a3422bad51258e69..e95cd9e4ad5fd3e527f3f870aaf40948b238c9a2 100644
--- a/substrate/client/db/src/lib.rs
+++ b/substrate/client/db/src/lib.rs
@@ -1357,6 +1357,8 @@ impl<Block: BlockT> Backend<Block> {
Ok(())
}
+ /// `remove_displaced` can be set to `false` if this is not the last of many subsequent calls
+ /// for performance reasons.
fn finalize_block_with_transaction(
&self,
transaction: &mut Transaction<DbHash>,
@@ -1365,6 +1367,7 @@ impl<Block: BlockT> Backend<Block> {
last_finalized: Option<Block::Hash>,
justification: Option<Justification>,
current_transaction_justifications: &mut HashMap<Block::Hash, Justification>,
+ remove_displaced: bool,
) -> ClientResult<MetaUpdate<Block>> {
// TODO: ensure best chain contains this block.
let number = *header.number();
@@ -1377,6 +1380,7 @@ impl<Block: BlockT> Backend<Block> {
hash,
with_state,
current_transaction_justifications,
+ remove_displaced,
)?;
if let Some(justification) = justification {
@@ -1454,7 +1458,8 @@ impl<Block: BlockT> Backend<Block> {
let mut current_transaction_justifications: HashMap<Block::Hash, Justification> =
HashMap::new();
- for (block_hash, justification) in operation.finalized_blocks {
+ let mut finalized_blocks = operation.finalized_blocks.into_iter().peekable();
+ while let Some((block_hash, justification)) = finalized_blocks.next() {
let block_header = self.blockchain.expect_header(block_hash)?;
meta_updates.push(self.finalize_block_with_transaction(
&mut transaction,
@@ -1463,6 +1468,7 @@ impl<Block: BlockT> Backend<Block> {
Some(last_finalized_hash),
justification,
&mut current_transaction_justifications,
+ finalized_blocks.peek().is_none(),
)?);
last_finalized_hash = block_hash;
last_finalized_num = *block_header.number();
@@ -1642,6 +1648,7 @@ impl<Block: BlockT> Backend<Block> {
hash,
operation.commit_state,
&mut current_transaction_justifications,
+ true,
)?;
} else {
// canonicalize blocks which are old enough, regardless of finality.
@@ -1766,9 +1773,10 @@ impl<Block: BlockT> Backend<Block> {
Ok(())
}
- // write stuff to a transaction after a new block is finalized.
- // this canonicalizes finalized blocks. Fails if called with a block which
- // was not a child of the last finalized block.
+ // Write stuff to a transaction after a new block is finalized. This canonicalizes finalized
+ // blocks. Fails if called with a block which was not a child of the last finalized block.
+ /// `remove_displaced` can be set to `false` if this is not the last of many subsequent calls
+ /// for performance reasons.
fn note_finalized(
&self,
transaction: &mut Transaction<DbHash>,
@@ -1776,6 +1784,7 @@ impl<Block: BlockT> Backend<Block> {
f_hash: Block::Hash,
with_state: bool,
current_transaction_justifications: &mut HashMap<Block::Hash, Justification>,
+ remove_displaced: bool,
) -> ClientResult<()> {
let f_num = *f_header.number();
@@ -1800,13 +1809,19 @@ impl<Block: BlockT> Backend<Block> {
apply_state_commit(transaction, commit);
}
- let new_displaced = self.blockchain.displaced_leaves_after_finalizing(f_hash, f_num)?;
- let finalization_outcome =
- FinalizationOutcome::new(new_displaced.displaced_leaves.clone().into_iter());
+ if remove_displaced {
+ let new_displaced = self.blockchain.displaced_leaves_after_finalizing(f_hash, f_num)?;
- self.blockchain.leaves.write().remove_displaced_leaves(&finalization_outcome);
+ self.blockchain.leaves.write().remove_displaced_leaves(FinalizationOutcome::new(
+ new_displaced.displaced_leaves.iter().copied(),
+ ));
- self.prune_blocks(transaction, f_num, &new_displaced, current_transaction_justifications)?;
+ if !matches!(self.blocks_pruning, BlocksPruning::KeepAll) {
+ self.prune_displaced_branches(transaction, &new_displaced)?;
+ }
+ }
+
+ self.prune_blocks(transaction, f_num, current_transaction_justifications)?;
Ok(())
}
@@ -1815,39 +1830,29 @@ impl<Block: BlockT> Backend<Block> {
&self,
transaction: &mut Transaction<DbHash>,
finalized_number: NumberFor<Block>,
- displaced: &DisplacedLeavesAfterFinalization<Block>,
current_transaction_justifications: &mut HashMap<Block::Hash, Justification>,
) -> ClientResult<()> {
- match self.blocks_pruning {
- BlocksPruning::KeepAll => {},
- BlocksPruning::Some(blocks_pruning) => {
- // Always keep the last finalized block
- let keep = std::cmp::max(blocks_pruning, 1);
- if finalized_number >= keep.into() {
- let number = finalized_number.saturating_sub(keep.into());
-
- // Before we prune a block, check if it is pinned
- if let Some(hash) = self.blockchain.hash(number)? {
- self.blockchain.insert_persisted_body_if_pinned(hash)?;
-
- // If the block was finalized in this transaction, it will not be in the db
- // yet.
- if let Some(justification) =
- current_transaction_justifications.remove(&hash)
- {
- self.blockchain.insert_justifications_if_pinned(hash, justification);
- } else {
- self.blockchain.insert_persisted_justifications_if_pinned(hash)?;
- }
- };
+ if let BlocksPruning::Some(blocks_pruning) = self.blocks_pruning {
+ // Always keep the last finalized block
+ let keep = std::cmp::max(blocks_pruning, 1);
+ if finalized_number >= keep.into() {
+ let number = finalized_number.saturating_sub(keep.into());
+
+ // Before we prune a block, check if it is pinned
+ if let Some(hash) = self.blockchain.hash(number)? {
+ self.blockchain.insert_persisted_body_if_pinned(hash)?;
+
+ // If the block was finalized in this transaction, it will not be in the db
+ // yet.
+ if let Some(justification) = current_transaction_justifications.remove(&hash) {
+ self.blockchain.insert_justifications_if_pinned(hash, justification);
+ } else {
+ self.blockchain.insert_persisted_justifications_if_pinned(hash)?;
+ }
+ };
- self.prune_block(transaction, BlockId::<Block>::number(number))?;
- }
- self.prune_displaced_branches(transaction, displaced)?;
- },
- BlocksPruning::KeepFinalized => {
- self.prune_displaced_branches(transaction, displaced)?;
- },
+ self.prune_block(transaction, BlockId::<Block>::number(number))?;
+ }
}
Ok(())
}
@@ -1858,11 +1863,9 @@ impl<Block: BlockT> Backend<Block> {
displaced: &DisplacedLeavesAfterFinalization<Block>,
) -> ClientResult<()> {
// Discard all blocks from displaced branches
- for (_, tree_route) in displaced.tree_routes.iter() {
- for r in tree_route.retracted() {
- self.blockchain.insert_persisted_body_if_pinned(r.hash)?;
- self.prune_block(transaction, BlockId::<Block>::hash(r.hash))?;
- }
+ for &hash in displaced.displaced_blocks.iter() {
+ self.blockchain.insert_persisted_body_if_pinned(hash)?;
+ self.prune_block(transaction, BlockId::<Block>::hash(hash))?;
}
Ok(())
}
@@ -2110,6 +2113,7 @@ impl<Block: BlockT> sc_client_api::backend::Backend<Block> for Backend<Block> {
None,
justification,
&mut current_transaction_justifications,
+ true,
)?;
self.storage.db.commit(transaction)?;
@@ -2547,7 +2551,7 @@ pub(crate) mod tests {
backend::{Backend as BTrait, BlockImportOperation as Op},
blockchain::Backend as BLBTrait,
};
- use sp_blockchain::{lowest_common_ancestor, lowest_common_ancestor_multiblock, tree_route};
+ use sp_blockchain::{lowest_common_ancestor, tree_route};
use sp_core::H256;
use sp_runtime::{
testing::{Block as RawBlock, ExtrinsicWrapper, Header},
@@ -3109,121 +3113,118 @@ pub(crate) mod tests {
}
#[test]
- fn lowest_common_ancestors_multiblock_works() {
+ fn displaced_leaves_after_finalizing_works() {
let backend = Backend::<Block>::new_test(1000, 100);
let blockchain = backend.blockchain();
- let block0 = insert_header(&backend, 0, Default::default(), None, Default::default());
+ let genesis_number = 0;
+ let genesis_hash =
+ insert_header(&backend, genesis_number, Default::default(), None, Default::default());
// fork from genesis: 3 prong.
// block 0 -> a1 -> a2 -> a3
- // |
- // -> b1 -> b2 -> c1 -> c2
- // |
- // -> d1 -> d2
- let a1 = insert_header(&backend, 1, block0, None, Default::default());
- let a2 = insert_header(&backend, 2, a1, None, Default::default());
- let a3 = insert_header(&backend, 3, a2, None, Default::default());
-
- // fork from genesis: 2 prong.
- let b1 = insert_header(&backend, 1, block0, None, H256::from([1; 32]));
- let b2 = insert_header(&backend, 2, b1, None, Default::default());
-
- // fork from b2.
- let c1 = insert_header(&backend, 3, b2, None, H256::from([2; 32]));
- let c2 = insert_header(&backend, 4, c1, None, Default::default());
-
- // fork from b1.
- let d1 = insert_header(&backend, 2, b1, None, H256::from([3; 32]));
- let d2 = insert_header(&backend, 3, d1, None, Default::default());
- {
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a3, b2]).unwrap().unwrap();
-
- assert_eq!(lca.hash, block0);
- assert_eq!(lca.number, 0);
- }
-
- {
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a1, a3]).unwrap().unwrap();
-
- assert_eq!(lca.hash, a1);
- assert_eq!(lca.number, 1);
- }
-
- {
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a3, a1]).unwrap().unwrap();
-
- assert_eq!(lca.hash, a1);
- assert_eq!(lca.number, 1);
- }
-
- {
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a2, a3]).unwrap().unwrap();
-
- assert_eq!(lca.hash, a2);
- assert_eq!(lca.number, 2);
- }
+ // \
+ // -> b1 -> b2 -> c1 -> c2
+ // \
+ // -> d1 -> d2
+ let a1_number = 1;
+ let a1_hash = insert_header(&backend, a1_number, genesis_hash, None, Default::default());
+ let a2_number = 2;
+ let a2_hash = insert_header(&backend, a2_number, a1_hash, None, Default::default());
+ let a3_number = 3;
+ let a3_hash = insert_header(&backend, a3_number, a2_hash, None, Default::default());
{
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a2, a1]).unwrap().unwrap();
-
- assert_eq!(lca.hash, a1);
- assert_eq!(lca.number, 1);
+ let displaced = blockchain
+ .displaced_leaves_after_finalizing(genesis_hash, genesis_number)
+ .unwrap();
+ assert_eq!(displaced.displaced_leaves, vec![]);
+ assert_eq!(displaced.displaced_blocks, vec![]);
}
-
{
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a2, a2]).unwrap().unwrap();
-
- assert_eq!(lca.hash, a2);
- assert_eq!(lca.number, 2);
+ let displaced_a1 =
+ blockchain.displaced_leaves_after_finalizing(a1_hash, a1_number).unwrap();
+ assert_eq!(displaced_a1.displaced_leaves, vec![]);
+ assert_eq!(displaced_a1.displaced_blocks, vec![]);
+
+ let displaced_a2 =
+ blockchain.displaced_leaves_after_finalizing(a2_hash, a3_number).unwrap();
+ assert_eq!(displaced_a2.displaced_leaves, vec![]);
+ assert_eq!(displaced_a2.displaced_blocks, vec![]);
+
+ let displaced_a3 =
+ blockchain.displaced_leaves_after_finalizing(a3_hash, a3_number).unwrap();
+ assert_eq!(displaced_a3.displaced_leaves, vec![]);
+ assert_eq!(displaced_a3.displaced_blocks, vec![]);
}
- {
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a3, d2, c2])
- .unwrap()
- .unwrap();
-
- assert_eq!(lca.hash, block0);
- assert_eq!(lca.number, 0);
- }
+ // fork from genesis: 2 prong.
+ let b1_number = 1;
+ let b1_hash = insert_header(&backend, b1_number, genesis_hash, None, H256::from([1; 32]));
+ let b2_number = 2;
+ let b2_hash = insert_header(&backend, b2_number, b1_hash, None, Default::default());
- {
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![c2, d2, b2])
- .unwrap()
- .unwrap();
+ // fork from b2.
+ let c1_number = 3;
+ let c1_hash = insert_header(&backend, c1_number, b2_hash, None, H256::from([2; 32]));
+ let c2_number = 4;
+ let c2_hash = insert_header(&backend, c2_number, c1_hash, None, Default::default());
- assert_eq!(lca.hash, b1);
- assert_eq!(lca.number, 1);
- }
+ // fork from b1.
+ let d1_number = 2;
+ let d1_hash = insert_header(&backend, d1_number, b1_hash, None, H256::from([3; 32]));
+ let d2_number = 3;
+ let d2_hash = insert_header(&backend, d2_number, d1_hash, None, Default::default());
{
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a1, a2, a3])
- .unwrap()
- .unwrap();
-
- assert_eq!(lca.hash, a1);
- assert_eq!(lca.number, 1);
+ let displaced_a1 =
+ blockchain.displaced_leaves_after_finalizing(a1_hash, a1_number).unwrap();
+ assert_eq!(
+ displaced_a1.displaced_leaves,
+ vec![(c2_number, c2_hash), (d2_number, d2_hash)]
+ );
+ let mut displaced_blocks = vec![b1_hash, b2_hash, c1_hash, c2_hash, d1_hash, d2_hash];
+ displaced_blocks.sort();
+ assert_eq!(displaced_a1.displaced_blocks, displaced_blocks);
+
+ let displaced_a2 =
+ blockchain.displaced_leaves_after_finalizing(a2_hash, a2_number).unwrap();
+ assert_eq!(displaced_a1.displaced_leaves, displaced_a2.displaced_leaves);
+ assert_eq!(displaced_a1.displaced_blocks, displaced_a2.displaced_blocks);
+
+ let displaced_a3 =
+ blockchain.displaced_leaves_after_finalizing(a3_hash, a3_number).unwrap();
+ assert_eq!(displaced_a1.displaced_leaves, displaced_a3.displaced_leaves);
+ assert_eq!(displaced_a1.displaced_blocks, displaced_a3.displaced_blocks);
}
-
{
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![b1, b2, d1])
- .unwrap()
- .unwrap();
-
- assert_eq!(lca.hash, b1);
- assert_eq!(lca.number, 1);
+ let displaced =
+ blockchain.displaced_leaves_after_finalizing(b1_hash, b1_number).unwrap();
+ assert_eq!(displaced.displaced_leaves, vec![(a3_number, a3_hash)]);
+ let mut displaced_blocks = vec![a1_hash, a2_hash, a3_hash];
+ displaced_blocks.sort();
+ assert_eq!(displaced.displaced_blocks, displaced_blocks);
}
-
{
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![]);
-
- assert_eq!(true, matches!(lca, Ok(None)));
+ let displaced =
+ blockchain.displaced_leaves_after_finalizing(b2_hash, b2_number).unwrap();
+ assert_eq!(
+ displaced.displaced_leaves,
+ vec![(a3_number, a3_hash), (d2_number, d2_hash)]
+ );
+ let mut displaced_blocks = vec![a1_hash, a2_hash, a3_hash, d1_hash, d2_hash];
+ displaced_blocks.sort();
+ assert_eq!(displaced.displaced_blocks, displaced_blocks);
}
-
{
- let lca = lowest_common_ancestor_multiblock(blockchain, vec![a1]).unwrap().unwrap();
-
- assert_eq!(lca.hash, a1);
- assert_eq!(lca.number, 1);
+ let displaced =
+ blockchain.displaced_leaves_after_finalizing(c2_hash, c2_number).unwrap();
+ assert_eq!(
+ displaced.displaced_leaves,
+ vec![(a3_number, a3_hash), (d2_number, d2_hash)]
+ );
+ let mut displaced_blocks = vec![a1_hash, a2_hash, a3_hash, d1_hash, d2_hash];
+ displaced_blocks.sort();
+ assert_eq!(displaced.displaced_blocks, displaced_blocks);
}
}
diff --git a/substrate/primitives/blockchain/Cargo.toml b/substrate/primitives/blockchain/Cargo.toml
index 67126d4d19ebdb267ad1da7cd1eb59a41fca347c..aedd720612c33d35722eca2f3815f562e15f23c0 100644
--- a/substrate/primitives/blockchain/Cargo.toml
+++ b/substrate/primitives/blockchain/Cargo.toml
@@ -24,6 +24,7 @@ parking_lot = { workspace = true, default-features = true }
schnellru = { workspace = true }
thiserror = { workspace = true }
sp-api = { workspace = true, default-features = true }
+sp-core = { workspace = true, default-features = true }
sp-consensus = { workspace = true, default-features = true }
sp-database = { workspace = true, default-features = true }
sp-runtime = { workspace = true, default-features = true }
diff --git a/substrate/primitives/blockchain/src/backend.rs b/substrate/primitives/blockchain/src/backend.rs
index 76393420da740b269a3790f7e69ed12ddf77b21a..a928217d5885454b704c527354cabb5ba49af923 100644
--- a/substrate/primitives/blockchain/src/backend.rs
+++ b/substrate/primitives/blockchain/src/backend.rs
@@ -21,15 +21,15 @@ use log::warn;
use parking_lot::RwLock;
use sp_runtime::{
generic::BlockId,
- traits::{Block as BlockT, CheckedSub, Header as HeaderT, NumberFor, Zero},
+ traits::{Block as BlockT, Header as HeaderT, NumberFor, Zero},
Justifications,
};
-use std::collections::{btree_map::BTreeMap, btree_set::BTreeSet};
+use std::collections::{btree_set::BTreeSet, HashMap, VecDeque};
use crate::{
error::{Error, Result},
- header_metadata::{self, HeaderMetadata},
- lowest_common_ancestor_multiblock, tree_route, TreeRoute,
+ header_metadata::HeaderMetadata,
+ tree_route, CachedHeaderMetadata,
};
/// Blockchain database header backend. Does not perform any validation.
@@ -128,6 +128,32 @@ where
{
}
+struct MinimalBlockMetadata<Block: BlockT> {
+ number: NumberFor<Block>,
+ hash: Block::Hash,
+ parent: Block::Hash,
+}
+
+impl<Block> Clone for MinimalBlockMetadata<Block>
+where
+ Block: BlockT,
+{
+ fn clone(&self) -> Self {
+ Self { number: self.number, hash: self.hash, parent: self.parent }
+ }
+}
+
+impl<Block> Copy for MinimalBlockMetadata<Block> where Block: BlockT {}
+
+impl<Block> From<&CachedHeaderMetadata<Block>> for MinimalBlockMetadata<Block>
+where
+ Block: BlockT,
+{
+ fn from(value: &CachedHeaderMetadata<Block>) -> Self {
+ Self { number: value.number, hash: value.hash, parent: value.parent }
+ }
+}
+
/// Blockchain database backend. Does not perform any validation.
pub trait Backend<Block: BlockT>:
HeaderBackend<Block> + HeaderMetadata<Block, Error = Error>
@@ -226,88 +252,128 @@ pub trait Backend<Block: BlockT>:
finalized_block_hash: Block::Hash,
finalized_block_number: NumberFor<Block>,
) -> std::result::Result<DisplacedLeavesAfterFinalization<Block>, Error> {
- let mut result = DisplacedLeavesAfterFinalization::default();
-
let leaves = self.leaves()?;
// If we have only one leaf there are no forks, and we can return early.
if finalized_block_number == Zero::zero() || leaves.len() == 1 {
- return Ok(result)
+ return Ok(DisplacedLeavesAfterFinalization::default())
}
- let first_leaf = leaves.first().ok_or(Error::Backend(
- "Unable to find any leaves. This should not happen.".to_string(),
- ))?;
- let leaf_block_header = self.expect_header(*first_leaf)?;
-
- // If the distance between the leafs and the finalized block is large, calculating
- // tree routes can be very expensive. In that case, we will try to find the
- // lowest common ancestor between all the leaves. The assumption here is that the forks are
- // close to the tip and not long. So the LCA can be computed from the header cache. If the
- // LCA is above the finalized block, we know that there are no displaced leaves by the
- // finalization.
- if leaf_block_header
- .number()
- .checked_sub(&finalized_block_number)
- .unwrap_or(0u32.into()) >
- header_metadata::LRU_CACHE_SIZE.into()
- {
- if let Some(lca) = lowest_common_ancestor_multiblock(self, leaves.clone())? {
- if lca.number > finalized_block_number {
- return Ok(result)
- } else {
- log::warn!("The distance between leafs and finalized block is large. Finalization can take a long time.");
- }
- };
- }
+ // Store hashes of finalized blocks for quick checking later, the last block if the
+ // finalized one
+ let mut finalized_chain = VecDeque::new();
+ finalized_chain
+ .push_front(MinimalBlockMetadata::from(&self.header_metadata(finalized_block_hash)?));
+
+ // Local cache is a performance optimization in case of finalized block deep below the
+ // tip of the chain with a lot of leaves above finalized block
+ let mut local_cache = HashMap::<Block::Hash, MinimalBlockMetadata<Block>>::new();
+
+ let mut result = DisplacedLeavesAfterFinalization {
+ displaced_leaves: Vec::with_capacity(leaves.len()),
+ displaced_blocks: Vec::with_capacity(leaves.len()),
+ };
+ let mut displaced_blocks_candidates = Vec::new();
- // For each leaf determine whether it belongs to a non-canonical branch.
for leaf_hash in leaves {
- let leaf_block_header = self.expect_header(leaf_hash)?;
- let leaf_number = *leaf_block_header.number();
+ let mut current_header_metadata =
+ MinimalBlockMetadata::from(&self.header_metadata(leaf_hash)?);
+ let leaf_number = current_header_metadata.number;
+
+ // Collect all block hashes until the height of the finalized block
+ displaced_blocks_candidates.clear();
+ while current_header_metadata.number > finalized_block_number {
+ displaced_blocks_candidates.push(current_header_metadata.hash);
+
+ let parent_hash = current_header_metadata.parent;
+ match local_cache.get(&parent_hash) {
+ Some(metadata_header) => {
+ current_header_metadata = *metadata_header;
+ },
+ None => {
+ current_header_metadata =
+ MinimalBlockMetadata::from(&self.header_metadata(parent_hash)?);
+ // Cache locally in case more branches above finalized block reference
+ // the same block hash
+ local_cache.insert(parent_hash, current_header_metadata);
+ },
+ }
+ }
+
+ // If points back to the finalized header then nothing left to do, this leaf will be
+ // checked again later
+ if current_header_metadata.hash == finalized_block_hash {
+ continue;
+ }
- let leaf_tree_route = match tree_route(self, leaf_hash, finalized_block_hash) {
- Ok(tree_route) => tree_route,
- Err(Error::UnknownBlock(_)) => {
- // Sometimes routes can't be calculated. E.g. after warp sync.
+ // Otherwise the whole leaf branch needs to be pruned, track it all the way to the
+ // point of branching from the finalized chain
+ result.displaced_leaves.push((leaf_number, leaf_hash));
+ result.displaced_blocks.extend(displaced_blocks_candidates.drain(..));
+ result.displaced_blocks.push(current_header_metadata.hash);
+ // Collect the rest of the displaced blocks of leaf branch
+ for distance_from_finalized in 1_u32.. {
+ // Find block at `distance_from_finalized` from finalized block
+ let (finalized_chain_block_number, finalized_chain_block_hash) =
+ match finalized_chain.iter().rev().nth(distance_from_finalized as usize) {
+ Some(header) => (header.number, header.hash),
+ None => {
+ let metadata = MinimalBlockMetadata::from(&self.header_metadata(
+ finalized_chain.front().expect("Not empty; qed").parent,
+ )?);
+ let result = (metadata.number, metadata.hash);
+ finalized_chain.push_front(metadata);
+ result
+ },
+ };
+
+ if current_header_metadata.number <= finalized_chain_block_number {
+ // Skip more blocks until we get all blocks on finalized chain until the height
+ // of the parent block
continue;
- },
- Err(e) => Err(e)?,
- };
+ }
- // Is it a stale fork?
- let needs_pruning = leaf_tree_route.common_block().hash != finalized_block_hash;
+ let parent_hash = current_header_metadata.parent;
+ if finalized_chain_block_hash == parent_hash {
+ // Reached finalized chain, nothing left to do
+ break;
+ }
- if needs_pruning {
- result.displaced_leaves.insert(leaf_hash, leaf_number);
- result.tree_routes.insert(leaf_hash, leaf_tree_route);
+ // Store displaced block and look deeper for block on finalized chain
+ result.displaced_blocks.push(parent_hash);
+ current_header_metadata =
+ MinimalBlockMetadata::from(&self.header_metadata(parent_hash)?);
}
}
- Ok(result)
+ // There could be duplicates shared by multiple branches, clean them up
+ result.displaced_blocks.sort_unstable();
+ result.displaced_blocks.dedup();
+
+ return Ok(result);
}
}
/// Result of [`Backend::displaced_leaves_after_finalizing`].
#[derive(Clone, Debug)]
pub struct DisplacedLeavesAfterFinalization<Block: BlockT> {
- /// A collection of hashes and block numbers for displaced leaves.
- pub displaced_leaves: BTreeMap<Block::Hash, NumberFor<Block>>,
+ /// A list of hashes and block numbers of displaced leaves.
+ pub displaced_leaves: Vec<(NumberFor<Block>, Block::Hash)>,
- /// A collection of tree routes from the leaves to finalized block.
- pub tree_routes: BTreeMap<Block::Hash, TreeRoute<Block>>,
+ /// A list of hashes displaced blocks from all displaced leaves.
+ pub displaced_blocks: Vec<Block::Hash>,
}
impl<Block: BlockT> Default for DisplacedLeavesAfterFinalization<Block> {
fn default() -> Self {
- Self { displaced_leaves: Default::default(), tree_routes: Default::default() }
+ Self { displaced_leaves: Vec::new(), displaced_blocks: Vec::new() }
}
}
impl<Block: BlockT> DisplacedLeavesAfterFinalization<Block> {
/// Returns a collection of hashes for the displaced leaves.
pub fn hashes(&self) -> impl Iterator<Item = Block::Hash> + '_ {
- self.displaced_leaves.keys().cloned()
+ self.displaced_leaves.iter().map(|(_, hash)| *hash)
}
}
diff --git a/substrate/primitives/blockchain/src/header_metadata.rs b/substrate/primitives/blockchain/src/header_metadata.rs
index c2054445b06765ad42b4d5a0718997fc9686cc14..30024765add3ced4b5295a023cb0980aae3d59fd 100644
--- a/substrate/primitives/blockchain/src/header_metadata.rs
+++ b/substrate/primitives/blockchain/src/header_metadata.rs
@@ -20,12 +20,16 @@
use parking_lot::RwLock;
use schnellru::{ByLength, LruMap};
-use sp_runtime::traits::{Block as BlockT, Header, NumberFor, One};
+use sp_core::U256;
+use sp_runtime::{
+ traits::{Block as BlockT, Header, NumberFor, One},
+ Saturating,
+};
/// Set to the expected max difference between `best` and `finalized` blocks at sync.
pub(crate) const LRU_CACHE_SIZE: u32 = 5_000;
-/// Get lowest common ancestor between two blocks in the tree.
+/// Get the lowest common ancestor between two blocks in the tree.
///
/// This implementation is efficient because our trees have very few and
/// small branches, and because of our current query pattern:
@@ -96,30 +100,6 @@ pub fn lowest_common_ancestor<Block: BlockT, T: HeaderMetadata<Block> + ?Sized>(
Ok(HashAndNumber { hash: header_one.hash, number: header_one.number })
}
-/// Get lowest common ancestor between multiple blocks.
-pub fn lowest_common_ancestor_multiblock<Block: BlockT, T: HeaderMetadata<Block> + ?Sized>(
- backend: &T,
- hashes: Vec<Block::Hash>,
-) -> Result<Option<HashAndNumber<Block>>, T::Error> {
- // Ensure the list of hashes is not empty
- let mut hashes_iter = hashes.into_iter();
-
- let first_hash = match hashes_iter.next() {
- Some(hash) => hash,
- None => return Ok(None),
- };
-
- // Start with the first hash as the initial LCA
- let first_cached = backend.header_metadata(first_hash)?;
- let mut lca = HashAndNumber { number: first_cached.number, hash: first_cached.hash };
- for hash in hashes_iter {
- // Calculate the LCA of the current LCA and the next hash
- lca = lowest_common_ancestor(backend, lca.hash, hash)?;
- }
-
- Ok(Some(lca))
-}
-
/// Compute a tree-route between two blocks. See tree-route docs for more details.
pub fn tree_route<Block: BlockT, T: HeaderMetadata<Block> + ?Sized>(
backend: &T,
@@ -129,15 +109,16 @@ pub fn tree_route<Block: BlockT, T: HeaderMetadata<Block> + ?Sized>(
let mut from = backend.header_metadata(from)?;
let mut to = backend.header_metadata(to)?;
- let mut from_branch = Vec::new();
- let mut to_branch = Vec::new();
-
+ let mut to_branch =
+ Vec::with_capacity(Into::<U256>::into(to.number.saturating_sub(from.number)).as_usize());
while to.number > from.number {
to_branch.push(HashAndNumber { number: to.number, hash: to.hash });
to = backend.header_metadata(to.parent)?;
}
+ let mut from_branch =
+ Vec::with_capacity(Into::<U256>::into(to.number.saturating_sub(from.number)).as_usize());
while from.number > to.number {
from_branch.push(HashAndNumber { number: from.number, hash: from.hash });
from = backend.header_metadata(from.parent)?;
@@ -156,6 +137,7 @@ pub fn tree_route<Block: BlockT, T: HeaderMetadata<Block> + ?Sized>(
// add the pivot block. and append the reversed to-branch
// (note that it's reverse order originals)
let pivot = from_branch.len();
+ from_branch.reserve_exact(to_branch.len() + 1);
from_branch.push(HashAndNumber { number: to.number, hash: to.hash });
from_branch.extend(to_branch.into_iter().rev());
@@ -173,7 +155,7 @@ pub struct HashAndNumber<Block: BlockT> {
/// A tree-route from one block to another in the chain.
///
-/// All blocks prior to the pivot in the deque is the reverse-order unique ancestry
+/// All blocks prior to the pivot in the vector is the reverse-order unique ancestry
/// of the first block, the block at the pivot index is the common ancestor,
/// and all blocks after the pivot is the ancestry of the second block, in
/// order.