Skip to content

Commit

Permalink
fix: cp-12.10.4 hotfix network version / unresponsive network inpage …
Browse files Browse the repository at this point in the history
…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.
-->

## **Description**

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)

## **Related issues**

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

## **Manual testing steps**
```
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


## **Screenshots/Recordings**

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

### **Before**

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

### **After**

![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)



## **Pre-merge author checklist**

- [ ] 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.

## **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.
  • Loading branch information
jiexi authored Feb 4, 2025
1 parent 200be79 commit af80c7b
Show file tree
Hide file tree
Showing 4 changed files with 783 additions and 13 deletions.
Loading

0 comments on commit af80c7b

Please sign in to comment.