refactor(snaps-rpc-methods)!: Use messenger for restricted method implementations by FrederikBolding · Pull Request #3968 · MetaMask/snaps

FrederikBolding · 2026-04-24T11:13:08Z

Migrate the restricted method implementations to use the messenger where applicable.

https://consensyssoftware.atlassian.net/browse/WPC-995

Note

Medium Risk
Refactors multiple restricted RPC methods to use messenger-based controller actions instead of direct hook functions, which could subtly change runtime behavior and error handling across permissions, dialogs/notifications, and keyring-derived entropy. Broad test updates mitigate risk but the surface area is large and touches keyring/approval/state controllers.

Overview
Migrates restricted RPC methods to messenger-based actions. snap_dialog, snap_notify, wallet_snap, snap_manageState, and entropy/key derivation methods now accept a Messenger and call controller actions (e.g., approvals, snap interface, snap state, keyring access, rate limiting) instead of invoking dedicated hook functions.

Centralizes entropy-source access via new getMnemonic/getMnemonicSeed utilities (backed by KeyringController:withKeyring) and adds shared action type definitions in src/types.ts. Permission specification building is updated to create per-method restricted messengers using declared actionNames, and tests are rewritten accordingly (with new negative cases for invalid keyrings/entropy sources). Coverage thresholds are adjusted slightly, and snaps-simulation removes the obsolete get-mnemonic-seed and interface hook implementations/tests.

^{Reviewed by Cursor Bugbot for commit de25c13. Bugbot is set up for automated code reviews on this repo. Configure here.}

Mrtenz · 2026-05-04T11:46:11Z

+  actionNames: [
+    'ApprovalController:addRequest',
+    'SnapInterfaceController:createInterface',
+    'SnapInterfaceController:getInterface',
+    'SnapInterfaceController:setInterfaceDisplayed',
+  ],


Maybe to match the UI messenger:

Suggested change

actionNames: [

'ApprovalController:addRequest',

'SnapInterfaceController:createInterface',

'SnapInterfaceController:getInterface',

'SnapInterfaceController:setInterfaceDisplayed',

],

capabilities: {

actions: [

'ApprovalController:addRequest',

'SnapInterfaceController:createInterface',

'SnapInterfaceController:getInterface',

'SnapInterfaceController:setInterfaceDisplayed',

],

}

createRestrictedMethodMessenger uses the actionNames naming, that's why I chose to name it like that for the specifications. Do you think it is cleaner to do: actionNames: capabilities.actions?

No strong opinion, actionNames is fine with me.

codecov · 2026-05-05T09:48:45Z

Codecov Report

❌ Patch coverage is 99.07407% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.58%. Comparing base (dcb5bf9) to head (de25c13).

Files with missing lines	Patch %	Lines
...ackages/snaps-rpc-methods/src/restricted/notify.ts	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3968      +/-   ##
==========================================
+ Coverage   98.56%   98.58%   +0.02%     
==========================================
  Files         429      427       -2     
  Lines       12365    12399      +34     
  Branches     1944     1948       +4     
==========================================
+ Hits        12187    12223      +36     
+ Misses        178      176       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

FrederikBolding · 2026-05-06T12:09:31Z

+        const { content, message, title, footerLink } =
+          validatedParams as NotificationArgs & {
+            content?: string;
+            title?: string;
+            footerLink?: { href: string; text: string };
+          };


This mess is copied from the extension, suggestions for improvements welcome 🫠

Mrtenz · 2026-05-06T20:01:47Z

