From 549bbfd05f0507a33c9df95111a71b0097f3b355 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 30 Nov 2016 14:43:18 -0800 Subject: [PATCH 1/5] Made integration test create a first vault --- development/test.html | 31 ++++++++++++++++++++++++++++++ test/integration/helpers.js | 4 ++-- test/integration/index.html | 2 +- test/integration/lib/first-time.js | 22 +++++++++++++++++++++ ui/app/actions.js | 3 ++- 5 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 development/test.html diff --git a/development/test.html b/development/test.html new file mode 100644 index 000000000..702be7fa0 --- /dev/null +++ b/development/test.html @@ -0,0 +1,31 @@ + + + + + MetaMask + + + + + + +
+ + + + + + diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 95c36017a..40f78d701 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -1,7 +1,7 @@ -function wait() { +function wait(time) { return new Promise(function(resolve, reject) { setTimeout(function() { resolve() - }, 500) + }, time || 500) }) } diff --git a/test/integration/index.html b/test/integration/index.html index ad4b4eb14..8a54cb829 100644 --- a/test/integration/index.html +++ b/test/integration/index.html @@ -15,7 +15,7 @@ - diff --git a/test/integration/lib/first-time.js b/test/integration/lib/first-time.js index a73b0cba3..e7d4ffaa2 100644 --- a/test/integration/lib/first-time.js +++ b/test/integration/lib/first-time.js @@ -1,3 +1,5 @@ +const PASSWORD = 'password123' + QUnit.test('agree to terms', function (assert) { var done = assert.async() let app @@ -6,10 +8,30 @@ QUnit.test('agree to terms', function (assert) { app = $('iframe').contents().find('#app-content .mock-app-root') app.find('.markdown').prop('scrollTop', 100000000) return wait() + }).then(function() { + var title = app.find('h1').text() assert.equal(title, 'MetaMask', 'title screen') + var pwBox = app.find('#password-box')[0] + var confBox = app.find('#password-box-confirm')[0] + + pwBox.value = PASSWORD + confBox.value = PASSWORD + return wait() + + }).then(function() { + + var createButton = app.find('button.primary')[0] + createButton.click() + + return wait(1500) + }).then(function() { + + var terms = app.find('h3.terms-header')[0] + assert.equal(terms.textContent, 'MetaMask Terms & Conditions', 'Showing TOS') + done() }) }) diff --git a/ui/app/actions.js b/ui/app/actions.js index 8f37b2e4c..d800091f2 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -204,10 +204,11 @@ function createNewVaultAndRestore (password, seed) { function createNewVaultAndKeychain (password) { return (dispatch) => { - background.createNewVaultAndKeychain(password, (err) => { + background.createNewVaultAndKeychain(password, (err, newState) => { if (err) { return dispatch(actions.showWarning(err.message)) } + dispatch(actions.updateMetamaskState(newState)) }) } } From fe533bbef2486bd22e4e23ee43927cff0e1d958e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 30 Nov 2016 15:18:26 -0800 Subject: [PATCH 2/5] Add more integration tests Integration tests now: - Scroll through terms - Accept terms - Confirm seed phrase - Verify account detail screen --- test/integration/lib/first-time.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/integration/lib/first-time.js b/test/integration/lib/first-time.js index e7d4ffaa2..76b10f568 100644 --- a/test/integration/lib/first-time.js +++ b/test/integration/lib/first-time.js @@ -32,6 +32,31 @@ QUnit.test('agree to terms', function (assert) { var terms = app.find('h3.terms-header')[0] assert.equal(terms.textContent, 'MetaMask Terms & Conditions', 'Showing TOS') + // Scroll through terms + var scrollable = app.find('.markdown')[0] + scrollable.scrollTop = scrollable.scrollHeight + + return wait(10) + }).then(function() { + + var button = app.find('button')[0] // Agree button + button.click() + + return wait(1000) + }).then(function() { + + var created = app.find('h3')[0] + assert.equal(created.textContent, 'Vault Created', 'Vault created screen') + + var button = app.find('button')[0] // Agree button + button.click() + + return wait(1000) + }).then(function() { + + var detail = app.find('.account-detail-section')[0] + assert.ok(detail, 'Account detail section loaded.') done() + }) }) From 049e351c9d78dc13a81ba962b04ef96694b13682 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 30 Nov 2016 16:01:51 -0800 Subject: [PATCH 3/5] Add integration tests for logging out and back in --- test/integration/lib/first-time.js | 28 +++++++++++++++++++++++++++- ui/app/actions.js | 3 ++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/test/integration/lib/first-time.js b/test/integration/lib/first-time.js index 76b10f568..d2fe31878 100644 --- a/test/integration/lib/first-time.js +++ b/test/integration/lib/first-time.js @@ -56,7 +56,33 @@ QUnit.test('agree to terms', function (assert) { var detail = app.find('.account-detail-section')[0] assert.ok(detail, 'Account detail section loaded.') - done() + var sandwich = app.find('.sandwich-expando')[0] + sandwich.click() + + return wait() + }).then(function() { + + var sandwich = app.find('.menu-droppo')[0] + var lock = sandwich.children[2] + assert.ok(lock, 'Lock menu item found') + lock.click() + + return wait(1000) + }).then(function() { + + var pwBox = app.find('#password-box')[0] + pwBox.value = PASSWORD + + var createButton = app.find('button.primary')[0] + createButton.click() + + return wait(1500) + }).then(function() { + + var detail = app.find('.account-detail-section')[0] + assert.ok(detail, 'Account detail section loaded again.') + + done() }) }) diff --git a/ui/app/actions.js b/ui/app/actions.js index d800091f2..0cc55136f 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -447,11 +447,12 @@ function updateMetamaskState (newState) { function lockMetamask () { return (dispatch) => { - background.setLocked((err) => { + background.setLocked((err, newState) => { dispatch(actions.hideLoadingIndication()) if (err) { return dispatch(actions.displayWarning(err.message)) } + dispatch(actions.updateMetamaskState(newState)) }) } } From 1880cda9b95f3274d56e1f4abc513d1d7ddd59c2 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 30 Nov 2016 19:34:17 -0800 Subject: [PATCH 4/5] Fix vault encrypting & unlocking bug This is only a bug in dev, but was committed yesterday. Sometimes the `encrypt` method was being passed values other than the password as the encryption key, leading to un-unlockable vaults. To find this, and avoid it for all time hereafter, I added several more steps to our oft-neglected integration test suite, which now fully initializes a vault, locks it, and unlocks it again, to make sure all of those steps definitely work always. --- app/scripts/keyring-controller.js | 16 ++++++++++++---- app/scripts/lib/config-manager.js | 5 +++-- test/integration/lib/encryptor-test.js | 4 ++++ test/integration/lib/first-time.js | 2 ++ ui/app/actions.js | 25 +++++++++++++++++++------ ui/app/reducers/app.js | 14 +++++++++++++- 6 files changed, 53 insertions(+), 13 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index ac9409dbb..9cfc3af97 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -167,7 +167,7 @@ module.exports = class KeyringController extends EventEmitter { this.configManager.setSelectedAccount(hexAccount) return this.setupAccounts(accounts) }) - .then(this.persistAllKeyrings.bind(this)) + .then(this.persistAllKeyrings.bind(this, password)) .then(this.fullUpdate.bind(this)) } @@ -226,9 +226,11 @@ module.exports = class KeyringController extends EventEmitter { }) .then((keyrings) => { this.keyrings = keyrings - return this.setupAccounts() + return this.fullUpdate() + }) + .catch((reason) => { + return reason }) - .then(this.fullUpdate.bind(this)) } // Add New Keyring @@ -250,6 +252,7 @@ module.exports = class KeyringController extends EventEmitter { this.keyrings.push(keyring) return this.setupAccounts(accounts) }) + .then(() => { return this.password }) .then(this.persistAllKeyrings.bind(this)) .then(() => { return keyring @@ -692,6 +695,9 @@ module.exports = class KeyringController extends EventEmitter { // Takes an account address and an iterator representing // the current number of named accounts. getBalanceAndNickname (account) { + if (!account) { + throw new Error('Problem loading account.') + } const address = normalize(account) this.ethStore.addAccount(address) return this.createNickname(address) @@ -725,7 +731,9 @@ module.exports = class KeyringController extends EventEmitter { // encrypts that array with the provided `password`, // and persists that encrypted string to storage. persistAllKeyrings (password = this.password) { - this.password = password + if (typeof password === 'string') { + this.password = password + } return Promise.all(this.keyrings.map((keyring) => { return Promise.all([keyring.type, keyring.serialize()]) .then((serializedKeyringArray) => { diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js index 8e63265d2..59cc2b63c 100644 --- a/app/scripts/lib/config-manager.js +++ b/app/scripts/lib/config-manager.js @@ -3,6 +3,7 @@ const MetamaskConfig = require('../config.js') const migrations = require('./migrations') const rp = require('request-promise') const ethUtil = require('ethereumjs-util') +const normalize = require('./sig-util').normalize const TESTNET_RPC = MetamaskConfig.network.testnet const MAINNET_RPC = MetamaskConfig.network.mainnet @@ -273,13 +274,13 @@ ConfigManager.prototype.getWalletNicknames = function () { } ConfigManager.prototype.nicknameForWallet = function (account) { - const address = ethUtil.addHexPrefix(account.toLowerCase()) + const address = normalize(account) const nicknames = this.getWalletNicknames() return nicknames[address] } ConfigManager.prototype.setNicknameForWallet = function (account, nickname) { - const address = ethUtil.addHexPrefix(account.toLowerCase()) + const address = normalize(account) const nicknames = this.getWalletNicknames() nicknames[address] = nickname var data = this.getData() diff --git a/test/integration/lib/encryptor-test.js b/test/integration/lib/encryptor-test.js index d42608152..897d22740 100644 --- a/test/integration/lib/encryptor-test.js +++ b/test/integration/lib/encryptor-test.js @@ -1,5 +1,7 @@ var encryptor = require('../../../app/scripts/lib/encryptor') +QUnit.module('encryptor') + QUnit.test('encryptor:serializeBufferForStorage', function (assert) { assert.expect(1) var buf = new Buffer(2) @@ -65,3 +67,5 @@ QUnit.test('encryptor:encrypt & decrypt with wrong password', function(assert) { done() }) }) + + diff --git a/test/integration/lib/first-time.js b/test/integration/lib/first-time.js index d2fe31878..12c573db1 100644 --- a/test/integration/lib/first-time.js +++ b/test/integration/lib/first-time.js @@ -1,5 +1,7 @@ const PASSWORD = 'password123' +QUnit.module('first time usage') + QUnit.test('agree to terms', function (assert) { var done = assert.async() let app diff --git a/ui/app/actions.js b/ui/app/actions.js index 0cc55136f..41be1004c 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -5,7 +5,11 @@ var actions = { goHome: goHome, // menu state getNetworkStatus: 'getNetworkStatus', - + // transition state + TRANSITION_FORWARD: 'TRANSITION_FORWARD', + TRANSITION_BACKWARD: 'TRANSITION_BACKWARD', + transitionForward, + transitionBackward, // remote state UPDATE_METAMASK_STATE: 'UPDATE_METAMASK_STATE', updateMetamaskState: updateMetamaskState, @@ -166,16 +170,25 @@ function tryUnlockMetamask (password) { if (err) { dispatch(actions.unlockFailed(err.message)) } else { - let selectedAccount - try { - selectedAccount = newState.metamask.selectedAccount - } catch (e) {} - dispatch(actions.unlockMetamask(selectedAccount)) + dispatch(actions.transitionForward()) + dispatch(actions.updateMetamaskState(newState)) } }) } } +function transitionForward() { + return { + type: this.TRANSITION_FORWARD, + } +} + +function transitionBackward() { + return { + type: this.TRANSITION_BACKWARD, + } +} + function confirmSeedWords () { return (dispatch) => { dispatch(actions.showLoadingIndication()) diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 1f40e90b3..67a926948 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -43,7 +43,19 @@ function reduceApp (state, action) { switch (action.type) { - // intialize + // transition methods + + case actions.TRANSITION_FORWARD: + return extend(appState, { + transForward: true, + }) + + case actions.TRANSITION_BACKWARD: + return extend(appState, { + transForward: false, + }) + + // intialize case actions.SHOW_CREATE_VAULT: return extend(appState, { From c43178360203b4571d9e00880b5bf2806908c179 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 1 Dec 2016 10:21:56 -0800 Subject: [PATCH 5/5] Remove redundant logging block --- app/scripts/keyring-controller.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 9cfc3af97..40c9695dd 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -228,9 +228,6 @@ module.exports = class KeyringController extends EventEmitter { this.keyrings = keyrings return this.fullUpdate() }) - .catch((reason) => { - return reason - }) } // Add New Keyring