From aa318fec40d33d0a92b3f8602b6b238047dade28 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sun, 14 Sep 2025 12:50:19 -0400 Subject: [PATCH 01/37] feat: have group align method print all provider errors --- .../src/MultichainAccountGroup.test.ts | 38 +++++++++++++++++-- .../src/MultichainAccountGroup.ts | 9 ++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts index 7b35e3c452d..47295d719f9 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts @@ -199,7 +199,7 @@ describe('MultichainAccount', () => { expect(providers[1].createAccounts).not.toHaveBeenCalled(); }); - it('warns if provider alignment fails', async () => { + it('warns if alignment fails for a single provider', async () => { const groupIndex = 0; const { group, providers, wallet } = setup({ groupIndex, @@ -208,7 +208,7 @@ describe('MultichainAccount', () => { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); providers[1].createAccounts.mockRejectedValueOnce( - new Error('Unable to create accounts'), + new Error('Provider 2: Unable to create accounts'), ); await group.align(); @@ -219,7 +219,39 @@ describe('MultichainAccount', () => { groupIndex, }); expect(consoleSpy).toHaveBeenCalledWith( - `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing`, + `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing. Provider threw the following error:\n- Error: Provider 2: Unable to create accounts`, + ); + }); + + it('warns if alignment fails for multiple providers', async () => { + const groupIndex = 0; + const { group, providers, wallet } = setup({ + groupIndex, + accounts: [[MOCK_WALLET_1_EVM_ACCOUNT], [], []], + }); + + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + providers[1].createAccounts.mockRejectedValueOnce( + new Error('Provider 2: Unable to create accounts'), + ); + + providers[2].createAccounts.mockRejectedValueOnce( + new Error('Provider 3: Unable to create accounts'), + ) + + await group.align(); + + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + expect(providers[1].createAccounts).toHaveBeenCalledWith({ + entropySource: wallet.entropySource, + groupIndex, + }); + expect(providers[2].createAccounts).toHaveBeenCalledWith({ + entropySource: wallet.entropySource, + groupIndex, + }); + expect(consoleSpy).toHaveBeenCalledWith( + `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing. Providers threw the following errors:\n- Error: Provider 2: Unable to create accounts\n- Error: Provider 3: Unable to create accounts`, ); }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index b552f95b7ca..8ec2558714a 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -233,8 +233,15 @@ export class MultichainAccountGroup< ); if (results.some((result) => result.status === 'rejected')) { + const rejectedResults = results.filter( + (result) => result.status === 'rejected', + ) as PromiseRejectedResult[]; + const errors = rejectedResults + .map((result) => `- ${result.reason}`) + .join('\n'); + const hasMultipleFailures = rejectedResults.length > 1; console.warn( - `Failed to fully align multichain account group for entropy ID: ${this.wallet.entropySource} and group index: ${this.groupIndex}, some accounts might be missing`, + `Failed to fully align multichain account group for entropy ID: ${this.wallet.entropySource} and group index: ${this.groupIndex}, some accounts might be missing. ${hasMultipleFailures ? 'Providers' : 'Provider'} threw the following ${hasMultipleFailures ? 'errors' : 'error'}:\n${errors}`, ); } } From b49f7b4fac95bffebe61b516766055078f686315 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 01:19:47 -0400 Subject: [PATCH 02/37] feat: construct service state at the top level and pass state slices to wallets and groups --- .../src/MultichainAccountGroup.ts | 82 +++++++-- .../src/MultichainAccountService.ts | 133 ++++++++------ .../src/MultichainAccountWallet.ts | 167 +++++++++++------- .../src/providers/BaseBip44AccountProvider.ts | 40 ++--- 4 files changed, 277 insertions(+), 145 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index c541c73daf9..306eec99360 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -8,10 +8,14 @@ import type { Bip44Account } from '@metamask/account-api'; import type { AccountSelector } from '@metamask/account-api'; import { type KeyringAccount } from '@metamask/keyring-api'; +import type { ServiceState, StateKeys } from './MultichainAccountService'; import type { MultichainAccountWallet } from './MultichainAccountWallet'; -import type { NamedAccountProvider } from './providers'; +import type { BaseBip44AccountProvider } from './providers'; import type { MultichainAccountServiceMessenger } from './types'; +export type GroupState = + ServiceState[StateKeys['entropySource']][StateKeys['groupIndex']]; + /** * A multichain account group that holds multiple accounts. */ @@ -25,17 +29,11 @@ export class MultichainAccountGroup< readonly #groupIndex: number; - readonly #providers: NamedAccountProvider[]; + readonly #providers: BaseBip44AccountProvider[]; - readonly #providerToAccounts: Map< - NamedAccountProvider, - Account['id'][] - >; + readonly #providerToAccounts: Map; - readonly #accountToProvider: Map< - Account['id'], - NamedAccountProvider - >; + readonly #accountToProvider: Map; readonly #messenger: MultichainAccountServiceMessenger; @@ -50,7 +48,7 @@ export class MultichainAccountGroup< }: { groupIndex: number; wallet: MultichainAccountWallet; - providers: NamedAccountProvider[]; + providers: BaseBip44AccountProvider[]; messenger: MultichainAccountServiceMessenger; }) { this.#id = toMultichainAccountGroupId(wallet.id, groupIndex); @@ -65,6 +63,30 @@ export class MultichainAccountGroup< this.#initialized = true; } + init(groupState: GroupState) { + for (const provider of this.#providers) { + const accountIds = groupState[provider.getName()]; + + if (accountIds) { + for (const accountId of accountIds) { + this.#accountToProvider.set(accountId, provider); + } + const providerAccounts = this.#providerToAccounts.get(provider); + if (!providerAccounts) { + this.#providerToAccounts.set(provider, accountIds); + } else { + providerAccounts.push(...accountIds); + } + // Add the accounts to the provider's internal list of account IDs + provider.addAccounts(accountIds); + } + } + } + + /** + * Add a method to update a group and emit the multichainAccountGroupUpdated event + */ + /** * Force multichain account synchronization. * @@ -175,6 +197,10 @@ export class MultichainAccountGroup< return allAccounts; } + getAccountIds(): Account['id'][] { + return [...this.#providerToAccounts.values()].flat(); + } + /** * Gets the account for a given account ID. * @@ -228,13 +254,43 @@ export class MultichainAccountGroup< groupIndex: this.groupIndex, }); } - return Promise.resolve(); + return Promise.reject(new Error('Already aligned')); }), ); + // Fetching the account list once from the AccountsController to avoid multiple calls + const accountsList = this.#messenger.call( + 'AccountsController:listMultichainAccounts', + ); + + const groupState: GroupState = {}; + + const addressBuckets = results.map((result, idx) => { + const addressSet = new Set(); + if (result.status === 'fulfilled') { + groupState[this.#providers[idx].getName()] = []; + result.value.forEach((account) => { + addressSet.add(account.address); + }); + } + return addressSet; + }); + + accountsList.forEach((account) => { + const { address } = account; + addressBuckets.forEach((addressSet, idx) => { + if (addressSet.has(address)) { + groupState[this.#providers[idx].getName()].push(account.id); + } + }); + }); + + this.init(groupState); + if (results.some((result) => result.status === 'rejected')) { const rejectedResults = results.filter( - (result) => result.status === 'rejected', + (result) => + result.status === 'rejected' && result.reason !== 'Already aligned', ) as PromiseRejectedResult[]; const errors = rejectedResults .map((result) => `- ${result.reason}`) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index c16be5c8989..2bffed9accc 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -10,15 +10,12 @@ import type { HdKeyring } from '@metamask/eth-hd-keyring'; import { mnemonicPhraseToBytes } from '@metamask/key-tree'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; import { areUint8ArraysEqual } from '@metamask/utils'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; -import type { - EvmAccountProviderConfig, - NamedAccountProvider, - SolAccountProviderConfig, -} from './providers'; +import type { BaseBip44AccountProvider } from './providers'; import { AccountProviderWrapper, isAccountProviderWrapper, @@ -32,13 +29,9 @@ export const serviceName = 'MultichainAccountService'; /** * The options that {@link MultichainAccountService} takes. */ -export type MultichainAccountServiceOptions = { +type MultichainAccountServiceOptions = { messenger: MultichainAccountServiceMessenger; - providers?: NamedAccountProvider[]; - providerConfigs?: { - [EvmAccountProvider.NAME]?: EvmAccountProviderConfig; - [SolAccountProvider.NAME]?: SolAccountProviderConfig; - }; + providers?: BaseBip44AccountProvider[]; }; /** Reverse mapping object used to map account IDs and their wallet/multichain account. */ @@ -47,13 +40,29 @@ type AccountContext> = { group: MultichainAccountGroup; }; +export type StateKeys = { + entropySource: EntropySourceId; + groupIndex: number; + providerName: string; +}; + +export type ServiceState = { + [entropySource: StateKeys['entropySource']]: { + [groupIndex: string]: { + [ + providerName: StateKeys['providerName'] + ]: Bip44Account['id'][]; + }; + }; +}; + /** * Service to expose multichain accounts capabilities. */ export class MultichainAccountService { readonly #messenger: MultichainAccountServiceMessenger; - readonly #providers: NamedAccountProvider[]; + readonly #providers: BaseBip44AccountProvider[]; readonly #wallets: Map< MultichainAccountWalletId, @@ -77,30 +86,19 @@ export class MultichainAccountService { * @param options.messenger - The messenger suited to this * MultichainAccountService. * @param options.providers - Optional list of account - * @param options.providerConfigs - Optional provider configs * providers. */ - constructor({ - messenger, - providers = [], - providerConfigs, - }: MultichainAccountServiceOptions) { + constructor({ messenger, providers = [] }: MultichainAccountServiceOptions) { this.#messenger = messenger; this.#wallets = new Map(); this.#accountIdToContext = new Map(); // TODO: Rely on keyring capabilities once the keyring API is used by all keyrings. this.#providers = [ - new EvmAccountProvider( - this.#messenger, - providerConfigs?.[EvmAccountProvider.NAME], - ), + new EvmAccountProvider(this.#messenger), new AccountProviderWrapper( this.#messenger, - new SolAccountProvider( - this.#messenger, - providerConfigs?.[SolAccountProvider.NAME], - ), + new SolAccountProvider(this.#messenger), ), // Custom account providers that can be provided by the MetaMask client. ...providers, @@ -155,6 +153,49 @@ export class MultichainAccountService { ); } + #getStateKeys(account: InternalAccount): StateKeys | null { + for (const provider of this.#providers) { + if (isBip44Account(account) && provider.isAccountCompatible(account)) { + return { + entropySource: account.options.entropy.id, + groupIndex: account.options.entropy.groupIndex, + providerName: provider.getName(), + }; + } + } + return null; + } + + #constructServiceState() { + const accounts = this.#messenger.call( + 'AccountsController:listMultichainAccounts', + ); + + const serviceState: ServiceState = {}; + const { keyrings } = this.#messenger.call('KeyringController:getState'); + + // We set up the wallet level keys for this constructed state object + for (const keyring of keyrings) { + if (keyring.type === (KeyringTypes.hd as string)) { + serviceState[keyring.metadata.id] = {}; + } + } + + for (const account of accounts) { + const keys = this.#getStateKeys(account); + if (keys) { + serviceState[keys.entropySource][keys.groupIndex][keys.providerName] ??= + []; + // ok to cast here because at this point we know that the account is BIP-44 compatible + serviceState[keys.entropySource][keys.groupIndex][ + keys.providerName + ].push(account.id); + } + } + + return serviceState; + } + /** * Initialize the service and constructs the internal reprensentation of * multichain accounts and wallets. @@ -163,30 +204,22 @@ export class MultichainAccountService { this.#wallets.clear(); this.#accountIdToContext.clear(); - // Create initial wallets. - const { keyrings } = this.#messenger.call('KeyringController:getState'); - for (const keyring of keyrings) { - if (keyring.type === (KeyringTypes.hd as string)) { - // Only HD keyrings have an entropy source/SRP. - const entropySource = keyring.metadata.id; - - // This will automatically "associate" all multichain accounts for that wallet - // (based on the accounts owned by each account providers). - const wallet = new MultichainAccountWallet({ - entropySource, - providers: this.#providers, - messenger: this.#messenger, - }); - this.#wallets.set(wallet.id, wallet); - - // Reverse mapping between account ID and their multichain wallet/account: - for (const group of wallet.getMultichainAccountGroups()) { - for (const account of group.getAccounts()) { - this.#accountIdToContext.set(account.id, { - wallet, - group, - }); - } + const serviceState = this.#constructServiceState(); + for (const entropySource of Object.keys(serviceState)) { + const wallet = new MultichainAccountWallet({ + entropySource, + providers: this.#providers, + messenger: this.#messenger, + }); + wallet.init(serviceState[entropySource]); + this.#wallets.set(wallet.id, wallet); + + for (const group of wallet.getMultichainAccountGroups()) { + for (const accountId of group.getAccountIds()) { + this.#accountIdToContext.set(accountId, { + wallet, + group, + }); } } } diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 9a1ff12261b..8e8ccb300f1 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -19,8 +19,12 @@ import { import { createProjectLogger } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { MultichainAccountGroup } from './MultichainAccountGroup'; -import type { NamedAccountProvider } from './providers'; +import { + type GroupState, + MultichainAccountGroup, +} from './MultichainAccountGroup'; +import type { ServiceState, StateKeys } from './MultichainAccountService'; +import type { BaseBip44AccountProvider } from './providers'; import type { MultichainAccountServiceMessenger } from './types'; /** @@ -29,12 +33,20 @@ import type { MultichainAccountServiceMessenger } from './types'; type AccountProviderDiscoveryContext< Account extends Bip44Account, > = { - provider: NamedAccountProvider; + provider: BaseBip44AccountProvider; stopped: boolean; groupIndex: number; accounts: Account[]; }; +type DiscoveredGroupsState = { + [groupIndex: string]: { + [providerName: string]: Bip44Account['address'][]; + }; +}; + +type WalletState = ServiceState[StateKeys['entropySource']]; + const log = createProjectLogger('multichain-account-service'); /** @@ -49,7 +61,7 @@ export class MultichainAccountWallet< readonly #id: MultichainAccountWalletId; - readonly #providers: NamedAccountProvider[]; + readonly #providers: BaseBip44AccountProvider[]; readonly #entropySource: EntropySourceId; @@ -67,7 +79,7 @@ export class MultichainAccountWallet< entropySource, messenger, }: { - providers: NamedAccountProvider[]; + providers: BaseBip44AccountProvider[]; entropySource: EntropySourceId; messenger: MultichainAccountServiceMessenger; }) { @@ -84,6 +96,23 @@ export class MultichainAccountWallet< this.#status = 'ready'; } + init(walletState: WalletState) { + for (const groupIndex of Object.keys(walletState)) { + // Have to convert to number because the state keys become strings when we construct the state object in the service + const indexAsNumber = Number(groupIndex); + const group = new MultichainAccountGroup({ + groupIndex: indexAsNumber, + wallet: this, + providers: this.#providers, + messenger: this.#messenger, + }); + + group.init(walletState[groupIndex]); + + this.#accountGroups.set(indexAsNumber, group); + } + } + /** * Force wallet synchronization. * @@ -298,11 +327,8 @@ export class MultichainAccountWallet< } let group = this.getMultichainAccountGroup(groupIndex); - if (group) { - // If the group already exists, we just `sync` it and returns the same - // reference. - group.sync(); + if (group) { return group; } @@ -315,61 +341,68 @@ export class MultichainAccountWallet< ), ); - // -------------------------------------------------------------------------------- - // READ THIS CAREFULLY: - // - // Since we're not "fully supporting multichain" for now, we still rely on single - // :accountCreated events to sync multichain account groups and wallets. Which means - // that even if of the provider fails, some accounts will still be created on some - // other providers and will become "available" on the `AccountsController`, like: - // - // 1. Creating a multichain account group for index 1 - // 2. EvmAccountProvider.createAccounts returns the EVM account for index 1 - // * AccountsController WILL fire :accountCreated for this account - // * This account WILL BE "available" on the AccountsController state - // 3. SolAccountProvider.createAccounts fails to create a Solana account for index 1 - // * AccountsController WON't fire :accountCreated for this account - // * This account WON'T be "available" on the Account - // 4. MultichainAccountService will receive a :accountCreated for the EVM account from - // step 2 and will create a new multichain account group for index 1, but it won't - // receive any event for the Solana account of this group. Thus, this group won't be - // "aligned" (missing "blockchain account" on this group). - // - // -------------------------------------------------------------------------------- - - // If any of the provider failed to create their accounts, then we consider the - // multichain account group to have failed too. - if (results.some((result) => result.status === 'rejected')) { - // NOTE: Some accounts might still have been created on other account providers. We - // don't rollback them. - const error = `Unable to create multichain account group for index: ${groupIndex}`; - - let warn = `${error}:`; - for (const result of results) { - if (result.status === 'rejected') { - warn += `\n- ${result.reason}`; - } + const didEveryProviderFail = results.every( + (result) => result.status === 'rejected', + ); + + const providerFailures = results.reduce((acc, result) => { + if (result.status === 'rejected') { + acc += `\n- ${result.reason}`; } - console.warn(warn); + return acc; + }, ''); - throw new Error(error); + if (didEveryProviderFail) { + // We throw an error if there's a failure on every provider + throw new Error( + `Unable to create multichain account group for index: ${groupIndex} due to provider failures:${providerFailures}`, + ); + } else if (providerFailures) { + // We warn there's failures on some providers and thus misalignment, but we still create the group + console.warn( + `Unable to create some accounts for group index: ${groupIndex}. Providers threw the following errors:${providerFailures}`, + ); } - // Because of the :accountAdded automatic sync, we might already have created the - // group, so we first try to get it. - group = this.getMultichainAccountGroup(groupIndex); - if (!group) { - // If for some reason it's still not created, we're creating it explicitly now: - group = new MultichainAccountGroup({ - wallet: this, - providers: this.#providers, - groupIndex, - messenger: this.#messenger, + // Get the accounts list from the AccountsController + // opting to do one call here instead of calling getAccounts() for each provider + // which would result in multiple calls to the AccountsController + const accountsList = this.#messenger.call( + 'AccountsController:listMultichainAccounts', + ); + + const groupState: GroupState = {}; + const addressBuckets = results.map((result, idx) => { + const addressSet = new Set(); + if (result.status === 'fulfilled') { + groupState[this.#providers[idx].getName()] = []; + result.value.forEach((account) => { + addressSet.add(account.address); + }); + } + return addressSet; + }); + + accountsList.forEach((account) => { + const { address } = account; + addressBuckets.forEach((addressSet, idx) => { + if (addressSet.has(address)) { + groupState[this.#providers[idx].getName()].push(account.id); + } }); - } + }); + + group = new MultichainAccountGroup({ + wallet: this, + providers: this.#providers, + groupIndex, + messenger: this.#messenger, + }); - // Register the account to our internal map. - this.#accountGroups.set(groupIndex, group); // `group` cannot be undefined here. + group.init(groupState); + + // Register the account(s) to our internal map. + this.#accountGroups.set(groupIndex, group); if (this.#initialized) { this.#messenger.publish( @@ -443,6 +476,7 @@ export class MultichainAccountWallet< // Start with the next available group index (so we can resume the discovery // from there). let maxGroupIndex = this.getNextGroupIndex(); + const discoveredGroupsState: DiscoveredGroupsState = {}; // One serialized loop per provider; all run concurrently const runProviderDiscovery = async ( @@ -459,10 +493,10 @@ export class MultichainAccountWallet< let accounts: Account[] = []; try { - accounts = await context.provider.discoverAccounts({ + accounts = (await context.provider.discoverAccounts({ entropySource: this.#entropySource, groupIndex: targetGroupIndex, - }); + })) as Account[]; } catch (error) { context.stopped = true; console.error(error); @@ -480,6 +514,16 @@ export class MultichainAccountWallet< context.accounts = context.accounts.concat(accounts); + const providerName = context.provider.getName(); + + if (!discoveredGroupsState[targetGroupIndex][providerName]) { + discoveredGroupsState[targetGroupIndex][providerName] = []; + } + + discoveredGroupsState[targetGroupIndex][providerName].push( + ...accounts.map((account) => account.address), + ); + const nextGroupIndex = targetGroupIndex + 1; context.groupIndex = nextGroupIndex; @@ -503,6 +547,9 @@ export class MultichainAccountWallet< // 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(); + for (const groupIndex of Object.keys(discoveredGroupsState)) { + + } // Align missing accounts from group. This is required to create missing account from non-discovered // indexes for some providers. diff --git a/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts b/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts index fd3e853d0c2..2bf89a141f9 100644 --- a/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts @@ -46,44 +46,40 @@ export type NamedAccountProvider< export abstract class BaseBip44AccountProvider implements NamedAccountProvider { protected readonly messenger: MultichainAccountServiceMessenger; + accounts: Bip44Account['id'][] = []; + constructor(messenger: MultichainAccountServiceMessenger) { this.messenger = messenger; } abstract getName(): string; - #getAccounts( - filter: (account: KeyringAccount) => boolean = () => true, - ): Bip44Account[] { - const accounts: Bip44Account[] = []; - - for (const account of this.messenger.call( - // NOTE: Even though the name is misleading, this only fetches all internal - // accounts, including EVM and non-EVM. We might wanna change this action - // name once we fully support multichain accounts. - 'AccountsController:listMultichainAccounts', - )) { - if ( - isBip44Account(account) && - this.isAccountCompatible(account) && - filter(account) - ) { - accounts.push(account); - } - } + addAccounts(accounts: Bip44Account['id'][]): void { + this.accounts.push(...accounts); + } - return accounts; + #getAccountsList(): Bip44Account['id'][] { + return this.accounts; } getAccounts(): Bip44Account[] { - return this.#getAccounts(); + const accountsList = this.#getAccountsList(); + const internalAccounts = this.messenger.call( + 'AccountsController:listMultichainAccounts', + ); + return accountsList.map( + (id) => + internalAccounts.find( + (account) => account.id === id, + ) as Bip44Account, + ); } getAccount( id: Bip44Account['id'], ): Bip44Account { // TODO: Maybe just use a proper find for faster lookup? - const [found] = this.#getAccounts((account) => account.id === id); + const [found] = this.getAccounts.find((account) => account.id === id); if (!found) { throw new Error(`Unable to find account: ${id}`); From 61041310a4f6cc7f7e67c011bd2c01f4e2b3e4de Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 11:42:21 -0400 Subject: [PATCH 03/37] chore: readd code removed by mistake --- .../src/MultichainAccountService.ts | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 2bffed9accc..f70b04a6fe2 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -15,7 +15,11 @@ import { areUint8ArraysEqual } from '@metamask/utils'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; -import type { BaseBip44AccountProvider } from './providers'; +import type { + BaseBip44AccountProvider, + EvmAccountProviderConfig, + SolAccountProviderConfig, +} from './providers'; import { AccountProviderWrapper, isAccountProviderWrapper, @@ -29,9 +33,13 @@ export const serviceName = 'MultichainAccountService'; /** * The options that {@link MultichainAccountService} takes. */ -type MultichainAccountServiceOptions = { +export type MultichainAccountServiceOptions = { messenger: MultichainAccountServiceMessenger; providers?: BaseBip44AccountProvider[]; + providerConfigs?: { + [EvmAccountProvider.NAME]?: EvmAccountProviderConfig; + [SolAccountProvider.NAME]?: SolAccountProviderConfig; + }; }; /** Reverse mapping object used to map account IDs and their wallet/multichain account. */ @@ -86,19 +94,30 @@ export class MultichainAccountService { * @param options.messenger - The messenger suited to this * MultichainAccountService. * @param options.providers - Optional list of account + * @param options.providerConfigs - Optional provider configs * providers. */ - constructor({ messenger, providers = [] }: MultichainAccountServiceOptions) { + constructor({ + messenger, + providers = [], + providerConfigs, + }: MultichainAccountServiceOptions) { this.#messenger = messenger; this.#wallets = new Map(); this.#accountIdToContext = new Map(); // TODO: Rely on keyring capabilities once the keyring API is used by all keyrings. this.#providers = [ - new EvmAccountProvider(this.#messenger), + new EvmAccountProvider( + this.#messenger, + providerConfigs?.[EvmAccountProvider.NAME], + ), new AccountProviderWrapper( this.#messenger, - new SolAccountProvider(this.#messenger), + new SolAccountProvider( + this.#messenger, + providerConfigs?.[SolAccountProvider.NAME], + ), ), // Custom account providers that can be provided by the MetaMask client. ...providers, From c45f612d33eefd2eccdd5a32f8debf089dd222a3 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 11:47:01 -0400 Subject: [PATCH 04/37] feat: add getAccounts method to AccountsController --- .../src/AccountsController.ts | 16 ++++++++++++++++ packages/accounts-controller/src/index.ts | 1 + 2 files changed, 17 insertions(+) diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 3ca7bee9817..05c7102aba1 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -124,6 +124,11 @@ export type AccountsControllerGetAccountAction = { handler: AccountsController['getAccount']; }; +export type AccountsControllerGetAccountsAction = { + type: `${typeof controllerName}:getAccounts`; + handler: AccountsController['getAccounts']; +}; + export type AccountsControllerUpdateAccountMetadataAction = { type: `${typeof controllerName}:updateAccountMetadata`; handler: AccountsController['updateAccountMetadata']; @@ -145,6 +150,7 @@ export type AccountsControllerActions = | AccountsControllerGetSelectedAccountAction | AccountsControllerGetNextAvailableAccountNameAction | AccountsControllerGetAccountAction + | AccountsControllerGetAccountsAction | AccountsControllerGetSelectedMultichainAccountAction | AccountsControllerUpdateAccountMetadataAction; @@ -303,6 +309,16 @@ export class AccountsController extends BaseController< return this.state.internalAccounts.accounts[accountId]; } + /** + * Returns the internal account objects for the given account IDs, if they exist. + * + * @param accountIds - The IDs of the accounts to retrieve. + * @returns The internal account objects, or undefined if the account(s) do not exist. + */ + getAccounts(accountIds: string[]): (InternalAccount | undefined)[] { + return accountIds.map((accountId) => this.getAccount(accountId)); + } + /** * Returns an array of all evm internal accounts. * diff --git a/packages/accounts-controller/src/index.ts b/packages/accounts-controller/src/index.ts index 2894b9d6e71..8c75201adcc 100644 --- a/packages/accounts-controller/src/index.ts +++ b/packages/accounts-controller/src/index.ts @@ -13,6 +13,7 @@ export type { AccountsControllerGetAccountByAddressAction, AccountsControllerGetNextAvailableAccountNameAction, AccountsControllerGetAccountAction, + AccountsControllerGetAccountsAction, AccountsControllerUpdateAccountMetadataAction, AllowedActions, AccountsControllerActions, From acf36e0796cf5b699c1dab6ec85bdb25b5e71b5b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 11:47:55 -0400 Subject: [PATCH 05/37] chore: update types to include getAccounts action --- packages/multichain-account-service/src/types.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 39372186d6a..e144cf9b136 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -9,6 +9,7 @@ import type { AccountsControllerAccountRemovedEvent, AccountsControllerGetAccountAction, AccountsControllerGetAccountByAddressAction, + AccountsControllerGetAccountsAction, AccountsControllerListMultichainAccountsAction, } from '@metamask/accounts-controller'; import type { RestrictedMessenger } from '@metamask/base-controller'; @@ -127,6 +128,7 @@ export type MultichainAccountServiceEvents = */ export type AllowedActions = | AccountsControllerListMultichainAccountsAction + | AccountsControllerGetAccountsAction | AccountsControllerGetAccountAction | AccountsControllerGetAccountByAddressAction | SnapControllerHandleSnapRequestAction From ce3cef4a8798591e41c515c190013be943b6177a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 11:49:43 -0400 Subject: [PATCH 06/37] refactor: derive account ID and use that to do a getAccounts call instead of getAccountByAddress which iterates through the whole of internal accounts in the AccountsController --- .../src/providers/EvmAccountProvider.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 6f2a4172eb8..717adc74dd6 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -1,4 +1,5 @@ import type { Bip44Account } from '@metamask/account-api'; +import { getUUIDFromAddressOfNormalAccount } from '@metamask/accounts-controller'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; @@ -90,6 +91,10 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return provider; } + #getAccountId(address: Hex): string { + return getUUIDFromAddressOfNormalAccount(address); + } + async #createAccount({ entropySource, groupIndex, @@ -133,9 +138,11 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { throwOnGap: true, }); + const accountId = this.#getAccountId(address); + const account = this.messenger.call( - 'AccountsController:getAccountByAddress', - address, + 'AccountsController:getAccount', + accountId, ); // We MUST have the associated internal account. @@ -217,9 +224,11 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { throw error; } + const accountId = this.#getAccountId(address); + const account = this.messenger.call( - 'AccountsController:getAccountByAddress', - address, + 'AccountsController:getAccount', + accountId, ); assertInternalAccountExists(account); assertIsBip44Account(account); From 891fa50c44ea7128956df28b476261e0fd30f894 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 14:05:22 -0400 Subject: [PATCH 07/37] feat: finish refactor --- .../src/MultichainAccountGroup.ts | 95 ++++-------- .../src/MultichainAccountService.ts | 103 ++++--------- .../src/MultichainAccountWallet.ts | 141 +++++------------- .../src/providers/BaseBip44AccountProvider.ts | 36 +++-- .../src/providers/EvmAccountProvider.ts | 34 +++++ 5 files changed, 151 insertions(+), 258 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index 306eec99360..11e43b88d2e 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -37,7 +37,6 @@ export class MultichainAccountGroup< readonly #messenger: MultichainAccountServiceMessenger; - // eslint-disable-next-line @typescript-eslint/prefer-readonly #initialized = false; constructor({ @@ -58,11 +57,15 @@ export class MultichainAccountGroup< this.#messenger = messenger; this.#providerToAccounts = new Map(); this.#accountToProvider = new Map(); - - this.sync(); - this.#initialized = true; } + /** + * Initialize the multichain account group and construct the internal representation of accounts. + * + * Note: This method can be called multiple times to update the group state. + * + * @param groupState - The group state. + */ init(groupState: GroupState) { for (const provider of this.#providers) { const accountIds = groupState[provider.getName()]; @@ -81,45 +84,10 @@ export class MultichainAccountGroup< provider.addAccounts(accountIds); } } - } - /** - * Add a method to update a group and emit the multichainAccountGroupUpdated event - */ - - /** - * Force multichain account synchronization. - * - * This can be used if account providers got new accounts that the multichain - * account doesn't know about. - */ - sync(): void { - // Clear reverse mapping and re-construct it entirely based on the refreshed - // list of accounts from each providers. - this.#accountToProvider.clear(); - - for (const provider of this.#providers) { - // Filter account only for that index. - const accounts = []; - for (const account of provider.getAccounts()) { - if ( - account.options.entropy.id === this.wallet.entropySource && - account.options.entropy.groupIndex === this.groupIndex - ) { - // We only use IDs to always fetch the latest version of accounts. - accounts.push(account.id); - } - } - this.#providerToAccounts.set(provider, accounts); - - // Reverse-mapping for fast indexing. - for (const id of accounts) { - this.#accountToProvider.set(id, provider); - } - } - - // Emit update event when group is synced (only if initialized) - if (this.#initialized) { + if (!this.#initialized) { + this.#initialized = true; + } else { this.#messenger.publish( 'MultichainAccountService:multichainAccountGroupUpdated', this, @@ -189,7 +157,8 @@ export class MultichainAccountGroup< // If for some reason we cannot get this account from the provider, it // might means it has been deleted or something, so we just filter it // out. - allAccounts.push(account); + // We cast here because TS cannot infer the type of the account from the provider + allAccounts.push(account as Account); } } } @@ -197,6 +166,11 @@ export class MultichainAccountGroup< return allAccounts; } + /** + * Gets the account IDs for this multichain account. + * + * @returns The account IDs. + */ getAccountIds(): Account['id'][] { return [...this.#providerToAccounts.values()].flat(); } @@ -215,7 +189,9 @@ export class MultichainAccountGroup< if (!provider) { return undefined; } - return provider.getAccount(id); + + // We cast here because TS cannot infer the type of the account from the provider + return provider.getAccount(id) as Account; } /** @@ -258,33 +234,16 @@ export class MultichainAccountGroup< }), ); - // Fetching the account list once from the AccountsController to avoid multiple calls - const accountsList = this.#messenger.call( - 'AccountsController:listMultichainAccounts', - ); - - const groupState: GroupState = {}; - - const addressBuckets = results.map((result, idx) => { - const addressSet = new Set(); + const groupState = results.reduce((state, result, idx) => { if (result.status === 'fulfilled') { - groupState[this.#providers[idx].getName()] = []; - result.value.forEach((account) => { - addressSet.add(account.address); - }); + state[this.#providers[idx].getName()] = result.value.map( + (account) => account.id, + ); } - return addressSet; - }); - - accountsList.forEach((account) => { - const { address } = account; - addressBuckets.forEach((addressSet, idx) => { - if (addressSet.has(address)) { - groupState[this.#providers[idx].getName()].push(account.id); - } - }); - }); + return state; + }, {}); + // Update group state this.init(groupState); if (results.some((result) => result.status === 'rejected')) { diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index f70b04a6fe2..fe21089e625 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -48,12 +48,18 @@ type AccountContext> = { group: MultichainAccountGroup; }; +/** + * The keys used to identify an account in the service state. + */ export type StateKeys = { entropySource: EntropySourceId; groupIndex: number; providerName: string; }; +/** + * The service state. + */ export type ServiceState = { [entropySource: StateKeys['entropySource']]: { [groupIndex: string]: { @@ -163,15 +169,15 @@ export class MultichainAccountService { 'MultichainAccountService:createMultichainAccountWallet', (...args) => this.createMultichainAccountWallet(...args), ); - - this.#messenger.subscribe('AccountsController:accountAdded', (account) => - this.#handleOnAccountAdded(account), - ); - this.#messenger.subscribe('AccountsController:accountRemoved', (id) => - this.#handleOnAccountRemoved(id), - ); } + /** + * Get the keys used to identify an account in the service state. + * + * @param account - The account to get the keys for. + * @returns The keys used to identify an account in the service state. + * Returns null if the account is not compatible with any provider. + */ #getStateKeys(account: InternalAccount): StateKeys | null { for (const provider of this.#providers) { if (isBip44Account(account) && provider.isAccountCompatible(account)) { @@ -185,6 +191,11 @@ export class MultichainAccountService { return null; } + /** + * Construct the service state. + * + * @returns The service state. + */ #constructServiceState() { const accounts = this.#messenger.call( 'AccountsController:listMultichainAccounts', @@ -244,77 +255,13 @@ export class MultichainAccountService { } } - #handleOnAccountAdded(account: KeyringAccount): void { - // We completely omit non-BIP-44 accounts! - if (!isBip44Account(account)) { - return; - } - - let sync = true; - - let wallet = this.#wallets.get( - toMultichainAccountWalletId(account.options.entropy.id), - ); - if (!wallet) { - // That's a new wallet. - wallet = new MultichainAccountWallet({ - entropySource: account.options.entropy.id, - providers: this.#providers, - messenger: this.#messenger, - }); - this.#wallets.set(wallet.id, wallet); - - // If that's a new wallet wallet. There's nothing to "force-sync". - sync = false; - } - - let group = wallet.getMultichainAccountGroup( - account.options.entropy.groupIndex, - ); - if (!group) { - // This new account is a new multichain account, let the wallet know - // it has to re-sync with its providers. - if (sync) { - wallet.sync(); - } - - group = wallet.getMultichainAccountGroup( - account.options.entropy.groupIndex, - ); - - // If that's a new multichain account. There's nothing to "force-sync". - sync = false; - } - - // We have to check against `undefined` in case `getMultichainAccount` is - // not able to find this multichain account (which should not be possible...) - if (group) { - if (sync) { - group.sync(); - } - - // Same here, this account should have been already grouped in that - // multichain account. - this.#accountIdToContext.set(account.id, { - wallet, - group, - }); - } - } - - #handleOnAccountRemoved(id: KeyringAccount['id']): void { - // Force sync of the appropriate wallet if an account got removed. - const found = this.#accountIdToContext.get(id); - if (found) { - const { wallet } = found; - - wallet.sync(); - } - - // Safe to call delete even if the `id` was not referencing a BIP-44 account. - this.#accountIdToContext.delete(id); - } - + /** + * Get the wallet matching the given entropy source. + * + * @param entropySource - The entropy source of the wallet. + * @returns The wallet matching the given entropy source. + * @throws If no wallet matches the given entropy source. + */ #getWallet( entropySource: EntropySourceId, ): MultichainAccountWallet> { diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 8e8ccb300f1..a011c1f0c4a 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -39,14 +39,11 @@ type AccountProviderDiscoveryContext< accounts: Account[]; }; -type DiscoveredGroupsState = { - [groupIndex: string]: { - [providerName: string]: Bip44Account['address'][]; - }; -}; - type WalletState = ServiceState[StateKeys['entropySource']]; +// type alias to make clear this state is generated by discovery +type DiscoveredGroupsState = WalletState; + const log = createProjectLogger('multichain-account-service'); /** @@ -69,7 +66,6 @@ export class MultichainAccountWallet< readonly #messenger: MultichainAccountServiceMessenger; - // eslint-disable-next-line @typescript-eslint/prefer-readonly #initialized = false; #status: MultichainAccountWalletStatus; @@ -91,9 +87,6 @@ export class MultichainAccountWallet< // Initial synchronization (don't emit events during initialization). this.#status = 'uninitialized'; - this.sync(); - this.#initialized = true; - this.#status = 'ready'; } init(walletState: WalletState) { @@ -111,60 +104,9 @@ export class MultichainAccountWallet< this.#accountGroups.set(indexAsNumber, group); } - } - - /** - * Force wallet synchronization. - * - * This can be used if account providers got new accounts that the wallet - * doesn't know about. - */ - sync(): void { - for (const provider of this.#providers) { - for (const account of provider.getAccounts()) { - const { entropy } = account.options; - - // Filter for this wallet only. - if (entropy.id !== this.entropySource) { - continue; - } - - // This multichain account might exists already. - let multichainAccount = this.#accountGroups.get(entropy.groupIndex); - if (!multichainAccount) { - multichainAccount = new MultichainAccountGroup({ - groupIndex: entropy.groupIndex, - wallet: this, - providers: this.#providers, - messenger: this.#messenger, - }); - - // This existing multichain account group might differ from the - // `createMultichainAccountGroup` behavior. When creating a new - // group, we expect the providers to all succeed. But here, we're - // just fetching the account lists from them, so this group might - // not be "aligned" yet (e.g having a missing Solana account). - // - // Since "aligning" is an async operation, it would have to be run - // after the first-sync. - // TODO: Implement align mechanism to create "missing" accounts. - - this.#accountGroups.set(entropy.groupIndex, multichainAccount); - } - } - } - - // Now force-sync all remaining multichain accounts. - for (const [ - groupIndex, - multichainAccount, - ] of this.#accountGroups.entries()) { - multichainAccount.sync(); - - // Clean up old multichain accounts. - if (!multichainAccount.hasAccounts()) { - this.#accountGroups.delete(groupIndex); - } + if (!this.#initialized) { + this.#initialized = true; + this.#status = 'ready'; } } @@ -341,7 +283,7 @@ export class MultichainAccountWallet< ), ); - const didEveryProviderFail = results.every( + const everyProviderFailed = results.every( (result) => result.status === 'rejected', ); @@ -352,7 +294,7 @@ export class MultichainAccountWallet< return acc; }, ''); - if (didEveryProviderFail) { + if (everyProviderFailed) { // We throw an error if there's a failure on every provider throw new Error( `Unable to create multichain account group for index: ${groupIndex} due to provider failures:${providerFailures}`, @@ -364,33 +306,15 @@ export class MultichainAccountWallet< ); } - // Get the accounts list from the AccountsController - // opting to do one call here instead of calling getAccounts() for each provider - // which would result in multiple calls to the AccountsController - const accountsList = this.#messenger.call( - 'AccountsController:listMultichainAccounts', - ); - - const groupState: GroupState = {}; - const addressBuckets = results.map((result, idx) => { - const addressSet = new Set(); + // No need to fetch the accounts list from the AccountsController since we already have the account IDs to be used in the controller + const groupState = results.reduce((state, result, idx) => { if (result.status === 'fulfilled') { - groupState[this.#providers[idx].getName()] = []; - result.value.forEach((account) => { - addressSet.add(account.address); - }); + state[this.#providers[idx].getName()] = result.value.map( + (account) => account.id, + ); } - return addressSet; - }); - - accountsList.forEach((account) => { - const { address } = account; - addressBuckets.forEach((addressSet, idx) => { - if (addressSet.has(address)) { - groupState[this.#providers[idx].getName()].push(account.id); - } - }); - }); + return state; + }, {}); group = new MultichainAccountGroup({ wallet: this, @@ -478,6 +402,15 @@ export class MultichainAccountWallet< let maxGroupIndex = this.getNextGroupIndex(); const discoveredGroupsState: DiscoveredGroupsState = {}; + const addDiscoveryResultToState = ( + result: Account[], + providerName: string, + groupIndex: number, + ) => { + const accountIds = result.map((account) => account.id); + discoveredGroupsState[groupIndex][providerName] = accountIds; + }; + // One serialized loop per provider; all run concurrently const runProviderDiscovery = async ( context: AccountProviderDiscoveryContext, @@ -516,13 +449,7 @@ export class MultichainAccountWallet< const providerName = context.provider.getName(); - if (!discoveredGroupsState[targetGroupIndex][providerName]) { - discoveredGroupsState[targetGroupIndex][providerName] = []; - } - - discoveredGroupsState[targetGroupIndex][providerName].push( - ...accounts.map((account) => account.address), - ); + addDiscoveryResultToState(accounts, providerName, targetGroupIndex); const nextGroupIndex = targetGroupIndex + 1; context.groupIndex = nextGroupIndex; @@ -544,11 +471,19 @@ export class MultichainAccountWallet< // 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(); - for (const groupIndex of Object.keys(discoveredGroupsState)) { - + // Create discovered groups + for (const [groupIndex, groupState] of Object.entries( + discoveredGroupsState, + )) { + const indexAsNumber = Number(groupIndex); + const group = new MultichainAccountGroup({ + wallet: this, + providers: this.#providers, + groupIndex: indexAsNumber, + messenger: this.#messenger, + }); + group.init(groupState); + this.#accountGroups.set(indexAsNumber, group); } // Align missing accounts from group. This is required to create missing account from non-discovered diff --git a/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts b/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts index 2bf89a141f9..aade4098762 100644 --- a/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BaseBip44AccountProvider.ts @@ -54,32 +54,50 @@ export abstract class BaseBip44AccountProvider implements NamedAccountProvider { abstract getName(): string; + /** + * Add accounts to the provider. + * + * @param accounts - The accounts to add. + */ addAccounts(accounts: Bip44Account['id'][]): void { this.accounts.push(...accounts); } + /** + * Get the accounts list for the provider. + * + * @returns The accounts list. + */ #getAccountsList(): Bip44Account['id'][] { return this.accounts; } + /** + * Get the accounts list for the provider from the AccountsController. + * + * @returns The accounts list. + */ getAccounts(): Bip44Account[] { const accountsList = this.#getAccountsList(); const internalAccounts = this.messenger.call( - 'AccountsController:listMultichainAccounts', - ); - return accountsList.map( - (id) => - internalAccounts.find( - (account) => account.id === id, - ) as Bip44Account, + 'AccountsController:getAccounts', + accountsList, ); + // we cast here because we know that the accounts are BIP-44 compatible + return internalAccounts as Bip44Account[]; } + /** + * Get the account for the provider. + * + * @param id - The account ID. + * @returns The account. + * @throws If the account is not found. + */ getAccount( id: Bip44Account['id'], ): Bip44Account { - // TODO: Maybe just use a proper find for faster lookup? - const [found] = this.getAccounts.find((account) => account.id === id); + const found = this.getAccounts().find((account) => account.id === id); if (!found) { throw new Error(`Unable to find account: ${id}`); diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 717adc74dd6..c64cbb4cbf0 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -91,10 +91,28 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return provider; } + /** + * Get the account ID for an EVM account. + * + * Note: Since the account ID is deterministic at the AccountsController level, + * we can use this method to get the account ID from the address. + * + * @param address - The address of the account. + * @returns The account ID. + */ #getAccountId(address: Hex): string { return getUUIDFromAddressOfNormalAccount(address); } + /** + * Create an EVM account. + * + * @param opts - The options for the creation of the account. + * @param opts.entropySource - The entropy source to use for the creation of the account. + * @param opts.groupIndex - The index of the group to create the account for. + * @param opts.throwOnGap - Whether to throw an error if the account index is not contiguous. + * @returns The account ID and a boolean indicating if the account was created. + */ async #createAccount({ entropySource, groupIndex, @@ -125,6 +143,14 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return result; } + /** + * Create accounts for the EVM provider. + * + * @param opts - The options for the creation of the accounts. + * @param opts.entropySource - The entropy source to use for the creation of the accounts. + * @param opts.groupIndex - The index of the group to create the accounts for. + * @returns The accounts for the EVM provider. + */ async createAccounts({ entropySource, groupIndex, @@ -154,6 +180,14 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { return accountsArray; } + /** + * Get the transaction count for an EVM account. + * This method uses a retry and timeout mechanism to handle transient failures. + * + * @param provider - The provider to use for the transaction count. + * @param address - The address of the account. + * @returns The transaction count. + */ async #getTransactionCount( provider: Provider, address: Hex, From 01096bf2098cdc7ffe1240658693b2f4ad7213d1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 18 Sep 2025 14:09:32 -0400 Subject: [PATCH 08/37] chore: add JSDoc for wallet init --- .../src/MultichainAccountWallet.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index a011c1f0c4a..d995ca9f2f0 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -89,6 +89,11 @@ export class MultichainAccountWallet< this.#status = 'uninitialized'; } + /** + * Initialize the wallet and construct the internal representation of multichain account groups. + * + * @param walletState - The wallet state. + */ init(walletState: WalletState) { for (const groupIndex of Object.keys(walletState)) { // Have to convert to number because the state keys become strings when we construct the state object in the service From 4c2e5820143db0046a744e4dddf2528e5ae2f89a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 19 Sep 2025 12:08:22 -0400 Subject: [PATCH 09/37] chore: remove accountId to context mapping since with the removal of accountAdded and accountRemoved handling, it is dead code --- .../src/MultichainAccountService.ts | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index fe21089e625..c6102f33900 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -83,11 +83,6 @@ export class MultichainAccountService { MultichainAccountWallet> >; - readonly #accountIdToContext: Map< - Bip44Account['id'], - AccountContext> - >; - /** * The name of the service. */ @@ -110,7 +105,6 @@ export class MultichainAccountService { }: MultichainAccountServiceOptions) { this.#messenger = messenger; this.#wallets = new Map(); - this.#accountIdToContext = new Map(); // TODO: Rely on keyring capabilities once the keyring API is used by all keyrings. this.#providers = [ @@ -232,7 +226,6 @@ export class MultichainAccountService { */ init(): void { this.#wallets.clear(); - this.#accountIdToContext.clear(); const serviceState = this.#constructServiceState(); for (const entropySource of Object.keys(serviceState)) { @@ -243,15 +236,6 @@ export class MultichainAccountService { }); wallet.init(serviceState[entropySource]); this.#wallets.set(wallet.id, wallet); - - for (const group of wallet.getMultichainAccountGroups()) { - for (const accountId of group.getAccountIds()) { - this.#accountIdToContext.set(accountId, { - wallet, - group, - }); - } - } } } @@ -276,19 +260,6 @@ export class MultichainAccountService { return wallet; } - /** - * Gets the account's context which contains its multichain wallet and - * multichain account group references. - * - * @param id - Account ID. - * @returns The account context if any, undefined otherwise. - */ - getAccountContext( - id: KeyringAccount['id'], - ): AccountContext> | undefined { - return this.#accountIdToContext.get(id); - } - /** * Gets a reference to the multichain account wallet matching this entropy source. * From 7cec854a0fa6d9a5515dee9e007d318e5ec568c1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 22 Sep 2025 11:01:34 -0400 Subject: [PATCH 10/37] feat: start to add logic that will let createMultichainAccountWallet handle createNewVaultAndKeychain and createNewVaultAndRestore code paths --- .../src/KeyringController.ts | 24 ++++- .../src/MultichainAccountService.ts | 88 +++++++++++++------ .../multichain-account-service/src/types.ts | 6 +- 3 files changed, 90 insertions(+), 28 deletions(-) diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 30cd02020fd..5e4cf83d7ef 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -184,6 +184,16 @@ export type KeyringControllerWithKeyringAction = { handler: KeyringController['withKeyring']; }; +export type KeyringControllerCreateNewVaultAndKeychainAction = { + type: `${typeof name}:createNewVaultAndKeychain`; + handler: KeyringController['createNewVaultAndKeychain']; +}; + +export type KeyringControllerCreateNewVaultAndRestoreAction = { + type: `${typeof name}:createNewVaultAndRestore`; + handler: KeyringController['createNewVaultAndRestore']; +}; + export type KeyringControllerAddNewKeyringAction = { type: `${typeof name}:addNewKeyring`; handler: KeyringController['addNewKeyring']; @@ -226,7 +236,9 @@ export type KeyringControllerActions = | KeyringControllerSignUserOperationAction | KeyringControllerAddNewAccountAction | KeyringControllerWithKeyringAction - | KeyringControllerAddNewKeyringAction; + | KeyringControllerAddNewKeyringAction + | KeyringControllerCreateNewVaultAndKeychainAction + | KeyringControllerCreateNewVaultAndRestoreAction; export type KeyringControllerEvents = | KeyringControllerStateChangeEvent @@ -1780,6 +1792,16 @@ export class KeyringController extends BaseController< `${name}:addNewKeyring`, this.addNewKeyring.bind(this), ); + + this.messagingSystem.registerActionHandler( + `${name}:createNewVaultAndKeychain`, + this.createNewVaultAndKeychain.bind(this), + ); + + this.messagingSystem.registerActionHandler( + `${name}:createNewVaultAndRestore`, + this.createNewVaultAndRestore.bind(this), + ); } /** diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index c6102f33900..e98dc416e6e 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -288,50 +288,86 @@ export class MultichainAccountService { } /** - * Creates a new multichain account wallet with the given mnemonic. + * Creates a new multichain account wallet by first creating a new vault and keyring. + * If just a password is provided, then a new vault and keyring are created with a randomly generated mnemonic. + * If a mnemonic and password are provided, then a new vault and keyring are created 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. + * @param options.password - The password to encrypt the vault with. * @throws If the mnemonic has already been imported. * @returns The new multichain account wallet. */ async createMultichainAccountWallet({ mnemonic, + password, }: { - mnemonic: string; + mnemonic?: string; + password: string; }): Promise>> { - const existingKeyrings = this.#messenger.call( - 'KeyringController:getKeyringsByType', - KeyringTypes.hd, - ) as HdKeyring[]; + if (!password) { + throw new Error('A password must be provided for this method.'); + } - const mnemonicAsBytes = mnemonicPhraseToBytes(mnemonic); + let wallet: MultichainAccountWallet>; - const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { - if (!keyring.mnemonic) { - return false; - } - return areUint8ArraysEqual(keyring.mnemonic, mnemonicAsBytes); - }); + if (mnemonic) { + const existingKeyrings = this.#messenger.call( + 'KeyringController:getKeyringsByType', + KeyringTypes.hd, + ) as HdKeyring[]; - if (alreadyHasImportedSrp) { - throw new Error('This Secret Recovery Phrase has already been imported.'); - } + const mnemonicAsBytes = mnemonicPhraseToBytes(mnemonic); - const result = await this.#messenger.call( - 'KeyringController:addNewKeyring', - KeyringTypes.hd, - { mnemonic }, - ); + const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { + if (!keyring.mnemonic) { + return false; + } + return areUint8ArraysEqual(keyring.mnemonic, mnemonicAsBytes); + }); - const wallet = new MultichainAccountWallet({ - providers: this.#providers, - entropySource: result.id, - messenger: this.#messenger, - }); + if (alreadyHasImportedSrp) { + throw new Error( + 'This Secret Recovery Phrase has already been imported.', + ); + } + + const result = await this.#messenger.call( + 'KeyringController:addNewKeyring', + KeyringTypes.hd, + { mnemonic }, + ); + } else { + try { + await this.#messenger.call( + 'KeyringController:createNewVaultAndKeychain', + password, + ); + + // we're sure to get the correct keyring as it's the only one at this point + const entropySourceId = (await this.#messenger.call( + 'KeyringController:withKeyring', + { type: KeyringTypes.hd }, + async ({ metadata }) => { + return metadata.id; + }, + )) as string; + + // createNewVaultAndKeychain creates an account on the newly created keyring + // we don't need to worry about not having this account in the service state yet + // because the discovery process will store it in state through the alignment in the end. + wallet = new MultichainAccountWallet({ + providers: this.#providers, + entropySource: entropySourceId, + messenger: this.#messenger, + }); + } catch { + throw new Error('Failed to create a new vault and keychain.'); + } + } this.#wallets.set(wallet.id, wallet); diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index e144cf9b136..9b3a567d2ea 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -16,6 +16,8 @@ import type { RestrictedMessenger } from '@metamask/base-controller'; import type { KeyringAccount } from '@metamask/keyring-api'; import type { KeyringControllerAddNewKeyringAction, + KeyringControllerCreateNewVaultAndKeychainAction, + KeyringControllerCreateNewVaultAndRestoreAction, KeyringControllerGetKeyringsByTypeAction, KeyringControllerGetStateAction, KeyringControllerStateChangeEvent, @@ -137,7 +139,9 @@ export type AllowedActions = | KeyringControllerGetKeyringsByTypeAction | KeyringControllerAddNewKeyringAction | NetworkControllerGetNetworkClientByIdAction - | NetworkControllerFindNetworkClientIdByChainIdAction; + | NetworkControllerFindNetworkClientIdByChainIdAction + | KeyringControllerCreateNewVaultAndKeychainAction + | KeyringControllerCreateNewVaultAndRestoreAction; /** * All events published by other modules that {@link MultichainAccountService} From f05e6da3d8526ddfa5cdf020de080a746cac3ee3 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 22 Sep 2025 11:03:14 -0400 Subject: [PATCH 11/37] feat: remove need for mnemonicAsBytes --- .../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 e98dc416e6e..23058560e29 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -305,7 +305,7 @@ export class MultichainAccountService { mnemonic, password, }: { - mnemonic?: string; + mnemonic?: Uint8Array; password: string; }): Promise>> { if (!password) { @@ -320,13 +320,11 @@ export class MultichainAccountService { KeyringTypes.hd, ) as HdKeyring[]; - const mnemonicAsBytes = mnemonicPhraseToBytes(mnemonic); - const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { if (!keyring.mnemonic) { return false; } - return areUint8ArraysEqual(keyring.mnemonic, mnemonicAsBytes); + return areUint8ArraysEqual(keyring.mnemonic, mnemonic); }); if (alreadyHasImportedSrp) { From 6c5b86ec8bf410ed41d43627f407d244b548d909 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 03:33:53 -0400 Subject: [PATCH 12/37] feat: update creatMultichainAccountWallet method to cover all entry points --- .../src/MultichainAccountService.ts | 217 ++++++++++++++---- 1 file changed, 169 insertions(+), 48 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 9d880d04837..66ff787f483 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -43,12 +43,6 @@ export type MultichainAccountServiceOptions = { }; }; -/** Reverse mapping object used to map account IDs and their wallet/multichain account. */ -type AccountContext> = { - wallet: MultichainAccountWallet; - group: MultichainAccountGroup; -}; - /** * The keys used to identify an account in the service state. */ @@ -71,6 +65,21 @@ export type ServiceState = { }; }; +enum CreateWalletFlow { + Restore = 'restore', + Import = 'import', + Create = 'create', +} + +type CreateWalletValidatedParams = + | { + flowType: CreateWalletFlow.Restore; + password: string; + mnemonic: Uint8Array; + } + | { flowType: CreateWalletFlow.Import; mnemonic: Uint8Array } + | { flowType: CreateWalletFlow.Create; password: string }; + /** * Service to expose multichain accounts capabilities. */ @@ -293,34 +302,47 @@ export class MultichainAccountService { } /** - * Creates a new multichain account wallet by first creating a new vault and keyring. - * If just a password is provided, then a new vault and keyring are created with a randomly generated mnemonic. - * If a mnemonic and password are provided, then a new vault and keyring are created 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. + * Gets the validated create wallet parameters. * * @param options - Options. * @param options.mnemonic - The mnemonic to use to create the new wallet. * @param options.password - The password to encrypt the vault with. - * @throws If the mnemonic has already been imported. - * @returns The new multichain account wallet. + * @param options.flowType - The flow type to use to create the new wallet. + * @returns The validated create wallet parameters. */ - async createMultichainAccountWallet({ + #getValidatedCreateWalletParams({ mnemonic, password, + flowType, }: { mnemonic?: Uint8Array; - password: string; - }): Promise>> { - if (!password) { - throw new Error('A password must be provided for this method.'); + password?: string; + flowType: CreateWalletFlow; + }) { + if (flowType === CreateWalletFlow.Restore && password && mnemonic) { + return { + password, + mnemonic, + flowType: CreateWalletFlow.Restore as const, + }; + } + + if (flowType === CreateWalletFlow.Import && mnemonic) { + return { mnemonic, flowType: CreateWalletFlow.Import as const }; + } + + if (flowType === CreateWalletFlow.Create && password) { + return { password, flowType: CreateWalletFlow.Create as const }; } - let wallet: MultichainAccountWallet>; + throw new Error('Invalid create wallet parameters.'); + } - log(`Creating new wallet...`); - if (mnemonic) { + async #createWalletByImport( + mnemonic: Uint8Array, + ): Promise>> { + log(`Creating new wallet by importing an existing mnemonic...`); + try { const existingKeyrings = this.#messenger.call( 'KeyringController:getKeyringsByType', KeyringTypes.hd, @@ -344,33 +366,132 @@ export class MultichainAccountService { KeyringTypes.hd, { mnemonic }, ); - } else { - try { - await this.#messenger.call( - 'KeyringController:createNewVaultAndKeychain', - password, - ); - // we're sure to get the correct keyring as it's the only one at this point - const entropySourceId = (await this.#messenger.call( - 'KeyringController:withKeyring', - { type: KeyringTypes.hd }, - async ({ metadata }) => { - return metadata.id; - }, - )) as string; - - // createNewVaultAndKeychain creates an account on the newly created keyring - // we don't need to worry about not having this account in the service state yet - // because the discovery process will store it in state through the alignment in the end. - wallet = new MultichainAccountWallet({ - providers: this.#providers, - entropySource: entropySourceId, - messenger: this.#messenger, - }); - } catch { - throw new Error('Failed to create a new vault and keychain.'); - } + // The wallet is ripe for discovery + return new MultichainAccountWallet({ + providers: this.#providers, + entropySource: result.id, + messenger: this.#messenger, + }); + } catch { + throw new Error( + 'Failed to create wallet by importing an existing mnemonic.', + ); + } + } + + async #createWalletByNewVault( + password: string, + ): Promise>> { + log(`Creating new wallet by creating a new vault and keychain...`); + try { + await this.#messenger.call( + 'KeyringController:createNewVaultAndKeychain', + password, + ); + + const entropySourceId = (await this.#messenger.call( + 'KeyringController:withKeyring', + { type: KeyringTypes.hd }, + async ({ metadata }) => { + return metadata.id; + }, + )) as string; + + // The wallet is ripe for discovery + return new MultichainAccountWallet({ + providers: this.#providers, + entropySource: entropySourceId, + messenger: this.#messenger, + }); + } catch { + throw new Error( + 'Failed to create wallet by creating a new vault and keychain.', + ); + } + } + + async #createWalletByRestore( + password: string, + mnemonic: Uint8Array, + ): Promise>> { + log(`Creating new wallet by restoring vault and keyring...`); + try { + await this.#messenger.call( + 'KeyringController:createNewVaultAndRestore', + password, + mnemonic, + ); + + const entropySourceId = (await this.#messenger.call( + 'KeyringController:withKeyring', + { type: KeyringTypes.hd }, + async ({ metadata }) => { + return metadata.id; + }, + )) as string; + + // The wallet is ripe for discovery + return new MultichainAccountWallet({ + providers: this.#providers, + entropySource: entropySourceId, + messenger: this.#messenger, + }); + } catch { + throw new Error( + 'Failed to create wallet by restoring a vault and keyring.', + ); + } + } + + /** + * Creates a new multichain account wallet by first creating a new vault and keyring. + * If just a password is provided, then a new vault and keyring are created with a randomly generated mnemonic. + * If a mnemonic and password are provided, then a new vault and keyring are created 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. + * @param options.password - The password to encrypt the vault with. + * @param options.flowType - The flow type to use to create the new wallet. + * @throws If the mnemonic has already been imported. + * @returns The new multichain account wallet. + */ + async createMultichainAccountWallet({ + mnemonic, + password, + flowType, + }: { + mnemonic?: Uint8Array; + password?: string; + flowType: CreateWalletFlow; + }): Promise>> { + const params: CreateWalletValidatedParams = + this.#getValidatedCreateWalletParams({ + mnemonic, + password, + flowType, + }); + + let wallet: + | MultichainAccountWallet> + | undefined; + + if (params.flowType === CreateWalletFlow.Import) { + wallet = await this.#createWalletByImport(params.mnemonic); + } else if (flowType === CreateWalletFlow.Create) { + wallet = await this.#createWalletByNewVault(params.password); + } else if (params.flowType === CreateWalletFlow.Restore) { + wallet = await this.#createWalletByRestore( + params.password, + params.mnemonic, + ); + } + + if (!wallet) { + throw new Error('Failed to create wallet.'); } this.#wallets.set(wallet.id, wallet); From 7d805efbe1a18a6c0225a211f83cb8135449a9b1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 03:37:50 -0400 Subject: [PATCH 13/37] chore: update JSDoc --- .../src/MultichainAccountService.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 66ff787f483..5c388abb2cd 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -445,9 +445,8 @@ export class MultichainAccountService { } /** - * Creates a new multichain account wallet by first creating a new vault and keyring. - * If just a password is provided, then a new vault and keyring are created with a randomly generated mnemonic. - * If a mnemonic and password are provided, then a new vault and keyring are created with the given mnemonic. + * Creates a new multichain account wallet by either importing an existing mnemonic, + * creating a new vault and keychain, or restoring a vault and keyring. * * 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. From 6b6e776263dbd10df89f2d259d6dbd2b70c8098d Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 03:45:31 -0400 Subject: [PATCH 14/37] chore: update log messages --- .../multichain-account-service/src/MultichainAccountGroup.ts | 3 +-- .../src/MultichainAccountService.ts | 1 - .../src/MultichainAccountWallet.ts | 5 +---- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index d825e3b9240..af1de7196b6 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -45,7 +45,6 @@ export class MultichainAccountGroup< readonly #log: Logger; - // eslint-disable-next-line @typescript-eslint/prefer-readonly #initialized = false; constructor({ @@ -106,7 +105,7 @@ export class MultichainAccountGroup< ); } - this.#log('Synchronized'); + this.#log('Finished initializing group state...'); } /** diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 5c388abb2cd..540c9fa1c26 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -7,7 +7,6 @@ import type { Bip44Account, } from '@metamask/account-api'; import type { HdKeyring } from '@metamask/eth-hd-keyring'; -import { mnemonicPhraseToBytes } from '@metamask/key-tree'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index e6b9dc5057b..031ff0a5eb5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -72,7 +72,6 @@ export class MultichainAccountWallet< readonly #log: Logger; - // eslint-disable-next-line @typescript-eslint/prefer-readonly #initialized = false; #status: MultichainAccountWalletStatus; @@ -126,7 +125,7 @@ export class MultichainAccountWallet< this.#status = 'ready'; } - this.#log('Synchronized'); + this.#log('Finished initializing wallet state...'); } /** @@ -483,8 +482,6 @@ export class MultichainAccountWallet< context.accounts = context.accounts.concat(accounts); - const providerName = context.provider.getName(); - addDiscoveryResultToState(accounts, providerName, targetGroupIndex); const nextGroupIndex = targetGroupIndex + 1; From 33e5e348459e1fe7820843c88d6d1c217c594c2b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 04:25:34 -0400 Subject: [PATCH 15/37] chore: add more JSDocs --- .../src/MultichainAccountService.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 540c9fa1c26..ea0bfa61334 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -337,6 +337,12 @@ export class MultichainAccountService { throw new Error('Invalid create wallet parameters.'); } + /** + * Creates a new multichain account wallet by importing an existing mnemonic. + * + * @param mnemonic - The mnemonic to use to create the new wallet. + * @returns The new multichain account wallet. + */ async #createWalletByImport( mnemonic: Uint8Array, ): Promise>> { @@ -379,6 +385,12 @@ export class MultichainAccountService { } } + /** + * Creates a new multichain account wallet by creating a new vault and keychain. + * + * @param password - The password to encrypt the vault with. + * @returns The new multichain account wallet. + */ async #createWalletByNewVault( password: string, ): Promise>> { @@ -410,6 +422,13 @@ export class MultichainAccountService { } } + /** + * Creates a new multichain account wallet by restoring a vault and keyring. + * + * @param password - The password to encrypt the vault with. + * @param mnemonic - The mnemonic to use to restore the new wallet. + * @returns The new multichain account wallet. + */ async #createWalletByRestore( password: string, mnemonic: Uint8Array, From f4d79c06da3092695d61099cc43f6d020a6d5209 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 16:48:48 -0400 Subject: [PATCH 16/37] test: update multichain service tests --- .../src/MultichainAccountService.test.ts | 492 +++++++----------- .../src/tests/messenger.ts | 2 + .../src/tests/providers.ts | 16 +- 3 files changed, 209 insertions(+), 301 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index dd72b2c2170..f446b001be4 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -1,13 +1,17 @@ /* eslint-disable jsdoc/require-jsdoc */ import type { Messenger } from '@metamask/base-controller'; +import { HdKeyring } from '@metamask/eth-hd-keyring'; import { mnemonicPhraseToBytes } from '@metamask/key-tree'; import type { KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType, SolAccountType } from '@metamask/keyring-api'; -import { KeyringTypes, type KeyringObject } from '@metamask/keyring-controller'; +import { KeyringMetadata, KeyringTypes, type KeyringObject } from '@metamask/keyring-controller'; import type { MultichainAccountServiceOptions } from './MultichainAccountService'; -import { MultichainAccountService } from './MultichainAccountService'; +import { + CreateWalletFlow, + MultichainAccountService, +} from './MultichainAccountService'; import type { NamedAccountProvider } from './providers'; import { AccountProviderWrapper } from './providers/AccountProviderWrapper'; import { @@ -35,7 +39,6 @@ import { getMultichainAccountServiceMessenger, getRootMessenger, makeMockAccountProvider, - mockAsInternalAccount, setupNamedAccountProvider, } from './tests'; import type { @@ -45,6 +48,7 @@ import type { MultichainAccountServiceEvents, MultichainAccountServiceMessenger, } from './types'; +import { EthKeyring } from '@metamask/keyring-internal-api'; // Mock providers. jest.mock('./providers/EvmAccountProvider', () => { @@ -66,6 +70,9 @@ type Mocks = { getState: jest.Mock; getKeyringsByType: jest.Mock; addNewKeyring: jest.Mock; + createNewVaultAndKeychain: jest.Mock; + createNewVaultAndRestore: jest.Mock; + withKeyring: jest.Mock; }; AccountsController: { listMultichainAccounts: jest.Mock; @@ -78,7 +85,6 @@ function mockAccountProvider( providerClass: new (messenger: MultichainAccountServiceMessenger) => Provider, mocks: MockAccountProvider, accounts: KeyringAccount[], - type: KeyringAccount['type'], ) { jest.mocked(providerClass).mockImplementation((...args) => { mocks.constructor(...args); @@ -88,8 +94,20 @@ function mockAccountProvider( setupNamedAccountProvider({ mocks, accounts, - filter: (account) => account.type === type, }); + + // Provide stable provider name and compatibility logic for grouping + if (providerClass === (EvmAccountProvider as unknown)) { + mocks.getName.mockReturnValue(EVM_ACCOUNT_PROVIDER_NAME); + mocks.isAccountCompatible?.mockImplementation( + (account: KeyringAccount) => account.type === EthAccountType.Eoa, + ); + } else if (providerClass === (SolAccountProvider as unknown)) { + mocks.getName.mockReturnValue(SOL_ACCOUNT_PROVIDER_NAME); + mocks.isAccountCompatible?.mockImplementation( + (account: KeyringAccount) => account.type === SolAccountType.DataAccount, + ); + } } function setup({ @@ -120,6 +138,9 @@ function setup({ getState: jest.fn(), getKeyringsByType: jest.fn(), addNewKeyring: jest.fn(), + createNewVaultAndKeychain: jest.fn(), + createNewVaultAndRestore: jest.fn(), + withKeyring: jest.fn(), }, AccountsController: { listMultichainAccounts: jest.fn(), @@ -148,6 +169,16 @@ function setup({ mocks.KeyringController.addNewKeyring, ); + messenger.registerActionHandler( + 'KeyringController:createNewVaultAndKeychain', + mocks.KeyringController.createNewVaultAndKeychain, + ); + + messenger.registerActionHandler( + 'KeyringController:createNewVaultAndRestore', + mocks.KeyringController.createNewVaultAndRestore, + ); + if (accounts) { mocks.AccountsController.listMultichainAccounts.mockImplementation( () => accounts, @@ -167,13 +198,11 @@ function setup({ EvmAccountProvider, mocks.EvmAccountProvider, accounts, - EthAccountType.Eoa, ); mockAccountProvider( SolAccountProvider, mocks.SolAccountProvider, accounts, - SolAccountType.DataAccount, ); } @@ -399,263 +428,7 @@ describe('MultichainAccountService', () => { }); }); - describe('getAccountContext', () => { - const entropy1 = 'entropy-1'; - const entropy2 = 'entropy-2'; - - const account1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) - .withId('mock-id-1') - .withEntropySource(entropy1) - .withGroupIndex(0) - .get(); - const account2 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) - .withId('mock-id-2') - .withEntropySource(entropy1) - .withGroupIndex(1) - .get(); - const account3 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) - .withId('mock-id-3') - .withEntropySource(entropy2) - .withGroupIndex(0) - .get(); - - const keyring1 = { - type: KeyringTypes.hd, - accounts: [account1.address, account2.address], - metadata: { id: entropy1, name: '' }, - }; - const keyring2 = { - type: KeyringTypes.hd, - accounts: [account2.address], - metadata: { id: entropy2, name: '' }, - }; - - const keyrings: KeyringObject[] = [keyring1, keyring2]; - - it('gets the wallet and multichain account for a given account ID', () => { - const accounts = [account1, account2, account3]; - const { service } = setup({ accounts, keyrings }); - - const wallet1 = service.getMultichainAccountWallet({ - entropySource: entropy1, - }); - const wallet2 = service.getMultichainAccountWallet({ - entropySource: entropy2, - }); - - const [multichainAccount1, multichainAccount2] = - wallet1.getMultichainAccountGroups(); - const [multichainAccount3] = wallet2.getMultichainAccountGroups(); - - const walletAndMultichainAccount1 = service.getAccountContext( - account1.id, - ); - const walletAndMultichainAccount2 = service.getAccountContext( - account2.id, - ); - const walletAndMultichainAccount3 = service.getAccountContext( - account3.id, - ); - - // NOTE: We use `toBe` here, cause we want to make sure we use the same - // references with `get*` service's methods. - expect(walletAndMultichainAccount1?.wallet).toBe(wallet1); - expect(walletAndMultichainAccount1?.group).toBe(multichainAccount1); - - expect(walletAndMultichainAccount2?.wallet).toBe(wallet1); - expect(walletAndMultichainAccount2?.group).toBe(multichainAccount2); - - expect(walletAndMultichainAccount3?.wallet).toBe(wallet2); - expect(walletAndMultichainAccount3?.group).toBe(multichainAccount3); - }); - - it('syncs the appropriate wallet and update reverse mapping on AccountsController:accountAdded', () => { - const accounts = [account1, account3]; // No `account2` for now. - const { service, messenger, mocks } = setup({ accounts, keyrings }); - - const wallet1 = service.getMultichainAccountWallet({ - entropySource: entropy1, - }); - expect(wallet1.getMultichainAccountGroups()).toHaveLength(1); - - // Now we're adding `account2`. - mocks.EvmAccountProvider.accounts = [account1, account2]; - messenger.publish( - 'AccountsController:accountAdded', - mockAsInternalAccount(account2), - ); - expect(wallet1.getMultichainAccountGroups()).toHaveLength(2); - - const [multichainAccount1, multichainAccount2] = - wallet1.getMultichainAccountGroups(); - - const walletAndMultichainAccount1 = service.getAccountContext( - account1.id, - ); - const walletAndMultichainAccount2 = service.getAccountContext( - account2.id, - ); - - // NOTE: We use `toBe` here, cause we want to make sure we use the same - // references with `get*` service's methods. - expect(walletAndMultichainAccount1?.wallet).toBe(wallet1); - expect(walletAndMultichainAccount1?.group).toBe(multichainAccount1); - - expect(walletAndMultichainAccount2?.wallet).toBe(wallet1); - expect(walletAndMultichainAccount2?.group).toBe(multichainAccount2); - }); - - it('syncs the appropriate multichain account and update reverse mapping on AccountsController:accountAdded', () => { - const otherAccount1 = MockAccountBuilder.from(account2) - .withGroupIndex(0) - .get(); - - const accounts = [account1]; // No `otherAccount1` for now. - const { service, messenger, mocks } = setup({ accounts, keyrings }); - - const wallet1 = service.getMultichainAccountWallet({ - entropySource: entropy1, - }); - expect(wallet1.getMultichainAccountGroups()).toHaveLength(1); - - // Now we're adding `account2`. - mocks.EvmAccountProvider.accounts = [account1, otherAccount1]; - messenger.publish( - 'AccountsController:accountAdded', - mockAsInternalAccount(otherAccount1), - ); - // Still 1, that's the same multichain account, but a new "blockchain - // account" got added. - expect(wallet1.getMultichainAccountGroups()).toHaveLength(1); - - const [multichainAccount1] = wallet1.getMultichainAccountGroups(); - - const walletAndMultichainAccount1 = service.getAccountContext( - account1.id, - ); - const walletAndMultichainOtherAccount1 = service.getAccountContext( - otherAccount1.id, - ); - - // NOTE: We use `toBe` here, cause we want to make sure we use the same - // references with `get*` service's methods. - expect(walletAndMultichainAccount1?.wallet).toBe(wallet1); - expect(walletAndMultichainAccount1?.group).toBe(multichainAccount1); - - expect(walletAndMultichainOtherAccount1?.wallet).toBe(wallet1); - expect(walletAndMultichainOtherAccount1?.group).toBe(multichainAccount1); - }); - - it('emits multichainAccountGroupUpdated event when syncing existing group on account added', () => { - const otherAccount1 = MockAccountBuilder.from(account2) - .withGroupIndex(0) - .get(); - - const accounts = [account1]; // No `otherAccount1` for now. - const { messenger, mocks } = setup({ accounts, keyrings }); - const publishSpy = jest.spyOn(messenger, 'publish'); - - // Now we're adding `otherAccount1` to an existing group. - mocks.EvmAccountProvider.accounts = [account1, otherAccount1]; - messenger.publish( - 'AccountsController:accountAdded', - mockAsInternalAccount(otherAccount1), - ); - - // Should emit updated event for the existing group - expect(publishSpy).toHaveBeenCalledWith( - 'MultichainAccountService:multichainAccountGroupUpdated', - expect.any(Object), - ); - }); - - it('creates new detected wallets and update reverse mapping on AccountsController:accountAdded', () => { - const accounts = [account1, account2]; // No `account3` for now (associated with "Wallet 2"). - const { service, messenger, mocks } = setup({ - accounts, - keyrings: [keyring1], - }); - - const wallet1 = service.getMultichainAccountWallet({ - entropySource: entropy1, - }); - expect(wallet1.getMultichainAccountGroups()).toHaveLength(2); - - // No wallet 2 yet. - expect(() => - service.getMultichainAccountWallet({ entropySource: entropy2 }), - ).toThrow('Unknown wallet, no wallet matching this entropy source'); - - // Now we're adding `account3`. - mocks.KeyringController.keyrings = [keyring1, keyring2]; - mocks.EvmAccountProvider.accounts = [account1, account2, account3]; - messenger.publish( - 'AccountsController:accountAdded', - mockAsInternalAccount(account3), - ); - const wallet2 = service.getMultichainAccountWallet({ - entropySource: entropy2, - }); - expect(wallet2).toBeDefined(); - expect(wallet2.getMultichainAccountGroups()).toHaveLength(1); - - const [multichainAccount3] = wallet2.getMultichainAccountGroups(); - - const walletAndMultichainAccount3 = service.getAccountContext( - account3.id, - ); - - // NOTE: We use `toBe` here, cause we want to make sure we use the same - // references with `get*` service's methods. - expect(walletAndMultichainAccount3?.wallet).toBe(wallet2); - expect(walletAndMultichainAccount3?.group).toBe(multichainAccount3); - }); - - it('ignores non-BIP-44 accounts on AccountsController:accountAdded', () => { - const accounts = [account1]; - const { service, messenger } = setup({ accounts, keyrings }); - - const wallet1 = service.getMultichainAccountWallet({ - entropySource: entropy1, - }); - const oldMultichainAccounts = wallet1.getMultichainAccountGroups(); - expect(oldMultichainAccounts).toHaveLength(1); - expect(oldMultichainAccounts[0].getAccounts()).toHaveLength(1); - - // Now we're publishing a new account that is not BIP-44 compatible. - messenger.publish( - 'AccountsController:accountAdded', - mockAsInternalAccount(MOCK_SNAP_ACCOUNT_2), - ); - - const newMultichainAccounts = wallet1.getMultichainAccountGroups(); - expect(newMultichainAccounts).toHaveLength(1); - expect(newMultichainAccounts[0].getAccounts()).toHaveLength(1); - }); - - it('syncs the appropriate wallet and update reverse mapping on AccountsController:accountRemoved', () => { - const accounts = [account1, account2]; - const { service, messenger, mocks } = setup({ accounts, keyrings }); - - const wallet1 = service.getMultichainAccountWallet({ - entropySource: entropy1, - }); - expect(wallet1.getMultichainAccountGroups()).toHaveLength(2); - - // Now we're removing `account2`. - mocks.EvmAccountProvider.accounts = [account1]; - messenger.publish('AccountsController:accountRemoved', account2.id); - expect(wallet1.getMultichainAccountGroups()).toHaveLength(1); - - const walletAndMultichainAccount2 = service.getAccountContext( - account2.id, - ); - - expect(walletAndMultichainAccount2).toBeUndefined(); - }); - }); - - describe('createNextMultichainAccount', () => { + describe('createNextMultichainAccountGroup', () => { it('creates the next multichain account group', async () => { const mockEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) @@ -943,6 +716,8 @@ describe('MultichainAccountService', () => { it('creates a multichain account wallet with MultichainAccountService:createMultichainAccountWallet', async () => { const { messenger, mocks } = setup({ accounts: [], keyrings: [] }); + const mnemonic = mnemonicPhraseToBytes(MOCK_MNEMONIC); + mocks.KeyringController.getKeyringsByType.mockImplementationOnce( () => [], ); @@ -954,7 +729,7 @@ describe('MultichainAccountService', () => { const wallet = await messenger.call( 'MultichainAccountService:createMultichainAccountWallet', - { mnemonic: MOCK_MNEMONIC }, + { mnemonic, flowType: CreateWalletFlow.Import }, ); expect(wallet).toBeDefined(); @@ -1084,48 +859,175 @@ describe('MultichainAccountService', () => { }); describe('createMultichainAccountWallet', () => { - it('creates a new multichain account wallet with the given mnemonic', async () => { - const { mocks, service } = setup({ - accounts: [], - keyrings: [], + it('throws an error if the create wallet parameters are invalid', async () => { + const { service } = setup({ accounts: [], keyrings: [] }); + await expect(() => + service.createMultichainAccountWallet({ + flowType: CreateWalletFlow.Create, + }), + ).rejects.toThrow('Invalid create wallet parameters.'); + }); + + // it('throws an error if the wallet is not created', async () => { + // const { service, mocks, messenger } = setup({ + // accounts: [], + // keyrings: [], + // }); + // const password = 'password'; + // mocks.KeyringController.createNewVaultAndKeychain.mockImplementationOnce( + // () => { + // mocks.KeyringController.keyrings.push(MOCK_HD_KEYRING_1); + // }, + // ); + + // messenger.registerActionHandler( + // 'KeyringController:withKeyring', + // async (_, operation) => { + // const newKeyring = mocks.KeyringController.keyrings.find( + // (keyring) => keyring.type === KeyringTypes.hd, + // ) as KeyringObject; + // return operation({ + // keyring: {} as unknown as EthKeyring, + // metadata: newKeyring.metadata, + // }); + // }, + // ); + // await expect(() => + // service.createMultichainAccountWallet({ + // flowType: CreateWalletFlow.Create, + // password, + // }), + // ).rejects.toThrow('Failed to create wallet.'); + // }); + + describe('createWalletByImport', () => { + it('creates a new multichain account wallet by the import flow', async () => { + const { mocks, service } = setup({ + accounts: [], + keyrings: [], + }); + + const mnemonic = mnemonicPhraseToBytes(MOCK_MNEMONIC); + + mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ + {}, + ]); + + mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ + id: 'abc', + name: '', + })); + + const wallet = await service.createMultichainAccountWallet({ + mnemonic, + flowType: CreateWalletFlow.Import, + }); + + expect(wallet).toBeDefined(); + expect(wallet.entropySource).toBe('abc'); }); - mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ - {}, - ]); + it("throws an error if there's already an existing keyring from the same mnemonic", async () => { + const { service, mocks } = setup({ accounts: [], keyrings: [] }); - mocks.KeyringController.addNewKeyring.mockImplementationOnce(() => ({ - id: 'abc', - name: '', - })); + const mnemonic = mnemonicPhraseToBytes(MOCK_MNEMONIC); - const wallet = await service.createMultichainAccountWallet({ - mnemonic: MOCK_MNEMONIC, + mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ + { + mnemonic, + }, + ]); + + await expect( + service.createMultichainAccountWallet({ + mnemonic, + flowType: CreateWalletFlow.Import, + }), + ).rejects.toThrow( + 'This Secret Recovery Phrase has already been imported.', + ); + + // Ensure we did not attempt to create a new keyring when duplicate is detected + expect(mocks.KeyringController.addNewKeyring).not.toHaveBeenCalled(); }); + }); - expect(wallet).toBeDefined(); - expect(wallet.entropySource).toBe('abc'); + describe('createWalletByNewVault', () => { + it('creates a new multichain account wallet by the new vault flow', async () => { + const { service, mocks, messenger } = setup({ + accounts: [], + keyrings: [], + }); + + const password = 'password'; + + mocks.KeyringController.createNewVaultAndKeychain.mockImplementationOnce( + () => { + mocks.KeyringController.keyrings.push(MOCK_HD_KEYRING_1); + }, + ); + + messenger.registerActionHandler( + 'KeyringController:withKeyring', + async (_, operation) => { + const newKeyring = mocks.KeyringController.keyrings.find( + (keyring) => keyring.type === KeyringTypes.hd, + ) as KeyringObject; + return operation({ + keyring: {} as unknown as EthKeyring, + metadata: newKeyring.metadata, + }); + }, + ); + + const newWallet = await service.createMultichainAccountWallet({ + password, + flowType: CreateWalletFlow.Create, + }); + + expect(newWallet).toBeDefined(); + expect(newWallet.entropySource).toBe(MOCK_HD_KEYRING_1.metadata.id); + }); }); - it("throws an error if there's already an existing keyring from the same mnemonic", async () => { - const { service, mocks } = setup({ accounts: [], keyrings: [] }); + describe('createWalletByRestore', () => { + it('creates a new multichain account wallet by the restore flow', async () => { + const { service, mocks, messenger } = setup({ + accounts: [], + keyrings: [], + }); - const mnemonic = mnemonicPhraseToBytes(MOCK_MNEMONIC); + const mnemonic = mnemonicPhraseToBytes(MOCK_MNEMONIC); + const password = 'password'; - mocks.KeyringController.getKeyringsByType.mockImplementationOnce(() => [ - { - mnemonic, - }, - ]); + mocks.KeyringController.createNewVaultAndRestore.mockImplementationOnce( + () => { + mocks.KeyringController.keyrings.push(MOCK_HD_KEYRING_1); + }, + ); + + messenger.registerActionHandler( + 'KeyringController:withKeyring', + async (_, operation) => { + const newKeyring = mocks.KeyringController.keyrings.find( + (keyring) => keyring.type === KeyringTypes.hd, + ) as KeyringObject; + return operation({ + keyring: {} as unknown as EthKeyring, + metadata: newKeyring.metadata, + }); + }, + ); - await expect( - service.createMultichainAccountWallet({ mnemonic: MOCK_MNEMONIC }), - ).rejects.toThrow( - 'This Secret Recovery Phrase has already been imported.', - ); + const newWallet = await service.createMultichainAccountWallet({ + password, + mnemonic, + flowType: CreateWalletFlow.Restore, + }); - // Ensure we did not attempt to create a new keyring when duplicate is detected - expect(mocks.KeyringController.addNewKeyring).not.toHaveBeenCalled(); + expect(newWallet).toBeDefined(); + expect(newWallet.entropySource).toBe(MOCK_HD_KEYRING_1.metadata.id); + }); }); }); }); diff --git a/packages/multichain-account-service/src/tests/messenger.ts b/packages/multichain-account-service/src/tests/messenger.ts index 0eba196ed77..8698e550585 100644 --- a/packages/multichain-account-service/src/tests/messenger.ts +++ b/packages/multichain-account-service/src/tests/messenger.ts @@ -47,6 +47,8 @@ export function getMultichainAccountServiceMessenger( 'KeyringController:addNewKeyring', 'NetworkController:findNetworkClientIdByChainId', 'NetworkController:getNetworkClientById', + 'KeyringController:createNewVaultAndKeychain', + 'KeyringController:createNewVaultAndRestore', ], }); } diff --git a/packages/multichain-account-service/src/tests/providers.ts b/packages/multichain-account-service/src/tests/providers.ts index bf186ab3963..62119040399 100644 --- a/packages/multichain-account-service/src/tests/providers.ts +++ b/packages/multichain-account-service/src/tests/providers.ts @@ -1,16 +1,17 @@ /* eslint-disable jsdoc/require-jsdoc */ import type { Bip44Account } from '@metamask/account-api'; -import { isBip44Account } from '@metamask/account-api'; import type { KeyringAccount } from '@metamask/keyring-api'; export type MockAccountProvider = { accounts: KeyringAccount[]; + accountsList: KeyringAccount['id'][]; constructor: jest.Mock; getAccount: jest.Mock; getAccounts: jest.Mock; createAccounts: jest.Mock; discoverAccounts: jest.Mock; + addAccounts: jest.Mock; isAccountCompatible?: jest.Mock; getName: jest.Mock; }; @@ -18,13 +19,16 @@ export type MockAccountProvider = { export function makeMockAccountProvider( accounts: KeyringAccount[] = [], ): MockAccountProvider { + const accountsIds = accounts.map((account) => account.id); return { accounts, + accountsList: accountsIds, constructor: jest.fn(), getAccount: jest.fn(), getAccounts: jest.fn(), createAccounts: jest.fn(), discoverAccounts: jest.fn(), + addAccounts: jest.fn(), isAccountCompatible: jest.fn(), getName: jest.fn(), }; @@ -34,21 +38,18 @@ export function setupNamedAccountProvider({ name = 'Mocked Provider', accounts, mocks = makeMockAccountProvider(), - filter = () => true, }: { name?: string; mocks?: MockAccountProvider; accounts: KeyringAccount[]; - filter?: (account: KeyringAccount) => boolean; }): MockAccountProvider { // You can mock this and all other mocks will re-use that list // of accounts. mocks.accounts = accounts; + mocks.accountsList = accounts.map((account) => account.id); const getAccounts = () => - mocks.accounts.filter( - (account) => isBip44Account(account) && filter(account), - ); + mocks.accounts.filter((account) => mocks.accountsList.includes(account.id)); mocks.getName.mockImplementation(() => name); @@ -59,6 +60,9 @@ export function setupNamedAccountProvider({ getAccounts().find((account) => account.id === id), ); mocks.createAccounts.mockResolvedValue([]); + mocks.addAccounts.mockImplementation((ids: string[]) => + mocks.accountsList.push(...ids), + ); return mocks; } From 350aa9333572c19f557af37cefce5b45db617383 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 16:50:08 -0400 Subject: [PATCH 17/37] fix: address type errors, remove try/catch block to not swallow errors, remove redundant state assignment, use assert to ensure wallet existence after creation --- .../src/MultichainAccountService.ts | 155 ++++++++---------- 1 file changed, 67 insertions(+), 88 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index ea0bfa61334..28042d507c1 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -10,7 +10,7 @@ import type { HdKeyring } from '@metamask/eth-hd-keyring'; import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; -import { areUint8ArraysEqual } from '@metamask/utils'; +import { areUint8ArraysEqual, assert } from '@metamask/utils'; import { projectLogger as log } from './logger'; import type { MultichainAccountGroup } from './MultichainAccountGroup'; @@ -64,7 +64,7 @@ export type ServiceState = { }; }; -enum CreateWalletFlow { +export enum CreateWalletFlow { Restore = 'restore', Import = 'import', Create = 'create', @@ -217,6 +217,7 @@ export class MultichainAccountService { for (const account of accounts) { const keys = this.#getStateKeys(account); if (keys) { + serviceState[keys.entropySource][keys.groupIndex] ??= {}; serviceState[keys.entropySource][keys.groupIndex][keys.providerName] ??= []; // ok to cast here because at this point we know that the account is BIP-44 compatible @@ -347,42 +348,34 @@ export class MultichainAccountService { mnemonic: Uint8Array, ): Promise>> { log(`Creating new wallet by importing an existing mnemonic...`); - try { - const existingKeyrings = this.#messenger.call( - 'KeyringController:getKeyringsByType', - KeyringTypes.hd, - ) as HdKeyring[]; - - const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { - if (!keyring.mnemonic) { - return false; - } - return areUint8ArraysEqual(keyring.mnemonic, mnemonic); - }); - - if (alreadyHasImportedSrp) { - throw new Error( - 'This Secret Recovery Phrase has already been imported.', - ); + const existingKeyrings = this.#messenger.call( + 'KeyringController:getKeyringsByType', + KeyringTypes.hd, + ) as HdKeyring[]; + + const alreadyHasImportedSrp = existingKeyrings.some((keyring) => { + if (!keyring.mnemonic) { + return false; } + return areUint8ArraysEqual(keyring.mnemonic, mnemonic); + }); - const result = await this.#messenger.call( - 'KeyringController:addNewKeyring', - KeyringTypes.hd, - { mnemonic }, - ); - - // The wallet is ripe for discovery - return new MultichainAccountWallet({ - providers: this.#providers, - entropySource: result.id, - messenger: this.#messenger, - }); - } catch { - throw new Error( - 'Failed to create wallet by importing an existing 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 }, + ); + + // The wallet is ripe for discovery + return new MultichainAccountWallet({ + providers: this.#providers, + entropySource: result.id, + messenger: this.#messenger, + }); } /** @@ -395,31 +388,25 @@ export class MultichainAccountService { password: string, ): Promise>> { log(`Creating new wallet by creating a new vault and keychain...`); - try { - await this.#messenger.call( - 'KeyringController:createNewVaultAndKeychain', - password, - ); - - const entropySourceId = (await this.#messenger.call( - 'KeyringController:withKeyring', - { type: KeyringTypes.hd }, - async ({ metadata }) => { - return metadata.id; - }, - )) as string; + await this.#messenger.call( + 'KeyringController:createNewVaultAndKeychain', + password, + ); - // The wallet is ripe for discovery - return new MultichainAccountWallet({ - providers: this.#providers, - entropySource: entropySourceId, - messenger: this.#messenger, - }); - } catch { - throw new Error( - 'Failed to create wallet by creating a new vault and keychain.', - ); - } + const entropySourceId = (await this.#messenger.call( + 'KeyringController:withKeyring', + { type: KeyringTypes.hd }, + async ({ metadata }) => { + return metadata.id; + }, + )) as string; + + // The wallet is ripe for discovery + return new MultichainAccountWallet({ + providers: this.#providers, + entropySource: entropySourceId, + messenger: this.#messenger, + }); } /** @@ -434,32 +421,26 @@ export class MultichainAccountService { mnemonic: Uint8Array, ): Promise>> { log(`Creating new wallet by restoring vault and keyring...`); - try { - await this.#messenger.call( - 'KeyringController:createNewVaultAndRestore', - password, - mnemonic, - ); - - const entropySourceId = (await this.#messenger.call( - 'KeyringController:withKeyring', - { type: KeyringTypes.hd }, - async ({ metadata }) => { - return metadata.id; - }, - )) as string; + await this.#messenger.call( + 'KeyringController:createNewVaultAndRestore', + password, + mnemonic, + ); - // The wallet is ripe for discovery - return new MultichainAccountWallet({ - providers: this.#providers, - entropySource: entropySourceId, - messenger: this.#messenger, - }); - } catch { - throw new Error( - 'Failed to create wallet by restoring a vault and keyring.', - ); - } + const entropySourceId = (await this.#messenger.call( + 'KeyringController:withKeyring', + { type: KeyringTypes.hd }, + async ({ metadata }) => { + return metadata.id; + }, + )) as string; + + // The wallet is ripe for discovery + return new MultichainAccountWallet({ + providers: this.#providers, + entropySource: entropySourceId, + messenger: this.#messenger, + }); } /** @@ -498,7 +479,7 @@ export class MultichainAccountService { if (params.flowType === CreateWalletFlow.Import) { wallet = await this.#createWalletByImport(params.mnemonic); - } else if (flowType === CreateWalletFlow.Create) { + } else if (params.flowType === CreateWalletFlow.Create) { wallet = await this.#createWalletByNewVault(params.password); } else if (params.flowType === CreateWalletFlow.Restore) { wallet = await this.#createWalletByRestore( @@ -507,9 +488,7 @@ export class MultichainAccountService { ); } - if (!wallet) { - throw new Error('Failed to create wallet.'); - } + assert(wallet, 'Failed to create wallet.'); this.#wallets.set(wallet.id, wallet); From ce322e0a3a6748dc36ef6a8ca21f6083366f7934 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 16:51:42 -0400 Subject: [PATCH 18/37] fix: lint fixes --- .../src/MultichainAccountService.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index f446b001be4..6aa71570167 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -1,11 +1,11 @@ /* eslint-disable jsdoc/require-jsdoc */ import type { Messenger } from '@metamask/base-controller'; -import { HdKeyring } from '@metamask/eth-hd-keyring'; import { mnemonicPhraseToBytes } from '@metamask/key-tree'; import type { KeyringAccount } from '@metamask/keyring-api'; import { EthAccountType, SolAccountType } from '@metamask/keyring-api'; -import { KeyringMetadata, KeyringTypes, type KeyringObject } from '@metamask/keyring-controller'; +import { KeyringTypes, type KeyringObject } from '@metamask/keyring-controller'; +import type { EthKeyring } from '@metamask/keyring-internal-api'; import type { MultichainAccountServiceOptions } from './MultichainAccountService'; import { @@ -48,7 +48,7 @@ import type { MultichainAccountServiceEvents, MultichainAccountServiceMessenger, } from './types'; -import { EthKeyring } from '@metamask/keyring-internal-api'; + // Mock providers. jest.mock('./providers/EvmAccountProvider', () => { From 31a6d7127fd842b6db4d08c608308f657518e55a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 16:53:10 -0400 Subject: [PATCH 19/37] chore: remove commented test --- .../src/MultichainAccountService.test.ts | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 6aa71570167..0742656547c 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -868,38 +868,6 @@ describe('MultichainAccountService', () => { ).rejects.toThrow('Invalid create wallet parameters.'); }); - // it('throws an error if the wallet is not created', async () => { - // const { service, mocks, messenger } = setup({ - // accounts: [], - // keyrings: [], - // }); - // const password = 'password'; - // mocks.KeyringController.createNewVaultAndKeychain.mockImplementationOnce( - // () => { - // mocks.KeyringController.keyrings.push(MOCK_HD_KEYRING_1); - // }, - // ); - - // messenger.registerActionHandler( - // 'KeyringController:withKeyring', - // async (_, operation) => { - // const newKeyring = mocks.KeyringController.keyrings.find( - // (keyring) => keyring.type === KeyringTypes.hd, - // ) as KeyringObject; - // return operation({ - // keyring: {} as unknown as EthKeyring, - // metadata: newKeyring.metadata, - // }); - // }, - // ); - // await expect(() => - // service.createMultichainAccountWallet({ - // flowType: CreateWalletFlow.Create, - // password, - // }), - // ).rejects.toThrow('Failed to create wallet.'); - // }); - describe('createWalletByImport', () => { it('creates a new multichain account wallet by the import flow', async () => { const { mocks, service } = setup({ From 4c485a6baf20ba4b647552b9619461d390d7d65b Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:46:52 -0400 Subject: [PATCH 20/37] refactor: make changes to messenger and providers to make the compatible with new changes --- packages/multichain-account-service/src/tests/messenger.ts | 1 + packages/multichain-account-service/src/tests/providers.ts | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/multichain-account-service/src/tests/messenger.ts b/packages/multichain-account-service/src/tests/messenger.ts index 8698e550585..de5452ceb53 100644 --- a/packages/multichain-account-service/src/tests/messenger.ts +++ b/packages/multichain-account-service/src/tests/messenger.ts @@ -49,6 +49,7 @@ export function getMultichainAccountServiceMessenger( 'NetworkController:getNetworkClientById', 'KeyringController:createNewVaultAndKeychain', 'KeyringController:createNewVaultAndRestore', + 'AccountsController:getAccounts', ], }); } diff --git a/packages/multichain-account-service/src/tests/providers.ts b/packages/multichain-account-service/src/tests/providers.ts index 62119040399..2ac1b148aae 100644 --- a/packages/multichain-account-service/src/tests/providers.ts +++ b/packages/multichain-account-service/src/tests/providers.ts @@ -19,10 +19,9 @@ export type MockAccountProvider = { export function makeMockAccountProvider( accounts: KeyringAccount[] = [], ): MockAccountProvider { - const accountsIds = accounts.map((account) => account.id); return { accounts, - accountsList: accountsIds, + accountsList: [], constructor: jest.fn(), getAccount: jest.fn(), getAccounts: jest.fn(), From c82de1766436bd3e75f0abef227a40d90e68d31e Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:48:38 -0400 Subject: [PATCH 21/37] refactor: simplify init and properly filter for rejected already aligned promise --- .../src/MultichainAccountGroup.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index af1de7196b6..8a12598ea51 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -82,17 +82,13 @@ export class MultichainAccountGroup< const accountIds = groupState[provider.getName()]; if (accountIds) { + this.#providerToAccounts.set(provider, accountIds); + // Add the accounts to the provider's internal list of account IDs + provider.addAccounts(accountIds); + for (const accountId of accountIds) { this.#accountToProvider.set(accountId, provider); } - const providerAccounts = this.#providerToAccounts.get(provider); - if (!providerAccounts) { - this.#providerToAccounts.set(provider, accountIds); - } else { - providerAccounts.push(...accountIds); - } - // Add the accounts to the provider's internal list of account IDs - provider.addAccounts(accountIds); } } @@ -270,7 +266,8 @@ export class MultichainAccountGroup< if (results.some((result) => result.status === 'rejected')) { const rejectedResults = results.filter( (result) => - result.status === 'rejected' && result.reason !== 'Already aligned', + result.status === 'rejected' && + result.reason.message !== 'Already aligned', ) as PromiseRejectedResult[]; const errors = rejectedResults .map((result) => `- ${result.reason}`) From 7d6e76ab893eb4b12152ef9369ab6a8f0abcb28c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:49:46 -0400 Subject: [PATCH 22/37] fix: properly initialize group state --- .../multichain-account-service/src/MultichainAccountWallet.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 031ff0a5eb5..6ec76f9a8e8 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -44,7 +44,7 @@ type AccountProviderDiscoveryContext< accounts: Account[]; }; -type WalletState = ServiceState[StateKeys['entropySource']]; +export type WalletState = ServiceState[StateKeys['entropySource']]; // type alias to make clear this state is generated by discovery type DiscoveredGroupsState = WalletState; @@ -434,6 +434,7 @@ export class MultichainAccountWallet< groupIndex: number, ) => { const accountIds = result.map((account) => account.id); + discoveredGroupsState[groupIndex] ??= {}; discoveredGroupsState[groupIndex][providerName] = accountIds; }; From 2d55dbdd68c16cd58f20bbf360d1b940a2ca3459 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:50:11 -0400 Subject: [PATCH 23/37] test: update multichain account group tests --- .../src/MultichainAccountGroup.test.ts | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts index 8ca6d7e4acf..d4674a20685 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts @@ -9,8 +9,12 @@ import type { Messenger } from '@metamask/base-controller'; import { EthScope, SolScope } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; -import { MultichainAccountGroup } from './MultichainAccountGroup'; +import { + type GroupState, + MultichainAccountGroup, +} from './MultichainAccountGroup'; import { MultichainAccountWallet } from './MultichainAccountWallet'; +import type { BaseBip44AccountProvider } from './providers'; import type { MockAccountProvider } from './tests'; import { MOCK_SNAP_ACCOUNT_2, @@ -54,23 +58,40 @@ function setup({ group: MultichainAccountGroup>; providers: MockAccountProvider[]; } { - const providers = accounts.map((providerAccounts) => { - return setupNamedAccountProvider({ accounts: providerAccounts }); + const providers = accounts.map((providerAccounts, idx) => { + return setupNamedAccountProvider({ + name: `Provider ${idx + 1}`, + accounts: providerAccounts, + }); }); const wallet = new MultichainAccountWallet>({ entropySource: MOCK_WALLET_1_ENTROPY_SOURCE, messenger: getMultichainAccountServiceMessenger(messenger), - providers, + providers: providers as unknown as BaseBip44AccountProvider[], }); const group = new MultichainAccountGroup({ wallet, groupIndex, - providers, + providers: providers as unknown as BaseBip44AccountProvider[], messenger: getMultichainAccountServiceMessenger(messenger), }); + // Initialize group state from provided accounts so that constructor tests + // observe accounts immediately + const groupState = providers.reduce((state, provider, idx) => { + const ids = accounts[idx] + .filter((a) => 'options' in a && a.options?.entropy) + .map((a) => a.id); + if (ids.length > 0) { + state[provider.getName()] = ids; + } + return state; + }, {}); + + group.init(groupState); + return { wallet, group, providers }; } @@ -95,6 +116,10 @@ describe('MultichainAccount', () => { expect(group.type).toBe(AccountGroupType.MultichainAccount); expect(group.groupIndex).toBe(groupIndex); expect(group.wallet).toStrictEqual(wallet); + expect(group.hasAccounts()).toBe(true); + expect(group.getAccountIds()).toStrictEqual( + expectedAccounts.map((a) => a.id), + ); expect(group.getAccounts()).toHaveLength(expectedAccounts.length); expect(group.getAccounts()).toStrictEqual(expectedAccounts); }); @@ -177,6 +202,10 @@ describe('MultichainAccount', () => { ], }); + providers[1].createAccounts.mockResolvedValueOnce([ + MOCK_WALLET_1_SOL_ACCOUNT, + ]); + await group.alignAccounts(); expect(providers[0].createAccounts).not.toHaveBeenCalled(); @@ -237,9 +266,9 @@ describe('MultichainAccount', () => { providers[2].createAccounts.mockRejectedValueOnce( new Error('Provider 3: Unable to create accounts'), - ) + ); - await group.align(); + await group.alignAccounts(); expect(providers[0].createAccounts).not.toHaveBeenCalled(); expect(providers[1].createAccounts).toHaveBeenCalledWith({ From b8a87ff5bed302df3ca7f3dd215f5cb21980a03c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:50:24 -0400 Subject: [PATCH 24/37] test: update multichain account wallet tests --- .../src/MultichainAccountWallet.test.ts | 197 ++++++++---------- 1 file changed, 91 insertions(+), 106 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 9d7beb9dbca..661971cedc5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -12,10 +12,15 @@ import { EthAccountType, SolAccountType, type EntropySourceId, + KeyringAccountEntropyTypeOption, } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; -import { MultichainAccountWallet } from './MultichainAccountWallet'; +import { + type WalletState, + MultichainAccountWallet, +} from './MultichainAccountWallet'; +import type { BaseBip44AccountProvider } from './providers'; import type { MockAccountProvider } from './tests'; import { MOCK_HD_ACCOUNT_1, @@ -66,22 +71,46 @@ function setup({ providers: MockAccountProvider[]; messenger: MultichainAccountServiceMessenger; } { - providers ??= accounts.map((providerAccounts, i) => { - return setupNamedAccountProvider({ - name: `Mocked Provider ${i}`, - accounts: providerAccounts, - }); - }); + const providersList = (providers ?? + accounts.map((providerAccounts, i) => { + return setupNamedAccountProvider({ + name: `Mocked Provider ${i}`, + accounts: providerAccounts, + }); + })) as MockAccountProvider[]; const serviceMessenger = getMultichainAccountServiceMessenger(messenger); const wallet = new MultichainAccountWallet>({ entropySource, - providers, + providers: providersList as unknown as BaseBip44AccountProvider[], messenger: serviceMessenger, }); - return { wallet, providers, messenger: serviceMessenger }; + const walletState = accounts.reduce( + (state, providerAccounts, idx) => { + const providerName = providersList[idx].getName(); + for (const account of providerAccounts) { + if ( + 'options' in account && + account.options?.entropy?.type === + KeyringAccountEntropyTypeOption.Mnemonic + ) { + const groupIndexKey = account.options.entropy.groupIndex; + state[groupIndexKey] ??= {}; + const groupState = state[groupIndexKey]; + groupState[providerName] ??= []; + groupState[providerName].push(account.id); + } + } + return state; + }, + {}, + ); + + wallet.init(walletState); + + return { wallet, providers: providersList, messenger: serviceMessenger }; } describe('MultichainAccountWallet', () => { @@ -151,94 +180,6 @@ describe('MultichainAccountWallet', () => { }); }); - describe('sync', () => { - it('force sync wallet after account provider got new account', () => { - const mockEvmAccount = MOCK_WALLET_1_EVM_ACCOUNT; - const provider = setupNamedAccountProvider({ - accounts: [mockEvmAccount], - }); - const { wallet } = setup({ - providers: [provider], - }); - - expect(wallet.getMultichainAccountGroups()).toHaveLength(1); - expect(wallet.getAccountGroups()).toHaveLength(1); // We can still get "basic" groups too. - - // Add a new account for the next index. - provider.getAccounts.mockReturnValue([ - mockEvmAccount, - { - ...mockEvmAccount, - options: { - ...mockEvmAccount.options, - entropy: { - ...mockEvmAccount.options.entropy, - groupIndex: 1, - }, - }, - }, - ]); - - // Force sync, so the wallet will "find" a new multichain account. - wallet.sync(); - expect(wallet.getAccountGroups()).toHaveLength(2); - expect(wallet.getMultichainAccountGroups()).toHaveLength(2); - }); - - it('skips non-matching wallet during sync', () => { - const mockEvmAccount = MOCK_WALLET_1_EVM_ACCOUNT; - const provider = setupNamedAccountProvider({ - accounts: [mockEvmAccount], - }); - const { wallet } = setup({ - providers: [provider], - }); - - expect(wallet.getMultichainAccountGroups()).toHaveLength(1); - - // Add a new account for another index but not for this wallet. - provider.getAccounts.mockReturnValue([ - mockEvmAccount, - { - ...mockEvmAccount, - options: { - ...mockEvmAccount.options, - entropy: { - ...mockEvmAccount.options.entropy, - id: 'mock-unknown-entropy-id', - groupIndex: 1, - }, - }, - }, - ]); - - // Even if we have a new account, it's not for this wallet, so it should - // not create a new multichain account! - wallet.sync(); - expect(wallet.getMultichainAccountGroups()).toHaveLength(1); - }); - - it('cleans up old multichain account group during sync', () => { - const mockEvmAccount = MOCK_WALLET_1_EVM_ACCOUNT; - const provider = setupNamedAccountProvider({ - accounts: [mockEvmAccount], - }); - const { wallet } = setup({ - providers: [provider], - }); - - expect(wallet.getMultichainAccountGroups()).toHaveLength(1); - - // Account for index 0 got removed, thus, the multichain account for index 0 - // will also be removed. - provider.getAccounts.mockReturnValue([]); - - // We should not have any multichain account anymore. - wallet.sync(); - expect(wallet.getMultichainAccountGroups()).toHaveLength(0); - }); - }); - describe('createMultichainAccountGroup', () => { it('creates a multichain account group for a given index', async () => { const groupIndex = 1; @@ -298,30 +239,73 @@ describe('MultichainAccountWallet', () => { ); }); - it('fails to create an account group if any of the provider fails to create its account', async () => { + it('creates an account group if only some of the providers fail to create its account', async () => { const groupIndex = 1; + // Baseline accounts at index 0 for two providers const mockEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) .withGroupIndex(0) .get(); + const mockSolAccount = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); const { wallet, providers } = setup({ - accounts: [[mockEvmAccount]], // 1 provider + accounts: [[mockEvmAccount], [mockSolAccount]], // 2 providers }); - const [provider] = providers; - provider.createAccounts.mockRejectedValueOnce( + const [failingProvider, succeedingProvider] = providers; + + // Arrange: first provider fails, second succeeds creating one account at index 1 + failingProvider.createAccounts.mockRejectedValueOnce( new Error('Unable to create accounts'), ); + const mockNextSolAccount = MockAccountBuilder.from(mockSolAccount) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(groupIndex) + .get(); + + succeedingProvider.createAccounts.mockResolvedValueOnce([ + mockNextSolAccount, + ]); + + succeedingProvider.getAccounts.mockReturnValueOnce([mockNextSolAccount]); + succeedingProvider.getAccount.mockReturnValueOnce(mockNextSolAccount); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); - await expect( - wallet.createMultichainAccountGroup(groupIndex), - ).rejects.toThrow( - 'Unable to create multichain account group for index: 1', - ); + const group = await wallet.createMultichainAccountGroup(groupIndex); + + // Should warn about partial failure but still create the group expect(consoleSpy).toHaveBeenCalled(); + expect(group.groupIndex).toBe(groupIndex); + const internalAccounts = group.getAccounts(); + expect(internalAccounts).toHaveLength(1); + expect(internalAccounts[0]).toStrictEqual(mockNextSolAccount); + }); + + it('fails to create an account group if all providers fail to create their accounts', async () => { + const { wallet, providers } = setup({ + accounts: [[], []], + }); + + const [failingProvider1, failingProvider2] = providers; + + failingProvider1.createAccounts.mockRejectedValueOnce( + new Error('Unable to create accounts'), + ); + + failingProvider2.createAccounts.mockRejectedValueOnce( + new Error('Unable to create accounts'), + ); + + await expect(wallet.createMultichainAccountGroup(0)).rejects.toThrow( + 'Unable to create multichain account group for index: 0 due to provider failures:\n- Error: Unable to create accounts\n- Error: Unable to create accounts', + ); + + expect(wallet.getAccountGroups()).toHaveLength(0); }); }); @@ -373,6 +357,7 @@ describe('MultichainAccountWallet', () => { expect(internalAccounts).toHaveLength(2); // EVM + SOL. expect(internalAccounts[0].type).toBe(EthAccountType.Eoa); expect(internalAccounts[1].type).toBe(SolAccountType.DataAccount); + expect(wallet.getAccountGroups()).toHaveLength(2); }); }); From 92d1c3adb814c73c9defa2b2be535107f57b9887 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:50:54 -0400 Subject: [PATCH 25/37] test: update provider tests so that they are compatible with the new logic --- .../src/providers/EvmAccountProvider.test.ts | 65 +++++++++++++------ .../src/providers/SolAccountProvider.test.ts | 18 ++++- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index b05986d4c9b..b36c1c1e920 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -1,5 +1,6 @@ /* eslint-disable jsdoc/require-jsdoc */ import { publicToAddress } from '@ethereumjs/util'; +import { getUUIDFromAddressOfNormalAccount } from '@metamask/accounts-controller'; import type { Messenger } from '@metamask/base-controller'; import { type KeyringMetadata } from '@metamask/keyring-controller'; import type { @@ -21,7 +22,9 @@ import { MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2, MOCK_HD_KEYRING_1, + MOCK_SOL_ACCOUNT_1, MockAccountBuilder, + mockAsInternalAccount, } from '../tests'; import type { AllowedActions, @@ -30,9 +33,13 @@ import type { MultichainAccountServiceEvents, } from '../types'; -jest.mock('@ethereumjs/util', () => ({ - publicToAddress: jest.fn(), -})); +jest.mock('@ethereumjs/util', () => { + const actual = jest.requireActual('@ethereumjs/util'); + return { + ...actual, + publicToAddress: jest.fn(), + }; +}); function mockNextDiscoveryAddress(address: string) { jest.mocked(publicToAddress).mockReturnValue(createBytes(address as Hex)); @@ -143,22 +150,26 @@ function setup({ >; keyring: MockEthKeyring; mocks: { - getAccountByAddress: jest.Mock; mockProviderRequest: jest.Mock; + getAccount: jest.Mock; }; } { const keyring = new MockEthKeyring(accounts); messenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', + 'AccountsController:getAccounts', () => accounts, ); - const mockGetAccountByAddress = jest - .fn() - .mockImplementation((address: string) => - keyring.accounts.find((account) => account.address === address), + const getAccount = jest.fn().mockImplementation((id) => { + return keyring.accounts.find( + (account) => + account.id === id || + getUUIDFromAddressOfNormalAccount(account.address) === id, ); + }); + + messenger.registerActionHandler('AccountsController:getAccount', getAccount); const mockProviderRequest = jest.fn().mockImplementation(({ method }) => { if (method === 'eth_getTransactionCount') { @@ -167,11 +178,6 @@ function setup({ throw new Error(`Unknown method: ${method}`); }); - messenger.registerActionHandler( - 'AccountsController:getAccountByAddress', - mockGetAccountByAddress, - ); - messenger.registerActionHandler( 'KeyringController:withKeyring', async (_, operation) => operation({ keyring, metadata: keyring.metadata }), @@ -206,8 +212,8 @@ function setup({ messenger, keyring, mocks: { - getAccountByAddress: mockGetAccountByAddress, mockProviderRequest, + getAccount, }, }; } @@ -228,12 +234,15 @@ describe('EvmAccountProvider', () => { }); it('gets a specific account', () => { - const account = MOCK_HD_ACCOUNT_1; + const customId = 'custom-id-123'; + const account = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withId(customId) + .get(); const { provider } = setup({ accounts: [account], }); - expect(provider.getAccount(account.id)).toStrictEqual(account); + expect(provider.getAccount(customId)).toStrictEqual(account); }); it('throws if account does not exist', () => { @@ -248,6 +257,22 @@ describe('EvmAccountProvider', () => { ); }); + it('returns true if an account is compatible', () => { + const account = MOCK_HD_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(true); + }); + + it('returns false if an account is not compatible', () => { + const account = MOCK_SOL_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(false); + }); + it('does not re-create accounts (idempotent)', async () => { const accounts = [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2]; const { provider } = setup({ @@ -268,8 +293,8 @@ describe('EvmAccountProvider', () => { accounts, }); - mocks.getAccountByAddress.mockReturnValue({ - ...MOCK_HD_ACCOUNT_1, + mocks.getAccount.mockReturnValue({ + ...mockAsInternalAccount(MOCK_HD_ACCOUNT_1), options: {}, // No options, so it cannot be BIP-44 compatible. }); @@ -300,7 +325,7 @@ describe('EvmAccountProvider', () => { }); // Simulate an account not found. - mocks.getAccountByAddress.mockImplementation(() => undefined); + mocks.getAccount.mockImplementation(() => undefined); await expect( provider.createAccounts({ diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index ccb380a4b74..13d56bd8b87 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -116,7 +116,7 @@ function setup({ const keyring = new MockSolanaKeyring(accounts); messenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', + 'AccountsController:getAccounts', () => accounts, ); @@ -196,6 +196,22 @@ describe('SolAccountProvider', () => { ); }); + it('returns true if an account is compatible', () => { + const account = MOCK_SOL_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(true); + }); + + it('returns false if an account is not compatible', () => { + const account = MOCK_HD_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(false); + }); + it('creates accounts', async () => { const accounts = [MOCK_SOL_ACCOUNT_1]; const { provider, keyring } = setup({ From 11cc1313a976bfdad7cfd3a958e74058e21e7ef7 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 19:54:36 -0400 Subject: [PATCH 26/37] test: update btc and trx tests to account for new logic --- .../src/providers/BtcAccountProvider.test.ts | 18 +++++++++++++++++- .../src/providers/TrxAccountProvider.test.ts | 18 +++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts index b50618154e7..24798994895 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts @@ -133,7 +133,7 @@ function setup({ const keyring = new MockBtcKeyring(accounts); messenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', + 'AccountsController:getAccounts', () => accounts, ); @@ -213,6 +213,22 @@ describe('BtcAccountProvider', () => { ); }); + it('returns true if an account is compatible', () => { + const account = MOCK_BTC_P2TR_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(true); + }); + + it('returns false if an account is not compatible', () => { + const account = MOCK_HD_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(false); + }); + it('creates accounts', async () => { const accounts = [MOCK_BTC_P2TR_ACCOUNT_1, MOCK_BTC_P2WPKH_ACCOUNT_1]; const { provider, keyring } = setup({ diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts index d52aaa25f95..5497b6bd6e6 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts @@ -116,7 +116,7 @@ function setup({ const keyring = new MockTronKeyring(accounts); messenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', + 'AccountsController:getAccounts', () => accounts, ); @@ -196,6 +196,22 @@ describe('TrxAccountProvider', () => { ); }); + it('returns true if an account is compatible', () => { + const account = MOCK_TRX_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(true); + }); + + it('returns false if an account is not compatible', () => { + const account = MOCK_HD_ACCOUNT_1; + const { provider } = setup({ + accounts: [account], + }); + expect(provider.isAccountCompatible(account)).toBe(false); + }); + it('creates accounts', async () => { const accounts = [MOCK_TRX_ACCOUNT_1]; const { provider, keyring } = setup({ From d28a3247cdbe1ce83b366afda15db02e0a86dc1f Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 20:35:14 -0400 Subject: [PATCH 27/37] chore: update changelogs --- packages/accounts-controller/CHANGELOG.md | 5 +++++ packages/keyring-controller/CHANGELOG.md | 5 +++++ .../multichain-account-service/CHANGELOG.md | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index 19f79cb4f55..6fdfacb59b3 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add a `getAccounts` method (and its associated action) that is the plural version of `getAccount` ([#6654](https://github.com/MetaMask/core/pull/6708)) + - This method is added to primarily be consumed in the `MultichainAccountService`. + ### Changed - Bump `@metamask/utils` from `^11.4.2` to `^11.8.1` ([#6588](https://github.com/MetaMask/core/pull/6588), [#6708](https://github.com/MetaMask/core/pull/6708)) diff --git a/packages/keyring-controller/CHANGELOG.md b/packages/keyring-controller/CHANGELOG.md index 7cc14c7d04f..5d6043655e3 100644 --- a/packages/keyring-controller/CHANGELOG.md +++ b/packages/keyring-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add actions for `createNewVaultAndKeychain` and `createNewVaultAndRestore` ([#6654](https://github.com/MetaMask/core/pull/6708)) + - These actions are meant to to be consumed by the `MultichainAccountService` in its `createMultichainAccountWallet` method. + ### Changed - Bump `@metamask/utils` from `^11.4.2` to `^11.8.1` ([#6588](https://github.com/MetaMask/core/pull/6588), [#6708](https://github.com/MetaMask/core/pull/6708)) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 354a6940c41..a8796a1ebf3 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -10,10 +10,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `{Btc/Trx}AccountProvider` account providers ([#6662](https://github.com/MetaMask/core/pull/6662)) +- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) + - Add logic in the `createMultichainAccountWallet` method in `MultichainAccountService` so that it can handle all entry points: importing an SRP, recovering a vault and creating a new vault. + - Add a `getAccountIds` method which returns all the account ids pertaining to a group. + - Add an `addAccounts` method on the `BaseBip44AccountProvider` class which keeps track of all the account IDs that pertain to it. ### Changed - Bump `@metamask/utils` from `^11.8.0` to `^11.8.1` ([#6708](https://github.com/MetaMask/core/pull/6708)) +- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) + - The `MultichainAccountService` is refactored to construct a top level service state for its `init` function, this state is passed down to the `MultichainAccountWallet` and `MultichainAccountGroup` classes in slices for them to construct their internal states. + - Additional state is generated at the entry points where it needs to be updated i.e. `createMultichainAccountGroup`, `discoverAccounts` and `alignAccounts`. + - We no longer prevent group creation if some providers' `createAccounts` calls fail during group creation, only if they all fail. + - The `getAccounts` method in the `BaseBip44AccountProvider` class no longer relies on fetching the entire list of internal accounts from the `AccountsController`, instead it gets the specific accounts that it stores in its internal accounts list. + - The `EvmAccountProvider` no longer fetches from the `AccountController` to get an account for its ID, we deterministically get the associated account ID through `getUUIDFromAddressOfNormalAccount`. + - The `EvmAccountProvider` now uses the `getAccount` method from the `AccountsController` when fetching an account after account creation as it is more efficient. + +### Removed + +- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) + - Remove `#handleOnAccountAdded` and `#handleOnAccountRemoved` methods in `MultichainAccountService` due to internal state being updated within the service. + - Remove `getAccountContext` (and associated map) in the `MultichainAccountService` as the service no longer uses that method. + - Remove the `sync` method in favor of the sole `init` method for both `MultichainAccountWallet` and `MultichainAccountGroup`. ## [1.2.0] From f308e9f0379025d68e40fa8f82a1a806da247921 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 20:35:24 -0400 Subject: [PATCH 28/37] fix: lint fixes --- .../src/MultichainAccountService.test.ts | 1 - .../multichain-account-service/src/MultichainAccountWallet.ts | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 0742656547c..e8afae4f814 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -49,7 +49,6 @@ import type { MultichainAccountServiceMessenger, } from './types'; - // Mock providers. jest.mock('./providers/EvmAccountProvider', () => { return { diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 6ec76f9a8e8..2b0a79c3ed5 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -49,7 +49,6 @@ export type WalletState = ServiceState[StateKeys['entropySource']]; // type alias to make clear this state is generated by discovery type DiscoveredGroupsState = WalletState; - /** * A multichain account wallet that holds multiple multichain accounts (one multichain account per * group index). @@ -325,7 +324,7 @@ export class MultichainAccountWallet< ); } else if (providerFailures) { // We warn there's failures on some providers and thus misalignment, but we still create the group - const message = `Unable to create some accounts for group index: ${groupIndex}. Providers threw the following errors:${providerFailures}` + const message = `Unable to create some accounts for group index: ${groupIndex}. Providers threw the following errors:${providerFailures}`; console.warn(message); this.#log(`${WARNING_PREFIX} ${message}`); } From 8bc12375d1f1891dfa9ab94c5267c9ba63127851 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 20:48:57 -0400 Subject: [PATCH 29/37] test: add test for getAccounts --- .../src/AccountsController.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index eb95dcb199a..216e72e24e1 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -2777,6 +2777,26 @@ describe('AccountsController', () => { }); }); + describe('getAccounts', () => { + it('returns a list of accounts based on the given account IDs', () => { + const { accountsController } = setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockAccount2.id]: mockAccount2, + }, + selectedAccount: mockAccount.id, + }, + }, + }); + + const result = accountsController.getAccounts([mockAccount.id]); + + expect(result).toStrictEqual([mockAccount]); + }); + }); + describe('getSelectedAccount', () => { it.each([ { From 19cfdf365bdce3e0bb83c60d49963028e0bf88e7 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 26 Sep 2025 21:39:55 -0400 Subject: [PATCH 30/37] feat: register getAccounts message handler --- packages/accounts-controller/src/AccountsController.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 05c7102aba1..2bd0fb04af9 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -1321,6 +1321,11 @@ export class AccountsController extends BaseController< this.getAccount.bind(this), ); + this.messagingSystem.registerActionHandler( + `AccountsController:getAccounts`, + this.getAccounts.bind(this), + ); + this.messagingSystem.registerActionHandler( `AccountsController:updateAccountMetadata`, this.updateAccountMetadata.bind(this), From 82fece8d7307d2a4feefd76dc5bf4d2d15818255 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 27 Sep 2025 12:20:06 -0400 Subject: [PATCH 31/37] chore: update JSDoc --- .../multichain-account-service/src/MultichainAccountService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 28042d507c1..08ce86dd4cc 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -448,7 +448,7 @@ export class MultichainAccountService { * creating a new vault and keychain, or restoring a vault and keyring. * * 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. + * `discoverAccounts` 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. From c812a8249e28c49d1bcc45c69e95774b61d6df5a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 27 Sep 2025 12:23:17 -0400 Subject: [PATCH 32/37] refactor: use Object.entries in wallet init --- .../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 2b0a79c3ed5..133c4e24599 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -103,7 +103,7 @@ export class MultichainAccountWallet< */ init(walletState: WalletState) { this.#log('Initializing wallet state...'); - for (const groupIndex of Object.keys(walletState)) { + for (const [groupIndex, groupState] of Object.entries(walletState)) { // Have to convert to number because the state keys become strings when we construct the state object in the service const indexAsNumber = Number(groupIndex); const group = new MultichainAccountGroup({ @@ -115,7 +115,7 @@ export class MultichainAccountWallet< this.#log(`Creating new group for index ${indexAsNumber}...`); - group.init(walletState[groupIndex]); + group.init(groupState); this.#accountGroups.set(indexAsNumber, group); } From 91aa52e838eaa675dc9a3520d6fa79547550118a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 27 Sep 2025 13:07:00 -0400 Subject: [PATCH 33/37] refactor: simplified error logic and include provider names in error reporting --- .../src/MultichainAccountGroup.test.ts | 10 ++++----- .../src/MultichainAccountGroup.ts | 22 +++++++++---------- .../src/MultichainAccountWallet.test.ts | 2 +- .../src/MultichainAccountWallet.ts | 11 +++++----- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts index d4674a20685..108a903fa15 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts @@ -237,7 +237,7 @@ describe('MultichainAccount', () => { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); providers[1].createAccounts.mockRejectedValueOnce( - new Error('Provider 2: Unable to create accounts'), + new Error('Unable to create accounts'), ); await group.alignAccounts(); @@ -248,7 +248,7 @@ describe('MultichainAccount', () => { groupIndex, }); expect(consoleSpy).toHaveBeenCalledWith( - `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing. Provider threw the following error:\n- Error: Provider 2: Unable to create accounts`, + `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing. Provider threw the following error:\n- Provider 2: Unable to create accounts`, ); }); @@ -261,11 +261,11 @@ describe('MultichainAccount', () => { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); providers[1].createAccounts.mockRejectedValueOnce( - new Error('Provider 2: Unable to create accounts'), + new Error('Unable to create accounts'), ); providers[2].createAccounts.mockRejectedValueOnce( - new Error('Provider 3: Unable to create accounts'), + new Error('Unable to create accounts'), ); await group.alignAccounts(); @@ -280,7 +280,7 @@ describe('MultichainAccount', () => { groupIndex, }); expect(consoleSpy).toHaveBeenCalledWith( - `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing. Providers threw the following errors:\n- Error: Provider 2: Unable to create accounts\n- Error: Provider 3: Unable to create accounts`, + `Failed to fully align multichain account group for entropy ID: ${wallet.entropySource} and group index: ${groupIndex}, some accounts might be missing. Providers threw the following errors:\n- Provider 2: Unable to create accounts\n- Provider 3: Unable to create accounts`, ); }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index 8a12598ea51..9b026a219ef 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -251,11 +251,19 @@ export class MultichainAccountGroup< }), ); + let failureMessage = ''; + let failureCount = 0; const groupState = results.reduce((state, result, idx) => { if (result.status === 'fulfilled') { state[this.#providers[idx].getName()] = result.value.map( (account) => account.id, ); + } else if ( + result.status === 'rejected' && + result.reason.message !== 'Already aligned' + ) { + failureCount += 1; + failureMessage += `\n- ${this.#providers[idx].getName()}: ${result.reason.message}`; } return state; }, {}); @@ -263,17 +271,9 @@ export class MultichainAccountGroup< // Update group state this.init(groupState); - if (results.some((result) => result.status === 'rejected')) { - const rejectedResults = results.filter( - (result) => - result.status === 'rejected' && - result.reason.message !== 'Already aligned', - ) as PromiseRejectedResult[]; - const errors = rejectedResults - .map((result) => `- ${result.reason}`) - .join('\n'); - const hasMultipleFailures = rejectedResults.length > 1; - const message = `Failed to fully align multichain account group for entropy ID: ${this.wallet.entropySource} and group index: ${this.groupIndex}, some accounts might be missing. ${hasMultipleFailures ? 'Providers' : 'Provider'} threw the following ${hasMultipleFailures ? 'errors' : 'error'}:\n${errors}`; + if (failureCount > 0) { + const hasMultipleFailures = failureCount > 1; + const message = `Failed to fully align multichain account group for entropy ID: ${this.wallet.entropySource} and group index: ${this.groupIndex}, some accounts might be missing. ${hasMultipleFailures ? 'Providers' : 'Provider'} threw the following ${hasMultipleFailures ? 'errors' : 'error'}:${failureMessage}`; this.#log(`${WARNING_PREFIX} ${message}`); console.warn(message); diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 661971cedc5..eb02a69773a 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -302,7 +302,7 @@ describe('MultichainAccountWallet', () => { ); await expect(wallet.createMultichainAccountGroup(0)).rejects.toThrow( - 'Unable to create multichain account group for index: 0 due to provider failures:\n- Error: Unable to create accounts\n- Error: Unable to create accounts', + 'Unable to create multichain account group for index: 0 due to provider failures:\n- Mocked Provider 0: Unable to create accounts\n- Mocked Provider 1: Unable to create accounts', ); expect(wallet.getAccountGroups()).toHaveLength(0); diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 133c4e24599..c1a49e5f922 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -306,17 +306,18 @@ export class MultichainAccountWallet< ), ); - const everyProviderFailed = results.every( - (result) => result.status === 'rejected', - ); + let failureCount = 0; - const providerFailures = results.reduce((acc, result) => { + const providerFailures = results.reduce((acc, result, idx) => { if (result.status === 'rejected') { - acc += `\n- ${result.reason}`; + failureCount += 1; + acc += `\n- ${this.#providers[idx].getName()}: ${result.reason.message}`; } return acc; }, ''); + const everyProviderFailed = failureCount === this.#providers.length; + if (everyProviderFailed) { // We throw an error if there's a failure on every provider throw new Error( From 4b19ed534e49e676332f6b62df2b76f210e9d0b1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 27 Sep 2025 13:10:14 -0400 Subject: [PATCH 34/37] test: have account group creation test check actual failure message --- .../src/MultichainAccountWallet.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index eb02a69773a..d250549bbd1 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -279,7 +279,9 @@ describe('MultichainAccountWallet', () => { const group = await wallet.createMultichainAccountGroup(groupIndex); // Should warn about partial failure but still create the group - expect(consoleSpy).toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith( + `Unable to create some accounts for group index: ${groupIndex}. Providers threw the following errors:\n- Mocked Provider 0: Unable to create accounts`, + ); expect(group.groupIndex).toBe(groupIndex); const internalAccounts = group.getAccounts(); expect(internalAccounts).toHaveLength(1); From c12435a4cd23558af57489ab0d8b5da8da9c6924 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 9 Oct 2025 01:25:54 -0400 Subject: [PATCH 35/37] test: update tests --- .../src/MultichainAccountWallet.test.ts | 70 +++++++++++++------ .../src/MultichainAccountWallet.ts | 5 ++ .../src/providers/BtcAccountProvider.test.ts | 2 +- 3 files changed, 56 insertions(+), 21 deletions(-) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index d250549bbd1..f554b10c814 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -76,6 +76,7 @@ function setup({ return setupNamedAccountProvider({ name: `Mocked Provider ${i}`, accounts: providerAccounts, + index: i, }); })) as MockAccountProvider[]; @@ -181,41 +182,41 @@ describe('MultichainAccountWallet', () => { }); describe('createMultichainAccountGroup', () => { - it('creates a multichain account group for a given index', async () => { - const groupIndex = 1; - - const mockEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) - .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) - .withGroupIndex(0) - .get(); + it('creates a multichain account group for a given index (waitForAllProvidersToFinishCreatingAccounts = false)', async () => { + const groupIndex = 0; const { wallet, providers } = setup({ - accounts: [[mockEvmAccount]], // 1 provider + accounts: [[], []], // 1 provider }); - const [provider] = providers; - const mockNextEvmAccount = MockAccountBuilder.from(mockEvmAccount) + const [evmProvider, solProvider] = providers; + const mockNextEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) .withGroupIndex(groupIndex) .get(); // 1. Create the accounts for the new index and returns their IDs. - provider.createAccounts.mockResolvedValueOnce([mockNextEvmAccount]); + evmProvider.createAccounts.mockResolvedValueOnce([mockNextEvmAccount]); // 2. When the wallet creates a new multichain account group, it will query // all accounts for this given index (so similar to the one we just created). - provider.getAccounts.mockReturnValueOnce([mockNextEvmAccount]); + evmProvider.getAccounts.mockReturnValueOnce([mockNextEvmAccount]); // 3. Required when we call `getAccounts` (below) on the multichain account. - provider.getAccount.mockReturnValueOnce(mockNextEvmAccount); + evmProvider.getAccount.mockReturnValueOnce(mockNextEvmAccount); + + solProvider.createAccounts.mockResolvedValueOnce([MOCK_SOL_ACCOUNT_1]); + solProvider.getAccounts.mockReturnValueOnce([MOCK_SOL_ACCOUNT_1]); + solProvider.getAccount.mockReturnValueOnce(MOCK_SOL_ACCOUNT_1); const specificGroup = await wallet.createMultichainAccountGroup(groupIndex); expect(specificGroup.groupIndex).toBe(groupIndex); const internalAccounts = specificGroup.getAccounts(); - expect(internalAccounts).toHaveLength(1); + expect(internalAccounts).toHaveLength(2); expect(internalAccounts[0].type).toBe(EthAccountType.Eoa); + expect(internalAccounts[1].type).toBe(SolAccountType.DataAccount); }); - it('returns the same reference when re-creating using the same index', async () => { + it('returns the same reference when re-creating using the same index (waitForAllProvidersToFinishCreatingAccounts = false)', async () => { const { wallet } = setup({ accounts: [[MOCK_HD_ACCOUNT_1]], }); @@ -226,7 +227,7 @@ describe('MultichainAccountWallet', () => { expect(newGroup).toBe(group); }); - it('fails to create an account beyond the next index', async () => { + it('fails to create an account beyond the next index (waitForAllProvidersToFinishCreatingAccounts = false)', async () => { const { wallet } = setup({ accounts: [[MOCK_HD_ACCOUNT_1]], }); @@ -239,7 +240,7 @@ describe('MultichainAccountWallet', () => { ); }); - it('creates an account group if only some of the providers fail to create its account', async () => { + it('creates an account group if only some of the providers fail to create its account (waitForAllProvidersToFinishCreatingAccounts = true)', async () => { const groupIndex = 1; // Baseline accounts at index 0 for two providers @@ -276,7 +277,9 @@ describe('MultichainAccountWallet', () => { succeedingProvider.getAccount.mockReturnValueOnce(mockNextSolAccount); const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); - const group = await wallet.createMultichainAccountGroup(groupIndex); + const group = await wallet.createMultichainAccountGroup(groupIndex, { + waitForAllProvidersToFinishCreatingAccounts: true, + }); // Should warn about partial failure but still create the group expect(consoleSpy).toHaveBeenCalledWith( @@ -288,7 +291,7 @@ describe('MultichainAccountWallet', () => { expect(internalAccounts[0]).toStrictEqual(mockNextSolAccount); }); - it('fails to create an account group if all providers fail to create their accounts', async () => { + it('fails to create an account group if all providers fail to create their accounts (waitForAllProvidersToFinishCreatingAccounts = true)', async () => { const { wallet, providers } = setup({ accounts: [[], []], }); @@ -303,12 +306,39 @@ describe('MultichainAccountWallet', () => { new Error('Unable to create accounts'), ); - await expect(wallet.createMultichainAccountGroup(0)).rejects.toThrow( + await expect( + wallet.createMultichainAccountGroup(0, { + waitForAllProvidersToFinishCreatingAccounts: true, + }), + ).rejects.toThrow( 'Unable to create multichain account group for index: 0 due to provider failures:\n- Mocked Provider 0: Unable to create accounts\n- Mocked Provider 1: Unable to create accounts', ); expect(wallet.getAccountGroups()).toHaveLength(0); }); + + it('logs an error if a non-EVM provider fails to create its account (waitForAllProvidersToFinishCreatingAccounts = false)', async () => { + const { wallet, providers } = setup({ + accounts: [[], []], + }); + + const [succeedingProvider, failingProvider] = providers; + + succeedingProvider.createAccounts.mockResolvedValueOnce([ + MOCK_HD_ACCOUNT_1, + ]); + + failingProvider.createAccounts.mockRejectedValueOnce( + new Error('Unable to create accounts'), + ); + + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + await wallet.createMultichainAccountGroup(0); + + expect(consoleSpy).toHaveBeenCalledWith( + 'Unable to create some accounts for group index: 0 with provider "Mocked Provider 1". Error: Unable to create accounts', + ); + }); }); describe('createNextMultichainAccountGroup', () => { diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 0f3195f33fb..ec89c7c2100 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -371,6 +371,9 @@ export class MultichainAccountWallet< 'EVM account provider must be first', ); + // Create the group here because the EVM provider will not fail. + // There isn't a failure scenario here since this function is only used by createNextMultichainAccountGroup (no throw on gap error). + // We have to deterministically create the group here because otherwise we can't set the group in state. group = new MultichainAccountGroup({ wallet: this, providers: this.#providers, @@ -389,6 +392,7 @@ export class MultichainAccountWallet< }) .catch((error) => { const errorMessage = `Unable to create some accounts for group index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`; + console.warn(errorMessage); this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error); }); @@ -407,6 +411,7 @@ export class MultichainAccountWallet< .catch((error) => { // Log errors from background providers but don't fail the operation const errorMessage = `Unable to create some accounts for group index: ${groupIndex} with provider "${provider.getName()}". Error: ${(error as Error).message}`; + console.warn(errorMessage); this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error); }); }); diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts index 284816d9621..74a1c351307 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts @@ -214,7 +214,7 @@ describe('BtcAccountProvider', () => { }); it('returns true if an account is compatible', () => { - const account = MOCK_BTC_P2TR_ACCOUNT_1; + const account = MOCK_BTC_P2WPKH_ACCOUNT_1; const { provider } = setup({ accounts: [account], }); From e9ab58d89fdaf4d6eab35e060e41018b96b92f84 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 9 Oct 2025 01:41:29 -0400 Subject: [PATCH 36/37] chore: remove old comment --- .../multichain-account-service/src/MultichainAccountService.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 08ce86dd4cc..5f6cf2e4ada 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -220,7 +220,6 @@ export class MultichainAccountService { serviceState[keys.entropySource][keys.groupIndex] ??= {}; serviceState[keys.entropySource][keys.groupIndex][keys.providerName] ??= []; - // ok to cast here because at this point we know that the account is BIP-44 compatible serviceState[keys.entropySource][keys.groupIndex][ keys.providerName ].push(account.id); From 521a71ba456dabcca7211838fb5fa45d16bac1cb Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 9 Oct 2025 01:41:37 -0400 Subject: [PATCH 37/37] chore: update changelog --- .../multichain-account-service/CHANGELOG.md | 42 +++++++++++-------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index eef8dbf47f5..2ea3b36ce6c 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -7,6 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) + - Add logic in the `createMultichainAccountWallet` method in `MultichainAccountService` so that it can handle all entry points: importing an SRP, recovering a vault and creating a new vault. + - Add a `getAccountIds` method which returns all the account ids pertaining to a group. + - Add an `addAccounts` method on the `BaseBip44AccountProvider` class which keeps track of all the account IDs that pertain to it. + +### Changed + +- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) + - The `MultichainAccountService` is refactored to construct a top level service state for its `init` function, this state is passed down to the `MultichainAccountWallet` and `MultichainAccountGroup` classes in slices for them to construct their internal states. + - Additional state is generated at the entry points where it needs to be updated i.e. `createMultichainAccountGroup`, `discoverAccounts` and `alignAccounts`. + - We no longer prevent group creation if some providers' `createAccounts` calls fail during group creation, only if they all fail. + - The `getAccounts` method in the `BaseBip44AccountProvider` class no longer relies on fetching the entire list of internal accounts from the `AccountsController`, instead it gets the specific accounts that it stores in its internal accounts list. + - The `EvmAccountProvider` no longer fetches from the `AccountController` to get an account for its ID, we deterministically get the associated account ID through `getUUIDFromAddressOfNormalAccount`. + - The `EvmAccountProvider` now uses the `getAccount` method from the `AccountsController` when fetching an account after account creation as it is more efficient. + +### Removed + +- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) + - Remove `#handleOnAccountAdded` and `#handleOnAccountRemoved` methods in `MultichainAccountService` due to internal state being updated within the service. + - Remove `getAccountContext` (and associated map) in the `MultichainAccountService` as the service no longer uses that method. + - Remove the `sync` method in favor of the sole `init` method for both `MultichainAccountWallet` and `MultichainAccountGroup`. + ## [1.6.0] ### Changed @@ -33,28 +57,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `{Btc/Trx}AccountProvider` account providers ([#6662](https://github.com/MetaMask/core/pull/6662)) -- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) - - Add logic in the `createMultichainAccountWallet` method in `MultichainAccountService` so that it can handle all entry points: importing an SRP, recovering a vault and creating a new vault. - - Add a `getAccountIds` method which returns all the account ids pertaining to a group. - - Add an `addAccounts` method on the `BaseBip44AccountProvider` class which keeps track of all the account IDs that pertain to it. ### Changed - Bump `@metamask/utils` from `^11.8.0` to `^11.8.1` ([#6708](https://github.com/MetaMask/core/pull/6708)) -- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) - - The `MultichainAccountService` is refactored to construct a top level service state for its `init` function, this state is passed down to the `MultichainAccountWallet` and `MultichainAccountGroup` classes in slices for them to construct their internal states. - - Additional state is generated at the entry points where it needs to be updated i.e. `createMultichainAccountGroup`, `discoverAccounts` and `alignAccounts`. - - We no longer prevent group creation if some providers' `createAccounts` calls fail during group creation, only if they all fail. - - The `getAccounts` method in the `BaseBip44AccountProvider` class no longer relies on fetching the entire list of internal accounts from the `AccountsController`, instead it gets the specific accounts that it stores in its internal accounts list. - - The `EvmAccountProvider` no longer fetches from the `AccountController` to get an account for its ID, we deterministically get the associated account ID through `getUUIDFromAddressOfNormalAccount`. - - The `EvmAccountProvider` now uses the `getAccount` method from the `AccountsController` when fetching an account after account creation as it is more efficient. - -### Removed - -- **BREAKING** A performance refactor was made around all the classes in this package ([#6654](https://github.com/MetaMask/core/pull/6708)) - - Remove `#handleOnAccountAdded` and `#handleOnAccountRemoved` methods in `MultichainAccountService` due to internal state being updated within the service. - - Remove `getAccountContext` (and associated map) in the `MultichainAccountService` as the service no longer uses that method. - - Remove the `sync` method in favor of the sole `init` method for both `MultichainAccountWallet` and `MultichainAccountGroup`. ## [1.2.0]