Skip to content

PaymentFailed event can carry a payment whose status is still Pending #1083

Description

@joostjager

AI, to be verified.

Summary

A client can receive a PaymentFailed event whose embedded payment details still say the payment is PENDING. The event type and failure reason say the outbound payment failed, while the payment object in the same event says it is still pending. The observed failing path is an initial BOLT11 send where the first hop is disconnected. LDK reports the send failure as Err(AllFailedResendSafe([Channel unavailable: Peer for first hop currently disconnected])), emits PaymentPathFailed, then emits PaymentFailed with reason: Some(RouteNotFound). ldk-server turns the ldk-node PaymentFailed event into a gRPC PaymentFailed event by reading Node::payment(payment_id) and embedding whatever payment details are returned. If ldk-node still has a stale Pending row, the gRPC event becomes a failed event containing a pending payment.

This is related to restart reconciliation issues around pending payment rows, but distinct: here LDK has already produced a terminal PaymentFailed event, and the user-visible event payload still contains pending status.

Impact

Applications consuming the event stream can treat the same payment as failed and pending at the same time. If the stale ldk-node row survives restart while LDK has already abandoned or failed the payment, Node::payment, list_payments, and ldk-server payment APIs can continue to show the payment as pending even though recovered LDK state no longer has a recent payment entry. A retry may also be blocked by ldk-node's duplicate-payment check because the stale local row is still Pending.

This is an API consistency and payment recovery bug, not a funds-loss claim.

Verified revisions and environment

  • ldk-node f0feefd0280fa439018262b3fafe325d5c8f3990.
  • ldk-server c0a906b4cc8fb96ba5a1ba60f7ffaf3fb629c53f.
  • rust-lightning c2eaf06e6aee06eaf28ebd8d91d23d0a2a661ef2.
  • Reproduction uses ldk-server end-to-end tests with two local ldk-server processes on regtest, bitcoind as the chain source, disk storage with SQLite, one funded direct channel, and an event subscription on the payer before the payment attempt.
  • No splicing or chain reorganization is required. The receiver, which is also the payer's first hop for the direct route, is disconnected before the payer attempts to send.

Observed payload

The contradictory event shape is:

{
  "payment_failed": {
    "payment": {
      "id": "<payment_id>",
      "amount_msat": 2584954,
      "direction": "OUTBOUND",
      "status": "PENDING"
    },
    "reason": 5
  }
}

The server may also log Unable to find payment with paymentId: <payment_id> around the same failure window, which suggests the payment row and failure event are not ordered consistently from the server's point of view.

Focused reproduction

  1. Start two ldk-server nodes on regtest and subscribe to events on the payer.
  2. Open and confirm a channel from payer to receiver.
  3. Have the receiver create a BOLT11 invoice for 2,584,954 msat.
  4. Drop or stop the receiver so the payer's first hop is disconnected.
  5. Have the payer attempt bolt11_send.
  6. Wait for a PaymentFailed event on the payer subscription.
  7. Assert that the event reason is RouteNotFound and that the embedded payment, if present, has status FAILED.

The focused e2e test shape is:

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn test_payment_failed_event_status_after_disconnected_first_hop() {
	let bitcoind = TestBitcoind::new();
	let server_a = LdkServerHandle::start(&bitcoind).await;
	let server_b = LdkServerHandle::start(&bitcoind).await;
	let mut events_a = server_a.client().subscribe_events().await.unwrap();
	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;

	let invoice_resp = server_b.client().bolt11_receive(Bolt11ReceiveRequest {
		amount_msat: Some(2_584_954),
		description: Some(Bolt11InvoiceDescription { kind: Some(bolt11_invoice_description::Kind::Direct("disconnected first hop".into())) }),
		expiry_secs: 3600,
	}).await.unwrap();

	drop(server_b);
	let _ = server_a.client().bolt11_send(Bolt11SendRequest {
		invoice: invoice_resp.invoice,
		amount_msat: None,
		route_parameters: None,
	}).await;

	let event_a = wait_for_event(&mut events_a, |e| matches!(e, Event::PaymentFailed(_))).await;
	match &event_a.event {
		Some(Event::PaymentFailed(payment_failed)) => {
			assert_eq!(payment_failed.reason, Some(PaymentFailureReason::RouteNotFound as i32));
			let payment = payment_failed.payment.as_ref().expect("payment details");
			assert_eq!(payment.status, PaymentStatus::Failed as i32);
		},
		other => panic!("expected PaymentFailed event, got {other:?}"),
	}
}

Run:

cargo test -p e2e-tests test_payment_failed_event_status_after_disconnected_first_hop -- --nocapture

Actual: the event can embed a payment object with PaymentStatus::Pending.

Likely boundary

ldk-node handles LdkEvent::PaymentFailed in src/event.rs by updating PaymentStore with PaymentStatus::Failed, then enqueuing Event::PaymentFailed. However, DataStore::update returns NotFound if the payment row is absent, and the failure status is not remembered for a later pending insert. If LDK emits PaymentFailed before the outbound payment row exists or before it is in the expected state, a pending row can later win.

ldk-server handles Event::PaymentFailed by calling a helper that reads event_node.payment(payment_id), converts whatever payment details it gets, and embeds those details in the gRPC PaymentFailed event. It does not force the embedded payment status to FAILED, so a stale ldk-node Pending row becomes a failed API event containing a pending payment.

Expected behavior

Once LDK emits PaymentFailed for a payment id, ldk-node should not expose that payment as Pending through payment APIs. If the failure arrives before the outbound payment row exists, ldk-node should upsert a failed row, remember the failure so a later pending insert cannot win, or reconcile the status before clients can observe it.

ldk-server should also defend its API invariant: any PaymentFailed event with embedded payment details should have payment.status == FAILED, or it should omit payment details rather than serializing a contradictory status.

Related work checked

ldk-node #969 is related because it covers restart reconciliation when ldk-node's payment store disagrees with LDK's recovered payment state. This issue is distinct because LDK has already produced PaymentFailed. Searches for PaymentFailed pending status, PaymentFailed PaymentStore pending, failed payment event pending, and RouteNotFound Pending payment did not find an exact existing ldk-node issue. Searches in ldk-server for Unable to find payment with paymentId and PaymentFailed pending did not find an exact server-side issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions