Lightning payment idempotency gate + listener resilience #56

Open

padreug wants to merge 2 commits from fix/payment-idempotency into fix/idempotent-migrations

padreug commented

2026-07-12 10:42:08 +00:00

Owner

Second PR of the refactor series (stacked on #55 — merge that first; this PR's base is fix/idempotent-migrations so only its own commits show).

Problem (CODE-REVIEW-2026-06 #4, #9, #10): Lightning payment recording relied entirely on Fava-side duplicate checks, which are read-then-write races. On restart with a persisted invoice queue, or webhook + poller firing together, the same payment_hash could be recorded twice. One unhandled exception in on_invoice_paid also killed the invoice listener for the rest of the process lifetime.

Changes:

New processed_payments table (migration m005, idempotent): payment_hash PRIMARY KEY makes exactly one claimant win via INSERT ... ON CONFLICT DO NOTHING. Claims are 'processing' while in flight, 'done' after; failures release the claim so redelivery retries; stale claims from a crashed process are cleared at listener startup.
Listener loop body wrapped in try/except — a poison payment logs and the loop continues.
POST /record-payment shares the same claim gate ('done' → replays "already recorded"; in-flight → 409) and now fails closed on Fava transport errors during the duplicate check (503) instead of proceeding to write.
Bug found along the way: the endpoint's duplicate check queried {base_url}/api/journal — base_url already ends in /api, so the check has been silently 404ing and never ran. Fixed.

Note: libra-#22 (account-sync racing Fava init) turned out to be already fixed — wait_for_account_sync gates on wait_for_fava_client() since 09a5d6e; that issue can be closed.

Tests: new tests/test_payment_idempotency.py — double delivery records once; failed recording releases the claim and retry succeeds; listener survives a poison payment and clears stale claims; endpoint 409s while in flight and replays safely. Full suite: 158 passed, 3 skipped, 8 xfailed.

🤖 Generated with Claude Code

Second PR of the refactor series (stacked on #55 — merge that first; this PR's base is `fix/idempotent-migrations` so only its own commits show). **Problem** (CODE-REVIEW-2026-06 #4, #9, #10): Lightning payment recording relied entirely on Fava-side duplicate checks, which are read-then-write races. On restart with a persisted invoice queue, or webhook + poller firing together, the same `payment_hash` could be recorded twice. One unhandled exception in `on_invoice_paid` also killed the invoice listener for the rest of the process lifetime. **Changes:** - New `processed_payments` table (migration m005, idempotent): `payment_hash` PRIMARY KEY makes exactly one claimant win via `INSERT ... ON CONFLICT DO NOTHING`. Claims are `'processing'` while in flight, `'done'` after; failures release the claim so redelivery retries; stale claims from a crashed process are cleared at listener startup. - Listener loop body wrapped in try/except — a poison payment logs and the loop continues. - `POST /record-payment` shares the same claim gate ('done' → replays "already recorded"; in-flight → 409) and now **fails closed** on Fava transport errors during the duplicate check (503) instead of proceeding to write. - Bug found along the way: the endpoint's duplicate check queried `{base_url}/api/journal` — `base_url` already ends in `/api`, so the check has been silently 404ing and never ran. Fixed. Note: libra-#22 (account-sync racing Fava init) turned out to be already fixed — `wait_for_account_sync` gates on `wait_for_fava_client()` since 09a5d6e; that issue can be closed. **Tests:** new `tests/test_payment_idempotency.py` — double delivery records once; failed recording releases the claim and retry succeeds; listener survives a poison payment and clears stale claims; endpoint 409s while in flight and replays safely. Full suite: 158 passed, 3 skipped, 8 xfailed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

padreug added 2 commits

2026-07-12 10:42:08 +00:00

fix(payments): add local idempotency gate for Lightning recording 4fdb358bb0

The Fava-side duplicate checks (add_entry_idempotent, journal-link
scan) are read-then-write races: on restart with a persisted invoice
queue, or webhook + poller firing together, both callers pass the
"not present" check and both insert.

New processed_payments table (m005) keyed on payment_hash; exactly
one claimant wins the INSERT ... ON CONFLICT DO NOTHING. Lifecycle:
'processing' while the write is in flight, 'done' after; failed
recordings release the claim so redelivery retries, and 'processing'
rows from a crashed process are cleared at listener startup.

Also wraps the invoice-listener loop body in try/except so one poison
payment can't kill payment recording for the process lifetime.

Addresses CODE-REVIEW-2026-06 findings #4 and #9.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(payments): record-payment fails closed and shares the claim gate 44e10caac7

Two fixes to POST /api/v1/record-payment:

- The Fava duplicate check caught every exception and proceeded to
  write, so a transient Fava blip produced double entries. It now
  fails closed: transport errors return 503 and the client retries.
  While here: the check queried {base_url}/api/journal, but base_url
  already ends in /api — the doubled path 404'd, meaning the
  duplicate check has silently never run.

- The endpoint now goes through the same processed_payments claim
  gate as the background invoice listener, so the webhook+poller pair
  can't both record the same payment_hash: a 'done' claim replays as
  "already recorded", an in-flight claim returns 409.

Addresses CODE-REVIEW-2026-06 finding #10.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

padreug referenced this pull request

2026-07-12 10:55:07 +00:00

Settlement balance guards, per-currency netting, Decimal rates #57

padreug referenced this pull request

2026-07-12 13:43:11 +00:00

Fava client hardening: write-lock coverage, shared HTTP client, JSON assertions #58

padreug referenced this pull request

2026-07-12 13:52:54 +00:00

Auth exact-matching, review-race guards, centralized account-name validation #59

padreug referenced this pull request

2026-07-12 14:01:56 +00:00

Hygiene sweep: dead code, role-assignment race, cached user lookup, stale files #60

padreug referenced this pull request

2026-07-12 14:13:07 +00:00

Split views_api into a domain package; unshadow /accounts/hierarchy #61

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/payment-idempotency:fix/payment-idempotency

git switch fix/payment-idempotency

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.