fix: dispatch buyer payouts at most once per settled order

migrations/20260815120000_order_payout_inflight.sql

@@ -0,0 +1,11 @@
+-- Track the payment hash of an in-flight buyer payout so a
+-- `settled-hold-invoice` order is paid at most once.
+--
+-- While this column is non-NULL a payout for the order is considered in
+-- flight: the failed-payment retry job skips the order and `AddInvoice`
+-- swaps are rejected, so no second payout can be dispatched against the
+-- same settled escrow. A reconciliation job resolves the marker against
+-- LND (Succeeded -> finalize, Failed/Unknown -> clear & re-arm retry,
+-- InFlight -> wait) so a stranded payout neither drains funds nor blocks
+-- the order forever.
+ALTER TABLE orders ADD COLUMN payout_payment_hash char(64);

migrations/20260815120100_order_payout_claimed_at.sql

@@ -0,0 +1,9 @@
+-- Timestamp (unix seconds) when a buyer payout was claimed, sealed together
+-- with `payout_payment_hash` in the same CAS.
+--
+-- Reconciliation ignores a marker younger than a grace window so a just-claimed
+-- payout is never misread as "unknown to LND" during the brief window between
+-- the claim and LND registering the payment. Without it, a reconciliation tick
+-- landing in that window could clear the marker and let a second payout be
+-- dispatched for the same escrow (a scriptable timing race).
+ALTER TABLE orders ADD COLUMN payout_claimed_at integer;

src/app/add_invoice.rs

@@ -13,7 +13,11 @@ use sqlx::{Pool, Sqlite};
 ///
 /// Uses a status-guarded targeted `UPDATE` (`WHERE status = settled-hold-invoice`)
 /// so a stale full-row write cannot resurrect payment state after
