Merge pull request lightningdevkit#4003 from yellowred/add_htlc_payment_hash_log

Log payment hash of the HTLC that causes a force closure

Author: TheBlueMatt

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
@@ -5599,8 +5601,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					let htlc_outbound = $holder_tx == htlc.offered;
 					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 expiry is {}", if htlc_outbound { "outbound" } else { "inbound "}, htlc.cltv_expiry);
-						return true;
+						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 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

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),

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,

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.

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]);
 	}