Skip to content

Commit dff0679

Browse files
chore(runway): cherry-pick fix(perps): fix stale data and missing price change after reconnection cp-7.70.1 (#27826)
- fix(perps): fix stale data and missing price change after reconnection (#27530) ## **Description** Two fixes for perps foreground reconnection: 1. **Stale data after background** — `connect()` returned early when `isConnected=true` (grace period kept state alive) without checking if the WebSocket was dead. Fixed by adding `ensureConnected()` that always forces disconnect + reconnect on foreground return. 2. **Price change "–%" persists after reconnect** — Prewarm called `subscribeToPrices()` without `includeMarketData`, so `assetCtxs` subscriptions (which provide `prevDayPx` for `percentChange24h`) were never re-established. Fixed by moving the `assetCtxs` subscription out of the `includeMarketData` guard in `subscribeToPrices()`. This is safe because `assetCtxs` is 1 subscription per DEX (2-3 total), not per-symbol. The expensive per-symbol `activeAssetCtx` subscriptions remain gated behind `includeMarketData`. ## **Changelog** CHANGELOG entry: Fixed stale perps data and missing 24h price change after returning from background ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Perps foreground reconnection Scenario: user returns after short background (grace period still active) Given user is on Perps screen with live data When user backgrounds app for 10s and returns Then data refreshes with live prices and positions And 24h price change % displays correctly (not "--%" ) Scenario: user returns after long background (grace period already fired) Given user is on Perps screen with live data When user backgrounds app for 60s and returns Then data refreshes with live prices and positions And 24h price change % displays correctly (not "--%" ) Scenario: initial mount unchanged Given user opens app fresh When user navigates to Perps Then connection establishes normally via connect() And 24h price change % displays correctly ``` ## **Screenshots/Recordings** ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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 - [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. ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches perps WebSocket lifecycle and subscription behavior; regressions could cause extra reconnects or missed/duplicated subscriptions, though changes are scoped and covered by updated tests. > > **Overview** > Fixes perps reconnection reliability by switching foreground handling from `connect()` to a new `PerpsConnectionManager.ensureConnected()` that **cancels any grace period, force-disconnects, resets ref-count, and reconnects**, deduplicating concurrent calls. > > Restores 24h % change after reconnection/prewarm by ensuring `HyperLiquidSubscriptionService.subscribeToPrices()` always establishes lightweight per-DEX `assetCtxs` subscriptions even when `includeMarketData` is false; price prewarm explicitly passes `includeMarketData: false` and documents the N² connection risk. > > Updates unit tests and architecture docs to reflect `ensureConnected()` usage and the new subscription expectations. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1549f85. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [1bf5d78](1bf5d78) Co-authored-by: abretonc7s <107169956+abretonc7s@users.noreply.github.com>
1 parent 0ae0e30 commit dff0679

9 files changed

Lines changed: 229 additions & 57 deletions

app/components/UI/Perps/providers/PerpsAlwaysOnProvider.test.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ jest.mock('../index', () => ({
4141
const mockUseSelector = useSelector as jest.MockedFunction<typeof useSelector>;
4242
const mockConnect = PerpsConnectionManager.connect as jest.Mock;
4343
const mockDisconnect = PerpsConnectionManager.disconnect as jest.Mock;
44+
const mockEnsureConnected = PerpsConnectionManager.ensureConnected as jest.Mock;
4445

4546
describe('PerpsAlwaysOnProvider', () => {
4647
let mockAppStateListener: ((state: string) => void) | null = null;
@@ -53,6 +54,7 @@ describe('PerpsAlwaysOnProvider', () => {
5354

5455
mockConnect.mockResolvedValue(undefined);
5556
mockDisconnect.mockResolvedValue(undefined);
57+
mockEnsureConnected.mockResolvedValue(undefined);
5658

5759
mockSubscriptionRemove = jest.fn();
5860
addEventListenerSpy = jest
@@ -167,15 +169,15 @@ describe('PerpsAlwaysOnProvider', () => {
167169
expect(mockDisconnect).toHaveBeenCalledTimes(1);
168170
});
169171

170-
it('calls connect after delay when app returns to foreground', () => {
172+
it('calls ensureConnected after delay when app returns to foreground', () => {
171173
render(
172174
<PerpsAlwaysOnProvider>
173175
<Text>child</Text>
174176
</PerpsAlwaysOnProvider>,
175177
);
176178

177179
// Clear the initial mount connect call
178-
mockConnect.mockClear();
180+
mockEnsureConnected.mockClear();
179181

180182
act(() => {
181183
mockAppStateListener?.('background');
@@ -185,13 +187,13 @@ describe('PerpsAlwaysOnProvider', () => {
185187
});
186188

187189
// Should not reconnect immediately — uses a timer delay
188-
expect(mockConnect).not.toHaveBeenCalled();
190+
expect(mockEnsureConnected).not.toHaveBeenCalled();
189191

190192
act(() => {
191193
jest.runAllTimers();
192194
});
193195

194-
expect(mockConnect).toHaveBeenCalledTimes(1);
196+
expect(mockEnsureConnected).toHaveBeenCalledTimes(1);
195197
});
196198

197199
it('cancels pending reconnect timer if app goes background before timer fires', () => {
@@ -201,7 +203,7 @@ describe('PerpsAlwaysOnProvider', () => {
201203
</PerpsAlwaysOnProvider>,
202204
);
203205

204-
mockConnect.mockClear();
206+
mockEnsureConnected.mockClear();
205207

206208
// Goes active — schedules reconnect timer
207209
act(() => {
@@ -217,8 +219,8 @@ describe('PerpsAlwaysOnProvider', () => {
217219
jest.runAllTimers();
218220
});
219221

220-
// connect should NOT have been called (timer was cancelled)
221-
expect(mockConnect).not.toHaveBeenCalled();
222+
// ensureConnected should NOT have been called (timer was cancelled)
223+
expect(mockEnsureConnected).not.toHaveBeenCalled();
222224
expect(mockDisconnect).toHaveBeenCalledTimes(1);
223225
});
224226

@@ -250,7 +252,7 @@ describe('PerpsAlwaysOnProvider', () => {
250252
</PerpsAlwaysOnProvider>,
251253
);
252254

253-
mockConnect.mockClear();
255+
mockEnsureConnected.mockClear();
254256
mockDisconnect.mockClear();
255257

256258
// Pull-down: active → inactive → active
@@ -266,7 +268,7 @@ describe('PerpsAlwaysOnProvider', () => {
266268
});
267269

268270
expect(mockDisconnect).toHaveBeenCalledTimes(1);
269-
expect(mockConnect).toHaveBeenCalledTimes(1);
271+
expect(mockEnsureConnected).toHaveBeenCalledTimes(1);
270272
});
271273

272274
it('calls disconnect and removes AppState subscription on unmount', () => {

app/components/UI/Perps/providers/PerpsAlwaysOnProvider.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export const PerpsAlwaysOnProvider: React.FC<{ children: React.ReactNode }> = ({
5858
} else if (nextState === 'active') {
5959
// Small delay to allow system to stabilize after background
6060
reconnectTimer = setTimeout(() => {
61-
PerpsConnectionManager.connect().catch((err) => {
61+
PerpsConnectionManager.ensureConnected().catch((err) => {
6262
Logger.error(ensureError(err, 'PerpsAlwaysOnProvider.reconnect'), {
6363
tags: { feature: PERPS_CONSTANTS.FeatureName },
6464
context: {

app/components/UI/Perps/providers/PerpsStreamManager.test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1046,9 +1046,10 @@ describe('PerpsStreamManager', () => {
10461046
await Promise.resolve();
10471047
});
10481048

1049-
// Now subscribeToPrices should have been called
1049+
// Now subscribeToPrices should have been called without includeMarketData
10501050
expect(mockSubscribeToPrices).toHaveBeenCalledWith({
10511051
symbols: ['BTC-PERP', 'ETH-PERP'],
1052+
includeMarketData: false,
10521053
callback: expect.any(Function),
10531054
});
10541055

@@ -1237,6 +1238,7 @@ describe('PerpsStreamManager', () => {
12371238
expect(mockSubscribeToPrices).toHaveBeenCalledTimes(1);
12381239
expect(mockSubscribeToPrices).toHaveBeenCalledWith({
12391240
symbols: ['ETH-PERP'],
1241+
includeMarketData: false,
12401242
callback: expect.any(Function),
12411243
});
12421244

app/components/UI/Perps/providers/PerpsStreamManager.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,13 @@ class PriceStreamChannel extends StreamChannel<Record<string, PriceUpdate>> {
520520
},
521521
);
522522

523-
// Subscribe to all market prices
523+
// WARNING: Do NOT set includeMarketData: true here. It triggers
524+
// per-symbol activeAssetCtx subscriptions (N symbols × N DEXs = N²
525+
// WebSocket connections). assetCtxs (1 per DEX) is always established
526+
// by the subscription service regardless of this flag.
524527
const unsub = controller.subscribeToPrices({
525528
symbols: this.allMarketSymbols,
529+
includeMarketData: false,
526530
callback: (updates: PriceUpdate[]) => {
527531
const priceMap: Record<string, PriceUpdate> = {};
528532
updates.forEach((update) => {

app/components/UI/Perps/services/PerpsConnectionManager.test.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ const resetManager = (manager: unknown) => {
123123
initPromise: Promise<void> | null;
124124
disconnectPromise: Promise<void> | null;
125125
pendingReconnectPromise: Promise<void> | null;
126+
ensureConnectedPromise: Promise<void> | null;
126127
unsubscribeFromStore: (() => void) | null;
127128
previousAddress: string | undefined;
128129
previousPerpsNetwork: 'mainnet' | 'testnet' | undefined;
@@ -157,6 +158,7 @@ const resetManager = (manager: unknown) => {
157158
m.initPromise = null;
158159
m.disconnectPromise = null;
159160
m.pendingReconnectPromise = null;
161+
m.ensureConnectedPromise = null;
160162
m.unsubscribeFromStore = null;
161163
m.previousAddress = undefined;
162164
m.previousPerpsNetwork = undefined;
@@ -1275,6 +1277,116 @@ describe('PerpsConnectionManager', () => {
12751277
});
12761278
});
12771279

1280+
describe('ensureConnected', () => {
1281+
it('cancels grace period, disconnects, and reconnects when connected', async () => {
1282+
// Establish connection first
1283+
await PerpsConnectionManager.connect();
1284+
expect(PerpsConnectionManager.getConnectionState().isConnected).toBe(
1285+
true,
1286+
);
1287+
1288+
// Simulate the state after AlwaysOnProvider calls disconnect() which
1289+
// decrements refCount to 0 and starts grace period
1290+
const m = PerpsConnectionManager as unknown as {
1291+
isInGracePeriod: boolean;
1292+
gracePeriodTimer: number | null;
1293+
connectionRefCount: number;
1294+
};
1295+
m.connectionRefCount = 0;
1296+
m.isInGracePeriod = true;
1297+
m.gracePeriodTimer = 123;
1298+
1299+
// Clear mocks to track ensureConnected calls
1300+
(Engine.context.PerpsController.disconnect as jest.Mock).mockClear();
1301+
(Engine.context.PerpsController.init as jest.Mock).mockClear();
1302+
1303+
await PerpsConnectionManager.ensureConnected();
1304+
1305+
// Grace period should be cancelled
1306+
expect(m.isInGracePeriod).toBe(false);
1307+
1308+
// Should have disconnected then reconnected
1309+
expect(Engine.context.PerpsController.disconnect).toHaveBeenCalled();
1310+
expect(Engine.context.PerpsController.init).toHaveBeenCalled();
1311+
expect(PerpsConnectionManager.getConnectionState().isConnected).toBe(
1312+
true,
1313+
);
1314+
});
1315+
1316+
it('reconnects after long background when grace period already fired', async () => {
1317+
// Establish connection, then simulate grace period already fired
1318+
await PerpsConnectionManager.connect();
1319+
1320+
// Simulate performActualDisconnection already ran (grace period fired)
1321+
const m = PerpsConnectionManager as unknown as {
1322+
isConnected: boolean;
1323+
isInitialized: boolean;
1324+
hasPreloaded: boolean;
1325+
isPreloading: boolean;
1326+
connectionRefCount: number;
1327+
};
1328+
m.isConnected = false;
1329+
m.isInitialized = false;
1330+
m.hasPreloaded = false;
1331+
m.isPreloading = false;
1332+
// Grace period fired → refCount was already 0 when disconnect ran
1333+
m.connectionRefCount = 0;
1334+
1335+
(Engine.context.PerpsController.init as jest.Mock).mockClear();
1336+
1337+
await PerpsConnectionManager.ensureConnected();
1338+
1339+
// Should have reconnected (connect() runs full init path since isConnected=false)
1340+
expect(Engine.context.PerpsController.init).toHaveBeenCalled();
1341+
expect(PerpsConnectionManager.getConnectionState().isConnected).toBe(
1342+
true,
1343+
);
1344+
});
1345+
1346+
it('connects when not previously connected', async () => {
1347+
// Manager starts in disconnected state (from resetManager in beforeEach)
1348+
(Engine.context.PerpsController.init as jest.Mock).mockClear();
1349+
1350+
await PerpsConnectionManager.ensureConnected();
1351+
1352+
expect(Engine.context.PerpsController.init).toHaveBeenCalled();
1353+
expect(PerpsConnectionManager.getConnectionState().isConnected).toBe(
1354+
true,
1355+
);
1356+
});
1357+
1358+
it('resets connectionRefCount to 1 after ensureConnected', async () => {
1359+
// Simulate refCount drift: connect() twice so refCount = 2
1360+
await PerpsConnectionManager.connect();
1361+
await PerpsConnectionManager.connect();
1362+
1363+
const m = PerpsConnectionManager as unknown as {
1364+
connectionRefCount: number;
1365+
};
1366+
expect(m.connectionRefCount).toBe(2);
1367+
1368+
await PerpsConnectionManager.ensureConnected();
1369+
1370+
// ensureConnected resets to 0 then connect() brings it to 1
1371+
expect(m.connectionRefCount).toBe(1);
1372+
});
1373+
1374+
it('deduplicates concurrent ensureConnected calls', async () => {
1375+
(Engine.context.PerpsController.init as jest.Mock).mockClear();
1376+
1377+
// Fire two calls concurrently
1378+
const [result1, result2] = await Promise.all([
1379+
PerpsConnectionManager.ensureConnected(),
1380+
PerpsConnectionManager.ensureConnected(),
1381+
]);
1382+
1383+
expect(result1).toBeUndefined();
1384+
expect(result2).toBeUndefined();
1385+
// init should only be called once since second call reuses the promise
1386+
expect(Engine.context.PerpsController.init).toHaveBeenCalledTimes(1);
1387+
});
1388+
});
1389+
12781390
describe('getActiveProviderName', () => {
12791391
it('returns activeProvider from PerpsController state', () => {
12801392
// Arrange

app/components/UI/Perps/services/PerpsConnectionManager.ts

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class PerpsConnectionManagerClass {
5050
private connectionRefCount = 0;
5151
private initPromise: Promise<void> | null = null;
5252
private disconnectPromise: Promise<void> | null = null;
53+
private ensureConnectedPromise: Promise<void> | null = null;
5354
private hasPreloaded = false;
5455
private isPreloading = false;
5556
private prewarmCleanups: (() => void)[] = [];
@@ -470,9 +471,12 @@ class PerpsConnectionManagerClass {
470471
}
471472

472473
/**
473-
* Perform the actual disconnection after grace period expires
474+
* Perform the actual disconnection after grace period expires.
475+
* @param options.force - Bypass refCount guard (used by ensureConnected).
474476
*/
475-
private async performActualDisconnection(): Promise<void> {
477+
private async performActualDisconnection(
478+
options: { force?: boolean } = {},
479+
): Promise<void> {
476480
DevLogger.log(
477481
`PerpsConnectionManager: Grace period expired, performing disconnection (refCount: ${this.connectionRefCount})`,
478482
);
@@ -481,8 +485,8 @@ class PerpsConnectionManagerClass {
481485
this.gracePeriodTimer = null;
482486
this.isInGracePeriod = false;
483487

484-
// Only disconnect if we still have no references
485-
if (this.connectionRefCount <= 0) {
488+
// Only disconnect if we still have no references (unless forced)
489+
if (options.force || this.connectionRefCount <= 0) {
486490
if (this.isConnected || this.isInitialized) {
487491
// Track that we're disconnecting
488492
this.isDisconnecting = true;
@@ -1061,6 +1065,54 @@ class PerpsConnectionManagerClass {
10611065
}
10621066
}
10631067

1068+
/**
1069+
* Called on foreground return. Always forces a full reconnect.
1070+
*
1071+
* The grace period timer handles battery savings (disconnects after 30s
1072+
* in background). But regardless of whether the timer fired or not,
1073+
* we cannot trust the WebSocket state after backgrounding:
1074+
* - Grace period fired: already disconnected, need fresh connect
1075+
* - Grace period didn't fire (iOS suspends JS timers): WebSocket
1076+
* likely dead but isConnected still true, need fresh connect
1077+
*
1078+
* By always doing disconnect + connect, behavior is identical on both
1079+
* iOS and Android regardless of how long the app was backgrounded.
1080+
*/
1081+
async ensureConnected(): Promise<void> {
1082+
// Guard against concurrent calls (e.g. rapid foreground transitions)
1083+
if (this.ensureConnectedPromise) {
1084+
return this.ensureConnectedPromise;
1085+
}
1086+
1087+
this.ensureConnectedPromise = this.performEnsureConnected();
1088+
try {
1089+
await this.ensureConnectedPromise;
1090+
} finally {
1091+
this.ensureConnectedPromise = null;
1092+
}
1093+
}
1094+
1095+
private async performEnsureConnected(): Promise<void> {
1096+
// Cancel grace period if still pending — we're taking over
1097+
this.cancelGracePeriod();
1098+
1099+
// Force clean state so connect() runs the full init → ping → preload path.
1100+
// Uses force: true to bypass the refCount guard — ensureConnected must
1101+
// always tear down, regardless of how many components hold references.
1102+
if (this.isConnected || this.isInitialized) {
1103+
await this.performActualDisconnection({ force: true });
1104+
}
1105+
1106+
// Reset refCount so connect() brings it to exactly 1.
1107+
// Without this, repeated background/foreground cycles would drift
1108+
// refCount upward (1→2→3…), eventually preventing grace-period
1109+
// disconnects from firing (they require refCount ≤ 0).
1110+
this.connectionRefCount = 0;
1111+
1112+
// Full reconnect: init → ping → preload
1113+
await this.connect();
1114+
}
1115+
10641116
async disconnect(): Promise<void> {
10651117
this.connectionRefCount--;
10661118
DevLogger.log(

app/controllers/perps/services/HyperLiquidSubscriptionService.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3275,8 +3275,8 @@ describe('HyperLiquidSubscriptionService', () => {
32753275

32763276
await jest.runAllTimersAsync();
32773277

3278-
// Should not call meta/metaAndAssetCtxs when market data not requested
3279-
expect(mockInfoClient.meta).not.toHaveBeenCalled();
3278+
// assetCtxs subscription is always established (lightweight, 1 per DEX)
3279+
// so meta may be called for the assetCtxs mapping, but metaAndAssetCtxs should not
32803280
expect(mockInfoClient.metaAndAssetCtxs).not.toHaveBeenCalled();
32813281

32823282
unsubscribe();

0 commit comments

Comments
 (0)