From 3893e1946baa92f69751ed86603817b9d135f9a9 Mon Sep 17 00:00:00 2001 From: Hash Money Date: Thu, 10 Sep 2026 19:54:07 -0700 Subject: [PATCH] Surface failed and pending outbound Lightning payments in list_transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ldk-node records a synchronously-failed send as a Failed outbound PaymentDetails before returning the error (bolt11 send_internal's SendingFailed arm; bolt12 and spontaneous likewise), and pay() only writes tx metadata when the send returns Ok — so a failed send lands in the no-metadata branch of list_transactions and was hidden by the status != Completed filter, leaving no trace in the transaction history. Surface outbound attempts regardless of status; non-completed inbound records (issued-but-unpaid invoices) stay hidden. The no-metadata branch's debug assertion assumed outbound records always carry metadata; that only holds for successful sends, so it fires today in any debug build that lists transactions after a synchronously-failed send. Replace it with the trusted loop's pattern (log_warn plus a _test-utils-gated assert), scoped to Succeeded — and even Succeeded can legitimately lack metadata after a crash between ldk-node's persist and the metadata write, hence warn-and-assert. Surfacing outbound records also exposed one internal leg: when try_mpp_bolt11's lightning portion fails synchronously after the trusted leg is in flight, ldk-node has recorded a failed outbound payment for it that would list as a second, standalone transaction. Nothing is in flight after a synchronous failure, so the MPP error path now removes that record; the attempt stays surfaced through the trusted leg, as that error path already intends. Trusted-backend records are unchanged: failed Spark sends either already surface through the metadata branch (keyed by the idempotency uuid, which never filtered on status) or never reach storage, and the CDK only records melts that reached Paid. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016zApnExfnDXm1Kxu8pf9P9 --- orange-sdk/src/lib.rs | 113 +++++++++++++++--- orange-sdk/tests/integration_tests.rs | 157 +++++++++++++++++++++++++- 2 files changed, 254 insertions(+), 16 deletions(-) diff --git a/orange-sdk/src/lib.rs b/orange-sdk/src/lib.rs index b51d72d..38adb1b 100644 --- a/orange-sdk/src/lib.rs +++ b/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}; use crate::dyn_store::DynStore; @@ -529,15 +529,24 @@ impl From 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, ) -> Option { 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; } @@ -1050,12 +1059,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) @@ -1555,6 +1580,24 @@ impl Wallet { Ok(id) => id, Err(e) => { log_error!(self.inner.logger, "Failed to send lightning MPP portion: {e:?}"); + // The lightning leg failed synchronously, so nothing is in flight — but + // ldk-node has still recorded a failed outbound payment for it, keyed by + // the invoice's payment hash. Remove that record: it is an internal MPP + // leg, not an independent payment, and the attempt is surfaced through + // the trusted leg below. + use ldk_node::lightning::ln::channelmanager::PaymentId as LdkPaymentId; + if let Err(remove_err) = self + .inner + .ln_wallet + .inner + .ldk_node + .remove_payment(&LdkPaymentId(payment_hash.0)) + { + log_error!( + self.inner.logger, + "Failed to remove failed MPP lightning leg record: {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. @@ -1779,7 +1822,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] @@ -1813,16 +1860,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 + )); } } diff --git a/orange-sdk/tests/integration_tests.rs b/orange-sdk/tests/integration_tests.rs index 7fd4361..10684ea 100644 --- a/orange-sdk/tests/integration_tests.rs +++ b/orange-sdk/tests/integration_tests.rs @@ -6,10 +6,13 @@ use bitcoin_payment_instructions::http_resolver::HTTPHrnResolver; use bitcoin_payment_instructions::{ParseError, PaymentInstructions}; use ldk_node::NodeError; use ldk_node::bitcoin::Network; -use ldk_node::lightning_invoice::{Bolt11InvoiceDescription, Description}; +use ldk_node::bitcoin::secp256k1::{Secp256k1, SecretKey}; +use ldk_node::lightning_invoice::{ + Bolt11InvoiceDescription, Currency, Description, InvoiceBuilder, PaymentHash, PaymentSecret, +}; use ldk_node::payment::{ConfirmationStatus, PaymentDirection, PaymentStatus}; use log::info; -use orange_sdk::{Event, PaymentInfo, PaymentType, TxStatus, WalletError}; +use orange_sdk::{Event, PaymentId, PaymentInfo, PaymentType, TxStatus, WalletError}; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -2003,6 +2006,156 @@ async fn test_invalid_payment_instructions() { .await; } +#[tokio::test(flavor = "multi_thread")] +#[test_log::test] +async fn test_failed_lightning_send_is_surfaced() { + test_utils::run_test(|params| async move { + let wallet = Arc::clone(¶ms.wallet); + let bitcoind = Arc::clone(¶ms.bitcoind); + let electrsd = Arc::clone(¶ms.electrsd); + let third_party = Arc::clone(¶ms.third_party); + + // Give the wallet a spendable lightning balance. + open_channel_from_lsp(&wallet, Arc::clone(&third_party)).await; + generate_blocks(&bitcoind, &electrsd, 6).await; + test_utils::wait_for_condition("wallet sync after channel open", || async { + wallet.channels().iter().any(|c| c.confirmations.is_some_and(|n| n > 0) && c.is_usable) + }) + .await; + + // An invoice signed by a key no node ever announced: route-finding fails + // synchronously, and ldk-node records the attempt as a failed outbound payment. + // Unlike the rejected payments above (which never reach the node and leave no + // record), this attempt must show up in the transaction list. + let pay_amt = Amount::from_sats(10_000).unwrap(); + let secp = Secp256k1::new(); + let no_such_node = SecretKey::from_slice(&[99; 32]).unwrap(); + let invoice = InvoiceBuilder::new(Currency::Regtest) + .description("no route to this payee".to_string()) + .payment_hash(PaymentHash([43; 32])) + .payment_secret(PaymentSecret([44; 32])) + .current_timestamp() + .min_final_cltv_expiry_delta(144) + .amount_milli_satoshis(pay_amt.milli_sats()) + .build_signed(|hash| secp.sign_ecdsa_recoverable(hash, &no_such_node)) + .unwrap(); + + let instr = wallet.parse_payment_instructions(invoice.to_string().as_str()).await.unwrap(); + let info = PaymentInfo::build(instr, None).unwrap(); + let res = wallet.pay(&info).await; + assert!( + matches!(res, Err(WalletError::LdkNodeFailure(NodeError::PaymentSendingFailed))), + "send to an unroutable payee must fail synchronously, got {res:?}" + ); + + let txs = wallet.list_transactions().await.unwrap(); + let outbound: Vec<_> = txs.iter().filter(|t| t.outbound).collect(); + assert_eq!(outbound.len(), 1, "the failed send should surface exactly once: {txs:?}"); + let failed = outbound[0]; + assert_eq!(failed.status, TxStatus::Failed); + assert_eq!(failed.amount, Some(pay_amt)); + match &failed.payment_type { + PaymentType::OutgoingLightningBolt11 { payment_preimage } => { + assert!(payment_preimage.is_none(), "a failed payment has no preimage"); + }, + pt => panic!("Payment type should be OutgoingLightningBolt11, got {pt:?}"), + } + }) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +#[test_log::test] +#[cfg_attr( + feature = "_cashu-tests", + ignore = "CDK's test mint/payment processor does not support partial MPP melts" +)] +async fn test_failed_mpp_lightning_leg_is_not_listed_separately() { + test_utils::run_test(|params| async move { + let wallet = Arc::clone(¶ms.wallet); + let bitcoind = Arc::clone(¶ms.bitcoind); + let third_party = Arc::clone(¶ms.third_party); + let electrsd = Arc::clone(¶ms.electrsd); + let lsp = Arc::clone(¶ms.lsp); + let desc = Bolt11InvoiceDescription::Direct(Description::empty()); + + // Fund the trusted wallet with 100 sats before a channel exists (once inbound + // liquidity exists, small receives route to the lightning wallet instead). + let trusted_amt = Amount::from_sats(100).unwrap(); + let uri = wallet.get_single_use_receive_uri(Some(trusted_amt)).await.unwrap(); + assert!(uri.from_trusted); + third_party.bolt11_payment().send(&uri.invoice, None).unwrap(); + test_utils::wait_for_condition("trusted balance funded", || async { + wallet.get_balance().await.unwrap().trusted == trusted_amt + }) + .await; + assert!(matches!(wait_next_event(&wallet).await, Event::PaymentReceived { .. })); + + // Open a lightning channel. + open_channel_from_lsp(&wallet, Arc::clone(&third_party)).await; + generate_blocks(&bitcoind, &electrsd, 6).await; + test_utils::wait_for_condition("wallet sync after channel open", || async { + wallet.channels().iter().any(|c| c.confirmations.is_some_and(|n| n > 0) && c.is_usable) + }) + .await; + + // Drain spendable lightning liquidity down to ~150 sats. The channel reserve + // keeps the *total* lightning balance well above that, which is exactly the + // gap this test needs: the MPP split passes the balance check, but the + // lightning leg exceeds what a route can actually carry and fails + // synchronously with RouteNotFound after the trusted leg is already in + // flight. + let sendable = + wallet.channels().iter().find(|c| c.is_usable).unwrap().next_outbound_htlc_limit_msat; + let drain = lsp.bolt11_payment().receive(sendable - 150_000, &desc, 300).unwrap(); + let drain_info = PaymentInfo::build( + wallet.parse_payment_instructions(&drain.to_string()).await.unwrap(), + None, + ) + .unwrap(); + wallet.pay(&drain_info).await.unwrap(); + assert!(matches!(wait_next_event(&wallet).await, Event::PaymentSuccessful { .. })); + test_utils::wait_for_condition("lightning balance drained below 200 sats", || async { + wallet + .channels() + .iter() + .find(|c| c.is_usable) + .is_some_and(|c| c.next_outbound_htlc_limit_msat < 200_000) + }) + .await; + + // 350 sats = 100 trusted + 250 lightning. The 250 sat lightning leg exceeds + // the ~150 sats of usable outbound liquidity, so it fails synchronously. + let pay_amt = Amount::from_sats(350).unwrap(); + let invoice = + third_party.bolt11_payment().receive(pay_amt.milli_sats(), &desc, 300).unwrap(); + let info = PaymentInfo::build( + wallet.parse_payment_instructions(&invoice.to_string()).await.unwrap(), + Some(pay_amt), + ) + .unwrap(); + assert!(wallet.pay(&info).await.is_err(), "MPP with an unroutable lightning leg must fail"); + + // The failed lightning leg is internal bookkeeping of the MPP attempt, which + // is surfaced through the trusted leg. It must not appear as an additional, + // standalone failed payment (the only other outbound row is the drain above). + let txs = wallet.list_transactions().await.unwrap(); + let failed_outbound = + txs.iter().filter(|t| t.outbound && t.status == TxStatus::Failed).count(); + assert_eq!( + failed_outbound, 0, + "the failed MPP lightning leg must not list on its own, got {txs:?}" + ); + let trusted_legs = + txs.iter().filter(|t| t.outbound && matches!(t.id, PaymentId::Trusted(_))).count(); + assert_eq!( + trusted_legs, 1, + "the MPP attempt should surface via the trusted leg, got {txs:?}" + ); + }) + .await; +} + #[tokio::test(flavor = "multi_thread")] #[test_log::test] async fn test_payment_with_expired_invoice() {