-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
Remove providerConfig from NetworkController #4254
base: main
Are you sure you want to change the base?
Conversation
d1359dd
to
9b3cc0a
Compare
Historically, the `providerConfig` property in NetworkController has been used to track the currently selected network as well as provide access to information about that network. The selected network is now tracked via `selectedNetworkClientId`, and information about that network can be retrieved by looking at the `networkConfigurations` property or the `configuration` property on the NetworkClient interface. This means that we no longer need `providerConfig` and we can remove this redundant state.
9b3cc0a
to
730b264
Compare
); | ||
const { chainId } = selectedNetworkClient.configuration; | ||
|
||
if (this.config.chainId !== chainId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a test-case for when chainId
differs? If not, that could explain the reduced coverage %?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into this and I don't think it's due to additional logic. Somehow I have made something that was inadvertently tested not tested anymore. Regrettably I have added an istanbul
directive to ignore the logic in question: be7f9c2
I'm going to break this up into multiple PRs as this one is rather large and difficult to review. Placing in draft until that's complete.
After that we can rebase this PR and reopen it for review. |
Explanation
Historically, the
providerConfig
property in NetworkController has been used to track the currently selected network as well as provide access to information about that network. The selected network is now tracked viaselectedNetworkClientId
, and information about that network can be retrieved by looking at thenetworkConfigurations
property or theconfiguration
property on the NetworkClient interface. This means that we no longer needproviderConfig
and we can remove this redundant state.References
Fixes #4185.
Changelog
@metamask/assets-controllers
Changed
NetworkController:getNetworkClientById
(#4254)@metamask/ens-controller
Changed
NetworkController:getNetworkClientById
(#4254)onNetworkDidChange
so that it may take the full NetworkController state instead of simply an object with aproviderConfig
property (#4254)@metamask/network-controller
Removed
providerConfig
property from state along withProviderConfig
type andNetworkController:getProviderConfig
messenger action (#4254)state.selectedNetworkClientId
, pass this to theNetworkController:getNetworkClientId
messenger action, and then access theconfiguration
property on the network client.networksMetadata
state property so that the keys in the object will only ever contain network client IDs and not RPC URLs (#4254)Checklist