address feedback

app/components/UI/Tokens/TokenList/PortfolioBalance/index.tsx

@@ -39,13 +39,13 @@ export const PortfolioBalance = React.memo(() => {
   const styles = createStyles(colors);
   const browserTabs = useSelector((state: RootState) => state.browser.tabs);
   const privacyMode = useSelector(selectPrivacyMode);
-  const ismultchainBalancesCollectionForMarketingEnabled = useSelector(
+  const isMultichainBalancesCollectionForMarketingEnabled = useSelector(
     (state: RootState) => state.security.dataCollectionForMarketing,
   );
   const navigation = useNavigation();
   const { trackEvent, isEnabled, createEventBuilder } = useMetrics();
 
-  const { multchainBalances } = useMultichainBalances();
+  const { multichainBalances } = useMultichainBalances();
 
   const onOpenPortfolio = useCallback(() => {
     const existingPortfolioTab = browserTabs.find(({ url }: BrowserTab) =>
@@ -69,7 +69,7 @@ export const PortfolioBalance = React.memo(() => {
       );
       portfolioUrl.searchParams.append(
         'marketingEnabled',
-        String(!!ismultchainBalancesCollectionForMarketingEnabled),
+        String(!!isMultichainBalancesCollectionForMarketingEnabled),
       );
 
       newTabUrl = portfolioUrl.href;
@@ -95,22 +95,22 @@ export const PortfolioBalance = React.memo(() => {
     trackEvent,
     createEventBuilder,
     isEnabled,
-    ismultchainBalancesCollectionForMarketingEnabled,
+    isMultichainBalancesCollectionForMarketingEnabled,
     browserTabs,
   ]);
 
   const renderAggregatedPercentage = () => {
-    if (!multchainBalances.shouldShowAggregatedPercentage) {
+    if (!multichainBalances.shouldShowAggregatedPercentage) {
       return null;
     }
 
-    if (multchainBalances.isPortfolioVieEnabled) {
+    if (multichainBalances.isPortfolioVieEnabled) {
       return (
         <AggregatedPercentageCrossChains
           privacyMode={privacyMode}
-          totalFiatCrossChains={multchainBalances.totalFiatBalance}
+          totalFiatCrossChains={multichainBalances.totalFiatBalance}
           tokenFiatBalancesCrossChains={
-            multchainBalances.tokenFiatBalancesCrossChains
+            multichainBalances.tokenFiatBalancesCrossChains
           }
         />
       );
@@ -119,10 +119,10 @@ export const PortfolioBalance = React.memo(() => {
     return (
       <AggregatedPercentage
         privacyMode={privacyMode}
-        ethFiat={multchainBalances.aggregatedBalance.ethFiat}
-        tokenFiat={multchainBalances.aggregatedBalance.tokenFiat}
-        tokenFiat1dAgo={multchainBalances.aggregatedBalance.tokenFiat1dAgo}
-        ethFiat1dAgo={multchainBalances.aggregatedBalance.ethFiat1dAgo}
+        ethFiat={multichainBalances.aggregatedBalance.ethFiat}
+        tokenFiat={multichainBalances.aggregatedBalance.tokenFiat}
+        tokenFiat1dAgo={multichainBalances.aggregatedBalance.tokenFiat1dAgo}
+        ethFiat1dAgo={multichainBalances.aggregatedBalance.ethFiat1dAgo}
       />
     );
   };
@@ -145,7 +145,7 @@ export const PortfolioBalance = React.memo(() => {
               testID={WalletViewSelectorsIDs.TOTAL_BALANCE_TEXT}
               variant={TextVariant.DisplayMD}
             >
-              {multchainBalances.displayBalance}
+              {multichainBalances.displayBalance}
             </SensitiveText>
             <TouchableOpacity
               onPress={() => toggleIsBalanceAndAssetsHidden(!privacyMode)}

app/components/hooks/useMultichainBalances/useMultichainBalances.test.ts

@@ -75,48 +75,48 @@ describe('useMultichainBalances', () => {
   });
 
   it('returns default values when no balances are available', () => {
-    (Engine.getTotalFiatAccountBalance as jest.Mock).mockReturnValue({
+    const aggregatedBalance = {
       ethFiat: 0,
       tokenFiat: 0,
       tokenFiat1dAgo: 0,
       ethFiat1dAgo: 0,
-    });
+    };
+
+    (Engine.getTotalFiatAccountBalance as jest.Mock).mockReturnValue(
+      aggregatedBalance,
+    );
 
     const { result } = renderHook(() => useMultichainBalances());
 
-    expect(result.current.multchainBalances).toEqual({
+    expect(result.current.multichainBalances).toEqual({
       displayBalance: '0 USD',
       tokenFiatBalancesCrossChains: [],
       totalFiatBalance: 0,
       totalTokenFiat: 0,
       shouldShowAggregatedPercentage: true,
       isPortfolioVieEnabled: false,
-      aggregatedBalance: {
-        ethFiat: 0,
-        tokenFiat: 0,
-        tokenFiat1dAgo: 0,
-        ethFiat1dAgo: 0,
-      },
+      aggregatedBalance,
     });
   });
 
   it('calculates display balance correctly with ETH and token balances', () => {
-    (Engine.getTotalFiatAccountBalance as jest.Mock).mockReturnValue({
+    const aggregatedBalance = {
       ethFiat: 100,
       tokenFiat: 50,
       tokenFiat1dAgo: 45,
       ethFiat1dAgo: 95,
-    });
+    };
+
+    (Engine.getTotalFiatAccountBalance as jest.Mock).mockReturnValue(
+      aggregatedBalance,
+    );
 
     const { result } = renderHook(() => useMultichainBalances());
 
-    expect(result.current.multchainBalances.displayBalance).toBe('150 USD');
-    expect(result.current.multchainBalances.aggregatedBalance).toEqual({
-      ethFiat: 100,
-      tokenFiat: 50,
-      tokenFiat1dAgo: 45,
-      ethFiat1dAgo: 95,
-    });
+    expect(result.current.multichainBalances.displayBalance).toBe('150 USD');
+    expect(result.current.multichainBalances.aggregatedBalance).toEqual(
+      aggregatedBalance,
+    );
   });
 
   it('handles portfolio view mode correctly', () => {
@@ -142,13 +142,13 @@ describe('useMultichainBalances', () => {
           {
             chainId: '0x1',
             nativeFiatValue: 500,
-            tokenFiatBalances: [500],
+            tokenFiatBalances: [mockTokenFiatBalance],
             tokensWithBalances: mockTokensWithBalances,
           },
           {
             chainId: '0x89',
             nativeFiatValue: 500,
-            tokenFiatBalances: [500],
+            tokenFiatBalances: [mockTokenFiatBalance],
             tokensWithBalances: mockTokensWithBalances,
           },
         ],
@@ -161,14 +161,14 @@ describe('useMultichainBalances', () => {
 
     const { result } = renderHook(() => useMultichainBalances());
 
-    expect(result.current.multchainBalances.isPortfolioVieEnabled).toBe(true);
-    expect(result.current.multchainBalances.totalFiatBalance).toBe(
+    expect(result.current.multichainBalances.isPortfolioVieEnabled).toBe(true);
+    expect(result.current.multichainBalances.totalFiatBalance).toBe(
       mockTotalFiatBalance,
     );
-    expect(result.current.multchainBalances.totalTokenFiat).toBe(
+    expect(result.current.multichainBalances.totalTokenFiat).toBe(
       mockTokenFiatBalance,
     );
-    expect(result.current.multchainBalances.displayBalance).toBe('1000 USD');
+    expect(result.current.multichainBalances.displayBalance).toBe('1000 USD');
   });
 
   it('does not show aggregated percentage on test networks', () => {
@@ -177,7 +177,7 @@ describe('useMultichainBalances', () => {
     const { result } = renderHook(() => useMultichainBalances());
 
     expect(
-      result.current.multchainBalances.shouldShowAggregatedPercentage,
+      result.current.multichainBalances.shouldShowAggregatedPercentage,
     ).toBe(false);
   });
 });

app/components/hooks/useMultichainBalances/useMultichainBalances.ts

@@ -159,7 +159,7 @@ const useMultichainBalances = (): UseMultichainBalancesHook => {
   };
 
   return {
-    multchainBalances: {
+    multichainBalances: {
       displayBalance: getDisplayBalance(),
       tokenFiatBalancesCrossChains:
         totalFiatBalancesCrossChain[selectedInternalAccount?.address as string]

app/components/hooks/useMultichainBalances/useMultichainBalances.types.ts

@@ -12,5 +12,5 @@ export interface MultichainBalancesData {
 }
 
 export interface UseMultichainBalancesHook {
-  multchainBalances: MultichainBalancesData;
+  multichainBalances: MultichainBalancesData;
 }

app/core/Engine/Engine.ts

@@ -223,6 +223,14 @@ import {
   getGlobalNetworkClientId,
 } from '../../util/networks/global-network';
 import { logEngineCreation } from './utils/logger';
+import {
+  SnapControllerClearSnapStateAction,
+  SnapControllerGetSnapAction,
+  SnapControllerGetSnapStateAction,
+  SnapControllerHandleRequestAction,
+  SnapControllerStateChangeAction,
+  SnapControllerUpdateSnapStateAction,
+} from './controllers/SnapController/constants';
 
 const NON_EMPTY = 'NON_EMPTY';
 
@@ -370,7 +378,7 @@ export class Engine {
       this.controllerMessenger.getRestricted({
         name: 'AccountsController',
         allowedEvents: [
-          'SnapController:stateChange',
+          SnapControllerStateChangeAction,
           'KeyringController:accountRemoved',
           'KeyringController:stateChange',
         ],
@@ -395,7 +403,7 @@ export class Engine {
         ],
         allowedActions: [
           AccountsControllerListMultichainAccountsAction,
-          'SnapController:handleRequest',
+          SnapControllerHandleRequestAction,
         ],
       });
 
@@ -417,7 +425,10 @@ export class Engine {
     });
 
     // Set up currency rate sync
-    setupCurrencyRateSync(this.controllerMessenger, multichainRatesController);
+    setupCurrencyRateSync(
+      multichainRatesControllerMessenger,
+      multichainRatesController,
+    );
     ///: END:ONLY_INCLUDE_IF
 
     const nftController = new NftController({
@@ -677,27 +688,27 @@ export class Engine {
       // @ts-ignore
       clearSnapState: this.controllerMessenger.call.bind(
         this.controllerMessenger,
-        'SnapController:clearSnapState',
+        SnapControllerClearSnapStateAction,
       ),
       getMnemonic: getPrimaryKeyringMnemonic.bind(this),
       getUnlockPromise: getAppState.bind(this),
       getSnap: this.controllerMessenger.call.bind(
         this.controllerMessenger,
-        'SnapController:get',
+        SnapControllerGetSnapAction,
       ),
       handleSnapRpcRequest: async (args: HandleSnapRequestArgs) =>
         await handleSnapRequest(this.controllerMessenger, args),
       // eslint-disable-next-line @typescript-eslint/ban-ts-comment
       // @ts-ignore
       getSnapState: this.controllerMessenger.call.bind(
         this.controllerMessenger,
-        'SnapController:getSnapState',
+        SnapControllerGetSnapStateAction,
       ),
       // eslint-disable-next-line @typescript-eslint/ban-ts-comment
       // @ts-ignore
       updateSnapState: this.controllerMessenger.call.bind(
         this.controllerMessenger,
-        'SnapController:updateSnapState',
+        SnapControllerUpdateSnapStateAction,
       ),
       maybeUpdatePhishingList: this.controllerMessenger.call.bind(
         this.controllerMessenger,
@@ -1029,7 +1040,7 @@ export class Engine {
           'KeyringController:getState',
           'KeyringController:getAccounts',
 
-          'SnapController:handleRequest',
+          SnapControllerHandleRequestAction,
           'UserStorageController:enableProfileSyncing',
         ],
         allowedEvents: ['KeyringController:unlock', 'KeyringController:lock'],
@@ -1076,7 +1087,7 @@ export class Engine {
       messenger: this.controllerMessenger.getRestricted({
         name: 'UserStorageController',
         allowedActions: [
-          'SnapController:handleRequest',
+          SnapControllerHandleRequestAction,
           'KeyringController:getState',
           'KeyringController:addNewAccount',
           'AuthenticationController:getBearerToken',
@@ -2229,7 +2240,8 @@ export default {
     keyringState: KeyringControllerState | null = null,
     metaMetricsId?: string,
   ) {
-    instance = Engine.instance || new Engine(state, keyringState, metaMetricsId);
+    instance =
+      Engine.instance || new Engine(state, keyringState, metaMetricsId);
     Object.freeze(instance);
     return instance;
   },

app/core/Engine/constants.ts

@@ -1,3 +1,6 @@
+///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
+import { SnapControllerStateChangeAction } from './controllers/SnapController/constants';
+///: END:ONLY_INCLUDE_IF
 /**
  * Messageable modules that are part of the Engine's context, but are not defined with state.
  * TODO: Replace with type guard once consistent inheritance for non-controllers is implemented. See: https://github.com/MetaMask/decisions/pull/41
@@ -34,7 +37,7 @@ export const BACKGROUND_STATE_CHANGE_EVENT_NAMES = [
   'TokensController:stateChange',
   'TransactionController:stateChange',
   ///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
-  'SnapController:stateChange',
+  SnapControllerStateChangeAction,
   'SnapsRegistry:stateChange',
   'SubjectMetadataController:stateChange',
   'AuthenticationController:stateChange',

app/core/Engine/controllers/AccountsController/utils.test.ts

@@ -11,6 +11,7 @@ import { withScope } from '@sentry/react-native';
 import { AGREED, METRICS_OPT_IN } from '../../../../constants/storage';
 import StorageWrapper from '../../../../store/storage-wrapper';
 import { logAccountsControllerCreation } from './logger';
+import { SnapControllerStateChangeAction } from '../SnapController/constants';
 
 jest.mock('@sentry/react-native', () => ({
   withScope: jest.fn(),
@@ -33,7 +34,7 @@ describe('accountControllersUtils', () => {
       accountsControllerMessenger = globalMessenger.getRestricted({
         name: 'AccountsController',
         allowedEvents: [
-          'SnapController:stateChange',
+          SnapControllerStateChangeAction,
           'KeyringController:accountRemoved',
           'KeyringController:stateChange',
         ],

app/core/Engine/controllers/RatesController/subscriptions.ts

@@ -2,25 +2,31 @@ import {
   RatesController,
   CurrencyRateState,
   CurrencyRateController,
+  CurrencyRateControllerEvents,
 } from '@metamask/assets-controllers';
 import Logger from '../../../../util/Logger';
+import { RestrictedControllerMessenger } from '@metamask/base-controller';
+
+// FIXME: This messenger type is not exported on `@metamask/assets-controllers`, so declare it here for now:
+type CurrencyRateControllerMessenger = RestrictedControllerMessenger<
+  CurrencyRateController['name'],
+  never,
+  CurrencyRateControllerEvents,
+  never,
+  CurrencyRateControllerEvents['type']
+>;
 
 /**
  * Sets up subscription to sync CurrencyRateController changes with RatesController
  * @param controllerMessenger - The main controller messenger
  * @param ratesController - The RatesController instance to sync with
  */
 export const setupCurrencyRateSync = (
-  controllerMessenger: {
-    subscribe: (
-      eventName: string,
-      handler: (state: CurrencyRateState) => void,
-    ) => void;
-  },
+  controllerMessenger: CurrencyRateControllerMessenger,
   ratesController: RatesController,
 ): void => {
   controllerMessenger.subscribe(
-    `${CurrencyRateController.name}:stateChange`,
+    'CurrencyRateController:stateChange',
     (state: CurrencyRateState) => {
       if (state.currentCurrency) {
         ratesController