diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json index e7eb5143ae5..feceaddb4ed 100644 --- a/eslint-warning-thresholds.json +++ b/eslint-warning-thresholds.json @@ -437,9 +437,6 @@ "packages/polling-controller/src/AbstractPollingController.ts": { "@typescript-eslint/prefer-readonly": 1 }, - "packages/preferences-controller/src/PreferencesController.test.ts": { - "prettier/prettier": 4 - }, "packages/queued-request-controller/src/QueuedRequestController.ts": { "@typescript-eslint/prefer-readonly": 2 }, diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index 8a5fd21cbca..d0241c9be5e 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -501,12 +501,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -534,7 +532,7 @@ describe('AccountsController', () => { messenger.publish( 'KeyringController:stateChange', - { isUnlocked: true, keyrings: [], keyringsMetadata: [] }, + { isUnlocked: true, keyrings: [] }, [], ); @@ -553,12 +551,10 @@ describe('AccountsController', () => { accounts: [mockAccount.address, mockAccount2.address], type: KeyringTypes.hd, id: '123', - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -595,12 +591,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address, mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -654,20 +648,18 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address], + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, { type: KeyringTypes.snap, accounts: [mockAccount3.address, mockAccount4.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', - }, - { - id: 'mock-id2', - name: 'mock-name2', + metadata: { + id: 'mock-id2', + name: 'mock-name2', + }, }, ], }; @@ -731,20 +723,18 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address], + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, { type: KeyringTypes.snap, accounts: [mockAccount3.address, mockAccount4.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', - }, - { - id: 'mock-id2', - name: 'mock-name2', + metadata: { + id: 'mock-id2', + name: 'mock-name2', + }, }, ], }; @@ -792,6 +782,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address], + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, { type: KeyringTypes.snap, @@ -799,16 +793,10 @@ describe('AccountsController', () => { // to the state (like if the Snap did remove it right before the keyring controller // state change got triggered). accounts: [mockAccount3.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', - }, - { - id: 'mock-id2', - name: 'mock-name2', + metadata: { + id: 'mock-id2', + name: 'mock-name2', + }, }, ], }; @@ -851,12 +839,10 @@ describe('AccountsController', () => { mockAccount2.address, mockAccount3.address, ], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -919,12 +905,10 @@ describe('AccountsController', () => { mockAccount2.address, mockAccount3.address, ], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -982,20 +966,18 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [], + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, { type: KeyringTypes.snap, accounts: [mockAccount3.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', - }, - { - id: 'mock-id2', - name: 'mock-name2', + metadata: { + id: 'mock-id2', + name: 'mock-name2', + }, }, ], }; @@ -1032,12 +1014,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address, mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1093,12 +1073,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address, mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1132,12 +1110,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1179,12 +1155,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address, mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1245,12 +1219,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address, mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1324,12 +1296,10 @@ describe('AccountsController', () => { mockAccountWithoutLastSelected.address, mockAccount2.address, ], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1400,12 +1370,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAccount.address, mockAccount2.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1452,12 +1420,10 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockReinitialisedAccount.address], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1554,12 +1520,10 @@ describe('AccountsController', () => { mockExistingAccount1.address, mockExistingAccount2.address, ], - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, ], }; @@ -1799,12 +1763,13 @@ describe('AccountsController', () => { 'KeyringController:getState', mockGetState.mockReturnValue({ keyrings: [ - { type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] }, - ], - keyringsMetadata: [ { - id: 'mock-keyring-id-0', - name: 'mock-keyring-id-name', + type: KeyringTypes.hd, + accounts: [mockAddress1, mockAddress2], + metadata: { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, }, ], }), @@ -1863,12 +1828,10 @@ describe('AccountsController', () => { { type: KeyringTypes.snap, accounts: [mockSnapAccount, mockSnapAccount2], - }, - ], - keyringsMetadata: [ - { - id: 'mock-keyring-id-1', - name: 'mock-keyring-id-name', + metadata: { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name', + }, }, ], }), @@ -1962,12 +1925,13 @@ describe('AccountsController', () => { 'KeyringController:getState', mockGetState.mockReturnValue({ keyrings: [ - { type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] }, - ], - keyringsMetadata: [ { - id: 'mock-keyring-id-0', - name: 'mock-keyring-id-name', + type: KeyringTypes.hd, + accounts: [mockAddress1, mockAddress2], + metadata: { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, }, ], }), @@ -2035,17 +1999,21 @@ describe('AccountsController', () => { 'KeyringController:getState', mockGetState.mockReturnValue({ keyrings: [ - { type: KeyringTypes.hd, accounts: [mockAddress1] }, - { type: KeyringTypes.snap, accounts: ['0x1234'] }, - ], - keyringsMetadata: [ { - id: 'mock-keyring-id-0', - name: 'mock-keyring-id-name', + type: KeyringTypes.hd, + accounts: [mockAddress1], + metadata: { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, }, { - id: 'mock-keyring-id-1', - name: 'mock-keyring-id-name2', + type: KeyringTypes.snap, + accounts: ['0x1234'], + metadata: { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name2', + }, }, ], }), @@ -2103,17 +2071,21 @@ describe('AccountsController', () => { 'KeyringController:getState', mockGetState.mockReturnValue({ keyrings: [ - { type: KeyringTypes.snap, accounts: ['0x1234'] }, - { type: KeyringTypes.hd, accounts: [mockAddress1] }, - ], - keyringsMetadata: [ { - id: 'mock-keyring-id-0', - name: 'mock-keyring-id-name', + type: KeyringTypes.snap, + accounts: ['0x1234'], + metadata: { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, }, { - id: 'mock-keyring-id-1', - name: 'mock-keyring-id-name2', + type: KeyringTypes.hd, + accounts: [mockAddress1], + metadata: { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name2', + }, }, ], }), @@ -2168,11 +2140,14 @@ describe('AccountsController', () => { messenger.registerActionHandler( 'KeyringController:getState', mockGetState.mockReturnValue({ - keyrings: [{ type: keyringType, accounts: [mockAddress1] }], - keyringsMetadata: [ + keyrings: [ { - id: 'mock-keyring-id-0', - name: 'mock-keyring-id-name', + type: keyringType, + accounts: [mockAddress1], + metadata: { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, }, ], }), @@ -2312,17 +2287,21 @@ describe('AccountsController', () => { 'KeyringController:getState', mockGetState.mockReturnValue({ keyrings: [ - { type: KeyringTypes.snap, accounts: ['0x1234'] }, - { type: KeyringTypes.hd, accounts: [mockAddress1] }, - ], - keyringsMetadata: [ { - id: 'mock-keyring-id-1', - name: 'mock-keyring-id-name', + type: KeyringTypes.snap, + accounts: ['0x1234'], + metadata: { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name', + }, }, { - id: 'mock-keyring-id-2', - name: 'mock-keyring-id-name2', + type: KeyringTypes.hd, + accounts: [mockAddress1], + metadata: { + id: 'mock-keyring-id-2', + name: 'mock-keyring-id-name2', + }, }, ], }), @@ -3076,20 +3055,18 @@ describe('AccountsController', () => { { type: 'HD Key Tree', accounts: [mockAccount.address], + metadata: { + id: 'mock-id', + name: 'mock-name', + }, }, { type: 'Simple Key Pair', accounts: simpleAddressess, - }, - ], - keyringsMetadata: [ - { - id: 'mock-id', - name: 'mock-name', - }, - { - id: 'mock-id2', - name: 'mock-name2', + metadata: { + id: 'mock-id2', + name: 'mock-name2', + }, }, ], }; diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 1abb03a64c9..7bf59e80e65 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -684,11 +684,11 @@ export class AccountsController extends BaseController< */ async #listNormalAccounts(): Promise { const internalAccounts: InternalAccount[] = []; - const { keyrings, keyringsMetadata } = this.messagingSystem.call( + const { keyrings } = this.messagingSystem.call( 'KeyringController:getState', ); - for (const [keyringIndex, keyring] of keyrings.entries()) { + for (const keyring of keyrings) { const keyringType = keyring.type; if (!isNormalKeyringType(keyringType as KeyringTypes)) { // We only consider "normal accounts" here, so keep looping @@ -702,7 +702,7 @@ export class AccountsController extends BaseController< if (isHdKeyringType(keyring.type as KeyringTypes)) { options = { - entropySource: keyringsMetadata[keyringIndex].id, + entropySource: keyring.metadata.id, // NOTE: We are not using the `hdPath` from the associated keyring here and // getting the keyring instance here feels a bit overkill. // This will be naturally fixed once every keyring start using `KeyringAccount` and implement the keyring API. diff --git a/packages/keyring-controller/CHANGELOG.md b/packages/keyring-controller/CHANGELOG.md index d6def954e41..fdd56be0a67 100644 --- a/packages/keyring-controller/CHANGELOG.md +++ b/packages/keyring-controller/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING** `keyringsMetadata` has been removed from the controller state ([#5725](https://github.com/MetaMask/core/pull/5725)) + - The metadata is now stored in each keyring object in the `state.keyrings` array. + - When updating to this version, we recommend removing the `keyringsMetadata` state and all state referencing a keyring ID with a migration. New metadata will be generated for each keyring automatically after the update. + ## [21.0.6] ### Changed diff --git a/packages/keyring-controller/jest.config.js b/packages/keyring-controller/jest.config.js index d8355e87e91..0d930e38202 100644 --- a/packages/keyring-controller/jest.config.js +++ b/packages/keyring-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 93.64, + branches: 94.25, functions: 100, - lines: 98.92, - statements: 98.93, + lines: 98.79, + statements: 98.8, }, }, diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 26f09defe41..97be3333362 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -127,6 +127,95 @@ describe('KeyringController', () => { }, ); }); + + it('allows removing a keyring builder without bricking the wallet when metadata was already generated', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: 'my vault', + }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: '', + metadata: { id: 'hd', name: '' }, + }, + { + type: 'Unsupported', + data: '', + metadata: { id: 'unsupported', name: '' }, + }, + { + type: KeyringTypes.hd, + data: '', + metadata: { id: 'hd2', name: '' }, + }, + ]); + + await controller.submitPassword(password); + + expect(controller.state.keyrings).toHaveLength(2); + expect(controller.state.keyrings[0].type).toBe(KeyringTypes.hd); + expect(controller.state.keyrings[0].metadata).toStrictEqual({ + id: 'hd', + name: '', + }); + expect(controller.state.keyrings[1].type).toBe(KeyringTypes.hd); + expect(controller.state.keyrings[1].metadata).toStrictEqual({ + id: 'hd2', + name: '', + }); + }, + ); + }); + + it('allows removing a keyring builder without bricking the wallet when metadata was not yet generated', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: 'my vault', + }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: 'HD Key Tree', + data: '', + metadata: { id: 'hd', name: '' }, + }, + { + type: 'HD Key Tree', + data: '', + metadata: { id: 'hd2', name: '' }, + }, + // This keyring was already unsupported + // (no metadata, and is at the end of the array) + { + type: MockKeyring.type, + data: 'unsupported', + }, + ]); + + await controller.submitPassword(password); + + expect(controller.state.keyrings).toHaveLength(2); + expect(controller.state.keyrings[0].type).toBe(KeyringTypes.hd); + expect(controller.state.keyrings[0].metadata).toStrictEqual({ + id: 'hd', + name: '', + }); + expect(controller.state.keyrings[1].type).toBe(KeyringTypes.hd); + expect(controller.state.keyrings[1].metadata).toStrictEqual({ + id: 'hd2', + name: '', + }); + }, + ); + }); }); describe('addNewAccount', () => { @@ -467,7 +556,7 @@ describe('KeyringController', () => { { cacheEncryptionKey }, async ({ controller, initialState }) => { const initialVault = controller.state.vault; - const initialKeyringsMetadata = controller.state.keyringsMetadata; + const initialKeyrings = controller.state.keyrings; await controller.createNewVaultAndRestore( password, uint8ArraySeed, @@ -475,12 +564,12 @@ describe('KeyringController', () => { expect(controller.state).not.toBe(initialState); expect(controller.state.vault).toBeDefined(); expect(controller.state.vault).toStrictEqual(initialVault); - expect(controller.state.keyringsMetadata).toHaveLength( - initialKeyringsMetadata.length, + expect(controller.state.keyrings).toHaveLength( + initialKeyrings.length, ); // new keyring metadata should be generated - expect(controller.state.keyringsMetadata).not.toStrictEqual( - initialKeyringsMetadata, + expect(controller.state.keyrings).not.toStrictEqual( + initialKeyrings, ); }, ); @@ -507,6 +596,10 @@ describe('KeyringController', () => { { data: serializedKeyring, type: 'HD Key Tree', + metadata: { + id: expect.any(String), + name: '', + }, }, ]); }, @@ -769,7 +862,7 @@ describe('KeyringController', () => { it('should export seed phrase with valid keyringId', async () => { await withController(async ({ controller, initialState }) => { - const keyringId = initialState.keyringsMetadata[0].id; + const keyringId = initialState.keyrings[0].metadata.id; const seed = await controller.exportSeedPhrase(password, keyringId); expect(seed).not.toBe(''); }); @@ -799,7 +892,7 @@ describe('KeyringController', () => { it('should throw invalid password error with valid keyringId', async () => { await withController( async ({ controller, encryptor, initialState }) => { - const keyringId = initialState.keyringsMetadata[0].id; + const keyringId = initialState.keyrings[0].metadata.id; jest .spyOn(encryptor, 'decrypt') .mockRejectedValueOnce(new Error('Invalid password')); @@ -1203,10 +1296,12 @@ describe('KeyringController', () => { ); const modifiedState = { ...initialState, - keyrings: [initialState.keyrings[0], newKeyring], - keyringsMetadata: [ - initialState.keyringsMetadata[0], - controller.state.keyringsMetadata[1], + keyrings: [ + initialState.keyrings[0], + { + ...newKeyring, + metadata: controller.state.keyrings[1].metadata, + }, ], }; expect(controller.state).toStrictEqual(modifiedState); @@ -1280,10 +1375,12 @@ describe('KeyringController', () => { }; const modifiedState = { ...initialState, - keyrings: [initialState.keyrings[0], newKeyring], - keyringsMetadata: [ - initialState.keyringsMetadata[0], - controller.state.keyringsMetadata[1], + keyrings: [ + initialState.keyrings[0], + { + ...newKeyring, + metadata: controller.state.keyrings[1].metadata, + }, ], }; expect(controller.state).toStrictEqual(modifiedState); @@ -1480,12 +1577,10 @@ describe('KeyringController', () => { await withController(async ({ controller }) => { await controller.addNewKeyring(KeyringTypes.hd); expect(controller.state.keyrings).toHaveLength(2); - expect(controller.state.keyringsMetadata).toHaveLength(2); await controller.removeAccount( controller.state.keyrings[1].accounts[0], ); expect(controller.state.keyrings).toHaveLength(1); - expect(controller.state.keyringsMetadata).toHaveLength(1); }); }); }); @@ -2635,10 +2730,12 @@ describe('KeyringController', () => { }); it('should unlock succesfully when the controller is instantiated with an existing `keyringsMetadata`', async () => { + // @ts-expect-error HdKeyring is not yet compatible with Keyring type. + stubKeyringClassWithAccount(HdKeyring, '0x123'); await withController( { cacheEncryptionKey, - state: { keyringsMetadata: [], vault: 'my vault' }, + state: { vault: 'my vault' }, skipVaultCreation: true, }, async ({ controller, encryptor }) => { @@ -2648,46 +2745,150 @@ describe('KeyringController', () => { data: { accounts: ['0x123'], }, + metadata: { + id: '123', + name: '', + }, }, ]); await controller.submitPassword(password); - expect(controller.state.keyringsMetadata).toHaveLength(1); - }, - ); - }); - - it('should throw an error when the controller is instantiated with an existing `keyringsMetadata` with too many objects', async () => { - await withController( - { - cacheEncryptionKey, - state: { - keyringsMetadata: [ - { id: '123', name: '' }, - { id: '456', name: '' }, - ], - vault: 'my vault', - }, - skipVaultCreation: true, - }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + expect(controller.state.keyrings).toStrictEqual([ { type: KeyringTypes.hd, - data: { - accounts: ['0x123'], + accounts: ['0x123'], + metadata: { + id: '123', + name: '', }, }, ]); - - await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.KeyringMetadataLengthMismatch, - ); }, ); }); + cacheEncryptionKey && + it('should generate new metadata when there is no metadata in the vault and cacheEncryptionKey is enabled', async () => { + const hdKeyringSerializeSpy = jest.spyOn( + HdKeyring.prototype, + 'serialize', + ); + await withController( + { + cacheEncryptionKey: true, + state: { + vault: 'my vault', + }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + const encryptWithKeySpy = jest.spyOn( + encryptor, + 'encryptWithKey', + ); + jest + .spyOn(encryptor, 'importKey') + // @ts-expect-error we are assigning a mock value + .mockResolvedValue('imported key'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + }, + ]); + hdKeyringSerializeSpy.mockResolvedValue({ + // @ts-expect-error we are assigning a mock value + accounts: ['0x123'], + }); + + await controller.submitPassword(password); + + expect(controller.state.keyrings).toStrictEqual([ + { + type: KeyringTypes.hd, + accounts: expect.any(Array), + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + }, + ); + }); + + !cacheEncryptionKey && + it('should generate new metadata when there is no metadata in the vault and cacheEncryptionKey is disabled', async () => { + const hdKeyringSerializeSpy = jest.spyOn( + HdKeyring.prototype, + 'serialize', + ); + await withController( + { + cacheEncryptionKey: false, + state: { + vault: 'my vault', + }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + }, + ]); + hdKeyringSerializeSpy.mockResolvedValue({ + // @ts-expect-error we are assigning a mock value + accounts: ['0x123'], + }); + + await controller.submitPassword(password); + + expect(controller.state.keyrings).toStrictEqual([ + { + type: KeyringTypes.hd, + accounts: expect.any(Array), + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + expect(encryptSpy).toHaveBeenCalledWith(password, [ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + }, + ); + }); + it('should unlock the wallet if the state has a duplicate account and the encryption parameters are outdated', async () => { stubKeyringClassWithAccount(MockKeyring, '0x123'); // @ts-expect-error HdKeyring is not yet compatible with Keyring type. @@ -2750,6 +2951,33 @@ describe('KeyringController', () => { ); }); + cacheEncryptionKey && + it('should not upgrade the vault encryption if the key encryptor has the same parameters', async () => { + await withController( + { + skipVaultCreation: true, + cacheEncryptionKey, + state: { vault: 'my vault' }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + }, + ]); + + await controller.submitPassword(password); + + expect(encryptSpy).not.toHaveBeenCalled(); + }, + ); + }); + !cacheEncryptionKey && it('should upgrade the vault encryption if the generic encryptor has different parameters', async () => { await withController( @@ -2777,6 +3005,36 @@ describe('KeyringController', () => { ); }); + it('should not upgrade the vault encryption if the encryptor has the same parameters and the keyring has metadata', async () => { + await withController( + { + skipVaultCreation: true, + cacheEncryptionKey, + state: { vault: 'my vault' }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: '123', + name: '', + }, + }, + ]); + + await controller.submitPassword(password); + + expect(encryptSpy).not.toHaveBeenCalled(); + }, + ); + }); + !cacheEncryptionKey && it('should throw error if password is of wrong type', async () => { await withController( @@ -2864,6 +3122,57 @@ describe('KeyringController', () => { ); }); + it('should update the vault if new metadata is created while unlocking', async () => { + jest.spyOn(HdKeyring.prototype, 'serialize').mockResolvedValue({ + // @ts-expect-error we are assigning a mock value + accounts: ['0x123'], + }); + await withController( + { + cacheEncryptionKey: true, + skipVaultCreation: true, + state: { + vault: JSON.stringify({ data: '0x123', salt: 'my salt' }), + // @ts-expect-error we want to force the controller to have an + // encryption salt equal to the one in the vault + encryptionSalt: 'my salt', + }, + }, + async ({ controller, initialState, encryptor }) => { + const encryptWithKeySpy = jest.spyOn(encryptor, 'encryptWithKey'); + jest + .spyOn(encryptor, 'importKey') + // @ts-expect-error we are assigning a mock value + .mockResolvedValue('imported key'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: '0x123', + }, + ]); + + await controller.submitEncryptionKey( + MOCK_ENCRYPTION_KEY, + initialState.encryptionSalt as string, + ); + + expect(controller.state.isUnlocked).toBe(true); + expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + }, + ); + }); + it('should throw error if vault unlocked has an unexpected shape', async () => { await withController( { cacheEncryptionKey: true }, @@ -2930,7 +3239,7 @@ describe('KeyringController', () => { it('should return seedphrase for a specific keyring', async () => { await withController(async ({ controller }) => { const seedPhrase = await controller.verifySeedPhrase( - controller.state.keyringsMetadata[0].id, + controller.state.keyrings[0].metadata.id, ); expect(seedPhrase).toBeDefined(); }); @@ -2965,7 +3274,7 @@ describe('KeyringController', () => { await withController(async ({ controller }) => { await controller.addNewKeyring(KeyringTypes.simple, [privateKey]); - const keyringId = controller.state.keyringsMetadata[1].id; + const keyringId = controller.state.keyrings[1].metadata.id; await expect(controller.verifySeedPhrase(keyringId)).rejects.toThrow( KeyringControllerError.UnsupportedVerifySeedPhrase, ); @@ -3073,7 +3382,7 @@ describe('KeyringController', () => { const fn = jest.fn(); const selector = { type: KeyringTypes.hd }; const keyring = controller.getKeyringsByType(KeyringTypes.hd)[0]; - const metadata = controller.state.keyringsMetadata[0]; + const { metadata } = controller.state.keyrings[0]; await controller.withKeyring(selector, fn); @@ -3211,7 +3520,7 @@ describe('KeyringController', () => { address: initialState.keyrings[0].accounts[0] as Hex, }; const keyring = controller.getKeyringsByType(KeyringTypes.hd)[0]; - const metadata = controller.state.keyringsMetadata[0]; + const { metadata } = controller.state.keyrings[0]; await controller.withKeyring(selector, fn); @@ -3253,11 +3562,11 @@ describe('KeyringController', () => { describe('when the keyring is selected by id', () => { it('should call the given function with the selected keyring', async () => { - await withController(async ({ controller, initialState }) => { + await withController(async ({ controller }) => { const fn = jest.fn(); const keyring = controller.getKeyringsByType(KeyringTypes.hd)[0]; - const selector = { id: initialState.keyringsMetadata[0].id }; - const metadata = controller.state.keyringsMetadata[0]; + const { metadata } = controller.state.keyrings[0]; + const selector = { id: metadata.id }; await controller.withKeyring(selector, fn); @@ -3268,7 +3577,7 @@ describe('KeyringController', () => { it('should return the result of the function', async () => { await withController(async ({ controller, initialState }) => { const fn = async () => Promise.resolve('hello'); - const selector = { id: initialState.keyringsMetadata[0].id }; + const selector = { id: initialState.keyrings[0].metadata.id }; expect(await controller.withKeyring(selector, fn)).toBe('hello'); }); @@ -3276,7 +3585,7 @@ describe('KeyringController', () => { it('should throw an error if the callback returns the selected keyring', async () => { await withController(async ({ controller, initialState }) => { - const selector = { id: initialState.keyringsMetadata[0].id }; + const selector = { id: initialState.keyrings[0].metadata.id }; await expect( controller.withKeyring(selector, async ({ keyring }) => { diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 97f95b90b09..b4147ae26b2 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -22,7 +22,6 @@ import type { KeyringClass } from '@metamask/keyring-utils'; import type { Eip1024EncryptedData, Hex, Json } from '@metamask/utils'; import { add0x, - assert, assertIsStrictHexString, bytesToHex, hasProperty, @@ -92,10 +91,6 @@ export type KeyringControllerState = { * Representations of managed keyrings. */ keyrings: KeyringObject[]; - /** - * Metadata for each keyring. - */ - keyringsMetadata: KeyringMetadata[]; /** * The encryption key derived from the password and used to encrypt * the vault. This is only stored if the `cacheEncryptionKey` option @@ -278,6 +273,10 @@ export type KeyringObject = { * Keyring type. */ type: string; + /** + * Additional data associated with the keyring. + */ + metadata: KeyringMetadata; }; /** @@ -319,6 +318,7 @@ export enum SignTypedDataVersion { export type SerializedKeyring = { type: string; data: Json; + metadata?: KeyringMetadata; }; /** @@ -326,7 +326,6 @@ export type SerializedKeyring = { */ type SessionState = { keyrings: SerializedKeyring[]; - keyringsMetadata: KeyringMetadata[]; password?: string; }; @@ -482,7 +481,6 @@ export const getDefaultKeyringState = (): KeyringControllerState => { return { isUnlocked: false, keyrings: [], - keyringsMetadata: [], }; }; @@ -566,12 +564,18 @@ function isSerializedKeyringsArray( * * Is used for adding the current keyrings to the state object. * - * @param keyring - The keyring to display. + * @param keyringWithMetadata - The keyring and its metadata. + * @param keyringWithMetadata.keyring - The keyring to display. + * @param keyringWithMetadata.metadata - The metadata of the keyring. * @returns A keyring display object, with type and accounts properties. */ -async function displayForKeyring( - keyring: EthKeyring, -): Promise<{ type: string; accounts: string[] }> { +async function displayForKeyring({ + keyring, + metadata, +}: { + keyring: EthKeyring; + metadata: KeyringMetadata; +}): Promise { const accounts = await keyring.getAccounts(); return { @@ -579,6 +583,7 @@ async function displayForKeyring( // Cast to `string[]` here is safe here because `accounts` has no nullish // values, and `normalize` returns `string` unless given a nullish value accounts: accounts.map(normalize) as string[], + metadata, }; } @@ -638,12 +643,10 @@ export class KeyringController extends BaseController< readonly #cacheEncryptionKey: boolean; - #keyrings: EthKeyring[]; + #keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; #unsupportedKeyrings: SerializedKeyring[]; - #keyringsMetadata: KeyringMetadata[]; - #password?: string; #qrKeyringStateListener?: ( @@ -674,7 +677,6 @@ export class KeyringController extends BaseController< vault: { persist: true, anonymous: false }, isUnlocked: { persist: false, anonymous: true }, keyrings: { persist: false, anonymous: false }, - keyringsMetadata: { persist: true, anonymous: false }, encryptionKey: { persist: false, anonymous: false }, encryptionSalt: { persist: false, anonymous: false }, }, @@ -691,7 +693,6 @@ export class KeyringController extends BaseController< this.#encryptor = encryptor; this.#keyrings = []; - this.#keyringsMetadata = state?.keyringsMetadata?.slice() ?? []; this.#unsupportedKeyrings = []; // This option allows the controller to cache an exported key @@ -989,7 +990,7 @@ export class KeyringController extends BaseController< const address = normalize(account); const candidates = await Promise.all( - this.#keyrings.map(async (keyring) => { + this.#keyrings.map(async ({ keyring }) => { return Promise.all([keyring, keyring.getAccounts()]); }), ); @@ -1026,7 +1027,9 @@ export class KeyringController extends BaseController< */ getKeyringsByType(type: KeyringTypes | string): unknown[] { this.#assertIsUnlocked(); - return this.#keyrings.filter((keyring) => keyring.type === type); + return this.#keyrings + .filter(({ keyring }) => keyring.type === type) + .map(({ keyring }) => keyring); } /** @@ -1442,14 +1445,29 @@ export class KeyringController extends BaseController< encryptionKey: string, encryptionSalt: string, ): Promise { - return this.#withRollback(async () => { - this.#keyrings = await this.#unlockKeyrings( + const { newMetadata } = await this.#withRollback(async () => { + const result = await this.#unlockKeyrings( undefined, encryptionKey, encryptionSalt, ); this.#setUnlocked(); + return result; }); + + try { + // if new metadata has been generated during login, we + // can attempt to upgrade the vault. + await this.#withRollback(async () => { + if (newMetadata) { + await this.#updateVault(); + } + }); + } catch (error) { + // We don't want to throw an error if the upgrade fails + // since the controller is already unlocked. + console.error('Failed to update vault during login:', error); + } } /** @@ -1460,21 +1478,25 @@ export class KeyringController extends BaseController< * @returns Promise resolving when the operation completes. */ async submitPassword(password: string): Promise { - await this.#withRollback(async () => { - this.#keyrings = await this.#unlockKeyrings(password); + const { newMetadata } = await this.#withRollback(async () => { + const result = await this.#unlockKeyrings(password); this.#setUnlocked(); + return result; }); try { - // If there are stronger encryption params available, we + // If there are stronger encryption params available, or + // if new metadata has been generated during login, we // can attempt to upgrade the vault. - await this.#withRollback(async () => - this.#upgradeVaultEncryptionParams(), - ); + await this.#withRollback(async () => { + if (newMetadata || this.#isNewEncryptionAvailable()) { + await this.#updateVault(); + } + }); } catch (error) { // We don't want to throw an error if the upgrade fails // since the controller is already unlocked. - console.error('Failed to upgrade vault encryption params:', error); + console.error('Failed to update vault during login:', error); } } @@ -1949,10 +1971,8 @@ export class KeyringController extends BaseController< * @returns The keyring. */ #getKeyringById(keyringId: string): EthKeyring | undefined { - const index = this.state.keyringsMetadata.findIndex( - (metadata) => metadata.id === keyringId, - ); - return this.#keyrings[index]; + return this.#keyrings.find(({ metadata }) => metadata.id === keyringId) + ?.keyring; } /** @@ -1963,7 +1983,7 @@ export class KeyringController extends BaseController< */ #getKeyringByIdOrDefault(keyringId?: string): EthKeyring | undefined { if (!keyringId) { - return this.#keyrings[0] as EthKeyring; + return this.#keyrings[0]?.keyring; } return this.#getKeyringById(keyringId); @@ -1976,11 +1996,13 @@ export class KeyringController extends BaseController< * @returns The keyring metadata. */ #getKeyringMetadata(keyring: unknown): KeyringMetadata { - const index = this.#keyrings.findIndex( - (keyringCandidate) => keyringCandidate === keyring, + const keyringWithMetadata = this.#keyrings.find( + (candidate) => candidate.keyring === keyring, ); - assert(index !== -1, KeyringControllerError.KeyringNotFound); - return this.#keyringsMetadata[index]; + if (!keyringWithMetadata) { + throw new Error(KeyringControllerError.KeyringNotFound); + } + return keyringWithMetadata.metadata; } /** @@ -2072,7 +2094,6 @@ export class KeyringController extends BaseController< this.#password = password; await this.#clearKeyrings(); - this.#keyringsMetadata = []; await this.#createKeyringWithFirstAccount(keyring.type, keyring.opts); this.#setUnlocked(); } @@ -2156,13 +2177,13 @@ export class KeyringController extends BaseController< includeUnsupported: true, }, ): Promise { - const serializedKeyrings = await Promise.all( - this.#keyrings.map(async (keyring) => { - const [type, data] = await Promise.all([ - keyring.type, - keyring.serialize(), - ]); - return { type, data }; + const serializedKeyrings: SerializedKeyring[] = await Promise.all( + this.#keyrings.map(async ({ keyring, metadata }) => { + return { + type: keyring.type, + data: await keyring.serialize(), + metadata, + }; }), ); @@ -2182,7 +2203,6 @@ export class KeyringController extends BaseController< async #getSessionState(): Promise { return { keyrings: await this.#getSerializedKeyrings(), - keyringsMetadata: this.#keyringsMetadata.slice(), // Force copy. password: this.#password, }; } @@ -2191,15 +2211,30 @@ export class KeyringController extends BaseController< * Restore a serialized keyrings array. * * @param serializedKeyrings - The serialized keyrings array. + * @returns The restored keyrings. */ async #restoreSerializedKeyrings( serializedKeyrings: SerializedKeyring[], - ): Promise { + ): Promise<{ + keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; + newMetadata: boolean; + }> { await this.#clearKeyrings(); + const keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[] = []; + let newMetadata = false; for (const serializedKeyring of serializedKeyrings) { - await this.#restoreKeyring(serializedKeyring); + const result = await this.#restoreKeyring(serializedKeyring); + if (result) { + const { keyring, metadata } = result; + keyrings.push({ keyring, metadata }); + if (result.newMetadata) { + newMetadata = true; + } + } } + + return { keyrings, newMetadata }; } /** @@ -2215,7 +2250,10 @@ export class KeyringController extends BaseController< password: string | undefined, encryptionKey?: string, encryptionSalt?: string, - ): Promise { + ): Promise<{ + keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; + newMetadata: boolean; + }> { return this.#withVaultLock(async () => { const encryptedVault = this.state.vault; if (!encryptedVault) { @@ -2276,13 +2314,8 @@ export class KeyringController extends BaseController< throw new Error(KeyringControllerError.VaultDataError); } - await this.#restoreSerializedKeyrings(vault); - - // The keyrings array and the keyringsMetadata array should - // always have the same length while the controller is unlocked. - if (this.#keyrings.length !== this.#keyringsMetadata.length) { - throw new Error(KeyringControllerError.KeyringMetadataLengthMismatch); - } + const { keyrings, newMetadata } = + await this.#restoreSerializedKeyrings(vault); const updatedKeyrings = await this.#getUpdatedKeyrings(); @@ -2294,7 +2327,7 @@ export class KeyringController extends BaseController< } }); - return this.#keyrings; + return { keyrings, newMetadata }; }); } @@ -2366,14 +2399,10 @@ export class KeyringController extends BaseController< } const updatedKeyrings = await this.#getUpdatedKeyrings(); - if (updatedKeyrings.length !== this.#keyringsMetadata.length) { - throw new Error(KeyringControllerError.KeyringMetadataLengthMismatch); - } this.update((state) => { state.vault = updatedState.vault; state.keyrings = updatedKeyrings; - state.keyringsMetadata = this.#keyringsMetadata.slice(); if (updatedState.encryptionKey) { state.encryptionKey = updatedState.encryptionKey; state.encryptionSalt = JSON.parse(updatedState.vault as string).salt; @@ -2385,22 +2414,18 @@ export class KeyringController extends BaseController< } /** - * Upgrade the vault encryption parameters if needed. + * Check if there are new encryption parameters available. * * @returns A promise resolving to `void`. */ - async #upgradeVaultEncryptionParams(): Promise { - this.#assertControllerMutexIsLocked(); + #isNewEncryptionAvailable(): boolean { const { vault } = this.state; - if ( - vault && - this.#password && - this.#encryptor.isVaultUpdated && - !this.#encryptor.isVaultUpdated(vault) - ) { - await this.#updateVault(); + if (!vault || !this.#password || !this.#encryptor.isVaultUpdated) { + return false; } + + return !this.#encryptor.isVaultUpdated(vault); } /** @@ -2413,7 +2438,7 @@ export class KeyringController extends BaseController< const keyrings = this.#keyrings; const keyringArrays = await Promise.all( - keyrings.map(async (keyring) => keyring.getAccounts()), + keyrings.map(async ({ keyring }) => keyring.getAccounts()), ); const addresses = keyringArrays.reduce((res, arr) => { return res.concat(arr); @@ -2460,11 +2485,7 @@ export class KeyringController extends BaseController< async #newKeyring(type: string, data?: unknown): Promise { const keyring = await this.#createKeyring(type, data); - if (this.#keyrings.length !== this.#keyringsMetadata.length) { - throw new Error('Keyring metadata missing'); - } - this.#keyrings.push(keyring); - this.#keyringsMetadata.push(getDefaultKeyringMetadata()); + this.#keyrings.push({ keyring, metadata: getDefaultKeyringMetadata() }); return keyring; } @@ -2536,7 +2557,7 @@ export class KeyringController extends BaseController< */ async #clearKeyrings() { this.#assertControllerMutexIsLocked(); - for (const keyring of this.#keyrings) { + for (const { keyring } of this.#keyrings) { await this.#destroyKeyring(keyring); } this.#keyrings = []; @@ -2552,22 +2573,30 @@ export class KeyringController extends BaseController< */ async #restoreKeyring( serialized: SerializedKeyring, - ): Promise { + ): Promise< + | { keyring: EthKeyring; metadata: KeyringMetadata; newMetadata: boolean } + | undefined + > { this.#assertControllerMutexIsLocked(); try { - const { type, data } = serialized; + const { type, data, metadata: serializedMetadata } = serialized; + let newMetadata = false; + let metadata = serializedMetadata; const keyring = await this.#createKeyring(type, data); // If metadata is missing, assume the data is from an installation before // we had keyring metadata. - if (this.#keyringsMetadata.length <= this.#keyrings.length) { - console.log(`Adding missing metadata for '${type}' keyring`); - this.#keyringsMetadata.push(getDefaultKeyringMetadata()); + if (!metadata) { + newMetadata = true; + metadata = getDefaultKeyringMetadata(); } // The keyring is added to the keyrings array only if it's successfully restored // and the metadata is successfully added to the controller - this.#keyrings.push(keyring); - return keyring; + this.#keyrings.push({ + keyring, + metadata, + }); + return { keyring, metadata, newMetadata }; } catch (error) { console.error(error); this.#unsupportedKeyrings.push(serialized); @@ -2596,26 +2625,24 @@ export class KeyringController extends BaseController< */ async #removeEmptyKeyrings(): Promise { this.#assertControllerMutexIsLocked(); - const validKeyrings: EthKeyring[] = []; - const validKeyringMetadata: KeyringMetadata[] = []; + const validKeyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[] = + []; // Since getAccounts returns a Promise // We need to wait to hear back form each keyring // in order to decide which ones are now valid (accounts.length > 0) await Promise.all( - this.#keyrings.map(async (keyring: EthKeyring, index: number) => { + this.#keyrings.map(async ({ keyring, metadata }) => { const accounts = await keyring.getAccounts(); if (accounts.length > 0) { - validKeyrings.push(keyring); - validKeyringMetadata.push(this.#keyringsMetadata[index]); + validKeyrings.push({ keyring, metadata }); } else { await this.#destroyKeyring(keyring); } }), ); this.#keyrings = validKeyrings; - this.#keyringsMetadata = validKeyringMetadata; } /** @@ -2642,11 +2669,6 @@ export class KeyringController extends BaseController< this.update((state) => { state.isUnlocked = true; - // If new keyringsMetadata was generated during the unlock operation, - // we'll have to update the state with the new array - if (this.#keyringsMetadata.length > state.keyringsMetadata.length) { - state.keyringsMetadata = this.#keyringsMetadata.slice(); - } }); this.messagingSystem.publish(`${name}:unlock`); } @@ -2700,13 +2722,11 @@ export class KeyringController extends BaseController< return this.#withControllerLock(async ({ releaseLock }) => { const currentSerializedKeyrings = await this.#getSerializedKeyrings(); const currentPassword = this.#password; - const currentKeyringsMetadata = this.#keyringsMetadata.slice(); try { return await callback({ releaseLock }); } catch (e) { // Keyrings and password are restored to their previous state - this.#keyringsMetadata = currentKeyringsMetadata; this.#password = currentPassword; await this.#restoreSerializedKeyrings(currentSerializedKeyrings); diff --git a/packages/keyring-controller/src/constants.ts b/packages/keyring-controller/src/constants.ts index abf89373050..d914a3d6f74 100644 --- a/packages/keyring-controller/src/constants.ts +++ b/packages/keyring-controller/src/constants.ts @@ -35,6 +35,5 @@ export enum KeyringControllerError { DataType = 'KeyringController - Incorrect data type provided', NoHdKeyring = 'KeyringController - No HD Keyring found', ControllerLockRequired = 'KeyringController - attempt to update vault during a non mutually exclusive operation', - KeyringMetadataLengthMismatch = 'KeyringController - keyring metadata length mismatch', LastAccountInPrimaryKeyring = 'KeyringController - Last account in primary keyring cannot be removed', } diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index 7445390462b..66e037c08a1 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -141,8 +141,16 @@ describe('metamask-notifications - init()', () => { const act = async (addresses: string[], assertion: () => void) => { mockKeyringControllerGetState.mockReturnValue({ isUnlocked: true, - keyrings: [{ accounts: addresses, type: KeyringTypes.hd }], - keyringsMetadata: [], + keyrings: [ + { + accounts: addresses, + type: KeyringTypes.hd, + metadata: { + id: '123', + name: '', + }, + }, + ], }); await actPublishKeyringStateChange(globalMessenger, addresses); @@ -270,7 +278,6 @@ describe('metamask-notifications - init()', () => { mocks.mockKeyringControllerGetState.mockReturnValue({ isUnlocked: false, // Wallet Locked keyrings: [], - keyringsMetadata: [], }); }); @@ -357,7 +364,6 @@ describe('metamask-notifications - init()', () => { mocks.mockKeyringControllerGetState.mockReturnValue({ isUnlocked: false, keyrings: [], - keyringsMetadata: [], }); }); expect(mockKeyringControllerGetState).toHaveBeenCalledTimes(1); @@ -952,8 +958,16 @@ describe('metamask-notifications - enableMetamaskNotifications()', () => { messengerMocks.mockKeyringControllerGetState.mockReturnValue({ isUnlocked: true, - keyrings: [{ accounts: [ADDRESS_1], type: KeyringTypes.hd }], - keyringsMetadata: [], + keyrings: [ + { + accounts: [ADDRESS_1], + type: KeyringTypes.hd, + metadata: { + id: '123', + name: '', + }, + }, + ], }); return { ...messengerMocks, mockCreateOnChainTriggers }; diff --git a/packages/preferences-controller/src/PreferencesController.test.ts b/packages/preferences-controller/src/PreferencesController.test.ts index 30ff3ea64e8..002d5adefbe 100644 --- a/packages/preferences-controller/src/PreferencesController.test.ts +++ b/packages/preferences-controller/src/PreferencesController.test.ts @@ -31,10 +31,13 @@ describe('PreferencesController', () => { useMultiRpcMigration: true, showIncomingTransactions: Object.values( ETHERSCAN_SUPPORTED_CHAIN_IDS, - ).reduce((acc, curr) => { - acc[curr] = true; - return acc; - }, {} as { [chainId in EtherscanSupportedHexChainId]: boolean }), + ).reduce( + (acc, curr) => { + acc[curr] = true; + return acc; + }, + {} as { [chainId in EtherscanSupportedHexChainId]: boolean }, + ), smartTransactionsOptInStatus: true, useSafeChainsListValidation: true, tokenSortConfig: { @@ -69,6 +72,10 @@ describe('PreferencesController', () => { { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, }, ], }, @@ -111,7 +118,16 @@ describe('PreferencesController', () => { 'KeyringController:stateChange', { ...getDefaultKeyringState(), - keyrings: [{ accounts: ['0x00'], type: 'CustomKeyring' }], + keyrings: [ + { + accounts: ['0x00'], + type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, + }, + ], }, [], ); @@ -141,7 +157,16 @@ describe('PreferencesController', () => { 'KeyringController:stateChange', { ...getDefaultKeyringState(), - keyrings: [{ accounts: ['0x00'], type: 'CustomKeyring' }], + keyrings: [ + { + accounts: ['0x00'], + type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, + }, + ], }, [], ); @@ -170,7 +195,16 @@ describe('PreferencesController', () => { 'KeyringController:stateChange', { ...getDefaultKeyringState(), - keyrings: [{ accounts: [], type: 'CustomKeyring' }], + keyrings: [ + { + accounts: [], + type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, + }, + ], }, [], ); @@ -203,6 +237,10 @@ describe('PreferencesController', () => { { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, }, ], }, @@ -237,10 +275,18 @@ describe('PreferencesController', () => { { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, }, { accounts: ['0x00', '0x01', '0x02'], type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, }, ], }, @@ -271,7 +317,16 @@ describe('PreferencesController', () => { 'KeyringController:stateChange', { ...getDefaultKeyringState(), - keyrings: [{ accounts: ['0x00', '0x01'], type: 'CustomKeyring' }], + keyrings: [ + { + accounts: ['0x00', '0x01'], + type: 'CustomKeyring', + metadata: { + id: 'mock-id', + name: '', + }, + }, + ], }, [], ); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts index eb01a865423..c8b7aefeaf9 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts @@ -995,7 +995,6 @@ describe('user-storage/user-storage-controller - snap handling', () => { messengerMocks.mockKeyringGetState.mockReturnValue({ isUnlocked: false, keyrings: [], - keyringsMetadata: [], }); const controller = new UserStorageController({ messenger: messengerMocks.messenger, @@ -1012,7 +1011,6 @@ describe('user-storage/user-storage-controller - snap handling', () => { messengerMocks.mockKeyringGetState.mockReturnValue({ isUnlocked: true, keyrings: [], - keyringsMetadata: [], }); const controller = new UserStorageController({ diff --git a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts index 4a1a3a606b6..eeb9f4861f6 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts @@ -132,7 +132,6 @@ export function mockUserStorageMessenger( ).mockReturnValue({ isUnlocked: true, keyrings: [], - keyringsMetadata: [], }); const mockAccountsListAccounts = jest.fn();