refactor: Simplify handleNetworkSwitch helper

The `handleNetworkSwitch` helper function no longer requires the `frequentRpcList` state as a parameter. Instead it takes a reference to the `PreferencesController`, and gets the RPC list state from that. Typically it would be preferrable to reference controller state from Redux rather than accesing it directly from the Engine global. However, this function will be called from non-UI contexts, so it has to get controller state from global context regardless. This change was done to simplify PR #6872, which is part of MetaMask/mobile-planning#798
MetaMask · Jul 25, 2023 · 8d209fe · 8d209fe
1 parent 19e52f7
commit 8d209fe
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 43 deletions.
diff --git a/app/components/Nav/App/index.js b/app/components/Nav/App/index.js
@@ -34,7 +34,7 @@ import Logger from '../../../util/Logger';
 import { trackErrorAsAnalytics } from '../../../util/analyticsV2';
 import { routingInstrumentation } from '../../../util/sentryUtils';
 import Analytics from '../../../core/Analytics/Analytics';
-import { connect, useSelector, useDispatch } from 'react-redux';
+import { connect, useDispatch } from 'react-redux';
 import {
   CURRENT_APP_VERSION,
   EXISTING_USER,
@@ -86,7 +86,6 @@ import EditAccountName from '../../Views/EditAccountName/EditAccountName';
 import WC2Manager, {
   isWC2Enabled,
 } from '../../../../app/core/WalletConnect/WalletConnectV2';
-import { selectFrequentRpcList } from '../../../selectors/preferencesController';
 
 const clearStackNavigatorOptions = {
   headerShown: false,
@@ -232,7 +231,6 @@ const App = ({ userLoggedIn }) => {
       dispatch(setCurrentBottomNavRoute(route));
     }
   };
-  const frequentRpcList = useSelector(selectFrequentRpcList);
 
   useEffect(() => {
     if (prevNavigator.current || !navigator) return;
@@ -315,7 +313,6 @@ const App = ({ userLoggedIn }) => {
             navigator.dispatch?.(CommonActions.navigate(params));
           },
         },
-        frequentRpcList,
         dispatch,
       });
       if (!prevNavigator.current) {
@@ -336,7 +333,7 @@ const App = ({ userLoggedIn }) => {
       }
       prevNavigator.current = navigator;
     }
