Don't abandon payments for duplicate invoices

When making an outbound BOLT12 payment, multiple invoices may be
received for the same payment id. Instead of abandoning the payment when
a duplicate invoice received, simply ignore it without responding with
an InvoiceError. This prevents abandoning in-progress payments and
sending unnecessary onion messages.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9dc8270d8a5df2a839f829d9bfa9f822437425dd..6b817e56bac26e471f58f141b774e5078449f369 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -10853,43 +10853,51 @@ where
                                        &self.logger, None, None, Some(invoice.payment_hash()),
                                );
 
-                               let result = {
-                                       let features = self.bolt12_invoice_features();
-                                       if invoice.invoice_features().requires_unknown_bits_from(&features) {
-                                               Err(InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures))
-                                       } else if self.default_configuration.manually_handle_bolt12_invoices {
-                                               let event = Event::InvoiceReceived {
-                                                       payment_id, invoice, context, responder,
-                                               };
-                                               self.pending_events.lock().unwrap().push_back((event, None));
-                                               return ResponseInstruction::NoResponse;
-                                       } else {
-                                               self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id)
-                                                       .map_err(|e| {
-                                                               log_trace!(logger, "Failed paying invoice: {:?}", e);
-                                                               InvoiceError::from_string(format!("{:?}", e))
-                                                       })
-                                       }
-                               };
+                               let features = self.bolt12_invoice_features();
+                               if invoice.invoice_features().requires_unknown_bits_from(&features) {
+                                       log_trace!(
+                                               logger, "Invoice requires unknown features: {:?}",
+                                               invoice.invoice_features(),
+                                       );
+                                       abandon_if_payment(context);
 
-                               match result {
-                                       Ok(_) => ResponseInstruction::NoResponse,
-                                       Err(err) => match responder {
-                                               Some(responder) => {
-                                                       abandon_if_payment(context);
-                                                       responder.respond(OffersMessage::InvoiceError(err))
-                                               },
+                                       let error = InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures);
+                                       let response = match responder {
+                                               Some(responder) => responder.respond(OffersMessage::InvoiceError(error)),
                                                None => {
-                                                       abandon_if_payment(context);
-                                                       log_trace!(
-                                                               logger,
-                                                               "An error response was generated, but there is no reply_path specified \
-                                                               for sending the response. Error: {}",
-                                                               err
-                                                       );
-                                                       return ResponseInstruction::NoResponse;
+                                                       log_trace!(logger, "No reply path to send error: {:?}", error);
+                                                       ResponseInstruction::NoResponse
                                                },
+                                       };
+                                       return response;
+                               }
+
+                               if self.default_configuration.manually_handle_bolt12_invoices {
+                                       let event = Event::InvoiceReceived {
+                                               payment_id, invoice, context, responder,
+                                       };
+                                       self.pending_events.lock().unwrap().push_back((event, None));
+                                       return ResponseInstruction::NoResponse;
+                               }
+
+                               match self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id) {
+                                       // Payments with SendingFailed error will already have been abandoned.
+                                       Err(Bolt12PaymentError::SendingFailed(e)) => {
+                                               log_trace!(logger, "Failed paying invoice: {:?}", e);
+
+                                               let err = InvoiceError::from_string(format!("{:?}", e));
+                                               match responder {
+                                                       Some(responder) => responder.respond(OffersMessage::InvoiceError(err)),
+                                                       None => {
+                                                               log_trace!(logger, "No reply path to send error: {:?}", err);
+                                                               ResponseInstruction::NoResponse
+                                                       },
+                                               }
                                        },
+                                       // Otherwise, don't abandon unknown, pending, or successful payments.
+                                       Err(Bolt12PaymentError::UnexpectedInvoice)
+                                               | Err(Bolt12PaymentError::DuplicateInvoice)
+                                               | Ok(()) => ResponseInstruction::NoResponse,
                                }
                        },
                        #[cfg(async_payments)]

diff --git a/lightning/src/ln/max_payment_path_len_tests.rs b/lightning/src/ln/max_payment_path_len_tests.rs
index fd027ea62b6abeab8c9a462178b19408300dccbc..096bcf9633c64e45238714b8cafa4a54ec63f3f6 100644 (file)
--- a/lightning/src/ln/max_payment_path_len_tests.rs
+++ b/lightning/src/ln/max_payment_path_len_tests.rs
@@ -388,7 +388,7 @@ fn bolt12_invoice_too_large_blinded_paths() {
        let invoice_om = nodes[1].onion_messenger.next_onion_message_for_peer(nodes[0].node.get_our_node_id()).unwrap();
        nodes[0].onion_messenger.handle_onion_message(&nodes[1].node.get_our_node_id(), &invoice_om);
        // TODO: assert on the invoice error once we support replying to invoice OMs with failure info
-       nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Failed paying invoice: SendingFailed(OnionPacketSizeExceeded)", 1);
+       nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Failed paying invoice: OnionPacketSizeExceeded", 1);
 
        let events = nodes[0].node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);

diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs
index a7fc92f527f0b90924cd908374880163922e82f9..e774bc081621a8c863caa6f0c034ba5f20a5ea28 100644 (file)
--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -2027,16 +2027,13 @@ fn fails_paying_invoice_more_than_once() {
        let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap();
        david.onion_messenger.handle_onion_message(&charlie_id, &onion_message);
 
-       // David pays the first invoice
+       // David initiates paying the first invoice
        let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {});
        let (invoice1, _) = extract_invoice(david, &onion_message);
 
        route_bolt12_payment(david, &[charlie, bob, alice], &invoice1);
        expect_recent_payment!(david, RecentPaymentDetails::Pending, payment_id);
 
-       claim_bolt12_payment(david, &[charlie, bob, alice], payment_context);
-       expect_recent_payment!(david, RecentPaymentDetails::Fulfilled, payment_id);
-
        disconnect_peers(alice, &[charlie]);
 
        // Alice sends the second invoice
@@ -2054,13 +2051,11 @@ fn fails_paying_invoice_more_than_once() {
        let (invoice2, _) = extract_invoice(david, &onion_message);
        assert_eq!(invoice1.payer_metadata(), invoice2.payer_metadata());
 
-       // David sends an error instead of paying the second invoice
-       let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
-       bob.onion_messenger.handle_onion_message(&david_id, &onion_message);
-
-       let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
-       alice.onion_messenger.handle_onion_message(&bob_id, &onion_message);
+       // David doesn't initiate paying the second invoice
+       assert!(david.onion_messenger.next_onion_message_for_peer(bob_id).is_none());
+       assert!(david.node.get_and_clear_pending_msg_events().is_empty());
 
-       let invoice_error = extract_invoice_error(alice, &onion_message);
-       assert_eq!(invoice_error, InvoiceError::from_string("DuplicateInvoice".to_string()));
+       // Complete paying the first invoice
+       claim_bolt12_payment(david, &[charlie, bob, alice], payment_context);
+       expect_recent_payment!(david, RecentPaymentDetails::Fulfilled, payment_id);
 }