Skip to content

Commit

Permalink
fix: SelectedNetworkController permission state change handler (#4368)
Browse files Browse the repository at this point in the history
## 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

<!--
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**: No longer sets the networkClientId for a newly permitted
domain unless the `useRequestQueuePreference` flag is true


## 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
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Alex Donesky <[email protected]>
  • Loading branch information
jiexi and adonesky1 authored Jun 5, 2024
1 parent ba07fd8 commit ab6bf37
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,11 @@ export class SelectedNetworkController extends BaseController<
path[0] === 'subjects' && path[1] !== undefined;
if (isChangingSubject && typeof path[1] === 'string') {
const domain = path[1];
if (op === 'add' && this.state.domains[domain] === undefined) {
if (
op === 'add' &&
this.state.domains[domain] === undefined &&
this.#useRequestQueuePreference
) {
this.setNetworkClientIdForDomain(
domain,
this.messagingSystem.call('NetworkController:getState')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,8 +617,10 @@ describe('SelectedNetworkController', () => {
});

describe('When a permission is added or removed', () => {
it('should add new domain to domains list on permission add', async () => {
const { controller, messenger } = setup();
it('should add new domain to domains list on permission add if #useRequestQueuePreference is true', async () => {
const { controller, messenger } = setup({
useRequestQueuePreference: true,
});
const mockPermission = {
parentCapability: 'eth_accounts',
id: 'example.com',
Expand All @@ -638,6 +640,29 @@ describe('SelectedNetworkController', () => {
expect(domains['example.com']).toBeDefined();
});

it('should not add new domain to domains list on permission add if #useRequestQueuePreference is false', async () => {
const { controller, messenger } = setup({
useRequestQueuePreference: false,
});
const mockPermission = {
parentCapability: 'eth_accounts',
id: 'example.com',
date: Date.now(),
caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }],
};

messenger.publish('PermissionController:stateChange', { subjects: {} }, [
{
op: 'add',
path: ['subjects', 'example.com', 'permissions'],
value: mockPermission,
},
]);

const { domains } = controller.state;
expect(domains['example.com']).toBeUndefined();
});

describe('on permission removal', () => {
it('should remove domain from domains list', async () => {
const { controller, messenger } = setup({
Expand Down

0 comments on commit ab6bf37

Please sign in to comment.