Skip to content

Commit

Permalink
Remove networkId from NetworkController (#1633)
Browse files Browse the repository at this point in the history
## Explanation

Wallets shouldn't be directly concerned about the network ID as this is
more of a node concept for gossip. What wallets really care about is
chain ID as that is the correct value to use to identify a chain, build
transactions, etc. Although these two values usually match (ignoring
hex/dec formatting), there are
[exceptions](https://medium.com/@pedrouid/chainid-vs-networkid-how-do-they-differ-on-ethereum-eec2ed41635b).

We want to remove `networkId` from the NetworkController state to
encourage the replacement of networkId with chainId in any usage
downstream (extension, mobile, etc). It will still be possible to get
networkId using the rpc client to make a call to the `net_version`
method for cases that truly still rely on it.

## References

* Fixes
[mmp-1068](MetaMask/MetaMask-planning#1068)

## Changelog

### `@metamask/controller-utils`

- **BREAKING**: `NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP` renamed to
`CHAIN_ID_TO_ETHERS_NETWORK_NAME_MAP ` and is now a mapping of `Hex`
chain ID to `BuiltInNetworkName`
- **REMOVED**: `NetworkId` constant and type

### `@metamask/ens-controller`

- **CHANGED**: From Network state, uses `providerConfig.chainId` instead
of `networkId` to determine ENS compatability

### `@metamask/network-controller`

- **REMOVED**: `NetworkId` type
- **REMOVED**:  From `NetworkState` removed `networkId` field
- **CHANGED**: No longer calls `net_version` to determine network status
(only relies on `eth_getBlockByNumber` now)
- **CHANGED**: For Built-in Infura Networks, `net_version` is no longer
locally resolved by the scaffold middleware


### `@metamask/transaction-controller`

- **BREAKING**: Uses `chainId` and `txParams.chainId` to determine if a
`TransactionMeta` object belongs to the current chain. No longer
considers `networkID`
- **BREAKING**: `TransactionMeta.chainId` is now a required field
- **REMOVED**: From `RemoteTransactionSourceRequest` removed
`currentNetworkId` field
- **CHANGED**: From `RemoteTransactionSource` updated
`isSupportedNetwork()` params to exclude networkId
- **DEPRECATED**: `TransactionMeta.networkID` is deprecated and marked
readonly
  • Loading branch information
jiexi authored and MajorLift committed Oct 11, 2023
1 parent 5c48d9d commit a5615ca
Show file tree
Hide file tree
Showing 23 changed files with 3,501 additions and 6,052 deletions.
3 changes: 1 addition & 2 deletions packages/assets-controllers/src/NftDetectionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,8 @@ export interface ApiNftCreator {
*
* NftDetection configuration
* @property interval - Polling interval used to fetch new token rates
* @property networkType - Network type ID as per net_version
* @property chainId - Current chain ID
* @property selectedAddress - Vault selected address
* @property tokens - List of tokens associated with the active vault
*/
export interface NftDetectionConfig extends BaseConfig {
interval: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,6 @@ function buildNetworkControllerStateWithProviderConfig(
return {
selectedNetworkClientId,
providerConfig,
networkId: '1',
networksMetadata: {
[selectedNetworkClientId]: {
EIPS: {},
Expand Down
24 changes: 15 additions & 9 deletions packages/controller-utils/src/constants.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { NetworkType, NetworksTicker, ChainId, NetworkId } from './types';
import {
NetworkType,
NetworksTicker,
ChainId,
BuiltInNetworkName,
} from './types';

export const RPC = 'rpc';
export const FALL_BACK_VS_CURRENCY = 'ETH';
Expand Down Expand Up @@ -125,13 +130,14 @@ export enum ApprovalType {
WatchAsset = 'wallet_watchAsset',
}

export const NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP: Record<
NetworkId,
NetworkType
export const CHAIN_ID_TO_ETHERS_NETWORK_NAME_MAP: Record<
ChainId,
BuiltInNetworkName
> = {
[NetworkId.goerli]: NetworkType.goerli,
[NetworkId.sepolia]: NetworkType.sepolia,
[NetworkId.mainnet]: NetworkType.mainnet,
[NetworkId['linea-goerli']]: NetworkType['linea-goerli'],
[NetworkId['linea-mainnet']]: NetworkType['linea-mainnet'],
[ChainId.goerli]: BuiltInNetworkName.Goerli,
[ChainId.sepolia]: BuiltInNetworkName.Sepolia,
[ChainId.mainnet]: BuiltInNetworkName.Mainnet,
[ChainId['linea-goerli']]: BuiltInNetworkName.LineaGoerli,
[ChainId['linea-mainnet']]: BuiltInNetworkName.LineaMainnet,
[ChainId.aurora]: BuiltInNetworkName.Aurora,
};
12 changes: 0 additions & 12 deletions packages/controller-utils/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,6 @@ export const ChainId = {
} as const;
export type ChainId = (typeof ChainId)[keyof typeof ChainId];

/**
* Decimal string network IDs of built-in Infura networks, by name.
*/
export const NetworkId = {
[InfuraNetworkType.mainnet]: '1',
[InfuraNetworkType.goerli]: '5',
[InfuraNetworkType.sepolia]: '11155111',
[InfuraNetworkType['linea-goerli']]: '59140',
[InfuraNetworkType['linea-mainnet']]: '59144',
} as const;
export type NetworkId = (typeof NetworkId)[keyof typeof NetworkId];

export enum NetworksTicker {
mainnet = 'ETH',
goerli = 'GoerliETH',
Expand Down
12 changes: 1 addition & 11 deletions packages/ens-controller/src/EnsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand Down Expand Up @@ -445,7 +444,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: null,
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand All @@ -464,9 +462,8 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1544',
providerConfig: {
chainId: toHex(1),
chainId: toHex(0),
type: NetworkType.mainnet,
ticker: NetworksTicker.mainnet,
},
Expand All @@ -490,7 +487,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand All @@ -514,7 +510,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand All @@ -537,7 +532,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand All @@ -563,7 +557,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand All @@ -589,7 +582,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand Down Expand Up @@ -617,7 +609,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand All @@ -644,7 +635,6 @@ describe('EnsController', () => {
provider: getProvider(),
onNetworkStateChange: (listener) => {
listener({
networkId: '1',
providerConfig: {
chainId: toHex(1),
type: NetworkType.mainnet,
Expand Down
49 changes: 16 additions & 33 deletions packages/ens-controller/src/EnsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,22 @@ import type {
import { Web3Provider } from '@ethersproject/providers';
import type { RestrictedControllerMessenger } from '@metamask/base-controller';
import { BaseControllerV2 } from '@metamask/base-controller';
import type { ChainId } from '@metamask/controller-utils';
import {
normalizeEnsName,
isValidHexAddress,
toChecksumHexAddress,
NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP,
CHAIN_ID_TO_ETHERS_NETWORK_NAME_MAP,
convertHexToDecimal,
} from '@metamask/controller-utils';
import type { NetworkState } from '@metamask/network-controller';
import type { Hex } from '@metamask/utils';
import { createProjectLogger, hasProperty } from '@metamask/utils';
import { createProjectLogger } from '@metamask/utils';
import ensNetworkMap from 'ethereum-ens-network-map';
import { toASCII } from 'punycode/';

const log = createProjectLogger('ens-controller');

/**
* Checks whether the given string is a known network ID.
*
* @param networkId - Network id.
* @returns Boolean indicating if the network ID is recognized.
*/
function isKnownNetworkId(
networkId: string | null,
): networkId is keyof typeof NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP {
return (
networkId !== null &&
hasProperty(NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP, networkId)
);
}

const name = 'EnsController';

/**
Expand Down Expand Up @@ -118,9 +104,7 @@ export class EnsController extends BaseControllerV2<
state?: Partial<EnsControllerState>;
provider?: ExternalProvider | JsonRpcFetchFunc;
onNetworkStateChange?: (
listener: (
networkState: Pick<NetworkState, 'networkId' | 'providerConfig'>,
) => void,
listener: (networkState: Pick<NetworkState, 'providerConfig'>) => void,
) => void;
}) {
super({
Expand All @@ -136,15 +120,14 @@ export class EnsController extends BaseControllerV2<
if (provider && onNetworkStateChange) {
onNetworkStateChange((networkState) => {
this.resetState();
const currentNetwork = networkState.networkId;
if (
isKnownNetworkId(currentNetwork) &&
this.#getNetworkEnsSupport(currentNetwork)
) {
const currentChainId = networkState.providerConfig.chainId;
if (this.#getChainEnsSupport(currentChainId)) {
this.#ethProvider = new Web3Provider(provider, {
chainId: convertHexToDecimal(networkState.providerConfig.chainId),
name: NETWORK_ID_TO_ETHERS_NETWORK_NAME_MAP[currentNetwork],
ensAddress: ensNetworkMap[currentNetwork],
chainId: convertHexToDecimal(currentChainId),
name: CHAIN_ID_TO_ETHERS_NETWORK_NAME_MAP[
currentChainId as ChainId
],
ensAddress: ensNetworkMap[parseInt(currentChainId, 16)],
});
} else {
this.#ethProvider = null;
Expand Down Expand Up @@ -269,13 +252,13 @@ export class EnsController extends BaseControllerV2<
}

/**
* Check if network supports ENS.
* Check if the chain supports ENS.
*
* @param networkId - Network id.
* @returns Boolean indicating if the network supports ENS.
* @param chainId - chain id.
* @returns Boolean indicating if the chain supports ENS.
*/
#getNetworkEnsSupport(networkId: string) {
return Boolean(ensNetworkMap[networkId]);
#getChainEnsSupport(chainId: string) {
return Boolean(ensNetworkMap[parseInt(chainId, 16)]);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/gas-fee-controller/src/GasFeeController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ const setupNetworkController = async ({
// Call this without awaiting to simulate what the extension or mobile app
// might do
networkController.initializeProvider();
// Ensure that the request for net_version that the network controller makes
// goes through
// Ensure that the request for eth_getBlockByNumber made by the PollingBlockTracker
// inside the NetworkController goes through
await clock.nextAsync();

return networkController;
Expand Down
Loading

0 comments on commit a5615ca

Please sign in to comment.