Skip to content

Migration 51: ensure chainId is set in state for default/infura providers#10170

Merged
danjm merged 3 commits intodevelopfrom
ensure-chainid-is-set-for-infura-networks
Jan 11, 2021
Merged

Migration 51: ensure chainId is set in state for default/infura providers#10170
danjm merged 3 commits intodevelopfrom
ensure-chainid-is-set-for-infura-networks

Conversation

@danjm
Copy link
Copy Markdown
Contributor

@danjm danjm commented Jan 11, 2021

Currently, some users are experiencing a disabled swaps buttom on the home screen, in v8.1.11

This can be recreated by:

  • building v8.1.4 (or any other version <v8.1.8) locally (you'll notice swaps button is enabled)
  • checking out the commit for v8.1.11 and building that locally (now swaps button is disabled)

This is because chainId wasn't set to a default in the network controller provider state until #9999 shipped in v8.1.8. However, it will get set whenever a user switches network. Users that first installed metamask prior to v8.1.8 but have not switched networks away from mainnet since installation will not have a chainId in their network controller provider state. With the changes in #10155, this will cause swaps to be disabled as their chainId in state will not match the hard-coded mainnet chainId

This PR fixes that issue by ensure that the provider data in state is updated to include the correct chainId for all default / infura networks (i.e. those for which we have hard coded chainIds).

This will also prevent similar errors from happening if/when we add any future code changes that rely on comparison of chainId in state to hard coded chainIds

Note: the hard coded chainIds can be found in controllers/network/enums.js

@danjm danjm requested a review from a team as a code owner January 11, 2021 04:20
@danjm danjm requested a review from NiranjanaBinoy January 11, 2021 04:20
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [be702d8]
Page Load Metrics (887 ± 63 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint4911468168
domContentLoaded556117588613163
load558117688713263
domInteractive556117488513163

Copy link
Copy Markdown
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One nit and one question.

@danjm
Copy link
Copy Markdown
Contributor Author

danjm commented Jan 11, 2021

@rekmarks comments addressed in f3f4d78

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [f3f4d78]
Page Load Metrics (819 ± 64 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaintNaNNaNNaNNaNNaN
domContentLoaded45294781713264
load45495681913364
domInteractive45294681613264

rekmarks
rekmarks previously approved these changes Jan 11, 2021
Copy link
Copy Markdown
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n/a

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [45b2280]
Page Load Metrics (636 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint347255105
domContentLoaded3537756349948
load3557776369948
domInteractive3537756349948

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@danjm danjm merged commit 2431a58 into develop Jan 11, 2021
@danjm danjm deleted the ensure-chainid-is-set-for-infura-networks branch January 11, 2021 14:39
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants