Skip to content

Commit

Permalink
Add all_paths_failed field to PaymentFailed
Browse files Browse the repository at this point in the history
see field docs for details
  • Loading branch information
valentinewallace committed Sep 17, 2021
1 parent 8f17631 commit c828ff4
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 7 deletions.
5 changes: 5 additions & 0 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2848,6 +2848,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
payment_hash,
rejected_by_dest: false,
network_update: None,
all_paths_failed: sessions.get().len() == 0,
#[cfg(test)]
error_code: None,
#[cfg(test)]
Expand Down Expand Up @@ -2886,12 +2887,14 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
let mut session_priv_bytes = [0; 32];
session_priv_bytes.copy_from_slice(&session_priv[..]);
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
let mut all_paths_failed = false;
if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) {
if !sessions.get_mut().remove(&session_priv_bytes) {
log_trace!(self.logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
return;
}
if sessions.get().len() == 0 {
all_paths_failed = true;
sessions.remove();
}
} else {
Expand All @@ -2914,6 +2917,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
payment_hash: payment_hash.clone(),
rejected_by_dest: !payment_retryable,
network_update,
all_paths_failed,
#[cfg(test)]
error_code: onion_error_code,
#[cfg(test)]
Expand All @@ -2939,6 +2943,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
payment_hash: payment_hash.clone(),
rejected_by_dest: path.len() == 1,
network_update: None,
all_paths_failed,
#[cfg(test)]
error_code: Some(*failure_code),
#[cfg(test)]
Expand Down
7 changes: 4 additions & 3 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ macro_rules! expect_payment_failed_with_update {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data } => {
Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
assert!(error_code.is_some(), "expected error_code.is_some() = true");
Expand Down Expand Up @@ -1070,7 +1070,7 @@ macro_rules! expect_payment_failed {
let events = $node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data } => {
Event::PaymentFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => {
assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value");
assert!(error_code.is_some(), "expected error_code.is_some() = true");
Expand Down Expand Up @@ -1367,9 +1367,10 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe
let events = origin_node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
Event::PaymentFailed { payment_hash, rejected_by_dest, all_paths_failed, .. } => {
assert_eq!(payment_hash, our_payment_hash);
assert!(rejected_by_dest);
assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
},
_ => panic!("Unexpected event"),
}
Expand Down
34 changes: 32 additions & 2 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4084,6 +4084,34 @@ fn test_no_txn_manager_serialize_deserialize() {
send_payment(&nodes[0], &[&nodes[1]], 1000000);
}

#[test]
fn mpp_failure() {
let chanmon_cfgs = create_chanmon_cfgs(4);
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
let nodes = create_network(4, &node_cfgs, &node_chanmgrs);

let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 2, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let chan_3_id = create_announced_chan_between_nodes(&nodes, 1, 3, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let chan_4_id = create_announced_chan_between_nodes(&nodes, 2, 3, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
let logger = test_utils::TestLogger::new();

let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[3]);
let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
let mut route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[3].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
let path = route.paths[0].clone();
route.paths.push(path);
route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();
route.paths[0][0].short_channel_id = chan_1_id;
route.paths[0][1].short_channel_id = chan_3_id;
route.paths[1][0].pubkey = nodes[2].node.get_our_node_id();
route.paths[1][0].short_channel_id = chan_2_id;
route.paths[1][1].short_channel_id = chan_4_id;
send_along_route_with_secret(&nodes[0], route, &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], 200_000, payment_hash, payment_secret);
fail_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_hash);
}

#[test]
fn test_dup_htlc_onchain_fails_on_reload() {
// When a Channel is closed, any outbound HTLCs which were relayed through it are simply
Expand Down Expand Up @@ -5914,9 +5942,10 @@ 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::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data } => {
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
assert_eq!(our_payment_hash.clone(), *payment_hash);
assert_eq!(*rejected_by_dest, false);
assert_eq!(*all_paths_failed, true);
assert_eq!(*network_update, None);
assert_eq!(*error_code, None);
assert_eq!(*error_data, None);
Expand Down Expand Up @@ -6000,9 +6029,10 @@ 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::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data } => {
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
assert_eq!(payment_hash_2.clone(), *payment_hash);
assert_eq!(*rejected_by_dest, false);
assert_eq!(*all_paths_failed, true);
assert_eq!(*network_update, None);
assert_eq!(*error_code, None);
assert_eq!(*error_data, None);
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/ln/onion_route_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,9 @@ 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::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _ } = &events[0] {
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed } = &events[0] {
assert_eq!(*rejected_by_dest, !expected_retryable);
assert_eq!(*all_paths_failed, true);
assert_eq!(*error_code, expected_error_code);
if expected_channel_update.is_some() {
match network_update {
Expand Down
3 changes: 3 additions & 0 deletions lightning/src/routing/network_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,7 @@ mod tests {
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
payment_hash: PaymentHash([0; 32]),
rejected_by_dest: false,
all_paths_failed: true,
network_update: Some(NetworkUpdate::ChannelUpdateMessage {
msg: valid_channel_update,
}),
Expand All @@ -1750,6 +1751,7 @@ mod tests {
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
payment_hash: PaymentHash([0; 32]),
rejected_by_dest: false,
all_paths_failed: true,
network_update: Some(NetworkUpdate::ChannelClosed {
short_channel_id,
is_permanent: false,
Expand All @@ -1771,6 +1773,7 @@ mod tests {
net_graph_msg_handler.handle_event(&Event::PaymentFailed {
payment_hash: PaymentHash([0; 32]),
rejected_by_dest: false,
all_paths_failed: true,
network_update: Some(NetworkUpdate::ChannelClosed {
short_channel_id,
is_permanent: true,
Expand Down
10 changes: 9 additions & 1 deletion lightning/src/util/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ pub enum Event {
/// [`NetworkGraph`]: crate::routing::network_graph::NetworkGraph
/// [`NetGraphMsgHandler`]: crate::routing::network_graph::NetGraphMsgHandler
network_update: Option<NetworkUpdate>,
/// For both single-path and multi-path payments, this is set if all paths of the payment have
/// failed. This will be set to false if (1) this is an MPP payment and (2) other parts of the
/// larger MPP payment were still in flight when this event was generated.
all_paths_failed: bool,
#[cfg(test)]
error_code: Option<u16>,
#[cfg(test)]
Expand Down Expand Up @@ -224,7 +228,7 @@ impl Writeable for Event {
(0, payment_preimage, required),
});
},
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update,
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed,
#[cfg(test)]
ref error_code,
#[cfg(test)]
Expand All @@ -239,6 +243,7 @@ impl Writeable for Event {
(0, payment_hash, required),
(1, network_update, option),
(2, rejected_by_dest, required),
(3, all_paths_failed, required),
});
},
&Event::PendingHTLCsForwardable { time_forwardable: _ } => {
Expand Down Expand Up @@ -322,15 +327,18 @@ impl MaybeReadable for Event {
let mut payment_hash = PaymentHash([0; 32]);
let mut rejected_by_dest = false;
let mut network_update = None;
let mut all_paths_failed = Some(true);
read_tlv_fields!(reader, {
(0, payment_hash, required),
(1, network_update, ignorable),
(2, rejected_by_dest, required),
(3, all_paths_failed, option),
});
Ok(Some(Event::PaymentFailed {
payment_hash,
rejected_by_dest,
network_update,
all_paths_failed: all_paths_failed.unwrap(),
#[cfg(test)]
error_code,
#[cfg(test)]
Expand Down

0 comments on commit c828ff4

Please sign in to comment.