Skip to content

Commit 00a364e

Browse files
mcmireNicolasMassart
mcmire
and
NicolasMassart
authored
Correct invalid initial selectedNetworkClientId (#15941)
<!-- 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? 2. What is the improvement/solution? --> Currently, when NetworkController is instantiated with pre-existing state that contains an invalid `selectedNetworkClientId` — that is, no RPC endpoint exists which has the same network client ID — then it throws an error. This was intentionally done to bring attention to possible bugs in NetworkController, but this has the unfortunate side effect of bricking users' wallets. To fix this, we now correct an invalid `selectedNetworkClientId` to point to the default RPC endpoint of the first network sorted by chain ID (which in the vast majority of cases will be Mainnet). We still do want to know about this, though, so we log the error in Sentry. ## **Related issues** See MetaMask/core#5851 for the equivalent changes to network-controller that were copied here. ## **Manual testing steps** 1. Check out this branch. 2. Run `yarn setup:expo`. 3. We now have to force the initial state to be invalid. Open `Engine.ts`, scroll down to where NetworkController is initialized, and apply these changes: ``` diff const networkControllerOpts = { infuraProjectId: process.env.MM_INFURA_PROJECT_ID || NON_EMPTY, - state: initialState.NetworkController, + state: { + ...initialState.NetworkController, + selectedNetworkClientId: "nonexistent", + }, messenger: this.controllerMessenger.getRestricted({ name: 'NetworkController', allowedEvents: [], allowedActions: [], }) as unknown as NetworkControllerMessenger, getRpcServiceOptions: () => ({ fetch, btoa, }), additionalDefaultNetworks: [ChainId['megaeth-testnet']], captureException, }; const networkController = new NetworkController(networkControllerOpts); + Logger.log('selectedNetworkClientId', networkController.state.selectedNetworkClientId); ``` 4. Run `yarn watch:clean`. 5. Load the app, onboarding if necessary. 6. You should be able to get to the home screen without issues. If you check the terminal you should see a line that says `selectedNetworkClientId mainnet`. This proves that although the initial `selectedNetworkClientId` was invalid, it was auto-corrected to "mainnet". ## **Screenshots/Recordings** (N/A) ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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. Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
1 parent aea0e8f commit 00a364e

2 files changed

Lines changed: 162 additions & 0 deletions

File tree

‎app/core/Engine/Engine.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ import { appMetadataControllerInit } from './controllers/app-metadata-controller
202202
import { InternalAccount } from '@metamask/keyring-internal-api';
203203
import { toFormattedAddress } from '../../util/address';
204204
import { BRIDGE_API_BASE_URL } from '../../constants/bridge';
205+
import { captureException } from '@sentry/react-native';
205206

206207
const NON_EMPTY = 'NON_EMPTY';
207208

@@ -317,6 +318,7 @@ export class Engine {
317318
btoa,
318319
}),
319320
additionalDefaultNetworks: [ChainId['megaeth-testnet']],
321+
captureException,
320322
};
321323
const networkController = new NetworkController(networkControllerOpts);
322324