-/// `payment_success` has already moved the order to `Success`. When the CAS
+/// `payment_success` has already moved the order to `Success`. The
+/// `payout_payment_hash IS NULL` guard additionally rejects the swap while a
+/// prior payout for the order is still in flight: without it, swapping to a
+/// fresh invoice and resetting `payment_attempts` re-arms a new payout on top of
+/// an already-dispatched one (the invoice-swap re-arm drain). When the CAS
 /// misses, returns `CantDo(NotAllowedByStatus)` and does not enqueue
 /// `InvoiceUpdated`.
 pub async fn pay_new_invoice(
@@ -22,7 +26,8 @@ pub async fn pay_new_invoice(
     msg: &Message,
 ) -> Result<(), MostroError> {
     let result = sqlx::query(
-        "UPDATE orders SET buyer_invoice = ?, payment_attempts = 0 WHERE id = ? AND status = ?",
+        "UPDATE orders SET buyer_invoice = ?, payment_attempts = 0 \
+         WHERE id = ? AND status = ? AND payout_payment_hash IS NULL",
     )
     .bind(&order.buyer_invoice)
     .bind(order.id)
@@ -33,7 +38,7 @@ pub async fn pay_new_invoice(
 
     if result.rows_affected() == 0 {
         tracing::warn!(
-            "Ignoring stale buyer invoice update for order {}: row no longer in settled-hold-invoice",
+            "Ignoring buyer invoice update for order {}: not in settled-hold-invoice or a payout is already in flight",
             order.id
         );
         return Err(MostroCantDo(CantDoReason::NotAllowedByStatus));
@@ -332,6 +337,48 @@ mod tests {
             .contains(&Action::InvoiceUpdated));
     }
 
+    /// A swap must be rejected while a payout for the order is already in
+    /// flight (`payout_payment_hash` set): otherwise a fresh invoice would reset
+    /// `payment_attempts` on top of a pending payout and re-arm a second one.
+    #[tokio::test]
+    async fn pay_new_invoice_rejects_swap_while_payout_in_flight() {
+        let pool = setup_pool().await;
+        let seller = Keys::generate().public_key();
+        let buyer = Keys::generate().public_key();
+
+        let mut order = waiting_invoice_sell_order(seller, buyer);
+        order.status = Status::SettledHoldInvoice.to_string();
+        order.payment_attempts = 2;
+        order.buyer_invoice = Some("lnbc-current".to_string());
+        let order = order.create(&pool).await.unwrap();
+
+        // A payout is already in flight for this order.
+        crate::db::claim_order_payout(&pool, order.id, &"a".repeat(64))
+            .await
+            .unwrap();
+
+        let mut swap = order.clone();
+        swap.buyer_invoice = Some("lnbc-new".to_string());
+        let result = pay_new_invoice(
+            &mut swap,
+            &pool,
+            &Message::new_order(Some(order.id), Some(1), None, Action::AddInvoice, None),
+        )
+        .await;
+
+        assert!(
+            matches!(result, Err(MostroCantDo(CantDoReason::NotAllowedByStatus))),
+            "swap must be rejected while a payout is in flight: {result:?}"
+        );
+        // Neither the invoice nor the attempts counter changed.
+        let stored = Order::by_id(&pool, order.id).await.unwrap().unwrap();
+        assert_eq!(stored.buyer_invoice.as_deref(), Some("lnbc-current"));
+        assert_eq!(stored.payment_attempts, 2);
+        assert!(!queued_actions_for(buyer)
+            .await
+            .contains(&Action::InvoiceUpdated));
+    }
+
     /// A `SettledHoldInvoice` order routes through `pay_new_invoice`: the
     /// payment-attempts counter is reset and the buyer is told the invoice
     /// was updated. No LND is involved so the handler returns `Ok`.

src/app/release.rs

@@ -7,10 +7,11 @@ use crate::lightning::LndConnector;
 use crate::lnurl::resolv_ln_address;
 use crate::nip33::{new_order_event_with_created_at, order_to_tags};
 use crate::util::{
-    enqueue_order_msg, get_order, mark_orderbook_publish_failed, monotonic_order_event_timestamp,
-    settle_seller_hold_invoice, update_order_event,
+    bytes_to_string, enqueue_order_msg, get_order, mark_orderbook_publish_failed,
+    monotonic_order_event_timestamp, settle_seller_hold_invoice, update_order_event,
 };
 use crate::Result;
+use bitcoin::hashes::hex::FromHex;
 
 use fedimint_tonic_lnd::lnrpc::payment::PaymentStatus;
 use lnurl::lightning_address::LightningAddress;
@@ -607,7 +608,33 @@ pub async fn do_payment(
         },
         None => payment_request,
     };
+
+    // Connect to LND *before* claiming: if the connection fails, `?` returns
+    // here without a claim, so a transient connect blip never leaves a marker
+    // set with no payment behind it.
     let mut ln_client_payment = LndConnector::new().await?;
+
+    // Idempotency claim: persist the payout invoice's `payment_hash` (and the
+    // claim timestamp) immediately before dispatch. While the marker is set,
+    // `find_failed_payment` skips this order and `pay_new_invoice` rejects
+    // invoice swaps, so no second payout can be dispatched for the same settled
+    // escrow. This CAS also loses to a concurrent claim (two scheduler ticks
+    // racing) — only the winner pays. Cleared on a confirmed-terminal outcome or
+    // by reconciliation; the timestamp keeps reconciliation from acting on this
+    // payout until LND has surely registered it (closing the reconcile-vs-send
+    // race). Placed right before `send_payment` so the window between claim and
+    // LND registering the payment is only the send call itself.
+    let payout_hash = decode_invoice(&payment_request)
+        .map(|inv| bytes_to_string(inv.payment_hash().as_ref()))
+        .map_err(|_| MostroInternalErr(ServiceError::InvoiceInvalidError))?;
+    if !crate::db::claim_order_payout(ctx.pool(), order.id, &payout_hash).await? {
+        warn!(
+            "Order {}: a payout is already in flight (or status changed); skipping duplicate send_payment",
+            order.id
+        );
+        return Ok(());
+    }
+
     let (tx, mut rx) = channel(100);
 
     let payment_task = ln_client_payment.send_payment(&payment_request, amount as i64, tx);
@@ -648,15 +675,24 @@ pub async fn do_payment(
                                 request_id,
                             )
                             .await;
+                            // Terminal success: release our own claim only.
+                            let _ =
+                                crate::db::clear_order_payout(ctx.pool(), order.id, &payout_hash)
+                                    .await;
                         }
                         PaymentStatus::Failed => {
                             warn!(
                                 "Order Id {}: Invoice with hash: {} has failed!",
                                 order.id, msg.payment.payment_hash
                             );
 
-                            // Mark payment as failed
+                            // Mark payment as failed and release our own claim
+                            // (scoped to this hash) so a fresh invoice can be
+                            // retried without erasing a newer claim.
                             check_failure_retries_or_log(&ctx, &order, request_id).await;
+                            let _ =
+                                crate::db::clear_order_payout(ctx.pool(), order.id, &payout_hash)
+                                    .await;
                         }
                         _ => {}
                     }
