diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index e43524b063b..a4af631e045 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add an optional `options` parameter to `MultichainAccountWallet.createMultichainAccountGroup()` ([#6759](https://github.com/MetaMask/core/pull/6759)) + - Introduces `options.waitForAllProvidersToFinishCreatingAccounts`, that will make `createMultichainAccountGroup` await either only the EVM provider or all the providers to have created their accounts depending on the value. Defaults to `false` (only awaits for EVM accounts creation by default). + ## [1.4.0] ### Changed diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 239b7068c5f..09d1b0e9420 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -347,9 +347,32 @@ describe('MultichainAccountWallet', () => { expect( await wallet.createMultichainAccountGroup(groupIndex), ).toBeDefined(); - await new Promise(process.nextTick); expect(mockSolProviderError).toHaveBeenCalled(); }); + + it('fails to create an account group if any of the provider fails to create its account and waitForAllProvidersToFinishCreatingAccounts is true', async () => { + const groupIndex = 1; + + const mockEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const { wallet, providers } = setup({ + accounts: [[mockEvmAccount]], // 1 provider + }); + const [provider] = providers; + provider.createAccounts.mockRejectedValueOnce( + new Error('Unable to create accounts'), + ); + + await expect( + wallet.createMultichainAccountGroup(groupIndex, { + waitForAllProvidersToFinishCreatingAccounts: true, + }), + ).rejects.toThrow( + 'Unable to create multichain account group for index: 1', + ); + }); }); describe('createNextMultichainAccountGroup', () => { diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 99d5093f30c..f90c0a73694 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -296,11 +296,22 @@ export class MultichainAccountWallet< * NOTE: This operation WILL lock the wallet's mutex. * * @param groupIndex - The group index to use. - * @throws If any of the account providers fails to create their accounts. + * @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`. + * @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 + * EVM provider will be thrown. * @returns The multichain account group for this group index. */ async createMultichainAccountGroup( groupIndex: number, + options: { + waitForAllProvidersToFinishCreatingAccounts?: boolean; + } = { waitForAllProvidersToFinishCreatingAccounts: false }, ): Promise> { return await this.#withLock('in-progress:create-accounts', async () => { const nextGroupIndex = this.getNextGroupIndex(); @@ -324,41 +335,72 @@ export class MultichainAccountWallet< this.#log(`Creating new group for index ${groupIndex}...`); - // Extract the EVM provider from the list of providers. - // We will only await the EVM provider to create its accounts, while - // all other providers will be started in the background. - const [evmProvider, ...otherProviders] = this.#providers; - assert( - evmProvider instanceof EvmAccountProvider, - 'EVM account provider must be first', - ); + if (options?.waitForAllProvidersToFinishCreatingAccounts) { + // Create account with all providers and await them. + const results = await Promise.allSettled( + this.#providers.map((provider) => + provider.createAccounts({ + entropySource: this.#entropySource, + groupIndex, + }), + ), + ); - // Create account with the EVM provider first and await it. - // If it fails, we don't start creating accounts with other providers. - try { - await evmProvider.createAccounts({ - entropySource: this.#entropySource, - groupIndex, - }); - } catch (error) { - const errorMessage = `Unable to create multichain account group for index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`; - this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error); - throw new Error(errorMessage); - } + // 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 message = `${error}:`; + for (const result of results) { + if (result.status === 'rejected') { + message += `\n- ${result.reason}`; + } + } + this.#log(`${WARNING_PREFIX} ${message}`); + console.warn(message); + + throw new Error(error); + } + } else { + // Extract the EVM provider from the list of providers. + // We will only await the EVM provider to create its accounts, while + // all other providers will be started in the background. + const [evmProvider, ...otherProviders] = this.#providers; + assert( + evmProvider instanceof EvmAccountProvider, + 'EVM account provider must be first', + ); - // Create account with other providers in the background - otherProviders.forEach((provider) => { - provider - .createAccounts({ + // Create account with the EVM provider first and await it. + // If it fails, we don't start creating accounts with other providers. + try { + await evmProvider.createAccounts({ entropySource: this.#entropySource, groupIndex, - }) - .catch((error) => { - // Log errors from background providers but don't fail the operation - const errorMessage = `Could not to create account with provider "${provider.getName()}" for multichain account group index: ${groupIndex}`; - this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error); }); - }); + } catch (error) { + const errorMessage = `Unable to create multichain account group for index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`; + this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error); + throw new Error(errorMessage); + } + + // Create account with other providers in the background + otherProviders.forEach((provider) => { + provider + .createAccounts({ + entropySource: this.#entropySource, + groupIndex, + }) + .catch((error) => { + // Log errors from background providers but don't fail the operation + const errorMessage = `Could not to create account with provider "${provider.getName()}" for multichain account group index: ${groupIndex}`; + this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error); + }); + }); + } // -------------------------------------------------------------------------------- // READ THIS CAREFULLY: @@ -419,7 +461,9 @@ export class MultichainAccountWallet< async createNextMultichainAccountGroup(): Promise< MultichainAccountGroup > { - return this.createMultichainAccountGroup(this.getNextGroupIndex()); + return this.createMultichainAccountGroup(this.getNextGroupIndex(), { + waitForAllProvidersToFinishCreatingAccounts: true, + }); } /**