ln+events: deprecate UnknownNextPeer in HTLCHandlingType

This variant of HTLCHandlingType contains infromation about the
failure cause along with its type - as an UnknownNextPeer is just an
InvalidForward that has the failure type UnknownNextPeer.

This commit deprecates the variant's use, while still writing it to
disk to allow the option to downgrade.

Author: carlaKC

lightning-liquidity/src/lsps2/service.rs

@@ -879,9 +879,7 @@ where
 	/// or if the payment queue is empty
 	///
 	/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
-	pub fn htlc_handling_failed(
-		&self, handling_type: HTLCHandlingType,
-	) -> Result<(), APIError> {
+	pub fn htlc_handling_failed(&self, handling_type: HTLCHandlingType) -> Result<(), APIError> {
 		if let HTLCHandlingType::ForwardFailed { channel_id, .. } = handling_type {
 			let peer_by_channel_id = self.peer_by_channel_id.read().unwrap();
 			if let Some(counterparty_node_id) = peer_by_channel_id.get(&channel_id) {

lightning/src/events/mod.rs

@@ -479,12 +479,16 @@ pub enum HTLCHandlingType {
 		channel_id: ChannelId,
 	},
 	/// Scenario where we are unsure of the next node to forward the HTLC to.
+	///
+	/// Deprecated: will only be used in versions before LDK v0.2.0.
 	UnknownNextHop {
 		/// Short channel id we are requesting to forward an HTLC to.
 		requested_forward_scid: u64,
 	},
 	/// We couldn't forward to the outgoing scid. An example would be attempting to send a duplicate
 	/// intercept HTLC.
+	///
+	/// In LDK v0.2.0 and greater, this variant replaces [`Self::UnknownNextHop`].
 	InvalidForward {
 		/// Short channel id we are requesting to forward an HTLC to.
 		requested_forward_scid: u64
@@ -1777,10 +1781,27 @@ impl Writeable for Event {
 			},
 			&Event::HTLCHandlingFailed { ref prev_channel_id, ref handling_type, ref handling_failure } => {
 				25u8.write(writer)?;
+
+				// The [`HTLCHandlingType::UnknownNextPeer`] variant is deprecated, but we want to
+				// continue writing it to allow downgrading. Detect the case where we're
+				// representing it as [`HTLCHandlingType::InvalidForward`] and
+				// [`LocalHTLCFailureReason::UnknownNextHop`] and write the old variant instead.
+				let downgradable_type = match (handling_type, handling_failure) {
+					(HTLCHandlingType::InvalidForward { requested_forward_scid },
+						Some(HTLCHandlingFailureReason::Local { reason }))
+					if *reason == LocalHTLCFailureReason::UnknownNextPeer =>
+					{
+						HTLCHandlingType::UnknownNextHop {
+							requested_forward_scid: *requested_forward_scid,
+						}
+    				}
+    			_ => handling_type.clone()
+				};
+
 				write_tlv_fields!(writer, {
 					(0, prev_channel_id, required),
 					(1, handling_failure, option),
-					(2, handling_type, required),
+					(2, downgradable_type, required),
 				})
 			},
 			&Event::BumpTransaction(ref event)=> {
@@ -2232,11 +2253,32 @@ impl MaybeReadable for Event {
 						(1, handling_failure, option),
 						(2, handling_type_opt, upgradable_required),
 					});
-					Ok(Some(Event::HTLCHandlingFailed {
+
+					let mut event = Event::HTLCHandlingFailed {
 						prev_channel_id,
 						handling_type: _init_tlv_based_struct_field!(handling_type_opt, upgradable_required),
 						handling_failure,
-					}))
+					};
+
+					// The [`HTLCHandlingType::UnknownNextPeer`] variant is deprecated, but we
+					// continue writing it to allow downgrading. If it was written, upgrade
+					// it to its new representation of [`HTLCHandlingType::InvalidForward`] and
+					// [`LocalHTLCFailureReason::UnknownNextHop`]. This will cover both the case
+					// where we have a legacy event
+					match event {
+						Event::HTLCHandlingFailed { ref handling_type, .. } => {
+							if let HTLCHandlingType::UnknownNextHop { requested_forward_scid } = handling_type {
+								event = Event::HTLCHandlingFailed {
+									prev_channel_id,
+									handling_type: HTLCHandlingType::InvalidForward { requested_forward_scid: *requested_forward_scid },
+									handling_failure: Some(LocalHTLCFailureReason::UnknownNextPeer.into()),
+								}
+							}
+						}
+						_ => panic!("HTLCHandlingFailed wrong type")
+					}
+
+					Ok(Some(event))
 				};
 				f()
 			},

lightning/src/ln/blinded_payment_tests.rs

@@ -626,7 +626,7 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck,
 
 					$curr_node.node.process_pending_htlc_forwards();
 					expect_htlc_handling_failed_destinations!($curr_node.node.get_and_clear_pending_events(),
-						vec![HTLCHandlingType::UnknownNextHop { requested_forward_scid: $failed_scid }]);
+						vec![HTLCHandlingType::InvalidForward { requested_forward_scid: $failed_scid }]);
 					$curr_node.node.process_pending_htlc_forwards();
 				},
 			}
