fix: retry payout stat-counting and atomic tx_hash write

Two bugs in payoutRetryService.retryPayout:

- The "already on-chain" path guarded the user-stats update with
  NOT EXISTS (... WHERE id = this AND status = 'completed'), but it had just
  marked that same row completed in the same transaction, so the guard was
  always true-negative and stats were never counted. Replaced with a
  conditional UPDATE ... WHERE status <> 'completed' RETURNING, so the row's
  transition drives an exactly-once stat count.
- A successful retry wrote tx_hash in a standalone query and then marked the
  payout completed in a separate transaction; a crash between the two left a
  tx_hash on a non-completed row. Now tx_hash + completion + stats happen in
  one transaction.

Adds payoutRetry.test.js (count-once, no-double-count, atomic completion).
Full suite: 16 suites / 134 tests.

docs/REMEDIATION_PLAN.md

@@ -28,7 +28,7 @@ Key: `S` ≤ half day · `M` ~1–3 days · `L` ~1 week+ · 🔴 launch-blocker
 - [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.
 - [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`).
+- [x] **1.4 S — Fix retry stat guard + non-atomic tx_hash write.** ✅ The "already on-chain" path used a `NOT EXISTS` guard against the row it had just marked completed (always skipped → never counted); replaced with a conditional `UPDATE … WHERE status <> 'completed' RETURNING` so stats count exactly once. Successful retries now store `tx_hash` + mark completed + count stats in ONE transaction (no stranded-tx_hash window). New `payoutRetry.test.js` covers count-once, no-double-count, and atomic completion.
 - [ ] **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`).
 

src/payments/payoutRetryService.js

@@ -119,22 +119,25 @@ class PayoutRetryService {
                     if (txStatus.exists) {
                         // Transaction is in blockchain/mempool - mark as completed, don't retry!
                         await this.db.withTransaction(async (client) => {
-                            await client.query(`
+                            // Transition to completed ONLY if not already completed. The RETURNING
+                            // tells us whether THIS call did the transition, so we count user stats
+                            // exactly once (the previous guard checked the same row it had just
+                            // updated, so it never counted at all).
+                            const transitioned = await client.query(`
                                 UPDATE payouts
                                 SET status = 'completed', processed_at = NOW()
-                                WHERE id = $1
+                                WHERE id = $1 AND status <> 'completed'
+                                RETURNING id
                             `, [id]);
 
-                            // Update user stats (may have been missed if original DB update failed)
-                            await client.query(`
-                                UPDATE users
-                                SET total_amount_won = COALESCE(total_amount_won, 0) + $1,
-                                    total_payouts_received = COALESCE(total_payouts_received, 0) + 1
-                                WHERE id = $2 AND NOT EXISTS (
-                                    SELECT 1 FROM payouts
-                                    WHERE id = $3 AND status = 'completed' AND processed_at IS NOT NULL
-                                )
-                            `, [amount, user_id, id]);
+                            if (transitioned.rows.length > 0) {
+                                await client.query(`
+                                    UPDATE users
+                                    SET total_amount_won = COALESCE(total_amount_won, 0) + $1,
+                                        total_payouts_received = COALESCE(total_payouts_received, 0) + 1
+                                    WHERE id = $2
+                                `, [amount, user_id]);
+                            }
                         });
                         console.log(`✅ Payout ${id} already in blockchain (${txStatus.confirmations} confirmations), marked completed`);
                         return;
@@ -169,31 +172,35 @@ class PayoutRetryService {
                 description: `Retry payout ${id} for game ${game_id}`
             });
 
-            // IMMEDIATELY store tx_hash (same pattern as gameModeManager)
-            if (payoutResult.success && payoutResult.txHash) {
-                await this.db.query(`
-                    UPDATE payouts SET tx_hash = $1, fee = $2 WHERE id = $3
-                `, [payoutResult.txHash, payoutResult.fee, id]);
+            if (!payoutResult.success) {
+                throw new AppError(`Payout ${id} retry did not succeed`);
             }
 
-            // Update payout record with success
+            // Store tx_hash, mark completed, and count stats ATOMICALLY in one transaction,
+            // so the process can't die between writing tx_hash and marking completed (which
+            // previously left a tx_hash on a non-completed row). The conditional transition
+            // (status <> 'completed') makes stat-counting exactly-once.
             await this.db.withTransaction(async (client) => {
-                await client.query(`
+                const transitioned = await client.query(`
                     UPDATE payouts
-                    SET status = 'completed',
+                    SET tx_hash = $1,
+                        fee = $2,
+                        status = 'completed',
                         processed_at = NOW(),
                         retry_count = COALESCE(retry_count, 0) + 1,
                         last_retry_at = NOW()
-                    WHERE id = $1
-                `, [id]);
+                    WHERE id = $3 AND status <> 'completed'
+                    RETURNING id
+                `, [payoutResult.txHash || null, payoutResult.fee || null, id]);
 
-                // Update user stats
-                await client.query(`
-                    UPDATE users
-                    SET total_amount_won = COALESCE(total_amount_won, 0) + $1,
-                        total_payouts_received = COALESCE(total_payouts_received, 0) + 1
-                    WHERE id = $2
-                `, [amount, user_id]);
+                if (transitioned.rows.length > 0) {
+                    await client.query(`
+                        UPDATE users
+                        SET total_amount_won = COALESCE(total_amount_won, 0) + $1,
+                            total_payouts_received = COALESCE(total_payouts_received, 0) + 1
+                        WHERE id = $2
+                    `, [amount, user_id]);
+                }
             });
 
             console.log(`✅ Payout ${id} retry succeeded: ${payoutResult.txHash}`);

test/payoutRetry.test.js

@@ -0,0 +1,87 @@
+/**
+ * Payout retry service tests (Phase 1.4).
+ *
+ * Guards two previously-broken behaviours:
+ *  - the "already on-chain" path counts user stats exactly once (the old NOT EXISTS guard
+ *    checked the same row it had just marked completed, so it never counted), and
+ *  - a successful retry stores tx_hash + marks completed + counts stats in ONE transaction
+ *    (no non-atomic tx_hash write that could strand a tx_hash on a non-completed row).
+ */
+
+const PayoutRetryService = require('../src/payments/payoutRetryService');
+
+function makeHarness() {
+  const client = { query: jest.fn().mockResolvedValue({ rows: [] }) };
+  const db = {
+    query: jest.fn().mockResolvedValue({ rows: [] }),
+    withTransaction: jest.fn().mockImplementation(async (cb) => cb(client))
+  };
+  return { client, db };
+}
+
+function svcWith(db, wallet) {
+  return new PayoutRetryService({
+    db, walletService: wallet, debugManager: { CONSOLE_LOGGING: false }, maxRetries: 3
+  });
+}
+
+const findCall = (mockFn, re) => mockFn.mock.calls.find(c => re.test(c[0]));
+
+describe('PayoutRetryService.retryPayout', () => {
+  test('already-on-chain: marks completed and counts stats exactly once', async () => {
+    const { client, db } = makeHarness();
+    client.query
+      .mockResolvedValueOnce({ rows: [{ id: 5 }] }) // payouts transition (did transition)
+      .mockResolvedValue({ rows: [] });             // users stat update
+    const wallet = {
+      checkTransactionStatus: jest.fn().mockResolvedValue({ exists: true, confirmations: 3 }),
+      processPayout: jest.fn()
+    };
+
+    await svcWith(db, wallet).retryPayout({ id: 5, user_id: 1, amount: '100', tx_hash: 'tx5', status: 'failed', retry_count: 0 });
+
+    // Transition guarded by `status <> 'completed'` (so it counts once).
+    expect(findCall(client.query, /UPDATE payouts[\s\S]*status <> 'completed'/i)).toBeDefined();
+    // Stats were counted.
+    expect(findCall(client.query, /UPDATE users[\s\S]*total_amount_won/i)).toBeDefined();
+    // Did NOT re-send funds (already on chain).
+    expect(wallet.processPayout).not.toHaveBeenCalled();
+  });
+
+  test('already-on-chain but already completed: does NOT double-count stats', async () => {
+    const { client, db } = makeHarness();
+    client.query
+      .mockResolvedValueOnce({ rows: [] }) // payouts transition affected 0 rows (already completed)
+      .mockResolvedValue({ rows: [] });
+    const wallet = {
+      checkTransactionStatus: jest.fn().mockResolvedValue({ exists: true, confirmations: 9 }),
+      processPayout: jest.fn()
+    };
+
+    await svcWith(db, wallet).retryPayout({ id: 5, user_id: 1, amount: '100', tx_hash: 'tx5', status: 'failed', retry_count: 0 });
+
+    expect(findCall(client.query, /UPDATE users[\s\S]*total_amount_won/i)).toBeUndefined();
+  });
+
+  test('successful retry: stores tx_hash, completes, counts stats in one transaction', async () => {
+    const { client, db } = makeHarness();
+    client.query
+      .mockResolvedValueOnce({ rows: [{ id: 7 }] }) // completion transition
+      .mockResolvedValue({ rows: [] });
+    const wallet = {
+      checkTransactionStatus: jest.fn(),
+      processPayout: jest.fn().mockResolvedValue({ success: true, txHash: 'txNEW', fee: 10 })
+    };
+
+    await svcWith(db, wallet).retryPayout({ id: 7, user_id: 1, amount: '100', payout_address: 'wow1addr', status: 'failed', retry_count: 0 });
+
+    // tx_hash + completion happen in the SAME statement (atomic).
+    const completion = findCall(client.query, /UPDATE payouts[\s\S]*tx_hash[\s\S]*status = 'completed'/i);
+    expect(completion).toBeDefined();
+    expect(completion[1]).toContain('txNEW');
+    // No standalone pre-transaction tx_hash write on db.query.
+    expect(findCall(db.query, /UPDATE payouts SET tx_hash = \$1, fee = \$2 WHERE id/i)).toBeUndefined();
+    // Stats counted once.
+    expect(findCall(client.query, /UPDATE users[\s\S]*total_amount_won/i)).toBeDefined();
+  });
+});