Merge branch '230113-consolidate-TokenDetectionController-DetectToken…

…sController' into 231220-TokensController-v2-migration

constraints.pro

@@ -117,7 +117,7 @@ repo_name(RepoUrl, RepoName) :-
   RepoNameLength is End - Start,
   sub_atom(RepoUrl, PrefixLength, RepoNameLength, SuffixLength, RepoName).
 
-% True if DependencyIdent starts with '@metamask' and ends with '-controller'
+% True if DependencyIdent starts with '@metamask' and ends with '-controller'.
 is_controller(DependencyIdent) :-
   Prefix = '@metamask/',
   atom_length(Prefix, PrefixLength),
@@ -308,43 +308,80 @@ gen_enforced_dependency(WorkspaceCwd, DependencyIdent, 'a range optionally start
   workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, DependencyType),
   \+ is_valid_version_range(DependencyRange).
 
-% All references to a workspace package must be up to date with the current
-% version of that package.
-gen_enforced_dependency(WorkspaceCwd, DependencyIdent, CorrectDependencyRange, DependencyType) :-
-  workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, DependencyType),
-  workspace_ident(OtherWorkspaceCwd, DependencyIdent),
-  workspace_version(OtherWorkspaceCwd, OtherWorkspaceVersion),
-  atomic_list_concat(['^', OtherWorkspaceVersion], CorrectDependencyRange).
-
-% All dependency ranges for a package must be synchronized across the monorepo
-% (the least version range wins), regardless of which "*dependencies" field
-% where the package appears.
+% All version ranges used to reference one workspace package in another
+% workspace package's `dependencies` or `devDependencies` must be the same.
+% Among all references to the same dependency across the monorepo, the one with
+% the smallest version range will win. (We handle `peerDependencies` in another
+% constraint, as it has slightly different logic.)
 gen_enforced_dependency(WorkspaceCwd, DependencyIdent, OtherDependencyRange, DependencyType) :-
   workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, DependencyType),
   workspace_has_dependency(OtherWorkspaceCwd, DependencyIdent, OtherDependencyRange, OtherDependencyType),
   WorkspaceCwd \= OtherWorkspaceCwd,
   DependencyRange \= OtherDependencyRange,
-  npm_version_range_out_of_sync(DependencyRange, OtherDependencyRange).
+  npm_version_range_out_of_sync(DependencyRange, OtherDependencyRange),
+  DependencyType \= 'peerDependencies',
+  OtherDependencyType \= 'peerDependencies'.
 
-% If a dependency is listed under "dependencies", it should not be listed under
-% "devDependencies". We match on the same dependency range so that if a
-% dependency is listed under both lists, their versions are synchronized and
-% then this constraint will apply and remove the "right" duplicate.
-gen_enforced_dependency(WorkspaceCwd, DependencyIdent, null, DependencyType) :-
-  workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, 'dependencies'),
+% All version ranges used to reference one workspace package in another
+% workspace package's `dependencies` or `devDependencies` must match the current
+% version of that package. (We handle `peerDependencies` in another rule.)
+gen_enforced_dependency(WorkspaceCwd, DependencyIdent, CorrectDependencyRange, DependencyType) :-
+  DependencyType \= 'peerDependencies',
   workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, DependencyType),
-  DependencyType == 'devDependencies'.
+  workspace_ident(OtherWorkspaceCwd, DependencyIdent),
+  workspace_version(OtherWorkspaceCwd, OtherWorkspaceVersion),
+  atomic_list_concat(['^', OtherWorkspaceVersion], CorrectDependencyRange).
 
