mirror of
https://github.com/Such-Software/wownerogue.git
synced 2026-10-17 14:21:25 +02:00
fix: credits BIGINT, idempotent migration 010, atomic migration runner
- Migration 018 widens users.credits and credit_transactions.amount/ balance_after from INTEGER to BIGINT (consistent with the atomic-money columns; removes a latent overflow risk on admin-configurable packages). - Migration 010 rewritten with conname-guarded DO blocks so it is idempotent and safe to re-run / recover from a partial apply (bare ADD CONSTRAINT previously hard-failed on re-run and could wedge startup). - The migration runner now applies each migration and records its ledger row in a single transaction, so a migration can never be applied-but-unrecorded (which would re-run a non-idempotent migration on the next boot). Verified end-to-end against a throwaway Postgres database: all 18 migrations apply cleanly, a second run skips via the ledger, credits is bigint, and the new constraints/indexes are present. Full suite: 16 suites / 134 tests.
This commit is contained in:
@@ -30,7 +30,7 @@ Key: `S` ≤ half day · `M` ~1–3 days · `L` ~1 week+ · 🔴 launch-blocker
|
||||
- [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.)_
|
||||
- [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.
|
||||
- [x] **1.5 S — Fix dead refund send-path.** ✅ The admin refund called a nonexistent `walletRPCService.sendPayment()` (guarded by a `typeof` check), so refunds were silently never sent while still reporting success. Now uses the real `processPayout()` (validates address + transfers, returns `{success, txHash}`) and refunds the **actually received** amount (`received_amount`, falling back to expected).
|
||||
- [ ] **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`).
|
||||
- [x] **1.6 S — `credits` → BIGINT; idempotent 010; atomic migration runner.** ✅ Migration 018 widens `users.credits` + `credit_transactions.amount/balance_after` to BIGINT. Migration 010 rewritten with conname-guarded `DO` blocks (re-runnable, no startup wedge). Runner now applies each migration **and** records the ledger row in one transaction (no applied-but-unrecorded state). Verified end-to-end: all 18 migrations applied cleanly to a throwaway Postgres DB, re-run skipped via the ledger, `credits` confirmed `bigint`, all CHECKs present.
|
||||
|
||||
## Phase 2 — Identity, abuse & concurrency (🟠)
|
||||
- [ ] **2.1 L — Unify identity on `user_id`.** One `IdentityService.resolve(socket)`; kill `getOrCreateUser(socketId)` keying (`gameModeManager.js:926`); key `activeGames` by `user_id`.
|
||||
|
||||
@@ -143,8 +143,22 @@ class DatabaseManager {
|
||||
const filePath = path.join(migrationsPath, file);
|
||||
const sql = await fs.readFile(filePath, 'utf8');
|
||||
console.log(`🔄 Running migration: ${file}`);
|
||||
await this.pool.query(sql);
|
||||
await this.pool.query(`INSERT INTO schema_migrations (filename) VALUES ($1)`, [file]);
|
||||
|
||||
// Apply the migration AND record it in the ledger atomically in one
|
||||
// transaction, so a migration can never be applied-but-unrecorded (which
|
||||
// would re-run a possibly-non-idempotent migration on the next boot).
|
||||
const client = await this.pool.connect();
|
||||
try {
|
||||
await client.query('BEGIN');
|
||||
await client.query(sql);
|
||||
await client.query(`INSERT INTO schema_migrations (filename) VALUES ($1)`, [file]);
|
||||
await client.query('COMMIT');
|
||||
} catch (migErr) {
|
||||
await client.query('ROLLBACK').catch(() => {});
|
||||
throw migErr;
|
||||
} finally {
|
||||
client.release();
|
||||
}
|
||||
console.log(`✅ Migration completed: ${file}`);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,34 +1,41 @@
|
||||
-- Migration 010: Security constraints to prevent invalid data states
|
||||
-- These constraints enforce business rules at the database level as a safety net
|
||||
-- These constraints enforce business rules at the database level as a safety net.
|
||||
--
|
||||
-- Each ADD CONSTRAINT is guarded by a conname check so the migration is idempotent and
|
||||
-- safe to re-run / recover from a partial apply. (Postgres has no ADD CONSTRAINT IF NOT
|
||||
-- EXISTS, and a bare ADD CONSTRAINT on an existing constraint hard-fails, which previously
|
||||
-- could wedge startup.)
|
||||
|
||||
-- Ensure credits can never go negative
|
||||
-- This catches any code bugs that might bypass application-level checks
|
||||
ALTER TABLE users ADD CONSTRAINT users_credits_non_negative
|
||||
CHECK (credits >= 0);
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'users_credits_non_negative') THEN
|
||||
ALTER TABLE users ADD CONSTRAINT users_credits_non_negative CHECK (credits >= 0);
|
||||
END IF;
|
||||
|
||||
-- Ensure total_amount_won can never go negative
|
||||
ALTER TABLE users ADD CONSTRAINT users_total_won_non_negative
|
||||
CHECK (total_amount_won >= 0);
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'users_total_won_non_negative') THEN
|
||||
ALTER TABLE users ADD CONSTRAINT users_total_won_non_negative CHECK (total_amount_won >= 0);
|
||||
END IF;
|
||||
|
||||
-- Ensure total_games_played can never go negative
|
||||
ALTER TABLE users ADD CONSTRAINT users_games_played_non_negative
|
||||
CHECK (total_games_played >= 0);
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'users_games_played_non_negative') THEN
|
||||
ALTER TABLE users ADD CONSTRAINT users_games_played_non_negative CHECK (total_games_played >= 0);
|
||||
END IF;
|
||||
|
||||
-- Ensure total_games_won can never go negative
|
||||
ALTER TABLE users ADD CONSTRAINT users_games_won_non_negative
|
||||
CHECK (total_games_won >= 0);
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'users_games_won_non_negative') THEN
|
||||
ALTER TABLE users ADD CONSTRAINT users_games_won_non_negative CHECK (total_games_won >= 0);
|
||||
END IF;
|
||||
|
||||
-- Ensure payout amounts are always positive
|
||||
ALTER TABLE payouts ADD CONSTRAINT payouts_amount_positive
|
||||
CHECK (amount > 0);
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'payouts_amount_positive') THEN
|
||||
ALTER TABLE payouts ADD CONSTRAINT payouts_amount_positive CHECK (amount > 0);
|
||||
END IF;
|
||||
|
||||
-- Ensure payment amounts are always positive
|
||||
ALTER TABLE payments ADD CONSTRAINT payments_amount_positive
|
||||
CHECK (expected_amount > 0);
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'payments_amount_positive') THEN
|
||||
ALTER TABLE payments ADD CONSTRAINT payments_amount_positive CHECK (expected_amount > 0);
|
||||
END IF;
|
||||
|
||||
-- Ensure multiplier is non-negative (0 means no payout)
|
||||
ALTER TABLE payouts ADD CONSTRAINT payouts_multiplier_non_negative
|
||||
CHECK (multiplier >= 0);
|
||||
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'payouts_multiplier_non_negative') THEN
|
||||
ALTER TABLE payouts ADD CONSTRAINT payouts_multiplier_non_negative CHECK (multiplier >= 0);
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
-- Add index for faster payout retry queries
|
||||
CREATE INDEX IF NOT EXISTS idx_payouts_retry_candidates ON payouts (status, retry_count, last_retry_at)
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
-- Migration 018: Widen credit columns to BIGINT
|
||||
--
|
||||
-- users.credits and credit_transactions.amount/balance_after were INTEGER (max ~2.1e9),
|
||||
-- inconsistent with the BIGINT atomic-money columns and a latent overflow risk since
|
||||
-- CREDITS_PACKAGES is admin-configurable with no upper bound. ALTER ... TYPE BIGINT is a
|
||||
-- no-op when the column is already BIGINT, so this is safe to re-run.
|
||||
ALTER TABLE users ALTER COLUMN credits TYPE BIGINT;
|
||||
|
||||
ALTER TABLE credit_transactions ALTER COLUMN amount TYPE BIGINT;
|
||||
ALTER TABLE credit_transactions ALTER COLUMN balance_after TYPE BIGINT;
|
||||
Reference in New Issue
Block a user