From 00b6fff50d4bf7a0ef5bb2413ea7df6437a7dabe Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 13 May 2026 09:46:44 -0400 Subject: [PATCH 1/5] fix: fetch fixes --- eslint-suppressions.json | 2 +- packages/account-tree-controller/CHANGELOG.md | 8 + ...countTreeController-method-action-types.ts | 22 +- .../src/AccountTreeController.test.ts | 43 +++ .../src/AccountTreeController.ts | 27 +- .../backup-and-sync/analytics/traces.test.ts | 1 + .../src/backup-and-sync/analytics/traces.ts | 1 + .../src/backup-and-sync/service/index.test.ts | 237 ++++++++++++ .../src/backup-and-sync/service/index.ts | 353 +++++++++++------- packages/account-tree-controller/src/index.ts | 1 + 10 files changed, 565 insertions(+), 130 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index f417b23b79..c68ea72ce2 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -37,7 +37,7 @@ "count": 1 }, "n/no-sync": { - "count": 22 + "count": 25 } }, "packages/account-tree-controller/src/backup-and-sync/service/index.ts": { diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index 197d72af33..d392f1ad25 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `AccountTreeController.syncWalletWithUserStorage(entropySourceId)` and the corresponding `AccountTreeController:syncWalletWithUserStorage` messenger action, which performs a bidirectional user-storage sync for a single entropy wallet (wallet metadata + groups) without iterating every local wallet. Use this in place of `syncWithUserStorage` after operations that only affect one wallet (e.g., SRP import). Does not mark the controller as having completed its first full sync. + +### Fixed + +- Skip the pre-emptive `multichain_accounts_groups/` GET when an account group has just been created locally and cannot yet exist in user storage, eliminating an unnecessary 404 round-trip on every account creation. + ## [7.2.0] ### Changed diff --git a/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts b/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts index ae86e20f01..98a549533e 100644 --- a/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts +++ b/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts @@ -176,6 +176,25 @@ export type AccountTreeControllerSyncWithUserStorageAtLeastOnceAction = { handler: AccountTreeController['syncWithUserStorageAtLeastOnce']; }; +/** + * Bi-directionally syncs a single entropy wallet with user storage, scoped + * by entropy source ID. Use this in place of `syncWithUserStorage` when + * only one wallet's state has changed (e.g., after an SRP import) to avoid + * the per-wallet fanout of fetches that a full sync triggers. + * + * IMPORTANT: + * If a full sync is already in progress, returns the ongoing full-sync + * promise. This call does NOT mark the controller as having completed + * its first full sync. + * + * @param entropySourceId - The entropy source ID of the wallet to sync. + * @returns A promise that resolves when the sync is complete. + */ +export type AccountTreeControllerSyncWalletWithUserStorageAction = { + type: `AccountTreeController:syncWalletWithUserStorage`; + handler: AccountTreeController['syncWalletWithUserStorage']; +}; + /** * Union of all AccountTreeController action types. */ @@ -193,4 +212,5 @@ export type AccountTreeControllerMethodActions = | AccountTreeControllerSetAccountGroupHiddenAction | AccountTreeControllerClearStateAction | AccountTreeControllerSyncWithUserStorageAction - | AccountTreeControllerSyncWithUserStorageAtLeastOnceAction; + | AccountTreeControllerSyncWithUserStorageAtLeastOnceAction + | AccountTreeControllerSyncWalletWithUserStorageAction; diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 71db24735f..25c43e93dd 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/packages/account-tree-controller/src/AccountTreeController.test.ts @@ -4830,6 +4830,49 @@ describe('AccountTreeController', () => { }); }); + describe('syncWalletWithUserStorage', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('calls performSyncForWallet on the syncing service', async () => { + const performSyncForWalletSpy = jest + .spyOn(BackupAndSyncService.prototype, 'performSyncForWallet') + .mockResolvedValue(undefined); + + const { controller } = setup({ + accounts: [MOCK_HARDWARE_ACCOUNT_1], + keyrings: [MOCK_HD_KEYRING_1], + }); + + controller.init(); + + await controller.syncWalletWithUserStorage('test-entropy-id'); + + expect(performSyncForWalletSpy).toHaveBeenCalledTimes(1); + expect(performSyncForWalletSpy).toHaveBeenCalledWith('test-entropy-id'); + }); + + it('handles sync errors gracefully', async () => { + const syncError = new Error('Sync failed'); + const performSyncForWalletSpy = jest + .spyOn(BackupAndSyncService.prototype, 'performSyncForWallet') + .mockRejectedValue(syncError); + + const { controller } = setup({ + accounts: [MOCK_HARDWARE_ACCOUNT_1], + keyrings: [MOCK_HD_KEYRING_1], + }); + + controller.init(); + + await expect( + controller.syncWalletWithUserStorage('test-entropy-id'), + ).rejects.toThrow(syncError.message); + expect(performSyncForWalletSpy).toHaveBeenCalledTimes(1); + }); + }); + describe('UserStorageController:stateChange subscription', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index 676df296fa..c7a4f996c3 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -59,6 +59,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'clearState', 'syncWithUserStorage', 'syncWithUserStorageAtLeastOnce', + 'syncWalletWithUserStorage', ] as const; const accountTreeControllerMetadata: StateMetadata = @@ -1104,9 +1105,13 @@ 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) + // Trigger atomic sync for new group (only for entropy wallets). + // The group was just created locally and cannot yet exist in user storage, + // so skip the pre-emptive GET and push directly. if (wallet.type === AccountWalletType.Entropy) { - this.#backupAndSyncService.enqueueSingleGroupSync(groupId); + this.#backupAndSyncService.enqueueSingleGroupSync(groupId, { + isFreshlyCreated: true, + }); } } else { group.accounts.push(id); @@ -1695,6 +1700,24 @@ export class AccountTreeController extends BaseController< return this.#backupAndSyncService.performFullSyncAtLeastOnce(); } + /** + * Bi-directionally syncs a single entropy wallet with user storage, scoped + * by entropy source ID. Use this in place of `syncWithUserStorage` when + * only one wallet's state has changed (e.g., after an SRP import) to avoid + * the per-wallet fanout of fetches that a full sync triggers. + * + * IMPORTANT: + * If a full sync is already in progress, returns the ongoing full-sync + * promise. This call does NOT mark the controller as having completed + * its first full sync. + * + * @param entropySourceId - The entropy source ID of the wallet to sync. + * @returns A promise that resolves when the sync is complete. + */ + async syncWalletWithUserStorage(entropySourceId: string): Promise { + return this.#backupAndSyncService.performSyncForWallet(entropySourceId); + } + /** * Creates an backup and sync context for sync operations. * Used by the backup and sync service. diff --git a/packages/account-tree-controller/src/backup-and-sync/analytics/traces.test.ts b/packages/account-tree-controller/src/backup-and-sync/analytics/traces.test.ts index 791ccdd563..c37b6773b0 100644 --- a/packages/account-tree-controller/src/backup-and-sync/analytics/traces.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/analytics/traces.test.ts @@ -7,6 +7,7 @@ describe('BackupAndSyncAnalytics - Traces', () => { it('contains expected trace names', () => { expect(TraceName).toStrictEqual({ AccountSyncFull: 'Multichain Account Syncing - Full', + AccountSyncWallet: 'Multichain Account Syncing - Wallet', }); }); }); diff --git a/packages/account-tree-controller/src/backup-and-sync/analytics/traces.ts b/packages/account-tree-controller/src/backup-and-sync/analytics/traces.ts index 7383fadebf..5340cac8ba 100644 --- a/packages/account-tree-controller/src/backup-and-sync/analytics/traces.ts +++ b/packages/account-tree-controller/src/backup-and-sync/analytics/traces.ts @@ -6,6 +6,7 @@ import type { export const TraceName = { AccountSyncFull: 'Multichain Account Syncing - Full', + AccountSyncWallet: 'Multichain Account Syncing - Wallet', } as const; /** diff --git a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts index ebbda948e3..e676797b33 100644 --- a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts @@ -4,7 +4,13 @@ import { BackupAndSyncService } from '.'; import type { AccountGroupObject } from '../../group'; import type { AccountWalletEntropyObject } from '../../wallet'; import { getProfileId } from '../authentication'; +import { syncGroupMetadata } from '../syncing'; import type { BackupAndSyncContext } from '../types'; +import { + getAllGroupsFromUserStorage, + getGroupFromUserStorage, + getWalletFromUserStorage, +} from '../user-storage'; // We only need to import the functions we actually spy on import { getLocalEntropyWallets } from '../utils'; @@ -20,6 +26,21 @@ const mockGetProfileId = getProfileId as jest.MockedFunction< >; const mockGetLocalEntropyWallets = getLocalEntropyWallets as jest.MockedFunction; +const mockGetGroupFromUserStorage = + getGroupFromUserStorage as jest.MockedFunction< + typeof getGroupFromUserStorage + >; +const mockGetWalletFromUserStorage = + getWalletFromUserStorage as jest.MockedFunction< + typeof getWalletFromUserStorage + >; +const mockGetAllGroupsFromUserStorage = + getAllGroupsFromUserStorage as jest.MockedFunction< + typeof getAllGroupsFromUserStorage + >; +const mockSyncGroupMetadata = syncGroupMetadata as jest.MockedFunction< + typeof syncGroupMetadata +>; describe('BackupAndSync - Service - BackupAndSyncService', () => { let mockContext: BackupAndSyncContext; @@ -271,6 +292,93 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { // Should have called getProfileId as part of group sync expect(mockGetProfileId).toHaveBeenCalled(); }); + + it('fetches the group from user storage by default before syncing', async () => { + mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = true; + + mockContext.groupIdToWalletId.set( + 'entropy:wallet-1/1', + 'entropy:wallet-1', + ); + mockContext.controller.state.accountTree.wallets = { + 'entropy:wallet-1': { + id: 'entropy:wallet-1', + type: AccountWalletType.Entropy, + metadata: { + entropy: { id: 'test-entropy-id' }, + name: 'Test Wallet', + }, + groups: { + 'entropy:wallet-1/1': { + id: 'entropy:wallet-1/1', + name: 'Test Group', + metadata: { + entropy: { groupIndex: 1 }, + }, + } as unknown as AccountGroupObject, + }, + } as unknown as AccountWalletEntropyObject, + }; + + backupAndSyncService.enqueueSingleGroupSync('entropy:wallet-1/1'); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(mockGetGroupFromUserStorage).toHaveBeenCalledWith( + expect.anything(), + 'test-entropy-id', + 1, + ); + expect(mockSyncGroupMetadata).toHaveBeenCalled(); + }); + + it('skips the user storage fetch when called with isFreshlyCreated and pushes directly', async () => { + mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = true; + + mockContext.groupIdToWalletId.set( + 'entropy:wallet-1/1', + 'entropy:wallet-1', + ); + const localGroup = { + id: 'entropy:wallet-1/1', + name: 'Test Group', + metadata: { + entropy: { groupIndex: 1 }, + }, + } as unknown as AccountGroupObject; + mockContext.controller.state.accountTree.wallets = { + 'entropy:wallet-1': { + id: 'entropy:wallet-1', + type: AccountWalletType.Entropy, + metadata: { + entropy: { id: 'test-entropy-id' }, + name: 'Test Wallet', + }, + groups: { + 'entropy:wallet-1/1': localGroup, + }, + } as unknown as AccountWalletEntropyObject, + }; + + backupAndSyncService.enqueueSingleGroupSync('entropy:wallet-1/1', { + isFreshlyCreated: true, + }); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + // The pre-emptive GET must be skipped for freshly created groups. + expect(mockGetGroupFromUserStorage).not.toHaveBeenCalled(); + + // syncGroupMetadata must still be invoked, and given null for the + // user-storage value so it short-circuits to a single PUT. + expect(mockSyncGroupMetadata).toHaveBeenCalledWith( + expect.anything(), + localGroup, + null, + 'test-entropy-id', + 'test-profile-id', + ); + }); }); describe('performFullSync', () => { @@ -710,4 +818,133 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { expect(syncExecutionCount).toBe(2); // Still only 2 }, 15000); // Increase timeout to 15 seconds }); + + describe('performSyncForWallet', () => { + beforeEach(() => { + setupMockUserStorageControllerState(true, true); + jest.clearAllMocks(); + mockGetLocalEntropyWallets.mockClear(); + }); + + it('returns early when backup and sync is disabled', async () => { + setupMockUserStorageControllerState(false, true); + + const result = + await backupAndSyncService.performSyncForWallet('test-entropy-id'); + + expect(result).toBeUndefined(); + expect(mockContext.controllerStateUpdateFn).not.toHaveBeenCalled(); + expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); + }); + + it('syncs only the matched wallet, not every local wallet', async () => { + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + { + id: 'entropy:wallet-2', + metadata: { entropy: { id: 'entropy-id-2' } }, + } as unknown as AccountWalletEntropyObject, + ]); + mockGetWalletFromUserStorage.mockResolvedValue(null); + mockGetAllGroupsFromUserStorage.mockResolvedValue([]); + + await backupAndSyncService.performSyncForWallet('entropy-id-2'); + + // The fetches should target only the requested wallet's entropy source. + expect(mockGetWalletFromUserStorage).toHaveBeenCalledTimes(1); + expect(mockGetWalletFromUserStorage).toHaveBeenCalledWith( + expect.anything(), + 'entropy-id-2', + ); + expect(mockGetAllGroupsFromUserStorage).toHaveBeenCalledTimes(1); + expect(mockGetAllGroupsFromUserStorage).toHaveBeenCalledWith( + expect.anything(), + 'entropy-id-2', + ); + expect(mockGetProfileId).toHaveBeenCalledTimes(1); + expect(mockGetProfileId).toHaveBeenCalledWith( + expect.anything(), + 'entropy-id-2', + ); + }); + + it('does not flip hasAccountTreeSyncingSyncedAtLeastOnce', async () => { + mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = false; + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + ]); + mockGetWalletFromUserStorage.mockResolvedValue(null); + mockGetAllGroupsFromUserStorage.mockResolvedValue([]); + + // Capture state mutations applied via controllerStateUpdateFn so we can + // assert the flag was never touched. + const draft: Record = { + hasAccountTreeSyncingSyncedAtLeastOnce: false, + isAccountTreeSyncingInProgress: false, + }; + (mockContext.controllerStateUpdateFn as jest.Mock).mockImplementation( + (updater: (state: typeof draft) => void) => { + updater(draft); + }, + ); + + await backupAndSyncService.performSyncForWallet('entropy-id-1'); + + expect(draft.hasAccountTreeSyncingSyncedAtLeastOnce).toBe(false); + // The in-progress flag is toggled on then off during the sync. + expect(draft.isAccountTreeSyncingInProgress).toBe(false); + }); + + it('returns the in-flight full sync promise when one is running', async () => { + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + ]); + + // Make the full sync hang so we can observe the wallet-scoped call + // dedup against it. + let resolveTrace: (() => void) | undefined; + const tracePromise = new Promise((resolve) => { + resolveTrace = resolve; + }); + (mockContext.traceFn as jest.Mock).mockImplementation( + (_: unknown, fn: () => unknown) => { + fn(); + return tracePromise; + }, + ); + + const fullSyncPromise = backupAndSyncService.performFullSync(); + const walletSyncPromise = + backupAndSyncService.performSyncForWallet('entropy-id-1'); + + expect(walletSyncPromise).toStrictEqual(fullSyncPromise); + + resolveTrace?.(); + await Promise.all([fullSyncPromise, walletSyncPromise]); + }); + + it('is a no-op when no local wallet matches the entropy source ID', async () => { + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + ]); + + await backupAndSyncService.performSyncForWallet('unknown-entropy-id'); + + expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); + expect(mockGetAllGroupsFromUserStorage).not.toHaveBeenCalled(); + expect(mockGetProfileId).not.toHaveBeenCalled(); + }); + }); }); 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 852af9daad..8dc6d407be 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 @@ -183,8 +183,15 @@ export class BackupAndSyncService { * If the first full sync has not yet occurred, it does nothing. * * @param groupId - The group ID to sync. + * @param options - Optional sync configuration. + * @param options.isFreshlyCreated - When true, skip the user-storage GET because + * the group was just created locally and cannot yet exist remotely. The push is + * still performed via `syncGroupMetadata`. */ - enqueueSingleGroupSync(groupId: AccountGroupId): void { + enqueueSingleGroupSync( + groupId: AccountGroupId, + options?: { isFreshlyCreated?: boolean }, + ): void { if ( !this.isBackupAndSyncEnabled || !this.hasSyncedAtLeastOnce || @@ -198,9 +205,11 @@ export class BackupAndSyncService { return; } + const isFreshlyCreated = options?.isFreshlyCreated ?? false; + // eslint-disable-next-line no-void void this.#atomicSyncQueue.enqueue(() => - this.#performSingleGroupSyncInner(groupId), + this.#performSingleGroupSyncInner(groupId, isFreshlyCreated), ); } @@ -314,123 +323,7 @@ export class BackupAndSyncService { // 2. Iterate over each local wallet for (const wallet of localSyncableWallets) { - const entropySourceId = wallet.metadata.entropy.id; - - let walletProfileId: ProfileId; - let walletFromUserStorage: UserStorageSyncedWallet | null; - let groupsFromUserStorage: UserStorageSyncedWalletGroup[]; - - try { - walletProfileId = await getProfileId( - this.#context, - entropySourceId, - ); - - [walletFromUserStorage, groupsFromUserStorage] = await Promise.all([ - getWalletFromUserStorage(this.#context, entropySourceId), - getAllGroupsFromUserStorage(this.#context, entropySourceId), - ]); - - // 2.1 Decide if we need to perform legacy account syncing - if ( - !walletFromUserStorage || - !walletFromUserStorage.isLegacyAccountSyncingDisabled - ) { - // 2.2 Perform legacy account syncing - // This will migrate legacy account data to the new structure. - // This operation will only be performed once. - await performLegacyAccountSyncing( - this.#context, - entropySourceId, - walletProfileId, - ); - } - } catch (error) { - const errorMessage = toErrorMessage(error); - - backupAndSyncLogger( - `Legacy syncing failed for wallet ${wallet.id}: ${errorMessage}`, - ); - throw new Error( - `Legacy syncing failed for wallet: ${errorMessage}`, - ); - } - - // 3. Execute multichain account syncing - let stateSnapshot: StateSnapshot | undefined; - - try { - // 3.1 Wallet syncing - // Create a state snapshot before processing each wallet for potential rollback - stateSnapshot = createStateSnapshot(this.#context); - - // Sync wallet metadata bidirectionally - await syncWalletMetadata( - this.#context, - wallet, - walletFromUserStorage, - walletProfileId, - ); - - // 3.2 Groups syncing - // If groups data does not exist in user storage yet, create it - if (!groupsFromUserStorage.length) { - // If no groups exist in user storage, we can push all groups from the wallet to the user storage and exit - await pushGroupToUserStorageBatch( - this.#context, - getLocalGroupsForEntropyWallet(this.#context, wallet.id), - entropySourceId, - ); - - continue; // No need to proceed with metadata comparison if groups are new - } - - // Create local groups for each group from user storage if they do not exist - // This will ensure that we have all groups available locally before syncing metadata - await createLocalGroupsFromUserStorage( - this.#context, - groupsFromUserStorage, - entropySourceId, - walletProfileId, - ); - - // Sync group metadata bidirectionally - await syncGroupsMetadata( - this.#context, - wallet, - groupsFromUserStorage, - entropySourceId, - walletProfileId, - ); - } catch (error) { - const errorMessage = toErrorMessage(error); - const errorString = `Error during multichain account syncing for wallet ${wallet.id}: ${errorMessage}`; - - backupAndSyncLogger(errorString); - - // Attempt to rollback state changes for this wallet - try { - if (!stateSnapshot) { - throw new Error( - `State snapshot is missing for wallet ${wallet.id}`, - ); - } - restoreStateFromSnapshot(this.#context, stateSnapshot); - backupAndSyncLogger( - `Rolled back state changes for wallet ${wallet.id}`, - ); - } catch (rollbackError) { - backupAndSyncLogger( - `Failed to rollback state for wallet ${wallet.id}:`, - rollbackError instanceof Error - ? rollbackError.message - : String(rollbackError), - ); - } - - // Continue with next wallet instead of failing the entire sync - continue; - } + await this.#performWalletSyncInner(wallet); } } catch (error) { backupAndSyncLogger('Error during multichain account syncing:', error); @@ -460,6 +353,205 @@ export class BackupAndSyncService { } } + /** + * Performs the bidirectional sync work for a single entropy wallet: + * legacy account syncing (when needed), wallet metadata sync, and group + * metadata sync. Mirrors the per-wallet body of {@link #performFullSyncInner}. + * + * Per-wallet errors are isolated: legacy sync failures bubble up, but + * multichain sync failures trigger a state rollback for that wallet only, + * allowing the caller to continue with the next wallet (full sync) or + * resolve cleanly (single-wallet sync). + * + * @param wallet - The local entropy wallet to sync. + */ + async #performWalletSyncInner( + wallet: AccountWalletEntropyObject, + ): Promise { + const entropySourceId = wallet.metadata.entropy.id; + + let walletProfileId: ProfileId; + let walletFromUserStorage: UserStorageSyncedWallet | null; + let groupsFromUserStorage: UserStorageSyncedWalletGroup[]; + + try { + walletProfileId = await getProfileId(this.#context, entropySourceId); + + [walletFromUserStorage, groupsFromUserStorage] = await Promise.all([ + getWalletFromUserStorage(this.#context, entropySourceId), + getAllGroupsFromUserStorage(this.#context, entropySourceId), + ]); + + // Decide if we need to perform legacy account syncing + if ( + !walletFromUserStorage || + !walletFromUserStorage.isLegacyAccountSyncingDisabled + ) { + // Perform legacy account syncing. + // This will migrate legacy account data to the new structure. + // This operation will only be performed once. + await performLegacyAccountSyncing( + this.#context, + entropySourceId, + walletProfileId, + ); + } + } catch (error) { + const errorMessage = toErrorMessage(error); + + backupAndSyncLogger( + `Legacy syncing failed for wallet ${wallet.id}: ${errorMessage}`, + ); + throw new Error(`Legacy syncing failed for wallet: ${errorMessage}`); + } + + // Execute multichain account syncing + let stateSnapshot: StateSnapshot | undefined; + + try { + // Wallet syncing. + // Create a state snapshot before processing each wallet for potential rollback + stateSnapshot = createStateSnapshot(this.#context); + + // Sync wallet metadata bidirectionally + await syncWalletMetadata( + this.#context, + wallet, + walletFromUserStorage, + walletProfileId, + ); + + // Groups syncing. + // If groups data does not exist in user storage yet, create it + if (!groupsFromUserStorage.length) { + // If no groups exist in user storage, we can push all groups from the wallet to the user storage and exit + await pushGroupToUserStorageBatch( + this.#context, + getLocalGroupsForEntropyWallet(this.#context, wallet.id), + entropySourceId, + ); + + return; // No need to proceed with metadata comparison if groups are new + } + + // Create local groups for each group from user storage if they do not exist + // This will ensure that we have all groups available locally before syncing metadata + await createLocalGroupsFromUserStorage( + this.#context, + groupsFromUserStorage, + entropySourceId, + walletProfileId, + ); + + // Sync group metadata bidirectionally + await syncGroupsMetadata( + this.#context, + wallet, + groupsFromUserStorage, + entropySourceId, + walletProfileId, + ); + } catch (error) { + const errorMessage = toErrorMessage(error); + const errorString = `Error during multichain account syncing for wallet ${wallet.id}: ${errorMessage}`; + + backupAndSyncLogger(errorString); + + // Attempt to rollback state changes for this wallet + try { + if (!stateSnapshot) { + throw new Error(`State snapshot is missing for wallet ${wallet.id}`); + } + restoreStateFromSnapshot(this.#context, stateSnapshot); + backupAndSyncLogger( + `Rolled back state changes for wallet ${wallet.id}`, + ); + } catch (rollbackError) { + backupAndSyncLogger( + `Failed to rollback state for wallet ${wallet.id}:`, + rollbackError instanceof Error + ? rollbackError.message + : String(rollbackError), + ); + } + } + } + + /** + * Performs a bidirectional sync with user storage for a single entropy wallet. + * + * Use this in place of {@link performFullSync} when only one wallet's state + * has changed (e.g., immediately after an SRP import) to avoid the + * per-wallet fanout of fetches that a full sync triggers. + * + * Behavior: + * - Returns early if backup and sync is disabled. + * - If a full sync is already in flight, returns the in-flight promise so + * callers don't race against it. + * - Does NOT flip `hasAccountTreeSyncingSyncedAtLeastOnce`. A scoped sync + * for one wallet does not satisfy the canonical "first full sync" + * contract, since other wallets may still need legacy migration. + * + * @param entropySourceId - The entropy source ID of the wallet to sync. + * @returns A promise that resolves when the sync is complete. + */ + async performSyncForWallet(entropySourceId: string): Promise { + if (!this.isBackupAndSyncEnabled) { + return undefined; + } + + // Defer to the in-flight full sync so we don't race against it. + if (this.#ongoingFullSyncPromise) { + return this.#ongoingFullSyncPromise; + } + + return this.#atomicSyncQueue.enqueue(() => + this.#performSyncForWalletInner(entropySourceId), + ); + } + + /** + * Performs the work for {@link performSyncForWallet} once it has been + * dequeued: locates the matching wallet, toggles the in-progress flag, + * and runs the per-wallet sync body under the wallet trace. + * + * @param entropySourceId - The entropy source ID of the wallet to sync. + */ + async #performSyncForWalletInner(entropySourceId: string): Promise { + if (this.isInProgress) { + return; + } + + const wallet = getLocalEntropyWallets(this.#context).find( + (candidate) => candidate.metadata.entropy.id === entropySourceId, + ); + + if (!wallet) { + return; + } + + this.#context.controllerStateUpdateFn( + (state: AccountTreeControllerState) => { + state.isAccountTreeSyncingInProgress = true; + }, + ); + + try { + await this.#context.traceFn( + { + name: TraceName.AccountSyncWallet, + }, + () => this.#performWalletSyncInner(wallet), + ); + } finally { + this.#context.controllerStateUpdateFn( + (state: AccountTreeControllerState) => { + state.isAccountTreeSyncingInProgress = false; + }, + ); + } + } + /** * Performs a single wallet's bidirectional metadata sync with user storage. * @@ -503,8 +595,14 @@ export class BackupAndSyncService { * Performs a single group's bidirectional metadata sync with user storage. * * @param groupId - The group ID to sync. + * @param isFreshlyCreated - When true, skip the user-storage GET because the + * group was just created locally and cannot yet exist remotely. `syncGroupMetadata` + * is given `null` so it short-circuits to a single PUT. */ - async #performSingleGroupSyncInner(groupId: AccountGroupId): Promise { + async #performSingleGroupSyncInner( + groupId: AccountGroupId, + isFreshlyCreated: boolean, + ): Promise { try { const walletId = this.#context.groupIdToWalletId.get(groupId); if (!walletId) { @@ -527,12 +625,15 @@ export class BackupAndSyncService { entropySourceId, ); - // Get the specific group from user storage - const groupFromUserStorage = await getGroupFromUserStorage( - this.#context, - entropySourceId, - group.metadata.entropy.groupIndex, - ); + // Skip the pre-emptive GET when we know the group was just created locally + // and therefore cannot yet exist in user storage. + const groupFromUserStorage = isFreshlyCreated + ? null + : await getGroupFromUserStorage( + this.#context, + entropySourceId, + group.metadata.entropy.groupIndex, + ); await syncGroupMetadata( this.#context, diff --git a/packages/account-tree-controller/src/index.ts b/packages/account-tree-controller/src/index.ts index 96df965c0a..d11ba574c7 100644 --- a/packages/account-tree-controller/src/index.ts +++ b/packages/account-tree-controller/src/index.ts @@ -33,6 +33,7 @@ export type { AccountTreeControllerClearStateAction, AccountTreeControllerSyncWithUserStorageAction, AccountTreeControllerSyncWithUserStorageAtLeastOnceAction, + AccountTreeControllerSyncWalletWithUserStorageAction, } from './AccountTreeController-method-action-types'; export type { AccountContext } from './AccountTreeController'; From ba468bc4ec8b595d3de53acd265c9f6f5c8349a1 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Thu, 28 May 2026 23:28:00 -0400 Subject: [PATCH 2/5] feat: update logic and revert removal of pre-emptive GET --- eslint-suppressions.json | 2 +- packages/account-tree-controller/CHANGELOG.md | 4 - ...countTreeController-method-action-types.ts | 16 +- .../src/AccountTreeController.test.ts | 33 +--- .../src/AccountTreeController.ts | 28 ++- .../src/backup-and-sync/service/index.test.ts | 178 +++--------------- .../src/backup-and-sync/service/index.ts | 94 +++------ 7 files changed, 83 insertions(+), 272 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c68ea72ce2..f417b23b79 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -37,7 +37,7 @@ "count": 1 }, "n/no-sync": { - "count": 25 + "count": 22 } }, "packages/account-tree-controller/src/backup-and-sync/service/index.ts": { diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index d392f1ad25..2265b5b867 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -11,10 +11,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `AccountTreeController.syncWalletWithUserStorage(entropySourceId)` and the corresponding `AccountTreeController:syncWalletWithUserStorage` messenger action, which performs a bidirectional user-storage sync for a single entropy wallet (wallet metadata + groups) without iterating every local wallet. Use this in place of `syncWithUserStorage` after operations that only affect one wallet (e.g., SRP import). Does not mark the controller as having completed its first full sync. -### Fixed - -- Skip the pre-emptive `multichain_accounts_groups/` GET when an account group has just been created locally and cannot yet exist in user storage, eliminating an unnecessary 404 round-trip on every account creation. - ## [7.2.0] ### Changed diff --git a/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts b/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts index 98a549533e..2ced2b7b21 100644 --- a/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts +++ b/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts @@ -177,18 +177,18 @@ export type AccountTreeControllerSyncWithUserStorageAtLeastOnceAction = { }; /** - * Bi-directionally syncs a single entropy wallet with user storage, scoped - * by entropy source ID. Use this in place of `syncWithUserStorage` when - * only one wallet's state has changed (e.g., after an SRP import) to avoid - * the per-wallet fanout of fetches that a full sync triggers. + * Enqueues a bidirectional sync with user storage for a single entropy + * wallet (fire-and-forget), scoped by entropy source ID. Use this in + * place of `syncWithUserStorage` when only one wallet's state has changed + * (e.g., after an SRP import) to avoid the per-wallet fanout of fetches + * that a full sync triggers. * * IMPORTANT: - * If a full sync is already in progress, returns the ongoing full-sync - * promise. This call does NOT mark the controller as having completed - * its first full sync. + * No-ops if a full sync is in progress (the full sync will cover this + * wallet). Does NOT mark the controller as having completed its first + * full sync. * * @param entropySourceId - The entropy source ID of the wallet to sync. - * @returns A promise that resolves when the sync is complete. */ export type AccountTreeControllerSyncWalletWithUserStorageAction = { type: `AccountTreeController:syncWalletWithUserStorage`; diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 25c43e93dd..64e509ff53 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/packages/account-tree-controller/src/AccountTreeController.test.ts @@ -4835,10 +4835,10 @@ describe('AccountTreeController', () => { jest.clearAllMocks(); }); - it('calls performSyncForWallet on the syncing service', async () => { - const performSyncForWalletSpy = jest - .spyOn(BackupAndSyncService.prototype, 'performSyncForWallet') - .mockResolvedValue(undefined); + it('calls enqueueSyncForWallet on the syncing service', () => { + const enqueueSyncForWalletSpy = jest + .spyOn(BackupAndSyncService.prototype, 'enqueueSyncForWallet') + .mockImplementation(() => undefined); const { controller } = setup({ accounts: [MOCK_HARDWARE_ACCOUNT_1], @@ -4847,29 +4847,10 @@ describe('AccountTreeController', () => { controller.init(); - await controller.syncWalletWithUserStorage('test-entropy-id'); + controller.syncWalletWithUserStorage('test-entropy-id'); - expect(performSyncForWalletSpy).toHaveBeenCalledTimes(1); - expect(performSyncForWalletSpy).toHaveBeenCalledWith('test-entropy-id'); - }); - - it('handles sync errors gracefully', async () => { - const syncError = new Error('Sync failed'); - const performSyncForWalletSpy = jest - .spyOn(BackupAndSyncService.prototype, 'performSyncForWallet') - .mockRejectedValue(syncError); - - const { controller } = setup({ - accounts: [MOCK_HARDWARE_ACCOUNT_1], - keyrings: [MOCK_HD_KEYRING_1], - }); - - controller.init(); - - await expect( - controller.syncWalletWithUserStorage('test-entropy-id'), - ).rejects.toThrow(syncError.message); - expect(performSyncForWalletSpy).toHaveBeenCalledTimes(1); + expect(enqueueSyncForWalletSpy).toHaveBeenCalledTimes(1); + expect(enqueueSyncForWalletSpy).toHaveBeenCalledWith('test-entropy-id'); }); }); diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index c7a4f996c3..a81f645622 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -1105,13 +1105,9 @@ 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). - // The group was just created locally and cannot yet exist in user storage, - // so skip the pre-emptive GET and push directly. + // Trigger atomic sync for new group (only for entropy wallets) if (wallet.type === AccountWalletType.Entropy) { - this.#backupAndSyncService.enqueueSingleGroupSync(groupId, { - isFreshlyCreated: true, - }); + this.#backupAndSyncService.enqueueSingleGroupSync(groupId); } } else { group.accounts.push(id); @@ -1701,21 +1697,21 @@ export class AccountTreeController extends BaseController< } /** - * Bi-directionally syncs a single entropy wallet with user storage, scoped - * by entropy source ID. Use this in place of `syncWithUserStorage` when - * only one wallet's state has changed (e.g., after an SRP import) to avoid - * the per-wallet fanout of fetches that a full sync triggers. + * Enqueues a bidirectional sync with user storage for a single entropy + * wallet (fire-and-forget), scoped by entropy source ID. Use this in + * place of `syncWithUserStorage` when only one wallet's state has changed + * (e.g., after an SRP import) to avoid the per-wallet fanout of fetches + * that a full sync triggers. * * IMPORTANT: - * If a full sync is already in progress, returns the ongoing full-sync - * promise. This call does NOT mark the controller as having completed - * its first full sync. + * No-ops if a full sync is in progress (the full sync will cover this + * wallet). Does NOT mark the controller as having completed its first + * full sync. * * @param entropySourceId - The entropy source ID of the wallet to sync. - * @returns A promise that resolves when the sync is complete. */ - async syncWalletWithUserStorage(entropySourceId: string): Promise { - return this.#backupAndSyncService.performSyncForWallet(entropySourceId); + syncWalletWithUserStorage(entropySourceId: string): void { + this.#backupAndSyncService.enqueueSyncForWallet(entropySourceId); } /** diff --git a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts index e676797b33..28b0973f97 100644 --- a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts @@ -4,11 +4,9 @@ import { BackupAndSyncService } from '.'; import type { AccountGroupObject } from '../../group'; import type { AccountWalletEntropyObject } from '../../wallet'; import { getProfileId } from '../authentication'; -import { syncGroupMetadata } from '../syncing'; import type { BackupAndSyncContext } from '../types'; import { getAllGroupsFromUserStorage, - getGroupFromUserStorage, getWalletFromUserStorage, } from '../user-storage'; // We only need to import the functions we actually spy on @@ -26,10 +24,6 @@ const mockGetProfileId = getProfileId as jest.MockedFunction< >; const mockGetLocalEntropyWallets = getLocalEntropyWallets as jest.MockedFunction; -const mockGetGroupFromUserStorage = - getGroupFromUserStorage as jest.MockedFunction< - typeof getGroupFromUserStorage - >; const mockGetWalletFromUserStorage = getWalletFromUserStorage as jest.MockedFunction< typeof getWalletFromUserStorage @@ -38,9 +32,6 @@ const mockGetAllGroupsFromUserStorage = getAllGroupsFromUserStorage as jest.MockedFunction< typeof getAllGroupsFromUserStorage >; -const mockSyncGroupMetadata = syncGroupMetadata as jest.MockedFunction< - typeof syncGroupMetadata ->; describe('BackupAndSync - Service - BackupAndSyncService', () => { let mockContext: BackupAndSyncContext; @@ -292,93 +283,6 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { // Should have called getProfileId as part of group sync expect(mockGetProfileId).toHaveBeenCalled(); }); - - it('fetches the group from user storage by default before syncing', async () => { - mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = true; - - mockContext.groupIdToWalletId.set( - 'entropy:wallet-1/1', - 'entropy:wallet-1', - ); - mockContext.controller.state.accountTree.wallets = { - 'entropy:wallet-1': { - id: 'entropy:wallet-1', - type: AccountWalletType.Entropy, - metadata: { - entropy: { id: 'test-entropy-id' }, - name: 'Test Wallet', - }, - groups: { - 'entropy:wallet-1/1': { - id: 'entropy:wallet-1/1', - name: 'Test Group', - metadata: { - entropy: { groupIndex: 1 }, - }, - } as unknown as AccountGroupObject, - }, - } as unknown as AccountWalletEntropyObject, - }; - - backupAndSyncService.enqueueSingleGroupSync('entropy:wallet-1/1'); - - await new Promise((resolve) => setTimeout(resolve, 10)); - - expect(mockGetGroupFromUserStorage).toHaveBeenCalledWith( - expect.anything(), - 'test-entropy-id', - 1, - ); - expect(mockSyncGroupMetadata).toHaveBeenCalled(); - }); - - it('skips the user storage fetch when called with isFreshlyCreated and pushes directly', async () => { - mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = true; - - mockContext.groupIdToWalletId.set( - 'entropy:wallet-1/1', - 'entropy:wallet-1', - ); - const localGroup = { - id: 'entropy:wallet-1/1', - name: 'Test Group', - metadata: { - entropy: { groupIndex: 1 }, - }, - } as unknown as AccountGroupObject; - mockContext.controller.state.accountTree.wallets = { - 'entropy:wallet-1': { - id: 'entropy:wallet-1', - type: AccountWalletType.Entropy, - metadata: { - entropy: { id: 'test-entropy-id' }, - name: 'Test Wallet', - }, - groups: { - 'entropy:wallet-1/1': localGroup, - }, - } as unknown as AccountWalletEntropyObject, - }; - - backupAndSyncService.enqueueSingleGroupSync('entropy:wallet-1/1', { - isFreshlyCreated: true, - }); - - await new Promise((resolve) => setTimeout(resolve, 10)); - - // The pre-emptive GET must be skipped for freshly created groups. - expect(mockGetGroupFromUserStorage).not.toHaveBeenCalled(); - - // syncGroupMetadata must still be invoked, and given null for the - // user-storage value so it short-circuits to a single PUT. - expect(mockSyncGroupMetadata).toHaveBeenCalledWith( - expect.anything(), - localGroup, - null, - 'test-entropy-id', - 'test-profile-id', - ); - }); }); describe('performFullSync', () => { @@ -819,7 +723,7 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { }, 15000); // Increase timeout to 15 seconds }); - describe('performSyncForWallet', () => { + describe('enqueueSyncForWallet', () => { beforeEach(() => { setupMockUserStorageControllerState(true, true); jest.clearAllMocks(); @@ -829,12 +733,24 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { it('returns early when backup and sync is disabled', async () => { setupMockUserStorageControllerState(false, true); - const result = - await backupAndSyncService.performSyncForWallet('test-entropy-id'); + backupAndSyncService.enqueueSyncForWallet('test-entropy-id'); + + // Give the queue a chance to run, then assert nothing happened. + await new Promise((resolve) => setTimeout(resolve, 10)); - expect(result).toBeUndefined(); - expect(mockContext.controllerStateUpdateFn).not.toHaveBeenCalled(); expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); + expect(mockGetLocalEntropyWallets).not.toHaveBeenCalled(); + }); + + it('returns early when a full sync is in progress', async () => { + mockContext.controller.state.isAccountTreeSyncingInProgress = true; + + backupAndSyncService.enqueueSyncForWallet('test-entropy-id'); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); + expect(mockGetLocalEntropyWallets).not.toHaveBeenCalled(); }); it('syncs only the matched wallet, not every local wallet', async () => { @@ -851,7 +767,9 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { mockGetWalletFromUserStorage.mockResolvedValue(null); mockGetAllGroupsFromUserStorage.mockResolvedValue([]); - await backupAndSyncService.performSyncForWallet('entropy-id-2'); + backupAndSyncService.enqueueSyncForWallet('entropy-id-2'); + + await new Promise((resolve) => setTimeout(resolve, 10)); // The fetches should target only the requested wallet's entropy source. expect(mockGetWalletFromUserStorage).toHaveBeenCalledTimes(1); @@ -871,7 +789,7 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { ); }); - it('does not flip hasAccountTreeSyncingSyncedAtLeastOnce', async () => { + it('does not flip hasAccountTreeSyncingSyncedAtLeastOnce or isAccountTreeSyncingInProgress', async () => { mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = false; mockGetLocalEntropyWallets.mockReturnValue([ { @@ -882,54 +800,12 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { mockGetWalletFromUserStorage.mockResolvedValue(null); mockGetAllGroupsFromUserStorage.mockResolvedValue([]); - // Capture state mutations applied via controllerStateUpdateFn so we can - // assert the flag was never touched. - const draft: Record = { - hasAccountTreeSyncingSyncedAtLeastOnce: false, - isAccountTreeSyncingInProgress: false, - }; - (mockContext.controllerStateUpdateFn as jest.Mock).mockImplementation( - (updater: (state: typeof draft) => void) => { - updater(draft); - }, - ); + backupAndSyncService.enqueueSyncForWallet('entropy-id-1'); - await backupAndSyncService.performSyncForWallet('entropy-id-1'); - - expect(draft.hasAccountTreeSyncingSyncedAtLeastOnce).toBe(false); - // The in-progress flag is toggled on then off during the sync. - expect(draft.isAccountTreeSyncingInProgress).toBe(false); - }); - - it('returns the in-flight full sync promise when one is running', async () => { - mockGetLocalEntropyWallets.mockReturnValue([ - { - id: 'entropy:wallet-1', - metadata: { entropy: { id: 'entropy-id-1' } }, - } as unknown as AccountWalletEntropyObject, - ]); - - // Make the full sync hang so we can observe the wallet-scoped call - // dedup against it. - let resolveTrace: (() => void) | undefined; - const tracePromise = new Promise((resolve) => { - resolveTrace = resolve; - }); - (mockContext.traceFn as jest.Mock).mockImplementation( - (_: unknown, fn: () => unknown) => { - fn(); - return tracePromise; - }, - ); - - const fullSyncPromise = backupAndSyncService.performFullSync(); - const walletSyncPromise = - backupAndSyncService.performSyncForWallet('entropy-id-1'); - - expect(walletSyncPromise).toStrictEqual(fullSyncPromise); + await new Promise((resolve) => setTimeout(resolve, 10)); - resolveTrace?.(); - await Promise.all([fullSyncPromise, walletSyncPromise]); + // Neither sync-state flag should be written by the scoped sync path. + expect(mockContext.controllerStateUpdateFn).not.toHaveBeenCalled(); }); it('is a no-op when no local wallet matches the entropy source ID', async () => { @@ -940,7 +816,9 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { } as unknown as AccountWalletEntropyObject, ]); - await backupAndSyncService.performSyncForWallet('unknown-entropy-id'); + backupAndSyncService.enqueueSyncForWallet('unknown-entropy-id'); + + await new Promise((resolve) => setTimeout(resolve, 10)); expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); expect(mockGetAllGroupsFromUserStorage).not.toHaveBeenCalled(); 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 8dc6d407be..844ae1502f 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 @@ -183,15 +183,8 @@ export class BackupAndSyncService { * If the first full sync has not yet occurred, it does nothing. * * @param groupId - The group ID to sync. - * @param options - Optional sync configuration. - * @param options.isFreshlyCreated - When true, skip the user-storage GET because - * the group was just created locally and cannot yet exist remotely. The push is - * still performed via `syncGroupMetadata`. */ - enqueueSingleGroupSync( - groupId: AccountGroupId, - options?: { isFreshlyCreated?: boolean }, - ): void { + enqueueSingleGroupSync(groupId: AccountGroupId): void { if ( !this.isBackupAndSyncEnabled || !this.hasSyncedAtLeastOnce || @@ -205,11 +198,9 @@ export class BackupAndSyncService { return; } - const isFreshlyCreated = options?.isFreshlyCreated ?? false; - // eslint-disable-next-line no-void void this.#atomicSyncQueue.enqueue(() => - this.#performSingleGroupSyncInner(groupId, isFreshlyCreated), + this.#performSingleGroupSyncInner(groupId), ); } @@ -478,50 +469,42 @@ export class BackupAndSyncService { } /** - * Performs a bidirectional sync with user storage for a single entropy wallet. + * Enqueues a bidirectional sync with user storage for a single entropy + * wallet (fire-and-forget), scoped by entropy source ID. * - * Use this in place of {@link performFullSync} when only one wallet's state - * has changed (e.g., immediately after an SRP import) to avoid the + * Use this in place of {@link performFullSync} when only one wallet's + * state has changed (e.g., immediately after an SRP import) to avoid the * per-wallet fanout of fetches that a full sync triggers. * * Behavior: * - Returns early if backup and sync is disabled. - * - If a full sync is already in flight, returns the in-flight promise so - * callers don't race against it. + * - Returns early if a full sync is in progress; the full sync will cover + * this wallet. * - Does NOT flip `hasAccountTreeSyncingSyncedAtLeastOnce`. A scoped sync * for one wallet does not satisfy the canonical "first full sync" * contract, since other wallets may still need legacy migration. * * @param entropySourceId - The entropy source ID of the wallet to sync. - * @returns A promise that resolves when the sync is complete. */ - async performSyncForWallet(entropySourceId: string): Promise { - if (!this.isBackupAndSyncEnabled) { - return undefined; - } - - // Defer to the in-flight full sync so we don't race against it. - if (this.#ongoingFullSyncPromise) { - return this.#ongoingFullSyncPromise; + enqueueSyncForWallet(entropySourceId: string): void { + if (!this.isBackupAndSyncEnabled || this.isInProgress) { + return; } - return this.#atomicSyncQueue.enqueue(() => + // eslint-disable-next-line no-void + void this.#atomicSyncQueue.enqueue(() => this.#performSyncForWalletInner(entropySourceId), ); } /** - * Performs the work for {@link performSyncForWallet} once it has been - * dequeued: locates the matching wallet, toggles the in-progress flag, - * and runs the per-wallet sync body under the wallet trace. + * Performs the work for {@link enqueueSyncForWallet} once it has been + * dequeued: locates the matching wallet and runs the per-wallet sync body + * under the wallet trace. * * @param entropySourceId - The entropy source ID of the wallet to sync. */ async #performSyncForWalletInner(entropySourceId: string): Promise { - if (this.isInProgress) { - return; - } - const wallet = getLocalEntropyWallets(this.#context).find( (candidate) => candidate.metadata.entropy.id === entropySourceId, ); @@ -530,26 +513,12 @@ export class BackupAndSyncService { return; } - this.#context.controllerStateUpdateFn( - (state: AccountTreeControllerState) => { - state.isAccountTreeSyncingInProgress = true; + await this.#context.traceFn( + { + name: TraceName.AccountSyncWallet, }, + () => this.#performWalletSyncInner(wallet), ); - - try { - await this.#context.traceFn( - { - name: TraceName.AccountSyncWallet, - }, - () => this.#performWalletSyncInner(wallet), - ); - } finally { - this.#context.controllerStateUpdateFn( - (state: AccountTreeControllerState) => { - state.isAccountTreeSyncingInProgress = false; - }, - ); - } } /** @@ -595,14 +564,8 @@ export class BackupAndSyncService { * Performs a single group's bidirectional metadata sync with user storage. * * @param groupId - The group ID to sync. - * @param isFreshlyCreated - When true, skip the user-storage GET because the - * group was just created locally and cannot yet exist remotely. `syncGroupMetadata` - * is given `null` so it short-circuits to a single PUT. */ - async #performSingleGroupSyncInner( - groupId: AccountGroupId, - isFreshlyCreated: boolean, - ): Promise { + async #performSingleGroupSyncInner(groupId: AccountGroupId): Promise { try { const walletId = this.#context.groupIdToWalletId.get(groupId); if (!walletId) { @@ -625,15 +588,12 @@ export class BackupAndSyncService { entropySourceId, ); - // Skip the pre-emptive GET when we know the group was just created locally - // and therefore cannot yet exist in user storage. - const groupFromUserStorage = isFreshlyCreated - ? null - : await getGroupFromUserStorage( - this.#context, - entropySourceId, - group.metadata.entropy.groupIndex, - ); + // Get the specific group from user storage + const groupFromUserStorage = await getGroupFromUserStorage( + this.#context, + entropySourceId, + group.metadata.entropy.groupIndex, + ); await syncGroupMetadata( this.#context, From f8faa2fa9d8a5f538f2092387b2add248011d8dc Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 May 2026 00:04:40 -0400 Subject: [PATCH 3/5] chore: add pr number --- packages/account-tree-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index aef0006ec8..143da637ab 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `AccountTreeController.syncWalletWithUserStorage(entropySourceId)` and the corresponding `AccountTreeController:syncWalletWithUserStorage` messenger action, which performs a bidirectional user-storage sync for a single entropy wallet (wallet metadata + groups) without iterating every local wallet. Use this in place of `syncWithUserStorage` after operations that only affect one wallet (e.g., SRP import) ([#xxxx](https://github.com/MetaMask/core/pull/xxxx)) +- `AccountTreeController.syncWalletWithUserStorage(entropySourceId)` and the corresponding `AccountTreeController:syncWalletWithUserStorage` messenger action, which performs a bidirectional user-storage sync for a single entropy wallet (wallet metadata + groups) without iterating every local wallet. Use this in place of `syncWithUserStorage` after operations that only affect one wallet (e.g., SRP import) ([#8929](https://github.com/MetaMask/core/pull/8929)) ## [7.5.0] From bc6efeb561d66e2f2b528431a1640334d51058c7 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Fri, 29 May 2026 13:06:45 -0400 Subject: [PATCH 4/5] fix: update #performSyncForWalletInner --- .../src/backup-and-sync/service/index.test.ts | 67 ++++++++++++++++++- .../src/backup-and-sync/service/index.ts | 28 ++++++-- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts index 28b0973f97..28689216f9 100644 --- a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts @@ -789,7 +789,7 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { ); }); - it('does not flip hasAccountTreeSyncingSyncedAtLeastOnce or isAccountTreeSyncingInProgress', async () => { + it('does not flip hasAccountTreeSyncingSyncedAtLeastOnce', async () => { mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce = false; mockGetLocalEntropyWallets.mockReturnValue([ { @@ -800,11 +800,74 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { mockGetWalletFromUserStorage.mockResolvedValue(null); mockGetAllGroupsFromUserStorage.mockResolvedValue([]); + const draft: Record = { + hasAccountTreeSyncingSyncedAtLeastOnce: false, + isAccountTreeSyncingInProgress: false, + }; + (mockContext.controllerStateUpdateFn as jest.Mock).mockImplementation( + (updater: (state: typeof draft) => void) => { + updater(draft); + }, + ); + backupAndSyncService.enqueueSyncForWallet('entropy-id-1'); await new Promise((resolve) => setTimeout(resolve, 10)); - // Neither sync-state flag should be written by the scoped sync path. + // Scoped sync must NOT satisfy the first-full-sync contract. + expect(draft.hasAccountTreeSyncingSyncedAtLeastOnce).toBe(false); + }); + + it('toggles isAccountTreeSyncingInProgress around the sync body', async () => { + let flagWhileRunning: boolean | undefined; + const draft: Record = { + isAccountTreeSyncingInProgress: false, + }; + (mockContext.controllerStateUpdateFn as jest.Mock).mockImplementation( + (updater: (state: typeof draft) => void) => { + updater(draft); + }, + ); + + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + ]); + // Capture the flag value at the moment the sync body runs — proves it + // was set true before the body started. + (mockContext.traceFn as jest.Mock).mockImplementation( + async (_: unknown, fn: () => Promise) => { + flagWhileRunning = draft.isAccountTreeSyncingInProgress as boolean; + await fn(); + }, + ); + mockGetWalletFromUserStorage.mockResolvedValue(null); + mockGetAllGroupsFromUserStorage.mockResolvedValue([]); + + backupAndSyncService.enqueueSyncForWallet('entropy-id-1'); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(flagWhileRunning).toBe(true); + // And reset after the sync completes. + expect(draft.isAccountTreeSyncingInProgress).toBe(false); + }); + + it('does not touch isAccountTreeSyncingInProgress when the wallet is not found', async () => { + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + ]); + + backupAndSyncService.enqueueSyncForWallet('unknown-entropy-id'); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + // No wallet found → no flag toggling, no work. expect(mockContext.controllerStateUpdateFn).not.toHaveBeenCalled(); }); 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 844ae1502f..a7e14b2153 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 @@ -502,6 +502,12 @@ export class BackupAndSyncService { * dequeued: locates the matching wallet and runs the per-wallet sync body * under the wallet trace. * + * Sets `isAccountTreeSyncingInProgress` for the duration of the sync so + * that `enqueueSingleGroupSync` calls triggered by `#insert` (when + * `createLocalGroupsFromUserStorage` materializes remote groups locally) + * are gated off and don't fanout per-group GET+PUT roundtrips for data we + * just pulled. + * * @param entropySourceId - The entropy source ID of the wallet to sync. */ async #performSyncForWalletInner(entropySourceId: string): Promise { @@ -513,12 +519,26 @@ export class BackupAndSyncService { return; } - await this.#context.traceFn( - { - name: TraceName.AccountSyncWallet, + this.#context.controllerStateUpdateFn( + (state: AccountTreeControllerState) => { + state.isAccountTreeSyncingInProgress = true; }, - () => this.#performWalletSyncInner(wallet), ); + + try { + await this.#context.traceFn( + { + name: TraceName.AccountSyncWallet, + }, + () => this.#performWalletSyncInner(wallet), + ); + } finally { + this.#context.controllerStateUpdateFn( + (state: AccountTreeControllerState) => { + state.isAccountTreeSyncingInProgress = false; + }, + ); + } } /** From 4cca49f5459c249d8a74e109953e465ee0e0062c Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Sat, 30 May 2026 13:26:20 -0400 Subject: [PATCH 5/5] refactor: make syncWalletWithUserStorage async --- eslint-suppressions.json | 2 +- ...countTreeController-method-action-types.ts | 16 ++-- .../src/AccountTreeController.test.ts | 33 ++++++-- .../src/AccountTreeController.ts | 20 ++--- .../src/backup-and-sync/service/index.test.ts | 77 +++++++++++-------- .../src/backup-and-sync/service/index.ts | 32 +++++--- 6 files changed, 109 insertions(+), 71 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index eedb60a53b..1d8a56bb3e 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -37,7 +37,7 @@ "count": 1 }, "n/no-sync": { - "count": 22 + "count": 23 } }, "packages/account-tree-controller/src/backup-and-sync/service/index.ts": { diff --git a/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts b/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts index e7c0c3953a..37442f7ee1 100644 --- a/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts +++ b/packages/account-tree-controller/src/AccountTreeController-method-action-types.ts @@ -200,18 +200,18 @@ export type AccountTreeControllerSyncWithUserStorageAtLeastOnceAction = { }; /** - * Enqueues a bidirectional sync with user storage for a single entropy - * wallet (fire-and-forget), scoped by entropy source ID. Use this in - * place of `syncWithUserStorage` when only one wallet's state has changed - * (e.g., after an SRP import) to avoid the per-wallet fanout of fetches - * that a full sync triggers. + * Bi-directionally syncs a single entropy wallet with user storage, scoped + * by entropy source ID. Use this in place of `syncWithUserStorage` when + * only one wallet's state has changed (e.g., after an SRP import) to + * avoid the per-wallet fanout of fetches that a full sync triggers. * * IMPORTANT: - * No-ops if a full sync is in progress (the full sync will cover this - * wallet). Does NOT mark the controller as having completed its first - * full sync. + * If a full sync is already in progress, returns the ongoing full-sync + * promise so callers cooperatively wait for it instead of racing. Does + * NOT mark the controller as having completed its first full sync. * * @param entropySourceId - The entropy source ID of the wallet to sync. + * @returns A promise that resolves when the sync is complete. */ export type AccountTreeControllerSyncWalletWithUserStorageAction = { type: `AccountTreeController:syncWalletWithUserStorage`; diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 734758c955..f8ba02087d 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/packages/account-tree-controller/src/AccountTreeController.test.ts @@ -5143,10 +5143,10 @@ describe('AccountTreeController', () => { jest.clearAllMocks(); }); - it('calls enqueueSyncForWallet on the syncing service', () => { - const enqueueSyncForWalletSpy = jest - .spyOn(BackupAndSyncService.prototype, 'enqueueSyncForWallet') - .mockImplementation(() => undefined); + it('calls performSyncForWallet on the syncing service', async () => { + const performSyncForWalletSpy = jest + .spyOn(BackupAndSyncService.prototype, 'performSyncForWallet') + .mockResolvedValue(undefined); const { controller } = setup({ accounts: [MOCK_HARDWARE_ACCOUNT_1], @@ -5155,10 +5155,29 @@ describe('AccountTreeController', () => { controller.init(); - controller.syncWalletWithUserStorage('test-entropy-id'); + await controller.syncWalletWithUserStorage('test-entropy-id'); - expect(enqueueSyncForWalletSpy).toHaveBeenCalledTimes(1); - expect(enqueueSyncForWalletSpy).toHaveBeenCalledWith('test-entropy-id'); + expect(performSyncForWalletSpy).toHaveBeenCalledTimes(1); + expect(performSyncForWalletSpy).toHaveBeenCalledWith('test-entropy-id'); + }); + + it('propagates errors from the syncing service', async () => { + const syncError = new Error('Sync failed'); + const performSyncForWalletSpy = jest + .spyOn(BackupAndSyncService.prototype, 'performSyncForWallet') + .mockRejectedValue(syncError); + + const { controller } = setup({ + accounts: [MOCK_HARDWARE_ACCOUNT_1], + keyrings: [MOCK_HD_KEYRING_1], + }); + + controller.init(); + + await expect( + controller.syncWalletWithUserStorage('test-entropy-id'), + ).rejects.toThrow(syncError.message); + expect(performSyncForWalletSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index cc4256cee1..28cceee217 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -1791,21 +1791,21 @@ export class AccountTreeController extends BaseController< } /** - * Enqueues a bidirectional sync with user storage for a single entropy - * wallet (fire-and-forget), scoped by entropy source ID. Use this in - * place of `syncWithUserStorage` when only one wallet's state has changed - * (e.g., after an SRP import) to avoid the per-wallet fanout of fetches - * that a full sync triggers. + * Bi-directionally syncs a single entropy wallet with user storage, scoped + * by entropy source ID. Use this in place of `syncWithUserStorage` when + * only one wallet's state has changed (e.g., after an SRP import) to + * avoid the per-wallet fanout of fetches that a full sync triggers. * * IMPORTANT: - * No-ops if a full sync is in progress (the full sync will cover this - * wallet). Does NOT mark the controller as having completed its first - * full sync. + * If a full sync is already in progress, returns the ongoing full-sync + * promise so callers cooperatively wait for it instead of racing. Does + * NOT mark the controller as having completed its first full sync. * * @param entropySourceId - The entropy source ID of the wallet to sync. + * @returns A promise that resolves when the sync is complete. */ - syncWalletWithUserStorage(entropySourceId: string): void { - this.#backupAndSyncService.enqueueSyncForWallet(entropySourceId); + async syncWalletWithUserStorage(entropySourceId: string): Promise { + return this.#backupAndSyncService.performSyncForWallet(entropySourceId); } /** diff --git a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts index 28689216f9..c674cf2659 100644 --- a/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/service/index.test.ts @@ -723,31 +723,17 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { }, 15000); // Increase timeout to 15 seconds }); - describe('enqueueSyncForWallet', () => { + describe('performSyncForWallet', () => { beforeEach(() => { setupMockUserStorageControllerState(true, true); jest.clearAllMocks(); mockGetLocalEntropyWallets.mockClear(); }); - it('returns early when backup and sync is disabled', async () => { + it('returns early (resolved) when backup and sync is disabled', async () => { setupMockUserStorageControllerState(false, true); - backupAndSyncService.enqueueSyncForWallet('test-entropy-id'); - - // Give the queue a chance to run, then assert nothing happened. - await new Promise((resolve) => setTimeout(resolve, 10)); - - expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); - expect(mockGetLocalEntropyWallets).not.toHaveBeenCalled(); - }); - - it('returns early when a full sync is in progress', async () => { - mockContext.controller.state.isAccountTreeSyncingInProgress = true; - - backupAndSyncService.enqueueSyncForWallet('test-entropy-id'); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await backupAndSyncService.performSyncForWallet('test-entropy-id'); expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); expect(mockGetLocalEntropyWallets).not.toHaveBeenCalled(); @@ -767,9 +753,7 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { mockGetWalletFromUserStorage.mockResolvedValue(null); mockGetAllGroupsFromUserStorage.mockResolvedValue([]); - backupAndSyncService.enqueueSyncForWallet('entropy-id-2'); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await backupAndSyncService.performSyncForWallet('entropy-id-2'); // The fetches should target only the requested wallet's entropy source. expect(mockGetWalletFromUserStorage).toHaveBeenCalledTimes(1); @@ -810,9 +794,7 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { }, ); - backupAndSyncService.enqueueSyncForWallet('entropy-id-1'); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await backupAndSyncService.performSyncForWallet('entropy-id-1'); // Scoped sync must NOT satisfy the first-full-sync contract. expect(draft.hasAccountTreeSyncingSyncedAtLeastOnce).toBe(false); @@ -846,16 +828,14 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { mockGetWalletFromUserStorage.mockResolvedValue(null); mockGetAllGroupsFromUserStorage.mockResolvedValue([]); - backupAndSyncService.enqueueSyncForWallet('entropy-id-1'); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await backupAndSyncService.performSyncForWallet('entropy-id-1'); expect(flagWhileRunning).toBe(true); // And reset after the sync completes. expect(draft.isAccountTreeSyncingInProgress).toBe(false); }); - it('does not touch isAccountTreeSyncingInProgress when the wallet is not found', async () => { + it('returns the in-flight full sync promise when one is running', async () => { mockGetLocalEntropyWallets.mockReturnValue([ { id: 'entropy:wallet-1', @@ -863,12 +843,27 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { } as unknown as AccountWalletEntropyObject, ]); - backupAndSyncService.enqueueSyncForWallet('unknown-entropy-id'); + // Make the full sync hang so we can observe the wallet-scoped call + // dedup against it. + let resolveTrace: (() => void) | undefined; + const tracePromise = new Promise((resolve) => { + resolveTrace = resolve; + }); + (mockContext.traceFn as jest.Mock).mockImplementation( + (_: unknown, fn: () => unknown) => { + fn(); + return tracePromise; + }, + ); + + const fullSyncPromise = backupAndSyncService.performFullSync(); + const walletSyncPromise = + backupAndSyncService.performSyncForWallet('entropy-id-1'); - await new Promise((resolve) => setTimeout(resolve, 10)); + expect(walletSyncPromise).toStrictEqual(fullSyncPromise); - // No wallet found → no flag toggling, no work. - expect(mockContext.controllerStateUpdateFn).not.toHaveBeenCalled(); + resolveTrace?.(); + await Promise.all([fullSyncPromise, walletSyncPromise]); }); it('is a no-op when no local wallet matches the entropy source ID', async () => { @@ -879,13 +874,27 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => { } as unknown as AccountWalletEntropyObject, ]); - backupAndSyncService.enqueueSyncForWallet('unknown-entropy-id'); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await backupAndSyncService.performSyncForWallet('unknown-entropy-id'); expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); expect(mockGetAllGroupsFromUserStorage).not.toHaveBeenCalled(); expect(mockGetProfileId).not.toHaveBeenCalled(); + // No flag toggle when there's no wallet to sync. + expect(mockContext.controllerStateUpdateFn).not.toHaveBeenCalled(); + }); + + it('returns early when the in-progress flag is already set', async () => { + mockContext.controller.state.isAccountTreeSyncingInProgress = true; + mockGetLocalEntropyWallets.mockReturnValue([ + { + id: 'entropy:wallet-1', + metadata: { entropy: { id: 'entropy-id-1' } }, + } as unknown as AccountWalletEntropyObject, + ]); + + await backupAndSyncService.performSyncForWallet('entropy-id-1'); + + expect(mockGetWalletFromUserStorage).not.toHaveBeenCalled(); }); }); }); 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 a7e14b2153..669412996e 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 @@ -469,36 +469,42 @@ export class BackupAndSyncService { } /** - * Enqueues a bidirectional sync with user storage for a single entropy - * wallet (fire-and-forget), scoped by entropy source ID. + * Performs a bidirectional sync with user storage for a single entropy + * wallet, scoped by entropy source ID. * * Use this in place of {@link performFullSync} when only one wallet's * state has changed (e.g., immediately after an SRP import) to avoid the * per-wallet fanout of fetches that a full sync triggers. * * Behavior: - * - Returns early if backup and sync is disabled. - * - Returns early if a full sync is in progress; the full sync will cover - * this wallet. + * - Returns early (resolved) if backup and sync is disabled. + * - If a full sync is already in flight, returns the in-flight promise so + * callers cooperatively wait for the broader operation instead of + * racing against it. * - Does NOT flip `hasAccountTreeSyncingSyncedAtLeastOnce`. A scoped sync * for one wallet does not satisfy the canonical "first full sync" * contract, since other wallets may still need legacy migration. * * @param entropySourceId - The entropy source ID of the wallet to sync. + * @returns A promise that resolves when the sync is complete. */ - enqueueSyncForWallet(entropySourceId: string): void { - if (!this.isBackupAndSyncEnabled || this.isInProgress) { - return; + async performSyncForWallet(entropySourceId: string): Promise { + if (!this.isBackupAndSyncEnabled) { + return undefined; } - // eslint-disable-next-line no-void - void this.#atomicSyncQueue.enqueue(() => + // Defer to the in-flight full sync so we don't race against it. + if (this.#ongoingFullSyncPromise) { + return this.#ongoingFullSyncPromise; + } + + return this.#atomicSyncQueue.enqueue(() => this.#performSyncForWalletInner(entropySourceId), ); } /** - * Performs the work for {@link enqueueSyncForWallet} once it has been + * Performs the work for {@link performSyncForWallet} once it has been * dequeued: locates the matching wallet and runs the per-wallet sync body * under the wallet trace. * @@ -511,6 +517,10 @@ export class BackupAndSyncService { * @param entropySourceId - The entropy source ID of the wallet to sync. */ async #performSyncForWalletInner(entropySourceId: string): Promise { + if (this.isInProgress) { + return; + } + const wallet = getLocalEntropyWallets(this.#context).find( (candidate) => candidate.metadata.entropy.id === entropySourceId, );