‎patches/@metamask+network-controller+23.2.1-backport.patch‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
diff --git a/node_modules/@metamask/network-controller/dist/NetworkController.cjs b/node_modules/@metamask/network-controller/dist/NetworkController.cjs
2+
index ca0fdc1..6c52629 100644
3+
--- a/node_modules/@metamask/network-controller/dist/NetworkController.cjs
4+
+++ b/node_modules/@metamask/network-controller/dist/NetworkController.cjs
5+
@@ -45,6 +45,7 @@ const eth_query_1 = __importDefault(require("@metamask/eth-query"));
6+
const rpc_errors_1 = require("@metamask/rpc-errors");
7+
const swappable_obj_proxy_1 = require("@metamask/swappable-obj-proxy");
8+
const utils_1 = require("@metamask/utils");
9+
+const immer_1 = require("immer");
10+
const fast_deep_equal_1 = __importDefault(require("fast-deep-equal"));
11+
const lodash_1 = require("lodash");
12+
const reselect_1 = require("reselect");
13+
@@ -286,7 +287,7 @@ function deriveInfuraNetworkNameFromRpcEndpointUrl(rpcEndpointUrl) {
14+
* @param state - The NetworkController state to verify.
15+
* @throws if the state is invalid in some way.
16+
*/
17+
-function validateNetworkControllerState(state) {
18+
+function validateInitialState(state) {
19+
const networkConfigurationEntries = Object.entries(state.networkConfigurationsByChainId);
20+
const networkClientIds = getAvailableNetworkClientIds(getNetworkConfigurations(state));
21+
if (networkConfigurationEntries.length === 0) {
22+
@@ -310,12 +311,27 @@ function validateNetworkControllerState(state) {
23+
if ([...new Set(networkClientIds)].length < networkClientIds.length) {
24+
throw new Error('NetworkController state has invalid `networkConfigurationsByChainId`: Every RPC endpoint across all network configurations must have a unique `networkClientId`');
25+
}
26+
- if (!networkClientIds.includes(state.selectedNetworkClientId)) {
27+
- throw new Error(
28+
- // This ESLint rule mistakenly produces an error.
29+
- // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
30+
- `NetworkController state is invalid: \`selectedNetworkClientId\` '${state.selectedNetworkClientId}' does not refer to an RPC endpoint within a network configuration`);
31+
- }
32+
+}
33+
+/**
34+
+ * Checks that the given initial NetworkController state is internally
35+
+ * consistent similar to `validateInitialState`, but if an anomaly is detected,
36+
+ * it does its best to correct the state and logs an error to Sentry.
37+
+ *
38+
+ * @param state - The NetworkController state to verify.
39+
+ * @param captureException - The function that logs an error to Sentry.
40+
+ * @returns The corrected state.
41+
+ */
42+
+function correctInitialState(state, captureException) {
43+
+ const networkConfigurationsSortedByChainId = getNetworkConfigurations(state).sort((a, b) => a.chainId.localeCompare(b.chainId));
44+
+ const networkClientIds = getAvailableNetworkClientIds(networkConfigurationsSortedByChainId);
45+
+ return (0, immer_1.produce)(state, (newState) => {
46+
+ if (!networkClientIds.includes(state.selectedNetworkClientId)) {
47+
+ const firstNetworkConfiguration = networkConfigurationsSortedByChainId[0];
48+
+ const newSelectedNetworkClientId = firstNetworkConfiguration.rpcEndpoints[firstNetworkConfiguration.defaultRpcEndpointIndex].networkClientId;
49+
+ captureException(new Error(`\`selectedNetworkClientId\` '${state.selectedNetworkClientId}' does not refer to an RPC endpoint within a network configuration; correcting to '${newSelectedNetworkClientId}'`));
50+
+ newState.selectedNetworkClientId = newSelectedNetworkClientId;
51+
+ }
52+
+ });
53+
}
54+
/**
55+
* Transforms a map of chain ID to network configuration to a map of network
56+
@@ -342,12 +358,13 @@ class NetworkController extends base_controller_1.BaseController {
57+
* @param options - The options; see {@link NetworkControllerOptions}.
58+
*/
59+
constructor(options) {
60+
- const { messenger, state, infuraProjectId, log, getRpcServiceOptions, additionalDefaultNetworks, } = options;
61+
+ const { messenger, state, infuraProjectId, log, getRpcServiceOptions, additionalDefaultNetworks, captureException, } = options;
62+
const initialState = {
63+
...getDefaultNetworkControllerState(additionalDefaultNetworks),
64+
...state,
65+
};
66+
- validateNetworkControllerState(initialState);
67+
+ validateInitialState(initialState);
68+
+ const correctedInitialState = correctInitialState(initialState, captureException);
69+
if (!infuraProjectId || typeof infuraProjectId !== 'string') {
70+
throw new Error('Invalid Infura project ID');
71+
}
72+
@@ -368,7 +385,7 @@ class NetworkController extends base_controller_1.BaseController {
73+
},
74+
},
75+
messenger,
76+
- state: initialState,
77+
+ state: correctedInitialState,
78+
});
79+
_NetworkController_instances.add(this);
80+
_NetworkController_ethQuery.set(this, void 0);
81+
diff --git a/node_modules/@metamask/network-controller/dist/NetworkController.mjs b/node_modules/@metamask/network-controller/dist/NetworkController.mjs
82+
index 0efca67..ff13585 100644
83+
--- a/node_modules/@metamask/network-controller/dist/NetworkController.mjs
84+
+++ b/node_modules/@metamask/network-controller/dist/NetworkController.mjs
85+
@@ -25,6 +25,7 @@ import { createEventEmitterProxy } from "@metamask/swappable-obj-proxy";
86+
import { hasProperty, isPlainObject, isStrictHexString } from "@metamask/utils";
87+
import $deepEqual from "fast-deep-equal";
88+
const deepEqual = $importDefault($deepEqual);
89+
+import { produce } from "immer";
90+
import $lodash from "lodash";
91+
const { cloneDeep } = $lodash;
92+
import { createSelector } from "reselect";
93+
@@ -262,7 +263,7 @@ function deriveInfuraNetworkNameFromRpcEndpointUrl(rpcEndpointUrl) {
94+
* @param state - The NetworkController state to verify.
95+
* @throws if the state is invalid in some way.
96+
*/
97+
-function validateNetworkControllerState(state) {
98+
+function validateInitialState(state) {
99+
const networkConfigurationEntries = Object.entries(state.networkConfigurationsByChainId);
100+
const networkClientIds = getAvailableNetworkClientIds(getNetworkConfigurations(state));
101+
if (networkConfigurationEntries.length === 0) {
102+
@@ -286,12 +287,27 @@ function validateNetworkControllerState(state) {
103+
if ([...new Set(networkClientIds)].length < networkClientIds.length) {
104+
throw new Error('NetworkController state has invalid `networkConfigurationsByChainId`: Every RPC endpoint across all network configurations must have a unique `networkClientId`');
105+
}
106+
- if (!networkClientIds.includes(state.selectedNetworkClientId)) {
107+
- throw new Error(
108+
- // This ESLint rule mistakenly produces an error.
109+
- // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
110+
- `NetworkController state is invalid: \`selectedNetworkClientId\` '${state.selectedNetworkClientId}' does not refer to an RPC endpoint within a network configuration`);
111+
- }
112+
+}
113+
+/**
114+
+ * Checks that the given initial NetworkController state is internally
115+
+ * consistent similar to `validateInitialState`, but if an anomaly is detected,
116+
+ * it does its best to correct the state and logs an error to Sentry.
117+
+ *
118+
+ * @param state - The NetworkController state to verify.
119+
+ * @param captureException - The function that logs an error to Sentry.
120+
+ * @returns The corrected state.
121+
+ */
122+
+function correctInitialState(state, captureException) {
123+
+ const networkConfigurationsSortedByChainId = getNetworkConfigurations(state).sort((a, b) => a.chainId.localeCompare(b.chainId));
124+
+ const networkClientIds = getAvailableNetworkClientIds(networkConfigurationsSortedByChainId);
125+
+ return produce(state, (newState) => {
126+
+ if (!networkClientIds.includes(state.selectedNetworkClientId)) {
127+
+ const firstNetworkConfiguration = networkConfigurationsSortedByChainId[0];
128+
+ const newSelectedNetworkClientId = firstNetworkConfiguration.rpcEndpoints[firstNetworkConfiguration.defaultRpcEndpointIndex].networkClientId;
129+
+ captureException(new Error(`\`selectedNetworkClientId\` '${state.selectedNetworkClientId}' does not refer to an RPC endpoint within a network configuration; correcting to '${newSelectedNetworkClientId}'`));
130+
+ newState.selectedNetworkClientId = newSelectedNetworkClientId;
131+
+ }
132+
+ });
133+
}
134+
/**
135+
* Transforms a map of chain ID to network configuration to a map of network
136+
@@ -318,12 +334,13 @@ export class NetworkController extends BaseController {
137+
* @param options - The options; see {@link NetworkControllerOptions}.
138+
*/
139+
constructor(options) {
140+
- const { messenger, state, infuraProjectId, log, getRpcServiceOptions, additionalDefaultNetworks, } = options;
141+
+ const { messenger, state, infuraProjectId, log, getRpcServiceOptions, additionalDefaultNetworks, captureException, } = options;
142+
const initialState = {
143+
...getDefaultNetworkControllerState(additionalDefaultNetworks),
144+
...state,
145+
};
146+
- validateNetworkControllerState(initialState);
147+
+ validateInitialState(initialState);
148+
+ const correctedInitialState = correctInitialState(initialState, captureException);
149+
if (!infuraProjectId || typeof infuraProjectId !== 'string') {
150+
throw new Error('Invalid Infura project ID');
151+
}
152+
@@ -344,7 +361,7 @@ export class NetworkController extends BaseController {
153+
},
154+
},
155+
messenger,
156+
- state: initialState,
157+
+ state: correctedInitialState,
158+
});
159+
_NetworkController_instances.add(this);
160+
_NetworkController_ethQuery.set(this, void 0);

0 commit comments

Comments
 (0)