-  getUnlockPromise,
-  getClientCryptography,
-}: GetBip32EntropyMethodHooks) {
+  methodHooks: { getUnlockPromise, getClientCryptography },


Wondering if we should add an action to the Snap controller for this, so we can fully remove the hooks?

The problem with these hooks is that they are client-specific. There is no platform-agnostic handler for getUnlockPromise that can also show the popup when needed on extension.

Ah, good point. That may be something to implement in the wallet service for each client, but right now this is fine.

Mrtenz · 2026-05-06T20:03:13Z

+  | ManageStateMessengerActions
+  | NotifyMessengerActions;
+
+export type RestrictedMethodMessenger = Messenger<


Does this mean we create a messenger that has access to all the actions used by these methods? Should it be scoped to individual methods?

It is narrowed to the individual methods when building the specifications for the PermissionController here: https://github.com/MetaMask/snaps/pull/3968/files#diff-610e0a55f8b1fdc2b25489c558d3338b11c4c91e5dae790f891de1e97d6109bdR84-R88

Mrtenz · 2026-05-06T20:14:19Z

I don't love that we have to copy (and maintain) a bunch of types. We should definitely reconsider a messenger types package for each controller.

Me neither.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit de25c13. Configure here.}

FrederikBolding added the DO-NOT-MERGE label Apr 24, 2026

FrederikBolding force-pushed the fb/restricted-methods-messenger branch from e80f1b8 to bd01104 Compare May 4, 2026 09:43

FrederikBolding changed the title ~~refactor(snaps-rpc-methods): Use messenger for method implementations~~ refactor(snaps-rpc-methods): Use messenger for restricted method implementations May 4, 2026

FrederikBolding removed the DO-NOT-MERGE label May 4, 2026

Mrtenz reviewed May 4, 2026

View reviewed changes

FrederikBolding changed the title ~~refactor(snaps-rpc-methods): Use messenger for restricted method implementations~~ refactor(snaps-rpc-methods)!: Use messenger for restricted method implementations May 5, 2026

FrederikBolding commented May 5, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/permissions.ts

FrederikBolding added 15 commits May 5, 2026 11:34

refactor(snaps-rpc-methods): Use messenger for method implementations

32f9eb9

Fix snaps-simulation

5d2a8e5

Migrate more RPC methods

3b477b9

Use hooks via messenger for simulation framework

4cf3f79

Replace some of mnemonic hook code

46058a4

Move ignore after lint

99a08ff

Add back shared hooks

ce9f550

Revert JSDoc change

ebd0918

Address PR comments and some tests

fe4534b

Fix lint

b831c40

Migrate tests

00f4206

Update coverage

17526bb

Update ignore

516e75c

Add a couple more test cases

0c3f32b

Add cast

5aa93a9

FrederikBolding force-pushed the fb/restricted-methods-messenger branch from 37fbc73 to 5aa93a9 Compare May 5, 2026 09:34

FrederikBolding marked this pull request as ready for review May 5, 2026 09:34

FrederikBolding requested a review from a team as a code owner May 5, 2026 09:34

FrederikBolding requested a review from Mrtenz May 5, 2026 09:34

cursor Bot reviewed May 5, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/restricted/index.ts

FrederikBolding added 2 commits May 6, 2026 14:02

Add missing type

2a748e9

Remove dead code

605c8a4

FrederikBolding commented May 6, 2026

View reviewed changes

cursor Bot reviewed May 6, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/utils.ts

FrederikBolding commented May 6, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/utils.ts Outdated

Fixup HD keyring type

c0bd4e3

cursor Bot reviewed May 6, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/utils.ts

Update coverage

91050f5

Mrtenz reviewed May 6, 2026

View reviewed changes

FrederikBolding added 2 commits May 8, 2026 09:56

Fix type mistakes in duplicated types

9b7c842

Solve more type issues

de25c13

cursor Bot reviewed May 8, 2026

View reviewed changes

Comment thread packages/snaps-rpc-methods/src/restricted/dialog.ts

Mrtenz approved these changes May 8, 2026

View reviewed changes

FrederikBolding added this pull request to the merge queue May 8, 2026

Merged via the queue into main with commit 73b3131 May 8, 2026
129 checks passed

FrederikBolding deleted the fb/restricted-methods-messenger branch May 8, 2026 11:21

Conversation

FrederikBolding commented Apr 24, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FrederikBolding commented Apr 24, 2026 •

edited by cursor Bot

Loading

codecov Bot commented May 5, 2026 •

edited

Loading