fix(market): drop structured price field from trader's market posts - #26
Merged
Merged
Conversation
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
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 hits an unhandled exception on it (HTTP 500 with HTML body, swallowed by sphere-sdk's `MarketModule.parseResponse` to an opaque "unexpected response (not JSON)").
Why the field shouldn't have been there
Changes
The whole chain that landed today
This is the third of three trader-roundtrip soak §6 fixes that built on each other:
(1) and (2) are still correct independent fixes — even though dropping the field sidesteps the precision problem, the SDK types should still match the rest of the SDK's bigint convention for any future caller that does want to set the field. They stay.
Test plan
Related