From ced36eb20186b0f1bf0c744ac31a5b7804120065 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 21 Nov 2016 19:40:30 -0800 Subject: Improve Keyring organization Separated public & private methods. (Fixes #845) Made class method `type()` into a simple property. (Fixes #846) --- app/scripts/keyring-controller.js | 4 +-- app/scripts/keyrings/hd.js | 67 +++++++++++++++++++++------------------ app/scripts/keyrings/simple.js | 21 +++++++++--- test/unit/keyrings/hd-test.js | 6 ++-- test/unit/keyrings/simple-test.js | 6 ++-- 5 files changed, 62 insertions(+), 42 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index cf761c88c..fdef9d7b3 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -67,7 +67,7 @@ module.exports = class KeyringController extends EventEmitter { currentFiat: this.configManager.getCurrentFiat(), conversionRate: this.configManager.getConversionRate(), conversionDate: this.configManager.getConversionDate(), - keyringTypes: this.keyringTypes.map((krt) => krt.type()), + keyringTypes: this.keyringTypes.map(krt => krt.type), identities: this.identities, } } @@ -312,7 +312,7 @@ module.exports = class KeyringController extends EventEmitter { getKeyringClassForType (type) { const Keyring = this.keyringTypes.reduce((res, kr) => { - if (kr.type() === type) { + if (kr.type === type) { return kr } else { return res diff --git a/app/scripts/keyrings/hd.js b/app/scripts/keyrings/hd.js index b98f29187..a28ef6736 100644 --- a/app/scripts/keyrings/hd.js +++ b/app/scripts/keyrings/hd.js @@ -2,17 +2,17 @@ const EventEmitter = require('events').EventEmitter const hdkey = require('ethereumjs-wallet/hdkey') const bip39 = require('bip39') const ethUtil = require('ethereumjs-util') -const sigUtil = require('../lib/sig-util') -const type = 'HD Key Tree' +// *Internal Deps +const sigUtil = require('../lib/sig-util') +// Options: const hdPathString = `m/44'/60'/0'/0` +const type = 'HD Key Tree' -module.exports = class HdKeyring extends EventEmitter { +class HdKeyring extends EventEmitter { - static type () { - return type - } + /* PUBLIC METHODS */ constructor (opts = {}) { super() @@ -20,6 +20,13 @@ module.exports = class HdKeyring extends EventEmitter { this.deserialize(opts) } + serialize () { + return { + mnemonic: this.mnemonic, + numberOfAccounts: this.wallets.length, + } + } + deserialize (opts = {}) { this.opts = opts || {} this.wallets = [] @@ -27,7 +34,7 @@ module.exports = class HdKeyring extends EventEmitter { this.root = null if ('mnemonic' in opts) { - this.initFromMnemonic(opts.mnemonic) + this._initFromMnemonic(opts.mnemonic) } if ('numberOfAccounts' in opts) { @@ -35,28 +42,9 @@ module.exports = class HdKeyring extends EventEmitter { } } - initFromMnemonic (mnemonic) { - this.mnemonic = mnemonic - const seed = bip39.mnemonicToSeed(mnemonic) - this.hdWallet = hdkey.fromMasterSeed(seed) - this.root = this.hdWallet.derivePath(hdPathString) - } - - serialize () { - return { - mnemonic: this.mnemonic, - numberOfAccounts: this.wallets.length, - } - } - - exportAccount (address) { - const wallet = this.getWalletForAccount(address) - return wallet.getPrivateKey().toString('hex') - } - addAccounts (numberOfAccounts = 1) { if (!this.root) { - this.initFromMnemonic(bip39.generateMnemonic()) + this._initFromMnemonic(bip39.generateMnemonic()) } const oldLen = this.wallets.length @@ -76,7 +64,7 @@ module.exports = class HdKeyring extends EventEmitter { // tx is an instance of the ethereumjs-transaction class. signTransaction (address, tx) { - const wallet = this.getWalletForAccount(address) + const wallet = this._getWalletForAccount(address) var privKey = wallet.getPrivateKey() tx.sign(privKey) return tx @@ -84,7 +72,7 @@ module.exports = class HdKeyring extends EventEmitter { // For eth_sign, we need to sign transactions: signMessage (withAccount, data) { - const wallet = this.getWalletForAccount(withAccount) + const wallet = this._getWalletForAccount(withAccount) const message = ethUtil.removeHexPrefix(data) var privKey = wallet.getPrivateKey() var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) @@ -92,10 +80,29 @@ module.exports = class HdKeyring extends EventEmitter { return rawMsgSig } - getWalletForAccount (account) { + exportAccount (address) { + const wallet = this._getWalletForAccount(address) + return wallet.getPrivateKey().toString('hex') + } + + + /* PRIVATE METHODS */ + + _initFromMnemonic (mnemonic) { + this.mnemonic = mnemonic + const seed = bip39.mnemonicToSeed(mnemonic) + this.hdWallet = hdkey.fromMasterSeed(seed) + this.root = this.hdWallet.derivePath(hdPathString) + } + + + _getWalletForAccount (account) { return this.wallets.find((w) => { const address = w.getAddress().toString('hex') return ((address === account) || (sigUtil.normalize(address) === account)) }) } } + +HdKeyring.type = type +module.exports = HdKeyring diff --git a/app/scripts/keyrings/simple.js b/app/scripts/keyrings/simple.js index ee743bc03..4fdccc4f7 100644 --- a/app/scripts/keyrings/simple.js +++ b/app/scripts/keyrings/simple.js @@ -4,7 +4,9 @@ const ethUtil = require('ethereumjs-util') const type = 'Simple Key Pair' const sigUtil = require('../lib/sig-util') -module.exports = class SimpleKeyring extends EventEmitter { +class SimpleKeyring extends EventEmitter { + + /* PUBLIC METHODS */ static type () { return type @@ -44,7 +46,7 @@ module.exports = class SimpleKeyring extends EventEmitter { // tx is an instance of the ethereumjs-transaction class. signTransaction (address, tx) { - const wallet = this.getWalletForAccount(address) + const wallet = this._getWalletForAccount(address) var privKey = wallet.getPrivateKey() tx.sign(privKey) return tx @@ -52,7 +54,7 @@ module.exports = class SimpleKeyring extends EventEmitter { // For eth_sign, we need to sign transactions: signMessage (withAccount, data) { - const wallet = this.getWalletForAccount(withAccount) + const wallet = this._getWalletForAccount(withAccount) const message = ethUtil.removeHexPrefix(data) var privKey = wallet.getPrivateKey() var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) @@ -60,8 +62,19 @@ module.exports = class SimpleKeyring extends EventEmitter { return rawMsgSig } - getWalletForAccount (account) { + exportAccount (address) { + const wallet = this._getWalletForAccount(address) + return wallet.getPrivateKey().toString('hex') + } + + + /* PRIVATE METHODS */ + + _getWalletForAccount (account) { return this.wallets.find(w => w.getAddress().toString('hex') === account) } } + +SimpleKeyring.type = type +module.exports = SimpleKeyring diff --git a/test/unit/keyrings/hd-test.js b/test/unit/keyrings/hd-test.js index c22ffc913..b841cd194 100644 --- a/test/unit/keyrings/hd-test.js +++ b/test/unit/keyrings/hd-test.js @@ -27,9 +27,9 @@ describe('hd-keyring', function() { assert.equal(accounts[1], secondAcct) }) - describe('Keyring.type()', function() { - it('is a class method that returns the type string.', function() { - const type = HdKeyring.type() + describe('Keyring.type', function() { + it('is a class property that returns the type string.', function() { + const type = HdKeyring.type assert.equal(typeof type, 'string') }) }) diff --git a/test/unit/keyrings/simple-test.js b/test/unit/keyrings/simple-test.js index ba000a7a8..96a2fdcf0 100644 --- a/test/unit/keyrings/simple-test.js +++ b/test/unit/keyrings/simple-test.js @@ -13,9 +13,9 @@ describe('simple-keyring', function() { keyring = new SimpleKeyring() }) - describe('Keyring.type()', function() { - it('is a class method that returns the type string.', function() { - const type = SimpleKeyring.type() + describe('Keyring.type', function() { + it('is a class property that returns the type string.', function() { + const type = SimpleKeyring.type assert.equal(type, TYPE_STR) }) }) -- cgit v1.2.3