Skip to content

Commit 442a300

Browse files
mcmireMajorLiftbergeronmikesposito
authored
Group network configurations by chain (#4286)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> Currently, in the client, it is possible to have multiple networks with different RPC endpoint URLs representing the same chain. This creates a problem because if all we have is a chain ID, we don't know which URL to use for requests. To solve this, we plan on consolidating the UX on the client side such that each network corresponds to exactly one chain. Users can then select which default RPC URL they'd like to use for requests. This commit implements the controller changes necessary to support this UX. Here are some more details on the changes here: - The concept of a network configuration has been repurposed such that instead of representing an RPC endpoint, it now represents a whole chain. - A network configuration may have multiple RPC endpoints, and one of them must be designated as the default. - Some RPC endpoints are special in that they represent Infura API URLs; these have the same object shape as "non-Infura" (custom) RPC endpoints, but the Infura project ID is hidden and injected into the RPC URL when creating the network client. - There is no longer a 1-to-1 relationship between network configuration and network client; rather, the 1-to-1 relationship exists between RPC endpoint and network client. This means that the ID of the network client which is created for an RPC endpoint is stored on that RPC endpoint instead of the whole network configuration. - The `networkConfigurations` state property has been replaced with `networkConfigurationsByChainId`. This continues to be an object, but the data inside is organized such that network configurations are identified by chain ID instead of network client ID as they were previously. - The methods `upsertNetworkConfiguration` and `removeNetworkConfiguration` have been removed. These methods always did more than simply add or remove a network configuration; they also updated the registry of network clients. Instead, these methods have been replaced with `addNetwork`, `updateNetwork`, and `removeNetwork`. - `addNetwork` creates new network clients for each RPC endpoint in the given network configuration. - `updateNetwork` takes a chain ID referring to a network configuration and a draft version of that network configuration, and adds or removes network clients for added or removed RPC endpoints. - `removeNetwork` takes a chain ID referring to a network configuration and removes the network clients for each of its RPC endpoints. - In addition, due to the changes to network configuration itself, there are new restrictions on `networkConfigurationsByChainId`, which are validated on initialization and on update. These are: - The network controller cannot be initialized with an empty collection of network configurations. This is because there must be a selected network client so that consumers have a provider to use out of the gate. - Consequently, the last network configuration cannot be removed. - The network configuration that contains a reference to the currently selected network client cannot be removed. - The chain ID of a network configuration must match the same chain that it's filed under in `networkConfigurationsByChainId`. - No two network configurations can have the same chain ID. - A RPC endpoint in a network configuration must have a well-formed URL. - A network configuration cannot have duplicate RPC endpoints. - No two RPC endpoints (regardless of network configuration) can have the same URL. Equality is currently determined by normalizing URLs as per RFC 3986 and may include data like request headers in the future. - If a network configuration has an Infura RPC endpoint, its chain ID must match the set chain ID of the network configuration. - Changing the chain ID of a network configuration is possible, but any existing Infura RPC endpoint must be replaced with the one that matches the new chain ID. - No two RPC endpoints (regardless of network configuration) can have the same network client ID. - Finally, the `trackMetaMetricsEvent` option has been removed from the constructor. This was previously used in `upsertNetworkConfiguration` to create a MetaMetrics event when a new network added, but I've added a new event `NetworkController:networkAdded` to allow the client to do this on its own accord. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? For example: * Fixes #12345 * Related to #67890 --> Fixes #4189. Fixes #3793. ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> (Updated in the PR.) ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --------- Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com> Co-authored-by: Brian Bergeron <brian.e.bergeron@gmail.com> Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com> Co-authored-by: Michele Esposito <michele@esposito.codes>
1 parent b4d5f3e commit 442a300

29 files changed

+12760
-3694
lines changed

‎packages/assets-controllers/src/AccountTrackerController.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { InternalAccount } from '@metamask/keyring-api';
44
import {
55
type NetworkClientId,
66
type NetworkClientConfiguration,
7-
defaultState as defaultnetworkControllerState,
7+
getDefaultNetworkControllerState,
88
} from '@metamask/network-controller';
99
import { getDefaultPreferencesState } from '@metamask/preferences-controller';
1010
import * as sinon from 'sinon';
@@ -618,7 +618,7 @@ async function withController<ReturnValue>(
618618
);
619619

620620
const mockNetworkState = jest.fn().mockReturnValue({
621-
...defaultnetworkControllerState,
621+
...getDefaultNetworkControllerState(),
622622
chainId: initialChainId,
623623
});
624624
messenger.registerActionHandler(

‎packages/assets-controllers/src/AssetsContractController.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ async function setupAssetContractControllers({
9191
allowedActions: [],
9292
allowedEvents: [],
9393
}),
94-
trackMetaMetricsEvent: jest.fn(),
9594
});
9695
if (useNetworkControllerProvider) {
9796
await networkController.initializeProvider();

‎packages/assets-controllers/src/NftController.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import type {
2525
NetworkClientConfiguration,
2626
NetworkClientId,
2727
} from '@metamask/network-controller';
28-
import { defaultState as defaultNetworkState } from '@metamask/network-controller';
28+
import { getDefaultNetworkControllerState } from '@metamask/network-controller';
2929
import {
3030
getDefaultPreferencesState,
3131
type PreferencesState,
@@ -352,7 +352,7 @@ function setupController({
352352
selectedNetworkClientId: NetworkClientId;
353353
}) => {
354354
messenger.publish('NetworkController:networkDidChange', {
355-
...defaultNetworkState,
355+
...getDefaultNetworkControllerState(),
356356
selectedNetworkClientId,
357357
});
358358
};

‎packages/assets-controllers/src/NftDetectionController.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import {
66
InfuraNetworkType,
77
} from '@metamask/controller-utils';
88
import {
9+
getDefaultNetworkControllerState,
910
NetworkClientType,
10-
defaultState as defaultNetworkState,
1111
} from '@metamask/network-controller';
1212
import type {
1313
NetworkClient,
@@ -1655,7 +1655,7 @@ async function withController<ReturnValue>(
16551655
messenger.registerActionHandler(
16561656
'NetworkController:getState',
16571657
jest.fn<NetworkState, []>().mockReturnValue({
1658-
...defaultNetworkState,
1658+
...getDefaultNetworkControllerState(),
16591659
...mockNetworkState,
16601660
}),
16611661
);

‎packages/assets-controllers/src/TokenDetectionController.test.ts‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import {
44
ChainId,
55
NetworkType,
66
convertHexToDecimal,
7-
BUILT_IN_NETWORKS,
7+
InfuraNetworkType,
88
} from '@metamask/controller-utils';
99
import type { InternalAccount } from '@metamask/keyring-api';
1010
import type { KeyringControllerState } from '@metamask/keyring-controller';
11-
import { defaultState as defaultNetworkState } from '@metamask/network-controller';
11+
import { getDefaultNetworkControllerState } from '@metamask/network-controller';
1212
import type {
1313
NetworkState,
1414
NetworkConfiguration,
@@ -28,6 +28,10 @@ import * as sinon from 'sinon';
2828

2929
import { advanceTime } from '../../../tests/helpers';
3030
import { createMockInternalAccount } from '../../accounts-controller/src/tests/mocks';
31+
import {
32+
buildCustomRpcEndpoint,
33+
buildInfuraNetworkConfiguration,
34+
} from '../../network-controller/tests/helpers';
3135
import { formatAggregatorNames } from './assetsUtil';
3236
import { TOKEN_END_POINT_API } from './token-service';
3337
import type {
@@ -110,22 +114,24 @@ const sampleTokenB = {
110114
};
111115

112116
const mockNetworkConfigurations: Record<string, NetworkConfiguration> = {
113-
[NetworkType.mainnet]: {
114-
...BUILT_IN_NETWORKS[NetworkType.mainnet],
115-
rpcUrl: 'https://mainnet.infura.io/v3/fakekey',
116-
},
117-
[NetworkType.goerli]: {
118-
...BUILT_IN_NETWORKS[NetworkType.goerli],
119-
rpcUrl: 'https://goerli.infura.io/v3/fakekey',
120-
},
117+
[InfuraNetworkType.mainnet]: buildInfuraNetworkConfiguration(
118+
InfuraNetworkType.mainnet,
119+
),
120+
[InfuraNetworkType.goerli]: buildInfuraNetworkConfiguration(
121+
InfuraNetworkType.goerli,
122+
),
121123
polygon: {
124+
blockExplorerUrls: ['https://polygonscan.com/'],
122125
chainId: '0x89',
123-
nickname: 'Polygon Mainnet',
124-
rpcUrl: `https://polygon-mainnet.infura.io/v3/fakekey`,
125-
ticker: 'MATIC',
126-
rpcPrefs: {
127-
blockExplorerUrl: 'https://polygonscan.com/',
128-
},
126+
defaultBlockExplorerUrlIndex: 0,
127+
defaultRpcEndpointIndex: 0,
128+
name: 'Polygon Mainnet',
129+
nativeCurrency: 'MATIC',
130+
rpcEndpoints: [
131+
buildCustomRpcEndpoint({
132+
url: 'https://polygon-mainnet.infura.io/v3/fakekey',
133+
}),
134+
],
129135
},
130136
};
131137

@@ -306,7 +312,7 @@ describe('TokenDetectionController', () => {
306312
},
307313
async ({ controller, mockNetworkState }) => {
308314
mockNetworkState({
309-
...defaultNetworkState,
315+
...getDefaultNetworkControllerState(),
310316
selectedNetworkClientId: NetworkType.goerli,
311317
});
312318
await controller.start();
@@ -393,7 +399,7 @@ describe('TokenDetectionController', () => {
393399
callActionSpy,
394400
}) => {
395401
mockNetworkState({
396-
...defaultNetworkState,
402+
...getDefaultNetworkControllerState(),
397403
selectedNetworkClientId: 'polygon',
398404
});
399405
mockGetNetworkClientById(
@@ -1400,7 +1406,7 @@ describe('TokenDetectionController', () => {
14001406
});
14011407

14021408
triggerNetworkDidChange({
1403-
...defaultNetworkState,
1409+
...getDefaultNetworkControllerState(),
14041410
selectedNetworkClientId: 'polygon',
14051411
});
14061412
await advanceTime({ clock, duration: 1 });
@@ -1461,7 +1467,7 @@ describe('TokenDetectionController', () => {
14611467
});
14621468

14631469
triggerNetworkDidChange({
1464-
...defaultNetworkState,
1470+
...getDefaultNetworkControllerState(),
14651471
selectedNetworkClientId: 'goerli',
14661472
});
14671473
await advanceTime({ clock, duration: 1 });
@@ -1512,7 +1518,7 @@ describe('TokenDetectionController', () => {
15121518
});
15131519

15141520
triggerNetworkDidChange({
1515-
...defaultNetworkState,
1521+
...getDefaultNetworkControllerState(),
15161522
selectedNetworkClientId: 'mainnet',
15171523
});
15181524
await advanceTime({ clock, duration: 1 });
@@ -1565,7 +1571,7 @@ describe('TokenDetectionController', () => {
15651571
});
15661572

15671573
triggerNetworkDidChange({
1568-
...defaultNetworkState,
1574+
...getDefaultNetworkControllerState(),
15691575
selectedNetworkClientId: 'polygon',
15701576
});
15711577
await advanceTime({ clock, duration: 1 });
@@ -1619,7 +1625,7 @@ describe('TokenDetectionController', () => {
16191625
});
16201626

16211627
triggerNetworkDidChange({
1622-
...defaultNetworkState,
1628+
...getDefaultNetworkControllerState(),
16231629
selectedNetworkClientId: 'polygon',
16241630
});
16251631
await advanceTime({ clock, duration: 1 });
@@ -1955,7 +1961,7 @@ describe('TokenDetectionController', () => {
19551961
callActionSpy,
19561962
}) => {
19571963
mockNetworkState({
1958-
...defaultNetworkState,
1964+
...getDefaultNetworkControllerState(),
19591965
selectedNetworkClientId: NetworkType.goerli,
19601966
});
19611967
triggerPreferencesStateChange({
@@ -2372,7 +2378,7 @@ async function withController<ReturnValue>(
23722378
const mockNetworkState = jest.fn<NetworkState, []>();
23732379
controllerMessenger.registerActionHandler(
23742380
'NetworkController:getState',
2375-
mockNetworkState.mockReturnValue({ ...defaultNetworkState }),
2381+
mockNetworkState.mockReturnValue({ ...getDefaultNetworkControllerState() }),
23762382
);
23772383
const mockTokensState = jest.fn<TokensControllerState, []>();
23782384
controllerMessenger.registerActionHandler(

‎packages/assets-controllers/src/TokenListController.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ describe('TokenListController', () => {
658658
);
659659
onNetworkStateChangeCallback({
660660
selectedNetworkClientId,
661-
networkConfigurations: {},
661+
networkConfigurationsByChainId: {},
662662
networksMetadata: {},
663663
// @ts-expect-error This property isn't used and will get removed later.
664664
providerConfig: {},
@@ -996,7 +996,7 @@ describe('TokenListController', () => {
996996
'NetworkController:stateChange',
997997
{
998998
selectedNetworkClientId: InfuraNetworkType.goerli,
999-
networkConfigurations: {},
999+
networkConfigurationsByChainId: {},
10001000
networksMetadata: {},
10011001
// @ts-expect-error This property isn't used and will get removed later.
10021002
providerConfig: {},
@@ -1017,7 +1017,7 @@ describe('TokenListController', () => {
10171017
'NetworkController:stateChange',
10181018
{
10191019
selectedNetworkClientId: selectedCustomNetworkClientId,
1020-
networkConfigurations: {},
1020+
networkConfigurationsByChainId: {},
10211021
networksMetadata: {},
10221022
// @ts-expect-error This property isn't used and will get removed later.
10231023
providerConfig: {},
@@ -1097,7 +1097,7 @@ describe('TokenListController', () => {
10971097
'NetworkController:stateChange',
10981098
{
10991099
selectedNetworkClientId: InfuraNetworkType.mainnet,
1100-
networkConfigurations: {},
1100+
networkConfigurationsByChainId: {},
11011101
networksMetadata: {},
11021102
// @ts-expect-error This property isn't used and will get removed later.
11031103
providerConfig: {},
@@ -1147,7 +1147,7 @@ describe('TokenListController', () => {
11471147
'NetworkController:stateChange',
11481148
{
11491149
selectedNetworkClientId: selectedCustomNetworkClientId,
1150-
networkConfigurations: {},
1150+
networkConfigurationsByChainId: {},
11511151
networksMetadata: {},
11521152
// @ts-expect-error This property isn't used and will get removed later.
11531153
providerConfig: {},

‎packages/assets-controllers/src/TokenRatesController.test.ts‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type {
1313
NetworkClientId,
1414
NetworkState,
1515
} from '@metamask/network-controller';
16-
import { defaultState as defaultNetworkState } from '@metamask/network-controller';
16+
import { getDefaultNetworkControllerState } from '@metamask/network-controller';
1717
import type { Hex } from '@metamask/utils';
1818
import { add0x } from '@metamask/utils';
1919
import assert from 'assert';
@@ -48,8 +48,6 @@ const defaultSelectedAccount = createMockInternalAccount({
4848
});
4949
const mockTokenAddress = '0x0000000000000000000000000000000000000010';
5050

51-
const defaultSelectedNetworkClientId = 'AAAA-BBBB-CCCC-DDDD';
52-
5351
type MainControllerMessenger = ControllerMessenger<
5452
AllowedActions | AddApprovalRequest,
5553
AllowedEvents
@@ -638,8 +636,8 @@ describe('TokenRatesController', () => {
638636
.spyOn(controller, 'updateExchangeRates')
639637
.mockResolvedValue();
640638
triggerNetworkStateChange({
641-
...defaultNetworkState,
642-
selectedNetworkClientId: defaultSelectedNetworkClientId,
639+
...getDefaultNetworkControllerState(),
640+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
643641
});
644642

645643
expect(updateExchangeRatesSpy).toHaveBeenCalledTimes(1);
@@ -666,8 +664,8 @@ describe('TokenRatesController', () => {
666664
.spyOn(controller, 'updateExchangeRates')
667665
.mockResolvedValue();
668666
triggerNetworkStateChange({
669-
...defaultNetworkState,
670-
selectedNetworkClientId: defaultSelectedNetworkClientId,
667+
...getDefaultNetworkControllerState(),
668+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
671669
});
672670

673671
expect(updateExchangeRatesSpy).toHaveBeenCalledTimes(1);
@@ -720,8 +718,8 @@ describe('TokenRatesController', () => {
720718
await controller.start();
721719
jest.spyOn(controller, 'updateExchangeRates').mockResolvedValue();
722720
triggerNetworkStateChange({
723-
...defaultNetworkState,
724-
selectedNetworkClientId: defaultSelectedNetworkClientId,
721+
...getDefaultNetworkControllerState(),
722+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
725723
});
726724

727725
expect(controller.state.marketData).toStrictEqual({});
@@ -774,8 +772,8 @@ describe('TokenRatesController', () => {
774772
await controller.start();
775773
jest.spyOn(controller, 'updateExchangeRates').mockResolvedValue();
776774
triggerNetworkStateChange({
777-
...defaultNetworkState,
778-
selectedNetworkClientId: defaultSelectedNetworkClientId,
775+
...getDefaultNetworkControllerState(),
776+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
779777
});
780778

781779
expect(controller.state.marketData).toStrictEqual({});
@@ -802,8 +800,8 @@ describe('TokenRatesController', () => {
802800
.spyOn(controller, 'updateExchangeRates')
803801
.mockResolvedValue();
804802
triggerNetworkStateChange({
805-
...defaultNetworkState,
806-
selectedNetworkClientId: defaultSelectedNetworkClientId,
803+
...getDefaultNetworkControllerState(),
804+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
807805
});
808806

809807
expect(updateExchangeRatesSpy).not.toHaveBeenCalled();
@@ -831,8 +829,8 @@ describe('TokenRatesController', () => {
831829
.spyOn(controller, 'updateExchangeRates')
832830
.mockResolvedValue();
833831
triggerNetworkStateChange({
834-
...defaultNetworkState,
835-
selectedNetworkClientId: defaultSelectedNetworkClientId,
832+
...getDefaultNetworkControllerState(),
833+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
836834
});
837835

838836
expect(updateExchangeRatesSpy).not.toHaveBeenCalled();
@@ -858,8 +856,8 @@ describe('TokenRatesController', () => {
858856
.spyOn(controller, 'updateExchangeRates')
859857
.mockResolvedValue();
860858
triggerNetworkStateChange({
861-
...defaultNetworkState,
862-
selectedNetworkClientId: defaultSelectedNetworkClientId,
859+
...getDefaultNetworkControllerState(),
860+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
863861
});
864862

865863
expect(updateExchangeRatesSpy).not.toHaveBeenCalled();
@@ -911,8 +909,8 @@ describe('TokenRatesController', () => {
911909
async ({ controller, triggerNetworkStateChange }) => {
912910
jest.spyOn(controller, 'updateExchangeRates').mockResolvedValue();
913911
triggerNetworkStateChange({
914-
...defaultNetworkState,
915-
selectedNetworkClientId: defaultSelectedNetworkClientId,
912+
...getDefaultNetworkControllerState(),
913+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
916914
});
917915

918916
expect(controller.state.marketData).toStrictEqual({});
@@ -964,8 +962,8 @@ describe('TokenRatesController', () => {
964962
async ({ controller, triggerNetworkStateChange }) => {
965963
jest.spyOn(controller, 'updateExchangeRates').mockResolvedValue();
966964
triggerNetworkStateChange({
967-
...defaultNetworkState,
968-
selectedNetworkClientId: defaultSelectedNetworkClientId,
965+
...getDefaultNetworkControllerState(),
966+
selectedNetworkClientId: 'AAAA-BBBB-CCCC-DDDD',
969967
});
970968

971969
expect(controller.state.marketData).toStrictEqual({});
@@ -2327,7 +2325,7 @@ async function withController<ReturnValue>(
23272325
controllerMessenger.registerActionHandler(
23282326
'NetworkController:getState',
23292327
networkStateMock.mockReturnValue({
2330-
...defaultNetworkState,
2328+
...getDefaultNetworkControllerState(),
23312329
...mockNetworkState,
23322330
}),
23332331
);
@@ -2445,7 +2443,7 @@ async function callUpdateExchangeRatesMethod({
24452443
// As with many BaseControllerV1-based controllers, runtime config
24462444
// modification is allowed by the API but not supported in practice.
24472445
triggerNetworkStateChange({
2448-
...defaultNetworkState,
2446+
...getDefaultNetworkControllerState(),
24492447
selectedNetworkClientId,
24502448
});
24512449
}

0 commit comments

Comments
 (0)