-% If a controller dependency (other than `base-controller`, `eth-keyring-controller` and
-% `polling-controller`) is listed under "dependencies", it should also be
-% listed under "peerDependencies". Each controller is a singleton, so we need
-% to ensure the versions used match expectations.
-gen_enforced_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, 'peerDependencies') :-
+% If a workspace package is listed under another workspace package's
+% `dependencies`, it should not also be listed under its `devDependencies`.
+gen_enforced_dependency(WorkspaceCwd, DependencyIdent, null, 'devDependencies') :-
+  workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, 'dependencies').
+
+% Each controller is a singleton, so we need to ensure the versions
+% used match expectations. To accomplish this, if a controller (other than
+% `base-controller`, `eth-keyring-controller` and `polling-controller`) is
+% listed under a workspace package's `dependencies`, it should also be listed
+% under its `peerDependencies`, and the major version of the peer dependency
+% should match the major part of the current version dependency, with the minor
+% and patch parts set to 0. If it is already listed there, then the major
+% version should match the current version of the package and the minor and
+% patch parts should be <= the corresponding parts.
+gen_enforced_dependency(WorkspaceCwd, DependencyIdent, CorrectPeerDependencyRange, 'peerDependencies') :-
   workspace_has_dependency(WorkspaceCwd, DependencyIdent, DependencyRange, 'dependencies'),
+  \+ workspace_has_dependency(WorkspaceCwd, DependencyIdent, _, 'peerDependencies'),
+  is_controller(DependencyIdent),
   DependencyIdent \= '@metamask/base-controller',
   DependencyIdent \= '@metamask/eth-keyring-controller',
   DependencyIdent \= '@metamask/polling-controller',
-  is_controller(DependencyIdent).
+  workspace_ident(DependencyWorkspaceCwd, DependencyIdent),
+  workspace_version(DependencyWorkspaceCwd, CurrentDependencyWorkspaceVersion),
+  parse_version_range(CurrentDependencyWorkspaceVersion, _, CurrentDependencyVersionMajor, _, _),
+  atomic_list_concat([CurrentDependencyVersionMajor, 0, 0], '.', CorrectPeerDependencyVersion),
+  atom_concat('^', CorrectPeerDependencyVersion, CorrectPeerDependencyRange).
+gen_enforced_dependency(WorkspaceCwd, DependencyIdent, CorrectPeerDependencyRange, 'peerDependencies') :-
+  workspace_has_dependency(WorkspaceCwd, DependencyIdent, SpecifiedPeerDependencyRange, 'peerDependencies'),
+  is_controller(DependencyIdent),
+  DependencyIdent \= '@metamask/base-controller',
+  DependencyIdent \= '@metamask/eth-keyring-controller',
+  DependencyIdent \= '@metamask/polling-controller',
+  workspace_ident(DependencyWorkspaceCwd, DependencyIdent),
+  workspace_version(DependencyWorkspaceCwd, CurrentDependencyVersion),
+  parse_version_range(CurrentDependencyVersion, _, CurrentDependencyVersionMajor, CurrentDependencyVersionMinor, CurrentDependencyVersionPatch),
+  parse_version_range(SpecifiedPeerDependencyRange, _, SpecifiedPeerDependencyVersionMajor, SpecifiedPeerDependencyVersionMinor, SpecifiedPeerDependencyVersionPatch),
+  (
+    (
+      SpecifiedPeerDependencyVersionMajor == CurrentDependencyVersionMajor,
+      (
+        SpecifiedPeerDependencyVersionMinor @< CurrentDependencyVersionMinor ;
+        (
+          SpecifiedPeerDependencyVersionMinor == CurrentDependencyVersionMinor,
+          SpecifiedPeerDependencyVersionPatch @=< CurrentDependencyVersionPatch
+        )
+      )
+    ) ->
+      CorrectPeerDependencyRange = SpecifiedPeerDependencyRange ;
+      atom_concat('^', CurrentDependencyVersion, CorrectPeerDependencyRange)
+  ).
 
 % All packages must specify a minimum Node version of 16.
 gen_enforced_field(WorkspaceCwd, 'engines.node', '>=16.0.0').

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@metamask/core-monorepo",
-  "version": "113.0.0",
+  "version": "115.0.0",
   "private": true,
   "description": "Monorepo for packages shared between MetaMask clients",
   "repository": {

packages/assets-controllers/src/TokenDetectionController.ts

@@ -249,6 +249,7 @@ export class TokenDetectionController extends StaticIntervalPollingController<
 
     this.messagingSystem.subscribe('KeyringController:lock', () => {
       this.#isUnlocked = false;
+      this.#stopPolling();
     });
 
     this.messagingSystem.subscribe(
@@ -313,7 +314,7 @@ export class TokenDetectionController extends StaticIntervalPollingController<
 
         if (isNetworkClientIdChanged && this.#isDetectionEnabledForNetwork) {
           this.#networkClientId = selectedNetworkClientId;
-          await this.detectTokens({
+          await this.#restartTokenDetection({
             networkClientId: this.#networkClientId,
           });
         }

packages/composable-controller/src/ComposableController.test.ts

@@ -120,21 +120,6 @@ describe('ComposableController', () => {
       });
     });
 
