Skip to content

Commit

Permalink
feat: remove updateIdentities and make syncIdentities private in pref…
Browse files Browse the repository at this point in the history 
…erences controller (#3976)

## Explanation

After completion of #3794 and
#3699, the method
`syncIdentities` is now only used internally on
`KeyringController:stateChange` event and `updateIdentities` is no
longer being used.

## References

* Fixes #3795

## Changelog

### `@metamask@preferences-controller`

- **REMOVED**: `syncIdentities` is now private as it's only used
internally to update state on KeyringController:stateChange event.
- **REMOVED**: `updateIdentities` has been removed, as it's not in use
anymore.

## 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
  • Loading branch information
@cryptodev-2s
cryptodev-2s authored Feb 27, 2024
1 parent d95851d commit 969dfd5
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 151 deletions.
158 changes: 47 additions & 111 deletions packages/preferences-controller/src/PreferencesController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,35 @@ describe('PreferencesController', () => {
expect(controller.state.selectedAddress).toBe('0x00');
});

it('should maintain existing identities when no accounts are present in keyrings', () => {
const identitiesState = {
'0x00': { address: '0x00', importTime: 1, name: 'Account 1' },
'0x01': { address: '0x01', importTime: 2, name: 'Account 2' },
'0x02': { address: '0x02', importTime: 3, name: 'Account 3' },
};
const messenger = getControllerMessenger();
const controller = setupPreferencesController({
options: {
state: {
identities: cloneDeep(identitiesState),
selectedAddress: '0x00',
},
},
messenger,
});

messenger.publish(
'KeyringController:stateChange',
{
...getDefaultKeyringState(),
keyrings: [{ accounts: [], type: 'CustomKeyring' }],
},
[],
);

expect(controller.state.identities).toStrictEqual(identitiesState);
});

it('should not update existing identities', () => {
const identitiesState = {
'0x00': { address: '0x00', importTime: 1, name: 'Account 1' },
Expand Down Expand Up @@ -305,117 +334,6 @@ describe('PreferencesController', () => {
expect(controller.state.identities['0x01'].name).toBe('qux');
});

it('should sync identities', () => {
const controller = setupPreferencesController();
controller.addIdentities(['0x00', '0x01']);
controller.syncIdentities(['0x00', '0x01']);
expect(controller.state.identities['0x00'].address).toBe('0x00');
expect(controller.state.identities['0x00'].name).toBe('Account 1');
expect(controller.state.identities['0x00'].importTime).toBeLessThanOrEqual(
Date.now(),
);
expect(controller.state.identities['0x01'].address).toBe('0x01');
expect(controller.state.identities['0x01'].name).toBe('Account 2');
expect(controller.state.identities['0x01'].importTime).toBeLessThanOrEqual(
Date.now(),
);
controller.syncIdentities(['0x00']);
expect(controller.state.identities['0x00'].address).toBe('0x00');
expect(controller.state.identities['0x00'].name).toBe('Account 1');
expect(controller.state.selectedAddress).toBe('0x00');
});

it('should throw error when syncing identities with empty array', () => {
const controller = setupPreferencesController();
expect(() => {
controller.syncIdentities([]);
}).toThrow('Expected non-empty array of addresses');
});

it('should add new identities', () => {
const controller = setupPreferencesController();
controller.updateIdentities(['0x00', '0x01']);
expect(controller.state.identities['0x00'].address).toBe('0x00');
expect(controller.state.identities['0x00'].name).toBe('Account 1');
expect(controller.state.identities['0x00'].importTime).toBeLessThanOrEqual(
Date.now(),
);
expect(controller.state.identities['0x01'].address).toBe('0x01');
expect(controller.state.identities['0x01'].name).toBe('Account 2');
expect(controller.state.identities['0x01'].importTime).toBeLessThanOrEqual(
Date.now(),
);
});

it('should not update existing identities', () => {
const controller = setupPreferencesController({
options: {
state: {
identities: { '0x01': { address: '0x01', name: 'Custom name' } },
},
},
});
controller.updateIdentities(['0x00', '0x01']);
expect(controller.state.identities['0x00'].address).toBe('0x00');
expect(controller.state.identities['0x00'].name).toBe('Account 1');
expect(controller.state.identities['0x00'].importTime).toBeLessThanOrEqual(
Date.now(),
);
expect(controller.state.identities['0x01'].address).toBe('0x01');
expect(controller.state.identities['0x01'].name).toBe('Custom name');
expect(controller.state.identities['0x01'].importTime).toBeUndefined();
});

it('should remove identities', () => {
const controller = setupPreferencesController({
options: {
state: {
identities: {
'0x01': { address: '0x01', name: 'Account 2' },
'0x00': { address: '0x00', name: 'Account 1' },
},
},
},
});
controller.updateIdentities(['0x00']);
expect(controller.state.identities).toStrictEqual({
'0x00': { address: '0x00', name: 'Account 1' },
});
});

it('should not update selected address if it is still among identities', () => {
const controller = setupPreferencesController({
options: {
state: {
identities: {
'0x01': { address: '0x01', name: 'Account 2' },
'0x00': { address: '0x00', name: 'Account 1' },
},
selectedAddress: '0x01',
},
},
});
controller.updateIdentities(['0x00', '0x01']);
expect(controller.state.selectedAddress).toBe('0x01');
});

it('should update selected address to first identity if it was removed from identities', () => {
const controller = setupPreferencesController({
options: {
state: {
identities: {
'0x01': { address: '0x01', name: 'Account 2' },
'0x02': { address: '0x02', name: 'Account 3' },
'0x00': { address: '0x00', name: 'Account 1' },
},
selectedAddress: '0x02',
},
},
});
controller.updateIdentities(['0x00', '0x01']);
expect(controller.state.selectedAddress).toBe('0x00');
});

it('should set IPFS gateway', () => {
const controller = setupPreferencesController();
controller.setIpfsGateway('https://ipfs.infura.io/ipfs/');
Expand Down Expand Up @@ -443,6 +361,24 @@ describe('PreferencesController', () => {
expect(controller.state.useNftDetection).toBe(true);
});

it('should throw an error when useNftDetection is set and openSeaEnabled is false', () => {
const controller = setupPreferencesController();
controller.setOpenSeaEnabled(false);
expect(() => controller.setUseNftDetection(true)).toThrow(
'useNftDetection cannot be enabled if openSeaEnabled is false',
);
});

it('should set featureFlags', () => {
const controller = setupPreferencesController();
controller.setFeatureFlag('Feature A', true);
controller.setFeatureFlag('Feature B', false);
expect(controller.state.featureFlags).toStrictEqual({
'Feature A': true,
'Feature B': false,
});
});

it('should set securityAlertsEnabled', () => {
const controller = setupPreferencesController();
controller.setSecurityAlertsEnabled(true);
Expand Down
42 changes: 2 additions & 40 deletions packages/preferences-controller/src/PreferencesController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export class PreferencesController extends BaseController<
}
}
if (accounts.size > 0) {
this.syncIdentities(Array.from(accounts));
this.#syncIdentities(Array.from(accounts));
}
},
);
Expand Down Expand Up @@ -321,14 +321,8 @@ export class PreferencesController extends BaseController<
* Synchronizes the current identity list with new identities.
*
* @param addresses - List of addresses corresponding to identities to sync.
* @returns Newly-selected address after syncing.
* @deprecated This will be removed in a future release
*/
syncIdentities(addresses: string[]) {
if (!addresses.length) {
throw new Error('Expected non-empty array of addresses');
}

#syncIdentities(addresses: string[]) {
addresses = addresses.map((address: string) =>
toChecksumHexAddress(address),
);
Expand All @@ -355,38 +349,6 @@ export class PreferencesController extends BaseController<
state.selectedAddress = addresses[0];
});
}

return this.state.selectedAddress;
}

/**
* Generates and stores a new list of stored identities based on address. If the selected address
* is unset, or if it refers to an identity that was removed, it will be set to the first
* identity.
*
* @param addresses - List of addresses to use as a basis for each identity.
*/
updateIdentities(addresses: string[]) {
addresses = addresses.map((address: string) =>
toChecksumHexAddress(address),
);
this.update((state) => {
const identities = addresses.reduce(
(ids: { [address: string]: Identity }, address, index) => {
ids[address] = state.identities[address] || {
address,
name: `Account ${index + 1}`,
importTime: Date.now(),
};
return ids;
},
{},
);
state.identities = identities;
if (!Object.keys(identities).includes(state.selectedAddress)) {
state.selectedAddress = Object.keys(identities)[0];
}
});
}

/**
Expand Down

0 comments on commit 969dfd5

Please sign in to comment.