diff --git a/app/scripts/metamask-controller.actions.test.js b/app/scripts/metamask-controller.actions.test.js new file mode 100644 index 000000000..d29bacaa3 --- /dev/null +++ b/app/scripts/metamask-controller.actions.test.js @@ -0,0 +1,110 @@ +import { strict as assert } from 'assert'; +import sinon from 'sinon'; +import proxyquire from 'proxyquire'; + +const Ganache = require('../../test/e2e/ganache'); + +const ganacheServer = new Ganache(); + +const browserPolyfillMock = { + runtime: { + id: 'fake-extension-id', + onInstalled: { + addListener: () => undefined, + }, + onMessageExternal: { + addListener: () => undefined, + }, + getPlatformInfo: async () => 'mac', + }, +}; + +let loggerMiddlewareMock; +const createLoggerMiddlewareMock = () => (req, res, next) => { + if (loggerMiddlewareMock) { + loggerMiddlewareMock.requests.push(req); + next((cb) => { + loggerMiddlewareMock.responses.push(res); + cb(); + }); + return; + } + next(); +}; + +const MetaMaskController = proxyquire('./metamask-controller', { + './lib/createLoggerMiddleware': { default: createLoggerMiddlewareMock }, +}).default; + +describe('MetaMaskController', function () { + let metamaskController; + const sandbox = sinon.createSandbox(); + const noop = () => undefined; + + before(async function () { + await ganacheServer.start(); + }); + + beforeEach(function () { + metamaskController = new MetaMaskController({ + showUserConfirmation: noop, + encryptor: { + encrypt(_, object) { + this.object = object; + return Promise.resolve('mock-encrypted'); + }, + decrypt() { + return Promise.resolve(this.object); + }, + }, + initLangCode: 'en_US', + platform: { + showTransactionNotification: () => undefined, + getVersion: () => 'foo', + }, + browser: browserPolyfillMock, + infuraProjectId: 'foo', + }); + }); + + afterEach(function () { + sandbox.restore(); + }); + + after(async function () { + await ganacheServer.quit(); + }); + + describe('#addNewAccount', function () { + it('two parallel calls with same accountCount give same result', async function () { + await metamaskController.createNewVaultAndKeychain('test@123'); + const [addNewAccountResult1, addNewAccountResult2] = await Promise.all([ + metamaskController.addNewAccount(1), + metamaskController.addNewAccount(1), + ]); + assert.deepEqual( + Object.keys(addNewAccountResult1.identities), + Object.keys(addNewAccountResult2.identities), + ); + }); + + it('two successive calls with same accountCount give same result', async function () { + await metamaskController.createNewVaultAndKeychain('test@123'); + const [addNewAccountResult1, addNewAccountResult2] = await Promise.all([ + metamaskController.addNewAccount(1), + Promise.resolve(1).then(() => metamaskController.addNewAccount(1)), + ]); + assert.deepEqual( + Object.keys(addNewAccountResult1.identities), + Object.keys(addNewAccountResult2.identities), + ); + }); + + it('two successive calls with different accountCount give different results', async function () { + await metamaskController.createNewVaultAndKeychain('test@123'); + const addNewAccountResult1 = await metamaskController.addNewAccount(1); + const addNewAccountResult2 = await metamaskController.addNewAccount(2); + assert.notDeepEqual(addNewAccountResult1, addNewAccountResult2); + }); + }); +}); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 40e8a8b78..2b4fd01fb 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2634,30 +2634,41 @@ export default class MetamaskController extends EventEmitter { /** * Adds a new account to the default (first) HD seed phrase Keyring. * + * @param accountCount * @returns {} keyState */ - async addNewAccount() { + async addNewAccount(accountCount) { const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0]; if (!primaryKeyring) { throw new Error('MetamaskController - No HD Key Tree found'); } const { keyringController } = this; - const oldAccounts = await keyringController.getAccounts(); - const keyState = await keyringController.addNewAccount(primaryKeyring); - const newAccounts = await keyringController.getAccounts(); + const { identities: oldIdentities } = + this.preferencesController.store.getState(); - await this.verifySeedPhrase(); + if (Object.keys(oldIdentities).length === accountCount) { + const oldAccounts = await keyringController.getAccounts(); + const keyState = await keyringController.addNewAccount(primaryKeyring); + const newAccounts = await keyringController.getAccounts(); - this.preferencesController.setAddresses(newAccounts); - newAccounts.forEach((address) => { - if (!oldAccounts.includes(address)) { - this.preferencesController.setSelectedAddress(address); - } - }); + await this.verifySeedPhrase(); - const { identities } = this.preferencesController.store.getState(); - return { ...keyState, identities }; + this.preferencesController.setAddresses(newAccounts); + newAccounts.forEach((address) => { + if (!oldAccounts.includes(address)) { + this.preferencesController.setSelectedAddress(address); + } + }); + + const { identities } = this.preferencesController.store.getState(); + return { ...keyState, identities }; + } + + return { + ...keyringController.memStore.getState(), + identities: oldIdentities, + }; } /** diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index ccdd0f38f..21b697fd2 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -734,7 +734,7 @@ describe('MetaMaskController', function () { }); it('#addNewAccount', async function () { - await metamaskController.addNewAccount(); + await metamaskController.addNewAccount(1); const getAccounts = await metamaskController.keyringController.getAccounts(); assert.equal(getAccounts.length, 2); diff --git a/ui/store/actions.js b/ui/store/actions.js index 39e9227bf..dbad71244 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -353,7 +353,9 @@ export function addNewAccount() { let newIdentities; try { - const { identities } = await promisifiedBackground.addNewAccount(); + const { identities } = await promisifiedBackground.addNewAccount( + Object.keys(oldIdentities).length, + ); newIdentities = identities; } catch (error) { dispatch(displayWarning(error.message)); diff --git a/ui/store/actions.test.js b/ui/store/actions.test.js index f217d08c7..4fed90e1f 100644 --- a/ui/store/actions.test.js +++ b/ui/store/actions.test.js @@ -19,6 +19,9 @@ const defaultState = { balance: '0x0', }, }, + identities: { + '0xFirstAddress': {}, + }, cachedBalances: { '0x1': { '0xFirstAddress': '0x0', @@ -387,7 +390,7 @@ describe('Actions', () => { metamask: { identities: {}, ...defaultState.metamask }, }); - const addNewAccount = background.addNewAccount.callsFake((cb) => + const addNewAccount = background.addNewAccount.callsFake((_, cb) => cb(null, { identities: {}, }), @@ -395,14 +398,14 @@ describe('Actions', () => { actions._setBackgroundConnection(background); - await store.dispatch(actions.addNewAccount()); + await store.dispatch(actions.addNewAccount(1)); expect(addNewAccount.callCount).toStrictEqual(1); }); it('displays warning error message when addNewAccount in background callback errors', async () => { const store = mockStore(); - background.addNewAccount.callsFake((cb) => { + background.addNewAccount.callsFake((_, cb) => { cb(new Error('error')); }); @@ -414,7 +417,7 @@ describe('Actions', () => { { type: 'HIDE_LOADING_INDICATION' }, ]; - await expect(store.dispatch(actions.addNewAccount())).rejects.toThrow( + await expect(store.dispatch(actions.addNewAccount(1))).rejects.toThrow( 'error', );