From 2ef35d4b596f3174e62d12e8f1e66477a50a7ae3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 17 Nov 2022 10:26:13 -0330 Subject: [PATCH 01/14] Bump @metamask/browser-passworder from 4.0.1 to 4.0.2 (#164) Bumps [@metamask/browser-passworder](https://github.com/MetaMask/browser-passworder) from 4.0.1 to 4.0.2. - [Release notes](https://github.com/MetaMask/browser-passworder/releases) - [Changelog](https://github.com/MetaMask/browser-passworder/blob/main/CHANGELOG.md) - [Commits](https://github.com/MetaMask/browser-passworder/compare/v4.0.1...v4.0.2) --- updated-dependencies: - dependency-name: "@metamask/browser-passworder" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index d030e255..3d9b8275 100644 --- a/yarn.lock +++ b/yarn.lock @@ -741,9 +741,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 From 2b2c71f97208eee65b107aa524fdb99fce3b0440 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 21 Nov 2022 10:09:44 -0330 Subject: [PATCH 02/14] Bump @metamask/auto-changelog from 3.0.0 to 3.1.0 (#165) Bumps [@metamask/auto-changelog](https://github.com/MetaMask/auto-changelog) from 3.0.0 to 3.1.0. - [Release notes](https://github.com/MetaMask/auto-changelog/releases) - [Changelog](https://github.com/MetaMask/auto-changelog/blob/main/CHANGELOG.md) - [Commits](https://github.com/MetaMask/auto-changelog/compare/v3.0.0...v3.1.0) --- updated-dependencies: - dependency-name: "@metamask/auto-changelog" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 3d9b8275..e40a042c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -715,8 +715,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 +724,7 @@ __metadata: yargs: ^17.0.1 bin: auto-changelog: dist/cli.js - checksum: ee6f41b466e8f0deb8bc454936513602c4767b7be94f704da3579e3100154c92779dcfde542076158138d5fcfb64ce491ab7fc30248ae097c7d903be0cad9fb4 + checksum: cd3c833100c19a80e0b7ae8c174fbbe225700eef5fbabd9b0d6c4b439c8af839792111a17d9ddf2e1939839736413afd7402d2cc08ece373622b5a410da5e9fe languageName: node linkType: hard From 46d8e170ade56436243e4c58f8d5ff89a1d67ff6 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Wed, 30 Nov 2022 20:43:20 +0530 Subject: [PATCH 03/14] Fix: saving serialized keyring for which corresponding keyring class is not present (#169) --- index.js | 13 +++++++++++-- test/index.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 07ef6471..f210ce20 100644 --- a/index.js +++ b/index.js @@ -48,6 +48,7 @@ class KeyringController extends EventEmitter { 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 @@ -562,6 +563,8 @@ class KeyringController extends EventEmitter { }), ); + serializedKeyrings.push(...this._unsupportedKeyrings); + let vault; let newEncryptionKey; @@ -679,7 +682,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,12 +695,16 @@ 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); + if (!Keyring) { + this._unsupportedKeyrings.push(serialized); + return undefined; + } const keyring = new Keyring(); await keyring.deserialize(data); // getAccounts also validates the accounts for some keyrings diff --git a/test/index.js b/test/index.js index 082f6631..0656724d 100644 --- a/test/index.js +++ b/test/index.js @@ -94,6 +94,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 }); @@ -245,6 +258,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 +350,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 () { From 74c7e2156b9ea36392ac823fd5cba5f3d4b7c511 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 1 Dec 2022 23:01:38 +0530 Subject: [PATCH 04/14] 8.1.0 (#170) --- CHANGELOG.md | 8 +++++++- package.json | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) 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/package.json b/package.json index bb90c5c2..f3cbf86c 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", From d1515390f52e1fdc1391e352b0eb808a173de003 Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Wed, 7 Dec 2022 19:49:39 +0100 Subject: [PATCH 05/14] chore: update eth-simple-keyring (#171) --- README.md | 2 +- index.js | 2 +- package.json | 2 +- yarn.lock | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 118 insertions(+), 4 deletions(-) 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 f210ce20..17f641d6 100644 --- a/index.js +++ b/index.js @@ -5,7 +5,7 @@ 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]; diff --git a/package.json b/package.json index f3cbf86c..c103b525 100644 --- a/package.json +++ b/package.json @@ -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/yarn.lock b/yarn.lock index e40a042c..47799e25 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" @@ -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" From 3165d72ae9fae7917743fc495dd0b7c39ebba70c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 8 Dec 2022 12:27:07 +0000 Subject: [PATCH 06/14] Bump qs from 6.5.2 to 6.5.3 (#173) Bumps [qs](https://github.com/ljharb/qs) from 6.5.2 to 6.5.3. - [Release notes](https://github.com/ljharb/qs/releases) - [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md) - [Commits](https://github.com/ljharb/qs/compare/v6.5.2...v6.5.3) --- updated-dependencies: - dependency-name: qs dependency-type: indirect ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 47799e25..85c9b802 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5816,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 From 6ef7f15e8594e7f25975601bf3b5672ec823ccc3 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Mon, 14 Nov 2022 15:20:30 +0000 Subject: [PATCH 07/14] Adds async init method to keyrings --- index.js | 28 ++++++++++++++++++++++------ test/index.js | 19 +++++++++++++++++++ test/lib/mock-keyring.js | 19 +++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 test/lib/mock-keyring.js diff --git a/index.js b/index.js index 17f641d6..6b6116bc 100644 --- a/index.js +++ b/index.js @@ -237,11 +237,11 @@ class KeyringController extends EventEmitter { * @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(); + await keyring.addAccounts(); } const accounts = await keyring.getAccounts(); @@ -700,12 +700,12 @@ class KeyringController extends EventEmitter { async _restoreKeyring(serialized) { const { type, data } = serialized; - const Keyring = this.getKeyringClassForType(type); - if (!Keyring) { + const keyring = await this._newKeyring(type); + if (!keyring) { this._unsupportedKeyrings.push(serialized); return undefined; } - const keyring = new Keyring(); + await keyring.deserialize(data); // getAccounts also validates the accounts for some keyrings await keyring.getAccounts(); @@ -873,6 +873,22 @@ class KeyringController extends EventEmitter { ); } } + + async _newKeyring(type, data) { + const Keyring = this.getKeyringClassForType(type); + + if (!Keyring) { + return undefined; + } + + const keyring = new Keyring(data); + + if (keyring.init) { + await keyring.init(); + } + + return keyring; + } } module.exports = KeyringController; diff --git a/test/index.js b/test/index.js index 0656724d..4055d079 100644 --- a/test/index.js +++ b/test/index.js @@ -6,6 +6,7 @@ const sinon = require('sinon'); const Wallet = require('ethereumjs-wallet').default; const KeyringController = 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, + keyringTypes: [KeyringMockWithInit], }); await keyringController.createNewVaultAndKeychain(password); @@ -240,6 +242,23 @@ describe('KeyringController', function () { const allAccounts = await keyringController.getAccounts(); expect(allAccounts).toHaveLength(3); }); + + it('should call init method if available', async function () { + const Keyring = keyringController.getKeyringClassForType( + 'Keyring Mock With Init', + ); + + const initSpy = sinon.spy(Keyring.prototype, 'init'); + + const keyring = await keyringController.addNewKeyring( + 'Keyring Mock With Init', + ); + + const keyringAccounts = await keyring.getAccounts(); + expect(keyringAccounts).toHaveLength(0); + + expect(initSpy.calledOnce).toBe(true); + }); }); describe('restoreKeyring', function () { diff --git a/test/lib/mock-keyring.js b/test/lib/mock-keyring.js new file mode 100644 index 00000000..919eb07e --- /dev/null +++ b/test/lib/mock-keyring.js @@ -0,0 +1,19 @@ +class KeyringMockWithInit { + init() { + return Promise.resolve(); + } + + getAccounts() { + return []; + } + + serialize() { + return Promise.resolve({}); + } +} + +KeyringMockWithInit.type = 'Keyring Mock With Init'; + +module.exports = { + KeyringMockWithInit, +}; From f1c971858a812dd8a782645fc6620e9681e2acd0 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Fri, 2 Dec 2022 16:33:33 +0000 Subject: [PATCH 08/14] instantiate keyrings with builder functions --- .eslintrc.js | 3 +++ index.js | 28 ++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index f371d0a7..534d285c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,5 +1,8 @@ module.exports = { root: true, + parserOptions: { + ecmaVersion: 2018, // to support object rest spread, e.g. {...x, ...y} + }, extends: ['@metamask/eslint-config'], env: { commonjs: true, diff --git a/index.js b/index.js index 6b6116bc..ef99ad55 100644 --- a/index.js +++ b/index.js @@ -8,7 +8,10 @@ const { normalize: normalizeAddress } = require('eth-sig-util'); const SimpleKeyring = require('@metamask/eth-simple-keyring'); const HdKeyring = require('@metamask/eth-hd-keyring'); -const keyringTypes = [SimpleKeyring, HdKeyring]; +const keyringTypes = [ + keyringBuilderFactory(SimpleKeyring), + keyringBuilderFactory(HdKeyring), +]; const KEYRINGS_TYPE_MAP = { HD_KEYRING: 'HD Key Tree', @@ -700,13 +703,12 @@ class KeyringController extends EventEmitter { async _restoreKeyring(serialized) { const { type, data } = serialized; - const keyring = await this._newKeyring(type); + const keyring = await this._newKeyring(type, data); if (!keyring) { this._unsupportedKeyrings.push(serialized); return undefined; } - await keyring.deserialize(data); // getAccounts also validates the accounts for some keyrings await keyring.getAccounts(); this.keyrings.push(keyring); @@ -875,13 +877,15 @@ class KeyringController extends EventEmitter { } async _newKeyring(type, data) { - const Keyring = this.getKeyringClassForType(type); + const keyringBuilder = this.getKeyringClassForType(type); - if (!Keyring) { + if (!keyringBuilder) { return undefined; } - const keyring = new Keyring(data); + const keyring = keyringBuilder(); + + await keyring.deserialize(data); if (keyring.init) { await keyring.init(); @@ -891,4 +895,16 @@ class KeyringController extends EventEmitter { } } +function keyringBuilderFactory(KeyringClass, BridgeClass) { + const builder = () => { + const constructorDependencies = BridgeClass ? new BridgeClass() : undefined; + return new KeyringClass(constructorDependencies); + }; + + builder.type = KeyringClass.type; + + return builder; +} + module.exports = KeyringController; +module.exports.keyringBuilderFactory = keyringBuilderFactory; From 6593deb49d98a7c5ff7bc137259bbec77d4340a2 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Mon, 5 Dec 2022 10:27:38 +0000 Subject: [PATCH 09/14] rename properties and method add test --- index.js | 26 +++++++++++++++----------- test/index.js | 14 +++++--------- test/lib/mock-keyring.js | 4 ++++ 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/index.js b/index.js index ef99ad55..712b555c 100644 --- a/index.js +++ b/index.js @@ -8,7 +8,7 @@ const { normalize: normalizeAddress } = require('eth-sig-util'); const SimpleKeyring = require('@metamask/eth-simple-keyring'); const HdKeyring = require('@metamask/eth-hd-keyring'); -const keyringTypes = [ +const defaultKeyringBuilders = [ keyringBuilderFactory(SimpleKeyring), keyringBuilderFactory(HdKeyring), ]; @@ -38,13 +38,15 @@ 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, }); @@ -233,7 +235,7 @@ 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. @@ -718,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, + ); } /** @@ -877,7 +881,7 @@ class KeyringController extends EventEmitter { } async _newKeyring(type, data) { - const keyringBuilder = this.getKeyringClassForType(type); + const keyringBuilder = this.getKeyringBuilderForType(type); if (!keyringBuilder) { return undefined; diff --git a/test/index.js b/test/index.js index 4055d079..fb5adc1c 100644 --- a/test/index.js +++ b/test/index.js @@ -6,6 +6,7 @@ const sinon = require('sinon'); const Wallet = require('ethereumjs-wallet').default; const KeyringController = require('..'); +const { keyringBuilderFactory } = require('..'); const { KeyringMockWithInit } = require('./lib/mock-keyring'); const mockEncryptor = require('./lib/mock-encryptor'); @@ -34,7 +35,7 @@ describe('KeyringController', function () { beforeEach(async function () { keyringController = new KeyringController({ encryptor: mockEncryptor, - keyringTypes: [KeyringMockWithInit], + keyringBuilders: [keyringBuilderFactory(KeyringMockWithInit)], }); await keyringController.createNewVaultAndKeychain(password); @@ -244,20 +245,15 @@ describe('KeyringController', function () { }); it('should call init method if available', async function () { - const Keyring = keyringController.getKeyringClassForType( - 'Keyring Mock With Init', - ); - - const initSpy = sinon.spy(Keyring.prototype, 'init'); + const initSpy = sinon.spy(KeyringMockWithInit.prototype, 'init'); const keyring = await keyringController.addNewKeyring( 'Keyring Mock With Init', ); - const keyringAccounts = await keyring.getAccounts(); - expect(keyringAccounts).toHaveLength(0); + expect(keyring).toBeInstanceOf(KeyringMockWithInit); - expect(initSpy.calledOnce).toBe(true); + sinon.assert.calledOnce(initSpy); }); }); diff --git a/test/lib/mock-keyring.js b/test/lib/mock-keyring.js index 919eb07e..cd57b9ae 100644 --- a/test/lib/mock-keyring.js +++ b/test/lib/mock-keyring.js @@ -10,6 +10,10 @@ class KeyringMockWithInit { serialize() { return Promise.resolve({}); } + + deserialize(_) { + return Promise.resolve(); + } } KeyringMockWithInit.type = 'Keyring Mock With Init'; From 78a33760f4bbc11dc359967d58562c069209fb49 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Mon, 5 Dec 2022 11:24:44 +0000 Subject: [PATCH 10/14] remove eslintrc changes --- .eslintrc.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 534d285c..f371d0a7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,8 +1,5 @@ module.exports = { root: true, - parserOptions: { - ecmaVersion: 2018, // to support object rest spread, e.g. {...x, ...y} - }, extends: ['@metamask/eslint-config'], env: { commonjs: true, From 500e1a33dce3f035c9fa1c1525b7d6f6a1cd8180 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Wed, 7 Dec 2022 11:20:23 +0000 Subject: [PATCH 11/14] make KeyringController a named export --- index.js | 6 ++++-- test/index.js | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 712b555c..6b6ad529 100644 --- a/index.js +++ b/index.js @@ -910,5 +910,7 @@ function keyringBuilderFactory(KeyringClass, BridgeClass) { return builder; } -module.exports = KeyringController; -module.exports.keyringBuilderFactory = keyringBuilderFactory; +module.exports = { + KeyringController, + keyringBuilderFactory, +}; diff --git a/test/index.js b/test/index.js index fb5adc1c..e4b4b749 100644 --- a/test/index.js +++ b/test/index.js @@ -5,8 +5,7 @@ const normalizeAddress = sigUtil.normalize; const sinon = require('sinon'); const Wallet = require('ethereumjs-wallet').default; -const KeyringController = require('..'); -const { keyringBuilderFactory } = require('..'); +const { KeyringController, keyringBuilderFactory } = require('..'); const { KeyringMockWithInit } = require('./lib/mock-keyring'); const mockEncryptor = require('./lib/mock-encryptor'); From a486f5b0eb5659c81faca5b2499081368f319868 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Fri, 9 Dec 2022 11:28:10 +0000 Subject: [PATCH 12/14] changes from review --- index.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 6b6ad529..da793009 100644 --- a/index.js +++ b/index.js @@ -880,6 +880,15 @@ 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); @@ -899,13 +908,18 @@ class KeyringController extends EventEmitter { } } -function keyringBuilderFactory(KeyringClass, BridgeClass) { - const builder = () => { - const constructorDependencies = BridgeClass ? new BridgeClass() : undefined; - return new KeyringClass(constructorDependencies); - }; +/** + * 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 = KeyringClass.type; + builder.type = Keyring.type; return builder; } From 53d8263163b8f16584f56ca48971cd198f5ba930 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Fri, 9 Dec 2022 11:49:53 +0000 Subject: [PATCH 13/14] remove await from addAccounts --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index da793009..2a8f0039 100644 --- a/index.js +++ b/index.js @@ -246,7 +246,7 @@ class KeyringController extends EventEmitter { if ((!opts || !opts.mnemonic) && type === KEYRINGS_TYPE_MAP.HD_KEYRING) { keyring.generateRandomMnemonic(); - await keyring.addAccounts(); + keyring.addAccounts(); } const accounts = await keyring.getAccounts(); From b26d58592fad6af237769f39b9309a4e0513a1df Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 9 Dec 2022 09:15:27 -0330 Subject: [PATCH 14/14] Bump minimatch from 3.0.4 to 3.1.2 (#172) Bumps [minimatch](https://github.com/isaacs/minimatch) from 3.0.4 to 3.1.2. - [Release notes](https://github.com/isaacs/minimatch/releases) - [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md) - [Commits](https://github.com/isaacs/minimatch/compare/v3.0.4...v3.1.2) --- updated-dependencies: - dependency-name: minimatch dependency-type: indirect ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 85c9b802..cf6f33ff 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5016,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