Behavior for NetworkController.findNetworkClientIdByChainId method/action is potentially surprising if more than one network has same chain ID

[mcmire]

This method returns the network client that hits the chain with the given identifier. This is a problem because more than one network client can be assigned to the same chain ID. This method just returns the first one that matches, but that may not be the network client that the consumer expected, especially if the user has added a custom network that overrides a built-in one (for instance, Mainnet).

[Gudahtt]

We intentionally mirrored the behavior of wallet_switchEthereumChain when implementing this. It prefers built-in networks first, and second to that it prefers older configurations to newer ones.

I wouldn't call it undefined, but it's certainly not a reasonable solution. For wallet_switchEthereumChain, I believe the plan was to update the UI to let the user choose a specific RPC endpoint. I'm not sure what other cases exist though, that solution might not be applicable to all of them.

[mcmire]

Right, it's coming back to me now, thanks for the pointer on that. I've updated the PR title to be more accurate.

The reason I'm bringing up this method is that now that it's part of the public API, we might be tempted to use it in order to get from chain ID to network client ID (in fact I've seen it pop up in a PR recently). The existence of this method implies that there is a one-to-one mapping between chain ID and network client ID, but that's not true at all. So it's a pretty big footgun — but you wouldn't know it if you just stumbled across this.

Considering that we had to create this for wallet_switchEthereumChain, but this is the only use case, I'm wondering if we should make this method intentionally difficult to use somehow — push this on to the consumer. Or at least we should rename this method so that people understand it's not safe to use.

[Gudahtt]

We can mark it as deprecated, that would work as a warning against using it

[mcmire]

This issue is still relevant, and I would mark it as a defect that we need to rectify. The Wallet API team can still use this method but I agree with deprecating it so that people do not use it long-term.

[desi]

We believe we can fix the method now rather than deprecate it.

[desi]

This might impact work on the Assets team.

[mcmire]

For more details on how we can fix the method, we can now:

• look up a network configuration with the given chain ID
• use the defaultRpcEndpointIndex to find the default RPC endpoint for that network configuration
• get the network client ID for that RPC endpoint
• look up the network client with that ID