diff --git a/docs/contributing.md b/docs/contributing.md index 43b9230d95e..e9007a0ccf4 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -46,9 +46,9 @@ If you need to customize the behavior of Jest for a package, see `jest.config.js ## Linting -[ESLint](https://eslint.org/docs/v8.x/) v8 (via [MetaMask's shared ESLint configurations](https://github.com/MetaMask/eslint-config)) is used to check for code quality issues, and [Prettier](https://prettier.io/docs/en/) is used to format files. +[ESLint](https://eslint.org) v9 (via [MetaMask's shared ESLint configurations](https://github.com/MetaMask/eslint-config)) is used to check for code quality issues, and [Prettier](https://prettier.io/docs/en/) is used to format files. -If you need to customize the behavior of ESLint, see `.eslintrc.js` in the root. +If you need to customize the behavior of ESLint, see `eslint.config.mjs` in the root. - Run `yarn lint` to lint all files and show possible violations across the monorepo. - Run `yarn lint:fix` to fix any automatically fixable violations. diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json new file mode 100644 index 00000000000..dcad81bed13 --- /dev/null +++ b/eslint-warning-thresholds.json @@ -0,0 +1,29 @@ +{ + "@typescript-eslint/consistent-type-exports": 19, + "@typescript-eslint/no-base-to-string": 3, + "@typescript-eslint/no-duplicate-enum-values": 2, + "@typescript-eslint/no-misused-promises": 3, + "@typescript-eslint/no-unsafe-enum-comparison": 59, + "@typescript-eslint/no-unused-vars": 36, + "@typescript-eslint/prefer-promise-reject-errors": 13, + "@typescript-eslint/prefer-readonly": 152, + "@typescript-eslint/switch-exhaustiveness-check": 10, + "import-x/namespace": 189, + "import-x/no-named-as-default": 1, + "import-x/no-named-as-default-member": 8, + "import-x/order": 209, + "jest/no-conditional-in-test": 104, + "jsdoc/check-tag-names": 372, + "jsdoc/require-returns": 22, + "jsdoc/tag-lines": 329, + "n/no-unsupported-features/node-builtins": 18, + "n/prefer-global/text-encoder": 4, + "n/prefer-global/text-decoder": 4, + "prettier/prettier": 116, + "promise/always-return": 3, + "promise/catch-or-return": 2, + "promise/param-names": 8, + "no-empty-function": 2, + "no-shadow": 8, + "no-unused-private-class-members": 6 +} diff --git a/eslint.config.mjs b/eslint.config.mjs index ec01ff56f2b..91185d8cb80 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -1,19 +1,12 @@ import base, { createConfig } from '@metamask/eslint-config'; -import nodejs from '@metamask/eslint-config-nodejs'; import jest from '@metamask/eslint-config-jest'; +import nodejs from '@metamask/eslint-config-nodejs'; import typescript from '@metamask/eslint-config-typescript'; -const config = createConfig( +const config = createConfig([ + ...base, { ignores: [ - 'yarn.lock', - '**/**.map', - '**/**.tsbuildinfo', - '**/*.json', - '**/*.md', - '**/LICENSE', - '**/*.sh', - '**/.DS_Store', '**/dist/**', '**/docs/**', '**/coverage/**', @@ -22,7 +15,6 @@ const config = createConfig( 'scripts/create-package/package-template/**', ], }, - ...base, { rules: { // Left disabled because various properties throughough this repo are snake_case because the @@ -32,14 +24,12 @@ const config = createConfig( 'id-length': 'off', // TODO: re-enble most of these rules - '@typescript-eslint/naming-convention': 'off', 'function-paren-newline': 'off', 'id-denylist': 'off', 'implicit-arrow-linebreak': 'off', - 'import/no-anonymous-default-export': 'off', - 'import/no-unassigned-import': 'off', + 'import-x/no-anonymous-default-export': 'off', + 'import-x/no-unassigned-import': 'off', 'lines-around-comment': 'off', - 'n/no-sync': 'off', 'no-async-promise-executor': 'off', 'no-case-declarations': 'off', 'no-invalid-this': 'off', @@ -53,6 +43,12 @@ const config = createConfig( 'off', { matchDescription: '^[A-Z`\\d_][\\s\\S]*[.?!`>)}]$' }, ], + + // TODO: These rules created more errors after the upgrade to ESLint 9. + // Re-enable these rules and address any lint violations. + 'import-x/no-named-as-default-member': 'warn', + 'prettier/prettier': 'warn', + 'no-empty-function': 'warn', }, settings: { jsdoc: { @@ -62,26 +58,36 @@ const config = createConfig( }, { files: [ - '**/jest.config.js', - '**/jest.environment.js', - '**/tests/**/*.{ts,js}', - '*.js', - '*.test.{ts,js}', + '**/*.{js,cjs,mjs}', + '**/*.test.{js,ts}', + '**/tests/**/*.{js,ts}', 'scripts/*.ts', - 'scripts/create-package/*.ts', - 'yarn.config.cjs', + 'scripts/create-package/**/*.ts', ], extends: [nodejs], + rules: { + // TODO: Re-enable this + 'n/no-sync': 'off', + // TODO: These rules created more errors after the upgrade to ESLint 9. + // Re-enable these rules and address any lint violations. + 'n/no-unsupported-features/node-builtins': 'warn', + }, }, { - files: ['*.test.{ts,js}', '**/tests/**/*.{ts,js}'], + files: ['**/*.test.{js,ts}', '**/tests/**/*.{js,ts}'], extends: [jest], + rules: { + // TODO: These rules created more errors after the upgrade to ESLint 9. + // Re-enable these rules and address any lint violations. + 'jest/no-conditional-in-test': 'warn', + }, }, { // These files are test helpers, not tests. We still use the Jest ESLint // config here to ensure that ESLint expects a test-like environment, but // various rules meant just to apply to tests have been disabled. - files: ['**/tests/**/*.{ts,js}', '!*.test.{ts,js}'], + files: ['**/tests/**/*.{js,ts}'], + ignores: ['**/*.test.{js,ts}'], rules: { 'jest/no-export': 'off', 'jest/require-top-level-describe': 'off', @@ -89,20 +95,31 @@ const config = createConfig( }, }, { - files: ['*.js', '*.cjs'], - parserOptions: { + files: ['**/*.{js,cjs}'], + languageOptions: { sourceType: 'script', - ecmaVersion: '2020', + ecmaVersion: 2020, }, }, { - files: ['*.ts'], + files: ['**/*.ts'], extends: [typescript], - parserOptions: { - tsconfigRootDir: import.meta.dirname, - project: ['./tsconfig.packages.json'], + languageOptions: { + parserOptions: { + tsconfigRootDir: import.meta.dirname, + project: './tsconfig.packages.json', + // Disable `projectService` because we run into out-of-memory issues. + // See this ticket for inspiration out how to solve this: + // + projectService: false, + }, }, rules: { + // This rule does not detect multiple imports of the same file where types + // are being imported in one case and runtime values are being imported in + // another + 'import-x/no-duplicates': 'off', + // Enable rules that are disabled in `@metamask/eslint-config-typescript` '@typescript-eslint/no-explicit-any': 'error', @@ -110,6 +127,7 @@ const config = createConfig( '@typescript-eslint/promise-function-async': 'off', // TODO: re-enable most of these rules + '@typescript-eslint/naming-convention': 'off', '@typescript-eslint/no-unnecessary-type-assertion': 'off', '@typescript-eslint/unbound-method': 'off', '@typescript-eslint/prefer-enum-initializers': 'off', @@ -118,26 +136,51 @@ const config = createConfig( '@typescript-eslint/prefer-reduce-type-parameter': 'off', 'no-restricted-syntax': 'off', 'no-restricted-globals': 'off', + + // TODO: These rules created more errors after the upgrade to ESLint 9. + // Re-enable these rules and address any lint violations. + '@typescript-eslint/consistent-type-exports': 'warn', + '@typescript-eslint/explicit-function-return-type': 'off', + '@typescript-eslint/no-base-to-string': 'warn', + '@typescript-eslint/no-duplicate-enum-values': 'warn', + '@typescript-eslint/no-misused-promises': 'warn', + '@typescript-eslint/no-unsafe-enum-comparison': 'warn', + '@typescript-eslint/no-unused-vars': 'warn', + '@typescript-eslint/only-throw-error': 'warn', + '@typescript-eslint/prefer-promise-reject-errors': 'warn', + '@typescript-eslint/prefer-readonly': 'warn', + '@typescript-eslint/switch-exhaustiveness-check': 'warn', + 'import-x/namespace': 'warn', + 'import-x/no-named-as-default': 'warn', + 'import-x/order': 'warn', + 'jsdoc/check-tag-names': 'warn', + 'jsdoc/require-returns': 'warn', + 'jsdoc/tag-lines': 'warn', + 'no-unused-private-class-members': 'warn', + 'promise/always-return': 'warn', + 'promise/catch-or-return': 'warn', + 'promise/param-names': 'warn', }, }, { files: ['tests/setupAfterEnv/matchers.ts'], - parserOptions: { + languageOptions: { sourceType: 'script', }, }, + // This should really be in `@metamask/eslint-config-typescript` { - files: ['*.d.ts'], + files: ['**/*.d.ts'], rules: { '@typescript-eslint/naming-convention': 'warn', - 'import/unambiguous': 'off', + 'import-x/unambiguous': 'off', }, }, { files: ['scripts/*.ts'], rules: { - // All scripts will have shebangs. - 'n/shebang': 'off', + // Scripts may be self-executable and thus have hashbangs. + 'n/hashbang': 'off', }, }, { @@ -145,8 +188,20 @@ const config = createConfig( rules: { // These files run under Node, and thus `require(...)` is expected. 'n/global-require': 'off', + + // TODO: These rules created more errors after the upgrade to ESLint 9. + // Re-enable these rules and address any lint violations. + 'n/prefer-global/text-encoder': 'warn', + 'n/prefer-global/text-decoder': 'warn', + 'no-shadow': 'warn', + }, + }, + { + files: ['**/*.mjs'], + languageOptions: { + sourceType: 'module', }, }, -); +]); export default config; diff --git a/package.json b/package.json index 123572835c8..a52e92956af 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "282.0.0", + "version": "283.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { @@ -23,7 +23,7 @@ "lint": "yarn lint:eslint && yarn lint:misc --check && yarn constraints && yarn lint:dependencies && yarn lint:teams", "lint:dependencies": "depcheck && yarn dedupe --check", "lint:dependencies:fix": "depcheck && yarn dedupe", - "lint:eslint": "eslint . --cache", + "lint:eslint": "yarn ts-node ./scripts/run-eslint.ts --cache", "lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies:fix", "lint:misc": "prettier --no-error-on-unmatched-pattern '**/*.json' '**/*.md' '**/*.yml' '!.yarnrc.yml' '!merged-packages/**' --ignore-path .gitignore", "lint:teams": "ts-node scripts/lint-teams-json.ts", diff --git a/packages/controller-utils/package.json b/packages/controller-utils/package.json index cb2ec2ee882..4cc3ceb4bf2 100644 --- a/packages/controller-utils/package.json +++ b/packages/controller-utils/package.json @@ -65,6 +65,7 @@ "@types/jest": "^27.4.1", "deepmerge": "^4.2.2", "jest": "^27.5.1", + "jest-environment-jsdom": "^27.5.1", "nock": "^13.3.1", "sinon": "^9.2.4", "ts-jest": "^27.1.4", diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index 14b41caedba..7f3358b64c9 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -618,7 +618,7 @@ function logOrRethrowError(error: unknown, codesToCatch: number[] = []) { throw error; } } else { - // eslint-disable-next-line @typescript-eslint/no-throw-literal + // eslint-disable-next-line @typescript-eslint/only-throw-error throw error; } } diff --git a/packages/json-rpc-engine/src/JsonRpcEngine.ts b/packages/json-rpc-engine/src/JsonRpcEngine.ts index 62fb5204d0d..c589e13358d 100644 --- a/packages/json-rpc-engine/src/JsonRpcEngine.ts +++ b/packages/json-rpc-engine/src/JsonRpcEngine.ts @@ -489,7 +489,7 @@ export class JsonRpcEngine extends SafeEventEmitter { // Now we re-throw the middleware processing error, if any, to catch it // further up the call chain. if (error) { - // eslint-disable-next-line @typescript-eslint/no-throw-literal + // eslint-disable-next-line @typescript-eslint/only-throw-error throw error; } } diff --git a/packages/json-rpc-middleware-stream/src/index.test.ts b/packages/json-rpc-middleware-stream/src/index.test.ts index d770cad07b3..6395c629a73 100644 --- a/packages/json-rpc-middleware-stream/src/index.test.ts +++ b/packages/json-rpc-middleware-stream/src/index.test.ts @@ -8,7 +8,7 @@ import { createStreamMiddleware, createEngineStream } from '.'; const artificialDelay = async (time = 0) => new Promise((resolve) => setTimeout(resolve, time)); // TODO: Replace `any` with type -// eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-explicit-any +// eslint-disable-next-line @typescript-eslint/no-empty-function,@typescript-eslint/no-explicit-any const noop = function (_a: any) {}; const jsonrpc = '2.0' as const; diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 4cc90868a8f..bf5b853aeeb 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -1580,7 +1580,6 @@ export class KeyringController extends BaseController< * @deprecated Use `withKeyring` instead. */ async cancelQRSynchronization(): Promise { - // eslint-disable-next-line n/no-sync (await this.getOrAddQRKeyring()).cancelSync(); } diff --git a/packages/message-manager/CHANGELOG.md b/packages/message-manager/CHANGELOG.md index ac90e79a53d..f9ca2a88193 100644 --- a/packages/message-manager/CHANGELOG.md +++ b/packages/message-manager/CHANGELOG.md @@ -9,8 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Base class of `DecryptMessageManager` and `EncryptionPublicKeyManager`(`AbstractMessageManager`) now expects new options to initialise ([#5103](https://github.com/MetaMask/core/pull/5103)) - Bump `@metamask/base-controller` from `^7.0.0` to `^7.1.0` ([#5079](https://github.com/MetaMask/core/pull/5079)) +### Removed + +- **BREAKING:** Removed internal event emitter (`hub` property) from `AbstractMessageManager` ([#5103](https://github.com/MetaMask/core/pull/5103)) +- **BREAKING:** `unapprovedMessage` and `updateBadge` removed from internal events. These events are now emitted from messaging system ([#5103](https://github.com/MetaMask/core/pull/5103)) + - Controllers should now listen to `DerivedManagerName:X` event instead of using internal event emitter. + ## [11.0.3] ### Changed diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index 740ed38e2ca..fcf520854c0 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -1,3 +1,4 @@ +import type { RestrictedControllerMessenger } from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; import type { @@ -20,10 +21,15 @@ type ConcreteMessageParamsMetamask = ConcreteMessageParams & { metamaskId?: string; }; +type ConcreteMessageManagerActions = never; +type ConcreteMessageManagerEvents = never; + class AbstractTestManager extends AbstractMessageManager< ConcreteMessage, ConcreteMessageParams, - ConcreteMessageParamsMetamask + ConcreteMessageParamsMetamask, + ConcreteMessageManagerActions, + ConcreteMessageManagerEvents > { addRequestToMessageParams( messageParams: MessageParams, @@ -56,6 +62,26 @@ class AbstractTestManager extends AbstractMessageManager< } } +const MOCK_MESSENGER = { + clearEventSubscriptions: jest.fn(), + publish: jest.fn(), + registerActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), +} as unknown as RestrictedControllerMessenger< + 'AbstractMessageManager', + never, + never, + string, + string +>; + +const MOCK_INITIAL_OPTIONS = { + additionalFinishStatuses: undefined, + messenger: MOCK_MESSENGER, + name: 'AbstractMessageManager' as const, + securityProviderRequest: undefined, +}; + const messageId = '1'; const messageId2 = '2'; const from = '0x0123'; @@ -78,20 +104,15 @@ const mockMessageParams = { from, test: testData }; describe('AbstractTestManager', () => { it('should set default state', () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); expect(controller.state).toStrictEqual({ unapprovedMessages: {}, unapprovedMessagesCount: 0, }); }); - it('should set default config', () => { - const controller = new AbstractTestManager(); - expect(controller.config).toStrictEqual({}); - }); - it('should add a valid message', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { @@ -115,7 +136,7 @@ describe('AbstractTestManager', () => { }); it('should get all messages', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const message = { id: messageId, messageParams: { @@ -148,11 +169,10 @@ describe('AbstractTestManager', () => { const securityProviderRequestMock: SecurityProviderRequest = jest .fn() .mockResolvedValue(securityProviderResponseMock); - const controller = new AbstractTestManager( - undefined, - undefined, - securityProviderRequestMock, - ); + const controller = new AbstractTestManager({ + ...MOCK_INITIAL_OPTIONS, + securityProviderRequest: securityProviderRequestMock, + }); await controller.addMessage({ id: messageId, messageParams: { @@ -180,7 +200,7 @@ describe('AbstractTestManager', () => { }); it('should reject a message', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { @@ -200,7 +220,7 @@ describe('AbstractTestManager', () => { }); it('should sign a message', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { @@ -221,12 +241,10 @@ describe('AbstractTestManager', () => { }); it('sets message to one of the allowed statuses', async () => { - const controller = new AbstractTestManager( - undefined, - undefined, - undefined, - ['test-status'], - ); + const controller = new AbstractTestManager({ + ...MOCK_INITIAL_OPTIONS, + additionalFinishStatuses: ['test-status'], + }); await controller.addMessage({ id: messageId, messageParams: { @@ -246,12 +264,10 @@ describe('AbstractTestManager', () => { }); it('should set a status to inProgress', async () => { - const controller = new AbstractTestManager( - undefined, - undefined, - undefined, - ['test-status'], - ); + const controller = new AbstractTestManager({ + ...MOCK_INITIAL_OPTIONS, + additionalFinishStatuses: ['test-status'], + }); await controller.addMessage({ id: messageId, messageParams: { @@ -285,7 +301,7 @@ describe('AbstractTestManager', () => { time: 123, type: 'eth_signTypedData', }; - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage(firstMessage); await controller.addMessage(secondMessage); expect(controller.getUnapprovedMessagesCount()).toBe(2); @@ -296,7 +312,7 @@ describe('AbstractTestManager', () => { }); it('should approve message', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const firstMessage = { from: '0xfoO', test: testData }; await controller.addMessage({ id: messageId, @@ -319,7 +335,7 @@ describe('AbstractTestManager', () => { describe('addRequestToMessageParams', () => { it('adds original request id and origin to messageParams', () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const result = controller.addRequestToMessageParams( mockMessageParams, @@ -336,7 +352,7 @@ describe('AbstractTestManager', () => { describe('createUnapprovedMessage', () => { it('creates a Message object with an unapproved status', () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const result = controller.createUnapprovedMessage( mockMessageParams, @@ -361,7 +377,7 @@ describe('AbstractTestManager', () => { emit: jest.fn(), })); - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -379,7 +395,7 @@ describe('AbstractTestManager', () => { }); it('throws an error if the message is not found', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); expect(() => controller.setMessageStatus(messageId, 'newstatus')).toThrow( 'AbstractMessageManager: Message not found for id: 1.', @@ -393,7 +409,7 @@ describe('AbstractTestManager', () => { emit: jest.fn(), })); - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -407,14 +423,13 @@ describe('AbstractTestManager', () => { controller.setMessageStatusAndResult(messageId, 'newRawSig', 'newstatus'); const messageAfter = controller.getMessage(messageId); - // expect(controller.hub.emit).toHaveBeenNthCalledWith(1, 'updateBadge'); expect(messageAfter?.status).toBe('newstatus'); }); }); describe('setMetadata', () => { it('should set the given message metadata', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -432,7 +447,7 @@ describe('AbstractTestManager', () => { }); it('should throw an error if message is not found', () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); expect(() => controller.setMetadata(messageId, { foo: 'bar' })).toThrow( 'AbstractMessageManager: Message not found for id: 1.', @@ -442,7 +457,7 @@ describe('AbstractTestManager', () => { describe('waitForFinishStatus', () => { it('signs the message when status is "signed"', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -452,7 +467,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'signed', rawSig: rawSigMock, }); @@ -462,7 +477,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when status is "rejected"', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -472,7 +487,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -483,7 +498,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when finishes with unknown status', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -493,7 +508,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'unknown', }); }, 100); @@ -508,7 +523,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when finishes with errored status', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -518,7 +533,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'errored', error: 'error message', }); @@ -529,4 +544,26 @@ describe('AbstractTestManager', () => { ); }); }); + + describe('clearUnapprovedMessages', () => { + it('clears the unapproved messages', () => { + const controller = new AbstractTestManager({ + ...MOCK_INITIAL_OPTIONS, + state: { + unapprovedMessages: { + '1': { + id: '1', + messageParams: { from: '0x1', test: 1 }, + status: 'unapproved', + time: 10, + type: 'type', + }, + }, + unapprovedMessagesCount: 1, + }, + }); + controller.clearUnapprovedMessages(); + expect(controller.getUnapprovedMessagesCount()).toBe(0); + }); + }); }); diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index bdd8401f54c..7027acd6862 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -1,29 +1,41 @@ -import type { BaseConfig, BaseState } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; +import type { + ActionConstraint, + EventConstraint, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import type { ApprovalType } from '@metamask/controller-utils'; -import type { Hex, Json } from '@metamask/utils'; +import type { Json } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import { EventEmitter } from 'events'; +import type { Draft } from 'immer'; import { v1 as random } from 'uuid'; +const stateMetadata = { + unapprovedMessages: { persist: false, anonymous: false }, + unapprovedMessagesCount: { persist: false, anonymous: false }, +}; + +const getDefaultState = () => ({ + unapprovedMessages: {}, + unapprovedMessagesCount: 0, +}); + /** * @type OriginalRequest * * Represents the original request object for adding a message. * @property origin? - Is it is specified, represents the origin */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface OriginalRequest { +export type OriginalRequest = { id?: number; origin?: string; securityAlertResponse?: Record; -} +}; /** - * @type Message + * @type AbstractMessage * * Represents and contains data about a signing type signature request. * @property id - An id to track and identify the message object @@ -33,10 +45,7 @@ export interface OriginalRequest { * @property securityProviderResponse - Response from a security provider, whether it is malicious or not * @property metadata - Additional data for the message, for example external identifiers */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface AbstractMessage { +export type AbstractMessage = { id: string; time: number; status: string; @@ -46,7 +55,7 @@ export interface AbstractMessage { securityAlertResponse?: Record; metadata?: Json; error?: string; -} +}; /** * @type AbstractMessageParams @@ -57,15 +66,12 @@ export interface AbstractMessage { * @property requestId? - Original request id * @property deferSetAsSigned? - Whether to defer setting the message as signed immediately after the keyring is told to sign it */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface AbstractMessageParams { +export type AbstractMessageParams = { from: string; origin?: string; requestId?: number; deferSetAsSigned?: boolean; -} +}; /** * @type MessageParamsMetamask @@ -76,12 +82,9 @@ export interface AbstractMessageParams { * @property from - Address from which the message is processed * @property origin? - Added for request origin identification */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface AbstractMessageParamsMetamask extends AbstractMessageParams { +export type AbstractMessageParamsMetamask = AbstractMessageParams & { metamaskId?: string; -} +}; /** * @type MessageManagerState @@ -90,15 +93,15 @@ export interface AbstractMessageParamsMetamask extends AbstractMessageParams { * @property unapprovedMessages - A collection of all Messages in the 'unapproved' state * @property unapprovedMessagesCount - The count of all Messages in this.unapprovedMessages */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions, @typescript-eslint/naming-convention -export interface MessageManagerState - extends BaseState { - unapprovedMessages: { [key: string]: M }; +export type MessageManagerState = { + unapprovedMessages: Record; unapprovedMessagesCount: number; -} +}; + +export type UpdateBadgeEvent = { + type: `${string}:updateBadge`; + payload: []; +}; /** * A function for verifying a message, whether it is malicious or not @@ -108,32 +111,82 @@ export type SecurityProviderRequest = ( messageType: string, ) => Promise; -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/naming-convention -type getCurrentChainId = () => Hex; +/** + * AbstractMessageManager constructor options. + * + * @property additionalFinishStatuses - Optional list of statuses that are accepted to emit a finished event. + * @property messenger - Controller messaging system. + * @property name - The name of the manager. + * @property securityProviderRequest - A function for verifying a message, whether it is malicious or not. + * @property state - Initial state to set on this controller. + */ +export type AbstractMessageManagerOptions< + Message extends AbstractMessage, + Action extends ActionConstraint, + Event extends EventConstraint, +> = { + additionalFinishStatuses?: string[]; + messenger: RestrictedControllerMessenger< + string, + Action, + Event | UpdateBadgeEvent, + string, + string + >; + name: string; + securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; +}; /** * Controller in charge of managing - storing, adding, removing, updating - Messages. */ export abstract class AbstractMessageManager< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, -> extends BaseControllerV1> { - protected messages: M[]; - - protected getCurrentChainId: getCurrentChainId | undefined; + Message extends AbstractMessage, + Params extends AbstractMessageParams, + ParamsMetamask extends AbstractMessageParamsMetamask, + Action extends ActionConstraint, + Event extends EventConstraint, +> extends BaseController< + string, + MessageManagerState, + RestrictedControllerMessenger< + string, + Action, + Event | UpdateBadgeEvent, + string, + string + > +> { + protected messages: Message[]; private readonly securityProviderRequest: SecurityProviderRequest | undefined; private readonly additionalFinishStatuses: string[]; + internalEvents = new EventEmitter(); + + constructor({ + additionalFinishStatuses, + messenger, + name, + securityProviderRequest, + state = {} as MessageManagerState, + }: AbstractMessageManagerOptions) { + super({ + messenger, + metadata: stateMetadata, + name, + state: { + ...getDefaultState(), + ...state, + }, + }); + this.messages = []; + this.securityProviderRequest = securityProviderRequest; + this.additionalFinishStatuses = additionalFinishStatuses ?? []; + } + /** * Adds request props to the messsage params and returns a new messageParams object. * @param messageParams - The messageParams to add the request props to. @@ -183,11 +236,16 @@ export abstract class AbstractMessageManager< * @param emitUpdateBadge - Whether to emit the updateBadge event. */ protected saveMessageList(emitUpdateBadge = true) { - const unapprovedMessages = this.getUnapprovedMessages(); - const unapprovedMessagesCount = this.getUnapprovedMessagesCount(); - this.update({ unapprovedMessages, unapprovedMessagesCount }); + this.update((state) => { + state.unapprovedMessages = + this.getUnapprovedMessages() as unknown as Record< + string, + Draft + >; + state.unapprovedMessagesCount = this.getUnapprovedMessagesCount(); + }); if (emitUpdateBadge) { - this.hub.emit('updateBadge'); + this.messagingSystem.publish(`${this.name as string}:updateBadge`); } } @@ -200,18 +258,26 @@ export abstract class AbstractMessageManager< protected setMessageStatus(messageId: string, status: string) { const message = this.getMessage(messageId); if (!message) { - throw new Error(`${this.name}: Message not found for id: ${messageId}.`); + throw new Error( + `${this.name as string}: Message not found for id: ${messageId}.`, + ); } - message.status = status; - this.updateMessage(message); - this.hub.emit(`${messageId}:${status}`, message); + const updatedMessage = { + ...message, + status, + }; + this.updateMessage(updatedMessage); + this.internalEvents.emit(`${messageId}:${status}`, updatedMessage); if ( status === 'rejected' || status === 'signed' || status === 'errored' || this.additionalFinishStatuses.includes(status) ) { - this.hub.emit(`${messageId}:finished`, message); + this.internalEvents.emit( + `${messageId as string}:finished`, + updatedMessage, + ); } } @@ -222,7 +288,7 @@ export abstract class AbstractMessageManager< * @param message - A Message that will replace an existing Message (with the id) in this.messages. * @param emitUpdateBadge - Whether to emit the updateBadge event. */ - protected updateMessage(message: M, emitUpdateBadge = true) { + protected updateMessage(message: Message, emitUpdateBadge = true) { const index = this.messages.findIndex((msg) => message.id === msg.id); /* istanbul ignore next */ if (index !== -1) { @@ -237,7 +303,7 @@ export abstract class AbstractMessageManager< * @param message - The message to verify. * @returns A promise that resolves to a secured message with additional security provider response data. */ - private async securityCheck(message: M): Promise { + private async securityCheck(message: Message): Promise { if (this.securityProviderRequest) { const securityProviderResponse = await this.securityProviderRequest( message, @@ -251,42 +317,11 @@ export abstract class AbstractMessageManager< return message; } - /** - * EventEmitter instance used to listen to specific message events - */ - hub: EventEmitter = new EventEmitter(); - - /** - * Name of this controller used during composition - */ - override name = 'AbstractMessageManager'; - - /** - * Creates an AbstractMessageManager instance. - * - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. - * @param securityProviderRequest - A function for verifying a message, whether it is malicious or not. - * @param additionalFinishStatuses - Optional list of statuses that are accepted to emit a finished event. - * @param getCurrentChainId - Optional function to get the current chainId. - */ - constructor( - config?: Partial, - state?: Partial>, - securityProviderRequest?: SecurityProviderRequest, - additionalFinishStatuses?: string[], - getCurrentChainId?: getCurrentChainId, - ) { - super(config, state); - this.defaultState = { - unapprovedMessages: {}, - unapprovedMessagesCount: 0, - }; - this.messages = []; - this.securityProviderRequest = securityProviderRequest; - this.additionalFinishStatuses = additionalFinishStatuses ?? []; - this.getCurrentChainId = getCurrentChainId; - this.initialize(); + clearUnapprovedMessages() { + this.update((state) => { + state.unapprovedMessages = {}; + state.unapprovedMessagesCount = 0; + }); } /** @@ -306,10 +341,10 @@ export abstract class AbstractMessageManager< getUnapprovedMessages() { return this.messages .filter((message) => message.status === 'unapproved') - .reduce((result: { [key: string]: M }, message: M) => { + .reduce((result: Record, message) => { result[message.id] = message; return result; - }, {}) as { [key: string]: M }; + }, {}); } /** @@ -318,7 +353,7 @@ export abstract class AbstractMessageManager< * * @param message - The Message to add to this.messages. */ - async addMessage(message: M) { + async addMessage(message: Message) { const securedMessage = await this.securityCheck(message); this.messages.push(securedMessage); this.saveMessageList(); @@ -352,7 +387,7 @@ export abstract class AbstractMessageManager< * plus data added by MetaMask. * @returns Promise resolving to the messageParams with the metamaskId property removed. */ - approveMessage(messageParams: PM): Promise

{ + approveMessage(messageParams: ParamsMetamask): Promise { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore this.setMessageStatusApproved(messageParams.metamaskId); @@ -414,8 +449,13 @@ export abstract class AbstractMessageManager< if (!message) { return; } - message.rawSig = result; - this.updateMessage(message, false); + this.updateMessage( + { + ...message, + rawSig: result, + }, + false, + ); } /** @@ -424,14 +464,20 @@ export abstract class AbstractMessageManager< * @param messageId - The id of the Message to update * @param metadata - The data with which to replace the metadata property in the message */ - setMetadata(messageId: string, metadata: Json) { const message = this.getMessage(messageId); if (!message) { - throw new Error(`${this.name}: Message not found for id: ${messageId}.`); + throw new Error( + `${this.name as string}: Message not found for id: ${messageId}.`, + ); } - message.metadata = metadata; - this.updateMessage(message, false); + this.updateMessage( + { + ...message, + metadata, + }, + false, + ); } /** @@ -441,7 +487,9 @@ export abstract class AbstractMessageManager< * @param messageParams - The messageParams to modify * @returns Promise resolving to the messageParams with the metamaskId property removed */ - abstract prepMessageForSigning(messageParams: PM): Promise

; + abstract prepMessageForSigning( + messageParams: ParamsMetamask, + ): Promise; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. @@ -454,7 +502,7 @@ export abstract class AbstractMessageManager< * @returns The id of the newly created message. */ abstract addUnapprovedMessage( - messageParams: PM, + messageParams: ParamsMetamask, request: OriginalRequest, version?: string, ): Promise; @@ -481,34 +529,35 @@ export abstract class AbstractMessageManager< ): Promise { const { metamaskId: messageId, ...messageParams } = messageParamsWithId; return new Promise((resolve, reject) => { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - this.hub.once(`${messageId}:finished`, (data: AbstractMessage) => { - switch (data.status) { - case 'signed': - return resolve(data.rawSig as string); - case 'rejected': - return reject( - new Error( - `MetaMask ${messageName} Signature: User denied message signature.`, - ), - ); - case 'errored': - return reject( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - new Error(`MetaMask ${messageName} Signature: ${data.error}`), - ); - default: - return reject( - new Error( - `MetaMask ${messageName} Signature: Unknown problem: ${JSON.stringify( - messageParams, - )}`, - ), - ); - } - }); + this.internalEvents.once( + `${messageId as string}:finished`, + (data: AbstractMessage) => { + switch (data.status) { + case 'signed': + return resolve(data.rawSig as string); + case 'rejected': + return reject( + new Error( + `MetaMask ${messageName} Signature: User denied message signature.`, + ), + ); + case 'errored': + return reject( + new Error( + `MetaMask ${messageName} Signature: ${data.error as string}`, + ), + ); + default: + return reject( + new Error( + `MetaMask ${messageName} Signature: Unknown problem: ${JSON.stringify( + messageParams, + )}`, + ), + ); + } + }, + ); }); } } diff --git a/packages/message-manager/src/DecryptMessageManager.test.ts b/packages/message-manager/src/DecryptMessageManager.test.ts index d81b336141a..1e59533ae5f 100644 --- a/packages/message-manager/src/DecryptMessageManager.test.ts +++ b/packages/message-manager/src/DecryptMessageManager.test.ts @@ -1,4 +1,18 @@ import { DecryptMessageManager } from './DecryptMessageManager'; +import type { DecryptMessageManagerMessenger } from './DecryptMessageManager'; + +const mockMessenger = { + registerActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), + publish: jest.fn(), + clearEventSubscriptions: jest.fn(), +} as unknown as DecryptMessageManagerMessenger; + +const mockInitialOptions = { + additionalFinishStatuses: undefined, + messenger: mockMessenger, + securityProviderRequest: undefined, +}; describe('DecryptMessageManager', () => { let controller: DecryptMessageManager; @@ -9,7 +23,7 @@ describe('DecryptMessageManager', () => { const dataMock = '0x12345'; beforeEach(() => { - controller = new DecryptMessageManager(); + controller = new DecryptMessageManager(mockInitialOptions); }); it('sets default state', () => { @@ -19,10 +33,6 @@ describe('DecryptMessageManager', () => { }); }); - it('sets default config', () => { - expect(controller.config).toStrictEqual({}); - }); - it('adds a valid message', async () => { const messageData = '0x123'; const messageTime = Date.now(); @@ -52,9 +62,7 @@ describe('DecryptMessageManager', () => { describe('addUnapprovedMessageAsync', () => { beforeEach(() => { - controller = new DecryptMessageManager(undefined, undefined, undefined, [ - 'decrypted', - ]); + controller = new DecryptMessageManager(mockInitialOptions); jest .spyOn(controller, 'addUnapprovedMessage') @@ -72,7 +80,7 @@ describe('DecryptMessageManager', () => { data: dataMock, }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'decrypted', rawSig: rawSigMock, }); @@ -88,7 +96,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -105,7 +113,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'errored', }); }, 100); @@ -122,7 +130,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'unknown', }); }, 100); diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index 4036e79d901..571ca9e760d 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -1,14 +1,52 @@ +import type { + ActionConstraint, + EventConstraint, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, AbstractMessageParams, AbstractMessageParamsMetamask, + MessageManagerState, OriginalRequest, + SecurityProviderRequest, } from './AbstractMessageManager'; import { AbstractMessageManager } from './AbstractMessageManager'; import { normalizeMessageData, validateDecryptedMessageData } from './utils'; +const managerName = 'DecryptMessageManager'; + +export type DecryptMessageManagerState = MessageManagerState; + +export type DecryptMessageManagerUnapprovedMessageAddedEvent = { + type: `${typeof managerName}:unapprovedMessage`; + payload: [AbstractMessageParamsMetamask]; +}; + +export type DecryptMessageManagerUpdateBadgeEvent = { + type: `${typeof managerName}:updateBadge`; + payload: []; +}; + +export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< + string, + ActionConstraint, + | EventConstraint + | DecryptMessageManagerUnapprovedMessageAddedEvent + | DecryptMessageManagerUpdateBadgeEvent, + string, + string +>; + +type DecryptMessageManagerOptions = { + messenger: DecryptMessageManagerMessenger; + securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; + additionalFinishStatuses?: string[]; +}; + /** * @type DecryptMessage * @@ -19,12 +57,9 @@ import { normalizeMessageData, validateDecryptedMessageData } from './utils'; * @property type - The json-prc signing method for which a signature request has been made. * A 'DecryptMessage' which always has a 'eth_decrypt' type */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface DecryptMessage extends AbstractMessage { +export type DecryptMessage = AbstractMessage & { messageParams: DecryptMessageParams; -} +}; /** * @type DecryptMessageParams @@ -32,12 +67,9 @@ export interface DecryptMessage extends AbstractMessage { * Represents the parameters to pass to the eth_decrypt method once the request is approved. * @property data - A hex string conversion of the raw buffer data of the signature request */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface DecryptMessageParams extends AbstractMessageParams { +export type DecryptMessageParams = AbstractMessageParams & { data: string; -} +}; /** * @type DecryptMessageParamsMetamask @@ -63,12 +95,26 @@ export interface DecryptMessageParamsMetamask export class DecryptMessageManager extends AbstractMessageManager< DecryptMessage, DecryptMessageParams, - DecryptMessageParamsMetamask + DecryptMessageParamsMetamask, + ActionConstraint, + | EventConstraint + | DecryptMessageManagerUnapprovedMessageAddedEvent + | DecryptMessageManagerUpdateBadgeEvent > { - /** - * Name of this controller used during composition - */ - override name = 'DecryptMessageManager' as const; + constructor({ + additionalFinishStatuses, + messenger, + securityProviderRequest, + state, + }: DecryptMessageManagerOptions) { + super({ + additionalFinishStatuses, + messenger, + name: managerName, + securityProviderRequest, + state, + }); + } /** * Creates a new Message with an 'unapproved' status using the passed messageParams. @@ -86,32 +132,35 @@ export class DecryptMessageManager extends AbstractMessageManager< const messageId = await this.addUnapprovedMessage(messageParams, req); return new Promise((resolve, reject) => { - this.hub.once(`${messageId}:finished`, (data: DecryptMessage) => { - switch (data.status) { - case 'decrypted': - return resolve(data.rawSig as string); - case 'rejected': - return reject( - new Error( - 'MetaMask DecryptMessage: User denied message decryption.', - ), - ); - case 'errored': - return reject( - new Error( - 'MetaMask DecryptMessage: This message cannot be decrypted.', - ), - ); - default: - return reject( - new Error( - `MetaMask DecryptMessage: Unknown problem: ${JSON.stringify( - messageParams, - )}`, - ), - ); - } - }); + this.internalEvents.once( + `${messageId}:finished`, + (data: DecryptMessage) => { + switch (data.status) { + case 'decrypted': + return resolve(data.rawSig as string); + case 'rejected': + return reject( + new Error( + 'MetaMask DecryptMessage: User denied message decryption.', + ), + ); + case 'errored': + return reject( + new Error( + 'MetaMask DecryptMessage: This message cannot be decrypted.', + ), + ); + default: + return reject( + new Error( + `MetaMask DecryptMessage: Unknown problem: ${JSON.stringify( + messageParams, + )}`, + ), + ); + } + }, + ); }); } @@ -144,7 +193,7 @@ export class DecryptMessageManager extends AbstractMessageManager< const messageId = messageData.id; await this.addMessage(messageData); - this.hub.emit(`unapprovedMessage`, { + this.messagingSystem.publish(`${managerName}:unapprovedMessage`, { ...updatedMessageParams, metamaskId: messageId, }); diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts index 8617c165340..81735a5fdaa 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts @@ -1,4 +1,18 @@ import { EncryptionPublicKeyManager } from './EncryptionPublicKeyManager'; +import type { EncryptionPublicKeyManagerMessenger } from './EncryptionPublicKeyManager'; + +const mockMessenger = { + registerActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), + publish: jest.fn(), + clearEventSubscriptions: jest.fn(), +} as unknown as EncryptionPublicKeyManagerMessenger; + +const mockInitialOptions = { + additionalFinishStatuses: undefined, + messenger: mockMessenger, + securityProviderRequest: undefined, +}; describe('EncryptionPublicKeyManager', () => { let controller: EncryptionPublicKeyManager; @@ -8,7 +22,7 @@ describe('EncryptionPublicKeyManager', () => { const rawSigMock = '231124fe67213512='; beforeEach(() => { - controller = new EncryptionPublicKeyManager(); + controller = new EncryptionPublicKeyManager(mockInitialOptions); }); it('sets default state', () => { @@ -18,10 +32,6 @@ describe('EncryptionPublicKeyManager', () => { }); }); - it('sets default config', () => { - expect(controller.config).toStrictEqual({}); - }); - it('adds a valid message', async () => { const messageTime = Date.now(); const messageStatus = 'unapproved'; @@ -48,12 +58,7 @@ describe('EncryptionPublicKeyManager', () => { describe('addUnapprovedMessageAsync', () => { beforeEach(() => { - controller = new EncryptionPublicKeyManager( - undefined, - undefined, - undefined, - ['received'], - ); + controller = new EncryptionPublicKeyManager(mockInitialOptions); jest .spyOn(controller, 'addUnapprovedMessage') @@ -70,7 +75,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'received', rawSig: rawSigMock, }); @@ -85,7 +90,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -101,7 +106,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'unknown', }); }, 100); diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index 6fdf1518f70..6654bc01329 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -1,14 +1,53 @@ +import type { + ActionConstraint, + EventConstraint, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, AbstractMessageParams, AbstractMessageParamsMetamask, + MessageManagerState, OriginalRequest, + SecurityProviderRequest, } from './AbstractMessageManager'; import { AbstractMessageManager } from './AbstractMessageManager'; import { validateEncryptionPublicKeyMessageData } from './utils'; +const managerName = 'EncryptionPublicKeyManager'; + +export type EncryptionPublicKeyManagerState = + MessageManagerState; + +export type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { + type: `${typeof managerName}:unapprovedMessage`; + payload: [AbstractMessageParamsMetamask]; +}; + +export type EncryptionPublicKeyManagerUpdateBadgeEvent = { + type: `${typeof managerName}:updateBadge`; + payload: []; +}; + +export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< + string, + ActionConstraint, + | EventConstraint + | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EncryptionPublicKeyManagerUpdateBadgeEvent, + string, + string +>; + +type EncryptionPublicKeyManagerOptions = { + messenger: EncryptionPublicKeyManagerMessenger; + securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; + additionalFinishStatuses?: string[]; +}; + /** * @type EncryptionPublicKey * @@ -20,12 +59,9 @@ import { validateEncryptionPublicKeyMessageData } from './utils'; * A 'Message' which always has a 'eth_getEncryptionPublicKey' type * @property rawSig - Encryption public key */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface EncryptionPublicKey extends AbstractMessage { +export type EncryptionPublicKey = AbstractMessage & { messageParams: EncryptionPublicKeyParams; -} +}; /** * @type EncryptionPublicKeyParams @@ -46,13 +82,10 @@ export type EncryptionPublicKeyParams = AbstractMessageParams; * @property from - Address from which to extract the encryption public key * @property origin? - Added for request origin identification */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface EncryptionPublicKeyParamsMetamask - extends AbstractMessageParamsMetamask { - data: string; -} +export type EncryptionPublicKeyParamsMetamask = + AbstractMessageParamsMetamask & { + data: string; + }; /** * Controller in charge of managing - storing, adding, removing, updating - Messages. @@ -60,12 +93,26 @@ export interface EncryptionPublicKeyParamsMetamask export class EncryptionPublicKeyManager extends AbstractMessageManager< EncryptionPublicKey, EncryptionPublicKeyParams, - EncryptionPublicKeyParamsMetamask + EncryptionPublicKeyParamsMetamask, + ActionConstraint, + | EventConstraint + | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EncryptionPublicKeyManagerUpdateBadgeEvent > { - /** - * Name of this controller used during composition - */ - override name = 'EncryptionPublicKeyManager' as const; + constructor({ + additionalFinishStatuses, + messenger, + securityProviderRequest, + state, + }: EncryptionPublicKeyManagerOptions) { + super({ + additionalFinishStatuses, + messenger, + name: managerName, + securityProviderRequest, + state, + }); + } /** * Creates a new Message with an 'unapproved' status using the passed messageParams. @@ -83,26 +130,29 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< const messageId = await this.addUnapprovedMessage(messageParams, req); return new Promise((resolve, reject) => { - this.hub.once(`${messageId}:finished`, (data: EncryptionPublicKey) => { - switch (data.status) { - case 'received': - return resolve(data.rawSig as string); - case 'rejected': - return reject( - new Error( - 'MetaMask EncryptionPublicKey: User denied message EncryptionPublicKey.', - ), - ); - default: - return reject( - new Error( - `MetaMask EncryptionPublicKey: Unknown problem: ${JSON.stringify( - messageParams, - )}`, - ), - ); - } - }); + this.internalEvents.once( + `${messageId}:finished`, + (data: EncryptionPublicKey) => { + switch (data.status) { + case 'received': + return resolve(data.rawSig as string); + case 'rejected': + return reject( + new Error( + 'MetaMask EncryptionPublicKey: User denied message EncryptionPublicKey.', + ), + ); + default: + return reject( + new Error( + `MetaMask EncryptionPublicKey: Unknown problem: ${JSON.stringify( + messageParams, + )}`, + ), + ); + } + }, + ); }); } @@ -134,7 +184,7 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< const messageId = messageData.id; await this.addMessage(messageData); - this.hub.emit(`unapprovedMessage`, { + this.messagingSystem.publish(`${this.name as string}:unapprovedMessage`, { ...updatedMessageParams, metamaskId: messageId, }); diff --git a/packages/network-controller/src/create-network-client.ts b/packages/network-controller/src/create-network-client.ts index dab4702a942..e6620184878 100644 --- a/packages/network-controller/src/create-network-client.ts +++ b/packages/network-controller/src/create-network-client.ts @@ -71,7 +71,6 @@ export function createNetworkClient( const rpcProvider = providerFromMiddleware(rpcApiMiddleware); const blockTrackerOpts = - // eslint-disable-next-line n/no-process-env process.env.IN_TEST && networkConfig.type === 'custom' ? { pollingInterval: SECOND } : {}; @@ -190,7 +189,6 @@ function createCustomNetworkMiddleware({ chainId: Hex; rpcApiMiddleware: JsonRpcMiddleware; }): JsonRpcMiddleware { - // eslint-disable-next-line n/no-process-env const testMiddlewares = process.env.IN_TEST ? [createEstimateGasDelayTestMiddleware()] : []; diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index 3f224b7170f..009fdc95138 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -13069,6 +13069,7 @@ function refreshNetworkTests({ initialState?: Partial; operation: (controller: NetworkController) => Promise; }) { + // eslint-disable-next-line jest/require-top-level-describe it('emits networkWillChange with state payload', async () => { await withController( { @@ -13097,6 +13098,7 @@ function refreshNetworkTests({ ); }); + // eslint-disable-next-line jest/require-top-level-describe it('emits networkDidChange with state payload', async () => { await withController( { @@ -13126,6 +13128,7 @@ function refreshNetworkTests({ }); if (expectedNetworkClientConfiguration.type === NetworkClientType.Custom) { + // eslint-disable-next-line jest/require-top-level-describe it('sets the provider to a custom RPC provider initialized with the RPC target and chain ID', async () => { await withController( { @@ -13167,8 +13170,7 @@ function refreshNetworkTests({ ); }); } else { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + // eslint-disable-next-line jest/require-top-level-describe it(`sets the provider to an Infura provider pointed to ${expectedNetworkClientConfiguration.network}`, async () => { await withController( { @@ -13209,6 +13211,7 @@ function refreshNetworkTests({ }); } + // eslint-disable-next-line jest/require-top-level-describe it('replaces the provider object underlying the provider proxy without creating a new instance of the proxy itself', async () => { await withController( { diff --git a/packages/notification-services-controller/package.json b/packages/notification-services-controller/package.json index bd0d95d4a3b..7515a0a3e62 100644 --- a/packages/notification-services-controller/package.json +++ b/packages/notification-services-controller/package.json @@ -113,7 +113,7 @@ "@lavamoat/preinstall-always-fail": "^2.1.0", "@metamask/auto-changelog": "^3.4.4", "@metamask/keyring-controller": "^19.0.3", - "@metamask/profile-sync-controller": "^4.0.1", + "@metamask/profile-sync-controller": "^4.1.0", "@types/jest": "^27.4.1", "@types/readable-stream": "^2.3.0", "contentful": "^10.15.0", diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/services/push/push-web.ts b/packages/notification-services-controller/src/NotificationServicesPushController/services/push/push-web.ts index ccfab4e40f5..67b46f68d0c 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/services/push/push-web.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/services/push/push-web.ts @@ -20,7 +20,7 @@ import type { PushNotificationEnv } from '../../types/firebase'; declare const self: ServiceWorkerGlobalScope; // Exported to help testing -// eslint-disable-next-line import/no-mutable-exports +// eslint-disable-next-line import-x/no-mutable-exports export let supportedCache: boolean | null = null; const getPushAvailability = async () => { diff --git a/packages/permission-controller/src/PermissionController.ts b/packages/permission-controller/src/PermissionController.ts index b5e33393f86..27cf026d018 100644 --- a/packages/permission-controller/src/PermissionController.ts +++ b/packages/permission-controller/src/PermissionController.ts @@ -1982,7 +1982,6 @@ export class PermissionController< target: string, ): void { if (!isPlainObject(caveat)) { - // eslint-disable-next-line @typescript-eslint/no-throw-literal throw new InvalidCaveatError(caveat, origin, target); } diff --git a/packages/phishing-controller/src/PhishingDetector.test.ts b/packages/phishing-controller/src/PhishingDetector.test.ts index f76b8099650..38e2f2b8b67 100644 --- a/packages/phishing-controller/src/PhishingDetector.test.ts +++ b/packages/phishing-controller/src/PhishingDetector.test.ts @@ -1110,7 +1110,7 @@ describe('PhishingDetector', () => { ]; // CID should be blocked - for await (const entry of expectedToBeBlocked) { + for (const entry of expectedToBeBlocked) { await withPhishingDetector( [ { diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index d52aab938a0..f22455e4b13 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -82,7 +82,6 @@ export function AbstractPollingControllerBaseMixin< const callbacks = this.#callbacks.get(keyToDelete); if (callbacks) { for (const callback of callbacks) { - // eslint-disable-next-line n/callback-return callback(JSON.parse(keyToDelete)); } callbacks.clear(); diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 3e21a911ae4..5cf6aa5b8d8 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [4.1.0] + +### Changed + +- Persist `isAccountSyncingReadyToBeDispatched` state value ([#5147](https://github.com/MetaMask/core/pull/5147)) + ## [4.0.1] ### Added @@ -416,7 +422,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Initial release -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@4.0.1...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@4.1.0...HEAD +[4.1.0]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@4.0.1...@metamask/profile-sync-controller@4.1.0 [4.0.1]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@4.0.0...@metamask/profile-sync-controller@4.0.1 [4.0.0]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@3.3.0...@metamask/profile-sync-controller@4.0.0 [3.3.0]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@3.2.0...@metamask/profile-sync-controller@3.3.0 diff --git a/packages/profile-sync-controller/package.json b/packages/profile-sync-controller/package.json index d109e073f9c..232671b4cd9 100644 --- a/packages/profile-sync-controller/package.json +++ b/packages/profile-sync-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/profile-sync-controller", - "version": "4.0.1", + "version": "4.1.0", "description": "The profile sync helps developers synchronize data across multiple clients and devices in a privacy-preserving way. All data saved in the user storage database is encrypted client-side to preserve privacy. The user storage provides a modular design, giving developers the flexibility to construct and manage their storage spaces in a way that best suits their needs", "keywords": [ "MetaMask", diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts index 619336ccae4..ec620709e3a 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -126,7 +126,7 @@ const metadata: StateMetadata = { anonymous: false, }, isAccountSyncingReadyToBeDispatched: { - persist: false, + persist: true, anonymous: false, }, isAccountSyncingInProgress: { diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts index 63df3b5721b..359520d8294 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts @@ -29,7 +29,7 @@ type PerformMainNetworkSyncProps = { * Ensures that listeners do not fire during main sync (prevent double requests) */ // Exported to help testing -// eslint-disable-next-line import/no-mutable-exports +// eslint-disable-next-line import-x/no-mutable-exports export let isMainNetworkSyncInProgress = false; /** diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index fd975016ca1..847f1b5898e 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -44,7 +44,7 @@ function buildMessenger() { * @param options.getSubjectNames - Permissions controller list of domains with permissions * @returns The network controller restricted messenger. */ -export function buildSelectedNetworkControllerMessenger({ +function buildSelectedNetworkControllerMessenger({ messenger = new ControllerMessenger< SelectedNetworkControllerActions | AllowedActions, SelectedNetworkControllerEvents | AllowedEvents diff --git a/packages/signature-controller/src/SignatureController.ts b/packages/signature-controller/src/SignatureController.ts index 54b8f1e63ec..239cd76386c 100644 --- a/packages/signature-controller/src/SignatureController.ts +++ b/packages/signature-controller/src/SignatureController.ts @@ -30,7 +30,7 @@ import { import type { NetworkControllerGetNetworkClientByIdAction } from '@metamask/network-controller'; import type { Hex, Json } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import { v1 as random } from 'uuid'; @@ -782,7 +782,6 @@ export class SignatureController extends BaseController< const originalStatus = metadata.status; - // eslint-disable-next-line n/callback-return callback(metadata); statusChanged = metadata.status !== originalStatus; @@ -802,7 +801,6 @@ export class SignatureController extends BaseController< #updateState(callback: (state: SignatureControllerState) => void) { return this.update((state) => { - // eslint-disable-next-line n/callback-return, n/no-callback-literal callback(state as unknown as SignatureControllerState); const unapprovedRequests = Object.values(state.signatureRequests).filter( diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index ff7c60f7eaf..a92967f2105 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -47,7 +47,7 @@ import type { Hex } from '@metamask/utils'; import { add0x, hexToNumber } from '@metamask/utils'; import { Mutex } from 'async-mutex'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import { EventEmitter } from 'events'; import { cloneDeep, mapValues, merge, pickBy, sortBy } from 'lodash'; import { v1 as random } from 'uuid'; @@ -3495,7 +3495,6 @@ export class TransactionController extends BaseController< const originalTransactionMeta = cloneDeep(transactionMeta); - // eslint-disable-next-line n/callback-return transactionMeta = callback(transactionMeta) ?? transactionMeta; if (skipValidation !== true) { diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.ts b/packages/transaction-controller/src/helpers/GasFeePoller.ts index b4b87a94dc0..82ad6090fef 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.ts @@ -7,7 +7,7 @@ import type { NetworkClientId, Provider } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import { projectLogger } from '../logger'; diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index 172f0d8c003..f8d97dd5c4c 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -1,7 +1,7 @@ import type { AccountsController } from '@metamask/accounts-controller'; import type { Hex } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import { incomingTransactionsLogger as log } from '../logger'; diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.ts index 54dbdb6ab4a..2542cb67178 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.ts @@ -3,7 +3,7 @@ import { createModuleLogger } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { MethodRegistry } from 'eth-method-registry'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import { projectLogger } from '../logger'; diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 5520fe3a717..557a5d7c302 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -5,7 +5,7 @@ import type { NetworkClientId, } from '@metamask/network-controller'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import { cloneDeep, merge } from 'lodash'; diff --git a/packages/user-operation-controller/src/UserOperationController.ts b/packages/user-operation-controller/src/UserOperationController.ts index 2c70bd80c66..eede19f0893 100644 --- a/packages/user-operation-controller/src/UserOperationController.ts +++ b/packages/user-operation-controller/src/UserOperationController.ts @@ -26,7 +26,7 @@ import { } from '@metamask/transaction-controller'; import { add0x } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import type { Patch } from 'immer'; import { cloneDeep } from 'lodash'; diff --git a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts index 54fa8ce6023..551ac6736a1 100644 --- a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts +++ b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts @@ -8,7 +8,7 @@ import type { import { BlockTrackerPollingControllerOnly } from '@metamask/polling-controller'; import { createModuleLogger, type Hex } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. -// eslint-disable-next-line import/no-nodejs-modules +// eslint-disable-next-line import-x/no-nodejs-modules import EventEmitter from 'events'; import { projectLogger } from '../logger'; diff --git a/scripts/run-eslint.ts b/scripts/run-eslint.ts new file mode 100644 index 00000000000..cabf068470b --- /dev/null +++ b/scripts/run-eslint.ts @@ -0,0 +1,289 @@ +import { ESLint } from 'eslint'; +import fs from 'fs'; +import path from 'path'; +import yargs from 'yargs'; + +const EXISTING_WARNINGS_FILE = path.resolve( + __dirname, + '../eslint-warning-thresholds.json', +); + +/** + * An object mapping rule IDs to their warning counts. + */ +type WarningCounts = Record; + +/** + * An object indicating the difference in warnings for a specific rule. + */ +type WarningComparison = { + /** The ID of the ESLint rule. */ + ruleId: string; + /** The previous count of warnings for the rule. */ + threshold: number; + /** The current count of warnings for the rule. */ + count: number; + /** The difference between the count and the threshold for the rule. */ + difference: number; +}; + +/** + * The warning severity of level of an ESLint rule. + */ +const WARNING = 1; + +// Run the script. +main().catch((error) => { + console.error(error); + process.exitCode = 1; +}); + +/** + * The entrypoint to this script. + */ +async function main() { + const { cache, fix, quiet } = parseCommandLineArguments(); + + const eslint = new ESLint({ cache, fix }); + const results = await runESLint(eslint, { fix, quiet }); + const hasErrors = results.some((result) => result.errorCount > 0); + + if (!quiet && !hasErrors) { + evaluateWarnings(results); + } +} + +/** + * Uses `yargs` to parse the arguments given to the script. + * + * @returns The parsed arguments. + */ +function parseCommandLineArguments() { + return yargs(process.argv.slice(2)) + .option('cache', { + type: 'boolean', + description: 'Cache results to speed up future runs', + default: false, + }) + .option('fix', { + type: 'boolean', + description: 'Automatically fix problems', + default: false, + }) + .option('quiet', { + type: 'boolean', + description: + 'Only report errors, disabling the warnings quality gate in the process', + default: false, + }) + .help().argv; +} + +/** + * Runs ESLint on the project files. + * + * @param eslint - The ESLint instance. + * @param options - The options for running ESLint. + * @param options.quiet - Whether to only report errors (true) or not (false). + * @param options.fix - Whether to automatically fix problems (true) or not + * (false). + * @returns A promise that resolves to the lint results. + */ +async function runESLint( + eslint: ESLint, + options: { quiet: boolean; fix: boolean }, +): Promise { + let results = await eslint.lintFiles(['.']); + + const formatter = await eslint.loadFormatter('stylish'); + const resultText = formatter.format(results); + console.log(resultText); + + if (options.quiet) { + results = ESLint.getErrorResults(results); + } + + if (options.fix) { + await ESLint.outputFixes(results); + } + + return results; +} + +/** + * This function represents the ESLint warnings quality gate, which will cause + * linting to pass or fail depending on how many new warnings have been + * produced. + * + * - If we have no record of warnings from a previous run, then we simply + * capture the new warnings in a file and continue. + * - If we have a record of warnings from a previous run and there are any + * changes to the number of warnings overall, then we list which ESLint rules + * had increases and decreases. If are were more warnings overall then we fail, + * otherwise we pass. + * + * @param results - The results of running ESLint. + */ +function evaluateWarnings(results: ESLint.LintResult[]) { + const warningThresholds = loadWarningThresholds(); + const warningCounts = getWarningCounts(results); + + if (Object.keys(warningThresholds).length === 0) { + console.log( + 'The following ESLint warnings were produced and will be captured as thresholds for future runs:\n', + ); + for (const [ruleId, count] of Object.entries(warningCounts)) { + console.log(`- ${ruleId}: ${count}`); + } + saveWarningThresholds(warningCounts); + } else { + const comparisons = compareWarnings(warningThresholds, warningCounts); + + const changes = comparisons.filter( + (comparison) => comparison.difference !== 0, + ); + const regressions = comparisons.filter( + (comparison) => comparison.difference > 0, + ); + + if (changes.length > 0) { + if (regressions.length > 0) { + console.log( + '🛑 New ESLint warnings have been introduced and need to be resolved for linting to pass:\n', + ); + process.exitCode = 1; + } else { + console.log( + 'The overall number of ESLint warnings have decreased, good work! ❤️ \n', + ); + } + for (const { ruleId, threshold, count, difference } of changes) { + console.log( + `- ${ruleId}: ${threshold} -> ${count} (${difference > 0 ? '+' : ''}${difference})`, + ); + } + // We are still seeing differences on CI when it comes to linting results. + // Only write the thresholds file if there are regressions in that case + // to prevent the "working directory clean" step in CI from failing. + // Also, it's okay to use `process.env` as this is a script. + // eslint-disable-next-line n/no-process-env + if (regressions.length > 0 || !process.env.CI) { + saveWarningThresholds(warningCounts); + } + } + } +} + +/** + * Loads previous warning counts from a file. + * + * @returns An object mapping rule IDs to their previous warning counts. + */ +function loadWarningThresholds(): WarningCounts { + if (fs.existsSync(EXISTING_WARNINGS_FILE)) { + const data = fs.readFileSync(EXISTING_WARNINGS_FILE, 'utf-8'); + return JSON.parse(data); + } + return {}; +} + +/** + * Saves current warning counts to a file so they can be used for a future run. + * + * @param warningCounts - An object mapping rule IDs to their current warning + * counts. + */ +function saveWarningThresholds(warningCounts: WarningCounts): void { + fs.writeFileSync( + EXISTING_WARNINGS_FILE, + `${JSON.stringify(warningCounts, null, 2)}\n`, + 'utf-8', + ); +} + +/** + * Given a list of results from an the ESLint run, counts the number of warnings + * produced per rule. + * + * @param results - The ESLint results. + * @returns An object mapping rule IDs to their warning counts, sorted by rule + * ID. + */ +function getWarningCounts(results: ESLint.LintResult[]): WarningCounts { + const warningCounts = results.reduce((acc, result) => { + for (const message of result.messages) { + if (message.severity === WARNING && message.ruleId) { + acc[message.ruleId] = (acc[message.ruleId] ?? 0) + 1; + } + } + return acc; + }, {} as WarningCounts); + + return Object.keys(warningCounts) + .sort(sortRules) + .reduce((sortedWarningCounts, key) => { + return { ...sortedWarningCounts, [key]: warningCounts[key] }; + }, {} as WarningCounts); +} + +/** + * Compares previous and current warning counts. + * + * @param warningThresholds - An object mapping rule IDs to the warning + * thresholds established in a previous run. + * @param warningCounts - An object mapping rule IDs to the current warning + * counts. + * @returns An array of objects indicating comparisons in warnings. + */ +function compareWarnings( + warningThresholds: WarningCounts, + warningCounts: WarningCounts, +): WarningComparison[] { + const ruleIds = Array.from( + new Set([...Object.keys(warningThresholds), ...Object.keys(warningCounts)]), + ); + return ruleIds + .map((ruleId) => { + const threshold = warningThresholds[ruleId] ?? 0; + const count = warningCounts[ruleId] ?? 0; + const difference = count - threshold; + return { ruleId, threshold, count, difference }; + }) + .sort((a, b) => sortRules(a.ruleId, b.ruleId)); +} + +/** + * Sorts rule IDs, ensuring that rules with namespaces appear before rules + * without. + * + * @param ruleIdA - The first rule ID. + * @param ruleIdB - The second rule ID. + * @returns A negative number if the first rule ID should come before the + * second, a positive number if the first should come _after_ the second, or 0 + * if they should stay where they are. + * @example + * ``` typescript + * sortRules( + * '@typescript-eslint/naming-convention', + * '@typescript-eslint/explicit-function-return-type' + * ) //=> 1 (sort A after B) + * sortRules( + * 'explicit-function-return-type', + * '@typescript-eslint/naming-convention' + * ) //=> 1 (sort A after B) + */ +function sortRules(ruleIdA: string, ruleIdB: string): number { + const [namespaceA, ruleA] = ruleIdA.includes('/') + ? ruleIdA.split('/') + : ['', ruleIdA]; + const [namespaceB, ruleB] = ruleIdB.includes('/') + ? ruleIdB.split('/') + : ['', ruleIdB]; + if (namespaceA && !namespaceB) { + return -1; + } + if (!namespaceA && namespaceB) { + return 1; + } + return namespaceA.localeCompare(namespaceB) || ruleA.localeCompare(ruleB); +} diff --git a/yarn.config.cjs b/yarn.config.cjs index b802565a6a0..aa8f334e172 100644 --- a/yarn.config.cjs +++ b/yarn.config.cjs @@ -1,4 +1,9 @@ -// @ts-check +/* @ts-check */ +// The ESLint JSDoc plugin usually disables this rule for TypeScript files, +// but for JavaScript files we are typechecking, we need to disable it manually. +// See: +/* eslint-disable jsdoc/no-undefined-types */ + // This file is used to define, among other configuration, rules that Yarn will // execute when you run `yarn constraints`. These rules primarily check the // manifests of each package in the monorepo to ensure they follow a standard diff --git a/yarn.lock b/yarn.lock index d28def18d56..f88317fe241 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2555,6 +2555,7 @@ __metadata: eth-ens-namehash: "npm:^2.0.8" fast-deep-equal: "npm:^3.1.3" jest: "npm:^27.5.1" + jest-environment-jsdom: "npm:^27.5.1" nock: "npm:^13.3.1" sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.4" @@ -3415,7 +3416,7 @@ __metadata: "@metamask/base-controller": "npm:^7.1.1" "@metamask/controller-utils": "npm:^11.4.5" "@metamask/keyring-controller": "npm:^19.0.3" - "@metamask/profile-sync-controller": "npm:^4.0.1" + "@metamask/profile-sync-controller": "npm:^4.1.0" "@metamask/utils": "npm:^11.0.1" "@types/jest": "npm:^27.4.1" "@types/readable-stream": "npm:^2.3.0" @@ -3596,7 +3597,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/profile-sync-controller@npm:^4.0.1, @metamask/profile-sync-controller@workspace:packages/profile-sync-controller": +"@metamask/profile-sync-controller@npm:^4.1.0, @metamask/profile-sync-controller@workspace:packages/profile-sync-controller": version: 0.0.0-use.local resolution: "@metamask/profile-sync-controller@workspace:packages/profile-sync-controller" dependencies: @@ -5784,11 +5785,11 @@ __metadata: linkType: hard "acorn-walk@npm:^8.1.1": - version: 8.3.3 - resolution: "acorn-walk@npm:8.3.3" + version: 8.3.4 + resolution: "acorn-walk@npm:8.3.4" dependencies: acorn: "npm:^8.11.0" - checksum: 10/59701dcb7070679622ba8e9c7f37577b4935565747ca0fd7c1c3ad30b3f1b1b008276282664e323b5495eb49f77fa12d3816fd06dc68e18f90fbebe759f71450 + checksum: 10/871386764e1451c637bb8ab9f76f4995d408057e9909be6fb5ad68537ae3375d85e6a6f170b98989f44ab3ff6c74ad120bc2779a3d577606e7a0cd2b4efcaf77 languageName: node linkType: hard @@ -10933,9 +10934,9 @@ __metadata: linkType: hard "nwsapi@npm:^2.2.0": - version: 2.2.12 - resolution: "nwsapi@npm:2.2.12" - checksum: 10/172119e9ef492467ebfb337f9b5fd12a94d2b519377cde3f6ec2f74a86f6d5c00ef3873539bed7142f908ffca4e35383179be2319d04a563071d146bfa3f1673 + version: 2.2.16 + resolution: "nwsapi@npm:2.2.16" + checksum: 10/1e5e086cdd4ca4a45f414d37f49bf0ca81d84ed31c6871ac68f531917d2910845db61f77c6d844430dc90fda202d43fce9603024e74038675de95229eb834dba languageName: node linkType: hard