Conversation
| }, | ||
| }, | ||
| }, | ||
| ...mockNetworkState({ |
There was a problem hiding this comment.
We could reduce the size of this PR by implementing this mockNetworkState helper in a separate PR, before the rest of the changes.
| let name = strings('network_information.unknown_network'); | ||
| if (providerConfig.nickname) { | ||
| name = providerConfig.nickname; | ||
| } else if (providerConfig.chainId === NETWORKS_CHAIN_ID.MAINNET) { |
There was a problem hiding this comment.
Why were these conditions added? For starters, the nickname property is guaranteed to be set for Ethereum Mainnet and Linea Mainnet, so this case will never be hit. And second, these nicknames are different than they were previously.
There was a problem hiding this comment.
this condition is responsible to display the network name on the select network nav bar , in mobile the network name for mainnet must always be Ethereum Main Network , I didn't want to change this in order to continue having the same behavior.
| engine: { | ||
| backgroundState: { | ||
| NetworkController: { | ||
| networkConfigurations: {}, |
There was a problem hiding this comment.
Was this change necessary? It's unclear why it would be required
There was a problem hiding this comment.
This change is needed because the updated state in util/test/initial-root-state no longer includes the networkConfigurations property ( because the network controller has been moved to v21 ). As a result, the migration script fails since it relies on the old state format that contains this property.
|
|
|
|
|
| chainId === CHAIN_IDS.LINEA_MAINNET || | ||
| chainId === CHAIN_IDS.GOERLI | ||
| ) { | ||
| return null; |
There was a problem hiding this comment.
Why is renderRpcNetworks skipped except for this set of hardcoded known chains? Is there any difference in functionality between the two cases?
There was a problem hiding this comment.
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
renderMainnetfunction. - Linea is rendered with the
renderLineaMainnetfunction. - All other networks are rendered through the
renderRpcNetworksfunction. - Test networks (testnets) are rendered using the
renderOtherNetworksfunction.
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.
|
|
|
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. |
7 participants
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
Related PRS
Related issues
Fixes: #11229
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist