ln+events: deprecate UnknownNextPeer in HTLCHandlingType

This variant of HTLCHandlingType contains information 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.

The deprecated attribute is not used because it cannot be silenced in
impl_writeable_tlv_based_enum, which uses UnknownNextPeer to remain
downgradable.

Author: carlaKC

lightning/src/events/mod.rs

@@ -480,12 +480,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
@@ -1797,10 +1801,26 @@ 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: 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)=> {
@@ -2255,11 +2275,31 @@ 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 and new events that are written with the legacy
+					// type be downgradable.
+					match event {
+						Event::HTLCHandlingFailed { handling_type: HTLCHandlingType::UnknownNextHop { requested_forward_scid }, .. } => {
+								event = Event::HTLCHandlingFailed {
+									prev_channel_id,
+									handling_type: HTLCHandlingType::InvalidForward { 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

@@ -5733,7 +5733,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
 
@@ -5752,7 +5752,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,
 					},
 				}
@@ -5931,7 +5931,7 @@ where
 												});
 
 												let reason = if $next_hop_unknown {
-													HTLCHandlingType::UnknownNextHop { requested_forward_scid: short_chan_id }
+													HTLCHandlingType::InvalidForward { requested_forward_scid: short_chan_id }
 												} else {
 													HTLCHandlingType::ReceiveFailed{ payment_hash }
 												};

lightning/src/ln/onion_route_tests.rs

@@ -549,7 +549,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())
@@ -1751,7 +1751,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);
 
@@ -1800,7 +1800,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);
 
@@ -1905,7 +1905,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);