fix(multichain-account-service)!: wait for Snap platform to be ready

eslint-suppressions.json

@@ -1231,14 +1231,6 @@
       "count": 1
     }
   },
-  "packages/multichain-account-service/src/MultichainAccountService.test.ts": {
-    "@typescript-eslint/explicit-function-return-type": {
-      "count": 1
-    },
-    "@typescript-eslint/naming-convention": {
-      "count": 4
-    }
-  },
   "packages/multichain-account-service/src/MultichainAccountService.ts": {
     "id-length": {
       "count": 1
@@ -1296,14 +1288,6 @@
       "count": 1
     }
   },
-  "packages/multichain-account-service/src/providers/SnapAccountProvider.ts": {
-    "@typescript-eslint/explicit-function-return-type": {
-      "count": 1
-    },
-    "@typescript-eslint/naming-convention": {
-      "count": 1
-    }
-  },
   "packages/multichain-account-service/src/providers/SolAccountProvider.ts": {
     "@typescript-eslint/naming-convention": {
       "count": 2
@@ -1312,11 +1296,6 @@
       "count": 1
     }
   },
-  "packages/multichain-account-service/src/providers/TrxAccountProvider.test.ts": {
-    "no-negated-condition": {
-      "count": 1
-    }
-  },
   "packages/multichain-account-service/src/providers/TrxAccountProvider.ts": {
     "@typescript-eslint/naming-convention": {
       "count": 2

packages/multichain-account-service/CHANGELOG.md

@@ -7,8 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Wait for Snap platform to be ready before any wallet/group operations ([#7266](https://github.com/MetaMask/core/pull/7266))
+- Add `SnapAccountProvider.withSnap` protected helper ([#7266](https://github.com/MetaMask/core/pull/7266))
+  - This is used to protect any Snap operation behind a guard that checks if the Snap platform is ready.
+- Add `MultichainAccountService:ensureCanUseSnapPlatform` method and action.
+  - This will resolve once the Snap platform is ready for the first time and will throw afterward if Snap platform has been disabled dynamically.
+  - This action is mostly used internally by any Snap-based account providers.
+
 ### Changed
 
+- **BREAKING:** The `SnapAccountProvider.client` property is now private ([#7266](https://github.com/MetaMask/core/pull/7266))
+  - You now need to use `SnapAccountProvider.withSnap` to access to it.
 - Bump `@metamask/snaps-controllers` from `^14.0.1` to `^17.2.0` ([#7550](https://github.com/MetaMask/core/pull/7550))
 - Bump `@metamask/snaps-sdk` from `^9.0.0` to `^10.3.0` ([#7550](https://github.com/MetaMask/core/pull/7550))
 - Bump `@metamask/snaps-utils` from `^11.0.0` to `^11.7.0` ([#7550](https://github.com/MetaMask/core/pull/7550))

packages/multichain-account-service/package.json

@@ -64,7 +64,8 @@
     "@metamask/snaps-utils": "^11.7.0",
     "@metamask/superstruct": "^3.1.0",
     "@metamask/utils": "^11.9.0",
-    "async-mutex": "^0.5.0"
+    "async-mutex": "^0.5.0",
+    "lodash": "^4.17.21"
   },
   "devDependencies": {
     "@metamask/account-api": "^0.12.0",

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

@@ -17,6 +17,7 @@ import {
   SOL_ACCOUNT_PROVIDER_NAME,
   SolAccountProvider,
 } from './providers/SolAccountProvider';
+import { SnapPlatformWatcher } from './snaps/SnapPlatformWatcher';
 import {
   MOCK_HARDWARE_ACCOUNT_1,
   MOCK_HD_ACCOUNT_1,
@@ -54,25 +55,40 @@ jest.mock('./providers/SolAccountProvider', () => {
 });
 
 type Mocks = {
+  // eslint-disable-next-line @typescript-eslint/naming-convention
   KeyringController: {
     keyrings: KeyringObject[];
     getState: jest.Mock;
     getKeyringsByType: jest.Mock;
     addNewKeyring: jest.Mock;
   };
+  // eslint-disable-next-line @typescript-eslint/naming-convention
   AccountsController: {
     listMultichainAccounts: 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;
 };
 
+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,
   accounts: KeyringAccount[],
   type: KeyringAccount['type'],
-) {
+): void {
   jest.mocked(providerClass).mockImplementation((...args) => {
     mocks.constructor(...args);
     return mocks as unknown as Provider;
@@ -100,6 +116,7 @@ async function setup({
   rootMessenger: RootMessenger;
   messenger: MultichainAccountServiceMessenger;
   mocks: Mocks;
+  spies: Spies;
 }> {
   const mocks: Mocks = {
     KeyringController: {
@@ -111,13 +128,34 @@ async function setup({
     AccountsController: {
       listMultichainAccounts: jest.fn(),
     },
+    SnapController: {
+      getState: jest.fn(),
+    },
     EvmAccountProvider: makeMockAccountProvider(),
     SolAccountProvider: 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,
@@ -181,6 +219,7 @@ async function setup({
     rootMessenger,
     messenger,
     mocks,
+    spies,
   };
 }
 
@@ -1004,6 +1043,21 @@ describe('MultichainAccountService', () => {
       await messenger.call('MultichainAccountService:resyncAccounts');
       expect(resyncAccountsSpy).toHaveBeenCalled();
     });
+
+    it('checks for Snap platform readiness with MultichainAccountService:ensureCanUseSnapPlatform', async () => {
+      const { messenger, service } = await setup({
+        accounts: [],
+      });
+
+      await service.ensureCanUseSnapPlatform();
+
+      const ensureCanUseSnapPlatformSpy = jest.spyOn(
+        service,
+        'ensureCanUseSnapPlatform',
+      );
+      await messenger.call('MultichainAccountService:ensureCanUseSnapPlatform');
+      expect(ensureCanUseSnapPlatformSpy).toHaveBeenCalled();
+    });
   });
 
   describe('resyncAccounts', () => {
@@ -1247,4 +1301,18 @@ describe('MultichainAccountService', () => {
       expect(mocks.KeyringController.addNewKeyring).not.toHaveBeenCalled();
     });
   });
+
+  describe('ensureCanUseSnapPlatform', () => {
+    it('delegates Snap platform readiness check to SnapPlatformWatcher (method)', async () => {
+      const { service, spies } = await setup({
+        accounts: [],
+      });
+
+      await service.ensureCanUseSnapPlatform();
+
+      expect(
+        spies.SnapPlatformWatcher.ensureCanUseSnapPlatform,
+      ).toHaveBeenCalledTimes(1);
+    });
+  });
 });

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

@@ -31,6 +31,7 @@ import {
   SOL_ACCOUNT_PROVIDER_NAME,
   SolAccountProviderConfig,
 } from './providers/SolAccountProvider';
+import { SnapPlatformWatcher } from './snaps/SnapPlatformWatcher';
 import type {
   MultichainAccountServiceConfig,
   MultichainAccountServiceMessenger,
@@ -64,6 +65,8 @@ type AccountContext<Account extends Bip44Account<KeyringAccount>> = {
 export class MultichainAccountService {
   readonly #messenger: MultichainAccountServiceMessenger;
 
+  readonly #watcher: SnapPlatformWatcher;
+
   readonly #providers: Bip44AccountProvider[];
 
   readonly #wallets: Map<
@@ -124,6 +127,8 @@ export class MultichainAccountService {
       ...providers,
     ];
 
+    this.#watcher = new SnapPlatformWatcher(messenger);
+
     this.#messenger.registerActionHandler(
       'MultichainAccountService:getMultichainAccountGroup',
       (...args) => this.getMultichainAccountGroup(...args),
@@ -168,6 +173,10 @@ export class MultichainAccountService {
       'MultichainAccountService:resyncAccounts',
       (...args) => this.resyncAccounts(...args),
     );
+    this.#messenger.registerActionHandler(
+      'MultichainAccountService:ensureCanUseSnapPlatform',
+      (...args) => this.ensureCanUseSnapPlatform(...args),
+    );
 
     this.#messenger.subscribe('AccountsController:accountAdded', (account) =>
       this.#handleOnAccountAdded(account),
@@ -261,6 +270,10 @@ export class MultichainAccountService {
     log('Providers got re-synced!');
   }
 
+  ensureCanUseSnapPlatform(): Promise<void> {
+    return this.#watcher.ensureCanUseSnapPlatform();
+  }
+
   #handleOnAccountAdded(account: KeyringAccount): void {
     // We completely omit non-BIP-44 accounts!
     if (!isBip44Account(account)) {

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

@@ -6,6 +6,7 @@ import type {
   EthKeyring,
   InternalAccount,
 } from '@metamask/keyring-internal-api';
+import { SnapControllerState } from '@metamask/snaps-controllers';
 
 import { AccountProviderWrapper } from './AccountProviderWrapper';
 import {
@@ -98,6 +99,11 @@ class MockBtcKeyring {
       return account;
     });
 }
+class MockBtcAccountProvider extends BtcAccountProvider {
+  override async ensureCanUseSnapPlatform(): Promise<void> {
+    // Override to avoid waiting during tests.
+  }
+}
 
 /**
  * Sets up a BtcAccountProvider for testing.
@@ -129,6 +135,11 @@ function setup({
 } {
   const keyring = new MockBtcKeyring(accounts);
 
+  messenger.registerActionHandler(
+    'SnapController:getState',
+    () => ({ isReady: true }) as SnapControllerState,
+  );
+
   messenger.registerActionHandler(
     'AccountsController:listMultichainAccounts',
     () => accounts,
@@ -158,7 +169,7 @@ function setup({
   const multichainMessenger = getMultichainAccountServiceMessenger(messenger);
   const provider = new AccountProviderWrapper(
     multichainMessenger,
-    new BtcAccountProvider(multichainMessenger, config),
+    new MockBtcAccountProvider(multichainMessenger, config),
   );
 
   return {
@@ -373,7 +384,7 @@ describe('BtcAccountProvider', () => {
 
       const multichainMessenger =
         getMultichainAccountServiceMessenger(messenger);
-      const btcProvider = new BtcAccountProvider(
+      const btcProvider = new MockBtcAccountProvider(
         multichainMessenger,
         undefined,
         mockTrace,
@@ -425,7 +436,7 @@ describe('BtcAccountProvider', () => {
 
       const multichainMessenger =
         getMultichainAccountServiceMessenger(messenger);
-      const btcProvider = new BtcAccountProvider(
+      const btcProvider = new MockBtcAccountProvider(
         multichainMessenger,
         undefined,
         mockTrace,
@@ -458,7 +469,7 @@ describe('BtcAccountProvider', () => {
 
       const multichainMessenger =
         getMultichainAccountServiceMessenger(messenger);
-      const btcProvider = new BtcAccountProvider(
+      const btcProvider = new MockBtcAccountProvider(
         multichainMessenger,
         undefined,
         mockTrace,