Gate holder broadcast queueing on funding confirmation
What changed, and why it matters
This change fixes a logic bug in Lightning Dev Kit's channel monitor. When a user manually broadcasts their own funding transaction (rather than letting LDK do it automatically), the software could previously try to broadcast a commitment transaction before the funding transaction had actually appeared on the blockchain. Because commitment transactions spend the funding output, broadcasting them too early would fail mempool validation and could generate unnecessary fee-bumping notifications for transactions that cannot yet confirm. The patch suppresses automatic commitment broadcasts until the funding transaction is observed on-chain, while still allowing an explicit override via a public API method.
Review and merge. The change is defensive and corrects a functional issue that could cause failed broadcasts and confusing fee-bumping events. No immediate security emergency, but users relying on manual funding broadcast should upgrade to avoid operational issues.
Security signals we found
Prevents broadcast of unconfirmable holder commitment transactions in manual-funding mode
Avoids spurious Event::BumpTransaction notifications for transactions dependent on unseen funding
Adds explicit override API with documented risk of unconfirmable broadcasts
Gates automatic monitor-initiated broadcasts on funding confirmation
Evidence from the diff
The commit modifies ChannelMonitorImpl::queue_latest_holder_commitment_txn_for_broadcast to accept a new require_funding_seen boolean. When true and the channel is in manual-broadcast mode (is_manual_broadcast) without the funding transaction having been seen on-chain (funding_seen_onchain), the method returns early and logs that it is skipping the broadcast. All internal automatic call sites (onchain event handling, block disconnection, transaction unconfirmation) pass true, so they are gated. The public ChannelMonitor::broadcast_latest_holder_commitment_txn API passes false, preserving an explicit override. Documentation is added to both methods explaining the behavior and its risks.
Changed components
lightning/src/chain/channelmonitor.rsChannelMonitor::broadcast_latest_holder_commitment_txnChannelMonitorImpl::queue_latest_holder_commitment_txn_for_broadcastmanual funding broadcast flowInspect captured patch +32 / −6
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 92f7e9e..175db8a 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -2346,6 +2346,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
/// close channel with their commitment transaction after a substantial amount of time. Best
/// may be to contact the other node operator out-of-band to coordinate other options available
/// to you.
+ ///
+ /// Note: For channels using manual funding broadcast (see
+ /// [`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 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
pub fn broadcast_latest_holder_commitment_txn<B: Deref, F: Deref, L: Deref>(
&self, broadcaster: &B, fee_estimator: &F, logger: &L,
) where
@@ -2356,10 +2366,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
let mut inner = self.inner.lock().unwrap();
let fee_estimator = LowerBoundedFeeEstimator::new(&**fee_estimator);
let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
+
inner.queue_latest_holder_commitment_txn_for_broadcast(
broadcaster,
&fee_estimator,
&logger,
+ false,
);
}
@@ -3977,8 +3989,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
#[rustfmt::skip]
+ /// Note: For channels where the funding transaction is being manually managed (see
+ /// [`crate::ln::channelmanager::ChannelManager::funding_transaction_generated_manual_broadcast`]),
+ /// this method returns without queuing any transactions until the funding transaction has been
+ /// observed on-chain, unless `require_funding_seen` is `false`. This prevents attempting to
+ /// broadcast unconfirmable holder commitment transactions before the funding is visible.
+ /// See also [`ChannelMonitor::broadcast_latest_holder_commitment_txn`].
+ ///
+ /// [`ChannelMonitor::broadcast_latest_holder_commitment_txn`]: crate::chain::channelmonitor::ChannelMonitor::broadcast_latest_holder_commitment_txn
pub(crate) fn queue_latest_holder_commitment_txn_for_broadcast<B: Deref, F: Deref, L: Deref>(
- &mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &WithChannelMonitor<L>
+ &mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &WithChannelMonitor<L>, require_funding_seen: bool,
)
where
B::Target: BroadcasterInterface,
@@ -3990,6 +4010,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
message: "ChannelMonitor-initiated commitment transaction broadcast".to_owned(),
};
let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(Some(reason));
+ // In manual-broadcast mode, if `require_funding_seen` is true and we have not yet observed
+ // the funding transaction on-chain, do not queue any transactions.
+ if require_funding_seen && self.is_manual_broadcast && !self.funding_seen_onchain {
+ log_info!(logger, "Not broadcasting holder commitment for manual-broadcast channel before funding appears on-chain");
+ return;
+ }
let conf_target = self.closure_conf_target();
self.onchain_tx_handler.update_claims_view_from_requests(
claimable_outpoints, self.best_block.height, self.best_block.height, broadcaster,
@@ -4312,7 +4338,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
log_trace!(logger, "Avoiding commitment broadcast, already detected confirmed spend onchain");
continue;
}
- self.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &bounded_fee_estimator, logger);
+ self.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &bounded_fee_estimator, logger, true);
} else if !self.holder_tx_signed {
log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast");
log_error!(logger, " in channel monitor for channel {}!", &self.channel_id());
@@ -5860,7 +5886,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// Only attempt to broadcast the new commitment after the `block_disconnected` call above so that
// it doesn't get removed from the set of pending claims.
if should_broadcast_commitment {
- self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, &bounded_fee_estimator, logger);
+ self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, &bounded_fee_estimator, logger, true);
}
self.best_block = fork_point;
@@ -5921,7 +5947,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// Only attempt to broadcast the new commitment after the `transaction_unconfirmed` call above so
// that it doesn't get removed from the set of pending claims.
if should_broadcast_commitment {
- self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, fee_estimator, logger);
+ self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, fee_estimator, logger, true);
}
}
@@ -7071,7 +7097,7 @@ mod tests {
let monitor = ChannelMonitor::new(
Secp256k1::new(), keys, Some(shutdown_script.into_inner()), 0, &ScriptBuf::new(),
&channel_parameters, true, 0, HolderCommitmentTransaction::dummy(0, funding_outpoint, Vec::new()),
- best_block, dummy_key, channel_id,
+ best_block, dummy_key, channel_id, false,
);
let nondust_htlcs = preimages_slice_to_htlcs!(preimages[0..10]);
@@ -7332,7 +7358,7 @@ mod tests {
let monitor = ChannelMonitor::new(
Secp256k1::new(), keys, Some(shutdown_script.into_inner()), 0, &ScriptBuf::new(),
&channel_parameters, true, 0, HolderCommitmentTransaction::dummy(0, funding_outpoint, Vec::new()),
- best_block, dummy_key, channel_id,
+ best_block, dummy_key, channel_id, false
);
let chan_id = monitor.inner.lock().unwrap().channel_id();
Why this scored 34/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.