Skip to content

Use token decimals for display amounts - #196

Merged
Miracle656 merged 16 commits into
Miracle656:mainfrom
krsnak:bounty/181
Sep 30, 2026
Merged

Miracle656 merged 16 commits into
Miracle656:mainfrom
krsnak:bounty/181

Conversation

@krsnak

@krsnak krsnak commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Closes #181

Summary

  • format token amounts using each token's cached decimals
  • preserve the existing 7-decimal output when metadata is unavailable
  • use a synchronous cache-only decimals lookup so read paths never trigger DB/RPC work
  • cover REST, GraphQL, WebSocket, account summaries/balances, popular assets, CSV and Parquet exports
  • add regression tests for 6-decimal tokens, default 7-decimal formatting, and cache-miss behavior

Validation

  • npm run typecheck
  • npm run build
  • full Jest suite: 447/447 tests passing
  • git diff --check

Note: the repository's issue description states the existing integration/load CI failures predate this issue.

@drips-wave

drips-wave Bot commented Sep 26, 2026

Copy link
Copy Markdown

@krsnak 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

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
@krsnak

krsnak commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi, just a quick note as Wave 9 is close to ending: this PR is ready for review. Local validation is green (typecheck, build, 447/447 Jest tests, diff check), and GitHub reports the PR as mergeable. The CI/Chaos workflows are currently in action_required, which looks like they need maintainer approval to run for this fork PR. Thanks!

