Fix PaymentForwarded fields on restart claim

Previously, we were spuriously using the upstream channel's info when we
should've been using the downstream channel's.

Author: valentinewallace

lightning/src/ln/channel.rs

@@ -7942,7 +7942,7 @@ where
 	/// when reconstructing the set of pending HTLCs when deserializing the `ChannelManager`.
 	pub(super) fn inbound_forwarded_htlcs(
 		&self,
-	) -> impl Iterator<Item = (PaymentHash, HTLCPreviousHopData, u64)> + '_ {
+	) -> impl Iterator<Item = (PaymentHash, HTLCPreviousHopData, OutboundHop)> + '_ {
 		// We don't want to return an HTLC as needing processing if it already has a resolution that's
 		// pending in the holding cell.
 		let htlc_resolution_in_holding_cell = |id: u64| -> bool {
@@ -7970,7 +7970,7 @@ where
 						phantom_shared_secret,
 						trampoline_shared_secret,
 						blinded_failure,
-						outbound_hop: OutboundHop { amt_msat, .. },
+						outbound_hop,
 					},
 			} => {
 				if htlc_resolution_in_holding_cell(htlc.htlc_id) {
@@ -7991,7 +7991,7 @@ where
 					counterparty_node_id: Some(counterparty_node_id),
 					cltv_expiry: Some(htlc.cltv_expiry),
 				};
-				Some((htlc.payment_hash, prev_hop_data, *amt_msat))
+				Some((htlc.payment_hash, prev_hop_data, *outbound_hop))
 			},
 			_ => None,
 		})

lightning/src/ln/channelmanager.rs

@@ -18610,11 +18610,11 @@ impl<
 		// that it is handled.
 		let mut already_forwarded_htlcs: HashMap<
 			(ChannelId, PaymentHash),
-			Vec<(HTLCPreviousHopData, u64)>,
+			Vec<(HTLCPreviousHopData, OutboundHop)>,
 		> = new_hash_map();
 		let prune_forwarded_htlc = |already_forwarded_htlcs: &mut HashMap<
 			(ChannelId, PaymentHash),
-			Vec<(HTLCPreviousHopData, u64)>,
+			Vec<(HTLCPreviousHopData, OutboundHop)>,
 		>,
 		                            prev_hop: &HTLCPreviousHopData,
 		                            payment_hash: &PaymentHash| {
@@ -18663,13 +18663,13 @@ impl<
 										.or_insert_with(Vec::new)
 										.push(update_add_htlc);
 								}
-								for (payment_hash, prev_hop, outbound_amt_msat) in
+								for (payment_hash, prev_hop, next_hop) in
 									funded_chan.inbound_forwarded_htlcs()
 								{
 									already_forwarded_htlcs
 										.entry((prev_hop.channel_id, payment_hash))
 										.or_insert_with(Vec::new)
-										.push((prev_hop, outbound_amt_msat));
+										.push((prev_hop, next_hop));
 								}
 							}
 						}
@@ -19378,34 +19378,33 @@ impl<
 				if let Some(forwarded_htlcs) =
 					already_forwarded_htlcs.remove(&(*channel_id, payment_hash))
 				{
-					for (prev_hop, outbound_amt_msat) in forwarded_htlcs {
+					for (prev_hop, next_hop) in forwarded_htlcs {
 						let new_pending_claim =
 							!pending_claims_to_replay.iter().any(|(src, _, _, _, _, _, _, _)| {
 								matches!(src, HTLCSource::PreviousHopData(hop) if hop.htlc_id == prev_hop.htlc_id && hop.channel_id == prev_hop.channel_id)
 							});
 						if new_pending_claim {
-							let counterparty_node_id = monitor.get_counterparty_node_id();
 							let is_downstream_closed = channel_manager
 								.per_peer_state
 								.read()
 								.unwrap()
-								.get(&counterparty_node_id)
+								.get(&next_hop.node_id)
 								.map_or(true, |peer_state_mtx| {
 									!peer_state_mtx
 										.lock()
 										.unwrap()
 										.channel_by_id
-										.contains_key(channel_id)
+										.contains_key(&next_hop.channel_id)
 								});
 							pending_claims_to_replay.push((
 								HTLCSource::PreviousHopData(prev_hop),
 								payment_preimage,
-								outbound_amt_msat,
+								next_hop.amt_msat,
 								is_downstream_closed,
-								counterparty_node_id,
-								monitor.get_funding_txo(),
-								*channel_id,
-								None,
+								next_hop.node_id,
+								next_hop.funding_txo,
+								next_hop.channel_id,
+								Some(next_hop.user_channel_id),
 							));
 						}
 					}

lightning/src/ln/reload_tests.rs

@@ -1958,14 +1958,8 @@ fn test_reload_node_with_preimage_in_monitor_claims_htlc() {
 	);
 
 	// When the claim is reconstructed during reload, a PaymentForwarded event is generated.
-	// This event has next_user_channel_id as None since the outbound HTLC was already removed.
 	// Fetching events triggers the pending monitor update (adding preimage) to be applied.
-	let events = nodes[1].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match &events[0] {
-		Event::PaymentForwarded { total_fee_earned_msat: Some(1000), .. } => {},
-		_ => panic!("Expected PaymentForwarded event"),
-	}
+	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);
 	check_added_monitors(&nodes[1], 1);
 
 	// Reconnect nodes[1] to nodes[0]. The claim should be in nodes[1]'s holding cell.