selected network controller: redirect domains to default endpoint #4679
Conversation
| Object.entries(this.state.domains).forEach( | ||
| ([domain, networkClientIdForDomain]) => { | ||
| const chainIdForDomain = | ||
| networkClientIdToChainId[networkClientIdForDomain]; | ||
|
|
||
| if (patch.op === 'remove' && !chainIdForDomain) { | ||
| // If the network was removed, fall back to the globally selected network | ||
| this.setNetworkClientIdForDomain( | ||
| domain, | ||
| selectedNetworkClientId, | ||
| ); |
There was a problem hiding this comment.
Wouldn't this set the chainId for the domain to the globally selected network if we just removed the rpcUrl that it was connected to? I would assume in this case we'd want to replace it with the new "default" rpcUrl for that chainId? And, if that network was removed entirely (not just the rpcUrl) we'd set it to the globally selected network?
There was a problem hiding this comment.
oh does the remove op only apply if the entire chainId is removed?
There was a problem hiding this comment.
Yes, the case you described of removing an RPC url falls into the replace operation.
Deleting the whole network falls into the remove operation.
|
I also updated the migration in MetaMask/metamask-extension#26433 to point domains to their default endpoint so its correct initially. |
| const patch = patches.find( | ||
| ({ op, path }) => | ||
| (op === 'replace' || op === 'remove') && | ||
| path[0] === 'networkConfigurationsByChainId', |
There was a problem hiding this comment.
Thoughts on using filter vs find? Not that it does currently, but patches could include multiple state changes
There was a problem hiding this comment.
This whole technique won't work if there are multiple changes. It relies on the fact that the network controller only exposes an APIs to update or remove a single network at a time.
If multiple networks were updated in 1 update, you wouldn't be able to correlate the missing network client ids to a particular chain id. Since there's only 1, we can do it with path[1].
This is relevant for the case where you simultaneously delete the default endpoint and replace it with a new one in 1 update
There was a problem hiding this comment.
unless I missed some overload of subscribe that gives you enough information to do so
bergeron
deleted the
brian/temp-selected-network-redirect-to-default2
branch
Explanation
Changes how the
SelectedNetworkControllerresponds to state updates from the network controller. Now when the default RPC endpoint changes for a network, it will redirect domains that were referencing a network client id on that network to the new default endpoint for that network. Instead of redirecting them to the globally selected network, as happened before.Accompanies MetaMask/metamask-extension#26433
References
Changelog
@metamask/selected-network-controllerChecklist