Skip to content

Commit fd0c24f

Browse files
shane-tmirceanismikespositoccharly
authored
chore: add metadata to account controller options for EVM HD accounts (#5618)
## Explanation In the multi-SRP context, we need to bubble up the Account metadata such that it is available in interactions with the AccountsController. This metadata will be used by the Backup & Sync feature to determine how to reconstruct the account when restoring from backup. ## References Fixes [Identity-90](https://consensyssoftware.atlassian.net/browse/IDENTITY-90) --------- Co-authored-by: Mircea Nistor <[email protected]> Co-authored-by: Michele Esposito <[email protected]> Co-authored-by: Charly Chevalier <[email protected]>
1 parent 6e8000b commit fd0c24f

File tree

5 files changed

+125
-14
lines changed

5 files changed

+125
-14
lines changed

packages/accounts-controller/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- Add new `setAccountNameAndSelectAccount` action ([#5714](https://github.com/MetaMask/core/pull/5714))
13+
- Add `entropySource` and `derivationPath` to EVM HD account options ([#5618](https://github.com/MetaMask/core/pull/5618))
1314

1415
### Changed
1516

packages/accounts-controller/src/AccountsController.test.ts

+66-2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { AccountsController, EMPTY_ACCOUNT } from './AccountsController';
3131
import {
3232
createExpectedInternalAccount,
3333
createMockInternalAccount,
34+
createMockInternalAccountOptions,
3435
ETH_EOA_METHODS,
3536
} from './tests/mocks';
3637
import {
@@ -901,6 +902,7 @@ describe('AccountsController', () => {
901902
name: 'Account 3',
902903
address: mockAccount3.address,
903904
keyringType: KeyringTypes.hd,
905+
options: {},
904906
}),
905907
]);
906908
});
@@ -1744,6 +1746,12 @@ describe('AccountsController', () => {
17441746
keyrings: [
17451747
{ type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] },
17461748
],
1749+
keyringsMetadata: [
1750+
{
1751+
id: 'mock-keyring-id-0',
1752+
name: 'mock-keyring-id-name',
1753+
},
1754+
],
17471755
}),
17481756
);
17491757

@@ -1772,12 +1780,14 @@ describe('AccountsController', () => {
17721780
id: 'mock-id',
17731781
address: mockAddress1,
17741782
keyringType: KeyringTypes.hd,
1783+
options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0),
17751784
}),
17761785
createExpectedInternalAccount({
17771786
name: 'Account 2',
17781787
id: 'mock-id2',
17791788
address: mockAddress2,
17801789
keyringType: KeyringTypes.hd,
1790+
options: createMockInternalAccountOptions(0, KeyringTypes.hd, 1),
17811791
}),
17821792
];
17831793
mockUUIDWithNormalAccounts(expectedAccounts);
@@ -1800,6 +1810,12 @@ describe('AccountsController', () => {
18001810
accounts: [mockSnapAccount, mockSnapAccount2],
18011811
},
18021812
],
1813+
keyringsMetadata: [
1814+
{
1815+
id: 'mock-keyring-id-1',
1816+
name: 'mock-keyring-id-name',
1817+
},
1818+
],
18031819
}),
18041820
);
18051821

@@ -1893,6 +1909,12 @@ describe('AccountsController', () => {
18931909
keyrings: [
18941910
{ type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] },
18951911
],
1912+
keyringsMetadata: [
1913+
{
1914+
id: 'mock-keyring-id-0',
1915+
name: 'mock-keyring-id-name',
1916+
},
1917+
],
18961918
}),
18971919
);
18981920

@@ -1918,12 +1940,16 @@ describe('AccountsController', () => {
19181940
messenger,
19191941
});
19201942
const expectedAccounts = [
1921-
mockAccount,
1943+
{
1944+
...mockAccount,
1945+
options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0),
1946+
},
19221947
createExpectedInternalAccount({
19231948
name: 'Account 2',
19241949
id: 'mock-id2',
19251950
address: mockAddress2,
19261951
keyringType: KeyringTypes.hd,
1952+
options: createMockInternalAccountOptions(0, KeyringTypes.hd, 1),
19271953
}),
19281954
];
19291955
mockUUIDWithNormalAccounts(expectedAccounts);
@@ -1957,6 +1983,16 @@ describe('AccountsController', () => {
19571983
{ type: KeyringTypes.hd, accounts: [mockAddress1] },
19581984
{ type: KeyringTypes.snap, accounts: ['0x1234'] },
19591985
],
1986+
keyringsMetadata: [
1987+
{
1988+
id: 'mock-keyring-id-0',
1989+
name: 'mock-keyring-id-name',
1990+
},
1991+
{
1992+
id: 'mock-keyring-id-1',
1993+
name: 'mock-keyring-id-name2',
1994+
},
1995+
],
19601996
}),
19611997
);
19621998

