Skip to content
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

chore: bump network controller 21.0.0 #11292

Closed
wants to merge 163 commits into from

Conversation

salimtb
Copy link
Contributor

@salimtb salimtb commented Sep 18, 2024

Description

This PR updates the network-controller from version 20.0.0 to 21.0.0. The migration introduces changes in the handling of network configurations, particularly regarding how network data is structured and referenced.

Key Changes:
Consolidation of Network Configurations:

Network configurations in v21 are now organized by chainId in a new networkConfigurationsByChainId structure, replacing the networkConfigurations section in v20.
The new structure supports multiple RPC endpoints per network, as seen in the list of rpcEndpoints, with support for Infura-based connections (e.g., https://mainnet.infura.io/v3/{infuraProjectId}).

Remark

As part of the update to version 21 (v21) of the network controller, we are making a key change in how networks are handled. Previously, networks could have the same chain ID but different RPC URLs. With v21, each network will now have a unique chain ID that can include multiple RPC URLs.

To support this, the UI will also be updated—for example, the RPC URL field in the network form will become a dropdown menu instead of a simple text input.

These changes have been implemented behind a feature flag, which will remain in place until the v21 upgrade is finalized.

To complete this migration, we have 3 PRs that need to be reviewed and merged together:

The first PR upgrades the network controller to v21.
The second PR adds the migration script and the necessary UI for migration.
The third PR removes the feature flag, making the v21 changes active.
For the review process, it's recommended to start with the third PR (removing the feature flag), then proceed to the > second and first PRs. This order ensures a smooth transition and proper implementation of the changes.

Related PRS

Related issues

Fixes: #11229

Manual testing steps

  • Verify that networkConfigurations has been replaced by networkConfigurationsByChainId.
  • Check that each network in networkConfigurationsByChainId is identified by its chainId.
  • Ensure that all rpcEndpoints are correctly listed for each network in networkConfigurationsByChainId.
  • Validate that the defaultRpcEndpointIndex points to a valid and functional RPC endpoint for each network.
  • Check that Infura-based RPC URLs (e.g., Mainnet, Goerli, Sepolia, Linea networks) are properly formatted with the correct {infuraProjectId}.
  • Verify that networks like Linea Goerli, Linea Sepolia, and Linea Mainnet are listed under networkConfigurationsByChainId with their correct chainId values.
  • Ensure that any unused or outdated custom network configurations (e.g., BNB Chain from v20) have been removed.
  • Check that all network names (e.g., "Mainnet", "Goerli", "Sepolia") in networkConfigurationsByChainId match their respective chainId values.
  • Validate that nativeCurrency fields are correctly set for each network in networkConfigurationsByChainId (e.g., ETH for Mainnet, SepoliaETH for Sepolia).
  • Ensure that each network's blockExplorerUrls is present and correctly configured where applicable.

Screenshots/Recordings

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.

@cryptodev-2s cryptodev-2s added the Run Smoke E2E Triggers smoke e2e on Bitrise label Oct 9, 2024
Copy link
Contributor

github-actions bot commented Oct 9, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: dbf808b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/42668c05-bf0a-468e-baf2-8efa1d234112

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

@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 9, 2024
@salimtb salimtb force-pushed the salim/bump-network-controller-21.0.0 branch from 72c0e3d to 1601553 Compare October 10, 2024 11:46
@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 10, 2024
Copy link
Contributor

github-actions bot commented Oct 10, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 0a56a68
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b9218208-db83-49e5-983e-a004776e9b6f

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

@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 10, 2024
@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 10, 2024
Copy link

sonarcloud bot commented Oct 10, 2024

chainId === CHAIN_IDS.LINEA_MAINNET ||
chainId === CHAIN_IDS.GOERLI
) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is renderRpcNetworks skipped except for this set of hardcoded known chains? Is there any difference in functionality between the two cases?

Copy link
Contributor Author

@salimtb salimtb Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We skip rendering certain networks in the if block because we need to handle them separately to manage their order and display settings. Specifically, test networks (testnets) need to appear at the bottom of the list, and their visibility should be controlled by a toggle option (whether to show or hide them).

For the primary networks, Mainnet and Linea, they should always appear as the first and second items in the network list by default.

  • Mainnet is rendered using the renderMainnet function.
  • Linea is rendered with the renderLineaMainnet function.
  • All other networks are rendered through the renderRpcNetworks function.
  • Test networks (testnets) are rendered using the renderOtherNetworks function.

To manage the display order and ensure proper handling, we exclude these specific networks from the networkConfiguration state, allowing us to render them with greater control in the desired order.

@salimtb salimtb added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 11, 2024
Copy link
Contributor

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 0a76ef0
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/df452b65-2acf-443d-ae06-05250066aa20

Note

  • This comment will auto-update when build completes
  • 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

@metamaskbot
Copy link
Collaborator

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0a76ef0
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/df452b65-2acf-443d-ae06-05250066aa20

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

@salimtb
Copy link
Contributor Author

salimtb commented Oct 11, 2024

This pull request was merged into this feature branch, so I need approval on that feature branch. I'm keeping the PR open to make the review process easier.

github-merge-queue bot pushed a commit that referenced this pull request Oct 11, 2024
…etworks with Distinct ChainIDs and Multiple RPC URLs (#11705)

<!--
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**
This PR refactors our network configuration to eliminate the use of
multiple networks with the same ChainID but different RPC URLs. Instead,
we are moving towards a setup where each network is uniquely identified
by a distinct ChainID and can have multiple RPC URLs associated with it.

This PR includes three merge commits. The first primarily addresses the
Network Controller upgrade, as outlined in issue
#[11229](#11229). The
second commit contains the script for migrating the state to v21, and
the third commit includes all the UI changes along with the fix for the
e2e tests.

For more details, please refer to
[this](https://github.com/orgs/MetaMask/projects/120/views/1) .

related PRs:

- #11292
- #11622
- #11436

<!--
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?
-->

## **Related issues**

Fixes:
#[11229](#11229)
#[11232](#11232)
#[11234](#11234)
#11233

## **Manual testing steps**

1. Go to add network flow and test

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**


https://drive.google.com/drive/folders/149Xji42k5of5Vl8nBlI0pFYFgPnWqILH?usp=drive_link

<!-- [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.
@salimtb salimtb marked this pull request as draft October 13, 2024 16:06
@salimtb salimtb closed this Oct 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. Run Smoke E2E Triggers smoke e2e on Bitrise team-assets
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Network-controller-upgrade ] Update Controller Version to v21
7 participants