From 2efab79f5bea899764c08dfb317ce70b5bebbb55 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 22 Nov 2016 23:16:36 -0800 Subject: [PATCH 1/6] Asynced keyrings and started on controller --- app/scripts/keyring-controller.js | 44 ++++++++++++++++++++----------- app/scripts/keyrings/hd.js | 17 +++++++----- app/scripts/keyrings/simple.js | 18 ++++++------- 3 files changed, 47 insertions(+), 32 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 68cf62f7a..1f58f3780 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -121,10 +121,13 @@ module.exports = class KeyringController extends EventEmitter { this.password = password const keyring = this.restoreKeyring(serialized) this.keyrings.push(keyring) - this.configManager.setSelectedAccount(keyring.getAccounts()[0]) - return this.persistAllKeyrings() + keyring.getAccounts() + .then((accounts) => { + this.configManager.setSelectedAccount(accounts[0]) + return this.persistAllKeyrings() + }) } - return + return Promise.resolve() }) } @@ -165,13 +168,18 @@ module.exports = class KeyringController extends EventEmitter { placeSeedWords (cb) { const firstKeyring = this.keyrings[0] - const seedWords = firstKeyring.serialize().mnemonic - this.configManager.setSeedWords(seedWords) + firstKeyring.serialize() + .then((serialized) => { - if (cb) { - cb() - } - this.emit('update') + const seedWords = serialized.mnemonic + this.configManager.setSeedWords(seedWords) + + if (cb) { + cb() + } + + this.emit('update') + }) } submitPassword (password, cb) { @@ -259,13 +267,19 @@ module.exports = class KeyringController extends EventEmitter { } persistAllKeyrings () { - const serialized = this.keyrings.map((keyring) => { - return { - type: keyring.type, - data: keyring.serialize(), - } + Promise.all(this.keyrings.map((keyring) => { + return Promise.all([keyring.type, keyring.serialize()]) + .then((serializedKeyringArray) => { + // Label the output values on each serialized Keyring: + return { + type: serializedKeyringArray[0], + data: serializedKeyringArray[1], + } + }) + })) + .then((serializedKeyrings) => { + return this.encryptor.encrypt(this.password, serializedKeyrings) }) - return this.encryptor.encrypt(this.password, serialized) .then((encryptedString) => { this.configManager.setVault(encryptedString) return true diff --git a/app/scripts/keyrings/hd.js b/app/scripts/keyrings/hd.js index a28ef6736..cfec56561 100644 --- a/app/scripts/keyrings/hd.js +++ b/app/scripts/keyrings/hd.js @@ -21,10 +21,10 @@ class HdKeyring extends EventEmitter { } serialize () { - return { + return Promise.resolve({ mnemonic: this.mnemonic, numberOfAccounts: this.wallets.length, - } + }) } deserialize (opts = {}) { @@ -40,6 +40,8 @@ class HdKeyring extends EventEmitter { if ('numberOfAccounts' in opts) { this.addAccounts(opts.numberOfAccounts) } + + return Promise.resolve() } addAccounts (numberOfAccounts = 1) { @@ -55,11 +57,12 @@ class HdKeyring extends EventEmitter { newWallets.push(wallet) this.wallets.push(wallet) } - return newWallets.map(w => w.getAddress().toString('hex')) + const hexWallets = newWallets.map(w => w.getAddress().toString('hex')) + return Promise.resolve(hexWallets) } getAccounts () { - return this.wallets.map(w => w.getAddress().toString('hex')) + return Promise.resolve(this.wallets.map(w => w.getAddress().toString('hex'))) } // tx is an instance of the ethereumjs-transaction class. @@ -67,7 +70,7 @@ class HdKeyring extends EventEmitter { const wallet = this._getWalletForAccount(address) var privKey = wallet.getPrivateKey() tx.sign(privKey) - return tx + return Promise.resolve(tx) } // For eth_sign, we need to sign transactions: @@ -77,12 +80,12 @@ class HdKeyring extends EventEmitter { var privKey = wallet.getPrivateKey() var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) - return rawMsgSig + return Promise.resolve(rawMsgSig) } exportAccount (address) { const wallet = this._getWalletForAccount(address) - return wallet.getPrivateKey().toString('hex') + return Promise.resolve(wallet.getPrivateKey().toString('hex')) } diff --git a/app/scripts/keyrings/simple.js b/app/scripts/keyrings/simple.js index 4fdccc4f7..8f339cf80 100644 --- a/app/scripts/keyrings/simple.js +++ b/app/scripts/keyrings/simple.js @@ -8,10 +8,6 @@ class SimpleKeyring extends EventEmitter { /* PUBLIC METHODS */ - static type () { - return type - } - constructor (opts) { super() this.type = type @@ -20,7 +16,7 @@ class SimpleKeyring extends EventEmitter { } serialize () { - return this.wallets.map(w => w.getPrivateKey().toString('hex')) + return Promise.resolve(this.wallets.map(w => w.getPrivateKey().toString('hex'))) } deserialize (wallets = []) { @@ -29,6 +25,7 @@ class SimpleKeyring extends EventEmitter { const wallet = Wallet.fromPrivateKey(b) return wallet }) + return Promise.resolve() } addAccounts (n = 1) { @@ -37,11 +34,12 @@ class SimpleKeyring extends EventEmitter { newWallets.push(Wallet.generate()) } this.wallets = this.wallets.concat(newWallets) - return newWallets.map(w => w.getAddress().toString('hex')) + const hexWallets = newWallets.map(w => w.getAddress().toString('hex')) + return Promise.resolve(hexWallets) } getAccounts () { - return this.wallets.map(w => w.getAddress().toString('hex')) + return Promise.resolve(this.wallets.map(w => w.getAddress().toString('hex'))) } // tx is an instance of the ethereumjs-transaction class. @@ -49,7 +47,7 @@ class SimpleKeyring extends EventEmitter { const wallet = this._getWalletForAccount(address) var privKey = wallet.getPrivateKey() tx.sign(privKey) - return tx + return Promise.resolve(tx) } // For eth_sign, we need to sign transactions: @@ -59,12 +57,12 @@ class SimpleKeyring extends EventEmitter { var privKey = wallet.getPrivateKey() var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) - return rawMsgSig + return Promise.resolve(rawMsgSig) } exportAccount (address) { const wallet = this._getWalletForAccount(address) - return wallet.getPrivateKey().toString('hex') + return Promise.resolve(wallet.getPrivateKey().toString('hex')) } From c77d355e98d3c61699a3a85dfc2ed1f320a9eb03 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 22 Nov 2016 23:36:11 -0800 Subject: [PATCH 2/6] Complete first pass at asyncrhonizing keyring controller --- app/scripts/keyring-controller.js | 84 +++++++++++++++++++------------ 1 file changed, 53 insertions(+), 31 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 1f58f3780..4cfe84b6b 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -117,17 +117,21 @@ module.exports = class KeyringController extends EventEmitter { const shouldMigrate = !!this.configManager.getWallet() && !this.configManager.getVault() return this.idStoreMigrator.migratedVaultForPassword(password) .then((serialized) => { + this.password = password + if (serialized && shouldMigrate) { - this.password = password - const keyring = this.restoreKeyring(serialized) - this.keyrings.push(keyring) - keyring.getAccounts() + return this.restoreKeyring(serialized) + .then((keyring) => { + return keyring.getAccounts() + }) .then((accounts) => { this.configManager.setSelectedAccount(accounts[0]) return this.persistAllKeyrings() }) + + } else { + return Promise.resolve() } - return Promise.resolve() }) } @@ -216,9 +220,13 @@ module.exports = class KeyringController extends EventEmitter { addNewAccount (keyRingNum = 0, cb) { const ring = this.keyrings[keyRingNum] - const accounts = ring.addAccounts(1) - this.setupAccounts(accounts) - this.persistAllKeyrings() + return ring.addAccounts(1) + .then((accounts) => { + return this.setupAccounts(accounts) + }) + .then(() => { + return this.persistAllKeyrings() + }) .then(() => { cb() }) @@ -228,9 +236,13 @@ module.exports = class KeyringController extends EventEmitter { } setupAccounts (accounts) { - var arr = accounts || this.getAccounts() - arr.forEach((account) => { - this.getBalanceAndNickname(account) + return this.getAccounts() + .then((loadedAccounts) => { + var arr = accounts || loadedAccounts + + arr.forEach((account) => { + this.getBalanceAndNickname(account) + }) }) } @@ -301,12 +313,13 @@ module.exports = class KeyringController extends EventEmitter { const Keyring = this.getKeyringClassForType(type) const keyring = new Keyring() keyring.deserialize(data) - - const accounts = keyring.getAccounts() - this.setupAccounts(accounts) - - this.keyrings.push(keyring) - return keyring + .then(() => { + return keyring.getAccounts() + }) + .then((accounts) => { + this.setupAccounts(accounts) + this.keyrings.push(keyring) + }) } getKeyringClassForType (type) { @@ -529,17 +542,19 @@ module.exports = class KeyringController extends EventEmitter { txParams.nonce = normalize(txParams.nonce) let tx = new Transaction(txParams) - tx = keyring.signTransaction(address, tx) - - // Add the tx hash to the persisted meta-tx object - var txHash = ethUtil.bufferToHex(tx.hash()) - var metaTx = this.configManager.getTx(txParams.metamaskId) - metaTx.hash = txHash - this.configManager.updateTx(metaTx) + keyring.signTransaction(address, tx) + .then((tx) => { + // Add the tx hash to the persisted meta-tx object + var txHash = ethUtil.bufferToHex(tx.hash()) + var metaTx = this.configManager.getTx(txParams.metamaskId) + metaTx.hash = txHash + this.configManager.updateTx(metaTx) + + // return raw serialized tx + var rawTx = ethUtil.bufferToHex(tx.serialize()) + cb(null, rawTx) + }) - // return raw serialized tx - var rawTx = ethUtil.bufferToHex(tx.serialize()) - cb(null, rawTx) } catch (e) { cb(e) } @@ -549,8 +564,11 @@ module.exports = class KeyringController extends EventEmitter { try { const keyring = this.getKeyringForAccount(msgParams.from) const address = normalize(msgParams.from) - const rawSig = keyring.signMessage(address, msgParams.data) - cb(null, rawSig) + return keyring.signMessage(address, msgParams.data) + .then((rawSig) => { + cb(null, rawSig) + return rawSig + }) } catch (e) { cb(e) } @@ -581,10 +599,14 @@ module.exports = class KeyringController extends EventEmitter { exportAccount (address, cb) { try { const keyring = this.getKeyringForAccount(address) - const privateKey = keyring.exportAccount(normalize(address)) - cb(null, privateKey) + return keyring.exportAccount(normalize(address)) + .then((privateKey) => { + cb(null, privateKey) + return privateKey + }) } catch (e) { cb(e) + return Promise.reject(e) } } From 600f5c31db47a06c0c9bb9a97c01476d77b0a0df Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Nov 2016 00:23:41 -0800 Subject: [PATCH 3/6] Mostly got async keyringController tests passing --- app/scripts/keyring-controller.js | 16 ++++--- test/unit/keyring-controller-test.js | 42 +++++++++-------- test/unit/keyrings/hd-test.js | 67 ++++++++++++++++++---------- test/unit/keyrings/simple-test.js | 37 +++++++++------ 4 files changed, 100 insertions(+), 62 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 4cfe84b6b..a36f5b752 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -99,16 +99,17 @@ module.exports = class KeyringController extends EventEmitter { mnemonic: seed, numberOfAccounts: 1, }, (err) => { - if (err) return cb(err) const firstKeyring = this.keyrings[0] const accounts = firstKeyring.getAccounts() const firstAccount = accounts[0] const hexAccount = normalize(firstAccount) this.configManager.setSelectedAccount(hexAccount) this.setupAccounts(accounts) - - this.emit('update') - cb() + this.persistAllKeyrings() + .then(() => { + this.emit('update') + cb(err) + }) }) }) } @@ -152,6 +153,7 @@ module.exports = class KeyringController extends EventEmitter { createFirstKeyTree (password, cb) { this.clearKeyrings() this.addNewKeyring('HD Key Tree', {numberOfAccounts: 1}, (err) => { + if (err) return cb(err) const accounts = this.keyrings[0].getAccounts() const firstAccount = accounts[0] const hexAccount = normalize(firstAccount) @@ -162,7 +164,7 @@ module.exports = class KeyringController extends EventEmitter { this.setupAccounts(accounts) this.persistAllKeyrings() .then(() => { - cb(err) + cb() }) .catch((reason) => { cb(reason) @@ -335,10 +337,10 @@ module.exports = class KeyringController extends EventEmitter { getAccounts () { const keyrings = this.keyrings || [] - return keyrings.map(kr => kr.getAccounts()) + return Promise.all(keyrings.map(kr => kr.getAccounts()) .reduce((res, arr) => { return res.concat(arr) - }, []) + }, [])) } setSelectedAccount (address, cb) { diff --git a/test/unit/keyring-controller-test.js b/test/unit/keyring-controller-test.js index b20def02e..d35c8927f 100644 --- a/test/unit/keyring-controller-test.js +++ b/test/unit/keyring-controller-test.js @@ -61,7 +61,7 @@ describe('KeyringController', function() { describe('#restoreKeyring', function() { - it(`should pass a keyring's serialized data back to the correct type.`, function() { + it(`should pass a keyring's serialized data back to the correct type.`, function(done) { const mockSerialized = { type: 'HD Key Tree', data: { @@ -74,12 +74,14 @@ describe('KeyringController', function() { mock.expects('getBalanceAndNickname') .exactly(1) - var keyring = keyringController.restoreKeyring(mockSerialized) - assert.equal(keyring.wallets.length, 1, 'one wallet restored') - assert.equal(keyring.getAccounts()[0], addresses[0]) - mock.verify() + keyringController.restoreKeyring(mockSerialized) + .then((keyring) => { + assert.equal(keyring.wallets.length, 1, 'one wallet restored') + assert.equal(keyring.getAccounts()[0], addresses[0]) + mock.verify() + done() + }) }) - }) describe('#migrateOldVaultIfAny', function() { @@ -92,6 +94,7 @@ describe('KeyringController', function() { }) .catch((reason) => { assert.ifError(reason) + done() }) }) }) @@ -110,15 +113,17 @@ describe('KeyringController', function() { }) describe('#saveAccountLabel', function() { - it ('sets the nickname', function() { + it ('sets the nickname', function(done) { const account = addresses[0] var nick = 'Test nickname' keyringController.identities[ethUtil.addHexPrefix(account)] = {} - const label = keyringController.saveAccountLabel(account, nick) - assert.equal(label, nick) - - const persisted = keyringController.configManager.nicknameForWallet(account) - assert.equal(persisted, nick) + keyringController.saveAccountLabel(account, nick, (err, label) => { + assert.ifError(err) + assert.equal(label, nick) + const persisted = keyringController.configManager.nicknameForWallet(account) + assert.equal(persisted, nick) + done() + }) }) this.timeout(10000) @@ -126,9 +131,7 @@ describe('KeyringController', function() { const account = addresses[0] var nick = 'Test nickname' keyringController.configManager.setNicknameForWallet(account, nick) - console.log('calling to restore') keyringController.createNewVaultAndRestore(password, seedWords, (err, state) => { - console.dir({err}) assert.ifError(err) const identity = keyringController.identities['0x' + account] @@ -143,12 +146,15 @@ describe('KeyringController', function() { describe('#getAccounts', function() { it('returns the result of getAccounts for each keyring', function() { keyringController.keyrings = [ - { getAccounts() { return [1,2,3] } }, - { getAccounts() { return [4,5,6] } }, + { getAccounts() { return Promise.resolve([1,2,3]) } }, + { getAccounts() { return Promise.resolve([4,5,6]) } }, ] - const result = keyringController.getAccounts() - assert.deepEqual(result, [1,2,3,4,5,6]) + keyringController.getAccounts() + .then((result) => { + assert.deepEqual(result, [1,2,3,4,5,6]) + done() + }) }) }) diff --git a/test/unit/keyrings/hd-test.js b/test/unit/keyrings/hd-test.js index bec1a8134..2c984267e 100644 --- a/test/unit/keyrings/hd-test.js +++ b/test/unit/keyrings/hd-test.js @@ -16,15 +16,18 @@ describe('hd-keyring', function() { keyring = new HdKeyring() }) - describe('constructor', function() { + describe('constructor', function(done) { keyring = new HdKeyring({ mnemonic: sampleMnemonic, numberOfAccounts: 2, }) const accounts = keyring.getAccounts() - assert.equal(accounts[0], firstAcct) - assert.equal(accounts[1], secondAcct) + .then((accounts) => { + assert.equal(accounts[0], firstAcct) + assert.equal(accounts[1], secondAcct) + done() + }) }) describe('Keyring.type', function() { @@ -44,49 +47,62 @@ describe('hd-keyring', function() { describe('#serialize empty wallets.', function() { it('serializes a new mnemonic', function() { - const output = keyring.serialize() - assert.equal(output.numberOfAccounts, 0) - assert.equal(output.mnemonic, null) + keyring.serialize() + .then((output) => { + assert.equal(output.numberOfAccounts, 0) + assert.equal(output.mnemonic, null) + }) }) }) describe('#deserialize a private key', function() { - it('serializes what it deserializes', function() { + it('serializes what it deserializes', function(done) { keyring.deserialize({ mnemonic: sampleMnemonic, numberOfAccounts: 1 }) - assert.equal(keyring.wallets.length, 1, 'restores two accounts') - keyring.addAccounts(1) - - const accounts = keyring.getAccounts() - assert.equal(accounts[0], firstAcct) - assert.equal(accounts[1], secondAcct) - assert.equal(accounts.length, 2) - - const serialized = keyring.serialize() - assert.equal(serialized.mnemonic, sampleMnemonic) + .then(() => { + assert.equal(keyring.wallets.length, 1, 'restores two accounts') + keyring.addAccounts(1) + + const accounts = keyring.getAccounts() + assert.equal(accounts[0], firstAcct) + assert.equal(accounts[1], secondAcct) + assert.equal(accounts.length, 2) + + keyring.serialize() + .then((serialized) => { + assert.equal(serialized.mnemonic, sampleMnemonic) + done() + }) + }) }) }) describe('#addAccounts', function() { describe('with no arguments', function() { - it('creates a single wallet', function() { + it('creates a single wallet', function(done) { keyring.addAccounts() - assert.equal(keyring.wallets.length, 1) + .then(() => { + assert.equal(keyring.wallets.length, 1) + done() + }) }) }) describe('with a numeric argument', function() { - it('creates that number of wallets', function() { + it('creates that number of wallets', function(done) { keyring.addAccounts(3) - assert.equal(keyring.wallets.length, 3) + .then(() => { + assert.equal(keyring.wallets.length, 3) + done() + }) }) }) }) describe('#getAccounts', function() { - it('calls getAddress on each wallet', function() { + it('calls getAddress on each wallet', function(done) { // Push a mock wallet const desiredOutput = 'foo' @@ -101,8 +117,11 @@ describe('hd-keyring', function() { }) const output = keyring.getAccounts() - assert.equal(output[0], desiredOutput) - assert.equal(output.length, 1) + .then((output) => { + assert.equal(output[0], desiredOutput) + assert.equal(output.length, 1) + done() + }) }) }) }) diff --git a/test/unit/keyrings/simple-test.js b/test/unit/keyrings/simple-test.js index 96a2fdcf0..979abdb69 100644 --- a/test/unit/keyrings/simple-test.js +++ b/test/unit/keyrings/simple-test.js @@ -28,19 +28,23 @@ describe('simple-keyring', function() { }) describe('#serialize empty wallets.', function() { - it('serializes an empty array', function() { - const output = keyring.serialize() - assert.deepEqual(output, []) + it('serializes an empty array', function(done) { + keyring.serialize() + .then((output) => { + assert.deepEqual(output, []) + done() + }) }) }) describe('#deserialize a private key', function() { it('serializes what it deserializes', function() { keyring.deserialize([privKeyHex]) - assert.equal(keyring.wallets.length, 1, 'has one wallet') - - const serialized = keyring.serialize() - assert.equal(serialized[0], privKeyHex) + .then(() => { + assert.equal(keyring.wallets.length, 1, 'has one wallet') + const serialized = keyring.serialize() + assert.equal(serialized[0], privKeyHex) + }) }) }) @@ -48,20 +52,24 @@ describe('simple-keyring', function() { describe('with no arguments', function() { it('creates a single wallet', function() { keyring.addAccounts() - assert.equal(keyring.wallets.length, 1) + .then(() => { + assert.equal(keyring.wallets.length, 1) + }) }) }) describe('with a numeric argument', function() { it('creates that number of wallets', function() { keyring.addAccounts(3) - assert.equal(keyring.wallets.length, 3) + .then(() => { + assert.equal(keyring.wallets.length, 3) + }) }) }) }) describe('#getAccounts', function() { - it('calls getAddress on each wallet', function() { + it('calls getAddress on each wallet', function(done) { // Push a mock wallet const desiredOutput = 'foo' @@ -75,9 +83,12 @@ describe('simple-keyring', function() { } }) - const output = keyring.getAccounts() - assert.equal(output[0], desiredOutput) - assert.equal(output.length, 1) + keyring.getAccounts() + .then((output) => { + assert.equal(output[0], desiredOutput) + assert.equal(output.length, 1) + done() + }) }) }) }) From 230a0ab876df82edc8147fd3c0ba18f10ffc4e3c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Nov 2016 11:58:34 -0800 Subject: [PATCH 4/6] Fix more keyring asyncifying tests --- app/scripts/keyring-controller.js | 5 +++-- test/unit/keyring-controller-test.js | 9 ++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index a36f5b752..f93202523 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -281,7 +281,7 @@ module.exports = class KeyringController extends EventEmitter { } persistAllKeyrings () { - Promise.all(this.keyrings.map((keyring) => { + return Promise.all(this.keyrings.map((keyring) => { return Promise.all([keyring.type, keyring.serialize()]) .then((serializedKeyringArray) => { // Label the output values on each serialized Keyring: @@ -314,13 +314,14 @@ module.exports = class KeyringController extends EventEmitter { const { type, data } = serialized const Keyring = this.getKeyringClassForType(type) const keyring = new Keyring() - keyring.deserialize(data) + return keyring.deserialize(data) .then(() => { return keyring.getAccounts() }) .then((accounts) => { this.setupAccounts(accounts) this.keyrings.push(keyring) + return keyring }) } diff --git a/test/unit/keyring-controller-test.js b/test/unit/keyring-controller-test.js index d35c8927f..056e465ca 100644 --- a/test/unit/keyring-controller-test.js +++ b/test/unit/keyring-controller-test.js @@ -77,10 +77,17 @@ describe('KeyringController', function() { keyringController.restoreKeyring(mockSerialized) .then((keyring) => { assert.equal(keyring.wallets.length, 1, 'one wallet restored') - assert.equal(keyring.getAccounts()[0], addresses[0]) + return keyring.getAccounts() + }) + .then((accounts) => { + assert.equal(accounts[0], addresses[0]) mock.verify() done() }) + .catch((reason) => { + assert.ifError(reason) + done() + }) }) }) From d9dc2eac6305abda8da1113f16868bd362283582 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Nov 2016 14:35:29 -0800 Subject: [PATCH 5/6] Fix more async usage of KeyringController --- app/scripts/keyring-controller.js | 57 +++++++++++++++++-------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index f93202523..4981c49df 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -98,9 +98,11 @@ module.exports = class KeyringController extends EventEmitter { this.addNewKeyring('HD Key Tree', { mnemonic: seed, numberOfAccounts: 1, - }, (err) => { + }).then(() => { const firstKeyring = this.keyrings[0] - const accounts = firstKeyring.getAccounts() + return firstKeyring.getAccounts() + }) + .then((accounts) => { const firstAccount = accounts[0] const hexAccount = normalize(firstAccount) this.configManager.setSelectedAccount(hexAccount) @@ -108,7 +110,7 @@ module.exports = class KeyringController extends EventEmitter { this.persistAllKeyrings() .then(() => { this.emit('update') - cb(err) + cb(err, this.getState()) }) }) }) @@ -129,7 +131,6 @@ module.exports = class KeyringController extends EventEmitter { this.configManager.setSelectedAccount(accounts[0]) return this.persistAllKeyrings() }) - } else { return Promise.resolve() } @@ -154,15 +155,17 @@ module.exports = class KeyringController extends EventEmitter { this.clearKeyrings() this.addNewKeyring('HD Key Tree', {numberOfAccounts: 1}, (err) => { if (err) return cb(err) - const accounts = this.keyrings[0].getAccounts() - const firstAccount = accounts[0] - const hexAccount = normalize(firstAccount) - this.configManager.setSelectedAccount(firstAccount) + this.keyrings[0].getAccounts() + .then((accounts) => { + const firstAccount = accounts[0] + const hexAccount = normalize(firstAccount) + this.configManager.setSelectedAccount(firstAccount) - this.placeSeedWords() - this.emit('newAccount', hexAccount) - this.setupAccounts(accounts) - this.persistAllKeyrings() + this.placeSeedWords() + this.emit('newAccount', hexAccount) + this.setupAccounts(accounts) + return this.persistAllKeyrings() + }) .then(() => { cb() }) @@ -176,7 +179,6 @@ module.exports = class KeyringController extends EventEmitter { const firstKeyring = this.keyrings[0] firstKeyring.serialize() .then((serialized) => { - const seedWords = serialized.mnemonic this.configManager.setSeedWords(seedWords) @@ -207,16 +209,23 @@ module.exports = class KeyringController extends EventEmitter { addNewKeyring (type, opts, cb) { const Keyring = this.getKeyringClassForType(type) const keyring = new Keyring(opts) - const accounts = keyring.getAccounts() - - this.keyrings.push(keyring) - this.setupAccounts(accounts) - this.persistAllKeyrings() - .then(() => { - cb() + return keyring.getAccounts() + .then((accounts) => { + this.keyrings.push(keyring) + return this.setupAccounts(accounts) + }).then(() => { + return this.persistAllKeyrings() + }).then(() => { + if (cb) { + cb(null, keyring) + } + return keyring }) .catch((reason) => { - cb(reason) + if (cb) { + cb(reason) + } + return reason }) } @@ -240,8 +249,7 @@ module.exports = class KeyringController extends EventEmitter { setupAccounts (accounts) { return this.getAccounts() .then((loadedAccounts) => { - var arr = accounts || loadedAccounts - + const arr = accounts || loadedAccounts arr.forEach((account) => { this.getBalanceAndNickname(account) }) @@ -544,7 +552,7 @@ module.exports = class KeyringController extends EventEmitter { txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) txParams.nonce = normalize(txParams.nonce) - let tx = new Transaction(txParams) + const tx = new Transaction(txParams) keyring.signTransaction(address, tx) .then((tx) => { // Add the tx hash to the persisted meta-tx object @@ -557,7 +565,6 @@ module.exports = class KeyringController extends EventEmitter { var rawTx = ethUtil.bufferToHex(tx.serialize()) cb(null, rawTx) }) - } catch (e) { cb(e) } From 9f67974133ae26b37d388cf95c995f76c0ee650f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Nov 2016 14:39:35 -0800 Subject: [PATCH 6/6] Fix last async keyring test --- test/unit/keyrings/hd-test.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/unit/keyrings/hd-test.js b/test/unit/keyrings/hd-test.js index 2c984267e..2d9e0ffd9 100644 --- a/test/unit/keyrings/hd-test.js +++ b/test/unit/keyrings/hd-test.js @@ -57,24 +57,26 @@ describe('hd-keyring', function() { describe('#deserialize a private key', function() { it('serializes what it deserializes', function(done) { + console.log('deserializing ' + sampleMnemonic) keyring.deserialize({ mnemonic: sampleMnemonic, numberOfAccounts: 1 }) .then(() => { + console.dir(keyring) assert.equal(keyring.wallets.length, 1, 'restores two accounts') - keyring.addAccounts(1) - - const accounts = keyring.getAccounts() + return keyring.addAccounts(1) + }).then(() => { + return keyring.getAccounts() + }).then((accounts) => { assert.equal(accounts[0], firstAcct) assert.equal(accounts[1], secondAcct) assert.equal(accounts.length, 2) - keyring.serialize() - .then((serialized) => { - assert.equal(serialized.mnemonic, sampleMnemonic) - done() - }) + return keyring.serialize() + }).then((serialized) => { + assert.equal(serialized.mnemonic, sampleMnemonic) + done() }) }) })