diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index 56cd7da61b2..7628cddad50 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-controller/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add new `setAccountNameAndSelectAccount` action ([#5714](https://github.com/MetaMask/core/pull/5714)) +- Add `entropySource` and `derivationPath` to EVM HD account options ([#5618](https://github.com/MetaMask/core/pull/5618)) ### Changed diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index 0644c1eb1cf..96d0b8d6d25 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -31,6 +31,7 @@ import { AccountsController, EMPTY_ACCOUNT } from './AccountsController'; import { createExpectedInternalAccount, createMockInternalAccount, + createMockInternalAccountOptions, ETH_EOA_METHODS, } from './tests/mocks'; import { @@ -901,6 +902,7 @@ describe('AccountsController', () => { name: 'Account 3', address: mockAccount3.address, keyringType: KeyringTypes.hd, + options: {}, }), ]); }); @@ -1744,6 +1746,12 @@ describe('AccountsController', () => { keyrings: [ { type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] }, ], + keyringsMetadata: [ + { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, + ], }), ); @@ -1772,12 +1780,14 @@ describe('AccountsController', () => { id: 'mock-id', address: mockAddress1, keyringType: KeyringTypes.hd, + options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0), }), createExpectedInternalAccount({ name: 'Account 2', id: 'mock-id2', address: mockAddress2, keyringType: KeyringTypes.hd, + options: createMockInternalAccountOptions(0, KeyringTypes.hd, 1), }), ]; mockUUIDWithNormalAccounts(expectedAccounts); @@ -1800,6 +1810,12 @@ describe('AccountsController', () => { accounts: [mockSnapAccount, mockSnapAccount2], }, ], + keyringsMetadata: [ + { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name', + }, + ], }), ); @@ -1893,6 +1909,12 @@ describe('AccountsController', () => { keyrings: [ { type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] }, ], + keyringsMetadata: [ + { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, + ], }), ); @@ -1918,12 +1940,16 @@ describe('AccountsController', () => { messenger, }); const expectedAccounts = [ - mockAccount, + { + ...mockAccount, + options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0), + }, createExpectedInternalAccount({ name: 'Account 2', id: 'mock-id2', address: mockAddress2, keyringType: KeyringTypes.hd, + options: createMockInternalAccountOptions(0, KeyringTypes.hd, 1), }), ]; mockUUIDWithNormalAccounts(expectedAccounts); @@ -1957,6 +1983,16 @@ describe('AccountsController', () => { { type: KeyringTypes.hd, accounts: [mockAddress1] }, { type: KeyringTypes.snap, accounts: ['0x1234'] }, ], + keyringsMetadata: [ + { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, + { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name2', + }, + ], }), ); @@ -1975,6 +2011,7 @@ describe('AccountsController', () => { id: 'mock-id', address: mockAddress1, keyringType: KeyringTypes.hd, + options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0), }), createExpectedInternalAccount({ name: 'Snap Account 1', // it is Snap Account 1 because it is the only snap account @@ -2014,6 +2051,16 @@ describe('AccountsController', () => { { type: KeyringTypes.snap, accounts: ['0x1234'] }, { type: KeyringTypes.hd, accounts: [mockAddress1] }, ], + keyringsMetadata: [ + { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, + { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name2', + }, + ], }), ); @@ -2032,6 +2079,7 @@ describe('AccountsController', () => { id: 'mock-id', address: mockAddress1, keyringType: KeyringTypes.hd, + options: createMockInternalAccountOptions(1, KeyringTypes.hd, 0), }), createExpectedInternalAccount({ name: 'Snap Account 1', // it is Snap Account 1 because it is the only snap account @@ -2057,7 +2105,6 @@ describe('AccountsController', () => { KeyringTypes.ledger, KeyringTypes.lattice, KeyringTypes.qr, - 'Custody - JSON - RPC', ])('should add accounts for %s type', async (keyringType) => { mockUUIDWithNormalAccounts([mockAccount]); @@ -2067,6 +2114,12 @@ describe('AccountsController', () => { 'KeyringController:getState', mockGetState.mockReturnValue({ keyrings: [{ type: keyringType, accounts: [mockAddress1] }], + keyringsMetadata: [ + { + id: 'mock-keyring-id-0', + name: 'mock-keyring-id-name', + }, + ], }), ); @@ -2096,6 +2149,7 @@ describe('AccountsController', () => { id: 'mock-id', address: mockAddress1, keyringType: keyringType as KeyringTypes, + options: createMockInternalAccountOptions(0, keyringType, 0), }), ]; @@ -2206,6 +2260,16 @@ describe('AccountsController', () => { { type: KeyringTypes.snap, accounts: ['0x1234'] }, { type: KeyringTypes.hd, accounts: [mockAddress1] }, ], + keyringsMetadata: [ + { + id: 'mock-keyring-id-1', + name: 'mock-keyring-id-name', + }, + { + id: 'mock-keyring-id-2', + name: 'mock-keyring-id-name2', + }, + ], }), ); diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 4fcc2c9ffc4..56c344e8849 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -38,7 +38,9 @@ import type { WritableDraft } from 'immer/dist/internal.js'; import type { MultichainNetworkControllerNetworkDidChangeEvent } from './types'; import { + getDerivationPathForIndex, getUUIDFromAddressOfNormalAccount, + isHdKeyringType, isNormalKeyringType, keyringTypeToName, } from './utils'; @@ -682,19 +684,32 @@ export class AccountsController extends BaseController< */ async #listNormalAccounts(): Promise { const internalAccounts: InternalAccount[] = []; - const { keyrings } = await this.messagingSystem.call( + const { keyrings, keyringsMetadata } = this.messagingSystem.call( 'KeyringController:getState', ); - for (const keyring of keyrings) { + + for (const [keyringIndex, keyring] of keyrings.entries()) { const keyringType = keyring.type; if (!isNormalKeyringType(keyringType as KeyringTypes)) { // We only consider "normal accounts" here, so keep looping continue; } - for (const address of keyring.accounts) { + for (const [accountIndex, address] of keyring.accounts.entries()) { const id = getUUIDFromAddressOfNormalAccount(address); + let options = {}; + + if (isHdKeyringType(keyring.type as KeyringTypes)) { + options = { + entropySource: keyringsMetadata[keyringIndex].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. + derivationPath: getDerivationPathForIndex(accountIndex), + }; + } + const nameLastUpdatedAt = this.#populateExistingMetadata( id, 'nameLastUpdatedAt', @@ -703,7 +718,7 @@ export class AccountsController extends BaseController< internalAccounts.push({ id, address, - options: {}, + options, methods: [ EthMethod.PersonalSign, EthMethod.Sign, diff --git a/packages/accounts-controller/src/tests/mocks.ts b/packages/accounts-controller/src/tests/mocks.ts index 18cc151224e..29be1c7eab2 100644 --- a/packages/accounts-controller/src/tests/mocks.ts +++ b/packages/accounts-controller/src/tests/mocks.ts @@ -37,6 +37,7 @@ export const createMockInternalAccount = ({ scopes, importTime = Date.now(), lastSelected = Date.now(), + options, }: { id?: string; address?: string; @@ -52,6 +53,7 @@ export const createMockInternalAccount = ({ }; importTime?: number; lastSelected?: number; + options?: Record; } = {}): InternalAccount => { const getInternalAccountDefaults = () => { switch (type) { @@ -80,7 +82,7 @@ export const createMockInternalAccount = ({ return { id, address, - options: {}, + options: options ?? {}, methods: methods ?? defaults.methods, scopes: scopes ?? defaults.scopes, type, @@ -104,3 +106,18 @@ export const createExpectedInternalAccount = ( lastSelected: expect.any(Number), }); }; + +export const createMockInternalAccountOptions = ( + keyringIndex: number, + keyringType: KeyringTypes, + groupIndex: number, +): Record => { + if (keyringType === KeyringTypes.hd) { + return { + entropySource: `mock-keyring-id-${keyringIndex}`, + derivationPath: `m/44'/60'/0'/0/${groupIndex}`, + }; + } + + return {}; +}; diff --git a/packages/accounts-controller/src/utils.ts b/packages/accounts-controller/src/utils.ts index 0c4e89a0662..3bc80288919 100644 --- a/packages/accounts-controller/src/utils.ts +++ b/packages/accounts-controller/src/utils.ts @@ -1,4 +1,4 @@ -import { isCustodyKeyring, KeyringTypes } from '@metamask/keyring-controller'; +import { KeyringTypes } from '@metamask/keyring-controller'; import { hexToBytes } from '@metamask/utils'; import { sha256 } from 'ethereum-cryptography/sha256'; import type { V4Options } from 'uuid'; @@ -11,12 +11,6 @@ import { v4 as uuid } from 'uuid'; * @returns The name of the keyring type. */ export function keyringTypeToName(keyringType: string): string { - // Custody keyrings are a special case, as they are not a single type - // they just start with the prefix `Custody` - if (isCustodyKeyring(keyringType)) { - return 'Custody'; - } - switch (keyringType) { case KeyringTypes.simple: { return 'Account'; @@ -85,3 +79,23 @@ export function isNormalKeyringType(keyringType: KeyringTypes): boolean { // adapted later on if we have new kind of keyrings! return keyringType !== KeyringTypes.snap; } + +/** + * Check if a keyring is a HD keyring. + * + * @param keyringType - The account's keyring type. + * @returns True if the keyring is a HD keyring, false otherwise. + */ +export function isHdKeyringType(keyringType: KeyringTypes): boolean { + return keyringType === KeyringTypes.hd; +} + +/** + * Get the derivation path for the index of an account within a HD keyring. + * + * @param index - The account index. + * @returns The derivation path. + */ +export function getDerivationPathForIndex(index: number): string { + return `m/44'/60'/0'/0/${index}`; +}