diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f855e4a66c..e228d8fb4b 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -129,6 +129,7 @@ /packages/profile-metrics-controller @MetaMask/mobile-platform @MetaMask/extension-platform ## Initialization +/packages/wallet/src/initialization/instances/approval-controller/ @MetaMask/confirmations /packages/wallet/src/initialization/instances/keyring-controller.ts @MetaMask/accounts-engineers @MetaMask/core-platform ## Package Release related diff --git a/README.md b/README.md index 8d6661b2bb..54235840db 100644 --- a/README.md +++ b/README.md @@ -567,7 +567,9 @@ linkStyle default opacity:0.5 user_operation_controller --> polling_controller; user_operation_controller --> transaction_controller; user_operation_controller --> eth_block_tracker; + wallet --> approval_controller; wallet --> base_controller; + wallet --> controller_utils; wallet --> keyring_controller; wallet --> messenger; wallet --> storage_service; diff --git a/packages/wallet/CHANGELOG.md b/packages/wallet/CHANGELOG.md index 4bd983db6c..787705fe66 100644 --- a/packages/wallet/CHANGELOG.md +++ b/packages/wallet/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING:** Wire `ApprovalController` into the default wallet initialization ([#8953](https://github.com/MetaMask/core/pull/8953)) + - The default `Wallet` now constructs an `ApprovalController` and registers its `ApprovalController:*` messenger actions. Consumers that pass their own `messenger` and already wire an `ApprovalController` must remove their own before upgrading, or the duplicate registration will collide. + - Adds an `approvalController` slot to `instanceOptions` with `showApprovalRequest` (the callback that surfaces pending approval requests to the user; defaults to a no-op) and `typesExcludedFromRateLimiting` (the approval types exempt from per-origin rate limiting; defaults to a baseline of EVM approval types). Both let consumers (extension, mobile, wallet-cli) inject their platform-specific values. + ## [2.0.0] ### Added diff --git a/packages/wallet/package.json b/packages/wallet/package.json index 646a0f8994..f4f1414434 100644 --- a/packages/wallet/package.json +++ b/packages/wallet/package.json @@ -53,8 +53,10 @@ "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" }, "dependencies": { + "@metamask/approval-controller": "^9.0.1", "@metamask/base-controller": "^9.1.0", "@metamask/browser-passworder": "^6.0.0", + "@metamask/controller-utils": "^12.1.0", "@metamask/keyring-controller": "^26.0.0", "@metamask/messenger": "^1.2.0", "@metamask/scure-bip39": "^2.1.1", diff --git a/packages/wallet/src/initialization/instances/approval-controller/approval-controller.test.ts b/packages/wallet/src/initialization/instances/approval-controller/approval-controller.test.ts new file mode 100644 index 0000000000..de5a517e69 --- /dev/null +++ b/packages/wallet/src/initialization/instances/approval-controller/approval-controller.test.ts @@ -0,0 +1,148 @@ +import { ApprovalController } from '@metamask/approval-controller'; +import { ApprovalType } from '@metamask/controller-utils'; +import { Messenger } from '@metamask/messenger'; + +import { defaultConfigurations } from '../../defaults'; +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '../../defaults'; +import { approvalController } from './approval-controller'; + +/** + * Creates a root messenger for use in tests. + * + * @returns A root messenger. + */ +function getRootMessenger(): RootMessenger { + return new Messenger({ namespace: 'Root' }); +} + +describe('approvalController', () => { + it('is registered as a default initialization configuration', () => { + // Proves the controller is part of the default ensemble that `initialize()` + // wires, without constructing a `Wallet` (which keeps this PR independent of + // the constructor-options shape). + expect(Object.values(defaultConfigurations)).toContain(approvalController); + }); + + it('initializes an ApprovalController with default state', () => { + const messenger = approvalController.getMessenger(getRootMessenger()); + + const instance = approvalController.init({ + state: undefined, + messenger, + options: {}, + }); + + expect(instance).toBeInstanceOf(ApprovalController); + expect(instance.state).toStrictEqual({ + pendingApprovals: {}, + pendingApprovalCount: 0, + approvalFlows: [], + }); + }); + + it('forwards the provided state to the controller', () => { + const messenger = approvalController.getMessenger(getRootMessenger()); + + const instance = approvalController.init({ + state: { + pendingApprovals: {}, + pendingApprovalCount: 3, + approvalFlows: [], + }, + messenger, + options: {}, + }); + + expect(instance.state.pendingApprovalCount).toBe(3); + }); + + it('uses the provided showApprovalRequest callback', () => { + const messenger = approvalController.getMessenger(getRootMessenger()); + const showApprovalRequest = jest.fn(); + + const instance = approvalController.init({ + state: undefined, + messenger, + options: { showApprovalRequest }, + }); + + instance.startFlow(); + + expect(showApprovalRequest).toHaveBeenCalledTimes(1); + }); + + it('defaults showApprovalRequest to a no-op when omitted', () => { + const messenger = approvalController.getMessenger(getRootMessenger()); + + const instance = approvalController.init({ + state: undefined, + messenger, + options: {}, + }); + + expect(() => instance.startFlow()).not.toThrow(); + }); + + // Pins the exact default exclusion set (independent of the source constant): + // each of these types must allow multiple pending requests from one origin. + // The pending promises never settle here; `.catch` only marks them handled. + it.each([ + ApprovalType.PersonalSign, + ApprovalType.EthSignTypedData, + ApprovalType.Transaction, + ApprovalType.WatchAsset, + ApprovalType.EthGetEncryptionPublicKey, + ApprovalType.EthDecrypt, + ])('excludes %s from rate limiting by default', (type) => { + const messenger = approvalController.getMessenger(getRootMessenger()); + + const instance = approvalController.init({ + state: undefined, + messenger, + options: {}, + }); + + instance.add({ origin: 'metamask.io', type }).catch(() => undefined); + instance.add({ origin: 'metamask.io', type }).catch(() => undefined); + + // Both requests are queued rather than the second being rejected. + expect(instance.state.pendingApprovalCount).toBe(2); + }); + + it('honors a custom typesExcludedFromRateLimiting list that overrides the default', () => { + const messenger = approvalController.getMessenger(getRootMessenger()); + + const instance = approvalController.init({ + state: undefined, + messenger, + // Empty override: nothing is excluded, not even the default types. + options: { typesExcludedFromRateLimiting: [] }, + }); + + instance + .add({ origin: 'metamask.io', type: ApprovalType.Transaction }) + .catch(() => undefined); + + // A second request of the same origin and type is now rate-limited. + expect(() => + instance.add({ origin: 'metamask.io', type: ApprovalType.Transaction }), + ).toThrow('already pending'); + }); + + it('exposes its actions through the root messenger', () => { + const rootMessenger = getRootMessenger(); + const messenger = approvalController.getMessenger(rootMessenger); + + approvalController.init({ state: undefined, messenger, options: {} }); + + expect(rootMessenger.call('ApprovalController:getState')).toStrictEqual({ + pendingApprovals: {}, + pendingApprovalCount: 0, + approvalFlows: [], + }); + }); +}); diff --git a/packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts b/packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts new file mode 100644 index 0000000000..728c002008 --- /dev/null +++ b/packages/wallet/src/initialization/instances/approval-controller/approval-controller.ts @@ -0,0 +1,49 @@ +import { + ApprovalController, + ApprovalControllerMessenger, +} from '@metamask/approval-controller'; +import { ApprovalType } from '@metamask/controller-utils'; +import { Messenger } from '@metamask/messenger'; + +import { InitializationConfiguration } from '../../types'; + +/** + * Approval types that are exempt from per-origin rate limiting, so more than one + * request of the same type can be pending at once. These are the common EVM + * types: signing, transactions, watch-asset, and encryption. + * + * Clients can replace this list via + * `instanceOptions.approvalController.typesExcludedFromRateLimiting` — the + * extension and mobile pass their own. `snap_dialog` will be added here once the + * wallet wires `SnapController`. + */ +const DEFAULT_TYPES_EXCLUDED_FROM_RATE_LIMITING = [ + ApprovalType.PersonalSign, + ApprovalType.EthSignTypedData, + ApprovalType.Transaction, + ApprovalType.WatchAsset, + ApprovalType.EthGetEncryptionPublicKey, + ApprovalType.EthDecrypt, +]; + +export const approvalController: InitializationConfiguration< + ApprovalController, + ApprovalControllerMessenger +> = { + name: 'ApprovalController', + init: ({ state, messenger, options }) => + new ApprovalController({ + state, + messenger, + showApprovalRequest: + options.showApprovalRequest ?? ((): void => undefined), + typesExcludedFromRateLimiting: + options.typesExcludedFromRateLimiting ?? + DEFAULT_TYPES_EXCLUDED_FROM_RATE_LIMITING, + }), + getMessenger: (parent) => + new Messenger({ + namespace: 'ApprovalController', + parent, + }), +}; diff --git a/packages/wallet/src/initialization/instances/approval-controller/types.ts b/packages/wallet/src/initialization/instances/approval-controller/types.ts new file mode 100644 index 0000000000..842750f636 --- /dev/null +++ b/packages/wallet/src/initialization/instances/approval-controller/types.ts @@ -0,0 +1,23 @@ +import type { + ApprovalControllerOptions, + ShowApprovalRequest, +} from '@metamask/approval-controller'; + +/** + * Per-instance options for the wallet's `ApprovalController`. Both fields are + * optional; see the controller's `init` for the defaults applied when omitted. + */ +export type ApprovalControllerInstanceOptions = { + /** + * Callback that surfaces a pending approval request to the user. Defaults to + * a no-op so the controller works headlessly. + */ + showApprovalRequest?: ShowApprovalRequest; + /** + * Approval types exempt from per-origin rate limiting. Defaults to a baseline + * of EVM approval types. + */ + typesExcludedFromRateLimiting?: NonNullable< + ApprovalControllerOptions['typesExcludedFromRateLimiting'] + >; +}; diff --git a/packages/wallet/src/initialization/instances/index.ts b/packages/wallet/src/initialization/instances/index.ts index 95b4a4a080..56cb78a012 100644 --- a/packages/wallet/src/initialization/instances/index.ts +++ b/packages/wallet/src/initialization/instances/index.ts @@ -1,2 +1,3 @@ +export { approvalController } from './approval-controller/approval-controller'; export { keyringController } from './keyring-controller'; export { storageService } from './storage-service'; diff --git a/packages/wallet/src/types.ts b/packages/wallet/src/types.ts index 19926a251e..a95699f8ef 100644 --- a/packages/wallet/src/types.ts +++ b/packages/wallet/src/types.ts @@ -7,6 +7,7 @@ import type { DefaultEvents, RootMessenger, } from './initialization/defaults'; +import type { ApprovalControllerInstanceOptions } from './initialization/instances/approval-controller/types'; import { GenericEncryptor } from './initialization/instances/keyring-controller'; import { InitializationConfiguration } from './initialization/types'; @@ -21,6 +22,7 @@ export type WalletOptions = { }; export type InstanceSpecificOptions = { + approvalController?: ApprovalControllerInstanceOptions; keyringController?: { encryptor?: GenericEncryptor; keyringBuilders?: KeyringControllerOptions['keyringBuilders']; diff --git a/packages/wallet/tsconfig.build.json b/packages/wallet/tsconfig.build.json index 376689caad..68e89ef628 100644 --- a/packages/wallet/tsconfig.build.json +++ b/packages/wallet/tsconfig.build.json @@ -6,7 +6,9 @@ "rootDir": "./src" }, "references": [ + { "path": "../approval-controller/tsconfig.build.json" }, { "path": "../base-controller/tsconfig.build.json" }, + { "path": "../controller-utils/tsconfig.build.json" }, { "path": "../keyring-controller/tsconfig.build.json" }, { "path": "../messenger/tsconfig.build.json" }, { "path": "../storage-service/tsconfig.build.json" } diff --git a/packages/wallet/tsconfig.json b/packages/wallet/tsconfig.json index 2d262243eb..7b905db657 100644 --- a/packages/wallet/tsconfig.json +++ b/packages/wallet/tsconfig.json @@ -4,7 +4,9 @@ "baseUrl": "./" }, "references": [ + { "path": "../approval-controller/tsconfig.json" }, { "path": "../base-controller/tsconfig.json" }, + { "path": "../controller-utils/tsconfig.json" }, { "path": "../keyring-controller/tsconfig.json" }, { "path": "../messenger/tsconfig.json" }, { "path": "../storage-service/tsconfig.json" } diff --git a/yarn.lock b/yarn.lock index a88099a7dd..8039304ba3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5885,9 +5885,11 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/wallet@workspace:packages/wallet" dependencies: + "@metamask/approval-controller": "npm:^9.0.1" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" "@metamask/browser-passworder": "npm:^6.0.0" + "@metamask/controller-utils": "npm:^12.1.0" "@metamask/keyring-controller": "npm:^26.0.0" "@metamask/messenger": "npm:^1.2.0" "@metamask/scure-bip39": "npm:^2.1.1"