-
-
Notifications
You must be signed in to change notification settings - Fork 230
Correct invalid initial selectedNetworkClientId #5851
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
base: main
Are you sure you want to change the base?
Conversation
291f6b3
to
3d36c69
Compare
Currently, when NetworkController is instantiated with pre-existing state that contains an invalid `selectedNetworkClientId` — that is, no RPC endpoint exists which has the same network client ID — then it throws an error. This was intentionally done to bring attention to possible bugs in NetworkController, but this has the unfortunate side effect of bricking users' wallets. To fix this, we now correct an invalid `selectedNetworkClientId` to point to the default RPC endpoint of the first network sorted by chain ID (which in the vast majority of cases will be Mainnet). We still do want to know about this, though, so we log the error in Sentry.
3d36c69
to
409b54c
Compare
NetworkControllerActions, | ||
NetworkControllerEvents | ||
>; | ||
messenger: RootMessenger; |
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.
The type of the messenger in these tests only took internal actions and events into consideration and not external actions, so I've had to correct all of them.
@@ -87,7 +89,7 @@ export function buildNetworkControllerMessenger( | |||
): NetworkControllerMessenger { | |||
return messenger.getRestricted({ | |||
name: 'NetworkController', | |||
allowedActions: [], | |||
allowedActions: ['ErrorReportingService:captureException'], |
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.
Here we need to add ErrorReportingService:captureException
to the list of external actions because we are now using it in the controller.
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.
LGTM!
Explanation
Currently, when NetworkController is instantiated with pre-existing state that contains an invalid
selectedNetworkClientId
— that is, no RPC endpoint exists which has the same network client ID — then it throws an error. This was intentionally done to bring attention to possible bugs in NetworkController, but this has the unfortunate side effect of bricking users' wallets.To fix this, we now correct an invalid
selectedNetworkClientId
to point to the default RPC endpoint of the first network sorted by chain ID (which in the vast majority of cases will be Mainnet). We still do want to know about this, though, so we log the error in Sentry.References
Fixes #5739.
Checklist