Money E2E harness + security hardening + production-grade UX pass

[vikramarun]

Comprehensive pass across three fronts: the outstanding live money-path E2E, a deep backend+frontend security review, and a production-grade UX upgrade. Main checkout untouched; all work verified.

1. Live E2E money-path smoke test (the outstanding item)

Real-stack harness — local Anvil + real Postgres + real wallets + the real server (SIWE, durable outbox, on-chain settlement). All money paths pass:

• Settled 1v1 (scripts/onchain-demo.sh) — draw returns stakes, funds conserved.
• Settled tournament — 3 wallets lock 1 USDC each on-chain → round-robin → pool distributed 65/25/10 → [10.95, 9.75, 9.30], sum conserved at 30 USDC.
• Abandoned tournament — buy-ins locked → server killed mid-tournament → restart marks it abandoned in Postgres → time jumps past the settle window → every entrant recovers their buy-in via on-chain claimRefund.

Committed as scripts/tournament-e2e.py, and fixed the stale tournament-demo.sh (it 403'd — never sent the organizer's SIWE session to /start — and expected launch tokens the server now #[serde(skip)]s).

2. Security hardening

Backend:

• [HIGH] Un-throttled game/tournament creation → DoS + oracle-gas drain. Added a per-IP create throttle to the creation routes, a global room ceiling in start_game (before any escrow opens), and a per-organizer cap on open buy-in tournaments (stops one wallet looping openTournament to burn oracle ETH).
• SIWE fail-closed if on-chain settlement is configured but SIWE_DOMAIN is unset (no localhost:3000 default in prod).
• UNIQUE(game_id) idempotency backstop on settlement_outbox (migration 0009, verified it applies cleanly to the live-schema DB).

Frontend:

• CSP + security headers (frame-ancestors 'none' blocks the deposit/sign-in clickjacking the audit flagged, plus object-src/base-uri, scoped connect-src, X-Frame-Options/nosniff/Referrer-Policy/Permissions-Policy). Verified it doesn't break the app or the WASM engine.
• Validate + encodeURIComponent wallet path segments in ProfileStats.

Audits confirmed the code is otherwise solid (no XSS sinks, seats bound to the SIWE wallet never a request body, admin owner-gated + fail-closed, no key leakage). Documented follow-ups: session token off localStorage; /ws/game token in a first frame not the URL; build-time escrow pinning; 1v1 restart recovery.

3. Production-grade UX (verified in-browser)

Board now at Chess.com/Lichess parity for core cues (verified live): player name-plates (name, engine, clock, captured material, active-turn highlight), last-move highlight, check indicator — shared across all three game views via Chessboard/PlayerBar/lib/board.

Also: rake/payout disclosure at every wager point; rated/casual tags; spectator page shows players/stake/TC + nav (was a bare UUID); SeatGame spectator socket reconnects instead of freezing mid-wager; fixed the cryptic 0.5+0 time-control bug; responsive header + :focus-visible + reduced-motion; how-it-works + trust footer with a live escrow-contract link; profile-history opponents link; /park duplicate redirects home; Gauntlet/Tournament in nav.

Deferred (need server support / larger refactor): game replay for finished games, clickable move-list scrubbing, settlement resolved/failed state, ratings-on-board.

Verification

• Rust: full workspace tests green (server 36, ledger, persistence-with-DB); backend E2E re-run against the modified binary still passes.
• Web: tsc --noEmit clean, production next build clean (all 10 routes), polyglot book test passes, CSP + engine confirmed in the live preview.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
openchess	Ready	Preview, Comment	Jul 12, 2026 7:27pm

[vikramarun]

Code review (8-angle, high effort) — findings + resolution

Ran 8 finder angles (line-by-line, removed-behavior, cross-file, reuse, simplification, efficiency, altitude, conventions) with per-candidate verification. Findings, most-severe first. All confirmed correctness bugs are fixed in 490a65c.

Fixed

1. [HIGH] payoutForStake fee math wrong (lib/escrow.ts). It raked the fee on the whole pot (2*stake), but ChessEscrow.settleGame (contracts/src/ChessEscrow.sol:259) rakes one stake — winner nets 2*stake - rake. At 1%/1 USDC the UI quoted 1.98 where the contract pays 1.99; the docstring's "matches on-chain arithmetic" was false. Fixed formula + corrected "1% fee on the pot" → "on the winnings" copy. Verified: 1→1.99, 100→199.00, 25→49.75.
2. [HIGH] Spectator page reconnected forever after game-over (app/game/[id]/page.tsx). Unlike SeatGame, it had no terminal flag, so after the room was reaped it churned /ws/game/{id} every ~16s indefinitely. Added the finished guard + a give-up cap (~8 dead reconnects) on both views.
3. [MED] "Loading game details…" forever for a finished/not-yet-live game (meta came only from /games/live). Now retries a few times, then degrades to a clear message.
4. [MED] PlayerBar material edge suppressed when a side was up material with no captures (e.g. after a promotion) — the +N badge was gated behind the captured-list. Decoupled.
5. [LOW] Migration 0009 boot-brick risk — a pre-existing duplicate game_id would abort CREATE UNIQUE INDEX and stop server boot. Added a safe defensive de-dup first (keep a settled row, else newest).

Accepted / documented (not code-changed)

• Global room ceiling & per-organizer tournament cap are best-effort (non-atomic check-then-insert with an async open_* in between). A concurrent burst can overshoot, bounded by the per-IP create throttle — acceptable for a DoS backstop. Added best-effort notes in the comments.
• Inline create throttle (vs a router layer) preserves the codebase's per-entry-point convention (same as reject_if_draining); the tradeoff (a shared layer would throttle same-path GET polls) is sound. Footgun: a future 7th create route must remember the call.
• Promotion phantom captured-pawn in the captured-glyph strip (cosmetic; a fully correct fix needs move history rather than FEN-diff). The numeric edge is now always correct.

Verified clean

No f64 money (payoutForStake all-bigint), every throttled route keeps its drain/maintenance check, seats still bound to the SIWE wallet, commit trailer present. tsc + production next build green; migration re-applies cleanly on boot.

Minor / optional polish (left as-is)

tcLabel called twice per lobby row; game/[id] imports both shortAddress and the deprecated shortAddr; SiteFooter EXPLORER map duplicates wagmi/chains blockExplorers; /games/live full-list fetch to read one game. None affect correctness.

[vikramarun]

Review→fix loop — converged after 4 rounds

Ran the multi-angle review repeatedly, fixing findings each round (with a standing check that the fixes didn't add complexity or new bugs), until a round surfaced nothing worth fixing.

Round	Real issues found & fixed	Notes
1	payout fee-math (HIGH), spectator endless-reconnect (HIGH), "Loading…" forever, PlayerBar edge-suppression, migration boot-brick	commit 490a65c
2	reconnect give-up premature-stop (would stall gauntlet/tournament), meta-latch showing "not live" next to a live board	+ consolidated the two drifted reconnect loops into lib/spectatorSocket.ts; removed a dead rating prop — commit 4b02a65
3	reconnect never-give-up regression (openedAt not reset → churns forever at 500ms if a reconnect never opens), CSP blocked Coinbase Wallet origins	commit eef6882
4	none — converged	fresh sweep verified give-up logic for all socket states, CSP completeness vs RainbowKit's default connectors (MetaMask/Rainbow ride WalletConnect = covered; core WS/WASM/CDN pass), Rust/SQL/scripts clean

Notably, rounds 2 and 3 caught real regressions introduced by the earlier rounds' own fixes (the give-up cap's edge cases, then the consolidated helper's openedAt staleness) — which is exactly what the loop was for.

On complexity: the only structural change the loop made was extracting the duplicated spectator-reconnect loop into one helper (a net simplification that also removed a real drift). The reviewer explicitly judged material(), the CSP, and the migration-dedup as appropriately simple and not worth reverting. The larger useLiveGame reducer-hook refactor was deliberately not done — it's pre-existing debt, and refactoring working money-path views (not fully testable locally) would risk introducing the very bugs this loop guards against.

Accepted (not fixed): material() shows a phantom captured pawn after a promotion — display-only; a correct fix needs move history and isn't worth the complexity.

Every round: tsc + production next build clean, and the reconnect/give-up behavior was verified live in-browser (including killing the server mid-spectate to confirm it reaches "disconnected" instead of churning).