@@ -725,7 +725,7 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) {
 
 	if intercept_node_fails {
 		nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap();
-		expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCHandlingType::UnknownNextHop { requested_forward_scid: intercept_scid }]);
+		expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCHandlingType::InvalidForward { requested_forward_scid: intercept_scid }]);
 		nodes[1].node.process_pending_htlc_forwards();
 		check_added_monitors!(&nodes[1], 1);
 		fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1]], false);

lightning/src/ln/channelmanager.rs

@@ -5722,7 +5722,7 @@ where
 			});
 
 			let reason = HTLCFailReason::from_failure_code(LocalHTLCFailureReason::UnknownNextPeer);
-			let destination = HTLCHandlingType::UnknownNextHop { requested_forward_scid: short_channel_id };
+			let destination = HTLCHandlingType::InvalidForward { requested_forward_scid: short_channel_id };
 			self.fail_htlc_backwards_internal(&htlc_source, &payment.forward_info.payment_hash, &reason, destination);
 		} else { unreachable!() } // Only `PendingHTLCRouting::Forward`s are intercepted
 
@@ -5741,7 +5741,7 @@ where
 							node_id: Some(*outgoing_counterparty_node_id),
 							channel_id: *outgoing_channel_id,
 						},
-					None => HTLCHandlingType::UnknownNextHop {
+					None => HTLCHandlingType::InvalidForward {
 						requested_forward_scid: outgoing_scid,
 					},
 				}

lightning/src/ln/onion_route_tests.rs

@@ -538,7 +538,7 @@ fn test_onion_failure() {
 	bogus_route.paths[0].hops[1].short_channel_id -= 1;
 	let short_channel_id = bogus_route.paths[0].hops[1].short_channel_id;
 	run_onion_failure_test("unknown_next_peer", 100, &nodes, &bogus_route, &payment_hash, &payment_secret, |_| {}, ||{}, true, Some(LocalHTLCFailureReason::UnknownNextPeer),
-	  Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent:true}), Some(short_channel_id), Some(HTLCHandlingType::UnknownNextHop { requested_forward_scid: short_channel_id }));
+	  Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent:true}), Some(short_channel_id), Some(HTLCHandlingType::InvalidForward { requested_forward_scid: short_channel_id }));
 
 	let short_channel_id = channels[1].0.contents.short_channel_id;
 	let amt_to_forward = nodes[1].node.per_peer_state.read().unwrap().get(&nodes[2].node.get_our_node_id())
@@ -1730,7 +1730,7 @@ fn test_phantom_failure_modified_cltv() {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_htlc_handling_failed_destinations!(
 		nodes[1].node.get_and_clear_pending_events(),
-		&[HTLCHandlingType::UnknownNextHop { requested_forward_scid: phantom_scid }]
+		&[HTLCHandlingType::InvalidForward { requested_forward_scid: phantom_scid }]
 	);
 	check_added_monitors(&nodes[1], 1);
 
@@ -1779,7 +1779,7 @@ fn test_phantom_failure_expires_too_soon() {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_htlc_handling_failed_destinations!(
 		nodes[1].node.get_and_clear_pending_events(),
-		&[HTLCHandlingType::UnknownNextHop { requested_forward_scid: phantom_scid }]
+		&[HTLCHandlingType::InvalidForward { requested_forward_scid: phantom_scid }]
 	);
 	check_added_monitors(&nodes[1], 1);
 
@@ -1884,7 +1884,7 @@ fn do_test_phantom_dust_exposure_failure(multiplier_dust_limit: bool) {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_htlc_handling_failed_destinations!(
 		nodes[1].node.get_and_clear_pending_events(),
-		&[HTLCHandlingType::UnknownNextHop { requested_forward_scid: phantom_scid }]
+		&[HTLCHandlingType::InvalidForward { requested_forward_scid: phantom_scid }]
 	);
 	check_added_monitors(&nodes[1], 1);
 

lightning/src/ln/payment_tests.rs

@@ -1939,7 +1939,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
 	if test == InterceptTest::Fail {
 		// Ensure we can fail the intercepted payment back.
 		nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap();
-		expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCHandlingType::UnknownNextHop { requested_forward_scid: intercept_scid }]);
+		expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCHandlingType::InvalidForward { requested_forward_scid: intercept_scid }]);
 		nodes[1].node.process_pending_htlc_forwards();
 		let update_fail = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
 		check_added_monitors!(&nodes[1], 1);
@@ -3406,7 +3406,7 @@ fn test_threaded_payment_retries() {
 		nodes[1].node.process_pending_htlc_forwards();
 		expect_htlc_handling_failed_destinations!(
 			nodes[1].node.get_and_clear_pending_events(),
-			&[HTLCHandlingType::UnknownNextHop { requested_forward_scid: route.paths[0].hops[1].short_channel_id }]
+			&[HTLCHandlingType::InvalidForward { requested_forward_scid: route.paths[0].hops[1].short_channel_id }]
 		);
 		check_added_monitors(&nodes[1], 1);