fix(perps): align decimal behavior and display parity cp-13.28.0#41855
Closed
abretonc7s wants to merge 36 commits into
Closed
fix(perps): align decimal behavior and display parity cp-13.28.0#41855abretonc7s wants to merge 36 commits into
abretonc7s wants to merge 36 commits into
Conversation
Create a clean base branch from current main that contains only the perps controller upgrade, decimal-logic changes, and required non-UI integration support so the UI formatting work can live entirely on the stacked branch. Constraint: Base PR must contain no ui/ changes at all Rejected: Continue using the original long-lived branch as PR A | three-dot history still surfaces UI files without a force-push or a replacement PR Confidence: high Scope-risk: moderate Directive: Keep this branch non-UI; all ui/ changes should stack on top in the formatting branch Tested: Targeted Jest for perps infrastructure + perps-controller-init; ESLint on touched non-UI files Not-tested: Full repo-wide CI and full branch-wide validation
Create a clean UI-only stacked branch on top of the non-UI controller base so all user-facing decimal behavior, display formatting, and related tests are reviewed in one place. Constraint: This replacement formatting branch must contain only ui/ files Rejected: Keep using the older stacked formatting branch | it still carried non-UI files from the previous split attempt Confidence: high Scope-risk: narrow Directive: Use this branch/PR for all UI-facing perps decimal and display review; do not review those changes on the non-UI base PR Tested: ESLint on all touched ui/ files Not-tested: Full UI Jest/recipe rerun on the replacement branch
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Contributor
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/perps (24 files, +616 -353)
📜 @MetaMask/policy-reviewers (4 files, +20 -0)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. |
Restore the display-price priority so the market detail header continues to track chart/live price updates instead of freezing on the initial market-data snapshot after the formatting refactor. Constraint: Keep the fix scoped to the stale header-price regression reported on the UI-only PR Rejected: Leave market.price first in the fallback chain | causes the live/chart branches to become unreachable once market data loads Confidence: high Scope-risk: narrow Directive: When multiple perps price sources exist, prefer the live/chart source for active display and keep market snapshot as fallback only Tested: ESLint ui/pages/perps/perps-market-detail-page.tsx; live perps recipe previously passed on this branch Not-tested: Full targeted recipe rerun after this exact one-line priority fix
Fix the remaining UI review regressions on the decimal/display parity branch by restoring live/chart-first header price priority, avoiding premature non-close in-progress toasts, and restoring the explicit USDC suffix in edit-margin available balance. Constraint: Keep the fix scoped to UI-only review regressions on the stacked perps parity branch Rejected: Leave the Bugbot findings unresolved | they describe real UI regressions introduced by the formatting branch Confidence: high Scope-risk: narrow Directive: When aligning extension with mobile formatting, preserve existing live-price precedence and user-visible denomination cues unless mobile explicitly differs Tested: ESLint on touched UI files; live CDP recipe pr-41558-decimal-formatting (42/42) Not-tested: Full branch CI rerun after push
Fix the remaining actionable UI review findings on the decimal/display parity branch by restoring live/chart-first display price precedence, suppressing premature non-close in-progress toasts, restoring the explicit USDC suffix in edit-margin available balance, and returning a full fallback fee result shape in usePerpsOrderFees. Constraint: Keep the fixes scoped to the UI branch and leave the mobile-parity-consistent order-form pricing behavior unchanged Rejected: Change usePerpsOrderForm price-source behavior | current mobile implementation also derives position size from display/fill price and values margin with mark/oracle price Confidence: high Scope-risk: narrow Directive: Treat Bugbot findings as actionable only when they contradict intended/mobile behavior or produce incomplete hook result shapes Tested: ESLint on touched UI files; Jest ui/hooks/perps/usePerpsOrderFees.test.ts; live CDP recipe pr-41558-decimal-formatting (42/42) Not-tested: Full branch CI rerun after push
Restore display-formatted asset symbols in usePerpsOrderForm so prefixed assets like HIP-3 markets render the same cleaned symbols across calculated position-size strings. Constraint: Keep the fix scoped to the remaining actionable UI Bugbot finding Rejected: Leave raw asset symbols in calculated display strings | produces inconsistent prefixed asset labels relative to sibling perps UI components Confidence: high Scope-risk: narrow Directive: When a hook returns user-visible perps asset strings, prefer getDisplaySymbol over raw asset identifiers Tested: Jest ui/hooks/perps/usePerpsOrderForm.test.ts --no-coverage; ESLint ui/hooks/perps/usePerpsOrderForm.ts Not-tested: Full branch CI rerun after push
Constraint: Need a fresh CI run on the cleaned #41558 branch head because the currently displayed failures are from the older poisoned branch state. Confidence: high Scope-risk: narrow Directive: Remove this commit only if CI can be retriggered another way without losing the clean branch state Tested: None (CI trigger only) Not-tested: Functional behavior unchanged
Restore the package baseline to current main, keep only the intended perps-controller bump, regenerate the lockfile, and apply the minimal smart-transactions type cast needed for the refreshed dependency graph to pass TypeScript. Constraint: Minimize the diff to main while keeping the perps controller update intact Rejected: Keep the older dependency baseline | produced missing geolocation and notification-services dependency-tree failures unrelated to the actual perps change Confidence: high Scope-risk: moderate Directive: Treat package.json on this branch as main baseline plus only the intentional perps-controller bump unless a new failing check proves another dependency change is required Tested: yarn install --mode=skip-build; yarn circular-deps:check; yarn lint:tsc; eslint on touched smart-transactions init file; targeted perps infrastructure/init Jest Not-tested: Full CI run after push
…ogic' into fix/tat-2699-ui-decimal-display-parity
Reapply the non-close in-progress toast guard after syncing the UI branch onto the refreshed controller base so submission failures still clear the stale in-progress toast correctly. Constraint: Preserve the successful UI recipe behavior while bringing the branch onto the latest non-UI base Rejected: Leave the stash-only local fix uncommitted | would lose the error-path correction on the updated branch Confidence: high Scope-risk: narrow Directive: After base syncs, re-check the non-close error path because it depends on both route-state navigation and local toast cleanup Tested: ESLint on touched code; live CDP recipe pr-41558-decimal-formatting (42/42) Not-tested: Full branch CI rerun after push
Contributor
Builds ready [802af3e]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
- Revert jest.config transformIgnorePatterns addition (keeps diff minimal vs main). - Apply yarn patch on @metamask/perps-controller 3.1.1 to expose ./formatters, ./calculations, and ./constants subpath exports. Temporary workaround — will be removed once the upstream fix lands in @metamask/perps-controller (tracked by TAT-2990).
Mirror mobile's usePerpsLiquidationPrice shape (lodash debounce with optional debounceMs). For market orders, entryPrice tracks currentPrice which updates per tick; without debouncing, perpsCalculateLiquidationPrice fires on every update. Defaults to 300ms so calls coalesce during active market conditions.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bf7c63e. Configure here.
Contributor
Builds ready [bf7c63e]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
…2990) @metamask/perps-controller's main entry statically imports @nktkas/hyperliquid which pulls ESM-only @noble/hashes/sha3.js. Jest's default transformIgnorePatterns skips node_modules, so any test that transitively renders perps UI fails with "SyntaxError: Cannot use import statement outside a module" — across 13 test suites spanning bridge, settings, notifications, account-overview, and more. Temporary: allow Babel to transpile the narrow ESM chain during tests. Revert once TAT-2990 lazy-loads the HyperLiquid SDK upstream so the main entry is CJS-safe again. Also reverts the short-lived yarn patch attempt on @metamask/perps-controller (patch protocol did not register cleanly through Yarn Berry's lockfile).
Contributor
Builds ready [c380fd5]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
- jest.integration.config.js: allowlist @metamask/perps-controller ESM chain (@nktkas/hyperliquid, @noble/hashes, @Scure, micro-*) mirroring the unit config fix. Required until the controller lazy-loads the HyperLiquid SDK upstream. - perps-market-detail-page.test.tsx: replace hand-rolled perps-controller mock with jest.requireActual spread so newly added exports (formatPerpsFiat, PRICE_RANGES_*) resolve automatically. - lavamoat/webpack/mv3/*/policy.json: whitelist Intl.NumberFormat for @metamask/perps-controller. Branch adds a new UI-side direct import of formatPerpsFiat via formatPerpsDisplayPrice.ts which pulls the controller into the MV3 UI bundle for the first time.
Contributor
Builds ready [dbf47a3]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Same fix as perps-market-detail-page.test.tsx (previous commit): the hand-rolled mock only stubbed PERPS_ERROR_CODES so the newly added formatPerpsFiat export was undefined, causing 34 tests to fail with "formatPerpsFiat is not a function" when the component renders formatPerpsDisplayPrice. Replace with requireActual spread. Validated: 161/161 across all 6 perps tests mocking the controller.
|
Contributor
Builds ready [50c32c3]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
7 tasks
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Description
This PR is stacked on top of #41558.
It keeps the perps UI decimal/display parity work isolated from the controller/base PR and aligns the key extension screens with current mobile behavior across market detail, order entry, position detail, reverse, remove-margin, close, and withdraw.
Temporary Jest workaround (TAT-2990)
This PR extends
jest.config.jsandjest.integration.config.jstransformIgnorePatternsto allow Babel to transpile@metamask/perps-controllerand its ESM-only transitive deps (@nktkas/hyperliquid,@noble/hashes,micro-eth-signer,micro-packed,@scure) during tests.Why: the controller's main entry statically imports
@nktkas/hyperliquidwhich imports@noble/hashes/sha3.js(ESM-only via@noble/hashes@2.0.1bumped by@nktkas/hyperliquid@0.30.3). Jest's defaulttransformIgnorePatternsskipsnode_modules, so 13 unit-test suites across bridge, settings, notifications, account-overview, design-system, etc. plus all 23 integration suites fail withSyntaxError: Cannot use import statement outside a moduleas soon as anything in this PR tightens the perps-controller import path.Scope: single well-commented block in each config, scoped to the known ESM chain. No other project behavior changes.
Exit plan: upstream fix is tracked in TAT-2990. Preferred solution is to lazy-load the HyperLiquid SDK in
HyperLiquidClientService(mirror the existingMYXProviderdynamic-import pattern) so the main entry is CJS-safe again. Once that ships and the extension bumps the controller version, bothtransformIgnorePatternsblocks are reverted.LavaMoat MV3 policy update
This PR adds
"Intl.NumberFormat": truefor@metamask/perps-controllerto all four MV3 policy flavors (main,beta,flask,experimental).Why: the new
ui/components/app/perps/utils/formatPerpsDisplayPrice.tsdirectly importsformatPerpsFiatandPRICE_RANGES_*from@metamask/perps-controller. This is the first UI-side direct import of the controller, so it now appears in the MV3 UI bundle as its own module entry.formatPerpsFiatusesIntl.NumberFormatinternally, and LavaMoat'slavamoat:autostep regenerates the policy to whitelist that global. CI's Check working tree step for MV3 enforces this — the policy must be committed or CI cannot go green.Changelog
CHANGELOG entry: null
Related issues
Fixes: N/A (stacked UI/display follow-up for TAT-2699 on top of #41558)
Related:
Manual testing steps
fix/tat-2699-fix-perps-decimal-logic.temp/agentic/recipes/teams/perps/recipes/pr-41558-decimal-formatting.json.temp/agentic/recipes/domains/perps/recipes/full-trade-lifecycle.json.scripts/perps/agentic/teams/perps/recipes/reference-decimal-key-screens.jsononmm-1.Validation Recipe
Extension full recipe.json
{ "title": "Full trade lifecycle — testnet, open ETH long, close, verify BTC market", "description": "End-to-end smoke test: ensures testnet, navigates to perps, opens a long position on ETH, closes it, then navigates to BTC market detail to verify multi-market navigation. Restores original network on teardown.", "validate": { "workflow": { "pre_conditions": ["wallet.unlocked", "perps.feature_enabled"], "setup": [ { "id": "save-original-network", "action": "eval_sync", "expression": "(function(){var m=stateHooks.store.getState().metamask;return JSON.stringify({network:m.isTestnet?'testnet':'mainnet',provider:m.activeProvider||'hyperliquid'})})()", "assert": { "all": [ { "operator": "length_gt", "field": "network", "value": 0 }, { "operator": "length_gt", "field": "provider", "value": 0 } ] }, "save_as": "original_network" } ], "teardown": [ { "id": "restore-network", "action": "call", "ref": "perps/ensure-perps-network", "params": { "network": "{{vars.original_network.network}}", "provider": "{{vars.original_network.provider}}" } }, { "id": "nav-perps-home", "action": "call", "ref": "perps/navigate-perps-tab" } ], "entry": "ensure-testnet", "nodes": { "ensure-testnet": { "action": "call", "ref": "perps/ensure-perps-network", "params": { "network": "testnet", "provider": "hyperliquid" }, "next": "nav-perps" }, "nav-perps": { "action": "call", "ref": "perps/navigate-perps-tab", "next": "screenshot-home" }, "screenshot-home": { "action": "screenshot", "filename": "01-perps-home", "next": "open-long-eth" }, "open-long-eth": { "action": "call", "ref": "perps/open-long-position", "params": { "symbol": "ETH", "side": "long", "amount": "10" }, "next": "screenshot-position" }, "screenshot-position": { "action": "screenshot", "filename": "02-eth-long-open", "next": "close-eth" }, "close-eth": { "action": "call", "ref": "perps/close-position", "params": { "symbol": "ETH", "percent": "100" }, "next": "screenshot-closed" }, "screenshot-closed": { "action": "screenshot", "filename": "03-eth-position-closed", "next": "nav-btc-market" }, "nav-btc-market": { "action": "call", "ref": "perps/navigate-to-market-detail", "params": { "symbol": "BTC" }, "next": "screenshot-btc" }, "screenshot-btc": { "action": "screenshot", "filename": "04-btc-market-detail", "next": "nav-back-perps" }, "nav-back-perps": { "action": "call", "ref": "perps/navigate-perps-tab", "next": "screenshot-final" }, "screenshot-final": { "action": "screenshot", "filename": "05-perps-home-final", "next": "done" }, "done": { "action": "end", "status": "pass", "message": "Full trade lifecycle completed: testnet setup, ETH open/close, BTC market navigation" } } } } }Screenshots/Recordings
Hosted asset folder:
https://github.com/abretonc7s/mm-extension-farm-artifacts/tree/main/reviews/41558/comparison-tableBefore
After
Validation status
Current review state:
temp/agentic/recipes/teams/perps/recipes/pr-41558-decimal-formatting.json: passed.temp/agentic/recipes/domains/perps/recipes/full-trade-lifecycle.json: passed.15/15).mm-1:scripts/perps/agentic/teams/perps/recipes/reference-decimal-key-screens.jsonpassed (71/71).Fresh parity read:
~5 minvs5 minutes)$0 USDCvs$0)Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches multiple perps trading surfaces and core calculation hooks (fees, liquidation price) and adds new background RPC calls/fallback behavior, which could affect displayed amounts and user decisions. Changes are mostly formatting/parity work but span several key flows (order entry, market detail, close/reverse/margin).
Overview
Perps UI now relies on
@metamask/perps-controllerformatting helpers (e.g.,formatPerpsFiat,formatPositionSize, funding/PnL formatters and price-range presets) across market detail, order entry, close/reverse, margin edit, TP/SL, and balance dropdown, shifting display output toward mobile parity (notably fewer forced trailing decimals) and adding severaldata-testids.Order calculations are updated to use controller-style size/margin computations and a new debounced
usePerpsLiquidationPricehook that callsperpsCalculateLiquidationPrice, with a local fallback formula if the RPC returns no value.usePerpsOrderFeesnow falls back to base fee rates (and computed fee amounts) on RPC failure/timeout instead of returningundefined.Tooling/build updates include a temporary Jest
transformIgnorePatternsexception to transpile the perps-controller’s ESM dependency chain during tests, and LavaMoat MV3 policy updates to allowIntl.NumberFormatfor@metamask/perps-controller.Reviewed by Cursor Bugbot for commit 50c32c3. Bugbot is set up for automated code reviews on this repo. Configure here.