From 86448c3499b757611c5fd029e43abe87715b65d8 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Thu, 18 Sep 2025 11:33:31 +0200 Subject: [PATCH 1/2] fix(account-tree-controller): enqueue backup & sync jobs using service events --- .../src/AccountTreeController.ts | 47 +++++++++++---- .../src/backup-and-sync/service/index.ts | 59 +++++++++++++++++++ packages/account-tree-controller/src/types.ts | 10 +++- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index 1537a6bb412..bd7a11cace7 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -5,13 +5,15 @@ import type { AccountGroupType, AccountSelector, MultichainAccountWalletId, + MultichainAccountGroup, + Bip44Account, } from '@metamask/account-api'; import type { MultichainAccountWalletStatus } from '@metamask/account-api'; import { type AccountId } from '@metamask/accounts-controller'; import type { StateMetadata } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import type { TraceCallback } from '@metamask/controller-utils'; -import { isEvmAccountType } from '@metamask/keyring-api'; +import { isEvmAccountType, KeyringAccount } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { BackupAndSyncEmitAnalyticsEventParams } from './backup-and-sync/analytics'; @@ -223,6 +225,20 @@ export class AccountTreeController extends BaseController< }, ); + this.messagingSystem.subscribe( + 'MultichainAccountService:multichainAccountGroupCreated', + (group) => { + this.#handleMultichainAccountGroupCreatedOrUpdated(group); + }, + ); + + this.messagingSystem.subscribe( + 'MultichainAccountService:multichainAccountGroupUpdated', + (group) => { + this.#handleMultichainAccountGroupCreatedOrUpdated(group); + }, + ); + this.#registerMessageHandlers(); } @@ -616,6 +632,25 @@ export class AccountTreeController extends BaseController< } } + /** + * Handles multichain account group created/updated event from + * the MultichainAccountService. + * + * @param multichainAccountGroup - Multichain account group being that got created or updated. + */ + #handleMultichainAccountGroupCreatedOrUpdated( + multichainAccountGroup: MultichainAccountGroup< + Bip44Account + >, + ): void { + // Trigger atomic sync for wallet and group (wallet will be synced only if it does + // not exist yet) + this.#backupAndSyncService.enqueueSingleWalletAndGroupSync( + multichainAccountGroup.wallet.id, + multichainAccountGroup.id, + ); + } + /** * Helper method to prune a group if it holds no accounts and additionally * prune the wallet if it holds no groups. This action should take place @@ -679,11 +714,6 @@ export class AccountTreeController extends BaseController< // the union tag `result.wallet.type`. } as AccountWalletObject; wallet = wallets[walletId]; - - // Trigger atomic sync for new wallet (only for entropy wallets) - if (wallet.type === AccountWalletType.Entropy) { - this.#backupAndSyncService.enqueueSingleWalletSync(walletId); - } } const groupId = result.group.id; @@ -705,11 +735,6 @@ export class AccountTreeController extends BaseController< // Map group ID to its containing wallet ID for efficient direct access this.#groupIdToWalletId.set(groupId, walletId); - - // Trigger atomic sync for new group (only for entropy wallets) - if (wallet.type === AccountWalletType.Entropy) { - this.#backupAndSyncService.enqueueSingleGroupSync(groupId); - } } else { group.accounts.push(account.id); } diff --git a/packages/account-tree-controller/src/backup-and-sync/service/index.ts b/packages/account-tree-controller/src/backup-and-sync/service/index.ts index 5bac3a2c76b..20977742fba 100644 --- a/packages/account-tree-controller/src/backup-and-sync/service/index.ts +++ b/packages/account-tree-controller/src/backup-and-sync/service/index.ts @@ -194,6 +194,34 @@ export class BackupAndSyncService { ); } + /** + * Enqueues a single group sync operation (fire-and-forget). + * If the first full sync has not yet occurred, it does nothing. + * + * @param walletId - The wallet ID to sync. + * @param groupId - The group ID to sync. + */ + enqueueSingleWalletAndGroupSync( + walletId: AccountWalletId, + groupId: AccountGroupId, + ): void { + if (!this.isBackupAndSyncEnabled || !this.hasSyncedAtLeastOnce) { + return; + } + + // eslint-disable-next-line no-void + void this.#atomicSyncQueue.enqueue(async () => { + const hasSyncedWalletAtLeastOnce = + await this.hasSyncedWalletAtLeastOnce(walletId); + + if (!hasSyncedWalletAtLeastOnce) { + await this.#performSingleWalletSyncInner(walletId); + } + + await this.#performSingleGroupSyncInner(groupId); + }); + } + /** * Performs a full synchronization of the local account tree with user storage, ensuring consistency * between local state and cloud-stored account data. @@ -449,6 +477,37 @@ export class BackupAndSyncService { } } + /** + * Performs a single wallet's bidirectional metadata sync with user storage. + * + * @param walletId - The wallet ID to sync. + * @returns True if this wallet has been synced already, false otherwise. + */ + async hasSyncedWalletAtLeastOnce( + walletId: AccountWalletId, + ): Promise { + try { + const wallet = this.#getEntropyWallet(walletId); + if (!wallet) { + return false; // Only sync entropy wallets + } + + const entropySourceId = wallet.metadata.entropy.id; + const walletFromUserStorage = await getWalletFromUserStorage( + this.#context, + entropySourceId, + ); + + return walletFromUserStorage !== null; + } catch (error) { + backupAndSyncLogger( + `Error in single wallet sync for ${walletId} (hasSyncWalletAtLeastOnce):`, + error, + ); + return false; + } + } + /** * Performs a single wallet's bidirectional metadata sync with user storage. * diff --git a/packages/account-tree-controller/src/types.ts b/packages/account-tree-controller/src/types.ts index 231a3de226d..8527ad80092 100644 --- a/packages/account-tree-controller/src/types.ts +++ b/packages/account-tree-controller/src/types.ts @@ -38,7 +38,12 @@ import type { AccountWalletObject, AccountTreeWalletPersistedMetadata, } from './wallet'; -import type { MultichainAccountServiceWalletStatusChangeEvent } from '../../multichain-account-service/src/types'; +import type { + MultichainAccountServiceGetMultichainAccountWalletsAction, + MultichainAccountServiceMultichainAccountGroupCreatedEvent, + MultichainAccountServiceMultichainAccountGroupUpdatedEvent, + MultichainAccountServiceWalletStatusChangeEvent, +} from '../../multichain-account-service/src/types'; // Backward compatibility aliases using indexed access types /** @@ -126,6 +131,7 @@ export type AllowedActions = | UserStorageController.UserStorageControllerPerformSetStorage | UserStorageController.UserStorageControllerPerformBatchSetStorage | AuthenticationController.AuthenticationControllerGetSessionProfile + | MultichainAccountServiceGetMultichainAccountWalletsAction | MultichainAccountServiceCreateMultichainAccountGroupAction; export type AccountTreeControllerActions = @@ -166,6 +172,8 @@ export type AllowedEvents = | AccountsControllerAccountRemovedEvent | AccountsControllerSelectedAccountChangeEvent | UserStorageController.UserStorageControllerStateChangeEvent + | MultichainAccountServiceMultichainAccountGroupCreatedEvent + | MultichainAccountServiceMultichainAccountGroupUpdatedEvent | MultichainAccountServiceWalletStatusChangeEvent; export type AccountTreeControllerEvents = From 3facabc00b67aa49c355e19827be6a07ba57dd63 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Thu, 18 Sep 2025 11:43:55 +0200 Subject: [PATCH 2/2] chore: revert wallet changes --- .../src/AccountTreeController.ts | 18 +++--- .../src/backup-and-sync/service/index.ts | 59 ------------------- 2 files changed, 8 insertions(+), 69 deletions(-) diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index bd7a11cace7..dd51a39bc32 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -636,19 +636,12 @@ export class AccountTreeController extends BaseController< * Handles multichain account group created/updated event from * the MultichainAccountService. * - * @param multichainAccountGroup - Multichain account group being that got created or updated. + * @param group - Multichain account group being that got created or updated. */ #handleMultichainAccountGroupCreatedOrUpdated( - multichainAccountGroup: MultichainAccountGroup< - Bip44Account - >, + group: MultichainAccountGroup>, ): void { - // Trigger atomic sync for wallet and group (wallet will be synced only if it does - // not exist yet) - this.#backupAndSyncService.enqueueSingleWalletAndGroupSync( - multichainAccountGroup.wallet.id, - multichainAccountGroup.id, - ); + this.#backupAndSyncService.enqueueSingleGroupSync(group.id); } /** @@ -714,6 +707,11 @@ export class AccountTreeController extends BaseController< // the union tag `result.wallet.type`. } as AccountWalletObject; wallet = wallets[walletId]; + + // Trigger atomic sync for new wallet (only for entropy wallets) + if (wallet.type === AccountWalletType.Entropy) { + this.#backupAndSyncService.enqueueSingleWalletSync(walletId); + } } const groupId = result.group.id; diff --git a/packages/account-tree-controller/src/backup-and-sync/service/index.ts b/packages/account-tree-controller/src/backup-and-sync/service/index.ts index 20977742fba..5bac3a2c76b 100644 --- a/packages/account-tree-controller/src/backup-and-sync/service/index.ts +++ b/packages/account-tree-controller/src/backup-and-sync/service/index.ts @@ -194,34 +194,6 @@ export class BackupAndSyncService { ); } - /** - * Enqueues a single group sync operation (fire-and-forget). - * If the first full sync has not yet occurred, it does nothing. - * - * @param walletId - The wallet ID to sync. - * @param groupId - The group ID to sync. - */ - enqueueSingleWalletAndGroupSync( - walletId: AccountWalletId, - groupId: AccountGroupId, - ): void { - if (!this.isBackupAndSyncEnabled || !this.hasSyncedAtLeastOnce) { - return; - } - - // eslint-disable-next-line no-void - void this.#atomicSyncQueue.enqueue(async () => { - const hasSyncedWalletAtLeastOnce = - await this.hasSyncedWalletAtLeastOnce(walletId); - - if (!hasSyncedWalletAtLeastOnce) { - await this.#performSingleWalletSyncInner(walletId); - } - - await this.#performSingleGroupSyncInner(groupId); - }); - } - /** * Performs a full synchronization of the local account tree with user storage, ensuring consistency * between local state and cloud-stored account data. @@ -477,37 +449,6 @@ export class BackupAndSyncService { } } - /** - * Performs a single wallet's bidirectional metadata sync with user storage. - * - * @param walletId - The wallet ID to sync. - * @returns True if this wallet has been synced already, false otherwise. - */ - async hasSyncedWalletAtLeastOnce( - walletId: AccountWalletId, - ): Promise { - try { - const wallet = this.#getEntropyWallet(walletId); - if (!wallet) { - return false; // Only sync entropy wallets - } - - const entropySourceId = wallet.metadata.entropy.id; - const walletFromUserStorage = await getWalletFromUserStorage( - this.#context, - entropySourceId, - ); - - return walletFromUserStorage !== null; - } catch (error) { - backupAndSyncLogger( - `Error in single wallet sync for ${walletId} (hasSyncWalletAtLeastOnce):`, - error, - ); - return false; - } - } - /** * Performs a single wallet's bidirectional metadata sync with user storage. *