@@ -1975,6 +2011,7 @@ describe('AccountsController', () => {
19752011
id: 'mock-id',
19762012
address: mockAddress1,
19772013
keyringType: KeyringTypes.hd,
2014+
options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0),
19782015
}),
19792016
createExpectedInternalAccount({
19802017
name: 'Snap Account 1', // it is Snap Account 1 because it is the only snap account
@@ -2014,6 +2051,16 @@ describe('AccountsController', () => {
20142051
{ type: KeyringTypes.snap, accounts: ['0x1234'] },
20152052
{ type: KeyringTypes.hd, accounts: [mockAddress1] },
20162053
],
2054+
keyringsMetadata: [
2055+
{
2056+
id: 'mock-keyring-id-0',
2057+
name: 'mock-keyring-id-name',
2058+
},
2059+
{
2060+
id: 'mock-keyring-id-1',
2061+
name: 'mock-keyring-id-name2',
2062+
},
2063+
],
20172064
}),
20182065
);
20192066

@@ -2032,6 +2079,7 @@ describe('AccountsController', () => {
20322079
id: 'mock-id',
20332080
address: mockAddress1,
20342081
keyringType: KeyringTypes.hd,
2082+
options: createMockInternalAccountOptions(1, KeyringTypes.hd, 0),
20352083
}),
20362084
createExpectedInternalAccount({
20372085
name: 'Snap Account 1', // it is Snap Account 1 because it is the only snap account
@@ -2057,7 +2105,6 @@ describe('AccountsController', () => {
20572105
KeyringTypes.ledger,
20582106
KeyringTypes.lattice,
20592107
KeyringTypes.qr,
2060-
'Custody - JSON - RPC',
20612108
])('should add accounts for %s type', async (keyringType) => {
20622109
mockUUIDWithNormalAccounts([mockAccount]);
20632110

@@ -2067,6 +2114,12 @@ describe('AccountsController', () => {
20672114
'KeyringController:getState',
20682115
mockGetState.mockReturnValue({
20692116
keyrings: [{ type: keyringType, accounts: [mockAddress1] }],
2117+
keyringsMetadata: [
2118+
{
2119+
id: 'mock-keyring-id-0',
2120+
name: 'mock-keyring-id-name',
2121+
},
2122+
],
20702123
}),
20712124
);
20722125

@@ -2096,6 +2149,7 @@ describe('AccountsController', () => {
20962149
id: 'mock-id',
20972150
address: mockAddress1,
20982151
keyringType: keyringType as KeyringTypes,
2152+
options: createMockInternalAccountOptions(0, keyringType, 0),
20992153
}),
21002154
];
21012155

@@ -2206,6 +2260,16 @@ describe('AccountsController', () => {
22062260
{ type: KeyringTypes.snap, accounts: ['0x1234'] },
22072261
{ type: KeyringTypes.hd, accounts: [mockAddress1] },
22082262
],
2263+
keyringsMetadata: [
2264+
{
2265+
id: 'mock-keyring-id-1',
2266+
name: 'mock-keyring-id-name',
2267+
},
2268+
{
2269+
id: 'mock-keyring-id-2',
2270+
name: 'mock-keyring-id-name2',
2271+
},
2272+
],
22092273
}),
22102274
);
22112275

packages/accounts-controller/src/AccountsController.ts

+19-4
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ import type { WritableDraft } from 'immer/dist/internal.js';
3838

3939
import type { MultichainNetworkControllerNetworkDidChangeEvent } from './types';
4040
import {
41+
getDerivationPathForIndex,
4142
getUUIDFromAddressOfNormalAccount,
43+
isHdKeyringType,
4244
isNormalKeyringType,
4345
keyringTypeToName,
4446
} from './utils';
@@ -682,19 +684,32 @@ export class AccountsController extends BaseController<
682684
*/
683685
async #listNormalAccounts(): Promise<InternalAccount[]> {
684686
const internalAccounts: InternalAccount[] = [];
685-
const { keyrings } = await this.messagingSystem.call(
687+
const { keyrings, keyringsMetadata } = this.messagingSystem.call(
686688
'KeyringController:getState',
687689
);
688-
for (const keyring of keyrings) {
690+
691+
for (const [keyringIndex, keyring] of keyrings.entries()) {
689692
const keyringType = keyring.type;
690693
if (!isNormalKeyringType(keyringType as KeyringTypes)) {
691694
// We only consider "normal accounts" here, so keep looping
692695
continue;
693696
}
694697

695-
for (const address of keyring.accounts) {
698+
for (const [accountIndex, address] of keyring.accounts.entries()) {
696699
const id = getUUIDFromAddressOfNormalAccount(address);
697700

701+
let options = {};
702+
703+
if (isHdKeyringType(keyring.type as KeyringTypes)) {
704+
options = {
705+
entropySource: keyringsMetadata[keyringIndex].id,
706+
// NOTE: We are not using the `hdPath` from the associated keyring here and
707+
// getting the keyring instance here feels a bit overkill.
708+
// This will be naturally fixed once every keyring start using `KeyringAccount` and implement the keyring API.
709+
derivationPath: getDerivationPathForIndex(accountIndex),
710+
};
711+
}
712+
698713
const nameLastUpdatedAt = this.#populateExistingMetadata(
699714
id,
700715
'nameLastUpdatedAt',
@@ -703,7 +718,7 @@ export class AccountsController extends BaseController<
703718
internalAccounts.push({
704719
id,
705720
address,
706-
options: {},
721+
options,
707722
methods: [
708723
EthMethod.PersonalSign,
709724
EthMethod.Sign,

packages/accounts-controller/src/tests/mocks.ts

+18-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export const createMockInternalAccount = ({
3737
scopes,
3838
importTime = Date.now(),
3939
lastSelected = Date.now(),
40+
options,
4041
}: {
4142
id?: string;
4243
address?: string;
@@ -52,6 +53,7 @@ export const createMockInternalAccount = ({
5253
};
5354
importTime?: number;
5455
lastSelected?: number;
56+
options?: Record<string, unknown>;
5557
} = {}): InternalAccount => {
5658
const getInternalAccountDefaults = () => {
5759
switch (type) {
@@ -80,7 +82,7 @@ export const createMockInternalAccount = ({
8082
return {
8183
id,
8284
address,
83-
options: {},
85+
options: options ?? {},
8486
methods: methods ?? defaults.methods,
8587
scopes: scopes ?? defaults.scopes,
8688
type,
@@ -104,3 +106,18 @@ export const createExpectedInternalAccount = (
104106
lastSelected: expect.any(Number),
105107
});
106108
};
109+
110+
export const createMockInternalAccountOptions = (
111+
keyringIndex: number,
112+
keyringType: KeyringTypes,
113+
groupIndex: number,
114+
): Record<string, string> => {
115+
if (keyringType === KeyringTypes.hd) {
116+
return {
117+
entropySource: `mock-keyring-id-${keyringIndex}`,
118+
derivationPath: `m/44'/60'/0'/0/${groupIndex}`,
119+
};
120+
}
121+
122+
return {};
123+
};

packages/accounts-controller/src/utils.ts

+21-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isCustodyKeyring, KeyringTypes } from '@metamask/keyring-controller';
1+
import { KeyringTypes } from '@metamask/keyring-controller';
22
import { hexToBytes } from '@metamask/utils';
33
import { sha256 } from 'ethereum-cryptography/sha256';
44
import type { V4Options } from 'uuid';
@@ -11,12 +11,6 @@ import { v4 as uuid } from 'uuid';
1111
* @returns The name of the keyring type.
1212
*/
1313
export function keyringTypeToName(keyringType: string): string {
14-
// Custody keyrings are a special case, as they are not a single type
15-
// they just start with the prefix `Custody`
16-
if (isCustodyKeyring(keyringType)) {
17-
return 'Custody';
18-
}
19-
2014
switch (keyringType) {
2115
case KeyringTypes.simple: {
2216
return 'Account';
@@ -85,3 +79,23 @@ export function isNormalKeyringType(keyringType: KeyringTypes): boolean {
8579
// adapted later on if we have new kind of keyrings!
8680
return keyringType !== KeyringTypes.snap;
8781
}
82+
83+
/**
84+
* Check if a keyring is a HD keyring.
85+
*
86+
* @param keyringType - The account's keyring type.
87+
* @returns True if the keyring is a HD keyring, false otherwise.
88+
*/
89+
export function isHdKeyringType(keyringType: KeyringTypes): boolean {
90+
return keyringType === KeyringTypes.hd;
91+
}
92+
93+
/**
94+
* Get the derivation path for the index of an account within a HD keyring.
95+
*
96+
* @param index - The account index.
97+
* @returns The derivation path.
98+
*/
99+
export function getDerivationPathForIndex(index: number): string {
100+
return `m/44'/60'/0'/0/${index}`;
101+
}

0 commit comments

Comments
 (0)