Glorious21 and others added 5 commits September 30, 2026 15:58
* Docs: Mainnet deployment guide (Miracle656#166)

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

Review fixes applied on top of Miracle656#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(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>
…xer (Miracle656#197)

Co-authored-by: Miracle656 <iupacnumen2020@gmail.com>
…allel ingest paths (Miracle656#219)

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

@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.

Good shape overall, and the most important thing is right: nothing rounds before storage. TokenTransfer.amount stays the raw i128 decimal string (prisma/schema.prisma:43), src/amount.ts is pure BigInt string arithmetic with no float anywhere, and displayAmount is a derived field computed at the response edge. Extracting the four copy-pasted STROOPS helpers (api.ts, db.ts, routes/accounts/transfers.ts) into one src/amount.ts is the right call, and adding contractId to prismaSelect when displayAmount is requested was a sharp catch.

Verified locally: npx tsc --noEmit and tsc -p tsconfig.test.json clean, full jest suite 43 suites / 508 tests, all passing on the merge of this branch with main.

Two things to fix before I merge, both about where the decimals come from.

1. In API-only mode there are no decimals at all — the feature is a silent no-op

getCachedTokenDecimals reads the in-memory cache in src/tokenCache.ts. That cache is populated by initTokenCache, and initTokenCache has exactly one non-test caller: src/indexer.ts:438, inside startIndexer. src/index.ts:69 skips startAllIndexers() entirely when SKIP_INDEXER=true.

So on an API-only deployment the cache is empty for every contract, getCachedTokenDecimals returns undefined for every row, and every single displayAmount falls back to DEFAULT_TOKEN_DECIMALS = 7 — i.e. exactly main's behaviour, with none of the new machinery doing anything. A 6-decimal token renders 10× off and nothing in the response says so. That is the failure mode the schema comment on TokenMetadata warns about in so many words: "a wrong decimals silently rescales every amount rendered from it by orders of magnitude."

Either seed the cache in API-only mode (call initTokenCache for each enabled network from src/index.ts regardless of SKIP_INDEXER, it is a single findMany), or have the API read decimals from TokenMetadata rather than from a cache only the indexer fills. The first is much cheaper. Either way, please add a test that exercises the path with the cache populated and asserts a 6-decimal token renders with 6 decimals through an actual route, not just through toDisplayAmount directly — src/__tests__/amount.test.ts only tests the formatter in isolation, so it would still pass with the whole wiring disconnected.

2. src/api.ts withDisplay clobbers the value db.ts just computed correctly

queryTransfers/queryAllTransfers now compute displayAmount with the right decimals via the tokenDecimals callback. But every route then re-maps the result:

transfers: result.transfers.map((transfer) => {
  if (transfer && typeof (transfer as { amount?: unknown }).amount === "string") {
    return withDisplay(transfer as { amount: string; contractId?: string }, network);
  }
  return transfer;
}),

and withDisplay overwrites displayAmount unconditionally, reading transfer.contractId. With ?$select=amount,displayAmount, projectRecord (src/lib/odata.ts:216) only copies the selected fields, so contractId — which you correctly made Prisma fetch — is projected out before this map runs. withDisplay therefore sees no contractId, falls back to 7, and replaces the correct value. Not a regression against main, but it silently defeats this PR for $select callers. Easiest fix is to not overwrite a displayAmount the query layer already produced, or to carry the resolved decimals alongside the row instead of re-deriving from contractId.

Minor, no action needed unless you want to: the fallback to 7 is completely silent. It may be worth a debug log or a counter the first time a contract's decimals are missed, so "my amounts look 10× wrong" is diagnosable from the outside.

Nice refactor — it just needs the data to actually be there.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB

boalambo and others added 8 commits September 30, 2026 19:35
…iracle656#220)

* Bring docker-compose.yml in line with env contract (Miracle656#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 Miracle656#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): include resolved network in Redis cache key (Miracle656#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 (Miracle656#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>
- 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 Miracle656#193
Follow-up to Miracle656#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
…opt-in includeTotal (Miracle656#218)

Co-authored-by: royalTreasure <295874283+royalTreasure@users.noreply.github.com>
…656#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>
…ycle budget (Miracle656#217)

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

krsnak commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both requested changes in commit 2bf3046.
API-only mode now seeds token decimals correctly, and displayAmount is no longer overwritten after $select projection. Added route-level regression coverage as well. Full Jest suite passes: 448/448 tests.

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 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.

Both requested changes are done. Checking each against the new commit (2bf3046):

1. API-only mode had no decimals — fixed, and fixed the cheap way I hoped for.

src/index.ts now calls initTokenCache for every enabledNetworks() before the indexer dispatch, so the cache is seeded whether or not SKIP_INDEXER is set. getCachedTokenDecimals therefore has real data on an API-only deployment, and the feature is no longer a silent no-op.

One thing I want to flag because it looks alarming in the diff and isn't: this commit deletes the SKIP_INDEXER early return. That is correct. main carried two guards — an early return and, further down, an if (SKIP_INDEXER) … else startAllIndexers(). The early return made that else unreachable dead code. Removing it is what lets the seed run in API-only mode, and the surviving guard still skips startAllIndexers(), so API-only mode is intact. I checked the merged file to be sure.

Also good: initTokenCache swallows its own errors, so awaiting it at the top of main() can't turn a database hiccup into an unhandled rejection and kill boot.

On the test: the new case populates the cache through initTokenCache and asserts params.tokenDecimals?.(CONTRACT_A, "testnet") === 6 through a live route. That's exactly the link that was severed, so it would now fail if the wiring came apart — which is what I was asking for. It's fair to note db is mocked in this suite, so the "1.000000" string is handed in rather than computed; the formatter itself is covered separately in amount.test.ts. Between the two, the path is pinned. Good enough.

2. withDisplay clobbering the query layer's value — fixed. transfer.displayAmount ?? toDisplayAmount(…) preserves what db.ts computed with the right decimals, and the ?$select=amount,displayAmount case the regression test covers now round-trips correctly instead of being overwritten with a 7-decimal fallback.

Rounding, re-confirmed on the merge: still nothing before storage. src/amount.ts is pure BigInt (10n ** BigInt(d), integer division, remainder padded) — no float, no toFixed, no truncation — and it runs only at the response edge. TokenTransfer.amount stays the raw i128 string.

What I changed while merging

Your branch was cut before several things landed on main today, so I resolved this myself rather than send it back for a rebase:

  • src/api.ts — import-block conflict only. #212 added httpRequestsTotal/httpRequestDurationSeconds to the ./metrics import while you added getCachedTokenDecimals and ./amount. Taken as a union; nothing overlapped.
  • src/__tests__/routes/transfers.test.ts — #218 made hasMore required on the queryTransfers return type, so your new test's mocked result stopped satisfying the cast. Added hasMore: false.

queryTransfers itself auto-merged cleanly — #218's includeTotal/totalField and your prismaSelect contractId + derived displayAmount are disjoint, and I verified both survive in the merged body.

Verified on the merge result: npx tsc --noEmit and tsc -p tsconfig.test.json both clean, and jest at 47 suites / 562 tests, all passing (main's baseline is 46/557 — this adds amount.test.ts and five tests).

One note for later, no action needed: the seed runs just after server.listen, so there's a brief window at boot where a request can still get the 7-decimal fallback. Given initTokenCache degrades gracefully and blocking listen on the database would be the worse trade, I'd leave it as is — just worth knowing it exists. The silent-fallback log I mentioned last time is still worth doing someday, but not here.

Nice work, and quick. Merging.

@Miracle656
Miracle656 merged commit 00ca078 into Miracle656:main Sep 30, 2026
1 check passed
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.

Use each token's real decimals for displayAmount