-
-
Notifications
You must be signed in to change notification settings - Fork 266
feat(perps): perps controller migration from mobile #7749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Complete migration of PerpsController implementation from MetaMask Mobile to enable cross-platform sharing of Perps (Perpetual Futures) functionality. This includes: - PerpsController (~3,000 lines) with full trading functionality - 8 state selectors for UI integration - Comprehensive TypeScript type definitions - 18 utility modules for calculations, formatting, validation - 8 service modules (Trading, MarketData, Eligibility, etc.) - HyperLiquidProvider with full protocol integration - AggregatedPerpsProvider for multi-provider support - Platform services for HyperLiquid client, subscriptions, wallet - Test infrastructure with mocks and 40 unit tests The package uses dependency injection via PerpsPlatformDependencies interface to remain platform-agnostic, allowing Mobile and Extension to provide their own implementations while sharing core business logic.
Add 17 test files migrated from metamask-mobile covering: - Amount conversion, margin, PnL, and position calculations - Order book grouping and order utilities - Market data transformation and sorting - HyperLiquid adapter and validation - TP/SL validation and string parsing utilities Add MIGRATION.md documenting migration status and coverage priorities. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution MetaMask internal reviewing guidelines:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
| }); | ||
|
|
||
| return { success: true, providerId }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switchProvider doesn't update the active provider instance
High Severity
The switchProvider method updates state.activeProvider but doesn't update activeProviderInstance, which is what getActiveProvider() returns. After calling switchProvider('aggregated'), the state shows 'aggregated' but all operations still route through the original provider instance. The method returns { success: true } misleadingly, since the switch doesn't actually take effect until a full disconnect/reinit cycle. This causes a mismatch between reported state and actual behavior.
Additional Locations (1)
| }); | ||
| } | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deposit request orphaned on early error in flow
Medium Severity
In depositWithConfirmation, if an error occurs after the deposit request is added to state.depositRequests (line 1581) but before the .then()/.catch() handlers are set up (line 1629), the outer catch block doesn't update the deposit request status to 'failed'. This happens when findNetworkClientIdForChain returns null (line 1587) or submit() throws synchronously. The deposit request remains permanently orphaned with status: 'pending'. Additionally, line 1729's comment is incorrect - lastDepositResult is not set in the outer catch for non-cancellation errors.
Explanation
This PR migrates the complete
PerpsControllerimplementation from MetaMask Mobile to the Core monorepo as@metamask/perps-controller.Background
The Perps (Perpetual Futures) feature was initially developed in MetaMask Mobile. To enable sharing this functionality across platforms (Mobile, Extension) and maintain a single source of truth, we are migrating the controller logic to the Core monorepo.
What's Included
Core Controller (~3,000 lines)
PerpsController.ts- Main controller extendingBaseControllerwith full trading functionalityselectors.ts- 8 state selectors for UI integrationTypes & Constants
types/- Comprehensive TypeScript type definitions for all Perps operationsconstants/- Configuration (perpsConfig, hyperLiquidConfig, errorCodes, eventNames)Services (8 modules)
TradingService- Order placement, cancellation, position managementMarketDataService- Market info, prices, order bookAccountService- Account state managementDepositService- Deposit flow handlingEligibilityService- User eligibility checksFeatureFlagConfigurationService- Remote feature flag integrationRewardsIntegrationService- MetaMask rewards integrationDataLakeService- Analytics data collectionProviders
HyperLiquidProvider- Full HyperLiquid protocol integration (~7,000 lines)AggregatedPerpsProvider- Multi-provider aggregation layerProviderRouter- Routing logic for multi-provider supportPlatform Services
HyperLiquidClientService- HTTP client for HyperLiquid APIHyperLiquidSubscriptionService- WebSocket subscription managementHyperLiquidWalletService- Wallet signing operationsUtilities (18 modules)
Architecture
The package uses dependency injection via
PerpsPlatformDependenciesinterface to remain platform-agnostic:This allows Mobile and Extension to provide their own implementations of these dependencies while sharing the core business logic.
ESLint Compliance
Most files pass Core ESLint rules. The following files have ESLint rules temporarily disabled due to the volume of code migrated:
PerpsController.tsHyperLiquidProvider.tsHyperLiquidSubscriptionService.tsThese suppressions allow the migration to proceed while maintaining functionality. They can be addressed incrementally in follow-up PRs.
Test Coverage
@nktkas/hyperliquidSDK (ESM module)serviceMocks.ts)providerMocks.ts)Coverage thresholds are currently set to 0% to allow incremental test migration. Full test coverage will be added in follow-up PRs.
References
Checklist
Note: This is a new package, so there are no breaking changes for existing consumers.
Note
Introduces a new
@metamask/perps-controllerpackage migrated from Mobile, providing platform-agnostic perps trading via dependency injection.PerpsController(~3k LOC) with DI, HIP-3 support, connection handling, and provider routing (directHyperLiquidProviderandAggregatedPerpsProviderwithSubscriptionMultiplexer)constants/,types/, selectors, and utilities for fees, validation, market data, and performance metrics@nktkas/hyperliquid, lowered coverage thresholds) and test/mocks scaffolding (~40 tests)Written by Cursor Bugbot for commit dede853. This will update automatically on new commits. Configure here.