-    it('should compose flat controller state', () => {
-      const composableMessenger = new ControllerMessenger().getRestricted({
-        name: 'ComposableController',
-      });
-      const controller = new ComposableController({
-        controllers: [new BarController(), new BazController()],
-        messenger: composableMessenger,
-      });
-
-      expect(controller.flatState).toStrictEqual({
-        bar: 'bar',
-        baz: 'baz',
-      });
-    });
-
     it('should notify listeners of nested state change', () => {
       const controllerMessenger = new ControllerMessenger<
         never,
@@ -188,28 +173,6 @@ describe('ComposableController', () => {
       });
     });
 
-    it('should compose flat controller state', () => {
-      const controllerMessenger = new ControllerMessenger<
-        never,
-        FooControllerEvent
-      >();
-      const fooControllerMessenger = controllerMessenger.getRestricted({
-        name: 'FooController',
-      });
-      const fooController = new FooController(fooControllerMessenger);
-      const composableControllerMessenger = controllerMessenger.getRestricted({
-        name: 'ComposableController',
-        allowedEvents: ['FooController:stateChange'],
-      });
-      const composableController = new ComposableController({
-        controllers: [fooController],
-        messenger: composableControllerMessenger,
-      });
-      expect(composableController.flatState).toStrictEqual({
-        foo: 'foo',
-      });
-    });
-
     it('should notify listeners of nested state change', () => {
       const controllerMessenger = new ControllerMessenger<
         never,
@@ -273,30 +236,6 @@ describe('ComposableController', () => {
       });
     });
 
-    it('should compose flat controller state', () => {
-      const barController = new BarController();
-      const controllerMessenger = new ControllerMessenger<
-        never,
-        FooControllerEvent
-      >();
-      const fooControllerMessenger = controllerMessenger.getRestricted({
-        name: 'FooController',
-      });
-      const fooController = new FooController(fooControllerMessenger);
-      const composableControllerMessenger = controllerMessenger.getRestricted({
-        name: 'ComposableController',
-        allowedEvents: ['FooController:stateChange'],
-      });
-      const composableController = new ComposableController({
-        controllers: [barController, fooController],
-        messenger: composableControllerMessenger,
-      });
-      expect(composableController.flatState).toStrictEqual({
-        bar: 'bar',
-        foo: 'foo',
-      });
-    });
-
     it('should notify listeners of BaseControllerV1 state change', () => {
       const barController = new BarController();
       const controllerMessenger = new ControllerMessenger<

packages/composable-controller/src/ComposableController.ts

@@ -99,21 +99,6 @@ export class ComposableController extends BaseController<
     );
   }
 
