Skip to content

feat: non-user-facing getUnlockPromise optional skipCount param #4006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maxime-oe wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: non-user-facing getUnlockPromise optional skipCount param #4006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions packages/snaps-rpc-methods/src/permitted/getState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { hasProperty, isObject } from '@metamask/utils';

import { manageStateBuilder } from '../restricted/manageState';
import type {
GetUnlockPromise,
JsonRpcRequestWithOrigin,
SnapControllerGetSnapStateAction,
} from '../types';
Expand All @@ -30,12 +31,7 @@ const hookNames: MethodHooksObject<GetStateMethodHooks> = {
};

export type GetStateMethodHooks = {
/**
* Wait for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;
};

export type GetStateMethodActions =
Expand Down Expand Up @@ -134,7 +130,7 @@ async function getStateImplementation(
const { key, encrypted = true } = validatedParams;

if (encrypted) {
await getUnlockPromise(true);
await getUnlockPromise(true, true);
}

const state = await messenger.call(
Expand Down
2 changes: 1 addition & 1 deletion packages/snaps-rpc-methods/src/permitted/listEntropySources.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('snap_listEntropySources', () => {
method: 'snap_listEntropySources',
});

expect(getUnlockPromise).toHaveBeenCalledWith(true);
expect(getUnlockPromise).toHaveBeenCalledWith(true, true);
expect(response).toStrictEqual({
jsonrpc: '2.0',
id: 1,
Expand Down
10 changes: 3 additions & 7 deletions packages/snaps-rpc-methods/src/permitted/listEntropySources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { getBip32PublicKeyBuilder } from '../restricted/getBip32PublicKey';
import { getBip44EntropyBuilder } from '../restricted/getBip44Entropy';
import { getEntropyBuilder } from '../restricted/getEntropy';
import type {
GetUnlockPromise,
JsonRpcRequestWithOrigin,
KeyringControllerGetStateAction,
} from '../types';
Expand All @@ -43,12 +44,7 @@ const hookNames: MethodHooksObject<ListEntropySourcesMethodHooks> = {
};

export type ListEntropySourcesMethodHooks = {
/**
* Wait for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;
};

export type ListEntropySourcesMethodActions =
Expand Down Expand Up @@ -139,7 +135,7 @@ async function listEntropySourcesImplementation(
return end(providerErrors.unauthorized());
}

await getUnlockPromise(true);
await getUnlockPromise(true, true);

const { keyrings } = messenger.call('KeyringController:getState');

Expand Down
10 changes: 3 additions & 7 deletions packages/snaps-rpc-methods/src/permitted/setState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
STORAGE_SIZE_LIMIT,
} from '../restricted/manageState';
import type {
GetUnlockPromise,
JsonRpcRequestWithOrigin,
SnapControllerGetSnapAction,
SnapControllerGetSnapStateAction,
Expand All @@ -41,12 +42,7 @@ const hookNames: MethodHooksObject<SetStateMethodHooks> = {
};

export type SetStateMethodHooks = {
/**
* Wait for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;
};

export type SetStateMethodActions =
Expand Down Expand Up @@ -192,7 +188,7 @@ async function setStateImplementation(
}

if (encrypted) {
await getUnlockPromise(true);
await getUnlockPromise(true, true);
}

const snapId = origin as SnapId;
Expand Down
12 changes: 5 additions & 7 deletions packages/snaps-rpc-methods/src/restricted/getBip32Entropy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import { SnapCaveatType } from '@metamask/snaps-utils';
import type { NonEmptyArray } from '@metamask/utils';
import { assert } from '@metamask/utils';

import type { KeyringControllerWithKeyringAction } from '../types';
import type {
GetUnlockPromise,
KeyringControllerWithKeyringAction,
} from '../types';
import type { MethodHooksObject } from '../utils';
import {
getMnemonic,
Expand All @@ -29,12 +32,7 @@ import {
const targetName = 'snap_getBip32Entropy';

export type GetBip32EntropyMethodHooks = {
/**
* Waits for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
Copy link
Copy Markdown

@cursor cursor Bot May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent skipCount argument across getUnlockPromise call sites

Medium Severity

This PR updates five call sites to pass getUnlockPromise(true, true) with the new skipCount argument, but four other call sites — in getBip32Entropy.ts, getBip32PublicKey.ts, getBip44Entropy.ts, and manageState.ts — still call getUnlockPromise(true) without it. The types for all these files were updated to GetUnlockPromise in this same PR, suggesting the call sites were intended to be updated as well. This inconsistency means these four methods won't skip the count while all others will.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 07d4fb9. Configure here.

Copy link
Copy Markdown
Author

@maxime-oe maxime-oe May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this PR as experiment, I apply the new skipCount only when I see the counter being incremented for operations that are getting "spammed" and make the counter increment for no user-facing reason. Doing it this way to minimize the changes.

getUnlockPromise: GetUnlockPromise;

/**
* Get the cryptographic functions to use for the client. This may return an
Expand Down
12 changes: 5 additions & 7 deletions packages/snaps-rpc-methods/src/restricted/getBip32PublicKey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import { boolean, object, optional, string } from '@metamask/superstruct';
import type { NonEmptyArray } from '@metamask/utils';
import { assertStruct } from '@metamask/utils';

import type { KeyringControllerWithKeyringAction } from '../types';
import type {
GetUnlockPromise,
KeyringControllerWithKeyringAction,
} from '../types';
import type { MethodHooksObject } from '../utils';
import {
getMnemonic,
Expand All @@ -35,12 +38,7 @@ import {
const targetName = 'snap_getBip32PublicKey';

export type GetBip32PublicKeyMethodHooks = {
/**
* Waits for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;

/**
* Get the cryptographic functions to use for the client. This may return an
Expand Down
12 changes: 5 additions & 7 deletions packages/snaps-rpc-methods/src/restricted/getBip44Entropy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,17 @@ import type {
import { SnapCaveatType } from '@metamask/snaps-utils';
import type { NonEmptyArray } from '@metamask/utils';

import type { KeyringControllerWithKeyringAction } from '../types';
import type {
GetUnlockPromise,
KeyringControllerWithKeyringAction,
} from '../types';
import type { MethodHooksObject } from '../utils';
import { getMnemonicSeed, getValueFromEntropySource } from '../utils';

const targetName = 'snap_getBip44Entropy';

export type GetBip44EntropyMethodHooks = {
/**
* Waits for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;

/**
* Get the cryptographic functions to use for the client. This may return an
Expand Down
14 changes: 6 additions & 8 deletions packages/snaps-rpc-methods/src/restricted/getEntropy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import { literal, object, optional, string } from '@metamask/superstruct';
import type { NonEmptyArray } from '@metamask/utils';
import { assertStruct } from '@metamask/utils';

import type { KeyringControllerWithKeyringAction } from '../types';
import type {
GetUnlockPromise,
KeyringControllerWithKeyringAction,
} from '../types';
import type { MethodHooksObject } from '../utils';
import {
deriveEntropyFromSeed,
Expand Down Expand Up @@ -119,12 +122,7 @@ export const getEntropyBuilder = Object.freeze({
} as const);

export type GetEntropyHooks = {
/**
* Waits for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;

/**
* Get the cryptographic functions to use for the client. This may return an
Expand Down Expand Up @@ -169,7 +167,7 @@ function getEntropyImplementation({
rpcErrors.invalidParams,
);

await getUnlockPromise(true);
await getUnlockPromise(true, true);

const seed = await getValueFromEntropySource(
getMnemonicSeed.bind(null, messenger),
Expand Down
11 changes: 4 additions & 7 deletions packages/snaps-rpc-methods/src/restricted/manageAccounts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
import type { Json, NonEmptyArray } from '@metamask/utils';
import { JsonStruct } from '@metamask/utils';

import type { GetUnlockPromise } from '../types';

const SnapMessageStruct = union([
object({
method: string(),
Expand All @@ -45,12 +47,7 @@ export type ManageAccountsMethodHooks = {
) => Promise<Json>;
}>;

/**
* Wait for the client to be unlocked.
*
* @returns A promise that resolves once the client is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;
};

type ManageAccountsSpecificationBuilderOptions = {
Expand Down Expand Up @@ -115,7 +112,7 @@ export function manageAccountsImplementation({

assert(params, SnapMessageStruct);

await getUnlockPromise(true);
await getUnlockPromise(true, true);

const keyring = await getSnapKeyring(origin);
return await keyring.handleKeyringSnapMessage(origin, params);
Expand Down
8 changes: 2 additions & 6 deletions packages/snaps-rpc-methods/src/restricted/manageState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { NonEmptyArray } from '@metamask/utils';
import { isObject, isValidJson } from '@metamask/utils';

import type {
GetUnlockPromise,
SnapControllerClearSnapStateAction,
SnapControllerGetSnapAction,
SnapControllerGetSnapStateAction,
Expand All @@ -31,12 +32,7 @@ export const STATE_ENCRYPTION_SALT = 'snap_manageState encryption';
const methodName = 'snap_manageState';

export type ManageStateMethodHooks = {
/**
* Waits for the extension to be unlocked.
*
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: GetUnlockPromise;
};

export type ManageStateMessengerActions =
Expand Down
5 changes: 5 additions & 0 deletions packages/snaps-rpc-methods/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,8 @@ export type WebSocketServiceGetAllAction = {
type: `WebSocketService:getAll`;
handler: (snapId: string) => GetWebSocketsResult;
};

export type GetUnlockPromise = (
shouldShowUnlockRequest: boolean,
skipCount?: boolean,
) => Promise<void>;
5 changes: 4 additions & 1 deletion packages/snaps-simulation/src/simulation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ export type PermittedMiddlewareHooks = {
* @param shouldShowUnlockRequest - Whether to show the unlock request.
* @returns A promise that resolves once the extension is unlocked.
*/
getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise<void>;
getUnlockPromise: (
shouldShowUnlockRequest: boolean,
skipCount?: boolean,
) => Promise<void>;

/**
* A hook that returns whether the client is active or not.
Expand Down
Loading