Skip to content
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

NetworkController API normalization: Add rollbackToPreviousProvider method. #1024

Closed
Gudahtt opened this issue Dec 13, 2022 · 3 comments · Fixed by #1132
Closed

NetworkController API normalization: Add rollbackToPreviousProvider method. #1024

Gudahtt opened this issue Dec 13, 2022 · 3 comments · Fixed by #1132
Assignees
Labels

Comments

@Gudahtt
Copy link
Member

Gudahtt commented Dec 13, 2022

We should add a method for rolling back to the previous provider config. This can be useful for handling connection failures when switching to a network.

This is part of a larger effort to normalize the API of both network controllers, to make them easier to merge.

@adonesky1
Copy link
Contributor

adonesky1 commented Mar 15, 2023

I realize this might be out of scope for this issue, but since NetworkConfigurations now live on the NetworkController rather than keeping a record of the full previous provider config, should we just keep previousProvider state value of either the provider type (in the case of an infura network) or the networkConfigurationId (in the case of a custom network) and then use either setProviderType or setActiveNetwork to roll it back? cc @Gudahtt @mcmire

@Gudahtt
Copy link
Member Author

Gudahtt commented Mar 15, 2023

That would make sense to me! The way in which we roll back is an internal implementation detail, we don't need it to match the extension. And for the previous provider state, we don't need to store that as controller state at all. I had planned on removing it from the extension (in this PR) since we don't use it anywhere except in the implementation of this method.

@adonesky1
Copy link
Contributor

And for the previous provider state, we don't need to store that as controller state at all

Ah yea we don't need it persisted to localStorage since its only ever relevant when attempting to change networks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants