← Watch feed
Add the blamed HTLC payment hash to `ClosureReason::HTLCsTimedOut`
Public commit record
85/100 · Strong
What the developer wrote
Authored by olegkubrakov
Add the blamed HTLC payment hash to `ClosureReason::HTLCsTimedOut`
When an HTLC timing out causes a channel to force-close, its useful
to have the payment hash available in a programatic way so that it
is always available for debugging. Thus, here, it is added to
`ClosureReason::HTLCsTimedOut` for inclusion in
`Event::ChanelClosed`.
Co-authored-by: Matt Corallo <git@bluematt.me>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This change is a small observability improvement, not a security fix. When a Lightning channel is force-closed because an HTLC (a conditional payment) timed out, the payment hash of the blamed HTLC is now included in the channel-closed event. Previously only a generic 'HTLCs timed out' reason was reported. This helps developers debug payment failures but does not change when or how channels close, nor does it fix any vulnerability.
Recommended action
No security action required. Treat as a normal API/observability improvement; review downstream consumers of `Event::ChannelClosed` so they handle the new `ClosureReason::HTLCsTimedOut { payment_hash: ... }` shape.
Security signals we found
01
No vulnerability pattern: change is purely diagnostic/observability
02
Public enum variant change is backward-compatible via optional TLV serialization
03
No new trust assumptions, no privilege changes, no input validation changes
Evidence from the diff
The commit refactors ClosureReason::HTLCsTimedOut from a unit variant into a struct variant carrying an Option<PaymentHash>. ChannelMonitorImpl::should_broadcast_holder_commitment_txn now returns Option<PaymentHash> instead of bool, propagating the payment hash of the first HTLC whose timeout triggers a force-close. Serialization is updated with a TLV field so old events deserialize with None. Tests are updated to expect the new variant shape. No consensus, cryptographic, or network behavior changes are present.
Changed components
lightning/src/chain/channelmonitor.rslightning/src/events/mod.rslightning/src/ln/functional_tests.rslightning/src/ln/monitor_tests.rslightning/src/ln/reorg_tests.rsInspect captured patch +61 / −23
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 2373f2c..f72eea8 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -5211,8 +5211,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
debug_assert!(self.best_block.height >= conf_height);
let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
- if should_broadcast {
- let (mut new_outpoints, mut new_outputs) = self.generate_claimable_outpoints_and_watch_outputs(Some(ClosureReason::HTLCsTimedOut));
+ 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));
claimable_outpoints.append(&mut new_outpoints);
watch_outputs.append(&mut new_outputs);
}
@@ -5560,7 +5562,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
#[rustfmt::skip]
fn should_broadcast_holder_commitment_txn<L: Deref>(
&self, logger: &WithChannelMonitor<L>
- ) -> bool where L::Target: Logger {
+ ) -> Option<PaymentHash> where L::Target: Logger {
// There's no need to broadcast our commitment transaction if we've seen one confirmed (even
// with 1 confirmation) as it'll be rejected as duplicate/conflicting.
if self.funding_spend_confirmed.is_some() ||
@@ -5569,7 +5571,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
_ => false,
}).is_some()
{
- return false;
+ return None;
}
// We need to consider all HTLCs which are:
// * in any unrevoked counterparty commitment transaction, as they could broadcast said
@@ -5600,7 +5602,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
if ( htlc_outbound && htlc.cltv_expiry + LATENCY_GRACE_PERIOD_BLOCKS <= height) ||
(!htlc_outbound && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && self.payment_preimages.contains_key(&htlc.payment_hash)) {
log_info!(logger, "Force-closing channel due to {} HTLC timeout - HTLC with payment hash {} expires at {}", if htlc_outbound { "outbound" } else { "inbound"}, htlc.payment_hash, htlc.cltv_expiry);
- return true;
+ return Some(htlc.payment_hash);
}
}
}
@@ -5619,7 +5621,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}
- false
+ None
}
/// Check if any transaction broadcasted is resolving HTLC output by a success or timeout on a holder
diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 15e38aa..6642498 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -406,7 +406,12 @@ pub enum ClosureReason {
/// was ready to be broadcast.
FundingBatchClosure,
/// One of our HTLCs timed out in a channel, causing us to force close the channel.
- HTLCsTimedOut,
+ HTLCsTimedOut {
+ /// The payment hash of an HTLC that timed out.
+ ///
+ /// Will be `None` for any event serialized by LDK prior to 0.2.
+ payment_hash: Option<PaymentHash>,
+ },
/// Our peer provided a feerate which violated our required minimum (fetched from our
/// [`FeeEstimator`] either as [`ConfirmationTarget::MinAllowedAnchorChannelRemoteFee`] or
/// [`ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee`]).
@@ -480,7 +485,12 @@ impl core::fmt::Display for ClosureReason {
ClosureReason::FundingBatchClosure => {
f.write_str("another channel in the same funding batch closed")
},
- ClosureReason::HTLCsTimedOut => f.write_str("htlcs on the channel timed out"),
+ ClosureReason::HTLCsTimedOut { payment_hash: Some(hash) } => f.write_fmt(format_args!(
+ "HTLC(s) on the channel timed out (including the HTLC with payment hash {hash})",
+ )),
+ ClosureReason::HTLCsTimedOut { payment_hash: None } => {
+ f.write_fmt(format_args!("HTLC(s) on the channel timed out"))
+ },
ClosureReason::PeerFeerateTooLow {
peer_feerate_sat_per_kw,
required_feerate_sat_per_kw,
@@ -508,7 +518,9 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
(15, FundingBatchClosure) => {},
(17, CounterpartyInitiatedCooperativeClosure) => {},
(19, LocallyInitiatedCooperativeClosure) => {},
- (21, HTLCsTimedOut) => {},
+ (21, HTLCsTimedOut) => {
+ (1, payment_hash, option),
+ },
(23, PeerFeerateTooLow) => {
(0, peer_feerate_sat_per_kw, required),
(2, required_feerate_sat_per_kw, required),
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 388db6d..8c09a96 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -856,7 +856,8 @@ fn do_test_fail_back_before_backwards_timeout(post_fail_back_action: PostFailBac
let timeout_blocks = TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1;
connect_blocks(&nodes[1], timeout_blocks);
let node_1_txn = test_txn_broadcast(&nodes[1], &chan_2, None, HTLCType::TIMEOUT);
- check_closed_event(&nodes[1], 1, ClosureReason::HTLCsTimedOut, false, &[node_c_id], 100_000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
+ check_closed_event(&nodes[1], 1, reason, false, &[node_c_id], 100_000);
check_closed_broadcast(&nodes[1], 1, true);
check_added_monitors(&nodes[1], 1);
@@ -910,7 +911,7 @@ fn do_test_fail_back_before_backwards_timeout(post_fail_back_action: PostFailBac
connect_blocks(&nodes[2], TEST_FINAL_CLTV - CLTV_CLAIM_BUFFER + 2);
let node_2_txn = test_txn_broadcast(&nodes[2], &chan_2, None, HTLCType::SUCCESS);
check_closed_broadcast!(nodes[2], true);
- let reason = ClosureReason::HTLCsTimedOut;
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
check_closed_event(&nodes[2], 1, reason, false, &[node_b_id], 100_000);
check_added_monitors(&nodes[2], 1);
@@ -1160,7 +1161,8 @@ pub fn channel_monitor_network_test() {
}
check_added_monitors(&nodes[4], 1);
test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
- check_closed_event!(nodes[4], 1, ClosureReason::HTLCsTimedOut, [node_d_id], 100000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash_2) };
+ check_closed_event!(nodes[4], 1, reason, [node_d_id], 100000);
mine_transaction(&nodes[4], &node_txn[0]);
check_preimage_claim(&nodes[4], &node_txn);
@@ -1177,7 +1179,8 @@ pub fn channel_monitor_network_test() {
nodes[3].chain_monitor.chain_monitor.watch_channel(chan_3.2, chan_3_mon),
Ok(ChannelMonitorUpdateStatus::Completed)
);
- check_closed_event!(nodes[3], 1, ClosureReason::HTLCsTimedOut, [node_id_4], 100000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash_2) };
+ check_closed_event!(nodes[3], 1, reason, [node_id_4], 100000);
}
#[xtest(feature = "_externalize_tests")]
@@ -5321,7 +5324,8 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
test_txn_broadcast(&nodes[1], &chan, None, htlc_type);
check_closed_broadcast!(nodes[1], true);
check_added_monitors(&nodes[1], 1);
- check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [node_a_id], 100000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
+ check_closed_event!(nodes[1], 1, reason, [node_a_id], 100000);
}
fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
@@ -5359,7 +5363,8 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
check_closed_broadcast!(nodes[0], true);
check_added_monitors(&nodes[0], 1);
- check_closed_event!(nodes[0], 1, ClosureReason::HTLCsTimedOut, [node_b_id], 100000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
+ check_closed_event!(nodes[0], 1, reason, [node_b_id], 100000);
}
fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no_close: bool) {
@@ -5414,7 +5419,8 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
check_closed_broadcast!(nodes[0], true);
check_added_monitors(&nodes[0], 1);
- check_closed_event!(nodes[0], 1, ClosureReason::HTLCsTimedOut, [node_b_id], 100000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(our_payment_hash) };
+ check_closed_event!(nodes[0], 1, reason, [node_b_id], 100000);
} else {
expect_payment_failed!(nodes[0], our_payment_hash, true);
}
@@ -8160,7 +8166,7 @@ pub fn test_concurrent_monitor_claim() {
send_payment(&nodes[0], &[&nodes[1]], 10_000_000);
// Route a HTLC from node 0 to node 1 (but don't settle)
- route_payment(&nodes[0], &[&nodes[1]], 9_000_000);
+ let (_, payment_hash_timeout, ..) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);
// Copy ChainMonitor to simulate watchtower Alice and update block height her ChannelMonitor timeout HTLC onchain
let chain_source = test_utils::TestChainSource::new(Network::Testnet);
@@ -8311,7 +8317,8 @@ pub fn test_concurrent_monitor_claim() {
let height = HTLC_TIMEOUT_BROADCAST + 1;
connect_blocks(&nodes[0], height - nodes[0].best_block_info().1);
check_closed_broadcast(&nodes[0], 1, true);
- check_closed_event!(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false, [node_b_id], 100000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash_timeout) };
+ check_closed_event!(&nodes[0], 1, reason, false, [node_b_id], 100000);
watchtower_alice.chain_monitor.block_connected(
&create_dummy_block(BlockHash::all_zeros(), 42, vec![bob_state_y.clone()]),
height,
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 8d24fe2..11dd133 100644
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -1407,14 +1407,22 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_
});
assert!(failed_payments.is_empty());
match &events[0] {
- Event::ChannelClosed { reason: ClosureReason::HTLCsTimedOut, .. } => {},
+ Event::ChannelClosed { reason: ClosureReason::HTLCsTimedOut { .. }, .. } => {},
_ => panic!(),
}
connect_blocks(&nodes[1], htlc_cltv_timeout + 1 - 10);
check_closed_broadcast!(nodes[1], true);
check_added_monitors!(nodes[1], 1);
- check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 1000000);
+ check_closed_events(&nodes[1], &[ExpectedCloseEvent {
+ channel_capacity_sats: Some(1_000_000),
+ channel_id: Some(chan_id),
+ counterparty_node_id: Some(nodes[0].node.get_our_node_id()),
+ discard_funding: false,
+ reason: None, // Could be due to any HTLC timing out, so don't bother checking
+ channel_funding_txo: None,
+ user_channel_id: None,
+ }]);
// Prior to channel closure, B considers the preimage HTLC as its own, and otherwise only
// lists the two on-chain timeout-able HTLCs as claimable balances.
diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index 8e26567..93b2a05 100644
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -487,7 +487,15 @@ fn test_set_outpoints_partial_claiming() {
// Connect blocks on node B
connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
check_closed_broadcast!(nodes[1], true);
- check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 1000000);
+ check_closed_events(&nodes[1], &[ExpectedCloseEvent {
+ channel_capacity_sats: Some(1_000_000),
+ channel_id: Some(chan.2),
+ counterparty_node_id: Some(nodes[0].node.get_our_node_id()),
+ discard_funding: false,
+ reason: None, // Could be due to either HTLC timing out, so don't bother checking
+ channel_funding_txo: None,
+ user_channel_id: None,
+ }]);
check_added_monitors!(nodes[1], 1);
// Verify node B broadcast 2 HTLC-timeout txn
let partial_claim_tx = {
@@ -818,7 +826,7 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1);
// Route a payment so we have an HTLC to claim as well.
- let _ = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+ let (_, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
if revoked_counterparty_commitment {
// Trigger a fee update such that we advance the state. We will have B broadcast its state
@@ -843,7 +851,8 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
check_closed_broadcast(&nodes[0], 1, true);
check_added_monitors(&nodes[0], 1);
- check_closed_event(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false, &[nodes[1].node.get_our_node_id()], 100_000);
+ let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
+ check_closed_event(&nodes[0], 1, reason, false, &[nodes[1].node.get_our_node_id()], 100_000);
if anchors {
handle_bump_close_event(&nodes[0]);
}
Risk score
Our methodology →Why this scored 20/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.