fix(market): use decimal-string bigints for all price fields - #483
Merged
Conversation
MarketModule's PostIntentRequest.price, SearchIntentResult.price, and
SearchFilters.{min,max}Price were typed as `number` — inconsistent with
MarketIntent.price (already `string`) and inconsistent with the SDK's
established bigint-serialization convention (TXF amount fields, transfer
payloads, token amounts everywhere else: bigint internally, decimal-string
on the wire).
This bites real callers. The trader-service intent engine
(trader-service/src/trader/intent-engine.ts:908) takes its bigint
midpoint rate (rate_min + rate_max) / 2n in 18-decimal smallest units and
casts to `Number(...)` before passing to MarketModule.postIntent. For
the trader-roundtrip soak's default 0.08-0.12 ETH/UCT band the midpoint
is 1e17 = 100_000_000_000_000_000, well past Number.MAX_SAFE_INTEGER
(2^53 ≈ 9.007e15). JavaScript Number stores 1e17 as a close-but-not-exact
double, and the market-api server responds with HTTP 500 — non-JSON, so
the trader logs only the status code with no diagnostic.
Fix:
- PostIntentRequest.price number → string
- SearchIntentResult.price number → string
- SearchFilters.{minPrice,maxPrice} number → string
The wire serialization in toSnakeCaseIntent / toSnakeCaseFilters is a
direct pass-through, so changing the input type changes the wire shape
from JSON number to JSON string without any new conversion logic.
MarketIntent.price was already `string`, confirming the server's read
shape uses strings — the server should also accept strings on the write
side without server changes (this is the existing convention for
TIP-0 / MarketModule round-tripping).
Tests:
- Existing test inputs `price: 100`, `price: 99.99`, `minPrice: 10`,
`maxPrice: 200` migrated to string form (`'100'`, `'99990000000000000000'`
i.e. 99.99 in 18-decimal smallest units, `'10'`, `'200'`).
- New precision-preservation test demonstrates the failure mode:
'100000000000000000' (10^17) round-trips through JSON without loss,
proving the string convention is what trader-service should use.
Surfaced by: #475 (trader-roundtrip soak)
end-to-end run §6 — market-api HTTP 500 on every attempt to post the
trader's intent.
Coordinating: trader-service/src/trader/intent-engine.ts will need a
follow-up to drop the `Number(...)` cast and pass `.toString()` instead.
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`MarketModule`'s `PostIntentRequest.price`, `SearchIntentResult.price`, and `SearchFilters.{min,max}Price` were typed as `number` — inconsistent with `MarketIntent.price` (already `string`) and inconsistent with the SDK's established bigint-serialization convention everywhere else (TXF amount fields, transfer payloads, every token amount in the wire layer: bigint internally, decimal-string on the wire).
Why this matters
The trader-service intent engine (`trader-service/src/trader/intent-engine.ts:908`) takes a bigint midpoint rate `(rate_min + rate_max) / 2n` in 18-decimal smallest units and casts to `Number(...)` before passing to `MarketModule.postIntent`. For the trader-roundtrip soak's default 0.08–0.12 ETH/UCT band:
The end-to-end soak in sphere-sdk#475 hits this consistently in §6 (intent post). Two consecutive runs both failed at the same line, with the same error, on the same payload.
Fix
The wire serialization in `toSnakeCaseIntent` / `toSnakeCaseFilters` is a direct pass-through, so changing the input type changes the wire shape from JSON number to JSON string without any new conversion logic.
`MarketIntent.price` was already `string`, confirming the server's read shape uses strings — the server should also accept strings on the write side without server changes (this is the existing convention for TIP-0 / MarketModule round-tripping). If the server unexpectedly rejects, a follow-up issue would track the server side.
Test plan
Coordinating change
`trader-service/src/trader/intent-engine.ts:908` will need a follow-up to drop the `Number(...)` cast and pass `.toString()` instead. That's a one-line change. Tracked as the next step in the conversation that surfaced this.
Related