From 3eaf3baa02ad43e40fb0249618ff2d50d66b7027 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:28:20 -0400 Subject: [PATCH 01/77] feat: add discoverAndCreateAccounts method to MultichainAccountWallet --- .../src/MultichainAccountWallet.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index bc068eee429..7321e9f78be 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -326,4 +326,43 @@ export class MultichainAccountWallet< await group.align(); } } + + async discoverAndCreateAccounts({ + skippedProviders = [], + }: { + skippedProviders: AccountProviderType[]; + }): Promise<{ + [providerType: string]: Bip44Account[]; + }> { + const discoverAndCreateAccountPromises = []; + for (const provider of this.#providers) { + // Update AccountProvider type to avoid type errors. + if (skippedProviders.includes(provider.providerType)) { + continue; + } + + discoverAndCreateAccountPromises.push([ + provider.providerType, + provider.discoverAndCreateAccounts({ + entropySource: this.#entropySource, + }), + ]); + } + await Promise.allSettled(discoverAndCreateAccountPromises.map((p) => p[1])); + const result = discoverAndCreateAccountPromises.reduce((acc, tuple) => { + const [providerType, promise] = tuple; + if (promise.status === 'fulfilled') { + acc[providerType] = promise.value; + } + return acc; + }, {}); + + // TODO: align groups here, not sure if the data flow from keyring controller -> accounts controller -> multichain service group creation + // would have took place at this point yet. + + // result type is as it is because the clients expect to know + // the count of accounts for each provider, might change to just a count + // instead of the array of actual accounts. + return result; + } } From 0eae422cc1fdc09b5e9b8764de7a7020dfc5cae5 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:28:55 -0400 Subject: [PATCH 02/77] feat: add provider type and remove groupIndex from discoverAndCreateAccounts --- .../src/providers/BaseAccountProvider.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/providers/BaseAccountProvider.ts b/packages/multichain-account-service/src/providers/BaseAccountProvider.ts index 54000c98279..6115276bd5a 100644 --- a/packages/multichain-account-service/src/providers/BaseAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BaseAccountProvider.ts @@ -42,8 +42,14 @@ export abstract class BaseAccountProvider { protected readonly messenger: MultichainAccountServiceMessenger; - constructor(messenger: MultichainAccountServiceMessenger) { + readonly providerType: AccountProviderType; + + constructor( + messenger: MultichainAccountServiceMessenger, + providerType: AccountProviderType, + ) { this.messenger = messenger; + this.providerType = providerType; } #getAccounts( @@ -121,9 +127,7 @@ export abstract class BaseAccountProvider abstract discoverAndCreateAccounts({ entropySource, - groupIndex, }: { entropySource: EntropySourceId; - groupIndex: number; }): Promise[]>; } From 6c792c7430acb0325d35559d2263d7c3b4b34f0f Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:29:40 -0400 Subject: [PATCH 03/77] feat: add method to get the EVM provider and fill in discoverAndCreateAccounts method --- .../src/providers/EvmAccountProvider.ts | 52 +++++++++++++++++-- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 3655133aa5f..9aafd690d3a 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -6,7 +6,9 @@ import type { EthKeyring, InternalAccount, } from '@metamask/keyring-internal-api'; +import type { Provider } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; +import type { MultichainAccountServiceMessenger } from 'src/types'; import { assertAreBip44Accounts, @@ -28,6 +30,10 @@ function assertInternalAccountExists( } export class EvmAccountProvider extends BaseAccountProvider { + constructor(messenger: MultichainAccountServiceMessenger) { + super(messenger, AccountProviderType.Evm); + } + isAccountCompatible(account: Bip44Account): boolean { return ( account.type === EthAccountType.Eoa && @@ -35,6 +41,18 @@ export class EvmAccountProvider extends BaseAccountProvider { ); } + getEvmProvider(): Provider { + const networkClientId = this.messenger.call( + 'NetworkController:findNetworkClientIdByChainId', + '0x1', + ); + const { provider } = this.messenger.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + return provider; + } + async createAccounts({ entropySource, groupIndex, @@ -76,10 +94,34 @@ export class EvmAccountProvider extends BaseAccountProvider { return accountsArray; } - async discoverAndCreateAccounts(_: { - entropySource: EntropySourceId; - groupIndex: number; - }) { - return []; // TODO: Implement account discovery. + async discoverAndCreateAccounts(opts: { entropySource: EntropySourceId }) { + const provider = this.getEvmProvider(); + const accounts = []; + // we start at 1 because we already have the first account. + for (let i = 1; ; i++) { + const [address] = await this.withKeyring( + { id: opts.entropySource }, + async ({ keyring }) => { + return await keyring.addAccounts(1); + }, + ); + const countHex = (await provider.request({ + method: 'eth_getTransactionCount', + params: [address, 'latest'], + })) as Hex; + const count = parseInt(countHex, 16); + if (count === 0) { + // If the account has no transactions, we can remove it. + await this.withKeyring( + { id: opts.entropySource }, + async ({ keyring }) => { + return await keyring.removeAccount(address); + }, + ); + break; + } + accounts.push(address); + } + return accounts; } } From 74790a56750e30bc348881eddc40a6badc7320d3 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:30:13 -0400 Subject: [PATCH 04/77] feat: update snap provider class to add providerType --- .../src/providers/SnapAccountProvider.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index 7d8a1cb783d..485a19b15a7 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -15,8 +15,12 @@ export type RestrictedSnapKeyringCreateAccount = ( export abstract class SnapAccountProvider extends BaseAccountProvider { readonly snapId: SnapId; - constructor(snapId: SnapId, messenger: MultichainAccountServiceMessenger) { - super(messenger); + constructor( + snapId: SnapId, + messenger: MultichainAccountServiceMessenger, + providerType: AccountProviderType, + ) { + super(messenger, providerType); this.snapId = snapId; } @@ -52,6 +56,5 @@ export abstract class SnapAccountProvider extends BaseAccountProvider { abstract discoverAndCreateAccounts(options: { entropySource: EntropySourceId; - groupIndex: number; }): Promise[]>; } From b6f8de3c8f9772e4a6de1976f89cd590f644fec2 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:30:48 -0400 Subject: [PATCH 05/77] feat: update sol provider to add providerType and remove groupIndex from discoverAndCreateAccounts --- .../src/providers/SolAccountProvider.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 094a7c6e082..290890cf320 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -16,7 +16,11 @@ export class SolAccountProvider extends SnapAccountProvider { static SOLANA_SNAP_ID = 'npm:@metamask/solana-wallet-snap' as SnapId; constructor(messenger: MultichainAccountServiceMessenger) { - super(SolAccountProvider.SOLANA_SNAP_ID, messenger); + super( + SolAccountProvider.SOLANA_SNAP_ID, + messenger, + AccountProviderType.Solana, + ); } isAccountCompatible(account: Bip44Account): boolean { @@ -60,10 +64,7 @@ export class SolAccountProvider extends SnapAccountProvider { return accounts; } - async discoverAndCreateAccounts(_: { - entropySource: EntropySourceId; - groupIndex: number; - }) { + async discoverAndCreateAccounts(_: { entropySource: EntropySourceId }) { return []; // TODO: Implement account discovery. } } From 6a6a184e2f39ff0f9daebfb67be28a1de3b0ce94 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:31:16 -0400 Subject: [PATCH 06/77] feat: add network controller actions to messenger --- packages/multichain-account-service/src/types.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index c8cb682b3e5..70e889f78f0 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -11,6 +11,10 @@ import type { KeyringControllerStateChangeEvent, KeyringControllerWithKeyringAction, } from '@metamask/keyring-controller'; +import type { + NetworkControllerFindNetworkClientIdByChainIdAction, + NetworkControllerGetNetworkClientByIdAction, +} from '@metamask/network-controller'; import type { HandleSnapRequest as SnapControllerHandleSnapRequestAction } from '@metamask/snaps-controllers'; import type { @@ -88,7 +92,9 @@ export type AllowedActions = | AccountsControllerGetAccountByAddressAction | SnapControllerHandleSnapRequestAction | KeyringControllerWithKeyringAction - | KeyringControllerGetStateAction; + | KeyringControllerGetStateAction + | NetworkControllerGetNetworkClientByIdAction + | NetworkControllerFindNetworkClientIdByChainIdAction; /** * All events published by other modules that {@link MultichainAccountService} From 2960c7eda7ac2cc6372649a909028a247753ad04 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:46:31 -0400 Subject: [PATCH 07/77] chore: remove comment --- .../src/providers/EvmAccountProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 9aafd690d3a..0dbad829c2e 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -97,7 +97,7 @@ export class EvmAccountProvider extends BaseAccountProvider { async discoverAndCreateAccounts(opts: { entropySource: EntropySourceId }) { const provider = this.getEvmProvider(); const accounts = []; - // we start at 1 because we already have the first account. + for (let i = 1; ; i++) { const [address] = await this.withKeyring( { id: opts.entropySource }, From ba635c913b939230aa1056d2a0571172693fddac Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 27 Aug 2025 09:55:53 -0400 Subject: [PATCH 08/77] chore: add JSdocs --- .../src/MultichainAccountWallet.ts | 9 +++++++++ .../src/providers/EvmAccountProvider.ts | 15 +++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 7321e9f78be..f8bfe326938 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -327,6 +327,15 @@ export class MultichainAccountWallet< } } + /** + * Discover and create accounts for all providers. + * + * NOTE: This method should only be called on a newly created wallet. + * + * @param opts - The options for the discovery and creation of accounts. + * @param opts.skippedProviders - The providers to skip. + * @returns The accounts for each provider. + */ async discoverAndCreateAccounts({ skippedProviders = [], }: { diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 0dbad829c2e..d9d8d34d78f 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -41,6 +41,11 @@ export class EvmAccountProvider extends BaseAccountProvider { ); } + /** + * Get the Evm provider. + * + * @returns The Evm provider. + */ getEvmProvider(): Provider { const networkClientId = this.messenger.call( 'NetworkController:findNetworkClientIdByChainId', @@ -94,6 +99,16 @@ export class EvmAccountProvider extends BaseAccountProvider { return accountsArray; } + /** + * Discover and create accounts for the Evm provider. + * + * NOTE: This method should only be called on a newly created wallet. + * There should be already one existing account on this associated entropy source. + * + * @param opts - The options for the discovery and creation of accounts. + * @param opts.entropySource - The entropy source to use for the discovery and creation of accounts. + * @returns The accounts for the Evm provider. + */ async discoverAndCreateAccounts(opts: { entropySource: EntropySourceId }) { const provider = this.getEvmProvider(); const accounts = []; From f2c16a190a4f82f034269fd7d88695e3c44f3fb0 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 09:51:00 -0400 Subject: [PATCH 09/77] refactor: move orchestraction into wallet class --- .../src/MultichainAccountWallet.ts | 103 +++++++++++++----- 1 file changed, 76 insertions(+), 27 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index f8bfe326938..b2020fe2bfd 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -341,37 +341,86 @@ export class MultichainAccountWallet< }: { skippedProviders: AccountProviderType[]; }): Promise<{ - [providerType: string]: Bip44Account[]; + [AccountProviderType]: Bip44Account[]; }> { - const discoverAndCreateAccountPromises = []; - for (const provider of this.#providers) { - // Update AccountProvider type to avoid type errors. - if (skippedProviders.includes(provider.providerType)) { - continue; - } + const providers = this.#providers.filter( + (provider) => !skippedProviders.includes(provider.providerType), + ); - discoverAndCreateAccountPromises.push([ - provider.providerType, - provider.discoverAndCreateAccounts({ - entropySource: this.#entropySource, - }), - ]); + const results: Record[]> = + {}; + const nextIndex = new Map( + providers.map((p) => [p, 0]), + ); + const stopped = new Set(); + const inFlight = new Map>(); + + let high = 0; + + const schedule = (p: Bip44Provider, index: number) => { + const run = (async () => { + try { + const accounts = await p.discoverAndCreateAccounts({ + entropySource: this.#entropySource, + groupIndex: index, + }); + + inFlight.delete(p); + + if (accounts.length > 0) { + stopped.add(p); + } else { + results[p.providerType] = [ + ...(results[p.providerType] ?? []), + ...accounts, + ]; + + const next = index + 1; + nextIndex.set(p, next); + if (next > high) { + high = next; + + for (const q of providers) { + if ( + !stopped.has(q) && + !inFlight.has(q) && + (nextIndex.get(q) ?? 0) < high + ) { + schedule(q, high); + } + } + } + + if (!stopped.has(p)) { + const target = Math.max(high, nextIndex.get(p) ?? 0); + schedule(p, target); + } + } + } catch { + inFlight.delete(p); + stopped.add(p); + } + })(); + + inFlight.set(p, run); + }; + + for (const p of providers) { + schedule(p, 0); } - await Promise.allSettled(discoverAndCreateAccountPromises.map((p) => p[1])); - const result = discoverAndCreateAccountPromises.reduce((acc, tuple) => { - const [providerType, promise] = tuple; - if (promise.status === 'fulfilled') { - acc[providerType] = promise.value; - } - return acc; - }, {}); - // TODO: align groups here, not sure if the data flow from keyring controller -> accounts controller -> multichain service group creation - // would have took place at this point yet. + while (inFlight.size > 0) { + await Promise.race(inFlight.values()); + } + + await this.alignGroups(); + + const out: Record[]> = {}; + + for (const p of providers) { + out[p.providerType] = results[p.providerType] ?? []; + } - // result type is as it is because the clients expect to know - // the count of accounts for each provider, might change to just a count - // instead of the array of actual accounts. - return result; + return out; } } From e4fcc7fa4bd3fe3c30ea3df1f6d68640c98f99fb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 09:51:44 -0400 Subject: [PATCH 10/77] refactor: re-add groupIndex to discoverAndCreateAccounts provider methods --- .../src/providers/BaseAccountProvider.ts | 2 + .../src/providers/EvmAccountProvider.ts | 65 ++++++++++++------- .../src/providers/SnapAccountProvider.ts | 1 + 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/packages/multichain-account-service/src/providers/BaseAccountProvider.ts b/packages/multichain-account-service/src/providers/BaseAccountProvider.ts index 6115276bd5a..845eadb3c5f 100644 --- a/packages/multichain-account-service/src/providers/BaseAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BaseAccountProvider.ts @@ -127,7 +127,9 @@ export abstract class BaseAccountProvider abstract discoverAndCreateAccounts({ entropySource, + groupIndex, }: { entropySource: EntropySourceId; + groupIndex: number; }): Promise[]>; } diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index d9d8d34d78f..5da8ec46a73 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -12,6 +12,7 @@ import type { MultichainAccountServiceMessenger } from 'src/types'; import { assertAreBip44Accounts, + assertIsBip44Account, BaseAccountProvider, } from './BaseAccountProvider'; @@ -107,36 +108,54 @@ export class EvmAccountProvider extends BaseAccountProvider { * * @param opts - The options for the discovery and creation of accounts. * @param opts.entropySource - The entropy source to use for the discovery and creation of accounts. + * @param opts.groupIndex - The index of the group to create the accounts for. * @returns The accounts for the Evm provider. */ - async discoverAndCreateAccounts(opts: { entropySource: EntropySourceId }) { + async discoverAndCreateAccounts(opts: { + entropySource: EntropySourceId; + groupIndex: number; + }) { const provider = this.getEvmProvider(); - const accounts = []; + const { entropySource, groupIndex } = opts; + // groupIndex starts as +1, because we already have one account in the associated keyring. + const actualGroupIndex = groupIndex + 1; - for (let i = 1; ; i++) { - const [address] = await this.withKeyring( - { id: opts.entropySource }, + const [address, didCreate] = await this.withKeyring< + EthKeyring, + [Hex, boolean] + >({ id: entropySource }, async ({ keyring }) => { + const existing = await keyring.getAccounts(); + if (actualGroupIndex < existing.length) { + return [existing[actualGroupIndex], false]; + } + const need = actualGroupIndex - existing.length + 1; + const added = await keyring.addAccounts(need); + const target = added[added.length - 1]; + return [target, true]; + }); + + const countHex = (await provider.request({ + method: 'eth_getTransactionCount', + params: [address, 'latest'], + })) as Hex; + const count = parseInt(countHex, 16); + + if (count === 0 && didCreate) { + await this.withKeyring( + { id: entropySource }, async ({ keyring }) => { - return await keyring.addAccounts(1); + keyring.removeAccount?.(address); }, ); - const countHex = (await provider.request({ - method: 'eth_getTransactionCount', - params: [address, 'latest'], - })) as Hex; - const count = parseInt(countHex, 16); - if (count === 0) { - // If the account has no transactions, we can remove it. - await this.withKeyring( - { id: opts.entropySource }, - async ({ keyring }) => { - return await keyring.removeAccount(address); - }, - ); - break; - } - accounts.push(address); + return []; } - return accounts; + + const account = this.messenger.call( + 'AccountsController:getAccountByAddress', + address, + ); + assertInternalAccountExists(account); + assertIsBip44Account(account); + return [account]; } } diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index 485a19b15a7..4d46b2de04b 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -56,5 +56,6 @@ export abstract class SnapAccountProvider extends BaseAccountProvider { abstract discoverAndCreateAccounts(options: { entropySource: EntropySourceId; + groupIndex: number; }): Promise[]>; } From 5994b14dcabf6fe804e57f6cbc3b14b46bfa31c0 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 09:54:25 -0400 Subject: [PATCH 11/77] refactor: add groupIndex for sol provider --- .../src/providers/SolAccountProvider.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 290890cf320..8c84795979d 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -64,7 +64,10 @@ export class SolAccountProvider extends SnapAccountProvider { return accounts; } - async discoverAndCreateAccounts(_: { entropySource: EntropySourceId }) { + async discoverAndCreateAccounts(_: { + entropySource: EntropySourceId; + groupIndex: number; + }) { return []; // TODO: Implement account discovery. } } From bc19d05956602b0ce2e27f2649ee772bcb584428 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 16:56:58 -0400 Subject: [PATCH 12/77] refactor: apply code review --- .../src/MultichainAccountWallet.ts | 125 +++++++++--------- .../src/providers/BaseAccountProvider.ts | 8 +- .../src/providers/EvmAccountProvider.ts | 4 - .../src/providers/SnapAccountProvider.ts | 8 +- .../src/providers/SolAccountProvider.ts | 6 +- 5 files changed, 69 insertions(+), 82 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index b2020fe2bfd..d2c46fa804b 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -332,95 +332,100 @@ export class MultichainAccountWallet< * * NOTE: This method should only be called on a newly created wallet. * - * @param opts - The options for the discovery and creation of accounts. - * @param opts.skippedProviders - The providers to skip. * @returns The accounts for each provider. */ - async discoverAndCreateAccounts({ - skippedProviders = [], - }: { - skippedProviders: AccountProviderType[]; - }): Promise<{ - [AccountProviderType]: Bip44Account[]; - }> { - const providers = this.#providers.filter( - (provider) => !skippedProviders.includes(provider.providerType), - ); + async discoverAndCreateAccounts(): Promise> { + const providers = this.#providers; + const providerContexts = new Map< + Bip44Provider, + { + stopped: boolean; + running?: Promise; + index: number; + count: number; + } + >(); - const results: Record[]> = - {}; - const nextIndex = new Map( - providers.map((p) => [p, 0]), - ); - const stopped = new Set(); - const inFlight = new Map>(); + for (const p of providers) { + providerContexts.set(p, { stopped: false, index: 0, count: 0 }); + } - let high = 0; + let maxGroupIndex = 0; const schedule = (p: Bip44Provider, index: number) => { - const run = (async () => { + const providerCtx = providerContexts.get(p); + if (!providerCtx) { + throw new Error(`Provider ${p} not found in providerContexts`); + } + + if (providerCtx.stopped || providerCtx.running) { + return; + } + + providerCtx.index = index; + + providerCtx.running = (async () => { try { const accounts = await p.discoverAndCreateAccounts({ entropySource: this.#entropySource, groupIndex: index, }); - inFlight.delete(p); - - if (accounts.length > 0) { - stopped.add(p); - } else { - results[p.providerType] = [ - ...(results[p.providerType] ?? []), - ...accounts, - ]; - - const next = index + 1; - nextIndex.set(p, next); - if (next > high) { - high = next; - - for (const q of providers) { - if ( - !stopped.has(q) && - !inFlight.has(q) && - (nextIndex.get(q) ?? 0) < high - ) { - schedule(q, high); - } - } - } + if (!accounts.length) { + providerCtx.stopped = true; + return; + } + + providerCtx.count += accounts.length; - if (!stopped.has(p)) { - const target = Math.max(high, nextIndex.get(p) ?? 0); - schedule(p, target); + const next = index + 1; + providerCtx.index = next; + + if (next > maxGroupIndex) { + maxGroupIndex = next; + for (const [q, qCtx] of providerContexts) { + if ( + !qCtx.stopped && + !qCtx.running && + qCtx.index < maxGroupIndex + ) { + schedule(q, maxGroupIndex); + } } } - } catch { - inFlight.delete(p); - stopped.add(p); + } catch (err) { + providerCtx.stopped = true; + console.error(err); + } finally { + providerCtx.running = undefined; + if (!providerCtx.stopped) { + const target = Math.max(maxGroupIndex, providerCtx.index); + schedule(p, target); + } } })(); - - inFlight.set(p, run); }; for (const p of providers) { schedule(p, 0); } - while (inFlight.size > 0) { - await Promise.race(inFlight.values()); + while ([...providerContexts.values()].some((ctx) => Boolean(ctx.running))) { + const racers = [...providerContexts.values()] + .map((c) => c.running) + .filter(Boolean) as Promise[]; + await Promise.race(racers); } await this.alignGroups(); - const out: Record[]> = {}; + const discoveredAccounts: Record = {}; - for (const p of providers) { - out[p.providerType] = results[p.providerType] ?? []; + for (const [p, ctx] of providerContexts) { + const groupKey = p.snapId ?? 'Evm'; + discoveredAccounts[groupKey] = ctx.count; } - return out; + return discoveredAccounts; } } diff --git a/packages/multichain-account-service/src/providers/BaseAccountProvider.ts b/packages/multichain-account-service/src/providers/BaseAccountProvider.ts index 845eadb3c5f..54000c98279 100644 --- a/packages/multichain-account-service/src/providers/BaseAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BaseAccountProvider.ts @@ -42,14 +42,8 @@ export abstract class BaseAccountProvider { protected readonly messenger: MultichainAccountServiceMessenger; - readonly providerType: AccountProviderType; - - constructor( - messenger: MultichainAccountServiceMessenger, - providerType: AccountProviderType, - ) { + constructor(messenger: MultichainAccountServiceMessenger) { this.messenger = messenger; - this.providerType = providerType; } #getAccounts( diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 5da8ec46a73..37b6a2e120f 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -31,10 +31,6 @@ function assertInternalAccountExists( } export class EvmAccountProvider extends BaseAccountProvider { - constructor(messenger: MultichainAccountServiceMessenger) { - super(messenger, AccountProviderType.Evm); - } - isAccountCompatible(account: Bip44Account): boolean { return ( account.type === EthAccountType.Eoa && diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index 4d46b2de04b..7d8a1cb783d 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -15,12 +15,8 @@ export type RestrictedSnapKeyringCreateAccount = ( export abstract class SnapAccountProvider extends BaseAccountProvider { readonly snapId: SnapId; - constructor( - snapId: SnapId, - messenger: MultichainAccountServiceMessenger, - providerType: AccountProviderType, - ) { - super(messenger, providerType); + constructor(snapId: SnapId, messenger: MultichainAccountServiceMessenger) { + super(messenger); this.snapId = snapId; } diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 8c84795979d..094a7c6e082 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -16,11 +16,7 @@ export class SolAccountProvider extends SnapAccountProvider { static SOLANA_SNAP_ID = 'npm:@metamask/solana-wallet-snap' as SnapId; constructor(messenger: MultichainAccountServiceMessenger) { - super( - SolAccountProvider.SOLANA_SNAP_ID, - messenger, - AccountProviderType.Solana, - ); + super(SolAccountProvider.SOLANA_SNAP_ID, messenger); } isAccountCompatible(account: Bip44Account): boolean { From 861acd252409594e00a37fb4ba01e54dc10941be Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 18:29:39 -0400 Subject: [PATCH 13/77] fix: use type guard to narrow provider type --- .../src/MultichainAccountWallet.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index e2fc7182f65..7e6573cc9d1 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -18,6 +18,8 @@ import { } from '@metamask/keyring-api'; import { MultichainAccountGroup } from './MultichainAccountGroup'; +import { Bip44AccountProvider } from './types'; +import { isSnapAccountProvider } from './providers'; /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per @@ -366,7 +368,7 @@ export class MultichainAccountWallet< async discoverAndCreateAccounts(): Promise> { const providers = this.#providers; const providerContexts = new Map< - Bip44Provider, + Bip44AccountProvider, { stopped: boolean; running?: Promise; @@ -381,7 +383,7 @@ export class MultichainAccountWallet< let maxGroupIndex = 0; - const schedule = (p: Bip44Provider, index: number) => { + const schedule = (p: Bip44AccountProvider, index: number) => { const providerCtx = providerContexts.get(p); if (!providerCtx) { throw new Error(`Provider ${p} not found in providerContexts`); @@ -451,7 +453,7 @@ export class MultichainAccountWallet< const discoveredAccounts: Record = {}; for (const [p, ctx] of providerContexts) { - const groupKey = p.snapId ?? 'Evm'; + const groupKey = isSnapAccountProvider(p) ? p.snapId : 'Evm'; discoveredAccounts[groupKey] = ctx.count; } From f7fbe26d29d10e2419f03dbf8a40f66f99f586fc Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 18:41:39 -0400 Subject: [PATCH 14/77] fix: lint fix --- .../multichain-account-service/src/MultichainAccountWallet.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 7e6573cc9d1..1cf31002927 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -18,7 +18,6 @@ import { } from '@metamask/keyring-api'; import { MultichainAccountGroup } from './MultichainAccountGroup'; -import { Bip44AccountProvider } from './types'; import { isSnapAccountProvider } from './providers'; /** From 9ed9bc30d4ff246f8a9ab7c61960649927cc6b38 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 Aug 2025 22:42:52 -0400 Subject: [PATCH 15/77] feat: add discovery for solana --- .../src/providers/SolAccountProvider.ts | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index f84c07d3655..a0b925a531b 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -1,12 +1,16 @@ import { type Bip44Account } from '@metamask/account-api'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; +import { SolScope } from '@metamask/keyring-api'; import { KeyringAccountEntropyTypeOption, SolAccountType, } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; +import { KeyringClient } from '@metamask/keyring-snap-client'; import type { SnapId } from '@metamask/snaps-sdk'; +import { HandlerType } from '@metamask/snaps-utils'; +import type { Json, JsonRpcRequest } from '@metamask/utils'; import type { MultichainAccountServiceMessenger } from 'src/types'; import { assertAreBip44Accounts } from './BaseBip44AccountProvider'; @@ -15,8 +19,30 @@ import { SnapAccountProvider } from './SnapAccountProvider'; export class SolAccountProvider extends SnapAccountProvider { static SOLANA_SNAP_ID = 'npm:@metamask/solana-wallet-snap' as SnapId; + readonly #client: KeyringClient; + constructor(messenger: MultichainAccountServiceMessenger) { super(SolAccountProvider.SOLANA_SNAP_ID, messenger); + this.#client = this.#getKeyringClientFromSnapId( + SolAccountProvider.SOLANA_SNAP_ID, + ); + } + + #getKeyringClientFromSnapId(snapId: string): KeyringClient { + return new KeyringClient({ + send: async (request: JsonRpcRequest) => { + const response = await this.messenger.call( + 'SnapController:handleRequest', + { + snapId: snapId as SnapId, + origin: 'metamask', + handler: HandlerType.OnKeyringRequest, + request, + }, + ); + return response as Promise; + }, + }); } isAccountCompatible(account: Bip44Account): boolean { @@ -60,10 +86,26 @@ export class SolAccountProvider extends SnapAccountProvider { return accounts; } - async discoverAndCreateAccounts(_: { + async discoverAndCreateAccounts({ + entropySource, + groupIndex, + }: { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - return []; // TODO: Implement account discovery. + const discoveredAccounts = await this.#client.discoverAccounts( + [SolScope.Mainnet], + entropySource, + groupIndex, + ); + + const createdAccounts = await Promise.all( + discoveredAccounts.map( + async (_account) => + await this.createAccounts({ entropySource, groupIndex }), + ), + ); + + return createdAccounts.flat(); } } From 3666ed1c935a65fd0dcebb6a4212e2e32c6a0282 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 30 Aug 2025 16:49:23 -0400 Subject: [PATCH 16/77] test: add evm provider tests --- .../src/providers/EvmAccountProvider.test.ts | 126 +++++++++++++++++- .../src/tests/accounts.ts | 2 +- .../src/tests/messenger.ts | 2 + 3 files changed, 124 insertions(+), 6 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 350afc4c58a..22001359896 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -1,12 +1,21 @@ import type { Messenger } from '@metamask/base-controller'; -import type { KeyringMetadata } from '@metamask/keyring-controller'; +import { EthAccountType, EthScope } from '@metamask/keyring-api'; +import { + KeyringTypes, + type KeyringMetadata, +} from '@metamask/keyring-controller'; import type { EthKeyring, InternalAccount, } from '@metamask/keyring-internal-api'; +import type { + AutoManagedNetworkClient, + CustomNetworkClientConfiguration, +} from '@metamask/network-controller'; import { EvmAccountProvider } from './EvmAccountProvider'; import { + ETH_EOA_METHODS, getMultichainAccountServiceMessenger, getRootMessenger, MOCK_HD_ACCOUNT_1, @@ -21,6 +30,7 @@ import type { MultichainAccountServiceEvents, } from '../types'; + class MockEthKeyring implements EthKeyring { readonly type = 'MockEthKeyring'; @@ -29,7 +39,7 @@ class MockEthKeyring implements EthKeyring { name: '', }; - readonly accounts: InternalAccount[]; + accounts: InternalAccount[]; constructor(accounts: InternalAccount[]) { this.accounts = accounts; @@ -56,6 +66,7 @@ class MockEthKeyring implements EthKeyring { MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) .withUuid() .withAddressSuffix(`${this.accounts.length}`) + .withGroupIndex(this.accounts.length) .get(), ); } @@ -64,6 +75,13 @@ class MockEthKeyring implements EthKeyring { .slice(newAccountsIndex) .map((account) => account.address); }); + + removeAccount = jest.fn().mockImplementation((address: string) => { + const index = this.accounts.findIndex((a) => a.address === address); + if (index >= 0) { + this.accounts.splice(index, 1); + } + }); } /** @@ -92,6 +110,7 @@ function setup({ keyring: MockEthKeyring; mocks: { getAccountByAddress: jest.Mock; + mockProviderRequest: jest.Mock; }; } { const keyring = new MockEthKeyring(accounts); @@ -106,6 +125,14 @@ function setup({ .mockImplementation((address: string) => keyring.accounts.find((account) => account.address === address), ); + + const mockProviderRequest = jest.fn().mockImplementation(({ method }) => { + if (method === 'eth_getTransactionCount') { + return '0x2'; + } + throw new Error(`Unknown method: ${method}`); + }); + messenger.registerActionHandler( 'AccountsController:getAccountByAddress', mockGetAccountByAddress, @@ -116,6 +143,24 @@ function setup({ async (_, operation) => operation({ keyring, metadata: keyring.metadata }), ); + messenger.registerActionHandler( + 'NetworkController:findNetworkClientIdByChainId', + () => 'mock-network-client-id', + ); + + messenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + () => { + const provider = { + request: mockProviderRequest, + }; + + return { + provider, + } as unknown as AutoManagedNetworkClient; + }, + ); + const provider = new EvmAccountProvider( getMultichainAccountServiceMessenger(messenger), ); @@ -126,6 +171,7 @@ function setup({ keyring, mocks: { getAccountByAddress: mockGetAccountByAddress, + mockProviderRequest, }, }; } @@ -223,17 +269,87 @@ describe('EvmAccountProvider', () => { ).rejects.toThrow('Internal account does not exist'); }); - it('discover accounts', async () => { + it('discover accounts at the next group index', async () => { const { provider } = setup({ - accounts: [], // No accounts by defaults, so we can discover them + accounts: [MOCK_HD_ACCOUNT_1], // There should always be one account by the time discovery is called. + }); + + const expectedAccount = { + id: expect.any(String), + address: `${MOCK_HD_ACCOUNT_1.address}1`, + options: { + entropy: { + id: MOCK_HD_KEYRING_1.metadata.id, + type: 'mnemonic', + groupIndex: 1, + derivationPath: '', + }, + }, + methods: [...ETH_EOA_METHODS], + scopes: [EthScope.Eoa], + type: EthAccountType.Eoa, + metadata: { + name: 'Account 1', + keyring: { type: KeyringTypes.hd }, + importTime: 0, + lastSelected: 0, + nameLastUpdatedAt: 0, + }, + }; + + // Discovery starts at index + 1 for EVM. + expect( + await provider.discoverAndCreateAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }), + ).toStrictEqual([expectedAccount]); + + expect(provider.getAccounts()).toStrictEqual([ + MOCK_HD_ACCOUNT_1, + expectedAccount, + ]); + }); + + it('removes discovered account if no transaction history is found', async () => { + const { provider, mocks } = setup({ + accounts: [MOCK_HD_ACCOUNT_1], }); - // TODO: Update this once we really implement the account discovery. + mocks.mockProviderRequest.mockReturnValue('0x0'); + + // Discovery starts at index + 1 for EVM. expect( await provider.discoverAndCreateAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, groupIndex: 0, }), ).toStrictEqual([]); + + expect(provider.getAccounts()).toStrictEqual([MOCK_HD_ACCOUNT_1]); + }); + + it('returns an existing account if it already exists', async () => { + const MOCK_ACCOUNT_2 = { + ...MOCK_HD_ACCOUNT_1, + address: '0x456', + options: { + entropy: { + ...MOCK_HD_ACCOUNT_1.options.entropy, + groupIndex: 1, + }, + }, + }; + const { provider } = setup({ + accounts: [MOCK_HD_ACCOUNT_1, MOCK_ACCOUNT_2], + }); + + // Discovery starts at index + 1 for EVM. + expect( + await provider.discoverAndCreateAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }), + ).toStrictEqual([MOCK_ACCOUNT_2]); }); }); diff --git a/packages/multichain-account-service/src/tests/accounts.ts b/packages/multichain-account-service/src/tests/accounts.ts index 25f57828632..0453b914fbc 100644 --- a/packages/multichain-account-service/src/tests/accounts.ts +++ b/packages/multichain-account-service/src/tests/accounts.ts @@ -18,7 +18,7 @@ import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import { v4 as uuid } from 'uuid'; -const ETH_EOA_METHODS = [ +export const ETH_EOA_METHODS = [ EthMethod.PersonalSign, EthMethod.Sign, EthMethod.SignTransaction, diff --git a/packages/multichain-account-service/src/tests/messenger.ts b/packages/multichain-account-service/src/tests/messenger.ts index 10c6b40861c..92922839293 100644 --- a/packages/multichain-account-service/src/tests/messenger.ts +++ b/packages/multichain-account-service/src/tests/messenger.ts @@ -43,6 +43,8 @@ export function getMultichainAccountServiceMessenger( 'SnapController:handleRequest', 'KeyringController:withKeyring', 'KeyringController:getState', + 'NetworkController:findNetworkClientIdByChainId', + 'NetworkController:getNetworkClientById', ], }); } From ec712e61b7fe8db3b846ef633a948d8cb5053dfe Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 30 Aug 2025 16:51:38 -0400 Subject: [PATCH 17/77] refactor: make accounts readonly again --- .../src/providers/EvmAccountProvider.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 22001359896..6b9dc276ed1 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -30,7 +30,6 @@ import type { MultichainAccountServiceEvents, } from '../types'; - class MockEthKeyring implements EthKeyring { readonly type = 'MockEthKeyring'; @@ -39,7 +38,7 @@ class MockEthKeyring implements EthKeyring { name: '', }; - accounts: InternalAccount[]; + readonly accounts: InternalAccount[]; constructor(accounts: InternalAccount[]) { this.accounts = accounts; From 0784bc3d4b8ec3d3505fb900b29a21d3aadfc0f5 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 16:04:49 -0400 Subject: [PATCH 18/77] refactor: simplify discoverAndCreateAccounts for solana --- .../src/providers/SolAccountProvider.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index a0b925a531b..48105bba30b 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -99,13 +99,11 @@ export class SolAccountProvider extends SnapAccountProvider { groupIndex, ); - const createdAccounts = await Promise.all( - discoveredAccounts.map( - async (_account) => - await this.createAccounts({ entropySource, groupIndex }), - ), - ); + // Solana Snap always returns one discovered account. + if (discoveredAccounts.length === 1) { + return this.createAccounts({ entropySource, groupIndex }); + } - return createdAccounts.flat(); + return Promise.resolve([]); } } From 1540fcf71d40550ee2bb675c42f758dda118c84d Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 16:05:28 -0400 Subject: [PATCH 19/77] test: add solana discovery tests --- .../src/providers/SolAccountProvider.test.ts | 58 +++++++++++++++---- .../src/tests/accounts.ts | 8 ++- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index cacfecc67a5..8271a380e02 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -14,6 +14,7 @@ import { MOCK_HD_ACCOUNT_1, MOCK_HD_KEYRING_1, MOCK_SOL_ACCOUNT_1, + MOCK_SOL_DISCOVERED_ACCOUNT_1, MockAccountBuilder, } from '../tests'; import type { @@ -239,17 +240,54 @@ describe('SolAccountProvider', () => { ).rejects.toThrow('Created account is not BIP-44 compatible'); }); - it('discover accounts', async () => { - const { provider } = setup({ - accounts: [], // No accounts by defaults, so we can discover them + it('discover accounts at a new group index creates an account', async () => { + const { provider, mocks } = setup({ + accounts: [], }); - // TODO: Update this once we really implement the account discovery. - expect( - await provider.discoverAndCreateAccounts({ - entropySource: MOCK_HD_KEYRING_1.metadata.id, - groupIndex: 0, - }), - ).toStrictEqual([]); + // Simulate one discovered account at the requested index. + mocks.handleRequest.mockReturnValue([MOCK_SOL_DISCOVERED_ACCOUNT_1]); + + const created = await provider.discoverAndCreateAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(created).toHaveLength(1); + // Ensure we did go through creation path + expect(mocks.keyring.createAccount).toHaveBeenCalled(); + // Provider should now expose one account (newly created) + expect(provider.getAccounts()).toHaveLength(1); + }); + + it('returns existing account if it already exists at index', async () => { + const { provider, mocks } = setup({ + accounts: [MOCK_SOL_ACCOUNT_1], + }); + + // Simulate one discovered account — should resolve to the existing one + mocks.handleRequest.mockReturnValue([MOCK_SOL_DISCOVERED_ACCOUNT_1]); + + const discovered = await provider.discoverAndCreateAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toStrictEqual([MOCK_SOL_ACCOUNT_1]); + }); + + it('should not return any accounts if no account is discovered', async () => { + const { provider, mocks } = setup({ + accounts: [], + }); + + mocks.handleRequest.mockReturnValue([]); + + const discovered = await provider.discoverAndCreateAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + + expect(discovered).toStrictEqual([]); }); }); diff --git a/packages/multichain-account-service/src/tests/accounts.ts b/packages/multichain-account-service/src/tests/accounts.ts index 0453b914fbc..b77d65cd713 100644 --- a/packages/multichain-account-service/src/tests/accounts.ts +++ b/packages/multichain-account-service/src/tests/accounts.ts @@ -1,7 +1,7 @@ /* eslint-disable jsdoc/require-jsdoc */ import type { Bip44Account } from '@metamask/account-api'; import { isBip44Account } from '@metamask/account-api'; -import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; +import type { DiscoveredAccount, EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { BtcAccountType, BtcMethod, @@ -132,6 +132,12 @@ export const MOCK_SOL_ACCOUNT_1: Bip44Account = { }, }; +export const MOCK_SOL_DISCOVERED_ACCOUNT_1: DiscoveredAccount = { + type: 'bip44', + scopes: [SolScope.Mainnet], + derivationPath: `m/44'/501'/0'/0'`, +}; + export const MOCK_BTC_P2WPKH_ACCOUNT_1: Bip44Account = { id: 'b0f030d8-e101-4b5a-a3dd-13f8ca8ec1db', type: BtcAccountType.P2wpkh, From 923a7338a3a771a7e10d69833a6a100128ef1c4c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 22:34:10 -0400 Subject: [PATCH 20/77] test: add discoverAndCreateAccounts tests for multichain account wallet class --- .../src/MultichainAccountWallet.test.ts | 105 ++++++++++++++++++ .../src/MultichainAccountWallet.ts | 29 +++-- .../multichain-account-service/src/types.ts | 6 + 3 files changed, 132 insertions(+), 8 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 1020347dd87..79d5e35c12e 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -498,4 +498,109 @@ describe('MultichainAccountWallet', () => { expect(providers[1].createAccounts).toHaveBeenCalledTimes(1); }); }); + + describe('discoverAndCreateAccounts', () => { + it('fast-forwards lagging providers to the highest group index', async () => { + const { wallet, providers } = setup({ + accounts: [[MOCK_HD_ACCOUNT_1], []], + }); + + // Fast provider: succeeds at indices 1,2 then stops at 3 + providers[0].discoverAndCreateAccounts + .mockImplementationOnce(() => + Promise.resolve([{} as unknown as Bip44Account]), + ) + .mockImplementationOnce(() => + Promise.resolve([{} as unknown as Bip44Account]), + ) + .mockImplementationOnce(() => Promise.resolve([])); + + // Slow provider: first call (index 0) resolves on a later tick, then it should be + // rescheduled directly at index 3 (the high-water) and stop there + providers[1].discoverAndCreateAccounts + .mockImplementationOnce( + () => + new Promise((resolve) => + setTimeout( + () => resolve([{} as unknown as Bip44Account]), + 100, + ), + ), + ) + .mockImplementationOnce(() => Promise.resolve([])); + + // Avoid side-effects from alignment for this orchestrator behavior test + jest.spyOn(wallet, 'alignGroups').mockResolvedValue(undefined); + + await wallet.discoverAndCreateAccounts(); + + // Assert call order per provider shows skipping ahead + const fastIndices = Array.from( + providers[0].discoverAndCreateAccounts.mock.calls, + ).map((c) => Number(c[0].groupIndex)); + expect(fastIndices).toStrictEqual([0, 1, 2]); + + const slowIndices = Array.from( + providers[1].discoverAndCreateAccounts.mock.calls, + ).map((c) => Number(c[0].groupIndex)); + expect(slowIndices).toStrictEqual([0, 2]); + }); + + it('stops scheduling a provider when it returns no accounts', async () => { + const { wallet, providers } = setup({ + accounts: [[MOCK_HD_ACCOUNT_1], []], + }); + + // p1 finds one at 0 then stops at 1 + providers[0].discoverAndCreateAccounts + .mockImplementationOnce(() => + Promise.resolve([{} as unknown as Bip44Account]), + ) + .mockImplementationOnce(() => Promise.resolve([])); + + // p2 stops immediately at 0 + providers[1].discoverAndCreateAccounts.mockImplementationOnce(() => + Promise.resolve([]), + ); + + jest.spyOn(wallet, 'alignGroups').mockResolvedValue(undefined); + + await wallet.discoverAndCreateAccounts(); + + expect(providers[0].discoverAndCreateAccounts).toHaveBeenCalledTimes(2); + expect(providers[1].discoverAndCreateAccounts).toHaveBeenCalledTimes(1); + }); + + it('marks a provider stopped on error and does not reschedule it', async () => { + const { wallet, providers } = setup({ + accounts: [[MOCK_HD_ACCOUNT_1], []], + }); + + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + jest.spyOn(wallet, 'alignGroups').mockResolvedValue(undefined); + + // First provider throws on its first step + providers[0].discoverAndCreateAccounts.mockImplementationOnce(() => + Promise.reject(new Error('Failed to discover accounts')), + ); + // Second provider stops immediately + providers[1].discoverAndCreateAccounts.mockImplementationOnce(() => + Promise.resolve([]), + ); + + await wallet.discoverAndCreateAccounts(); + + // Thrown provider should have been called once and not rescheduled + expect(providers[0].discoverAndCreateAccounts).toHaveBeenCalledTimes(1); + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)); + expect((consoleSpy.mock.calls[0][0] as Error).message).toBe( + 'Failed to discover accounts', + ); + + // Other provider proceeds normally + expect(providers[1].discoverAndCreateAccounts).toHaveBeenCalledTimes(1); + + consoleSpy.mockRestore(); + }); + }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 1cf31002927..1704310b6e5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -19,6 +19,7 @@ import { import { MultichainAccountGroup } from './MultichainAccountGroup'; import { isSnapAccountProvider } from './providers'; +import type { Bip44AccountProvider } from './types'; /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per @@ -383,14 +384,13 @@ export class MultichainAccountWallet< let maxGroupIndex = 0; const schedule = (p: Bip44AccountProvider, index: number) => { - const providerCtx = providerContexts.get(p); - if (!providerCtx) { - throw new Error(`Provider ${p} not found in providerContexts`); - } - - if (providerCtx.stopped || providerCtx.running) { - return; - } + // Ok to cast here because we know that the ctx will always be set. + const providerCtx = providerContexts.get(p) as { + stopped: boolean; + running?: Promise; + index: number; + count: number; + }; providerCtx.index = index; @@ -414,6 +414,14 @@ export class MultichainAccountWallet< if (next > maxGroupIndex) { maxGroupIndex = next; for (const [q, qCtx] of providerContexts) { + /* istanbul ignore next + * Reason: This branch triggers only when a lagging provider is + * momentarily "not running" exactly as another provider advances + * the high group index. Because the coordinator self‑reschedules in + * `finally`, tests cannot reliably create this timing window without + * racy sleeps. We keep this path for robustness and exclude it from + * coverage. + */ if ( !qCtx.stopped && !qCtx.running && @@ -452,6 +460,11 @@ export class MultichainAccountWallet< const discoveredAccounts: Record = {}; for (const [p, ctx] of providerContexts) { + /* istanbul ignore next + * In production, Snap providers exist and hit the `isSnapAccountProvider` + * branch. Unit tests here mock generic providers only, so that path is + * not exercised. + */ const groupKey = isSnapAccountProvider(p) ? p.snapId : 'Evm'; discoveredAccounts[groupKey] = ctx.count; } diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 889e8b7e759..4e7f2cbf09f 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -1,3 +1,4 @@ +import type { AccountProvider, Bip44Account } from '@metamask/account-api'; import type { AccountsControllerAccountAddedEvent, AccountsControllerAccountRemovedEvent, @@ -6,6 +7,7 @@ import type { AccountsControllerListMultichainAccountsAction, } from '@metamask/accounts-controller'; import type { RestrictedMessenger } from '@metamask/base-controller'; +import type { KeyringAccount } from '@metamask/keyring-api'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, @@ -128,3 +130,7 @@ export type MultichainAccountServiceMessenger = RestrictedMessenger< AllowedActions['type'], AllowedEvents['type'] >; + +export type Bip44AccountProvider = AccountProvider< + Bip44Account +>; From 54acf85c0d4f427c8812786a448596860a94db03 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 22:41:28 -0400 Subject: [PATCH 21/77] refactor: update mock providers to both start with no accounts since mock providers make no distinction between provider type, remove unnecessary assertions --- .../src/MultichainAccountWallet.test.ts | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 79d5e35c12e..d6a74fc0edd 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -502,21 +502,17 @@ describe('MultichainAccountWallet', () => { describe('discoverAndCreateAccounts', () => { it('fast-forwards lagging providers to the highest group index', async () => { const { wallet, providers } = setup({ - accounts: [[MOCK_HD_ACCOUNT_1], []], + accounts: [[], []], }); - // Fast provider: succeeds at indices 1,2 then stops at 3 + // Fast provider: succeeds at indices 0,1 then stops at 2 providers[0].discoverAndCreateAccounts - .mockImplementationOnce(() => - Promise.resolve([{} as unknown as Bip44Account]), - ) - .mockImplementationOnce(() => - Promise.resolve([{} as unknown as Bip44Account]), - ) + .mockImplementationOnce(() => Promise.resolve([{}])) + .mockImplementationOnce(() => Promise.resolve([{}])) .mockImplementationOnce(() => Promise.resolve([])); // Slow provider: first call (index 0) resolves on a later tick, then it should be - // rescheduled directly at index 3 (the high-water) and stop there + // rescheduled directly at index 2 (the high-water) and stop there providers[1].discoverAndCreateAccounts .mockImplementationOnce( () => @@ -551,14 +547,12 @@ describe('MultichainAccountWallet', () => { accounts: [[MOCK_HD_ACCOUNT_1], []], }); - // p1 finds one at 0 then stops at 1 + // First provider finds one at 0 then stops at 1 providers[0].discoverAndCreateAccounts - .mockImplementationOnce(() => - Promise.resolve([{} as unknown as Bip44Account]), - ) + .mockImplementationOnce(() => Promise.resolve([{}])) .mockImplementationOnce(() => Promise.resolve([])); - // p2 stops immediately at 0 + // Second provider stops immediately at 0 providers[1].discoverAndCreateAccounts.mockImplementationOnce(() => Promise.resolve([]), ); @@ -573,7 +567,7 @@ describe('MultichainAccountWallet', () => { it('marks a provider stopped on error and does not reschedule it', async () => { const { wallet, providers } = setup({ - accounts: [[MOCK_HD_ACCOUNT_1], []], + accounts: [[], []], }); const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); @@ -599,8 +593,6 @@ describe('MultichainAccountWallet', () => { // Other provider proceeds normally expect(providers[1].discoverAndCreateAccounts).toHaveBeenCalledTimes(1); - - consoleSpy.mockRestore(); }); }); }); From 61d2212d2799ca0fed3c10ccc6658ab5b3db0dee Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 22:43:43 -0400 Subject: [PATCH 22/77] chore: update JSdoc comment --- .../multichain-account-service/src/MultichainAccountWallet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 1704310b6e5..20264e487d5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -363,7 +363,7 @@ export class MultichainAccountWallet< * * NOTE: This method should only be called on a newly created wallet. * - * @returns The accounts for each provider. + * @returns The discoveredaccounts for each provider. */ async discoverAndCreateAccounts(): Promise> { const providers = this.#providers; From 4faeea16147f8dc28357d8f1fe2b3525f2502296 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 22:43:59 -0400 Subject: [PATCH 23/77] refactor: add spacing --- .../multichain-account-service/src/MultichainAccountWallet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 20264e487d5..7e3c581b746 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -363,7 +363,7 @@ export class MultichainAccountWallet< * * NOTE: This method should only be called on a newly created wallet. * - * @returns The discoveredaccounts for each provider. + * @returns The discovered accounts for each provider. */ async discoverAndCreateAccounts(): Promise> { const providers = this.#providers; From 8b0afe717dcda89ecd8944859bc1f4f190920801 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 22:49:32 -0400 Subject: [PATCH 24/77] chore: add changelog entries --- packages/multichain-account-service/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 21dcf8c507e..571fd716307 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `setBasicFunctionality` method to control providers state and trigger wallets alignment ([#6332](https://github.com/MetaMask/core/pull/6332)) - Add `AccountProviderWrapper` to handle Snap account providers behavior according to the basic functionality flag. +- Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6396](https://github.com/MetaMask/core/pull/6397)) + +- Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6396](https://github.com/MetaMask/core/pull/6397)) + ### Changed - Bump `@metamask/base-controller` from `^8.1.0` to `^8.2.0` ([#6355](https://github.com/MetaMask/core/pull/6355)) From 071b83f83a431e196f93cab1eeff323e15d1409a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 23:02:14 -0400 Subject: [PATCH 25/77] chore: lint fixes --- packages/multichain-account-service/CHANGELOG.md | 4 ++-- packages/multichain-account-service/src/tests/accounts.ts | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 571fd716307..2ffb1793c78 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -12,9 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `setBasicFunctionality` method to control providers state and trigger wallets alignment ([#6332](https://github.com/MetaMask/core/pull/6332)) - Add `AccountProviderWrapper` to handle Snap account providers behavior according to the basic functionality flag. -- Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6396](https://github.com/MetaMask/core/pull/6397)) +- Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) -- Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6396](https://github.com/MetaMask/core/pull/6397)) +- Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) ### Changed diff --git a/packages/multichain-account-service/src/tests/accounts.ts b/packages/multichain-account-service/src/tests/accounts.ts index b77d65cd713..d9944832cb4 100644 --- a/packages/multichain-account-service/src/tests/accounts.ts +++ b/packages/multichain-account-service/src/tests/accounts.ts @@ -1,7 +1,11 @@ /* eslint-disable jsdoc/require-jsdoc */ import type { Bip44Account } from '@metamask/account-api'; import { isBip44Account } from '@metamask/account-api'; -import type { DiscoveredAccount, EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; +import type { + DiscoveredAccount, + EntropySourceId, + KeyringAccount, +} from '@metamask/keyring-api'; import { BtcAccountType, BtcMethod, From e5d17b396b40865c26306c3743dcd2c1beb9cc4b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 23:09:21 -0400 Subject: [PATCH 26/77] fix: prettier fix --- packages/multichain-account-service/CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 2ffb1793c78..9ef691ee306 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -11,9 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `setBasicFunctionality` method to control providers state and trigger wallets alignment ([#6332](https://github.com/MetaMask/core/pull/6332)) - Add `AccountProviderWrapper` to handle Snap account providers behavior according to the basic functionality flag. - - Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) - - Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) ### Changed From 0d3a9178c10d2ada08b209b01437ffb7c1e8b7f5 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 23:29:53 -0400 Subject: [PATCH 27/77] feat: sync wallet before calling align groups --- .../multichain-account-service/src/MultichainAccountWallet.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 7e3c581b746..31dca82283c 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -455,6 +455,10 @@ export class MultichainAccountWallet< await Promise.race(racers); } + // Sync the wallet after discovery to ensure that the newly added accounts are added into their groups. + // We can potentially remove this if we know that this race condition is not an issue in practice. + this.sync(); + await this.alignGroups(); const discoveredAccounts: Record = {}; From f7d62ca242f75d04a5f1495def02b7a661760e9b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 31 Aug 2025 23:39:15 -0400 Subject: [PATCH 28/77] fix: update return type for keyring client\'s send action --- .../src/providers/SolAccountProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 48105bba30b..581bf4cbfeb 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -40,7 +40,7 @@ export class SolAccountProvider extends SnapAccountProvider { request, }, ); - return response as Promise; + return response as Json; }, }); } From 1de127671fc66feee5d720975594bf15e24c9c30 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 15:26:06 -0400 Subject: [PATCH 29/77] feat: relax withKeyring type to accept options --- .../multichain-account-service/src/types.ts | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 4e7f2cbf09f..345515b23a2 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -11,8 +11,10 @@ import type { KeyringAccount } from '@metamask/keyring-api'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, - KeyringControllerWithKeyringAction, + KeyringMetadata, + KeyringSelector, } from '@metamask/keyring-controller'; +import type { EthKeyring } from '@metamask/keyring-internal-api'; import type { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction, @@ -105,7 +107,7 @@ export type AllowedActions = | AccountsControllerGetAccountAction | AccountsControllerGetAccountByAddressAction | SnapControllerHandleSnapRequestAction - | KeyringControllerWithKeyringAction + | KeyringControllerWithKeyringWithOptionsAction | KeyringControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction | NetworkControllerFindNetworkClientIdByChainIdAction; @@ -134,3 +136,27 @@ export type MultichainAccountServiceMessenger = RestrictedMessenger< export type Bip44AccountProvider = AccountProvider< Bip44Account >; + +/** + * Locally widen the withKeyring action so the messenger accepts the optional options + * parameter (createIfMissing/createWithData) in addition to selector and operation. + */ +export type KeyringControllerWithKeyringWithOptionsAction = { + type: 'KeyringController:withKeyring'; + handler: < + SelectedKeyring extends EthKeyring = EthKeyring, + CallbackResult = void, + >( + selector: KeyringSelector, + operation: ({ + keyring, + metadata, + }: { + keyring: SelectedKeyring; + metadata: KeyringMetadata; + }) => Promise, + options?: + | { createIfMissing?: false } + | { createIfMissing: true; createWithData?: unknown }, + ) => Promise; +}; From 6467328cbf5f19169a809851f3e723346d7a90fe Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 15:26:43 -0400 Subject: [PATCH 30/77] feat: refactor discovery logic to not +1 groupIndex --- .../src/providers/EvmAccountProvider.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index a39c8513eb9..240432a1fa3 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -112,21 +112,17 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { }) { const provider = this.getEvmProvider(); const { entropySource, groupIndex } = opts; - // groupIndex starts as +1, because we already have one account in the associated keyring. - const actualGroupIndex = groupIndex + 1; const [address, didCreate] = await this.withKeyring< EthKeyring, [Hex, boolean] >({ id: entropySource }, async ({ keyring }) => { const existing = await keyring.getAccounts(); - if (actualGroupIndex < existing.length) { - return [existing[actualGroupIndex], false]; + if (groupIndex < existing.length) { + return [existing[groupIndex], false]; } - const need = actualGroupIndex - existing.length + 1; - const added = await keyring.addAccounts(need); - const target = added[added.length - 1]; - return [target, true]; + const [added] = await keyring.addAccounts(1); + return [added, true]; }); const countHex = (await provider.request({ @@ -135,7 +131,8 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { })) as Hex; const count = parseInt(countHex, 16); - if (count === 0 && didCreate) { + // We don't want to remove the account if it's the first one. + if (count === 0 && didCreate && groupIndex !== 0) { await this.withKeyring( { id: entropySource }, async ({ keyring }) => { From 00a649d7a1e4a775370d716fef43c4afdfdba5ea Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 15:27:12 -0400 Subject: [PATCH 31/77] test: update evm provider tests to not be expecting +1 on the groupIndex --- .../src/providers/EvmAccountProvider.test.ts | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 6b9dc276ed1..d343a5bed78 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -270,17 +270,17 @@ describe('EvmAccountProvider', () => { it('discover accounts at the next group index', async () => { const { provider } = setup({ - accounts: [MOCK_HD_ACCOUNT_1], // There should always be one account by the time discovery is called. + accounts: [], }); const expectedAccount = { id: expect.any(String), - address: `${MOCK_HD_ACCOUNT_1.address}1`, + address: `${MOCK_HD_ACCOUNT_1.address}0`, options: { entropy: { id: MOCK_HD_KEYRING_1.metadata.id, type: 'mnemonic', - groupIndex: 1, + groupIndex: 0, derivationPath: '', }, }, @@ -296,7 +296,6 @@ describe('EvmAccountProvider', () => { }, }; - // Discovery starts at index + 1 for EVM. expect( await provider.discoverAndCreateAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, @@ -304,10 +303,7 @@ describe('EvmAccountProvider', () => { }), ).toStrictEqual([expectedAccount]); - expect(provider.getAccounts()).toStrictEqual([ - MOCK_HD_ACCOUNT_1, - expectedAccount, - ]); + expect(provider.getAccounts()).toStrictEqual([expectedAccount]); }); it('removes discovered account if no transaction history is found', async () => { @@ -317,30 +313,21 @@ describe('EvmAccountProvider', () => { mocks.mockProviderRequest.mockReturnValue('0x0'); - // Discovery starts at index + 1 for EVM. expect( await provider.discoverAndCreateAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, - groupIndex: 0, + groupIndex: 1, }), ).toStrictEqual([]); + await Promise.resolve(); + expect(provider.getAccounts()).toStrictEqual([MOCK_HD_ACCOUNT_1]); }); it('returns an existing account if it already exists', async () => { - const MOCK_ACCOUNT_2 = { - ...MOCK_HD_ACCOUNT_1, - address: '0x456', - options: { - entropy: { - ...MOCK_HD_ACCOUNT_1.options.entropy, - groupIndex: 1, - }, - }, - }; const { provider } = setup({ - accounts: [MOCK_HD_ACCOUNT_1, MOCK_ACCOUNT_2], + accounts: [MOCK_HD_ACCOUNT_1], }); // Discovery starts at index + 1 for EVM. @@ -349,6 +336,6 @@ describe('EvmAccountProvider', () => { entropySource: MOCK_HD_KEYRING_1.metadata.id, groupIndex: 0, }), - ).toStrictEqual([MOCK_ACCOUNT_2]); + ).toStrictEqual([MOCK_HD_ACCOUNT_1]); }); }); From 1907177cc720a24cc5cc58ffcd4b134f0b32a297 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 15:27:28 -0400 Subject: [PATCH 32/77] feat: add createMultichainAccountWallet method --- .../src/MultichainAccountService.ts | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index c67e32c70dc..29e78587ca3 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -8,7 +8,8 @@ import type { AccountProvider, } from '@metamask/account-api'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; -import { KeyringTypes } from '@metamask/keyring-controller'; +import { KeyringMetadata, KeyringTypes } from '@metamask/keyring-controller'; +import type { EthKeyring } from '@metamask/keyring-internal-api'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; @@ -19,6 +20,8 @@ import { import { EvmAccountProvider } from './providers/EvmAccountProvider'; import { SolAccountProvider } from './providers/SolAccountProvider'; import type { MultichainAccountServiceMessenger } from './types'; +import { assert, Hex } from '@metamask/utils'; + export const serviceName = 'MultichainAccountService'; @@ -293,6 +296,58 @@ export class MultichainAccountService { return Array.from(this.#wallets.values()); } + /** + * Creates a new multichain account wallet with the given mnemonic. + * + * NOTE: This method should only be called in client code where a mutex lock is acquired. + * `discoverAndCreateAccounts` should be called after this method to discover and create accounts. + * + * @param options - Options. + * @param options.mnemonic - The mnemonic to use to create the new wallet. + * @returns The a tuple of the new multichain account wallet and the entropy source id. + */ + async createMultichainAccountWallet({ + mnemonic, + }: { + mnemonic: string; + }): Promise< + [MultichainAccountWallet>, EntropySourceId] + > { + // create a new wallet with the given mnemonic + const result = (await this.#messenger.call( + 'KeyringController:withKeyring', + // We intentionally use index -1 to create a new keyring. + { type: KeyringTypes.hd, index: -1 }, + async ({ + keyring, + metadata, + }: { + keyring: EthKeyring; + metadata: KeyringMetadata; + }) => { + const accounts = await keyring.getAccounts(); + return [accounts, metadata]; + }, + { createIfMissing: true, createWithData: { mnemonic } }, + )) as [Hex[], KeyringMetadata]; + + const [accounts, metadata] = result; + // Make sure the keyring has no accounts after creating it. + assert( + accounts.length === 0 && metadata.id, + `Expected keyring with no accounts`, + ); + + const wallet = new MultichainAccountWallet({ + providers: this.#providers, + entropySource: metadata.id, + }); + + this.#wallets.set(wallet.id, wallet); + + return [wallet, metadata.id]; + } + /** * Gets a reference to the multichain account group matching this entropy source * and a group index. From f63b37cb935e9aab2a2648944697f348f12e6754 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 15:48:34 -0400 Subject: [PATCH 33/77] test: add tests for createMultichainAccountWallet --- .../src/MultichainAccountService.test.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 4ccb187013b..d7117bb88a0 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -54,6 +54,7 @@ type Mocks = { KeyringController: { keyrings: KeyringObject[]; getState: jest.Mock; + withKeyring: jest.Mock; }; AccountsController: { listMultichainAccounts: jest.Mock; @@ -102,6 +103,7 @@ function setup({ KeyringController: { keyrings, getState: jest.fn(), + withKeyring: jest.fn(), }, AccountsController: { listMultichainAccounts: jest.fn(), @@ -120,6 +122,11 @@ function setup({ mocks.KeyringController.getState, ); + messenger.registerActionHandler( + 'KeyringController:withKeyring', + mocks.KeyringController.withKeyring, + ); + if (accounts) { mocks.AccountsController.listMultichainAccounts.mockImplementation( () => accounts, @@ -954,4 +961,48 @@ describe('MultichainAccountService', () => { expect(wrapper.isAccountCompatible(MOCK_HD_ACCOUNT_1)).toBe(false); }); }); + + describe('createMultichainAccountWallet', () => { + it('creates a multichain account wallet with MultichainAccountService:createMultichainAccountWallet', async () => { + const { mocks, service } = setup({ + accounts: [], + keyrings: [], + }); + + // Make the messenger withKeyring call invoke our operation so the service code runs + mocks.KeyringController.withKeyring.mockImplementationOnce( + async (_selector, op, _opts) => { + const keyring = { getAccounts: jest.fn().mockResolvedValue([]) }; + const metadata = { id: 'abc', type: KeyringTypes.hd }; + return op({ keyring, metadata }); + }, + ); + + const [wallet, entropySource] = + await service.createMultichainAccountWallet({ + mnemonic: 'test', + }); + + expect(wallet).toBeDefined(); + expect(entropySource).toBe('abc'); + }); + + it("throws an error if there's already an existing keyring with accounts", async () => { + const { service, mocks } = setup({ accounts: [], keyrings: [] }); + + mocks.KeyringController.withKeyring.mockImplementationOnce( + async (_selector, op, _opts) => { + const keyring = { + getAccounts: jest.fn().mockResolvedValue(['0xabc']), + }; + const metadata = { id: 'abc', type: KeyringTypes.hd }; + return op({ keyring, metadata }); + }, + ); + + await expect( + service.createMultichainAccountWallet({ mnemonic: 'test' }), + ).rejects.toThrow('Expected keyring with no accounts'); + }); + }); }); From eca202afd5ad6f21c7af84a105eefa62228d00e9 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 15:50:36 -0400 Subject: [PATCH 34/77] chore: update changelog --- packages/multichain-account-service/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 9ef691ee306..8cae8b24482 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `AccountProviderWrapper` to handle Snap account providers behavior according to the basic functionality flag. - Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) +- Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) ### Changed From b35ead85fe282b5a6a64a01fc15cf661eaf0f72c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 16:00:13 -0400 Subject: [PATCH 35/77] refactor: use if statement to avoid importing utils package --- .../src/MultichainAccountService.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 29e78587ca3..59e1fa70df6 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -8,8 +8,12 @@ import type { AccountProvider, } from '@metamask/account-api'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; -import { KeyringMetadata, KeyringTypes } from '@metamask/keyring-controller'; +import { + type KeyringMetadata, + KeyringTypes, +} from '@metamask/keyring-controller'; import type { EthKeyring } from '@metamask/keyring-internal-api'; +import { type Hex } from '@metamask/utils'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; @@ -20,8 +24,6 @@ import { import { EvmAccountProvider } from './providers/EvmAccountProvider'; import { SolAccountProvider } from './providers/SolAccountProvider'; import type { MultichainAccountServiceMessenger } from './types'; -import { assert, Hex } from '@metamask/utils'; - export const serviceName = 'MultichainAccountService'; @@ -332,11 +334,11 @@ export class MultichainAccountService { )) as [Hex[], KeyringMetadata]; const [accounts, metadata] = result; + // Make sure the keyring has no accounts after creating it. - assert( - accounts.length === 0 && metadata.id, - `Expected keyring with no accounts`, - ); + if (accounts.length > 0) { + throw new Error('Expected keyring with no accounts'); + } const wallet = new MultichainAccountWallet({ providers: this.#providers, From e52859e69379e2afb89447d3d1eddae413c36bdb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 16:05:41 -0400 Subject: [PATCH 36/77] feat: update messenger actions with new create multichain account wallet action --- packages/multichain-account-service/src/types.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 345515b23a2..09a21b31e68 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -76,6 +76,11 @@ export type MultichainAccountServiceGetIsAlignmentInProgressAction = { handler: MultichainAccountService['getIsAlignmentInProgress']; }; +export type MultichainAccountServiceCreateMultichainAccountWalletAction = { + type: `${typeof serviceName}:createMultichainAccountWallet`; + handler: MultichainAccountService['createMultichainAccountWallet']; +}; + /** * All actions that {@link MultichainAccountService} registers so that other * modules can call them. @@ -90,7 +95,8 @@ export type MultichainAccountServiceActions = | MultichainAccountServiceSetBasicFunctionalityAction | MultichainAccountServiceAlignWalletAction | MultichainAccountServiceAlignWalletsAction - | MultichainAccountServiceGetIsAlignmentInProgressAction; + | MultichainAccountServiceGetIsAlignmentInProgressAction + | MultichainAccountServiceCreateMultichainAccountWalletAction; /** * All events that {@link MultichainAccountService} publishes so that other modules From 6aab24b888aa1e17d7f4aa7ffbd5a31a3dcc2a4b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 16:06:01 -0400 Subject: [PATCH 37/77] feat: register action handler for createMultichainAccountWallet --- .../src/MultichainAccountService.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 59e1fa70df6..97470ab2746 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -129,6 +129,10 @@ export class MultichainAccountService { 'MultichainAccountService:getIsAlignmentInProgress', () => this.getIsAlignmentInProgress(), ); + this.#messenger.registerActionHandler( + 'MultichainAccountService:createMultichainAccountWallet', + (...args) => this.createMultichainAccountWallet(...args), + ); this.#messenger.subscribe('AccountsController:accountAdded', (account) => this.#handleOnAccountAdded(account), From ce178b20cdb47d536b89a271b38626fdfb217828 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 16:06:21 -0400 Subject: [PATCH 38/77] test: add test for createMultichainAccountWallet action --- .../src/MultichainAccountService.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index d7117bb88a0..62d46ff89ca 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -839,6 +839,26 @@ describe('MultichainAccountService', () => { expect(isInProgress).toBe(false); }); + + it('creates a multichain account wallet with MultichainAccountService:createMultichainAccountWallet', async () => { + const { messenger, mocks } = setup({ accounts: [], keyrings: [] }); + + mocks.KeyringController.withKeyring.mockImplementationOnce( + async (_selector, op, _opts) => { + const keyring = { getAccounts: jest.fn().mockResolvedValue([]) }; + const metadata = { id: 'abc', type: KeyringTypes.hd }; + return op({ keyring, metadata }); + }, + ); + + const [wallet, entropySource] = await messenger.call( + 'MultichainAccountService:createMultichainAccountWallet', + { mnemonic: 'test' }, + ); + + expect(wallet).toBeDefined(); + expect(entropySource).toBe('abc'); + }); }); describe('setBasicFunctionality', () => { From 146041eaf769098927e0883a9ddc0b8ea965ac65 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 16:09:13 -0400 Subject: [PATCH 39/77] chore: update changelog again --- packages/multichain-account-service/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index b26a3c7d807..34aa9ece5ba 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) +- Register action handler for `createMultichainAccountWallet` method ([#6397](https://github.com/MetaMask/core/pull/6397)) ### Changed From b3c5bbd296bb1afc5ba57b3cda149a9695b39278 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 1 Sep 2025 16:18:26 -0400 Subject: [PATCH 40/77] fix: update changelog entries to be under unreleased --- packages/multichain-account-service/CHANGELOG.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 34aa9ece5ba..b00b4178045 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -7,16 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) +- Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) +- Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) + - An action handler was also registered for this method so that it can be called from the clients. + ## [0.6.0] ### Added - Add `setBasicFunctionality` method to control providers state and trigger wallets alignment ([#6332](https://github.com/MetaMask/core/pull/6332)) - Add `AccountProviderWrapper` to handle Snap account providers behavior according to the basic functionality flag. -- Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) -- Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) -- Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) -- Register action handler for `createMultichainAccountWallet` method ([#6397](https://github.com/MetaMask/core/pull/6397)) ### Changed From fe0416283830b0f1bae3830feca28ae5eced192c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 2 Sep 2025 13:25:58 -0400 Subject: [PATCH 41/77] feat: apply code review --- .../multichain-account-service/CHANGELOG.md | 2 +- .../src/MultichainAccountGroup.ts | 16 +++-- .../src/MultichainAccountService.test.ts | 3 + .../src/MultichainAccountService.ts | 6 +- .../src/MultichainAccountWallet.test.ts | 33 ++++++--- .../src/MultichainAccountWallet.ts | 71 +++++++++++++------ .../src/providers/AccountProviderWrapper.ts | 4 ++ .../src/providers/BaseBip44AccountProvider.ts | 12 +++- .../src/providers/EvmAccountProvider.test.ts | 5 ++ .../src/providers/EvmAccountProvider.ts | 4 ++ .../src/providers/SolAccountProvider.test.ts | 16 +++-- .../src/providers/SolAccountProvider.ts | 4 ++ .../src/tests/providers.ts | 2 + .../multichain-account-service/src/types.ts | 6 -- 14 files changed, 131 insertions(+), 53 deletions(-) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index b00b4178045..32c0e8f2d03 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `discoverAndCreateAccounts` methods for Evm and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) +- Add `discoverAndCreateAccounts` methods for EVM and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) - An action handler was also registered for this method so that it can be called from the clients. diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index 55c257cc248..495199e1696 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -6,10 +6,10 @@ import { } from '@metamask/account-api'; import type { Bip44Account } from '@metamask/account-api'; import type { AccountSelector } from '@metamask/account-api'; -import type { AccountProvider } from '@metamask/account-api'; import { type KeyringAccount } from '@metamask/keyring-api'; import type { MultichainAccountWallet } from './MultichainAccountWallet'; +import type { NamedAccountProvider } from './providers'; /** * A multichain account group that holds multiple accounts. @@ -24,11 +24,17 @@ export class MultichainAccountGroup< readonly #groupIndex: number; - readonly #providers: AccountProvider[]; + readonly #providers: NamedAccountProvider[]; - readonly #providerToAccounts: Map, Account['id'][]>; + readonly #providerToAccounts: Map< + NamedAccountProvider, + Account['id'][] + >; - readonly #accountToProvider: Map>; + readonly #accountToProvider: Map< + Account['id'], + NamedAccountProvider + >; constructor({ groupIndex, @@ -37,7 +43,7 @@ export class MultichainAccountGroup< }: { groupIndex: number; wallet: MultichainAccountWallet; - providers: AccountProvider[]; + providers: NamedAccountProvider[]; }) { this.#id = toMultichainAccountGroupId(wallet.id, groupIndex); this.#groupIndex = groupIndex; diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 62d46ff89ca..bb711c5f246 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -111,6 +111,9 @@ function setup({ EvmAccountProvider: makeMockAccountProvider(), SolAccountProvider: makeMockAccountProvider(), }; + // Default provider names can be overridden per test using mockImplementation + mocks.EvmAccountProvider.getName.mockImplementation(() => 'EVM'); + mocks.SolAccountProvider.getName.mockImplementation(() => 'Solana'); mocks.KeyringController.getState.mockImplementation(() => ({ isUnlocked: true, diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 97470ab2746..79396093adf 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -5,7 +5,6 @@ import { import type { MultichainAccountWalletId, Bip44Account, - AccountProvider, } from '@metamask/account-api'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { @@ -17,6 +16,7 @@ import { type Hex } from '@metamask/utils'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; +import type { NamedAccountProvider } from './providers'; import { AccountProviderWrapper, isAccountProviderWrapper, @@ -32,7 +32,7 @@ export const serviceName = 'MultichainAccountService'; */ type MultichainAccountServiceOptions = { messenger: MultichainAccountServiceMessenger; - providers?: AccountProvider>[]; + providers?: NamedAccountProvider[]; }; /** Reverse mapping object used to map account IDs and their wallet/multichain account. */ @@ -47,7 +47,7 @@ type AccountContext> = { export class MultichainAccountService { readonly #messenger: MultichainAccountServiceMessenger; - readonly #providers: AccountProvider>[]; + readonly #providers: NamedAccountProvider[]; readonly #wallets: Map< MultichainAccountWalletId, diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index d6a74fc0edd..3d740f1f5dd 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -63,6 +63,12 @@ function setup({ } describe('MultichainAccountWallet', () => { + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + describe('constructor', () => { it('constructs a multichain account wallet', () => { const entropySource = MOCK_WALLET_1_ENTROPY_SOURCE; @@ -505,6 +511,9 @@ describe('MultichainAccountWallet', () => { accounts: [[], []], }); + providers[0].getName.mockImplementation(() => 'EVM'); + providers[1].getName.mockImplementation(() => 'Solana'); + // Fast provider: succeeds at indices 0,1 then stops at 2 providers[0].discoverAndCreateAccounts .mockImplementationOnce(() => Promise.resolve([{}])) @@ -512,23 +521,23 @@ describe('MultichainAccountWallet', () => { .mockImplementationOnce(() => Promise.resolve([])); // Slow provider: first call (index 0) resolves on a later tick, then it should be - // rescheduled directly at index 2 (the high-water) and stop there + // rescheduled directly at index 2 (the max group index) and stop there providers[1].discoverAndCreateAccounts .mockImplementationOnce( - () => - new Promise((resolve) => - setTimeout( - () => resolve([{} as unknown as Bip44Account]), - 100, - ), - ), + () => new Promise((resolve) => setTimeout(() => resolve([{}]), 100)), ) .mockImplementationOnce(() => Promise.resolve([])); // Avoid side-effects from alignment for this orchestrator behavior test jest.spyOn(wallet, 'alignGroups').mockResolvedValue(undefined); - await wallet.discoverAndCreateAccounts(); + jest.useFakeTimers(); + const discovery = wallet.discoverAndCreateAccounts(); + // Allow fast provider microtasks to run and advance maxGroupIndex first + await Promise.resolve(); + await Promise.resolve(); + jest.advanceTimersByTime(100); + await discovery; // Assert call order per provider shows skipping ahead const fastIndices = Array.from( @@ -547,6 +556,9 @@ describe('MultichainAccountWallet', () => { accounts: [[MOCK_HD_ACCOUNT_1], []], }); + providers[0].getName.mockImplementation(() => 'EVM'); + providers[1].getName.mockImplementation(() => 'Solana'); + // First provider finds one at 0 then stops at 1 providers[0].discoverAndCreateAccounts .mockImplementationOnce(() => Promise.resolve([{}])) @@ -570,6 +582,9 @@ describe('MultichainAccountWallet', () => { accounts: [[], []], }); + providers[0].getName.mockImplementation(() => 'EVM'); + providers[1].getName.mockImplementation(() => 'Solana'); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); jest.spyOn(wallet, 'alignGroups').mockResolvedValue(undefined); diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 31dca82283c..47f39c58bd5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -11,15 +11,13 @@ import type { MultichainAccountWallet as MultichainAccountWalletDefinition, } from '@metamask/account-api'; import type { AccountGroupId } from '@metamask/account-api'; -import type { AccountProvider } from '@metamask/account-api'; import { type EntropySourceId, type KeyringAccount, } from '@metamask/keyring-api'; import { MultichainAccountGroup } from './MultichainAccountGroup'; -import { isSnapAccountProvider } from './providers'; -import type { Bip44AccountProvider } from './types'; +import type { NamedAccountProvider } from './providers'; /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per @@ -31,7 +29,7 @@ export class MultichainAccountWallet< { readonly #id: MultichainAccountWalletId; - readonly #providers: AccountProvider[]; + readonly #providers: NamedAccountProvider[]; readonly #entropySource: EntropySourceId; @@ -43,7 +41,7 @@ export class MultichainAccountWallet< providers, entropySource, }: { - providers: AccountProvider[]; + providers: NamedAccountProvider[]; entropySource: EntropySourceId; }) { this.#id = toMultichainAccountWalletId(entropySource); @@ -368,31 +366,47 @@ export class MultichainAccountWallet< async discoverAndCreateAccounts(): Promise> { const providers = this.#providers; const providerContexts = new Map< - Bip44AccountProvider, + NamedAccountProvider, { stopped: boolean; running?: Promise; - index: number; + groupIndex: number; count: number; } >(); for (const p of providers) { - providerContexts.set(p, { stopped: false, index: 0, count: 0 }); + providerContexts.set(p, { stopped: false, groupIndex: 0, count: 0 }); } let maxGroupIndex = 0; - const schedule = (p: Bip44AccountProvider, index: number) => { + const schedule = async (p: NamedAccountProvider, index: number) => { // Ok to cast here because we know that the ctx will always be set. const providerCtx = providerContexts.get(p) as { stopped: boolean; running?: Promise; - index: number; + groupIndex: number; count: number; }; - providerCtx.index = index; + /* istanbul ignore next + * Reason: This guard triggers only if `schedule` is invoked for the same + * provider while a previous step is still pending. Because the + * coordinator self‑reschedules in `finally` and catch‑up scheduling checks + * `!qCtx.running`, creating this exact interleaving in unit tests without + * racy sleeps is brittle. We keep the guard for safety and exclude it from + * coverage. + */ + if (providerCtx.running) { + // Wait for current scheduled job before starting a new one. + await providerCtx.running.catch((error) => { + console.error(error); + }); + providerCtx.running = undefined; + } + + providerCtx.groupIndex = index; providerCtx.running = (async () => { try { @@ -401,6 +415,7 @@ export class MultichainAccountWallet< groupIndex: index, }); + // Stopping condition: Account discovery is halted if no accounts are returned by the provider. if (!accounts.length) { providerCtx.stopped = true; return; @@ -409,7 +424,7 @@ export class MultichainAccountWallet< providerCtx.count += accounts.length; const next = index + 1; - providerCtx.index = next; + providerCtx.groupIndex = next; if (next > maxGroupIndex) { maxGroupIndex = next; @@ -425,9 +440,9 @@ export class MultichainAccountWallet< if ( !qCtx.stopped && !qCtx.running && - qCtx.index < maxGroupIndex + qCtx.groupIndex < maxGroupIndex ) { - schedule(q, maxGroupIndex); + qCtx.running = schedule(q, maxGroupIndex); } } } @@ -437,23 +452,35 @@ export class MultichainAccountWallet< } finally { providerCtx.running = undefined; if (!providerCtx.stopped) { - const target = Math.max(maxGroupIndex, providerCtx.index); - schedule(p, target); + const target = Math.max(maxGroupIndex, providerCtx.groupIndex); + providerCtx.running = schedule(p, target); } } })(); }; + const currentGroupIndex = this.getNextGroupIndex(); + // Start discovery for all providers. for (const p of providers) { - schedule(p, 0); + const pCtx = providerContexts.get(p) as { + stopped: boolean; + running?: Promise; + groupIndex: number; + count: number; + }; + pCtx.running = schedule(p, currentGroupIndex); } - while ([...providerContexts.values()].some((ctx) => Boolean(ctx.running))) { - const racers = [...providerContexts.values()] + let racers: Promise[] = []; + do { + racers = [...providerContexts.values()] .map((c) => c.running) .filter(Boolean) as Promise[]; - await Promise.race(racers); - } + + if (racers.length) { + await Promise.race(racers); + } + } while (racers.length); // Sync the wallet after discovery to ensure that the newly added accounts are added into their groups. // We can potentially remove this if we know that this race condition is not an issue in practice. @@ -469,7 +496,7 @@ export class MultichainAccountWallet< * branch. Unit tests here mock generic providers only, so that path is * not exercised. */ - const groupKey = isSnapAccountProvider(p) ? p.snapId : 'Evm'; + const groupKey = p.getName(); discoveredAccounts[groupKey] = ctx.count; } diff --git a/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts b/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts index 4efea96c3fc..04605380660 100644 --- a/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts +++ b/packages/multichain-account-service/src/providers/AccountProviderWrapper.ts @@ -21,6 +21,10 @@ export class AccountProviderWrapper extends BaseBip44AccountProvider { this.provider = provider; } + override getName(): string { + return this.provider.getName(); + } + /** * Set the enabled state for this provider. * diff --git a/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts b/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts index 3f4ca0e82ad..8a46df92f73 100644 --- a/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts @@ -37,15 +37,21 @@ export function assertAreBip44Accounts( accounts.forEach(assertIsBip44Account); } -export abstract class BaseBip44AccountProvider - implements AccountProvider> -{ +export type NamedAccountProvider< + Account extends Bip44Account = Bip44Account, +> = AccountProvider & { + getName(): string; +}; + +export abstract class BaseBip44AccountProvider implements NamedAccountProvider { protected readonly messenger: MultichainAccountServiceMessenger; constructor(messenger: MultichainAccountServiceMessenger) { this.messenger = messenger; } + abstract getName(): string; + #getAccounts( filter: (account: KeyringAccount) => boolean = () => true, ): Bip44Account[] { diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index d343a5bed78..fca1e999e86 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -176,6 +176,11 @@ function setup({ } describe('EvmAccountProvider', () => { + it('getName returns EVM', () => { + const { provider } = setup({ accounts: [] }); + expect(provider.getName()).toBe('EVM'); + }); + it('gets accounts', () => { const accounts = [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2]; const { provider } = setup({ diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 240432a1fa3..117efb2319b 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -37,6 +37,10 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { ); } + getName(): string { + return 'EVM'; + } + /** * Get the Evm provider. * diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index 8271a380e02..6e82f587a87 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -23,6 +23,7 @@ import type { MultichainAccountServiceActions, MultichainAccountServiceEvents, } from '../types'; +import { AccountProviderWrapper } from './AccountProviderWrapper'; class MockSolanaKeyring { readonly type = 'MockSolanaKeyring'; @@ -99,7 +100,7 @@ function setup({ >; accounts?: InternalAccount[]; } = {}): { - provider: SolAccountProvider; + provider: AccountProviderWrapper; messenger: Messenger< MultichainAccountServiceActions | AllowedActions, MultichainAccountServiceEvents | AllowedEvents @@ -140,8 +141,10 @@ function setup({ }), ); - const provider = new SolAccountProvider( - getMultichainAccountServiceMessenger(messenger), + const multichainMessenger = getMultichainAccountServiceMessenger(messenger); + const provider = new AccountProviderWrapper( + multichainMessenger, + new SolAccountProvider(multichainMessenger), ); return { @@ -158,6 +161,11 @@ function setup({ } describe('SolAccountProvider', () => { + it('getName returns Solana', () => { + const { provider } = setup({ accounts: [] }); + expect(provider.getName()).toBe('Solana'); + }); + it('gets accounts', () => { const accounts = [MOCK_SOL_ACCOUNT_1]; const { provider } = setup({ @@ -276,7 +284,7 @@ describe('SolAccountProvider', () => { expect(discovered).toStrictEqual([MOCK_SOL_ACCOUNT_1]); }); - it('should not return any accounts if no account is discovered', async () => { + it('does not return any accounts if no account is discovered', async () => { const { provider, mocks } = setup({ accounts: [], }); diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 581bf4cbfeb..5b034857dab 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -28,6 +28,10 @@ export class SolAccountProvider extends SnapAccountProvider { ); } + getName(): string { + return 'Solana'; + } + #getKeyringClientFromSnapId(snapId: string): KeyringClient { return new KeyringClient({ send: async (request: JsonRpcRequest) => { diff --git a/packages/multichain-account-service/src/tests/providers.ts b/packages/multichain-account-service/src/tests/providers.ts index a8e05b76376..dfc81f3f7f1 100644 --- a/packages/multichain-account-service/src/tests/providers.ts +++ b/packages/multichain-account-service/src/tests/providers.ts @@ -11,6 +11,7 @@ export type MockAccountProvider = { createAccounts: jest.Mock; discoverAndCreateAccounts: jest.Mock; isAccountCompatible?: jest.Mock; + getName: jest.Mock; }; export function makeMockAccountProvider( @@ -23,6 +24,7 @@ export function makeMockAccountProvider( createAccounts: jest.fn(), discoverAndCreateAccounts: jest.fn(), isAccountCompatible: jest.fn(), + getName: jest.fn(), }; } diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 09a21b31e68..a474ca1555e 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -1,4 +1,3 @@ -import type { AccountProvider, Bip44Account } from '@metamask/account-api'; import type { AccountsControllerAccountAddedEvent, AccountsControllerAccountRemovedEvent, @@ -7,7 +6,6 @@ import type { AccountsControllerListMultichainAccountsAction, } from '@metamask/accounts-controller'; import type { RestrictedMessenger } from '@metamask/base-controller'; -import type { KeyringAccount } from '@metamask/keyring-api'; import type { KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, @@ -139,10 +137,6 @@ export type MultichainAccountServiceMessenger = RestrictedMessenger< AllowedEvents['type'] >; -export type Bip44AccountProvider = AccountProvider< - Bip44Account ->; - /** * Locally widen the withKeyring action so the messenger accepts the optional options * parameter (createIfMissing/createWithData) in addition to selector and operation. From d9e6afd563487e715323ad17cfbf2d85bcce5ab7 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 2 Sep 2025 13:33:53 -0400 Subject: [PATCH 42/77] chore: remove unneeded istanbul ignore --- .../src/MultichainAccountWallet.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 47f39c58bd5..245a7bff875 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -491,11 +491,6 @@ export class MultichainAccountWallet< const discoveredAccounts: Record = {}; for (const [p, ctx] of providerContexts) { - /* istanbul ignore next - * In production, Snap providers exist and hit the `isSnapAccountProvider` - * branch. Unit tests here mock generic providers only, so that path is - * not exercised. - */ const groupKey = p.getName(); discoveredAccounts[groupKey] = ctx.count; } From 80d10043164b3a863b4aa904d8888c75388d00a1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 2 Sep 2025 13:43:28 -0400 Subject: [PATCH 43/77] fix: fix import order --- .../src/providers/SolAccountProvider.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index 6e82f587a87..52720d5d62e 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -7,6 +7,7 @@ import type { InternalAccount, } from '@metamask/keyring-internal-api'; +import { AccountProviderWrapper } from './AccountProviderWrapper'; import { SolAccountProvider } from './SolAccountProvider'; import { getMultichainAccountServiceMessenger, @@ -23,7 +24,6 @@ import type { MultichainAccountServiceActions, MultichainAccountServiceEvents, } from '../types'; -import { AccountProviderWrapper } from './AccountProviderWrapper'; class MockSolanaKeyring { readonly type = 'MockSolanaKeyring'; From f8201277afa0f5c28b9e15e5cb6c4abdb6f99fa8 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 3 Sep 2025 12:06:07 -0400 Subject: [PATCH 44/77] feat: add getKeyringsByType action to messenger --- packages/multichain-account-service/src/types.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index a474ca1555e..141aa213198 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -7,6 +7,7 @@ import type { } from '@metamask/accounts-controller'; import type { RestrictedMessenger } from '@metamask/base-controller'; import type { + KeyringControllerGetKeyringsByTypeAction, KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, KeyringMetadata, @@ -114,7 +115,8 @@ export type AllowedActions = | KeyringControllerWithKeyringWithOptionsAction | KeyringControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction - | NetworkControllerFindNetworkClientIdByChainIdAction; + | NetworkControllerFindNetworkClientIdByChainIdAction + | KeyringControllerGetKeyringsByTypeAction; /** * All events published by other modules that {@link MultichainAccountService} From 0d8fd74e98e31b245b63746c826385cd5ac29b1d Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 3 Sep 2025 14:13:28 -0400 Subject: [PATCH 45/77] refactor: use addKeyring action and add logic to check for existing keyring from the supplied mnemonic to createMultichainAccountWallet --- .../multichain-account-service/package.json | 1 + .../src/MultichainAccountService.test.ts | 70 +++++++++++-------- .../src/MultichainAccountService.ts | 50 +++++++------ .../src/tests/accounts.ts | 3 + .../src/tests/messenger.ts | 2 + .../multichain-account-service/src/types.ts | 34 ++------- .../multichain-account-service/src/utils.ts | 21 ++++++ yarn.lock | 1 + 8 files changed, 97 insertions(+), 85 deletions(-) create mode 100644 packages/multichain-account-service/src/utils.ts diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index 8abc7d42262..9b304212f92 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -53,6 +53,7 @@ "@metamask/keyring-internal-api": "^8.1.0", "@metamask/keyring-snap-client": "^7.0.0", "@metamask/keyring-utils": "^3.1.0", + "@metamask/scure-bip39": "^2.1.1", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", "@metamask/superstruct": "^3.1.0" diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index bb711c5f246..9ca61fe1b35 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -14,6 +14,7 @@ import { MOCK_HARDWARE_ACCOUNT_1, MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2, + MOCK_MNEMONIC, MOCK_SNAP_ACCOUNT_1, MOCK_SNAP_ACCOUNT_2, MOCK_SOL_ACCOUNT_1, @@ -35,6 +36,7 @@ import type { MultichainAccountServiceEvents, MultichainAccountServiceMessenger, } from './types'; +import { convertMnemonicToWordlistIndices } from './utils'; // Mock providers. jest.mock('./providers/EvmAccountProvider', () => { @@ -54,7 +56,8 @@ type Mocks = { KeyringController: { keyrings: KeyringObject[]; getState: jest.Mock; - withKeyring: jest.Mock; + getKeyringsByType: jest.Mock; + addNewKeyring: jest.Mock; }; AccountsController: { listMultichainAccounts: jest.Mock; @@ -103,7 +106,8 @@ function setup({ KeyringController: { keyrings, getState: jest.fn(), - withKeyring: jest.fn(), + getKeyringsByType: jest.fn(), + addNewKeyring: jest.fn(), }, AccountsController: { listMultichainAccounts: jest.fn(), @@ -126,8 +130,13 @@ function setup({ ); messenger.registerActionHandler( - 'KeyringController:withKeyring', - mocks.KeyringController.withKeyring, + 'KeyringController:getKeyringsByType', + mocks.KeyringController.getKeyringsByType, + ); + + messenger.registerActionHandler( + 'KeyringController:addNewKeyring', + mocks.KeyringController.addNewKeyring, ); if (accounts) { @@ -846,17 +855,18 @@ describe('MultichainAccountService', () => { it('creates a multichain account wallet with MultichainAccountService:createMultichainAccountWallet', async () => { const { messenger, mocks } = setup({ accounts: [], keyrings: [] }); - mocks.KeyringController.withKeyring.mockImplementationOnce( - async (_selector, op, _opts) => { - const keyring = { getAccounts: jest.fn().mockResolvedValue([]) }; - const metadata = { id: 'abc', type: KeyringTypes.hd }; - return op({ keyring, metadata }); - }, + mocks.KeyringController.getKeyringsByType.mockImplementationOnce( + () => [], ); + mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ + id: 'abc', + name: '', + })); + const [wallet, entropySource] = await messenger.call( 'MultichainAccountService:createMultichainAccountWallet', - { mnemonic: 'test' }, + { mnemonic: MOCK_MNEMONIC }, ); expect(wallet).toBeDefined(); @@ -992,40 +1002,40 @@ describe('MultichainAccountService', () => { keyrings: [], }); - // Make the messenger withKeyring call invoke our operation so the service code runs - mocks.KeyringController.withKeyring.mockImplementationOnce( - async (_selector, op, _opts) => { - const keyring = { getAccounts: jest.fn().mockResolvedValue([]) }; - const metadata = { id: 'abc', type: KeyringTypes.hd }; - return op({ keyring, metadata }); - }, + mocks.KeyringController.getKeyringsByType.mockImplementationOnce( + () => [], ); + mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ + id: 'abc', + name: '', + })); + const [wallet, entropySource] = await service.createMultichainAccountWallet({ - mnemonic: 'test', + mnemonic: MOCK_MNEMONIC, }); expect(wallet).toBeDefined(); expect(entropySource).toBe('abc'); }); - it("throws an error if there's already an existing keyring with accounts", async () => { + it("throws an error if there's already an existing keyring from the same mnemonic", async () => { const { service, mocks } = setup({ accounts: [], keyrings: [] }); - mocks.KeyringController.withKeyring.mockImplementationOnce( - async (_selector, op, _opts) => { - const keyring = { - getAccounts: jest.fn().mockResolvedValue(['0xabc']), - }; - const metadata = { id: 'abc', type: KeyringTypes.hd }; - return op({ keyring, metadata }); + const convertedMnemonic = convertMnemonicToWordlistIndices(MOCK_MNEMONIC); + + mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ + { + mnemonic: convertedMnemonic, }, - ); + ]); await expect( - service.createMultichainAccountWallet({ mnemonic: 'test' }), - ).rejects.toThrow('Expected keyring with no accounts'); + service.createMultichainAccountWallet({ mnemonic: MOCK_MNEMONIC }), + ).rejects.toThrow( + 'This Secret Recovery Phrase has already been imported.', + ); }); }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 79396093adf..97f3b49116b 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -12,7 +12,6 @@ import { KeyringTypes, } from '@metamask/keyring-controller'; import type { EthKeyring } from '@metamask/keyring-internal-api'; -import { type Hex } from '@metamask/utils'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; @@ -24,6 +23,7 @@ import { import { EvmAccountProvider } from './providers/EvmAccountProvider'; import { SolAccountProvider } from './providers/SolAccountProvider'; import type { MultichainAccountServiceMessenger } from './types'; +import { convertEnglishWordlistIndicesToCodepoints } from './utils'; export const serviceName = 'MultichainAccountService'; @@ -319,39 +319,37 @@ export class MultichainAccountService { }): Promise< [MultichainAccountWallet>, EntropySourceId] > { - // create a new wallet with the given mnemonic - const result = (await this.#messenger.call( - 'KeyringController:withKeyring', - // We intentionally use index -1 to create a new keyring. - { type: KeyringTypes.hd, index: -1 }, - async ({ - keyring, - metadata, - }: { - keyring: EthKeyring; - metadata: KeyringMetadata; - }) => { - const accounts = await keyring.getAccounts(); - return [accounts, metadata]; - }, - { createIfMissing: true, createWithData: { mnemonic } }, - )) as [Hex[], KeyringMetadata]; - - const [accounts, metadata] = result; - - // Make sure the keyring has no accounts after creating it. - if (accounts.length > 0) { - throw new Error('Expected keyring with no accounts'); + const existingKeyrings = this.#messenger.call( + 'KeyringController:getKeyringsByType', + KeyringTypes.hd, + ) as (EthKeyring & { mnemonic: Uint8Array })[]; + + const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { + return ( + Buffer.from( + convertEnglishWordlistIndicesToCodepoints(keyring.mnemonic), + ).toString('utf8') === mnemonic + ); + }); + + if (alreadyHasImportedSrp) { + throw new Error('This Secret Recovery Phrase has already been imported.'); } + const result = (await this.#messenger.call( + 'KeyringController:addNewKeyring', + KeyringTypes.hd, + { mnemonic }, + )) as KeyringMetadata; + const wallet = new MultichainAccountWallet({ providers: this.#providers, - entropySource: metadata.id, + entropySource: result.id, }); this.#wallets.set(wallet.id, wallet); - return [wallet, metadata.id]; + return [wallet, result.id]; } /** diff --git a/packages/multichain-account-service/src/tests/accounts.ts b/packages/multichain-account-service/src/tests/accounts.ts index d9944832cb4..e4eb2d6d15c 100644 --- a/packages/multichain-account-service/src/tests/accounts.ts +++ b/packages/multichain-account-service/src/tests/accounts.ts @@ -22,6 +22,9 @@ import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import { v4 as uuid } from 'uuid'; +export const MOCK_MNEMONIC = + 'abandon ability able about above absent absorb abstract absurd abuse access accident'; + export const ETH_EOA_METHODS = [ EthMethod.PersonalSign, EthMethod.Sign, diff --git a/packages/multichain-account-service/src/tests/messenger.ts b/packages/multichain-account-service/src/tests/messenger.ts index 92922839293..d17df13763c 100644 --- a/packages/multichain-account-service/src/tests/messenger.ts +++ b/packages/multichain-account-service/src/tests/messenger.ts @@ -45,6 +45,8 @@ export function getMultichainAccountServiceMessenger( 'KeyringController:getState', 'NetworkController:findNetworkClientIdByChainId', 'NetworkController:getNetworkClientById', + 'KeyringController:getKeyringsByType', + 'KeyringController:addNewKeyring', ], }); } diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 141aa213198..e7673246b7a 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -7,13 +7,12 @@ import type { } from '@metamask/accounts-controller'; import type { RestrictedMessenger } from '@metamask/base-controller'; import type { + KeyringControllerAddNewKeyringAction, KeyringControllerGetKeyringsByTypeAction, KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, - KeyringMetadata, - KeyringSelector, + KeyringControllerWithKeyringAction, } from '@metamask/keyring-controller'; -import type { EthKeyring } from '@metamask/keyring-internal-api'; import type { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetNetworkClientByIdAction, @@ -112,11 +111,12 @@ export type AllowedActions = | AccountsControllerGetAccountAction | AccountsControllerGetAccountByAddressAction | SnapControllerHandleSnapRequestAction - | KeyringControllerWithKeyringWithOptionsAction + | KeyringControllerWithKeyringAction | KeyringControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction | NetworkControllerFindNetworkClientIdByChainIdAction - | KeyringControllerGetKeyringsByTypeAction; + | KeyringControllerGetKeyringsByTypeAction + | KeyringControllerAddNewKeyringAction; /** * All events published by other modules that {@link MultichainAccountService} @@ -138,27 +138,3 @@ export type MultichainAccountServiceMessenger = RestrictedMessenger< AllowedActions['type'], AllowedEvents['type'] >; - -/** - * Locally widen the withKeyring action so the messenger accepts the optional options - * parameter (createIfMissing/createWithData) in addition to selector and operation. - */ -export type KeyringControllerWithKeyringWithOptionsAction = { - type: 'KeyringController:withKeyring'; - handler: < - SelectedKeyring extends EthKeyring = EthKeyring, - CallbackResult = void, - >( - selector: KeyringSelector, - operation: ({ - keyring, - metadata, - }: { - keyring: SelectedKeyring; - metadata: KeyringMetadata; - }) => Promise, - options?: - | { createIfMissing?: false } - | { createIfMissing: true; createWithData?: unknown }, - ) => Promise; -}; diff --git a/packages/multichain-account-service/src/utils.ts b/packages/multichain-account-service/src/utils.ts new file mode 100644 index 00000000000..c23e1d8dd2a --- /dev/null +++ b/packages/multichain-account-service/src/utils.ts @@ -0,0 +1,21 @@ +import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english'; + +export const convertEnglishWordlistIndicesToCodepoints = ( + wordlistIndices: Uint8Array, +): Buffer => { + return Buffer.from( + Array.from(new Uint16Array(wordlistIndices.buffer)) + .map((i) => wordlist[i]) + .join(' '), + ); +}; + +export const convertMnemonicToWordlistIndices = ( + mnemonic: string, +): Uint8Array => { + const indices = mnemonic + .toString() + .split(' ') + .map((word) => wordlist.indexOf(word)); + return new Uint8Array(new Uint16Array(indices).buffer); +}; diff --git a/yarn.lock b/yarn.lock index 093469eb609..f6d04a3bada 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3798,6 +3798,7 @@ __metadata: "@metamask/keyring-snap-client": "npm:^7.0.0" "@metamask/keyring-utils": "npm:^3.1.0" "@metamask/providers": "npm:^22.1.0" + "@metamask/scure-bip39": "npm:^2.1.1" "@metamask/snaps-controllers": "npm:^14.0.1" "@metamask/snaps-sdk": "npm:^9.0.0" "@metamask/snaps-utils": "npm:^11.0.0" From 4becc364daea45bcb6266f2c9e1f2730d832d6e8 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 3 Sep 2025 14:14:21 -0400 Subject: [PATCH 46/77] chore: update JSDoc --- .../multichain-account-service/src/MultichainAccountService.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 97f3b49116b..9c15e71faa5 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -310,6 +310,7 @@ export class MultichainAccountService { * * @param options - Options. * @param options.mnemonic - The mnemonic to use to create the new wallet. + * @throws If the mnemonic has already been imported. * @returns The a tuple of the new multichain account wallet and the entropy source id. */ async createMultichainAccountWallet({ From 1baee79bc184a4e0207daece02c51e6a81e19eed Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 3 Sep 2025 16:31:59 -0400 Subject: [PATCH 47/77] chore: fix JSdoc --- .../src/providers/EvmAccountProvider.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 117efb2319b..707a1cf6f3f 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -103,7 +103,6 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { * Discover and create accounts for the Evm provider. * * NOTE: This method should only be called on a newly created wallet. - * There should be already one existing account on this associated entropy source. * * @param opts - The options for the discovery and creation of accounts. * @param opts.entropySource - The entropy source to use for the discovery and creation of accounts. From 0631293c27f62247a5a800df9f4cf0fd82167085 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 3 Sep 2025 16:41:35 -0400 Subject: [PATCH 48/77] fix: remove double Buffer.from --- .../src/MultichainAccountService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 9c15e71faa5..0596a77cd27 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -327,9 +327,9 @@ export class MultichainAccountService { const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { return ( - Buffer.from( - convertEnglishWordlistIndicesToCodepoints(keyring.mnemonic), - ).toString('utf8') === mnemonic + convertEnglishWordlistIndicesToCodepoints(keyring.mnemonic).toString( + 'utf8', + ) === mnemonic ); }); From ffe7ad68a7ca65c2df478ab540c10c4ddca89dd7 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 09:26:30 -0400 Subject: [PATCH 49/77] fix: return provider's running promise in schedule function to prevent hanging --- .../multichain-account-service/src/MultichainAccountWallet.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 245a7bff875..2b6fb301618 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -457,6 +457,8 @@ export class MultichainAccountWallet< } } })(); + + return providerCtx.running; }; const currentGroupIndex = this.getNextGroupIndex(); From e501f17c787d7a050a5537b966a28831c84762eb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 09:30:26 -0400 Subject: [PATCH 50/77] refactor: move provider discovery context type into types --- .../src/MultichainAccountWallet.ts | 22 ++++--------------- .../multichain-account-service/src/types.ts | 10 +++++++++ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 2b6fb301618..0fd1a2dd3e6 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -18,6 +18,7 @@ import { import { MultichainAccountGroup } from './MultichainAccountGroup'; import type { NamedAccountProvider } from './providers'; +import { ProviderDiscoveryContext } from './types'; /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per @@ -367,12 +368,7 @@ export class MultichainAccountWallet< const providers = this.#providers; const providerContexts = new Map< NamedAccountProvider, - { - stopped: boolean; - running?: Promise; - groupIndex: number; - count: number; - } + ProviderDiscoveryContext >(); for (const p of providers) { @@ -383,12 +379,7 @@ export class MultichainAccountWallet< const schedule = async (p: NamedAccountProvider, index: number) => { // Ok to cast here because we know that the ctx will always be set. - const providerCtx = providerContexts.get(p) as { - stopped: boolean; - running?: Promise; - groupIndex: number; - count: number; - }; + const providerCtx = providerContexts.get(p) as ProviderDiscoveryContext; /* istanbul ignore next * Reason: This guard triggers only if `schedule` is invoked for the same @@ -464,12 +455,7 @@ export class MultichainAccountWallet< const currentGroupIndex = this.getNextGroupIndex(); // Start discovery for all providers. for (const p of providers) { - const pCtx = providerContexts.get(p) as { - stopped: boolean; - running?: Promise; - groupIndex: number; - count: number; - }; + const pCtx = providerContexts.get(p) as ProviderDiscoveryContext; pCtx.running = schedule(p, currentGroupIndex); } diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index e7673246b7a..f818f49c5d4 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -138,3 +138,13 @@ export type MultichainAccountServiceMessenger = RestrictedMessenger< AllowedActions['type'], AllowedEvents['type'] >; + +/** + * The context for a provider discovery. + */ +export type ProviderDiscoveryContext = { + stopped: boolean; + running?: Promise; + groupIndex: number; + count: number; +}; From 1a5180225467b30ef31bd5247b46588722c989c3 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 09:50:35 -0400 Subject: [PATCH 51/77] fix: lint fix --- .../multichain-account-service/src/MultichainAccountWallet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 0fd1a2dd3e6..e63375805b4 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -18,7 +18,7 @@ import { import { MultichainAccountGroup } from './MultichainAccountGroup'; import type { NamedAccountProvider } from './providers'; -import { ProviderDiscoveryContext } from './types'; +import type { ProviderDiscoveryContext } from './types'; /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per From 0ed635198b1e2665f02d338a83d78374ccd1e5a3 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Thu, 4 Sep 2025 19:12:33 +0200 Subject: [PATCH 52/77] chore(multichain-account-service): refactor and improvements around discovery logic (#6468) ## Explanation Improvements. ## References N/A ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --------- Co-authored-by: Hassan Malik <41640681+hmalik88@users.noreply.github.com> --- .../src/MultichainAccountWallet.ts | 157 +++++++----------- 1 file changed, 61 insertions(+), 96 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index e63375805b4..a0423158aee 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -18,7 +18,16 @@ import { import { MultichainAccountGroup } from './MultichainAccountGroup'; import type { NamedAccountProvider } from './providers'; -import type { ProviderDiscoveryContext } from './types'; + +/** + * The context for a provider discovery. + */ +export type AccountProviderDiscoveryContext = { + provider: NamedAccountProvider; + stopped: boolean; + groupIndex: number; + count: number; +}; /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per @@ -365,122 +374,78 @@ export class MultichainAccountWallet< * @returns The discovered accounts for each provider. */ async discoverAndCreateAccounts(): Promise> { - const providers = this.#providers; - const providerContexts = new Map< - NamedAccountProvider, - ProviderDiscoveryContext - >(); - - for (const p of providers) { - providerContexts.set(p, { stopped: false, groupIndex: 0, count: 0 }); - } + // Start with the next available group index (so we can resume the discovery + // from there). + let maxGroupIndex = this.getNextGroupIndex(); - let maxGroupIndex = 0; - - const schedule = async (p: NamedAccountProvider, index: number) => { - // Ok to cast here because we know that the ctx will always be set. - const providerCtx = providerContexts.get(p) as ProviderDiscoveryContext; - - /* istanbul ignore next - * Reason: This guard triggers only if `schedule` is invoked for the same - * provider while a previous step is still pending. Because the - * coordinator self‑reschedules in `finally` and catch‑up scheduling checks - * `!qCtx.running`, creating this exact interleaving in unit tests without - * racy sleeps is brittle. We keep the guard for safety and exclude it from - * coverage. - */ - if (providerCtx.running) { - // Wait for current scheduled job before starting a new one. - await providerCtx.running.catch((error) => { - console.error(error); - }); - providerCtx.running = undefined; - } + // One serialized loop per provider; all run concurrently + const runProviderDiscovery = async ( + context: AccountProviderDiscoveryContext, + ) => { + const step = (stepName: string) => + `[${context.provider.getName()}] Discover ${stepName} (groupIndex=${context.groupIndex})`; - providerCtx.groupIndex = index; + while (!context.stopped) { + // Fast‑forward to current high‑water mark + const targetGroupIndex = Math.max(context.groupIndex, maxGroupIndex); - providerCtx.running = (async () => { + console.log(step('STARTED')); + + let accounts: Bip44Account[] = []; try { - const accounts = await p.discoverAndCreateAccounts({ + accounts = await context.provider.discoverAndCreateAccounts({ entropySource: this.#entropySource, - groupIndex: index, + groupIndex: targetGroupIndex, }); + } catch (error) { + context.stopped = true; + console.error(error); + console.error(step('FAILED'), error); + break; + } - // Stopping condition: Account discovery is halted if no accounts are returned by the provider. - if (!accounts.length) { - providerCtx.stopped = true; - return; - } - - providerCtx.count += accounts.length; - - const next = index + 1; - providerCtx.groupIndex = next; - - if (next > maxGroupIndex) { - maxGroupIndex = next; - for (const [q, qCtx] of providerContexts) { - /* istanbul ignore next - * Reason: This branch triggers only when a lagging provider is - * momentarily "not running" exactly as another provider advances - * the high group index. Because the coordinator self‑reschedules in - * `finally`, tests cannot reliably create this timing window without - * racy sleeps. We keep this path for robustness and exclude it from - * coverage. - */ - if ( - !qCtx.stopped && - !qCtx.running && - qCtx.groupIndex < maxGroupIndex - ) { - qCtx.running = schedule(q, maxGroupIndex); - } - } - } - } catch (err) { - providerCtx.stopped = true; - console.error(err); - } finally { - providerCtx.running = undefined; - if (!providerCtx.stopped) { - const target = Math.max(maxGroupIndex, providerCtx.groupIndex); - providerCtx.running = schedule(p, target); - } + if (!accounts.length) { + console.log(step('STOPPED')); + context.stopped = true; + break; } - })(); - return providerCtx.running; - }; + console.log(step('SUCCEED')); - const currentGroupIndex = this.getNextGroupIndex(); - // Start discovery for all providers. - for (const p of providers) { - const pCtx = providerContexts.get(p) as ProviderDiscoveryContext; - pCtx.running = schedule(p, currentGroupIndex); - } + context.count += accounts.length; - let racers: Promise[] = []; - do { - racers = [...providerContexts.values()] - .map((c) => c.running) - .filter(Boolean) as Promise[]; + const nextGroupIndex = targetGroupIndex + 1; + context.groupIndex = nextGroupIndex; - if (racers.length) { - await Promise.race(racers); + if (nextGroupIndex > maxGroupIndex) { + maxGroupIndex = nextGroupIndex; + } } - } while (racers.length); + }; + + const providerContexts: AccountProviderDiscoveryContext[] = + this.#providers.map((provider) => ({ + provider, + stopped: false, + groupIndex: maxGroupIndex, + count: 0, + })); + + // Start discovery for each providers. + await Promise.all(providerContexts.map(runProviderDiscovery)); // Sync the wallet after discovery to ensure that the newly added accounts are added into their groups. // We can potentially remove this if we know that this race condition is not an issue in practice. this.sync(); + // Align missing accounts from group. This is required to create missing account from non-discovered + // indexes for some providers. await this.alignGroups(); const discoveredAccounts: Record = {}; - - for (const [p, ctx] of providerContexts) { - const groupKey = p.getName(); - discoveredAccounts[groupKey] = ctx.count; + for (const context of providerContexts) { + const providerName = context.provider.getName(); + discoveredAccounts[providerName] = context.count; } return discoveredAccounts; From 7f3c25752e2874a80221e9db1b7830e651b35f13 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 13:44:13 -0400 Subject: [PATCH 53/77] chore: remove provider context type from types --- packages/multichain-account-service/src/types.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index f818f49c5d4..e7673246b7a 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -138,13 +138,3 @@ export type MultichainAccountServiceMessenger = RestrictedMessenger< AllowedActions['type'], AllowedEvents['type'] >; - -/** - * The context for a provider discovery. - */ -export type ProviderDiscoveryContext = { - stopped: boolean; - running?: Promise; - groupIndex: number; - count: number; -}; From b4e21f2f13a136fe76af39cd87de4e218c249c15 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 13:44:29 -0400 Subject: [PATCH 54/77] chore: add breaking entry for multichain account service messenger --- packages/multichain-account-service/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index c143f0ce864..a7692e59554 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) - An action handler was also registered for this method so that it can be called from the clients. +- **BREAKING** Add additional allowed actions to the `MultichainAccountService` messenger + - `NetworkController:getNetworkClientById`, `NetworkController:findNetworkClientIdByChainId`, `KeyringController:getKeyringsByType` and `KeyringController:addNewKeyring` actions were added. ### Changed From 9f870c4365978bd295716fb47dc5e698a57a5e75 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 13:49:49 -0400 Subject: [PATCH 55/77] fix: update Evm to EVM in JSDocs --- .../src/providers/EvmAccountProvider.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 707a1cf6f3f..77a95f3268b 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -42,9 +42,9 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { } /** - * Get the Evm provider. + * Get the EVM provider. * - * @returns The Evm provider. + * @returns The EVM provider. */ getEvmProvider(): Provider { const networkClientId = this.messenger.call( @@ -100,14 +100,14 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { } /** - * Discover and create accounts for the Evm provider. + * Discover and create accounts for the EVM provider. * * NOTE: This method should only be called on a newly created wallet. * * @param opts - The options for the discovery and creation of accounts. * @param opts.entropySource - The entropy source to use for the discovery and creation of accounts. * @param opts.groupIndex - The index of the group to create the accounts for. - * @returns The accounts for the Evm provider. + * @returns The accounts for the EVM provider. */ async discoverAndCreateAccounts(opts: { entropySource: EntropySourceId; From 88809725d428adadaa2a03163ca38c5cf61e452e Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 13:50:36 -0400 Subject: [PATCH 56/77] chore: remove condition that is no longer true from JSDoc --- .../multichain-account-service/src/MultichainAccountWallet.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index a0423158aee..901c48ea182 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -369,8 +369,6 @@ export class MultichainAccountWallet< /** * Discover and create accounts for all providers. * - * NOTE: This method should only be called on a newly created wallet. - * * @returns The discovered accounts for each provider. */ async discoverAndCreateAccounts(): Promise> { From 55a39fbba3f93b40c5b57694ec73545ab50a83f7 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 13:51:19 -0400 Subject: [PATCH 57/77] refactor: only return newly created wallet instead of tuple in createMultichainAccountWallet since there is already a entropy source getter on the wallet --- .../src/MultichainAccountService.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 0596a77cd27..9217613aef0 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -317,9 +317,7 @@ export class MultichainAccountService { mnemonic, }: { mnemonic: string; - }): Promise< - [MultichainAccountWallet>, EntropySourceId] - > { + }): Promise>> { const existingKeyrings = this.#messenger.call( 'KeyringController:getKeyringsByType', KeyringTypes.hd, @@ -350,7 +348,7 @@ export class MultichainAccountService { this.#wallets.set(wallet.id, wallet); - return [wallet, result.id]; + return wallet; } /** From 2e60f1dde8a2f2acea0be68f055071bdd7bebd2a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 13:53:13 -0400 Subject: [PATCH 58/77] chore: rename utils to mnemonic --- .../src/MultichainAccountService.test.ts | 2 +- .../multichain-account-service/src/MultichainAccountService.ts | 2 +- .../multichain-account-service/src/{utils.ts => mnemonic.ts} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename packages/multichain-account-service/src/{utils.ts => mnemonic.ts} (100%) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 9ca61fe1b35..3780a573e29 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -5,6 +5,7 @@ import type { KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType, SolAccountType } from '@metamask/keyring-api'; import { KeyringTypes, type KeyringObject } from '@metamask/keyring-controller'; +import { convertMnemonicToWordlistIndices } from './mnemonic'; import { MultichainAccountService } from './MultichainAccountService'; import { AccountProviderWrapper } from './providers/AccountProviderWrapper'; import { EvmAccountProvider } from './providers/EvmAccountProvider'; @@ -36,7 +37,6 @@ import type { MultichainAccountServiceEvents, MultichainAccountServiceMessenger, } from './types'; -import { convertMnemonicToWordlistIndices } from './utils'; // Mock providers. jest.mock('./providers/EvmAccountProvider', () => { diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 9217613aef0..327c2b1d919 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -13,6 +13,7 @@ import { } from '@metamask/keyring-controller'; import type { EthKeyring } from '@metamask/keyring-internal-api'; +import { convertEnglishWordlistIndicesToCodepoints } from './mnemonic'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; import type { NamedAccountProvider } from './providers'; @@ -23,7 +24,6 @@ import { import { EvmAccountProvider } from './providers/EvmAccountProvider'; import { SolAccountProvider } from './providers/SolAccountProvider'; import type { MultichainAccountServiceMessenger } from './types'; -import { convertEnglishWordlistIndicesToCodepoints } from './utils'; export const serviceName = 'MultichainAccountService'; diff --git a/packages/multichain-account-service/src/utils.ts b/packages/multichain-account-service/src/mnemonic.ts similarity index 100% rename from packages/multichain-account-service/src/utils.ts rename to packages/multichain-account-service/src/mnemonic.ts From 1bde1e7aa11780d093f36956bf4d181fbe8538d0 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 14:00:20 -0400 Subject: [PATCH 59/77] fix: update service tests --- .../src/MultichainAccountService.test.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 3780a573e29..f114c4a45b2 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -864,13 +864,13 @@ describe('MultichainAccountService', () => { name: '', })); - const [wallet, entropySource] = await messenger.call( + const wallet = await messenger.call( 'MultichainAccountService:createMultichainAccountWallet', { mnemonic: MOCK_MNEMONIC }, ); expect(wallet).toBeDefined(); - expect(entropySource).toBe('abc'); + expect(wallet.entropySource).toBe('abc'); }); }); @@ -1011,13 +1011,12 @@ describe('MultichainAccountService', () => { name: '', })); - const [wallet, entropySource] = - await service.createMultichainAccountWallet({ - mnemonic: MOCK_MNEMONIC, - }); + const wallet = await service.createMultichainAccountWallet({ + mnemonic: MOCK_MNEMONIC, + }); expect(wallet).toBeDefined(); - expect(entropySource).toBe('abc'); + expect(wallet.entropySource).toBe('abc'); }); it("throws an error if there's already an existing keyring from the same mnemonic", async () => { From f13020b66c927d370bb7dcdcfeff842c369b2a01 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 15:42:49 -0400 Subject: [PATCH 60/77] feat: add chainId const --- .../src/providers/EvmAccountProvider.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 77a95f3268b..fe8d1519fbf 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -15,6 +15,8 @@ import { BaseBip44AccountProvider, } from './BaseBip44AccountProvider'; +const ETH_MAINNET_CHAIN_ID = '0x1'; + /** * Asserts an internal account exists. * @@ -49,7 +51,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { getEvmProvider(): Provider { const networkClientId = this.messenger.call( 'NetworkController:findNetworkClientIdByChainId', - '0x1', + ETH_MAINNET_CHAIN_ID, ); const { provider } = this.messenger.call( 'NetworkController:getNetworkClientById', From d0f403d3d9adc039cf6e40269b763aa23abade7a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 15:43:21 -0400 Subject: [PATCH 61/77] feat: use logger from utils in discoverAndCreateAccounts --- packages/multichain-account-service/package.json | 3 ++- .../src/MultichainAccountWallet.ts | 13 ++++++++----- yarn.lock | 3 ++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index 26214e082c4..4a1df7ff226 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -56,7 +56,8 @@ "@metamask/scure-bip39": "^2.1.1", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", - "@metamask/superstruct": "^3.1.0" + "@metamask/superstruct": "^3.1.0", + "@metamask/utils": "11.4.2" }, "devDependencies": { "@metamask/account-api": "^0.9.0", diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 901c48ea182..6a4fbfbbfa5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -15,6 +15,7 @@ import { type EntropySourceId, type KeyringAccount, } from '@metamask/keyring-api'; +import { createProjectLogger } from '@metamask/utils'; import { MultichainAccountGroup } from './MultichainAccountGroup'; import type { NamedAccountProvider } from './providers'; @@ -29,6 +30,8 @@ export type AccountProviderDiscoveryContext = { count: number; }; +const log = createProjectLogger('multichain-account-service'); + /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per * group index). @@ -380,14 +383,14 @@ export class MultichainAccountWallet< const runProviderDiscovery = async ( context: AccountProviderDiscoveryContext, ) => { - const step = (stepName: string) => + const message = (stepName: string) => `[${context.provider.getName()}] Discover ${stepName} (groupIndex=${context.groupIndex})`; while (!context.stopped) { // Fast‑forward to current high‑water mark const targetGroupIndex = Math.max(context.groupIndex, maxGroupIndex); - console.log(step('STARTED')); + log(message('STARTED')); let accounts: Bip44Account[] = []; try { @@ -398,17 +401,17 @@ export class MultichainAccountWallet< } catch (error) { context.stopped = true; console.error(error); - console.error(step('FAILED'), error); + log(message('FAILED'), error); break; } if (!accounts.length) { - console.log(step('STOPPED')); + log(message('STOPPED')); context.stopped = true; break; } - console.log(step('SUCCEED')); + log(message('SUCCEED')); context.count += accounts.length; diff --git a/yarn.lock b/yarn.lock index c25205e60e6..805371205bb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3833,6 +3833,7 @@ __metadata: "@metamask/snaps-sdk": "npm:^9.0.0" "@metamask/snaps-utils": "npm:^11.0.0" "@metamask/superstruct": "npm:^3.1.0" + "@metamask/utils": "npm:11.4.2" "@types/jest": "npm:^27.4.1" "@types/uuid": "npm:^8.3.0" deepmerge: "npm:^4.2.2" @@ -4811,7 +4812,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.4.0, @metamask/utils@npm:^11.4.2": +"@metamask/utils@npm:11.4.2, @metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.4.0, @metamask/utils@npm:^11.4.2": version: 11.4.2 resolution: "@metamask/utils@npm:11.4.2" dependencies: From aab0051dfe55111df39cb21c3cb113850f286613 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 15:45:15 -0400 Subject: [PATCH 62/77] fix: typos --- .../multichain-account-service/src/MultichainAccountWallet.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 6a4fbfbbfa5..1380b2b068f 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -384,7 +384,7 @@ export class MultichainAccountWallet< context: AccountProviderDiscoveryContext, ) => { const message = (stepName: string) => - `[${context.provider.getName()}] Discover ${stepName} (groupIndex=${context.groupIndex})`; + `[${context.provider.getName()}] Discovery ${stepName} (groupIndex=${context.groupIndex})`; while (!context.stopped) { // Fast‑forward to current high‑water mark @@ -411,7 +411,7 @@ export class MultichainAccountWallet< break; } - log(message('SUCCEED')); + log(message('SUCCEEDED')); context.count += accounts.length; From 78499344dee582b08c1bb6a0fadded3013d2addb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 15:52:40 -0400 Subject: [PATCH 63/77] fix: utils version --- packages/multichain-account-service/package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index 4a1df7ff226..d2afc7e06ba 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -57,7 +57,7 @@ "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", "@metamask/superstruct": "^3.1.0", - "@metamask/utils": "11.4.2" + "@metamask/utils": "^11.4.2" }, "devDependencies": { "@metamask/account-api": "^0.9.0", diff --git a/yarn.lock b/yarn.lock index 805371205bb..e6b239c047e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3833,7 +3833,7 @@ __metadata: "@metamask/snaps-sdk": "npm:^9.0.0" "@metamask/snaps-utils": "npm:^11.0.0" "@metamask/superstruct": "npm:^3.1.0" - "@metamask/utils": "npm:11.4.2" + "@metamask/utils": "npm:^11.4.2" "@types/jest": "npm:^27.4.1" "@types/uuid": "npm:^8.3.0" deepmerge: "npm:^4.2.2" @@ -4812,7 +4812,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/utils@npm:11.4.2, @metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.4.0, @metamask/utils@npm:^11.4.2": +"@metamask/utils@npm:^11.0.1, @metamask/utils@npm:^11.1.0, @metamask/utils@npm:^11.4.0, @metamask/utils@npm:^11.4.2": version: 11.4.2 resolution: "@metamask/utils@npm:11.4.2" dependencies: From f6832cf63d39b475db501d69a0346f5b8d492beb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 16:41:58 -0400 Subject: [PATCH 64/77] refactor: apply code review --- .../multichain-account-service/package.json | 1 + .../src/MultichainAccountService.ts | 9 +- .../src/providers/EvmAccountProvider.ts | 95 +++++++++++-------- yarn.lock | 15 +++ 4 files changed, 76 insertions(+), 44 deletions(-) diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index d2afc7e06ba..ed723b47863 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -63,6 +63,7 @@ "@metamask/account-api": "^0.9.0", "@metamask/accounts-controller": "^33.0.0", "@metamask/auto-changelog": "^3.4.4", + "@metamask/eth-hd-keyring": "^12.1.0", "@metamask/keyring-controller": "^23.0.0", "@metamask/providers": "^22.1.0", "@metamask/snaps-controllers": "^14.0.1", diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 327c2b1d919..d41ca220150 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -6,12 +6,12 @@ import type { MultichainAccountWalletId, Bip44Account, } from '@metamask/account-api'; +import type { HdKeyring } from '@metamask/eth-hd-keyring'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { type KeyringMetadata, KeyringTypes, } from '@metamask/keyring-controller'; -import type { EthKeyring } from '@metamask/keyring-internal-api'; import { convertEnglishWordlistIndicesToCodepoints } from './mnemonic'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; @@ -311,7 +311,7 @@ export class MultichainAccountService { * @param options - Options. * @param options.mnemonic - The mnemonic to use to create the new wallet. * @throws If the mnemonic has already been imported. - * @returns The a tuple of the new multichain account wallet and the entropy source id. + * @returns The new multichain account wallet. */ async createMultichainAccountWallet({ mnemonic, @@ -321,9 +321,12 @@ export class MultichainAccountService { const existingKeyrings = this.#messenger.call( 'KeyringController:getKeyringsByType', KeyringTypes.hd, - ) as (EthKeyring & { mnemonic: Uint8Array })[]; + ) as HdKeyring[]; const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { + if (!keyring.mnemonic) { + return false; + } return ( convertEnglishWordlistIndicesToCodepoints(keyring.mnemonic).toString( 'utf8', diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index fe8d1519fbf..132cbc9baff 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -60,33 +60,40 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return provider; } - async createAccounts({ + async createAccount({ entropySource, groupIndex, }: { entropySource: EntropySourceId; groupIndex: number; - }) { - const [address] = await this.withKeyring( + }): Promise<[Hex, boolean]> { + const result = await this.withKeyring( { id: entropySource }, async ({ keyring }) => { - const accounts = await keyring.getAccounts(); - if (groupIndex < accounts.length) { - // Nothing new to create, we just re-use the existing accounts here, - return [accounts[groupIndex]]; - } - - // For now, we don't allow for gap, so if we need to create a new - // account, this has to be the next one. - if (groupIndex !== accounts.length) { - throw new Error('Trying to create too many accounts'); + const existing = await keyring.getAccounts(); + if (groupIndex < existing.length) { + return [existing[groupIndex], false]; } - - // Create next account (and returns their addresses). - return await keyring.addAccounts(1); + const [added] = await keyring.addAccounts(1); + return [added, true]; }, ); + return result; + } + + async createAccounts({ + entropySource, + groupIndex, + }: { + entropySource: EntropySourceId; + groupIndex: number; + }) { + const [address] = await this.createAccount({ + entropySource, + groupIndex, + }); + const account = this.messenger.call( 'AccountsController:getAccountByAddress', address, @@ -104,8 +111,6 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { /** * Discover and create accounts for the EVM provider. * - * NOTE: This method should only be called on a newly created wallet. - * * @param opts - The options for the discovery and creation of accounts. * @param opts.entropySource - The entropy source to use for the discovery and creation of accounts. * @param opts.groupIndex - The index of the group to create the accounts for. @@ -118,32 +123,40 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { const provider = this.getEvmProvider(); const { entropySource, groupIndex } = opts; - const [address, didCreate] = await this.withKeyring< - EthKeyring, - [Hex, boolean] - >({ id: entropySource }, async ({ keyring }) => { - const existing = await keyring.getAccounts(); - if (groupIndex < existing.length) { - return [existing[groupIndex], false]; - } - const [added] = await keyring.addAccounts(1); - return [added, true]; + const [address, didCreate] = await this.createAccount({ + entropySource, + groupIndex, }); - const countHex = (await provider.request({ - method: 'eth_getTransactionCount', - params: [address, 'latest'], - })) as Hex; - const count = parseInt(countHex, 16); - // We don't want to remove the account if it's the first one. - if (count === 0 && didCreate && groupIndex !== 0) { - await this.withKeyring( - { id: entropySource }, - async ({ keyring }) => { - keyring.removeAccount?.(address); - }, - ); + const shouldCleanup = didCreate && groupIndex !== 0; + try { + const countHex = (await provider.request({ + method: 'eth_getTransactionCount', + params: [address, 'latest'], + })) as Hex; + const count = parseInt(countHex, 16); + + if (count === 0 && shouldCleanup) { + await this.withKeyring( + { id: entropySource }, + async ({ keyring }) => { + keyring.removeAccount?.(address); + }, + ); + return []; + } + } catch { + // If the RPC request fails and we just created this account for discovery, + // remove it to avoid leaving a dangling account. + if (shouldCleanup) { + await this.withKeyring( + { id: entropySource }, + async ({ keyring }) => { + keyring.removeAccount?.(address); + }, + ); + } return []; } diff --git a/yarn.lock b/yarn.lock index e6b239c047e..26ae3e25808 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3232,6 +3232,20 @@ __metadata: languageName: node linkType: hard +"@metamask/eth-hd-keyring@npm:^12.1.0": + version: 12.1.0 + resolution: "@metamask/eth-hd-keyring@npm:12.1.0" + dependencies: + "@ethereumjs/util": "npm:^9.1.0" + "@metamask/eth-sig-util": "npm:^8.2.0" + "@metamask/key-tree": "npm:^10.0.2" + "@metamask/scure-bip39": "npm:^2.1.1" + "@metamask/utils": "npm:^11.1.0" + ethereum-cryptography: "npm:^2.1.2" + checksum: 10/90664a9874f424155631e27407937831a7c9f9e1077b15d11b35fcde93e4d0769aeae6c6ac9a2d0984d6e31a0225f4640070ab69f795c5b81e8381859b072347 + languageName: node + linkType: hard + "@metamask/eth-json-rpc-filters@npm:^9.0.0": version: 9.0.0 resolution: "@metamask/eth-json-rpc-filters@npm:9.0.0" @@ -3821,6 +3835,7 @@ __metadata: "@metamask/accounts-controller": "npm:^33.0.0" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/base-controller": "npm:^8.3.0" + "@metamask/eth-hd-keyring": "npm:^12.1.0" "@metamask/eth-snap-keyring": "npm:^16.1.0" "@metamask/keyring-api": "npm:^20.1.0" "@metamask/keyring-controller": "npm:^23.0.0" From 46fc0d3c8def1ae15b18e8cbd121f62d9f3bba7f Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 16:44:24 -0400 Subject: [PATCH 65/77] fix: update test description --- .../src/MultichainAccountService.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index f114c4a45b2..2b9ff30c5eb 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -996,7 +996,7 @@ describe('MultichainAccountService', () => { }); describe('createMultichainAccountWallet', () => { - it('creates a multichain account wallet with MultichainAccountService:createMultichainAccountWallet', async () => { + it('creates a new multichain account wallet with the given mnemonic', async () => { const { mocks, service } = setup({ accounts: [], keyrings: [], From c63a2bd1f3c2ca99d070bf2ba476f260d9d9f7b8 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 18:10:22 -0400 Subject: [PATCH 66/77] feat: add throwOnGap param to createAccount --- .../src/providers/EvmAccountProvider.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 132cbc9baff..785a40b3bc6 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -60,12 +60,14 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return provider; } - async createAccount({ + async #createAccount({ entropySource, groupIndex, + throwOnGap = false, }: { entropySource: EntropySourceId; groupIndex: number; + throwOnGap?: boolean; }): Promise<[Hex, boolean]> { const result = await this.withKeyring( { id: entropySource }, @@ -74,6 +76,13 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { if (groupIndex < existing.length) { return [existing[groupIndex], false]; } + + // For now, we don't allow for gap, so if we need to create a new + // account, this has to be the next one. + if (groupIndex !== existing.length && throwOnGap) { + throw new Error('Trying to create too many accounts'); + } + const [added] = await keyring.addAccounts(1); return [added, true]; }, @@ -89,9 +98,10 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { entropySource: EntropySourceId; groupIndex: number; }) { - const [address] = await this.createAccount({ + const [address] = await this.#createAccount({ entropySource, groupIndex, + throwOnGap: true, }); const account = this.messenger.call( @@ -123,7 +133,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { const provider = this.getEvmProvider(); const { entropySource, groupIndex } = opts; - const [address, didCreate] = await this.createAccount({ + const [address, didCreate] = await this.#createAccount({ entropySource, groupIndex, }); From 6187f4e2fc081f8009ec4f78fa25a40ed4fa9056 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 18:22:27 -0400 Subject: [PATCH 67/77] refactor: apply code review --- .../src/MultichainAccountService.test.ts | 6 +-- .../src/providers/EvmAccountProvider.test.ts | 42 ++++++++++--------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 2b9ff30c5eb..ee7ed14024b 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -1002,9 +1002,9 @@ describe('MultichainAccountService', () => { keyrings: [], }); - mocks.KeyringController.getKeyringsByType.mockImplementationOnce( - () => [], - ); + mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ + {}, + ]); mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ id: 'abc', diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index fca1e999e86..942e6911228 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -279,26 +279,10 @@ describe('EvmAccountProvider', () => { }); const expectedAccount = { + ...MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withAddressSuffix('0') + .get(), id: expect.any(String), - address: `${MOCK_HD_ACCOUNT_1.address}0`, - options: { - entropy: { - id: MOCK_HD_KEYRING_1.metadata.id, - type: 'mnemonic', - groupIndex: 0, - derivationPath: '', - }, - }, - methods: [...ETH_EOA_METHODS], - scopes: [EthScope.Eoa], - type: EthAccountType.Eoa, - metadata: { - name: 'Account 1', - keyring: { type: KeyringTypes.hd }, - importTime: 0, - lastSelected: 0, - nameLastUpdatedAt: 0, - }, }; expect( @@ -330,12 +314,30 @@ describe('EvmAccountProvider', () => { expect(provider.getAccounts()).toStrictEqual([MOCK_HD_ACCOUNT_1]); }); + it('removes discovered account if RPC request fails', async () => { + const { provider, mocks } = setup({ + accounts: [MOCK_HD_ACCOUNT_1], + }); + + mocks.mockProviderRequest.mockImplementation(() => { + throw new Error('RPC request failed'); + }); + + expect( + await provider.discoverAndCreateAccounts({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 1, + }), + ).toStrictEqual([]); + + expect(provider.getAccounts()).toStrictEqual([MOCK_HD_ACCOUNT_1]); + }); + it('returns an existing account if it already exists', async () => { const { provider } = setup({ accounts: [MOCK_HD_ACCOUNT_1], }); - // Discovery starts at index + 1 for EVM. expect( await provider.discoverAndCreateAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, From 66aa686384d0a756181991685a485d40ca34463a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 18:24:59 -0400 Subject: [PATCH 68/77] feat: add type alias for discoverAndCreateAccounts return type --- .../src/MultichainAccountWallet.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 1380b2b068f..3c9386a4565 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -30,6 +30,10 @@ export type AccountProviderDiscoveryContext = { count: number; }; +export type AccountDiscoveryMetrics = { + [providerName: string]: number; +}; + const log = createProjectLogger('multichain-account-service'); /** @@ -374,7 +378,7 @@ export class MultichainAccountWallet< * * @returns The discovered accounts for each provider. */ - async discoverAndCreateAccounts(): Promise> { + async discoverAndCreateAccounts(): Promise { // Start with the next available group index (so we can resume the discovery // from there). let maxGroupIndex = this.getNextGroupIndex(); From 7230071d77b90f06f14d567fb78bd44b9e20a87a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 18:47:09 -0400 Subject: [PATCH 69/77] fix: remove unused vars --- .../src/providers/EvmAccountProvider.test.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 942e6911228..6ee151969d1 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -1,9 +1,5 @@ import type { Messenger } from '@metamask/base-controller'; -import { EthAccountType, EthScope } from '@metamask/keyring-api'; -import { - KeyringTypes, - type KeyringMetadata, -} from '@metamask/keyring-controller'; +import { type KeyringMetadata } from '@metamask/keyring-controller'; import type { EthKeyring, InternalAccount, @@ -15,7 +11,6 @@ import type { import { EvmAccountProvider } from './EvmAccountProvider'; import { - ETH_EOA_METHODS, getMultichainAccountServiceMessenger, getRootMessenger, MOCK_HD_ACCOUNT_1, From badf9f98ba88135a4d854b031ed2eae7c14210ac Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 4 Sep 2025 19:12:21 -0400 Subject: [PATCH 70/77] fix: change eth-hd-keyring version to be consistent --- packages/multichain-account-service/package.json | 2 +- yarn.lock | 16 +--------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index ed723b47863..8da38e85af3 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -63,7 +63,7 @@ "@metamask/account-api": "^0.9.0", "@metamask/accounts-controller": "^33.0.0", "@metamask/auto-changelog": "^3.4.4", - "@metamask/eth-hd-keyring": "^12.1.0", + "@metamask/eth-hd-keyring": "^12.0.0", "@metamask/keyring-controller": "^23.0.0", "@metamask/providers": "^22.1.0", "@metamask/snaps-controllers": "^14.0.1", diff --git a/yarn.lock b/yarn.lock index 26ae3e25808..0273ff7f1eb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3232,20 +3232,6 @@ __metadata: languageName: node linkType: hard -"@metamask/eth-hd-keyring@npm:^12.1.0": - version: 12.1.0 - resolution: "@metamask/eth-hd-keyring@npm:12.1.0" - dependencies: - "@ethereumjs/util": "npm:^9.1.0" - "@metamask/eth-sig-util": "npm:^8.2.0" - "@metamask/key-tree": "npm:^10.0.2" - "@metamask/scure-bip39": "npm:^2.1.1" - "@metamask/utils": "npm:^11.1.0" - ethereum-cryptography: "npm:^2.1.2" - checksum: 10/90664a9874f424155631e27407937831a7c9f9e1077b15d11b35fcde93e4d0769aeae6c6ac9a2d0984d6e31a0225f4640070ab69f795c5b81e8381859b072347 - languageName: node - linkType: hard - "@metamask/eth-json-rpc-filters@npm:^9.0.0": version: 9.0.0 resolution: "@metamask/eth-json-rpc-filters@npm:9.0.0" @@ -3835,7 +3821,7 @@ __metadata: "@metamask/accounts-controller": "npm:^33.0.0" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/base-controller": "npm:^8.3.0" - "@metamask/eth-hd-keyring": "npm:^12.1.0" + "@metamask/eth-hd-keyring": "npm:^12.0.0" "@metamask/eth-snap-keyring": "npm:^16.1.0" "@metamask/keyring-api": "npm:^20.1.0" "@metamask/keyring-controller": "npm:^23.0.0" From 1176ee7c12f176b50313bd6e29ea5568e85f69c8 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 08:15:25 -0400 Subject: [PATCH 71/77] chore: add return types to EVM provider --- .../src/providers/EvmAccountProvider.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 785a40b3bc6..deac3cd8471 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -1,5 +1,5 @@ import type { Bip44Account } from '@metamask/account-api'; -import type { EntropySourceId } from '@metamask/keyring-api'; +import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { @@ -97,7 +97,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { }: { entropySource: EntropySourceId; groupIndex: number; - }) { + }): Promise[]> { const [address] = await this.#createAccount({ entropySource, groupIndex, @@ -129,7 +129,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { async discoverAndCreateAccounts(opts: { entropySource: EntropySourceId; groupIndex: number; - }) { + }): Promise[]> { const provider = this.getEvmProvider(); const { entropySource, groupIndex } = opts; From 428cc30d18bbd3d0088c468ca63f46563fec3e22 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 09:19:55 -0400 Subject: [PATCH 72/77] chore: remove createMultichainAccountWallet code --- .../multichain-account-service/CHANGELOG.md | 4 +- .../multichain-account-service/package.json | 2 - .../src/MultichainAccountService.test.ts | 80 ------------------- .../src/MultichainAccountService.ts | 64 +-------------- .../src/mnemonic.ts | 21 ----- .../src/tests/accounts.ts | 3 - .../src/tests/messenger.ts | 2 - .../multichain-account-service/src/types.ts | 14 +--- yarn.lock | 2 - 9 files changed, 4 insertions(+), 188 deletions(-) delete mode 100644 packages/multichain-account-service/src/mnemonic.ts diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index a7692e59554..564bd94029c 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -11,10 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `discoverAndCreateAccounts` methods for EVM and Solana providers ([#6397](https://github.com/MetaMask/core/pull/6397)) - Add `discoverAndCreateAccounts` method to `MultichainAccountWallet` to orchestrate provider discovery ([#6397](https://github.com/MetaMask/core/pull/6397)) -- Add `createMultichainAccountWallet` method to create a new multichain account wallet from a mnemonic ([#6397](https://github.com/MetaMask/core/pull/6397)) - - An action handler was also registered for this method so that it can be called from the clients. - **BREAKING** Add additional allowed actions to the `MultichainAccountService` messenger - - `NetworkController:getNetworkClientById`, `NetworkController:findNetworkClientIdByChainId`, `KeyringController:getKeyringsByType` and `KeyringController:addNewKeyring` actions were added. + - `NetworkController:getNetworkClientById` and `NetworkController:findNetworkClientIdByChainId` were added. ### Changed diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index 8da38e85af3..bb7df90fb5c 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -53,7 +53,6 @@ "@metamask/keyring-internal-api": "^8.1.0", "@metamask/keyring-snap-client": "^7.0.0", "@metamask/keyring-utils": "^3.1.0", - "@metamask/scure-bip39": "^2.1.1", "@metamask/snaps-sdk": "^9.0.0", "@metamask/snaps-utils": "^11.0.0", "@metamask/superstruct": "^3.1.0", @@ -63,7 +62,6 @@ "@metamask/account-api": "^0.9.0", "@metamask/accounts-controller": "^33.0.0", "@metamask/auto-changelog": "^3.4.4", - "@metamask/eth-hd-keyring": "^12.0.0", "@metamask/keyring-controller": "^23.0.0", "@metamask/providers": "^22.1.0", "@metamask/snaps-controllers": "^14.0.1", diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index ee7ed14024b..6c389c27f51 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -5,7 +5,6 @@ import type { KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType, SolAccountType } from '@metamask/keyring-api'; import { KeyringTypes, type KeyringObject } from '@metamask/keyring-controller'; -import { convertMnemonicToWordlistIndices } from './mnemonic'; import { MultichainAccountService } from './MultichainAccountService'; import { AccountProviderWrapper } from './providers/AccountProviderWrapper'; import { EvmAccountProvider } from './providers/EvmAccountProvider'; @@ -15,7 +14,6 @@ import { MOCK_HARDWARE_ACCOUNT_1, MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2, - MOCK_MNEMONIC, MOCK_SNAP_ACCOUNT_1, MOCK_SNAP_ACCOUNT_2, MOCK_SOL_ACCOUNT_1, @@ -56,8 +54,6 @@ type Mocks = { KeyringController: { keyrings: KeyringObject[]; getState: jest.Mock; - getKeyringsByType: jest.Mock; - addNewKeyring: jest.Mock; }; AccountsController: { listMultichainAccounts: jest.Mock; @@ -106,8 +102,6 @@ function setup({ KeyringController: { keyrings, getState: jest.fn(), - getKeyringsByType: jest.fn(), - addNewKeyring: jest.fn(), }, AccountsController: { listMultichainAccounts: jest.fn(), @@ -129,16 +123,6 @@ function setup({ mocks.KeyringController.getState, ); - messenger.registerActionHandler( - 'KeyringController:getKeyringsByType', - mocks.KeyringController.getKeyringsByType, - ); - - messenger.registerActionHandler( - 'KeyringController:addNewKeyring', - mocks.KeyringController.addNewKeyring, - ); - if (accounts) { mocks.AccountsController.listMultichainAccounts.mockImplementation( () => accounts, @@ -851,27 +835,6 @@ describe('MultichainAccountService', () => { expect(isInProgress).toBe(false); }); - - it('creates a multichain account wallet with MultichainAccountService:createMultichainAccountWallet', async () => { - const { messenger, mocks } = setup({ accounts: [], keyrings: [] }); - - mocks.KeyringController.getKeyringsByType.mockImplementationOnce( - () => [], - ); - - mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ - id: 'abc', - name: '', - })); - - const wallet = await messenger.call( - 'MultichainAccountService:createMultichainAccountWallet', - { mnemonic: MOCK_MNEMONIC }, - ); - - expect(wallet).toBeDefined(); - expect(wallet.entropySource).toBe('abc'); - }); }); describe('setBasicFunctionality', () => { @@ -994,47 +957,4 @@ describe('MultichainAccountService', () => { expect(wrapper.isAccountCompatible(MOCK_HD_ACCOUNT_1)).toBe(false); }); }); - - describe('createMultichainAccountWallet', () => { - it('creates a new multichain account wallet with the given mnemonic', async () => { - const { mocks, service } = setup({ - accounts: [], - keyrings: [], - }); - - mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ - {}, - ]); - - mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ - id: 'abc', - name: '', - })); - - const wallet = await service.createMultichainAccountWallet({ - mnemonic: MOCK_MNEMONIC, - }); - - expect(wallet).toBeDefined(); - expect(wallet.entropySource).toBe('abc'); - }); - - it("throws an error if there's already an existing keyring from the same mnemonic", async () => { - const { service, mocks } = setup({ accounts: [], keyrings: [] }); - - const convertedMnemonic = convertMnemonicToWordlistIndices(MOCK_MNEMONIC); - - mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ - { - mnemonic: convertedMnemonic, - }, - ]); - - await expect( - service.createMultichainAccountWallet({ mnemonic: MOCK_MNEMONIC }), - ).rejects.toThrow( - 'This Secret Recovery Phrase has already been imported.', - ); - }); - }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index d41ca220150..fc2192eab97 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -6,14 +6,9 @@ import type { MultichainAccountWalletId, Bip44Account, } from '@metamask/account-api'; -import type { HdKeyring } from '@metamask/eth-hd-keyring'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; -import { - type KeyringMetadata, - KeyringTypes, -} from '@metamask/keyring-controller'; +import { KeyringTypes } from '@metamask/keyring-controller'; -import { convertEnglishWordlistIndicesToCodepoints } from './mnemonic'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; import type { NamedAccountProvider } from './providers'; @@ -129,11 +124,6 @@ export class MultichainAccountService { 'MultichainAccountService:getIsAlignmentInProgress', () => this.getIsAlignmentInProgress(), ); - this.#messenger.registerActionHandler( - 'MultichainAccountService:createMultichainAccountWallet', - (...args) => this.createMultichainAccountWallet(...args), - ); - this.#messenger.subscribe('AccountsController:accountAdded', (account) => this.#handleOnAccountAdded(account), ); @@ -302,58 +292,6 @@ export class MultichainAccountService { return Array.from(this.#wallets.values()); } - /** - * Creates a new multichain account wallet with the given mnemonic. - * - * NOTE: This method should only be called in client code where a mutex lock is acquired. - * `discoverAndCreateAccounts` should be called after this method to discover and create accounts. - * - * @param options - Options. - * @param options.mnemonic - The mnemonic to use to create the new wallet. - * @throws If the mnemonic has already been imported. - * @returns The new multichain account wallet. - */ - async createMultichainAccountWallet({ - mnemonic, - }: { - mnemonic: string; - }): Promise>> { - const existingKeyrings = this.#messenger.call( - 'KeyringController:getKeyringsByType', - KeyringTypes.hd, - ) as HdKeyring[]; - - const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { - if (!keyring.mnemonic) { - return false; - } - return ( - convertEnglishWordlistIndicesToCodepoints(keyring.mnemonic).toString( - 'utf8', - ) === mnemonic - ); - }); - - if (alreadyHasImportedSrp) { - throw new Error('This Secret Recovery Phrase has already been imported.'); - } - - const result = (await this.#messenger.call( - 'KeyringController:addNewKeyring', - KeyringTypes.hd, - { mnemonic }, - )) as KeyringMetadata; - - const wallet = new MultichainAccountWallet({ - providers: this.#providers, - entropySource: result.id, - }); - - this.#wallets.set(wallet.id, wallet); - - return wallet; - } - /** * Gets a reference to the multichain account group matching this entropy source * and a group index. diff --git a/packages/multichain-account-service/src/mnemonic.ts b/packages/multichain-account-service/src/mnemonic.ts deleted file mode 100644 index c23e1d8dd2a..00000000000 --- a/packages/multichain-account-service/src/mnemonic.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english'; - -export const convertEnglishWordlistIndicesToCodepoints = ( - wordlistIndices: Uint8Array, -): Buffer => { - return Buffer.from( - Array.from(new Uint16Array(wordlistIndices.buffer)) - .map((i) => wordlist[i]) - .join(' '), - ); -}; - -export const convertMnemonicToWordlistIndices = ( - mnemonic: string, -): Uint8Array => { - const indices = mnemonic - .toString() - .split(' ') - .map((word) => wordlist.indexOf(word)); - return new Uint8Array(new Uint16Array(indices).buffer); -}; diff --git a/packages/multichain-account-service/src/tests/accounts.ts b/packages/multichain-account-service/src/tests/accounts.ts index e4eb2d6d15c..d9944832cb4 100644 --- a/packages/multichain-account-service/src/tests/accounts.ts +++ b/packages/multichain-account-service/src/tests/accounts.ts @@ -22,9 +22,6 @@ import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import { v4 as uuid } from 'uuid'; -export const MOCK_MNEMONIC = - 'abandon ability able about above absent absorb abstract absurd abuse access accident'; - export const ETH_EOA_METHODS = [ EthMethod.PersonalSign, EthMethod.Sign, diff --git a/packages/multichain-account-service/src/tests/messenger.ts b/packages/multichain-account-service/src/tests/messenger.ts index d17df13763c..92922839293 100644 --- a/packages/multichain-account-service/src/tests/messenger.ts +++ b/packages/multichain-account-service/src/tests/messenger.ts @@ -45,8 +45,6 @@ export function getMultichainAccountServiceMessenger( 'KeyringController:getState', 'NetworkController:findNetworkClientIdByChainId', 'NetworkController:getNetworkClientById', - 'KeyringController:getKeyringsByType', - 'KeyringController:addNewKeyring', ], }); } diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index e7673246b7a..889e8b7e759 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -7,8 +7,6 @@ import type { } from '@metamask/accounts-controller'; import type { RestrictedMessenger } from '@metamask/base-controller'; import type { - KeyringControllerAddNewKeyringAction, - KeyringControllerGetKeyringsByTypeAction, KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, KeyringControllerWithKeyringAction, @@ -74,11 +72,6 @@ export type MultichainAccountServiceGetIsAlignmentInProgressAction = { handler: MultichainAccountService['getIsAlignmentInProgress']; }; -export type MultichainAccountServiceCreateMultichainAccountWalletAction = { - type: `${typeof serviceName}:createMultichainAccountWallet`; - handler: MultichainAccountService['createMultichainAccountWallet']; -}; - /** * All actions that {@link MultichainAccountService} registers so that other * modules can call them. @@ -93,8 +86,7 @@ export type MultichainAccountServiceActions = | MultichainAccountServiceSetBasicFunctionalityAction | MultichainAccountServiceAlignWalletAction | MultichainAccountServiceAlignWalletsAction - | MultichainAccountServiceGetIsAlignmentInProgressAction - | MultichainAccountServiceCreateMultichainAccountWalletAction; + | MultichainAccountServiceGetIsAlignmentInProgressAction; /** * All events that {@link MultichainAccountService} publishes so that other modules @@ -114,9 +106,7 @@ export type AllowedActions = | KeyringControllerWithKeyringAction | KeyringControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction - | NetworkControllerFindNetworkClientIdByChainIdAction - | KeyringControllerGetKeyringsByTypeAction - | KeyringControllerAddNewKeyringAction; + | NetworkControllerFindNetworkClientIdByChainIdAction; /** * All events published by other modules that {@link MultichainAccountService} diff --git a/yarn.lock b/yarn.lock index 0273ff7f1eb..dbbeee660df 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3821,7 +3821,6 @@ __metadata: "@metamask/accounts-controller": "npm:^33.0.0" "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/base-controller": "npm:^8.3.0" - "@metamask/eth-hd-keyring": "npm:^12.0.0" "@metamask/eth-snap-keyring": "npm:^16.1.0" "@metamask/keyring-api": "npm:^20.1.0" "@metamask/keyring-controller": "npm:^23.0.0" @@ -3829,7 +3828,6 @@ __metadata: "@metamask/keyring-snap-client": "npm:^7.0.0" "@metamask/keyring-utils": "npm:^3.1.0" "@metamask/providers": "npm:^22.1.0" - "@metamask/scure-bip39": "npm:^2.1.1" "@metamask/snaps-controllers": "npm:^14.0.1" "@metamask/snaps-sdk": "npm:^9.0.0" "@metamask/snaps-utils": "npm:^11.0.0" From 7b04612c552577c61784e19368835a05d7a56669 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 10:18:56 -0400 Subject: [PATCH 73/77] fix: update logging with the target group index --- .../src/MultichainAccountWallet.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 3c9386a4565..72de9328911 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -387,14 +387,14 @@ export class MultichainAccountWallet< const runProviderDiscovery = async ( context: AccountProviderDiscoveryContext, ) => { - const message = (stepName: string) => - `[${context.provider.getName()}] Discovery ${stepName} (groupIndex=${context.groupIndex})`; + const message = (stepName: string, groupIndex: number) => + `[${context.provider.getName()}] Discovery ${stepName} (groupIndex=${groupIndex})`; while (!context.stopped) { // Fast‑forward to current high‑water mark const targetGroupIndex = Math.max(context.groupIndex, maxGroupIndex); - log(message('STARTED')); + log(message('STARTED', targetGroupIndex)); let accounts: Bip44Account[] = []; try { @@ -405,17 +405,17 @@ export class MultichainAccountWallet< } catch (error) { context.stopped = true; console.error(error); - log(message('FAILED'), error); + log(message('FAILED', targetGroupIndex), error); break; } if (!accounts.length) { - log(message('STOPPED')); + log(message('STOPPED', targetGroupIndex)); context.stopped = true; break; } - log(message('SUCCEEDED')); + log(message('SUCCEEDED', targetGroupIndex)); context.count += accounts.length; From a3a730d8133551e8476e7d269accbab0500b33f9 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 10:58:31 -0400 Subject: [PATCH 74/77] refactor: apply code review --- .../src/MultichainAccountWallet.ts | 5 ++++- .../src/providers/EvmAccountProvider.ts | 9 ++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 72de9328911..ae29c92a9b3 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -23,13 +23,16 @@ import type { NamedAccountProvider } from './providers'; /** * The context for a provider discovery. */ -export type AccountProviderDiscoveryContext = { +type AccountProviderDiscoveryContext = { provider: NamedAccountProvider; stopped: boolean; groupIndex: number; count: number; }; +/** + * The metrics resulting from account discovery. + */ export type AccountDiscoveryMetrics = { [providerName: string]: number; }; diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index deac3cd8471..173e3765f3b 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -77,9 +77,8 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return [existing[groupIndex], false]; } - // For now, we don't allow for gap, so if we need to create a new - // account, this has to be the next one. - if (groupIndex !== existing.length && throwOnGap) { + // If the throwOnGap flag is set, we throw an error to prevent index gaps. + if (throwOnGap && groupIndex !== existing.length) { throw new Error('Trying to create too many accounts'); } @@ -156,7 +155,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { ); return []; } - } catch { + } catch (error) { // If the RPC request fails and we just created this account for discovery, // remove it to avoid leaving a dangling account. if (shouldCleanup) { @@ -167,7 +166,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { }, ); } - return []; + throw error; } const account = this.messenger.call( From 9c5ea6239219313371f888af01923ee32c979d6b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 11:05:50 -0400 Subject: [PATCH 75/77] fix: EVM provider test --- .../src/providers/EvmAccountProvider.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 6ee151969d1..b75b7e80221 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -318,12 +318,12 @@ describe('EvmAccountProvider', () => { throw new Error('RPC request failed'); }); - expect( - await provider.discoverAndCreateAccounts({ + await expect( + provider.discoverAndCreateAccounts({ entropySource: MOCK_HD_KEYRING_1.metadata.id, groupIndex: 1, }), - ).toStrictEqual([]); + ).rejects.toThrow('RPC request failed'); expect(provider.getAccounts()).toStrictEqual([MOCK_HD_ACCOUNT_1]); }); From 35fe26de117654e4d4c8d3dbedbc83013ad3625a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 11:28:28 -0400 Subject: [PATCH 76/77] fix: updated solana discovery to account for different derivation schemes --- .../src/providers/SolAccountProvider.ts | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 5b034857dab..e094d5868f7 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -56,19 +56,20 @@ export class SolAccountProvider extends SnapAccountProvider { ); } - async createAccounts({ + override async createAccounts({ entropySource, groupIndex, + derivationPath = `m/44'/501'/${groupIndex}'/0'`, }: { entropySource: EntropySourceId; groupIndex: number; + derivationPath?: string; }): Promise[]> { const createAccount = await this.getRestrictedSnapAccountCreator(); // Create account without any confirmation nor selecting it. // TODO: Use the new keyring API `createAccounts` method with the "bip-44:derive-index" // type once ready. - const derivationPath = `m/44'/501'/${groupIndex}'/0'`; const account = await createAccount({ entropySource, derivationPath, @@ -103,11 +104,20 @@ export class SolAccountProvider extends SnapAccountProvider { groupIndex, ); - // Solana Snap always returns one discovered account. - if (discoveredAccounts.length === 1) { - return this.createAccounts({ entropySource, groupIndex }); + if (!discoveredAccounts.length) { + return []; } - return Promise.resolve([]); + const createdAccounts = await Promise.all( + discoveredAccounts.map((d) => + this.createAccounts({ + entropySource, + groupIndex, + derivationPath: d.derivationPath, + }), + ), + ); + + return createdAccounts.flat(); } } From 022a00b1d0cdb8c06e19d56a82b4d23b1e0f9d23 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 5 Sep 2025 11:59:03 -0400 Subject: [PATCH 77/77] refactor: change implementation of solana discovery to preserve public api --- .../src/providers/SolAccountProvider.ts | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index e094d5868f7..de674e005b2 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -1,4 +1,4 @@ -import { type Bip44Account } from '@metamask/account-api'; +import { assertIsBip44Account, type Bip44Account } from '@metamask/account-api'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { SolScope } from '@metamask/keyring-api'; import { @@ -13,7 +13,6 @@ import { HandlerType } from '@metamask/snaps-utils'; import type { Json, JsonRpcRequest } from '@metamask/utils'; import type { MultichainAccountServiceMessenger } from 'src/types'; -import { assertAreBip44Accounts } from './BaseBip44AccountProvider'; import { SnapAccountProvider } from './SnapAccountProvider'; export class SolAccountProvider extends SnapAccountProvider { @@ -56,28 +55,19 @@ export class SolAccountProvider extends SnapAccountProvider { ); } - override async createAccounts({ + async #createAccount({ entropySource, groupIndex, - derivationPath = `m/44'/501'/${groupIndex}'/0'`, + derivationPath, }: { entropySource: EntropySourceId; groupIndex: number; - derivationPath?: string; - }): Promise[]> { + derivationPath: string; + }): Promise> { const createAccount = await this.getRestrictedSnapAccountCreator(); + const account = await createAccount({ entropySource, derivationPath }); - // Create account without any confirmation nor selecting it. - // TODO: Use the new keyring API `createAccounts` method with the "bip-44:derive-index" - // type once ready. - const account = await createAccount({ - entropySource, - derivationPath, - }); - - // Solana Snap does not use BIP-44 typed options for the moment - // so we "inject" them (the `AccountsController` does a similar thing - // for the moment). + // Ensure entropy is present before type assertion validation account.options.entropy = { type: KeyringAccountEntropyTypeOption.Mnemonic, id: entropySource, @@ -85,10 +75,24 @@ export class SolAccountProvider extends SnapAccountProvider { derivationPath, }; - const accounts = [account]; - assertAreBip44Accounts(accounts); + assertIsBip44Account(account); + return account; + } - return accounts; + async createAccounts({ + entropySource, + groupIndex, + }: { + entropySource: EntropySourceId; + groupIndex: number; + }): Promise[]> { + const derivationPath = `m/44'/501'/${groupIndex}'/0'`; + const account = await this.#createAccount({ + entropySource, + groupIndex, + derivationPath, + }); + return [account]; } async discoverAndCreateAccounts({ @@ -110,7 +114,7 @@ export class SolAccountProvider extends SnapAccountProvider { const createdAccounts = await Promise.all( discoveredAccounts.map((d) => - this.createAccounts({ + this.#createAccount({ entropySource, groupIndex, derivationPath: d.derivationPath, @@ -118,6 +122,6 @@ export class SolAccountProvider extends SnapAccountProvider { ), ); - return createdAccounts.flat(); + return createdAccounts; } }