1157 lines
86 KiB
Markdown
1157 lines
86 KiB
Markdown
---
|
||
title: DB Query & Schema Audit — 2026-06-06
|
||
tags: [audit, database, performance, schema, query-patterns]
|
||
created: 2026-06-06
|
||
updated: 2026-06-07
|
||
---
|
||
|
||
# DB Query & Schema Audit — 2026-06-06
|
||
|
||
> [!info] Scope
|
||
> Full automated multi-agent audit of all Drizzle repository implementations, service layer query patterns, and schema definitions. 8 parallel agents covered `src/services/`, `src/db/repositories/drizzle/`, and `src/db/schema/`. Synthesised and ranked by a 9th agent. 104 findings total.
|
||
|
||
## Already Fixed (in this session)
|
||
|
||
| Issue | PR / Commit |
|
||
|---|---|
|
||
| Stats endpoint: 10 serial `findPurchaseRequests` queries → 1 `GROUP BY status` | `2484150` v2.9.13 |
|
||
| Offer filter: per-offer `sameUser()` calls (N×2 queries) → resolve-once + in-memory filter | `2484150` v2.9.13 |
|
||
| Seller notification fan-out: in-JS filter + 100-row hard cap → SQL `watchedCategory` filter, no limit | `2484150` v2.9.13 |
|
||
| Auth user hydration: `rowToUser` child lookups per row → batched token/passkey/referrer hydration per result set | `4aa6ccb` v2.9.14 |
|
||
| Auth user save: users/tokens/passkeys independent writes → single `BEGIN`/`COMMIT` transaction with rollback | `4aa6ccb` v2.9.14 |
|
||
| Auth token/passkey save loops: one INSERT per child row → bulk token/passkey inserts inside the transaction | `4aa6ccb` v2.9.14 |
|
||
| Bulk notification canonicalization: one UUID→legacy lookup per notification → one batched lookup per bulk call | `2a56f98` v2.9.15 |
|
||
| Offer acceptance loser notifications: sequential per-offer INSERTs → one `createNotificationsBulk` call | `2a56f98` v2.9.15 |
|
||
| Template batch conversion: first-seller templates fetched twice → first-pass template cache reused | `2a56f98` v2.9.15 |
|
||
| Dispute statistics: four repeated status count scans → one status `GROUP BY` query | `2a56f98` v2.9.15 |
|
||
| Chat query reads: full-table `findRows()` scan → SQL pushdown for participant/archive/unread filters with in-memory fallback only for unsupported predicates | `3ad3bbe` v2.9.16 |
|
||
| Chat JSONB filters: missing `participants`/`unread_counts` GIN indexes → added schema indexes and migration `0020_chat_jsonb_indexes.sql` | `3ad3bbe` v2.9.16 |
|
||
| Buyer purchase request list: per-request latest-payment lookup → one `DISTINCT ON (purchase_request_id)` payment query per page | `3ad3bbe` v2.9.16 |
|
||
| Notification bulk mark/delete: serial per-id writes with no cap → set-based repo operations plus 100-id API cap | `3ad3bbe` v2.9.16 |
|
||
| Payments admin list: unbounded `findAllPayments` plus per-row buyer/seller lookups → single joined payment/user query | `0835be9` v2.9.17 |
|
||
| Seller template list: per-template seller/category lookups → batched seller/category page lookups | `0835be9` v2.9.17 |
|
||
| Payment coordinator rejected-seller notifications: per-seller notification loop → one `createNotificationsBulk` call | `0835be9` v2.9.17 |
|
||
| Category path lookup: one query per ancestor level → one recursive CTE returning root-to-leaf path | `0835be9` v2.9.17 |
|
||
| Payment export: `listForExport` fetched all matching payments → bounded export query with default/max limits | `5ff0013` v2.9.18 |
|
||
| Seller lookup: `findSellers` without input limit loaded all sellers → safe default cap while preserving explicit limits | `5ff0013` v2.9.18 |
|
||
| Active template seller list/detail: full seller/template table scans → capped list query and scoped single-seller detail query | `5ff0013` v2.9.18 |
|
||
| SHKeeper migration report: loaded all SHKeeper payments → bounded sorted scan with explicit `maxRecords` cap | `5ff0013` v2.9.18 |
|
||
| M2: `updatePurchaseRequestStatus` 3rd redundant read → reuse first `currentRequest` with status override | `2abba67` v2.9.19 |
|
||
| M5: `createOffer` fetched PR twice → cached first `requestForOffer` reused for notification path | `2abba67` v2.9.19 |
|
||
| M7: `createReviewRecord` sequential `resolveUserUuid` → `Promise.all` parallel | `2abba67` v2.9.19 |
|
||
| M8: `getUserPoints` sequential `findActiveLevelConfigByLevel` × 2 → `Promise.all` parallel | `2abba67` v2.9.19 |
|
||
| M9: `getUnreadMessageCountByUser` full scan + JS filter → single SQL aggregation with `jsonb_array_elements` | `current` v2.9.26 |
|
||
| M12: `PgQuery.exec` sort/skip/limit pushed to SQL in `findPgUsersByQuery` + all producers accept pgQuery param | `current` v2.9.26 |
|
||
| M6: `getRequestTemplateStats` 4 queries (3 in parallel) → 1 combined aggregate + 1 top-5 query | `2abba67` v2.9.19 |
|
||
| M11: `create()` / `normalizeUserFilter()` sequential `resolveUserUuid` calls → `Promise.all` parallel | `2abba67` v2.9.19 |
|
||
| H40/M31: Missing indexes on `payments` (provider, purchaseRequestId, provider+status, pr+status+created) → migration `0021_missing_indexes.sql` | `2abba67` v2.9.19 |
|
||
| M35: Missing partial index on `reviews.purchaseRequestId` → migration `0021_missing_indexes.sql` | `2abba67` v2.9.19 |
|
||
| M36: Missing index on `payment_quotes.expiresAt` → migration `0021_missing_indexes.sql` | `2abba67` v2.9.19 |
|
||
| M33: Missing index on `categories.parent_id` → added to `categoryStore.ts` SQL schema init | `2abba67` v2.9.19 |
|
||
| M27: Duplicate-detection in `createPurchaseRequest` in-JS `find()` over 20-row page → targeted `findRecentDuplicateRequest` SQL query | `2abba67` v2.9.19 |
|
||
| M28: `DrizzleChatRepo.count()` fetched all rows and used `.length` → single `SELECT COUNT(*)` using same SQL predicates as `findRows` | `2abba67` v2.9.19 |
|
||
| H39: `fundsLedgerEntries` missing composite indexes on `(paymentId,entryType)`, `(purchaseRequestId,entryType)`, `occurredAt` → migration `0021_missing_indexes.sql` | `2abba67` v2.9.19 |
|
||
| H41: `purchase_requests` missing composite index `(buyerId,status)`, `seller_offers` missing `(status,createdAt)` and `(sellerId,status)` → schema + migration `0021_missing_indexes.sql` | `3955430` v2.9.19 |
|
||
| H42: `seller_offers` missing partial index on `validUntil` for expiry sweeper queries → schema + migration `0021_missing_indexes.sql` | `3955430` v2.9.19 |
|
||
| M1: Request Network reconciliation serial per-payment HTTPS loop → bounded parallel status fetches with sequential DB application | `61aa42a` v2.9.20 |
|
||
| M3: `findPaymentById` payment + buyer + seller SELECTs → one joined payment/user query | `61aa42a` v2.9.20 |
|
||
| M4: `mirrorQuoteToPaymentMetadata` find-then-update → single JSONB metadata merge UPDATE | `61aa42a` v2.9.20 |
|
||
| M10: `ChatService.addParticipant` per-user validation loop → `userRepo.findManyByIds` batch lookup | `61aa42a` v2.9.20 |
|
||
| M20: delivery attempt delivery-info lookup + insert/select → single `INSERT ... SELECT` and joined list query | `61aa42a` v2.9.20 |
|
||
| M22: `findPgUsersByQuery` unbounded SELECT → 1000-row safety cap | `61aa42a` v2.9.20 |
|
||
| M23: `BlogService.getAllPosts` default limit 100 → default limit 20 | `61aa42a` v2.9.20 |
|
||
| M29: `payments.metadata->>'legacyOrderId'` path lacked index → partial functional index in migration `0022_more_indexes.sql` | `61aa42a` v2.9.20 |
|
||
| M34: `derived_destinations` missing `(buyerId,chainId)` index → schema index + migration `0022_more_indexes.sql` | `61aa42a` v2.9.20 |
|
||
| M37: `point_transactions` source-only queries could not use `(type,source)` index → `idx_pt_source` schema index + migration | `61aa42a` v2.9.20 |
|
||
| M42: `createPointTransaction` read-then-insert idempotency check → `onConflictDoNothing` upsert with conflict fetch | `61aa42a` v2.9.20 |
|
||
| H11: `processReferralReward` duplicate referrer snapshot reads → reuse atomic reward result for socket payload and return value | `885745e` v2.9.20 |
|
||
| H12: `updateReferralStats` count/update outside a transaction → serializable row-lock transaction | `885745e` v2.9.20 |
|
||
| H26: `processReferralReward` independent points + referralStats writes → one idempotent `grantReferralReward` transaction for points, referralStats, and ledger row | `885745e` v2.9.20 |
|
||
| H24: `verifyAndMarkDeliveryCodeUsed` read-check-then-write race → one conditional `UPDATE ... RETURNING` decides delivery-code consumption, with post-miss read only for failure reason | `f22794a` / `51ca048` v2.9.21 |
|
||
| H13: `DrizzleChatRepo.create` insert-then-update welcome message → build initial system message, lastMessage, and unread counts in the INSERT payload | `2c5e80d` v2.9.24 |
|
||
| H25: `updatePoints` + `createPointTransaction` could run outside one transaction → runtime guard requires a transaction-bound repo or explicit tx for both money writes | `2c5e80d` v2.9.24 |
|
||
| M16: `releaseDeletedUserEmail` read-then-write release race → one conditional `UPDATE ... WHERE email/status ... RETURNING` atomically releases deleted-user emails | `fcee958` v2.9.25 |
|
||
| C6: `notifications.user_id` text recipient key → UUID FK to `users(id)` with legacy/UUID repo resolution and migration backfill | `38d0e76` v2.9.26 |
|
||
| C7: `disputes` purchase-request/user relationship columns text → UUID FKs with legacy/UUID repo resolution and migration backfill | `b743b5e` v2.9.28 |
|
||
| H22: `DisputeService.createDispute` split chat/dispute writes → serializable dispute transaction, immediate chat cleanup on rollback, and TTL orphan-dispute-chat sweep | `8fc2309` / `4766eba` v2.9.32 |
|
||
| H23: `DisputeService.resolveDispute` dispute update + releaseHold separate writes → one serializable transaction with transaction-bound dispute/release-hold repos | `8fc2309` v2.9.30 |
|
||
| H28: dispute timeline/evidence read-modify-write arrays → atomic SQL JSONB append expressions | `8fc2309` v2.9.30 |
|
||
| H37: disputes status/priority/category plain text → pgEnum columns plus data-normalizing migration | `8fc2309` v2.9.30 |
|
||
|
||
---
|
||
|
||
## Summary
|
||
|
||
| Severity | Count |
|
||
|---|---|
|
||
| Critical | 8 |
|
||
| High | 42 |
|
||
| Medium | 44 |
|
||
| Low | 10 |
|
||
| **Total** | **104** |
|
||
|
||
---
|
||
|
||
## 🔴 Critical (8)
|
||
|
||
### 1. rowToUser fires 3 extra queries per user row — 1+3N queries on every user fetch
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/auth/authStore.ts:345-382`
|
||
|
||
Every call to `rowToUser` issues 3 additional Postgres round-trips: one for refresh tokens (`user_refresh_tokens`), one for passkeys (`user_passkeys`), and one for the referred-by legacy id. `findPgUsersByQuery` maps `rowToUser` over every result row, so fetching N users produces 1 + 3N queries. All auth paths — login, token refresh, profile, Telegram auth — go through this.
|
||
|
||
**Fix:** Join all three child tables in the parent SELECT using LEFT JOIN … ARRAY_AGG / JSON_AGG, or issue three batch queries (WHERE user_id = ANY($ids)) before mapping, then build an in-memory map keyed by user_id to populate each result without per-row round-trips.
|
||
|
||
---
|
||
|
||
### 2. DrizzleChatRepo.findRows fetches entire chats table into memory before filtering
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/db/repositories/drizzle/DrizzleChatRepo.ts:544-558`
|
||
|
||
`findRows()` runs `SELECT * FROM chats ORDER BY last_activity` with no WHERE clause, pulling every chat row (including the full JSONB `messages` array — potentially hundreds of messages per row) into Node.js memory, then applies a JavaScript-level `matchesQuery()` predicate. Every public read path — `findOne()`, `findForUser()`, `count()`, and `getUnreadMessageCountByUser()` — goes through this method. Memory grows O(total_chats × avg_messages), causing OOM or multi-second pauses on each user request.
|
||
|
||
**Fix:** Translate the common query predicates (participant userId, isActive, settings_is_archived) into SQL WHERE clauses. The participants JSONB column supports GIN-indexed containment: `WHERE participants @> '[{"userId": "<id>", "isActive": true}]'::jsonb`. Add a GIN index on `participants` and push filtering to the database. The in-memory engine can remain as a fallback for uncommon predicates only.
|
||
|
||
---
|
||
|
||
### 3. chats.participants and unread_counts JSONB columns have no GIN index
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/chat.ts:95-114`
|
||
|
||
The `participants` (line 95), `messages` (line 100), and `unreadCounts` (line 111) JSONB columns have no GIN indexes. The schema only indexes `type`, `created_by`, and `last_activity`. Every user-facing chat query ("get my chats", participant membership, unread count) that reaches SQL level will perform a full sequential scan of JSONB arrays. Even after the in-memory filter is replaced with SQL, queries remain O(n) without GIN indexes.
|
||
|
||
**Fix:** Add GIN indexes in the schema: `participantsGinIdx: index('chats_participants_gin_idx').using('gin').on(t.participants)` and `unreadCountsGinIdx: index('chats_unread_counts_gin_idx').using('gin').on(t.unreadCounts)`. Long-term: normalize participants into a `chat_participants` table and unread counts into `chat_unread_counts` with B-tree indexes on (user_id).
|
||
|
||
---
|
||
|
||
### 4. findAllPayments fires 1 + 2N queries — one findPaymentById (3 queries) per row in the payments table
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:535-542`
|
||
|
||
`findAllPayments` fetches every payment row (unbounded SELECT *), then calls `this.findPaymentById(row.id)` for every row in `Promise.all`. Each `findPaymentById` fires two additional SELECT queries against users (buyer + seller). With N payments this is 1 + 2N round-trips, and the outer SELECT has no pagination — it materialises the entire payments table.
|
||
|
||
**Fix:** Replace with a single JOIN query: `SELECT payments.*, b.first_name, b.last_name, b.email, s.first_name, s.last_name, s.email FROM payments LEFT JOIN users b ON b.id = payments.buyer_id LEFT JOIN users s ON s.id = payments.seller_id ORDER BY payments.created_at DESC LIMIT $limit OFFSET $offset`. Remove the per-row `findPaymentById` call.
|
||
|
||
---
|
||
|
||
### 5. findPurchaseRequestsByBuyer fires one additional payment query per request row
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:1033-1053`
|
||
|
||
After fetching one page of `purchase_requests` rows, the method wraps every row in `Promise.all` and fires a separate SELECT against the `payments` table per row (lines 1035-1045). A page of 20 requests produces 21 DB round-trips. The inner query is also not supported by a covering index on `(purchase_request_id, status, created_at DESC)`.
|
||
|
||
**Fix:** Collect all request IDs from the page, run a single `SELECT DISTINCT ON (purchase_request_id) * FROM payments WHERE purchase_request_id = ANY($ids) AND status IN (...) ORDER BY purchase_request_id, created_at DESC`, then join the results in memory. Alternatively use a lateral join in the original SQL statement.
|
||
|
||
---
|
||
|
||
### 6. notifications.user_id stored as text instead of uuid — no FK constraint or type enforcement
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/notification.ts:21`
|
||
|
||
`notifications.userId` is declared as `text('user_id').notNull()` while every other user FK in the schema is `uuid`. This bypasses UUID type enforcement, referential integrity (no FK constraint to `users(id)`), and join efficiency. The composite indexes on `(userId, createdAt)`, `(userId, isRead)`, and `(userId, category)` all operate on a text column, making them incompatible with a future FK migration without a rebuild.
|
||
|
||
**Fix:** Change to `uuid('user_id').notNull().references(() => users.id, { onDelete: 'cascade' })`. If the FK constraint must be deferred during migration, at minimum change to `uuid` type and add the FK in a follow-on migration.
|
||
|
||
---
|
||
|
||
### 7. disputes FK columns (purchaseRequestId, buyerId, sellerId, adminId) stored as text instead of uuid
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/dispute.ts:83-86` | **FIXED** `b743b5e` v2.9.28
|
||
|
||
All four entity-reference columns on the disputes table are `text`, bypassing UUID type safety, FK integrity, and efficient indexed equality matching. A dispute can reference a deleted purchase request or user without any database-level error. The schema comment at line 8 acknowledges this as intentional during cutover, but there is no migration plan or deadline to fix it.
|
||
|
||
**Fix:** Change `purchaseRequestId` to `uuid('purchase_request_id').notNull()` with a `foreignKey()` to `purchase_requests(id)`, `buyerId` and `sellerId` to `uuid` with FKs to `users(id)`, and `adminId` to nullable `uuid` with FK. Follow the `foreignKey()` block pattern already used in `payment.ts`.
|
||
|
||
---
|
||
|
||
### 8. savePgUser writes users, tokens, and passkeys without a database transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/auth/authStore.ts:410-539`
|
||
|
||
`savePgUser` performs: (1) an UPSERT into `users`, (2) DELETE + per-token INSERT loop into `user_refresh_tokens`, (3) DELETE + per-passkey INSERT loop into `user_passkeys` — all as independent statements on the pool (no transaction). A crash or query failure mid-way leaves a partially-updated row. A password change could land in `users` while old refresh tokens remain valid (token reuse). Every auth path — login, Google sign-in, Telegram, registration — calls this function.
|
||
|
||
**Fix:** Acquire a single pool client, wrap all three operations in `BEGIN`/`COMMIT`/`ROLLBACK`. Also consolidate the double-save patterns in login (lines 405-426), Google sign-in (lines 1381-1416), and Telegram auth (lines 459-613) so each request calls `save()` exactly once inside this transaction.
|
||
|
||
---
|
||
|
||
## 🟠 High (42)
|
||
|
||
### 1. refresh-token and passkey save loops issue one INSERT per item instead of a bulk statement
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/auth/authStore.ts:508-539`
|
||
|
||
After the main user UPSERT, the code iterates `user.refreshTokens.slice(-10)` (lines 508-514) and `user.passkeys` (lines 516-539), firing one INSERT per item. A user with 10 tokens + 2 passkeys produces 12 sequential INSERTs per login, token refresh, or Telegram auth. This compounds the missing-transaction problem above.
|
||
|
||
**Fix:** Replace token loop with a single `INSERT INTO user_refresh_tokens (token, user_id) SELECT unnest($1::text[]), $2::uuid ON CONFLICT DO UPDATE`. Replace passkey loop with a single multi-row `VALUES` INSERT with ON CONFLICT. Both are inside the transaction described in the savePgUser finding.
|
||
|
||
---
|
||
|
||
### 2. createNotificationsBulk runs one toCanonicalUserId SELECT per notification in the batch
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/notification/NotificationService.ts:66-77`
|
||
|
||
`createNotificationsBulk` maps every notification through `Promise.all` calling `toCanonicalUserId` per item. Each `toCanonicalUserId` fires `SELECT legacy_object_id FROM users WHERE id = $1`. For a 50-seller broadcast this produces 50 Postgres queries purely for id normalisation before the bulk INSERT.
|
||
|
||
**Fix:** Collect all UUID-shaped ids from the batch, run a single `SELECT id, legacy_object_id FROM users WHERE id = ANY($1)`, build a lookup map, and apply it in-memory before issuing the bulk INSERT.
|
||
|
||
---
|
||
|
||
### 3. bulkMarkAsRead and bulkDelete process each notification in a serial loop with no array-size cap
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/notification/notificationController.ts:218-232, 256-270`
|
||
|
||
Both `bulkMarkAsRead` and `bulkDelete` iterate `notificationIds` with `for (const id of notificationIds)` and call `markAsRead` / `deleteNotification` individually. This produces N sequential DB writes plus N socket emissions. The array is user-supplied with no size cap, allowing an attacker to trigger an arbitrarily large number of DB round-trips.
|
||
|
||
**Fix:** Add a bulk-update method to the notification repo using `UPDATE … WHERE id = ANY($1)` and `DELETE … WHERE id = ANY($1)`. Add a hard cap (e.g. 100 ids) validated at the API layer before processing.
|
||
|
||
---
|
||
|
||
### 4. listRequestTemplates fires two extra queries per row for seller and category lookups
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:2077-2080`
|
||
|
||
After fetching a page of `request_templates` rows, the code calls `Promise.all(rows.map(async row => mapRequestTemplateRow(row.template, { seller: await this.findTemplateSeller(...), category: await this.findTemplateCategory(...) })))`. Each `findTemplateSeller` and `findTemplateCategory` issues an individual SELECT. For a page of 20 rows this is 1 (page) + 1 (count) + 40 (seller/category) = 42 round-trips.
|
||
|
||
**Fix:** Collect unique `sellerIds` and `categoryIds` from the page, batch-fetch with `WHERE id = ANY($ids)`, build in-memory Maps, then pass them into `mapRequestTemplateRow` without per-row queries.
|
||
|
||
---
|
||
|
||
### 5. batchConvertTemplates fetches each template twice — once to group by seller, once to process
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/marketplace/RequestTemplateService.ts:498-523`
|
||
|
||
The first loop (lines 498-507) calls `this.getTemplateByShareableLink(item.shareableLink)` for every cart item to build `sellerGroups`. The second loop (lines 521-523) calls the same method again for every item in `firstSellerItems`. For a cart with N items this is 2N round-trips where N would suffice.
|
||
|
||
**Fix:** Cache the template objects from the first loop in a `Map<shareableLink, template>` and reuse them in the second loop instead of re-fetching.
|
||
|
||
---
|
||
|
||
### 6. acceptOffer sends one notification INSERT per rejected sibling offer in a sequential loop
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/marketplace/SellerOfferService.ts:459-469`
|
||
|
||
The `for (const rejectedOffer of freshlyRejected)` loop calls `this.notificationService.createNotification(...)` for each rejected sibling sequentially. With many competing sellers this fires N individual INSERT statements. `createNotificationsBulk` already exists (used in `PurchaseRequestService`) and should be used here.
|
||
|
||
**Fix:** Collect all rejected-seller notification payloads into an array and call `this.notificationService.createNotificationsBulk(...)` once after the loop.
|
||
|
||
---
|
||
|
||
### 7. paymentCoordinator: per-seller createNotification loop for rejected sellers
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/payment/paymentCoordinator.ts:511-533`
|
||
|
||
After the PG transaction commits, the code iterates `pgRejectedSellerIds` and issues one `notificationService.createNotification()` INSERT per rejected seller in a `for...of await` loop. For a purchase request with many competing sellers this becomes N sequential DB round-trips.
|
||
|
||
**Fix:** Add or reuse a `createNotificationBatch(notifications[])` method to NotificationService and bulk-insert all rejection notifications in a single query.
|
||
|
||
---
|
||
|
||
### 8. getCategoryPath issues one DB query per ancestor level in a sequential while-loop
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/marketplace/categoryStore.ts:406-418`
|
||
|
||
`getCategoryPathRecords` walks the category hierarchy by calling `findCategoryById` once per level inside a `while` loop. A 3-level hierarchy costs 3 sequential round-trips. The `categories` table already has `parent_id` with a self-referential FK, making a recursive CTE the correct single-query solution.
|
||
|
||
**Fix:** Replace the while-loop with a single `WITH RECURSIVE path AS (SELECT … UNION ALL SELECT c.* FROM categories c JOIN path p ON c.id = p.parent_id) SELECT * FROM path` query.
|
||
|
||
---
|
||
|
||
### 9. getStatistics in DrizzleDisputeRepo fires 4 separate COUNT queries instead of one GROUP BY
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleDisputeRepo.ts:338-379`
|
||
|
||
The method issues 4 `COUNT(*)` calls filtered by individual status values (total, pending, in_progress, resolved) — all 4 scan the same table rows. Additionally, two concurrent reads can see an INSERT between the 4 queries, producing an inconsistent total.
|
||
|
||
**Fix:** Replace the 4 `this.count()` calls with a single `SELECT status, COUNT(*) FROM disputes [WHERE ...] GROUP BY status`, then sum the rows in JS to extract total/pending/in_progress/resolved. Reduces 6 total queries (4 counts + 2 group-bys) to 3.
|
||
|
||
---
|
||
|
||
### 10. sweepDerivedDestinations issues one sequential RPC balance query per destination
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/payment/wallets/sweepService.ts:642-748`
|
||
|
||
The `for (const dest of destinations)` loop awaits `queryTokenBalance` (an external RPC HTTP call) for every destination one at a time. With 50+ derived destinations this is 50+ sequential RPC round-trips. The optional `sweepNative` path adds another RPC call per destination.
|
||
|
||
**Fix:** Fan out the balance queries in parallel using `Promise.all` with a concurrency limit (e.g. `p-limit(10)`) before deciding which addresses need sweeping. Sequentialise only the broadcast step to avoid nonce collisions.
|
||
|
||
---
|
||
|
||
### 11. processReferralReward calls getUserPointsSnapshot twice for the same user in the same request
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/points/PointsService.ts:398-414`
|
||
|
||
When `global.io` is truthy, `getUserPointsSnapshot(referrer.id)` is called at line 398 inside the socket-emit block, and then called again unconditionally at line 413 to build the return value. Two identical reads of the same data in one request.
|
||
|
||
**Fix:** Call `getUserPointsSnapshot` once, store the result, and reuse it for both the socket emit and the return value.
|
||
|
||
---
|
||
|
||
### 12. updateReferralStats makes a separate countReferredUsers query before the UPDATE without a transaction
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzlePointsRepo.ts:607-617`
|
||
|
||
`updateReferralStats` calls `this.countReferredUsers(referrerId)` (a separate SELECT on line 608) then uses the result in an `UPDATE users` (line 609). There is no transaction, so the count is stale if a concurrent referral is added between the two statements.
|
||
|
||
**Fix:** Replace with a single atomic SQL UPDATE: `SET referral_stats_active = (SELECT COUNT(*) FROM users WHERE referred_by_id = $1)` as a correlated subquery, eliminating the preliminary SELECT.
|
||
|
||
---
|
||
|
||
### 13. DrizzleChatRepo.create inserts then immediately updates to add welcome message — two round-trips without a transaction
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleChatRepo.ts:631-701`
|
||
|
||
`create()` does `db.insert(chats)...returning()` (line 667), mutates the document in JS, then calls `db.update(chats).set(...)` (line 691). If the server crashes or the second query fails, a chat row exists without its welcome message. The two operations are not wrapped in a transaction.
|
||
|
||
**Fix:** Build the full initial document (including the welcome message) before inserting, so only a single INSERT is needed. If the two-step approach must be kept, wrap both in `db.transaction()`.
|
||
|
||
---
|
||
|
||
### 14. AML ledger entry written outside the core-money transaction in paymentCoordinator
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/payment/paymentCoordinator.ts:303-308, 441-471`
|
||
|
||
The AML fee ledger entry (lines 441-471) is written after `db.transaction()` commits (lines 272-365). If the process crashes after the payment is marked completed but before the ledger entry is written, the fee is silently skipped. The catch block swallows the error with 'Don't block payment completion for ledger failure', so there is no retry or alert. The TODO at line 303 acknowledges this gap.
|
||
|
||
**Fix:** Once `fundsLedgerService` is on PG, move `appendFundsLedgerEntry` inside the `db.transaction()` block. Until then, add a durable job/retry queue (e.g. a `ledger_retry_queue` table) for the ledger write instead of swallowing the error.
|
||
|
||
---
|
||
|
||
### 15. request-network webhook does two writes (payment update + PR escrow-funded) without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/payment/request-network/requestNetworkWebhook.ts:103-130`
|
||
|
||
`paymentRepo.updateById(payment.id, ...)` (line 103) and `getMarketplaceRepo().markPurchaseRequestEscrowFunded(payment.purchaseRequestId)` (line 127) are two separate writes with no wrapping transaction. A crash between them leaves the payment marked `completed` but the purchase request not advanced, causing the seller's stepper to stall. The catch block on line 128 only logs a warning.
|
||
|
||
**Fix:** Wrap both writes in a single `db.transaction()`. At minimum ensure the PR update is idempotent and add a reconciliation check so stalled PRs are detected and healed.
|
||
|
||
---
|
||
|
||
### 16. batchConvertTemplates creates purchase-requests, offers, and status updates without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/marketplace/RequestTemplateService.ts:521-598`
|
||
|
||
The per-item loop calls `createPurchaseRequest`, `createTemplateOffer`, `updateOfferStatus`, `updatePurchaseRequest`, `incrementUsageCount`, and `findPurchaseRequestById` as separate non-transactional operations. A crash after `createPurchaseRequest` but before `incrementUsageCount` leaves an orphaned request with the usage counter un-bumped; a crash after `createTemplateOffer` but before `updatePurchaseRequest` leaves the offer unlinked from the request.
|
||
|
||
**Fix:** Wrap each item's write sequence in a Drizzle `db.transaction()`. At minimum wrap `(createPurchaseRequest + createTemplateOffer + updatePurchaseRequest.selectedOfferId + incrementUsageCount)` for each item.
|
||
|
||
---
|
||
|
||
### 17. completeTemplateRequestsPayment updates multiple purchase-request rows without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/marketplace/RequestTemplateService.ts:634-641`
|
||
|
||
The method reads all requests with `Promise.all` (line 622) and then updates them one-by-one in a `for` loop (lines 634-639) with no surrounding transaction. A crash mid-loop leaves some requests updated and others not, creating inconsistent payment state across cart items.
|
||
|
||
**Fix:** Wrap the entire update sequence in a single database transaction, or issue a single `UPDATE … WHERE id = ANY($1)` bulk statement.
|
||
|
||
---
|
||
|
||
### 18. verifyEmailWithCode: user save, referrer stat update, and temp-verification delete are not transactional
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/auth/authController.ts:756-868`
|
||
|
||
Registration completes in separate writes: (1) `referrer.save()` increments referralStats, (2) `user.save()` creates the account, (3) `TempVerification.findByIdAndDelete` removes the pending record — all independent. A crash between any two leaves the email address stuck in `temp_verification` (user cannot re-register) or the referrer's count incremented without the user existing.
|
||
|
||
**Fix:** Wrap steps 1-3 in a single Postgres transaction. Perform the token-generation and second `user.save()` inside the same transaction before responding.
|
||
|
||
---
|
||
|
||
### 19. login calls user.save() twice in the same request path without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/auth/authController.ts:405-426`
|
||
|
||
The login handler calls `user.save()` twice: once for `lastLoginAt` (line 407) and again after appending the refresh token (line 425). Each `save()` triggers the full `savePgUser` UPSERT + token-DELETE-INSERT + passkey-DELETE-INSERT sequence. No transaction spans both writes; a crash between them can leave an inconsistent token state.
|
||
|
||
**Fix:** Mutate all fields (`lastLoginAt`, `refreshTokens`) before calling `user.save()` once, wrapped in the transaction described in the savePgUser finding.
|
||
|
||
---
|
||
|
||
### 20. Google sign-in calls user.save() twice in the same request without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/auth/authController.ts:1381-1416`
|
||
|
||
`googleSignIn` calls `existingUser.save()` at line 1398 (profile/lastLoginAt update), then again at line 1416 after pushing the refresh token. Two separate `savePgUser` executions each with their own DELETE+INSERT loops, without a shared transaction.
|
||
|
||
**Fix:** Consolidate all mutations and call `save()` once, wrapped in a transaction.
|
||
|
||
---
|
||
|
||
### 21. Telegram auth performs up to 4 separate user.save() calls without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/auth/authController.ts:459-613`
|
||
|
||
`telegramAuth` can call `user.save()` for a new user (line 533), `user.save()` inside `issueTokensForUser` (line 148), and a try-catch save for mutations (line 558). Each invokes the full `savePgUser` path. With no transaction, a failure between any two leaves the DB inconsistent.
|
||
|
||
**Fix:** Merge all mutations and issue a single `save()` inside a transaction before responding.
|
||
|
||
---
|
||
|
||
### 22. DisputeService.createDispute: chat creation and dispute creation are not atomic | **FIXED** `4766eba` v2.9.32
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/dispute/DisputeService.ts:91-176`
|
||
|
||
The service creates a chat (step 1, line 112) and then creates the dispute (step 2, line 148) as two separate unconnected writes. A failure between the two steps leaves an orphan chat document with no corresponding dispute. The comment at line 91 acknowledges this with a TODO but treats the orphan as acceptable.
|
||
|
||
**Fix:** Once the dispute table is fully on Postgres, wrap both the chat INSERT and the dispute INSERT in a single `db.transaction()`. During the migration period, implement a compensating cleanup job that deletes orphaned dispute-type chats older than a TTL that have no matching dispute.
|
||
|
||
---
|
||
|
||
### 23. DisputeService.resolveDispute: dispute update and releaseHold are not atomic | **FIXED** `8fc2309` v2.9.30
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/dispute/DisputeService.ts:307-341`
|
||
|
||
`resolveDispute()` first writes the resolved status to the dispute (line 312), then calls `releaseHoldResolve()` on the purchase request (line 323) as a separate operation. If the second call fails, the dispute is marked resolved but the escrow hold remains active, permanently blocking fund release.
|
||
|
||
**Fix:** Wrap the dispute update and the purchase request hold release in a single `db.transaction(async (tx) => { ... })` with Drizzle, passing the transaction context into both repo calls.
|
||
|
||
---
|
||
|
||
### 24. verifyAndMarkDeliveryCodeUsed: read-check-then-write is not atomic — TOCTOU race
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:1371-1415`
|
||
|
||
The method reads the delivery code row at line 1377, checks `isUsed`/expired/code equality in application code, then issues an UPDATE at line 1389. Between the read and the update another concurrent request can read the same 'not used' state and both proceed. The WHERE predicate on the UPDATE partially mitigates this, but early returns for 'used'/'expired'/'invalid' happen before any lock is taken, so the application-layer checks can diverge from DB state.
|
||
|
||
**Fix:** Use a single `UPDATE delivery_codes SET is_used = true, ... WHERE id = $1 AND is_used = false AND expires_at > NOW() AND code = $2 RETURNING *` and derive all error branches from whether a row was returned, eliminating the read entirely.
|
||
|
||
---
|
||
|
||
### 25. updatePoints + createPointTransaction not enforced to run in the same transaction — balance can diverge from ledger
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleUserRepo.ts:480-514, 615-675`
|
||
|
||
`updatePoints` and `createPointTransaction` each accept an optional `tx?: DrizzleTx`. The doc-comment warns the caller MUST wrap both in a single SERIALIZABLE transaction but there is no runtime enforcement. If any caller omits the transaction, the points balance can diverge from the `point_transactions` ledger. The `SELECT … FOR UPDATE` inside `updatePoints` acquires and immediately releases the lock under autocommit when `tx` is not provided, providing zero protection.
|
||
|
||
**Fix:** Add a runtime assertion: throw if `tx` is undefined inside both methods. Alternatively refactor into a single atomic `addPoints(userId, input)` method that wraps both writes in one SERIALIZABLE transaction internally.
|
||
|
||
---
|
||
|
||
### 26. processReferralReward: addPoints + updateReferralStats are two independent writes without a transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/points/PointsService.ts:357-425`
|
||
|
||
`addPoints` (line 384) and `updateReferralStats` (line 395) are called sequentially with no shared transaction. A crash between the two leaves the points ledger incremented but the referral stat counter stale, permanently diverging the two denormalised values.
|
||
|
||
**Fix:** Expose a single atomic repo method that writes the point transaction and increments referral stats in one `BEGIN`/`COMMIT` block.
|
||
|
||
---
|
||
|
||
### 27. verifyPayment creates a payment record then propagates completion in separate non-atomic steps
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/payment/paymentController.ts:887-914`
|
||
|
||
`getPaymentRepo().create(...)` (line 887) and `propagatePaymentCompletion(payment, ...)` (line 914) are two separate non-transactional operations. `propagatePaymentCompletion` calls `getMarketplaceRepo().updatePurchaseRequest` and `new SellerOfferService().acceptOffer`. A crash after the payment INSERT but before the marketplace update leaves a `completed` payment with no corresponding PR/offer state change.
|
||
|
||
**Fix:** Wrap the payment creation and propagation in a single `db.transaction()`, or route through the existing `PaymentCoordinator.executePaymentUpdate` path which already has the PG transaction guard.
|
||
|
||
---
|
||
|
||
### 28. DrizzleDisputeRepo read-modify-write on timeline/evidence arrays has no transaction — concurrent writes silently overwrite each other | **FIXED** `8fc2309` v2.9.30
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleDisputeRepo.ts:209-265, 304-336`
|
||
|
||
`assignAdmin()`, `updateStatus()`, and `addEvidence()` all call `loadRow(disputeId)` to read existing `timeline`/`evidence` arrays, append in JS, then write back in a separate UPDATE. There is no wrapping transaction. Two concurrent admin assignments or evidence uploads will each read the same state, and the second UPDATE silently overwrites the first timeline entry.
|
||
|
||
**Fix:** Wrap each read-modify-write pair in `db.transaction()` with `SELECT ... FOR UPDATE` on the dispute row, or use a single atomic SQL append: `timeline = timeline || $newEntry::jsonb` inside the UPDATE statement, eliminating the preliminary SELECT.
|
||
|
||
---
|
||
|
||
### 29. updatePurchaseRequest does not lock the row before updating selectedOfferId — double-accept race
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:1147-1168`
|
||
|
||
`updatePurchaseRequest` updates `selectedOfferId` with a plain UPDATE and no transaction and no SELECT FOR UPDATE. `acceptOffer` correctly locks the `seller_offers` row, but `purchase_requests.selected_offer_id` is updated through this method without a matching lock. A concurrent call with a different offerId can overwrite `selectedOfferId` between the two updates.
|
||
|
||
**Fix:** Move the responsibility of updating `purchase_requests.selected_offer_id` inside the same transaction as the offer lock in `acceptOffer`, or add `SELECT … FOR UPDATE` on the `purchase_requests` row before setting `selectedOfferId`.
|
||
|
||
---
|
||
|
||
### 30. listForExport fetches all matching payments with no LIMIT — can exhaust heap
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:566-578`
|
||
|
||
`listForExport()` executes a SELECT with no LIMIT. All filter parameters (buyerId, sellerId, startDate, endDate) are optional. An admin export with a wide date range or no filter materialises the entire matching set into Node.js heap as an array of full Payment DTOs (including JSONB metadata and blockchain blobs). This is also exposed via `paymentController.ts` lines 282-285 and 320-325.
|
||
|
||
**Fix:** Add a hard cap (e.g. 10,000 rows) with documentation, or use a Node.js streaming query (`node-postgres QueryStream`). At minimum add `.limit(MAX_EXPORT_ROWS)` and return a flag indicating whether the result was truncated.
|
||
|
||
---
|
||
|
||
### 31. findSellers loads all matching sellers into memory when input.limit is undefined
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/db/repositories/drizzle/DrizzleUserRepo.ts:265-309`
|
||
|
||
When `input.limit` is not supplied (the notification fan-out path), `findSellers()` omits LIMIT and returns all matching rows including full JSONB profile and preferences blobs. With thousands of sellers this materialises the entire result set into Node.js heap before the notification loop starts. Callers in `PurchaseRequestService` (lines 293-307 and 705-762) also pass no limit.
|
||
|
||
**Fix:** Return an async generator / cursor for the no-limit path, or paginate the fan-out loop in the notification service. Add a safety cap (e.g. 500) with a logged warning. Select only the columns actually needed (id, email, telegramVerified, preferences) rather than SELECT *.
|
||
|
||
---
|
||
|
||
### 32. findSellersWithActiveRequestTemplates and findSellerWithActiveRequestTemplates fetch entire tables with no LIMIT
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:1848-1964`
|
||
|
||
`findSellersWithActiveRequestTemplates` (line 1849) issues a three-way JOIN with no LIMIT and performs in-memory sort and grouping. `findSellerWithActiveRequestTemplates` (lines 1953-1964) calls it and then does an in-memory `Array.find` to return one seller — O(total sellers × templates) work for one record.
|
||
|
||
**Fix:** Add a dedicated SQL query that accepts a single `sellerId` and filters with `WHERE request_templates.seller_id = $id` to avoid the full table scan. For the list endpoint, add LIMIT/OFFSET and push GROUP BY and ORDER BY into SQL.
|
||
|
||
---
|
||
|
||
### 33. generateShkeeperMigrationReport fetches ALL shkeeper payments without a limit
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/services/payment/migration/reportService.ts:240-241`
|
||
|
||
`Payment.find({ provider: 'shkeeper' })` returns every shkeeper payment document with no limit. On a production database this loads all documents into memory. The subsequent `FundsLedgerEntry.aggregate` (lines 247-258) then passes all `_id` values in a single `$in` clause, potentially exceeding MongoDB's BSON size limit.
|
||
|
||
**Fix:** Add `.limit(options.sampleSize || DEFAULT_SAMPLE_SIZE)` to the initial `Payment.find` call. For the aggregate, use a `$lookup` pipeline or batch the `$in` query.
|
||
|
||
---
|
||
|
||
### 34. Chat messages stored as a JSONB array column — full-document rewrite on every message send, no DB-level pagination
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/chat.ts:100-103`
|
||
|
||
All messages for a chat are in a single `messages jsonb` column. Every `sendMessage()`, `editMessage()`, `deleteMessage()`, or `markMessagesAsRead()` does a full read-modify-write of the entire array. For a chat with 10,000 messages this means writing ~10 MB of JSON on each send. `getChatMessages()` in ChatService.ts (lines 444-451) fetches all messages then sorts and slices in JavaScript — there is no way to paginate at the DB level.
|
||
|
||
**Fix:** Migrate messages to a separate `chat_messages` table (chat_id FK, sender_id, content, message_type, timestamp, is_read, is_edited, reply_to_id). This enables server-side cursor/keyset pagination, targeted UPDATE for mark-as-read, atomic INSERT for new messages, and GIN index on content for search. Move unread_counts to a `chat_unread_counts` table similarly.
|
||
|
||
---
|
||
|
||
### 35. Chat participants and unread counts stored as JSONB arrays instead of relational tables
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/chat.ts:95-114`
|
||
|
||
The `participants` and `unreadCounts` JSONB columns are queried for specific userIds and counts on every user-facing chat request. These are fundamentally relational: each participant belongs to one chat, has a userId FK, role, and isActive flag. Storing them as JSONB means membership lookups are full table scans and the DB cannot enforce FK integrity or use B-tree indexes on userId.
|
||
|
||
**Fix:** Create a `chat_participants` table (chat_id, user_id, role, is_active, joined_at, last_seen) and a `chat_unread_counts` table (chat_id, user_id, count). Index `(user_id, is_active)` on participants and `(user_id, count)` on unread_counts. This converts O(N) JSONB scans to O(1) indexed lookups.
|
||
|
||
---
|
||
|
||
### 36. Dispute messages stored as JSONB array — unbounded growth, no sender/read-status filtering without full scan
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/dispute.ts:99-102`
|
||
|
||
The `messages` column stores all dispute messages in a single JSONB array. Any filter by `senderId`, `isRead`, or timestamp must deserialise the entire array. There is no GIN index. Every message append requires a full read-modify-write of the blob.
|
||
|
||
**Fix:** Create a `dispute_messages` table (dispute_id, sender_id, content, timestamp, is_read, attachments). Index on `(dispute_id, timestamp)`. This enables efficient per-dispute pagination and sender filtering.
|
||
|
||
---
|
||
|
||
### 37. disputes status/priority/category stored as plain text instead of pgEnum | **FIXED** `8fc2309` v2.9.30
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/dispute.ts:90-92`
|
||
|
||
status, priority, and category are `text` with TypeScript-only `$type<>` casts. Postgres accepts any string. Invalid values are silently stored and will pollute the `status_priority` composite index and all single-column status/priority/category indexes, returning wrong results for mistyped values.
|
||
|
||
**Fix:** Create `pgEnum` declarations for `dispute_status`, `dispute_priority`, and `dispute_category` using the values in the TypeScript type aliases at lines 29-43, following the pattern used for `paymentStatus`, `purchaseRequestStatus`, etc.
|
||
|
||
---
|
||
|
||
### 38. blogPosts.status stored as text instead of pgEnum
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/blogPost.ts:50`
|
||
|
||
`status` is declared as `text` with a `$type<BlogPostStatus>` cast only. Postgres accepts any string. The composite indexes `blog_posts_status_published_idx`, `blog_posts_category_status_idx`, and `blog_posts_featured_status_idx` all rely on this column and return incorrect results for mistyped values.
|
||
|
||
**Fix:** Create a `blog_post_status` pgEnum with values `('draft', 'published', 'archived')` and use it as the column type.
|
||
|
||
---
|
||
|
||
### 39. fundsLedgerEntries missing composite indexes on (paymentId/purchaseRequestId, entryType) and standalone index on occurredAt
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/fundsLedgerEntry.ts:77, 104, 127-133`
|
||
|
||
Existing indexes are on `(purchaseRequestId, createdAt)` and `(paymentId, createdAt)` but neither includes `entryType`. Queries filtering by both a parent FK and `entryType` require a seq-scan filter on top of the index result. Additionally, `occurredAt` (the financial-event timestamp, distinct from `createdAt`) has no index, requiring full table scans for date-range reconciliation and daily settlement reports.
|
||
|
||
**Fix:** Add `index('idx_funds_ledger_pr_entry_type').on(t.purchaseRequestId, t.entryType)`, `index('idx_funds_ledger_payment_entry_type').on(t.paymentId, t.entryType)`, and `index('idx_funds_ledger_occurred_at').on(t.occurredAt)`. Consider a composite `(currency, occurredAt)` index for per-currency settlement queries.
|
||
|
||
---
|
||
|
||
### 40. payments table missing standalone indexes on provider, purchaseRequestId, and (provider, status)
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/payment.ts:97, 120, 155-271`
|
||
|
||
Three separate missing-index gaps: (1) `provider` has no standalone or leading index — cleanup queries that MUST scope by provider (per project MEMORY) do full scans. (2) `purchaseRequestId` FK has no standalone index — general-purpose 'all payments for purchase request X regardless of status' lookups scan the full table since the partial-unique indexes require specific provider/status predicates. (3) No composite `(provider, status)` index — reconciliation queries filtering by both columns scan the status index then filter by provider in-memory.
|
||
|
||
**Fix:** Add `index('payments_provider_idx').on(t.provider)`, `index('payments_purchase_request_id_idx').on(t.purchaseRequestId)`, and `index('payments_provider_status_idx').on(t.provider, t.status, t.createdAt)` to the schema index block.
|
||
|
||
---
|
||
|
||
### 41. purchase_requests missing composite index on (buyerId, status) and seller_offers missing (status, createdAt) and (sellerId, status)
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/purchaseRequest.ts:208-213`
|
||
|
||
The most common buyer-facing query is 'my active/pending purchase requests' — individual indexes exist on buyerId and status but no composite `(buyerId, status)`. For seller_offers: separate indexes exist on status and createdAt but no composite `(status, createdAt)` for 'list pending offers ordered by date', and no composite `(sellerId, status)` for the common seller panel query.
|
||
|
||
**Fix:** In `purchaseRequest.ts`: add `index('idx_pr_buyer_status').on(t.buyerId, t.status)`. In `sellerOffer.ts`: add `index('idx_seller_offers_status_created').on(t.status, t.createdAt)` and `index('idx_seller_offers_seller_status').on(t.sellerId, t.status)`.
|
||
|
||
---
|
||
|
||
### 42. seller_offers missing partial index on valid_until for expired-offer sweeper queries
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/sellerOffer.ts:220-253`
|
||
|
||
`findExpiredSellerOffers` and `markExpiredSellerOffersAsWithdrawn` both filter `WHERE valid_until < $asOf AND status = 'pending'`. There is no index on `valid_until`, requiring a full-table scan on every expiry sweep job run.
|
||
|
||
**Fix:** Add a partial index: `index('idx_seller_offers_valid_until_pending').on(t.validUntil).where(sql\`valid_until IS NOT NULL AND status = 'pending'\`)` in `sellerOffer.ts`.
|
||
|
||
---
|
||
|
||
## 🟡 Medium (44)
|
||
|
||
### 1. reconciliationService issues one sequential HTTP request per payment — up to 200 serial HTTPS calls
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/payment/reconciliation/requestNetworkReconciliationService.ts:316-373`
|
||
|
||
The `for (const payment of selectedPayments)` loop awaits `fetchRequestNetworkStatus(reference, context)` for each payment one at a time. With the default limit of 200 payments this is up to 200 sequential HTTPS requests to the Request Network API, taking potentially minutes per reconciliation run.
|
||
|
||
**Fix:** Batch the status lookups using `Promise.all` with a concurrency limit (e.g. `p-limit(10)`). The `coordinator.executePaymentUpdate` step must remain sequential per-payment to avoid race conditions, but the read step can safely be parallelised.
|
||
|
||
---
|
||
|
||
### 2. updatePurchaseRequestStatus does read, atomic UPDATE, then a second read — three round-trips
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/marketplace/PurchaseRequestService.ts:557-594`
|
||
|
||
The method reads the current row (line 558), performs an atomic CAS UPDATE with `.returning()` (lines 567-573), then re-reads the row (line 580) to obtain the 'full row shape'. The `.returning()` clause on the UPDATE already returns the full updated row; the third query is redundant.
|
||
|
||
**Fix:** Remove the second `findPurchaseRequestById` call at line 580. Map the `atomicResult[0]` row directly to the `PurchaseRequestRow` type expected downstream.
|
||
|
||
---
|
||
|
||
### 3. findPaymentById issues two separate SELECT queries for buyer and seller instead of a JOIN
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:490-533` | **FIXED** `61aa42a` v2.9.20
|
||
|
||
`findPaymentById` does SELECT * FROM payments WHERE id = $1, then conditionally fires one SELECT for the buyer and one for the seller. This is 3 sequential queries per call. Since `findAllPayments` calls this once per row, the cost multiplies to 1 + 2N.
|
||
|
||
**Fix:** Replace with a single LEFT JOIN: `SELECT p.*, b.first_name, b.last_name, b.email AS buyer_email, s.first_name, s.last_name, s.email AS seller_email FROM payments p LEFT JOIN users b ON b.id = p.buyer_id LEFT JOIN users s ON s.id = p.seller_id WHERE p.id = $1`.
|
||
|
||
---
|
||
|
||
### 4. quoteRepo.mirrorQuoteToPaymentMetadata does findById then updateById — two queries for one write
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/payment/priceOracle/quoteRepo.ts:53-70` | **FIXED**
|
||
|
||
`mirrorQuoteToPaymentMetadata` calls `paymentRepo.findById(paymentId)` solely to spread `payment.metadata` before calling `paymentRepo.updateById`. This is a read-then-write that adds an extra round-trip every time a quote is persisted. Called from both `persistQuote` and `persistQuoteForMongoPayment`.
|
||
|
||
**Fix:** Use a Postgres JSONB merge expression (`jsonb_set` / `||`) in the UPDATE itself so existing metadata keys are preserved without a prior SELECT. Alternatively expose a `mergeMetadata(id, patch)` repo method that does the merge server-side.
|
||
|
||
---
|
||
|
||
### 5. createOffer fetches purchase request twice in the same call path
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/marketplace/SellerOfferService.ts:152-212` | **FIXED**
|
||
|
||
`createOffer` calls `findPurchaseRequestById(offerData.purchaseRequestId)` at line 152 to validate status, then calls the same method again at line 195 after saving the offer. The request row does not change between the two reads and the first result should be reused.
|
||
|
||
**Fix:** Store the result of the first `findPurchaseRequestById` call in a local variable and reuse it at line 195 instead of issuing a second query.
|
||
|
||
---
|
||
|
||
### 6. getRequestTemplateStats issues four parallel queries that can be collapsed into one
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:2258-2280`
|
||
|
||
`getRequestTemplateStats` fires four separate queries against the same `request_templates` table filtered by the same `seller_id`: total count, active count, total usage sum, and top-5 rows. The first three scan the same rows.
|
||
|
||
**Fix:** Combine the three aggregation queries into one: `SELECT COUNT(*) AS total, COUNT(*) FILTER (WHERE is_active = true) AS active, COALESCE(SUM(usage_count), 0) AS total_usage FROM request_templates WHERE seller_id = $id`. Keep the top-5 query separate.
|
||
|
||
---
|
||
|
||
### 7. createReviewRecord issues two sequential user-UUID lookups instead of one parallel call
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/marketplace/reviewStore.ts:221-222` | **FIXED**
|
||
|
||
`resolveUserUuid(input.sellerId)` and `resolveUserUuid(input.reviewerId)` are called sequentially with `await` on lines 221-222. Each is an independent `SELECT id FROM users` query and both are always needed.
|
||
|
||
**Fix:** Issue both in parallel: `const [sellerUuid, reviewerUuid] = await Promise.all([resolveUserUuid(input.sellerId), resolveUserUuid(input.reviewerId)])`.
|
||
|
||
---
|
||
|
||
### 8. getUserPoints issues two sequential single-row lookups for currentLevel and nextLevel
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/points/PointsService.ts:158-165` | **FIXED** `2abba67` v2.9.19
|
||
|
||
`findActiveLevelConfigByLevel(snapshot.points.level)` and `findActiveLevelConfigByLevel(snapshot.points.level + 1)` are two independent `SELECT … WHERE level = $1 LIMIT 1` queries on every GET /points request.
|
||
|
||
**Fix:** Fetch both in one query: `SELECT * FROM level_configs WHERE level IN ($1, $2) AND is_active = true`, or cache the small level_configs table in memory (it rarely changes).
|
||
|
||
---
|
||
|
||
### 9. getUnreadMessageCountByUser fetches all matching chat rows and iterates unreadCounts array in JS
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzleChatRepo.ts:755-772` | **FIXED** `current` v2.9.26
|
||
|
||
The method calls `this.findRows({...})` (full table scan), deserialises every chat row, filters in JS, then scans the `unreadCounts` JSONB array for each matching row. This is O(N*M) where N = all chats, M = participants per chat. Called on every page load where the unread badge is shown.
|
||
|
||
**Fix:** Replaced with single SQL query using `jsonb_array_elements` and `SUM` aggregation. Filters participants and unreadCounts at the SQL level, eliminating JS iteration.
|
||
|
||
---
|
||
|
||
### 10. ChatService.addParticipant fires N individual userRepo.findById queries for participant validation
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/chat/ChatService.ts:782-788` | **FIXED**
|
||
|
||
`Promise.all(participantIds.map((id) => userRepo.findById(id)))` fires one SELECT per participant concurrently. While Promise.all avoids sequential waits, it still sends N separate queries to the database, opening N statements from the pool.
|
||
|
||
**Fix:** Replace with a single `userRepo.findManyByIds(participantIds)` call (`WHERE id = ANY($1)`), then check results for missing/inactive users.
|
||
|
||
---
|
||
|
||
### 11. resolveUserUuid issues a separate DB round-trip per legacy ObjectId in payment create and filter paths
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:88-100, 116-129, 786-793` | **FIXED**
|
||
|
||
`resolveUserUuid()` executes one SELECT per legacy ObjectId. `create()` calls it twice (buyer + seller). `normalizeUserFilter` calls it sequentially for buyerId then sellerId. No batching or caching: creating multiple payments in a loop issues at least two extra round-trips per iteration.
|
||
|
||
**Fix:** Batch legacy-id resolution: accept a string array, resolve with one `WHERE legacy_object_id = ANY($1)` query, return a `Map<legacyId, uuid>`. Alternatively use a short-lived in-process LRU cache since user ids change extremely rarely.
|
||
|
||
---
|
||
|
||
### 12. PgQuery.exec applies sort/skip/limit in-memory after a full unbounded fetch
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/auth/authStore.ts:168-203` | **FIXED** `current` v2.9.26
|
||
|
||
The PgQuery wrapper's `.sort()`, `.skip()`, `.limit()` methods are applied inside `exec()` in JavaScript after fetching ALL rows from Postgres. For admin user-list queries this means every row is transferred from the DB and sorted in Node.js, with DB indexes ignored.
|
||
|
||
**Fix:** Push sort (ORDER BY), skip (OFFSET), and limit into the SQL queries generated by `findPgUsersByQuery`. Updated all 12 PgQuery producers to accept `_pgQuery` parameter. Removed in-memory sort/slice logic for array results.
|
||
|
||
---
|
||
|
||
### 13. convertTemplateToRequest creates a request then offer with no transaction
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/marketplace/RequestTemplateService.ts:362-395`
|
||
|
||
`convertTemplateToRequest` calls `createPurchaseRequest` (line 362), conditionally `createTemplateOffer` (line 390), `incrementUsageCount` (line 393), and `findPurchaseRequestById` (line 394) as separate operations. A failure between any two steps leaves the database in a partially created state.
|
||
|
||
**Fix:** Wrap all writes for a single template conversion in a database transaction.
|
||
|
||
---
|
||
|
||
### 14. executePaymentUpdate re-runs updatePurchaseRequestStatus after the PG transaction commits — fragile idempotency assumption
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/payment/paymentCoordinator.ts:496-537`
|
||
|
||
After the PG transaction commits (line 365), the code unconditionally calls `updatePurchaseRequestStatus` again at line 500 ('Re-run in notification-only mode'). That method calls `new SellerOfferService().acceptOffer(selectedOfferId)`, which may issue additional DB writes even though the transaction already flipped the offer statuses. Idempotency is assumed but not enforced by any DB constraint.
|
||
|
||
**Fix:** Pass a `notifyOnly: true` flag to `updatePurchaseRequestStatus` that skips all DB writes entirely when `pgTransactionCompleted` is true, and only emits socket events and notifications.
|
||
|
||
---
|
||
|
||
### 15. addPoints reads snapshot before write without a transaction — TOCTOU on level-up check
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/points/PointsService.ts:106-109`
|
||
|
||
`getUserPointsSnapshot(userId)` is called (line 106) to capture the pre-add level, then `pointsRepo.addPoints` is called (line 107). With concurrent requests there is no serialisation: two simultaneous addPoints calls can both read the same pre-level snapshot, both detect a level-up, and emit duplicate `level-up` socket events.
|
||
|
||
**Fix:** Perform the read-and-write atomically in the repo layer with `SELECT … FOR UPDATE` inside a transaction, or return `previousLevel` from `addPoints` itself.
|
||
|
||
---
|
||
|
||
### 16. releaseDeletedUserEmail performs read-then-write without SELECT FOR UPDATE — concurrent release race
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/auth/authStore.ts:717-730`
|
||
|
||
The function reads the deleted user (line 721) then updates it (line 725) in two separate non-transactional statements. A concurrent registration could read the same row, see `status = deleted`, and both proceed to release the same email, causing a duplicate-release race.
|
||
|
||
**Fix:** Use a single `UPDATE users SET email = … WHERE email = $1 AND status = 'deleted' RETURNING id` to make the check-and-update atomic.
|
||
|
||
---
|
||
|
||
### 17. verifyCurrentUserEmail: race between pendingEmail uniqueness check and promotion
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/user/userController.ts:538-557`
|
||
|
||
The controller checks `User.findOne({ email: user.pendingEmail, _id: { $ne: userId } })` (line 539) then promotes `user.email = user.pendingEmail` (line 546) as two independent operations. A concurrent registration could claim the same email address between the check and the save, bypassing the uniqueness guard.
|
||
|
||
**Fix:** Use a single `UPDATE users SET email = pendingEmail WHERE id = $1 AND NOT EXISTS (SELECT 1 FROM users WHERE email = pendingEmail AND id <> $1)` in one statement, or wrap with `SELECT … FOR UPDATE`.
|
||
|
||
---
|
||
|
||
### 18. shopSettingsStore: resolveSellerUuid called outside the transaction client before BEGIN
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/services/marketplace/shopSettingsStore.ts:224`
|
||
|
||
`resolveSellerUuid(sellerId)` runs against the pool (not the transaction `client`) before `BEGIN` is issued. If the resolve fails or returns null due to timing, the INSERT proceeds with `seller_id = null` despite the user existing. There is no retry inside the transaction.
|
||
|
||
**Fix:** Move the `resolveSellerUuid` call inside the transaction, using the transaction `client` instead of the pool, so the lookup and the upsert see the same snapshot.
|
||
|
||
---
|
||
|
||
### 19. DisputeService: assignAdmin, updateStatus, addEvidence read-then-write without optimistic locking
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleDisputeRepo.ts:209-265, 304-336`
|
||
|
||
All three write methods call `loadRow()` to read existing `timeline`/`evidence` arrays, append in JS, then UPDATE. No version column or wrapping transaction. Two concurrent calls read the same state; the second UPDATE silently overwrites the first timeline entry. This is the repo-layer counterpart to the service-level finding on DisputeService.ts.
|
||
|
||
**Fix:** Add an `updatedAt` optimistic lock: reject if `updatedAt` changed between read and write, or use a Postgres JSONB append operator (`timeline = timeline || $newEntry::jsonb`) inside a single UPDATE statement to avoid the read.
|
||
|
||
---
|
||
|
||
### 20. logDeliveryAttempts uses a two-query pattern (fetch delivery_info id, then insert) instead of a single query
|
||
|
||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:1457-1476`
|
||
|
||
`logDeliveryAttempt` and `listDeliveryAttempts` both issue a SELECT to find the `delivery_info` id by `purchase_request_id`, then use the result in a second INSERT or SELECT. This is a two-round-trip pattern that also silently returns without error if no `delivery_info` row exists yet.
|
||
|
||
**Fix:** For the INSERT, use `INSERT INTO delivery_attempts (delivery_info_id, ...) SELECT id, ... FROM purchase_request_delivery_info WHERE purchase_request_id = $pgId`. For the SELECT, JOIN delivery_attempts to purchase_request_delivery_info in one query.
|
||
|
||
---
|
||
|
||
### 21. Template checkout duplicate-cleanup fetches up to 100 payments with in-memory JSONB metadata filtering
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/services/payment/paymentCoordinator.ts:558-590`
|
||
|
||
Lines 558-564 call `paymentRepo.list({ provider: 'shkeeper', direction: 'in', startDate: ..., limit: 100 })` to find duplicates, then lines 565-581 apply a multi-condition JavaScript `.filter()` over the 100-row dataset. The filter checks metadata fields (`meta.templateCheckout`, `meta.isTemplateCheckout`, `meta.templateCheckoutId`) stored inside JSONB and not indexed. The 100-row cap is an arbitrary workaround.
|
||
|
||
**Fix:** Push filter conditions into the repo query using indexed columns (e.g. `purchaseRequestId`, `providerPaymentId` prefix). For metadata-based conditions, add a partial GIN index on `metadata` for template-checkout fields, or add a dedicated boolean `is_template_checkout` column.
|
||
|
||
---
|
||
|
||
### 22. findPgUsersByQuery fetches all matching users with no LIMIT, then pipes through 3N sub-queries via rowToUser
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/services/auth/authStore.ts:644-649`
|
||
|
||
`SELECT * FROM users WHERE …` with no LIMIT. Callers such as `AuthUserModel.find({})` can return every user in the table. The result is piped through `Promise.all(result.rows.map(rowToUser))`, triggering 3 sub-queries per row — a full table scan multiplied by 3N queries.
|
||
|
||
**Fix:** Add a hard cap (LIMIT 1000) at the SQL layer. All callers that need pagination should pass explicit skip/limit parameters down to the SQL query. Fix `rowToUser` to use batch queries (see rowToUser N+1 finding) to avoid the 3N multiplier.
|
||
|
||
---
|
||
|
||
### 23. BlogService.getAllPosts defaults to limit=100 including full content column
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/services/blog/BlogService.ts:38-45`
|
||
|
||
`getAllPosts()` passes `limit = 100` to the repo's `listAll()`. An admin calling without a limit always receives up to 100 full blog post rows including the `content` column (potentially large HTML/markdown). There is no streaming or content truncation.
|
||
|
||
**Fix:** For admin list views, exclude the heavy `content` column (return only metadata: id, title, slug, status, publishedAt, views). Add a separate `getPostById` call when the admin needs full content. Lower the default page size to 20.
|
||
|
||
---
|
||
|
||
### 24. findByPurchaseRequestIdsAndStatuses latestPerRequest deduplication fetches all historical rows then filters in JS | **FIXED** `f5e53cb` v2.9.31
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:444-475`
|
||
|
||
When `options.latestPerRequest` is true, the query fetches every matching payment for all requestIds with no LIMIT, maps to DTOs, then deduplicates in a JS Map keeping only the first (latest) per requestId. For N requests each with M historical rows this fetches N*M rows and discards M-1 per request in JS.
|
||
|
||
**Fix:** Push deduplication to Postgres using `DISTINCT ON (purchase_request_id)`: `SELECT DISTINCT ON (purchase_request_id) * FROM payments WHERE ... ORDER BY purchase_request_id, created_at DESC`. Or use a window function CTE with `ROW_NUMBER() OVER (PARTITION BY purchase_request_id ORDER BY created_at DESC) = 1`.
|
||
|
||
---
|
||
|
||
### 25. findPurchaseRequests totalItems set to filteredRows.length after in-memory filter — breaks pagination contract
|
||
|
||
> **Category:** Bad Pagination | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:979-999`
|
||
|
||
The DB COUNT(*) query (lines 893-905) counts rows before the seller-visibility in-memory filter (lines 979-989). `pagination.totalItems` is then set to `filteredRows.length` (the post-filter count of the current page), not the true total. A client calculating `totalPages` will compute a value smaller than reality, causing pagination to stop early. The code documents this risk: 'RISK: in-memory filter after DB pagination means effective page size shrinks.'
|
||
|
||
**Fix:** Push the seller-visibility filter into SQL using conditional JOINs with `seller_offers` and `purchase_request_preferred_sellers`, conditioned on request status. The in-memory switch block (lines 957-973) maps directly to SQL CASE / conditional JOIN predicates.
|
||
|
||
---
|
||
|
||
### 26. findForUser applies skip/limit in JavaScript after a full DB fetch — true DB pagination not used
|
||
|
||
> **Category:** Bad Pagination | **File:** `src/db/repositories/drizzle/DrizzleChatRepo.ts:596-605`
|
||
|
||
`findForUser` calls `this.findRows(query)` (full table scan), then does `rows.slice(skip, skip + limit)` in JS. For page 10 of 20 results, 200+ rows are fetched and discarded from the DB transfer before slicing.
|
||
|
||
**Fix:** Push pagination to SQL once the JSONB participant filter is moved to SQL: add `.offset(skip).limit(limit)` to the Drizzle query inside `findRows`, or restructure to accept pagination arguments.
|
||
|
||
---
|
||
|
||
### 27. Duplicate-detection in createPurchaseRequest uses in-memory array search over a paginated fetch
|
||
|
||
> **Category:** Bad Pagination | **File:** `src/services/marketplace/PurchaseRequestService.ts:222-232`
|
||
|
||
Lines 222-231 fetch the buyer's 20 most recent requests (`limit: 20`) then call `.find()` in JavaScript to check for a duplicate. If the buyer has submitted more than 20 requests in rapid succession, the 21st duplicate is not detected. The correct approach is a DB-side query.
|
||
|
||
**Fix:** Replace the fetch-and-filter with a targeted query: `SELECT 1 FROM purchase_requests WHERE buyer_id = $1 AND title = $2 AND description = $3 AND created_at > now() - interval '5 minutes' LIMIT 1`.
|
||
|
||
---
|
||
|
||
### 28. count() method re-executes the full findRows() scan instead of using SQL COUNT
|
||
|
||
> **Category:** Bad Pagination | **File:** `src/db/repositories/drizzle/DrizzleChatRepo.ts:607-610`
|
||
|
||
`count(query)` calls `this.findRows(query)` and returns `rows.length`. This fetches every matching chat row (including full JSONB payload) just to count them.
|
||
|
||
**Fix:** Implement `count()` with `db.select({ count: sql<number>\`count(*)::int\` }).from(chats).where(...)` once predicates are pushed to SQL.
|
||
|
||
---
|
||
|
||
### 29. payments.metadata JSONB paths queried without expression or GIN indexes
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:339-346, 363-380, 870-876`
|
||
|
||
`findByLegacyOrderId` (line 339) filters on `metadata->>'legacyOrderId'`, `findRequestNetworkPaymentByReferences` (lines 363-380) filters on five different JSONB paths inside metadata, and `updateByOrderId` (line 870) also filters on `metadata->>'legacyOrderId'`. The schema defines a functional index only on `blockchain->>'transactionHash'`. There is no expression index on `(metadata->>'legacyOrderId')` and no GIN index on metadata.
|
||
|
||
**Fix:** Add a functional B-tree index: `CREATE INDEX payments_metadata_legacy_order_id_idx ON payments((metadata->>'legacyOrderId')) WHERE metadata->>'legacyOrderId' IS NOT NULL`. For the multi-path Request Network lookup, add a GIN index: `CREATE INDEX payments_metadata_gin ON payments USING GIN(metadata)`, or extract `requestNetworkRequestId` as a dedicated column.
|
||
|
||
---
|
||
|
||
### 30. blog_posts.tags GIN index and ILIKE search indexes missing | **FIXED** `f5e53cb` v2.9.31
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/blogPost.ts:49, 61-71`
|
||
|
||
Two gaps: (1) The `tags` array column has no GIN index — `listPublished()` filters with `tag = any(blog_posts.tags)` and `search()` uses `unnest(tags)`, both doing full-table scans. (2) The `search()` method uses `title ILIKE '%query%'` and `description ILIKE '%query%'` — leading-wildcard ILIKE cannot use B-tree indexes, always causing sequential scans.
|
||
|
||
**Fix:** Add `index('blog_posts_tags_gin_idx').using('gin').on(t.tags)`. For ILIKE search, create a `pg_trgm` GIN index: `CREATE INDEX blog_posts_search_trgm_idx ON blog_posts USING gin (title gin_trgm_ops, description gin_trgm_ops)` via raw SQL migration, or use a `tsvector` generated column with GIN index.
|
||
|
||
---
|
||
|
||
### 31. No covering index on payments(purchase_request_id, status, created_at DESC) for hot payment-lookup paths
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/purchaseRequest.ts:206-248`
|
||
|
||
`findFundedPaymentsForRequest`, `findActivePaymentForRequest`, `findPurchaseRequestsByBuyer`, and `findPurchaseRequestById` all filter payments `WHERE purchase_request_id = $id AND status IN (...) ORDER BY created_at DESC LIMIT 1`. No compound index on `(purchase_request_id, status, created_at DESC)` exists, forcing a filter+sort over all payments for a given request.
|
||
|
||
**Fix:** Add `CREATE INDEX idx_payments_pr_status_created ON payments(purchase_request_id, status, created_at DESC)` in the payments schema file.
|
||
|
||
---
|
||
|
||
### 32. No compound index on seller_offers(seller_id, status) for seller offer listing queries
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/sellerOffer.ts:220-253`
|
||
|
||
`findSellerOffersBySeller` (line 1601) filters `WHERE seller_id = $id AND status = $status ORDER BY created_at DESC`. The schema has separate single-column indexes on `seller_id` and `status` but no composite `(seller_id, status, created_at DESC)`, requiring a re-check pass after using either index.
|
||
|
||
**Fix:** Add `index('idx_seller_offers_seller_status').on(t.sellerId, t.status)` in `sellerOffer.ts`.
|
||
|
||
---
|
||
|
||
### 33. categories table lacks an index on parent_id UUID column
|
||
|
||
> **Category:** Missing Index | **File:** `src/services/marketplace/categoryStore.ts:238-241`
|
||
|
||
The DDL in `ensurePostgresCategorySchema` creates indexes on legacy text columns but the `parent_id uuid` FK column used in the recursive CTE path and in the dedup UPDATE has no index. Joins on `child.parent_id = ranked.id` require a sequential scan of the children side.
|
||
|
||
**Fix:** Add `CREATE INDEX IF NOT EXISTS categories_parent_id_idx ON categories (parent_id)` to the schema-init block.
|
||
|
||
---
|
||
|
||
### 34. derived_destinations missing composite index on (buyerId, chainId)
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/derivedDestination.ts:176-183`
|
||
|
||
The service looks up a derived address for a `(buyer, chain)` pair. A standalone `buyerId` index exists but no `(buyerId, chainId)` composite. The partial unique index on `(buyerId, sellerId, chainId)` requires all three columns and is partial (`WHERE seller_id IS NOT NULL`).
|
||
|
||
**Fix:** Add `index('idx_derived_destinations_buyer_chain').on(t.buyerId, t.chainId)`.
|
||
|
||
---
|
||
|
||
### 35. reviews missing index on purchaseRequestId FK
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/review.ts:69`
|
||
|
||
`purchaseRequestId` is a nullable FK column. The query 'does this purchase request already have a review?' has no supporting index and will scan the reviews table.
|
||
|
||
**Fix:** Add a partial index: `index('reviews_purchase_request_id_idx').on(t.purchaseRequestId).where(sql\`purchase_request_id IS NOT NULL\`)`.
|
||
|
||
---
|
||
|
||
### 36. payment_quotes missing index on expiresAt for quote expiry queries
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/paymentQuote.ts:166`
|
||
|
||
`payment_quotes` stores `expiresAt` that must be checked before checkout proceeds. There is no index on `expiresAt`. Periodic cleanup jobs ('find all expired quotes with pending payments') do full table scans.
|
||
|
||
**Fix:** Add `index('payment_quotes_expires_at_idx').on(t.expiresAt)`.
|
||
|
||
---
|
||
|
||
### 37. point_transactions (type, source) index cannot serve source-only queries
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/pointTransaction.ts:130-134`
|
||
|
||
The `(type, source)` composite index has `type` as the leading column. Admin queries filtering by `source='referral'` or `source='admin'` alone cannot efficiently use this index.
|
||
|
||
**Fix:** Either reorder to `(source, type)` or add a separate index on `t.source`.
|
||
|
||
---
|
||
|
||
### 38. reviews table stores seller/reviewer as legacy text IDs rather than proper UUID FKs | **FIXED** `c39b14a` v2.9.32
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/services/marketplace/reviewStore.ts:88-102`
|
||
|
||
The `reviews` table uses `seller_legacy_object_id text` and `reviewer_legacy_object_id text` as the join columns in queries. The `seller_user_id uuid REFERENCES users(id)` and `reviewer_user_id uuid REFERENCES users(id)` FK columns are present but set to `ON DELETE SET NULL` and are not used in any query. The unique index is built on the legacy text columns, not the UUID FKs, so referential integrity is only partially enforced.
|
||
|
||
**Fix:** Migrate queries to use the UUID FK columns (`seller_user_id`, `reviewer_user_id`) which have real FK constraints. Rebuild the unique/covering indexes on those columns. Drop the legacy text columns once the migration is complete.
|
||
|
||
---
|
||
|
||
### 39. notifications.type and notifications.category lack pgEnum — any string accepted by Postgres | **FIXED** `f5e53cb` v2.9.31
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/notification.ts:24-25`
|
||
|
||
Both `type` and `category` are text columns with TypeScript `$type<>` casts only. Invalid type/category values are silently stored. The composite index on `(userId, category)` delivers wrong results for mistyped category values.
|
||
|
||
**Fix:** Create `notification_type` and `notification_category` pgEnums matching the TypeScript union types and use them as column types.
|
||
|
||
---
|
||
|
||
### 40. FundsLedgerBalance derived fields use JS float64 arithmetic on PG numeric(38,18) values — precision loss on crypto amounts | **FIXED** `c39b14a` v2.9.32
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:1440-1470`
|
||
|
||
`pgNumericToNumber()` (line 187-191) converts each PG `numeric(38,18)` aggregate via `parseFloat` to a JS number. `grossBalance`, `heldBalance`, and `availableBalance` are then computed by JS float64 subtraction. For crypto amounts with 10+ significant decimal digits, float64 arithmetic can produce rounding errors. `FundsLedgerBalance.availableBalance` gates escrow operations — a rounding error could cause an incorrect allow/deny decision.
|
||
|
||
**Fix:** Compute all derived balances inside a single SQL query using PG numeric arithmetic, returning them as `text`. Change `FundsLedgerBalance` interface fields to `string` type, or use `decimal.js` for JS-side derivation to match the precision of the double-release guard in `confirmReleaseRefundWithLedger`.
|
||
|
||
---
|
||
|
||
### 41. trezor_accounts.nextAddressIndex has no concurrency safety mechanism — concurrent transactions can derive duplicate addresses | **FIXED** `c39b14a` v2.9.32
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/trezorAccount.ts:78`
|
||
|
||
`nextAddressIndex` tracks the next BIP44 address index to allocate. Unlike `derivedDestinations.derivationIndex` which documents requiring `SELECT...FOR UPDATE` or a PG sequence, `nextAddressIndex` has only a `CHECK (>= 0)` constraint. Concurrent transactions can read the same value and derive duplicate addresses.
|
||
|
||
**Fix:** Add a comment referencing the required `SELECT...FOR UPDATE` locking pattern (as done for `derivedDestinations.derivationIndex`), or replace `nextAddressIndex` with a dedicated PG sequence (`CREATE SEQUENCE trezor_next_address_index_seq`).
|
||
|
||
---
|
||
|
||
### 42. createPointTransaction idempotency check is a non-atomic read-then-insert — concurrent backfills produce duplicate point rows
|
||
|
||
> **Category:** Data Integrity | **File:** `src/db/repositories/drizzle/DrizzleUserRepo.ts:648-668`
|
||
|
||
When `input.legacyObjectId` is set, the code does a SELECT to check existence (lines 654-657) then conditionally INSERTs (line 669). The code comment explicitly warns: 'this is a read-then-insert, not atomic; under concurrent backfill it may still produce duplicates.' Two concurrent calls with the same `legacyObjectId` can both pass the SELECT check and both INSERT, inflating users' points balances.
|
||
|
||
**Fix:** Verify the partial unique index `point_transactions_legacy_object_id_uq` (defined in `pointTransaction.ts` line 150) is applied in the migration. Replace the read-then-insert with `.insert(...).onConflictDoNothing().returning()` matching the pattern used for `fundsLedgerEntries`.
|
||
|
||
---
|
||
|
||
### 43. purchase_requests.categoryId and selectedOfferId have no FK references declared — orphaned rows can be created | **FIXED** `8fc2309` v2.9.30
|
||
|
||
> **Category:** Data Integrity | **File:** `src/db/schema/purchaseRequest.ts:143, 175`
|
||
|
||
`categoryId` is declared as `uuid('category_id').notNull()` and `selectedOfferId` as `uuid('selected_offer_id')` with no `.references()` or `foreignKey()` declarations. Drizzle `relations()` declarations are ORM-only and do not emit DDL FK constraints. Orphaned purchase requests referencing deleted categories or offers can be created without error.
|
||
|
||
**Fix:** Add `.references(() => categories.id)` to `categoryId` and `.references(() => sellerOffers.id, { onDelete: 'set null' })` to `selectedOfferId`, or use the `foreignKey()` block pattern already used in `payment.ts`.
|
||
|
||
---
|
||
|
||
### 44. purchase_request child table FK columns lack .references() declarations — cascade deletes not enforced by DDL | **FIXED** `8fc2309` v2.9.30
|
||
|
||
> **Category:** Data Integrity | **File:** `src/db/schema/purchaseRequest.ts:260, 315, 338, 363`
|
||
|
||
`purchaseRequestDeliveryInfo.purchaseRequestId`, `purchaseRequestDeliveryAddress.deliveryInfoId`, `purchaseRequestSellerDeliveryInfo.deliveryInfoId`, and `deliveryAttempts.deliveryInfoId` are plain uuid columns with FK noted only in comments. If drizzle-kit regenerates DDL from the schema, all FK constraints will be absent. The delivery code flow is security-critical and requires cascade delete integrity.
|
||
|
||
**Fix:** Add `.references(() => purchaseRequests.id, { onDelete: 'cascade' })` to `purchaseRequestDeliveryInfo.purchaseRequestId` and corresponding references to the other child table FK columns.
|
||
|
||
---
|
||
|
||
## 🟢 Low (10)
|
||
|
||
### 1. amnScannerPayInService issues three sequential updateById calls on the same payment row | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** N+1 Query | **File:** `src/services/payment/amnScanner/amnScannerPayInService.ts:140-147, 159-169, 194-203`
|
||
|
||
After finding or creating the payment, the service calls `paymentRepo.updateById` three times in sequence on the same row: to re-point the blockchain rail, to write the derived destination, and to store the provider payment id. Each call is a separate round-trip and a separate UPDATE. `requestNetworkPayInService.ts` has the same pattern at lines 206-214 and 262-279.
|
||
|
||
**Fix:** Accumulate all three field patches in a local object and issue a single `updateById` call after all data is available. The adapter call (which determines `providerPaymentId`) is the only true dependency boundary.
|
||
|
||
---
|
||
|
||
### 2. getStats issues two aggregate queries over the same table that can be collapsed into one | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** N+1 Query | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:661-680`
|
||
|
||
`getStats()` runs two sequential queries with identical WHERE conditions: one GROUP BY status for per-status counts and one ungrouped aggregate for the grand total count and sum. The grand total is derivable from the first query's rows or via `GROUP BY ROLLUP(status)`.
|
||
|
||
**Fix:** Derive the grand total from the first query's rows by summing all `row.count` values. Add `COALESCE(SUM(amount::numeric), 0)` to the per-status query and sum via `decimal.js`. This removes one full table scan per call.
|
||
|
||
---
|
||
|
||
### 3. findOffersByPurchaseRequest and findExpiredSellerOffers use SELECT * with no LIMIT | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Unbounded Fetch | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:1588-1598, 1731-1744`
|
||
|
||
`findOffersByPurchaseRequest` (line 1593) and `findExpiredSellerOffers` (line 1732) each fetch all matching rows with SELECT * and no LIMIT. For high-activity purchase requests or delayed expiry sweeps these can materialise large result sets. `findExpiredSellerOffers` fetches full rows when only the IDs are needed for the sweep.
|
||
|
||
**Fix:** Add a reasonable LIMIT to `findOffersByPurchaseRequest`. For `markExpiredSellerOffersAsWithdrawn`, remove the separate `findExpiredSellerOffers` call since the bulk UPDATE already handles all rows in one query — the caller should not need the full row list unless sending notifications.
|
||
|
||
---
|
||
|
||
### 4. fundsLedgerEntry.paymentId has no covering index for entryType aggregate queries
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/fundsLedgerEntry.ts:69-71, 127-133`
|
||
|
||
The schema defines `idx_funds_ledger_payment_created` on `(paymentId, createdAt)` but `getLedgerBalanceByPaymentId` aggregates entries grouped by `entryType`. Without an index on `(paymentId, entryType)`, the aggregate must scan all entries for a payment then group in memory. (Covered by the composite index finding above; listed separately for implementation clarity.)
|
||
|
||
**Fix:** Add `index('idx_funds_ledger_payment_entry_type').on(t.paymentId, t.entryType)` to the schema.
|
||
|
||
---
|
||
|
||
### 5. id_map missing index on newId for reverse-lookup (UUID to ObjectId) | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/idMap.ts:33`
|
||
|
||
The `id_map` table has a unique index on `(collection, legacyObjectId)` for forward lookup but no index on `newId`. The reverse lookup (UUID to ObjectId) needed for logging and reconciliation will scan the full table.
|
||
|
||
**Fix:** Add `index('id_map_new_id_idx').on(t.newId)` or `uniqueIndex('id_map_new_id_uq').on(t.newId)` if `newId` values are globally unique across all collections.
|
||
|
||
---
|
||
|
||
### 6. trezor_derived_addresses missing index on (trezorAccountId, purpose) | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/schema/trezorAccount.ts:117-161`
|
||
|
||
The PK on `(trezorAccountId, addressIndex)` covers account-scoped lookups but queries for 'the active deposit address for this account' filter by `purpose`. There is no index including `purpose`, requiring a full scan of the account's addresses filtered by purpose.
|
||
|
||
**Fix:** Add `index('idx_trezor_derived_addresses_account_purpose').on(t.trezorAccountId, t.purpose)`.
|
||
|
||
---
|
||
|
||
### 7. sellerOffers.priceAmount uses numeric(18,8) instead of project-wide numeric(38,18) | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/sellerOffer.ts:147`
|
||
|
||
`priceAmount` uses `numeric(18,8)` while all other money columns in the project use `numeric(38,18)` (payments.amount, fundsLedgerEntries.amount, purchaseRequests.budgetMin/Max, etc.). This means `priceAmount` cannot round-trip to full precision when compared against payment amounts.
|
||
|
||
**Fix:** Change `priceAmount` to `numeric('price_amount', { precision: 38, scale: 18 })` to match the project-wide convention.
|
||
|
||
---
|
||
|
||
### 8. users.profile JSONB embeds walletAddress used for payment matching without any index | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Wrong Schema | **File:** `src/db/schema/users.ts:98-109`
|
||
|
||
The `profile` JSONB blob contains `walletAddress`, `walletType`, and `walletProvider`. `walletAddress` is used in blockchain payment matching for EVM providers. Keeping it inside JSONB means any wallet address lookup requires an unindexed JSON path expression.
|
||
|
||
**Fix:** Extract `walletAddress` as a `varchar` column with a sparse index `WHERE walletAddress IS NOT NULL`, and use the already-defined `walletType` pgEnum as a first-class column. This enables indexed payment address matching.
|
||
|
||
---
|
||
|
||
### 9. Dead unused query builder variable in findByUserId wastes an allocation on every call | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Other | **File:** `src/db/repositories/drizzle/DrizzlePaymentRepo.ts:606-615`
|
||
|
||
Line 606 assigns `let q = db.select().from(payments).where(conditions).orderBy(...)` but this variable is never awaited or used. The actual fetch on lines 609-615 is a separate `db.select()` call with identical conditions. Every call to `findByUserId` constructs a dead query object.
|
||
|
||
**Fix:** Delete line 606 (`let q = ...`). The fetch is already correctly performed by the `rows` constant on lines 609-615.
|
||
|
||
---
|
||
|
||
### 10. getLeaderboard passes caller-supplied limit directly to SQL with no upper-bound cap | **FIXED** `5752f13` v2.9.29
|
||
|
||
> **Category:** Missing Index | **File:** `src/db/repositories/drizzle/DrizzlePointsRepo.ts:622-658`
|
||
|
||
`getLeaderboard(limit: number)` passes the caller-supplied `limit` directly to the SQL LIMIT clause with no clamping. A caller passing a very large limit (or 0/negative) could return the entire users table.
|
||
|
||
**Fix:** Add `const safeLimit = Math.min(Math.max(1, Math.floor(Number(limit) || 10)), 100)` and use `safeLimit` in the query.
|
||
|
||
---
|
||
|
||
|
||
## How to Use This Document
|
||
|
||
1. Work top-down by severity — Critical first.
|
||
2. Each fix is self-contained: the description gives the root cause, the fix gives the exact approach.
|
||
3. After each fix is landed, tick it off in the Activity Log with the commit SHA.
|
||
4. Schema changes (wrong-schema, missing-index) require a new Drizzle migration (`npx drizzle-kit generate`).
|
||
|
||
## Related Docs
|
||
|
||
- [[Performance Audit - 2026-05-24]]
|
||
- [[Postgres Runtime Cutover Status]]
|
||
- [[Database Operations]]
|
||
- [[Activity Log]]
|