fix(market): pass intent price as decimal-string bigint, not Number - #25
Merged
Conversation
intent-engine.ts:908 computed the bigint midpoint of the trader's rate
band and cast to Number before calling MarketModule.postIntent. For a
typical 18-decimal quote asset, the midpoint of a realistic rate band
(e.g. 0.08–0.12 ETH/UCT) is 1e17 = 100_000_000_000_000_000 — past
Number.MAX_SAFE_INTEGER (2^53 ≈ 9e15). The IEEE 754 double stored a
close-but-not-exact value, JSON serialised it as a number, and the
market-api server rejected with HTTP 500 (non-JSON body, so the trader
only logged the opaque status code with no actionable diagnostic).
Fix:
- intent-engine.ts: drop `Number(...)` cast; pass `midpointRate.toString()`
- types.ts MarketPostRequest.price: number → string
- types.ts MarketSearchResult.price: number → string
- types.ts MarketSearchFilters.{min,max}Price: number → string
Aligns with sphere-sdk PR #483 which makes the same change on the SDK
side (PostIntentRequest.price, SearchIntentResult.price,
SearchFilters.{min,max}Price all become `string`). This is the
established bigint serialisation convention everywhere else in the SDK
(TXF amount fields, transfer payloads, all token amounts) — bigint
internally, decimal-string on the wire.
Surfaced by: unicity-sphere/sphere-sdk#475 (trader-roundtrip soak) §6
which failed with `HTTP 500 — unexpected response (not JSON)` on two
consecutive runs.
Tests: e2e/trader-intent-lifecycle.e2e.test.ts T1.1 expectation updated
from `postCall.price === 475` to `postCall.price === '475'`. 698/698
test suite passes.
The market-api server SHOULD accept string prices (MarketIntent.price
in the SDK was ALREADY `string`, so the read shape uses strings — the
server clearly round-trips strings on reads). If a server-side change
is also needed, that's a follow-up; this is the client-side correctness
fix.
Related: sphere-sdk PR #483 (SDK type alignment).
The price-as-string types from sphere-sdk PR #483 land at b2fd028; this PR's intent-engine + types.ts changes need that SDK build to typecheck. Both CI and docker-publish workflow pins moved in lockstep so the next v0.3 release tag builds against the same SHA.
4 tasks
vrogojin
added a commit
that referenced
this pull request
Jun 11, 2026
) The market-api at market-api.unicity.network is a semantic-search database — counterparties discover us by description matching. The trader was sending a redundant structured `price` field at the top of the request body, and the deployed server hit an unhandled exception on it (HTTP 500 with HTML body, swallowed by MarketModule's parseResponse to an opaque "unexpected response (not JSON)"). The trader's own counterparty-discovery already parses the description (parseDescription extracts rate_min/rate_max/volume_min/volume_max fields per TIP-0), so emitting a separate single-point price was useless even when the server accepted it. Successful posts in the live feed (@alphasentinel0X, @gulungtikar990, @w025ixd) all omit the field. Changes: - intent-engine.ts:904: drop the `price: midpointRate.toString()` argument; the description carries the full rate band. - intent-engine.ts:896: drop the now-unused `midpointRate` calc. - types.ts MarketPostRequest.price: required → optional, with a doc block explaining when (not) to use it. Tests: - e2e/trader-intent-lifecycle.e2e.test.ts T1.1: assertion changed from `postCall.price === '475'` to `postCall.price === undefined`. Description assertions unchanged — the rate band is still asserted to round-trip through encodeDescription. - 698/698 test suite passes. Surfaced by: unicity-sphere/sphere-sdk#475 (trader-roundtrip soak) §6. Final root cause from a chain of three: (1) Number() precision loss fixed in PR #25, (2) types out of step with SDK's bigint-string convention fixed via sphere-sdk PR #483 + this PR's earlier commits, and (3) the actual server-side schema mismatch — the field shouldn't have been there at all. This commit ships the (3) fix. Related: sphere-sdk PR #483 (SDK side type changes — already merged).
4 tasks
vrogojin
added a commit
that referenced
this pull request
Jun 12, 2026
…hout) The trader was serialising rate_min/rate_max/volume_min/volume_max as bigint smallest-units in the description and search query. For 18-decimal quote assets like ETH the rate 0.08-0.12 became the string "80000000000000000-120000000000000000", which the semantic-search engine on the deployed market-api server choked on (large-number-dash-large-number patterns return HTTP 500 with HTML body). It was never tested live — the trader's e2e suite mocks MarketAdapter so the bigint values never round-tripped through the real server. Existing test fixtures (e.g. rate_min: '450', volume_max: '1000') and internal math (rate × volume = 475000) reveal the original design: rates are dimensionless ratios, volumes are in BASE whole units. The bigint typing came from a later misinterpretation that conflated them with token amounts. This change makes that explicit: * TradingIntent.rate_min/rate_max/volume_min/volume_max/volume_filled bigint → string (decimal strings, e.g. "0.08", "50") * DealTerms.rate, DealTerms.volume bigint → string * parseDescription regex: \d+ → \d+(?:\.\d+)? to accept decimals * validateIntentParams / validateDealTerms: accept decimal strings, compare via Number (2^53 ceiling is plenty for trading ratios + volumes) * MAX_RATE / MAX_VOLUME: 2^128 → Number.MAX_SAFE_INTEGER * intent-engine matching, fan-out volume allocation, midpoint calc: switched to Number arithmetic * PaymentsAdapter gets `getDecimals(coinId)` backed by sphere-sdk's TokenRegistry.getTokenDecimals (the one place we DO need decimals — at the smallest-unit boundary) * trader-main.ts onDealAccepted: converts whole-unit volume × rate to smallest-units bigint via toSmallestUnitsBigInt() at the wallet reservation boundary * swap-executor.ts buildSwapDealInput: optional getDecimals lookup converts whole-unit terms to smallest-units integer strings before calling sphere.swap.proposeSwap (which requires positive-integer-string per SwapModule.ts:1186) * Tests sweep: 56 sites of `123n` → `'123'` across intent/deal/match assertions; ledger amount assertions (getAvailable) left as bigint since that side IS smallest-units The four-stop fix chain that landed today on the soak's §6 was: 1. trader-service #25 — drop Number() cast (precision) 2. sphere-sdk #483 — align SDK types to bigint-string 3. trader-service #26 — drop structured price field (server rejected it) 4. this PR — rate/volume in human-units throughout (server rejected the bigint-in-text-query too) Verified: - npx tsc --noEmit: clean - npm test: 698/698 pass (added Number arithmetic tests on top of existing string-fixture coverage) - npm run build: tsup clean Surfaced by: unicity-sphere/sphere-sdk#475 §6/§8 (trader-roundtrip soak) which produced HTTP 500 against /api/intents and /api/search with the bigint-in-text formats.
vrogojin
added a commit
that referenced
this pull request
Jun 12, 2026
…hout) (#27) The trader was serialising rate_min/rate_max/volume_min/volume_max as bigint smallest-units in the description and search query. For 18-decimal quote assets like ETH the rate 0.08-0.12 became the string "80000000000000000-120000000000000000", which the semantic-search engine on the deployed market-api server choked on (large-number-dash-large-number patterns return HTTP 500 with HTML body). It was never tested live — the trader's e2e suite mocks MarketAdapter so the bigint values never round-tripped through the real server. Existing test fixtures (e.g. rate_min: '450', volume_max: '1000') and internal math (rate × volume = 475000) reveal the original design: rates are dimensionless ratios, volumes are in BASE whole units. The bigint typing came from a later misinterpretation that conflated them with token amounts. This change makes that explicit: * TradingIntent.rate_min/rate_max/volume_min/volume_max/volume_filled bigint → string (decimal strings, e.g. "0.08", "50") * DealTerms.rate, DealTerms.volume bigint → string * parseDescription regex: \d+ → \d+(?:\.\d+)? to accept decimals * validateIntentParams / validateDealTerms: accept decimal strings, compare via Number (2^53 ceiling is plenty for trading ratios + volumes) * MAX_RATE / MAX_VOLUME: 2^128 → Number.MAX_SAFE_INTEGER * intent-engine matching, fan-out volume allocation, midpoint calc: switched to Number arithmetic * PaymentsAdapter gets `getDecimals(coinId)` backed by sphere-sdk's TokenRegistry.getTokenDecimals (the one place we DO need decimals — at the smallest-unit boundary) * trader-main.ts onDealAccepted: converts whole-unit volume × rate to smallest-units bigint via toSmallestUnitsBigInt() at the wallet reservation boundary * swap-executor.ts buildSwapDealInput: optional getDecimals lookup converts whole-unit terms to smallest-units integer strings before calling sphere.swap.proposeSwap (which requires positive-integer-string per SwapModule.ts:1186) * Tests sweep: 56 sites of `123n` → `'123'` across intent/deal/match assertions; ledger amount assertions (getAvailable) left as bigint since that side IS smallest-units The four-stop fix chain that landed today on the soak's §6 was: 1. trader-service #25 — drop Number() cast (precision) 2. sphere-sdk #483 — align SDK types to bigint-string 3. trader-service #26 — drop structured price field (server rejected it) 4. this PR — rate/volume in human-units throughout (server rejected the bigint-in-text-query too) Verified: - npx tsc --noEmit: clean - npm test: 698/698 pass (added Number arithmetic tests on top of existing string-fixture coverage) - npm run build: tsup clean Surfaced by: unicity-sphere/sphere-sdk#475 §6/§8 (trader-roundtrip soak) which produced HTTP 500 against /api/intents and /api/search with the bigint-in-text formats.
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
`intent-engine.ts:908` computed the bigint midpoint of the trader's rate band and cast to `Number` before calling `MarketModule.postIntent`. For a typical 18-decimal quote asset, the midpoint of a realistic rate band (e.g. 0.08–0.12 ETH/UCT) is `1e17 = 100_000_000_000_000_000` — past `Number.MAX_SAFE_INTEGER` (2⁵³ ≈ 9 × 10¹⁵). The IEEE 754 double stored a close-but-not-exact value, JSON serialised it as a number, and the market-api server rejected with HTTP 500 (non-JSON body, so the trader only logged the opaque status code with no actionable diagnostic).
Fix
Aligns with sphere-sdk PR #483 which makes the same change on the SDK side (`PostIntentRequest.price`, `SearchIntentResult.price`, `SearchFilters.{min,max}Price` all become `string`).
This is the established bigint serialisation convention everywhere else in the SDK (TXF amount fields, transfer payloads, all token amounts): bigint internally, decimal-string on the wire. Avoids JavaScript's `Number` precision loss for values above 2⁵³.
Why this is a client-side fix
`MarketIntent.price` in sphere-sdk was already `string` — the read shape uses strings. The server clearly round-trips strings on reads. So the server should accept string prices on writes without server changes. If a server-side change is also needed, that's tracked separately; this is the client-side correctness fix.
Test plan
Related