Avoid returning full HTLC info in get_pending_or_resolved_outbound_htlcs

We previously returned a `HTLCOutputInCommitment`, even though we only
needed the HTLC's `payment_hash` and `amount_msat`. We make this change
as we don't want callers to rely on the HTLC's
`transaction_output_index`, as that can change across `FundingScope`s
due to splices.

Author: wpaulino

lightning/src/chain/channelmonitor.rs

@@ -2923,7 +2923,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// HTLCs which were resolved on-chain (i.e. where the final HTLC resolution was done by an
 	/// event from this `ChannelMonitor`).
 	#[rustfmt::skip]
-	pub(crate) fn get_all_current_outbound_htlcs(&self) -> HashMap<HTLCSource, (HTLCOutputInCommitment, Option<PaymentPreimage>)> {
+	pub(crate) fn get_all_current_outbound_htlcs(&self) -> HashMap<HTLCSource, (PaymentHash, u64, Option<PaymentPreimage>)> {
 		let mut res = new_hash_map();
 		// Just examine the available counterparty commitment transactions. See docs on
 		// `fail_unbroadcast_htlcs`, below, for justification.
@@ -2933,7 +2933,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 				if let Some(ref latest_outpoints) = us.funding.counterparty_claimable_outpoints.get($txid) {
 					for &(ref htlc, ref source_option) in latest_outpoints.iter() {
 						if let &Some(ref source) = source_option {
-							res.insert((**source).clone(), (htlc.clone(),
+							res.insert((**source).clone(), (htlc.payment_hash, htlc.amount_msat,
 								us.counterparty_fulfilled_htlcs.get(&SentHTLCId::from_source(source)).cloned()));
 						}
 					}
@@ -2957,7 +2957,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// Currently, the preimage is unused, however if it is present in the relevant internal state
 	/// an HTLC is always included even if it has been resolved.
 	#[rustfmt::skip]
-	pub(crate) fn get_pending_or_resolved_outbound_htlcs(&self) -> HashMap<HTLCSource, (HTLCOutputInCommitment, Option<PaymentPreimage>)> {
+	pub(crate) fn get_pending_or_resolved_outbound_htlcs(&self) -> HashMap<HTLCSource, (PaymentHash, u64, Option<PaymentPreimage>)> {
 		let us = self.inner.lock().unwrap();
 		// We're only concerned with the confirmation count of HTLC transactions, and don't
 		// actually care how many confirmations a commitment transaction may or may not have. Thus,
@@ -3009,7 +3009,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 							let counterparty_resolved_preimage_opt =
 								us.counterparty_fulfilled_htlcs.get(&SentHTLCId::from_source(source)).cloned();
 							if !htlc_update_confd || counterparty_resolved_preimage_opt.is_some() {
-								res.insert(source.clone(), (htlc.clone(), counterparty_resolved_preimage_opt));
+								res.insert(source.clone(), (htlc.payment_hash, htlc.amount_msat, counterparty_resolved_preimage_opt));
 							}
 						} else {
 							panic!("Outbound HTLCs should have a source");

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -4402,7 +4402,7 @@ fn test_claim_to_closed_channel_blocks_forwarded_preimage_removal() {
 	assert!(get_monitor!(nodes[1], chan_b.2)
 		.get_all_current_outbound_htlcs()
 		.iter()
-		.any(|(_, (_, preimage))| *preimage == Some(payment_preimage)));
+		.any(|(_, (_, _, preimage))| *preimage == Some(payment_preimage)));
 
 	// Once we complete the `ChannelMonitorUpdate` on channel A, and the `ChannelManager` processes
 	// background events (via `get_and_clear_pending_msg_events`), the final `ChannelMonitorUpdate`
@@ -4418,7 +4418,7 @@ fn test_claim_to_closed_channel_blocks_forwarded_preimage_removal() {
 	assert!(!get_monitor!(nodes[1], chan_b.2)
 		.get_all_current_outbound_htlcs()
 		.iter()
-		.any(|(_, (_, preimage))| *preimage == Some(payment_preimage)));
+		.any(|(_, (_, _, preimage))| *preimage == Some(payment_preimage)));
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], None, true, false);
 }
 

lightning/src/ln/channelmanager.rs

@@ -16273,13 +16273,11 @@ where
 				}
 
 				if is_channel_closed {
-					for (htlc_source, (htlc, _)) in monitor.get_pending_or_resolved_outbound_htlcs()
+					for (htlc_source, (payment_hash, _, _)) in
+						monitor.get_pending_or_resolved_outbound_htlcs()
 					{
-						let logger = WithChannelMonitor::from(
-							&args.logger,
-							monitor,
-							Some(htlc.payment_hash),
-						);
+						let logger =
+							WithChannelMonitor::from(&args.logger, monitor, Some(payment_hash));
 						if let HTLCSource::OutboundRoute {
 							payment_id, session_priv, path, ..
 						} = htlc_source
@@ -16293,22 +16291,19 @@ where
 							session_priv_bytes[..].copy_from_slice(&session_priv[..]);
 							pending_outbounds.insert_from_monitor_on_startup(
 								payment_id,
-								htlc.payment_hash,
+								payment_hash,
 								session_priv_bytes,
 								&path,
 								best_block_height,
 								logger,
 							);
 						}
 					}
-					for (htlc_source, (htlc, preimage_opt)) in
+					for (htlc_source, (payment_hash, _, preimage_opt)) in
 						monitor.get_all_current_outbound_htlcs()
 					{
-						let logger = WithChannelMonitor::from(
-							&args.logger,
-							monitor,
-							Some(htlc.payment_hash),
-						);
+						let logger =
+							WithChannelMonitor::from(&args.logger, monitor, Some(payment_hash));
 						match htlc_source {
 							HTLCSource::PreviousHopData(prev_hop_data) => {
 								let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| {
@@ -16326,7 +16321,7 @@ where
 											update_add_htlc.htlc_id == prev_hop_data.htlc_id;
 										if matches {
 											log_info!(logger, "Removing pending to-decode HTLC with hash {} as it was forwarded to the closed channel {}",
-												&htlc.payment_hash, &monitor.channel_id());
+												&payment_hash, &monitor.channel_id());
 										}
 										!matches
 									});
@@ -16337,7 +16332,7 @@ where
 										if let HTLCForwardInfo::AddHTLC(htlc_info) = forward {
 											if pending_forward_matches_htlc(&htlc_info) {
 												log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}",
-													&htlc.payment_hash, &monitor.channel_id());
+													&payment_hash, &monitor.channel_id());
 												false
 											} else { true }
 										} else { true }
@@ -16347,7 +16342,7 @@ where
 								pending_intercepted_htlcs.as_mut().unwrap().retain(|intercepted_id, htlc_info| {
 									if pending_forward_matches_htlc(&htlc_info) {
 										log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}",
-											&htlc.payment_hash, &monitor.channel_id());
+											&payment_hash, &monitor.channel_id());
 										pending_events_read.retain(|(event, _)| {
 											if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event {
 												intercepted_id != ev_id
@@ -16404,7 +16399,7 @@ where
 				let mut fail_read = false;
 				let outbound_claimed_htlcs_iter = monitor.get_all_current_outbound_htlcs()
 					.into_iter()
-					.filter_map(|(htlc_source, (htlc, preimage_opt))| {
+					.filter_map(|(htlc_source, (_, amount_msat, preimage_opt))| {
 						if let HTLCSource::PreviousHopData(prev_hop) = &htlc_source {
 							if let Some(payment_preimage) = preimage_opt {
 								let inbound_edge_monitor = args.channel_monitors.get(&prev_hop.channel_id);
@@ -16492,7 +16487,7 @@ where
 									);
 								}
 
-								Some((htlc_source, payment_preimage, htlc.amount_msat,
+								Some((htlc_source, payment_preimage, amount_msat,
 									is_channel_closed, monitor.get_counterparty_node_id(),
 									monitor.get_funding_txo(), monitor.channel_id()))
 							} else { None }