chore(runway): cherry-pick fix: include hip3 dexes when building position map on init#24300
Conversation
…24293) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> After the HIP-3 refactor that replaced webData3 with individual clearinghouseState/openOrders subscriptions, HIP-3 market positions were not showing up on initial load. Additionally, when they did appear (e.g., after switching accounts), there was a ~1 second delay between crypto perps and HIP-3 positions appearing. **Root Cause** A race condition between subscription creation and DEX discovery: 1. HyperLiquidSubscriptionService initializes with empty `enabledDexs` subscribeToPositions() creates subscriptions using empty `enabledDexs` (main DEX only) 2. DEX discovery runs asynchronously via `buildAssetMapping()` 3. `updateFeatureFlags()` is called with discovered DEXs, but subscriptions already exist for main DEX only 4. HIP-3 positions never appear because no subscriptions were created for HIP-3 DEXs **Solution** 1. Promise-based DEX discovery wait Added `dexDiscoveryPromise` and `dexDiscoveryResolver `fields `createUserDataSubscription()` waits for DEX discovery when HIP-3 is enabled but `enabledDexs` is empty `updateFeatureFlags()` resolves the promise when DEXs are discovered 5-second timeout prevents indefinite waiting 2. Synchronized position subscription notifications Added `expectedDexs` and `initializedDexs` tracking sets `aggregateAndNotifySubscribers()` waits for ALL expected DEXs to send initial data before notifying Ensures crypto and HIP-3 positions appear simultaneously 3. Consistent position ordering Modified aggregation to always show main DEX (crypto perps) first, then HIP-3 positions **Error handling:** 1. Failed DEX subscriptions are removed from `expectedDexs` so they don't block notifications for other DEXs 2. Both `ensureClearinghouseStateSubscription` and `ensureOpenOrdersSubscription` mark DEXs as initialize Changes to `HyperLiquidSubscriptionService.ts`: - Added DEX discovery synchronization promise mechanism - Added expectedDexs/initializedDexs tracking for synchronized notifications - Added wait logic in createUserDataSubscription() for HIP-3 DEX discovery - Modified aggregateAndNotifySubscribers() to wait for all DEXs before notifying - Modified aggregation ordering to put main DEX first - Added initializedDexs.add() to both subscription callbacks - Added expectedDexs.delete() in catch blocks for failed subscriptions - Added cleanup for tracking state in cleanupSharedWebData3Subscription() - Updated tests to call updateFeatureFlags() to simulate DEX discovery before subscribing in HyperLiquidSubscriptionService.test.ts <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Fixes bug where hip3 positions were not loaded on initial PerpsContext load Fixes: Verified crypto and HIP-3 positions appear simultaneously on initial load Verified position ordering (crypto first, HIP-3 second) Verified account switching still works correctly Verified that TP/SL still works as intended https://github.com/user-attachments/assets/e70a47f5-f6e7-4b0b-931b-d8d666806511 - [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 - [x] 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. - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Improves HIP-3 multi-DEX handling and subscription lifecycle to deliver consistent, aggregated user data. > > - Introduces DEX discovery synchronization (`waitForDexDiscovery`) and resolves pending waits in `updateFeatureFlags` > - Tracks `expectedDexs`/`initializedDexs` to delay notifications until all DEXs send initial data; orders aggregation preserves main DEX first > - When HIP-3 is enabled, establishes per-DEX `clearinghouseState` and `openOrders` subscriptions upon feature-flag updates if user-data subscribers exist > - `subscribeTo...` waits for DEX discovery before creating HIP-3 subscriptions; error paths avoid blocking other DEX updates > - Aggregation and caches updated to merge per-DEX positions/orders/account; clearing `clearAll` resets DEX tracking > - Tests adjusted to simulate DEX discovery and verify restoration and TP/SL inclusion in orders > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 67f8a9a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
|
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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| initialized: Array.from(this.initializedDexs), | ||
| }); | ||
| return; // Don't notify yet - waiting for more DEXs | ||
| } |
There was a problem hiding this comment.
Missing timeout for DEX initialization blocks positions indefinitely
Medium Severity
The aggregateAndNotifySubscribers method waits for all expected DEXs to send initial data before notifying subscribers, but lacks a timeout mechanism. While the PR handles failed subscriptions by removing DEXs from expectedDexs in catch blocks, it doesn't handle the case where a subscription succeeds but the server never sends data (e.g., slow or unresponsive HIP-3 DEX). In this scenario, initializedDexs would never include that DEX, causing the function to return early indefinitely at line 1456. Users would see no positions at all, even from the responsive main DEX. The DEX discovery wait has a 5-second timeout, but this initialization wait does not.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsAnalysis SummaryChanged Files
Change NatureThe changes are focused on the HyperLiquid subscription service within the Perps (Perpetuals) module. Based on the git diff analysis: Key Changes in Service File:
Key Changes in Test File:
Impact AssessmentDirect Impact:
Files that import this service:
Risk Level Justification:
Test Tag SelectionSelected: SmokePerps
Not Selected:
Confidence85% - High confidence because:
|
| this.dexAccountCache.set(cacheKey, accountState); | ||
|
|
||
| // Mark this DEX as initialized (has sent first data) | ||
| this.initializedDexs.add(cacheKey); |
There was a problem hiding this comment.
DEX initialization tracking doesn't distinguish data types
Medium Severity
Both ensureClearinghouseStateSubscription and ensureOpenOrdersSubscription callbacks mark the same DEX key in initializedDexs. If all openOrders callbacks fire before any clearinghouseState callbacks (due to network timing), all DEXs get marked as "initialized" and aggregateAndNotifySubscribers proceeds—but dexPositionsCache remains empty since positions come from clearinghouseState. This causes position subscribers to receive an empty array initially, defeating the stated goal of ensuring "positions from all DEXs appear simultaneously." The initialization tracking conflates "received any data" with "received position data."
Additional Locations (1)
|
|
||
| // Clear DEX tracking for synchronized notifications | ||
| this.expectedDexs.clear(); | ||
| this.initializedDexs.clear(); |
There was a problem hiding this comment.
New state fields not cleared in clearAll() method
Low Severity
The PR adds new state fields (dexDiscoveryPromise, dexDiscoveryResolver, expectedDexs, initializedDexs) and adds cleanup for expectedDexs/initializedDexs in cleanupSharedWebData3Subscription(), but clearAll() doesn't clear any of these four fields. Additionally, dexDiscoveryPromise and dexDiscoveryResolver aren't cleared in either cleanup method. If clearAll() is called and the service is reused, stale DEX discovery state could cause subsequent subscriptions to skip the DEX discovery wait (using an already-settled promise) instead of waiting for fresh DEX discovery.
|
|
||
| // Mark this DEX as initialized (has sent first data) | ||
| this.initializedDexs.add(cacheKey); | ||
|
|
There was a problem hiding this comment.
Missing try/catch in new WebSocket subscription callbacks
Medium Severity
The new clearinghouseState and openOrders subscription callbacks lack try/catch error handling, unlike the existing webData2 callback which wraps its entire body in try/catch. If the WebSocket sends malformed data or if processing throws (e.g., data.clearinghouseState.assetPositions accessed when clearinghouseState is undefined, or data.orders passed to extractTPSLFromOrders when undefined), the exception would propagate unhandled. This could crash the subscription or leave the service in an inconsistent state. The outer try/catch (lines 1283, 1376) only catches subscription setup errors, not asynchronous callback execution errors.
Additional Locations (1)
|
metamaskbot
added
the
release-7.61.6
|
No release label on PR. Adding release label release-7.61.6 on PR, as PR was cherry-picked in branch 7.61.6. |
3 participants
Description
After the HIP-3 refactor that replaced webData3 with individual
clearinghouseState/openOrders subscriptions, HIP-3 market positions were
not showing up on initial load. Additionally, when they did appear
(e.g., after switching accounts), there was a ~1 second delay between
crypto perps and HIP-3 positions appearing.
Root Cause
A race condition between subscription creation and DEX discovery:
enabledDexssubscribeToPositions() creates subscriptions using empty
enabledDexs(main DEX only)
buildAssetMapping()updateFeatureFlags()is called with discovered DEXs, butsubscriptions already exist for main DEX only
for HIP-3 DEXs
Solution
Added
dexDiscoveryPromiseanddexDiscoveryResolverfieldscreateUserDataSubscription()waits for DEX discovery when HIP-3 isenabled but
enabledDexsis emptyupdateFeatureFlags()resolves the promise when DEXs are discovered5-second timeout prevents indefinite waiting
Added
expectedDexsandinitializedDexstracking setsaggregateAndNotifySubscribers()waits for ALL expected DEXs to sendinitial data before notifying
Ensures crypto and HIP-3 positions appear simultaneously
Modified aggregation to always show main DEX (crypto perps) first, then
HIP-3 positions
Error handling:
expectedDexsso theydon't block notifications for other DEXs
ensureClearinghouseStateSubscriptionandensureOpenOrdersSubscriptionmark DEXs as initializeChanges to
HyperLiquidSubscriptionService.ts:notifications
discovery
notifying
cleanupSharedWebData3Subscription()
before subscribing in HyperLiquidSubscriptionService.test.ts
Changelog
CHANGELOG entry: Fixes bug where hip3 positions were not loaded on
initial PerpsContext load
Related issues
Fixes:
Manual testing steps
Verified crypto and HIP-3 positions appear simultaneously on initial
load
Verified position ordering (crypto first, HIP-3 second)
Verified account switching still works correctly
Verified that TP/SL still works as intended
Screenshots/Recordings
Screen.Recording.2026-01-07.at.11.26.25.AM.mov
Pre-merge author checklist
Docs and MetaMask Mobile
Coding
Standards.
if applicable
guidelines).
Not required for external contributors.
Pre-merge reviewer checklist
app, test code being changed).
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Note
Ensures HIP-3 markets appear correctly and simultaneously on initial load by synchronizing DEX discovery and per-DEX subscriptions.
dexDiscoveryPromiseand resolve inupdateFeatureFlags(); delay subscription setup until DEXs are knownexpectedDexs/initializedDexsto hold notifications inaggregateAndNotifySubscribers()until all DEXs send initial data; order aggregation as main''first, then HIP-3 DEXsupdateFeatureFlags(), establishclearinghouseState/openOrderssubscriptions for newly discovered DEXs when user-data subscribers exist; remove failed DEXs fromexpectedDexssetDexMetaCache(),setDexAssetCtxsCache(), andgetDexAssetCtxsCache()for pre-fetched meta/ctxs reuse; clear tracking inclearAll()createUserDataSubscription()waits for discovery when HIP-3 is enabled; callbacks mark DEX initializedupdateFeatureFlags) and to validate TP/SL inclusion, restoration flows, and orderingWritten by Cursor Bugbot for commit 992009f. This will update automatically on new commits. Configure here.
be25b63