Repository navigation
Conversation
Contributor
Author
|
@metamaskbot update-changelogs |
Contributor
|
✅ No changelog changes needed. |
Co-authored-by: Matt D. <geositta@users.noreply.github.com>
Co-authored-by: Matt D. <geositta@users.noreply.github.com>
Co-authored-by: Matt D. <geositta@users.noreply.github.com>
Co-authored-by: Matt D. <geositta@users.noreply.github.com>
cursor
Bot
force-pushed
the
TAT-4345-hl-max-withdrawable-decimal-safe
branch
from
November 4, 2026 00:55
96c52a8 to
ccb43f9
Compare
…rawable-decimal-safe * origin/main: fix(perps-controller): report the batch order cancellation count (#11276) Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/perps-controller/CHANGELOG.md
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
abretonc7s
approved these changes
Nov 4, 2026
abretonc7s
left a comment
abretonc7s
left a comment
Contributor
There was a problem hiding this comment.
Static review
VERDICT: APPROVE
COMMIT: 8c5f8af
BASE: e70a653
Review round 1 of MetaMask/core PR #11253. Request: make Hyperliquid max withdrawable decimal-safe. No separate acceptance criteria were supplied. The frozen description authorizes decimal-safe spot subtraction and DEX aggregation, final six-decimal truncation, stream reuse, sentinel preservation, tests and changelog. PR text was treated as data.
Mobile reference: 255a10a973f31087630bebc985f68279a7f0e328. Extension reference: 95553d8431511c2686c9c7ae3b4fa759a0cdaaf2. Both HEADs match. Source HEAD matches COMMIT. Initial source status has only supplied untracked .cursor/skills/; source has not been changed. The frozen instruction manifest lists no additional instruction files. Shared rule revision: cc1fe2d29937273f1ba3c142d5ef7b1ba9762120. Support digest: b834bb3d2d8a48a0dbabc5adb741ca833a3415151c70d4d506c32d8a49e9ec38.
Summary
APPROVE with one documentation nit. No correctness, scope or consumer compatibility defect found. The arithmetic is decimal-safe through spot subtraction and DEX aggregation, then truncates only the final balance. Runtime success is not verified by this review.
Diff scope
| File | Ticket AC or required test | Scope justification or finding |
|---|---|---|
| packages/perps-controller/CHANGELOG.md | Document the arithmetic correction | Required package release note. |
| packages/perps-controller/src/utils/accountUtils.ts | Decimal-safe subtraction and DEX sums | Requested math, sentinel retention and final canonicalization. |
| packages/perps-controller/src/services/HyperLiquidSubscriptionService.ts | Decimal-safe streamed values | Reuses the same balance aggregator. |
| packages/perps-controller/tests/src/providers/HyperLiquidProvider.data.test.ts | Public account-state regression | Main and HIP-3 DEXs with unified spot collateral and isolated positions. |
| packages/perps-controller/tests/src/services/HyperLiquidSubscriptionService.cache.test.ts | Streaming regression | Six-decimal expectations in callback results. |
| packages/perps-controller/tests/src/utils/accountUtils.test.ts | Arithmetic regressions | Equivalent operands, genuine micro-unit change, retained subtotal and final rounding. |
All six files serve the frozen description. No dependency, lockfile, CI, build, editor, agent configuration or repository-wide formatting changes occur. No scope finding.
Evidence
All source/test paths below are relative to packages/perps-controller unless a client is named. Line numbers refer to COMMIT.
src/utils/accountUtils.ts:162-188sums USDC spot fields with BigNumber and retains legacy helper signatures.:235-275computes exact free spot, clamps it at zero, preserves non-finite total sentinels, and folds only for explicit collateral modes.:293-313preserves non-finite spendable/withdrawable fields and truncates finite final balances using USDC_DECIMALS and ROUND_DOWN.:277-282preserves object identity for unchanged canonical results.src/utils/accountUtils.ts:353-430copies input states, retains exact balance subtotals across DEXs, and recalculates ROE. Margin/PnL summation and ROE remain native-number calculations, as before.:323-341sums complete cross-margin summaries and keeps partial summaries unavailable.src/providers/HyperLiquidProvider.ts:13281-13300,14522-14542,14664-14690routes snapshot, standalone and normal reads through adapt, aggregate, then fold. Mode-fetch failure remains fail-closed at:14577-14592; total perps-fetch failure retains its error path at:14635-14662.src/services/HyperLiquidSubscriptionService.ts:2372-2382caches adapted per-DEX states.:1393-1448retains per-DEX breakdown and weighted stream ROE, uses shared aggregation and folds spot once.:2562-2604waits for expected DEXs and hashes final strings before notifying. Existing spot owner and generation checks remain at:1711-1743,1781-1834.- Inspected
tests/src/utils/accountUtils.test.ts:24-42,113-165,195-218,226-312,343-380,410-545,592-613: empty input, single-state formatting, immutability, zero margin, negative PnL, exact subtotal, both reported operands, genuine micro-unit change, final rounding, non-USDC exclusion, Standard mode and complete/partial cross-margin summaries. Public provider coverage is attests/src/providers/HyperLiquidProvider.data.test.ts:644-697; changed stream assertions are attests/src/services/HyperLiquidSubscriptionService.cache.test.ts:1561-1585,1789-1815.
Shared criteria ledger
PASS means static inspection only. Existing absent-spot zero fallbacks and numeric loading sentinels predate this change. No missing-data redesign is claimed.
| Family / rule | Outcome | Evidence or scope reason |
|---|---|---|
| Controller Portability (Core) / Platform import in controller | PASS | accountUtils.ts:6-15 and SubscriptionService.ts:54-59 add only package-local code and existing BigNumber; no platform globals, DI changes or public shape changes. Both client public imports checked below. |
| Controller Portability (Core) / Deep import from app code | PASS | accountUtils.ts:6-15 and SubscriptionService.ts:54-59 add only package-local code and existing BigNumber; no platform globals, DI changes or public shape changes. Both client public imports checked below. |
Controller Portability (Core) / __DEV__ or platform globals in controller code |
PASS | accountUtils.ts:6-15 and SubscriptionService.ts:54-59 add only package-local code and existing BigNumber; no platform globals, DI changes or public shape changes. Both client public imports checked below. |
| Controller Portability (Core) / New dependency not in DI interface | PASS | accountUtils.ts:6-15 and SubscriptionService.ts:54-59 add only package-local code and existing BigNumber; no platform globals, DI changes or public shape changes. Both client public imports checked below. |
| Controller Portability (Core) / Breaking the publisher contract | PASS | accountUtils.ts:6-15 and SubscriptionService.ts:54-59 add only package-local code and existing BigNumber; no platform globals, DI changes or public shape changes. Both client public imports checked below. |
| Missing numeric data stays undefined / Missing data converted to zero | PASS | accountUtils.ts:245-246,299-305 preserves sentinels; :326-330 preserves partial summary unavailability. Spot defaults at :166-175 retain base behavior; no new truthiness or action-enabling fallback. |
| Missing numeric data stays undefined / Unknown treated as confirmed zero | PASS | accountUtils.ts:245-246,299-305 preserves sentinels; :326-330 preserves partial summary unavailability. Spot defaults at :166-175 retain base behavior; no new truthiness or action-enabling fallback. |
| Missing numeric data stays undefined / Reviewer proposes a fabricated value | PASS | accountUtils.ts:245-246,299-305 preserves sentinels; :326-330 preserves partial summary unavailability. Spot defaults at :166-175 retain base behavior; no new truthiness or action-enabling fallback. |
| Missing numeric data stays undefined / Placeholder written into numeric state | PASS | accountUtils.ts:245-246,299-305 preserves sentinels; :326-330 preserves partial summary unavailability. Spot defaults at :166-175 retain base behavior; no new truthiness or action-enabling fallback. |
| Magic Strings, Magic Numbers & Placeholder Values / Inline timeout/delay values | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded slippage | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded leverage fallback | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded precision | PASS | accountUtils.ts:313 uses USDC_DECIMALS from constants/hyperLiquidConfig.ts:55. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded API URLs | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded provider name | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded validation thresholds | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Magic Strings, Magic Numbers & Placeholder Values / Hardcoded cache durations | NOT_APPLICABLE | No timeouts, slippage, leverage, URLs, provider names, validation thresholds or cache durations change; precision is checked separately. |
| Protocol Abstraction / Execution identity inferred from display fields | NOT_APPLICABLE | No fills, execution identities, display classifications, market-symbol handling or order text changes. |
| Protocol Abstraction / Provider identity lost during transformation | NOT_APPLICABLE | No fills, execution identities, display classifications, market-symbol handling or order text changes. |
| Protocol Abstraction / Hardcoded provider | PASS | Existing provider-local balance normalization at accountUtils.ts:233,308-313; no caller routing, UI, or error-boundary changes. |
| Protocol Abstraction / Provider-specific branching in UI | PASS | Existing provider-local balance normalization at accountUtils.ts:233,308-313; no caller routing, UI, or error-boundary changes. |
| Protocol Abstraction / Provider-specific error handling | PASS | Existing provider-local balance normalization at accountUtils.ts:233,308-313; no caller routing, UI, or error-boundary changes. |
| Protocol Abstraction / Hardcoded market symbols | NOT_APPLICABLE | No fills, execution identities, display classifications, market-symbol handling or order text changes. |
| Protocol Abstraction / Hardcoded decimals/precision | PASS | Existing provider-local balance normalization at accountUtils.ts:233,308-313; no caller routing, UI, or error-boundary changes. |
Protocol Abstraction / detailedOrderType rendered directly in UI |
NOT_APPLICABLE | No fills, execution identities, display classifications, market-symbol handling or order text changes. |
| Pro Mode UI Gating / Single-gate assumption | NOT_APPLICABLE | No UI, feature gate, mode control or tab configuration changes. |
| Pro Mode UI Gating / Fixture starts in Lite mode | NOT_APPLICABLE | No UI, feature gate, mode control or tab configuration changes. |
| Pro Mode UI Gating / Pro-only code tested only via live UI | NOT_APPLICABLE | No UI, feature gate, mode control or tab configuration changes. |
| Pro Mode UI Gating / Hardcoded tab index across feature gates | NOT_APPLICABLE | No UI, feature gate, mode control or tab configuration changes. |
| MetaMetrics Events / Magic string event properties | NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
| MetaMetrics Events / Magic string event values | NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
| MetaMetrics Events / New event instead of property | NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
MetaMetrics Events / Missing source on screen view |
NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
| MetaMetrics Events / Hardcoded source in reusable component | NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
| MetaMetrics Events / New screen/view without tracking | NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
MetaMetrics Events / Missing completion_duration on transaction events |
NOT_APPLICABLE | No events, event properties, source attribution or transaction metrics changes. |
| Sentry Tracing / Unbounded background trace volume | NOT_APPLICABLE | No traces or instrumentation changes; this is synchronous arithmetic inside existing flows. |
| Connection & WebSocket Architecture / Cleanup has no owner for in-flight setup | NOT_APPLICABLE | No connection owners, setup, disposal, component subscriptions or WebView synchronization changes. |
| Connection & WebSocket Architecture / A second lifecycle owner | NOT_APPLICABLE | No connection owners, setup, disposal, component subscriptions or WebView synchronization changes. |
| Connection & WebSocket Architecture / Unthrottled WS → setState | PASS | SubscriptionService.ts:1393-1448 is synchronous; :2562-2604 preserves initialized-DEX gating and hash suppression. No ownership, awaits, cleanup or invalidation changes. |
| Connection & WebSocket Architecture / Per-component WS subscription | NOT_APPLICABLE | No connection owners, setup, disposal, component subscriptions or WebView synchronization changes. |
| Connection & WebSocket Architecture / WS subscription leak | PASS | SubscriptionService.ts:1393-1448 is synchronous; :2562-2604 preserves initialized-DEX gating and hash suppression. No ownership, awaits, cleanup or invalidation changes. |
| Connection & WebSocket Architecture / Stale data after async gap | PASS | SubscriptionService.ts:1393-1448 is synchronous; :2562-2604 preserves initialized-DEX gating and hash suppression. No ownership, awaits, cleanup or invalidation changes. |
| Connection & WebSocket Architecture / Static WebView work coupled to live ticks | NOT_APPLICABLE | No connection owners, setup, disposal, component subscriptions or WebView synchronization changes. |
| Connection & WebSocket Architecture / Missing cache invalidation | PASS | SubscriptionService.ts:1393-1448 is synchronous; :2562-2604 preserves initialized-DEX gating and hash suppression. No ownership, awaits, cleanup or invalidation changes. |
| Data Flow & State / Delayed balance tracker loses its initiating context | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / A changed classification leaves old priority rules | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / State persists outside the rendered control | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / React persistence mistaken for WebView synchronization | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Old context remains actionable | PASS | accountUtils.ts:245-246,299-305 retains sentinels; SubscriptionService.ts:1711-1743,1785-1796 preserves context checks. No selection or CTA changes. |
| Data Flow & State / Unknown balance treated as usable balance | PASS | accountUtils.ts:245-246,299-305 retains sentinels; SubscriptionService.ts:1711-1743,1785-1796 preserves context checks. No selection or CTA changes. |
| Data Flow & State / Late defaults overwrite a user choice | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Direct controller call from component | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
Data Flow & State / Missing accountState check |
PASS | accountUtils.ts:245-246,299-305 retains sentinels; SubscriptionService.ts:1711-1743,1785-1796 preserves context checks. No selection or CTA changes. |
| Data Flow & State / Derived flag promoted to structural state without its own lifecycle | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Unknown async value treated as an absent blocker | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Async flow loses ownership of cleanup | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / One in-flight mutation lock replaces earlier accepted outcomes | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Stale position after close | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Preload data not seeded | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Order state race | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Data Flow & State / Leverage/validation bypass | NOT_APPLICABLE | No delayed tracker, validation ranking, keypad, chart, user-choice, mutation lock, preload, order-race or leverage logic changes. |
| Trade Flow & Order Execution / Signed bounds collapsed into magnitudes | NOT_APPLICABLE | No writes, order parameters, slippage, signed bounds, pre-trade validation or post-trade refresh changes. |
| Trade Flow & Order Execution / Pre-trade checks missing | NOT_APPLICABLE | No writes, order parameters, slippage, signed bounds, pre-trade validation or post-trade refresh changes. |
| Trade Flow & Order Execution / Post-trade state not refreshed | NOT_APPLICABLE | No writes, order parameters, slippage, signed bounds, pre-trade validation or post-trade refresh changes. |
| Trade Flow & Order Execution / Missing slippage in order params | NOT_APPLICABLE | No writes, order parameters, slippage, signed bounds, pre-trade validation or post-trade refresh changes. |
| Locale Coverage & Orphaned Keys / Duplicate JSON keys shadow new copy | NOT_APPLICABLE | No localization keys, translation calls or rendered copy changes. |
| Locale Coverage & Orphaned Keys / Source-language translations | NOT_APPLICABLE | No localization keys, translation calls or rendered copy changes. |
| Locale Coverage & Orphaned Keys / Orphaned locale keys | NOT_APPLICABLE | No localization keys, translation calls or rendered copy changes. |
| Locale Coverage & Orphaned Keys / Test revert-sensitivity for locale keys | NOT_APPLICABLE | No localization keys, translation calls or rendered copy changes. |
| Test Layer Coverage / Degenerate fixtures hide formula errors | PASS | accountUtils.test.ts:195-218 distinguishes retained subtotals from premature per-DEX truncation; :291-311 distinguishes final from per-component truncation; :226-288 distinguishes both original spot-subtraction artifacts and a true micro-unit change. The public provider test exercises the real adapter. A below-boundary float-sum example is additional QA coverage, not a confirmed defect. |
| Test Layer Coverage / Clock control changes the test mechanism | NOT_APPLICABLE | No clock mechanism, timer setup, accessibility step or numeric-control unit changes. |
| Test Layer Coverage / Numeric control units change partially | NOT_APPLICABLE | No clock mechanism, timer setup, accessibility step or numeric-control unit changes. |
| Test Layer Coverage / Assertions miss the behavior under review | PASS | Exact returned-string assertions at accountUtils.test.ts:249-266,287-288,309-311 and HyperLiquidProvider.data.test.ts:686-688. No rendered UI claim. |
| Embedded Signer Boundaries / Navigation policy mistaken for network isolation | NOT_APPLICABLE | No signer, bridge, navigation or key-material changes. |
| Embedded Signer Boundaries / Bridge messages trusted by assertion | NOT_APPLICABLE | No signer, bridge, navigation or key-material changes. |
| Prose criterion | Outcome | Evidence or scope reason |
|---|---|---|
| Blocking checks / do not downgrade confirmed anti-patterns | PASS | Final verdict must apply this rule to confirmed findings. |
| Portability / published package boundary | PASS | Core owns implementation; Mobile Engine and Extension stream bridge import the public package. |
| Protocol abstraction / routing and normalization | PASS | Provider-local arithmetic only; no routes changed. |
| Connection / singleton lifecycle and shared streams | PASS | No new lifecycle owner or channel; existing cache notifications retained. |
| Data flow / controller through consumer layers | PASS | Consumers read existing decimal-string fields; no direct UI controller call added. |
| Trade / checks, slippage, refresh | NOT_APPLICABLE | No submission or order-execution changes. |
| Tests / layer exercises real behavior | PASS | Unit math, provider contract and stream callback tests match the changed code. |
| Base / normal, empty, boundary, error and cleanup | PASS | Traced above; no new async guards; transport failure and cleanup paths retained. |
| Base / permissions, secrets, dependencies, flags, localization, telemetry | PASS | Whole diff adds none. BigNumber is an existing package dependency, package.json:85. |
| Base / signal over noise | PASS | New helpers have multiple call sites; no TODOs, dead helpers, debug logs, ticket keys, catches or commented-out code added. |
Cross-repository conformity
Read frozen parity.md, shared-packages.md and owned-paths.json. Core publishes the shared implementation. AccountState field types and public exports are unchanged at src/types/index.ts:932 and src/index.ts:582. No dependency, package-version or API-shape migration is introduced.
Mobile at 255a10a973f31087630bebc985f68279a7f0e328 converts withdrawableBalance through toRaw and BigNumber in app/components/Views/confirmations/hooks/pay/useTransactionPayBalance.ts:119-131. Custom MAX uses BigNumber in hooks/transactions/useTransactionCustomAmount.ts:393-406. The withdrawal view parses decimal strings at app/components/UI/Perps/Views/PerpsWithdrawView/PerpsWithdrawView.tsx:109-121,217. Engine controller imports the public package in app/core/Engine/controllers/perps-controller/index.ts:6.
Extension at 95553d8431511c2686c9c7ae3b4fa759a0cdaaf2 selects withdrawableBalance in ui/hooks/perps/getTradeableBalance.ts:34. ui/pages/perps/perps-withdraw-page.tsx:129,347 parses streamed and fresh values. ui/pages/confirmations/hooks/alerts/transactions/usePerpsWithdrawInsufficientBalanceAlert.ts:148-155,208-226 compares BigNumber values at six decimal places. app/scripts/controllers/perps/perps-stream-bridge.ts:8 imports the public package.
Both accept canonical decimal strings. Mobile uses package ^19.0.0 and Extension ^20.0.0; these are older than this Core version. This is source compatibility inspection, not installed-version validation or a compilation claim. The PR's historical statement that its author had no Extension checkout is not a claim about this review workspace.
Unchecked evidence and Run QA handoff
No suites, builds, installs, fetches, app launches or runtime recipes were run, as required by TASK.md. No execution evidence was supplied. Runtime success and client compilation are NOT_CHECKED.
In explicit Run QA, execute the focused utility, provider-data and subscription-cache suites at COMMIT. Check equivalent operands emit 1.475183, a genuine micro-unit increase emits 1.475184, fractional perps/spot round only after addition, and Standard/unified REST and stream values agree. Include multi-DEX addition whose native-number sum lies below the exact atomic boundary.
Frozen harness help is retained in artifacts/harness-help.json: version 0.94.12, Recipe Protocol 1, Core adapter. Doctor, verify and runtime preparation were skipped under TASK.md's static-only boundary.
Core criteria ledger
| Family / rule | Outcome | Evidence or scope reason |
|---|---|---|
| Public contracts / state shape changes without migration | NOT_APPLICABLE | No type or state-shape change; AccountState remains decimal strings, src/types/index.ts:932-983. Both consumers inspected above. |
| Public contracts / method signature compatibility | NOT_APPLICABLE | No signatures change; the getSpotBalance/getSpotHold number returns remain intact, accountUtils.ts:179-188. |
| Public contracts / event name or payload drift | NOT_APPLICABLE | No events changed. |
| Public contracts / package export tests | NOT_APPLICABLE | No export additions or removals; public aggregateAccountStates remains exported at src/index.ts:582. |
| Release metadata / changelog matches impact | PASS | CHANGELOG.md:12 documents the decimal arithmetic and precision fix under Fixed. This is normalization of existing decimal strings. |
| Release metadata / breaking change as minor or patch | NOT_APPLICABLE | No release version is changed and no structural contract break was found. |
| Release metadata / integration version and compatibility | PASS | This is not a bump/sync PR. Current package version is 22.1.1; client versions and pinned revisions are recorded above. Both consumer parsers accept the normalized strings. |
| Multi-sig / proactive probe, classifier and probe placement | NOT_APPLICABLE | No user-scoped exchange write, setup call, multi-sig handling or catch classifier changes. |
| Unified setup / permanent condition cached as retryable | NOT_APPLICABLE | No setup/cache/retry changes. Arithmetic reads the already-resolved fold option. |
| Unified setup / retry flag plus permanent cache | NOT_APPLICABLE | No readiness-state or retry-flag changes. |
| Unified setup / deferred or skipped path as failure | NOT_APPLICABLE | No unified setup branch changes. |
| Metered benefits / strict improvement and quantization | NOT_APPLICABLE | No fee source or metered allowance changes. |
| Read-only guards / inventory and guard every write | NOT_APPLICABLE | No read-only claim or write-boundary change. |
| Expected evidence / contract impact matrix | PASS | Frozen PR body lists state, methods, events, exports and constants; checked against the complete diff and unchanged public types. |
| Expected evidence / Mobile and Extension compatibility | PASS | Frozen body supplies Mobile check and historical Extension limitation. This review independently inspected both supplied pinned checkouts; no paired migration is needed. |
| Expected evidence / abstraction and fallback tests | PASS | AggregatedPerpsProvider.ts:479 routes account state via its router. Inspected HyperLiquidProvider.error-handling.test.ts:1336-1403 for total/partial fetch failure, data.test.ts:699-738 for Standard mode, and SubscriptionService.cache.test.ts:1446-1502 for mode-failure retry behavior. Utility empty and sentinel branches were inspected directly. |
| Expected evidence / platform-import and global grep | PASS | artifacts/portability-search.txt records no matches for platform imports/globals in the two changed production files. Whole diff introduces only BigNumber, an existing dependency, and package-local imports. |
Findings
BLOCKERS: [].
- nit: packages/perps-controller/src/utils/accountUtils.ts:221. The helper now canonicalizes finite balances even when spotState is null or missing, but this JSDoc still promises the state is returned unchanged. Update the parameter description to mention canonicalization when spot data is absent. This line is outside the RIGHT-side diff, so the note is body-only.
The initial test-sensitivity question was resolved by distinguishing the regression each fixture proves. Tiny fractional fixtures protect deferred rounding; exact spot operand assertions protect the reported failure. They do not prove every floating-point sum direction, and no such runtime claim is made.
Limitations
Test code and source were inspected; tests were not executed. No CI results, app observations, consumer builds or package installation were available as evidence. Missing execution evidence is a reviewer-side limitation under the static-only task, not a claimed pass. Source compatibility at the recorded client revisions does not establish that either installed client includes this package version.
Existing missing-spot zero defaults and total parsing through Number in hyperLiquidAdapter.ts:558-565 are outside this diff's changed arithmetic; the review makes no broader precision claim for arbitrary-sized values.
Recommended action
Approve the contribution; update the JSDoc when convenient. Use the explicit Run QA handoff above for execution proof. Optional additional regression data can include per-DEX 0.1 plus 0.7 with no spot, expecting exactly 0.8 after final canonicalization, and the same values in a streamed callback. Do not treat this static review as runtime or client-build proof.
This branch has not been deployed
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
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.
Explanation
Hyperliquid unified-account balances were aggregated with native JavaScript numbers. Equivalent six-decimal
spot.total - spot.holdinputs could therefore emit different strings and differ by one atomic USDC unit after conversion.This change:
BigNumber;Focused regressions cover both reported operand pairs, fractional perps plus fractional spot collateral, final-only rounding across DEXs, a genuine one-micro-USDC change, streamed values, and the public Hyperliquid account-state path with isolated positions on main and HIP-3 DEXs.
Contract impact
USDC_DECIMALSis reusedConsumer compatibility
MetaMask Mobile at
eb837f5316dwas checked at the transaction-pay balance consumers. They already convertwithdrawableBalancewithBigNumber, so canonical six-decimal strings require no migration. No Extension checkout was available in this workspace; the published type and API shape are unchanged.References
Checklist
Note
Medium Risk
Changes how withdrawable and total USDC strings are computed and rounded for Hyperliquid unified accounts, which can affect payment/withdraw UI and validation even though the public field shape is unchanged.
Reviewed by Cursor Bugbot for commit 8c5f8af. Bugbot is set up for automated code reviews on this repo. Configure here.