From 36dc63bc048e62b5ef0d1b0385e530afdad3fefa Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 9 Sep 2016 19:42:18 -0700 Subject: [PATCH 01/14] Add new eth-lightwallet salting to vault. eth-lightwallet was previously not salting vault passwords, potentially making it easier to crack them once obtained. This branch incorporates the API changes to allow us to take advantage of the new salting logic. This is still throwing deprecation warnings, but that's actually a bug in eth-lightwallet I wrote, [I've submitted a PR for that here](https://github.com/ConsenSys/eth-lightwallet/pull/116). Fixes #555 --- app/scripts/lib/idStore.js | 100 ++++++++++++++++--------------------- test/unit/idStore-test.js | 4 +- 2 files changed, 46 insertions(+), 58 deletions(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 26aa02ef7..9f4961b0b 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -3,7 +3,7 @@ const inherits = require('util').inherits const async = require('async') const ethUtil = require('ethereumjs-util') const EthQuery = require('eth-query') -const LightwalletKeyStore = require('eth-lightwallet').keystore +const KeyStore = require('eth-lightwallet').keystore const clone = require('clone') const extend = require('xtend') const createId = require('web3-provider-engine/util/random-id') @@ -50,15 +50,15 @@ IdentityStore.prototype.createNewVault = function (password, entropy, cb) { if (serializedKeystore) { this.configManager.setData({}) } + this._createIdmgmt(password, null, entropy, (err) => { if (err) return cb(err) - this._loadIdentities() - this._didUpdate() this._autoFaucet() this.configManager.setShowSeedWords(true) var seedWords = this._idmgmt.getSeed() + cb(null, seedWords) }) } @@ -143,6 +143,7 @@ IdentityStore.prototype.revealAccount = function (cb) { keyStore.setDefaultHdDerivationPath(this.hdPathString) keyStore.generateNewAddress(derivedKey, 1) + configManager.setWallet(keyStore.serialize()) this._loadIdentities() @@ -436,72 +437,57 @@ IdentityStore.prototype._mayBeFauceting = function (i) { // IdentityStore.prototype.tryPassword = function (password, cb) { - this._createIdmgmt(password, null, null, cb) -} - -IdentityStore.prototype._createIdmgmt = function (password, seed, entropy, cb) { - const configManager = this.configManager + var serializedKeystore = this.configManager.getWallet() + var keyStore = KeyStore.deserialize(serializedKeystore) - var keyStore = null - LightwalletKeyStore.deriveKeyFromPassword(password, (err, derivedKey) => { + keyStore.keyFromPassword(password, (err, pwDerivedKey) => { if (err) return cb(err) - var serializedKeystore = configManager.getWallet() - - if (seed) { - try { - keyStore = this._restoreFromSeed(password, seed, derivedKey) - } catch (e) { - return cb(e) - } - - // returning user, recovering from storage - } else if (serializedKeystore) { - keyStore = LightwalletKeyStore.deserialize(serializedKeystore) - var isCorrect = keyStore.isDerivedKeyCorrect(derivedKey) - if (!isCorrect) return cb(new Error('Lightwallet - password incorrect')) - - // first time here - } else { - keyStore = this._createFirstWallet(entropy, derivedKey) - } - this._keyStore = keyStore - this._idmgmt = new IdManagement({ - keyStore: keyStore, - derivedKey: derivedKey, - hdPathSTring: this.hdPathString, - configManager: this.configManager, - }) + const isCorrect = keyStore.isDerivedKeyCorrect(pwDerivedKey) + if (!isCorrect) return cb(new Error('Lightwallet - password incorrect')) cb() }) } -IdentityStore.prototype._restoreFromSeed = function (password, seed, derivedKey) { - const configManager = this.configManager - var keyStore = new LightwalletKeyStore(seed, derivedKey, this.hdPathString) - keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) - keyStore.setDefaultHdDerivationPath(this.hdPathString) - - keyStore.generateNewAddress(derivedKey, 1) - configManager.setWallet(keyStore.serialize()) - if (global.METAMASK_DEBUG) { - console.log('restored from seed. saved to keystore') +IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, cb) { + const opts = { password } + if (seedPhrase) { + opts.seedPhrase = seedPhrase } - return keyStore + + KeyStore.createVault(opts, (err, keyStore) => { + if (err) return cb(err) + + this._keyStore = keyStore + + keyStore.keyFromPassword(password, (err, derivedKey) => { + if (err) return cb(err) + + keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) + + this._createFirstWallet(derivedKey) + + this._idmgmt = new IdManagement({ + keyStore: keyStore, + derivedKey: derivedKey, + configManager: this.configManager, + }) + + cb() + this._loadIdentities() + this._didUpdate() + }) + }) } -IdentityStore.prototype._createFirstWallet = function (entropy, derivedKey) { - const configManager = this.configManager - var secretSeed = LightwalletKeyStore.generateRandomSeed(entropy) - var keyStore = new LightwalletKeyStore(secretSeed, derivedKey, this.hdPathString) - keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) +IdentityStore.prototype._createFirstWallet = function (derivedKey) { + const keyStore = this._keyStore keyStore.setDefaultHdDerivationPath(this.hdPathString) - - keyStore.generateNewAddress(derivedKey, 1) - configManager.setWallet(keyStore.serialize()) - console.log('saved to keystore') - return keyStore + keyStore.generateNewAddress(derivedKey) + var addresses = keyStore.getAddresses() + this._ethStore.addAccount(addresses[0]) + this.configManager.setWallet(keyStore.serialize()) } // get addresses and normalize address hexString diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index ee4613236..cbbec43b5 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -23,6 +23,7 @@ describe('IdentityStore', function() { }) idStore.createNewVault(password, entropy, (err, seeds) => { + assert.ifError(err, 'createNewVault threw error') seedWords = seeds originalKeystore = idStore._idmgmt.keyStore done() @@ -59,6 +60,7 @@ describe('IdentityStore', function() { describe('#recoverFromSeed BIP44 compliance', function() { let seedWords = 'picnic injury awful upper eagle junk alert toss flower renew silly vague' let firstAccount = '0x5d8de92c205279c10e5669f797b853ccef4f739a' + const salt = 'lightwalletSalt' let password = 'secret!' let accounts = [] @@ -70,7 +72,7 @@ describe('IdentityStore', function() { idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { accounts.push(acct) }, + addAccount(acct) { accounts.push('0x' + acct) }, }, }) }) From 03168c6b91f024d928c426868facc556f3ba427c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 9 Sep 2016 19:52:37 -0700 Subject: [PATCH 02/14] Bump changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d89c2438..ff1d4c2e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - MetaMask logo now renders as super lightweight SVG, improving compatibility and performance. - Now showing loading indication during vault unlocking, to clarify behavior for users who are experience slow unlocks. - Now only initially creates one wallet when restoring a vault, to reduce some users' confusion. +- Improved the security of vault encryption by ensuring passwords are always uniquely salted. ## 2.10.2 2016-09-02 From 6d6130e2a6d79a29d67995cbf4fa22ea4870ef69 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 11:23:11 -0700 Subject: [PATCH 03/14] Camelcase dataset key for react --- ui/app/components/shapeshift-form.js | 4 ++-- ui/app/first-time/restore-vault.js | 6 +++--- ui/app/send.js | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ui/app/components/shapeshift-form.js b/ui/app/components/shapeshift-form.js index 58b7942c3..77c0d12b1 100644 --- a/ui/app/components/shapeshift-form.js +++ b/ui/app/components/shapeshift-form.js @@ -69,7 +69,7 @@ ShapeshiftForm.prototype.renderMain = function () { h('input#fromCoin.buy-inputs.ex-coins', { type: 'text', list: 'coinList', - dataset: { + dataSet: { persistentFormId: 'input-coin', }, style: { @@ -165,7 +165,7 @@ ShapeshiftForm.prototype.renderMain = function () { h('input#fromCoinAddress.buy-inputs', { type: 'text', placeholder: `Your ${coin} Refund Address`, - dataset: { + dataSet: { persistentFormId: 'refund-address', }, style: { diff --git a/ui/app/first-time/restore-vault.js b/ui/app/first-time/restore-vault.js index 4c1f21008..c34cd7e66 100644 --- a/ui/app/first-time/restore-vault.js +++ b/ui/app/first-time/restore-vault.js @@ -41,7 +41,7 @@ RestoreVaultScreen.prototype.render = function () { // wallet seed entry h('h3', 'Wallet Seed'), h('textarea.twelve-word-phrase.letter-spacey', { - dataset: { + dataSet: { persistentFormId: 'wallet-seed', }, placeholder: 'Enter your secret twelve word phrase here to restore your vault.', @@ -52,7 +52,7 @@ RestoreVaultScreen.prototype.render = function () { type: 'password', id: 'password-box', placeholder: 'New Password (min 8 chars)', - dataset: { + dataSet: { persistentFormId: 'password', }, style: { @@ -67,7 +67,7 @@ RestoreVaultScreen.prototype.render = function () { id: 'password-box-confirm', placeholder: 'Confirm Password', onKeyPress: this.onMaybeCreate.bind(this), - dataset: { + dataSet: { persistentFormId: 'password-confirmation', }, style: { diff --git a/ui/app/send.js b/ui/app/send.js index 009866cf7..e660e90a8 100644 --- a/ui/app/send.js +++ b/ui/app/send.js @@ -138,7 +138,7 @@ SendTransactionScreen.prototype.render = function () { h('input.large-input', { name: 'address', placeholder: 'Recipient Address', - dataset: { + dataSet: { persistentFormId: 'recipient-address', }, }), @@ -154,7 +154,7 @@ SendTransactionScreen.prototype.render = function () { style: { marginRight: 6, }, - dataset: { + dataSet: { persistentFormId: 'tx-amount', }, }), @@ -192,7 +192,7 @@ SendTransactionScreen.prototype.render = function () { width: '100%', resize: 'none', }, - dataset: { + dataSet: { persistentFormId: 'tx-data', }, }), From 6763871c416001e19c224b90a99ed7d9ece69158 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 11:46:50 -0700 Subject: [PATCH 04/14] Captured #640 in failing test --- app/scripts/lib/idStore.js | 5 +++++ test/unit/idStore-test.js | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 9f4961b0b..d2d37b0f4 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -464,6 +464,11 @@ IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, keyStore.keyFromPassword(password, (err, derivedKey) => { if (err) return cb(err) + this._ethStore._currentState = { + accounts: {}, + transactions: {}, + } + keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) this._createFirstWallet(derivedKey) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index cbbec43b5..3d28ddf90 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -83,8 +83,19 @@ describe('IdentityStore', function() { assert.ifError(err) let newKeystore = idStore._idmgmt.keyStore + assert.equal(accounts[0], firstAccount) - done() + + accounts = [] + const secondSeed = 'radar blur cabbage chef fix engine embark joy scheme fiction master release' + const secondAcct = '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9' + + idStore.recoverFromSeed(password, secondSeed, (err) => { + + let accounts = idStore._getAddresses() + assert.equal(accounts[0], secondAcct) + done() + }) }) }) }) From 59fd86383fbf379724c3bcca6da9d5a6192bbcf2 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 12:08:27 -0700 Subject: [PATCH 05/14] Correctly clear ethStore cache on new vault restore --- app/scripts/lib/idStore.js | 23 ++++++++++++++++------- test/unit/idStore-test.js | 28 ++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index d2d37b0f4..69ffd3f72 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -451,7 +451,11 @@ IdentityStore.prototype.tryPassword = function (password, cb) { } IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, cb) { - const opts = { password } + const opts = { + password, + hdPathString: this.hdPathString, + } + if (seedPhrase) { opts.seedPhrase = seedPhrase } @@ -464,10 +468,7 @@ IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, keyStore.keyFromPassword(password, (err, derivedKey) => { if (err) return cb(err) - this._ethStore._currentState = { - accounts: {}, - transactions: {}, - } + this.purgeCache() keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) @@ -486,10 +487,16 @@ IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, }) } +IdentityStore.prototype.purgeCache = function () { + this._getAddresses().forEach((address) => { + this._ethStore.del(address) + }) +} + IdentityStore.prototype._createFirstWallet = function (derivedKey) { const keyStore = this._keyStore keyStore.setDefaultHdDerivationPath(this.hdPathString) - keyStore.generateNewAddress(derivedKey) + keyStore.generateNewAddress(derivedKey, 1) var addresses = keyStore.getAddresses() this._ethStore.addAccount(addresses[0]) this.configManager.setWallet(keyStore.serialize()) @@ -497,7 +504,9 @@ IdentityStore.prototype._createFirstWallet = function (derivedKey) { // get addresses and normalize address hexString IdentityStore.prototype._getAddresses = function () { - return this._keyStore.getAddresses(this.hdPathString).map((address) => { return '0x' + address }) + return this._keyStore.getAddresses(this.hdPathString).map((address) => { + return ethUtil.addHexPrefix(address) + }) } IdentityStore.prototype._autoFaucet = function () { diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 3d28ddf90..422db6f7b 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -1,6 +1,7 @@ var assert = require('assert') var IdentityStore = require('../../app/scripts/lib/idStore') var configManagerGen = require('../lib/mock-config-manager') +const ethUtil = require('ethereumjs-util') describe('IdentityStore', function() { @@ -18,7 +19,7 @@ describe('IdentityStore', function() { idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { accounts.push(acct) }, + addAccount(acct) { accounts.push(ethUtil.addHexPrefix(acct)) }, }, }) @@ -39,7 +40,7 @@ describe('IdentityStore', function() { idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { newAccounts.push(acct) }, + addAccount(acct) { newAccounts.push(ethUtil.addHexPrefix(acct)) }, }, }) }) @@ -62,6 +63,9 @@ describe('IdentityStore', function() { let firstAccount = '0x5d8de92c205279c10e5669f797b853ccef4f739a' const salt = 'lightwalletSalt' + const secondSeed = 'radar blur cabbage chef fix engine embark joy scheme fiction master release' + const secondAcct = '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9' + let password = 'secret!' let accounts = [] let idStore @@ -72,11 +76,15 @@ describe('IdentityStore', function() { idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { accounts.push('0x' + acct) }, + addAccount(acct) { accounts.push(ethUtil.addHexPrefix(acct)) }, }, }) }) + beforeEach(function() { + accounts = [] + }) + it('should return the expected first account', function (done) { idStore.recoverFromSeed(password, seedWords, (err) => { @@ -87,9 +95,6 @@ describe('IdentityStore', function() { assert.equal(accounts[0], firstAccount) accounts = [] - const secondSeed = 'radar blur cabbage chef fix engine embark joy scheme fiction master release' - const secondAcct = '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9' - idStore.recoverFromSeed(password, secondSeed, (err) => { let accounts = idStore._getAddresses() @@ -98,5 +103,16 @@ describe('IdentityStore', function() { }) }) }) + + it('should return the expected second account', function (done) { + idStore.recoverFromSeed(password, secondSeed, (err) => { + assert.ifError(err) + + let newKeystore = idStore._idmgmt.keyStore + + assert.equal(accounts[0], firstAccount) + done() + }) + }) }) }) From 1b77d5300b1ace16f96cf626d69925a26b5f0d29 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 12:15:05 -0700 Subject: [PATCH 06/14] Clean up tests --- test/unit/idStore-test.js | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 422db6f7b..1028e35ea 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -59,12 +59,7 @@ describe('IdentityStore', function() { }) describe('#recoverFromSeed BIP44 compliance', function() { - let seedWords = 'picnic injury awful upper eagle junk alert toss flower renew silly vague' - let firstAccount = '0x5d8de92c205279c10e5669f797b853ccef4f739a' - const salt = 'lightwalletSalt' - - const secondSeed = 'radar blur cabbage chef fix engine embark joy scheme fiction master release' - const secondAcct = '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9' + const salt = 'lightwalletSalt' let password = 'secret!' let accounts = [] @@ -86,31 +81,35 @@ describe('IdentityStore', function() { }) it('should return the expected first account', function (done) { + let seedWords = 'picnic injury awful upper eagle junk alert toss flower renew silly vague' + let firstAccount = '0x5d8de92c205279c10e5669f797b853ccef4f739a' idStore.recoverFromSeed(password, seedWords, (err) => { assert.ifError(err) - let newKeystore = idStore._idmgmt.keyStore - assert.equal(accounts[0], firstAccount) - - accounts = [] - idStore.recoverFromSeed(password, secondSeed, (err) => { - - let accounts = idStore._getAddresses() - assert.equal(accounts[0], secondAcct) - done() - }) + done() }) }) it('should return the expected second account', function (done) { + const secondSeed = 'radar blur cabbage chef fix engine embark joy scheme fiction master release' + const secondAcct = '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9' + idStore.recoverFromSeed(password, secondSeed, (err) => { assert.ifError(err) + assert.equal(accounts[0], secondAcct) + done() + }) + }) - let newKeystore = idStore._idmgmt.keyStore + it('should return the expected third account', function (done) { + const thirdSeed = 'phone coyote caught pattern found table wedding list tumble broccoli chief swing' + const thirdAcct = '0xb0e868f24bc7fec2bce2efc2b1c344d7569cd9d2' - assert.equal(accounts[0], firstAccount) + idStore.recoverFromSeed(password, thirdSeed, (err) => { + assert.ifError(err) + assert.equal(accounts[0], thirdAcct) done() }) }) From 8922ae1a55d3c5bf743c85aec5e454cf189f94f1 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 12:35:52 -0700 Subject: [PATCH 07/14] Made bip44 assertions easier to add to --- test/unit/idStore-test.js | 63 ++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 1028e35ea..09d8f1b8f 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -2,6 +2,7 @@ var assert = require('assert') var IdentityStore = require('../../app/scripts/lib/idStore') var configManagerGen = require('../lib/mock-config-manager') const ethUtil = require('ethereumjs-util') +const async = require('async') describe('IdentityStore', function() { @@ -65,6 +66,29 @@ describe('IdentityStore', function() { let accounts = [] let idStore + var assertions = [ + { + seed: 'picnic injury awful upper eagle junk alert toss flower renew silly vague', + account: '0x5d8de92c205279c10e5669f797b853ccef4f739a', + }, + { + seed: 'radar blur cabbage chef fix engine embark joy scheme fiction master release', + account: '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9', + }, + { + seed: 'phone coyote caught pattern found table wedding list tumble broccoli chief swing', + account: '0xb0e868f24bc7fec2bce2efc2b1c344d7569cd9d2', + }, + { + seed: 'recycle tag bird palace blue village anxiety census cook soldier example music', + account: '0xab34a45920afe4af212b96ec51232aaa6a33f663', + }, + { + seed: 'half glimpse tape cute harvest sweet bike voyage actual floor poet lazy', + account: '0x28e9044597b625ac4beda7250011670223de43b2', + } + ] + before(function() { window.localStorage = {} // Hacking localStorage support into JSDom @@ -80,36 +104,21 @@ describe('IdentityStore', function() { accounts = [] }) - it('should return the expected first account', function (done) { - let seedWords = 'picnic injury awful upper eagle junk alert toss flower renew silly vague' - let firstAccount = '0x5d8de92c205279c10e5669f797b853ccef4f739a' - - idStore.recoverFromSeed(password, seedWords, (err) => { - assert.ifError(err) - - assert.equal(accounts[0], firstAccount) - done() + it('should enforce seed compliance with TestRPC', function (done) { + const tests = assertions.map((assertion) => { + return function (cb) { + idStore.recoverFromSeed(password, assertion.seed, (err) => { + assert.ifError(err) + + console.log('comparing %s to %s', accounts[0], assertion.account) + assert.equal(accounts[0], assertion.account) + cb() + }) + } }) - }) - - it('should return the expected second account', function (done) { - const secondSeed = 'radar blur cabbage chef fix engine embark joy scheme fiction master release' - const secondAcct = '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9' - - idStore.recoverFromSeed(password, secondSeed, (err) => { - assert.ifError(err) - assert.equal(accounts[0], secondAcct) - done() - }) - }) - - it('should return the expected third account', function (done) { - const thirdSeed = 'phone coyote caught pattern found table wedding list tumble broccoli chief swing' - const thirdAcct = '0xb0e868f24bc7fec2bce2efc2b1c344d7569cd9d2' - idStore.recoverFromSeed(password, thirdSeed, (err) => { + async.series(tests, function(err, results) { assert.ifError(err) - assert.equal(accounts[0], thirdAcct) done() }) }) From cdd367dc3989d89092c322279972ce5ee18c276e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 12:38:04 -0700 Subject: [PATCH 08/14] Add more bip44 assertions --- test/unit/idStore-test.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 09d8f1b8f..4327013b3 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -86,7 +86,15 @@ describe('IdentityStore', function() { { seed: 'half glimpse tape cute harvest sweet bike voyage actual floor poet lazy', account: '0x28e9044597b625ac4beda7250011670223de43b2', - } + }, + { + seed: 'flavor tiger carpet motor angry hungry document inquiry large critic usage liar', + account: '0xb571be96558940c4e9292e1999461aa7499fb6cd', + }, + { + seed: 'this is a twelve word phrase seven eight nine ten eleven twelve', + account: '0x814e8ec0c3647e140b8e09228fc374b2a867fe48', + }, ] before(function() { From b1590f179e77d80193cc5e82d430c789c6fd816f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 12:39:50 -0700 Subject: [PATCH 09/14] Remove log --- test/unit/idStore-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 4327013b3..28111a85e 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -118,7 +118,6 @@ describe('IdentityStore', function() { idStore.recoverFromSeed(password, assertion.seed, (err) => { assert.ifError(err) - console.log('comparing %s to %s', accounts[0], assertion.account) assert.equal(accounts[0], assertion.account) cb() }) From 5e60b2f0c4c1a88839a87cc93d867c9c40d1090d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 12:57:11 -0700 Subject: [PATCH 10/14] Correct assertion for BIP32 compliance According to [axic's work here](https://github.com/MetaMask/metamask-plugin/issues/640#issuecomment-246133672), MetaMask is generating the correct address, so I've corrected that assertion accordingly. --- test/unit/idStore-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 28111a85e..b3bb1a49c 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -73,7 +73,7 @@ describe('IdentityStore', function() { }, { seed: 'radar blur cabbage chef fix engine embark joy scheme fiction master release', - account: '0xac39b311dceb2a4b2f5d8461c1cdaf756f4f7ae9', + account: '0xe15D894BeCB0354c501AE69429B05143679F39e0', }, { seed: 'phone coyote caught pattern found table wedding list tumble broccoli chief swing', From 9b861b6687d5b079560a67b35e4a4890da643b03 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 10 Sep 2016 15:45:34 -0700 Subject: [PATCH 11/14] Fixed caching bug Fixed bug where the second new vault created in an IdStore would initially return the accounts from the original store. Also fixed some tests that were incorrect. --- app/scripts/lib/idStore.js | 7 +------ test/unit/idStore-test.js | 11 +++++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 69ffd3f72..afb91a335 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -75,7 +75,6 @@ IdentityStore.prototype.recoverFromSeed = function (password, seed, cb) { if (err) return cb(err) this._loadIdentities() - this._didUpdate() cb(null, this.getState()) }) } @@ -394,7 +393,6 @@ IdentityStore.prototype._loadIdentities = function () { var addresses = this._getAddresses() addresses.forEach((address, i) => { // // add to ethStore - this._ethStore.addAccount(address) // add to identities const defaultLabel = 'Wallet ' + (i + 1) const nickname = configManager.nicknameForWallet(address) @@ -413,7 +411,6 @@ IdentityStore.prototype.saveAccountLabel = function (account, label, cb) { configManager.setNicknameForWallet(account, label) this._loadIdentities() cb(null, label) - this._didUpdate() } // mayBeFauceting @@ -481,8 +478,6 @@ IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, }) cb() - this._loadIdentities() - this._didUpdate() }) }) } @@ -497,9 +492,9 @@ IdentityStore.prototype._createFirstWallet = function (derivedKey) { const keyStore = this._keyStore keyStore.setDefaultHdDerivationPath(this.hdPathString) keyStore.generateNewAddress(derivedKey, 1) + this.configManager.setWallet(keyStore.serialize()) var addresses = keyStore.getAddresses() this._ethStore.addAccount(addresses[0]) - this.configManager.setWallet(keyStore.serialize()) } // get addresses and normalize address hexString diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index b3bb1a49c..a763eb0e7 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -73,7 +73,7 @@ describe('IdentityStore', function() { }, { seed: 'radar blur cabbage chef fix engine embark joy scheme fiction master release', - account: '0xe15D894BeCB0354c501AE69429B05143679F39e0', + account: '0xe15d894becb0354c501ae69429b05143679f39e0', }, { seed: 'phone coyote caught pattern found table wedding list tumble broccoli chief swing', @@ -91,10 +91,6 @@ describe('IdentityStore', function() { seed: 'flavor tiger carpet motor angry hungry document inquiry large critic usage liar', account: '0xb571be96558940c4e9292e1999461aa7499fb6cd', }, - { - seed: 'this is a twelve word phrase seven eight nine ten eleven twelve', - account: '0x814e8ec0c3647e140b8e09228fc374b2a867fe48', - }, ] before(function() { @@ -115,10 +111,13 @@ describe('IdentityStore', function() { it('should enforce seed compliance with TestRPC', function (done) { const tests = assertions.map((assertion) => { return function (cb) { + accounts = [] idStore.recoverFromSeed(password, assertion.seed, (err) => { assert.ifError(err) - assert.equal(accounts[0], assertion.account) + var received = accounts[0].toLowerCase() + var expected = assertion.account.toLowerCase() + assert.equal(received, expected) cb() }) } From c2105c4070c63c7ad6fbdc4c5be8f3f182a16b52 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 12 Sep 2016 08:29:18 -0700 Subject: [PATCH 12/14] Fix account list formatting when only a single item. - Makes account list items no longer flex larger than they should be. - Makes the add account button not flex larger than it should be. - Adds a line under the add account button to define its size. --- ui/app/accounts/account-list-item.js | 13 +++++-------- ui/app/accounts/index.js | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/ui/app/accounts/account-list-item.js b/ui/app/accounts/account-list-item.js index 0b4acdfec..4e0d69ed7 100644 --- a/ui/app/accounts/account-list-item.js +++ b/ui/app/accounts/account-list-item.js @@ -7,14 +7,14 @@ const EthBalance = require('../components/eth-balance') const CopyButton = require('../components/copyButton') const Identicon = require('../components/identicon') -module.exports = NewComponent +module.exports = AccountListItem -inherits(NewComponent, Component) -function NewComponent () { +inherits(AccountListItem, Component) +function AccountListItem () { Component.call(this) } -NewComponent.prototype.render = function () { +AccountListItem.prototype.render = function () { const identity = this.props.identity var isSelected = this.props.selectedAddress === identity.address var account = this.props.accounts[identity.address] @@ -23,9 +23,6 @@ NewComponent.prototype.render = function () { return ( h(`.accounts-list-option.flex-row.flex-space-between.pointer.hover-white${selectedClass}`, { key: `account-panel-${identity.address}`, - style: { - flex: '1 0 auto', - }, onClick: (event) => this.props.onShowDetail(identity.address, event), }, [ @@ -73,7 +70,7 @@ NewComponent.prototype.render = function () { ) } -NewComponent.prototype.pendingOrNot = function () { +AccountListItem.prototype.pendingOrNot = function () { const pending = this.props.pending if (pending.length === 0) return null return h('.pending-dot', pending.length) diff --git a/ui/app/accounts/index.js b/ui/app/accounts/index.js index c20900c1e..d3c84d387 100644 --- a/ui/app/accounts/index.js +++ b/ui/app/accounts/index.js @@ -84,7 +84,7 @@ AccountsScreen.prototype.render = function () { }) }), - h('hr.horizontal-line', {key: 'horizontal-line1'}), + h('hr.horizontal-line'), h('div.footer.hover-white.pointer', { key: 'reveal-account-bar', onClick: () => { @@ -92,7 +92,6 @@ AccountsScreen.prototype.render = function () { }, style: { display: 'flex', - flex: '1 0 auto', height: '40px', paddint: '10px', justifyContent: 'center', @@ -101,6 +100,7 @@ AccountsScreen.prototype.render = function () { }, [ h('i.fa.fa-plus.fa-lg', {key: ''}), ]), + h('hr.horizontal-line'), ]), unconfTxList.length ? ( From f51a13abaf8808968db279c060ec360c174253c7 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 12 Sep 2016 08:39:46 -0700 Subject: [PATCH 13/14] Fix ethStore pushed addresses Needed to add hex prefix always. --- app/scripts/lib/idStore.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index afb91a335..337bf7254 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -494,7 +494,7 @@ IdentityStore.prototype._createFirstWallet = function (derivedKey) { keyStore.generateNewAddress(derivedKey, 1) this.configManager.setWallet(keyStore.serialize()) var addresses = keyStore.getAddresses() - this._ethStore.addAccount(addresses[0]) + this._ethStore.addAccount(ethUtil.addHexPrefix(addresses[0])) } // get addresses and normalize address hexString From 5c1d8e299e68be6a74935f4eff56f68a9ccf5b72 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 12 Sep 2016 08:50:42 -0700 Subject: [PATCH 14/14] Select first address when restoring vault Fixes #642 --- app/scripts/lib/idStore.js | 12 ++++++++++-- test/unit/idStore-test.js | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 337bf7254..8b7e3ad3b 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -59,6 +59,7 @@ IdentityStore.prototype.createNewVault = function (password, entropy, cb) { this.configManager.setShowSeedWords(true) var seedWords = this._idmgmt.getSeed() + cb(null, seedWords) }) } @@ -124,7 +125,7 @@ IdentityStore.prototype.getSelectedAddress = function () { return configManager.getSelectedAccount() } -IdentityStore.prototype.setSelectedAddress = function (address, cb) { +IdentityStore.prototype.setSelectedAddressSync = function (address) { const configManager = this.configManager if (!address) { var addresses = this._getAddresses() @@ -132,7 +133,12 @@ IdentityStore.prototype.setSelectedAddress = function (address, cb) { } configManager.setSelectedAccount(address) - if (cb) return cb(null, address) + return address +} + +IdentityStore.prototype.setSelectedAddress = function (address, cb) { + const resultAddress = this.setSelectedAddressSync(address) + if (cb) return cb(null, resultAddress) } IdentityStore.prototype.revealAccount = function (cb) { @@ -477,6 +483,8 @@ IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, configManager: this.configManager, }) + this.setSelectedAddressSync() + cb() }) }) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index a763eb0e7..1ed1bf9a7 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -128,5 +128,23 @@ describe('IdentityStore', function() { done() }) }) + + it('should allow restoring and unlocking again', function (done) { + const assertion = assertions[0] + idStore.recoverFromSeed(password, assertion.seed, (err) => { + assert.ifError(err) + + var received = accounts[0].toLowerCase() + var expected = assertion.account.toLowerCase() + assert.equal(received, expected) + + + idStore.submitPassword(password, function(err, account) { + assert.ifError(err) + assert.equal(account, expected) + done() + }) + }) + }) }) })