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.

Author: jkczyz

lightning/src/ln/channelmanager.rs

@@ -10845,43 +10845,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)]

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);

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);
 }