Include PaymentPathRetry data in PaymentPathFailed
authorJeffrey Czyz <jkczyz@gmail.com>
Thu, 21 Oct 2021 22:52:53 +0000 (17:52 -0500)
committerJeffrey Czyz <jkczyz@gmail.com>
Mon, 25 Oct 2021 15:18:11 +0000 (10:18 -0500)
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.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/routing/network_graph.rs
lightning/src/routing/router.rs
lightning/src/util/events.rs

index 418ef5f9b4af031b88c15b2876944d190db20226..385a031cabbec5db8cb1b78e6a5f88aaf1e4b441 100644 (file)
@@ -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)]
index aeca6e85b7a0b1814bbf06d1ba17b8e7a15cea78..6d509f8f07b62edf442810a1c9e216f238f6a034 100644 (file)
@@ -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);
index 163c4ae15d5e1b1af80d3ab6e50c20a8694be8df..39c1aa378d1bf490ac9e18ffe32d2285d99cfbeb 100644 (file)
@@ -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);
index f65b8fa657a5fc2e77da24a33578ecf51089e8c4..216cb45d4bdd3f54799c10bad0e9cb24a54af48e 100644 (file)
@@ -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,
                        });
index 6976943dd526cc1f32794238c75b964387052fb8..4d1deb61d2520e57608a7bea52c43c628d848950 100644 (file)
@@ -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);
index 7ea369f551408bed9533c520a6adc8232ebf7b44..df2532e4c4a755cfa4218b297b000aa47ef69fb9 100644 (file)
@@ -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,6 +461,7 @@ 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),
@@ -459,6 +469,7 @@ impl MaybeReadable for Event {
                                                (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)]