Skip to content

Commit 8512029

Browse files
ccharlyccharly
committed
fix(bridge-status-controller): use SnapKeyring.submitRequest to dispatch Snap keyring requests
1 parent 70455d1 commit 8512029

File tree

8 files changed

+71
-61
lines changed

8 files changed

+71
-61
lines changed

‎packages/bridge-status-controller/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- **BREAKING:** Now requires a new `withSnapKeyringFn` parameter when constructing the controller.
13+
- This will be used to access the `SnapKeyring` instance and submit keyring request to a Snap.
14+
- **BREAKING:** No longer depend on `@metamask/snaps-controllers` dependency.
15+
- The Snap request handling is now delegated to the `SnapKeyring` via the new `withSnapKeyringFn` callback.
16+
1017
## [29.1.0]
1118

1219
### Added

‎packages/bridge-status-controller/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@
6161
"@metamask/accounts-controller": "^30.0.0",
6262
"@metamask/auto-changelog": "^3.4.4",
6363
"@metamask/bridge-controller": "^32.1.0",
64+
"@metamask/eth-snap-keyring": "^13.0.0",
6465
"@metamask/gas-fee-controller": "^23.0.0",
6566
"@metamask/multichain-transactions-controller": "^2.0.0",
6667
"@metamask/network-controller": "^23.5.1",
67-
"@metamask/snaps-controllers": "^12.3.1",
6868
"@metamask/transaction-controller": "^57.0.0",
6969
"@types/jest": "^27.4.1",
7070
"deepmerge": "^4.2.2",
@@ -83,7 +83,6 @@
8383
"@metamask/gas-fee-controller": "^23.0.0",
8484
"@metamask/multichain-transactions-controller": "^2.0.0",
8585
"@metamask/network-controller": "^23.0.0",
86-
"@metamask/snaps-controllers": "^12.0.0",
8786
"@metamask/transaction-controller": "^57.0.0"
8887
},
8988
"engines": {

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2854,34 +2854,6 @@ Array [
28542854
Array [
28552855
"AccountsController:getSelectedMultichainAccount",
28562856
],
2857-
Array [
2858-
"SnapController:handleRequest",
2859-
Object {
2860-
"handler": "onKeyringRequest",
2861-
"origin": "metamask",
2862-
"request": Object {
2863-
"id": "test-uuid-1234",
2864-
"jsonrpc": "2.0",
2865-
"method": "keyring_submitRequest",
2866-
"params": Object {
2867-
"account": undefined,
2868-
"id": "test-uuid-1234",
2869-
"request": Object {
2870-
"method": "signAndSendTransaction",
2871-
"params": Object {
2872-
"account": Object {
2873-
"address": "0x123...",
2874-
},
2875-
"scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp",
2876-
"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=",
2877-
},
2878-
},
2879-
"scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp",
2880-
},
2881-
},
2882-
"snapId": "test-snap",
2883-
},
2884-
],
28852857
Array [
28862858
"BridgeController:trackUnifiedSwapBridgeEvent",
28872859
"Unified SwapBridge Snap Confirmation Page Viewed",

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,7 @@ const executePollingWithPendingStatus = async () => {
540540
messenger: getMessengerMock(),
541541
clientId: BridgeClientId.EXTENSION,
542542
fetchFn: jest.fn(),
543+
withSnapKeyringFn: jest.fn(),
543544
addTransactionFn: jest.fn(),
544545
estimateGasFeeFn: jest.fn(),
545546
addUserOperationFromTransactionFn: jest.fn(),
@@ -566,6 +567,10 @@ const executePollingWithPendingStatus = async () => {
566567

567568
// Define mocks at the top level
568569
const mockFetchFn = jest.fn();
570+
const mockWithSnapKeyringFn = jest.fn();
571+
const mockKeyring = {
572+
submitRequest: jest.fn(),
573+
};
569574
const mockMessengerCall = jest.fn();
570575
const mockSelectedAccount = {
571576
id: 'test-account-id',
@@ -593,6 +598,7 @@ const getController = (call: jest.Mock, traceFn?: jest.Mock) => {
593598
} as never,
594599
clientId: BridgeClientId.EXTENSION,
595600
fetchFn: mockFetchFn,
601+
withSnapKeyringFn: mockWithSnapKeyringFn,
596602
addTransactionFn,
597603
estimateGasFeeFn,
598604
addUserOperationFromTransactionFn,
@@ -614,6 +620,13 @@ describe('BridgeStatusController', () => {
614620
beforeEach(() => {
615621
jest.clearAllMocks();
616622
jest.clearAllTimers();
623+
624+
// We re-wire up the mocked Snap keyring callback, so we can just
625+
// use `mockSnapKeyring.submitRequest` when we need to mock Snap
626+
// keyring requests.
627+
mockWithSnapKeyringFn.mockImplementation(async (callback) => {
628+
return await callback({ keyring: mockKeyring });
629+
});
617630
});
618631

619632
describe('constructor', () => {
@@ -622,6 +635,7 @@ describe('BridgeStatusController', () => {
622635
messenger: getMessengerMock(),
623636
clientId: BridgeClientId.EXTENSION,
624637
fetchFn: jest.fn(),
638+
withSnapKeyringFn: jest.fn(),
625639
addTransactionFn: jest.fn(),
626640
estimateGasFeeFn: jest.fn(),
627641
addUserOperationFromTransactionFn: jest.fn(),
@@ -635,6 +649,7 @@ describe('BridgeStatusController', () => {
635649
messenger: getMessengerMock(),
636650
clientId: BridgeClientId.EXTENSION,
637651
fetchFn: jest.fn(),
652+
withSnapKeyringFn: jest.fn(),
638653
addTransactionFn: jest.fn(),
639654
estimateGasFeeFn: jest.fn(),
640655
addUserOperationFromTransactionFn: jest.fn(),
@@ -672,6 +687,7 @@ describe('BridgeStatusController', () => {
672687
},
673688
clientId: BridgeClientId.EXTENSION,
674689
fetchFn: jest.fn(),
690+
withSnapKeyringFn: jest.fn(),
675691
addTransactionFn: jest.fn(),
676692
estimateGasFeeFn: jest.fn(),
677693
});
@@ -694,6 +710,7 @@ describe('BridgeStatusController', () => {
694710
messenger: getMessengerMock(),
695711
clientId: BridgeClientId.EXTENSION,
696712
fetchFn: jest.fn(),
713+
withSnapKeyringFn: jest.fn(),
697714
addTransactionFn: jest.fn(),
698715
estimateGasFeeFn: jest.fn(),
699716
addUserOperationFromTransactionFn: jest.fn(),
@@ -732,6 +749,7 @@ describe('BridgeStatusController', () => {
732749
messenger: messengerMock,
733750
clientId: BridgeClientId.EXTENSION,
734751
fetchFn: jest.fn(),
752+
withSnapKeyringFn: jest.fn(),
735753
addTransactionFn: jest.fn(),
736754
estimateGasFeeFn: jest.fn(),
737755
addUserOperationFromTransactionFn: jest.fn(),
@@ -813,6 +831,7 @@ describe('BridgeStatusController', () => {
813831
messenger: messengerMock,
814832
clientId: BridgeClientId.EXTENSION,
815833
fetchFn: jest.fn(),
834+
withSnapKeyringFn: jest.fn(),
816835
addTransactionFn: jest.fn(),
817836
estimateGasFeeFn: jest.fn(),
818837
addUserOperationFromTransactionFn: jest.fn(),
@@ -852,6 +871,7 @@ describe('BridgeStatusController', () => {
852871
messenger: messengerMock,
853872
clientId: BridgeClientId.EXTENSION,
854873
fetchFn: jest.fn(),
874+
withSnapKeyringFn: jest.fn(),
855875
addTransactionFn: jest.fn(),
856876
estimateGasFeeFn: jest.fn(),
857877
addUserOperationFromTransactionFn: jest.fn(),
@@ -893,6 +913,7 @@ describe('BridgeStatusController', () => {
893913
messenger: messengerMock,
894914
clientId: BridgeClientId.EXTENSION,
895915
fetchFn: jest.fn(),
916+
withSnapKeyringFn: jest.fn(),
896917
addTransactionFn: jest.fn(),
897918
estimateGasFeeFn: jest.fn(),
898919
addUserOperationFromTransactionFn: jest.fn(),
@@ -956,6 +977,7 @@ describe('BridgeStatusController', () => {
956977
messenger: messengerMock,
957978
clientId: BridgeClientId.EXTENSION,
958979
fetchFn: jest.fn(),
980+
withSnapKeyringFn: jest.fn(),
959981
addTransactionFn: jest.fn(),
960982
estimateGasFeeFn: jest.fn(),
961983
addUserOperationFromTransactionFn: jest.fn(),
@@ -1044,6 +1066,7 @@ describe('BridgeStatusController', () => {
10441066
messenger: messengerMock,
10451067
clientId: BridgeClientId.EXTENSION,
10461068
fetchFn: jest.fn(),
1069+
withSnapKeyringFn: jest.fn(),
10471070
addTransactionFn: jest.fn(),
10481071
estimateGasFeeFn: jest.fn(),
10491072
addUserOperationFromTransactionFn: jest.fn(),
@@ -1131,6 +1154,7 @@ describe('BridgeStatusController', () => {
11311154
messenger: messengerMock,
11321155
clientId: BridgeClientId.EXTENSION,
11331156
fetchFn: jest.fn(),
1157+
withSnapKeyringFn: jest.fn(),
11341158
addTransactionFn: jest.fn(),
11351159
estimateGasFeeFn: jest.fn(),
11361160
addUserOperationFromTransactionFn: jest.fn(),
@@ -1232,6 +1256,7 @@ describe('BridgeStatusController', () => {
12321256
messenger: messengerMock,
12331257
clientId: BridgeClientId.EXTENSION,
12341258
fetchFn: jest.fn(),
1259+
withSnapKeyringFn: jest.fn(),
12351260
addTransactionFn: jest.fn(),
12361261
estimateGasFeeFn: jest.fn(),
12371262
addUserOperationFromTransactionFn: jest.fn(),
@@ -1430,8 +1455,9 @@ describe('BridgeStatusController', () => {
14301455
});
14311456

14321457
it('should successfully submit a Solana transaction', async () => {
1458+
mockKeyring.submitRequest.mockResolvedValueOnce('signature');
1459+
14331460
mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
1434-
mockMessengerCall.mockResolvedValueOnce('signature');
14351461

14361462
mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
14371463
mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
@@ -1486,7 +1512,7 @@ describe('BridgeStatusController', () => {
14861512

14871513
it('should handle snap controller errors', async () => {
14881514
mockMessengerCall.mockReturnValueOnce(mockSolanaAccount);
1489-
mockMessengerCall.mockRejectedValueOnce(new Error('Snap error'));
1515+
mockKeyring.submitRequest.mockRejectedValueOnce(new Error('Snap error'));
14901516

14911517
const { controller, startPollingForBridgeTxStatusSpy } =
14921518
getController(mockMessengerCall);
@@ -2244,6 +2270,7 @@ describe('BridgeStatusController', () => {
22442270
messenger: mockBridgeStatusMessenger,
22452271
clientId: BridgeClientId.EXTENSION,
22462272
fetchFn: jest.fn(),
2273+
withSnapKeyringFn: jest.fn(),
22472274
addTransactionFn: jest.fn(),
22482275
estimateGasFeeFn: jest.fn(),
22492276
addUserOperationFromTransactionFn: jest.fn(),

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from '@metamask/bridge-controller';
1919
import type { TraceCallback } from '@metamask/controller-utils';
2020
import { toHex } from '@metamask/controller-utils';
21+
import type { SnapKeyring } from '@metamask/eth-snap-keyring';
2122
import { EthAccountType } from '@metamask/keyring-api';
2223
import { StaticIntervalPollingController } from '@metamask/polling-controller';
2324
import type {
@@ -79,6 +80,11 @@ const metadata: StateMetadata<BridgeStatusControllerState> = {
7980
},
8081
};
8182

83+
// Expecting a bound function that calls KeyringController.withKeyring selecting the Snap keyring
84+
type WithSnapKeyringFn = <ReturnType>(
85+
operation: ({ keyring }: { keyring: SnapKeyring }) => Promise<ReturnType>,
86+
) => Promise<ReturnType>;
87+
8288
/** The input to start polling for the {@link BridgeStatusController} */
8389
type BridgeStatusPollingInput = FetchBridgeTxStatusArgs;
8490

@@ -105,6 +111,8 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
105111

106112
readonly #estimateGasFeeFn: typeof TransactionController.prototype.estimateGasFee;
107113

114+
readonly #withSnapKeyringFn: WithSnapKeyringFn;
115+
108116
readonly #addUserOperationFromTransactionFn?: typeof UserOperationController.prototype.addUserOperationFromTransaction;
109117

110118
readonly #trace: TraceCallback;
@@ -116,6 +124,7 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
116124
fetchFn,
117125
addTransactionFn,
118126
addUserOperationFromTransactionFn,
127+
withSnapKeyringFn,
119128
estimateGasFeeFn,
120129
config,
121130
traceFn,
@@ -126,6 +135,7 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
126135
fetchFn: FetchFunction;
127136
addTransactionFn: typeof TransactionController.prototype.addTransaction;
128137
estimateGasFeeFn: typeof TransactionController.prototype.estimateGasFee;
138+
withSnapKeyringFn: WithSnapKeyringFn;
129139
addUserOperationFromTransactionFn?: typeof UserOperationController.prototype.addUserOperationFromTransaction;
130140
config?: {
131141
customBridgeApiBaseUrl?: string;
@@ -147,6 +157,7 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
147157
this.#fetchFn = fetchFn;
148158
this.#addTransactionFn = addTransactionFn;
149159
this.#addUserOperationFromTransactionFn = addUserOperationFromTransactionFn;
160+
this.#withSnapKeyringFn = withSnapKeyringFn;
150161
this.#estimateGasFeeFn = estimateGasFeeFn;
151162
this.#config = {
152163
customBridgeApiBaseUrl:
@@ -560,10 +571,13 @@ export class BridgeStatusController extends StaticIntervalPollingController<Brid
560571
);
561572
}
562573
const keyringRequest = getKeyringRequest(quoteResponse, selectedAccount);
563-
const keyringResponse = (await this.messagingSystem.call(
564-
'SnapController:handleRequest',
565-
keyringRequest,
566-
)) as string | { result: Record<string, string> };
574+
const keyringResponse = await this.#withSnapKeyringFn(
575+
async ({ keyring }) => {
576+
return (await keyring.submitRequest(keyringRequest)) as
577+
| string
578+
| { result: Record<string, string> };
579+
},
580+
);
567581

568582
// The extension client actually redirects before it can do anytyhing with this meta
569583
const txMeta = handleSolanaTxResponse(

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import type {
2424
NetworkControllerGetNetworkClientByIdAction,
2525
NetworkControllerGetStateAction,
2626
} from '@metamask/network-controller';
27-
import type { HandleSnapRequest } from '@metamask/snaps-controllers';
2827
import type {
2928
TransactionControllerGetStateAction,
3029
TransactionControllerTransactionConfirmedEvent,
@@ -330,7 +329,6 @@ type AllowedActions =
330329
| NetworkControllerGetStateAction
331330
| NetworkControllerGetNetworkClientByIdAction
332331
| AccountsControllerGetSelectedMultichainAccountAction
333-
| HandleSnapRequest
334332
| TransactionControllerGetStateAction
335333
| BridgeControllerAction<BridgeBackgroundAction.GET_BRIDGE_ERC20_ALLOWANCE>
336334
| BridgeControllerAction<BridgeBackgroundAction.TRACK_METAMETRICS_EVENT>

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

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type QuoteMetadata,
88
type QuoteResponse,
99
} from '@metamask/bridge-controller';
10+
import type { SnapKeyring } from '@metamask/eth-snap-keyring';
1011
import { SolScope } from '@metamask/keyring-api';
1112
import {
1213
TransactionStatus,
@@ -123,34 +124,25 @@ export const handleLineaDelay = async (
123124
}
124125
};
125126

127+
// NOTE: We cannot use `KeyringRequest` as the `SnapKeyring.submitRequest` method
128+
// uses a different shape, so we just alias this instead.
129+
type SnapKeyringRequest = Parameters<SnapKeyring['submitRequest']>[0];
130+
126131
export const getKeyringRequest = (
127132
quoteResponse: Omit<QuoteResponse<string>, 'approval'> & QuoteMetadata,
128133
selectedAccount: AccountsControllerState['internalAccounts']['accounts'][string],
129-
) => {
130-
const keyringReqId = uuid();
131-
const snapRequestId = uuid();
132-
134+
): SnapKeyringRequest => {
133135
return {
136+
// An origin is now required on the inner-request. We use `'metamask'` here since
137+
// those requests will be dispatched by MetaMask itself (internal use only).
134138
origin: 'metamask',
135-
snapId: selectedAccount.metadata.snap?.id as never,
136-
handler: 'onKeyringRequest' as never,
137-
request: {
138-
id: keyringReqId,
139-
jsonrpc: '2.0',
140-
method: 'keyring_submitRequest',
141-
params: {
142-
request: {
143-
params: {
144-
account: { address: selectedAccount.address },
145-
transaction: quoteResponse.trade,
146-
scope: SolScope.Mainnet,
147-
},
148-
method: 'signAndSendTransaction',
149-
},
150-
id: snapRequestId,
151-
account: selectedAccount.id,
152-
scope: SolScope.Mainnet,
153-
},
139+
params: {
140+
account: { address: selectedAccount.address },
141+
transaction: quoteResponse.trade,
142+
scope: SolScope.Mainnet,
154143
},
144+
method: 'signAndSendTransaction',
145+
account: selectedAccount.id,
146+
scope: SolScope.Mainnet,
155147
};
156148
};

‎yarn.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2775,6 +2775,7 @@ __metadata:
27752775
"@metamask/base-controller": "npm:^8.0.1"
27762776
"@metamask/bridge-controller": "npm:^32.1.0"
27772777
"@metamask/controller-utils": "npm:^11.9.0"
2778+
"@metamask/eth-snap-keyring": "npm:^13.0.0"
27782779
"@metamask/gas-fee-controller": "npm:^23.0.0"
27792780
"@metamask/keyring-api": "npm:^18.0.0"
27802781
"@metamask/multichain-transactions-controller": "npm:^2.0.0"

0 commit comments

Comments
 (0)