From cbf2058deae19ea2bb7b3145f7b42dbc18fb7905 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Wed, 25 Mar 2026 16:00:57 +0100 Subject: [PATCH 1/4] fix(account-tree-controller/backup-and-sync): do not send extra metadata properties --- .../user-storage/format-utils.test.ts | 36 +++++++++++++++++++ .../user-storage/format-utils.ts | 26 +++++++++++--- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.test.ts b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.test.ts index 2e9540c31c3..888f14b6674 100644 --- a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.test.ts @@ -11,10 +11,16 @@ import { assertValidLegacyUserStorageAccount, } from './validation'; import type { AccountGroupMultichainAccountObject } from '../../group'; +import { backupAndSyncLogger } from '../../logger'; import type { AccountWalletEntropyObject } from '../../wallet'; import type { BackupAndSyncContext, UserStorageSyncedWallet } from '../types'; jest.mock('./validation'); +jest.mock('../../logger'); + +const mockBackupAndSyncLogger = backupAndSyncLogger as jest.MockedFunction< + typeof backupAndSyncLogger +>; const mockAssertValidUserStorageWallet = assertValidUserStorageWallet as jest.MockedFunction< @@ -109,6 +115,36 @@ describe('BackupAndSync - UserStorage - FormatUtils', () => { groupIndex: 0, }); }); + + it('falls back to blank metadata and logs if mask throws due to invalid field types', () => { + mockContext.controller.state.accountGroupsMetadata[mockGroup.id] = { + // `name` should be an UpdatableField object, not a plain string + name: 'invalid-not-an-object' as never, + }; + + const result = formatGroupForUserStorageUsage(mockContext, mockGroup); + + expect(result).toStrictEqual({ groupIndex: 0 }); + expect(mockBackupAndSyncLogger).toHaveBeenCalledWith( + expect.stringContaining( + 'Error trying to format group for user storage usage', + ), + ); + }); + + it('strips fields not in the schema (e.g. lastSelected)', () => { + mockContext.controller.state.accountGroupsMetadata[mockGroup.id] = { + name: { value: 'Group Name', lastUpdatedAt: 123456 }, + lastSelected: 999999, + }; + + const result = formatGroupForUserStorageUsage(mockContext, mockGroup); + + expect(result).toStrictEqual({ + name: { value: 'Group Name', lastUpdatedAt: 123456 }, + groupIndex: 0, + }); + }); }); describe('parseWalletFromUserStorageResponse', () => { diff --git a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts index 0e9fb3e22c8..c72b208539f 100644 --- a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts +++ b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts @@ -1,9 +1,12 @@ +import { mask } from '@metamask/superstruct'; + import { assertValidUserStorageWallet, assertValidUserStorageGroup, assertValidLegacyUserStorageAccount, } from './validation'; import type { AccountGroupMultichainAccountObject } from '../../group'; +import { backupAndSyncLogger } from '../../logger'; import type { AccountWalletEntropyObject } from '../../wallet'; import type { BackupAndSyncContext, @@ -11,6 +14,7 @@ import type { UserStorageSyncedWallet, UserStorageSyncedWalletGroup, } from '../types'; +import { UserStorageSyncedWalletGroupSchema } from '../types'; /** * Formats the wallet for user storage usage. @@ -52,10 +56,24 @@ export const formatGroupForUserStorageUsage = ( const persistedGroupMetadata = context.controller.state.accountGroupsMetadata[group.id]; - return { - ...(persistedGroupMetadata ?? {}), - groupIndex: group.metadata.entropy.groupIndex, - }; + try { + // We mask and we try catch, since `mask` will throw if the persisted metadata has + // fields with wrong types. + return mask( + { + ...(persistedGroupMetadata ?? {}), + groupIndex: group.metadata.entropy.groupIndex, + }, + UserStorageSyncedWalletGroupSchema, + ); + } catch (error) { + backupAndSyncLogger( + `Error trying to format group for user storage usage: ${error instanceof Error ? error.message : String(error)}`, + ); + + // If anything goes wrong with this group, we use blank metadata for it. + return { groupIndex: group.metadata.entropy.groupIndex }; + } }; /** From f5b0c029b2f5155db485be5b67481a3ab1a43d47 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Wed, 25 Mar 2026 16:32:21 +0100 Subject: [PATCH 2/4] chore: cosmetic --- .../src/backup-and-sync/user-storage/format-utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts index c72b208539f..aeab628b069 100644 --- a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts +++ b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts @@ -55,6 +55,7 @@ export const formatGroupForUserStorageUsage = ( // This can be null if the user has not manually set a name, pinned or hidden the group const persistedGroupMetadata = context.controller.state.accountGroupsMetadata[group.id]; + const { groupIndex } = group.metadata.entropy; try { // We mask and we try catch, since `mask` will throw if the persisted metadata has @@ -62,7 +63,7 @@ export const formatGroupForUserStorageUsage = ( return mask( { ...(persistedGroupMetadata ?? {}), - groupIndex: group.metadata.entropy.groupIndex, + groupIndex, }, UserStorageSyncedWalletGroupSchema, ); @@ -72,7 +73,7 @@ export const formatGroupForUserStorageUsage = ( ); // If anything goes wrong with this group, we use blank metadata for it. - return { groupIndex: group.metadata.entropy.groupIndex }; + return { groupIndex }; } }; From afe010990085a82ca6b965c4b1e8dd91c18aa638 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Wed, 25 Mar 2026 16:42:05 +0100 Subject: [PATCH 3/4] feat: add toErrorMessage to trick coverage --- .../src/backup-and-sync/service/index.ts | 7 +++---- .../src/backup-and-sync/syncing/group.ts | 3 ++- .../user-storage/format-utils.ts | 9 +++++---- .../user-storage/network-operations.ts | 9 +++++---- .../src/backup-and-sync/utils/errors.test.ts | 17 +++++++++++++++++ .../src/backup-and-sync/utils/errors.ts | 9 +++++++++ .../src/backup-and-sync/utils/index.ts | 1 + 7 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 packages/account-tree-controller/src/backup-and-sync/utils/errors.test.ts create mode 100644 packages/account-tree-controller/src/backup-and-sync/utils/errors.ts 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 64b7523eb10..1b2d81e77de 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 @@ -32,6 +32,7 @@ import { restoreStateFromSnapshot, getLocalEntropyWallets, getLocalGroupsForEntropyWallet, + toErrorMessage, } from '../utils'; import type { StateSnapshot } from '../utils'; @@ -345,8 +346,7 @@ export class BackupAndSyncService { ); } } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); + const errorMessage = toErrorMessage(error); const errorString = `Legacy syncing failed for wallet ${wallet.id}: ${errorMessage}`; backupAndSyncLogger(errorString); @@ -400,8 +400,7 @@ export class BackupAndSyncService { walletProfileId, ); } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); + const errorMessage = toErrorMessage(error); const errorString = `Error during multichain account syncing for wallet ${wallet.id}: ${errorMessage}`; backupAndSyncLogger(errorString); diff --git a/packages/account-tree-controller/src/backup-and-sync/syncing/group.ts b/packages/account-tree-controller/src/backup-and-sync/syncing/group.ts index a4e1fd1c099..fbcbcc519e6 100644 --- a/packages/account-tree-controller/src/backup-and-sync/syncing/group.ts +++ b/packages/account-tree-controller/src/backup-and-sync/syncing/group.ts @@ -17,6 +17,7 @@ import { pushGroupToUserStorageBatch, } from '../user-storage/network-operations'; import { getLocalGroupsForEntropyWallet } from '../utils'; +import { toErrorMessage } from '../utils/errors'; /** * Creates multiple multichain account groups in batch (from 0 to maxGroupIndex). @@ -90,7 +91,7 @@ export const createMultichainAccountGroupsBatch = async ( backupAndSyncLogger( `Failed to create account groups batch:`, // istanbul ignore next - error instanceof Error ? error.message : String(error), + toErrorMessage(error), ); } }; diff --git a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts index aeab628b069..4ea79279c42 100644 --- a/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts +++ b/packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts @@ -15,6 +15,7 @@ import type { UserStorageSyncedWalletGroup, } from '../types'; import { UserStorageSyncedWalletGroupSchema } from '../types'; +import { toErrorMessage } from '../utils/errors'; /** * Formats the wallet for user storage usage. @@ -69,7 +70,7 @@ export const formatGroupForUserStorageUsage = ( ); } catch (error) { backupAndSyncLogger( - `Error trying to format group for user storage usage: ${error instanceof Error ? error.message : String(error)}`, + `Error trying to format group for user storage usage: ${toErrorMessage(error)}`, ); // If anything goes wrong with this group, we use blank metadata for it. @@ -95,7 +96,7 @@ export const parseWalletFromUserStorageResponse = ( return walletData; } catch (error: unknown) { throw new Error( - `Error trying to parse wallet from user storage response: ${error instanceof Error ? error.message : String(error)}`, + `Error trying to parse wallet from user storage response: ${toErrorMessage(error)}`, ); } }; @@ -118,7 +119,7 @@ export const parseGroupFromUserStorageResponse = ( return groupData; } catch (error: unknown) { throw new Error( - `Error trying to parse group from user storage response: ${error instanceof Error ? error.message : String(error)}`, + `Error trying to parse group from user storage response: ${toErrorMessage(error)}`, ); } }; @@ -141,7 +142,7 @@ export const parseLegacyAccountFromUserStorageResponse = ( return accountData; } catch (error: unknown) { throw new Error( - `Error trying to parse legacy account from user storage response: ${error instanceof Error ? error.message : String(error)}`, + `Error trying to parse legacy account from user storage response: ${toErrorMessage(error)}`, ); } }; diff --git a/packages/account-tree-controller/src/backup-and-sync/user-storage/network-operations.ts b/packages/account-tree-controller/src/backup-and-sync/user-storage/network-operations.ts index 53690a37f4b..999568ce3dc 100644 --- a/packages/account-tree-controller/src/backup-and-sync/user-storage/network-operations.ts +++ b/packages/account-tree-controller/src/backup-and-sync/user-storage/network-operations.ts @@ -22,6 +22,7 @@ import type { UserStorageSyncedWallet, UserStorageSyncedWalletGroup, } from '../types'; +import { toErrorMessage } from '../utils/errors'; /** * Retrieves the wallet from user storage. @@ -53,7 +54,7 @@ export const getWalletFromUserStorage = async ( return parseWalletFromUserStorageResponse(walletData); } catch (error) { backupAndSyncLogger( - `Failed to parse wallet data from user storage: ${error instanceof Error ? error.message : String(error)}`, + `Failed to parse wallet data from user storage: ${toErrorMessage(error)}`, ); return null; } @@ -119,7 +120,7 @@ export const getAllGroupsFromUserStorage = async ( return parseGroupFromUserStorageResponse(stringifiedGroup); } catch (error) { backupAndSyncLogger( - `Failed to parse group data from user storage: ${error instanceof Error ? error.message : String(error)}`, + `Failed to parse group data from user storage: ${toErrorMessage(error)}`, ); return null; } @@ -163,7 +164,7 @@ export const getGroupFromUserStorage = async ( return parseGroupFromUserStorageResponse(groupData); } catch (error) { backupAndSyncLogger( - `Failed to parse group data from user storage: ${error instanceof Error ? error.message : String(error)}`, + `Failed to parse group data from user storage: ${toErrorMessage(error)}`, ); return null; } @@ -270,7 +271,7 @@ export const getAllLegacyUserStorageAccounts = async ( return parseLegacyAccountFromUserStorageResponse(stringifiedAccount); } catch (error) { backupAndSyncLogger( - `Failed to parse legacy account data from user storage: ${error instanceof Error ? error.message : String(error)}`, + `Failed to parse legacy account data from user storage: ${toErrorMessage(error)}`, ); return null; } diff --git a/packages/account-tree-controller/src/backup-and-sync/utils/errors.test.ts b/packages/account-tree-controller/src/backup-and-sync/utils/errors.test.ts new file mode 100644 index 00000000000..2879bf4ad8c --- /dev/null +++ b/packages/account-tree-controller/src/backup-and-sync/utils/errors.test.ts @@ -0,0 +1,17 @@ +import { toErrorMessage } from './errors'; + +describe('toErrorMessage', () => { + it('returns the message property for Error instances', () => { + expect(toErrorMessage(new Error('something went wrong'))).toBe( + 'something went wrong', + ); + }); + + it('returns String() for non-Error values', () => { + expect(toErrorMessage('raw string error')).toBe('raw string error'); + expect(toErrorMessage(42)).toBe('42'); + expect(toErrorMessage(null)).toBe('null'); + expect(toErrorMessage(undefined)).toBe('undefined'); + expect(toErrorMessage({ code: 500 })).toBe('[object Object]'); + }); +}); diff --git a/packages/account-tree-controller/src/backup-and-sync/utils/errors.ts b/packages/account-tree-controller/src/backup-and-sync/utils/errors.ts new file mode 100644 index 00000000000..0cdd4b396f9 --- /dev/null +++ b/packages/account-tree-controller/src/backup-and-sync/utils/errors.ts @@ -0,0 +1,9 @@ +/** + * Extracts a string message from an unknown error value. + * + * @param error - The caught error value. + * @returns The error message string. + */ +export function toErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/packages/account-tree-controller/src/backup-and-sync/utils/index.ts b/packages/account-tree-controller/src/backup-and-sync/utils/index.ts index 0471403012a..7e83644ae21 100644 --- a/packages/account-tree-controller/src/backup-and-sync/utils/index.ts +++ b/packages/account-tree-controller/src/backup-and-sync/utils/index.ts @@ -1 +1,2 @@ export * from './controller'; +export { toErrorMessage } from './errors'; From cfa55f2d2cec2d7cae10cbe43e267e8757a9284f Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Wed, 25 Mar 2026 17:00:44 +0100 Subject: [PATCH 4/4] chore: changelog --- packages/account-tree-controller/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index ede359028ce..ed15f0ce965 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 -- Add `lastSelected` (timestamp) to account group tree node metadata ([#8261](https://github.com/MetaMask/core/pull/8261)) +- Add `lastSelected` (timestamp) to account group tree node metadata ([#8261](https://github.com/MetaMask/core/pull/8261)), ([#8300](https://github.com/MetaMask/core/pull/8300)) - `group.metadata.lastSelected` is set to `Date.now()` whenever a group becomes the selected group, either via `setSelectedAccountGroup` or `AccountsController:selectedAccountChange`. - The value is persisted in `accountGroupsMetadata` and restored on `init`/`reinit`. - The value is not synchronize through backup and sync. @@ -25,6 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Filter out extra properties from groups before sending to user storage ([#8300](https://github.com/MetaMask/core/pull/8300)) + - The `lastSelected` metadata should not be persisted, to we automatically remove it from the payload. + - This uses the user storage schema to only keep what needs to be persisted. - Fix `AccountTreeControllerMessenger` type so that the union type for events is not `any` ([#8240](https://github.com/MetaMask/core/pull/8240)) ## [5.0.1]