feat(account-tree-controller): do not enqueue single syncs during full syncs (#6651)

## Explanation

This PR prevents single sync events from being enqueued if a full sync
is already in progress.
This prevents unnecessary client side network calls.

## References

Fixes: https://consensyssoftware.atlassian.net/browse/MUL-901

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs),
highlighting breaking changes as necessary
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

Author: mathieuartu

packages/account-tree-controller/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Single group sync events will not get enqueued anymore if a full sync is in progress ([#6651](https://github.com/MetaMask/core/pull/6651))
+  - This prevents too many unnecessary storage fetches (which would prevent being rate limited).
+  - This could rarely lead to inconsistencies until the next single updates or next full sync.
+
 ## [0.16.1]
 
 ### Added

packages/account-tree-controller/src/backup-and-sync/service/index.test.ts

@@ -94,6 +94,9 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => {
       expect(mockContext.messenger.call).toHaveBeenCalledWith(
         'UserStorageController:getState',
       );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
     });
 
     it('returns early when account syncing is disabled', () => {
@@ -106,6 +109,23 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => {
       expect(mockContext.messenger.call).toHaveBeenCalledWith(
         'UserStorageController:getState',
       );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
+    });
+
+    it('returns early when a full sync has not completed at least once', () => {
+      mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce =
+        false;
+      backupAndSyncService.enqueueSingleWalletSync('entropy:wallet-1');
+      // Should not have called any messenger functions beyond the state check
+      expect(mockContext.messenger.call).toHaveBeenCalledTimes(1);
+      expect(mockContext.messenger.call).toHaveBeenCalledWith(
+        'UserStorageController:getState',
+      );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
     });
 
     it('enqueues single wallet sync when enabled and synced at least once', async () => {
@@ -158,6 +178,9 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => {
       expect(mockContext.messenger.call).toHaveBeenCalledWith(
         'UserStorageController:getState',
       );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
     });
 
     it('returns early when account syncing is disabled', () => {
@@ -170,6 +193,40 @@ describe('BackupAndSync - Service - BackupAndSyncService', () => {
       expect(mockContext.messenger.call).toHaveBeenCalledWith(
         'UserStorageController:getState',
       );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
+    });
+
+    it('returns early when a full sync is already in progress', () => {
+      mockContext.controller.state.isAccountTreeSyncingInProgress = true;
+
+      backupAndSyncService.enqueueSingleGroupSync('entropy:wallet-1/1');
+
+      // Should not have called any messenger functions beyond the state check
+      expect(mockContext.messenger.call).toHaveBeenCalledTimes(1);
+      expect(mockContext.messenger.call).toHaveBeenCalledWith(
+        'UserStorageController:getState',
+      );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
+    });
+
+    it('returns early when a full sync has not completed at least once', () => {
+      mockContext.controller.state.hasAccountTreeSyncingSyncedAtLeastOnce =
+        false;
+
+      backupAndSyncService.enqueueSingleGroupSync('entropy:wallet-1/1');
+
+      // Should not have called any messenger functions beyond the state check
+      expect(mockContext.messenger.call).toHaveBeenCalledTimes(1);
+      expect(mockContext.messenger.call).toHaveBeenCalledWith(
+        'UserStorageController:getState',
+      );
+      expect(mockContext.messenger.call).not.toHaveBeenCalledWith(
+        'UserStorageController:performGetStorage',
+      );
     });
 
     it('enqueues group sync when enabled and synced at least once', async () => {

packages/account-tree-controller/src/backup-and-sync/service/index.ts

@@ -184,7 +184,16 @@ export class BackupAndSyncService {
    * @param groupId - The group ID to sync.
    */
   enqueueSingleGroupSync(groupId: AccountGroupId): void {
-    if (!this.isBackupAndSyncEnabled || !this.hasSyncedAtLeastOnce) {
+    if (
+      !this.isBackupAndSyncEnabled ||
+      !this.hasSyncedAtLeastOnce ||
+      // This prevents rate limiting scenarios where full syncs trigger group creations
+      // that in turn enqueue the same single group syncs that the full sync just did.
+      // This can very rarely lead to inconsistencies, but will be fixed on the next full sync.
+      // TODO: let's improve this in the future by tracking the updates done in the full sync and
+      // comparing against that.
+      this.isInProgress
+    ) {
       return;
     }