From f9c6ce59d21139b21e1d716d67b414e7230e8d73 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 31 May 2026 09:57:56 -0400 Subject: [PATCH] fix: CHECK constraints on money-table statuses; cover processing in payout guard A typo in a status string (e.g. 'complete' vs 'completed') silently produced a money row no index/predicate matched, orphaning funds. - Migration 017 adds idempotent NOT VALID CHECK constraints on payments.status, payments.payment_type and payouts.status. NOT VALID enforces all new writes without failing on any pre-existing rows; conname guards make it re-runnable. - The in-code existing-payout guard in completeGame now also covers the 'processing' status (matching the unique index from migration 015), so a duplicate win can't create a second payable row while a payout is mid-flight. Full suite: 15 suites / 131 tests. --- docs/REMEDIATION_PLAN.md | 2 +- src/game/gameModeManager.js | 2 +- src/migrations/017_money_status_checks.sql | 30 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 src/migrations/017_money_status_checks.sql diff --git a/docs/REMEDIATION_PLAN.md b/docs/REMEDIATION_PLAN.md index a020730..79b895d 100644 --- a/docs/REMEDIATION_PLAN.md +++ b/docs/REMEDIATION_PLAN.md @@ -27,7 +27,7 @@ Key: `S` ≤ half day · `M` ~1–3 days · `L` ~1 week+ · 🔴 launch-blocker ## Phase 1 — Money correctness hardening (🟠) - [x] **1.1 M — `src/money/atomic.js` BigInt module.** ✅ Exact integer money math (`toBig`/`toSafe`/`sum`/`add`/`mulByDecimal`/`format`). `calculatePayout` now computes `base * multiplier` via BigInt (half-up rounding, narrows to number only when exact). `walletRPCService` batch total + fee sum use `money.sum` instead of float `reduce`. Property tests (`moneyAtomic.test.js`) prove exactness on amounts above 2^53 where the old float path drifted. - [x] **1.2 S — Snapshot payout config at game start.** ✅ Migration 016 adds `payout_escape_amount`/`payout_treasure_amount` (BIGINT) + multipliers to `games`. Both start paths stamp the resolved payout amounts (via `_computePayoutSnapshot`); `completeGame` pays from the snapshot (falls back to live calc only for pre-snapshot games). New `payoutSnapshot.test.js` proves a mid-game config change can't alter an in-flight payout. -- [ ] **1.3 S — Close payout uniqueness holes.** Cover `processing` in the unique index; guard INSERT (`gameModeManager.js:1161`); add status/outcome/mode enum CHECKs. +- [x] **1.3 S — Close payout uniqueness holes.** ✅ `processing` covered in the unique index (migration 015) and in the in-code existing-payout guard. Migration 017 adds idempotent `NOT VALID` CHECK constraints on `payments.status`, `payments.payment_type`, and `payouts.status` so a typo'd status can't silently orphan a money row (enforces new writes, won't choke on legacy rows). _(games.status/game_mode CHECKs deferred — transient states, lower money-impact.)_ - [ ] **1.4 S — Fix retry stat guard + non-atomic tx_hash write** (`src/payments/payoutRetryService.js:121,174`). - [ ] **1.5 S — Remove dead refund send-path** (`src/index.js:795`). - [ ] **1.6 S — `credits` → BIGINT; make migration 010 idempotent** (`src/migrations/010_security_constraints.sql`); per-migration transactions in runner (`src/db/databaseManager.js:146`). diff --git a/src/game/gameModeManager.js b/src/game/gameModeManager.js index dbd41e9..6544449 100644 --- a/src/game/gameModeManager.js +++ b/src/game/gameModeManager.js @@ -1193,7 +1193,7 @@ class GameModeManager { // Check if payout was already processed for this game (prevent duplicates) if (gameDbId) { const existingPayout = await this.db.query(` - SELECT id FROM payouts WHERE game_id = $1 AND status IN ('pending', 'completed') + SELECT id FROM payouts WHERE game_id = $1 AND status IN ('pending', 'processing', 'completed') `, [gameDbId]); if (existingPayout.rows.length > 0) { console.log(`⚠️ Payout already exists for game ${gameId}, skipping duplicate`); diff --git a/src/migrations/017_money_status_checks.sql b/src/migrations/017_money_status_checks.sql new file mode 100644 index 0000000..c0951b1 --- /dev/null +++ b/src/migrations/017_money_status_checks.sql @@ -0,0 +1,30 @@ +-- Migration 017: CHECK constraints on money-table status/type columns +-- +-- payments.status, payouts.status and payments.payment_type were free-form VARCHARs. +-- A typo in any one query (e.g. 'complete' vs 'completed') silently produces a row that +-- no index/predicate matches — orphaning money (a payout stuck in a misspelled status is +-- invisible to both the batcher and the retry candidate query). These CHECKs catch typos. +-- +-- Added NOT VALID so they enforce all NEW writes without failing on any pre-existing rows, +-- and guarded by conname checks so the migration is idempotent / safe to re-run. + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'payments_status_check') THEN + ALTER TABLE payments + ADD CONSTRAINT payments_status_check + CHECK (status IN ('pending', 'confirmed', 'expired', 'refunded')) NOT VALID; + END IF; + + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'payments_payment_type_check') THEN + ALTER TABLE payments + ADD CONSTRAINT payments_payment_type_check + CHECK (payment_type IN ('single_game', 'credits_package')) NOT VALID; + END IF; + + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'payouts_status_check') THEN + ALTER TABLE payouts + ADD CONSTRAINT payouts_status_check + CHECK (status IN ('pending', 'processing', 'completed', 'failed', 'needs_review', 'permanently_failed', 'batched')) NOT VALID; + END IF; +END $$;