From 144d4882adf910d3daeab3ddba0b4e8793d7c25a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 24 Jul 2024 16:33:02 -0500 Subject: [PATCH] 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. --- lightning/src/ln/channelmanager.rs | 74 ++++++++++--------- .../src/ln/max_payment_path_len_tests.rs | 2 +- lightning/src/ln/offers_tests.rs | 19 ++--- 3 files changed, 49 insertions(+), 46 deletions(-) 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); } -- 2.39.5