Always broadcast closing txn in monitor manually broadcast
What the developer wrote
Authored by Matt Corallo
Always broadcast closing txn in monitor manually broadcast
In 6c5ef049b8d0ec174d7368d48b7b429efffb4a61 we prevented broadcast
of the commitment transactions if the funding transaction has not
yet appeared on-chain for manual-broadcast channels to avoid
spurious bumps or unbroadcastable transactions. It also updated
the documentation on
`ChanelMonitor::broadcast_latest_holder_commitment_txn` to
explicitly state that it will override the manual-broadcast state
and broadcast the latest commitment anyway.
However, 4131680db4c9ff934e83940be1158f8b1cc0f8cf accidentally
reverted this behavior by updating
`generate_claimable_outpoints_and_watch_outputs`, which is caled by
`broadcast_latest_holder_commitment_txn` to also refuse to
broadcast if funding has not been seen on chain.
Here we fix this, passing through the `require_funding_seen` bool
to allow `broadcast_latest_holder_commitment_txn` to broadcast
immediately.
What changed, and why it matters
This commit fixes a regression in the Lightning Dev Kit's channel monitor. A previous change accidentally made it impossible to manually force-broadcast a closing transaction for certain channels before the funding transaction was seen on-chain. The fix restores the intended behavior: when a user explicitly asks to broadcast the latest commitment transaction, the monitor will do so immediately, while other automatic paths still wait for the funding transaction to appear on-chain to avoid creating invalid or premature transactions.
Review and merge the fix. After deployment, ensure manual-broadcast channel users can once again force-close via `broadcast_latest_holder_commitment_txn` before on-chain funding confirmation. Consider adding regression tests covering both automatic and manual broadcast paths for this guard.
Security signals we found
Regression fix restoring documented override behavior for manual-broadcast channels
Prevents a scenario where a user-initiated commitment broadcast could be silently ignored
Maintains the safety guard for automatic broadcast paths to avoid invalid/ premature transactions
No explicit security advisory, CVE, or researcher attribution in commit or supplied references
Evidence from the diff
The patch adds a require_funding_seen parameter to generate_claimable_outpoints_and_watch_outputs. Callers that are part of automatic broadcast paths pass true, preserving the guard that returns empty vectors for manual-broadcast channels whose funding tx is not yet on-chain. The explicit broadcast_latest_holder_commitment_txn path passes false (via queue_latest_holder_commitment_txn_for_broadcast), restoring its documented behavior of overriding the manual-broadcast wait and broadcasting immediately. This corrects an accidental revert introduced in commit 4131680db4c9ff934e83940be1158f8b1cc0f8cf.
Changed components
lightning/src/chain/channelmonitor.rsChannelMonitorImpl::generate_claimable_outpoints_and_watch_outputsChannelMonitorImpl::queue_latest_holder_commitment_txn_for_broadcastChannelMonitor::broadcast_latest_holder_commitment_txnInspect captured patch +8 / −5
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index f49764b..3b0b393 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -3933,6 +3933,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
#[rustfmt::skip]
fn generate_claimable_outpoints_and_watch_outputs(
&mut self, generate_monitor_event_with_reason: Option<ClosureReason>,
+ require_funding_seen: bool,
) -> (Vec<PackageTemplate>, Vec<TransactionOutputs>) {
let funding = get_confirmed_funding_scope!(self);
let holder_commitment_tx = &funding.current_holder_commitment_tx;
@@ -3987,7 +3988,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
// In manual-broadcast mode, if we have not yet observed the funding transaction on-chain,
// return empty vectors.
- if self.is_manual_broadcast && !self.funding_seen_onchain {
+ if require_funding_seen && self.is_manual_broadcast && !self.funding_seen_onchain {
return (Vec::new(), Vec::new());
} else {
(claimable_outpoints, watch_outputs)
@@ -4004,7 +4005,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
///
/// [`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>, require_funding_seen: bool,
+ &mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &WithChannelMonitor<L>,
+ require_funding_seen: bool,
)
where
B::Target: BroadcasterInterface,
@@ -4015,7 +4017,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
broadcasted_latest_txn: Some(true),
message: "ChannelMonitor-initiated commitment transaction broadcast".to_owned(),
};
- let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(Some(reason));
+ let (claimable_outpoints, _) =
+ self.generate_claimable_outpoints_and_watch_outputs(Some(reason), require_funding_seen);
// 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 {
@@ -5618,7 +5621,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
if should_broadcast_commitment {
let (mut claimables, mut outputs) =
- self.generate_claimable_outpoints_and_watch_outputs(None);
+ self.generate_claimable_outpoints_and_watch_outputs(None, false);
claimable_outpoints.append(&mut claimables);
watch_outputs.append(&mut outputs);
}
@@ -5660,7 +5663,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
if let Some(payment_hash) = should_broadcast {
let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
let (mut new_outpoints, mut new_outputs) =
- self.generate_claimable_outpoints_and_watch_outputs(Some(reason));
+ self.generate_claimable_outpoints_and_watch_outputs(Some(reason), false);
claimable_outpoints.append(&mut new_outpoints);
watch_outputs.append(&mut new_outputs);
}
Why this scored 44/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.