From: Jeffrey Czyz <jkczyz@gmail.com>
Date: Wed, 24 Jul 2024 21:33:02 +0000 (-0500)
Subject: Don't abandon payments for duplicate invoices
X-Git-Tag: v0.0.124-beta~12^2~14
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=144d4882adf910d3daeab3ddba0b4e8793d7c25a;p=rust-lightning
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 9dc8270d8..6b817e56b 100644
--- 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 fd027ea62..096bcf963 100644
--- 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 a7fc92f52..e774bc081 100644
--- 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);
}