Rename PaymentFailed -> PaymentPathFailed
authorValentine Wallace <vwallace@protonmail.com>
Mon, 20 Sep 2021 16:18:49 +0000 (12:18 -0400)
committerValentine Wallace <vwallace@protonmail.com>
Tue, 21 Sep 2021 21:39:40 +0000 (17:39 -0400)
Since we don't want to imply to users that a payment has
completely failed when it really has just partially
failed

fuzz/src/chanmon_consistency.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/routing/network_graph.rs
lightning/src/util/events.rs

index f85ac9b9def2839542eefa00b0777a2838612302..4177b50275931722565fc1468aa7e5b08f29d76d 100644 (file)
@@ -827,7 +827,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                                        }
                                                },
                                                events::Event::PaymentSent { .. } => {},
-                                               events::Event::PaymentFailed { .. } => {},
+                                               events::Event::PaymentPathFailed { .. } => {},
                                                events::Event::PaymentForwarded { .. } if $node == 1 => {},
                                                events::Event::PendingHTLCsForwardable { .. } => {
                                                        nodes[$node].process_pending_htlc_forwards();
index 36f226823945b3a4cc5f0e07485788198b7719d8..c3262f17374327b42bc40c987350a05a6d8441d9 100644 (file)
@@ -78,7 +78,7 @@ fn do_test_simple_monitor_permanent_update_fail(persister_fail: bool) {
        };
 
        // TODO: Once we hit the chain with the failure transaction we should check that we get a
-       // PaymentFailed event
+       // PaymentPathFailed event
 
        assert_eq!(nodes[0].node.list_channels().len(), 0);
        check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
@@ -267,7 +267,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail
        check_closed_broadcast!(nodes[0], true);
 
        // TODO: Once we hit the chain with the failure transaction we should check that we get a
-       // PaymentFailed event
+       // PaymentPathFailed event
 
        assert_eq!(nodes[0].node.list_channels().len(), 0);
        check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
@@ -1819,7 +1819,7 @@ fn test_monitor_update_on_pending_forwards() {
 
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 2);
-       if let Event::PaymentFailed { payment_hash, rejected_by_dest, .. } = events[0] {
+       if let Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] {
                assert_eq!(payment_hash, payment_hash_1);
                assert!(rejected_by_dest);
        } else { panic!("Unexpected event!"); }
index 8ce19cc0c40d408e2ba2f42428034dc8789451b9..a3b8715f70706ea810351e1dab634df919a7aeb8 100644 (file)
@@ -489,7 +489,7 @@ pub struct ChannelManager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref,
        /// The session_priv bytes of outbound payments which are pending resolution.
        /// The authoritative state of these HTLCs resides either within Channels or ChannelMonitors
        /// (if the channel has been force-closed), however we track them here to prevent duplicative
-       /// PaymentSent/PaymentFailed events. Specifically, in the case of a duplicative
+       /// PaymentSent/PaymentPathFailed events. Specifically, in the case of a duplicative
        /// update_fulfill_htlc message after a reconnect, we may "claim" a payment twice.
        /// Additionally, because ChannelMonitors are often not re-serialized after connecting block(s)
        /// which may generate a claim event, we may receive similar duplicate claim/fail MonitorEvents
@@ -2882,7 +2882,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) {
                                                if sessions.get_mut().remove(&session_priv_bytes) {
                                                        self.pending_events.lock().unwrap().push(
-                                                               events::Event::PaymentFailed {
+                                                               events::Event::PaymentPathFailed {
                                                                        payment_hash,
                                                                        rejected_by_dest: false,
                                                                        network_update: None,
@@ -2951,7 +2951,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // process_onion_failure we should close that channel as it implies our
                                                // next-hop is needlessly blaming us!
                                                self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentFailed {
+                                                       events::Event::PaymentPathFailed {
                                                                payment_hash: payment_hash.clone(),
                                                                rejected_by_dest: !payment_retryable,
                                                                network_update,
@@ -2977,7 +2977,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // TODO: For non-temporary failures, we really should be closing the
                                                // channel here as we apparently can't relay through them anyway.
                                                self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentFailed {
+                                                       events::Event::PaymentPathFailed {
                                                                payment_hash: payment_hash.clone(),
                                                                rejected_by_dest: path.len() == 1,
                                                                network_update: None,
index 5690c834e73860d37729934c38c507bb648b3299..e30046c9993762725aa8d7e3fd97eaa2adb486d0 100644 (file)
@@ -1073,7 +1073,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::PaymentPathFailed { 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");
@@ -1102,7 +1102,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::PaymentPathFailed { 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");
@@ -1399,7 +1399,7 @@ 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, all_paths_failed, .. } => {
+                               Event::PaymentPathFailed { 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);
index a7714796af4f07b46fd47e7e355da1db26cb685a..00dacc420d3df2d607180c70e1ec7fb73a38167e 100644 (file)
@@ -3020,7 +3020,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                _ => panic!("Unexepected event"),
        }
        match events[1] {
-               Event::PaymentFailed { ref payment_hash, .. } => {
+               Event::PaymentPathFailed { ref payment_hash, .. } => {
                        assert_eq!(*payment_hash, fourth_payment_hash);
                },
                _ => panic!("Unexpected event"),
@@ -3076,7 +3076,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                        let events = nodes[0].node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 3);
                        match events[0] {
-                               Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
+                               Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        // If we delivered B's RAA we got an unknown preimage error, not something
                                        // that we should update our routing table for.
@@ -3087,14 +3087,14 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                                _ => panic!("Unexpected event"),
                        }
                        match events[1] {
-                               Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
+                               Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        assert!(network_update.is_some());
                                },
                                _ => panic!("Unexpected event"),
                        }
                        match events[2] {
-                               Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
+                               Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => {
                                        assert!(failed_htlcs.insert(payment_hash.0));
                                        assert!(network_update.is_some());
                                },
@@ -3190,7 +3190,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() {
        assert_eq!(events.len(), 2);
        // Check that Alice fails backward the pending HTLC from the second payment.
        match events[0] {
-               Event::PaymentFailed { payment_hash, .. } => {
+               Event::PaymentPathFailed { payment_hash, .. } => {
                        assert_eq!(payment_hash, failed_payment_hash);
                },
                _ => panic!("Unexpected event"),
@@ -3392,7 +3392,7 @@ fn test_simple_peer_disconnect() {
                        _ => panic!("Unexpected event"),
                }
                match events[1] {
-                       Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
+                       Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } => {
                                assert_eq!(payment_hash, payment_hash_5);
                                assert!(rejected_by_dest);
                        },
@@ -4192,7 +4192,7 @@ fn test_dup_htlc_onchain_fails_on_reload() {
        //
        // If, due to an on-chain event, an HTLC is failed/claimed, and then we serialize the
        // ChannelManager, we generally expect there not to be a duplicate HTLC fail/claim (eg via a
-       // PaymentFailed event appearing). However, because we may not serialize the relevant
+       // PaymentPathFailed event appearing). However, because we may not serialize the relevant
        // ChannelMonitor at the same time, this isn't strictly guaranteed. In order to provide this
        // consistency, the ChannelManager explicitly tracks pending-onchain-resolution outbound HTLCs
        // and de-duplicates ChannelMonitor events.
@@ -5518,7 +5518,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
        let mut as_failds = HashSet::new();
        let mut as_updates = 0;
        for event in as_events.iter() {
-               if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
+               if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
                        assert!(as_failds.insert(*payment_hash));
                        if *payment_hash != payment_hash_2 {
                                assert_eq!(*rejected_by_dest, deliver_last_raa);
@@ -5543,7 +5543,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
        let mut bs_failds = HashSet::new();
        let mut bs_updates = 0;
        for event in bs_events.iter() {
-               if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
+               if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event {
                        assert!(bs_failds.insert(*payment_hash));
                        if *payment_hash != payment_hash_1 && *payment_hash != payment_hash_5 {
                                assert_eq!(*rejected_by_dest, deliver_last_raa);
@@ -6068,7 +6068,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::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
+               &Event::PaymentPathFailed { 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);
@@ -6155,7 +6155,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::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => {
+               &Event::PaymentPathFailed { 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);
@@ -7107,12 +7107,12 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) {
        assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
        connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
        let events = nodes[0].node.get_and_clear_pending_events();
-       // Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx
+       // Only 2 PaymentPathFailed events should show up, over-dust HTLC has to be failed by timeout tx
        assert_eq!(events.len(), 2);
        let mut first_failed = false;
        for event in events {
                match event {
-                       Event::PaymentFailed { payment_hash, .. } => {
+                       Event::PaymentPathFailed { payment_hash, .. } => {
                                if payment_hash == payment_hash_1 {
                                        assert!(!first_failed);
                                        first_failed = true;
@@ -7202,14 +7202,14 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
                        assert_eq!(events.len(), 2);
                        let first;
                        match events[0] {
-                               Event::PaymentFailed { payment_hash, .. } => {
+                               Event::PaymentPathFailed { payment_hash, .. } => {
                                        if payment_hash == dust_hash { first = true; }
                                        else { first = false; }
                                },
                                _ => panic!("Unexpected event"),
                        }
                        match events[1] {
-                               Event::PaymentFailed { payment_hash, .. } => {
+                               Event::PaymentPathFailed { payment_hash, .. } => {
                                        if first { assert_eq!(payment_hash, non_dust_hash); }
                                        else { assert_eq!(payment_hash, dust_hash); }
                                },
index 70d5a0c0bdfe30362668824ed6e77e0e267755a0..090200856fde82c397823c4884a2c9e2cc1ab524 100644 (file)
@@ -163,7 +163,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::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed } = &events[0] {
+       if let &Event::PaymentPathFailed { 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);
index 16ff80189e96b7dd249d8d96b5112b305e2f272b..c39dde7bfd2927616398efb7f8b2e384d54ff028 100644 (file)
@@ -113,7 +113,7 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
 impl<C: Deref, L: Deref> EventHandler for NetGraphMsgHandler<C, L>
 where C::Target: chain::Access, L::Target: Logger {
        fn handle_event(&self, event: &Event) {
-               if let Event::PaymentFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
+               if let Event::PaymentPathFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event {
                        if let Some(network_update) = network_update {
                                self.handle_network_update(network_update);
                        }
@@ -127,7 +127,7 @@ where C::Target: chain::Access, L::Target: Logger {
 /// Provides interface to help with initial routing sync by
 /// serving historical announcements.
 ///
-/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentFailed`] to the
+/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the
 /// [`NetworkGraph`].
 pub struct NetGraphMsgHandler<C: Deref, L: Deref>
 where C::Target: chain::Access, L::Target: Logger
@@ -1725,7 +1725,7 @@ mod tests {
 
                        assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none());
 
-                       net_graph_msg_handler.handle_event(&Event::PaymentFailed {
+                       net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
                                all_paths_failed: true,
@@ -1748,7 +1748,7 @@ mod tests {
                                }
                        };
 
-                       net_graph_msg_handler.handle_event(&Event::PaymentFailed {
+                       net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
                                all_paths_failed: true,
@@ -1770,7 +1770,7 @@ mod tests {
 
                // Permanent closing deletes a channel
                {
-                       net_graph_msg_handler.handle_event(&Event::PaymentFailed {
+                       net_graph_msg_handler.handle_event(&Event::PaymentPathFailed {
                                payment_hash: PaymentHash([0; 32]),
                                rejected_by_dest: false,
                                all_paths_failed: true,
index be9296161768314aeae20da5feecdd69eeb8fa76..a2f48e060dd3db97bf6e2c8698567716459acb34 100644 (file)
@@ -170,8 +170,8 @@ pub enum Event {
        /// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target
        /// and we got back the payment preimage for it).
        ///
-       /// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentFailed` event.
-       /// In this situation, you SHOULD treat this payment as having succeeded.
+       /// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentPathFailed`
+       /// event. In this situation, you SHOULD treat this payment as having succeeded.
        PaymentSent {
                /// The preimage to the hash given to ChannelManager::send_payment.
                /// Note that this serves as a payment receipt, if you wish to have such a thing, you must
@@ -180,7 +180,7 @@ pub enum Event {
        },
        /// Indicates an outbound payment we made failed. Probably some intermediary node dropped
        /// something. You may wish to retry with a different route.
-       PaymentFailed {
+       PaymentPathFailed {
                /// The hash which was given to ChannelManager::send_payment.
                payment_hash: PaymentHash,
                /// Indicates the payment was rejected for some reason by the recipient. This implies that
@@ -291,7 +291,7 @@ impl Writeable for Event {
                                        (0, payment_preimage, required),
                                });
                        },
-                       &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed,
+                       &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed,
                                #[cfg(test)]
                                ref error_code,
                                #[cfg(test)]
@@ -409,7 +409,7 @@ impl MaybeReadable for Event {
                                                (2, rejected_by_dest, required),
                                                (3, all_paths_failed, option),
                                        });
-                                       Ok(Some(Event::PaymentFailed {
+                                       Ok(Some(Event::PaymentPathFailed {
                                                payment_hash,
                                                rejected_by_dest,
                                                network_update,