fix(bridge-status-controller)!: use SnapKeyring.submitRequest to dispatch Snap keyring requests

packages/bridge-status-controller/CHANGELOG.md

@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **BREAKING:** Now requires a new `withSnapKeyringFn` parameter when constructing the controller. ([#5924](https://github.com/MetaMask/core/pull/5924))
+  - This will be used to access the `SnapKeyring` instance and submit keyring request to a Snap.
+- **BREAKING:** No longer depend on `@metamask/snaps-controllers` dependency. ([#5924](https://github.com/MetaMask/core/pull/5924))
+  - The Snap request handling is now delegated to the `SnapKeyring` via the new `withSnapKeyringFn` callback.
 - Bump `@metamask/controller-utils` to `^11.10.0` ([#5935](https://github.com/MetaMask/core/pull/5935))
 
 ## [29.1.0]

packages/bridge-status-controller/package.json

@@ -61,10 +61,10 @@
     "@metamask/accounts-controller": "^30.0.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/bridge-controller": "^32.1.1",
+    "@metamask/eth-snap-keyring": "^13.0.0",
     "@metamask/gas-fee-controller": "^23.0.0",
     "@metamask/multichain-transactions-controller": "^2.0.0",
     "@metamask/network-controller": "^23.6.0",
-    "@metamask/snaps-controllers": "^12.3.1",
     "@metamask/transaction-controller": "^57.2.0",
     "@types/jest": "^27.4.1",
     "deepmerge": "^4.2.2",
@@ -83,7 +83,6 @@
     "@metamask/gas-fee-controller": "^23.0.0",
     "@metamask/multichain-transactions-controller": "^2.0.0",
     "@metamask/network-controller": "^23.0.0",
-    "@metamask/snaps-controllers": "^12.0.0",
     "@metamask/transaction-controller": "^57.0.0"
   },
   "engines": {

packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap

@@ -2854,34 +2854,6 @@ Array [
   Array [
     "AccountsController:getSelectedMultichainAccount",
   ],
-  Array [
-    "SnapController:handleRequest",
-    Object {
-      "handler": "onKeyringRequest",
-      "origin": "metamask",
-      "request": Object {
-        "id": "test-uuid-1234",
-        "jsonrpc": "2.0",
-        "method": "keyring_submitRequest",
-        "params": Object {
-          "account": undefined,
-          "id": "test-uuid-1234",
-          "request": Object {
-            "method": "signAndSendTransaction",
-            "params": Object {
-              "account": Object {
-                "address": "0x123...",
-              },
-              "scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp",
-              "transaction": "AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACAAQAHDXLY8oVRIwA8ZdRSGjM5RIZJW8Wv+Twyw3NqU4Hov+OHoHp/dmeDvstKbICW3ezeGR69t3/PTAvdXgZVdJFJXaxkoKXUTWfEAyQyCCG9nwVoDsd10OFdnM9ldSi+9SLqHpqWVDV+zzkmftkF//DpbXxqeH8obNXHFR7pUlxG9uNVOn64oNsFdeUvD139j1M51iRmUY839Y25ET4jDRscT081oGb+rLnywLjLSrIQx6MkqNBhCFbxqY1YmoGZVORW/QMGRm/lIRcy/+ytunLDm+e8jOW7xfcSayxDmzpAAAAAjJclj04kifG7PRApFI4NgwtaE5na/xCEBI572Nvp+FkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAbd9uHXZaGT2cvhRs7reawctIXtX1s3kTqM9YV+/wCpBHnVW/IxwG7udMVuzmgVB/2xst6j9I5RArHNola8E4+0P/on9df2SnTAmx8pWHneSwmrNt/J3VFLMhqns4zl6JmXkZ+niuxMhAGrmKBaBo94uMv2Sl+Xh3i+VOO0m5BdNZ1ElenbwQylHQY+VW1ydG1MaUEeNpG+EVgswzPMwPoLBgAFAsBcFQAGAAkDQA0DAAAAAAAHBgABAhMICQAHBgADABYICQEBCAIAAwwCAAAAUEYVOwAAAAAJAQMBEQoUCQADBAETCgsKFw0ODxARAwQACRQj5RfLl3rjrSoBAAAAQ2QAAVBGFTsAAAAAyYZnBwAAAABkAAAJAwMAAAEJDAkAAAIBBBMVCQjGASBMKQwnooTbKNxdBwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAUHTKomh4KXvNgA0ovYKS5F8GIOBgAAAAAAAAAAAAAAAAAQgAAAAAAAAAAAAAAAAAAAAAAAEIF7RFOAwAAAAAAAAAAAAAAaAIAAAAAAAC4CwAAAAAAAOAA2mcAAAAAAAAAAAAAAAAAAAAApapuIXG0FuHSfsU8qME9s/kaic0AAwGCsZdSuxV5eCm+Ria4LEQPgTg4bg65gNrTAefEzpAfPQgCABIMAgAAAAAAAAAAAAAACAIABQwCAAAAsIOFAAAAAAADWk6DVOZO8lMFQg2r0dgfltD6tRL/B1hH3u00UzZdgqkAAxEqIPdq2eRt/F6mHNmFe7iwZpdrtGmHNJMFlK7c6Bc6k6kjBezr6u/tAgvu3OGsJSwSElmcOHZ21imqH/rhJ2KgqDJdBPFH4SYIM1kBAAA=",
-            },
-          },
-          "scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp",
-        },
-      },
-      "snapId": "test-snap",
-    },
-  ],
   Array [
     "BridgeController:trackUnifiedSwapBridgeEvent",
     "Unified SwapBridge Snap Confirmation Page Viewed",

packages/bridge-status-controller/src/bridge-status-controller.test.ts

@@ -540,6 +540,7 @@ const executePollingWithPendingStatus = async () => {
     messenger: getMessengerMock(),
     clientId: BridgeClientId.EXTENSION,
     fetchFn: jest.fn(),
+    withSnapKeyringFn: jest.fn(),
     addTransactionFn: jest.fn(),
     estimateGasFeeFn: jest.fn(),
     addUserOperationFromTransactionFn: jest.fn(),
@@ -566,6 +567,10 @@ const executePollingWithPendingStatus = async () => {
 
 // Define mocks at the top level
 const mockFetchFn = jest.fn();
+const mockWithSnapKeyringFn = jest.fn();
+const mockKeyring = {
+  submitRequest: jest.fn(),
+};
 const mockMessengerCall = jest.fn();
 const mockSelectedAccount = {
   id: 'test-account-id',
@@ -593,6 +598,7 @@ const getController = (call: jest.Mock, traceFn?: jest.Mock) => {
     } as never,
     clientId: BridgeClientId.EXTENSION,
     fetchFn: mockFetchFn,
+    withSnapKeyringFn: mockWithSnapKeyringFn,
     addTransactionFn,
     estimateGasFeeFn,
     addUserOperationFromTransactionFn,
@@ -614,6 +620,13 @@ describe('BridgeStatusController', () => {
   beforeEach(() => {
     jest.clearAllMocks();
     jest.clearAllTimers();
+
+    // We re-wire up the mocked Snap keyring callback, so we can just
+    // use `mockSnapKeyring.submitRequest` when we need to mock Snap
+    // keyring requests.
+    mockWithSnapKeyringFn.mockImplementation(async (callback) => {
+      return await callback({ keyring: mockKeyring });
+    });
   });
 
   describe('constructor', () => {
@@ -622,6 +635,7 @@ describe('BridgeStatusController', () => {
         messenger: getMessengerMock(),
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -635,6 +649,7 @@ describe('BridgeStatusController', () => {
         messenger: getMessengerMock(),
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -672,6 +687,7 @@ describe('BridgeStatusController', () => {
         },
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
       });
@@ -694,6 +710,7 @@ describe('BridgeStatusController', () => {
         messenger: getMessengerMock(),
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -732,6 +749,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -813,6 +831,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -852,6 +871,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -893,6 +913,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -956,6 +977,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -1044,6 +1066,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -1131,6 +1154,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -1232,6 +1256,7 @@ describe('BridgeStatusController', () => {
         messenger: messengerMock,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),
@@ -1430,8 +1455,9 @@ describe('BridgeStatusController', () => {
     });
 
     it('should successfully submit a Solana transaction', async () => {
+      mockKeyring.submitRequest.mockResolvedValueOnce('signature');
+
       mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
-      mockMessengerCall.mockResolvedValueOnce('signature');
 
       mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
       mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
@@ -1486,7 +1512,7 @@ describe('BridgeStatusController', () => {
 
     it('should handle snap controller errors', async () => {
       mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
-      mockMessengerCall.mockRejectedValueOnce(new Error('Snap error'));
+      mockKeyring.submitRequest.mockRejectedValueOnce(new Error('Snap error'));
 
       const { controller, startPollingForBridgeTxStatusSpy } =
         getController(mockMessengerCall);
@@ -2244,6 +2270,7 @@ describe('BridgeStatusController', () => {
         messenger: mockBridgeStatusMessenger,
         clientId: BridgeClientId.EXTENSION,
         fetchFn: jest.fn(),
+        withSnapKeyringFn: jest.fn(),
         addTransactionFn: jest.fn(),
         estimateGasFeeFn: jest.fn(),
         addUserOperationFromTransactionFn: jest.fn(),

packages/bridge-status-controller/src/bridge-status-controller.ts

@@ -18,6 +18,7 @@ import {
 } from '@metamask/bridge-controller';
 import type { TraceCallback } from '@metamask/controller-utils';
 import { toHex } from '@metamask/controller-utils';
+import type { SnapKeyring } from '@metamask/eth-snap-keyring';
 import { EthAccountType } from '@metamask/keyring-api';
 import { StaticIntervalPollingController } from '@metamask/polling-controller';
 import type {
@@ -79,6 +80,11 @@ const metadata: StateMetadata<BridgeStatusControllerState> = {
   },
 };
 
+// Expecting a bound function that calls KeyringController.withKeyring selecting the Snap keyring
+type WithSnapKeyringFn = <ReturnType>(
+  operation: ({ keyring }: { keyring: SnapKeyring }) => Promise<ReturnType>,
+) => Promise<ReturnType>;
+
 /** The input to start polling for the {@link BridgeStatusController} */
 type BridgeStatusPollingInput = FetchBridgeTxStatusArgs;
 
@@ -105,6 +111,8 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
 
   readonly #estimateGasFeeFn: typeof TransactionController.prototype.estimateGasFee;
 
+  readonly #withSnapKeyringFn: WithSnapKeyringFn;
+
   readonly #addUserOperationFromTransactionFn?: typeof UserOperationController.prototype.addUserOperationFromTransaction;
 
   readonly #trace: TraceCallback;
@@ -116,6 +124,7 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
     fetchFn,
     addTransactionFn,
     addUserOperationFromTransactionFn,
+    withSnapKeyringFn,
     estimateGasFeeFn,
     config,
     traceFn,
@@ -126,6 +135,7 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
     fetchFn: FetchFunction;
     addTransactionFn: typeof TransactionController.prototype.addTransaction;
     estimateGasFeeFn: typeof TransactionController.prototype.estimateGasFee;
+    withSnapKeyringFn: WithSnapKeyringFn;
     addUserOperationFromTransactionFn?: typeof UserOperationController.prototype.addUserOperationFromTransaction;
     config?: {
       customBridgeApiBaseUrl?: string;
@@ -147,6 +157,7 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
     this.#fetchFn = fetchFn;
     this.#addTransactionFn = addTransactionFn;
     this.#addUserOperationFromTransactionFn = addUserOperationFromTransactionFn;
+    this.#withSnapKeyringFn = withSnapKeyringFn;
     this.#estimateGasFeeFn = estimateGasFeeFn;
     this.#config = {
       customBridgeApiBaseUrl:
@@ -560,10 +571,13 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
       );
     }
     const keyringRequest = getKeyringRequest(quoteResponse, selectedAccount);
-    const keyringResponse = (await this.messagingSystem.call(
-      'SnapController:handleRequest',
-      keyringRequest,
-    )) as string | { result: Record<string, string> };
+    const keyringResponse = await this.#withSnapKeyringFn(
+      async ({ keyring }) => {
+        return (await keyring.submitRequest(keyringRequest)) as
+          | string
+          | { result: Record<string, string> };
+      },
+    );
 
     // The extension client actually redirects before it can do anytyhing with this meta
     const txMeta = handleSolanaTxResponse(

packages/bridge-status-controller/src/types.ts

@@ -24,7 +24,6 @@ import type {
   NetworkControllerGetNetworkClientByIdAction,
   NetworkControllerGetStateAction,
 } from '@metamask/network-controller';
-import type { HandleSnapRequest } from '@metamask/snaps-controllers';
 import type {
   TransactionControllerGetStateAction,
   TransactionControllerTransactionConfirmedEvent,
@@ -330,7 +329,6 @@ type AllowedActions =
   | NetworkControllerGetStateAction
   | NetworkControllerGetNetworkClientByIdAction
   | AccountsControllerGetSelectedMultichainAccountAction
-  | HandleSnapRequest
   | TransactionControllerGetStateAction
   | BridgeControllerAction<BridgeBackgroundAction.GET_BRIDGE_ERC20_ALLOWANCE>
   | BridgeControllerAction<BridgeBackgroundAction.TRACK_METAMETRICS_EVENT>

packages/bridge-status-controller/src/utils/transaction.ts

@@ -7,6 +7,7 @@ import {
   type QuoteMetadata,
   type QuoteResponse,
 } from '@metamask/bridge-controller';
+import type { SnapKeyring } from '@metamask/eth-snap-keyring';
 import { SolScope } from '@metamask/keyring-api';
 import {
   TransactionStatus,
@@ -123,34 +124,25 @@ export const handleLineaDelay = async (
   }
 };
 
+// NOTE: We cannot use `KeyringRequest` as the `SnapKeyring.submitRequest` method
+// uses a different shape, so we just alias this instead.
+type SnapKeyringRequest = Parameters<SnapKeyring['submitRequest']>[0];
+
 export const getKeyringRequest = (
   quoteResponse: Omit<QuoteResponse<string>, 'approval'> & QuoteMetadata,
   selectedAccount: AccountsControllerState['internalAccounts']['accounts'][string],
-) => {
-  const keyringReqId = uuid();
-  const snapRequestId = uuid();
-
+): SnapKeyringRequest => {
   return {
+    // An origin is now required on the inner-request. We use `'metamask'` here since
+    // those requests will be dispatched by MetaMask itself (internal use only).
     origin: 'metamask',
-    snapId: selectedAccount.metadata.snap?.id as never,
-    handler: 'onKeyringRequest' as never,
-    request: {
-      id: keyringReqId,
-      jsonrpc: '2.0',
-      method: 'keyring_submitRequest',
-      params: {
-        request: {
-          params: {
-            account: { address: selectedAccount.address },
-            transaction: quoteResponse.trade,
-            scope: SolScope.Mainnet,
-          },
-          method: 'signAndSendTransaction',
-        },
-        id: snapRequestId,
-        account: selectedAccount.id,
-        scope: SolScope.Mainnet,
-      },
+    params: {
+      account: { address: selectedAccount.address },
+      transaction: quoteResponse.trade,
+      scope: SolScope.Mainnet,
     },
+    method: 'signAndSendTransaction',
+    account: selectedAccount.id,
+    scope: SolScope.Mainnet,
   };
 };

yarn.lock

@@ -2806,12 +2806,12 @@ __metadata:
     "@metamask/base-controller": "npm:^8.0.1"
     "@metamask/bridge-controller": "npm:^32.1.1"
     "@metamask/controller-utils": "npm:^11.10.0"
+    "@metamask/eth-snap-keyring": "npm:^13.0.0"
     "@metamask/gas-fee-controller": "npm:^23.0.0"
     "@metamask/keyring-api": "npm:^18.0.0"
     "@metamask/multichain-transactions-controller": "npm:^2.0.0"
     "@metamask/network-controller": "npm:^23.6.0"
     "@metamask/polling-controller": "npm:^13.0.0"
-    "@metamask/snaps-controllers": "npm:^12.3.1"
     "@metamask/superstruct": "npm:^3.1.0"
     "@metamask/transaction-controller": "npm:^57.2.0"
     "@metamask/user-operation-controller": "npm:^36.0.0"
@@ -2834,7 +2834,6 @@ __metadata:
     "@metamask/gas-fee-controller": ^23.0.0
     "@metamask/multichain-transactions-controller": ^2.0.0
     "@metamask/network-controller": ^23.0.0
-    "@metamask/snaps-controllers": ^12.0.0
     "@metamask/transaction-controller": ^57.0.0
   languageName: unknown
   linkType: soft