diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index e48912185d..1b07b1ab19 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] +### Fixed + +- Wallet alignment now only creates the missing `(provider, group index)` pairs instead of re-creating the whole range for every provider, avoiding redundant `createAccounts` calls (and their traces) ([#9269](https://github.com/MetaMask/core/pull/9269)) + - Adds `MultichainAccountGroup.isProviderAligned(provider)` to check alignment per provider. + ## [11.1.0] ### Added diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index 8b7af73e08..992dd1e015 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -263,13 +263,27 @@ export class MultichainAccountGroup< */ isAligned(): boolean { return this.#providers.every((provider) => - provider.isAligned( - { - entropySource: this.#wallet.entropySource, - groupIndex: this.#groupIndex, - }, - this.#providerToAccounts.get(provider) ?? [], - ), + this.isProviderAligned(provider), + ); + } + + /** + * Check whether a single provider has an aligned account in this group. + * + * A provider is aligned when the account IDs it contributed to this group are + * non-empty and owned by it. Disabled {@link AccountProviderWrapper} instances + * always report `true`. + * + * @param provider - The provider to check. + * @returns `true` when the provider is aligned for this group. + */ + isProviderAligned(provider: Bip44AccountProvider): boolean { + return provider.isAligned( + { + entropySource: this.#wallet.entropySource, + groupIndex: this.#groupIndex, + }, + this.#providerToAccounts.get(provider) ?? [], ); } } diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 92c0eeec16..a91654ce4d 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -883,12 +883,87 @@ describe('MultichainAccountWallet', () => { await wallet.alignAccounts(); - // Sol provider is missing group 1; should be called via the batch range API covering all groups. + // Sol provider is missing group 1 only; it should be called for that + // missing sub-range only, NOT the already-aligned group 0. expect(providers[1].createAccounts).toHaveBeenCalledWith({ type: AccountCreationType.Bip44DeriveIndexRange, entropySource: wallet.entropySource, - range: { from: 0, to: 1 }, + range: { from: 1, to: 1 }, + }); + expect(providers[1].createAccounts).not.toHaveBeenCalledWith( + expect.objectContaining({ range: { from: 0, to: 1 } }), + ); + + // EVM provider already has both groups aligned, so it must not be asked + // to create (or re-trace) any account during alignment. + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + }); + + it('does not re-create accounts for providers that are already aligned across the whole range', async () => { + // Both groups have EVM + SOL accounts already; nothing is missing for SOL, + // but EVM is missing group 1 to force the wallet out of the aligned state. + 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 mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .withUuid() + .get(); + + const { wallet, providers } = setup({ + // EVM only has group 0 (missing group 1), SOL has both groups. + accounts: [[mockEvmAccount0], [mockSolAccount0, mockSolAccount1]], + }); + + await wallet.alignAccounts(); + + // SOL is aligned for every group in the range, so it must be skipped + // entirely (no spans, no work). + expect(providers[1].createAccounts).not.toHaveBeenCalled(); + }); + + it('creates accounts only for the non-contiguous missing sub-ranges', async () => { + // EVM present for groups 0, 1, 2, 3. SOL present for groups 0 and 2, so it + // is missing the non-contiguous indices 1 and 3. + const evmAccounts = [0, 1, 2, 3].map((groupIndex) => + MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(groupIndex) + .withUuid() + .get(), + ); + const solAccounts = [0, 2].map((groupIndex) => + MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(groupIndex) + .withUuid() + .get(), + ); + + const { wallet, providers } = setup({ + accounts: [evmAccounts, solAccounts], + }); + + await wallet.alignAccounts(); + + // Two separate single-index sub-ranges, one per gap. + expect(providers[1].createAccounts).toHaveBeenCalledWith({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: wallet.entropySource, + range: { from: 1, to: 1 }, + }); + expect(providers[1].createAccounts).toHaveBeenCalledWith({ + type: AccountCreationType.Bip44DeriveIndexRange, + entropySource: wallet.entropySource, + range: { from: 3, to: 3 }, }); + expect(providers[1].createAccounts).toHaveBeenCalledTimes(2); }); it('updates a group when a provider returns accounts during alignment', async () => { diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index 731fa3f777..0018b987b3 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -305,19 +305,23 @@ export class MultichainAccountWallet< } /** - * Build group state for a range of group indices by calling all providers in parallel. + * Build group state by calling all providers in parallel. * * This is a non-locking shared core used by both creation and alignment paths. + * Each provider is asked which contiguous sub-ranges of group indices it needs + * accounts created for via `getSubRanges`, so callers can skip indices that are + * already satisfied (e.g. during alignment). * - * @param from - Starting group index (inclusive). - * @param to - Ending group index (inclusive). * @param providers - The providers to create accounts for. + * @param getSubRanges - Resolver returning the sub-ranges to create for a + * given provider. Returning an empty array means the provider is skipped. * @returns The collected group state and any provider failure messages. */ - async #buildGroupStateForRange( - from: number, - to: number, + async #buildGroupState( providers: Bip44AccountProvider[], + getSubRanges: ( + provider: Bip44AccountProvider, + ) => Required[], ): Promise<{ groupStateByGroupIndex: Map; failures: string[]; @@ -327,23 +331,27 @@ export class MultichainAccountWallet< const results = await Promise.allSettled( providers.map(async (provider) => { const providerName = provider.getName(); - const accounts = await this.#createAccountsRangeForProvider( - provider, - from, - to, - ); - accounts.forEach((account) => { - const { groupIndex } = account.options.entropy; - let groupState = groupStateByGroupIndex.get(groupIndex); - if (!groupState) { - groupState = {}; - groupStateByGroupIndex.set(groupIndex, groupState); - } - if (!groupState[providerName]) { - groupState[providerName] = []; - } - groupState[providerName].push(account.id); - }); + const subRanges = getSubRanges(provider); + + for (const { from, to } of subRanges) { + const accounts = await this.#createAccountsRangeForProvider( + provider, + from, + to, + ); + accounts.forEach((account) => { + const { groupIndex } = account.options.entropy; + let groupState = groupStateByGroupIndex.get(groupIndex); + if (!groupState) { + groupState = {}; + groupStateByGroupIndex.set(groupIndex, groupState); + } + if (!groupState[providerName]) { + groupState[providerName] = []; + } + groupState[providerName].push(account.id); + }); + } }), ); @@ -360,6 +368,46 @@ export class MultichainAccountWallet< return { groupStateByGroupIndex, failures }; } + /** + * Compute the contiguous sub-ranges of group indices in `[from, to]` for which + * the given provider is NOT aligned (i.e. is missing an account). + * + * Already-aligned indices are skipped so alignment never re-creates (or + * re-traces) accounts that already exist. A group that does not exist yet is + * treated as unaligned. + * + * @param provider - The provider to compute missing sub-ranges for. + * @param from - Starting group index (inclusive). + * @param to - Ending group index (inclusive). + * @returns The contiguous sub-ranges where the provider needs accounts. + */ + #getUnalignedSubRangesForProvider( + provider: Bip44AccountProvider, + from: number, + to: number, + ): Required[] { + const subRanges: Required[] = []; + + let runStart: number | undefined; + for (let groupIndex = from; groupIndex <= to; groupIndex++) { + const group = this.getMultichainAccountGroup(groupIndex); + const aligned = group ? group.isProviderAligned(provider) : false; + + if (!aligned) { + runStart ??= groupIndex; + } else if (runStart !== undefined) { + subRanges.push({ from: runStart, to: groupIndex - 1 }); + runStart = undefined; + } + } + + if (runStart !== undefined) { + subRanges.push({ from: runStart, to }); + } + + return subRanges; + } + /** * Internal method to create a range of multichain account groups. * @@ -395,7 +443,7 @@ export class MultichainAccountWallet< this.#log(`Creating groups from index ${from} to ${to}...`); const { groupStateByGroupIndex, failures } = - await this.#buildGroupStateForRange(from, to, providers); + await this.#buildGroupState(providers, () => [{ from, to }]); // Check for provider failures — always treated as hard errors. if (failures.length) { @@ -461,7 +509,9 @@ export class MultichainAccountWallet< }, async () => { const { groupStateByGroupIndex, failures } = - await this.#buildGroupStateForRange(from, to, providers); + await this.#buildGroupState(providers, (provider) => + this.#getUnalignedSubRangesForProvider(provider, from, to), + ); if (failures.length) { const error = failures.reduce(