feat!: move SnapPlatformWatcher in SnapAccountService + add :ensureReady and use it in MultichainAccountService (MetaMask#8715)

## Explanation

Now that we have a central place for Snap accounts logic, we can move
the `SnapPlatformWatcher` so we can use it (with an action) on the
`MultichainAccountService`.

Also adding a new `:ensureReady(snapId)` action that will be improved in
the future to make sure everything is ready when interacting with an
account management Snap (migration, platform is ready, keyring is ready,
etc...)

## References

N/A

## Checklist

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

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Introduces breaking messenger/API changes around Snap readiness (new
required `SnapAccountService:ensureReady`, removed
`MultichainAccountService:ensureCanUseSnapPlatform`), which can break
consumers and affect Snap-based account operations if integration is
incomplete.
> 
> **Overview**
> **Moves Snap platform readiness checks out of
`@metamask/multichain-account-service` and into
`@metamask/snap-account-service`.** `SnapAccountService` now owns
`SnapPlatformWatcher`, adds a new
`SnapAccountService:ensureReady(snapId)` messenger action, and exposes
config for onboarding gating and Snap-keyring wait timeout.
> 
> **Updates multichain Snap providers to use the new readiness flow.**
`MultichainAccountService` drops its `ensureCanUseSnapPlatform`
method/action and related config/types, and
`SnapAccountProvider.ensureCanUseSnapPlatform()` is renamed to
`ensureReady()` and now calls `SnapAccountService:ensureReady`. Tests,
messenger policies, dependency graph, and package deps/tsconfig
references are updated accordingly.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c41e954. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: ccharly

README.md

@@ -388,6 +388,7 @@ linkStyle default opacity:0.5
   multichain_account_service --> base_controller;
   multichain_account_service --> keyring_controller;
   multichain_account_service --> messenger;
+  multichain_account_service --> snap_account_service;
   multichain_account_service --> controller_utils;
   multichain_api_middleware --> accounts_controller;
   multichain_api_middleware --> chain_agnostic_permission;
@@ -507,6 +508,7 @@ linkStyle default opacity:0.5
   signature_controller --> logging_controller;
   signature_controller --> messenger;
   signature_controller --> network_controller;
+  snap_account_service --> keyring_controller;
   snap_account_service --> messenger;
   social_controllers --> base_controller;
   social_controllers --> base_data_service;

eslint-suppressions.json

@@ -1299,19 +1299,14 @@
       "count": 2
     }
   },
-  "packages/multichain-account-service/src/snaps/SnapPlatformWatcher.ts": {
-    "no-restricted-syntax": {
-      "count": 2
-    }
-  },
   "packages/multichain-account-service/src/tests/accounts.ts": {
     "@typescript-eslint/explicit-function-return-type": {
       "count": 7
     }
   },
   "packages/multichain-account-service/src/tests/messenger.ts": {
     "no-restricted-syntax": {
-      "count": 2
+      "count": 1
     }
   },
   "packages/multichain-api-middleware/src/handlers/types.ts": {
@@ -2172,6 +2167,21 @@
       "count": 2
     }
   },
+  "packages/snap-account-service/src/SnapAccountService.test.ts": {
+    "no-restricted-syntax": {
+      "count": 2
+    }
+  },
+  "packages/snap-account-service/src/SnapPlatformWatcher.test.ts": {
+    "no-restricted-syntax": {
+      "count": 2
+    }
+  },
+  "packages/snap-account-service/src/SnapPlatformWatcher.ts": {
+    "no-restricted-syntax": {
+      "count": 2
+    }
+  },
   "packages/subscription-controller/src/SubscriptionController.test.ts": {
     "no-restricted-syntax": {
       "count": 2

packages/multichain-account-service/CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- **BREAKING:** The service messenger now requires the `SnapAccountService:ensureReady` action to be declared ([#8715](https://github.com/MetaMask/core/pull/8715))
+- **BREAKING:** Delegate Snap platform readiness to `@metamask/snap-account-service` ([#8715](https://github.com/MetaMask/core/pull/8715))
+  - Removed `MultichainAccountService.ensureCanUseSnapPlatform()` method and the corresponding `MultichainAccountService:ensureCanUseSnapPlatform` messenger action.
+  - Removed the `MultichainAccountServiceEnsureCanUseSnapPlatformAction` type export.
+  - Removed `MultichainAccountServiceOptions.ensureOnboardingComplete`. Configure it via `SnapAccountService`'s `config.snapPlatformWatcher.ensureOnboardingComplete` instead.
+  - Removed `MultichainAccountServiceConfig.snapPlatformWatcher` and the `SnapPlatformWatcherConfig` type export. Configure the keyring-wait timeout via `SnapAccountService`'s `config.snapPlatformWatcher.snapKeyringWaitTimeoutMs` instead.
+  - The service messenger no longer needs `SnapController:getState` or `SnapController:stateChange`.
+- **BREAKING:** Rename `SnapAccountProvider.ensureCanUseSnapPlatform()` to `ensureReady()` ([#8715](https://github.com/MetaMask/core/pull/8715))
+
 ## [9.0.0]
 
 ### Added

packages/multichain-account-service/package.json

@@ -64,6 +64,7 @@
     "@metamask/keyring-snap-client": "^9.0.2",
     "@metamask/keyring-utils": "^3.2.1",
     "@metamask/messenger": "^1.2.0",
+    "@metamask/snap-account-service": "^0.0.0",
     "@metamask/snaps-controllers": "^19.0.0",
     "@metamask/snaps-sdk": "^11.0.0",
     "@metamask/snaps-utils": "^12.1.2",

packages/multichain-account-service/src/MultichainAccountService-method-action-types.ts

@@ -34,11 +34,6 @@ export type MultichainAccountServiceResyncAccountsAction = {
   handler: MultichainAccountService['resyncAccounts'];
 };
 
-export type MultichainAccountServiceEnsureCanUseSnapPlatformAction = {
-  type: `MultichainAccountService:ensureCanUseSnapPlatform`;
-  handler: MultichainAccountService['ensureCanUseSnapPlatform'];
-};
-
 /**
  * Gets a reference to the multichain account wallet matching this entropy source.
  *
@@ -201,7 +196,6 @@ export type MultichainAccountServiceAlignWalletAction = {
 export type MultichainAccountServiceMethodActions =
   | MultichainAccountServiceInitAction
   | MultichainAccountServiceResyncAccountsAction
-  | MultichainAccountServiceEnsureCanUseSnapPlatformAction
   | MultichainAccountServiceGetMultichainAccountWalletAction
   | MultichainAccountServiceGetMultichainAccountWalletsAction
   | MultichainAccountServiceCreateMultichainAccountWalletAction

packages/multichain-account-service/src/MultichainAccountService.test.ts

@@ -38,7 +38,6 @@ import {
   TRX_ACCOUNT_PROVIDER_NAME,
   TrxAccountProvider,
 } from './providers/TrxAccountProvider';
-import { SnapPlatformWatcher } from './snaps/SnapPlatformWatcher';
 import type { RootMessenger, MockAccountProvider } from './tests';
 import {
   MOCK_HARDWARE_ACCOUNT_1,
@@ -53,7 +52,6 @@ import {
 import {
   MOCK_HD_KEYRING_1,
   MOCK_HD_KEYRING_2,
-  MOCK_SNAP_KEYRING,
   getMultichainAccountServiceMessenger,
   getRootMessenger,
   makeMockAccountProvider,
@@ -114,10 +112,6 @@ type Mocks = {
     captureException: jest.Mock;
   };
   // eslint-disable-next-line @typescript-eslint/naming-convention
-  SnapController: {
-    getState: jest.Mock;
-  };
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   EvmAccountProvider: MockAccountProvider;
   // eslint-disable-next-line @typescript-eslint/naming-convention
   SolAccountProvider: MockAccountProvider;
@@ -127,13 +121,6 @@ type Mocks = {
   TrxAccountProvider: MockAccountProvider;
 };
 
-type Spies = {
-  // eslint-disable-next-line @typescript-eslint/naming-convention
-  SnapPlatformWatcher: {
-    ensureCanUseSnapPlatform: jest.SpyInstance;
-  };
-};
-
 function mockAccountProvider<Provider extends Bip44AccountProvider>(
   providerClass: new (messenger: MultichainAccountServiceMessenger) => Provider,
   mocks: MockAccountProvider,
@@ -193,7 +180,6 @@ async function setup({
   rootMessenger: RootMessenger;
   messenger: MultichainAccountServiceMessenger;
   mocks: Mocks;
-  spies: Spies;
 }> {
   const mocks: Mocks = {
     KeyringController: {
@@ -212,36 +198,15 @@ async function setup({
     ErrorReportingService: {
       captureException: jest.fn(),
     },
-    SnapController: {
-      getState: jest.fn(),
-    },
     EvmAccountProvider: makeMockAccountProvider(),
     SolAccountProvider: makeMockAccountProvider(),
     BtcAccountProvider: makeMockAccountProvider(),
     TrxAccountProvider: makeMockAccountProvider(),
   };
 
-  const spies: Spies = {
-    SnapPlatformWatcher: {
-      ensureCanUseSnapPlatform: jest.spyOn(
-        SnapPlatformWatcher.prototype,
-        'ensureCanUseSnapPlatform',
-      ),
-    },
-  };
-
   // Required for the `assert` on `MultichainAccountWallet.createMultichainAccountGroup`.
   Object.setPrototypeOf(mocks.EvmAccountProvider, EvmAccountProvider.prototype);
 
-  mocks.SnapController.getState.mockImplementation(() => ({
-    isReady: true,
-  }));
-
-  rootMessenger.registerActionHandler(
-    'SnapController:getState',
-    mocks.SnapController.getState,
-  );
-
   mocks.KeyringController.getState.mockImplementation(() => ({
     isUnlocked: true,
     keyrings: mocks.KeyringController.keyrings,
@@ -339,7 +304,6 @@ async function setup({
     rootMessenger,
     messenger,
     mocks,
-    spies,
   };
 }
 
@@ -1223,20 +1187,6 @@ describe('MultichainAccountService', () => {
         MOCK_HD_ACCOUNT_1.address,
       );
     });
-
-    it('checks for Snap platform readiness with MultichainAccountService:ensureCanUseSnapPlatform', async () => {
-      const { rootMessenger, spies } = await setup({
-        accounts: [],
-        keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2, MOCK_SNAP_KEYRING],
-      });
-
-      await rootMessenger.call(
-        'MultichainAccountService:ensureCanUseSnapPlatform',
-      );
-      expect(
-        spies.SnapPlatformWatcher.ensureCanUseSnapPlatform,
-      ).toHaveBeenCalled();
-    });
   });
 
   describe('resyncAccounts', () => {
@@ -1608,19 +1558,4 @@ describe('MultichainAccountService', () => {
       });
     });
   });
-
-  describe('ensureCanUseSnapPlatform', () => {
-    it('delegates Snap platform readiness check to SnapPlatformWatcher (method)', async () => {
-      const { service, spies } = await setup({
-        accounts: [],
-        keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2, MOCK_SNAP_KEYRING],
-      });
-
-      await service.ensureCanUseSnapPlatform();
-
-      expect(
-        spies.SnapPlatformWatcher.ensureCanUseSnapPlatform,
-      ).toHaveBeenCalledTimes(1);
-    });
-  });
 });

packages/multichain-account-service/src/MultichainAccountService.ts

@@ -40,7 +40,6 @@ import {
   SOL_ACCOUNT_PROVIDER_NAME,
   SolAccountProviderConfig,
 } from './providers/SolAccountProvider';
-import { SnapPlatformWatcher } from './snaps/SnapPlatformWatcher';
 import type {
   MultichainAccountServiceConfig,
   MultichainAccountServiceMessenger,
@@ -61,10 +60,6 @@ export type MultichainAccountServiceOptions = {
     [TRX_ACCOUNT_PROVIDER_NAME]?: TrxAccountProviderConfig;
   };
   config?: MultichainAccountServiceConfig;
-  /**
-   * When provided, used to prevent using Snap platform before onboarding completion.
-   */
-  ensureOnboardingComplete?: () => Promise<void>;
 };
 
 /**
@@ -116,7 +111,6 @@ const MESSENGER_EXPOSED_METHODS = [
   'createMultichainAccountWallet',
   'resyncAccounts',
   'removeMultichainAccountWallet',
-  'ensureCanUseSnapPlatform',
   'init',
 ] as const;
 
@@ -126,8 +120,6 @@ const MESSENGER_EXPOSED_METHODS = [
 export class MultichainAccountService {
   readonly #messenger: MultichainAccountServiceMessenger;
 
-  readonly #watcher: SnapPlatformWatcher;
-
   readonly #providers: Bip44AccountProvider[];
 
   readonly #trace: TraceCallback;
@@ -151,15 +143,12 @@ export class MultichainAccountService {
    * @param options.providers - Optional list of account
    * @param options.providerConfigs - Optional provider configs
    * @param options.config - Optional config.
-   * @param options.ensureOnboardingComplete - Optional callback to ensure
-   * onboarding is completed before using the Snap platform.
    */
   constructor({
     messenger,
     providers = [],
     providerConfigs,
     config,
-    ensureOnboardingComplete,
   }: MultichainAccountServiceOptions) {
     this.#messenger = messenger;
     this.#wallets = new Map();
@@ -211,11 +200,6 @@ export class MultichainAccountService {
       ...providers,
     ];
 
-    this.#watcher = new SnapPlatformWatcher(messenger, {
-      ensureOnboardingComplete,
-      snapKeyringWaitTimeoutMs: config?.snapPlatformWatcher?.timeoutMs,
-    });
-
     this.#messenger.registerMethodActionHandlers(
       this,
       MESSENGER_EXPOSED_METHODS,
@@ -349,10 +333,6 @@ export class MultichainAccountService {
     log('Providers got re-synced!');
   }
 
-  ensureCanUseSnapPlatform(): Promise<void> {
-    return this.#watcher.ensureCanUseSnapPlatform();
-  }
-
   /**
    * Get the wallet matching the given entropy source.
    *

packages/multichain-account-service/src/index.ts

@@ -8,7 +8,6 @@ export type {
 } from './types';
 export type {
   MultichainAccountServiceResyncAccountsAction,
-  MultichainAccountServiceEnsureCanUseSnapPlatformAction,
   MultichainAccountServiceGetMultichainAccountWalletAction,
   MultichainAccountServiceGetMultichainAccountWalletsAction,
   MultichainAccountServiceCreateMultichainAccountWalletAction,

packages/multichain-account-service/src/providers/BtcAccountProvider.test.ts

@@ -141,7 +141,7 @@ class MockBtcKeyring {
 }
 
 class MockBtcAccountProvider extends BtcAccountProvider {
-  override async ensureCanUseSnapPlatform(): Promise<void> {
+  override async ensureReady(): Promise<void> {
     // Override to avoid waiting during tests.
   }
 }

packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts

@@ -173,8 +173,8 @@ const setup = ({
       },
       handleRequest: jest.fn(),
     },
-    MultichainAccountService: {
-      ensureCanUseSnapPlatform: jest.fn(),
+    SnapAccountService: {
+      ensureReady: jest.fn(),
     },
   };
 
@@ -185,13 +185,11 @@ const setup = ({
   mocks.AccountsController.listMultichainAccounts.mockReturnValue(accounts);
 
   messenger.registerActionHandler(
-    'MultichainAccountService:ensureCanUseSnapPlatform',
-    mocks.MultichainAccountService.ensureCanUseSnapPlatform,
+    'SnapAccountService:ensureReady',
+    mocks.SnapAccountService.ensureReady,
   );
   // Make the platform ready right away (having a resolved promise is enough).
-  mocks.MultichainAccountService.ensureCanUseSnapPlatform.mockResolvedValue(
-    undefined,
-  );
+  mocks.SnapAccountService.ensureReady.mockResolvedValue(undefined);
 
   messenger.registerActionHandler(
     'SnapController:handleRequest',
@@ -239,10 +237,7 @@ const setup = ({
       ),
   );
 
-  const serviceMessenger = getMultichainAccountServiceMessenger(messenger, {
-    // We need this extra action to be able to mock it.
-    actions: ['MultichainAccountService:ensureCanUseSnapPlatform'],
-  });
+  const serviceMessenger = getMultichainAccountServiceMessenger(messenger);
   const config = deepmerge(
     DEFAULT_TEST_CONFIG,
     configOverride as SnapAccountProviderConfig,
@@ -954,15 +949,13 @@ describe('SnapAccountProvider', () => {
     });
   });
 
-  describe('ensureCanUseSnapPlatform', () => {
-    it('delegates Snap platform readiness check to SnapPlatformWatcher', async () => {
+  describe('ensureReady', () => {
+    it('delegates Snap platform readiness check to SnapAccountService:ensureReady', async () => {
       const { provider, mocks } = setup();
 
-      await provider.ensureCanUseSnapPlatform();
+      await provider.ensureReady();
 
-      expect(
-        mocks.MultichainAccountService.ensureCanUseSnapPlatform,
-      ).toHaveBeenCalledTimes(1);
+      expect(mocks.SnapAccountService.ensureReady).toHaveBeenCalledTimes(1);
     });
   });
 });