Skip to content

Commit

Permalink
refactor: move getProviderConfig out of ducks/metamask.js to `sha…
Browse files Browse the repository at this point in the history
…red/selectors/networks.ts` (#27646)

Converts some from functions from JS to TS. Also updates some functions
to match the actual expect return values.

Why only these functions? I'm trying to solve circular dependency
issues. `getProviderConfig` is so widely used in the codebase, and makes
use of multiple selectors itself, it makes it very complicated to
untangle. I've put it in a new file (`seelctors/networks.ts`) just to,
hopefully temporarily, simplify untangling other circular dependency
issues.


<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.


## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27646?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**


### **Before**


### **After**


## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.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.
-->
  • Loading branch information
davidmurdoch authored Nov 21, 2024
1 parent e748576 commit 5116c32
Show file tree
Hide file tree
Showing 98 changed files with 462 additions and 326 deletions.
3 changes: 1 addition & 2 deletions app/scripts/lib/ppom/ppom-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import { SIGNING_METHODS } from '../../../../shared/constants/transaction';
import { PreferencesController } from '../../controllers/preferences-controller';
import { AppStateController } from '../../controllers/app-state-controller';
import { SECURITY_ALERT_RESPONSE_CHECKING_CHAIN } from '../../../../shared/constants/security-provider';
// eslint-disable-next-line import/no-restricted-paths
import { getProviderConfig } from '../../../../ui/ducks/metamask/metamask';
import { getProviderConfig } from '../../../../shared/modules/selectors/networks';
import { trace, TraceContext, TraceName } from '../../../../shared/lib/trace';
import {
generateSecurityAlertId,
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/lib/ppom/ppom-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export async function isChainSupported(chainId: Hex): Promise<boolean> {
`Error fetching supported chains from security alerts API`,
);
}
return supportedChainIds.includes(chainId);
return supportedChainIds.includes(chainId as Hex);
}

