fix: Prevent rpc request before onboarding is completed

.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch

@@ -0,0 +1,24 @@
+diff --git a/dist/chunk-VNCJZRDU.js b/dist/chunk-VNCJZRDU.js
+index 78251fc9517438d90261ed53172d2a4133d3a5fd..3ecc0c2838ac12c0ff5b5521a82b08955287956b 100644
+--- a/dist/chunk-VNCJZRDU.js
++++ b/dist/chunk-VNCJZRDU.js
+@@ -323,7 +323,6 @@ var NetworkController = class extends _basecontroller.BaseController {
+   async initializeProvider() {
+     _chunkZ4BLTVTBjs.__privateMethod.call(void 0, this, _ensureAutoManagedNetworkClientRegistryPopulated, ensureAutoManagedNetworkClientRegistryPopulated_fn).call(this);
+     _chunkZ4BLTVTBjs.__privateMethod.call(void 0, this, _applyNetworkSelection, applyNetworkSelection_fn).call(this);
+-    await this.lookupNetwork();
+   }
+   /**
+    * Refreshes the network meta with EIP-1559 support and the network status
+diff --git a/dist/chunk-XWP6GXMK.mjs b/dist/chunk-XWP6GXMK.mjs
+index fb7e30d27367a38e8a357ff9e744894a4505b5a5..884bb1d124934a613847d7fd398fbc6cb02cade8 100644
+--- a/dist/chunk-XWP6GXMK.mjs
++++ b/dist/chunk-XWP6GXMK.mjs
+@@ -323,7 +323,6 @@ var NetworkController = class extends BaseController {
+   async initializeProvider() {
+     __privateMethod(this, _ensureAutoManagedNetworkClientRegistryPopulated, ensureAutoManagedNetworkClientRegistryPopulated_fn).call(this);
+     __privateMethod(this, _applyNetworkSelection, applyNetworkSelection_fn).call(this);
+-    await this.lookupNetwork();
+   }
+   /**
+    * Refreshes the network meta with EIP-1559 support and the network status

.yarn/patches/PATCH.txt

@@ -0,0 +1,6 @@
+@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch
+
+We remove lookupNetwork from initializeProvider in the network controller to prevent network requests before user onboarding is completed.
+The network lookup is done after onboarding is completed, and when the extension reloads if onboarding has been completed.
+This patch is part of a temporary fix that will be reverted soon to make way for a more permanent solution. https://github.com/MetaMask/metamask-extension/pull/23005
+You can see the changes before compilation on this branch: https://github.com/MetaMask/core/compare/pnf/ext-23622-review?expand=1

app/scripts/lib/account-tracker.js

@@ -78,14 +78,6 @@ export default class AccountTracker {
     this.onboardingController = opts.onboardingController;
     this.controllerMessenger = opts.controllerMessenger;
 
-    // blockTracker.currentBlock may be null
-    this.#currentBlockNumberByChainId = {
-      [this.getCurrentChainId()]: this.#blockTracker.getCurrentBlock(),
-    };
-    this.#blockTracker.once('latest', (blockNumber) => {
-      this.#currentBlockNumberByChainId[this.getCurrentChainId()] = blockNumber;
-    });
-
     // subscribe to account removal
     opts.onAccountRemoved((address) => this.removeAccounts([address]));
 
@@ -124,6 +116,14 @@ export default class AccountTracker {
    * Starts polling with global selected network
    */
   start() {
+    // blockTracker.currentBlock may be null
+    this.#currentBlockNumberByChainId = {
+      [this.getCurrentChainId()]: this.#blockTracker.getCurrentBlock(),
+    };
+    this.#blockTracker.once('latest', (blockNumber) => {
+      this.#currentBlockNumberByChainId[this.getCurrentChainId()] = blockNumber;
+    });
+
     // remove first to avoid double add
     this.#blockTracker.removeListener('latest', this.#updateForBlock);
     // add listener

app/scripts/metamask-controller.js

@@ -489,7 +489,6 @@ export default class MetamaskController extends EventEmitter {
 
     // TODO: Delete when ready to remove `networkVersion` from provider object
     this.deprecatedNetworkId = null;
-    this.updateDeprecatedNetworkId();
     networkControllerMessenger.subscribe(
       'NetworkController:networkDidChange',
       () => this.updateDeprecatedNetworkId(),
@@ -1433,6 +1432,7 @@ export default class MetamaskController extends EventEmitter {
         const { completedOnboarding: prevCompletedOnboarding } = prevState;
         const { completedOnboarding: currCompletedOnboarding } = currState;
         if (!prevCompletedOnboarding && currCompletedOnboarding) {
+          this.postOnboardingInitialization();
           this.triggerNetworkrequests();
         }
       }, this.onboardingController.store.getState()),
@@ -1638,7 +1638,6 @@ export default class MetamaskController extends EventEmitter {
       },
     );
 
