From 46b68c517dba57bde545ea8e56b47fe3af768579 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 21 Oct 2021 17:52:53 -0500 Subject: [PATCH] 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. --- lightning/src/ln/channelmanager.rs | 3 + lightning/src/ln/functional_tests.rs | 4 +- lightning/src/ln/onion_route_tests.rs | 2 +- lightning/src/routing/network_graph.rs | 3 + lightning/src/routing/router.rs | 89 ++++++++++++++++++++++++-- lightning/src/util/events.rs | 18 +++++- 6 files changed, 108 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 418ef5f9..385a031c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3036,6 +3036,7 @@ impl 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 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 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)] diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index aeca6e85..6d509f8f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/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); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 163c4ae1..39c1aa37 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -162,7 +162,7 @@ fn run_onion_failure_test_with_fail_intercept(_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); diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index f65b8fa6..216cb45d 100644 --- a/lightning/src/routing/network_graph.rs +++ b/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, }); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 6976943d..4d1deb61 100644 --- a/lightning/src/routing/router.rs +++ b/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, } +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); + +impl Writeable for RouteHint { + fn write(&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(reader: &mut R) -> Result { + 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, } +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 { pub fn get_keysend_route( 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 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( + our_node_pubkey: &PublicKey, retry: &PaymentPathRetry, network: &NetworkGraph, + first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S +) -> Result +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( 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 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); diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 7ea369f5..df2532e4 100644 --- a/lightning/src/util/events.rs +++ b/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, + /// 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, #[cfg(test)] error_code: Option, #[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> = 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)] -- 2.30.2