Surface failed and pending payments across backends

orange-sdk/src/lib.rs

@@ -27,7 +27,7 @@ use ldk_node::lightning::ln::msgs::SocketAddress;
 use ldk_node::lightning::util::logger::Logger as _;
 use ldk_node::lightning::{log_debug, log_error, log_info, log_trace, log_warn};
 use ldk_node::lightning_invoice::Bolt11Invoice;
-use ldk_node::payment::{PaymentDetails, PaymentDirection, PaymentKind};
+use ldk_node::payment::{PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus};
 use ldk_node::{BuildError, ChannelDetails, NodeError};
 #[cfg(feature = "_test-utils")]
 pub use lightning_wallet::list_node_payments;
@@ -536,15 +536,24 @@ impl From<NodeError> for WalletError {
 	}
 }
 
-fn should_surface_lightning_payment_without_metadata(status: TxStatus, kind: &PaymentKind) -> bool {
-	status == TxStatus::Completed || matches!(kind, PaymentKind::Onchain { .. })
+fn should_surface_lightning_payment_without_metadata(
+	status: TxStatus, kind: &PaymentKind, direction: PaymentDirection,
+) -> bool {
+	// Hide only non-completed *inbound* Lightning records (issued-but-unpaid
+	// invoices are noise). Outbound attempts always surface: a failed or
+	// still-pending send the user made must appear in their history —
+	// otherwise a failed payment leaves no record at all.
+	status == TxStatus::Completed
+		|| matches!(kind, PaymentKind::Onchain { .. })
+		|| direction == PaymentDirection::Outbound
 }
 
 fn lightning_payment_without_metadata_to_transaction(
 	payment: &PaymentDetails, fee: Option<Amount>,
 ) -> Option<Transaction> {
 	let status = payment.status.into();
-	if !should_surface_lightning_payment_without_metadata(status, &payment.kind) {
+	if !should_surface_lightning_payment_without_metadata(status, &payment.kind, payment.direction)
+	{
 		return None;
 	}
 
@@ -830,6 +839,9 @@ impl Wallet {
 	}
 
 	/// Lists the transactions which have been made.
+	/// Submitted payments include pending and confirmed failed sends across backends.
+	/// Errors before submission, such as invoice validation or quote preparation failures,
+	/// do not create history entries. An uncertain send outcome remains pending.
 	pub async fn list_transactions(&self) -> Result<Vec<Transaction>, WalletError> {
 		let (trusted_payments, splice_outs) = tokio::join!(
 			self.inner.trusted.list_payments(),
@@ -1080,12 +1092,28 @@ impl Wallet {
 					},
 				}
 			} else {
-				debug_assert_ne!(
-					payment.direction,
-					PaymentDirection::Outbound,
-					"Missing outbound lightning payment metadata entry on {}",
-					payment.id
-				);
+				// Only a *successful* outbound payment is expected to have a metadata entry
+				// (`pay()` writes one whenever the send returns `Ok`). Outbound records can
+				// legitimately lack metadata when a send fails synchronously — ldk-node
+				// inserts a `Failed` record before returning `Err`, so `pay()` never
+				// observes an id — or briefly while a `Pending` record awaits the upsert.
+				// Even `Succeeded` is only a should-never-happen: a crash between
+				// ldk-node persisting the payment and the metadata write leaves one.
+				if payment.direction == PaymentDirection::Outbound
+					&& payment.status == PaymentStatus::Succeeded
+				{
+					log_warn!(
+						self.inner.logger,
+						"Missing outbound lightning payment metadata entry on {}",
+						payment.id
+					);
+					#[cfg(feature = "_test-utils")]
+					debug_assert!(
+						false,
+						"Missing outbound lightning payment metadata entry on {}",
+						payment.id
+					);
+				}
 
 				if let Some(transaction) =
 					lightning_payment_without_metadata_to_transaction(&payment, fee)
@@ -1315,9 +1343,11 @@ impl Wallet {
 					let res = self.inner.trusted.pay(method, instructions.amount).await;
 					match res {
 						Ok(id) => {
+							// A backend that is already paying this request returns the
+							// in-flight payment's id, so the entry may already exist.
 							self.inner
 								.tx_metadata
-								.insert(
+								.upsert(
 									PaymentId::Trusted(id),
 									TxMetadata {
 										ty: TxType::Payment { ty: ty() },
@@ -1556,6 +1586,20 @@ impl Wallet {
 		);
 
 		let payment_hash = invoice.payment_hash();
+
+		// The lightning leg would be rejected as a duplicate if this invoice is already being
+		// paid or has been paid. Check before the trusted leg goes out, since that leg cannot be
+		// recalled and would only fail once the receiver's MPP timeout expires. This is best
+		// effort; ldk-node still rejects a duplicate the check does not see.
+		if self
+			.inner
+			.ln_wallet
+			.recent_outbound_bolt11(payment_hash)?
+			.is_some_and(|p| matches!(p.status, PaymentStatus::Pending | PaymentStatus::Succeeded))
+		{
+			return Err(WalletError::LdkNodeFailure(NodeError::DuplicatePayment));
+		}
+
 		self.inner.event_queue.begin_mpp_setup(payment_hash).await;
 
 		// Pay the trusted portion first; the receiver will hold it until the lightning portion
@@ -1582,6 +1626,28 @@ impl Wallet {
 			Ok(id) => id,
 			Err(e) => {
 				log_error!(self.inner.logger, "Failed to send lightning MPP portion: {e:?}");
+				// Only remove a failed leg. DuplicatePayment refers to an existing payment
+				// and must never delete its pending or completed history.
+				let failed_leg = if e == NodeError::DuplicatePayment {
+					None
+				} else {
+					self.inner
+						.ln_wallet
+						.recent_outbound_bolt11(payment_hash)
+						.ok()
+						.flatten()
+						.filter(|p| p.status == PaymentStatus::Failed)
+				};
+				if let Some(leg) = failed_leg {
+					if let Err(remove_err) =
+						self.inner.ln_wallet.inner.ldk_node.remove_payment(&leg.id)
+					{
+						log_error!(
+							self.inner.logger,
+							"Failed to remove failed MPP leg: {remove_err:?}"
+						);
+					}
+				}
 				// The trusted leg is already in flight but there will be no lightning leg to
 				// complete the MPP. Record it as a plain payment so its eventual (failed) terminal
 				// event surfaces normally rather than waiting on a sibling leg that never comes.
@@ -1807,7 +1873,11 @@ mod tests {
 			tx_type: None,
 		};
 
-		assert!(should_surface_lightning_payment_without_metadata(TxStatus::Pending, &kind));
+		assert!(should_surface_lightning_payment_without_metadata(
+			TxStatus::Pending,
+			&kind,
+			PaymentDirection::Inbound
+		));
 	}
 
 	#[test]
@@ -1841,16 +1911,54 @@ mod tests {
 	}
 
 	#[test]
-	fn pending_non_onchain_lightning_payments_without_metadata_are_hidden() {
+	fn pending_inbound_non_onchain_lightning_payments_without_metadata_are_hidden() {
 		let kind = PaymentKind::Spontaneous { hash: PaymentHash([42; 32]), preimage: None };
 
-		assert!(!should_surface_lightning_payment_without_metadata(TxStatus::Pending, &kind));
+		assert!(!should_surface_lightning_payment_without_metadata(
+			TxStatus::Pending,
+			&kind,
+			PaymentDirection::Inbound
+		));
 	}
 
 	#[test]
 	fn completed_lightning_payments_without_metadata_are_listed() {
 		let kind = PaymentKind::Spontaneous { hash: PaymentHash([42; 32]), preimage: None };
 
-		assert!(should_surface_lightning_payment_without_metadata(TxStatus::Completed, &kind));
+		assert!(should_surface_lightning_payment_without_metadata(
+			TxStatus::Completed,
+			&kind,
+			PaymentDirection::Inbound
+		));
+	}
+
+	#[test]
+	fn failed_and_pending_outbound_payments_are_listed() {
+		// A failed or in-flight send the user made must appear in their
+		// history — a failed payment that leaves no record erodes trust
+		// in the send flow (the wallet UI can't show what it never sees).
+		let kind = PaymentKind::Bolt11 {
+			hash: PaymentHash([42; 32]),
+			preimage: None,
+			secret: None,
+			counterparty_skimmed_fee_msat: None,
+		};
+
+		assert!(should_surface_lightning_payment_without_metadata(
+			TxStatus::Failed,
+			&kind,
+			PaymentDirection::Outbound
+		));
+		assert!(should_surface_lightning_payment_without_metadata(
+			TxStatus::Pending,
+			&kind,
+			PaymentDirection::Outbound
+		));
+		// Inbound failures (expired unpaid invoices) stay hidden.
+		assert!(!should_surface_lightning_payment_without_metadata(
+			TxStatus::Failed,
+			&kind,
+			PaymentDirection::Inbound
+		));
 	}
 }

orange-sdk/src/lightning_wallet.rs

@@ -21,6 +21,7 @@ use ldk_node::lightning::ln::msgs::SocketAddress;
 use ldk_node::lightning::util::logger::Logger as _;
 use ldk_node::lightning::{log_debug, log_error, log_info};
 use ldk_node::lightning_invoice::{Bolt11Invoice, Bolt11InvoiceDescription, Description};
+use ldk_node::lightning_types::payment::PaymentHash;
 use ldk_node::payment::{
 	ConfirmationStatus, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus,
 };
@@ -299,6 +300,21 @@ impl LightningWallet {
 		list_node_payments(&self.inner.ldk_node)
 	}
 
+	/// The most recent outbound BOLT 11 record for `payment_hash`, if one was created recently.
+	///
+	/// Only the newest page of the payment list is read, so this neither derives an LDK payment
+	/// ID from the hash nor walks the history. The failed MPP leg this finds was inserted
+	/// moments ago, and the duplicate check that also uses it is best effort, since ldk-node
+	/// rejects a duplicate itself.
+	pub(crate) fn recent_outbound_bolt11(
+		&self, payment_hash: PaymentHash,
+	) -> Result<Option<PaymentDetails>, NodeError> {
+		Ok(self.inner.ldk_node.list_payments(None)?.payments.into_iter().find(|p| {
+			p.direction == PaymentDirection::Outbound
+				&& matches!(p.kind, PaymentKind::Bolt11 { hash, .. } if hash == payment_hash)
+		}))
+	}
+
 	pub(crate) fn get_balance(&self) -> LightningWalletBalance {
 		let balances = self.inner.ldk_node.list_balances();
 		LightningWalletBalance {