Idempotency on on-chain retirement (audit C2)

[brawlaphant]

Summary

Closes the audit's C2-MEDIUM finding: executeRetirement() had no protection against double-execution. If called twice in quick succession, both calls passed checkPrepaidBalance(), both passed authorizePayment(), and both called signAndBroadcast() — two transactions on chain, prepaid balance debited twice.

What this PR does

Two layers of protection:

1. Wallet-level mutex (src/services/wallet.ts)

signAndBroadcast() now serializes through a per-address chained promise. CosmJS auto-sequence handling races when two broadcasts overlap (both fetch the same sequence number, one tx fails) — the mutex prevents that, and as a side effect prevents two broadcasts from the same wallet hitting the chain simultaneously.

2. Idempotency-Key cache (src/services/retirement.ts + db.ts)

• New retirement_idempotency_keys table — keyed by client-supplied opaque key, 24h TTL.
• REST /api/v1/retire reads the standard Idempotency-Key header (or idempotency_key body field as fallback for clients that can't easily set headers).
• executeRetirement checks the cache before any work and returns the cached result on hit.
• Successful results persist with INSERT OR IGNORE so a racing duplicate cannot clobber the first.
• getCachedRetirementResult tolerates malformed JSON; cache write failures are logged but never fail the retirement (on-chain side already succeeded).

The MCP tool path is unaffected — it doesn't pass a key or db, and runs through the same code path it always has. New behavior is purely opt-in via the REST API.

OpenAPI spec updated to document the new header + body field.

Files changed

• src/services/wallet.ts — per-address mutex around signAndBroadcast
• src/server/db.ts — retirement_idempotency_keys table + getCachedRetirementResult / cacheRetirementResult / pruneExpiredIdempotencyKeys
• src/services/retirement.ts — idempotencyKey and db added to RetirementParams; cache check at entry, cache write after success
• src/server/api-routes.ts — read Idempotency-Key header / idempotency_key body field on /api/v1/retire
• src/server/openapi.json — document the new parameter
• src/__tests__/retirement-idempotency.test.ts — 8 new tests

Test plan

• npm run typecheck — clean
• npm test — 57/57 passing (49 prior + 8 new)
• npm run build — clean
• Reviewer: check that 24h TTL is the right value (Stripe's standard is 24h; tunable via the SQL constant if not)
• Reviewer: confirm mutex placement inside signAndBroadcast doesn't conflict with the existing _subscriberLocks in routes.ts (it shouldn't — that lock is per-subscriber, this is per-wallet, and they nest cleanly)

Why narrow

Deliberately scoped to the audit finding. Not included here:

• Auto-pruning of expired idempotency rows on a schedule (helper exists; wiring to a cron is a separate concern).
• Idempotency on the MCP tool path. Each LLM tool call should produce a distinct retirement, so auto-deduping would be wrong without an explicit mechanism for the LLM to opt in.

Refs: AUDIT.md C2-MEDIUM

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request implements idempotency for credit retirements and introduces a per-address mutex for wallet broadcasts to prevent transaction sequence collisions. It adds a database-backed cache for retirement results, updates API routes to handle idempotency keys from headers or request bodies, and modifies the retirement service to utilize this cache. Feedback identifies a security risk where idempotency keys should be scoped to individual users to prevent cross-user collisions and notes a remaining race condition where concurrent requests with the same key could bypass the cache check and lead to double retirements.

[gemini-code-assist]

The idempotency key should be scoped to the authenticated user to prevent collisions. In a multi-tenant system, different users might provide the same Idempotency-Key (e.g., a simple integer or a common string). Without scoping, one user could receive the cached retirement result of another user, which is a security risk.

Suggested change

	const idempotencyKey = typeof rawKey === "string" && rawKey.trim().length > 0 ? rawKey.trim() : undefined;
	const idempotencyKey = typeof rawKey === "string" && rawKey.trim().length > 0 ? `u${user.id}:${rawKey.trim()}` : undefined;

[gemini-code-assist]

This implementation contains a race condition for concurrent requests using the same idempotency key. If two requests arrive simultaneously, both will pass this initial cache check because the first hasn't finished and persisted its result yet. While the wallet mutex in signAndBroadcast prevents sequence number collisions, it will still allow both transactions to be broadcast sequentially, resulting in a double retirement. This fails to fully address the audit finding C2 for concurrent cases.

To fix this, you should use a mutex based on the idempotencyKey and re-verify the cache immediately after acquiring the lock.