set last provider when switching to a customRPC#10084
Conversation
d3c042a to
fecb664
Compare
Builds ready [fecb664]
Page Load Metrics (576 ± 16 ms)
|
|
Great find! Any idea when this bug was introduced? |
|
Actually after going another layer down, this wasn't a regression. I fixed a different issue before I joined MetaMask (#8197 my first merged PR) and I originally hunted this down because it felt like a regression of that fix. I think, though, this has existed for a while -- at least I don't see where lastConnectedProvider was set ever in this flow as I go back through the history |
| closeMenu={() => this.props.hideNetworkDropdown()} | ||
| onClick={() => { | ||
| if (isPrefixedFormattedHexString(chainId)) { | ||
| setPreviousProvider(providerType) |
There was a problem hiding this comment.
What does this do when providerType is rpc 🤔
Maybe this is a separate bug that is out of scope here, but I think it would blow up in that case, as setProviderType in the network controller asserts that type isn't rpc.
There was a problem hiding this comment.
Hmm, good point. I'll address this in a follow-up.
Gudahtt
left a comment
There was a problem hiding this comment.
LGTM! I suspect this behavior is still broken in the case where it tries to switch back to a custom RPC endpoint, but that is independent of what you're addressing here.
Fixes: a regression in falling back to the last connected provider when attempting to connect to a custom rpc (localhost as an example). Previously the last successfully connected provider would be reconnected. Presently when attempting to connect to a custom RPC the currently connected provider is not set as the 'lastConnectedProvider', which means that the provider the UI falls back to in the case of an error or user exiting the loading screen is not the most recently connected provider. In some cases, such as if the user loads the extension and immediately chooses a custom RPC, the fallback provider is 'ropsten' which is hardcoded in the UI in case there is no
lastConnectedProvider.This PR resolves this by adding the call to
setPreviousProviderto the custom RPC code. All other cases use thesetProviderTypewhich triggers the correct UI flow of first setting thelastConnectedProvider