diff options
| author | Adam Soltys <[email protected]> | 2026-05-17 23:22:51 -0700 |
|---|---|---|
| committer | Adam Soltys <[email protected]> | 2026-05-17 23:22:51 -0700 |
| commit | 0e368dab0943067c060ad45578a4c4cbdc3434a6 (patch) | |
| tree | bdca246458c0f80ed1e60a369069d50a72e0c217 | |
| parent | 3b7a1efaa1040de29d97640b371f24b41a6562a0 (diff) | |
| download | coinos-server-0e368dab0943067c060ad45578a4c4cbdc3434a6.tar.gz coinos-server-0e368dab0943067c060ad45578a4c4cbdc3434a6.zip | |
mirror payment records to arc + atomic /confirm lock to prevent double-credits
Follows up the Apr 29 double-credit incident. Two parallel callers of
/confirm (walletnotify, catchUp, bulk sweeps) could each read
p.confirmed=false and each run the multi() that increments balance —
because main-db pointers were sometimes missing while arc still had them,
gf()'s arc fallback returned stale state. Three pieces here:
- routes/payments.ts: SET NX EX 60 lock around the /confirm credit block
keyed by `confirmlock:${txid}:${vout}`. Concurrent callers short-circuit
cleanly. The lock is released on success and on every early-return.
- lib/payments.ts credit() + routes/payments.ts /confirm: mirror the
payment/invoice records and the `payment:<txid>:<vout>` / `payment:<hash>`
pointer keys into arc as soon as they're written. gf() will then find
the prior credit via arc fallback even if the main-db copy is wiped by
a future data-loss event.
- routes/payments.ts hide `revertedDuplicate` payments from the user's
payment list — matches the same filter added to lib/nwc.ts list_transactions
so reverted records never surface in either rendering path.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
| -rw-r--r-- | lib/payments.ts | 11 | ||||
| -rw-r--r-- | routes/payments.ts | 30 |
2 files changed, 37 insertions, 4 deletions
diff --git a/lib/payments.ts b/lib/payments.ts index 9d658294..5d2bf0e6 100644 --- a/lib/payments.ts +++ b/lib/payments.ts @@ -1,6 +1,6 @@ import config from "$config"; import api from "$lib/api"; -import { db, g, ga, gf, s } from "$lib/db"; +import { archive, db, g, ga, gf, s, sa } from "$lib/db"; import { generate } from "$lib/invoices"; import ln from "$lib/ln"; import { err, l, warn } from "$lib/logging"; @@ -298,8 +298,13 @@ export const credit = async ({ p.confirmed = false; balanceKey = "pending"; await s(`payment:${txid}:${vout}`, id); + // Mirror the txid:vout pointer into arc so future bulk /confirm sweeps find + // the prior credit via gf() fallback and don't double-credit. See + // feedback_apr29_double_credit_incident.md for the incident this prevents. + await sa(`payment:${txid}:${vout}`, id); } else { await s(`payment:${hash}`, id); + await sa(`payment:${hash}`, id); } const m = await db.multi(); @@ -323,6 +328,10 @@ export const credit = async ({ .set(`${aid || uid}:payments:last`, p.created) .exec(); + // Mirror the payment record + invoice into arc for the same protection. + await sa(`payment:${p.id}`, p); + await sa(`invoice:${inv.id}`, inv); + if (inv.items?.length) { formatReceipt(inv.items, inv.currency); p.items = inv.items; diff --git a/routes/payments.ts b/routes/payments.ts index 43c098b6..7dc45058 100644 --- a/routes/payments.ts +++ b/routes/payments.ts @@ -1,7 +1,7 @@ import config from "$config"; import api from "$lib/api"; import { requirePin } from "$lib/auth"; -import { archive, db, g, gf, s } from "$lib/db"; +import { archive, db, g, gf, s, sa } from "$lib/db"; import { generate } from "$lib/invoices"; import { replay } from "$lib/lightning"; import ln from "$lib/ln"; @@ -135,6 +135,7 @@ export default { warn("user", id, "missing payment", pid); return; } + if (p.revertedDuplicate) return; if (received && p.amount < 0) return; if (p.created < start || p.created > end) return; if (p.type === PaymentType.internal) @@ -485,9 +486,24 @@ export default { }); } else if (confirmations >= 1) { const id = `payment:${txid}:${vout}`; + // Atomic guard against concurrent /confirm callers double-crediting: + // walletnotify, catchUp, and bulk sweeps can all fire for the same + // txid:vout in parallel. Without this, two callers can each read + // p.confirmed=false and each run the multi() that increments balance. + const lockKey = `confirmlock:${txid}:${vout}`; + const acquired = await db.set(lockKey, "1", { NX: true, EX: 60 }); + if (!acquired) continue; + const p = await getPayment(`${txid}:${vout}`); - if (!p) return db.sAdd("missed", id); - if (p.confirmed) return; + if (!p) { + await db.sAdd("missed", id); + await db.del(lockKey); + return; + } + if (p.confirmed) { + await db.del(lockKey); + return; + } const invoice = await getInvoice(address); const { id: iid } = invoice; @@ -506,6 +522,14 @@ export default { .incrBy(`balance:${p.aid || p.uid}`, p.amount) .exec(); + // Mirror the now-confirmed record into arc so a future bulk sweep + // finds it via gf() fallback even if the main-db pointer is wiped + // (see feedback_apr29_double_credit_incident.md). + await sa(`payment:${p.id}`, p); + await sa(`invoice:${iid}`, invoice); + + await db.del(lockKey); + const user = await g(`user:${p.uid}`); await completePayment(invoice, p, user); } |
