Skip to content

comment out effect in new-network-info#22964

Closed
jiexi wants to merge 1 commit intodevelopfrom
jl/debug-rpc-add-chain-freeze
Closed

comment out effect in new-network-info#22964
jiexi wants to merge 1 commit intodevelopfrom
jl/debug-rpc-add-chain-freeze

Conversation

@jiexi
Copy link
Copy Markdown
Member

@jiexi jiexi commented Feb 14, 2024

Description

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • 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 properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@jiexi jiexi requested a review from a team as a code owner February 14, 2024 23:28
@jiexi jiexi marked this pull request as draft February 14, 2024 23:28
@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.

@jiexi
Copy link
Copy Markdown
Member Author

jiexi commented Feb 14, 2024

Bisect Logs:

  • 90ae498

    • Feb 28th, 2023
    • does not occur
  • 131321a

    • Aug 29th, 2023
    • does not occur
  • 224bd6e

    • Nov 30th, 2023
    • occurs
  • 74a7ab5

    • Oct 31st, 2023
    • occurs
  • 0a215ca

    • Sep 30th, 2023
    • occurs
  • f134493

    • Sep 15th, 2023
    • I don't think so? Didn't see the empty error message
      But seemed to be able to get the provider/wallet unresponsive when trying to add an rpc confirmation shows spinner and then I closed the modal and tried to continue to use the dapp/wallet ui
    • if you wait for the spinner to finish, you'll get a try again and switch network buttons. Clicking switch networks and changing the network ensures you are not locked up
  • 315c043

    • Sep 22nd, 2023
    • occurs
  • 9f7ccfc

    • Sep 20th
    • does not occur
  • 0bc3aa9

    • Sep 21st
    • does not occur
  • 55475df

    • Sep 22nd, 2023 commit right before 315c043
    • occurs
  • 967ed42

    • mid day Sep 22nd
    • occurs
  • 1e65bb8

    • quarter day Sep 22nd
    • does not occur
  • 439de24

  • 5f368d5

    • commit right before 967ed42
    • occurs
  • d587999

    • commit right after 439de24 (right before 5f368d5)
    • I don't think so? Didn't see the empty error message
    • i don't seem to get a spinner when the rpc can't be reached, but canceling and continuing with other rpc seems fine
  • 5f368d5

    • confirmed occurs again
    • WE HAVE CULPRIT

adonesky1 added a commit that referenced this pull request Mar 1, 2024
…ate (#23130)

- Bumps to the the latest version of
`@metamask/selected-network-controller` which fixes issue where the
`SelectedNetworkController` adds any and all domains the user visits to
its `domains` state whether or not the user has connected to these
sites.
- Adds a migration to clean up the undesired state caused by this bug.
- Adds logic to listen for updates to permissions state and add newly
connected/permissioned dapps to `domains` state in the
`SelectedNetworkController`. I've created a ticket
[here](https://app.zenhub.com/workspaces/wallet-api-platform-63bee08a4e3b9d001108416e/issues/gh/metamask/metamask-planning/2142)
to move this logic into the `SelectedNetworkController` itself.
- Adds logic to couple the `useRequestQueue` feature flag with the
`perDomainNetwork` feature flag. I've created a ticket
[here](https://app.zenhub.com/workspaces/wallet-api-platform-63bee08a4e3b9d001108416e/issues/gh/metamask/metamask-planning/2143)
to remove the latter state since it is redundant and requires keeping
these two booleans in sync.
- Removes calls to `setNetworkClientIdForMetamask` since the [latest
version of
`@metamask/selected-network-controller`](https://github.com/MetaMask/core/blob/main/packages/selected-network-controller/CHANGELOG.md#800)
removes this method (and the `metamask` domain from the `domains` state
entirely).
- Adds logic to use provider and blockTracker from the
`SelectedNetworkController` when feature flag is on and connected origin
is in `domains` state (which can only happen when the dapp is connected)
in `setupProviderEngine` for RPC middleware stack.
- Fixes a bug where if a dapp prompts an `addEthereumChain` request and
the user then accepts the subsequent `switchEthereumChain` call that is
chained on, the `networkClientID` set for that dapp in the
`SelectedNetworkController` domains state will not be updated.

Manual Testing Steps

well first you need to [comment out some
code](#22964) that is
causing [a separate bug (found on
develop)](#22937)
but one that will make manually testing this a nightmare.

1. Go to `settings` > `experimental` > `Select networks for each site`
and toggle this setting on
2. Put the globally selected network to ethereum mainnet
3. Connect to [uniswap](https://app.uniswap.org/swap)
4. Connect to [pancakeswap](https://pancakeswap.finance/)
5. Vist [Aave](https://app.aave.com/) but don't connect the wallet
6. Open devtools in the wallet UI and run the follow command in the
console:
```
const { metamask } = await window.stateHooks.getCleanAppState();
console.log(metamask.domains)
```
7. You should see the following log:
```
{https://app.uniswap.org: 'mainnet', https://pancakeswap.finance: 'mainnet'}
```
8. Now call `switchEthereumChain` on one of the connected dapps (they
each have UI elements that do this)
9. Now go back to the other dapp (where the chain hasn't been switched)
and initiate a transaction
10. You should receive a `switchEthereumChain` confirmation back to
Ethereum Mainnet before the transaction confirmation is show.
@jiexi jiexi closed this Mar 27, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2024
@HowardBraham HowardBraham deleted the jl/debug-rpc-add-chain-freeze branch January 19, 2026 21:03
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.

1 participant