diff --git a/CHANGELOG.md b/CHANGELOG.md index 6380b8fe..51af87d7 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index dcff3658..85b92c53 100644 --- a/README.md +++ b/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. diff --git a/index.js b/index.js index 07ef6471..2a8f0039 100644 --- a/index.js +++ b/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} 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} The deserialized keyring. + * @returns {Promise} 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} 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, +}; diff --git a/package.json b/package.json index bb90c5c2..c103b525 100644 --- a/package.json +++ b/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": { diff --git a/test/index.js b/test/index.js index 082f6631..e4b4b749 100644 --- a/test/index.js +++ b/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 () { diff --git a/test/lib/mock-keyring.js b/test/lib/mock-keyring.js new file mode 100644 index 00000000..cd57b9ae --- /dev/null +++ b/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, +}; diff --git a/yarn.lock b/yarn.lock index d030e255..cf6f33ff 100644 --- a/yarn.lock +++ b/yarn.lock @@ -462,6 +462,26 @@ __metadata: languageName: node linkType: hard +"@ethereumjs/rlp@npm:^4.0.0-beta.2": + version: 4.0.0 + resolution: "@ethereumjs/rlp@npm:4.0.0" + bin: + rlp: bin/rlp + checksum: 407dfb8b1e09b4282e6be561e8d74f8939da78f460c08456c7ba2fb273fc42ee16027955a07085abfd7600ffb466c4c4add159885e67abb91bc85db9dd81ffb5 + languageName: node + linkType: hard + +"@ethereumjs/util@npm:^8.0.0": + version: 8.0.2 + resolution: "@ethereumjs/util@npm:8.0.2" + dependencies: + "@ethereumjs/rlp": ^4.0.0-beta.2 + async: ^3.2.4 + ethereum-cryptography: ^1.1.2 + checksum: 652a40f9dffb9ed749c8adff21c924dec7a6d38129e480ab35ae2e56644bb6e49fcdb5f0cd0451fb6878de7e16cf117e87e34ba01f311bf216b4a8dd5b23aa90 + languageName: node + linkType: hard + "@gar/promisify@npm:^1.1.3": version: 1.1.3 resolution: "@gar/promisify@npm:1.1.3" @@ -715,8 +735,8 @@ __metadata: linkType: hard "@metamask/auto-changelog@npm:^3.0.0": - version: 3.0.0 - resolution: "@metamask/auto-changelog@npm:3.0.0" + version: 3.1.0 + resolution: "@metamask/auto-changelog@npm:3.1.0" dependencies: diff: ^5.0.0 execa: ^5.1.1 @@ -724,7 +744,7 @@ __metadata: yargs: ^17.0.1 bin: auto-changelog: dist/cli.js - checksum: ee6f41b466e8f0deb8bc454936513602c4767b7be94f704da3579e3100154c92779dcfde542076158138d5fcfb64ce491ab7fc30248ae097c7d903be0cad9fb4 + checksum: cd3c833100c19a80e0b7ae8c174fbbe225700eef5fbabd9b0d6c4b439c8af839792111a17d9ddf2e1939839736413afd7402d2cc08ece373622b5a410da5e9fe languageName: node linkType: hard @@ -741,9 +761,9 @@ __metadata: linkType: hard "@metamask/browser-passworder@npm:^4.0.1": - version: 4.0.1 - resolution: "@metamask/browser-passworder@npm:4.0.1" - checksum: 17bd1d9448b02cdc4f623612ef20298aa829db387a5a1e127ac2cd6108b2bfc1b6b9ca861a61537f6871a926c6906ba4e9291a63ad093e1c574309289efb5dbd + version: 4.0.2 + resolution: "@metamask/browser-passworder@npm:4.0.2" + checksum: 997c330b1c72f7135d0fd2a7f4b0dce37b3c2e6b30e92f048fa8d4f8c949f5b669dcc3064790f41df30ee2e53a9e64a812df72e00527736be704cce2cf4f6e49 languageName: node linkType: hard @@ -808,6 +828,53 @@ __metadata: languageName: node linkType: hard +"@metamask/eth-sig-util@npm:^5.0.1": + version: 5.0.2 + resolution: "@metamask/eth-sig-util@npm:5.0.2" + dependencies: + "@ethereumjs/util": ^8.0.0 + bn.js: ^4.11.8 + ethereum-cryptography: ^1.1.2 + ethjs-util: ^0.1.6 + tweetnacl: ^1.0.3 + tweetnacl-util: ^0.15.1 + checksum: 1fbf1a0f5e654058f0219c9018dbebadf53036c9c3b47c8faf1cac54816532bb18996821736f526ac4e3d579afcaf502af4ad07e88158a50f015141858b08a90 + languageName: node + linkType: hard + +"@metamask/eth-simple-keyring@npm:^5.0.0": + version: 5.0.0 + resolution: "@metamask/eth-simple-keyring@npm:5.0.0" + dependencies: + "@ethereumjs/util": ^8.0.0 + "@metamask/eth-sig-util": ^5.0.1 + ethereum-cryptography: ^1.1.2 + randombytes: ^2.1.0 + checksum: 6fd05173531b84f6fb816b90ab8cfb176ac3300f07daa51c4adac673fa17dbcc6ce1ebdf064b9f66549f37476bbc54eb2dd9d28935d57654ef62b935a1e31e1d + languageName: node + linkType: hard + +"@noble/hashes@npm:1.1.2": + version: 1.1.2 + resolution: "@noble/hashes@npm:1.1.2" + checksum: 3c2a8cb7c2e053811032f242155d870c5eb98844d924d69702244d48804cb03b42d4a666c49c2b71164420d8229cb9a6f242b972d50d5bb2f1d673b98b041de2 + languageName: node + linkType: hard + +"@noble/hashes@npm:~1.1.1": + version: 1.1.4 + resolution: "@noble/hashes@npm:1.1.4" + checksum: 663c1e7ebaa3ca4ff836c799a9dd697fda861ae8b9923a4850659fbf5c3d5f7cf541e77aee3c1bcc6f81a815b7f88fad75c004c0f30eda301deeedc2c9435368 + languageName: node + linkType: hard + +"@noble/secp256k1@npm:1.6.3, @noble/secp256k1@npm:~1.6.0": + version: 1.6.3 + resolution: "@noble/secp256k1@npm:1.6.3" + checksum: 16eb3242533e645deb64444c771515f66bdc2ee0759894efd42fdeed4ab226ed29827aaaf6caa27d3d95b831452fd4246aa1007cd688aa462ad48fc084ab76e6 + languageName: node + linkType: hard + "@nodelib/fs.scandir@npm:2.1.5": version: 2.1.5 resolution: "@nodelib/fs.scandir@npm:2.1.5" @@ -884,6 +951,34 @@ __metadata: languageName: node linkType: hard +"@scure/base@npm:~1.1.0": + version: 1.1.1 + resolution: "@scure/base@npm:1.1.1" + checksum: b4fc810b492693e7e8d0107313ac74c3646970c198bbe26d7332820886fa4f09441991023ec9aa3a2a51246b74409ab5ebae2e8ef148bbc253da79ac49130309 + languageName: node + linkType: hard + +"@scure/bip32@npm:1.1.0": + version: 1.1.0 + resolution: "@scure/bip32@npm:1.1.0" + dependencies: + "@noble/hashes": ~1.1.1 + "@noble/secp256k1": ~1.6.0 + "@scure/base": ~1.1.0 + checksum: e6102ab9038896861fca5628b8a97f3c4cb24a073cc9f333c71c747037d82e4423d1d111fd282ba212efaf73cbc5875702567fb4cf13b5f0eb23a5bab402e37e + languageName: node + linkType: hard + +"@scure/bip39@npm:1.1.0": + version: 1.1.0 + resolution: "@scure/bip39@npm:1.1.0" + dependencies: + "@noble/hashes": ~1.1.1 + "@scure/base": ~1.1.0 + checksum: c4361406f092a45e511dc572c89f497af6665ad81cb3fd7bf78e6772f357f7ae885e129ef0b985cb3496a460b4811318f77bc61634d9b0a8446079a801b6003c + languageName: node + linkType: hard + "@sinonjs/commons@npm:^1.6.0, @sinonjs/commons@npm:^1.8.3": version: 1.8.3 resolution: "@sinonjs/commons@npm:1.8.3" @@ -1502,6 +1597,13 @@ __metadata: languageName: node linkType: hard +"async@npm:^3.2.4": + version: 3.2.4 + resolution: "async@npm:3.2.4" + checksum: 43d07459a4e1d09b84a20772414aa684ff4de085cbcaec6eea3c7a8f8150e8c62aa6cd4e699fe8ee93c3a5b324e777d34642531875a0817a35697522c1b02e89 + languageName: node + linkType: hard + "asynckit@npm:^0.4.0": version: 0.4.0 resolution: "asynckit@npm:0.4.0" @@ -2754,6 +2856,7 @@ __metadata: "@metamask/eslint-config-jest": ^7.0.0 "@metamask/eslint-config-nodejs": ^7.0.1 "@metamask/eth-hd-keyring": ^4.0.2 + "@metamask/eth-simple-keyring": ^5.0.0 eslint: ^7.29.0 eslint-config-prettier: ^8.3.0 eslint-plugin-import: ^2.23.4 @@ -2761,7 +2864,6 @@ __metadata: eslint-plugin-node: ^11.1.0 eslint-plugin-prettier: ^3.4.0 eth-sig-util: ^3.0.1 - eth-simple-keyring: ^4.2.0 ethereumjs-wallet: ^1.0.1 jest: ^27.0.6 obs-store: ^4.0.3 @@ -2818,6 +2920,18 @@ __metadata: languageName: node linkType: hard +"ethereum-cryptography@npm:^1.1.2": + version: 1.1.2 + resolution: "ethereum-cryptography@npm:1.1.2" + dependencies: + "@noble/hashes": 1.1.2 + "@noble/secp256k1": 1.6.3 + "@scure/bip32": 1.1.0 + "@scure/bip39": 1.1.0 + checksum: 0ef55f141acad45b1ba1db58ce3d487155eb2d0b14a77b3959167a36ad324f46762873257def75e7f00dbe8ac78aabc323d2207830f85e63a42a1fb67063a6ba + languageName: node + linkType: hard + "ethereumjs-abi@npm:^0.6.8": version: 0.6.8 resolution: "ethereumjs-abi@npm:0.6.8" @@ -4902,11 +5016,11 @@ __metadata: linkType: hard "minimatch@npm:^3.0.4": - version: 3.0.4 - resolution: "minimatch@npm:3.0.4" + version: 3.1.2 + resolution: "minimatch@npm:3.1.2" dependencies: brace-expansion: ^1.1.7 - checksum: 66ac295f8a7b59788000ea3749938b0970344c841750abd96694f80269b926ebcafad3deeb3f1da2522978b119e6ae3a5869b63b13a7859a456b3408bd18a078 + checksum: c154e566406683e7bcb746e000b84d74465b3a832c45d59912b9b55cd50dee66e5c4b1e5566dba26154040e51672f9aa450a9aef0c97cfc7336b78b7afb9540a languageName: node linkType: hard @@ -5702,9 +5816,9 @@ __metadata: linkType: hard "qs@npm:~6.5.2": - version: 6.5.2 - resolution: "qs@npm:6.5.2" - checksum: 24af7b9928ba2141233fba2912876ff100403dba1b08b20c3b490da9ea6c636760445ea2211a079e7dfa882a5cf8f738337b3748c8bdd0f93358fa8881d2db8f + version: 6.5.3 + resolution: "qs@npm:6.5.3" + checksum: 6f20bf08cabd90c458e50855559539a28d00b2f2e7dddcb66082b16a43188418cb3cb77cbd09268bcef6022935650f0534357b8af9eeb29bf0f27ccb17655692 languageName: node linkType: hard