Prevent duplicate PaymentSent events

by removing all pending outbound payments associated with the same
MPP payment after the preimage is received

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9b0f78b22bc013b6f7c66097279a390091542ad4..0389bd5cd198aa28ea879ba0050bf89dafa6c66d 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3140,17 +3140,13 @@ 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();
-                               if let Some(sessions) = outbounds.get_mut(&mpp_id) {
-                                       if sessions.remove(&session_priv_bytes) {
-                                               self.pending_events.lock().unwrap().push(
-                                                       events::Event::PaymentSent { payment_preimage }
-                                               );
-                                               if sessions.len() == 0 {
-                                                       outbounds.remove(&mpp_id);
-                                               }
-                                       } else {
-                                               log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
-                                       }
+                               let found_payment = if let Some(mut sessions) = outbounds.remove(&mpp_id) {
+                                       sessions.remove(&session_priv_bytes)
+                               } else { false };
+                               if found_payment {
+                                       self.pending_events.lock().unwrap().push(
+                                               events::Event::PaymentSent { payment_preimage }
+                                       );
                                } else {
                                        log_trace!(self.logger, "Received duplicative fulfill for HTLC with payment_preimage {}", log_bytes!(payment_preimage.0));
                                }
@@ -5691,7 +5687,8 @@ mod tests {
                nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
                check_added_monitors!(nodes[0], 1);
 
-               // There's an existing bug that generates a PaymentSent event for each MPP path, so handle that here.
+               // Note that successful MPP payments will generate 1 event upon the first path's success. No
+               // further events will be generated for subsequence path successes.
                let events = nodes[0].node.get_and_clear_pending_events();
                match events[0] {
                        Event::PaymentSent { payment_preimage: ref preimage } => {
@@ -5699,12 +5696,6 @@ mod tests {
                        },
                        _ => panic!("Unexpected event"),
                }
-               match events[1] {
-                       Event::PaymentSent { payment_preimage: ref preimage } => {
-                               assert_eq!(payment_preimage, *preimage);
-                       },
-                       _ => panic!("Unexpected event"),
-               }
        }
 
        #[test]

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 0389da4977c054c5e6f7f2836a8f27767fb73c9e..ffca2ecd5739060f67cd4c486dbe8b660b4f32a6 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1242,9 +1242,11 @@ pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, exp
 
                if !skip_last {
                        last_update_fulfill_dance!(origin_node, expected_route.first().unwrap());
-                       expect_payment_sent!(origin_node, our_payment_preimage);
                }
        }
+       if !skip_last {
+               expect_payment_sent!(origin_node, our_payment_preimage);
+       }
 }
 
 pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_preimage: PaymentPreimage) {

diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs
index d63dd88b76163c86fd16410e468eb03f7d95fcc6..bf1bd074314f701bcfc41e628c24a291e955522a 100644 (file)
--- a/lightning/src/util/events.rs
+++ b/lightning/src/util/events.rs
@@ -112,8 +112,11 @@ pub enum Event {
                /// payment is to pay an invoice or to send a spontaneous payment.
                purpose: PaymentPurpose,
        },
-       /// Indicates an outbound payment we made succeeded (ie it made it all the way to its target
+       /// 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.
        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