feat: bump keyring controller 7.5.0 (#8035)

## **Description** This PR bumps the `@metamask/keyring-controller` version from `6.0.0` to `7.5.0`. These are the relevant changes to take into consideration during review, - **BREAKING**: Remove `keyringTypes` property from the KeyringController state ([#1441](MetaMask/core#1441)) - **BREAKING**: Constructor `KeyringControllerOptions` type changed ([#1441](MetaMask/core#1441)) - The `KeyringControllerOptions.state` accepted type is now `{ vault?: string }` - The `KeyringControllerOptions.keyringBuilders` type is now `{ (): Keyring<Json>; type: string }[]` - **BREAKING**: The `address` type accepted by the `removeAccount` method is now `Hex` ([#1441](MetaMask/core#1441)) - **BREAKING**: The `signTypedMessage` method now returns a `Promise<string>` ([#1441](MetaMask/core#1441)) - **BREAKING**: The `signTransaction` method now requires a `TypedTransaction` from `@ethereumjs/tx@^4` for the `transaction` argument, and returns a `Promise<TxData>` ([#1441](MetaMask/core#1441)) - **BREAKING:** Rename `Keyring` type to `KeyringObject` ([#1441](MetaMask/core#1441)) - **BREAKING:** `addNewAccount` now throws if address of new account is not a hex string ([#1441](MetaMask/core#1441)) - **BREAKING:** `exportSeedPhrase` now throws if first keyring does not have a mnemonic ([#1441](MetaMask/core#1441)) - **BREAKING:** `verifySeedPhrase` now throws if HD keyring does not have a mnemonic ([#1441](MetaMask/core#1441)) ## **Related issues** Fixes: #8180 ## **Manual testing steps** Use cases or flows to verify, 1. Onboarding 2. Import SRP 3. Import private key 4. Reveal SRP 5. Reveal private key 6. Add new account 7. Connect QR wallet 8. Signed type messages 9. Remove imported account 10. Lock and unlock wallet ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [X ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [X ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · Jan 6, 2024 · 445ef2b · 445ef2b
1 parent 2b409a5
commit 445ef2b
Show file tree

Hide file tree

Showing 9 changed files with 154 additions and 163 deletions.
diff --git a/app/components/UI/AccountSelectorList/AccountSelectorList.tsx b/app/components/UI/AccountSelectorList/AccountSelectorList.tsx
@@ -4,6 +4,7 @@ import { Alert, ListRenderItem, Platform, View } from 'react-native';
 import { FlatList } from 'react-native-gesture-handler';
 import { useSelector } from 'react-redux';
 import { KeyringTypes } from '@metamask/keyring-controller';
+import type { Hex } from '@metamask/utils';
 
 // External dependencies.
 import Cell, {
@@ -80,7 +81,7 @@ const AccountSelectorList = ({
       isSelected,
       index,
     }: {
-      address: string;
+      address: Hex;
       imported: boolean;
       isSelected: boolean;
       index: number;

diff --git a/app/core/BackupVault/backupVault.test.ts b/app/core/BackupVault/backupVault.test.ts
@@ -17,7 +17,6 @@ describe('backupVault', () => {
       vault: undefined,
       keyrings: [],
       isUnlocked: false,
-      keyringTypes: [],
     };
     const response = await backupVault(keyringState);
     expect(response.success).toBe(false);

diff --git a/app/core/Engine.test.js b/app/core/Engine.test.js
@@ -63,6 +63,14 @@ describe('Engine', () => {
           },
         },
       },
+      KeyringController: {
+        ...backgroundState.KeyringController,
+        vault: {
+          cipher: 'mock-cipher',
+          iv: 'mock-iv',
+          lib: 'original',
+        },
+      },
     };
 
     expect(backgroundState).toStrictEqual(initialState);

diff --git a/app/core/Engine.ts b/app/core/Engine.ts
@@ -548,6 +548,7 @@ class Engine {
         allowedActions: ['KeyringController:getState'],
       }),
       state: initialKeyringState || initialState.KeyringController,
+      // @ts-expect-error To Do: Update the type of QRHardwareKeyring to Keyring<Json>
       keyringBuilders: [qrKeyringBuilder],
     });
 

