Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move getProviderConfig out of ducks/metamask.js to shared/selectors/networks.ts #27646

Merged
merged 14 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this cast necessary? It looks like it already has the type Hex

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not. I think this is just due to doing so many refactors and merges. I can fix it here, or fix it in a follow up PR.

}

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];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be unused. Maybe not needed anymore? Or is this used by a later PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise for AllowedBridgeChainIds and MultichainNetworkIds


/**
* 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 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit confusing 🤔 It's only nested under metamask in the UI, but this is in shared.

I guess if you just wanted a type representing the actual NetworkController state you'd import the type from there instead though.

Copy link
Member

@Gudahtt Gudahtt Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever we do, it will change again soon when we unflatten the metamask state, so we can re-evaluate what we want to do then. All of the background state types we use in the UI are a mess right now.


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 @@ -399,26 +399,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
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import {
getPreferences,
getSelectedInternalAccount,
getShouldHideZeroBalanceTokens,
getNetworkConfigurationsByChainId,
getChainIdsToPoll,
} 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
2 changes: 1 addition & 1 deletion ui/components/app/detected-token/detected-token.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
import {
getCurrentChainId,
getDetectedTokensInCurrentNetwork,
getSelectedNetworkClientId,
} from '../../../selectors';
import { getSelectedNetworkClientId } 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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,6 @@ jest.mock('../../../selectors', () => {
getAccountType: mockGetAccountType,
getSelectedInternalAccount: mockGetSelectedAccount,
getCurrentChainId: jest.fn(() => '0x5'),
getSelectedNetworkClientId: jest.fn(() => 'goerli'),
getNetworkConfigurationsByChainId: jest.fn(() => ({
'0x5': {
chainId: '0x5',
rpcEndpoints: [{ networkClientId: 'goerli' }],
},
})),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions were moved, and the tests pass without these being mocked. so I removed them.

};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
getCustodyAccountDetails,
getIsCustodianSupportedChain,
} from '../../../selectors/institutional/selectors';
import { getProviderConfig } from '../../../ducks/metamask/metamask';
import { getProviderConfig } from '../../../../shared/modules/selectors/networks';
///: END:ONLY_INCLUDE_IF
import SelectedAccount from './selected-account.component';

Expand Down
Loading
Loading