diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 2c57c22256e..ae63071c1be 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1086,14 +1086,6 @@ "count": 7 } }, - "packages/multichain-account-service/src/utils.ts": { - "@typescript-eslint/explicit-function-return-type": { - "count": 1 - }, - "id-denylist": { - "count": 1 - } - }, "packages/multichain-api-middleware/src/handlers/types.ts": { "@typescript-eslint/naming-convention": { "count": 2 diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 2c5ceee4713..75a85c259ab 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -7,8 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add new `createMultichainAccountGroups` support to create multiple groups in batch ([#7801](https://github.com/MetaMask/core/pull/7801)) + ### Changed +- Optimize `EvmAccountProvider.createAccounts` for range operations ([#7801](https://github.com/MetaMask/core/pull/7801)) + - Batch account creation with single a `withKeyring` call for entire range instead of one call per account. + - Batch account creation with single `keyring.addAccounts` call. + - Fetch all accounts in single `AccountsController:getAccounts` call instead of multiple `getAccount` calls. + - Significantly reduces lock acquisitions and API calls for batch operations. - Bump `@metamask/accounts-controller` from `^36.0.0` to `^36.0.1` ([#7996](https://github.com/MetaMask/core/pull/7996)) ## [7.0.0] diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index b491a74f7e5..8f652b72683 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -19,7 +19,7 @@ import type { MultichainAccountWallet } from './MultichainAccountWallet'; import type { Bip44AccountProvider } from './providers'; import { isAccountProviderWrapper } from './providers'; import type { MultichainAccountServiceMessenger } from './types'; -import { createSentryError } from './utils'; +import { createSentryError, toErrorMessage } from './utils'; export type GroupState = ServiceState[StateKeys['entropySource']][StateKeys['groupIndex']]; @@ -108,15 +108,6 @@ export class MultichainAccountGroup< } } } - - if (this.#initialized) { - this.#messenger.publish( - 'MultichainAccountService:multichainAccountGroupUpdated', - this, - ); - } else { - this.#initialized = true; - } } /** @@ -128,6 +119,8 @@ export class MultichainAccountGroup< this.#log('Initializing group state...'); this.#setState(groupState); this.#log('Finished initializing group state...'); + + this.#initialized = true; } /** @@ -139,6 +132,13 @@ export class MultichainAccountGroup< this.#log('Updating group state...'); this.#setState(groupState); this.#log('Finished updating group state...'); + + if (this.#initialized) { + this.#messenger.publish( + 'MultichainAccountService:multichainAccountGroupUpdated', + this, + ); + } } /** @@ -298,9 +298,7 @@ export class MultichainAccountGroup< return accounts; } catch (error) { // istanbul ignore next - this.#log( - `${WARNING_PREFIX} ${error instanceof Error ? error.message : String(error)}`, - ); + this.#log(`${WARNING_PREFIX} ${toErrorMessage(error)}`); const sentryError = createSentryError( `Unable to align accounts with provider "${provider.getName()}"`, error as Error, diff --git a/packages/multichain-account-service/src/MultichainAccountService-method-action-types.ts b/packages/multichain-account-service/src/MultichainAccountService-method-action-types.ts index caf82281de1..03cbc011162 100644 --- a/packages/multichain-account-service/src/MultichainAccountService-method-action-types.ts +++ b/packages/multichain-account-service/src/MultichainAccountService-method-action-types.ts @@ -141,6 +141,19 @@ export type MultichainAccountServiceCreateMultichainAccountGroupAction = { handler: MultichainAccountService['createMultichainAccountGroup']; }; +/** + * Creates multiple multichain account groups up to maxGroupIndex. + * + * @param params - Parameters for creating account groups. + * @param params.entropySource - The entropy source ID. + * @param params.maxGroupIndex - Maximum group index to create (0 to maxGroupIndex inclusive). + * @returns Array of created multichain account groups. + */ +export type MultichainAccountServiceCreateMultichainAccountGroupsAction = { + type: `MultichainAccountService:createMultichainAccountGroups`; + handler: MultichainAccountService['createMultichainAccountGroups']; +}; + /** * Set basic functionality state and trigger alignment if enabled. * When basic functionality is disabled, snap-based providers are disabled. @@ -186,6 +199,7 @@ export type MultichainAccountServiceMethodActions = | MultichainAccountServiceGetMultichainAccountGroupsAction | MultichainAccountServiceCreateNextMultichainAccountGroupAction | MultichainAccountServiceCreateMultichainAccountGroupAction + | MultichainAccountServiceCreateMultichainAccountGroupsAction | MultichainAccountServiceSetBasicFunctionalityAction | MultichainAccountServiceAlignWalletsAction | MultichainAccountServiceAlignWalletAction; diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 474c2675bfb..55beff3b186 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -500,14 +500,34 @@ describe('MultichainAccountService', () => { .withGroupIndex(0) .get(); - const { service } = await setup({ accounts: [mockEvmAccount] }); + const { service, mocks } = await setup({ accounts: [mockEvmAccount] }); + + // Groups cannot be empty, we need mock the next account creation too. + const mockNextEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withUuid() + .withGroupIndex(1) + .get(); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + mockNextEvmAccount, + ]); const nextGroup = await service.createNextMultichainAccountGroup({ entropySource: MOCK_HD_KEYRING_1.metadata.id, }); expect(nextGroup.groupIndex).toBe(1); - // NOTE: There won't be any account for this group, since we're not - // mocking the providers. + + // We always fetch account objects from the provider, so we also have + // to mock this. + mocks.EvmAccountProvider.getAccount.mockReturnValueOnce( + mockNextEvmAccount, + ); + const accounts = nextGroup.getAccounts(); + expect(mocks.EvmAccountProvider.getAccount).toHaveBeenCalledWith( + mockNextEvmAccount.id, + ); + expect(accounts).toHaveLength(1); + expect(accounts[0]).toStrictEqual(mockNextEvmAccount); }); it('emits multichainAccountGroupCreated event when creating next group', async () => { @@ -516,11 +536,21 @@ describe('MultichainAccountService', () => { .withGroupIndex(0) .get(); - const { service, messenger } = await setup({ + const { service, messenger, mocks } = await setup({ accounts: [mockEvmAccount], }); const publishSpy = jest.spyOn(messenger, 'publish'); + // Groups cannot be empty, we need mock the next account creation too. + const mockNextEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withUuid() + .withGroupIndex(1) + .get(); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + mockNextEvmAccount, + ]); + const nextGroup = await service.createNextMultichainAccountGroup({ entropySource: MOCK_HD_KEYRING_1.metadata.id, }); @@ -572,11 +602,21 @@ describe('MultichainAccountService', () => { .withGroupIndex(0) .get(); - const { service, messenger } = await setup({ + const { service, messenger, mocks } = await setup({ accounts: [mockEvmAccount], }); const publishSpy = jest.spyOn(messenger, 'publish'); + // Groups cannot be empty, we need mock the next account creation too. + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withUuid() + .withGroupIndex(1) + .get(); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + mockEvmAccount1, + ]); + const group = await service.createMultichainAccountGroup({ entropySource: MOCK_HD_KEYRING_1.metadata.id, groupIndex: 1, @@ -589,6 +629,153 @@ describe('MultichainAccountService', () => { }); }); + describe('createMultichainAccountGroups', () => { + it('creates multiple multichain account groups up to maxGroupIndex', async () => { + // Start with group 0 existing to initialize the wallet. + const mockEvmAccount0 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount0 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + + const { service, mocks } = await setup({ + accounts: [mockEvmAccount0, mockSolAccount0], + }); + + // Mock accounts that will be returned when creating groups 1, 2. + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + const mockEvmAccount2 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(2) + .get(); + + const mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + const mockSolAccount2 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(2) + .get(); + + // Mock EVM provider to return new accounts for range 1-2. + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + mockEvmAccount1, + mockEvmAccount2, + ]); + + // Mock SOL provider for new groups. + mocks.SolAccountProvider.createAccounts.mockResolvedValueOnce([ + mockSolAccount1, + ]); + mocks.SolAccountProvider.createAccounts.mockResolvedValueOnce([ + mockSolAccount2, + ]); + + const groups = await service.createMultichainAccountGroups({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + maxGroupIndex: 2, + }); + + expect(groups).toHaveLength(3); + expect(groups[0].groupIndex).toBe(0); // Existing group. + expect(groups[1].groupIndex).toBe(1); // New group. + expect(groups[2].groupIndex).toBe(2); // New group. + + // Verify EVM provider was called with range for new groups. + expect(mocks.EvmAccountProvider.createAccounts).toHaveBeenCalledWith({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 1, + to: 2, + }, + }); + }); + + it('creates multiple groups via messenger action handler', async () => { + const mockEvmAccount0 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + + const { messenger } = await setup({ + accounts: [mockEvmAccount0, mockEvmAccount1], + }); + + const groups = await messenger.call( + 'MultichainAccountService:createMultichainAccountGroups', + { + entropySource: MOCK_HD_KEYRING_1.metadata.id, + maxGroupIndex: 1, + }, + ); + + expect(groups).toHaveLength(2); + expect(groups[0].groupIndex).toBe(0); + expect(groups[1].groupIndex).toBe(1); + }); + + it('publishes multichainAccountGroupCreated events for each new group', async () => { + // Start with group 0 existing to initialize the wallet. + const mockEvmAccount0 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount0 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + + const { service, messenger, mocks } = await setup({ + accounts: [mockEvmAccount0, mockSolAccount0], + }); + + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + const mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + + // Mock EVM provider to return account for group 1. + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + mockEvmAccount1, + ]); + + // Mock SOL provider for group 1. + mocks.SolAccountProvider.createAccounts.mockResolvedValueOnce([ + mockSolAccount1, + ]); + + const publishSpy = jest.spyOn(messenger, 'publish'); + + await service.createMultichainAccountGroups({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + maxGroupIndex: 1, + }); + + // Should publish event for the new group (group 1). + // Group 0 already existed, so it shouldn't publish an event for it. + expect(publishSpy).toHaveBeenCalledWith( + 'MultichainAccountService:multichainAccountGroupCreated', + expect.objectContaining({ groupIndex: 1 }), + ); + }); + }); + describe('alignWallets', () => { it('aligns all multichain account wallets', async () => { const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) @@ -723,15 +910,23 @@ describe('MultichainAccountService', () => { it('create the next multichain account group with MultichainAccountService:createNextMultichainAccountGroup', async () => { const accounts = [MOCK_HD_ACCOUNT_1]; - const { messenger } = await setup({ accounts }); + const { messenger, mocks } = await setup({ accounts }); + + // Groups cannot be empty, we need mock the next account creation too. + const mockNextEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withUuid() + .withGroupIndex(1) + .get(); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + mockNextEvmAccount, + ]); const nextGroup = await messenger.call( 'MultichainAccountService:createNextMultichainAccountGroup', { entropySource: MOCK_HD_KEYRING_1.metadata.id }, ); expect(nextGroup.groupIndex).toBe(1); - // NOTE: There won't be any account for this group, since we're not - // mocking the providers. }); it('creates a multichain account group with MultichainAccountService:createMultichainAccountGroup', async () => { @@ -837,6 +1032,10 @@ describe('MultichainAccountService', () => { name: '', })); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + MOCK_HD_ACCOUNT_1, + ]); + const wallet = await messenger.call( 'MultichainAccountService:createMultichainAccountWallet', { mnemonic, type: 'import' }, @@ -1107,6 +1306,10 @@ describe('MultichainAccountService', () => { name: '', })); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + MOCK_HD_ACCOUNT_1, + ]); + const wallet = await service.createMultichainAccountWallet({ mnemonic, type: 'import', @@ -1169,6 +1372,10 @@ describe('MultichainAccountService', () => { }, ); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + MOCK_HD_ACCOUNT_1, + ]); + const newWallet = await service.createMultichainAccountWallet({ password, type: 'create', @@ -1208,6 +1415,10 @@ describe('MultichainAccountService', () => { }, ); + mocks.EvmAccountProvider.createAccounts.mockResolvedValueOnce([ + MOCK_HD_ACCOUNT_1, + ]); + const newWallet = await service.createMultichainAccountWallet({ password, mnemonic, diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 792c1f9b451..56380049f8c 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -97,6 +97,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'getMultichainAccountWallets', 'createNextMultichainAccountGroup', 'createMultichainAccountGroup', + 'createMultichainAccountGroups', 'setBasicFunctionality', 'alignWallets', 'alignWallet', @@ -608,6 +609,27 @@ export class MultichainAccountService { ); } + /** + * Creates multiple multichain account groups up to maxGroupIndex. + * + * @param params - Parameters for creating account groups. + * @param params.entropySource - The entropy source ID. + * @param params.maxGroupIndex - Maximum group index to create (0 to maxGroupIndex inclusive). + * @returns Array of created multichain account groups. + */ + async createMultichainAccountGroups({ + entropySource, + maxGroupIndex, + }: { + entropySource: EntropySourceId; + maxGroupIndex: number; + }): Promise>[]> { + return await this.#getWallet(entropySource).createMultichainAccountGroups( + maxGroupIndex, + { waitForAllProvidersToFinishCreatingAccounts: false }, + ); + } + /** * Set basic functionality state and trigger alignment if enabled. * When basic functionality is disabled, snap-based providers are disabled. diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 0cebe96967d..30e327322f3 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -102,6 +102,15 @@ function setup({ return { wallet, providers: providersList, messenger: serviceMessenger }; } +async function waitForProvidersToFinishCreatingAccounts( + providers: MockAccountProvider[], +): Promise { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for (const _ of providers) { + await new Promise((resolve) => setTimeout(resolve, 0)); + } +} + describe('MultichainAccountWallet', () => { afterEach(() => { jest.clearAllTimers(); @@ -194,10 +203,15 @@ describe('MultichainAccountWallet', () => { solProvider.getAccounts.mockReturnValueOnce([MOCK_SOL_ACCOUNT_1]); solProvider.getAccount.mockReturnValueOnce(MOCK_SOL_ACCOUNT_1); + // By default, we are not waiting for all providers to finish creating accounts, so the group should be created + // BUT we only have the guarantee to have EVM accounts in the group, as the SOL provider might still be creating + // the account asynchronously. const specificGroup = await wallet.createMultichainAccountGroup(groupIndex); expect(specificGroup.groupIndex).toBe(groupIndex); + await waitForProvidersToFinishCreatingAccounts(providers); + const internalAccounts = specificGroup.getAccounts(); expect(internalAccounts).toHaveLength(2); expect(internalAccounts[0].type).toBe(EthAccountType.Eoa); @@ -228,7 +242,7 @@ describe('MultichainAccountWallet', () => { ); }); - it('creates an account group if only some of the providers fail to create its account (waitForAllProvidersToFinishCreatingAccounts = true)', async () => { + it('does not create 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 @@ -264,19 +278,11 @@ describe('MultichainAccountWallet', () => { succeedingProvider.getAccounts.mockReturnValueOnce([mockNextEvmAccount]); succeedingProvider.getAccount.mockReturnValueOnce(mockNextEvmAccount); - const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); - const group = await wallet.createMultichainAccountGroup(groupIndex, { - waitForAllProvidersToFinishCreatingAccounts: true, - }); - - // Should warn about partial failure but still create the group - expect(consoleSpy).toHaveBeenCalledWith( - `Unable to create some accounts for group index: ${groupIndex}. Providers threw the following errors:\n- Mocked Provider 1: Unable to create accounts`, - ); - expect(group.groupIndex).toBe(groupIndex); - const internalAccounts = group.getAccounts(); - expect(internalAccounts).toHaveLength(1); - expect(internalAccounts[0]).toStrictEqual(mockNextEvmAccount); + await expect( + wallet.createMultichainAccountGroup(groupIndex, { + waitForAllProvidersToFinishCreatingAccounts: true, + }), + ).rejects.toThrow('Unable to create accounts'); }); it('captures an error when a provider fails to create its account', async () => { @@ -292,7 +298,9 @@ describe('MultichainAccountWallet', () => { wallet.createMultichainAccountGroup(groupIndex), ).rejects.toThrow('Unable to create accounts'); expect(captureExceptionSpy).toHaveBeenCalledWith( - new Error('Unable to create account with provider "Mocked Provider 0"'), + new Error( + 'Unable to create some accounts with provider "Mocked Provider 0"', + ), ); expect(captureExceptionSpy.mock.lastCall[0]).toHaveProperty( 'cause', @@ -319,7 +327,7 @@ describe('MultichainAccountWallet', () => { 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', + 'Unable to create some accounts for group index 0 with provider "Mocked Provider 1". Error: Unable to create accounts', ); }); }); @@ -376,6 +384,236 @@ describe('MultichainAccountWallet', () => { }); }); + describe('createMultichainAccountGroups', () => { + it('creates multiple groups from 0 to maxGroupIndex when no groups exist', async () => { + const { wallet, providers } = setup({ + accounts: [[], []], + }); + + const [evmProvider, solProvider] = providers; + + // Mock EVM provider to return accounts for groups 0, 1, 2. + const evmAccounts = [ + MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(), + MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(), + MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(2) + .get(), + ]; + evmProvider.createAccounts.mockResolvedValueOnce(evmAccounts); + + // Mock SOL provider for each group. + for (let i = 0; i <= 2; i++) { + const solAccount = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(i) + .get(); + solProvider.createAccounts.mockResolvedValueOnce([solAccount]); + } + + const groups = await wallet.createMultichainAccountGroups(2); + + expect(groups).toHaveLength(3); + expect(groups[0].groupIndex).toBe(0); + expect(groups[1].groupIndex).toBe(1); + expect(groups[2].groupIndex).toBe(2); + expect(wallet.getAccountGroups()).toHaveLength(3); + }); + + it('returns existing groups and creates new ones when some groups already exist', async () => { + const mockEvmAccount0 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount0 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + + const { wallet, providers } = setup({ + accounts: [[mockEvmAccount0], [mockSolAccount0]], + }); + + const [evmProvider, solProvider] = providers; + + // Mock EVM provider to return accounts for groups 1, 2 (group 0 already exists). + const evmAccounts = [ + MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(), + MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(2) + .get(), + ]; + evmProvider.createAccounts.mockResolvedValueOnce(evmAccounts); + + // Mock SOL provider for groups 1 and 2. + for (let i = 1; i <= 2; i++) { + const solAccount = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(i) + .get(); + solProvider.createAccounts.mockResolvedValueOnce([solAccount]); + } + + const groups = await wallet.createMultichainAccountGroups(2); + + expect(groups).toHaveLength(3); + expect(groups[0].groupIndex).toBe(0); // Existing group. + expect(groups[1].groupIndex).toBe(1); // New group. + expect(groups[2].groupIndex).toBe(2); // New group. + expect(wallet.getAccountGroups()).toHaveLength(3); + }); + + it('returns all existing groups when maxGroupIndex is less than nextGroupIndex', async () => { + const mockEvmAccount0 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + const mockEvmAccount2 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(2) + .get(); + + const { wallet } = setup({ + accounts: [[mockEvmAccount0, mockEvmAccount1, mockEvmAccount2]], + }); + + // Request groups 0-1 when groups 0-2 exist. + const groups = await wallet.createMultichainAccountGroups(1); + + expect(groups).toHaveLength(2); + expect(groups[0].groupIndex).toBe(0); + expect(groups[1].groupIndex).toBe(1); + // Verify we didn't create any new groups. + expect(wallet.getAccountGroups()).toHaveLength(3); + }); + + it('throws when maxGroupIndex is negative', async () => { + const { wallet } = setup({ + accounts: [[]], + }); + + await expect(wallet.createMultichainAccountGroups(-1)).rejects.toThrow( + 'maxGroupIndex must be >= 0', + ); + }); + + it('captures an error with batch mode message when EVM provider fails', async () => { + const { wallet, providers, messenger } = setup({ + accounts: [[]], + }); + + const [evmProvider] = providers; + const providerError = new Error('EVM provider failed'); + evmProvider.createAccounts.mockRejectedValueOnce(providerError); + + const captureExceptionSpy = jest.spyOn(messenger, 'captureException'); + + await expect(wallet.createMultichainAccountGroups(2)).rejects.toThrow( + 'EVM provider failed', + ); + + expect(captureExceptionSpy).toHaveBeenCalledWith( + new Error( + 'Unable to create some accounts (batch) with provider "Mocked Provider 0"', + ), + ); + expect(captureExceptionSpy.mock.lastCall[0]).toHaveProperty( + 'cause', + providerError, + ); + }); + + it('waits for all providers when waitForAllProvidersToFinishCreatingAccounts is true', async () => { + const { wallet, providers } = setup({ + accounts: [[], []], + }); + + const [evmProvider, solProvider] = providers; + + // Mock EVM provider. + const evmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + evmProvider.createAccounts.mockResolvedValueOnce([evmAccount]); + + // Mock SOL provider. + const solAccount = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + solProvider.createAccounts.mockResolvedValueOnce([solAccount]); + + const groups = await wallet.createMultichainAccountGroups(0, { + waitForAllProvidersToFinishCreatingAccounts: true, + }); + + expect(groups).toHaveLength(1); + expect(groups[0].groupIndex).toBe(0); + + // Verify both providers were called. + expect(evmProvider.createAccounts).toHaveBeenCalled(); + expect(solProvider.createAccounts).toHaveBeenCalled(); + }); + + it('captures unexpected errors in background group creation', async () => { + const { wallet, providers, messenger } = setup({ + accounts: [[], []], + }); + + const [evmProvider, solProvider] = providers; + + // Mock EVM provider. + const evmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + evmProvider.createAccounts.mockResolvedValueOnce([evmAccount]); + + // Mock SOL provider to fail during background account creation. + // This will cause the .then() handler to have failures to process. + solProvider.createAccounts.mockRejectedValueOnce( + new Error('Provider failure'), + ); + + const captureExceptionSpy = jest.spyOn(messenger, 'captureException'); + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + + // Spy on console.warn and make it throw during background processing. + // This simulates an unexpected error in the .then() handler, which should + // be caught by the catch block at lines 475-486. + jest.spyOn(console, 'warn').mockImplementation(() => { + throw new Error('Unexpected error in background processing'); + }); + + // Create groups without waiting (triggers background processing). + await wallet.createMultichainAccountGroups(0); + + // Wait for background processing to complete. + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Unexpected error while creating groups (in the background): Unexpected error in background processing', + ); + expect(captureExceptionSpy).toHaveBeenCalled(); + }); + }); + describe('alignAccounts', () => { it('creates missing accounts only for providers with no accounts associated with a particular group index', async () => { const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index c5e05414cab..3f7d6171f11 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -30,7 +30,7 @@ import type { ServiceState, StateKeys } from './MultichainAccountService'; import type { Bip44AccountProvider } from './providers'; import { EvmAccountProvider } from './providers/EvmAccountProvider'; import type { MultichainAccountServiceMessenger } from './types'; -import { createSentryError } from './utils'; +import { createSentryError, toErrorMessage } from './utils'; /** * The context for a provider discovery. @@ -198,110 +198,338 @@ export class MultichainAccountWallet< } /** - * Create accounts with non‑EVM providers. Optional throttling is managed by each provider internally. - * When awaitAll is true, waits for all providers and throws if any failed. - * When false, starts work in background and logs errors without throwing. + * Create accounts for a given provider and group index range. * - * @param options - Method options. - * @param options.groupIndex - The group index to create accounts for. - * @param options.providers - The non‑EVM account providers. - * @param options.awaitAll - Whether to wait for all providers to finish. - * @param options.group - The group object pertaining to the group index to create accounts for. - * @throws If awaitAll is true and any provider fails to create accounts. - * @returns A promise that resolves when done (if awaitAll is true) or immediately (if false). + * @param provider - The provider to create accounts for. + * @param from - The starting group index (inclusive). + * @param to - The ending group index (inclusive). + * @returns The created accounts. */ - async #createNonEvmAccounts({ - groupIndex, - providers, - awaitAll, - group, - }: { - groupIndex: number; - providers: Bip44AccountProvider[]; - awaitAll: boolean; - group: MultichainAccountGroup; - }): Promise { - if (awaitAll) { - const tasks = providers.map((provider) => - provider - .createAccounts({ - type: AccountCreationType.Bip44DeriveIndex, - entropySource: this.#entropySource, - groupIndex, - }) - .catch((error) => { - const sentryError = createSentryError( - `Unable to create account with provider "${provider.getName()}"`, - error, - { - groupIndex, - provider: provider.getName(), - }, - ); - this.#messenger.captureException?.(sentryError); - throw error; - }), + async #createAccountsRangeForProvider( + provider: Bip44AccountProvider, + from: number, + to: number, + ): Promise[]> { + const isBatching = to > from; + + try { + return await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: this.#entropySource, + range: { + from, + to, + }, + }); + } catch (error) { + const modeDescription = isBatching + ? 'some accounts (batch)' + : 'some accounts'; + const rangeDescription = isBatching + ? `from group index ${from} to ${to}` + : `for group index ${to}`; + + const errorMessage = `Unable to create ${modeDescription} ${rangeDescription} with provider "${provider.getName()}". Error: ${(error as Error).message}`; + console.warn(errorMessage); + this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error); + + const sentryError = createSentryError( + `Unable to create ${modeDescription} with provider "${provider.getName()}"`, + error as Error, + { + range: { + from, + to, + }, + provider: provider.getName(), + isBatching, + }, + ); + this.#messenger.captureException?.(sentryError); + throw error; + } + } + + /** + * Create or update a multichain account group state for a given group index and group state. + * + * @param groupIndex The group's index. + * @param groupState The group's state to create or update the group with. + * @returns The created or updated multichain account group. + */ + #createOrUpdateMultichainAccountGroup( + groupIndex: number, + groupState: GroupState, + ): [MultichainAccountGroup, boolean] { + let group = this.#accountGroups.get(groupIndex); + if (group) { + // NOTE: This will publish an update event automatically. + group.update(groupState); + + this.#log(`Group updated: [${group.id}]`); + + return [group, false]; + } + + group = new MultichainAccountGroup({ + wallet: this, + providers: this.#providers, + groupIndex, + messenger: this.#messenger, + }); + group.init(groupState); + + this.#accountGroups.set(groupIndex, group); + + this.#log(`New group created: [${group.id}]`); + + if (this.#initialized) { + this.#messenger.publish( + 'MultichainAccountService:multichainAccountGroupCreated', + group, ); + } - const results = await Promise.allSettled(tasks); + return [group, true]; + } - const providerFailures = results.reduce((acc, result, idx) => { - let newAcc = acc; - if (result.status === 'rejected') { - newAcc += `\n- ${providers[idx].getName()}: ${result.reason.message}`; - } - return newAcc; - }, ''); - - if (providerFailures.length) { - // 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}`; - console.warn(message); - this.#log(`${WARNING_PREFIX} ${message}`); + /** + * Internal method to create a range of multichain account groups. + * + * IMPORTANT: This method assumes the caller has already acquired the wallet lock. + * + * @param fromGroupIndex - The starting group index (inclusive). + * @param toGroupIndex - The ending group index (inclusive). + * @param options - Options to configure the account creation. + * @param options.waitForAllProvidersToFinishCreatingAccounts - Whether to wait for all + * account providers to finish creating their accounts before returning. If `false`, only + * the EVM provider will be awaited, while all other providers will create their accounts + * in the background. Defaults to `false`. + * @returns Array of created multichain account groups. + */ + async #createMultichainAccountGroupsRange( + fromGroupIndex: number, + toGroupIndex: number, + options: { + waitForAllProvidersToFinishCreatingAccounts?: boolean; + }, + ): Promise[]> { + const groups: MultichainAccountGroup[] = []; + + // Get existing groups (fromGroupIndex to nextGroupIndex-1). + let from = fromGroupIndex; + const to = toGroupIndex; + for (; from <= to; from++) { + const group = this.getMultichainAccountGroup(from); + if (group) { + groups.push(group); + } else { + break; // Assuming we have no gap, if the group does not exist, we can stop and create the remaining ones. } + } + + // Create new groups now. + if (from <= to) { + this.#log(`Creating groups from index ${from} to ${to}...`); + + const [evmProvider, ...otherProviders] = this.#providers; + assert( + evmProvider instanceof EvmAccountProvider, + 'EVM account provider must be first', + ); + + // Create EVM accounts for all new groups using range. + const evmProviderName = evmProvider.getName(); + const evmAccounts = await this.#createAccountsRangeForProvider( + evmProvider, + from, + to, + ); + + // Map EVM accounts to group indices. + const evmGroupStateByGroupIndex = new Map(); + evmAccounts.forEach((account) => { + const { groupIndex } = account.options.entropy; + evmGroupStateByGroupIndex.set(groupIndex, { + [evmProviderName]: [account.id], + }); + }); + + // Execute creation of non-EVM accounts in parallel, but sequentially after EVM accounts + // since they depend on the group creation that happens after we get the EVM accounts + // (because of the need to update the group state with the non-EVM accounts once they + // are created). + const createNonEvmAccounts = async ( + provider: Bip44AccountProvider, + groupStateByGroupIndex: Map, + ): Promise => { + const nonEvmProviderName = provider.getName(); + const nonEvmAccounts = await this.#createAccountsRangeForProvider( + provider, + from, + to, + ); + + // Map non-EVM accounts to group indices. + nonEvmAccounts.forEach((account) => { + const { groupIndex } = account.options.entropy; + + let groupState = groupStateByGroupIndex.get(groupIndex); + if (!groupState) { + groupState = { + [nonEvmProviderName]: [], + }; + groupStateByGroupIndex.set(groupIndex, groupState); + } + + if (!groupState[nonEvmProviderName]) { + groupState[nonEvmProviderName] = []; + } - // 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') { - state[providers[idx].getName()] = result.value.map( - (account) => account.id, + groupState[nonEvmProviderName].push(account.id); + }); + }; + + // Finalize group states with accounts from other providers, then create groups + // and update states with the associated accounts. + const createOrUpdateMultichainAccountGroupsFrom = async ( + allGroupStateByGroupIndex: Map, + ): Promise => { + for (let groupIndex = from; groupIndex <= to; groupIndex++) { + const groupState = allGroupStateByGroupIndex.get(groupIndex); + + if (groupState) { + const [group, created] = this.#createOrUpdateMultichainAccountGroup( + groupIndex, + groupState, + ); + + if (created) { + groups.push(group); + } + } else { + this.#log( + `${WARNING_PREFIX} Failed to create new group for group index: ${groupIndex} because no accounts were created for it`, + ); + } + } + }; + + // Check for provider failures and return an appropriate error message. + const getProviderFailuresErrorMessage = ( + providers: Bip44AccountProvider[], + results: PromiseSettledResult[], + { background }: { background: boolean }, + ): string | null => { + const failures = providers.reduce( + (providerFailures: string[], provider, index) => { + // We assume results and providers are in the same order because they are generated + // with `Promise.allSettled(providers.map(...))`. + const result = results[index]; + + if (result?.status === 'rejected') { + const providerName = provider.getName(); + + providerFailures.push( + `[${providerName}] ${toErrorMessage(result.reason)}`, + ); + } + + return providerFailures; + }, + [], + ); + + if (failures.length) { + return failures.reduce( + (message, failure) => `${message}\n- ${failure}`, + `Unable to create some accounts${background ? ' (in the background)' : ''}. Providers threw the following errors:\n`, ); } - return state; - }, {}); - group.update(groupState); - } else { - // Create account with other providers in the background - providers.forEach((provider) => { - provider - .createAccounts({ - type: AccountCreationType.Bip44DeriveIndex, - entropySource: this.#entropySource, - groupIndex, - }) - .then((accounts) => { - const accountIds = accounts.map((account) => account.id); - group.update({ [provider.getName()]: accountIds }); - return group; + return null; + }; + + if (options.waitForAllProvidersToFinishCreatingAccounts) { + // We re-use existing mapping and will extend it with non-EVM accounts. + const groupStateByGroupIndex = evmGroupStateByGroupIndex; + + // We continue updating the group states with non-EVM accounts now. + const results = await Promise.allSettled( + otherProviders.map((provider) => + createNonEvmAccounts(provider, groupStateByGroupIndex), + ), + ); + + // Check for failures, in this mode, if any of the providers fail, we treat this as an hard-error. + const failures = getProviderFailuresErrorMessage( + otherProviders, + results, + { background: false }, + ); + if (failures) { + throw new Error(failures); + } + + // We can finalize the group creation and updates now that we have all accounts from all providers. + await createOrUpdateMultichainAccountGroupsFrom(groupStateByGroupIndex); + } else { + // We can finalize with the EVM accounts for now, since the non-EVM accounts are going to get + // created in the background. + await createOrUpdateMultichainAccountGroupsFrom( + evmGroupStateByGroupIndex, + ); + + // We can use a new mapping now, since EVM should have been created by now. + const groupStateByGroupIndex = new Map(); + + // We create the non-EVM accounts in the background, and update the groups as the accounts are created. + // eslint-disable-next-line no-void + void Promise.allSettled( + otherProviders.map((provider) => + createNonEvmAccounts(provider, groupStateByGroupIndex), + ), + ) + .then(async (results) => { + // In background mode, we still want to log if there are failures, but we don't want to throw since some + // accounts might have been created "partially". + const failures = getProviderFailuresErrorMessage( + otherProviders, + results, + { background: true }, + ); + if (failures) { + // We warn there's failures on some providers and thus misalignment, but we still create the group + console.warn(failures); + this.#log(`${WARNING_PREFIX} ${failures}`); + } + + // If some providers succeeded, we still want to update the groups accordingly. + if (results.some((result) => result.status === 'fulfilled')) { + // We re-finalize everything to update the groups with the accounts from the non-EVM providers as they come in. + await createOrUpdateMultichainAccountGroupsFrom( + groupStateByGroupIndex, + ); + } + + return undefined; }) .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); - const sentryError = createSentryError( - `Unable to create account with provider "${provider.getName()}"`, - error, - { - groupIndex, - provider: provider.getName(), - }, + // We catch this for safety, but nothing should really fail at this points since we + // are catching all providers errors with `allSettled`. + // Since it's a background operation, there's not much we can do unless just logging + // the error. + const errorMessage = `Unexpected error while creating groups (in the background): ${(error as Error).message}`; + console.error(errorMessage); + this.#log(`${ERROR_PREFIX} ${errorMessage}`, error); + this.#messenger.captureException?.( + createSentryError(errorMessage, error as Error), ); - this.#messenger.captureException?.(sentryError); }); - }); + } } + + return groups; } /** @@ -385,6 +613,7 @@ export class MultichainAccountWallet< * account providers to finish creating their accounts before returning. If `false`, only * the EVM provider will be awaited, while all other providers will create their accounts * in the background. Defaults to `false`. + * @throws If groupIndex is greater than the next available group index. * @throws If any of the account providers fails to create their accounts and * the `waitForAllProvidersToFinishCreatingAccounts` option is set to `true`. If `false`, * errors from non-EVM providers will be logged but ignored, and only errors from the @@ -399,95 +628,65 @@ export class MultichainAccountWallet< ): Promise> { return await this.#withLock('in-progress:create-accounts', async () => { const nextGroupIndex = this.getNextGroupIndex(); + + // Validate that we can only create the next available group or an existing one. if (groupIndex > nextGroupIndex) { throw new Error( `You cannot use a group index that is higher than the next available one: expected <=${nextGroupIndex}, got ${groupIndex}`, ); } - let group = this.getMultichainAccountGroup(groupIndex); - - if (group) { + // If the group already exists, return it. + const existingGroup = this.getMultichainAccountGroup(groupIndex); + if (existingGroup) { this.#log( - `Trying to re-create existing group: [${group.id}] (idempotent)`, + `Trying to re-create existing group: [${existingGroup.id}] (idempotent)`, ); - return group; + return existingGroup; } - this.#log(`Creating new group for index ${groupIndex}...`); - - // Extract the EVM provider from the list of providers. - // We always await EVM account creation first. - const [evmProvider, ...otherProviders] = this.#providers; - assert( - evmProvider instanceof EvmAccountProvider, - 'EVM account provider must be first', - ); - - const evmAccounts = await evmProvider - .createAccounts({ - type: AccountCreationType.Bip44DeriveIndex, - entropySource: this.#entropySource, - groupIndex, - }) - .then((accounts) => accounts.map((account) => account.id)) - .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); - const sentryError = createSentryError( - `Unable to create account with provider "${evmProvider.getName()}"`, - error as Error, - { - groupIndex, - provider: evmProvider.getName(), - }, - ); - this.#messenger.captureException?.(sentryError); - throw error; - }); - - group = new MultichainAccountGroup({ - wallet: this, - providers: this.#providers, + // Create the group using the private range method. + const groups = await this.#createMultichainAccountGroupsRange( groupIndex, - messenger: this.#messenger, - }); - - group.init({ [evmProvider.getName()]: evmAccounts }); - - // We then create accounts with other providers (some being throttled if configured). - // Depending on the options, we either await all providers or run them in background. - if (options?.waitForAllProvidersToFinishCreatingAccounts) { - await this.#createNonEvmAccounts({ - groupIndex, - providers: otherProviders, - awaitAll: true, - group, - }); - } else { - // eslint-disable-next-line no-void - void this.#createNonEvmAccounts({ - groupIndex, - providers: otherProviders, - awaitAll: false, - group, - }); - } - - // Register the account(s) to our internal map. - this.#accountGroups.set(groupIndex, group); + groupIndex, + options, + ); - this.#log(`New group created: [${group.id}]`); + const group = groups[0]; + assert(group, `Expected group at index ${groupIndex} to exist`); + return group; + }); + } - if (this.#initialized) { - this.#messenger.publish( - 'MultichainAccountService:multichainAccountGroupCreated', - group, - ); + /** + * Creates multiple multichain account groups up to maxGroupIndex. + * + * NOTE: This operation WILL lock the wallet's mutex. + * + * @param maxGroupIndex - The maximum group index to create (creates 0 to maxGroupIndex inclusive). + * @param options - Options to configure the account creation. + * @param options.waitForAllProvidersToFinishCreatingAccounts - Whether to wait for all + * account providers to finish creating their accounts before returning. Defaults to false. + * @throws If maxGroupIndex is less than 0 or if account creation fails. + * @returns Array of created multichain account groups. + */ + async createMultichainAccountGroups( + maxGroupIndex: number, + options: { + waitForAllProvidersToFinishCreatingAccounts?: boolean; + } = { waitForAllProvidersToFinishCreatingAccounts: false }, + ): Promise[]> { + return await this.#withLock('in-progress:create-accounts', async () => { + if (maxGroupIndex < 0) { + throw new Error('maxGroupIndex must be >= 0'); } - return group; + // Create groups from 0 to maxGroupIndex using the private range method. + return this.#createMultichainAccountGroupsRange( + 0, + maxGroupIndex, + options, + ); }); } diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts index c7202e8be83..a118295b2ce 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts @@ -281,6 +281,79 @@ describe('BtcAccountProvider', () => { expect(newAccounts[0]).toStrictEqual(MOCK_BTC_P2WPKH_ACCOUNT_1); }); + it('creates multiple accounts using Bip44DeriveIndexRange', async () => { + const accounts = [MOCK_BTC_P2WPKH_ACCOUNT_1]; + const { provider, keyring } = setup({ + accounts, + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 1, + to: 3, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.createAccount).toHaveBeenCalledTimes(3); + + // Verify each account has the correct group index + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(1); + expect( + isBip44Account(newAccounts[1]) && + newAccounts[1].options.entropy.groupIndex, + ).toBe(2); + expect( + isBip44Account(newAccounts[2]) && + newAccounts[2].options.entropy.groupIndex, + ).toBe(3); + }); + + it('creates accounts with range starting from 0', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 0, + to: 2, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.createAccount).toHaveBeenCalledTimes(3); + }); + + it('creates a single account when range from equals to', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 5, + to: 5, + }, + }); + + expect(newAccounts).toHaveLength(1); + expect(keyring.createAccount).toHaveBeenCalledTimes(1); + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(5); + }); + it('throws if the account creation process takes too long', async () => { const { provider, mocks } = setup({ accounts: [], diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts index 75b0109ba86..1c17a171c6b 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts @@ -52,6 +52,7 @@ export class BtcAccountProvider extends SnapAccountProvider { scopes: [BtcScope.Mainnet, BtcScope.Testnet], bip44: { deriveIndex: true, + deriveIndexRange: true, }, }; @@ -105,9 +106,36 @@ export class BtcAccountProvider extends SnapAccountProvider { ): Promise[]> { assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, + `${AccountCreationType.Bip44DeriveIndexRange}`, ]); - const { entropySource, groupIndex } = options; + const { entropySource } = options; + + if (options.type === AccountCreationType.Bip44DeriveIndexRange) { + const { range } = options; + return this.withSnap(async ({ keyring }) => { + const accounts: Bip44Account[] = []; + + // TODO: Use `SnapKeyring.createAccounts` when that functionality is implemented on the Snap + // itself, instead of creating accounts one by one. + for ( + let groupIndex = range.from; + groupIndex <= range.to; + groupIndex++ + ) { + const createdAccounts = await this.#createAccounts({ + keyring, + entropySource, + groupIndex, + }); + accounts.push(...createdAccounts); + } + + return accounts; + }); + } + + const { groupIndex } = options; return this.withSnap(async ({ keyring }) => { return this.#createAccounts({ keyring, entropySource, groupIndex }); diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts index 8756aec2b71..f3bb51f3e32 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.test.ts @@ -1,4 +1,5 @@ import { publicToAddress } from '@ethereumjs/util'; +import { isBip44Account } from '@metamask/account-api'; import { getUUIDFromAddressOfNormalAccount } from '@metamask/accounts-controller'; import { AccountCreationType } from '@metamask/keyring-api'; import type { KeyringMetadata } from '@metamask/keyring-controller'; @@ -155,7 +156,14 @@ function setup({ messenger.registerActionHandler( 'AccountsController:getAccounts', - () => accounts, + (accountIds: string[]) => + keyring.accounts.filter( + (account) => + accountIds.includes(account.id) || + accountIds.includes( + getUUIDFromAddressOfNormalAccount(account.address), + ), + ), ); const mockGetAccount = jest.fn().mockImplementation((id) => { @@ -301,6 +309,133 @@ describe('EvmAccountProvider', () => { expect(newAccounts[0]).toStrictEqual(MOCK_HD_ACCOUNT_1); }); + it('creates multiple accounts using Bip44DeriveIndexRange', async () => { + const accounts = [MOCK_HD_ACCOUNT_1]; + const { provider, keyring } = setup({ + accounts, + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 1, + to: 3, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.addAccounts).toHaveBeenCalledTimes(1); + expect(keyring.addAccounts).toHaveBeenCalledWith(3); + + // Verify each account has the correct group index. + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(1); + expect( + isBip44Account(newAccounts[1]) && + newAccounts[1].options.entropy.groupIndex, + ).toBe(2); + expect( + isBip44Account(newAccounts[2]) && + newAccounts[2].options.entropy.groupIndex, + ).toBe(3); + }); + + it('creates accounts with range starting from 0', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 0, + to: 2, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.addAccounts).toHaveBeenCalledTimes(1); + expect(keyring.addAccounts).toHaveBeenCalledWith(3); + }); + + it('creates a single account when range from equals to', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + // First create accounts 0-4 to avoid gaps. + await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 0, + to: 4, + }, + }); + + // Now create a single account at index 5 where from equals to. + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 5, + to: 5, + }, + }); + + expect(newAccounts).toHaveLength(1); + expect(keyring.addAccounts).toHaveBeenCalledTimes(2); // 1 call for range 0-4, 1 call for account 5. + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(5); + }); + + it('throws when trying to create gaps with range', async () => { + const { provider } = setup({ + accounts: [MOCK_HD_ACCOUNT_1], + }); + + await expect( + provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 5, + to: 10, + }, + }), + ).rejects.toThrow('Trying to create too many accounts'); + }); + + it('returns existing accounts when range includes already created accounts', async () => { + const accounts = [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2]; + const { provider, keyring } = setup({ + accounts, + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 0, + to: 3, + }, + }); + + // Should return 4 accounts: 2 existing (indices 0,1) + 2 new (indices 2,3). + expect(newAccounts).toHaveLength(4); + expect(newAccounts[0]).toStrictEqual(MOCK_HD_ACCOUNT_1); + expect(newAccounts[1]).toStrictEqual(MOCK_HD_ACCOUNT_2); + // Only 2 new accounts should be created (indices 2 and 3) in a single batched call. + expect(keyring.addAccounts).toHaveBeenCalledTimes(1); + expect(keyring.addAccounts).toHaveBeenCalledWith(2); + }); + it('throws if the created account is not BIP-44 compatible', async () => { const accounts = [MOCK_HD_ACCOUNT_1]; const { provider, mocks } = setup({ diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 0cceb218fea..6b26c4092ff 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -20,6 +20,7 @@ import type { EthKeyring, InternalAccount, } from '@metamask/keyring-internal-api'; +import { AccountId } from '@metamask/keyring-utils'; import type { Provider } from '@metamask/network-controller'; import { add0x, assert, bytesToHex, isStrictHexString } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; @@ -81,6 +82,7 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { scopes: [EthScope.Eoa], bip44: { deriveIndex: true, + deriveIndexRange: true, }, }; @@ -191,9 +193,71 @@ export class EvmAccountProvider extends BaseBip44AccountProvider { ): Promise[]> { assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, + `${AccountCreationType.Bip44DeriveIndexRange}`, ]); - const { entropySource, groupIndex } = options; + const { entropySource } = options; + + if (options.type === AccountCreationType.Bip44DeriveIndexRange) { + const { range } = options; + + // Use a single withKeyring call for the entire range. + const accountIds = await this.withKeyring( + { id: entropySource }, + async ({ keyring }) => { + const existing = await keyring.getAccounts(); + const result: AccountId[] = []; + + // Collect existing accounts within the range. + for ( + let groupIndex = range.from; + groupIndex <= range.to; + groupIndex++ + ) { + if (groupIndex < existing.length) { + // Account already exists. + result.push(this.#getAccountId(existing[groupIndex])); + } + } + + // Determine if we need to create new accounts. + const from = Math.max(range.from, existing.length); + if (from <= range.to) { + // Validate no gaps: we can only create accounts starting from existing.length. + if (from !== existing.length) { + throw new Error('Trying to create too many accounts'); + } + + // Calculate how many new accounts to create. + const accountsToCreate = range.to - existing.length + 1; + + // Create all new accounts in one call. + const newAccounts = await keyring.addAccounts(accountsToCreate); + result.push( + ...newAccounts.map((address) => this.#getAccountId(address)), + ); + } + + return result; + }, + ); + + const accounts: InternalAccount[] = []; + for (const account of this.messenger.call( + 'AccountsController:getAccounts', + accountIds, + )) { + assertInternalAccountExists(account); + this.accounts.add(account.id); + accounts.push(account); + } + + assertAreBip44Accounts(accounts); + return accounts; + } + + // Handle Bip44DeriveIndex (single account creation). + const { groupIndex } = options; const [address] = await this.#createAccount({ entropySource, diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts index d774d5379c2..4963f9d609e 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.test.ts @@ -281,6 +281,79 @@ describe('SolAccountProvider', () => { expect(newAccounts[0]).toStrictEqual(MOCK_SOL_ACCOUNT_1); }); + it('creates multiple accounts using Bip44DeriveIndexRange', async () => { + const accounts = [MOCK_SOL_ACCOUNT_1]; + const { provider, keyring } = setup({ + accounts, + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 1, + to: 3, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.createAccount).toHaveBeenCalledTimes(3); + + // Verify each account has the correct group index + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(1); + expect( + isBip44Account(newAccounts[1]) && + newAccounts[1].options.entropy.groupIndex, + ).toBe(2); + expect( + isBip44Account(newAccounts[2]) && + newAccounts[2].options.entropy.groupIndex, + ).toBe(3); + }); + + it('creates accounts with range starting from 0', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 0, + to: 2, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.createAccount).toHaveBeenCalledTimes(3); + }); + + it('creates a single account when range from equals to', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 5, + to: 5, + }, + }); + + expect(newAccounts).toHaveLength(1); + expect(keyring.createAccount).toHaveBeenCalledTimes(1); + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(5); + }); + it('throws if the account creation process takes too long', async () => { const { provider, mocks } = setup({ accounts: [], diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 779f367f987..455ee26a102 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -54,6 +54,7 @@ export class SolAccountProvider extends SnapAccountProvider { scopes: [SolScope.Mainnet, SolScope.Devnet, SolScope.Testnet], bip44: { deriveIndex: true, + deriveIndexRange: true, }, }; @@ -109,9 +110,42 @@ export class SolAccountProvider extends SnapAccountProvider { ): Promise[]> { assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, + `${AccountCreationType.Bip44DeriveIndexRange}`, ]); - const { entropySource, groupIndex } = options; + const { entropySource } = options; + + if (options.type === AccountCreationType.Bip44DeriveIndexRange) { + const { range } = options; + return this.withSnap(async ({ keyring }) => { + const accounts: Bip44Account[] = []; + + // TODO: Use `SnapKeyring.createAccounts` when that functionality is implemented on the Snap + // itself, instead of creating accounts one by one. + for ( + let groupIndex = range.from; + groupIndex <= range.to; + groupIndex++ + ) { + const account = await this.withMaxConcurrency(async () => { + const derivationPath = `m/44'/501'/${groupIndex}'/0'`; + return this.#createAccount({ + keyring, + entropySource, + groupIndex, + derivationPath, + }); + }); + + this.accounts.add(account.id); + accounts.push(account); + } + + return accounts; + }); + } + + const { groupIndex } = options; return this.withSnap(async ({ keyring }) => { return this.withMaxConcurrency(async () => { diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts index 78117f7ccce..10bf6ce44e1 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts @@ -262,6 +262,79 @@ describe('TrxAccountProvider', () => { expect(newAccounts[0]).toStrictEqual(MOCK_TRX_ACCOUNT_1); }); + it('creates multiple accounts using Bip44DeriveIndexRange', async () => { + const accounts = [MOCK_TRX_ACCOUNT_1]; + const { provider, keyring } = setup({ + accounts, + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 1, + to: 3, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.createAccount).toHaveBeenCalledTimes(3); + + // Verify each account has the correct group index + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(1); + expect( + isBip44Account(newAccounts[1]) && + newAccounts[1].options.entropy.groupIndex, + ).toBe(2); + expect( + isBip44Account(newAccounts[2]) && + newAccounts[2].options.entropy.groupIndex, + ).toBe(3); + }); + + it('creates accounts with range starting from 0', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 0, + to: 2, + }, + }); + + expect(newAccounts).toHaveLength(3); + expect(keyring.createAccount).toHaveBeenCalledTimes(3); + }); + + it('creates a single account when range from equals to', async () => { + const { provider, keyring } = setup({ + accounts: [], + }); + + const newAccounts = await provider.createAccounts({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: MOCK_HD_KEYRING_1.metadata.id, + range: { + from: 5, + to: 5, + }, + }); + + expect(newAccounts).toHaveLength(1); + expect(keyring.createAccount).toHaveBeenCalledTimes(1); + expect( + isBip44Account(newAccounts[0]) && + newAccounts[0].options.entropy.groupIndex, + ).toBe(5); + }); + it('throws if the account creation process takes too long', async () => { const { provider, mocks } = setup({ accounts: [], diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts index 327cd5a4e8a..64beb19c619 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts @@ -53,6 +53,7 @@ export class TrxAccountProvider extends SnapAccountProvider { scopes: [TrxScope.Mainnet, TrxScope.Shasta], bip44: { deriveIndex: true, + deriveIndexRange: true, }, }; @@ -106,9 +107,36 @@ export class TrxAccountProvider extends SnapAccountProvider { ): Promise[]> { assertCreateAccountOptionIsSupported(options, [ `${AccountCreationType.Bip44DeriveIndex}`, + `${AccountCreationType.Bip44DeriveIndexRange}`, ]); - const { entropySource, groupIndex } = options; + const { entropySource } = options; + + if (options.type === AccountCreationType.Bip44DeriveIndexRange) { + const { range } = options; + return this.withSnap(async ({ keyring }) => { + const accounts: Bip44Account[] = []; + + // TODO: Use `SnapKeyring.createAccounts` when that functionality is implemented on the Snap + // itself, instead of creating accounts one by one. + for ( + let groupIndex = range.from; + groupIndex <= range.to; + groupIndex++ + ) { + const createdAccounts = await this.#createAccounts({ + keyring, + entropySource, + groupIndex, + }); + accounts.push(...createdAccounts); + } + + return accounts; + }); + } + + const { groupIndex } = options; return this.withSnap(async ({ keyring }) => { return this.#createAccounts({ diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 074ccb328b2..671c5a1d0ed 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -37,6 +37,7 @@ import type { import type { serviceName } from './MultichainAccountService'; import type { MultichainAccountServiceMethodActions } from './MultichainAccountService-method-action-types'; + /** * All actions that {@link MultichainAccountService} registers so that other * modules can call them. diff --git a/packages/multichain-account-service/src/utils.test.ts b/packages/multichain-account-service/src/utils.test.ts new file mode 100644 index 00000000000..c6b31f9e1d6 --- /dev/null +++ b/packages/multichain-account-service/src/utils.test.ts @@ -0,0 +1,28 @@ +import { toErrorMessage } from './utils'; + +describe('toErrorMessage', () => { + it('returns the message of an Error instance', () => { + const error = new Error('something went wrong'); + expect(toErrorMessage(error)).toBe(error.message); + }); + + it('returns the string representation of a non-Error value', () => { + expect(toErrorMessage('raw string')).toBe('raw string'); + }); + + it('converts a number to string', () => { + expect(toErrorMessage(42)).toBe('42'); + }); + + it('converts null to string', () => { + expect(toErrorMessage(null)).toBe('null'); + }); + + it('converts undefined to string', () => { + expect(toErrorMessage(undefined)).toBe('undefined'); + }); + + it('converts an object to string', () => { + expect(toErrorMessage({ foo: 'bar' })).toBe('[object Object]'); + }); +}); diff --git a/packages/multichain-account-service/src/utils.ts b/packages/multichain-account-service/src/utils.ts index 1c446077e18..a4fad066f0d 100644 --- a/packages/multichain-account-service/src/utils.ts +++ b/packages/multichain-account-service/src/utils.ts @@ -4,21 +4,34 @@ * NOTE: Sentry defaults to a depth of 3 when extracting non-native attributes. * As such, the context depth shouldn't be too deep. * - * @param msg - The error message to create a Sentry error from. + * @param message - The error message to create a Sentry error from. * @param innerError - The inner error to create a Sentry error from. * @param context - The context to add to the Sentry error. * @returns A Sentry error. */ export const createSentryError = ( - msg: string, + message: string, innerError: Error, - context: Record, -) => { - const error = new Error(msg) as Error & { + context?: Record, +): Error => { + const error = new Error(message) as Error & { cause: Error; context: typeof context; }; error.cause = innerError; - error.context = context; + if (context) { + error.context = context; + } return error; }; + +/** + * Converts an unknown error value to a string message. + * + * @param error - The error to convert. + * @returns The error message if the error is an `Error` instance, otherwise + * the string representation of the value. + */ +export function toErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +}