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

fix: hotfix network version / unresponsive network inpage … #30114

Merged

Conversation

adonesky1
Copy link
Contributor

@adonesky1 adonesky1 commented Feb 4, 2025

…provider (#30111)

Previously the inpage provider would withhold events for chainChanged events (and property value updates, i.e. window.ethereum.chainId and .networkVersion) when the dapp's network was changed to an rpc endpoint that was unresponsive or did not support net_version. The dapp would instead receive a disconnect event.

Now the inpage provider always emits chainChanged and networkChanged events (and exposes the correct values on window.ethereum.chainId and .networkVersion) when the selected network for the dapp has changed regardless of if the network being changed to is responsive or if it supports net_version requests. It does this by having the wallet send a loading for networkVersion when it cannot be resolved (same behavior as before) AND a new isConnected property in the metamask_getProviderState request and metamask_chainChanged events (these are different from the events emittted by window.ethereum). isConnected is derived from whether the NetworkController.state.networkMetadata[].status value is the Available constant.

Open in GitHub Codespaces

See: #29936 Providers patch from commit (d919ab6b):
MetaMask/providers#404

window.ethereum.on('accountsChanged', (data) => console.log('accountsChanged', data))
window.ethereum.on('chainChanged', (data) => console.log('chainChanged', data))
window.ethereum.on('networkChanged', (data) => console.log('networkChanged', data))
window.ethereum.on('connect', (data) => console.log('connect', data))
window.ethereum.on('disconnect', (data) => console.log('disconnect', data))
  1. Go to a webpage. Enter the following in console
  2. Change to Linea, see that the correct values are emitted for the chainChanged and networkChanged events
  3. Change to Sepolia, see that the correct values are emitted for the chainChanged and networkChanged events
  4. Change to a network that is non-responsive, see that chainChanged emits with the correct chainId, but networkChanged emits with a null value, AND there is a disconnect event emitted
  5. Change back to a working network, see that the correct values are emitted for the chainChanged and networkChanged events, AND there is a connect event emitted with the new chainId
  6. Do the same above with a responsive network that does not have net_version implemented. Se that the correct values are emitted for the chainChanged, that networkChanged emits null, and that there is no disconnect event emitted

Screenshot 2025-02-04 at 8 34 13 AM

Screenshot 2025-02-04 at 8 39 33 AM

  • I've followed MetaMask Contributor Docs and MetaMask Extension Coding
    Standards
    .

  • I've completed the PR template to the best of my ability

  • I’ve included tests if applicable

  • I’ve documented my code using JSDoc format if applicable

  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).

  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Description

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

…provider (#30111)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

Previously the inpage provider would withhold events for chainChanged
events (and property value updates, i.e. window.ethereum.chainId and
.networkVersion) when the dapp's network was changed to an rpc endpoint
that was unresponsive or did not support net_version. The dapp would
instead receive a disconnect event.

Now the inpage provider always emits chainChanged and networkChanged
events (and exposes the correct values on window.ethereum.chainId and
.networkVersion) when the selected network for the dapp has changed
regardless of if the network being changed to is responsive or if it
supports net_version requests. It does this by having the wallet send a
loading for networkVersion when it cannot be resolved (same behavior as
before) AND a new isConnected property in the metamask_getProviderState
request and metamask_chainChanged events (these are different from the
events emittted by window.ethereum). isConnected is derived from whether
the NetworkController.state.networkMetadata[].status value is the
Available constant.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30109?quickstart=1)

See: #29936
Providers patch from commit (d919ab6b):
MetaMask/providers#404
```
window.ethereum.on('accountsChanged', (data) => console.log('accountsChanged', data))
window.ethereum.on('chainChanged', (data) => console.log('chainChanged', data))
window.ethereum.on('networkChanged', (data) => console.log('networkChanged', data))
window.ethereum.on('connect', (data) => console.log('connect', data))
window.ethereum.on('disconnect', (data) => console.log('disconnect', data))
```
1. Go to a webpage. Enter the following in console
2. Change to Linea, see that the correct values are emitted for the
`chainChanged` and `networkChanged` events
3. Change to Sepolia, see that the correct values are emitted for the
`chainChanged` and `networkChanged` events
4. Change to a network that is non-responsive, see that `chainChanged`
emits with the correct chainId, but `networkChanged` emits with a null
value, AND there is a `disconnect` event emitted
5. Change back to a working network, see that the correct values are
emitted for the `chainChanged` and `networkChanged` events, AND there is
a `connect` event emitted with the new chainId
6. Do the same above with a responsive network that does not have
`net_version` implemented. Se that the correct values are emitted for
the `chainChanged`, that `networkChanged` emits null, and that there is
no `disconnect` event emitted

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<!-- [screenshots/recordings] -->

![Screenshot 2025-02-04 at 8 34
13 AM](https://github.com/user-attachments/assets/f6558363-f848-445a-811e-d1a052a4e6c0)

![Screenshot 2025-02-04 at 8 39
33 AM](https://github.com/user-attachments/assets/12bff524-bb3c-4bf8-b4d2-c6a01ce63b33)

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Copy link
Contributor

github-actions bot commented Feb 4, 2025

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.

Copy link

socket-security bot commented Feb 4, 2025

@jiexi jiexi changed the title fix: cp-12.10.4 hotfix network version / unresponsive network inpage … fix: hotfix network version / unresponsive network inpage … Feb 4, 2025
@adonesky1
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot metamaskbot requested review from a team as code owners February 4, 2025 19:49
@danjm danjm merged commit 336c2a3 into Version-v12.10.4 Feb 4, 2025
60 of 61 checks passed
@danjm danjm deleted the ad/cherry-pick-provider-version-bump-12.10.4 branch February 4, 2025 21:05
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [990860e]
Page Load Metrics (1721 ± 71 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14922088172615072
domContentLoaded14762075169914469
load14852087172114771
domInteractive24141433115
backgroundConnect105222147
firstReactRender16101422713
getState45313157
initialActions00000
loadScripts10971665127012660
setupStore66714168
uiStartup16972442197720498

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

Successfully merging this pull request may close these issues.

5 participants