feat(perps): sync PerpsController implementation for npm publishing#7941
Open
abretonc7s wants to merge 16 commits intomainfrom
Open
feat(perps): sync PerpsController implementation for npm publishing#7941abretonc7s wants to merge 16 commits intomainfrom
abretonc7s wants to merge 16 commits intomainfrom
Conversation
Add all required dependencies to perps-controller package.json (account-tree-controller, bridge-controller, keyring-controller, network-controller, transaction-controller, etc.) and corresponding tsconfig.build.json project references so the package builds correctly when source files are synced from mobile.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning MetaMask internal reviewing guidelines:
Ignoring alerts on:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
7 tasks
Synced from mobile PR #26110: - toggleTestnet: check InitializationState.Failed after init() and rollback isTestnet on failure (matching switchProvider pattern) - depositWithConfirmation: replace never-resolving promise with Promise.resolve(transactionMeta.id) when placeOrder=true
Hoist currentDepositId before try block so it is accessible in the outer catch. When a pre-submission error occurs (e.g. missing networkClientId), the deposit request is now marked as 'failed' instead of staying permanently 'pending' in state.
Prevent standalone preload from leaking WebSocket providers by caching the HyperLiquidProvider instance across standalone calls and cleaning it up at lifecycle boundaries (init, disconnect, toggleTestnet, etc.). Reorder switchProvider() to check "already active" before validating the providers map, so it returns a no-op success before init().
Add uuidv4() fallback for currentDepositId which TypeScript cannot narrow inside the update() callback after the variable was hoisted to let binding with string | undefined type.
The base tsconfig.json was missing project references that tsconfig.build.json already had, causing TS2345 errors for imports like @metamask/account-tree-controller during type checking.
Jest exits with code 1 when no test files exist. Add a minimal placeholder test in tests/ (outside src/ so the Mobile sync script does not delete it) and scope collectCoverageFrom to that file only, preventing 0% coverage failures on the synced source.
…cription Register all 34 PerpsControllerActions via registerMethodActionHandlers() so inter-controller communication works through the messenger in core. Store the RemoteFeatureFlagController:stateChange subscription handle and clean it up in disconnect() to prevent leaks.
… and account switch - Remove feature-flag unsubscribe from disconnect() so geo-blocking and HIP-3 flag changes keep propagating after reconnect cycles - Add deposit request lifecycle tracking for the deposit+order flow so requests transition from pending to completed/failed/cancelled - Clear cached user data when switching to a non-EVM account group to prevent stale positions/orders from the previous EVM account
Contributor
Author
|
@SocketSecurity ignore npm/@nktkas/hyperliquid@0.30.3 |
Contributor
Author
|
@SocketSecurity ignore npm/@inquirer/external-editor@2.0.3 |
Contributor
Author
|
@SocketSecurity ignore npm/@myx-trade/sdk@0.1.265 |
Contributor
Author
|
@SocketSecurity ignore npm/@noble/curves@2.0.1 |
Contributor
Author
|
@SocketSecurity ignore npm/ox@0.12.1 |
Contributor
Author
|
@SocketSecurity ignore npm/viem@2.45.3 |
Contributor
Author
|
@SocketSecurity ignore npm/@scure/bip39@1.6.0 |
Contributor
Author
|
@SocketSecurity ignore npm/rxjs@7.8.2 |
Contributor
Author
|
@SocketSecurity ignore npm/wretch@2.11.1 |
Add a type import and use it in the test to satisfy both import-x/unambiguous (requires ES module syntax) and @typescript-eslint/no-unused-vars.
Member
|
The lint job seems to be failing (you need to run Also:
I thought we had discussed how this was a bad idea, and that we should migrate the controller fully to core instead? I'm not following what the plan is here. |
packages/perps-controller/src/PerpsController-method-action-types.ts
Outdated
Show resolved
Hide resolved
github-merge-queue bot
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Feb 20, 2026
## **Description** Defensive hardening for edge cases in `PerpsController` surfaced by static analysis on Core PR [#7941](MetaMask/core#7941). None of these bugs are currently triggerable in production — the app works correctly today — but all represent latent issues that could bite under future changes or unusual network conditions. ### What changed **1. `toggleTestnet()` — false success on silent init failure** `performInitialization()` catches errors internally and sets `InitializationState.Failed` instead of throwing. `toggleTestnet()` was not checking for this, so it could return `{ success: true }` even when the network switch failed. This patch: - Adds the same `InitializationState.Failed` check after `await this.init()` - Rolls back `isTestnet` to its previous value on failure **2. `depositWithConfirmation()` — never-resolving promise when `placeOrder=true`** The `placeOrder` path created `new Promise(() => {})` — a promise that can never be GC'd and would hang any consumer who awaits it. Replaced with `Promise.resolve(transactionMeta.id)` for proper fire-and-forget semantics. **3. `depositWithConfirmation()` — failed deposits remain permanently pending** `depositWithConfirmation` adds a pending entry to `state.depositRequests` before validating `networkClientId`. If the validation throws, the deposit request stays `status: 'pending'` forever. This patch marks the deposit request as `failed` in the outer catch when a pre-submission error occurs. **4. Feature flag listener lost after disconnect** `disconnect()` unsubscribed `RemoteFeatureFlagController:stateChange` but no reconnect path re-subscribed. After disconnect → reconnect, geo-blocking and HIP-3 flag changes stopped propagating. The feature-flag subscription is a controller-lifetime concern (not session-lifetime), so we removed the unsubscribe from `disconnect()` entirely — the subscription now lives for the full controller lifecycle. **5. `depositWithConfirmation()` — deposit+order request stays pending forever** When `placeOrder=true`, the `if (!placeOrder)` guard skips the `.then()/.catch()` lifecycle block that transitions the deposit request to `completed`/`failed`. No other handler exists for this path. Added an `else if` branch that attaches lifecycle tracking to `addResult.result` (the real transaction promise) for the deposit+order flow. **6. Non-EVM account switch leaves stale cache** The account-change handler only cleared cached user data when `currentAddress` is truthy. Switching to an account group with no EVM account (e.g., Bitcoin-only) skipped cache clearing, leaving stale positions/orders visible. Restructured the condition so cache is always cleared when the account group changes, with preload guarded separately behind `if (currentAddress)`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: bugbot findings from Core PR [#7941](MetaMask/core#7941) ## **Manual testing steps** These are defensive fixes for edge cases not reachable through normal UI flows. Verification is via unit tests. ```gherkin Feature: Perps network toggle resilience Scenario: toggleTestnet reports accurate status when init fails silently Given user is on mainnet with perps initialized When user toggles to testnet and initialization fails internally Then toggleTestnet returns { success: false } And isTestnet is rolled back to its previous value Feature: Perps deposit promise contract Scenario: depositWithConfirmation result promise resolves when placeOrder is true Given user initiates a deposit with placeOrder=true When the transaction is submitted Then the result promise resolves with the transaction ID Feature: Perps deposit request lifecycle Scenario: deposit request is marked failed on pre-submission error Given user initiates a deposit and the deposit request is added as pending When the networkClientId lookup fails Then the deposit request status is updated to 'failed' Scenario: deposit+order request transitions from pending Given user initiates a deposit with placeOrder=true When the transaction completes or fails Then the deposit request status is updated to 'completed' or 'failed' Feature: Feature flag subscription resilience Scenario: geo-blocking flags propagate after disconnect/reconnect Given perps controller is initialized with feature flag subscription When disconnect() is called and controller reconnects Then feature flag changes still propagate correctly Feature: Account switch cache clearing Scenario: switching to non-EVM account clears stale perps cache Given user has cached perps data from an EVM account When user switches to a Bitcoin-only account group Then cached positions, orders, and account state are cleared ``` ## **Screenshots/Recordings** N/A — no UI changes, logic-only hardening. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
…cy injection Synced from metamask-mobile-delta. Cross-controller access now goes through minimal interfaces defined in PerpsPlatformDependencies rather than direct package imports. Feature controller deps (network, keyring, transaction, authentication, remote-feature-flag, account-tree, rewards) removed from package.json. Remaining deps are stable utilities only.
| limit, | ||
| endTime, | ||
| context: this.#createServiceContext('fetchHistoricalCandles'), | ||
| }); |
There was a problem hiding this comment.
Deposit request tracking silently broken when ID undefined
High Severity
When #depositService.prepareTransaction() returns undefined for currentDepositId, the deposit request gets a fresh UUID via currentDepositId ?? uuidv4() as its id, but this generated UUID is never assigned back to currentDepositId. All subsequent .find() calls that search req.id === currentDepositId compare against undefined, so the request is never found. Deposit lifecycle updates (success, failure, cancellation) are silently lost, leaving the request permanently stuck as 'pending' in persisted state.
Additional Locations (2)
| state.depositInProgress = false; | ||
| state.lastDepositTransactionId = null; | ||
| // Don't set lastDepositResult - no toast needed | ||
| }); |
There was a problem hiding this comment.
Cancelled deposits stay pending in persisted state forever
Medium Severity
When a user cancels a deposit in the non-placeOrder flow, the result.catch handler clears depositInProgress and lastDepositTransactionId but does not update the corresponding deposit request's status from 'pending' to 'cancelled'. Since depositRequests has persist: true, these orphaned pending requests accumulate in persisted storage and can never be resolved through normal UI flows, only through the manual clearPendingTransactionRequests reset action.
| limitPrice?: string; // Limit price (for limit orders) | ||
| orderType?: OrderType; // Market vs limit | ||
| timestamp: number; // When the config was saved (for expiration check) | ||
| }; |
There was a problem hiding this comment.
State type missing selectedPaymentToken in pendingConfig
Medium Severity
The pendingConfig shape in PerpsControllerState does not include selectedPaymentToken, yet savePendingTradeConfiguration spreads a config object containing selectedPaymentToken into pendingConfig, and getPendingTradeConfiguration declares selectedPaymentToken in its return type. The field is stored and retrieved at runtime but is invisible to the type system, meaning consumers relying on the state type will not see it, and type-safe state serialization or validation could silently drop it.
Additional Locations (1)
grvgoel81
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Feb 23, 2026
## **Description** Defensive hardening for edge cases in `PerpsController` surfaced by static analysis on Core PR [#7941](MetaMask/core#7941). None of these bugs are currently triggerable in production — the app works correctly today — but all represent latent issues that could bite under future changes or unusual network conditions. ### What changed **1. `toggleTestnet()` — false success on silent init failure** `performInitialization()` catches errors internally and sets `InitializationState.Failed` instead of throwing. `toggleTestnet()` was not checking for this, so it could return `{ success: true }` even when the network switch failed. This patch: - Adds the same `InitializationState.Failed` check after `await this.init()` - Rolls back `isTestnet` to its previous value on failure **2. `depositWithConfirmation()` — never-resolving promise when `placeOrder=true`** The `placeOrder` path created `new Promise(() => {})` — a promise that can never be GC'd and would hang any consumer who awaits it. Replaced with `Promise.resolve(transactionMeta.id)` for proper fire-and-forget semantics. **3. `depositWithConfirmation()` — failed deposits remain permanently pending** `depositWithConfirmation` adds a pending entry to `state.depositRequests` before validating `networkClientId`. If the validation throws, the deposit request stays `status: 'pending'` forever. This patch marks the deposit request as `failed` in the outer catch when a pre-submission error occurs. **4. Feature flag listener lost after disconnect** `disconnect()` unsubscribed `RemoteFeatureFlagController:stateChange` but no reconnect path re-subscribed. After disconnect → reconnect, geo-blocking and HIP-3 flag changes stopped propagating. The feature-flag subscription is a controller-lifetime concern (not session-lifetime), so we removed the unsubscribe from `disconnect()` entirely — the subscription now lives for the full controller lifecycle. **5. `depositWithConfirmation()` — deposit+order request stays pending forever** When `placeOrder=true`, the `if (!placeOrder)` guard skips the `.then()/.catch()` lifecycle block that transitions the deposit request to `completed`/`failed`. No other handler exists for this path. Added an `else if` branch that attaches lifecycle tracking to `addResult.result` (the real transaction promise) for the deposit+order flow. **6. Non-EVM account switch leaves stale cache** The account-change handler only cleared cached user data when `currentAddress` is truthy. Switching to an account group with no EVM account (e.g., Bitcoin-only) skipped cache clearing, leaving stale positions/orders visible. Restructured the condition so cache is always cleared when the account group changes, with preload guarded separately behind `if (currentAddress)`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: bugbot findings from Core PR [#7941](MetaMask/core#7941) ## **Manual testing steps** These are defensive fixes for edge cases not reachable through normal UI flows. Verification is via unit tests. ```gherkin Feature: Perps network toggle resilience Scenario: toggleTestnet reports accurate status when init fails silently Given user is on mainnet with perps initialized When user toggles to testnet and initialization fails internally Then toggleTestnet returns { success: false } And isTestnet is rolled back to its previous value Feature: Perps deposit promise contract Scenario: depositWithConfirmation result promise resolves when placeOrder is true Given user initiates a deposit with placeOrder=true When the transaction is submitted Then the result promise resolves with the transaction ID Feature: Perps deposit request lifecycle Scenario: deposit request is marked failed on pre-submission error Given user initiates a deposit and the deposit request is added as pending When the networkClientId lookup fails Then the deposit request status is updated to 'failed' Scenario: deposit+order request transitions from pending Given user initiates a deposit with placeOrder=true When the transaction completes or fails Then the deposit request status is updated to 'completed' or 'failed' Feature: Feature flag subscription resilience Scenario: geo-blocking flags propagate after disconnect/reconnect Given perps controller is initialized with feature flag subscription When disconnect() is called and controller reconnects Then feature flag changes still propagate correctly Feature: Account switch cache clearing Scenario: switching to non-EVM account clears stale perps cache Given user has cached perps data from an EVM account When user switches to a Bitcoin-only account group Then cached positions, orders, and account state are cleared ``` ## **Screenshots/Recordings** N/A — no UI changes, logic-only hardening. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
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
2 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.
Explanation
Syncs `PerpsController` from Mobile into Core as `@metamask/perps-controller`, updated with controller isolation via platform dependency injection.
What changed in the latest sync (metamask-mobile#26394): Cross-controller access has been refactored from direct package imports to minimal interfaces defined by `PerpsPlatformDependencies`. Feature controller dependencies (network, keyring, transaction, authentication, remote-feature-flag, account-tree, rewards) have been removed from `package.json`. The remaining deps are stable utilities (`@metamask/base-controller`, `@metamask/controller-utils`, `@metamask/utils`, `@metamask/abi-utils`, `@metamask/keyring-api`, `@metamask/messenger`) plus the trading SDKs. This directly addresses Mark Stacey's concern from the ADR-42 review: PerpsController no longer creates obstruction for other teams making cross-package changes in Core, since it pins no feature controller versions.
Approach: Mobile remains the source of truth. A sync script copies controller source to this package. The question of whether Core or an independent repo is the right publishing location is still being resolved with the team — see position doc. This PR keeps the Core path viable. The sync script is a transitional mechanism that goes away when Mobile folds into Core as `apps/mobile`.
Key components:
CI / Testing: A minimal placeholder test (`tests/placeholder.test.ts`) is included to satisfy Core's CI requirement. It lives outside `src/` so the sync script (`rsync --delete` on `src/`) does not remove it. Full unit tests remain in Mobile alongside the source of truth and will be migrated when development moves fully to Core.
References
Checklist