chore(network-controller): Simplify network switch detection in lookupNetwork #6391
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ookupNetwork A while back, `lookupNetworkByClientId` was added a while back, presumably as a multichain-compatible replacement of `lookupNetwork`. However, `lookupNetwork` already takes an (optional) network client ID; in fact, it calls `lookupNetworkByClientId` internally. This duplication is confusing, and it would make for a simpler API and make NetworkController more maintainable if we removed `lookupNetworkByClientId` entirely. This commit does that and refactors `lookupNetwork` to be more understandable. It also ensures that `lookupNetwork` is well-tested both when given a network client ID and not given one.
…pNetwork `lookupNetwork` operates on an arbitrary network or, if given nothing, the globally selected network. The behavior between these two cases is largely the same, except that in the "selected network" case, it publishes one of two events, `NetworkController:infuraIsBlocked` or `NetworkController:infuraIsUnblocked`. This happens after making a request to the network, and while the request is running, it is possible for the network to change. This creates a potential problem because if that happens, the events that we want to fire would no longer represent the network they were intended to represent. `lookupNetwork` accounts for this by subscribing to `NetworkController:networkDidChange` before starting the request; if it's fired, then it exits early (with the presumption that the network switch called its own version of `lookupNetwork` and will publish `NetworkController:infuraIsBlocked` or `NetworkController:infuraIsUnblocked` at that point in time). However, this approach to detect a network switch is complicated and unnecessary. We can simply capture `selectedNetworkClientId` before we start the request, ask what it is afterward, and compare the two to know whether the network switched. This means that we can bring the implementation of the two branches within `lookupNetwork` closer together (and we can eliminate some tests).
|
Adding |
Base automatically changed from
remove-lookup-network-by-network-client-id
to
main
August 27, 2025 19:08
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
lookupNetworkoperates on an arbitrary network or, if given nothing, the globally selected network. The behavior between these two cases is largely the same, except that in the "selected network" case, it publishes one of two events,NetworkController:infuraIsBlockedorNetworkController:infuraIsUnblocked. This happens after making a request to the network, and while the request is running, it is possible for the network to change. This creates a potential problem because if that happens, the events that we want to fire would no longer represent the network they were intended to represent.lookupNetworkaccounts for this by subscribing toNetworkController:networkDidChangebefore starting the request; if it's fired, then it exits early (with the presumption that the network switch called its own version oflookupNetworkand will publishNetworkController:infuraIsBlockedorNetworkController:infuraIsUnblockedat that point in time).However, this approach to detect a network switch is complicated and unnecessary. We can simply capture
selectedNetworkClientIdbefore we start the request, ask what it is afterward, and compare the two to know whether the network switched. This means that we can bring the implementation of the two branches withinlookupNetworkcloser together (and we can eliminate some tests).References
Checklist