fix(market): treat rate/volume as decimal strings (human-units throughout) - #27
Merged
Merged
Conversation
…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
force-pushed
the
refactor/decimal-rates-volumes
branch
from
June 12, 2026 13:54
9ad721d to
5093791
Compare
vrogojin
added a commit
that referenced
this pull request
Jun 13, 2026
…ler (#28) PR #27 changed TradingIntent.rate_min/rate_max/volume_min/volume_max from bigint to string but missed the trader-command-handler.ts gate at the wire boundary. The handler still ran safeParseBigint() first, which rejected '0.08' with "rate_min must be a non-negative integer string". The shape downstream is decimal string everywhere (validateIntentParams, TradingIntent record, encodeDescription) — the handler just needs to assert the input IS a string and pass it through. Detailed validation remains in validateIntentParams (called immediately below). Verified end-to-end in unicity-sphere/sphere-sdk#475 trader-roundtrip soak: alice's `sphere trader create-intent --rate-min 0.08 ...` now succeeds at the trader gate (previously failed before reaching the intent engine). 698 / 698 tests pass.
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 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` — and the semantic-search engine on the deployed market-api server returns HTTP 500 (HTML body) when it encounters large-number-dash-large-number patterns in the query string.
It was never tested live: the trader's e2e suite mocks `MarketAdapter`, so the bigint values never round-tripped through the real server.
Why this is the right shape
Existing test fixtures show the original design: rates were small dimensionless ratios, volumes were in BASE whole units, and the math `rate × volume = 475 × 1000 = 475000` worked at small numbers:
```js
// test/e2e/trader-intent-lifecycle.e2e.test.ts:190
rate_min: '450', rate_max: '500', volume_min: '100', volume_max: '1000'
```
The bigint typing came from a later misinterpretation that conflated rate/volume with token amounts. This PR restores the original intent.
Changes
Where decimals lookup happens
Only at the smallest-unit boundary. Description / search query / parseDescription / encodeDescription / matching all work in human decimal strings. Smallest-unit conversion lives in two places:
Both call the SDK's `getTokenDecimals(coinId)` via the new `PaymentsAdapter.getDecimals`.
The four-stop chain that landed today on soak §6
Each was a real bug. The first three didn't unblock the soak alone; this fourth completes the chain.
Test plan
Related