Adds async init method to keyrings

CHANGELOG.md

@@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [8.1.0]
+### Changed
+- Allow deserializing vaults with unrecognized keyrings ([#169](https://github.com/MetaMask/KeyringController/pull/169))
+  - When deserializing a vault with an unrecognized keyring, the controller will no longer crash. The unrecognized keyring vault data will be preserved in the vault for future use, but will otherwise be ignored.
+
 ## [8.0.1]
 ### Fixed
 - Restore full state return value ([#161](https://github.com/MetaMask/KeyringController/pull/161))
@@ -58,7 +63,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Validate user imported seedphrase across all bip39 wordlists ([#77](https://github.com/MetaMask/KeyringController/pull/77))
 
 
-[Unreleased]: https://github.com/MetaMask/KeyringController/compare/v8.0.1...HEAD
+[Unreleased]: https://github.com/MetaMask/KeyringController/compare/v8.1.0...HEAD
+[8.1.0]: https://github.com/MetaMask/KeyringController/compare/v8.0.1...v8.1.0
 [8.0.1]: https://github.com/MetaMask/KeyringController/compare/v8.0.0...v8.0.1
 [8.0.0]: https://github.com/MetaMask/KeyringController/compare/v7.0.2...v8.0.0
 [7.0.2]: https://github.com/MetaMask/KeyringController/compare/v7.0.1...v7.0.2

README.md

@@ -18,7 +18,7 @@ The KeyringController has three main responsibilities:
 
 ```javascript
 const KeyringController = require('eth-keyring-controller');
-const SimpleKeyring = require('eth-simple-keyring');
+const SimpleKeyring = require('@metamask/eth-simple-keyring');
 
 const keyringController = new KeyringController({
   keyringTypes: [SimpleKeyring], // optional array of types to support.

index.js

@@ -5,10 +5,13 @@ const ObservableStore = require('obs-store');
 const encryptor = require('@metamask/browser-passworder');
 const { normalize: normalizeAddress } = require('eth-sig-util');
 
-const SimpleKeyring = require('eth-simple-keyring');
+const SimpleKeyring = require('@metamask/eth-simple-keyring');
 const HdKeyring = require('@metamask/eth-hd-keyring');
 
-const keyringTypes = [SimpleKeyring, HdKeyring];
+const defaultKeyringBuilders = [
+  keyringBuilderFactory(SimpleKeyring),
+  keyringBuilderFactory(HdKeyring),
+];
 
 const KEYRINGS_TYPE_MAP = {
   HD_KEYRING: 'HD Key Tree',
@@ -35,19 +38,22 @@ class KeyringController extends EventEmitter {
   constructor(opts) {
     super();
     const initState = opts.initState || {};
-    this.keyringTypes = opts.keyringTypes
-      ? keyringTypes.concat(opts.keyringTypes)
-      : keyringTypes;
+    this.keyringBuilders = opts.keyringBuilders
+      ? defaultKeyringBuilders.concat(opts.keyringBuilders)
+      : defaultKeyringBuilders;
     this.store = new ObservableStore(initState);
     this.memStore = new ObservableStore({
       isUnlocked: false,
-      keyringTypes: this.keyringTypes.map((keyringType) => keyringType.type),
+      keyringTypes: this.keyringBuilders.map(
+        (keyringBuilder) => keyringBuilder.type,
+      ),
       keyrings: [],
       encryptionKey: null,
     });
 
     this.encryptor = opts.encryptor || encryptor;
     this.keyrings = [];
+    this._unsupportedKeyrings = [];
 
     // This option allows the controller to cache an exported key
     // for use in decrypting and encrypting data without password
@@ -229,15 +235,15 @@ class KeyringController extends EventEmitter {
    * and the current decrypted Keyrings array.
    *
    * All Keyring classes implement a unique `type` string,
-   * and this is used to retrieve them from the keyringTypes array.
+   * and this is used to retrieve them from the keyringBuilders array.
    *
    * @param {string} type - The type of keyring to add.
    * @param {Object} opts - The constructor options for the keyring.
    * @returns {Promise<Keyring>} The new keyring.
    */
   async addNewKeyring(type, opts) {
-    const Keyring = this.getKeyringClassForType(type);
-    const keyring = new Keyring(opts);
+    const keyring = await this._newKeyring(type, opts);
+
     if ((!opts || !opts.mnemonic) && type === KEYRINGS_TYPE_MAP.HD_KEYRING) {
       keyring.generateRandomMnemonic();
       keyring.addAccounts();
@@ -562,6 +568,8 @@ class KeyringController extends EventEmitter {
       }),
     );
 
+    serializedKeyrings.push(...this._unsupportedKeyrings);
+
     let vault;
     let newEncryptionKey;
 
@@ -679,7 +687,9 @@ class KeyringController extends EventEmitter {
    */
   async restoreKeyring(serialized) {
     const keyring = await this._restoreKeyring(serialized);
-    await this._updateMemStoreKeyrings();
+    if (keyring) {
+      await this._updateMemStoreKeyrings();
+    }
     return keyring;
   }
 
@@ -690,14 +700,17 @@ class KeyringController extends EventEmitter {
    * On success, returns the resulting keyring instance.
    *
    * @param {Object} serialized - The serialized keyring.
-   * @returns {Promise<Keyring>} The deserialized keyring.
+   * @returns {Promise<Keyring|undefined>} The deserialized keyring or undefined if the keyring type is unsupported.
    */
   async _restoreKeyring(serialized) {
     const { type, data } = serialized;
 
-    const Keyring = this.getKeyringClassForType(type);
-    const keyring = new Keyring();
-    await keyring.deserialize(data);
+    const keyring = await this._newKeyring(type, data);
+    if (!keyring) {
+      this._unsupportedKeyrings.push(serialized);
+      return undefined;
+    }
+
     // getAccounts also validates the accounts for some keyrings
     await keyring.getAccounts();
     this.keyrings.push(keyring);
@@ -707,16 +720,18 @@ class KeyringController extends EventEmitter {
   /**
    * Get Keyring Class For Type
    *
-   * Searches the current `keyringTypes` array
-   * for a Keyring class whose unique `type` property
+   * Searches the current `keyringBuilders` array
+   * for a Keyring builder whose unique `type` property
    * matches the provided `type`,
    * returning it if it exists.
    *
    * @param {string} type - The type whose class to get.
    * @returns {Keyring|undefined} The class, if it exists.
    */
-  getKeyringClassForType(type) {
-    return this.keyringTypes.find((keyring) => keyring.type === type);
+  getKeyringBuilderForType(type) {
+    return this.keyringBuilders.find(
+      (keyringBuilder) => keyringBuilder.type === type,
+    );
   }
 
   /**
@@ -864,6 +879,52 @@ class KeyringController extends EventEmitter {
       );
     }
   }
+
+  /**
+   * Instantiate, initialize and return a new keyring
+   *
+   * The keyring instantiated is of the given `type`.
+   *
+   * @param {string} type - The type of keyring to add.
+   * @param {Object} data - The data to restore a previously serialized keyring.
+   * @returns {Promise<Keyring>} The new keyring.
+   */
+  async _newKeyring(type, data) {
+    const keyringBuilder = this.getKeyringBuilderForType(type);
+
+    if (!keyringBuilder) {
+      return undefined;
+    }
+
+    const keyring = keyringBuilder();
+
+    await keyring.deserialize(data);
+
+    if (keyring.init) {
+      await keyring.init();
+    }
+
+    return keyring;
+  }
+}
+
+/**
+ * Get builder function for `Keyring`
+ *
+ * Returns a builder function for `Keyring` with a `type` property.
+ *
+ * @param {typeof Keyring} Keyring - The Keyring class for the builder.
+ * @returns {function: Keyring} A builder function for the given Keyring.
+ */
+function keyringBuilderFactory(Keyring) {
+  const builder = () => new Keyring();
+
+  builder.type = Keyring.type;
+
+  return builder;
 }
 
-module.exports = KeyringController;
+module.exports = {
+  KeyringController,
+  keyringBuilderFactory,
+};

package.json

@@ -1,6 +1,6 @@
 {
   "name": "eth-keyring-controller",
-  "version": "8.0.1",
+  "version": "8.1.0",
   "description": "A module for managing various keyrings of Ethereum accounts, encrypting them, and using them.",
   "keywords": [
     "ethereum",
@@ -33,8 +33,8 @@
     "@metamask/bip39": "^4.0.0",
     "@metamask/browser-passworder": "^4.0.1",
     "@metamask/eth-hd-keyring": "^4.0.2",
+    "@metamask/eth-simple-keyring": "^5.0.0",
     "eth-sig-util": "^3.0.1",
-    "eth-simple-keyring": "^4.2.0",
     "obs-store": "^4.0.3"
   },
   "devDependencies": {

test/index.js

@@ -5,7 +5,8 @@ const normalizeAddress = sigUtil.normalize;
 const sinon = require('sinon');
 const Wallet = require('ethereumjs-wallet').default;
 
-const KeyringController = require('..');
+const { KeyringController, keyringBuilderFactory } = require('..');
+const { KeyringMockWithInit } = require('./lib/mock-keyring');
 const mockEncryptor = require('./lib/mock-encryptor');
 
 const password = 'password123';
@@ -33,6 +34,7 @@ describe('KeyringController', function () {
   beforeEach(async function () {
     keyringController = new KeyringController({
       encryptor: mockEncryptor,
+      keyringBuilders: [keyringBuilderFactory(KeyringMockWithInit)],
     });
 
     await keyringController.createNewVaultAndKeychain(password);
@@ -94,6 +96,19 @@ describe('KeyringController', function () {
     });
   });
 
+  describe('persistAllKeyrings', function () {
+    it('should persist keyrings in _unsupportedKeyrings array', async function () {
+      const unsupportedKeyring = 'DUMMY_KEYRING';
+      keyringController._unsupportedKeyrings = [unsupportedKeyring];
+      await keyringController.persistAllKeyrings();
+
+      const { vault } = keyringController.store.getState();
+      const keyrings = await mockEncryptor.decrypt(password, vault);
+      expect(keyrings.indexOf(unsupportedKeyring) > -1).toBe(true);
+      expect(keyrings).toHaveLength(2);
+    });
+  });
+
   describe('createNewVaultAndKeychain', function () {
     it('should create a new vault', async function () {
       keyringController.store.updateState({ vault: null });
@@ -227,6 +242,18 @@ describe('KeyringController', function () {
       const allAccounts = await keyringController.getAccounts();
       expect(allAccounts).toHaveLength(3);
     });
+
+    it('should call init method if available', async function () {
+      const initSpy = sinon.spy(KeyringMockWithInit.prototype, 'init');
+
+      const keyring = await keyringController.addNewKeyring(
+        'Keyring Mock With Init',
+      );
+
+      expect(keyring).toBeInstanceOf(KeyringMockWithInit);
+
+      sinon.assert.calledOnce(initSpy);
+    });
   });
 
   describe('restoreKeyring', function () {
@@ -245,6 +272,13 @@ describe('KeyringController', function () {
       const accounts = await keyring.getAccounts();
       expect(accounts[0]).toBe(walletOneAddresses[0]);
     });
+    it('should return undefined if keyring type is not supported.', async function () {
+      const unsupportedKeyring = { type: 'Ledger Keyring', data: 'DUMMY' };
+      const keyring = await keyringController.restoreKeyring(
+        unsupportedKeyring,
+      );
+      expect(keyring).toBeUndefined();
+    });
   });
 
   describe('getAccounts', function () {
@@ -330,6 +364,16 @@ describe('KeyringController', function () {
         expect(keyring.wallets).toHaveLength(1);
       });
     });
+    it('add serialized keyring to _unsupportedKeyrings array if keyring type is not known', async function () {
+      const _unsupportedKeyrings = [{ type: 'Ledger Keyring', data: 'DUMMY' }];
+      mockEncryptor.encrypt(password, _unsupportedKeyrings);
+      await keyringController.setLocked();
+      const keyrings = await keyringController.unlockKeyrings(password);
+      expect(keyrings).toHaveLength(0);
+      expect(keyringController._unsupportedKeyrings).toStrictEqual(
+        _unsupportedKeyrings,
+      );
+    });
   });
 
   describe('verifyPassword', function () {

test/lib/mock-keyring.js

@@ -0,0 +1,23 @@
+class KeyringMockWithInit {
+  init() {
+    return Promise.resolve();
+  }
+
+  getAccounts() {
+    return [];
+  }
+
+  serialize() {
+    return Promise.resolve({});
+  }
+
+  deserialize(_) {
+    return Promise.resolve();
+  }
+}
+
+KeyringMockWithInit.type = 'Keyring Mock With Init';
+
+module.exports = {
+  KeyringMockWithInit,
+};