Skip to content

Commit

Permalink
Fix SelectedNetworkController default metamask networkClientId (#1757)
Browse files Browse the repository at this point in the history
## Explanation

Currently, the SelectedNetworkController does not set a value for the
default `metamask` domain on initial `NetworkController:stateChange`
event because it does not count `selectedNetworkId` as actually changed
in the patch yet. This manifests itself in extension with the
SelectedNetworkMiddleware setting undefined for the
`req.networkClientId` attribute. ~~This PR adds one more check to the
event listener to ensure that the first `NetworkController:stateChange`
event is used to populate the networkClientId for the default `metamask`
domain.~~ See resolution in comments

## References

* Fixes MetaMask/MetaMask-planning#1370

## 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**: `setNetworkClientIdForDomain()` will now ignore the passed
in domain value and set the networkClientId for the `metamask` domain
instead when the `state.perDomainNetwork` flag is false (default)

## 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>
  • Loading branch information
jiexi and adonesky1 authored Oct 4, 2023
1 parent add71d1 commit 55546df
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ export class SelectedNetworkController extends BaseControllerV2<
networkClientId: NetworkClientId,
) {
this.update((state) => {
state.domains[domain] = networkClientId;
if (state.perDomainNetwork) {
state.domains[domain] = networkClientId;
return;
}
state.domains[METAMASK_DOMAIN] = networkClientId;
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,31 @@ describe('SelectedNetworkController', () => {
});

describe('setNetworkClientIdForDomain', () => {
it('can set the networkClientId for a domain', () => {
it('sets the networkClientId for the metamask domain, when the perDomainNetwork option is false (default)', () => {
const options: SelectedNetworkControllerOptions = {
messenger: buildSelectedNetworkControllerMessenger(), // Mock the messenger
};
const controller = new SelectedNetworkController(options);
const domain = 'example.com';
const networkClientId = 'network1';
controller.setNetworkClientIdForDomain(domain, networkClientId);
expect(controller.state.domains[domain]).toBe(networkClientId);
const networkClientId = 'network2';
controller.setNetworkClientIdForDomain('not-metamask', networkClientId);
expect(controller.state.domains.metamask).toBe(networkClientId);
});

it('can set the networkClientId for the metamask domain specifically', () => {
it('sets the networkClientId for the passed in domain, when the perDomainNetwork option is true ,', () => {
const options: SelectedNetworkControllerOptions = {
messenger: buildSelectedNetworkControllerMessenger(), // Mock the messenger
};
const controller = new SelectedNetworkController(options);
const networkClientId = 'network2';
controller.setNetworkClientIdForMetamask(networkClientId);
expect(controller.state.domains.metamask).toBe(networkClientId);
controller.state.perDomainNetwork = true;
const domain = 'example.com';
const networkClientId = 'network1';
controller.setNetworkClientIdForDomain(domain, networkClientId);
expect(controller.state.domains[domain]).toBe(networkClientId);
});
});

describe('getNetworkClientIdForDomain', () => {
it('gives the metamask domain when the perDomainNetwork option is false (default)', () => {
it('returns the networkClientId for the metamask domain, when the perDomainNetwork option is false (default)', () => {
const options: SelectedNetworkControllerOptions = {
messenger: buildSelectedNetworkControllerMessenger(), // Mock the messenger
};
Expand All @@ -55,7 +56,7 @@ describe('SelectedNetworkController', () => {
expect(result).toBe(networkClientId);
});

it('when the perDomainNetwork feature flag is on, it returns items other than the metamask domain', () => {
it('returns the networkClientId for the passed in domain, when the perDomainNetwork option is true', () => {
const options: SelectedNetworkControllerOptions = {
messenger: buildSelectedNetworkControllerMessenger(), // Mock the messenger
};
Expand Down

0 comments on commit 55546df

Please sign in to comment.