diff --git a/app/core/Ledger/Ledger.ts b/app/core/Ledger/Ledger.ts
@@ -1,10 +1,8 @@
 import Engine from '../Engine';
-import {
-  KeyringController,
-  SignTypedDataVersion,
-} from '@metamask/keyring-controller';
+import { SignTypedDataVersion } from '@metamask/keyring-controller';
 import ExtendedKeyringTypes from '../../constants/keyringTypes';
 
+// Mock interface for the SerializationOptions
 interface SerializationOptions {
   vault: any;
   keyrings: any[];
@@ -13,6 +11,7 @@ interface SerializationOptions {
   encryptionSalt: string;
 }
 
+// Mock interface for the LedgerKeyring
 interface LedgerKeyring {
   setTransport: (transport: any, deviceId: string) => void;
   getAppAndVersion: () => Promise<{ appName: string }>;
@@ -25,52 +24,27 @@ interface LedgerKeyring {
   getName: () => string;
 }
 
-/**
- * Get EthKeyringController from KeyringController
- *
- * @returns The EthKeyringController
- */
-const getEthKeyringController = () => {
-  const keyringController: KeyringController = Engine.context.KeyringController;
-  return keyringController.getEthKeyringController();
-};
-
-/**
- * A method replicate #fullupdate from the keyring controller.
- */
-export const syncKeyringState = async (): Promise<void> => {
-  const keyringController = Engine.context.KeyringController;
-  const ethKeyringController = getEthKeyringController();
-  const { vault } = ethKeyringController.store.getState();
-  const { keyrings, isUnlocked, encryptionKey, encryptionSalt } =
-    ethKeyringController.memStore.getState();
-
-  keyringController.update(() => ({
-    vault,
-    keyrings,
-    isUnlocked,
-    encryptionKey,
-    encryptionSalt,
-  }));
-};
-
 /**
  * Add LedgerKeyring.
  *
  * @returns The Ledger Keyring
  */
-export const addLedgerKeyring = async (): Promise<LedgerKeyring> =>
-  (await getEthKeyringController().addNewKeyring(
+export const addLedgerKeyring = async (): Promise<LedgerKeyring> => {
+  const keyringController = Engine.context.KeyringController;
+  return (await keyringController.addNewKeyring(
     ExtendedKeyringTypes.ledger,
   )) as unknown as LedgerKeyring;
+};
 
 /**
  * Retrieve the existing LedgerKeyring or create a new one.
  *
  * @returns The stored Ledger Keyring
  */
 export const getLedgerKeyring = async (): Promise<LedgerKeyring> => {
-  const keyring = getEthKeyringController().getKeyringsByType(
+  const keyringController = Engine.context.KeyringController;
+  // There should only be one ledger keyring.
+  const keyring = keyringController().getKeyringsByType(
     ExtendedKeyringTypes.ledger,
   )[0] as unknown as LedgerKeyring;
 
@@ -90,11 +64,9 @@ export const restoreLedgerKeyring = async (
   keyringSerialized: SerializationOptions,
 ): Promise<void> => {
   const keyringController = Engine.context.KeyringController;
-  const ethKeyringController = getEthKeyringController();
 
   (await getLedgerKeyring()).deserialize(keyringSerialized);
-  keyringController.updateIdentities(await ethKeyringController.getAccounts());
-  await syncKeyringState();
+  keyringController.updateIdentities(await keyringController.getAccounts());
 };
 
 /**
@@ -124,11 +96,11 @@ export const unlockLedgerDefaultAccount = async (): Promise<{
   balance: string;
 }> => {
   const keyringController = Engine.context.KeyringController;
-  const ethKeyringController = getEthKeyringController();
+  const preferencesController = Engine.context.PreferencesController;
   const keyring = await getLedgerKeyring();
-  const oldAccounts = await ethKeyringController.getAccounts();
-  await ethKeyringController.addNewAccount(keyring);
-  const newAccounts = await ethKeyringController.getAccounts();
+  const oldAccounts = await keyringController.getAccounts();
+  await keyringController.addNewAccountForKeyring(keyring);
+  const newAccounts = await keyringController.getAccounts();
 
   keyringController.updateIdentities(newAccounts);
   newAccounts.forEach((address: string) => {
@@ -137,11 +109,10 @@ export const unlockLedgerDefaultAccount = async (): Promise<{
         // The first ledger account is always returned.
         keyringController.setAccountLabel(address, `${keyring.getName()} 1`);
       }
-      keyringController.setSelectedAddress(address);
+      preferencesController.setSelectedAddress(address);
     }
   });
-  await ethKeyringController.persistAllKeyrings();
-  await syncKeyringState();
+  await keyringController.persistAllKeyrings();
 
   const address = await keyring.getDefaultAccount();
   return {
@@ -171,15 +142,14 @@ export const closeRunningAppOnLedger = async (): Promise<void> => {
  */
 export const forgetLedger = async (): Promise<void> => {
   const keyringController = Engine.context.KeyringController;
-  const ethKeyringController = getEthKeyringController();
+  const preferencesController = Engine.context.PreferencesController;
   const keyring = await getLedgerKeyring();
   keyring.forgetDevice();
 
-  const accounts: string[] = await ethKeyringController.getAccounts();
-  keyringController.setSelectedAddress(accounts[0]);
+  const accounts: string[] = await keyringController.getAccounts();
+  preferencesController.setSelectedAddress(accounts[0]);
 
-  await ethKeyringController.persistAllKeyrings();
-  await syncKeyringState();
+  await keyringController.persistAllKeyrings();
 };
 
 /**
@@ -205,7 +175,8 @@ export const ledgerSignTypedMessage = async (
   version: SignTypedDataVersion,
 ): Promise<string> => {
   await getLedgerKeyring();
-  return await getEthKeyringController().signTypedMessage(
+  const keyringController = Engine.context.KeyringController;
+  return await keyringController.signTypedMessage(
     {
       from: messageParams.from,
       data: messageParams.data as

diff --git a/app/util/test/initial-background-state.json b/app/util/test/initial-background-state.json
@@ -1,8 +1,12 @@
 {
   "KeyringController": {
     "isUnlocked": false,
-    "keyringTypes": [],
-    "keyrings": []
+    "keyrings": [],
+    "vault": {
+      "cipher": "mock-cipher",
+      "iv": "mock-iv",
+      "lib": "original"
+    }
   },
   "LoggingController": {
     "logs": {}

diff --git a/package.json b/package.json
@@ -185,7 +185,7 @@
     "@metamask/etherscan-link": "^2.0.0",
     "@metamask/gas-fee-controller": "^5.0.0",
     "@metamask/key-tree": "^9.0.0",
-    "@metamask/keyring-controller": "^6.0.0",
+    "@metamask/keyring-controller": "^7.5.0",
     "@metamask/logging-controller": "^1.0.1",
     "@metamask/network-controller": "^8.0.0",
     "@metamask/permission-controller": "^4.0.1",

diff --git a/.../@metamask+keyring-controller+6.1.0.patch → .../@metamask+keyring-controller+7.5.0.patch b/.../@metamask+keyring-controller+6.1.0.patch → .../@metamask+keyring-controller+7.5.0.patch
@@ -1,13 +1,14 @@
 diff --git a/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts b/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts
-index dff777b..a61b5ed 100644
+index a05f2a9..1291081 100644
 --- a/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts
 +++ b/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts
-@@ -1,3 +1,4 @@
+@@ -1,4 +1,5 @@
+ import type { TxData, TypedTransaction } from '@ethereumjs/tx';
 +import { KeyringController as EthKeyringController } from '@metamask/eth-keyring-controller';
- import { MetaMaskKeyring as QRKeyring, IKeyringState as IQRKeyringState } from '@keystonehq/metamask-airgapped-keyring';
- import { BaseControllerV2, RestrictedControllerMessenger } from '@metamask/base-controller';
- import { PreferencesController } from '@metamask/preferences-controller';
-@@ -141,6 +142,18 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
+ import { type MetaMaskKeyring as QRKeyring, type IKeyringState as IQRKeyringState } from '@keystonehq/metamask-airgapped-keyring';
+ import type { RestrictedControllerMessenger } from '@metamask/base-controller';
+ import { BaseControllerV2 } from '@metamask/base-controller';
+@@ -167,6 +168,19 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
       * @param opts.state - Initial state to set on this controller.
       */
      constructor({ removeIdentity, syncIdentities, updateIdentities, setSelectedAddress, setAccountLabel, encryptor, keyringBuilders, cacheEncryptionKey, messenger, state, }: KeyringControllerOptions);
@@ -20,16 +21,17 @@ index dff777b..a61b5ed 100644
 +     * ===============================================================================
 +     */
 +    /**
-+      * Gets the internal keyring controller.
-+    */
++     * Gets the internal keyring controller.
++     */
 +    getEthKeyringController(): typeof EthKeyringController;
++
      /**
       * Adds a new account to the default (first) HD seed phrase keyring.
       *
-@@ -342,7 +355,10 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
+@@ -407,7 +421,10 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
      }[]>;
      unlockQRHardwareWalletAccount(index: number): Promise<void>;
-     getAccountKeyringType(account: string): Promise<KeyringTypes>;
+     getAccountKeyringType(account: string): Promise<string>;
 -    forgetQRDevice(): Promise<void>;
 +    forgetQRDevice(): Promise<{
 +        removedAccounts: string[];
@@ -38,14 +40,16 @@ index dff777b..a61b5ed 100644
  }
  export default KeyringController;
  //# sourceMappingURL=KeyringController.d.ts.map
+\ No newline at end of file
 diff --git a/node_modules/@metamask/keyring-controller/dist/KeyringController.js b/node_modules/@metamask/keyring-controller/dist/KeyringController.js
-index c905cc0..16f35e4 100644
+index c4dd1d0..ddc4155 100644
 --- a/node_modules/@metamask/keyring-controller/dist/KeyringController.js
 +++ b/node_modules/@metamask/keyring-controller/dist/KeyringController.js
-@@ -139,6 +139,19 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
-         this.setSelectedAddress = setSelectedAddress;
+@@ -154,6 +154,21 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
          this.setAccountLabel = setAccountLabel;
+         __classPrivateFieldGet(this, _KeyringController_instances, "m", _KeyringController_registerMessageHandlers).call(this);
      }
++    
 +    /**
 +     * ============================== PATCH INFORMATION ==============================
 +     * This patch was added for ledger integration. It adds a new method to the
@@ -54,25 +58,28 @@ index c905cc0..16f35e4 100644
 +     * ===============================================================================
 +     */
 +    /**
-+      * Gets the internal keyring controller.
-+    */
++     * Gets the internal keyring controller.
++     */
 +    getEthKeyringController() {
 +        return __classPrivateFieldGet(this, _KeyringController_keyring, "f");
 +    }
++
      /**
       * Adds a new account to the default (first) HD seed phrase keyring.
       *
-@@ -678,13 +691,21 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
+@@ -745,13 +760,23 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
      }
      forgetQRDevice() {
          return __awaiter(this, void 0, void 0, function* () {
 +            /**
-+              * ============================== PATCH INFORMATION ==============================
-+              * This patch addresses an issue regarding the forget device functionality. It
-+              * improves the logic  to correctly remove the QR accounts and update the
-+              * identities as needed.
-+              * ===============================================================================
-+            */
++             * ============================== PATCH INFORMATION ==============================
++             * This patch addresses an issue regarding the forget device functionality. It
++             * improves the logic  to correctly remove the QR accounts and update the
++             * identities as needed.
++             * 
++             * Solved in https://github.com/MetaMask/core/pull/3641
++             * ===============================================================================
++             */
              const keyring = yield this.getOrAddQRKeyring();
 +            const allAccounts = (yield __classPrivateFieldGet(this, _KeyringController_keyring, "f").getAccounts());
              keyring.forgetDevice();