feat(wallet): wire `ApprovalController` into default initialization by sirtimid · Pull Request #8953 · MetaMask/core

sirtimid · 2026-06-01T14:45:37Z

Explanation

Wires ApprovalController into @metamask/wallet's default initialization, so consumers can use its messenger actions (ApprovalController:addRequest, startFlow, acceptRequest, …).

@metamask/wallet is the shared controller-integration layer for the extension, mobile, and wallet-cli, so the two construction values that differ between those clients are exposed as injectable options under instanceOptions.approvalController rather than hardcoded. How each environment supplies them:

Option	Extension	Mobile	Default (e.g. wallet-cli)
`showApprovalRequest`	its confirmation-UI opener (`showUserConfirmation`)	no-op — drives approvals through state	no-op `() => undefined`
`typesExcludedFromRateLimiting`	the 6 default types + smart-tx status page + `snap_dialog`	`Transaction` + `WatchAsset` + smart-tx status + `snap_dialog`	6 common EVM approval types — signing, transaction, watch-asset, encryption

Note the two clients don't even share the same string for the smart-transaction status type, so there's no single correct list to hardcode — clients pass their own; consumers that omit it get the baseline above.

ApprovalController owns only its own actions/events, so its messenger is a plain namespaced child with no delegation. The wiring is intentionally thin: ApprovalController is a dependency-free primitive, so there's little to integrate today — its role grows as later controllers (transaction, signature, user-operation, …) delegate to ApprovalController:addRequest/startFlow through the wallet's root messenger, defined here once instead of in each client. Adds the @metamask/approval-controller and @metamask/controller-utils dependencies.

References

How the clients construct ApprovalController today (the source for the table above):

Extension: approval-controller-init.ts · approval-controller-messenger.ts
Mobile: approval-controller-init.ts · approval-controller-messenger.ts

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've communicated my changes to consumers by updating changelogs for packages I've changed
I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

High Risk
Breaking change to shared wallet bootstrap affects all clients’ messenger/controller wiring; mis-migration causes duplicate ApprovalController registration and broken confirmations flows.

Overview
BREAKING: The default @metamask/wallet initialization now builds an ApprovalController and registers ApprovalController:* on the root messenger. Extension/mobile clients that already construct their own must drop that wiring before upgrading or messenger registration will collide.

A new approvalController initialization module wires a namespaced child messenger, forwards persisted state, and exposes optional instanceOptions.approvalController hooks: showApprovalRequest (defaults to a no-op) and typesExcludedFromRateLimiting (defaults to six common EVM approval types). Tests cover defaults, overrides, rate-limit behavior, and root-messenger actions.

Package metadata adds @metamask/approval-controller and @metamask/controller-utils, updates the README dependency graph, assigns CODEOWNERS for the new init folder, and documents the change in the wallet changelog.

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

