Merge pull request #3876 from TheBlueMatt/2025-06-disconnect-less-info

Drop the need for fork headers when calling Listen's disconnect

Author: jkczyz

fuzz/src/full_stack.rs

@@ -343,8 +343,9 @@ impl<'a> MoneyLossDetector<'a> {
 				self.header_hashes[self.height - 1].0,
 				self.header_hashes[self.height].1,
 			);
-			self.manager.block_disconnected(&header, self.height as u32);
-			self.monitor.block_disconnected(&header, self.height as u32);
+			let best_block = BestBlock::new(header.prev_blockhash, self.height as u32 - 1);
+			self.manager.blocks_disconnected(best_block);
+			self.monitor.blocks_disconnected(best_block);
 			self.height -= 1;
 			let removal_height = self.height;
 			self.txids_confirmed.retain(|_, height| removal_height != *height);

lightning-block-sync/src/init.rs

@@ -9,6 +9,7 @@ use bitcoin::hash_types::BlockHash;
 use bitcoin::network::Network;
 
 use lightning::chain;
+use lightning::chain::BestBlock;
 
 use std::ops::Deref;
 
@@ -230,8 +231,8 @@ impl<'a, L: chain::Listen + ?Sized> chain::Listen for DynamicChainListener<'a, L
 		unreachable!()
 	}
 
-	fn block_disconnected(&self, header: &Header, height: u32) {
-		self.0.block_disconnected(header, height)
+	fn blocks_disconnected(&self, fork_point: BestBlock) {
+		self.0.blocks_disconnected(fork_point)
 	}
 }
 
@@ -257,7 +258,7 @@ impl<'a, L: chain::Listen + ?Sized> chain::Listen for ChainListenerSet<'a, L> {
 		}
 	}
 
-	fn block_disconnected(&self, _header: &Header, _height: u32) {
+	fn blocks_disconnected(&self, _fork_point: BestBlock) {
 		unreachable!()
 	}
 }
@@ -300,19 +301,16 @@ mod tests {
 		let fork_chain_3 = main_chain.fork_at_height(3);
 
 		let listener_1 = MockChainListener::new()
-			.expect_block_disconnected(*fork_chain_1.at_height(4))
-			.expect_block_disconnected(*fork_chain_1.at_height(3))
-			.expect_block_disconnected(*fork_chain_1.at_height(2))
+			.expect_blocks_disconnected(*fork_chain_1.at_height(1))
 			.expect_block_connected(*main_chain.at_height(2))
 			.expect_block_connected(*main_chain.at_height(3))
 			.expect_block_connected(*main_chain.at_height(4));
 		let listener_2 = MockChainListener::new()
-			.expect_block_disconnected(*fork_chain_2.at_height(4))
-			.expect_block_disconnected(*fork_chain_2.at_height(3))
+			.expect_blocks_disconnected(*fork_chain_2.at_height(2))
 			.expect_block_connected(*main_chain.at_height(3))
 			.expect_block_connected(*main_chain.at_height(4));
 		let listener_3 = MockChainListener::new()
-			.expect_block_disconnected(*fork_chain_3.at_height(4))
+			.expect_blocks_disconnected(*fork_chain_3.at_height(3))
 			.expect_block_connected(*main_chain.at_height(4));
 
 		let listeners = vec![
@@ -337,23 +335,17 @@ mod tests {
 		let fork_chain_3 = fork_chain_2.fork_at_height(3);
 
 		let listener_1 = MockChainListener::new()
-			.expect_block_disconnected(*fork_chain_1.at_height(4))
-			.expect_block_disconnected(*fork_chain_1.at_height(3))
-			.expect_block_disconnected(*fork_chain_1.at_height(2))
+			.expect_blocks_disconnected(*fork_chain_1.at_height(1))
 			.expect_block_connected(*main_chain.at_height(2))
 			.expect_block_connected(*main_chain.at_height(3))
 			.expect_block_connected(*main_chain.at_height(4));
 		let listener_2 = MockChainListener::new()
-			.expect_block_disconnected(*fork_chain_2.at_height(4))
-			.expect_block_disconnected(*fork_chain_2.at_height(3))
-			.expect_block_disconnected(*fork_chain_2.at_height(2))
+			.expect_blocks_disconnected(*fork_chain_2.at_height(1))
 			.expect_block_connected(*main_chain.at_height(2))
 			.expect_block_connected(*main_chain.at_height(3))
 			.expect_block_connected(*main_chain.at_height(4));
 		let listener_3 = MockChainListener::new()
-			.expect_block_disconnected(*fork_chain_3.at_height(4))
-			.expect_block_disconnected(*fork_chain_3.at_height(3))
-			.expect_block_disconnected(*fork_chain_3.at_height(2))
+			.expect_blocks_disconnected(*fork_chain_3.at_height(1))
 			.expect_block_connected(*main_chain.at_height(2))
 			.expect_block_connected(*main_chain.at_height(3))
 			.expect_block_connected(*main_chain.at_height(4));
@@ -380,7 +372,7 @@ mod tests {
 		let old_tip = fork_chain.tip();
 
 		let listener = MockChainListener::new()
-			.expect_block_disconnected(*old_tip)
+			.expect_blocks_disconnected(*fork_chain.at_height(1))
 			.expect_block_connected(*new_tip);
 
 		let listeners = vec![(old_tip.block_hash, &listener as &dyn chain::Listen)];

lightning-block-sync/src/lib.rs

@@ -49,7 +49,7 @@ use bitcoin::hash_types::BlockHash;
 use bitcoin::pow::Work;
 
 use lightning::chain;
-use lightning::chain::Listen;
+use lightning::chain::{BestBlock, Listen};
 
 use std::future::Future;
 use std::ops::Deref;
@@ -398,12 +398,15 @@ where
 	}
 
 	/// Notifies the chain listeners of disconnected blocks.
-	fn disconnect_blocks(&mut self, mut disconnected_blocks: Vec<ValidatedBlockHeader>) {
-		for header in disconnected_blocks.drain(..) {
+	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);
+				assert_eq!(cached_header, *header);
 			}
-			self.chain_listener.block_disconnected(&header.header, header.height);
+		}
+		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);
 		}
 	}
 
