Conversation
|
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. |
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/perps (8 files, +3178 -62)
|
Builds ready [35e5639]
UI Startup Metrics (1400 ± 107 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [3c428d4]
UI Startup Metrics (1427 ± 102 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
geositta
left a comment
There was a problem hiding this comment.
A few bugs I noticed that might help with linting.
|
|
||
| // Preset percentage options for quick selection | ||
| const TP_PRESETS = [10, 25, 50, 100]; | ||
| const SL_PRESETS = [10, 25, 50, 75]; |
There was a problem hiding this comment.
Short TP preset can produce zero price
Medium Severity
TP_PRESETS includes 100, but percentToPrice computes short take-profit as entryPrice * (1 - percent/100). For short positions, the +100% preset produces 0.00, creating an invalid TP price and incorrect auto-close behavior.
Additional Locations (1)
Builds ready [263fbbb]
UI Startup Metrics (1443 ± 112 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| const freshPositions = await controller.getPositions({ | ||
| skipCache: true, | ||
| }); | ||
| streamManager.pushPositionsWithOverrides(freshPositions); |
There was a problem hiding this comment.
Margin update can be falsely reported failed
Medium Severity
handleSaveMargin treats getPositions refresh failures as if updateMargin failed. After a successful updateMargin, an error during getPositions falls into the same catch, sets marginError, and skips onToggle. This can leave users seeing a failure even though margin was already changed.
Additional Locations (1)
Builds ready [5f5632f]
UI Startup Metrics (1384 ± 108 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| const newPrice = percentToPrice(numValue, false); | ||
| onStopLossPriceChange(newPrice); | ||
| } | ||
| } |
There was a problem hiding this comment.
Percent input breaks on comma locales
Medium Severity
tpPercent/slPercent are rendered with locale-aware formatNumber, but percent input parsing only accepts dot-decimal via /^-?\d*\.?\d*$/ and parseFloat. In comma-decimal locales, displayed values like 10,0 cannot be edited reliably and can be parsed incorrectly, causing wrong TP/SL price updates.
Additional Locations (2)
Builds ready [f4d031b]
UI Startup Metrics (1393 ± 89 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
geositta
left a comment
There was a problem hiding this comment.
Found two issues that I will create separate tasks for.
| // Format price for display (with locale-aware formatting) | ||
| const formatPrice = useCallback( | ||
| (value: number): string => { | ||
| return formatNumber(value, { |
There was a problem hiding this comment.
One thing I noticed formatPrice (which wraps formatNumber from useFormatters) is locale-aware via Intl.NumberFormat, so in comma decimal locales it can produce values like 1.234,56. Since this formatted string is stored directly as the form value, the blur handler's parseFloat(limitPrice.replace(/,/gu, '')) would turn that into 1.234.56, and parseFloat silently truncates at the second dot -> 1.234. The value degrades with each focus/blur cycle.
Worth revisiting in separate issue to store the canonical value as a plain dot-decimal string (e.g. numValue.toFixed(2)) and only use formatPrice for display purposes, or parse using a locale aware approach that round-trips cleanly.
Consider this non-blocking for landing behind the feature flag since it only manifests in non-English locales, but I will create a ticket to avoid incorrect limit prices for those users.
Builds ready [f551d27]
UI Startup Metrics (1413 ± 102 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|


Description
This PR implements a mock provider infrastructure for the Perps feature to enable UI development and integration without depending on the preview
@metamask/perps-controllerpackage. This allows us to merge Perps UI components into the main branch while keeping the actual controller integration isolated in an upstream branch.The upstream branch #40078 is rebased to this one so that the controller integration can be isolated to it's individual branch independent of UI.
Background
The Perps feature requires integration with
@metamask/perps-controller, which is currently a preview package that cannot be merged into the main branch. To unblock UI development and allow incremental merges, we need a way to develop and test Perps UI components without the actual controller dependency.Approach
This PR introduces a mock provider pattern that mirrors the real controller/provider API but returns static
mock data. This approach provides several benefits:
stabilization
branch
later requires no UI code changes, only alias update.
Changes Made
1. Mock Infrastructure Pattern
Established a consistent mocking pattern across the codebase:
ui/{hooks,providers}/perps/[module]/
├── index.ts (re-exports from index.mock.ts)
└── index.mock.ts (mock implementation)
This pattern ensures:
Import paths remain unchanged when switching from mock to real implementation
Clear separation between mock and real code
Easy to identify and remove mocks when ready (or to alternatively keep them for unit/e2e integration)
Migration Path
Imports of
@metamask/perps-controllerare currently aliased to the mock implementation. Once the production package is publishhed, we can remove this alias in the upstream branch and do the following:ui/providers/perps/index.tsfrom./index.mockto real implementationsui/hooks/perps/stream/index.tsfrom./index.mockto real implementationsui/providers/perps/PerpsStreamManager/index.tsfrom./index.mockto real implementation.mock.tsfilesAlternatively, these files can be kept and altered for a robust unit test and e2e suite.
Testing
Manual Testing
The Perps UI should work with mock data:
What Won't Work (Expected)
Since these are mocks returning static/empty data:
Benefits
branch
.mock.tsfilesRelated PRs
perps/poc-controller-integration-minimalChangelog
CHANGELOG entry: Adds initial Perps UI
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Moderate risk due to new build/test module aliasing for
@metamask/perps-controllerand a large mock surface area that can mask integration issues when swapped for the real controller.Overview
Introduces a local mock implementation of
@metamask/perps-controllerunderui/__mocks__/perps/perps-controller, and wires it in via webpack alias, JestmoduleNameMapper, and TypeScriptpathsto unblock Perps UI development without the real package;depcheckis updated to ignore the dependency.Expands the Perps UI surface: adds an
EditMarginExpandablecomponent (with unit tests) that callsupdateMargin, refreshes positions via the stream manager, and shows liquidation/risk info computed byusePerpsMarginCalculations/marginUtils. Order cards are updated to display Long/Short labeling, computed USD order value formatting, and click handling expectations; Perps constants gain HIP-3 market typing/mappings and chart config renamesChartDurationtoTimeDuration.Updates i18n strings (adds new perps copy/tooltips and refactors sort/filter keys) and adjusts mock perps data structures/types to align with the controller-style types used by the new UI.
Written by Cursor Bugbot for commit f551d27. This will update automatically on new commits. Configure here.