← Watch feed
Allow filtered block rescans at the current tip
Public commit record
88/100 · Strong
What the developer wrote
Authored by Elias Rohrer
Allow filtered block rescans at the current tip
A valid same-block replay through Listen could panic OutputSweeper and
ChannelManager because both required every filtered_block_connected call
to advance the chain.
Treat callbacks matching the current tip as rescans, process their
additional transaction data, and skip the duplicate best-block update
while retaining chain-order checks for new blocks.
Fixes #3920
Reported by Project Loupe
Co-Authored-By: HAL 9000
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit fixes a bug where replaying the current blockchain block through a normal listener callback could crash two core Lightning components (ChannelManager and OutputSweeper) with a panic. The fix recognizes a same-block replay as a 'rescan,' processes any extra transaction data it carries, and avoids updating the already-current best block. It is a denial-of-service/crash bug triggered by valid, chain-ordered behavior from upstream block sources, not by malformed data.
Recommended action
Apply the patch. If running a node built from an affected commit, ensure the block source cannot replay the current tip, or upgrade promptly. Monitor for any upstream advisories from Lightning Dev Kit.
Security signals we found
01
panic in chain listener callback
02
same-block replay/rescan mishandling
03
assertion failure on valid chain input
04
denial-of-service via crash of ChannelManager/OutputSweeper
05
issue reported by external party (Project Loupe)
Evidence from the diff
The filtered_block_connected implementation in ChannelManager and OutputSweeper asserted that every connected header must extend the previous best block by exactly one height. A same-block replay (same block_hash and height as the current tip) therefore failed those assertions and panicked. The patch introduces an is_rescan check: when the incoming header matches the current tip, it skips the chain-order assertions, still calls the transaction-confirmed handler so new txdata is processed, and skips the best-block update. Tests are added for both components.
Changed components
lightning/src/ln/channelmanager.rslightning/src/util/sweep.rschain::Listen::filtered_block_connectedInspect captured patch +118 / −12
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 08a2cb7..e293136 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -16526,16 +16526,23 @@ impl<
> chain::Listen for ChannelManager<M, T, ES, NS, SP, F, R, MR, L>
{
fn filtered_block_connected(&self, header: &Header, txdata: &TransactionData, height: u32) {
- {
+ let is_rescan = {
let best_block = self.best_block.read().unwrap();
- assert_eq!(best_block.block_hash, header.prev_blockhash,
- "Blocks must be connected in chain-order - the connected header must build on the last connected header");
- assert_eq!(best_block.height, height - 1,
- "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height");
- }
+ let is_rescan =
+ best_block.block_hash == header.block_hash() && best_block.height == height;
+ if !is_rescan {
+ assert_eq!(best_block.block_hash, header.prev_blockhash,
+ "Blocks must be connected in chain-order - the connected header must build on the last connected header");
+ assert_eq!(best_block.height, height - 1,
+ "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height");
+ }
+ is_rescan
+ };
self.transactions_confirmed(header, txdata, height);
- self.best_block_updated(header, height);
+ if !is_rescan {
+ self.best_block_updated(header, height);
+ }
}
fn blocks_disconnected(&self, fork_point: BlockLocator) {
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index fdf092d..1f5bd39 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -3005,6 +3005,28 @@ pub fn test_drop_messages_peer_disconnect_b() {
do_test_drop_messages_peer_disconnect(6, false);
}
+#[xtest(feature = "_externalize_tests")]
+pub fn test_filtered_block_connected_allows_same_block_rescan() {
+ let chanmon_cfgs = create_chanmon_cfgs(1);
+ let node_cfgs = create_node_cfgs(1, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(1, &node_cfgs, &[None]);
+ let nodes = create_network(1, &node_cfgs, &node_chanmgrs);
+
+ let best_block = nodes[0].node.current_best_block();
+ let height = best_block.height + 1;
+ let header = create_dummy_header(best_block.block_hash, height);
+ nodes[0].node.filtered_block_connected(&header, &[], height);
+ nodes[0].node.filtered_block_connected(&header, &[], height);
+
+ let current_best_block = nodes[0].node.current_best_block();
+ assert_eq!(current_best_block.block_hash, header.block_hash());
+ assert_eq!(current_best_block.height, height);
+ assert_eq!(
+ current_best_block.get_hash_at_height(best_block.height),
+ Some(best_block.block_hash)
+ );
+}
+
#[xtest(feature = "_externalize_tests")]
pub fn test_channel_ready_without_best_block_updated() {
// Previously, if we were offline when a funding transaction was locked in, and then we came
diff --git a/lightning/src/util/sweep.rs b/lightning/src/util/sweep.rs
index 883cc4a..e1f562f 100644
--- a/lightning/src/util/sweep.rs
+++ b/lightning/src/util/sweep.rs
@@ -763,13 +763,19 @@ where
&self, header: &Header, txdata: &chain::transaction::TransactionData, height: u32,
) {
let mut state_lock = self.sweeper_state.lock().unwrap();
- assert_eq!(state_lock.best_block.block_hash, header.prev_blockhash,
- "Blocks must be connected in chain-order - the connected header must build on the last connected header");
- assert_eq!(state_lock.best_block.height, height - 1,
- "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height");
+ let is_rescan = state_lock.best_block.block_hash == header.block_hash()
+ && state_lock.best_block.height == height;
+ if !is_rescan {
+ assert_eq!(state_lock.best_block.block_hash, header.prev_blockhash,
+ "Blocks must be connected in chain-order - the connected header must build on the last connected header");
+ assert_eq!(state_lock.best_block.height, height - 1,
+ "Blocks must be connected in chain-order - the connected block height must be one greater than the previous height");
+ }
self.transactions_confirmed_internal(&mut state_lock, header, txdata, height);
- self.best_block_updated_internal(&mut state_lock, header, height);
+ if !is_rescan {
+ self.best_block_updated_internal(&mut state_lock, header, height);
+ }
}
fn blocks_disconnected(&self, fork_point: BlockLocator) {
@@ -1329,4 +1335,75 @@ mod tests {
"pending_sweep flag was not reset when the future was dropped",
);
}
+
+ #[test]
+ fn filtered_block_connected_allows_same_block_rescan() {
+ let best_block = BlockLocator::new(BlockHash::all_zeros(), 0);
+ let sweeper: OutputSweeper<
+ DummyBroadcaster,
+ Box<DummyChangeDestSource>,
+ DummyFeeEstimator,
+ DummyFilter,
+ PendingKVStore,
+ DummyLogger,
+ DummyOutputSpender,
+ > = OutputSweeper::new(
+ best_block.clone(),
+ DummyBroadcaster,
+ DummyFeeEstimator,
+ None,
+ DummyOutputSpender,
+ Box::new(DummyChangeDestSource),
+ PendingKVStore,
+ DummyLogger,
+ );
+
+ let header = Header {
+ version: bitcoin::block::Version::NO_SOFT_FORK_SIGNALLING,
+ prev_blockhash: best_block.block_hash,
+ merkle_root: bitcoin::hash_types::TxMerkleNode::all_zeros(),
+ time: 1,
+ bits: bitcoin::pow::CompactTarget::from_consensus(42),
+ nonce: 42,
+ };
+ let tracked_outpoint = bitcoin::OutPoint { txid: Txid::all_zeros(), vout: 0 };
+ let spending_tx = Transaction {
+ version: Version::TWO,
+ lock_time: LockTime::ZERO,
+ input: vec![bitcoin::TxIn { previous_output: tracked_outpoint, ..Default::default() }],
+ output: Vec::new(),
+ };
+ let descriptor = SpendableOutputDescriptor::StaticOutput {
+ outpoint: OutPoint { txid: tracked_outpoint.txid, index: tracked_outpoint.vout as u16 },
+ output: TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() },
+ channel_keys_id: None,
+ };
+ sweeper.sweeper_state.lock().unwrap().outputs.push(TrackedSpendableOutput {
+ descriptor,
+ channel_id: None,
+ counterparty_node_id: None,
+ status: OutputSpendStatus::PendingFirstConfirmation {
+ first_broadcast_hash: best_block.block_hash,
+ latest_broadcast_height: best_block.height,
+ latest_spending_tx: spending_tx.clone(),
+ },
+ });
+
+ sweeper.filtered_block_connected(&header, &[], 1);
+ let txdata = [(0, &spending_tx)];
+ sweeper.filtered_block_connected(&header, &txdata, 1);
+
+ let current_best_block = sweeper.current_best_block();
+ assert_eq!(current_best_block.block_hash, header.block_hash());
+ assert_eq!(current_best_block.height, 1);
+ assert_eq!(current_best_block.get_hash_at_height(0), Some(best_block.block_hash));
+ assert!(matches!(
+ sweeper.tracked_spendable_outputs()[0].status,
+ OutputSpendStatus::PendingThresholdConfirmations {
+ confirmation_height: 1,
+ confirmation_hash,
+ ..
+ } if confirmation_hash == header.block_hash()
+ ));
+ }
}
Risk score
Our methodology →Why this scored 60/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.