-    this.networkController.lookupNetwork();
     this.decryptMessageController = new DecryptMessageController({
       getState: this.getState.bind(this),
       messenger: this.controllerMessenger.getRestricted({
@@ -2196,6 +2195,15 @@ export default class MetamaskController extends EventEmitter {
     this.extension.runtime.onMessageExternal.addListener(onMessageReceived);
     // Fire a ping message to check if other extensions are running
     checkForMultipleVersionsRunning();
+
+    if (this.onboardingController.store.getState().completedOnboarding) {
+      this.postOnboardingInitialization();
+    }
+  }
+
+  postOnboardingInitialization() {
+    this.updateDeprecatedNetworkId();
+    this.networkController.lookupNetwork();
   }
 
   triggerNetworkrequests() {

package.json

@@ -231,7 +231,12 @@
     "@babel/runtime@npm:^7.4.0": "patch:@babel/runtime@npm%3A7.24.0#~/.yarn/patches/@babel-runtime-npm-7.24.0-7eb1dd11a2.patch",
     "@babel/runtime@npm:^7.18.3": "patch:@babel/runtime@npm%3A7.24.0#~/.yarn/patches/@babel-runtime-npm-7.24.0-7eb1dd11a2.patch",
     "@babel/runtime@npm:^7.8.3": "patch:@babel/runtime@npm%3A7.24.0#~/.yarn/patches/@babel-runtime-npm-7.24.0-7eb1dd11a2.patch",
-    "@babel/runtime@npm:^7.8.4": "patch:@babel/runtime@npm%3A7.24.0#~/.yarn/patches/@babel-runtime-npm-7.24.0-7eb1dd11a2.patch"
+    "@babel/runtime@npm:^7.8.4": "patch:@babel/runtime@npm%3A7.24.0#~/.yarn/patches/@babel-runtime-npm-7.24.0-7eb1dd11a2.patch",
+    "@metamask/network-controller@npm:^17.2.0": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
+    "@metamask/network-controller@npm:^17.0.0": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
+    "@metamask/network-controller@npm:^15.0.0": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
+    "@metamask/network-controller@npm:^18.0.1": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
+    "@metamask/network-controller@npm:^17.2.1": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch"
   },
   "dependencies": {
     "@babel/runtime": "patch:@babel/runtime@npm%3A7.24.0#~/.yarn/patches/@babel-runtime-npm-7.24.0-7eb1dd11a2.patch",
@@ -293,7 +298,7 @@
     "@metamask/message-manager": "^7.3.0",
     "@metamask/metamask-eth-abis": "^3.0.0",
     "@metamask/name-controller": "^4.2.0",
-    "@metamask/network-controller": "^18.0.0",
+    "@metamask/network-controller": "patch:@metamask/network-controller@npm%3A18.0.1#~/.yarn/patches/@metamask-network-controller-npm-18.0.1-c4d0cfaecd.patch",
     "@metamask/notification-controller": "^3.0.0",
     "@metamask/obs-store": "^8.1.0",
     "@metamask/permission-controller": "^8.0.0",

test/e2e/tests/onboarding/onboarding.spec.js

@@ -12,6 +12,12 @@ const {
   locateAccountBalanceDOM,
   defaultGanacheOptions,
   WALLET_PASSWORD,
+  onboardingBeginCreateNewWallet,
+  onboardingChooseMetametricsOption,
+  onboardingCreatePassword,
+  onboardingRevealAndConfirmSRP,
+  onboardingCompleteWalletCreation,
+  regularDelayMs,
 } = require('../../helpers');
 const FixtureBuilder = require('../../fixture-builder');
 
@@ -301,4 +307,140 @@ describe('MetaMask onboarding @no-mmi', function () {
       },
     );
   });
+
+  it("doesn't make any network requests to infura before onboarding is completed", async function () {
+    async function mockInfura(mockServer) {
+      const infuraUrl =
+        'https://mainnet.infura.io/v3/00000000000000000000000000000000';
+      const sampleAddress = '1111111111111111111111111111111111111111';
+
+      return [
+        await mockServer
+          .forPost(infuraUrl)
+          .withJsonBodyIncluding({ method: 'eth_blockNumber' })
+          .thenCallback(() => {
+            return {
+              statusCode: 200,
+              json: {
+                jsonrpc: '2.0',
+                id: '1111111111111111',
+                result: '0x1',
+              },
+            };
+          }),
+        await mockServer
+          .forPost(infuraUrl)
+          .withJsonBodyIncluding({ method: 'eth_getBalance' })
+          .thenCallback(() => {
+            return {
+              statusCode: 200,
+              json: {
+                jsonrpc: '2.0',
+                id: '1111111111111111',
+                result: '0x1',
+              },
+            };
+          }),
+        await mockServer
+          .forPost(infuraUrl)
+          .withJsonBodyIncluding({ method: 'eth_getBlockByNumber' })
+          .thenCallback(() => {
+            return {
+              statusCode: 200,
+              json: {
+                jsonrpc: '2.0',
+                id: '1111111111111111',
+                result: {},
+              },
+            };
+          }),
+        await mockServer
+          .forPost(infuraUrl)
+          .withJsonBodyIncluding({ method: 'eth_call' })
+          .thenCallback(() => {
+            return {
+              statusCode: 200,
+              json: {
+                jsonrpc: '2.0',
+                id: '1111111111111111',
+                result: `0x000000000000000000000000${sampleAddress}`,
+              },
+            };
+          }),
+        await mockServer
+          .forPost(infuraUrl)
+          .withJsonBodyIncluding({ method: 'net_version' })
+          .thenCallback(() => {
+            return {
+              statusCode: 200,
+              json: { id: 8262367391254633, jsonrpc: '2.0', result: '1337' },
+            };
+          }),
+      ];
+    }
+
+    await withFixtures(
+      {
+        fixtures: new FixtureBuilder({ onboarding: true })
+          .withNetworkControllerOnMainnet()
+          .build(),
+        ganacheOptions: defaultGanacheOptions,
+        title: this.test.fullTitle(),
+        testSpecificMock: mockInfura,
+      },
+      async ({ driver, mockedEndpoint: mockedEndpoints }) => {
+        const password = 'password';
+
+        await driver.navigate();
+
+        await onboardingBeginCreateNewWallet(driver);
+        await onboardingChooseMetametricsOption(driver, false);
+        await onboardingCreatePassword(driver, password);
+        await onboardingRevealAndConfirmSRP(driver);
+        await onboardingCompleteWalletCreation(driver);
+
+        // pin extension walkthrough screen
+        await driver.clickElement('[data-testid="pin-extension-next"]');
+
+        await driver.delay(regularDelayMs);
+
+        for (let i = 0; i < mockedEndpoints.length; i += 1) {
+          const mockedEndpoint = await mockedEndpoints[i];
+          const isPending = await mockedEndpoint.isPending();
+          assert.equal(
+            isPending,
+            true,
+            `${mockedEndpoints[i]} mock should still be pending before onboarding`,
+          );
+          const requests = await mockedEndpoint.getSeenRequests();
+
+          assert.equal(
+            requests.length,
+            0,
+            `${mockedEndpoints[i]} should make no requests before onboarding`,
+          );
+        }
+
+        await driver.clickElement('[data-testid="pin-extension-done"]');
+        // requests happen here!
+
+        for (let i = 0; i < mockedEndpoints.length; i += 1) {
+          const mockedEndpoint = await mockedEndpoints[i];
+
+          await driver.wait(async () => {
+            const isPending = await mockedEndpoint.isPending();
+            return isPending === false;
+          }, driver.timeout);
+
+          const requests = await mockedEndpoint.getSeenRequests();
+
+          assert.equal(
+            requests.length > 0,
+            true,
+            `${mockedEndpoints[i]} should make requests after onboarding`,
+          );
+        }
+      },
+    );
+  });
 });

ui/pages/routes/routes.component.js

@@ -640,11 +640,12 @@ export default class Routes extends Component {
       isUnlocked &&
       !shouldShowSeedPhraseReminder;
 
-    let isLoadingShown = isLoading;
+    let isLoadingShown = isLoading && completedOnboarding;
 
     ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
     isLoadingShown =
       isLoading &&
+      completedOnboarding &&
       !pendingConfirmations.some(
         (confirmation) =>
           confirmation.type ===
@@ -707,7 +708,9 @@ export default class Routes extends Component {
         }
         <Box className="main-container-wrapper">
           {isLoadingShown ? <Loading loadingMessage={loadMessage} /> : null}
-          {!isLoading && isNetworkLoading ? <LoadingNetwork /> : null}
+          {!isLoading && isNetworkLoading && completedOnboarding ? (
+            <LoadingNetwork />
+          ) : null}
           {this.renderRoutes()}
         </Box>
         {isUnlocked ? <Alerts history={this.props.history} /> : null}