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.
This commit is contained in:
Jeffrey Czyz 2021-10-21 17:52:53 -05:00
parent 8b9cf93b11
commit 46b68c517d
No known key found for this signature in database
GPG key ID: 3A4E08275D5E96D2
6 changed files with 108 additions and 11 deletions

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

View file

@ -6008,7 +6008,7 @@ fn test_fail_holding_cell_htlc_upon_free() {
let events = nodes[0].node.get_and_clear_pending_events(); let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1); assert_eq!(events.len(), 1);
match &events[0] { 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!(our_payment_hash.clone(), *payment_hash);
assert_eq!(*rejected_by_dest, false); assert_eq!(*rejected_by_dest, false);
assert_eq!(*all_paths_failed, true); 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(); let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1); assert_eq!(events.len(), 1);
match &events[0] { 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!(payment_hash_2.clone(), *payment_hash);
assert_eq!(*rejected_by_dest, false); assert_eq!(*rejected_by_dest, false);
assert_eq!(*all_paths_failed, true); assert_eq!(*all_paths_failed, true);

View 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(); let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1); 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!(*rejected_by_dest, !expected_retryable);
assert_eq!(*all_paths_failed, true); assert_eq!(*all_paths_failed, true);
assert_eq!(*error_code, expected_error_code); assert_eq!(*error_code, expected_error_code);

View file

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

View 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. /// The recipient of a payment.
#[derive(Clone, Debug)]
pub struct Payee { pub struct Payee {
/// The node id of the payee. /// The node id of the payee.
pubkey: PublicKey, pubkey: PublicKey,
@ -146,6 +170,12 @@ pub struct Payee {
pub route_hints: Vec<RouteHint>, pub route_hints: Vec<RouteHint>,
} }
impl_writeable_tlv_based!(Payee, {
(0, pubkey, required),
(2, features, option),
(4, route_hints, vec_type),
});
impl Payee { impl Payee {
/// Creates a payee with the node id of the given `pubkey`. /// Creates a payee with the node id of the given `pubkey`.
pub fn new(pubkey: PublicKey) -> Self { pub fn new(pubkey: PublicKey) -> Self {
@ -180,6 +210,28 @@ impl Payee {
#[derive(Clone, Debug, Hash, Eq, PartialEq)] #[derive(Clone, Debug, Hash, Eq, PartialEq)]
pub struct RouteHint(pub Vec<RouteHintHop>); 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. /// A channel descriptor for a hop along a payment path.
#[derive(Clone, Debug, Hash, Eq, PartialEq)] #[derive(Clone, Debug, Hash, Eq, PartialEq)]
pub struct RouteHintHop { pub struct RouteHintHop {
@ -197,6 +249,15 @@ pub struct RouteHintHop {
pub htlc_maximum_msat: Option<u64>, 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)] #[derive(Eq, PartialEq)]
struct RouteGraphNode { struct RouteGraphNode {
node_id: NodeId, 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>( pub fn get_keysend_route<L: Deref, S: routing::Score>(
our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey, our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey,
first_hops: Option<&[&ChannelDetails]>, last_hops: &[&RouteHint], final_value_msat: u64, 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> ) -> Result<Route, LightningError>
where L::Target: Logger { where L::Target: Logger {
let route_hints = last_hops.iter().map(|hint| (*hint).clone()).collect(); let route_hints = last_hops.iter().map(|hint| (*hint).clone()).collect();
let payee = Payee::for_keysend(*payee).with_route_hints(route_hints); let payee = Payee::for_keysend(*payee).with_route_hints(route_hints);
get_route( 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. /// 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>( pub fn get_route<L: Deref, S: routing::Score>(
our_node_pubkey: &PublicKey, payee: &Payee, network: &NetworkGraph, our_node_pubkey: &PublicKey, payee: &Payee, network: &NetworkGraph,
first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv: u32, logger: L, first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
scorer: &S logger: L, scorer: &S
) -> Result<Route, LightningError> ) -> Result<Route, LightningError>
where L::Target: Logger { where L::Target: Logger {
let payee_node_id = NodeId::from_pubkey(&payee.pubkey); 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.fee_msat = value_contribution_msat;
ordered_hops.last_mut().unwrap().0.hop_use_fee_msat = 0; 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: {:?}", 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); ordered_hops.len(), value_contribution_msat, ordered_hops);

View file

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