Adds an `ApprovalController` initialization configuration to the default wallet ensemble, following the new-design recipe (namespaced `getMessenger` + `init` returning the instance). The controller owns only its own actions/events, so no external messenger delegation is required. Consumers can supply the `showApprovalRequest` callback (which surfaces a pending approval request to the user) via the new `instanceOptions.approvalController` slot; it defaults to a no-op so the controller works headlessly. EVM signing/transaction approval types are excluded from per-origin rate limiting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Make the wiring usable by extension and mobile, not just wallet-cli: expose `typesExcludedFromRateLimiting` as an injectable option alongside `showApprovalRequest`, defaulting to the platform-agnostic EVM signing/transaction baseline. The rate-limiting exclusion set genuinely differs per platform — the extension and mobile each append client-specific types (their smart-transaction status page and `snap_dialog`) and even use different string values for the same concept — so no single hardcoded list is correct. Clients now pass their exact list; consumers that omit it get the EVM default. `showApprovalRequest` continues to default to a no-op (matching mobile; the extension injects its own). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…tting - Regenerate the root README dependency graph for the new `@metamask/approval-controller` and `@metamask/controller-utils` deps (`readme-content:check`). - Apply `oxfmt` formatting to the approval-controller files (`lint:misc:check`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Anticipate #8946 (which makes `instanceOptions` — including a required `storageService.storage` — mandatory on the `Wallet` constructor): drop the `new Wallet(...)` integration tests from `Wallet.test.ts` and instead assert the controller is in the default configuration set from the colocated unit test. This keeps the PR independent of the constructor-options shape (no `new Wallet()` to break) and leaves `Wallet.test.ts` untouched so it merges cleanly with #8946 in either order. Coverage stays at 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Assign `packages/wallet/src/initialization/instances/approval-controller.ts` to @MetaMask/confirmations (the approval-controller domain team), alongside @MetaMask/core-platform (the wallet package owner) — mirroring the existing keyring-controller initialization entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…l-controller # Conflicts: # packages/wallet/CHANGELOG.md

FrederikBolding · 2026-06-02T08:28:41Z

+ * use different string values for the same concept — so there is no single
+ * correct list to hardcode here.
+ */
+const DEFAULT_TYPES_EXCLUDED_FROM_RATE_LIMITING = [


How did you pick these?

These are the six EVM ApprovalType values carried over from the old-design wallet port (origin/feat/wallet-library), which mirrored the extension's exclusion list. They're also the superset of what the clients exclude today: both extension and mobile exempt Transaction and WatchAsset; the extension additionally exempts the signing (personal_sign, eth_signTypedData) and encryption (eth_getEncryptionPublicKey, eth_decrypt) approvals.

I used that as the platform-agnostic baseline and dropped the two client-specific string entries — the smart-transaction status type (whose value even differs between extension and mobile) and snap_dialog (which only matters once SnapController is wired) — making the whole list overridable via instanceOptions.approvalController.typesExcludedFromRateLimiting instead.

Happy to trim the default to just the intersection (Transaction + WatchAsset) if you'd prefer a more conservative baseline.

Let's have someone from the confirmations team sanity check that 👍

Looks fine to me, as long as it's the superset of the two, we were meaning to align mobile with extension.

I would add snap_dialog if we are going for a superset default.

Though I guess the problem with that would be the client specific smart TX string

Thought about this more and kept the default as the EVM baseline (no snap_dialog). The reasoning: the default only applies to consumers that don't pass typesExcludedFromRateLimiting, and both extension and mobile pass their own full lists — so the default isn't what aligns the clients (that happens via each client's override at adoption time). That means a superset default doesn't really buy us anything, and it can't be a true superset anyway because of the smart-TX string divergence you flagged. So the default stays minimal; snap_dialog joins it when we wire SnapController (nothing emits it until then), and clients still get exact parity via the override. Happy to add it back if you'd still prefer the superset.

Ideally we didn't have the divergence in the smart TX string, then we could use a superset and not have overrides.

But up to @matthewwalsh0 whether he thinks that is worth the trouble right now.

We can double-check both clients and remove in future once we know the superset is safe?

- CODEOWNERS: mirror the approval-controller package owner (@MetaMask/confirmations only) for the instance file, per review. - Reword the default rate-limiting-exclusion JSDoc: the list also covers asset-watch and encryption approval types, not only signing/transaction (flagged by Cursor Bugbot). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…l-controller

…ew feedback - Changelog: move the ApprovalController entry to [Unreleased] (main cut a 2.0.0 release that absorbed the StorageService / importSecretRecoveryPhrase entries) — fixes the "Validate changelog diffs" CI job. - Type `typesExcludedFromRateLimiting` via the upstream `ApprovalControllerOptions` indexed type, matching the keyring slot convention (review: type-design). - Correct the default-exclusion JSDoc: it mirrors the extension baseline; mobile exempts only a subset (review: comment accuracy). - Tests: pin the full default exclusion set (parametrized, with a positive pendingApprovalCount assertion) and verify init forwards `state` (review: test coverage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

FrederikBolding

LGTM, let's get someone from @MetaMask/confirmations to review this as well.

matthewwalsh0 · 2026-06-02T14:28:48Z

 /packages/profile-metrics-controller            @MetaMask/mobile-platform @MetaMask/extension-platform

 ## Initialization
+/packages/wallet/src/initialization/instances/approval-controller.ts @MetaMask/confirmations


Would it be easier to have a directory per controller so we can include other utils or constants etc if needed?

So /packages/wallet/src/initialization/instances/approval-controller/ ?

Done 343634d Since it now makes the layout inconsistent do we need to migrate on a follow-up keyring + storage to the new directory/type convention so the package isn't left in two styles? @FrederikBolding I can do it if you agree as well

Sounds good to me, we can move the encryptor out from keyring-controller then!

matthewwalsh0 · 2026-06-02T14:30:35Z

+ * use different string values for the same concept — so there is no single
+ * correct list to hardcode here.
+ */
+const DEFAULT_TYPES_EXCLUDED_FROM_RATE_LIMITING = [


Looks fine to me, as long as it's the superset of the two, we were meaning to align mobile with extension.

…l-controller

@matthewwalsh0

- Restructure into a per-controller directory (`instances/approval-controller/{approval-controller,types}.ts`) so it can hold a dedicated type + future utils (review: @matthewwalsh0). - Extract an `ApprovalControllerInstanceOptions` type and reference it from `InstanceSpecificOptions` (review: @matthewwalsh0). - Add `snap_dialog` to the default rate-limit-exclusion set (superset baseline the clients share); the smart-tx status type stays client-injected since its string differs per platform (review: @FrederikBolding, @matthewwalsh0). - Mark the changelog entry BREAKING: the default Wallet now registers `ApprovalController:*`, so consumers wiring their own must remove it on upgrade to avoid a messenger collision (review: @FrederikBolding). - Point the CODEOWNERS entry at the new directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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 343634d. Configure here.}

Drop `snap_dialog` from the default exclusion list. The default isn't the mechanism that aligns the clients (they each pass their own full list via `instanceOptions`), so a superset default earns nothing and can't be a true superset anyway (the smart-tx status string differs per platform). Keep the default as the EVM baseline; `snap_dialog` joins it when `SnapController` is wired. Simplify the explaining comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…l-controller

Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>

…tureFlagController Migrates the RemoteFeatureFlagController instance to the per-controller directory convention (introduced by #8953, extended by #8977): `instances/remote-feature-flag-controller/` now holds the config, the colocated test, and a `RemoteFeatureFlagControllerInstanceOptions` type in its own `types.ts`. `InstanceSpecificOptions` references that type instead of an inline shape, and `instances/index.ts` + the CODEOWNERS `## Initialization` entry use the directory form. No public exports or option shapes change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

sirtimid requested a review from a team as a code owner June 1, 2026 14:45

sirtimid temporarily deployed to default-branch June 1, 2026 14:45 — with GitHub Actions Inactive

sirtimid marked this pull request as draft June 1, 2026 15:10

sirtimid marked this pull request as ready for review June 1, 2026 18:38

sirtimid temporarily deployed to default-branch June 1, 2026 18:38 — with GitHub Actions Inactive

sirtimid had a problem deploying to default-branch June 1, 2026 18:38 — with GitHub Actions Failure

FrederikBolding reviewed Jun 1, 2026

View reviewed changes

Comment thread packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts

sirtimid and others added 3 commits June 1, 2026 21:11

Merge remote-tracking branch 'origin/main' into sirtimid/wire-approva…

bf7de2b

…l-controller # Conflicts: # packages/wallet/CHANGELOG.md

cursor Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts

FrederikBolding reviewed Jun 2, 2026

View reviewed changes

Comment thread .github/CODEOWNERS Outdated

FrederikBolding reviewed Jun 2, 2026

View reviewed changes

sirtimid and others added 3 commits June 2, 2026 11:48

Merge remote-tracking branch 'origin/main' into sirtimid/wire-approva…

f32320c

…l-controller

sirtimid requested a review from FrederikBolding June 2, 2026 10:13

FrederikBolding reviewed Jun 2, 2026

View reviewed changes

Comment thread packages/wallet/CHANGELOG.md Outdated

FrederikBolding reviewed Jun 2, 2026

View reviewed changes

matthewwalsh0 requested changes Jun 2, 2026

View reviewed changes

sirtimid and others added 2 commits June 2, 2026 16:47

Merge remote-tracking branch 'origin/main' into sirtimid/wire-approva…

3a5030b

…l-controller

cursor Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts Outdated

sirtimid and others added 2 commits June 2, 2026 17:06

Merge remote-tracking branch 'origin/main' into sirtimid/wire-approva…

a9dad92

…l-controller

sirtimid requested review from FrederikBolding and matthewwalsh0 June 2, 2026 15:11

matthewwalsh0 previously approved these changes Jun 2, 2026

View reviewed changes

sirtimid enabled auto-merge June 2, 2026 15:22

FrederikBolding reviewed Jun 2, 2026

View reviewed changes

Comment thread packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts Outdated

Apply suggestion from @FrederikBolding

e533d67

Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>

sirtimid dismissed matthewwalsh0’s stale review via e533d67 June 2, 2026 15:38

Merge branch 'main' into sirtimid/wire-approval-controller

ce8d061

sirtimid requested review from FrederikBolding and matthewwalsh0 June 2, 2026 15:39

FrederikBolding approved these changes Jun 2, 2026

View reviewed changes

sirtimid added this pull request to the merge queue Jun 2, 2026

Merged via the queue into main with commit 43c2ac9 Jun 2, 2026
370 checks passed

sirtimid deleted the sirtimid/wire-approval-controller branch June 2, 2026 15:45

sirtimid mentioned this pull request Jun 2, 2026

refactor(wallet): migrate keyring + storage instances to directory convention #8977

Open

4 tasks

matthewwalsh0 mentioned this pull request Jun 2, 2026

feat(wallet): wire TransactionController into default initialization #8975

Draft

4 tasks

Uh oh!

Conversation

sirtimid commented Jun 1, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Checklist

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!

matthewwalsh0 Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FrederikBolding left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirtimid Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sirtimid commented Jun 1, 2026 •

edited by cursor Bot

Loading

matthewwalsh0 Jun 2, 2026 •

edited

Loading

sirtimid Jun 2, 2026 •

edited

Loading

matthewwalsh0 Jun 2, 2026 •

edited

Loading