Broadcast holder commitment immediately on alternative funding reorg
What the developer wrote
Authored by Wilmer Paulino
Broadcast holder commitment immediately on alternative funding reorg
We can't rely waiting on another (or the same) renegotiated funding
transaction to confirm, since it may never happen. We also don't want to
rely on the counterparty to broadcast for us, or require manual
intervention from the user, so we choose to broadcast the new holder
commitment immediately. This ensures we're able to claim funds from an
already force closed channel after an alternative funding reorg.
What changed, and why it matters
This change fixes a situation in the Lightning Dev Kit where, after a blockchain reorganization that undoes an alternative funding transaction, the user's own commitment transaction could fail to be broadcast automatically. Previously the code would cancel stale claims but then wait for a new funding transaction to confirm before broadcasting the user's valid commitment. If that confirmation never happened, the user might not recover funds from a force-closed channel. The patch now broadcasts the user's latest commitment immediately after such a reorg, without relying on the counterparty or manual action.
Treat as a recommended reliability/security fix. Users running nodes with alternative funding flows should upgrade to ensure holder commitments are broadcast promptly after reorgs that undo alternative funding transactions. Review related reorg handling paths for similar deferred-broadcast patterns.
Security signals we found
Funds-recovery reliability fix after blockchain reorganization
Automatic broadcast of holder commitment instead of relying on counterparty or user
Change in control flow: broadcast now occurs immediately after stale claim cancellation
Broadening of trigger condition from holder_tx_signed to include funding_spend_seen
No explicit CVE, advisory, or researcher attribution in commit or supplied references
Evidence from the diff
In channelmonitor.rs, when an alternative funding confirmation is removed via block_disconnected or transaction_unconfirmed, the code now sets a flag and, after invoking OnchainTxHandler cleanup, calls queue_latest_holder_commitment_txn_for_broadcast to broadcast the current holder commitment. The condition triggering this was broadened from holder_tx_signed to also include funding_spend_seen. OnchainTxHandler’s transaction_unconfirmed and block_disconnected now take broadcaster by reference so the same broadcaster can be reused for the subsequent broadcast. The previous behavior canceled stale claims but deferred broadcasting the new holder commitment until another funding transaction confirmed, which could leave funds unclaimed if no such confirmation occurred.
Changed components
lightning/src/chain/channelmonitor.rslightning/src/chain/onchaintx.rsChannelMonitorImpl block_disconnected handlerChannelMonitorImpl transaction_unconfirmed handlerOnchainTxHandler block_disconnected and transaction_unconfirmed methodsInspect captured patch +27 / −11
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index ff7cbf3..e5f351e 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -4866,7 +4866,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
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,
+ height + 1, &broadcaster, conf_target, &self.destination_script, fee_estimator, logger,
);
Vec::new()
} else { Vec::new() }
@@ -5341,16 +5341,18 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height < 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 {
self.alternative_funding_confirmed.take();
- if self.holder_tx_signed {
+ if self.holder_tx_signed || self.funding_spend_seen {
// Cancel any previous claims that are no longer valid as they stemmed from a
- // different funding transaction. We'll wait until we see a funding transaction
- // confirm again before attempting to broadcast the new valid holder commitment.
+ // different funding transaction.
let new_holder_commitment_txid =
self.funding.current_holder_commitment_tx.trust().txid();
self.cancel_prev_commitment_claims(&logger, &new_holder_commitment_txid);
+
+ should_broadcast_commitment = true;
}
}
}
@@ -5358,9 +5360,15 @@ 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
+ 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
+ // 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.best_block = BestBlock::new(header.prev_blockhash, height - 1);
}
@@ -5395,24 +5403,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
debug_assert!(!self.onchain_events_awaiting_threshold_conf.iter().any(|ref entry| entry.txid == *txid));
// TODO: Replace with `take_if` once our MSRV is >= 1.80.
+ let mut should_broadcast_commitment = false;
if let Some((alternative_funding_txid, _)) = self.alternative_funding_confirmed.as_ref() {
if alternative_funding_txid == txid {
self.alternative_funding_confirmed.take();
- if self.holder_tx_signed {
+ if self.holder_tx_signed || self.funding_spend_seen {
// Cancel any previous claims that are no longer valid as they stemmed from a
- // different funding transaction. We'll wait until we see a funding transaction
- // confirm again before attempting to broadcast the new valid holder commitment.
+ // different funding transaction.
let new_holder_commitment_txid =
self.funding.current_holder_commitment_tx.trust().txid();
self.cancel_prev_commitment_claims(&logger, &new_holder_commitment_txid);
+
+ should_broadcast_commitment = true;
}
}
}
let conf_target = self.closure_conf_target();
self.onchain_tx_handler.transaction_unconfirmed(
- txid, broadcaster, conf_target, &self.destination_script, fee_estimator, logger
+ txid, &broadcaster, conf_target, &self.destination_script, fee_estimator, logger
);
+
+ // 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);
+ }
}
/// Filters a block's `txdata` for transactions spending watched outputs or for any child
diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index 95df05f..67ee019 100644
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -1120,7 +1120,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
pub(super) fn transaction_unconfirmed<B: Deref, F: Deref, L: Logger>(
&mut self,
txid: &Txid,
- broadcaster: B,
+ broadcaster: &B,
conf_target: ConfirmationTarget,
destination_script: &Script,
fee_estimator: &LowerBoundedFeeEstimator<F>,
@@ -1146,7 +1146,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
#[rustfmt::skip]
pub(super) fn block_disconnected<B: Deref, F: Deref, L: Logger>(
- &mut self, height: u32, broadcaster: B, conf_target: ConfirmationTarget,
+ &mut self, height: u32, broadcaster: &B, conf_target: ConfirmationTarget,
destination_script: &Script, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
)
where B::Target: BroadcasterInterface,
Why this scored 50/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.