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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Queued request controller not working as expected in mmi build #28441

Open
hjetpoluru opened this issue Nov 13, 2024 · 1 comment
Open

[Bug]: Queued request controller not working as expected in mmi build #28441

hjetpoluru opened this issue Nov 13, 2024 · 1 comment
Labels
regression-develop Regression bug that was found on development branch, but not yet present in production release-12.7.0 Issue or pull request that will be included in release 12.7.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-api-platform type-bug

Comments

@hjetpoluru
Copy link
Contributor

Describe the bug

I was trying to fix the flaky test for the switchEthereum function, and there is inconsistent behavior where switching to the network does not happen.

Github ticket - #28195
Test Path - /test/e2e/json-rpc/switchEthereumChain.spec.js
Test scenario - Switch Ethereum Chain for two dapps queues switchEthereumChain request from second dapp after send tx request

Thanks to @seaona for mentioning about the defect #27977.
Later, I realized that Alex has made a fix #28371 and linking the Slack discussion in v12.7.0 RC Thread but it requires additional effort for the mmi I build due to the code fence.
Hence, I have created this ticket for tracking purposes.

Expected behavior

No response

Screenshots/Recordings

Screen.Recording.2024-11-13.at.9.40.57.AM.mov

Steps to reproduce

Perquisite: mmi build is needed

  1. Open the extension app and Toggle off the request queue setting.
  2. Open dApp with the Ethereum network
  3. Initiate a send transaction on the dApp.
  4. Switch to dApp to localhost using the switchNetwork command below in console log

await window.ethereum.request({"method":"wallet_switchEthereumChain",params: [{ chainId: '0x539' }],})
5. In the send transaction, switch and confirm the queued notification for switchEthereumChain.

Error messages or log output

No response

Detection stage

On the development branch

Version

12.7.0

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response
@hjetpoluru hjetpoluru added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-api-platform labels Nov 13, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Nov 13, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Nov 13, 2024
@hjetpoluru hjetpoluru added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Nov 13, 2024
@metamaskbot metamaskbot added the regression-develop Regression bug that was found on development branch, but not yet present in production label Nov 13, 2024
@adonesky1
Copy link
Contributor

Hmmm so this "bug" is happening on the normal extension build too, but I think instead of "fixing" it we should
a) switch this test to not turn off queueing
b) accelerate removing the toggle to turn off queuing which we've meant to do for a while now.

This was referenced Nov 20, 2024
Merged
Draft
adonesky1 added a commit to MetaMask/core that referenced this issue Nov 22, 2024
@adonesky1
chore: Remove useRequestQueue toggle to turn on/off Per Dapp Select…
4814cf1 
…ed Network Feature (#4941)

## Explanation

The "Select networks for each site" preference toggle on the
experimental settings page has been live for many releases now since the
toggle has been turned on by default. We meant to remove it a while ago.

<img width="770" alt="Screenshot 2024-11-19 at 10 58 04 AM"
src="https://github.com/user-attachments/assets/aeca2483-c019-4f9f-b44e-520d10db9eee">

This PR removes the togglability of per dapp selected network from the
`SelectedNetworkController` and `QueuedRequestMiddleware`

The extension to integrate this change will also have to implement a
migration to remove this state.

Another reason we should remove this now is that having this setting
turned off is [causing a
bug](MetaMask/metamask-extension#28441) to
result with `wallet_switchEthereumChain` and the interaction between the
new chain permissions feature and the absence of queuing.

## References

Extension PR w/ preview builds of this PR (plus full removal of the
toggle): MetaMask/metamask-extension#28577

When integrated in the extension will resolve this issue:
MetaMask/metamask-extension#28441

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/queued-request-controller`

- **REMOVED**: `createQueuedRequestMiddleware` no longer expects a
`useRequestQueue` property in its param options.


### `@metamask/selected-network-controller`

- **REMOVED**: SelectedNetworkController constructure no longer expects
either `useRequestQueuePreference` or `onPreferencesStateChange` params

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
regression-develop Regression bug that was found on development branch, but not yet present in production release-12.7.0 Issue or pull request that will be included in release 12.7.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-api-platform type-bug
Projects
Bugs by severity
Status: To be fixed
Bugs by team
Status: To be fixed
Milestone
No milestone
Development

No branches or pull requests
3 participants
@adonesky1 @metamaskbot @hjetpoluru