diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 0f50399dad5..869b035e9af 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 + +- Allow for multichain account group alignment through the `align` method ([#6326](https://github.com/MetaMask/core/pull/6326)) + - You can now call alignment from the group, wallet and service levels. + ### Changed - Bump `@metamask/base-controller` from `^8.0.1` to `^8.1.0` ([#6284](https://github.com/MetaMask/core/pull/6284)) diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts index 2ae820fdbf0..6241297ae6c 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.test.ts @@ -146,4 +146,62 @@ describe('MultichainAccount', () => { expect(group.select({ scopes: [SolScope.Mainnet] })).toStrictEqual([]); }); }); + + describe('align', () => { + it('creates missing accounts only for providers with no accounts', async () => { + const groupIndex = 0; + const { group, providers, wallet } = setup({ + groupIndex, + accounts: [ + [MOCK_WALLET_1_EVM_ACCOUNT], // provider[0] already has group 0 + [], // provider[1] missing group 0 + ], + }); + + await group.align(); + + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + expect(providers[1].createAccounts).toHaveBeenCalledWith({ + entropySource: wallet.entropySource, + groupIndex, + }); + }); + + it('does nothing when already aligned', async () => { + const groupIndex = 0; + const { group, providers } = setup({ + groupIndex, + accounts: [[MOCK_WALLET_1_EVM_ACCOUNT], [MOCK_WALLET_1_SOL_ACCOUNT]], + }); + + await group.align(); + + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + expect(providers[1].createAccounts).not.toHaveBeenCalled(); + }); + + it('warns if provider alignment fails', 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('Unable to create accounts'), + ); + + await group.align(); + + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + expect(providers[1].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`, + ); + }); + }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountGroup.ts b/packages/multichain-account-service/src/MultichainAccountGroup.ts index bc238d5b597..55c257cc248 100644 --- a/packages/multichain-account-service/src/MultichainAccountGroup.ts +++ b/packages/multichain-account-service/src/MultichainAccountGroup.ts @@ -188,4 +188,30 @@ export class MultichainAccountGroup< select(selector: AccountSelector): Account[] { return select(this.getAccounts(), selector); } + + /** + * Align the multichain account group. + * + * This will create accounts for providers that don't have any accounts yet. + */ + async align(): Promise { + const results = await Promise.allSettled( + this.#providers.map((provider) => { + const accounts = this.#providerToAccounts.get(provider); + if (!accounts || accounts.length === 0) { + return provider.createAccounts({ + entropySource: this.wallet.entropySource, + groupIndex: this.groupIndex, + }); + } + return Promise.resolve(); + }), + ); + + if (results.some((result) => result.status === 'rejected')) { + 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`, + ); + } + } } diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index cd2681d61cf..f159ce69941 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -15,6 +15,7 @@ import { MOCK_HD_ACCOUNT_2, MOCK_SNAP_ACCOUNT_1, MOCK_SNAP_ACCOUNT_2, + MOCK_SOL_ACCOUNT_1, MockAccountBuilder, } from './tests'; import { @@ -574,6 +575,57 @@ describe('MultichainAccountService', () => { }); }); + describe('alignWallets', () => { + it('aligns all multichain account wallets', async () => { + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_2.metadata.id) + .withGroupIndex(0) + .get(); + const { service, mocks } = setup({ + accounts: [mockEvmAccount1, mockSolAccount1], + }); + + await service.alignWallets(); + + expect(mocks.EvmAccountProvider.createAccounts).toHaveBeenCalledWith({ + entropySource: MOCK_HD_KEYRING_2.metadata.id, + groupIndex: 0, + }); + expect(mocks.SolAccountProvider.createAccounts).toHaveBeenCalledWith({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + }); + }); + + describe('alignWallet', () => { + it('aligns a specific multichain account wallet', async () => { + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_2.metadata.id) + .withGroupIndex(0) + .get(); + const { service, mocks } = setup({ + accounts: [mockEvmAccount1, mockSolAccount1], + }); + + await service.alignWallet(MOCK_HD_KEYRING_1.metadata.id); + + expect(mocks.SolAccountProvider.createAccounts).toHaveBeenCalledWith({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(mocks.EvmAccountProvider.createAccounts).not.toHaveBeenCalled(); + }); + }); + describe('actions', () => { it('gets a multichain account with MultichainAccountService:getMultichainAccount', () => { const accounts = [MOCK_HD_ACCOUNT_1]; @@ -647,5 +699,55 @@ describe('MultichainAccountService', () => { expect(firstGroup.getAccounts()).toHaveLength(1); expect(firstGroup.getAccounts()[0]).toStrictEqual(MOCK_HD_ACCOUNT_1); }); + + it('aligns a multichain account wallet with MultichainAccountService:alignWallet', async () => { + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_2.metadata.id) + .withGroupIndex(0) + .get(); + const { messenger, mocks } = setup({ + accounts: [mockEvmAccount1, mockSolAccount1], + }); + + await messenger.call( + 'MultichainAccountService:alignWallet', + MOCK_HD_KEYRING_1.metadata.id, + ); + + expect(mocks.SolAccountProvider.createAccounts).toHaveBeenCalledWith({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + expect(mocks.EvmAccountProvider.createAccounts).not.toHaveBeenCalled(); + }); + + it('aligns all multichain account wallets with MultichainAccountService:alignWallets', async () => { + const mockEvmAccount1 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockSolAccount1 = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_2.metadata.id) + .withGroupIndex(0) + .get(); + const { messenger, mocks } = setup({ + accounts: [mockEvmAccount1, mockSolAccount1], + }); + + await messenger.call('MultichainAccountService:alignWallets'); + + expect(mocks.EvmAccountProvider.createAccounts).toHaveBeenCalledWith({ + entropySource: MOCK_HD_KEYRING_2.metadata.id, + groupIndex: 0, + }); + expect(mocks.SolAccountProvider.createAccounts).toHaveBeenCalledWith({ + entropySource: MOCK_HD_KEYRING_1.metadata.id, + groupIndex: 0, + }); + }); }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index b9a4273d899..4937d038461 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -105,6 +105,14 @@ export class MultichainAccountService { 'MultichainAccountService:createMultichainAccountGroup', (...args) => this.createMultichainAccountGroup(...args), ); + this.#messenger.registerActionHandler( + 'MultichainAccountService:alignWallets', + (...args) => this.alignWallets(...args), + ); + this.#messenger.registerActionHandler( + 'MultichainAccountService:alignWallet', + (...args) => this.alignWallet(...args), + ); } /** @@ -350,4 +358,22 @@ export class MultichainAccountService { groupIndex, ); } + + /** + * Align all multichain account wallets. + */ + async alignWallets(): Promise { + const wallets = this.getMultichainAccountWallets(); + await Promise.all(wallets.map((w) => w.alignGroups())); + } + + /** + * Align a specific multichain account wallet. + * + * @param entropySource - The entropy source of the multichain account wallet. + */ + async alignWallet(entropySource: EntropySourceId): Promise { + const wallet = this.getMultichainAccountWallet({ entropySource }); + await wallet.alignGroups(); + } } diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index 3c832e5467f..c11964db3dd 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -346,4 +346,67 @@ describe('MultichainAccountWallet', () => { expect(internalAccounts[1].type).toBe(SolAccountType.DataAccount); }); }); + + describe('alignGroups', () => { + 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) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const mockEvmAccount2 = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(1) + .get(); + const mockSolAccount = MockAccountBuilder.from(MOCK_SOL_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(0) + .get(); + const { wallet, providers } = setup({ + accounts: [[mockEvmAccount1, mockEvmAccount2], [mockSolAccount]], + }); + + await wallet.alignGroups(); + + // EVM provider already has group 0 and 1; should not be called. + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + + // Sol provider is missing group 1; should be called to create it. + expect(providers[1].createAccounts).toHaveBeenCalledWith({ + entropySource: wallet.entropySource, + groupIndex: 1, + }); + }); + }); + + describe('alignGroup', () => { + it('aligns a specific multichain account group', async () => { + 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(1) + .get(); + const { wallet, providers } = setup({ + accounts: [[mockEvmAccount], [mockSolAccount]], + }); + + await wallet.alignGroup(0); + + // EVM provider already has group 0; should not be called. + expect(providers[0].createAccounts).not.toHaveBeenCalled(); + + // Sol provider is missing group 0; should be called to create it. + expect(providers[1].createAccounts).toHaveBeenCalledWith({ + entropySource: wallet.entropySource, + groupIndex: 0, + }); + + expect(providers[1].createAccounts).not.toHaveBeenCalledWith({ + entropySource: wallet.entropySource, + groupIndex: 1, + }); + }); + }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index be9b7af0601..bc068eee429 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -306,4 +306,24 @@ export class MultichainAccountWallet< > { return this.createMultichainAccountGroup(this.getNextGroupIndex()); } + + /** + * Align all multichain account groups. + */ + async alignGroups(): Promise { + const groups = this.getMultichainAccountGroups(); + await Promise.all(groups.map((g) => g.align())); + } + + /** + * Align a specific multichain account group. + * + * @param groupIndex - The group index to align. + */ + async alignGroup(groupIndex: number): Promise { + const group = this.getMultichainAccountGroup(groupIndex); + if (group) { + await group.align(); + } + } } diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index c6b5f847791..3655133aa5f 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -9,7 +9,7 @@ import type { import type { Hex } from '@metamask/utils'; import { - assertIsBip44Account, + assertAreBip44Accounts, BaseAccountProvider, } from './BaseAccountProvider'; @@ -69,9 +69,11 @@ export class EvmAccountProvider extends BaseAccountProvider { // We MUST have the associated internal account. assertInternalAccountExists(account); - assertIsBip44Account(account); - return [account]; + const accountsArray = [account]; + assertAreBip44Accounts(accountsArray); + + return accountsArray; } async discoverAndCreateAccounts(_: { diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index 298b22c5e53..7d8a1cb783d 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -6,10 +6,7 @@ import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { Json, SnapId } from '@metamask/snaps-sdk'; import type { MultichainAccountServiceMessenger } from 'src/types'; -import { - assertAreBip44Accounts, - BaseAccountProvider, -} from './BaseAccountProvider'; +import { BaseAccountProvider } from './BaseAccountProvider'; export type RestrictedSnapKeyringCreateAccount = ( options: Record, @@ -24,23 +21,7 @@ export abstract class SnapAccountProvider extends BaseAccountProvider { this.snapId = snapId; } - /** - * Execute the operation to create accounts. - * - * All accounts have to be BIP-44 compatible, otherwise this method will throw. - * - * @param createAccounts - Callback to create all accounts for this provider. The first - * argument of this callback is a function that can be used to create Snap account on - * the associated Snap of this provider. It will automatically skips any account - * creation confirmations if possible. - * @throws If any of the created accounts are not BIP-44 compatible. - * @returns The list of created accounts. - */ - protected async withCreateAccount( - createAccounts: ( - createAccount: RestrictedSnapKeyringCreateAccount, - ) => Promise, - ): Promise[]> { + protected async getRestrictedSnapAccountCreator(): Promise { // NOTE: We're not supposed to make the keyring instance escape `withKeyring` but // we have to use the `SnapKeyring` instance to be able to create Solana account // without triggering UI confirmation. @@ -54,17 +35,12 @@ export abstract class SnapAccountProvider extends BaseAccountProvider { keyring.createAccount.bind(keyring), ); - const accounts = await createAccounts((options) => + return (options) => createAccount(this.snapId, options, { displayAccountNameSuggestion: false, displayConfirmation: false, setSelectedAccount: false, - }), - ); - - assertAreBip44Accounts(accounts); - - return accounts; + }); } abstract isAccountCompatible(account: Bip44Account): boolean; diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 4fcbb64a1f4..094a7c6e082 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -9,7 +9,7 @@ import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { SnapId } from '@metamask/snaps-sdk'; import type { MultichainAccountServiceMessenger } from 'src/types'; -import { assertIsBip44Account } from './BaseAccountProvider'; +import { assertAreBip44Accounts } from './BaseAccountProvider'; import { SnapAccountProvider } from './SnapAccountProvider'; export class SolAccountProvider extends SnapAccountProvider { @@ -33,30 +33,31 @@ export class SolAccountProvider extends SnapAccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - return this.withCreateAccount(async (createAccount) => { - // Create account without any confirmation nor selecting it. - // TODO: Use the new keyring API `createAccounts` method with the "bip-44:derive-index" - // type once ready. - const derivationPath = `m/44'/501'/${groupIndex}'/0'`; - const account = await createAccount({ - entropySource, - derivationPath, - }); + const createAccount = await this.getRestrictedSnapAccountCreator(); - // Solana Snap does not use BIP-44 typed options for the moment - // so we "inject" them (the `AccountsController` does a similar thing - // for the moment). - account.options.entropy = { - type: KeyringAccountEntropyTypeOption.Mnemonic, - id: entropySource, - groupIndex, - derivationPath, - }; + // Create account without any confirmation nor selecting it. + // TODO: Use the new keyring API `createAccounts` method with the "bip-44:derive-index" + // type once ready. + const derivationPath = `m/44'/501'/${groupIndex}'/0'`; + const account = await createAccount({ + entropySource, + derivationPath, + }); - assertIsBip44Account(account); + // Solana Snap does not use BIP-44 typed options for the moment + // so we "inject" them (the `AccountsController` does a similar thing + // for the moment). + account.options.entropy = { + type: KeyringAccountEntropyTypeOption.Mnemonic, + id: entropySource, + groupIndex, + derivationPath, + }; - return [account]; - }); + const accounts = [account]; + assertAreBip44Accounts(accounts); + + return accounts; } async discoverAndCreateAccounts(_: { diff --git a/packages/multichain-account-service/src/types.ts b/packages/multichain-account-service/src/types.ts index 1dd73e799f8..c8cb682b3e5 100644 --- a/packages/multichain-account-service/src/types.ts +++ b/packages/multichain-account-service/src/types.ts @@ -48,6 +48,16 @@ export type MultichainAccountServiceCreateMultichainAccountGroupAction = { handler: MultichainAccountService['createMultichainAccountGroup']; }; +export type MultichainAccountServiceAlignWalletAction = { + type: `${typeof serviceName}:alignWallet`; + handler: MultichainAccountService['alignWallet']; +}; + +export type MultichainAccountServiceAlignWalletsAction = { + type: `${typeof serviceName}:alignWallets`; + handler: MultichainAccountService['alignWallets']; +}; + /** * All actions that {@link MultichainAccountService} registers so that other * modules can call them. @@ -58,7 +68,9 @@ export type MultichainAccountServiceActions = | MultichainAccountServiceGetMultichainAccountWalletAction | MultichainAccountServiceGetMultichainAccountWalletsAction | MultichainAccountServiceCreateNextMultichainAccountGroupAction - | MultichainAccountServiceCreateMultichainAccountGroupAction; + | MultichainAccountServiceCreateMultichainAccountGroupAction + | MultichainAccountServiceAlignWalletAction + | MultichainAccountServiceAlignWalletsAction; /** * All events that {@link MultichainAccountService} publishes so that other modules