Include PaymentPathRetry data in PaymentPathFailed

When a payment path fails, it may be retried. Typically, this means
re-computing the route after updating the NetworkGraph and channel
scores in order to avoid the failing hop. The last hop in
PaymentPathFailed's path field contains the pubkey, amount, and CLTV
values needed to pass to get_route. However, it does not contain the
payee's features and route hints from the invoice.

Include the entire set of parameters in PaymentPathRetry and add it to
the PaymentPathFailed event. Add a get_retry_route wrapper around
get_route that takes PaymentPathRetry. This allows an EventHandler to
retry failed payment paths using the payee's route hints and features.

Author: jkczyz

lightning/src/ln/channelmanager.rs

@@ -3036,6 +3036,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 									all_paths_failed: payment.get().remaining_parts() == 0,
 									path: path.clone(),
 									short_channel_id: None,
+									retry: None,
 									#[cfg(test)]
 									error_code: None,
 									#[cfg(test)]
@@ -3103,6 +3104,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 								all_paths_failed,
 								path: path.clone(),
 								short_channel_id,
+								retry: None,
 #[cfg(test)]
 								error_code: onion_error_code,
 #[cfg(test)]
@@ -3131,6 +3133,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 								all_paths_failed,
 								path: path.clone(),
 								short_channel_id: Some(path.first().unwrap().short_channel_id),
+								retry: None,
 #[cfg(test)]
 								error_code: Some(*failure_code),
 #[cfg(test)]

lightning/src/ln/functional_tests.rs

@@ -6008,7 +6008,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
 	match &events[0] {
-		&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, ref error_code, ref error_data } => {
+		&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, retry: _, ref error_code, ref error_data } => {
 			assert_eq!(our_payment_hash.clone(), *payment_hash);
 			assert_eq!(*rejected_by_dest, false);
 			assert_eq!(*all_paths_failed, true);
@@ -6092,7 +6092,7 @@ fn test_free_and_fail_holding_cell_htlcs() {
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
 	match &events[0] {
-		&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, ref error_code, ref error_data } => {
+		&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, retry: _, ref error_code, ref error_data } => {
 			assert_eq!(payment_hash_2.clone(), *payment_hash);
 			assert_eq!(*rejected_by_dest, false);
 			assert_eq!(*all_paths_failed, true);

lightning/src/ln/onion_route_tests.rs

@@ -162,7 +162,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
 
 	let events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 1);
-	if let &Event::PaymentPathFailed { payment_hash: _, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, ref error_code, error_data: _ } = &events[0] {
+	if let &Event::PaymentPathFailed { payment_hash: _, ref rejected_by_dest, ref network_update, ref all_paths_failed, path: _, ref short_channel_id, retry: _, ref error_code, error_data: _ } = &events[0] {
 		assert_eq!(*rejected_by_dest, !expected_retryable);
 		assert_eq!(*all_paths_failed, true);
 		assert_eq!(*error_code, expected_error_code);

lightning/src/routing/network_graph.rs

@@ -1814,6 +1814,7 @@ mod tests {
 					msg: valid_channel_update,
 				}),
 				short_channel_id: None,
+				retry: None,
 				error_code: None,
 				error_data: None,
 			});
@@ -1840,6 +1841,7 @@ mod tests {
 					is_permanent: false,
 				}),
 				short_channel_id: None,
+				retry: None,
 				error_code: None,
 				error_data: None,
 			});
@@ -1864,6 +1866,7 @@ mod tests {
 					is_permanent: true,
 				}),
 				short_channel_id: None,
+				retry: None,
 				error_code: None,
 				error_data: None,
 			});

lightning/src/routing/router.rs

@@ -129,7 +129,31 @@ impl Readable for Route {
 	}
 }
 
