feat: unsync hidden & pinned accounts metadata

packages/account-tree-controller/CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Stop syncing account group `pinned` and `hidden` metadata with user storage ([#10186](https://github.com/MetaMask/core/pull/10186))
+  - These fields remain persisted locally and can still be imported/exported via `:{import,export}State`.
 - Bump `@metamask/profile-sync-controller` from `^32.0.0` to `^32.1.0` ([#10184](https://github.com/MetaMask/core/pull/10184))
 
 ## [10.0.1]

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);
-    }
   }
 
   /**

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<typeof metadataExports.compareAndSyncMetadata>[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<typeof metadataExports.compareAndSyncMetadata>[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',
-          },
-        }),
-      );
     });
   });
 

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;
 }
 

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<typeof UserStorageSyncedWalletSchema>;
 
-export type UserStorageSyncedWalletGroup = AccountTreeGroupPersistedMetadata & {
+export type UserStorageSyncedWalletGroup = Omit<
+  AccountTreeGroupPersistedMetadata,
+  'pinned' | 'hidden' | 'lastSelected'
+> & {
   groupIndex: AccountGroupMultichainAccountObject['metadata']['entropy']['groupIndex'];
 } & Infer<typeof UserStorageSyncedWalletGroupSchema>;
 

packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.test.ts

@@ -95,18 +95,19 @@ 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;
 
       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);

packages/account-tree-controller/src/backup-and-sync/user-storage/format-utils.ts

@@ -53,17 +53,20 @@
   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,