Skip to content

Commit 93c5524

Browse files
authored
feat(perps): sync controller code with extension (#28509)
## **Description** Makes `@metamask/perps-controller` safe to consume from the MetaMask extension without pulling `@myx-trade/sdk` into the static webpack bundle or violating the LavaMoat policy. Mobile owns the source-of-truth for the perps controller package via `scripts/perps/validate-core-sync.sh`, so the fix is made in mobile and then rsync'd to core on a matching branch. Companion core PR: MetaMask/core#8398 — supersedes MetaMask/core#8374. ### What changed 1. **`app/controllers/perps/PerpsController.ts`** — add `/* webpackIgnore: true */` magic comment to the `MYXProvider` dynamic `import()` so webpack (extension) skips static resolution of the intentionally-unshipped module. Metro (mobile) ignores the magic comment, so runtime behavior on mobile is unchanged. 2. **`app/controllers/perps/index.ts`** — remove 8 MYX adapter functions from the public barrel (`adaptMarketFromMYX`, `adaptPriceFromMYX`, `adaptMarketDataFromMYX`, `filterMYXExclusiveMarkets`, `isOverlappingMarket`, `buildPoolSymbolMap`, `buildSymbolPoolsMap`, `extractSymbolFromPoolId`). These are still used internally by `MYXProvider`, which imports them via relative path `../utils/myxAdapter`. No runtime change in mobile — the functions are still available to MYXProvider. 3. **`app/controllers/perps/utils/index.ts`** — drop the `export * from './myxAdapter'` barrel re-export so the utils index no longer transitively re-exports MYX symbols. 4. **Drift fix — `perpsConnectionAttemptContext`** — moved from `app/util/perpsConnectionAttemptContext.ts` into `app/controllers/perps/utils/perpsConnectionAttemptContext.ts` and exported from `@metamask/perps-controller` so: - `HyperLiquidClientService` (inside the perps package) imports it via a relative path that stays inside the synced directory — required for the core sync to build. - `PerpsConnectionManager` (outside the perps package) imports it from `@metamask/perps-controller` — satisfies the `no-restricted-imports` rule for code outside `app/controllers/perps/`. 5. **`.eslintrc.js` — align mobile perps override with core's base rules.** Added `BinaryExpression[operator='in']`, `WithStatement`, and `SequenceExpression` selectors to the existing `no-restricted-syntax` rule in the `app/controllers/perps/**` override. Mobile's override was missing these, which meant `'x' in y` type-guards passed mobile lint silently and then landed in core as new `no-restricted-syntax` suppressions at sync time. The override now mirrors what `@metamask/eslint-config` enforces in core, catching the violations at source. 6. **Replace `'x' in y` with `hasProperty(y, 'x')` across 8 perps-controller files** — `HyperLiquidProvider.ts`, `HyperLiquidSubscriptionService.ts`, `TradingService.ts`, `marketDataTransform.ts`, `hyperLiquidAdapter.ts`, `types/index.ts`, `types/transactionTypes.ts`, `utils/errorUtils.ts`. Uses `hasProperty` from `@metamask/utils` (the idiomatic replacement documented in core's eslint config). 24 of the 28 occurrences were converted cleanly; the remaining 4 are kept as `'in'` behind `/* eslint-disable no-restricted-syntax */` blocks because `hasProperty` narrows the KEY but not the discriminated-union branch — losing access to `.filled`, `.resting`, `.oid`, `.totalSz` on the HyperLiquid SDK types and losing `keyof typeof HYPERLIQUID_ASSET_CONFIGS` narrowing. Each disable comment is documented with the narrowing rationale. 7. **`scripts/perps/validate-core-sync.sh` — hardened preflight + per-file suppression delta check.** - **Preflight freshness check** (`chore(perps-sync): hard-fail sync script when core main has drift`): the previous preflight only compared the core working tree against `.sync-state.json.lastSyncedCoreCommit`, so it could not detect that someone else had merged a competing perps-controller change into `origin/main` while this branch was in review. That gap let [#8398](MetaMask/core#8398) land in a conflicting state with [#8333](MetaMask/core#8333). The new check at the top of `step_conflict_check` fetches `origin/main`, runs `git rev-list --count HEAD..origin/main -- packages/perps-controller/`, and **hard-fails** (not warns) with the offending commit list + a merge suggestion if any such commits exist. Gracefully degrades to `WARN` if offline or if `origin/main` is missing. - **Per-file/per-rule suppression delta check** (`chore(perps-sync): hard-fail on per-file suppression delta increase`): step 6 now snapshots the per-file/per-rule suppression counts from `eslint-suppressions.json` BEFORE running `--fix` / `--suppress-all` / `--prune-suppressions`, then diffs against the post-run counts and hard-fails if any (file, rule) pair's count INCREASED. Reducing counts (mobile fixes removing previously-suppressed violations) is always allowed. Increases mean the current sync is introducing NEW violations that would land in core as fresh suppressions and must be fixed at source. The script prints every offending file+rule pair and points at `hasProperty()` from `@metamask/utils` as the canonical replacement. Replaces the old "WARN if count > 20" heuristic. This is the canonical local detection point for the problem that *"it should have been detected locally!"* — a violation now fails the sync script at step 6 BEFORE the core PR is opened. ### Not included (intentional) An earlier draft of this PR also declared `MetaMetricsController:trackEvent` in `PerpsControllerAllowedActions` to let the extension drop a type cast. That change was **reverted** because mobile has no `MetaMetricsController` — it uses a `MetaMetrics` singleton (`app/core/Analytics/MetaMetrics.ts`) — so adding the action to the allowed-actions union forced `@metamask/messenger`'s parent type to narrow to `never` and broke `app/core/Engine/messengers/perps-controller-messenger/index.ts`. The extension keeps its existing `as unknown as PackagePerpsControllerMessenger` cast workaround until both host apps share a real `MetaMetricsController`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: [TAT-2863](https://consensyssoftware.atlassian.net/browse/TAT-2863) Related: MetaMask/core#8374 (superseded by companion core PR #8398) ## **Manual testing steps** ```gherkin Feature: Perps controller behavior unchanged after MYX export cleanup Scenario: User opens the Perps tab on testnet Given the wallet is unlocked and funded on HyperLiquid testnet When user navigates to the Perps tab Then the markets list loads And the HyperLiquid provider is selected by default Scenario: User opens and closes a BTC long position Given the wallet has testnet funds When user opens a $11 BTC long market order Then a position appears in the positions list When user closes the position at 100% Then the position is removed from the positions list ``` Automated verification (all passing locally): - `yarn lint` on all touched files — clean - `NODE_OPTIONS='--max-old-space-size=8192' npx tsc --noEmit` — clean (full, no incremental cache) - `yarn jest app/controllers/perps/PerpsController.test.ts app/controllers/perps/services/HyperLiquidSubscriptionService.test.ts app/controllers/perps/services/TradingService.test.ts` — 412 passing - `bash scripts/perps/validate-core-sync.sh --core-path …/core` — all 9 steps PASS. Suppression count drops from 30 → **6** after the lint cleanup. ## **Screenshots/Recordings** N/A — this PR is a bundler / import-graph + lint cleanup with zero UI impact. Mobile runtime behavior is unchanged because: - The `webpackIgnore` magic comment is extension/webpack-only; Metro ignores it. - The 8 removed exports are still used internally by `MYXProvider` via relative import. - `perpsConnectionAttemptContext` keeps its module-level state, just at a new file path. - `hasProperty(obj, key)` is runtime-equivalent to `key in obj` for plain objects (both delegate to `Object.prototype.hasOwnProperty` semantics via the prototype chain). ## **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 (existing tests untouched; new location for moved test file is covered by the same suite) - [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** > Medium risk because it changes perps controller module boundaries/exports and touches trading/provider code paths (type-guard refactors and a few scoped lint suppressions), plus alters sync script behavior that can block releases if misconfigured. > > **Overview** > Improves extension-safe consumption of `@metamask/perps-controller` by ensuring the optional `MYXProvider` stays excluded from webpack’s static bundle and by removing MYX adapter re-exports from the public perps barrels. > > Aligns mobile’s perps linting with core by restricting the `in` operator (and a couple of other syntaxes) and refactors multiple perps files to use `hasProperty()` instead of `'key' in obj`, keeping a few documented `in` usages behind targeted `no-restricted-syntax` disables where TypeScript narrowing is required. > > Moves/exports `perpsConnectionAttemptContext` under the perps utils surface so in-package code imports remain sync-safe and out-of-package callers use the package export, and hardens `scripts/perps/validate-core-sync.sh` with an `origin/main` freshness check plus a per-file/per-rule eslint suppression delta hard-fail (including prettier formatting of `eslint-suppressions.json`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1f90a9b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 905043c commit 93c5524

18 files changed

Lines changed: 175 additions & 41 deletions

.eslintrc.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,22 @@ module.exports = {
287287
message:
288288
'Use ES private class fields (#field) instead of TypeScript private keyword.',
289289
},
290+
// Mirror @metamask/eslint-config base rule — prevents `'x' in obj`
291+
// type-guards that would land in core as new `no-restricted-syntax`
292+
// suppressions. Use `hasProperty()` from `@metamask/utils` instead.
293+
{
294+
selector: "BinaryExpression[operator='in']",
295+
message:
296+
'The "in" operator is not allowed. Use `hasProperty()` from `@metamask/utils` instead.',
297+
},
298+
{
299+
selector: 'WithStatement',
300+
message: 'With statements are not allowed',
301+
},
302+
{
303+
selector: 'SequenceExpression',
304+
message: 'Sequence expressions are not allowed',
305+
},
290306
],
291307
'id-denylist': [
292308
'error',

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
PERPS_ERROR_CODES,
2626
wait,
2727
TradingReadinessCache,
28+
withPerpsConnectionAttemptContext,
2829
type ReconnectOptions,
2930
} from '@metamask/perps-controller';
3031
import { getStreamManagerInstance } from '../providers/PerpsStreamManager';
@@ -34,7 +35,6 @@ import {
3435
} from '../selectors/perpsController';
3536
import { selectHip3ConfigVersion } from '../selectors/featureFlags';
3637
import { ensureError } from '../../../../util/errorUtils';
37-
import { withPerpsConnectionAttemptContext } from '../../../../util/perpsConnectionAttemptContext';
3838
import { PERPS_CONNECTION_SOURCE } from '../constants/perpsConfig';
3939

