Skip to content

Surface failed and pending outbound Lightning payments in list_transactions #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hash-money wants to merge 1 commit into from
Closed

Surface failed and pending outbound Lightning payments in list_transactions #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 99 additions & 14 deletions orange-sdk/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -529,15 +529,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;
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
));
}
}
157 changes: 155 additions & 2 deletions orange-sdk/tests/integration_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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(&params.wallet);
let bitcoind = Arc::clone(&params.bitcoind);
let electrsd = Arc::clone(&params.electrsd);
let third_party = Arc::clone(&params.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(&params.wallet);
let bitcoind = Arc::clone(&params.bitcoind);
let third_party = Arc::clone(&params.third_party);
let electrsd = Arc::clone(&params.electrsd);
let lsp = Arc::clone(&params.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() {
Expand Down
Loading