fix: avoid float precision loss in Limitless balances#683
Conversation
realfishsam
left a comment
There was a problem hiding this comment.
PR Review: VERIFIED
What This Does
Adds a scaledIntegerToNumber(value, decimals) helper in limitless/utils.ts that performs integer division/modulo in bigint arithmetic before converting to float, eliminating the precision loss that occurs when rawBalance > Number.MAX_SAFE_INTEGER (≈ 9 quadrillion USDC raw units). Replaces two unsafe conversion call sites: parseFloat(rawBalance.toString()) / Math.pow(10, USDC_DECIMALS) in index.ts and parseFloat(utils.formatUnits(balance, decimals)) in client.ts.
Blast Radius
- Limitless only — two internal methods (
LimitlessExchange.fetchBalanceinindex.tsandLimitlessClient.getBalanceinclient.ts) - No changes to unified types, OpenAPI schema, SDK clients, or other exchanges
- The helper is not exported from the exchange
index.ts, so it doesn't widen the public API surface
Consumer Verification
Before (base branch — parseFloat(rawBalance.toString()) / Math.pow(10, USDC_DECIMALS)):
raw = 9007199254740993n (> Number.MAX_SAFE_INTEGER)
parseFloat("9007199254740993") → 9007199254740992 (rounds, loses the last digit)
9007199254740992 / 1_000_000 → 9007199254.740992 ← WRONG (off by 1 ulp in the 6th decimal)
fetchBalance would return 9007199254.740992 USDC instead of 9007199254.740993 USDC.
After (PR branch — scaledIntegerToNumber):
raw = 9007199254740993n
whole = 9007199254740993n / 1000000n = 9007199254n (exact bigint division)
fraction= 9007199254740993n % 1000000n = 740993n (exact bigint remainder)
result = Number(9007199254) + Number(740993)/Number(1000000)
= 9007199254 + 0.740993 = 9007199254.740993 ← CORRECT
Both the index.ts path (native bigint from on-chain call) and the client.ts path (ethers v5 BigNumber via toBigInt() / toString() fallback) are fixed. Confirmed in Node.js:
BEFORE (parseFloat/pow): 9007199254.740992 WRONG (off by 1 ulp)
AFTER (scaledIntegerToNumber): 9007199254.740993 CORRECT
BigNumber toString fallback: 1234567.890123 CORRECT
decimals=0 edge case: 42 CORRECT
Server starts cleanly on PR branch; POST /api/limitless/fetchBalance is reachable (returns expected auth error without credentials, as intended).
Test Results
- Build: PASS (tsc clean)
- New unit tests (
limitless-balance.core.test.ts): PASS (2/2) - Full suite on PR branch: 3 suites failing, 53 tests failing
- Full suite on main: 4 suites failing, 57 tests failing
The 3 failing suites on the PR branch (exchange-normalizers, param-forwarding, watch-order-book-api) are identical to failures on main — pre-existing, unrelated to this change. The PR branch actually resolves a 4th suite (schema-drift.test.ts) that fails on current main, because the PR is based on an older commit (389e781) that predates the drift regression on main.
- E2E smoke (Limitless fetchMarkets): PASS — live markets returned with correct shape
Findings
-
Number(decimals)inclient.ts:401relies on implicit toString() coercion —contract.decimals()returns an ethers v5 BigNumber;Number(bigNumber)falls through totoString()(returns "6") then converts, giving 6. This works and is validated byscaledIntegerToNumber's guard, butNumber(decimals.toString())ordecimals.toNumber()would be more explicit. Not blocking. -
index.ts:316–320still containsparseFloat(m.matchedSize) ... / Math.pow(10, USDC_DECIMALS)for order fill sizes — out of scope for this PR (those values originate as API string floats, not raw on-chain integers, so the overflow path via> MAX_SAFE_INTEGERis different). Not a regression introduced here. -
mergeable_state: "dirty"— branch needs a rebase onto current main before it can be merged. Unrelated to correctness of the fix.
PMXT Pipeline Check
- Field propagation (3-layer): N/A — no new fields
- OpenAPI sync: N/A — no schema changes
- Financial precision: OK — this PR is precisely fixing a financial precision bug
- Type safety: OK — no
anyintroduced, no!assertions, generic type union handles bothbigintand ethers BigNumber - Auth safety: N/A
Semver Impact
patch — pure bug fix to an internal balance conversion helper, no API surface change
Risk
On-chain balance calls require real credentials on Base mainnet, so the exact balance return value at the fetchBalance API boundary was verified via unit test and math proof rather than a live end-to-end call. The negative-balance branch in scaledIntegerToNumber is unreachable in practice (ERC-20 balanceOf returns uint256), but the guard is harmless.
Generated by Claude Code
cac3480 to
11a52bb
Compare
Fixes #675.
This avoids precision loss when converting raw USDC integer balances for Limitless.
Changes:
scaledIntegerToNumberhelper using integer division/modulo before final number conversion.parseFloat(utils.formatUnits(...))andparseFloat(rawBalance.toString()) / Math.pow(...).Number.MAX_SAFE_INTEGERand ethersBigNumberinput.Verification:
npm --workspace=pmxt-core test -- --runInBand core/test/unit/limitless-balance.core.test.tsnpm --workspace=pmxt-core run build