@@ -724,6 +760,67 @@ async fn payment_success(
     Ok(())
 }
 
+/// Reconcile a single in-flight buyer payout against LND.
+///
+/// Called by the scheduler for every order whose `payout_payment_hash` is set.
+/// This is the counterpart to `do_payment`'s in-process status watcher: it
+/// resolves the durable marker for payouts whose watcher never delivered a
+/// terminal update — a held/stranded HTLC, or a payout whose watcher task was
+/// lost across a restart. Without it, such an order would stay locked forever
+/// (regressing the do-payment-stuck bug); with it, the marker is authoritative
+/// only until LND confirms the real outcome:
+///
+/// - `Succeeded` → finalize as `Success` (idempotent via the status CAS) and
+///   clear the marker.
+/// - `Failed` / `Unknown` / not found → clear the marker and re-arm retry;
+///   `send_payment`'s own pre-send `track_payment_v2` check backstops any race
+///   where LND actually still has the payment.
+/// - `InFlight` → leave as is; the payout is genuinely pending.
+pub async fn reconcile_inflight_payout(
+    ctx: &AppContext,
+    ln_client: &mut LndConnector,
+    order_id: uuid::Uuid,
+    payout_payment_hash: &str,
+) -> Result<(), MostroError> {
+    let pool = ctx.pool();
+
+    // A payment hash is exactly 32 bytes (64 hex chars). Decode with the same
+    // `FromHex` used across the codebase and length-check it; a bad-hex or
+    // wrong-length marker is corrupt, so treat it as malformed and re-arm rather
+    // than sending a truncated hash to LND.
+    let hash_bytes = match Vec::<u8>::from_hex(payout_payment_hash) {
+        Ok(bytes) if bytes.len() == 32 => bytes,
+        _ => {
+            warn!("Order {order_id}: malformed payout_payment_hash; clearing and re-arming retry");
+            crate::db::fail_order_payout(pool, order_id, payout_payment_hash).await?;
+            return Ok(());
+        }
+    };
+
+    match ln_client.lookup_payment_status(&hash_bytes).await {
+        Ok(Some(PaymentStatus::Succeeded)) => {
+            if let Ok(Some(mut order)) = Order::by_id(pool, order_id).await {
+                let my_keys = ctx.keys().clone();
+                if let Ok(buyer_pubkey) = order.get_buyer_pubkey() {
+                    let _ = payment_success(ctx, &mut order, buyer_pubkey, &my_keys, None).await;
+                }
+            }
+            crate::db::clear_order_payout(pool, order_id, payout_payment_hash).await?;
+        }
+        Ok(Some(PaymentStatus::Failed)) | Ok(Some(PaymentStatus::Unknown)) | Ok(None) => {
+            crate::db::fail_order_payout(pool, order_id, payout_payment_hash).await?;
+        }
+        Ok(Some(PaymentStatus::InFlight)) => {
+            // Still pending — do not re-dispatch; a later tick will reconcile.
+        }
+        Err(e) => {
+            warn!("Order {order_id}: payout reconciliation lookup failed: {e}");
+        }
+    }
+
+    Ok(())
+}
+
 /// Check if order is range type
 /// Add parent range id and update max amount
 /// publish a new replaceable kind nostr event with the status updated