-  }, [dispatch, handleDeeplink, frequentRpcList, navigator]);
+  }, [dispatch, handleDeeplink, navigator]);
 
   useEffect(() => {
     const initAnalytics = async () => {

diff --git a/app/components/Views/SendFlow/AddressTo/AddressTo.tsx b/app/components/Views/SendFlow/AddressTo/AddressTo.tsx
@@ -10,7 +10,6 @@ import { NetworkSwitchErrorType } from '../../../../constants/error';
 import Routes from '../../../../constants/navigation/Routes';
 import Engine from '../../../../core/Engine';
 import { selectNetwork } from '../../../../selectors/networkController';
-import { selectFrequentRpcList } from '../../../../selectors/preferencesController';
 import { handleNetworkSwitch } from '../../../../util/networks';
 import { AddressTo } from '../../../UI/AddressInputs';
 import { createQRScannerNavDetails } from '../../QRScanner';
@@ -34,16 +33,19 @@ const SendFlowAddressTo = ({
 
   const network = useSelector(selectNetwork);
 
-  const frequentRpcList = useSelector(selectFrequentRpcList);
-
   const showAlertAction = (config: any) => dispatch(showAlert(config));
 
   const onHandleNetworkSwitch = (chain_id: string) => {
     try {
-      const { NetworkController, CurrencyRateController } = Engine.context;
-      const networkSwitch = handleNetworkSwitch(chain_id, frequentRpcList, {
+      const {
+        NetworkController,
+        CurrencyRateController,
+        PreferencesController,
+      } = Engine.context;
+      const networkSwitch = handleNetworkSwitch(chain_id, {
         networkController: NetworkController,
         currencyRateController: CurrencyRateController,
+        preferencesController: PreferencesController,
       });
 
       if (!networkSwitch) return;

diff --git a/app/components/Views/SendFlow/SendTo/index.js b/app/components/Views/SendFlow/SendTo/index.js
@@ -66,7 +66,6 @@ import {
   selectTicker,
 } from '../../../../selectors/networkController';
 import {
-  selectFrequentRpcList,
   selectIdentities,
   selectSelectedAddress,
 } from '../../../../selectors/preferencesController';
@@ -139,10 +138,6 @@ class SendFlow extends PureComponent {
      * Indicates whether the current transaction is a deep link transaction
      */
     isPaymentRequest: PropTypes.bool,
-    /**
-     * Frequent RPC list from PreferencesController
-     */
-    frequentRpcList: PropTypes.array,
     /**
      * Boolean that indicates if the network supports buy
      */
@@ -253,11 +248,16 @@ class SendFlow extends PureComponent {
 
   handleNetworkSwitch = (chainId) => {
     try {
-      const { NetworkController, CurrencyRateController } = Engine.context;
-      const { showAlert, frequentRpcList } = this.props;
-      const network = handleNetworkSwitch(chainId, frequentRpcList, {
+      const {
+        NetworkController,
+        CurrencyRateController,
+        PreferencesController,
+      } = Engine.context;
+      const { showAlert } = this.props;
+      const network = handleNetworkSwitch(chainId, {
         networkController: NetworkController,
         currencyRateController: CurrencyRateController,
+        preferencesController: PreferencesController,
       });
 
       if (!network) return;
@@ -642,7 +642,6 @@ const mapStateToProps = (state) => ({
   network: selectNetwork(state),
   providerType: selectProviderType(state),
   isPaymentRequest: state.transaction.paymentRequest,
-  frequentRpcList: selectFrequentRpcList(state),
   isNativeTokenBuySupported: isNetworkBuyNativeTokenSupported(
     selectChainId(state),
     getRampNetworks(state),

diff --git a/app/core/DeeplinkManager.js b/app/core/DeeplinkManager.js
@@ -29,10 +29,9 @@ import { isNetworkBuySupported } from '../components/UI/Ramp/utils';
 import { Minimizer } from './NativeModules';
 
 class DeeplinkManager {
-  constructor({ navigation, frequentRpcList, dispatch }) {
+  constructor({ navigation, dispatch }) {
     this.navigation = navigation;
     this.pendingDeeplink = null;
-    this.frequentRpcList = frequentRpcList;
     this.dispatch = dispatch;
   }
 
@@ -48,10 +47,12 @@ class DeeplinkManager {
    * @param switchToChainId - Corresponding chain id for new network
    */
   _handleNetworkSwitch = (switchToChainId) => {
-    const { NetworkController, CurrencyRateController } = Engine.context;
-    const network = handleNetworkSwitch(switchToChainId, this.frequentRpcList, {
+    const { NetworkController, CurrencyRateController, PreferencesController } =
+      Engine.context;
+    const network = handleNetworkSwitch(switchToChainId, {
       networkController: NetworkController,
       currencyRateController: CurrencyRateController,
+      preferencesController: PreferencesController,
     });
 
     if (!network) return;
@@ -432,13 +433,12 @@ class DeeplinkManager {
 let instance = null;
 
 const SharedDeeplinkManager = {
-  init: ({ navigation, frequentRpcList, dispatch }) => {
+  init: ({ navigation, dispatch }) => {
     if (instance) {
       return;
     }
     instance = new DeeplinkManager({
       navigation,
-      frequentRpcList,
       dispatch,
     });
   },

diff --git a/app/util/networks/handleNetworkSwitch.ts b/app/util/networks/handleNetworkSwitch.ts
@@ -1,14 +1,19 @@
 import { getNetworkTypeById } from './index';
+import type { NetworkController } from '@metamask/network-controller';
+import type { PreferencesController } from '@metamask/preferences-controller';
+import type { CurrencyRateController } from '@metamask/assets-controllers';
 
 const handleNetworkSwitch = (
   switchToChainId: string,
-  frequentRpcList: {
-    rpcUrl: string;
-    chainId: string;
-    ticker: string;
-    nickname: string;
-  }[],
-  { networkController, currencyRateController }: any,
+  {
+    networkController,
+    currencyRateController,
+    preferencesController,
+  }: {
+    networkController: NetworkController;
+    currencyRateController: CurrencyRateController;
+    preferencesController: PreferencesController;
+  },
 ): string | undefined => {
   // If not specified, use the current network
   if (!switchToChainId) {
@@ -23,8 +28,8 @@ const handleNetworkSwitch = (
     return;
   }
 
-  const rpc = frequentRpcList.find(
-    ({ chainId }: { chainId: string }) => chainId === switchToChainId,
+  const rpc = preferencesController.state.frequentRpcList.find(
+    ({ chainId }) => chainId === switchToChainId,
   );
 
   if (rpc) {

diff --git a/app/util/networks/index.test.ts b/app/util/networks/index.test.ts
@@ -35,6 +35,9 @@ jest.mock('./../../core/Engine', () => ({
         },
       },
     },
+    PreferencesController: {
+      state: {},
+    },
   },
 }));
 
@@ -201,18 +204,17 @@ describe('NetworkUtils::handleNetworkSwitch', () => {
     },
   ];
 
-  const { NetworkController, CurrencyRateController } = Engine.context as any;
+  const { NetworkController, CurrencyRateController, PreferencesController } =
+    Engine.context as any;
 
   it('should change networks to the provided one', () => {
     const network = mockRPCFrequentList[0];
-    const newNetwork = handleNetworkSwitch(
-      network.chainId,
-      mockRPCFrequentList,
-      {
-        networkController: NetworkController,
-        currencyRateController: CurrencyRateController,
-      },
-    );
+    PreferencesController.state.frequentRpcList = mockRPCFrequentList;
+    const newNetwork = handleNetworkSwitch(network.chainId, {
+      networkController: NetworkController,
+      currencyRateController: CurrencyRateController,
+      preferencesController: PreferencesController,
+    });
     expect(newNetwork).toBe(network.nickname);
   });
 });