diff --git a/README.md b/README.md index bf2f4335a3..a87c5c0c7f 100644 --- a/README.md +++ b/README.md @@ -481,6 +481,7 @@ linkStyle default opacity:0.5 perps_controller --> controller_utils; perps_controller --> messenger; perps_controller --> account_tree_controller; + perps_controller --> authenticated_user_storage; perps_controller --> geolocation_controller; perps_controller --> keyring_controller; perps_controller --> network_controller; diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 5af48982df..2fa4a0659a 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Sync `watchlistMarkets` with `AuthenticatedUserStorageService` so the watchlist is persisted server-side per authenticated user account ([#9010](https://github.com/MetaMask/core/pull/9010)) +- `toggleWatchlistMarket` now performs an optimistic local-state update followed by an async AUS read-merge-write; on failure the local state is reverted. +- On `init()`, `state.watchlistMarkets` is hydrated from AUS (source of truth). If no remote watchlist exists yet for the active exchange, any existing local markets are migrated to AUS in a one-time push. +- When unauthenticated, or when the active provider is not mapped to an AUS exchange key (e.g. `'aggregated'`), the controller falls back to local-only state without surfacing errors to callers. +- `toggleWatchlistMarket` return type changed from `void` to `Promise` to allow callers to await the remote write. +- Add `resolveWatchlistExchangeKey(activeProvider)` helper that maps a `PerpsActiveProviderMode` to the corresponding `PerpsWatchlistMarkets` exchange key, returning `null` for unsupported modes ([#9010](https://github.com/MetaMask/core/pull/9010)) + +### Fixed + +- Fix `#syncWatchlistFromRemote` to use exchange-key presence instead of symbol count when deciding whether to hydrate from AUS, so an intentionally cleared remote watchlist is honored rather than overwritten by stale local favorites ([#9010](https://github.com/MetaMask/core/pull/9010)) + ## [8.3.0] ### Added diff --git a/packages/perps-controller/package.json b/packages/perps-controller/package.json index fa551442f1..9f119bc67d 100644 --- a/packages/perps-controller/package.json +++ b/packages/perps-controller/package.json @@ -109,6 +109,7 @@ }, "devDependencies": { "@metamask/account-tree-controller": "^7.5.3", + "@metamask/authenticated-user-storage": "^2.1.0", "@metamask/auto-changelog": "^6.1.0", "@metamask/geolocation-controller": "^0.1.3", "@metamask/keyring-controller": "^27.1.0", diff --git a/packages/perps-controller/src/PerpsController-method-action-types.ts b/packages/perps-controller/src/PerpsController-method-action-types.ts index 1351266616..e1880e9f24 100644 --- a/packages/perps-controller/src/PerpsController-method-action-types.ts +++ b/packages/perps-controller/src/PerpsController-method-action-types.ts @@ -976,8 +976,17 @@ export type PerpsControllerSaveOrderBookGroupingAction = { }; /** - * Toggle watchlist status for a market - * Watchlist markets are stored per network (testnet/mainnet) + * Toggle watchlist status for a market. + * + * Updates local state immediately (optimistic UI) and then syncs the new + * watchlist to AuthenticatedUserStorageService. If the remote write fails, + * the local state is reverted so it stays consistent with AUS. + * + * When the user is unauthenticated, or the active provider is not yet + * supported by the AUS schema, the controller continues operating with + * local-persisted state only — no error is surfaced to the caller. + * + * Watchlist markets are stored per network (testnet/mainnet). * * @param symbol - The trading pair symbol. */ diff --git a/packages/perps-controller/src/PerpsController.ts b/packages/perps-controller/src/PerpsController.ts index 19fcf28418..150feec05e 100644 --- a/packages/perps-controller/src/PerpsController.ts +++ b/packages/perps-controller/src/PerpsController.ts @@ -1,3 +1,7 @@ +import type { + NotificationPreferences, + PerpsWatchlistMarkets, +} from '@metamask/authenticated-user-storage'; import { BaseController, ControllerGetStateAction, @@ -153,6 +157,30 @@ export function firstNonEmpty(...vals: (string | undefined)[]): string { ); } +/** + * Maps an active provider mode to the corresponding exchange key used in the + * AUS {@link PerpsWatchlistMarkets} schema. + * + * Returns `null` for modes that are not yet represented in the AUS schema + * (e.g. `'aggregated'`), which signals callers to skip remote sync and fall + * back to local state only. Add new entries here as additional DEX providers + * gain AUS watchlist support. + * + * @param activeProvider - The current active provider mode from controller state. + * @returns The matching `PerpsWatchlistMarkets` key, or `null` if unsupported. + */ +export function resolveWatchlistExchangeKey( + activeProvider: PerpsActiveProviderMode, +): keyof PerpsWatchlistMarkets | null { + const map: Partial< + Record + > = { + hyperliquid: 'hyperliquid', + myx: 'myx', + }; + return map[activeProvider] ?? null; +} + /** * Resolves MYX auth config from provider credentials, handling * testnet/mainnet fallback logic. @@ -912,6 +940,21 @@ export class PerpsController extends BaseController< #eligibilityCheckDeferred: boolean; + /** + * Serial promise queue for all AUS watchlist operations (hydration and + * individual toggles). Chaining every operation onto this field ensures + * that: + * + * - A toggle that fires immediately after init() always runs *after* the + * init hydration finishes (Bug 3). + * - Concurrent toggles are serialised so the last PUT reflects all changes + * rather than racing with each other (Bug 4). + * + * Errors from individual operations are swallowed inside the queue so that + * a failed operation does not stall subsequent ones. + */ + #ausQueue: Promise = Promise.resolve(); + // Store options for dependency injection (allows core package to inject platform-specific services) readonly #options: PerpsControllerOptions; @@ -1665,6 +1708,14 @@ export class PerpsController extends BaseController< attempts: attempt, }); + // Hydrate watchlist from AUS (non-blocking — transient failures are + // caught inside and must not prevent init from completing). + // Assigning to #ausQueue ensures subsequent toggleWatchlistMarket + // calls wait for hydration before running their own GET-merge-PUT. + this.#ausQueue = this.#syncWatchlistFromRemote().catch(() => { + // Errors are already logged inside #syncWatchlistFromRemote. + }); + return; // Exit retry loop on success } catch (error) { lastError = ensureError(error, 'PerpsController.performInitialization'); @@ -5070,12 +5121,21 @@ export class PerpsController extends BaseController< } /** - * Toggle watchlist status for a market - * Watchlist markets are stored per network (testnet/mainnet) + * Toggle watchlist status for a market. + * + * Updates local state immediately (optimistic UI) and then syncs the new + * watchlist to AuthenticatedUserStorageService. If the remote write fails, + * the local state is reverted so it stays consistent with AUS. + * + * When the user is unauthenticated, or the active provider is not yet + * supported by the AUS schema, the controller continues operating with + * local-persisted state only — no error is surfaced to the caller. + * + * Watchlist markets are stored per network (testnet/mainnet). * * @param symbol - The trading pair symbol. */ - toggleWatchlistMarket(symbol: string): void { + async toggleWatchlistMarket(symbol: string): Promise { const currentNetwork = this.state.isTestnet ? 'testnet' : 'mainnet'; const currentWatchlist = this.state.watchlistMarkets[currentNetwork]; const isWatchlisted = currentWatchlist.includes(symbol); @@ -5087,17 +5147,54 @@ export class PerpsController extends BaseController< action: isWatchlisted ? 'remove' : 'add', }); + // Step 1: Optimistic local state update — UI reflects change immediately. this.update((state) => { if (isWatchlisted) { - // Remove from watchlist state.watchlistMarkets[currentNetwork] = currentWatchlist.filter( (marketSymbol) => marketSymbol !== symbol, ); } else { - // Add to watchlist state.watchlistMarkets[currentNetwork] = [...currentWatchlist, symbol]; } }); + + this.#getMetrics().trackPerpsEvent(PerpsAnalyticsEvent.UiInteraction, { + [PERPS_EVENT_PROPERTY.INTERACTION_TYPE]: + PERPS_EVENT_VALUE.INTERACTION_TYPE.FAVORITE_TOGGLED, + [PERPS_EVENT_PROPERTY.ASSET]: symbol, + [PERPS_EVENT_PROPERTY.ACTION_TYPE]: isWatchlisted + ? PERPS_EVENT_VALUE.ACTION_TYPE.UNFAVORITE_MARKET + : PERPS_EVENT_VALUE.ACTION_TYPE.FAVORITE_MARKET, + [PERPS_EVENT_PROPERTY.FAVORITES_COUNT]: + this.state.watchlistMarkets[currentNetwork].length, + }); + + // Step 2: Persist to AUS; revert local state if the write fails. + // Enqueue behind #ausQueue so that: + // - concurrent toggles serialize their GET-merge-PUT sequences, and + // - any in-flight init hydration completes before we issue a write. + try { + await new Promise((resolve, reject) => { + this.#ausQueue = this.#ausQueue + .then(() => this.#persistWatchlistToRemote(currentNetwork)) + .then(resolve, reject) + // Swallow the error on the queue chain so later operations can run. + .catch(() => undefined); + }); + } catch (error) { + this.#logError( + ensureError(error, 'PerpsController.toggleWatchlistMarket'), + this.#getErrorContext('toggleWatchlistMarket', { + symbol, + network: currentNetwork, + action: isWatchlisted ? 'remove' : 'add', + }), + ); + // Revert the optimistic update. + this.update((state) => { + state.watchlistMarkets[currentNetwork] = currentWatchlist; + }); + } } /** @@ -5121,6 +5218,177 @@ export class PerpsController extends BaseController< return this.state.watchlistMarkets[currentNetwork]; } + /** + * Writes the current local watchlist to AuthenticatedUserStorageService + * using a read-merge-write strategy to avoid overwriting other preferences. + * + * Skips silently when: + * - The active provider has no AUS exchange key (e.g. `'aggregated'`). + * - The remote preferences blob does not yet exist (returns `null` / 404). + * In that case, `NotificationServicesController.createOnChainTriggers` is + * the canonical owner that creates the initial blob. + * + * Throws on remote write failure so the caller can decide whether to revert. + * + * @param network - Which network's list to sync ('testnet' | 'mainnet'). + */ + async #persistWatchlistToRemote( + network: 'testnet' | 'mainnet', + ): Promise { + const exchangeKey = resolveWatchlistExchangeKey(this.state.activeProvider); + if (!exchangeKey) { + this.#debugLog( + 'PerpsController: Skipping AUS watchlist sync — provider not mapped', + { activeProvider: this.state.activeProvider }, + ); + return; + } + + const prefs = await this.messenger.call( + 'AuthenticatedUserStorageService:getNotificationPreferences', + ); + + if (!prefs) { + this.#debugLog( + 'PerpsController: Skipping AUS watchlist write — preferences blob not yet initialised', + { exchangeKey, network }, + ); + return; + } + + const existingWatchlist: PerpsWatchlistMarkets = prefs.perps + .watchlistMarkets ?? { + hyperliquid: { testnet: [], mainnet: [] }, + myx: { testnet: [], mainnet: [] }, + }; + + const nextWatchlistMarkets: PerpsWatchlistMarkets = { + ...existingWatchlist, + [exchangeKey]: { + ...existingWatchlist[exchangeKey], + [network]: this.state.watchlistMarkets[network], + }, + }; + + const nextPrefs: NotificationPreferences = { + ...prefs, + perps: { + ...prefs.perps, + watchlistMarkets: nextWatchlistMarkets, + }, + }; + + await this.messenger.call( + 'AuthenticatedUserStorageService:putNotificationPreferences', + nextPrefs, + ); + + this.#debugLog('PerpsController: Watchlist synced to AUS', { + exchangeKey, + network, + count: this.state.watchlistMarkets[network].length, + }); + } + + /** + * Hydrates `state.watchlistMarkets` from AuthenticatedUserStorageService on + * controller initialisation. + * + * AUS is the source of truth; local state is used as an offline cache. + * This method also handles the one-time migration from local-only state to + * AUS for users who had a watchlist before AUS sync was introduced. + * + * All remote errors are swallowed so a transient network failure does not + * block the rest of `init()`. + */ + async #syncWatchlistFromRemote(): Promise { + const exchangeKey = resolveWatchlistExchangeKey(this.state.activeProvider); + if (!exchangeKey) { + this.#debugLog( + 'PerpsController: Skipping AUS watchlist hydration — provider not mapped', + { activeProvider: this.state.activeProvider }, + ); + return; + } + + try { + const prefs = await this.messenger.call( + 'AuthenticatedUserStorageService:getNotificationPreferences', + ); + + if (!prefs) { + this.#debugLog( + 'PerpsController: No AUS preferences blob — using local watchlist', + ); + return; + } + + const remoteExchangeWatchlist = + prefs.perps.watchlistMarkets?.[exchangeKey]; + + // AUS is the source of truth: an absent exchange key means this device + // has not been migrated yet — push any local favorites up once. + // A present key (even with empty arrays) must be honored as-is, + // including an intentional remote clear. + if (remoteExchangeWatchlist === undefined) { + // Blob exists but has no watchlist for this exchange yet. + // If local state has any markets, push them up as a one-time migration. + const { testnet, mainnet } = this.state.watchlistMarkets; + const hasLocalMarkets = testnet.length > 0 || mainnet.length > 0; + + if (hasLocalMarkets) { + this.#debugLog('PerpsController: Migrating local watchlist to AUS', { + exchangeKey, + testnetCount: testnet.length, + mainnetCount: mainnet.length, + }); + // Push testnet and mainnet together via a single read-merge-write. + // Start from existing remote watchlistMarkets (or empty fallback) so + // that other exchanges already stored in AUS are not overwritten. + const existingWatchlist: PerpsWatchlistMarkets = prefs.perps + .watchlistMarkets ?? { + hyperliquid: { testnet: [], mainnet: [] }, + myx: { testnet: [], mainnet: [] }, + }; + const nextWatchlistMarkets: PerpsWatchlistMarkets = { + ...existingWatchlist, + [exchangeKey]: { testnet, mainnet }, + }; + const nextPrefs: NotificationPreferences = { + ...prefs, + perps: { + ...prefs.perps, + watchlistMarkets: nextWatchlistMarkets, + }, + }; + await this.messenger.call( + 'AuthenticatedUserStorageService:putNotificationPreferences', + nextPrefs, + ); + this.#debugLog('PerpsController: Local watchlist migrated to AUS', { + exchangeKey, + }); + } + } else { + // AUS has an entry for this exchange — hydrate local state from it. + this.update((state) => { + state.watchlistMarkets.testnet = remoteExchangeWatchlist.testnet; + state.watchlistMarkets.mainnet = remoteExchangeWatchlist.mainnet; + }); + this.#debugLog('PerpsController: Watchlist hydrated from AUS', { + exchangeKey, + testnetCount: remoteExchangeWatchlist.testnet.length, + mainnetCount: remoteExchangeWatchlist.mainnet.length, + }); + } + } catch (error) { + this.#logError( + ensureError(error, 'PerpsController.syncWatchlistFromRemote'), + this.#getErrorContext('syncWatchlistFromRemote'), + ); + } + } + /** * Report order events to data lake API with retry (non-blocking) * Thin delegation to DataLakeService diff --git a/packages/perps-controller/src/types/messenger.ts b/packages/perps-controller/src/types/messenger.ts index 5489121617..96d3a1b6f6 100644 --- a/packages/perps-controller/src/types/messenger.ts +++ b/packages/perps-controller/src/types/messenger.ts @@ -6,6 +6,10 @@ import type { AccountsControllerGetSelectedAccountAction, AccountsControllerSelectedAccountChangeEvent, } from '@metamask/accounts-controller'; +import type { + AuthenticatedUserStorageServiceGetNotificationPreferencesAction, + AuthenticatedUserStorageServicePutNotificationPreferencesAction, +} from '@metamask/authenticated-user-storage'; import type { GeolocationControllerGetGeolocationAction } from '@metamask/geolocation-controller'; import type { KeyringControllerGetStateAction, @@ -38,7 +42,9 @@ export type PerpsControllerAllowedActions = | RemoteFeatureFlagControllerGetStateAction | AccountsControllerGetSelectedAccountAction | AccountTreeControllerGetAccountsFromSelectedAccountGroupAction - | AuthenticationController.AuthenticationControllerGetBearerTokenAction; + | AuthenticationController.AuthenticationControllerGetBearerTokenAction + | AuthenticatedUserStorageServiceGetNotificationPreferencesAction + | AuthenticatedUserStorageServicePutNotificationPreferencesAction; /** * Events from other controllers that PerpsController is allowed to subscribe to. diff --git a/packages/perps-controller/tests/src/PerpsController.state.test.ts b/packages/perps-controller/tests/src/PerpsController.state.test.ts index 99871b8c9f..35ef732ba8 100644 --- a/packages/perps-controller/tests/src/PerpsController.state.test.ts +++ b/packages/perps-controller/tests/src/PerpsController.state.test.ts @@ -4,8 +4,6 @@ * Clean, focused test suite for PerpsController */ -/* eslint-disable @typescript-eslint/no-explicit-any */ - import { GasFeeEstimateLevel, GasFeeEstimateType, @@ -43,17 +41,8 @@ import { firstNonEmpty, resolveMyxAuthConfig, } from '../../src/PerpsController'; -import type { PerpsControllerState } from '../../src/PerpsController'; import { PERPS_ERROR_CODES } from '../../src/perpsErrorCodes'; import { HyperLiquidProvider } from '../../src/providers/HyperLiquidProvider'; -import type { - AccountState, - GetAvailableDexsParams, - PerpsProvider, - PerpsPlatformDependencies, - PerpsProviderType, - SubscribeAccountParams, -} from '../../src/types'; import { PerpsAnalyticsEvent } from '../../src/types'; jest.mock('../../src/providers/HyperLiquidProvider'); @@ -64,7 +53,7 @@ const mockAddTransaction = jest.fn(); jest.mock( '../../../util/transaction-controller', () => ({ - addTransaction: (...args: unknown[]) => mockAddTransaction(...args), + addTransaction: (...args) => mockAddTransaction(...args), }), { virtual: true }, ); @@ -237,7 +226,7 @@ jest.mock('../../src/services/DataLakeService', () => ({ // Mock FeatureFlagConfigurationService as a class with instance methods const mockFeatureFlagConfigurationServiceInstance = { - refreshEligibility: jest.fn((options: any) => { + refreshEligibility: jest.fn((options) => { // Simulate the service's behavior: extract blocked regions from remote flags const remoteFlags = options.remoteFeatureFlagControllerState.remoteFeatureFlags; @@ -270,7 +259,7 @@ const mockFeatureFlagConfigurationServiceInstance = { } }), refreshHip3Config: jest.fn(), - setBlockedRegions: jest.fn((options: any) => { + setBlockedRegions: jest.fn((options) => { // Simulate setBlockedRegions behavior const { list, source, context } = options; if (context.setBlockedRegionList && context.getBlockedRegionList) { @@ -303,137 +292,74 @@ jest.mock('../../src/services/FeatureFlagConfigurationService', () => ({ * This follows the pattern used in RewardsController.test.ts */ class TestablePerpsController extends PerpsController { - /** - * Test-only method to update state directly. - * Exposed for scenarios where state needs to be manipulated - * outside the normal public API (e.g., testing error conditions). - * @param callback - */ - public testUpdate(callback: (state: PerpsControllerState) => void) { + testUpdate(callback) { this.update(callback); } - /** - * Test-only method to mark controller as initialized. - * Common test scenario that requires internal state changes. - */ - public testMarkInitialized() { + testMarkInitialized() { this.isInitialized = true; this.update((state) => { state.initializationState = InitializationState.Initialized; }); } - /** - * Test-only method to set the providers map with complete providers. - * Used in most tests to inject mock providers. - * Also sets activeProviderInstance to the first provider (default provider). - * @param providers - */ - public testSetProviders(providers: Map) { + testSetProviders(providers) { this.providers = providers; - // Set activeProviderInstance to the first provider (typically 'hyperliquid') const firstProvider = providers.values().next().value; if (firstProvider) { this.activeProviderInstance = firstProvider; } } - /** - * Test-only method to set the providers map with partial providers. - * Used explicitly in tests that verify error handling with incomplete providers. - * Type cast is intentional and necessary for testing graceful degradation. - * @param providers - */ - public testSetPartialProviders( - providers: Map>, - ) { - this.providers = providers as Map; + testSetPartialProviders(providers) { + this.providers = providers; } - /** - * Test-only method to get the providers map. - * Used to verify provider state in tests. - */ - public testGetProviders(): Map { + testGetProviders() { return this.providers; } - /** - * Test-only method to set initialization state. - * Allows tests to simulate both initialized and uninitialized states. - * @param value - */ - public testSetInitialized(value: boolean) { + testSetInitialized(value) { this.isInitialized = value; } - /** - * Test-only method to get initialization state. - * Used to verify initialization status in tests. - */ - public testGetInitialized(): boolean { + testGetInitialized() { return this.isInitialized; } - /** - * Test-only method to get blocked region list. - * Used to verify geo-blocking configuration in tests. - */ - public testGetBlockedRegionList(): { source: string; list: string[] } { + testGetBlockedRegionList() { return this.blockedRegionList; } - /** - * Test-only method to set blocked region list. - * Used to test priority logic (remote vs fallback). - * @param list - * @param source - */ - public testSetBlockedRegionList( - list: string[], - source: 'remote' | 'fallback', - ) { + testSetBlockedRegionList(list, source) { this.setBlockedRegionList(list, source); } - /** - * Test accessor for protected method refreshEligibilityOnFeatureFlagChange. - * Wrapper is necessary because protected methods can't be called from test code. - * @param remoteFlags - */ - public testRefreshEligibilityOnFeatureFlagChange(remoteFlags: any) { + testRefreshEligibilityOnFeatureFlagChange(remoteFlags) { this.refreshEligibilityOnFeatureFlagChange(remoteFlags); } - /** - * Test accessor for protected method reportOrderToDataLake. - * Wrapper is necessary because protected methods can't be called from test code. - * @param data - */ - public testReportOrderToDataLake(data: any): Promise { + testReportOrderToDataLake(data) { return this.reportOrderToDataLake(data); } - public testHasStandaloneProvider(): boolean { + testHasStandaloneProvider() { return this.hasStandaloneProvider(); } - public testRegisterMYXProvider( - MYXProvider: new (opts: Record) => PerpsProvider, - ) { - this.registerMYXProvider(MYXProvider as never); + testRegisterMYXProvider(MYXProvider) { + this.registerMYXProvider(MYXProvider); } - public testHandleMYXImportError(error: unknown) { + testHandleMYXImportError(error) { this.handleMYXImportError(error); } } describe('PerpsController', () => { - let controller: TestablePerpsController; - let mockProvider: jest.Mocked; - let mockInfrastructure: jest.Mocked; + let controller; + let mockProvider; + let mockInfrastructure; // Helper to mark controller as initialized for tests const markControllerAsInitialized = () => { @@ -443,34 +369,33 @@ describe('PerpsController', () => { beforeEach(() => { jest.clearAllMocks(); - ( - jest.requireMock('../../src/services/EligibilityService') - .EligibilityService as jest.Mock - ).mockImplementation(() => mockEligibilityServiceInstance); - ( - jest.requireMock('../../src/services/DepositService') - .DepositService as jest.Mock - ).mockImplementation(() => mockDepositServiceInstance); - ( - jest.requireMock('../../src/services/MarketDataService') - .MarketDataService as jest.Mock - ).mockImplementation(() => mockMarketDataServiceInstance); - ( - jest.requireMock('../../src/services/TradingService') - .TradingService as jest.Mock - ).mockImplementation(() => mockTradingServiceInstance); - ( - jest.requireMock('../../src/services/AccountService') - .AccountService as jest.Mock - ).mockImplementation(() => mockAccountServiceInstance); - ( - jest.requireMock('../../src/services/DataLakeService') - .DataLakeService as jest.Mock - ).mockImplementation(() => mockDataLakeServiceInstance); - ( - jest.requireMock('../../src/services/FeatureFlagConfigurationService') - .FeatureFlagConfigurationService as jest.Mock - ).mockImplementation(() => mockFeatureFlagConfigurationServiceInstance); + jest + .requireMock('../../src/services/EligibilityService') + .EligibilityService.mockImplementation( + () => mockEligibilityServiceInstance, + ); + jest + .requireMock('../../src/services/DepositService') + .DepositService.mockImplementation(() => mockDepositServiceInstance); + jest + .requireMock('../../src/services/MarketDataService') + .MarketDataService.mockImplementation( + () => mockMarketDataServiceInstance, + ); + jest + .requireMock('../../src/services/TradingService') + .TradingService.mockImplementation(() => mockTradingServiceInstance); + jest + .requireMock('../../src/services/AccountService') + .AccountService.mockImplementation(() => mockAccountServiceInstance); + jest + .requireMock('../../src/services/DataLakeService') + .DataLakeService.mockImplementation(() => mockDataLakeServiceInstance); + jest + .requireMock('../../src/services/FeatureFlagConfigurationService') + .FeatureFlagConfigurationService.mockImplementation( + () => mockFeatureFlagConfigurationServiceInstance, + ); mockEligibilityServiceInstance.checkEligibility.mockResolvedValue(true); mockMarketDataServiceInstance.getPositions.mockResolvedValue([]); @@ -496,7 +421,7 @@ describe('PerpsController', () => { mockMarketDataServiceInstance.getAvailableDexs.mockResolvedValue([]); mockFeatureFlagConfigurationServiceInstance.refreshEligibility.mockImplementation( - (options: any) => { + (options) => { const remoteFlags = options.remoteFeatureFlagControllerState.remoteFeatureFlags; const perpsGeoBlockedRegionsFeatureFlag = @@ -531,7 +456,7 @@ describe('PerpsController', () => { }, ); mockFeatureFlagConfigurationServiceInstance.setBlockedRegions.mockImplementation( - (options: any) => { + (options) => { const { list, source, context } = options; if (context.setBlockedRegionList && context.getBlockedRegionList) { const currentList = context.getBlockedRegionList(); @@ -555,12 +480,12 @@ describe('PerpsController', () => { ); // Reset Engine.context mocks to default state to prevent test interdependence - ( - Engine.context.RewardsController.getPerpsDiscountForAccount as jest.Mock - ).mockResolvedValue(null); - ( - Engine.context.NetworkController.getNetworkClientById as jest.Mock - ).mockReturnValue({ configuration: { chainId: '0x1' } }); + Engine.context.RewardsController.getPerpsDiscountForAccount.mockResolvedValue( + null, + ); + Engine.context.NetworkController.getNetworkClientById.mockReturnValue({ + configuration: { chainId: '0x1' }, + }); // Create a fresh mock provider for each test mockProvider = createMockHyperLiquidProvider(); @@ -589,11 +514,9 @@ describe('PerpsController', () => { ); mockProvider.getWithdrawalRoutes.mockReturnValue([]); - ( - HyperLiquidProvider as jest.MockedClass - ).mockImplementation(() => mockProvider); + HyperLiquidProvider.mockImplementation(() => mockProvider); - const mockCall = jest.fn().mockImplementation((action: string) => { + const mockCall = jest.fn().mockImplementation((action) => { if (action === 'RemoteFeatureFlagController:getState') { return { remoteFeatureFlags: { @@ -643,13 +566,13 @@ describe('PerpsController', () => { value !== null && 'mockClear' in value ) { - (value as jest.Mock).mockClear(); + value.mockClear(); } }); } - (mockInfrastructure.metrics.trackPerpsEvent as jest.Mock).mockClear(); - (mockInfrastructure.logger.error as jest.Mock).mockClear(); - (mockInfrastructure.debugLogger.log as jest.Mock).mockClear(); + mockInfrastructure.metrics.trackPerpsEvent.mockClear(); + mockInfrastructure.logger.error.mockClear(); + mockInfrastructure.debugLogger.log.mockClear(); }); describe('state management', () => { it('returns positions without updating state', async () => { @@ -661,7 +584,7 @@ describe('PerpsController', () => { positionValue: '5000', unrealizedPnl: '500', marginUsed: '2500', - leverage: { type: 'cross' as const, value: 2 }, + leverage: { type: 'cross', value: 2 }, liquidationPrice: '1500', maxLeverage: 100, returnOnEquity: '10.0', @@ -808,7 +731,7 @@ describe('PerpsController', () => { newOrder: { symbol: 'BTC', isBuy: true, - orderType: 'limit' as const, + orderType: 'limit', price: '51000', size: '0.2', }, @@ -844,7 +767,7 @@ describe('PerpsController', () => { newOrder: { symbol: 'BTC', isBuy: true, - orderType: 'limit' as const, + orderType: 'limit', price: '51000', size: '0.2', }, @@ -998,27 +921,27 @@ describe('PerpsController', () => { expect(watchlist).toEqual([]); }); - it('toggles watchlist market (add)', () => { - controller.toggleWatchlistMarket('BTC'); + it('toggles watchlist market (add)', async () => { + await controller.toggleWatchlistMarket('BTC'); const watchlist = controller.getWatchlistMarkets(); expect(watchlist).toContain('BTC'); expect(controller.isWatchlistMarket('BTC')).toBe(true); }); - it('toggles watchlist market (remove)', () => { - controller.toggleWatchlistMarket('BTC'); - controller.toggleWatchlistMarket('BTC'); + it('toggles watchlist market (remove)', async () => { + await controller.toggleWatchlistMarket('BTC'); + await controller.toggleWatchlistMarket('BTC'); const watchlist = controller.getWatchlistMarkets(); expect(watchlist).not.toContain('BTC'); expect(controller.isWatchlistMarket('BTC')).toBe(false); }); - it('handles multiple watchlist markets', () => { - controller.toggleWatchlistMarket('BTC'); - controller.toggleWatchlistMarket('ETH'); - controller.toggleWatchlistMarket('SOL'); + it('handles multiple watchlist markets', async () => { + await controller.toggleWatchlistMarket('BTC'); + await controller.toggleWatchlistMarket('ETH'); + await controller.toggleWatchlistMarket('SOL'); const watchlist = controller.getWatchlistMarkets(); expect(watchlist).toHaveLength(3); @@ -1027,12 +950,12 @@ describe('PerpsController', () => { expect(watchlist).toContain('SOL'); }); - it('persist watchlist per network', () => { + it('persist watchlist per network', async () => { // Add to watchlist on mainnet (default is testnet in dev, so set to false) controller.testUpdate((state) => { state.isTestnet = false; }); - controller.toggleWatchlistMarket('BTC'); + await controller.toggleWatchlistMarket('BTC'); const mainnetWatchlist = controller.getWatchlistMarkets(); expect(mainnetWatchlist).toContain('BTC'); @@ -1045,7 +968,7 @@ describe('PerpsController', () => { expect(testnetWatchlist).toEqual([]); // Add to watchlist on testnet - controller.toggleWatchlistMarket('ETH'); + await controller.toggleWatchlistMarket('ETH'); expect(controller.getWatchlistMarkets()).toContain('ETH'); expect(controller.isWatchlistMarket('ETH')).toBe(true); @@ -1058,6 +981,466 @@ describe('PerpsController', () => { }); }); + describe('AUS watchlist sync', () => { + /** + * Minimal valid NotificationPreferences blob used across these tests. + * `watchlistMarkets` is intentionally absent so individual tests can + * control whether the field is present or not. + */ + const MOCK_PREFS_BASE = { + walletActivity: { + inAppNotificationsEnabled: true, + pushNotificationsEnabled: true, + accounts: [], + }, + marketing: { + inAppNotificationsEnabled: false, + pushNotificationsEnabled: false, + }, + perps: { + inAppNotificationsEnabled: true, + pushNotificationsEnabled: true, + }, + socialAI: { + inAppNotificationsEnabled: false, + pushNotificationsEnabled: false, + mutedTraderProfileIds: [], + }, + }; + + let ausController; + let mockAusCall; + let mockAusInfrastructure; + + beforeEach(() => { + mockAusCall = jest.fn().mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { + remoteFeatureFlags: { + perpsPerpTradingGeoBlockedCountriesV2: { blockedRegions: [] }, + }, + }; + } + // By default, behave as if no blob exists (unauthenticated / 404). + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.resolve(null); + } + if ( + action === + 'AuthenticatedUserStorageService:putNotificationPreferences' + ) { + return Promise.resolve(undefined); + } + return undefined; + }); + + mockAusInfrastructure = createMockInfrastructure(); + ausController = new TestablePerpsController({ + messenger: createMockMessenger({ call: mockAusCall }), + state: getDefaultPerpsControllerState(), + infrastructure: mockAusInfrastructure, + }); + }); + + it('local state updates immediately (optimistic) when AUS returns null blob', async () => { + // AUS returns null → no remote write, but local state should still change. + await ausController.toggleWatchlistMarket('BTC'); + + expect(ausController.getWatchlistMarkets()).toContain('BTC'); + expect(mockAusCall).toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:getNotificationPreferences', + ); + expect(mockAusCall).not.toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:putNotificationPreferences', + expect.anything(), + ); + }); + + it('writes merged watchlist to AUS when a preferences blob exists', async () => { + const existingPrefs = { + ...MOCK_PREFS_BASE, + perps: { + ...MOCK_PREFS_BASE.perps, + watchlistMarkets: { + hyperliquid: { testnet: [], mainnet: [] }, + myx: { testnet: [], mainnet: [] }, + }, + }, + }; + + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { remoteFeatureFlags: {} }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.resolve(existingPrefs); + } + if ( + action === + 'AuthenticatedUserStorageService:putNotificationPreferences' + ) { + return Promise.resolve(undefined); + } + return undefined; + }); + + // Default state is testnet; toggle on testnet. + ausController.testUpdate((state) => { + state.isTestnet = true; + state.activeProvider = 'hyperliquid'; + }); + + await ausController.toggleWatchlistMarket('BTC'); + + expect(ausController.getWatchlistMarkets()).toContain('BTC'); + + // Verify put was called with merged prefs. + expect(mockAusCall).toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:putNotificationPreferences', + expect.objectContaining({ + perps: expect.objectContaining({ + watchlistMarkets: expect.objectContaining({ + hyperliquid: expect.objectContaining({ + testnet: expect.arrayContaining(['BTC']), + }), + }), + }), + }), + ); + }); + + it('reverts local state when AUS PUT fails', async () => { + const existingPrefs = { + ...MOCK_PREFS_BASE, + perps: { + ...MOCK_PREFS_BASE.perps, + watchlistMarkets: { + hyperliquid: { testnet: [], mainnet: [] }, + myx: { testnet: [], mainnet: [] }, + }, + }, + }; + + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { remoteFeatureFlags: {} }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.resolve(existingPrefs); + } + if ( + action === + 'AuthenticatedUserStorageService:putNotificationPreferences' + ) { + return Promise.reject(new Error('AUS server error')); + } + return undefined; + }); + + ausController.testUpdate((state) => { + state.isTestnet = false; + state.activeProvider = 'hyperliquid'; + }); + + // After toggle, local state should optimistically contain BTC. + // After PUT fails, it should be reverted. + await ausController.toggleWatchlistMarket('BTC'); + + expect(ausController.getWatchlistMarkets()).not.toContain('BTC'); + expect(mockAusInfrastructure.logger.error).toHaveBeenCalled(); + }); + + it('skips AUS sync when activeProvider is aggregated', async () => { + ausController.testUpdate((state) => { + state.activeProvider = 'aggregated'; + }); + + await ausController.toggleWatchlistMarket('BTC'); + + // Local state changes. + expect(ausController.getWatchlistMarkets()).toContain('BTC'); + // AUS is never contacted. + expect(mockAusCall).not.toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:getNotificationPreferences', + ); + expect(mockAusCall).not.toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:putNotificationPreferences', + expect.anything(), + ); + }); + + it('does not throw when AUS GET throws (unauthenticated)', async () => { + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { remoteFeatureFlags: {} }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.reject(new Error('Unauthenticated')); + } + return undefined; + }); + + ausController.testUpdate((state) => { + state.isTestnet = false; + }); + + // Should not throw — failure is handled internally. + await expect( + ausController.toggleWatchlistMarket('BTC'), + ).resolves.toBeUndefined(); + + // Local state is reverted since the AUS path failed. + expect(ausController.getWatchlistMarkets()).not.toContain('BTC'); + }); + + it('tracks analytics event when toggling watchlist market', async () => { + ausController.testUpdate((state) => { + state.isTestnet = false; + state.activeProvider = 'hyperliquid'; + }); + + await ausController.toggleWatchlistMarket('ETH'); + + expect( + mockAusInfrastructure.metrics.trackPerpsEvent, + ).toHaveBeenCalledWith( + PerpsAnalyticsEvent.UiInteraction, + expect.objectContaining({ + interaction_type: 'favorite_toggled', + asset: 'ETH', + }), + ); + }); + + describe('init hydration from AUS', () => { + it('hydrates local watchlist from AUS on successful init', async () => { + const remotePrefs = { + ...MOCK_PREFS_BASE, + perps: { + ...MOCK_PREFS_BASE.perps, + watchlistMarkets: { + hyperliquid: { + testnet: ['BTC', 'ETH'], + mainnet: ['SOL'], + }, + myx: { testnet: [], mainnet: [] }, + }, + }, + }; + + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { + remoteFeatureFlags: { + perpsPerpTradingGeoBlockedCountriesV2: { blockedRegions: [] }, + }, + }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.resolve(remotePrefs); + } + return undefined; + }); + + ausController.testUpdate((state) => { + state.activeProvider = 'hyperliquid'; + }); + + await ausController.init(); + + // Allow the non-blocking #syncWatchlistFromRemote promise to settle. + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(ausController.state.watchlistMarkets.testnet).toEqual([ + 'BTC', + 'ETH', + ]); + expect(ausController.state.watchlistMarkets.mainnet).toEqual(['SOL']); + }); + + it('hydrates (clears) local watchlist when remote entry exists with empty arrays', async () => { + // Remote blob has the hyperliquid key present but both arrays are empty — + // this represents an intentional clear by another device. The controller + // must honor the remote state rather than treating it as "not migrated". + const remotePrefsEmptyWatchlist = { + ...MOCK_PREFS_BASE, + perps: { + ...MOCK_PREFS_BASE.perps, + watchlistMarkets: { + hyperliquid: { testnet: [], mainnet: [] }, + myx: { testnet: [], mainnet: [] }, + }, + }, + }; + + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { + remoteFeatureFlags: { + perpsPerpTradingGeoBlockedCountriesV2: { blockedRegions: [] }, + }, + }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.resolve(remotePrefsEmptyWatchlist); + } + return undefined; + }); + + // Local state has stale favorites from before the remote clear. + const staleState = getDefaultPerpsControllerState(); + staleState.activeProvider = 'hyperliquid'; + staleState.watchlistMarkets.testnet = ['BTC', 'ETH']; + staleState.watchlistMarkets.mainnet = ['SOL']; + + const clearController = new TestablePerpsController({ + messenger: createMockMessenger({ call: mockAusCall }), + state: staleState, + infrastructure: mockAusInfrastructure, + }); + + await clearController.init(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Local state must be cleared to match the remote empty arrays. + expect(clearController.state.watchlistMarkets.testnet).toEqual([]); + expect(clearController.state.watchlistMarkets.mainnet).toEqual([]); + + // No migration PUT should be issued — the remote key is present. + expect(mockAusCall).not.toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:putNotificationPreferences', + expect.anything(), + ); + }); + + it('performs one-time migration when blob exists but has no watchlist for the active provider', async () => { + const remotePrefsWithoutWatchlist = { ...MOCK_PREFS_BASE }; + + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { + remoteFeatureFlags: { + perpsPerpTradingGeoBlockedCountriesV2: { blockedRegions: [] }, + }, + }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.resolve(remotePrefsWithoutWatchlist); + } + if ( + action === + 'AuthenticatedUserStorageService:putNotificationPreferences' + ) { + return Promise.resolve(undefined); + } + return undefined; + }); + + // Local state has some markets saved before AUS was introduced. + const initialState = getDefaultPerpsControllerState(); + initialState.watchlistMarkets.testnet = ['BTC']; + initialState.watchlistMarkets.mainnet = ['ETH', 'SOL']; + initialState.activeProvider = 'hyperliquid'; + + const migrationController = new TestablePerpsController({ + messenger: createMockMessenger({ call: mockAusCall }), + state: initialState, + infrastructure: mockAusInfrastructure, + }); + + await migrationController.init(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Verify local markets were pushed to AUS. + expect(mockAusCall).toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:putNotificationPreferences', + expect.objectContaining({ + perps: expect.objectContaining({ + watchlistMarkets: expect.objectContaining({ + hyperliquid: expect.objectContaining({ + testnet: ['BTC'], + mainnet: ['ETH', 'SOL'], + }), + }), + }), + }), + ); + }); + + it('skips hydration when AUS blob is null', async () => { + // AUS returns null — local state is untouched. + const localState = getDefaultPerpsControllerState(); + localState.watchlistMarkets.mainnet = ['BTC']; + localState.activeProvider = 'hyperliquid'; + + const nullBlobController = new TestablePerpsController({ + messenger: createMockMessenger({ call: mockAusCall }), + state: localState, + infrastructure: mockAusInfrastructure, + }); + + await nullBlobController.init(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Local state unchanged. + expect(nullBlobController.state.watchlistMarkets.mainnet).toEqual([ + 'BTC', + ]); + expect(mockAusCall).not.toHaveBeenCalledWith( + 'AuthenticatedUserStorageService:putNotificationPreferences', + expect.anything(), + ); + }); + + it('does not throw when AUS GET throws during init', async () => { + mockAusCall.mockImplementation((action) => { + if (action === 'RemoteFeatureFlagController:getState') { + return { + remoteFeatureFlags: { + perpsPerpTradingGeoBlockedCountriesV2: { blockedRegions: [] }, + }, + }; + } + if ( + action === + 'AuthenticatedUserStorageService:getNotificationPreferences' + ) { + return Promise.reject(new Error('Network error')); + } + return undefined; + }); + + // init() should still succeed; the watchlist sync error is handled internally. + await expect(ausController.init()).resolves.toBeUndefined(); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(mockAusInfrastructure.logger.error).toHaveBeenCalled(); + }); + }); + }); + describe('additional subscriptions', () => { it('subscribes to orders', () => { const mockUnsubscribe = jest.fn(); @@ -1096,15 +1479,13 @@ describe('PerpsController', () => { it('updates accountState when subscribeToAccount callback receives non-null account', () => { const originalCallback = jest.fn(); - let wrappedCallback: (account: AccountState | null) => void = () => { + let wrappedCallback = () => { /* assigned by mock */ }; - mockProvider.subscribeToAccount.mockImplementation( - (p: SubscribeAccountParams) => { - wrappedCallback = p.callback; - return jest.fn(); - }, - ); + mockProvider.subscribeToAccount.mockImplementation((p) => { + wrappedCallback = p.callback; + return jest.fn(); + }); markControllerAsInitialized(); controller.testSetProviders(new Map([['hyperliquid', mockProvider]])); diff --git a/packages/perps-controller/tsconfig.build.json b/packages/perps-controller/tsconfig.build.json index 02b3a42838..3b9c75ab90 100644 --- a/packages/perps-controller/tsconfig.build.json +++ b/packages/perps-controller/tsconfig.build.json @@ -7,6 +7,7 @@ }, "references": [ { "path": "../account-tree-controller/tsconfig.build.json" }, + { "path": "../authenticated-user-storage/tsconfig.build.json" }, { "path": "../base-controller/tsconfig.build.json" }, { "path": "../bridge-controller/tsconfig.build.json" }, { "path": "../controller-utils/tsconfig.build.json" }, diff --git a/packages/perps-controller/tsconfig.json b/packages/perps-controller/tsconfig.json index 324879cf43..8f5557e19c 100644 --- a/packages/perps-controller/tsconfig.json +++ b/packages/perps-controller/tsconfig.json @@ -7,6 +7,9 @@ { "path": "../account-tree-controller" }, + { + "path": "../authenticated-user-storage" + }, { "path": "../base-controller" }, diff --git a/yarn.lock b/yarn.lock index 9fc2eab3c2..0c92d077b6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7902,6 +7902,7 @@ __metadata: dependencies: "@metamask/abi-utils": "npm:^2.0.3" "@metamask/account-tree-controller": "npm:^7.5.3" + "@metamask/authenticated-user-storage": "npm:^2.1.0" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" "@metamask/controller-utils": "npm:^12.3.0"