From a86f8dc25d0b9a821e5635cad9a8ec37b3ac8976 Mon Sep 17 00:00:00 2001 From: Padreug Date: Thu, 14 May 2026 19:38:32 +0200 Subject: [PATCH] fix(v2): refuse /retry when any leg already completed (double-pay guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught while answering the user's question about retry behaviour. The /retry endpoint previously voided FAILED legs and flipped the settlement back to 'pending', which then re-ran process_settlement. But process_settlement re-creates every leg from scratch (super_fee + operator_split + dca legs); it doesn't dedupe against already-completed ones. So if a previous distribution attempt completed some legs and failed others (status='errored' with mixed leg outcomes), hitting /retry would re-pay every successful leg — actually double-paying real sats. Fix: refuse /retry with 400 when count_completed_legs_for_settlement > 0. The error message tells the operator their options: - Edit the commission_splits ruleset to remove already-paid targets before retrying - Or pay the missing legs out-of-band For the all-failed case (no completed legs), /retry continues to work as before — all-or-nothing retry is safe. This mirrors the existing partial-dispense guard (distribution.apply_partial_dispense_and_redistribute) which refuses when any leg has completed for the same reason (Lightning sats can't be clawed back). Splitpayments doesn't have this concern because each split is a separate one-off payment with no retry semantics — they just log and move on. Our model has an explicit retry but needs the symmetric double-pay guard. Future enhancement (post-v1): make process_settlement leg-aware so it skips already-completed (settlement_id, leg_type, target) tuples on re-run. Would let /retry handle partial-success cases too. Tracked informally as an open thread; not on the omnibus issue yet. 76/76 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- views_api.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/views_api.py b/views_api.py index 5e008e0..e09ea00 100644 --- a/views_api.py +++ b/views_api.py @@ -14,6 +14,7 @@ from lnbits.decorators import check_super_user, check_user_exists from .crud import ( append_settlement_note, + count_completed_legs_for_settlement, create_dca_client, create_deposit, create_machine, @@ -573,7 +574,16 @@ async def api_retry_settlement( Voids any failed legs (completed legs are NEVER re-paid — Lightning sats already moved) and flips status 'errored' → 'pending', then re-invokes process_settlement. The optimistic-lock claim guards - against a concurrent listener re-fire racing this retry.""" + against a concurrent listener re-fire racing this retry. + + REFUSES when any leg has already completed. Reason: process_settlement + re-creates every leg from scratch (super_fee + operator_split + dca); + if a previous attempt already completed some of them, retrying would + DOUBLE-PAY those legs. For partial-success failures, the operator + needs to either edit the commission_splits ruleset to remove the + already-paid targets before retry, or manually pay the missing legs + out-of-band. + """ settlement = await get_settlement(settlement_id) if settlement is None: raise HTTPException(HTTPStatus.NOT_FOUND, "Settlement not found") @@ -586,6 +596,15 @@ async def api_retry_settlement( f"settlement status must be 'errored' to retry " f"(currently '{settlement.status}')", ) + completed = await count_completed_legs_for_settlement(settlement_id) + if completed > 0: + raise HTTPException( + HTTPStatus.BAD_REQUEST, + f"refusing to retry: {completed} leg(s) already completed. " + "Re-running distribution would double-pay them. Edit the " + "commission_splits ruleset to remove the already-paid targets, " + "or manually pay the missing legs.", + ) updated = await reset_settlement_for_retry(settlement_id) if updated is None or updated.status != "pending": raise HTTPException(