From 30aae3f6f004d08448236cca24458b32b0f1fa39 Mon Sep 17 00:00:00 2001 From: gantunesr <17601467+gantunesr@users.noreply.github.com> Date: Thu, 10 Sep 2026 18:18:34 -0300 Subject: [PATCH] feat: unsync hidden & pinned accounts metadata --- packages/account-tree-controller/CHANGELOG.md | 5 + .../src/AccountTreeController.ts | 18 --- .../src/backup-and-sync/syncing/group.test.ts | 149 +++--------------- .../src/backup-and-sync/syncing/group.ts | 36 ----- .../src/backup-and-sync/types.ts | 12 +- .../user-storage/format-utils.test.ts | 9 +- .../user-storage/format-utils.ts | 9 +- .../user-storage/validation.test.ts | 15 +- packages/account-tree-controller/src/group.ts | 4 +- 9 files changed, 61 insertions(+), 196 deletions(-) diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index 1a661763360..79d427ec1e5 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Stop syncing account group `pinned` and `hidden` metadata with user storage + - These fields remain persisted locally and can still be imported/exported via `:{import,export}State`. + ## [10.0.0] ### Changed diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index 2734a3eb37a..ba0ea0e83d1 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -1767,15 +1767,6 @@ export class AccountTreeController extends BaseController< if (walletId) { this.#publishAccountGroupUpdated(walletId, groupId); } - - // Trigger atomic sync for group pinning (only for groups from entropy wallets) - if ( - walletId && - this.state.accountTree.wallets[walletId].type === - AccountWalletType.Entropy - ) { - this.#backupAndSyncService.enqueueSingleGroupSync(groupId); - } } /** @@ -1813,15 +1804,6 @@ export class AccountTreeController extends BaseController< if (walletId) { this.#publishAccountGroupUpdated(walletId, groupId); } - - // Trigger atomic sync for group hiding (only for groups from entropy wallets) - if ( - walletId && - this.state.accountTree.wallets[walletId].type === - AccountWalletType.Entropy - ) { - this.#backupAndSyncService.enqueueSingleGroupSync(groupId); - } } /** diff --git a/packages/account-tree-controller/src/backup-and-sync/syncing/group.test.ts b/packages/account-tree-controller/src/backup-and-sync/syncing/group.test.ts index 1953d3dba14..d267014d2da 100644 --- a/packages/account-tree-controller/src/backup-and-sync/syncing/group.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/syncing/group.test.ts @@ -306,126 +306,39 @@ describe('BackupAndSync - Syncing - Group', () => { /* eslint-enable jest/no-conditional-expect */ }); - it('handles pinned metadata validation and apply local update', async () => { + it('does not sync pinned or hidden metadata', async () => { mockContext.controller.state.accountGroupsMetadata[mockLocalGroup.id] = { pinned: { value: false, lastUpdatedAt: 1000 }, - }; - - let validatePinnedFunction: - | Parameters< - typeof metadataExports.compareAndSyncMetadata - >[0]['validateUserStorageValue'] - | undefined; - let applyPinnedUpdate: - | Parameters< - typeof metadataExports.compareAndSyncMetadata - >[0]['applyLocalUpdate'] - | undefined; - - mockCompareAndSyncMetadata.mockImplementation( - async ( - options: Parameters[0], - ) => { - if ( - options.userStorageMetadata && - 'value' in options.userStorageMetadata && - typeof options.userStorageMetadata.value === 'boolean' - ) { - validatePinnedFunction = options.validateUserStorageValue; - applyPinnedUpdate = options.applyLocalUpdate; - } - return false; - }, - ); - - await syncGroupMetadata( - mockContext, - mockLocalGroup, - { - pinned: { value: true, lastUpdatedAt: 2000 }, - } as unknown as UserStorageSyncedWalletGroup, - 'test-entropy', - 'test-profile', - ); - - expect(validatePinnedFunction).toBeDefined(); - expect(applyPinnedUpdate).toBeDefined(); - /* eslint-disable jest/no-conditional-expect */ - if (validatePinnedFunction) { - expect(validatePinnedFunction(true)).toBe(true); - expect(validatePinnedFunction(false)).toBe(true); - expect(validatePinnedFunction('invalid')).toBe(false); - expect(validatePinnedFunction(null)).toBe(false); - } - - if (applyPinnedUpdate) { - await applyPinnedUpdate(true); - expect( - mockContext.controller.setAccountGroupPinned, - ).toHaveBeenCalledWith(mockLocalGroup.id, true); - } - /* eslint-enable jest/no-conditional-expect */ - }); - - it('handles hidden metadata validation and apply local update', async () => { - mockContext.controller.state.accountGroupsMetadata[mockLocalGroup.id] = { hidden: { value: false, lastUpdatedAt: 1000 }, }; - - let validateHiddenFunction: - | Parameters< - typeof metadataExports.compareAndSyncMetadata - >[0]['validateUserStorageValue'] - | undefined; - let applyHiddenUpdate: - | Parameters< - typeof metadataExports.compareAndSyncMetadata - >[0]['applyLocalUpdate'] - | undefined; - - mockCompareAndSyncMetadata.mockImplementation( - async ( - options: Parameters[0], - ) => { - if ( - options.userStorageMetadata && - 'value' in options.userStorageMetadata && - typeof options.userStorageMetadata.value === 'boolean' - ) { - validateHiddenFunction = options.validateUserStorageValue; - applyHiddenUpdate = options.applyLocalUpdate; - } - return false; - }, - ); + mockCompareAndSyncMetadata.mockResolvedValue(false); await syncGroupMetadata( mockContext, mockLocalGroup, { - hidden: { value: true, lastUpdatedAt: 2000 }, - } as unknown as UserStorageSyncedWalletGroup, + groupIndex: 0, + name: { value: 'Remote Name', lastUpdatedAt: 2000 }, + }, 'test-entropy', 'test-profile', ); - expect(validateHiddenFunction).toBeDefined(); - expect(applyHiddenUpdate).toBeDefined(); - /* eslint-disable jest/no-conditional-expect */ - if (validateHiddenFunction) { - expect(validateHiddenFunction(true)).toBe(true); - expect(validateHiddenFunction(false)).toBe(true); - expect(validateHiddenFunction('invalid')).toBe(false); - expect(validateHiddenFunction(123)).toBe(false); - } - - if (applyHiddenUpdate) { - await applyHiddenUpdate(false); - expect( - mockContext.controller.setAccountGroupHidden, - ).toHaveBeenCalledWith(mockLocalGroup.id, false); - } - /* eslint-enable jest/no-conditional-expect */ + expect(mockCompareAndSyncMetadata).toHaveBeenCalledTimes(1); + expect(mockCompareAndSyncMetadata).toHaveBeenCalledWith( + expect.objectContaining({ + analytics: { + action: BackupAndSyncAnalyticsEvent.GroupRenamed, + profileId: 'test-profile', + }, + }), + ); + expect( + mockContext.controller.setAccountGroupPinned, + ).not.toHaveBeenCalled(); + expect( + mockContext.controller.setAccountGroupHidden, + ).not.toHaveBeenCalled(); }); }); @@ -489,7 +402,7 @@ describe('BackupAndSync - Syncing - Group', () => { expect(mockPushGroupToUserStorageBatch).toHaveBeenCalled(); }); - it('handles metadata sync for name, pinned, and hidden fields', async () => { + it('handles metadata sync for name only', async () => { const localGroup = { id: 'entropy:test-entropy/0', metadata: { entropy: { groupIndex: 0 } }, @@ -511,15 +424,13 @@ describe('BackupAndSync - Syncing - Group', () => { { groupIndex: 0, name: { value: 'Remote Name', lastUpdatedAt: 2000 }, - pinned: { value: false, lastUpdatedAt: 2000 }, - hidden: { value: true, lastUpdatedAt: 2000 }, }, ], 'test-entropy', 'test-profile', ); - expect(mockCompareAndSyncMetadata).toHaveBeenCalledTimes(3); + expect(mockCompareAndSyncMetadata).toHaveBeenCalledTimes(1); expect(mockCompareAndSyncMetadata).toHaveBeenCalledWith( expect.objectContaining({ analytics: { @@ -528,22 +439,6 @@ describe('BackupAndSync - Syncing - Group', () => { }, }), ); - expect(mockCompareAndSyncMetadata).toHaveBeenCalledWith( - expect.objectContaining({ - analytics: { - action: BackupAndSyncAnalyticsEvent.GroupPinnedStatusChanged, - profileId: 'test-profile', - }, - }), - ); - expect(mockCompareAndSyncMetadata).toHaveBeenCalledWith( - expect.objectContaining({ - analytics: { - action: BackupAndSyncAnalyticsEvent.GroupHiddenStatusChanged, - profileId: 'test-profile', - }, - }), - ); }); }); 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 b65214c7386..e8d1def9708 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 @@ -174,42 +174,6 @@ async function syncGroupMetadataAndCheckIfPushNeeded( shouldPushGroup ||= shouldPushForName; - // Compare and sync pinned metadata - const shouldPushForPinned = await compareAndSyncMetadata({ - context, - localMetadata: groupPersistedMetadata?.pinned, - userStorageMetadata: groupFromUserStorage.pinned, - validateUserStorageValue: (value) => - UserStorageSyncedWalletGroupSchema.schema.pinned.schema.value.is(value), - applyLocalUpdate: (pinned: boolean) => { - context.controller.setAccountGroupPinned(localGroup.id, pinned); - }, - analytics: { - action: BackupAndSyncAnalyticsEvent.GroupPinnedStatusChanged, - profileId, - }, - }); - - shouldPushGroup ||= shouldPushForPinned; - - // Compare and sync hidden metadata - const shouldPushForHidden = await compareAndSyncMetadata({ - context, - localMetadata: groupPersistedMetadata?.hidden, - userStorageMetadata: groupFromUserStorage.hidden, - validateUserStorageValue: (value) => - UserStorageSyncedWalletGroupSchema.schema.hidden.schema.value.is(value), - applyLocalUpdate: (hidden: boolean) => { - context.controller.setAccountGroupHidden(localGroup.id, hidden); - }, - analytics: { - action: BackupAndSyncAnalyticsEvent.GroupHiddenStatusChanged, - profileId, - }, - }); - - shouldPushGroup ||= shouldPushForHidden; - return shouldPushGroup; } diff --git a/packages/account-tree-controller/src/backup-and-sync/types.ts b/packages/account-tree-controller/src/backup-and-sync/types.ts index a2d37797b4e..8cea23387a1 100644 --- a/packages/account-tree-controller/src/backup-and-sync/types.ts +++ b/packages/account-tree-controller/src/backup-and-sync/types.ts @@ -12,6 +12,7 @@ import { boolean, number, optional, + type, } from '@metamask/superstruct'; import type { Infer, Struct } from '@metamask/superstruct'; @@ -47,11 +48,11 @@ export const UserStorageSyncedWalletSchema = object({ /** * Superstruct schema for UserStorageSyncedWalletGroup validation. + * Uses `type` rather than `object` so previously synced `pinned`/`hidden` + * fields on remote records are still accepted (they are ignored by sync). */ -export const UserStorageSyncedWalletGroupSchema = object({ +export const UserStorageSyncedWalletGroupSchema = type({ name: optional(UpdatableFieldSchema(string())), - pinned: optional(UpdatableFieldSchema(boolean())), - hidden: optional(UpdatableFieldSchema(boolean())), groupIndex: number(), }); @@ -69,7 +70,10 @@ export const LegacyUserStorageSyncedAccountSchema = object({ export type UserStorageSyncedWallet = AccountTreeWalletPersistedMetadata & Infer; -export type UserStorageSyncedWalletGroup = AccountTreeGroupPersistedMetadata & { +export type UserStorageSyncedWalletGroup = Omit< + AccountTreeGroupPersistedMetadata, + 'pinned' | 'hidden' | 'lastSelected' +> & { groupIndex: AccountGroupMultichainAccountObject['metadata']['entropy']['groupIndex']; } & Infer; 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 b4b75a567e0..b6f3b845751 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 @@ -95,10 +95,11 @@ describe('BackupAndSync - UserStorage - FormatUtils', () => { }); describe('formatGroupForUserStorageUsage', () => { - it('returns group metadata with groupIndex', () => { + it('returns group name metadata with groupIndex', () => { const groupMetadata = { name: { value: 'Group Name', lastUpdatedAt: 123456 }, pinned: { value: true, lastUpdatedAt: 123456 }, + hidden: { value: true, lastUpdatedAt: 123456 }, }; mockContext.controller.state.accountGroupsMetadata[mockGroup.id] = groupMetadata; @@ -106,7 +107,7 @@ describe('BackupAndSync - UserStorage - FormatUtils', () => { const result = formatGroupForUserStorageUsage(mockContext, mockGroup); expect(result).toStrictEqual({ - ...groupMetadata, + name: groupMetadata.name, groupIndex: 0, }); }); @@ -135,10 +136,12 @@ describe('BackupAndSync - UserStorage - FormatUtils', () => { ); }); - it('strips fields not in the schema (e.g. lastSelected)', () => { + it('strips fields not in the schema (e.g. lastSelected, pinned, hidden)', () => { mockContext.controller.state.accountGroupsMetadata[mockGroup.id] = { name: { value: 'Group Name', lastUpdatedAt: 123456 }, lastSelected: 999999, + pinned: { value: true, lastUpdatedAt: 123456 }, + hidden: { value: true, lastUpdatedAt: 123456 }, }; const result = formatGroupForUserStorageUsage(mockContext, mockGroup); 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 efb85e3b417..ffa9dfa2154 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 @@ -53,17 +53,20 @@ export const formatGroupForUserStorageUsage = ( context: BackupAndSyncContext, group: AccountGroupMultichainAccountObject, ): UserStorageSyncedWalletGroup => { - // This can be null if the user has not manually set a name, pinned or hidden the group + // This can be null if the user has not manually set a name 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 - // fields with wrong types. + // fields with wrong types. Only `name` is synced; `pinned`, `hidden`, and + // `lastSelected` are local-only. return mask( { - ...(persistedGroupMetadata ?? {}), + ...(persistedGroupMetadata?.name !== undefined + ? { name: persistedGroupMetadata.name } + : {}), groupIndex, }, UserStorageSyncedWalletGroupSchema, diff --git a/packages/account-tree-controller/src/backup-and-sync/user-storage/validation.test.ts b/packages/account-tree-controller/src/backup-and-sync/user-storage/validation.test.ts index 5252d59b7e3..90e5495aa72 100644 --- a/packages/account-tree-controller/src/backup-and-sync/user-storage/validation.test.ts +++ b/packages/account-tree-controller/src/backup-and-sync/user-storage/validation.test.ts @@ -53,6 +53,15 @@ describe('BackupAndSync - UserStorage - Validation', () => { describe('assertValidUserStorageGroup', () => { it('passes for valid group data', () => { + const validGroupData = { + name: { value: 'Test Group', lastUpdatedAt: 1234567890 }, + groupIndex: 0, + }; + + expect(() => assertValidUserStorageGroup(validGroupData)).not.toThrow(); + }); + + it('still accepts previously synced pinned and hidden fields', () => { const validGroupData = { name: { value: 'Test Group', lastUpdatedAt: 1234567890 }, pinned: { value: true, lastUpdatedAt: 1234567890 }, @@ -89,9 +98,9 @@ describe('BackupAndSync - UserStorage - Validation', () => { value: 'Valid Name', lastUpdatedAt: null, // This should cause a validation error }, - pinned: { - value: 'not boolean', // This should cause a validation error - lastUpdatedAt: 1234567890, + name: { + value: 'Valid Name', + lastUpdatedAt: null, // This should cause a validation error }, }; diff --git a/packages/account-tree-controller/src/group.ts b/packages/account-tree-controller/src/group.ts index 11ed0557d15..4149606b0fa 100644 --- a/packages/account-tree-controller/src/group.ts +++ b/packages/account-tree-controller/src/group.ts @@ -24,9 +24,9 @@ import type { AccountWalletObject } from './wallet.js'; export type AccountTreeGroupPersistedMetadata = { /** Custom name set by user, overrides default naming logic */ name?: UpdatableField; - /** Whether this group is pinned in the UI */ + /** Whether this group is pinned in the UI (local-only, not synced) */ pinned?: UpdatableField; - /** Whether this group is hidden in the UI */ + /** Whether this group is hidden in the UI (local-only, not synced) */ hidden?: UpdatableField; /** Timestamp of the last time this group was selected (local-only, not synced) */ lastSelected?: number;