Lost recovery after restore-session when invoice payment failed in settled-hold-invoice state

[Catrya]

If an order reaches the settled-hold-invoice status but the Lightning payment to the buyer invoice fails, Mostro sends an add-invoice action request so the user can provide a new invoice.

However, if the user performs a restore-session at this point, Mostro only returns the order with status settled-hold-invoice, does not resend the add-invoice action or any signal indicating that the previous payment attempt failed.

Because of this, the app has no way to know that:

• the invoice payment failed, and
• the buyer must provide a new invoice.

As a result, the order appears as successfully settled, while in reality the buyer has not been paid and has no way to receive their sats anymore.

[arkanoider]

@mostronator can you propose a fix for this? 😄

[BraCR10]

Hello team! How is this issue going? Can I work on this one, or is someone already working on it?

[codaMW]

Hello @BraCR10, Have you already taken this issue? Thank you

[BraCR10]

Hello @BraCR10, Have you already taken this issue? Thank you

Hi @codaMW, feel free to suggest a solution

[mostronatorcoder]

Detailed Implementation Plan for Restore-Session Fix

After analyzing the full payment flow, here is a safe implementation plan that addresses the restore-session bug without introducing race conditions or double-payment risks.

---

Root Cause

When a payment to the buyer fails after settled-hold-invoice:

1. Mostro sends Action::AddInvoice to request a new invoice.
2. If the buyer performs restore-session before responding, Mostro returns the order with status settled-hold-invoice without re-sending AddInvoice.
3. The buyer sees the order as successfully settled but has no way to know a payment retry is needed.

---

Implementation Plan

1. Modify restore_session_action

When returning orders during restore-session, check for orders with:

• status == settled-hold-invoice
• failed_payment == true

For matching orders, include Action::AddInvoice alongside the order in the response.

// In restore_session_action
for order in user_orders {
    if order.status == Status::SettledHoldInvoice 
        && order.failed_payment {
        // Re-send AddInvoice so the buyer knows they need to provide a new invoice
        enqueue_order_msg(
            None,
            Some(order.id),
            Action::AddInvoice,
            Some(Payload::Order(SmallOrder::from(order.clone()))),
            buyer_pubkey,
            None,
        ).await;
    }
    // Continue with normal order return...
}

2. Why This is Safe (No Double-Payment Risk)

The current codebase already has two independent protections against double payment:

a) Atomic state transition in payment_success:

let result = sqlx::query(
    "UPDATE orders SET status = ?, event_id = ? WHERE id = ? AND status = ?"
)
.bind(Status::Success.to_string())
.bind(&order_updated.id)
.bind(Status::SettledHoldInvoice.to_string())
.execute(pool).await;

if result.rows_affected() == 0 {
    tracing::warn!("Order {} already processed by another task");
    return Ok(()); // Abort — already paid
}

Only one concurrent task can transition from settled-hold-invoice to success.

b) Serial scheduler execution:
job_retry_failed_payments processes orders one at a time (await per iteration), so the scheduler itself never triggers concurrent payments for the same order.

c) Invoice overwrite in pay_new_invoice:
When the buyer sends add-invoice, the new invoice overwrites the previous one in the database. Only the most recent invoice will be paid by the scheduler.

---

Potential Race Conditions & Mitigations

Scenario	Risk	Current Protection	Additional Mitigation Needed
Buyer sends invoice from Client A and Client B simultaneously	Two different invoices for same order	Scheduler is serial; only last invoice is stored; atomic transition prevents double success	None — safe by design
Buyer sends invoice while scheduler is actively retrying	Invoice changes mid-payment	do_payment reads invoice at start; scheduler processes one at a time	None — safe
Restore-session sends AddInvoice while scheduler already has payment in flight	Buyer confused by duplicate prompts	No duplicate payment risk, but UX issue	Optional: skip re-sending AddInvoice if payment_attempts > 0 && payment_attempts < max_retries (scheduler is already retrying)

---

Recommended Additional Safeguard (Optional but Recommended)

In pay_new_invoice, add a guard to prevent overwriting an invoice while a payment is actively being retried:

pub async fn pay_new_invoice(
    order: &mut Order,
    pool: &Pool<Sqlite>,
    msg: &Message,
) -> Result<(), MostroError> {
    // If a retry is already scheduled and we have not exceeded max attempts,
    // inform the buyer to wait rather than accepting a new invoice
    if order.failed_payment && order.payment_attempts > 0 {
        // Optionally: send a message saying "Payment retry in progress, please wait"
        // before accepting the new invoice
    }
    
    order.payment_attempts = 0;
    order.buyer_invoice = validate_invoice(&msg, &order).await?;
    order.clone().update(pool).await?;
    
    enqueue_order_msg(
        msg.get_inner_message_kind().request_id,
        Some(order.id),
        Action::InvoiceUpdated,
        None,
        buyer_pubkey,
        None,
    ).await;
    Ok(())
}

---

Files to Modify

1. src/app/restore_session.rs — Add AddInvoice re-send logic for failed-payment orders
2. src/app/add_invoice.rs — Optional guard in pay_new_invoice (recommended)

---

Testing Checklist

• Simulate failed payment → restore-session → verify AddInvoice is re-sent
• Simulate two clients sending different invoices → verify only last one is paid
• Simulate concurrent scheduler retry + new invoice submission → verify no double payment
• Verify payment_success atomic transition still works (rows_affected guard)

---

Summary

This fix is safe to implement. The existing payment retry mechanism already prevents double payments through atomic state transitions and serial scheduler execution. The restore-session fix simply ensures the buyer is informed that a retry is needed, without changing the payment safety model.