+/// Parameters needed to re-compute a [`Route`] for retrying a failed payment path.
+///
+/// Provided in [`Event::PaymentPathFailed`] and passed to [`get_retry_route`].
+///
+/// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
+#[derive(Clone, Debug)]
+pub struct PaymentPathRetry {
+	/// The recipient of the failed payment path.
+	pub payee: Payee,
+
+	/// The amount in msats sent on the failed payment path.
+	pub final_value_msat: u64,
+
+	/// The CLTV on the final hop of the failed payment path.
+	pub final_cltv_expiry_delta: u32,
+}
+
+impl_writeable_tlv_based!(PaymentPathRetry, {
+	(0, payee, required),
+	(2, final_value_msat, required),
+	(4, final_cltv_expiry_delta, required),
+});
+
 /// The recipient of a payment.
+#[derive(Clone, Debug)]
 pub struct Payee {
 	/// The node id of the payee.
 	pubkey: PublicKey,
@@ -146,6 +170,12 @@ pub struct Payee {
 	pub route_hints: Vec<RouteHint>,
 }
 
+impl_writeable_tlv_based!(Payee, {
+	(0, pubkey, required),
+	(2, features, option),
+	(4, route_hints, vec_type),
+});
+
 impl Payee {
 	/// Creates a payee with the node id of the given `pubkey`.
 	pub fn new(pubkey: PublicKey) -> Self {
@@ -180,6 +210,28 @@ impl Payee {
 #[derive(Clone, Debug, Hash, Eq, PartialEq)]
 pub struct RouteHint(pub Vec<RouteHintHop>);
 
+
+impl Writeable for RouteHint {
+	fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		(self.0.len() as u64).write(writer)?;
+		for hop in self.0.iter() {
+			hop.write(writer)?;
+		}
+		Ok(())
+	}
+}
+
+impl Readable for RouteHint {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		let hop_count: u64 = Readable::read(reader)?;
+		let mut hops = Vec::with_capacity(cmp::min(hop_count, 16) as usize);
+		for _ in 0..hop_count {
+			hops.push(Readable::read(reader)?);
+		}
+		Ok(Self(hops))
+	}
+}
+
 /// A channel descriptor for a hop along a payment path.
 #[derive(Clone, Debug, Hash, Eq, PartialEq)]
 pub struct RouteHintHop {
@@ -197,6 +249,15 @@ pub struct RouteHintHop {
 	pub htlc_maximum_msat: Option<u64>,
 }
 
+impl_writeable_tlv_based!(RouteHintHop, {
+	(0, src_node_id, required),
+	(1, htlc_minimum_msat, option),
+	(2, short_channel_id, required),
+	(3, htlc_maximum_msat, option),
+	(4, fees, required),
+	(6, cltv_expiry_delta, required),
+});
+
 #[derive(Eq, PartialEq)]
 struct RouteGraphNode {
 	node_id: NodeId,
@@ -413,13 +474,31 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 pub fn get_keysend_route<L: Deref, S: routing::Score>(
 	our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey,
 	first_hops: Option<&[&ChannelDetails]>, last_hops: &[&RouteHint], final_value_msat: u64,
-	final_cltv: u32, logger: L, scorer: &S
+	final_cltv_expiry_delta: u32, logger: L, scorer: &S
 ) -> Result<Route, LightningError>
 where L::Target: Logger {
 	let route_hints = last_hops.iter().map(|hint| (*hint).clone()).collect();
 	let payee = Payee::for_keysend(*payee).with_route_hints(route_hints);
 	get_route(
-		our_node_pubkey, &payee, network, first_hops, final_value_msat, final_cltv, logger, scorer
+		our_node_pubkey, &payee, network, first_hops, final_value_msat, final_cltv_expiry_delta,
+		logger, scorer
+	)
+}
+
+/// Gets a route suitable for retrying a failed payment path.
+///
+/// Used to re-compute a [`Route`] when handling a [`Event::PaymentPathFailed`]. Any adjustments to
+/// the [`NetworkGraph`] and channel scores should be made prior to calling this function.
+///
+/// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
+pub fn get_retry_route<L: Deref, S: routing::Score>(
+	our_node_pubkey: &PublicKey, retry: &PaymentPathRetry, network: &NetworkGraph,
+	first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S
+) -> Result<Route, LightningError>
+where L::Target: Logger {
+	get_route(
+		our_node_pubkey, &retry.payee, network, first_hops, retry.final_value_msat,
+		retry.final_cltv_expiry_delta, logger, scorer
 	)
 }
 
@@ -443,8 +522,8 @@ where L::Target: Logger {
 /// htlc_minimum_msat/htlc_maximum_msat *are* checked as they may change based on the receiving node.
 pub fn get_route<L: Deref, S: routing::Score>(
 	our_node_pubkey: &PublicKey, payee: &Payee, network: &NetworkGraph,
-	first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv: u32, logger: L,
-	scorer: &S
+	first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
+	logger: L, scorer: &S
 ) -> Result<Route, LightningError>
 where L::Target: Logger {
 	let payee_node_id = NodeId::from_pubkey(&payee.pubkey);
@@ -1154,7 +1233,7 @@ where L::Target: Logger {
 				}
 				ordered_hops.last_mut().unwrap().0.fee_msat = value_contribution_msat;
 				ordered_hops.last_mut().unwrap().0.hop_use_fee_msat = 0;
-				ordered_hops.last_mut().unwrap().0.cltv_expiry_delta = final_cltv;
+				ordered_hops.last_mut().unwrap().0.cltv_expiry_delta = final_cltv_expiry_delta;
 
 				log_trace!(logger, "Found a path back to us from the target with {} hops contributing up to {} msat: {:?}",
 					ordered_hops.len(), value_contribution_msat, ordered_hops);

lightning/src/util/events.rs

@@ -20,7 +20,7 @@ use ln::msgs::DecodeError;
 use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 use routing::network_graph::NetworkUpdate;
 use util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper};
-use routing::router::RouteHop;
+use routing::router::{PaymentPathRetry, RouteHop};
 
 use bitcoin::blockdata::script::Script;
 use bitcoin::hashes::Hash;
@@ -216,6 +216,13 @@ pub enum Event {
 		/// If this is `Some`, then the corresponding channel should be avoided when the payment is
 		/// retried. May be `None` for older [`Event`] serializations.
 		short_channel_id: Option<u64>,
+		/// Parameters needed to re-compute a [`Route`] for retrying the failed path.
+		///
+		/// See [`get_retry_route`] for details.
+		///
+		/// [`Route`]: crate::routing::router::Route
+		/// [`get_retry_route`]: crate::routing::router::get_retry_route
+		retry: Option<PaymentPathRetry>,
 #[cfg(test)]
 		error_code: Option<u16>,
 #[cfg(test)]
@@ -322,8 +329,9 @@ impl Writeable for Event {
 					(1, payment_hash, required),
 				});
 			},
-			&Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update,
-			                            ref all_paths_failed, ref path, ref short_channel_id,
+			&Event::PaymentPathFailed {
+				ref payment_hash, ref rejected_by_dest, ref network_update,
+				ref all_paths_failed, ref path, ref short_channel_id, ref retry,
 				#[cfg(test)]
 				ref error_code,
 				#[cfg(test)]
@@ -341,6 +349,7 @@ impl Writeable for Event {
 					(3, all_paths_failed, required),
 					(5, path, vec_type),
 					(7, short_channel_id, option),
+					(9, retry, option),
 				});
 			},
 			&Event::PendingHTLCsForwardable { time_forwardable: _ } => {
@@ -452,13 +461,15 @@ impl MaybeReadable for Event {
 					let mut all_paths_failed = Some(true);
 					let mut path: Option<Vec<RouteHop>> = Some(vec![]);
 					let mut short_channel_id = None;
+					let mut retry = None;
 					read_tlv_fields!(reader, {
 						(0, payment_hash, required),
 						(1, network_update, ignorable),
 						(2, rejected_by_dest, required),
 						(3, all_paths_failed, option),
 						(5, path, vec_type),
 						(7, short_channel_id, ignorable),
+						(9, retry, option),
 					});
 					Ok(Some(Event::PaymentPathFailed {
 						payment_hash,
@@ -467,6 +478,7 @@ impl MaybeReadable for Event {
 						all_paths_failed: all_paths_failed.unwrap(),
 						path: path.unwrap(),
 						short_channel_id,
+						retry,
 						#[cfg(test)]
 						error_code,
 						#[cfg(test)]