function normalizePPOMRequest(
Expand Down
6 changes: 2 additions & 4 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,8 @@ import {
} from '../../shared/lib/transactions-controller-utils';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { getCurrentChainId } from '../../ui/selectors';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { getProviderConfig } from '../../ui/ducks/metamask/metamask';
import { getCurrentChainId } from '../../ui/selectors/selectors';
import { getProviderConfig } from '../../shared/modules/selectors/networks';
import { endTrace, trace } from '../../shared/lib/trace';
// eslint-disable-next-line import/no-restricted-paths
import { isSnapId } from '../../ui/helpers/utils/snaps';
Expand Down
2 changes: 2 additions & 0 deletions shared/constants/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export const ALLOWED_BRIDGE_CHAIN_IDS = [
CHAIN_IDS.BASE,
];

export type AllowedBridgeChainIds = (typeof ALLOWED_BRIDGE_CHAIN_IDS)[number];

export const BRIDGE_DEV_API_BASE_URL = 'https://bridge.dev-api.cx.metamask.io';
export const BRIDGE_PROD_API_BASE_URL = 'https://bridge.api.cx.metamask.io';
export const BRIDGE_API_BASE_URL = process.env.BRIDGE_USE_DEV_APIS
Expand Down
2 changes: 2 additions & 0 deletions shared/constants/multichain/networks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export type MultichainProviderConfig = ProviderConfigWithImageUrl & {
isAddressCompatible: (address: string) => boolean;
};

export type MultichainNetworkIds = `${MultichainNetworks}`;

export enum MultichainNetworks {
BITCOIN = 'bip122:000000000019d6689c085ae165831e93',
BITCOIN_TESTNET = 'bip122:000000000933ea01ad0ee984209779ba',
Expand Down
2 changes: 2 additions & 0 deletions shared/constants/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export const NETWORK_TYPES = {
LINEA_MAINNET: 'linea-mainnet',
} as const;

export type NetworkTypes = (typeof NETWORK_TYPES)[keyof typeof NETWORK_TYPES];

/**
* An object containing shortcut names for any non-builtin network. We need
* this to be able to differentiate between networks that require custom
Expand Down
114 changes: 114 additions & 0 deletions shared/modules/selectors/networks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import {
RpcEndpointType,
type NetworkConfiguration,
type NetworkState as _NetworkState,
} from '@metamask/network-controller';
import { createSelector } from 'reselect';
import { NetworkStatus } from '../../constants/network';
import { createDeepEqualSelector } from './util';

export type NetworkState = { metamask: _NetworkState };

export type NetworkConfigurationsState = {
metamask: {
networkConfigurations: Record<
string,
MetaMaskExtensionNetworkConfiguration
>;
};
};

export type SelectedNetworkClientIdState = {
metamask: {
selectedNetworkClientId: string;
};
};

export type MetaMaskExtensionNetworkConfiguration = NetworkConfiguration;

export type NetworkConfigurationsByChainIdState = {
metamask: Pick<_NetworkState, 'networkConfigurationsByChainId'>;
};

export type ProviderConfigState = NetworkConfigurationsByChainIdState &
SelectedNetworkClientIdState;

export const getNetworkConfigurationsByChainId = createDeepEqualSelector(
(state: NetworkConfigurationsByChainIdState) =>
state.metamask.networkConfigurationsByChainId,
(networkConfigurationsByChainId) => networkConfigurationsByChainId,
);

export function getSelectedNetworkClientId(
state: SelectedNetworkClientIdState,
) {
return state.metamask.selectedNetworkClientId;
}

/**
* Get the provider configuration for the current selected network.
*
* @param state - Redux state object.
*/
export const getProviderConfig = createSelector(
(state: ProviderConfigState) => getNetworkConfigurationsByChainId(state),
getSelectedNetworkClientId,
(networkConfigurationsByChainId, selectedNetworkClientId) => {
for (const network of Object.values(networkConfigurationsByChainId)) {
for (const rpcEndpoint of network.rpcEndpoints) {
if (rpcEndpoint.networkClientId === selectedNetworkClientId) {
const blockExplorerUrl =
network.defaultBlockExplorerUrlIndex === undefined
? undefined
: network.blockExplorerUrls?.[
network.defaultBlockExplorerUrlIndex
];

return {
chainId: network.chainId,
ticker: network.nativeCurrency,
rpcPrefs: { ...(blockExplorerUrl && { blockExplorerUrl }) },
type:
rpcEndpoint.type === RpcEndpointType.Custom
? 'rpc'
: rpcEndpoint.networkClientId,
...(rpcEndpoint.type === RpcEndpointType.Custom && {
id: rpcEndpoint.networkClientId,
nickname: network.name,
rpcUrl: rpcEndpoint.url,
}),
};
}
}
}
return undefined; // should not be reachable
},
);

export function getNetworkConfigurations(
state: NetworkConfigurationsState,
): Record<string, MetaMaskExtensionNetworkConfiguration> {
return state.metamask.networkConfigurations;
}

/**
* Returns true if the currently selected network is inaccessible or whether no
* provider has been set yet for the currently selected network.
*
* @param state - Redux state object.
*/
export function isNetworkLoading(state: NetworkState) {
const selectedNetworkClientId = getSelectedNetworkClientId(state);
return (
selectedNetworkClientId &&
state.metamask.networksMetadata[selectedNetworkClientId].status !==
NetworkStatus.Available
);
}

export function getInfuraBlocked(state: NetworkState) {
return (
state.metamask.networksMetadata[getSelectedNetworkClientId(state)]
.status === NetworkStatus.Blocked
);
}
9 changes: 4 additions & 5 deletions shared/modules/selectors/smart-transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
// eslint-disable-next-line import/no-restricted-paths
} from '../../../ui/selectors/selectors'; // TODO: Migrate shared selectors to this file.
import { isProduction } from '../environment';
import { NetworkState } from './networks';

type SmartTransactionsMetaMaskState = {
metamask: {
Expand Down Expand Up @@ -113,9 +114,7 @@ export const getCurrentChainSupportsSmartTransactions = (
return getAllowedSmartTransactionsChainIds().includes(chainId);
};

const getIsAllowedRpcUrlForSmartTransactions = (
state: SmartTransactionsMetaMaskState,
) => {
const getIsAllowedRpcUrlForSmartTransactions = (state: NetworkState) => {
const chainId = getCurrentChainId(state);
if (!isProduction() || SKIP_STX_RPC_URL_CHECK_CHAIN_IDS.includes(chainId)) {
// Allow any STX RPC URL in development and testing environments or for specific chain IDs.
Expand All @@ -131,7 +130,7 @@ const getIsAllowedRpcUrlForSmartTransactions = (
};

export const getSmartTransactionsEnabled = (
state: SmartTransactionsMetaMaskState,
state: SmartTransactionsMetaMaskState & NetworkState,
): boolean => {
const supportedAccount = accountSupportsSmartTx(state);
// TODO: Create a new proxy service only for MM feature flags.
Expand All @@ -150,7 +149,7 @@ export const getSmartTransactionsEnabled = (
};

export const getIsSmartTransaction = (
state: SmartTransactionsMetaMaskState,
state: SmartTransactionsMetaMaskState & NetworkState,
): boolean => {
const smartTransactionsPreferenceEnabled =
getSmartTransactionsPreferenceEnabled(state);
Expand Down
File renamed without changes.
8 changes: 6 additions & 2 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -402,26 +402,30 @@
"name": "Custom Mainnet RPC",
"nativeCurrency": "ETH",
"defaultRpcEndpointIndex": 0,
"ticker": "ETH",
"rpcEndpoints": [
{
"type": "custom",
"url": "https://testrpc.com",
"networkClientId": "testNetworkConfigurationId"
}
]
],
"blockExplorerUrls": []
},
"0x5": {
"chainId": "0x5",
"name": "Goerli",
"nativeCurrency": "ETH",
"defaultRpcEndpointIndex": 0,
"ticker": "ETH",
"rpcEndpoints": [
{
"type": "custom",
"url": "https://goerli.com",
"networkClientId": "goerli"
}
]
],
"blockExplorerUrls": []
}
},
"internalAccounts": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import React, { useEffect, useRef, useState, useContext, useMemo } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
getCurrentNetwork,
getNetworkConfigurationsByChainId,
getPreferences,
} from '../../../../../selectors';
import { getCurrentNetwork, getPreferences } from '../../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks';
import {
Box,
ButtonBase,
Expand Down
2 changes: 1 addition & 1 deletion ui/components/app/assets/asset-list/asset-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import {
getAllDetectedTokensForSelectedAddress,
getDetectedTokensInCurrentNetwork,
getIstokenDetectionInactiveOnNonMainnetSupportedNetwork,
getNetworkConfigurationsByChainId,
getPreferences,
getSelectedAccount,
} from '../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../shared/modules/selectors/networks';
import {
getMultichainIsEvm,
getMultichainSelectedAccountCachedBalance,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import {
getCurrentChainId,
getCurrentNetwork,
getPreferences,
getNetworkConfigurationsByChainId,
getChainIdsToPoll,
getShouldHideZeroBalanceTokens,
getSelectedAccount,
} from '../../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks';
import { useI18nContext } from '../../../../../hooks/useI18nContext';
import { SelectableListItem } from '../sort-control/sort-control';
import { Text } from '../../../../component-library/text/text';
Expand Down
6 changes: 2 additions & 4 deletions ui/components/app/currency-input/currency-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import { Box } from '../../component-library';
import { BlockSize } from '../../../helpers/constants/design-system';
import UnitInput from '../../ui/unit-input';
import CurrencyDisplay from '../../ui/currency-display';
import {
getNativeCurrency,
getProviderConfig,
} from '../../../ducks/metamask/metamask';
import { getNativeCurrency } from '../../../ducks/metamask/metamask';
import { getProviderConfig } from '../../../../shared/modules/selectors/networks';
import {
getCurrentChainId,
getCurrentCurrency,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import {
getCurrentChainId,
getCurrentNetwork,
getDetectedTokensInCurrentNetwork,
getNetworkConfigurationsByChainId,
getPreferences,
} from '../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../shared/modules/selectors/networks';

import Popover from '../../../ui/popover';
import Box from '../../../ui/box';
Expand Down
6 changes: 4 additions & 2 deletions ui/components/app/detected-token/detected-token.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import {
getAllDetectedTokensForSelectedAddress,
getCurrentChainId,
getDetectedTokensInCurrentNetwork,
getNetworkConfigurationsByChainId,
getPreferences,
getSelectedNetworkClientId,
} from '../../../selectors';
import {
getSelectedNetworkClientId,
getNetworkConfigurationsByChainId,
} from '../../../../shared/modules/selectors/networks';
import { MetaMetricsContext } from '../../../contexts/metametrics';

import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import * as actions from '../../../store/actions';
import {
getAllEnabledNetworks,
getNetworkIdentifier,
isNetworkLoading,
} from '../../../selectors';
import { getProviderConfig } from '../../../ducks/metamask/metamask';
import {
getProviderConfig,
isNetworkLoading,
} from '../../../../shared/modules/selectors/networks';
import LoadingNetworkScreen from './loading-network-screen.component';

const DEPRECATED_TEST_NET_CHAINIDS = ['0x3', '0x2a', '0x4'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { connect } from 'react-redux';
import { compose } from 'redux';
import withModalProps from '../../../../helpers/higher-order-components/with-modal-props';
import { removeNetwork } from '../../../../store/actions';
import { getNetworkConfigurationsByChainId } from '../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../shared/modules/selectors/networks';
import ConfirmDeleteNetwork from './confirm-delete-network.component';

const mapStateToProps = (state, ownProps) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../../shared/constants/metametrics';
import {
getCurrentChainId,
getNetworkConfigurationsByChainId,
} from '../../../../selectors';
import { getCurrentChainId } from '../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../shared/modules/selectors/networks';

function mapStateToProps(state) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, { useContext } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { getBlockExplorerLink } from '@metamask/etherscan-link';
import { type TransactionMeta } from '@metamask/transaction-controller';
import { type NetworkClientConfiguration } from '@metamask/network-controller';
import {
getRpcPrefsForCurrentProvider,
getTransaction,
Expand Down Expand Up @@ -38,19 +37,15 @@ export default function TransactionAlreadyConfirmed() {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(getTransaction as any)(state, originalTransactionId),
);
const rpcPrefs: NetworkClientConfiguration = useSelector(
getRpcPrefsForCurrentProvider,
);
const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider);

const viewTransaction = () => {
// TODO: Fix getBlockExplorerLink arguments compatible with the actual controller types
const blockExplorerLink = getBlockExplorerLink(
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
transaction as any,
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
rpcPrefs as any,
rpcPrefs,
);
global.platform.openTab({
url: blockExplorerLink,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { setShowMultiRpcModal } from '../../../store/actions';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { getEnvironmentType } from '../../../../app/scripts/lib/util';
import { getNetworkConfigurationsByChainId } from '../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../shared/modules/selectors/networks';
import { ENVIRONMENT_TYPE_POPUP } from '../../../../shared/constants/app';
import NetworkListItem from './network-list-item/network-list-item';

Expand Down
Loading

0 comments on commit 5116c32

Please sign in to comment.