-  /**
-   * Flat state representation, one that isn't keyed
-   * of controller name. Instead, all child controller state is merged
-   * together into a single, flat object.
-   *
-   * @returns Merged state representation of all child controllers.
-   */
-  get flatState() {
-    let flatState = {};
-    for (const controller of this.#controllers) {
-      flatState = { ...flatState, ...controller.state };
-    }
-    return flatState;
-  }
-
   /**
    * Adds a child controller instance to composable controller state,
    * or updates the state of a child controller.

packages/keyring-controller/jest.config.js

@@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 100,
+      branches: 95.71,
       functions: 100,
-      lines: 100,
-      statements: 100,
+      lines: 99.21,
+      statements: 99.22,
     },
   },
 

packages/keyring-controller/package.json

@@ -33,7 +33,10 @@
   "dependencies": {
     "@keystonehq/metamask-airgapped-keyring": "^0.13.1",
     "@metamask/base-controller": "^4.1.1",
-    "@metamask/eth-keyring-controller": "^17.0.1",
+    "@metamask/browser-passworder": "^4.3.0",
+    "@metamask/eth-hd-keyring": "^7.0.1",
+    "@metamask/eth-sig-util": "^7.0.1",
+    "@metamask/eth-simple-keyring": "^6.0.1",
     "@metamask/keyring-api": "^3.0.0",
     "@metamask/message-manager": "^7.3.8",
     "@metamask/utils": "^8.3.0",
@@ -48,7 +51,6 @@
     "@keystonehq/bc-ur-registry-eth": "^0.9.0",
     "@lavamoat/allow-scripts": "^2.3.1",
     "@metamask/auto-changelog": "^3.4.4",
-    "@metamask/eth-sig-util": "^7.0.1",
     "@metamask/scure-bip39": "^2.1.1",
     "@types/jest": "^27.4.1",
     "deepmerge": "^4.2.2",

packages/keyring-controller/src/KeyringController.test.ts

@@ -3,10 +3,7 @@ import { TransactionFactory } from '@ethereumjs/tx';
 import { CryptoHDKey, ETHSignature } from '@keystonehq/bc-ur-registry-eth';
 import { MetaMaskKeyring as QRKeyring } from '@keystonehq/metamask-airgapped-keyring';
 import { ControllerMessenger } from '@metamask/base-controller';
-import {
-  KeyringControllerError,
-  keyringBuilderFactory,
-} from '@metamask/eth-keyring-controller';
+import HDKeyring from '@metamask/eth-hd-keyring';
 import {
   normalize,
   recoverPersonalSignature,
@@ -34,6 +31,7 @@ import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring';
 import { MockKeyring } from '../tests/mocks/mockKeyring';
 import MockShallowGetAccountsKeyring from '../tests/mocks/mockShallowGetAccountsKeyring';
 import { buildMockTransaction } from '../tests/mocks/mockTransaction';
+import { KeyringControllerError } from './constants';
 import type {
   KeyringControllerEvents,
   KeyringControllerMessenger,
@@ -45,6 +43,7 @@ import {
   AccountImportStrategy,
   KeyringController,
   KeyringTypes,
+  keyringBuilderFactory,
 } from './KeyringController';
 
 jest.mock('uuid', () => {
@@ -527,6 +526,20 @@ describe('KeyringController', () => {
               },
             );
           });
+
+          it('should throw error if the first account is not found on the keyring', async () => {
+            jest
+              .spyOn(HDKeyring.prototype, 'getAccounts')
+              .mockResolvedValue([]);
+            await withController(
+              { skipVaultCreation: true },
+              async ({ controller }) => {
+                await expect(
+                  controller.createNewVaultAndKeychain(password),
+                ).rejects.toThrow(KeyringControllerError.NoFirstAccount);
+              },
+            );
+          });
         });
 
         describe('when there is an existing vault', () => {
@@ -1214,7 +1227,7 @@ describe('KeyringController', () => {
               data: '',
               from: initialState.keyrings[0].accounts[0],
             }),
-          ).toThrow("Can't sign an empty message");
+          ).rejects.toThrow("Can't sign an empty message");
         });
       });