Skip to content

perf(api): drop the default COUNT from list queries, add hasMore and opt-in includeTotal - #218

Merged
Miracle656 merged 1 commit into
Miracle656:mainfrom
royalTreasure:fix/list-queries-no-count
Sep 30, 2026
Merged

Miracle656 merged 1 commit into
Miracle656:mainfrom
royalTreasure:fix/list-queries-no-count

Conversation

@royalTreasure

Copy link
Copy Markdown
Contributor

closes #206

Summary

Every paged list query ran a full COUNT in a transaction next to its findMany. The queries already fetched take: n+1 to build nextCursor, so the count only fed a total most clients ignore.

Changes

  • src/db.ts: queryTransfers, queryAllTransfers, queryNftTransfers and queryAccountSummaries issue no COUNT by default (no transaction either). They return hasMore from the existing n+1 fetch. New includeTotal param runs the exact count and returns total; otherwise total is absent.
  • REST: ?includeTotal=true on /transfers/incoming|outgoing|address/:address, /accounts/:address/transfers and /nfts/transfers; hasMore in the responses. GraphQL: transfers(includeTotal: Boolean = false), total is now nullable, added hasMore: Boolean!. JSON:API meta carries hasMore.
  • README and OpenAPI (openapi.json regenerated with npm run docs:openapi) say the total costs more.
  • Tests: new src/__tests__/listPagination.test.ts (all four queries: no count by default, page-exactly-full boundary gives hasMore=false, one-past gives hasMore=true plus cursor, includeTotal returns exact total). Route test for forwarding includeTotal. Existing mocks gained hasMore; the network-scoping count test now passes includeTotal: true.

Behaviour change (please note)

total is no longer in list responses unless includeTotal=true. Clients that read total must opt in; hasMore / nextCursor replace it for paging.

Index check and timings

Postgres 17 (embedded), TokenTransfer seeded with 2,000,000 rows, one hot address with 133,333 incoming rows, table on a laptop with load average about 25, so wall-clock numbers are noisy; buffers are the steadier signal. Query shapes copied from Prisma's SQL for toAddress = ? ORDER BY ledger DESC, id DESC LIMIT 51.

query (hot address, 133k rows) before after
COUNT 204-418 ms, 90-248 buffers not issued by default
page (LIMIT 51) 2-14 ms, 45-48 buffers same query, 2-14 ms
request total COUNT + page, about 205-430 ms page only, about 2-14 ms

A rare address (20 rows, all in the oldest ledgers, worst case for a ledger-order scan) pages in 0.26 ms via the existing (network, toAddress) index. I also tried composite (network, toAddress, ledger DESC, id DESC) indexes: on this data the page time did not meaningfully improve (0.6 ms vs 2-14 ms, within load noise) and the planner already picks an index that avoids a big sort, so I did not add them. The existing indexes support the paged query without the count masking a gap. Data is synthetic and evenly distributed; heavily skewed real data may behave differently.

Verification

  • npx jest src/__tests__/listPagination.test.ts src/__tests__/network.test.ts src/__tests__/routes/transfers.test.ts src/__tests__/jsonapi.test.ts: 4 suites, 122 passed.
  • npx jest src/__tests__/listPagination.test.ts src/__tests__/staleReads.test.ts src/__tests__/graphql.test.ts src/__tests__/networkSelector.test.ts: 4 suites, 48 passed.
  • New tests against the original db.ts: 20 failed (fail-before confirmed), pass after.
  • npm run typecheck: clean.
  • Full suite and vitest integration tests not run (no Docker; shared loaded machine). An earlier run of graphql.test.ts hit a 5000 ms timeout on one test while the machine was under heavy load; it passed on rerun.

@drips-wave

drips-wave Bot commented Sep 30, 2026

Copy link
Copy Markdown

@royalTreasure Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally on your branch merged with current main.

  • npx tsc --noEmit -p tsconfig.test.json: clean
  • Full jest suite: 542 passed / 45 suites, 0 failed
  • npm run docs:openapi regenerates openapi.json byte-identical to what you committed, so the spec is genuinely in sync rather than hand-edited

The issue asked for before/after timings on a seeded table and you produced them with buffer counts alongside wall clock, which is the right instinct on a loaded machine — 204-418 ms of COUNT removed from every default list request, against a 2-14 ms page. Reporting that you tried composite (network, toAddress, ledger DESC, id DESC) indexes and then did not add them because the planner already avoids the sort is more useful than adding them speculatively, and it answers the part of the issue about whether the count was masking a missing index.

listPagination.test.ts is the part I'd single out. Asserting on which operations the client issued — countCalls() empty, transactionCalls === 0, take === 6 for a limit of 5 — tests the actual claim rather than the shape of the response, and describe.each over all four query functions means a fifth list query added later without the same treatment is a visible gap. Both boundary cases (page exactly full vs one row past) are the ones that get this wrong in practice. Fixtures are a valid strkey and a StrKey.encodeContract built from raw bytes, with an assertion proving it.

Also checked: queryAccountSummaries has no route callers, so dropping its default total breaks nothing silently, and optionalQueryBool rejects anything that is not true/false/1/0 rather than coercing it, so ?includeTotal=yes is a 400 instead of a quiet false.

The documented behaviour changes — total absent unless opted in, and GraphQL total: Int! becoming Int — are what #206 asked for, so they go in as intended. Worth a line in release notes for any consumer reading total today.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

@Miracle656
Miracle656 merged commit 2180569 into Miracle656:main Sep 30, 2026
3 of 5 checks passed
Miracle656 added a commit to krsnak/wraith that referenced this pull request Sep 30, 2026
Two mechanical integrations with changes that landed on main after this
branch was cut:

- src/api.ts: import-block conflict only. Miracle656#212 added httpRequestsTotal /
  httpRequestDurationSeconds to the ./metrics import; this branch added
  getCachedTokenDecimals and ./amount. Taken as a union — the two touch
  nothing in common.
- src/__tests__/routes/transfers.test.ts: Miracle656#218 made hasMore a required
  field on the queryTransfers return type, so the new 6-decimal $select
  test's mocked result no longer satisfied the cast. Added hasMore: false.

The queryTransfers body auto-merged: Miracle656#218's includeTotal/totalField and
this branch's prismaSelect contractId + derived displayAmount are
disjoint, and both survive.

Verified on the merge result: tsc --noEmit and tsc -p tsconfig.test.json
both clean, jest 47 suites / 562 tests all passing (main baseline 46/557).

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
Miracle656 added a commit that referenced this pull request Sep 30, 2026
* Use token decimals for display amounts

* docs(wave): W094-W098 batch draft (published as #203-207)

Five issues, 800 points. The headline is W094: `src/indexer.ts:491` switches
between pollOnce and pollParallel on INGEST_WORKERS, and `indexer/parallel.ts`
imports only fetchEventsSafe, parseEvents, upsertTransfers, setLastIndexedLedger
and emitTransfer — no NFT parsing, no metadata, no account summaries. Raising
the worker count for throughput silently stops indexing whole categories, with
no error to notice.

Also: /offramp/orders/:orderId serves an order with no authorization, NFT
metadata is fetched one serial round trip at a time, and every paged query pays
for a full COUNT.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

* Docs: Mainnet deployment guide (#166) (#211)

* Docs: Mainnet deployment guide (#166)

* docs(mainnet): correct the native SAC ids, fix the backup link, add DIRECT_DATABASE_URL

Review fixes applied on top of #211:

- The mainnet block used CDLZFC3SY…, which is the *testnet* native XLM SAC
  (Asset.native().contractId(Networks.TESTNET)). Mainnet is CAS3J7GY…
  (Networks.PUBLIC). The testnet block used CDMLFMKMM…, which is not the
  native SAC on either network. Both corrected, with the derivation inlined
  so the next reader can check rather than trust.
- Added a warning that SAC_CONTRACT_IDS must be set explicitly on mainnet,
  because the built-in fallback in src/indexer.ts is the wrong address.
- ../W076 did not resolve to anything; pointed at ./backup-restore.md.
- Added DIRECT_DATABASE_URL to both env blocks — prisma/schema.prisma
  declares directUrl, and boot-time schema sync fails without it.
- Linked ./DUAL_NETWORK.md for the NETWORKS=testnet,mainnet option.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

---------

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

* fix: narrow XDR error classification (#209)

* fix(offramp): require a bearer token to read an order (#216)

* fix(offramp): require a bearer token to read an order

GET /offramp/orders/:orderId served any order to whoever held its id: the
bank payout amount, the deposit address and the rate.

Orders now get a random public id (ofr_ + 128 bits) in place of the
provider's, and the creator is handed a bearer token (oft_ + 256 bits) once,
in the create response. Only its SHA-256 is stored. Lookups need
Authorization: Bearer; a missing header is 401, and an unknown id, a
malformed id and a wrong token are all the same 404.

Failed lookups are rate-limited separately from the app-wide limiter (10 per
15 minutes per IP by default); successful polling does not count.

Replaying a creation with the same idempotencyKey now also needs the same
walletAddress (409 otherwise) and re-issues the token, since only its hash is
kept. The lookup response no longer names the provider (source: live) or
returns its id, and a database failure returns a generic 500 instead of
reaching the global handler.

* test(offramp): use a valid strkey for the second wallet fixture

GBBB...SAM was 56 characters but failed StrKey.isValidEd25519PublicKey.
The test passes either way today, because the route does not validate the
address, but a fixture that is not a real strkey breaks the moment it is
handed to anything that decodes one.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

---------

Co-authored-by: blockchain-maxis <267648998+blockchain-maxis@users.noreply.github.com>
Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

* fix: skip malformed events in parseEvents instead of wedging the indexer (#197)

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

* fix(indexer): share one per-batch pipeline between the single and parallel ingest paths (#219)

Co-authored-by: DevTobis <232918735+DevTobis@users.noreply.github.com>

* Bring docker-compose.yml in line with env contract (#194) (#220)

* Bring docker-compose.yml in line with env contract (#194)

- Remove obsolete version: "3.9" key
- Add all missing env keys from .env.example:
  - DIRECT_DATABASE_URL
  - STELLAR_NETWORK
  - SOROBAN_RPC_URL (with STELLAR_RPC_URL as backward-compat alias)
  - NETWORKS
  - SAC_CONTRACT_IDS (and per-network variants)
  - NFT_CONTRACT_IDS (and per-network variants)
  - RETENTION_DAYS
  - CACHE_ENABLED and all Redis cache config
  - TOMBSTONE_CHECK_EVERY_CYCLES
  - LP_POOL_CONTRACT_IDS variants
  - SKIP_INDEXER
- Keep CONTRACT_IDS as documented backward-compat alias
- Add optional redis service with cache profile for CACHE_ENABLED support
- Update README to use SAC_CONTRACT_IDS and add cache profile instructions
- Add test to validate compose file meets requirements

* test(compose): derive the env-contract check from .env.example

The drift test built envKeys from .env.example and then never used it,
asserting against a hand-maintained list instead — so a key added to
.env.example and forgotten in docker-compose.yml still passed, which is the
one failure #194 is about. The assertions were also substring matches on the
whole file: toContain("SAC_CONTRACT_IDS") is satisfied by
SAC_CONTRACT_IDS_TESTNET, and toContain("CONTRACT_IDS") by either, so they
held on a file declaring none of them.

Now it parses the wraith service's own environment block (anchored on the
service, since db has an environment block too) and compares declared keys
against every uncommented key in .env.example, with an explicit, empty
exclusion list. Verified it fails on an added key and passes without one.

Also: the docker compose config case ran unconditionally and called fail(),
which is not defined under jest-circus, so on a machine or CI runner without
Docker it failed with a ReferenceError. Unit tests here do not require
Docker (the integration suite is vitest + Docker), so it now skips instead.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

---------

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

* Fix/cache network key (#202)

* fix(cache): include resolved network in Redis cache key (#182)

defaultKeyFn now prepends req.network (set by networkMiddleware) to the cache key so mainnet and testnet requests never collide, even when the network is supplied via the X-Network header rather than ?network=.

The ?network= query param is filtered from the query segment since it is already captured by req.network, ensuring ?network=mainnet and X-Network: mainnet produce identical keys.

Existing keys change shape and will expire naturally over their TTL.

* fix(cache): include resolved network in Redis cache key (#182)

* chore: drop the unrelated package-lock.json change

The branch re-resolved fsevents and dropped its "dev": true marker. Nothing
in this PR touches dependencies, so restore the lockfile to main's.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

---------

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

* Add npm run db:seed with deterministic fixtures (#221)

- Create shared fixture module in src/fixtures.ts with deterministic test data
  across two addresses (ALICE, BOB, CAROL) and two contracts (CONTRACT_A, CONTRACT_B)
- Add seed script at scripts/seed.ts with --network flag support (testnet/mainnet)
- Add db:seed script to package.json
- Update integration tests to use shared fixture module instead of local copy
- Update README Quick Start with seed step and real sample output
- Add fixture validation test to verify data structure and coverage

The seed script uses skipDuplicates on unique constraints, making it safe to
run multiple times without duplicating rows. Fixtures include TokenTransfer,
NftTransfer, and AccountSummary rows with deterministic eventId values.

Resolves #193

* fix(seed): accept --network mainnet as well as --network=mainnet

Follow-up to #221: this fix was pushed to the PR branch but did not make it
into the squash. The docstring advertised the space-separated form while
parseArgs matched only --network=, so 'npm run db:seed -- --network mainnet'
silently seeded testnet. Both spellings now parse; an unknown value still
exits 1.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

* Fix token decimals review issues

* perf(api): drop the default COUNT from list queries, add hasMore and opt-in includeTotal (#218)

Co-authored-by: royalTreasure <295874283+royalTreasure@users.noreply.github.com>

* feat: instrument HTTP surface in Prometheus (#189) (#212)

* feat: instrument HTTP surface in Prometheus

* fix(metrics): instrument before networkMiddleware and the rate limiter

Mounted after them, the HTTP middleware never saw the requests those two
reject, so 429s and invalid-?network= 400s were absent from
http_requests_total. Moved to the top of the chain, right after cors().

Also documents the two new metrics in the README table and records why the
route label must stay req.route.path: req.baseUrl is the matched mount path,
and src/api/accounts.ts mounts a router at "/:address/transfers", so baseUrl
carries the real address and would make the label unbounded.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

---------

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

* fix(indexer): bound NFT metadata lookups with a worker pool and per-cycle budget (#217)

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>

---------

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>
Co-authored-by: Gloria <glorious217@gmail.com>
Co-authored-by: Jemimah <ekongjemimah@gmail.com>
Co-authored-by: blockchain-maxis <blockchainmaxis@gmail.com>
Co-authored-by: blockchain-maxis <267648998+blockchain-maxis@users.noreply.github.com>
Co-authored-by: Apulupie <167634780+Frun1na@users.noreply.github.com>
Co-authored-by: Tobiz <deborahayoola2000@gmail.com>
Co-authored-by: DevTobis <232918735+DevTobis@users.noreply.github.com>
Co-authored-by: BOA <97275013+boalambo@users.noreply.github.com>
Co-authored-by: Bathoul Mohammed <funds0033@gmail.com>
Co-authored-by: royaldev <chiditreasure15@gmail.com>
Co-authored-by: royalTreasure <295874283+royalTreasure@users.noreply.github.com>
Co-authored-by: Collins Ezedike-egwom <62267326+collinsezedike@users.noreply.github.com>
Co-authored-by: That guy <120946193+ezedike-evan@users.noreply.github.com>
@Miracle656 Miracle656 mentioned this pull request Oct 1, 2026
5 of 11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Every paged query pays for a full COUNT

2 participants