If anyone wants to pick this up, I recommend implementing the restore-session change first (file 1), then optionally the pay_new_invoice guard (file 2) as a follow-up.

[mostronatorcoder]

Example: Restore-Session Response Format

Here is an example of how the restore-session response should look before and after the fix.

---

Before (Current Behavior) — Buyer Loses Context

{
  "version": 1,
  "request_id": 42,
  "action": "RestoreSession",
  "content": {
    "orders": [
      {
        "id": "order-uuid-123",
        "status": "settled-hold-invoice",
        "amount": 100000,
        "fiat_amount": 50,
        "fiat_code": "USD",
        "payment_method": "bank transfer",
        "premium": 0,
        "buyer_pubkey": "npub1buyer...",
        "seller_pubkey": "npub1seller..."
      }
    ]
  }
}

Problem: The buyer sees status: settled-hold-invoice and assumes the trade completed successfully. They have no indication that the Lightning payment failed and that they must provide a new invoice.

---

After (Fixed Behavior) — Buyer Informed of Retry Need

{
  "version": 1,
  "request_id": 42,
  "action": "RestoreSession",
  "content": {
    "orders": [
      {
        "id": "order-uuid-123",
        "status": "settled-hold-invoice",
        "amount": 100000,
        "fiat_amount": 50,
        "fiat_code": "USD",
        "payment_method": "bank transfer",
        "premium": 0,
        "buyer_pubkey": "npub1buyer...",
        "seller_pubkey": "npub1seller..."
      }
    ]
  }
}

Plus an additional message queued:

{
  "version": 1,
  "request_id": null,
  "action": "AddInvoice",
  "content": {
    "order": {
      "id": "order-uuid-123",
      "status": "settled-hold-invoice",
      "amount": 100000,
      "fiat_amount": 50,
      "fiat_code": "USD",
      "payment_method": "bank transfer",
      "premium": 0,
      "buyer_pubkey": "npub1buyer...",
      "seller_pubkey": "npub1seller..."
    }
  }
}

Result: The buyer immediately sees AddInvoice alongside the order, clearly indicating that a new invoice is required for the payment retry.

---

Implementation Note

The fix should add the AddInvoice message to the same message queue used by restore_session_action, ensuring it is delivered atomically with the restore-session response. The client will receive both messages and can display the appropriate UI prompt.

[grunch]

@codaMW this is a good one if you want to work on it

[arkanoider]

Example: Restore-Session Response Format

Here is an example of how the restore-session response should look before and after the fix.

Before (Current Behavior) — Buyer Loses Context

{
"version": 1,
"request_id": 42,
"action": "RestoreSession",
"content": {
"orders": [
{
"id": "order-uuid-123",
"status": "settled-hold-invoice",
"amount": 100000,
"fiat_amount": 50,
"fiat_code": "USD",
"payment_method": "bank transfer",
"premium": 0,
"buyer_pubkey": "npub1buyer...",
"seller_pubkey": "npub1seller..."
}
]
}
}
Problem: The buyer sees status: settled-hold-invoice and assumes the trade completed successfully. They have no indication that the Lightning payment failed and that they must provide a new invoice.

After (Fixed Behavior) — Buyer Informed of Retry Need

{
"version": 1,
"request_id": 42,
"action": "RestoreSession",
"content": {
"orders": [
{
"id": "order-uuid-123",
"status": "settled-hold-invoice",
"amount": 100000,
"fiat_amount": 50,
"fiat_code": "USD",
"payment_method": "bank transfer",
"premium": 0,
"buyer_pubkey": "npub1buyer...",
"seller_pubkey": "npub1seller..."
}
]
}
}
Plus an additional message queued:

{
"version": 1,
"request_id": null,
"action": "AddInvoice",
"content": {
"order": {
"id": "order-uuid-123",
"status": "settled-hold-invoice",
"amount": 100000,
"fiat_amount": 50,
"fiat_code": "USD",
"payment_method": "bank transfer",
"premium": 0,
"buyer_pubkey": "npub1buyer...",
"seller_pubkey": "npub1seller..."
}
}
}
Result: The buyer immediately sees AddInvoice alongside the order, clearly indicating that a new invoice is required for the payment retry.

Implementation Note

The fix should add the AddInvoice message to the same message queue used by restore_session_action, ensuring it is delivered atomically with the restore-session response. The client will receive both messages and can display the appropriate UI prompt.

Mostronator solutions looks clean and simple! Yes @codaMW this could be really helpful for mostro restore functionality.

[codaMW]

Thanks @grunch, I'd like to work on this. The implementation plan from mostronatorcoder looks solid. My approach:

Modify restore_session_action to detect orders with status == settled-hold-invoice and failed_payment == true, and re-queue Action::AddInvoice for those orders
No changes to the payment safety model, the existing atomic state transition and serial scheduler already prevent double payments

I'll start with file 1 (src/app/restore_session.rs) and open a draft PR once I have something to show. Will the fix also need a test simulating the failed-payment → restore-session flow?

[grunch]

Thanks @grunch, I'd like to work on this. The implementation plan from mostronatorcoder looks solid. My approach:

Modify restore_session_action to detect orders with status == settled-hold-invoice and failed_payment == true, and re-queue Action::AddInvoice for those orders No changes to the payment safety model, the existing atomic state transition and serial scheduler already prevent double payments

I'll start with file 1 (src/app/restore_session.rs) and open a draft PR once I have something to show. Will the fix also need a test simulating the failed-payment → restore-session flow?

yes just include Action::AddInvoice alongside the order in the response.