Feat/p2e feature flags 87 88 89 90 92 93 95#103
Conversation
#63) - server.test.ts: missing payload/bad signature/stale auth_date → 401, empty ADMIN_TELEGRAM_IDS → 503; refactored signedInitData via buildInitData helper - adminStore.test.ts: new file — 14 unit tests for adjustBalance covering credit/debit/set happy paths, player sync, balance_event skip on delta=0, audit via SQL and via auditLog, not_found/invalid_input/conflict error cases
Stats: default 24h window, 30d window, invalid window 400, response shape. User search: query passthrough, empty result. User detail: found 200, not_found 404, missing accountId 400. Audit: events array, filter passthrough, invalid operation 400. Balance adjust errors: not_found 404, conflict 409, invalid balance kind 400. Admin backend unavailable: no adminStore configured 503.
Add P2eConfig to PaymentsConfig (P2E_REWARDS_ENABLED, P2E_CLAIMS_ENABLED, P2E_ALLOWED_JURISDICTIONS, P2E_BLOCKED_JURISDICTIONS, P2E_MINIMUM_AGE). Both flags default to false. Add GET /p2e/status endpoint and requireP2e guard for future claim routes. Log P2E state at startup.
Add reward_pool and reward_pool_audit_event tables to SpacetimeDB schema. Implement rewardPoolStore with createPool, addFunds, reserveFromPool, releaseFromPool, payoutFromPool, getPool, listPools. Enforce budget limits, write audit rows on every operation, and sanitize internal notes from responses. Wire admin routes POST /admin/pool/* into server with RewardPoolError handling.
Add reward_campaign table to SpacetimeDB schema. Implement rewardCampaignStore with createCampaign (draft), activateCampaign (requires P2E_REWARDS_ENABLED + legal gate + funded pool), pauseCampaign, endCampaign, cancelCampaign, approveLegalGate, and isCampaignActive pure helper. Activation sets 'upcoming' when startTime is in the future.
- Add reward_point_event table to SpacetimeDB schema - Implement calculateBasePoints (pure): win 10, draw 5, loss 2, clean win +5, rating band 1.5x - Forfeit/timeout/disconnect losses award 0 points - createRewardPointsStore: idempotent by matchId, daily cap 500, season cap 10k, pair daily cap 100 - All caps computed from single SELECT; no re-fetch after INSERT - 24 tests covering win/draw/loss/clean-win/forfeit/timeout/duplicate/caps
- createClaimableRewardStore: generateReward (idempotent by idempotencyKey), transitionStatus with valid transitions, audit trail - Status lifecycle: pending→eligibility_review→claimable→claim_started→paid|rejected|expired|canceled - Terminal states (paid/rejected/expired/canceled) block further transitions - Every status change writes a claimable_reward_event audit row - 21 tests: generation, idempotency, status transitions, expiration, audit trail
- RewardPayoutProvider interface: quote, createPayout, syncStatus, cancel?, handleWebhook - createManualPayoutProvider: in-memory MVP for testnet/controlled payouts (pending until admin processes) - createPayoutProvider(type) factory; unsupported types throw not_supported - createFakePayoutProvider for tests: overrides individual methods - PayoutProviderError with stable codes: provider_unavailable, invalid_input, duplicate_payout, not_found, not_supported - 20 tests: quote, create, syncStatus, cancel, webhook success/failure/duplicate/outage
- assessRisk (pure): scores risk from self-match (+100), repeated pair farming (+20), high forfeit rate >40% (+30), abnormal win rate >95% for 5+ matches (+15), low avg rounds (+20) - holdForReview: true when riskScore >= 30 (configurable) - createAbuseDetectionStore: queries reward_point_event window, builds signals, returns RiskAssessment - Signals are human-readable but threshold values not exposed to user-facing messages - 16 tests: zero risk, self-match, pair farming, high forfeit, abnormal win rate, stacked signals, store evaluateAccount
ilyar
left a comment
There was a problem hiding this comment.
Reviewed together with PR #101 and PR #102. This PR is aligned with the future P2E direction at a high level, but it should not be merged in its current form.
Blocking issues:
-
The PR changes the SpacetimeDB schema by adding reward/campaign/pool/claim tables, but does not update generated bindings for TMA or payments. This violates the current project rules and will leave consumers out of sync with the backend schema.
-
The reward pool admin routes are not wired into the production payments service.
server.tssupports injectedrewardPoolStoreand tests inject it, butapps/payments/src/index.tscreates onlyadminStoreand callscreatePaymentsServerwithout arewardPoolStore. In the real service,/admin/pool/*will return 503. -
New stores convert timestamps incorrectly in immediate responses. Several places call
microsToIso(Number(ts / 1000n)), butmicrosToIsoalready expects microseconds and divides by 1000. This makes created/updated timestamps resolve near 1970 instead of now. Seen inrewardPoolStore.ts,rewardCampaignStore.ts,rewardPoints.ts, andclaimableRewardStore.ts. -
Idempotency is claimed but not enforced at the schema level. For example,
claimable_reward.idempotency_keyis only indexed, not unique, and reward point settlement uses a pre-insert SELECT plus random UUID primary key. Retry/concurrency can create duplicates. -
The legal gate is currently mostly declarative.
P2E_ALLOWED_JURISDICTIONS,P2E_BLOCKED_JURISDICTIONS, andP2E_MINIMUM_AGEare parsed, but not actually enforced byrequireP2eor campaign activation. Either implement enforcement or clearly scope this PR as scaffolding only.
Process note: PR #103 already contains the two commits from PR #102. Please merge #102 first, then rebase/update this branch so the diff contains only the P2E/schema work.
microsToIso() expects microseconds and divides by 1000 internally. The immediate-return paths in createPool, createCampaign, generateReward, and settleMatchReward were calling microsToIso(Number(ts / 1000n)) which pre-divided microseconds to milliseconds, then microsToIso divided again to seconds, giving dates near 1970 instead of now. Also wires createRewardPoolStore into index.ts so /admin/pool/* routes are reachable in production instead of always returning 503.
|
Fixed:
Still open (need discussion or tooling):
|
ilyar
left a comment
There was a problem hiding this comment.
Re-reviewed after commit 6900af3.
Resolved since the previous review:
- production
/admin/pool/*routes now receiverewardPoolStorefromapps/payments/src/index.ts; - the timestamp double-division issue in immediate responses is fixed;
- PR #102 has been merged, so this PR diff is now focused on the P2E/schema work.
Remaining blocking issues:
-
SpacetimeDB schema changed but generated bindings were not updated.
apps/spacetime/spacetimedb/src/index.tsaddsreward_campaign,reward_pool,reward_point_event,claimable_reward, and related tables, but the PR still contains nomodule_bindingsupdates. Even if these tables are private/admin-only today, schema changes need all active generated consumers regenerated or a clear documented reason why a consumer is intentionally not updated. -
Reward pool reservation is not atomic.
reserveFromPool()reads the pool, computesremaining, then sends a separate absoluteUPDATE reward_pool SET reserved_amount = .... Two concurrent reservations can both pass the same remaining check and both report success, while only the last absolute update is persisted. For a pre-funded reward pool, this can over-allocate claimable rewards. This should be moved into a transactional/server-authoritative reducer or use an atomic compare-and-update pattern with conflict handling. -
Idempotency is still only best-effort.
claimable_reward.idempotency_keyis indexed but not unique, whilegenerateReward()does SELECT-before-INSERT with a randomrewardId.reward_point_eventsimilarly uses a randomeventIdand checks existing rows in application code before inserting. Retry/concurrency can still create duplicates. Please enforce idempotency at the schema/key level or make the deterministic idempotency key the primary key. -
The legal gate remains mostly declarative.
P2E_ALLOWED_JURISDICTIONS,P2E_BLOCKED_JURISDICTIONS, andP2E_MINIMUM_AGEare parsed, but not enforced byrequireP2e()or campaign activation. Either implement enforcement now or explicitly scope this PR as P2E scaffolding and avoid claiming that a real legal gate exists.
CI note: GitHub verify is green. These are architecture/data-integrity blockers, not compile/test failures.
|
Clarification after looking at PR #101 and PR #103 together: The schema/bindings rule proposed in #101 should not be read as a mechanical requirement to regenerate unused bindings in every possible case. It should be read as a review gate:
For this PR, that means the bindings blocker can be resolved either by regenerating This clarification only applies to the bindings item. The other remaining blockers from the review still stand independently: atomic reward-pool reservation, schema-level idempotency, and whether the legal gate is implemented or explicitly scoped as scaffolding. |
No description provided.