4040
interface ConnectOptions {

app/controllers/perps/PerpsController.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1694,7 +1694,9 @@ export class PerpsController extends BaseController<
16941694
// IMPORTANT: Must use import() — NOT require() — for core/extension tree-shaking.
16951695
// require() is synchronous and bundlers include it in the main bundle.
16961696
// import() enables true code splitting so MYX is excluded when not enabled.
1697-
this.#myxRegistrationPromise = import('./providers/MYXProvider')
1697+
this.#myxRegistrationPromise = import(
1698+
/* webpackIgnore: true */ './providers/MYXProvider'
1699+
)
16981700
.then(({ MYXProvider }) => {
16991701
this.registerMYXProvider(MYXProvider);
17001702
return undefined;

app/controllers/perps/index.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -464,15 +464,10 @@ export {
464464
} from './utils';
465465
export type { HyperLiquidMarketData } from './utils';
466466
export {
467-
adaptMarketFromMYX,
468-
adaptPriceFromMYX,
469-
adaptMarketDataFromMYX,
470-
filterMYXExclusiveMarkets,
471-
isOverlappingMarket,
472-
buildPoolSymbolMap,
473-
buildSymbolPoolsMap,
474-
extractSymbolFromPoolId,
475-
} from './utils';
467+
getPerpsConnectionAttemptContext,
468+
withPerpsConnectionAttemptContext,
469+
} from './utils/perpsConnectionAttemptContext';
470+
export type { PerpsConnectionAttemptContext } from './utils/perpsConnectionAttemptContext';
476471
export {
477472
MAX_MARKET_PATTERN_LENGTH,
478473
escapeRegex,

app/controllers/perps/providers/HyperLiquidProvider.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CaipAccountId } from '@metamask/utils';
1+
import { CaipAccountId, hasProperty } from '@metamask/utils';
22
import type { Hex } from '@metamask/utils';
33
import { v4 as uuidv4 } from 'uuid';
44

@@ -1968,14 +1968,19 @@ export class HyperLiquidProvider implements PerpsProvider {
19681968

19691969
// Check order status
19701970
const status = result.response?.data?.statuses?.[0];
1971-
if (isStatusObject(status) && 'error' in status) {
1971+
if (isStatusObject(status) && hasProperty(status, 'error')) {
19721972
return { success: false, error: String(status.error) };
19731973
}
19741974

1975+
// Note: `in` narrows the HyperLiquid SDK discriminated union to the
1976+
// branch that has `filled`; `hasProperty` only narrows the key and
1977+
// types `status.filled` as `unknown`, which loses access to `.totalSz`.
1978+
/* eslint-disable no-restricted-syntax */
19751979
const filledSize =
19761980
isStatusObject(status) && 'filled' in status
19771981
? parseFloat(status.filled?.totalSz ?? '0')
19781982
: 0;
1983+
/* eslint-enable no-restricted-syntax */
19791984

19801985
this.#deps.debugLogger.log(
19811986
'HyperLiquidProvider: USDC→USDH swap completed',
@@ -3426,10 +3431,15 @@ export class HyperLiquidProvider implements PerpsProvider {
34263431
}
34273432

34283433
const status = result.response?.data?.statuses?.[0];
3434+
// Note: `in` narrows the HyperLiquid SDK discriminated union to the
3435+
// branch that has the property; `hasProperty` types the property as
3436+
// `unknown`, losing downstream access to `.oid`, `.totalSz`, `.avgPx`.
3437+
/* eslint-disable no-restricted-syntax */
34293438
const restingOrder =
34303439
isStatusObject(status) && 'resting' in status ? status.resting : null;
34313440
const filledOrder =
34323441
isStatusObject(status) && 'filled' in status ? status.filled : null;
3442+
/* eslint-enable no-restricted-syntax */
34333443

34343444
// Success - auto-rebalance excess funds
34353445
if (isHip3Order && transferInfo && dexName) {
@@ -4143,7 +4153,8 @@ export class HyperLiquidProvider implements PerpsProvider {
41434153
const { statuses } = result.response.data;
41444154
const successCount = statuses.filter(
41454155
(stat) =>
4146-
isStatusObject(stat) && ('filled' in stat || 'resting' in stat),
4156+
isStatusObject(stat) &&
4157+
(hasProperty(stat, 'filled') || hasProperty(stat, 'resting')),
41474158
).length;
41484159
const failureCount = statuses.length - successCount;
41494160

@@ -4153,7 +4164,7 @@ export class HyperLiquidProvider implements PerpsProvider {
41534164
const status = statuses[i];
41544165
const isSuccess =
41554166
isStatusObject(status) &&
4156-
('filled' in status || 'resting' in status);
4167+
(hasProperty(status, 'filled') || hasProperty(status, 'resting'));
41574168

41584169
if (isSuccess && hip3Transfers[i]) {
41594170
const { sourceDex, freedMargin } = hip3Transfers[i];
@@ -4179,9 +4190,9 @@ export class HyperLiquidProvider implements PerpsProvider {
41794190
symbol: positionsToClose[index].symbol,
41804191
success:
41814192
isStatusObject(status) &&
4182-
('filled' in status || 'resting' in status),
4193+
(hasProperty(status, 'filled') || hasProperty(status, 'resting')),
41834194
error:
4184-
isStatusObject(status) && 'error' in status
4195+
isStatusObject(status) && hasProperty(status, 'error')
41854196
? String(status.error)
41864197
: undefined,
41874198
})),
@@ -5998,7 +6009,7 @@ export class HyperLiquidProvider implements PerpsProvider {
59986009
// Extract HIP-3 DEX names (filter out null which is main DEX)
59996010
const hip3DexNames: string[] = [];
60006011
allDexs.forEach((dex) => {
6001-
if (dex !== null && 'name' in dex) {
6012+
if (dex !== null && hasProperty(dex, 'name')) {
60026013
hip3DexNames.push(dex.name);
60036014
}
60046015
});

app/controllers/perps/services/HyperLiquidClientService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
WebSocketTransport,
88
} from '@nktkas/hyperliquid';
99

10-
import { getPerpsConnectionAttemptContext } from '../../../util/perpsConnectionAttemptContext';
1110
import { CandlePeriod, calculateCandleCount } from '../constants/chartConfig';
1211
import { HYPERLIQUID_TRANSPORT_CONFIG } from '../constants/hyperLiquidConfig';
1312
import { PERPS_CONSTANTS } from '../constants/perpsConfig';
@@ -20,6 +19,7 @@ import type {
2019
import type { HyperLiquidNetwork } from '../types/config';
2120
import type { CandleData } from '../types/perps-types';
2221
import { ensureError } from '../utils/errorUtils';
22+
import { getPerpsConnectionAttemptContext } from '../utils/perpsConnectionAttemptContext';
2323

2424
/**
2525
* Maximum number of reconnection attempts before giving up.

app/controllers/perps/services/HyperLiquidSubscriptionService.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { CaipAccountId } from '@metamask/utils';
2+
import { hasProperty } from '@metamask/utils';
23
import type {
34
ISubscription,
45
AllMidsWsEvent,
@@ -2583,9 +2584,9 @@ export class HyperLiquidSubscriptionService {
25832584
const isPerpsContext = (
25842585
event: ActiveAssetCtxWsEvent | ActiveSpotAssetCtxWsEvent,
25852586
): event is ActiveAssetCtxWsEvent =>
2586-
'funding' in event.ctx &&
2587-
'openInterest' in event.ctx &&
2588-
'oraclePx' in event.ctx;
2587+
hasProperty(event.ctx, 'funding') &&
2588+
hasProperty(event.ctx, 'openInterest') &&
2589+
hasProperty(event.ctx, 'oraclePx');
25892590

25902591
const { ctx } = data;
25912592

@@ -2904,7 +2905,7 @@ export class HyperLiquidSubscriptionService {
29042905
// Use cached meta to map ctxs array indices to symbols (no REST API call!)
29052906
validatedMeta.universe.forEach((asset, index) => {
29062907
const ctx = data.ctxs[index];
2907-
if (ctx && 'funding' in ctx) {
2908+
if (ctx && hasProperty(ctx, 'funding')) {
29082909
// This is a perps context
29092910
const ctxPrice = ctx.midPx ?? ctx.markPx;
29102911
const openInterestUSD = calculateOpenInterestUSD(

app/controllers/perps/services/TradingService.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,6 @@ export class TradingService {
15161516

15171517
this.#deps.debugLogger.log('[closePositions] Batch method check', {
15181518
providerType: provider.protocolId,
1519-
hasBatchMethod: 'closePositions' in provider,
15201519
providerKeys: Object.keys(provider).filter((key) =>
15211520
key.includes('close'),
15221521
),

app/controllers/perps/types/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { hasProperty } from '@metamask/utils';
12
import type {
23
CaipAccountId,
34
CaipChainId,
@@ -1643,8 +1644,8 @@ export function isVersionGatedFeatureFlag(
16431644
return (
16441645
typeof value === 'object' &&
16451646
value !== null &&
1646-
'enabled' in value &&
1647-
'minimumVersion' in value &&
1647+
hasProperty(value, 'enabled') &&
1648+
hasProperty(value, 'minimumVersion') &&
16481649
typeof (value as { enabled: unknown }).enabled === 'boolean' &&
16491650
typeof (value as { minimumVersion: unknown }).minimumVersion === 'string'
16501651
);

app/controllers/perps/types/transactionTypes.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Shared transaction types for Perps deposits and withdrawals
33
* Provides a unified structure while maintaining separate use cases
44
*/
5+
import { hasProperty } from '@metamask/utils';
56

67
/**
78
* Base type with core properties shared between all transaction results
@@ -62,7 +63,7 @@ export type TransactionRecord = {
6263
export function isTransactionRecord(
6364
result: LastTransactionResult | TransactionRecord,
6465
): result is TransactionRecord {
65-
return 'id' in result && 'status' in result;
66+
return hasProperty(result, 'id') && hasProperty(result, 'status');
6667
}
6768

6869
/**
@@ -74,5 +75,5 @@ export function isTransactionRecord(
7475
export function isLastTransactionResult(
7576
result: LastTransactionResult | TransactionRecord,
7677
): result is LastTransactionResult {
77-
return !('id' in result) || !('status' in result);
78+
return !hasProperty(result, 'id') || !hasProperty(result, 'status');
7879
}

0 commit comments

Comments
 (0)