Replace `Cache::block_disconnected` with `blocks_disconnected`
What changed, and why it matters
This commit is a follow-up code cleanup in rust-lightning's block synchronization module. It changes how the code handles blockchain reorganizations (when the chain temporarily forks and then switches to a different branch). Previously, the code tracked and notified about each individual block that was disconnected; now it only passes the fork point, matching an earlier change made to the main listener interface. The commit removes an internal list of disconnected blocks and updates the cache API accordingly. There is no direct evidence in the commit that this fixes a security vulnerability.
Treat as a normal refactoring/correctness commit. Reviewers should verify that the new fork-point-only semantics correctly invalidate cached headers during reorgs and that `BestBlock` is constructed with the correct height and hash. No immediate security response is indicated by the commit itself.
Security signals we found
API semantic alignment after prior listener disconnect change
Removal of per-block disconnect tracking in favor of fork-point notification
No explicit security bug, CVE, or vulnerability description in commit
Potential for subtle correctness issues around reorg handling if fork point height/hash mismatched
Evidence from the diff
The commit refactors lightning-block-sync to align Cache disconnect semantics with the Listen trait change from commit 403dc1a48bb71ae794f6883ae0b760aad44cda39. It replaces Cache::block_disconnected(&BlockHash) -> Option<ValidatedBlockHeader> with Cache::blocks_disconnected(&ValidatedBlockHeader), removes ChainDifference::disconnected_blocks, and updates callers to pass only the common ancestor/fork point. The UnboundedCache now uses retain to drop headers above the fork point height. The ChainNotifier::disconnect_blocks method now directly notifies listeners with BestBlock::new(fork_point.block_hash, fork_point.height). The change is semantic and API-shaping, not a targeted security patch.
Changed components
lightning-block-sync/src/lib.rslightning-block-sync/src/init.rsCache traitUnboundedCacheChainNotifierChainDifference structInspect captured patch +19 / −26
diff --git a/lightning-block-sync/src/init.rs b/lightning-block-sync/src/init.rs
index 61f44c6..07575c6 100644
--- a/lightning-block-sync/src/init.rs
+++ b/lightning-block-sync/src/init.rs
@@ -175,7 +175,9 @@ where
let mut chain_notifier = ChainNotifier { header_cache, chain_listener };
let difference =
chain_notifier.find_difference(best_header, &old_header, &mut chain_poller).await?;
- chain_notifier.disconnect_blocks(difference.disconnected_blocks);
+ if difference.common_ancestor != old_header {
+ chain_notifier.disconnect_blocks(difference.common_ancestor);
+ }
(difference.common_ancestor, difference.connected_blocks)
};
@@ -215,9 +217,7 @@ impl<'a, C: Cache> Cache for ReadOnlyCache<'a, C> {
unreachable!()
}
- fn block_disconnected(&mut self, _block_hash: &BlockHash) -> Option<ValidatedBlockHeader> {
- None
- }
+ fn blocks_disconnected(&mut self, _fork_point: &ValidatedBlockHeader) {}
}
/// Wrapper for supporting dynamically sized chain listeners.
diff --git a/lightning-block-sync/src/lib.rs b/lightning-block-sync/src/lib.rs
index 0259304..3b9b137 100644
--- a/lightning-block-sync/src/lib.rs
+++ b/lightning-block-sync/src/lib.rs
@@ -202,9 +202,11 @@ pub trait Cache {
/// disconnected later if needed.
fn block_connected(&mut self, block_hash: BlockHash, block_header: ValidatedBlockHeader);
- /// Called when a block has been disconnected from the best chain. Once disconnected, a block's
- /// header is no longer needed and thus can be removed.
- fn block_disconnected(&mut self, block_hash: &BlockHash) -> Option<ValidatedBlockHeader>;
+ /// Called when blocks have been disconnected from the best chain. Only the fork point
+ /// (best common ancestor) is provided.
+ ///
+ /// Once disconnected, a block's header is no longer needed and thus can be removed.
+ fn blocks_disconnected(&mut self, fork_point: &ValidatedBlockHeader);
}
/// Unbounded cache of block headers keyed by block hash.
@@ -219,8 +221,8 @@ impl Cache for UnboundedCache {
self.insert(block_hash, block_header);
}
- fn block_disconnected(&mut self, block_hash: &BlockHash) -> Option<ValidatedBlockHeader> {
- self.remove(block_hash)
+ fn blocks_disconnected(&mut self, fork_point: &ValidatedBlockHeader) {
+ self.retain(|_, block_info| block_info.height < fork_point.height);
}
}
@@ -315,9 +317,6 @@ struct ChainDifference {
/// If there are any disconnected blocks, this is where the chain forked.
common_ancestor: ValidatedBlockHeader,
- /// Blocks that were disconnected from the chain since the last poll.
- disconnected_blocks: Vec<ValidatedBlockHeader>,
-
/// Blocks that were connected to the chain since the last poll.
connected_blocks: Vec<ValidatedBlockHeader>,
}
@@ -341,7 +340,9 @@ where
.find_difference(new_header, old_header, chain_poller)
.await
.map_err(|e| (e, None))?;
- self.disconnect_blocks(difference.disconnected_blocks);
+ if difference.common_ancestor != *old_header {
+ self.disconnect_blocks(difference.common_ancestor);
+ }
self.connect_blocks(difference.common_ancestor, difference.connected_blocks, chain_poller)
.await
}
@@ -354,7 +355,6 @@ where
&self, current_header: ValidatedBlockHeader, prev_header: &ValidatedBlockHeader,
chain_poller: &mut P,
) -> BlockSourceResult<ChainDifference> {
- let mut disconnected_blocks = Vec::new();
let mut connected_blocks = Vec::new();
let mut current = current_header;
let mut previous = *prev_header;
@@ -369,7 +369,6 @@ where
let current_height = current.height;
let previous_height = previous.height;
if current_height <= previous_height {
- disconnected_blocks.push(previous);
previous = self.look_up_previous_header(chain_poller, &previous).await?;
}
if current_height >= previous_height {
@@ -379,7 +378,7 @@ where
}
let common_ancestor = current;
- Ok(ChainDifference { common_ancestor, disconnected_blocks, connected_blocks })
+ Ok(ChainDifference { common_ancestor, connected_blocks })
}
/// Returns the previous header for the given header, either by looking it up in the cache or
@@ -394,16 +393,10 @@ where
}
/// Notifies the chain listeners of disconnected blocks.
- fn disconnect_blocks(&mut self, disconnected_blocks: Vec<ValidatedBlockHeader>) {
- for header in disconnected_blocks.iter() {
- if let Some(cached_header) = self.header_cache.block_disconnected(&header.block_hash) {
- assert_eq!(cached_header, *header);
- }
- }
- if let Some(block) = disconnected_blocks.last() {
- let fork_point = BestBlock::new(block.header.prev_blockhash, block.height - 1);
- self.chain_listener.blocks_disconnected(fork_point);
- }
+ fn disconnect_blocks(&mut self, fork_point: ValidatedBlockHeader) {
+ self.header_cache.blocks_disconnected(&fork_point);
+ let best_block = BestBlock::new(fork_point.block_hash, fork_point.height);
+ self.chain_listener.blocks_disconnected(best_block);
}
/// Notifies the chain listeners of connected blocks.
Why this scored 21/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.