@@ -615,7 +618,7 @@ mod chain_notifier_tests {
 		let new_tip = fork_chain.tip();
 		let old_tip = main_chain.tip();
 		let chain_listener = &MockChainListener::new()
-			.expect_block_disconnected(*old_tip)
+			.expect_blocks_disconnected(*fork_chain.at_height(1))
 			.expect_block_connected(*new_tip);
 		let mut notifier =
 			ChainNotifier { header_cache: &mut main_chain.header_cache(0..=2), chain_listener };
@@ -635,8 +638,7 @@ mod chain_notifier_tests {
 		let new_tip = fork_chain.tip();
 		let old_tip = main_chain.tip();
 		let chain_listener = &MockChainListener::new()
-			.expect_block_disconnected(*old_tip)
-			.expect_block_disconnected(*main_chain.at_height(2))
+			.expect_blocks_disconnected(*main_chain.at_height(1))
 			.expect_block_connected(*new_tip);
 		let mut notifier =
 			ChainNotifier { header_cache: &mut main_chain.header_cache(0..=3), chain_listener };
@@ -656,7 +658,7 @@ mod chain_notifier_tests {
 		let new_tip = fork_chain.tip();
 		let old_tip = main_chain.tip();
 		let chain_listener = &MockChainListener::new()
-			.expect_block_disconnected(*old_tip)
+			.expect_blocks_disconnected(*fork_chain.at_height(1))
 			.expect_block_connected(*fork_chain.at_height(2))
 			.expect_block_connected(*new_tip);
 		let mut notifier =

lightning-block-sync/src/test_utils.rs

@@ -13,6 +13,7 @@ use bitcoin::transaction;
 use bitcoin::Transaction;
 
 use lightning::chain;
+use lightning::chain::BestBlock;
 
 use std::cell::RefCell;
 use std::collections::VecDeque;
@@ -203,7 +204,7 @@ impl chain::Listen for NullChainListener {
 		&self, _header: &Header, _txdata: &chain::transaction::TransactionData, _height: u32,
 	) {
 	}
-	fn block_disconnected(&self, _header: &Header, _height: u32) {}
+	fn blocks_disconnected(&self, _fork_point: BestBlock) {}
 }
 
 pub struct MockChainListener {
@@ -231,8 +232,8 @@ impl MockChainListener {
 		self
 	}
 
-	pub fn expect_block_disconnected(self, block: BlockHeaderData) -> Self {
-		self.expected_blocks_disconnected.borrow_mut().push_back(block);
+	pub fn expect_blocks_disconnected(self, fork_point: BlockHeaderData) -> Self {
+		self.expected_blocks_disconnected.borrow_mut().push_back(fork_point);
 		self
 	}
 }
@@ -264,14 +265,17 @@ impl chain::Listen for MockChainListener {
 		}
 	}
 
-	fn block_disconnected(&self, header: &Header, height: u32) {
+	fn blocks_disconnected(&self, fork_point: BestBlock) {
 		match self.expected_blocks_disconnected.borrow_mut().pop_front() {
 			None => {
-				panic!("Unexpected block disconnected: {:?}", header.block_hash());
+				panic!(
+					"Unexpected block(s) disconnected {} at height {}",
+					fork_point.block_hash, fork_point.height,
+				);
 			},
-			Some(expected_block) => {
-				assert_eq!(header.block_hash(), expected_block.header.block_hash());
-				assert_eq!(height, expected_block.height);
+			Some(expected) => {
+				assert_eq!(fork_point.block_hash, expected.header.block_hash());
+				assert_eq!(fork_point.height, expected.height);
 			},
 		}
 	}

lightning-liquidity/src/manager.rs

@@ -790,14 +790,11 @@ where
 		self.best_block_updated(header, height);
 	}
 
-	fn block_disconnected(&self, header: &bitcoin::block::Header, height: u32) {
-		let new_height = height - 1;
+	fn blocks_disconnected(&self, fork_point: BestBlock) {
 		if let Some(best_block) = self.best_block.write().unwrap().as_mut() {
-			assert_eq!(best_block.block_hash, header.block_hash(),
-				"Blocks must be disconnected in chain-order - the disconnected header must be the last connected header");
-			assert_eq!(best_block.height, height,
-				"Blocks must be disconnected in chain-order - the disconnected block must have the correct height");
-			*best_block = BestBlock::new(header.prev_blockhash, new_height)
+			assert!(best_block.height > fork_point.height,
+				"Blocks disconnected must indicate disconnection from the current best height, i.e. the new chain tip must be lower than the previous best height");
+			*best_block = fork_point;
 		}
 
 		// TODO: Call block_disconnected on all sub-modules that require it, e.g., LSPS1MessageHandler.

lightning/src/chain/chainmonitor.rs

@@ -35,7 +35,7 @@ use crate::chain::channelmonitor::{
 	WithChannelMonitor,
 };
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::chain::{ChannelMonitorUpdateStatus, Filter, WatchedOutput};
+use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Filter, WatchedOutput};
 use crate::events::{self, Event, EventHandler, ReplayEvent};
 use crate::ln::channel_state::ChannelDetails;
 #[cfg(peer_storage)]
@@ -1055,18 +1055,17 @@ where
 		self.event_notifier.notify();
 	}
 
-	fn block_disconnected(&self, header: &Header, height: u32) {
+	fn blocks_disconnected(&self, fork_point: BestBlock) {
 		let monitor_states = self.monitors.read().unwrap();
 		log_debug!(
 			self.logger,
-			"Latest block {} at height {} removed via block_disconnected",
-			header.block_hash(),
-			height
+			"Block(s) removed to height {} via blocks_disconnected. New best block is {}",
+			fork_point.height,
+			fork_point.block_hash,
 		);
 		for monitor_state in monitor_states.values() {
-			monitor_state.monitor.block_disconnected(
-				header,
-				height,
+			monitor_state.monitor.blocks_disconnected(
+				fork_point,
 				&*self.broadcaster,
 				&*self.fee_estimator,
 				&self.logger,

lightning/src/chain/channelmonitor.rs

@@ -2308,23 +2308,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 
 	/// Determines if the disconnected block contained any transactions of interest and updates
 	/// appropriately.
-	#[rustfmt::skip]
-	pub fn block_disconnected<B: Deref, F: Deref, L: Deref>(
-		&self,
-		header: &Header,
-		height: u32,
-		broadcaster: B,
-		fee_estimator: F,
-		logger: &L,
+	pub fn blocks_disconnected<B: Deref, F: Deref, L: Deref>(
+		&self, fork_point: BestBlock, broadcaster: B, fee_estimator: F, logger: &L,
 	) where
 		B::Target: BroadcasterInterface,
 		F::Target: FeeEstimator,
 		L::Target: Logger,
 	{
 		let mut inner = self.inner.lock().unwrap();
 		let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
-		inner.block_disconnected(
-			header, height, broadcaster, fee_estimator, &logger)
+		inner.blocks_disconnected(fork_point, broadcaster, fee_estimator, &logger)
 	}
 
 	/// Processes transactions confirmed in a block with the given header and height, returning new
@@ -2358,10 +2351,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 
 	/// Processes a transaction that was reorganized out of the chain.
 	///
-	/// Used instead of [`block_disconnected`] by clients that are notified of transactions rather
+	/// Used instead of [`blocks_disconnected`] by clients that are notified of transactions rather
 	/// than blocks. See [`chain::Confirm`] for calling expectations.
 	///
-	/// [`block_disconnected`]: Self::block_disconnected
+	/// [`blocks_disconnected`]: Self::blocks_disconnected
 	#[rustfmt::skip]
 	pub fn transaction_unconfirmed<B: Deref, F: Deref, L: Deref>(
 		&self,
@@ -5124,8 +5117,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			log_trace!(logger, "Best block re-orged, replaced with new block {} at height {}", block_hash, height);
 			self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height <= height);
 			let conf_target = self.closure_conf_target();
-			self.onchain_tx_handler.block_disconnected(
-				height + 1, &broadcaster, conf_target, &self.destination_script, fee_estimator, logger,
+			self.onchain_tx_handler.blocks_disconnected(
+				height, &broadcaster, conf_target, &self.destination_script, fee_estimator, logger,
 			);
 			Vec::new()
 		} else { Vec::new() }
@@ -5452,12 +5445,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 							!unmatured_htlcs.contains(&source),
 							"An unmature HTLC transaction conflicts with a maturing one; failed to \
 							 call either transaction_unconfirmed for the conflicting transaction \
-							 or block_disconnected for a block containing it.");
+							 or blocks_disconnected for a block before it.");
 						debug_assert!(
 							!matured_htlcs.contains(&source),
 							"A matured HTLC transaction conflicts with a maturing one; failed to \
 							 call either transaction_unconfirmed for the conflicting transaction \
-							 or block_disconnected for a block containing it.");
+							 or blocks_disconnected for a block before it.");
 						matured_htlcs.push(source.clone());
 					}
 
@@ -5605,23 +5598,26 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	}
 
 	#[rustfmt::skip]
-	fn block_disconnected<B: Deref, F: Deref, L: Deref>(
-		&mut self, header: &Header, height: u32, broadcaster: B, fee_estimator: F, logger: &WithChannelMonitor<L>
+	fn blocks_disconnected<B: Deref, F: Deref, L: Deref>(
+		&mut self, fork_point: BestBlock, broadcaster: B, fee_estimator: F, logger: &WithChannelMonitor<L>
 	) where B::Target: BroadcasterInterface,
 		F::Target: FeeEstimator,
 		L::Target: Logger,
 	{
-		log_trace!(logger, "Block {} at height {} disconnected", header.block_hash(), height);
+		let new_height = fork_point.height;
+		log_trace!(logger, "Block(s) disconnected to height {}", new_height);
+		assert!(self.best_block.height > fork_point.height,
+			"Blocks disconnected must indicate disconnection from the current best height, i.e. the new chain tip must be lower than the previous best height");
 
 		//We may discard:
 		//- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected
 		//- maturing spendable output has transaction paying us has been disconnected
-		self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height < height);
+		self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height <= new_height);
 
 		// TODO: Replace with `take_if` once our MSRV is >= 1.80.
 		let mut should_broadcast_commitment = false;
 		if let Some((_, conf_height)) = self.alternative_funding_confirmed.as_ref() {
-			if *conf_height == height {
+			if *conf_height > new_height {
 				self.alternative_funding_confirmed.take();
 				if self.holder_tx_signed || self.funding_spend_seen {
 					// Cancel any previous claims that are no longer valid as they stemmed from a
@@ -5637,8 +5633,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 		let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
 		let conf_target = self.closure_conf_target();
-		self.onchain_tx_handler.block_disconnected(
-			height, &broadcaster, conf_target, &self.destination_script, &bounded_fee_estimator, logger
+		self.onchain_tx_handler.blocks_disconnected(
+			new_height, &broadcaster, conf_target, &self.destination_script, &bounded_fee_estimator, logger
 		);
 
 		// Only attempt to broadcast the new commitment after the `block_disconnected` call above so that
@@ -5647,7 +5643,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, &bounded_fee_estimator, logger);
 		}
 
-		self.best_block = BestBlock::new(header.prev_blockhash, height - 1);
+		self.best_block = fork_point;
 	}
 
 	#[rustfmt::skip]
@@ -6121,8 +6117,8 @@ where
 		self.0.block_connected(header, txdata, height, &*self.1, &*self.2, &self.3);
 	}
 
-	fn block_disconnected(&self, header: &Header, height: u32) {
-		self.0.block_disconnected(header, height, &*self.1, &*self.2, &self.3);
+	fn blocks_disconnected(&self, fork_point: BestBlock) {
+		self.0.blocks_disconnected(fork_point, &*self.1, &*self.2, &self.3);
 	}
 }