fixup: address comments:

keep doc/comments to 100 chars max per line
adjust channel monitor comments
move funding search to filter_block
make is_manual_broadcast and funding_seen_onchain odd. its ok to be odd

Author: martinsaposnic

lightning-liquidity/src/lsps2/service.rs

@@ -1023,9 +1023,9 @@ where
 	/// were being held for that JIT channel are forwarded. In a `client_trusts_lsp` flow, once
 	/// the fee has been fully paid, the channel's funding transaction will be broadcasted.
 	///
-	/// Note that `next_channel_id` and `skimmed_fee_msat` are required to be provided. Therefore, the corresponding
-	/// [`Event::PaymentForwarded`] events need to be generated and serialized by LDK versions
-	/// greater or equal to 0.0.122.
+	/// Note that `next_channel_id` and `skimmed_fee_msat` are required to be provided.
+	/// Therefore, the corresponding [`Event::PaymentForwarded`] events need to be generated and
+	/// serialized by LDK versions greater or equal to 0.0.122.
 	///
 	/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
 	pub fn payment_forwarded(

lightning/src/chain/channelmonitor.rs

@@ -1169,7 +1169,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	funding: FundingScope,
 	pending_funding: Vec<FundingScope>,
 
+	/// True if this channel was configured for manual funding broadcasts. Monitors written by
+	/// versions prior to introducing the flag will load with `false` until a new update persists it.
 	is_manual_broadcast: bool,
+	/// True once we've observed either funding transaction on-chain. Older monitors assume this is
+	/// `true` when absent during upgrade so holder broadcasts aren't gated unexpectedly.
 	funding_seen_onchain: bool,
 
 	latest_update_id: u64,
@@ -1701,8 +1705,8 @@ pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
 		(31, channel_monitor.funding.channel_parameters, required),
 		(32, channel_monitor.pending_funding, optional_vec),
 		(34, channel_monitor.alternative_funding_confirmed, option),
-		(36, channel_monitor.is_manual_broadcast, required),
-		(38, channel_monitor.funding_seen_onchain, required),
+		(35, channel_monitor.is_manual_broadcast, required),
+		(37, channel_monitor.funding_seen_onchain, required),
 	});
 
 	Ok(())
@@ -2298,8 +2302,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// [`crate::ln::channelmanager::ChannelManager::funding_transaction_generated_manual_broadcast`]),
 	/// automatic broadcasts are suppressed until the funding transaction has been observed on-chain.
 	/// Calling this method overrides that suppression and queues the latest holder commitment
-	/// transaction for broadcast even if the funding has not yet been seen on-chain. This is unsafe
-	/// and may result in unconfirmable transactions.
+	/// transaction for broadcast even if the funding has not yet been seen on-chain. This may result
+	/// in unconfirmable transactions being broadcast or [`Event::BumpTransaction`] notifications for
+	/// transactions that cannot be confirmed until the funding transaction is visible.
+	///
+	/// [`Event::BumpTransaction`]: crate::events::Event::BumpTransaction
 	#[rustfmt::skip]
 	pub fn broadcast_latest_holder_commitment_txn<B: Deref, F: Deref, L: Deref>(
 		&self, broadcaster: &B, fee_estimator: &F, logger: &L
@@ -5246,14 +5253,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		F::Target: FeeEstimator,
 		L::Target: Logger,
 	{
-		for &(_, tx) in txdata.iter() {
-			let txid = tx.compute_txid();
-			if txid == self.funding.funding_txid() ||
-			self.pending_funding.iter().any(|f| f.funding_txid() == txid)
-			{
-				self.funding_seen_onchain = true;
-			}
-		}
 		let txn_matched = self.filter_block(txdata);
 		for tx in &txn_matched {
 			let mut output_val = Amount::ZERO;
@@ -5833,10 +5832,17 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 	/// Filters a block's `txdata` for transactions spending watched outputs or for any child
 	/// transactions thereof.
+	/// While iterating, this also tracks whether we observed the funding transaction.
 	#[rustfmt::skip]
-	fn filter_block<'a>(&self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> {
+	fn filter_block<'a>(&mut self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> {
 		let mut matched_txn = new_hash_set();
 		txdata.iter().filter(|&&(_, tx)| {
+			let txid = tx.compute_txid();
+			if !self.funding_seen_onchain && (txid == self.funding.funding_txid() ||
+				self.pending_funding.iter().any(|f| f.funding_txid() == txid))
+			{
+				self.funding_seen_onchain = true;
+			}
 			let mut matches = self.spends_watched_output(tx);
 			for input in tx.input.iter() {
 				if matches { break; }
@@ -5845,7 +5851,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				}
 			}
 			if matches {
-				matched_txn.insert(tx.compute_txid());
+				matched_txn.insert(txid);
 			}
 			matches
 		}).map(|(_, tx)| *tx).collect()
@@ -6506,8 +6512,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(31, channel_parameters, (option: ReadableArgs, None)),
 			(32, pending_funding, optional_vec),
 			(34, alternative_funding_confirmed, option),
-			(36, is_manual_broadcast, option),
-			(38, funding_seen_onchain, option),
+			(35, is_manual_broadcast, option),
+			(37, funding_seen_onchain, option),
 		});
 		// Note that `payment_preimages_with_info` was added (and is always written) in LDK 0.1, so
 		// we can use it to determine if this monitor was last written by LDK 0.1 or later.