Skip to content

fix: SelectedNetworkController permission state change handler#4368

Merged
adonesky1 merged 5 commits intomainfrom
jl/fix-selected-network-controller-permission-state-change-handler
Jun 5, 2024
Merged

fix: SelectedNetworkController permission state change handler#4368
adonesky1 merged 5 commits intomainfrom
jl/fix-selected-network-controller-permission-state-change-handler

Conversation

@jiexi
Copy link
Copy Markdown
Member

@jiexi jiexi commented Jun 4, 2024

Explanation

Fixes a bug with SelectedNetworkController where it incorrectly sets the networkClientId for a newly permitted domain when the useRequestQueue flag is set to false.

References

See: MetaMask/metamask-extension#25046

Changelog

@metamask/selected-network-controller

  • FIXED: No longer sets the networkClientId for a newly permitted domain unless the useRequestQueuePreference flag is true

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

@jiexi jiexi marked this pull request as ready for review June 5, 2024 00:58
@jiexi jiexi requested a review from a team June 5, 2024 00:58
Gudahtt
Gudahtt previously approved these changes Jun 5, 2024
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!

…roller.test.ts

Co-authored-by: Alex Donesky <adonesky@gmail.com>
Copy link
Copy Markdown
Contributor

@adonesky1 adonesky1 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 pushed a commit to MetaMask/metamask-extension that referenced this pull request Jun 5, 2024
…25046)

Hotfix for v11.6.6 that patches @metamask/selected-network-controller
12.0.1 to include a check for if the useRequestQueue flag is set before
setting the networkClientId for a domain on permission controller state
change additions

* Core PR: MetaMask/core#4368
* Core patch branch:
https://github.com/MetaMask/core/tree/jl/patch-selected-network-controller-12.0.1-permission-state-change-guard

---------

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@adonesky1 adonesky1 merged commit ab6bf37 into main Jun 5, 2024
@adonesky1 adonesky1 deleted the jl/fix-selected-network-controller-permission-state-change-handler branch June 5, 2024 15:36
jiexi added a commit that referenced this pull request Jun 10, 2024
…uePreference guard (#4388)

## Explanation

Our previous SelectedNetworkController fix that [added a guard to
permission state changes](#4368)
was not sufficient. This PR properly addresses the underlying issue of
setting networkClientId for domains when the `useRequestQueuePreference`
flag is false by moving the guard into the `setNetworkClientId()` method
itself

## References

* Fixes: MetaMask/metamask-extension#25097

## 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/selected-network-controller`

- **FIXED**: `setNetworkClientId()` will now result in a noop if
`useRequestQueuePreference` is false


## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Alex Donesky <adonesky@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants