Skip to content

Correct invalid initial selectedNetworkClientId #15941

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

Draft
wants to merge 2 commits into
base: release/7.47.0
Choose a base branch
from

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented May 30, 2025

Description

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:
          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

After

Pre-merge author checklist

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.

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.
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@mcmire mcmire added the Run Smoke E2E Requires smoke E2E testing label May 30, 2025
Copy link
Contributor

github-actions bot commented May 30, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b8b7301
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4ec35936-24d0-426f-a3d7-2837f1cb37e5

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@mcmire mcmire added Run Smoke E2E Requires smoke E2E testing and removed Run Smoke E2E Requires smoke E2E testing labels Jun 2, 2025
Copy link
Contributor

github-actions bot commented Jun 2, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 5004088
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1fdfa51d-d482-4cd9-9cc2-d51df291feec

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

Copy link

sonarqubecloud bot commented Jun 2, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run Smoke E2E Requires smoke E2E testing team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants