Skip to content

Commit 1f14fa8

Browse files
fix: Wait for unlock before handling manageAccounts requests (#3686)
`getSnapKeyring` fails when the client is locked since we cannot access the `KeyringController`. As with every other Snaps API, we want to wait for the client to be unlocked before allowing use of functionality that accesses the vault. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds `getUnlockPromise` to `manageAccounts` and awaits unlock before routing requests to the snap keyring, with tests updated accordingly. > > - **RPC method `snap_manageAccounts`**: > - Add `getUnlockPromise` to method hooks and await `getUnlockPromise(true)` before calling `getSnapKeyring` and routing to `handleKeyringSnapMessage` in `manageAccounts.ts`. > - Update `specificationBuilder` and `manageAccountsBuilder.methodHooks` to include `getUnlockPromise`. > - **Tests**: > - Update `manageAccounts.test.ts` to include `getUnlockPromise` in hooks and mocks; maintain validation and routing assertions. > - **Misc**: > - Slightly adjust coverage threshold for lines in `jest.config.js`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ba55813. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ea29b05 commit 1f14fa8

File tree

3 files changed

+22
-1
lines changed

3 files changed

+22
-1
lines changed

packages/snaps-rpc-methods/jest.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module.exports = deepmerge(baseConfig, {
1212
global: {
1313
branches: 95.68,
1414
functions: 98.75,
15-
lines: 98.98,
15+
lines: 98.99,
1616
statements: 98.69,
1717
},
1818
},

packages/snaps-rpc-methods/src/restricted/manageAccounts.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('specification', () => {
2828
it('builds specification', () => {
2929
const methodHooks = {
3030
getSnapKeyring: jest.fn(),
31+
getUnlockPromise: jest.fn(),
3132
};
3233

3334
expect(
@@ -61,6 +62,7 @@ describe('builder', () => {
6162
manageAccountsBuilder.specificationBuilder({
6263
methodHooks: {
6364
getSnapKeyring: jest.fn(),
65+
getUnlockPromise: jest.fn(),
6466
},
6567
}),
6668
).toMatchObject({
@@ -83,9 +85,11 @@ describe('manageAccountsImplementation', () => {
8385
it('should throw params are not set', async () => {
8486
const mockKeyring = new SnapKeyringMock();
8587
const getSnapKeyring = jest.fn().mockResolvedValue(mockKeyring);
88+
const getUnlockPromise = jest.fn();
8689

8790
const manageAccounts = manageAccountsImplementation({
8891
getSnapKeyring,
92+
getUnlockPromise,
8993
});
9094

9195
await expect(
@@ -105,9 +109,11 @@ describe('manageAccountsImplementation', () => {
105109
it('should throw params accountId is not set', async () => {
106110
const mockKeyring = new SnapKeyringMock();
107111
const getSnapKeyring = jest.fn().mockResolvedValue(mockKeyring);
112+
const getUnlockPromise = jest.fn();
108113

109114
const manageAccounts = manageAccountsImplementation({
110115
getSnapKeyring,
116+
getUnlockPromise,
111117
});
112118

113119
await expect(
@@ -127,13 +133,15 @@ describe('manageAccountsImplementation', () => {
127133
it('should route request to snap keyring', async () => {
128134
const mockKeyring = new SnapKeyringMock();
129135
const getSnapKeyring = jest.fn().mockResolvedValue(mockKeyring);
136+
const getUnlockPromise = jest.fn();
130137

131138
const createAccountSpy = jest
132139
.spyOn(mockKeyring, 'handleKeyringSnapMessage')
133140
.mockResolvedValue(true);
134141

135142
const manageAccounts = manageAccountsImplementation({
136143
getSnapKeyring,
144+
getUnlockPromise,
137145
});
138146

139147
const requestResponse = await manageAccounts({

packages/snaps-rpc-methods/src/restricted/manageAccounts.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ export type ManageAccountsMethodHooks = {
4444
message: Message,
4545
) => Promise<Json>;
4646
}>;
47+
48+
/**
49+
* Wait for the client to be unlocked.
50+
*
51+
* @returns A promise that resolves once the client is unlocked.
52+
*/
53+
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
4754
};
4855

4956
type ManageAccountsSpecificationBuilderOptions = {
@@ -89,12 +96,14 @@ export const specificationBuilder: PermissionSpecificationBuilder<
8996
*
9097
* @param hooks - The RPC method hooks.
9198
* @param hooks.getSnapKeyring - A function to get the snap keyring.
99+
* @param hooks.getUnlockPromise - The function to get the unlock promise.
92100
* @returns The method implementation which either returns `null` for a
93101
* successful state update/deletion or returns the decrypted state.
94102
* @throws If the params are invalid.
95103
*/
96104
export function manageAccountsImplementation({
97105
getSnapKeyring,
106+
getUnlockPromise,
98107
}: ManageAccountsMethodHooks) {
99108
return async function manageAccounts(
100109
options: RestrictedMethodOptions<ManageAccountsParams>,
@@ -105,6 +114,9 @@ export function manageAccountsImplementation({
105114
} = options;
106115

107116
assert(params, SnapMessageStruct);
117+
118+
await getUnlockPromise(true);
119+
108120
const keyring = await getSnapKeyring(origin);
109121
return await keyring.handleKeyringSnapMessage(origin, params);
110122
};
@@ -115,5 +127,6 @@ export const manageAccountsBuilder = Object.freeze({
115127
specificationBuilder,
116128
methodHooks: {
117129
getSnapKeyring: true,
130+
getUnlockPromise: true,
118131
},
119132
} as const);

0 commit comments

Comments
 (0)