qa(audit): numeric-validation — 18 Rule #2 fixes across 5 modules#14
Merged
Conversation
First commit of `qa/2026-05-06-numeric-validation-audit` cycle. Previous v0.13.0 cycle found 3 Rule #2 violations via the helper-extract pattern (NaN silent-pass / NaN propagation / silent empty book). The review hypothesis was that other modules without that treatment likely harbor the same class of bug. trade-validator.ts confirms it: 5 numeric input paths were unguarded against NaN, all of which would have slipped past their downstream comparison checks because NaN comparisons are always false. ### Production fixes (5) All throw `PerpError("EXCHANGE_ERROR", ...)` so the calling trade flow surfaces a real venue/parsing failure instead of a confusing "$NaN available" or false-positive "insufficient liquidity" message. 1. `markPrice` — `<= 0` post-check skipped NaN; now rejected upfront. 2. `balance.available` — would silently take the "insufficient balance" branch and emit `$NaN available` to the user. 3. orderbook level `px` / `sz` — would inflate `availableLiquidity` to NaN and emit "Insufficient liquidity: $NaN" with no real check. Also rejects zero/negative price (level should not exist). 4. reduce-only `pos.size` (`parseFloat`) — would make `params.size > posSize` always false, silently passing the size check. 5. `marketInfo.fundingRate` output — substitutes 0 + warning instead of emitting `NaN` to JSON-parsing agents (output sanitization, not silent-pass: warning surfaces the missing data). ### New tests (6, 38 → 44 in trade-validator.test.ts) One regression case per fix above — each verifies the explicit throw or, for fundingRate, the 0-substitute + warning emission. ### Pattern note for follow-up The audit pattern that found these bugs: grep `Number(...)` and `parseFloat(...)` calls without an adjacent `Number.isFinite` / `Number.isInteger` guard, then trace each downstream comparison to see if NaN slips through. lighter.ts has many `|| 0` fallback patterns that look like the same class — next commit candidate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second commit of `qa/2026-05-06-numeric-validation-audit` cycle.
Confirms hypothesis: every module without the helper-extract treatment
harbors the same class of NaN silent-pass — lighter.ts had 9 sites
silently coercing to 0 via `Number(... || 0)`.
### Helper
`LighterAdapter._toFiniteNumber(value, fieldName, defaultValue=0)` —
public static for unit-testability. Rules:
- undefined / null → `defaultValue` (venue may legitimately omit a
field for an empty account / position)
- finite number / parseable string → returned as-is
- NaN / ±Infinity / non-numeric string → throw `EXCHANGE_ERROR` with
the field name in the message (so triage can attribute the failure
to the exact venue endpoint)
### Production sites updated (9, getBalance + getPositions)
Balance:
- `total_asset_value` (line 545)
- `available_balance` (line 546)
- `collateral` (line 547)
- `position.unrealized_pnl` reduce (line 549)
Positions:
- `position` filter (line 574)
- `posSize` (line 576)
- `position_value` (used in markPrice + leverage calcs, lines 584/592)
- `total_asset_value` for leverage equity (line 593)
Each site previously hid corruption as a phantom $0 balance / 0
position size — exactly the failure mode that misled the v0.12.13
HL portfolio undercount fix.
### Tests (new file `lighter-toFinite.test.ts`, 9 cases)
- finite numbers / numeric strings unchanged
- undefined / null → default (with custom default override)
- NaN → throw with exchange tag in details
- ±Infinity → throw
- non-numeric strings (`"abc"`, `"12abc"`) → throw
- empty string `""` → 0 (Number("") === 0, allowed)
- field name + value present in error message for triage
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third commit of the numeric-validation-audit cycle. Applies the same Rule #2 pattern across three more sites that the helper-extract audit skipped: ### event-stream.ts (observability silent skips) `Number(p.markPrice)` / `Number(p.liquidationPrice)` and balance delta computations would silently fail their `> 0` / `> 0.01` threshold checks on NaN, suppressing the very alerts users rely on (critical-distance liquidation_warning, balance_update events). Now `Number.isFinite` checks gate each branch — corruption is logged to stderr, the misleading "no alert" outcome is replaced with a visible warning, and downstream comparison logic only runs when the inputs are trustworthy. ### rebalance.ts (planner input) `computeRebalancePlan` consumes `Number(bal.equity)` etc. without a finiteness check. A NaN value would propagate into the sum-and-target math and the execute step would attempt nonsense moves. Throws `EXCHANGE_ERROR` for the affected adapter so `Promise.allSettled` filters it out — partial result instead of corrupt plan. ### hyperliquid-outcome.ts:439 (cycle leftover) Previous v0.13.0 cycle's `getOrderbook` fix replaced `book?.levels ?? [[],[]]` with an explicit throw, but `Number(book.time ?? 0)` was left untouched and would coerce a non-numeric venue time to 0 (1970 epoch — silently wrong, not "missing"). Now distinguishes "time omitted" (legit 0) from "time corrupted" (throws). ### Tests No new unit tests — these sites are stream/aggregation/integration shaped, not pure helpers. Existing 106 tests across the previously audited helpers all pass (verified). The cycle's helper-extract audit pattern continues to grow regression coverage; cross-cutting observability sites are recorded here for follow-up if integration test infrastructure expands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cycle artifact for `qa/2026-05-06-numeric-validation-audit`. Documents how the v0.13.0 cycle's "tip of the iceberg" hypothesis was tested across 5 modules and confirmed by 18 additional Rule #2 violations. Includes: - Hypothesis verification table (3 → 21 total findings across 2 cycles) - Per-module audit pattern + before/after table - Production interface change summary (no envelope drift on healthy responses; throws now surface previously hidden corruption) - Follow-up plan: mcp-server output sanitization, cross-adapter audit, Number.isFinite uniformity, dep-approval gates (coverage / fast-check) - Appendix A: cumulative finding count table - Appendix B: re-usable grep commands for the next audit cycle Container ground-truth at HEAD `ed533cb`: 75 files / 1396 tests / 23.09s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Findings (18 production fixes)
trade-validator.tsf381c0bexchanges/lighter.ts26d78d7event-stream.tsed533cbrebalance.tsed533cbexchanges/hyperliquid-outcome.ts:439ed533cbApproach
이전 사이클의 audit 패턴 follow-through:
LighterAdapter._toFiniteNumber, public static)Number.isFinite가드 + 명시 throw — helper 추출 어려운 곳 (event stream)Tests (+15)
trade-validator.test.ts38 → 44 (+6) — Rule v0.2.1: scaled TP, pnl/liquidation alerts, bot fix #2 NaN 가드 case 마다 1개exchanges/lighter-toFinite.test.ts신규 9 case — helper 단위 테스트stream / rebalance / outcome-time 은 inline 가드라 단위 테스트 추가 안 함 (integration shape, 보고서에 follow-up 명시).
자세한 보고
docs/qa-reports/2026-05-06-numeric-validation-audit.md참조 (228 줄, 가설 검증 표 + 모듈별 변경 + 부록).Out of scope — follow-up
pacifica.ts/hyperliquid.ts/aster.ts동일 audit —cross-adapter-matrixcycle 의 일부.Number.isFinite통일 — 가독성 (현재 일부Number.isNaN || Infinity명시).@vitest/coverage-v8(coverage 정량),fast-check(property test).Test plan
pnpm test(unit suite): 1396 / 1396 passed (host + container)pnpm build(tsc strict): cleanLive ↔ Unit Test Cross-Validation
해당 사유: 이번 PR 은
--json명령 envelope 변경 없음 (정상 응답 동일, 잘못된 venue 응답에 한해서만 명시 EXCHANGE_ERROR throw 추가). 라이브 호출 ↔ 단위 테스트 매핑은 v0.13.0 사이클 보고서 Appendix B 가 그대로 유효.Section 3 / 13 — 절대 금지 항목 준수
main 머지/push (이 PR 자체는 사용자 명시 승인) · npm publish ✗ · git tag ✗ · GitHub Release ✗ · mainnet 실거래 ✗ · deps 메이저 변경 ✗ · 새 npm 패키지 추가 ✗
🤖 Generated with Claude Code