From: Valentine Wallace Date: Fri, 17 Sep 2021 01:09:46 +0000 (-0400) Subject: Add all_paths_failed field to PaymentFailed X-Git-Tag: v0.0.101~15^2 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=c828ff42c006abf3ceffe552dfe510f9f3538e11;p=rust-lightning Add all_paths_failed field to PaymentFailed see field docs for details --- diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0389bd5cd..f985976a7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2848,6 +2848,7 @@ impl ChannelMana payment_hash, rejected_by_dest: false, network_update: None, + all_paths_failed: sessions.get().len() == 0, #[cfg(test)] error_code: None, #[cfg(test)] @@ -2886,12 +2887,14 @@ impl 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 { @@ -2914,6 +2917,7 @@ impl 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)] @@ -2939,6 +2943,7 @@ impl 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)] diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index ffca2ecd5..dd1fedd97 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -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"); @@ -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"); @@ -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"), } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 7db74900c..85eef456c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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 @@ -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); @@ -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); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index bcc05fdb0..70d5a0c0b 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -163,8 +163,9 @@ 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::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 { diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index ca656039d..16ff80189 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -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, }), @@ -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, @@ -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, diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index bf1bd0743..df4d90373 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -141,6 +141,10 @@ pub enum Event { /// [`NetworkGraph`]: crate::routing::network_graph::NetworkGraph /// [`NetGraphMsgHandler`]: crate::routing::network_graph::NetGraphMsgHandler network_update: Option, + /// 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, #[cfg(test)] @@ -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)] @@ -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: _ } => { @@ -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)]