From e9712a13ecd96db0bcc9d22998fc4df2ecdeeebc Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 11 Aug 2017 14:19:35 -0700 Subject: [PATCH 01/88] Create tests for TxStateManager --- app/scripts/controllers/transactions.js | 209 ++-------------------- app/scripts/lib/tx-state-manager.js | 201 +++++++++++++++++++++ test/unit/tx-controller-test.js | 222 +++-------------------- test/unit/tx-state-manager-test.js | 227 ++++++++++++++++++++++++ 4 files changed, 471 insertions(+), 388 deletions(-) create mode 100644 app/scripts/lib/tx-state-manager.js create mode 100644 test/unit/tx-state-manager-test.js diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 58c468e22..48a882a71 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -4,6 +4,7 @@ const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') +const TransactionStateManger = require('../lib/tx-state-manager') const TxProviderUtil = require('../lib/tx-utils') const PendingTransactionTracker = require('../lib/pending-tx-tracker') const createId = require('../lib/random-id') @@ -12,18 +13,23 @@ const NonceTracker = require('../lib/nonce-tracker') module.exports = class TransactionController extends EventEmitter { constructor (opts) { super() - this.store = new ObservableStore(extend({ - transactions: [], - }, opts.initState)) - this.memStore = new ObservableStore({}) this.networkStore = opts.networkStore || new ObservableStore({}) this.preferencesStore = opts.preferencesStore || new ObservableStore({}) - this.txHistoryLimit = opts.txHistoryLimit this.provider = opts.provider this.blockTracker = opts.blockTracker this.signEthTx = opts.signTransaction this.ethStore = opts.ethStore + this.memStore = new ObservableStore({}) + this.query = new EthQuery(this.provider) + this.txProviderUtil = new TxProviderUtil(this.provider) + + this.txStateManager = new TransactionStateManger(extend({ + transactions: [], + txHistoryLimit: opts.txHistoryLimit, + getNetwork: this.getNetwork.bind(this), + }, opts.initState)) + this.nonceTracker = new NonceTracker({ provider: this.provider, getPendingTransactions: (address) => { @@ -34,8 +40,6 @@ module.exports = class TransactionController extends EventEmitter { }) }, }) - this.query = new EthQuery(this.provider) - this.txProviderUtil = new TxProviderUtil(this.provider) this.pendingTxTracker = new PendingTransactionTracker({ provider: this.provider, @@ -69,7 +73,7 @@ module.exports = class TransactionController extends EventEmitter { this.blockTracker.on('sync', this.pendingTxTracker.queryPendingTxs.bind(this.pendingTxTracker)) // memstore is computed from a few different stores this._updateMemstore() - this.store.subscribe(() => this._updateMemstore()) + this.txStateManager.subscribe(() => this._updateMemstore()) this.networkStore.subscribe(() => this._updateMemstore()) this.preferencesStore.subscribe(() => this._updateMemstore()) } @@ -86,84 +90,27 @@ module.exports = class TransactionController extends EventEmitter { return this.preferencesStore.getState().selectedAddress } - // Returns the number of txs for the current network. - getTxCount () { - return this.getTxList().length - } - - // Returns the full tx list across all networks - getFullTxList () { - return this.store.getState().transactions - } - getUnapprovedTxCount () { return Object.keys(this.getUnapprovedTxList()).length } getPendingTxCount () { - return this.getTxsByMetaData('status', 'signed').length + return this.txStateManager.getTxsByMetaData('status', 'signed').length } // Returns the tx list - getTxList () { - const network = this.getNetwork() - const fullTxList = this.getFullTxList() - return this.getTxsByMetaData('metamaskNetworkId', network, fullTxList) - } - // gets tx by Id and returns it - getTx (txId) { - const txList = this.getTxList() - const txMeta = txList.find(txData => txData.id === txId) - return txMeta - } getUnapprovedTxList () { - const txList = this.getTxList() - return txList.filter((txMeta) => txMeta.status === 'unapproved') - .reduce((result, tx) => { + const txList = this.txStateManager.getTxsByMetaData('status', 'unapproved') + return txList.reduce((result, tx) => { result[tx.id] = tx return result }, {}) } - updateTx (txMeta) { - // create txMeta snapshot for history - const txMetaForHistory = clone(txMeta) - // dont include previous history in this snapshot - delete txMetaForHistory.history - // add snapshot to tx history - if (!txMeta.history) txMeta.history = [] - txMeta.history.push(txMetaForHistory) - - const txId = txMeta.id - const txList = this.getFullTxList() - const index = txList.findIndex(txData => txData.id === txId) - if (!txMeta.history) txMeta.history = [] - txMeta.history.push(txMetaForHistory) - - txList[index] = txMeta - this._saveTxList(txList) - this.emit('update') - } - // Adds a tx to the txlist addTx (txMeta) { - const txCount = this.getTxCount() - const network = this.getNetwork() - const fullTxList = this.getFullTxList() - const txHistoryLimit = this.txHistoryLimit - - // checks if the length of the tx history is - // longer then desired persistence limit - // and then if it is removes only confirmed - // or rejected tx's. - // not tx's that are pending or unapproved - if (txCount > txHistoryLimit - 1) { - const index = fullTxList.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected') && network === txMeta.metamaskNetworkId)) - fullTxList.splice(index, 1) - } - fullTxList.push(txMeta) - this._saveTxList(fullTxList) + this.txStateManager.addTx(txMeta) this.emit('update') this.once(`${txMeta.id}:signed`, function (txId) { @@ -211,7 +158,7 @@ module.exports = class TransactionController extends EventEmitter { // add default tx params await this.addTxDefaults(txMeta) // save txMeta - this.addTx(txMeta) + this.txStateManager.addTx(txMeta) return txMeta } @@ -306,133 +253,11 @@ module.exports = class TransactionController extends EventEmitter { this.updateTx(txMeta) } - /* - Takes an object of fields to search for eg: - let thingsToLookFor = { - to: '0x0..', - from: '0x0..', - status: 'signed', - err: undefined, - } - and returns a list of tx with all - options matching - - ****************HINT**************** - | `err: undefined` is like looking | - | for a tx with no err | - | so you can also search txs that | - | dont have something as well by | - | setting the value as undefined | - ************************************ - - this is for things like filtering a the tx list - for only tx's from 1 account - or for filltering for all txs from one account - and that have been 'confirmed' - */ - getFilteredTxList (opts) { - let filteredTxList - Object.keys(opts).forEach((key) => { - filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList) - }) - return filteredTxList - } - - getTxsByMetaData (key, value, txList = this.getTxList()) { - return txList.filter((txMeta) => { - if (txMeta.txParams[key]) { - return txMeta.txParams[key] === value - } else { - return txMeta[key] === value - } - }) - } - - // STATUS METHODS - // get::set status - - // should return the status of the tx. - getTxStatus (txId) { - const txMeta = this.getTx(txId) - return txMeta.status - } - - // should update the status of the tx to 'rejected'. - setTxStatusRejected (txId) { - this._setTxStatus(txId, 'rejected') - } - - // should update the status of the tx to 'approved'. - setTxStatusApproved (txId) { - this._setTxStatus(txId, 'approved') - } - - // should update the status of the tx to 'signed'. - setTxStatusSigned (txId) { - this._setTxStatus(txId, 'signed') - } - - // should update the status of the tx to 'submitted'. - setTxStatusSubmitted (txId) { - this._setTxStatus(txId, 'submitted') - } - - // should update the status of the tx to 'confirmed'. - setTxStatusConfirmed (txId) { - this._setTxStatus(txId, 'confirmed') - } - - setTxStatusFailed (txId, err) { - const txMeta = this.getTx(txId) - txMeta.err = { - message: err.toString(), - stack: err.stack, - } - this.updateTx(txMeta) - this._setTxStatus(txId, 'failed') - } - - // merges txParams obj onto txData.txParams - // use extend to ensure that all fields are filled - updateTxParams (txId, txParams) { - const txMeta = this.getTx(txId) - txMeta.txParams = extend(txMeta.txParams, txParams) - this.updateTx(txMeta) - } - /* _____________________________________ | | | PRIVATE METHODS | |______________________________________*/ - - // Should find the tx in the tx list and - // update it. - // should set the status in txData - // - `'unapproved'` the user has not responded - // - `'rejected'` the user has responded no! - // - `'approved'` the user has approved the tx - // - `'signed'` the tx is signed - // - `'submitted'` the tx is sent to a server - // - `'confirmed'` the tx has been included in a block. - // - `'failed'` the tx failed for some reason, included on tx data. - _setTxStatus (txId, status) { - const txMeta = this.getTx(txId) - txMeta.status = status - this.emit(`${txMeta.id}:${status}`, txId) - if (status === 'submitted' || status === 'rejected') { - this.emit(`${txMeta.id}:finished`, txMeta) - } - this.updateTx(txMeta) - this.emit('updateBadge') - } - - // Saves the new/updated txList. - // Function is intended only for internal use - _saveTxList (transactions) { - this.store.updateState({ transactions }) - } - _updateMemstore () { const unapprovedTxs = this.getUnapprovedTxList() const selectedAddressTxList = this.getFilteredTxList({ diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js new file mode 100644 index 000000000..91271cc4a --- /dev/null +++ b/app/scripts/lib/tx-state-manager.js @@ -0,0 +1,201 @@ +const clone = require('clone') +const extend = require('xtend') +const ObservableStore = require('obs-store') + +module.exports = class TransactionStateManger extends ObservableStore { + constructor ({initState, txHistoryLimit, getNetwork}) { + super(initState) + this.txHistoryLimit = txHistoryLimit + this.getNetwork = getNetwork + } + // Returns the number of txs for the current network. + getTxCount () { + return this.getTxList().length + } + + getTxList () { + const network = this.getNetwork() + const fullTxList = this.getFullTxList() + return fullTxList.filter((txMeta) => txMeta.metamaskNetworkId === network) + } + + getFullTxList () { + return this.getState().transactions + } + + addTx (txMeta) { + const transactions = this.getFullTxList() + const txCount = this.getTxCount() + const txHistoryLimit = this.txHistoryLimit + + // checks if the length of the tx history is + // longer then desired persistence limit + // and then if it is removes only confirmed + // or rejected tx's. + // not tx's that are pending or unapproved + if (txCount > txHistoryLimit - 1) { + const index = transactions.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected'))) + transactions.splice(index, 1) + } + transactions.push(txMeta) + transactions.push(txMeta) + this._saveTxList(transactions) + } + // gets tx by Id and returns it + getTx (txId) { + const txMeta = this.getTxsByMetaData('id', txId)[0] + return txMeta + } + + updateTx (txMeta) { + // create txMeta snapshot for history + const txMetaForHistory = clone(txMeta) + // dont include previous history in this snapshot + delete txMetaForHistory.history + // add snapshot to tx history + if (!txMeta.history) txMeta.history = [] + txMeta.history.push(txMetaForHistory) + + const txId = txMeta.id + const txList = this.getFullTxList() + const index = txList.findIndex(txData => txData.id === txId) + if (!txMeta.history) txMeta.history = [] + txMeta.history.push(txMetaForHistory) + + txList[index] = txMeta + this._saveTxList(txList) + } + + + // merges txParams obj onto txData.txParams + // use extend to ensure that all fields are filled + updateTxParams (txId, txParams) { + const txMeta = this.getTx(txId) + txMeta.txParams = extend(txMeta.txParams, txParams) + this.updateTx(txMeta) + } + +/* + Takes an object of fields to search for eg: + let thingsToLookFor = { + to: '0x0..', + from: '0x0..', + status: 'signed', + err: undefined, + } + and returns a list of tx with all + options matching + + ****************HINT**************** + | `err: undefined` is like looking | + | for a tx with no err | + | so you can also search txs that | + | dont have something as well by | + | setting the value as undefined | + ************************************ + + this is for things like filtering a the tx list + for only tx's from 1 account + or for filltering for all txs from one account + and that have been 'confirmed' + */ + getFilteredTxList (opts, initialList) { + let filteredTxList = initialList + Object.keys(opts).forEach((key) => { + filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList) + }) + return filteredTxList + } + + getTxsByMetaData (key, value, txList = this.getTxList()) { + return txList.filter((txMeta) => { + if (txMeta.txParams[key]) { + return txMeta.txParams[key] === value + } else { + return txMeta[key] === value + } + }) + } + + // STATUS METHODS + // statuses: + // - `'unapproved'` the user has not responded + // - `'rejected'` the user has responded no! + // - `'approved'` the user has approved the tx + // - `'signed'` the tx is signed + // - `'submitted'` the tx is sent to a server + // - `'confirmed'` the tx has been included in a block. + // - `'failed'` the tx failed for some reason, included on tx data. + + // get::set status + + // should return the status of the tx. + getTxStatus (txId) { + const txMeta = this.getTx(txId) + return txMeta.status + } + + // should update the status of the tx to 'rejected'. + setTxStatusRejected (txId) { + this._setTxStatus(txId, 'rejected') + } + + // should update the status of the tx to 'approved'. + setTxStatusApproved (txId) { + this._setTxStatus(txId, 'approved') + } + + // should update the status of the tx to 'signed'. + setTxStatusSigned (txId) { + this._setTxStatus(txId, 'signed') + } + + // should update the status of the tx to 'submitted'. + setTxStatusSubmitted (txId) { + this._setTxStatus(txId, 'submitted') + } + + // should update the status of the tx to 'confirmed'. + setTxStatusConfirmed (txId) { + this._setTxStatus(txId, 'confirmed') + } + + setTxStatusFailed (txId, reason) { + const txMeta = this.getTx(txId) + txMeta.err = reason + this.updateTx(txMeta) + this._setTxStatus(txId, 'failed') + } + +/* _____________________________________ +| | +| PRIVATE METHODS | +|______________________________________*/ + + // Should find the tx in the tx list and + // update it. + // should set the status in txData + // - `'unapproved'` the user has not responded + // - `'rejected'` the user has responded no! + // - `'approved'` the user has approved the tx + // - `'signed'` the tx is signed + // - `'submitted'` the tx is sent to a server + // - `'confirmed'` the tx has been included in a block. + // - `'failed'` the tx failed for some reason, included on tx data. + _setTxStatus (txId, status) { + const txMeta = this.getTx(txId) + txMeta.status = status + this.emit(`${txMeta.id}:${status}`, txId) + if (status === 'submitted' || status === 'rejected') { + this.emit(`${txMeta.id}:finished`, txMeta) + } + this.updateTx(txMeta) + this.emit('updateBadge') + } + + // Saves the new/updated txList. + // Function is intended only for internal use + _saveTxList (transactions) { + this.updateState({ transactions }) + } +} \ No newline at end of file diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 57d7deccd..40be490c0 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -47,11 +47,12 @@ describe('Transaction Controller', function () { metamaskNetworkId: currentNetworkId, txParams, } - txController._saveTxList([txMeta]) + txController.txStateManager._saveTxList([txMeta]) stub = sinon.stub(txController, 'addUnapprovedTransaction').returns(Promise.resolve(txMeta)) }) afterEach(function () { + txController.txStateManager._saveTxList([]) stub.restore() }) @@ -69,7 +70,7 @@ describe('Transaction Controller', function () { txController.once('newUnaprovedTx', (txMetaFromEmit) => { setTimeout(() => { txController.setTxHash(txMetaFromEmit.id, '0x0') - txController.setTxStatusSubmitted(txMetaFromEmit.id) + txController.txStateManager.setTxStatusSubmitted(txMetaFromEmit.id) }, 10) }) @@ -84,7 +85,7 @@ describe('Transaction Controller', function () { it('should reject when finished and status is rejected', function (done) { txController.once('newUnaprovedTx', (txMetaFromEmit) => { setTimeout(() => { - txController.setTxStatusRejected(txMetaFromEmit.id) + txController.txStateManager.setTxStatusRejected(txMetaFromEmit.id) }, 10) }) @@ -107,7 +108,7 @@ describe('Transaction Controller', function () { assert(('txParams' in txMeta), 'should have a txParams') assert(('history' in txMeta), 'should have a history') - const memTxMeta = txController.getTx(txMeta.id) + const memTxMeta = txController.txStateManager.getTx(txMeta.id) assert.deepEqual(txMeta, memTxMeta, `txMeta should be stored in txController after adding it\n expected: ${txMeta} \n got: ${memTxMeta}`) addTxDefaultsStub.restore() done() @@ -160,202 +161,31 @@ describe('Transaction Controller', function () { }) }) - describe('#getTxList', function () { - it('when new should return empty array', function () { - var result = txController.getTxList() - assert.ok(Array.isArray(result)) - assert.equal(result.length, 0) - }) - }) - describe('#addTx', function () { - it('adds a tx returned in getTxList', function () { - var tx = { id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx, noop) - var result = txController.getTxList() - assert.ok(Array.isArray(result)) - assert.equal(result.length, 1) - assert.equal(result[0].id, 1) - }) - - it('does not override txs from other networks', function () { - var tx = { id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } - var tx2 = { id: 2, status: 'confirmed', metamaskNetworkId: otherNetworkId, txParams: {} } - txController.addTx(tx, noop) - txController.addTx(tx2, noop) - var result = txController.getFullTxList() - var result2 = txController.getTxList() - assert.equal(result.length, 2, 'txs were deleted') - assert.equal(result2.length, 1, 'incorrect number of txs on network.') - }) - - it('cuts off early txs beyond a limit', function () { - const limit = txController.txHistoryLimit - for (let i = 0; i < limit + 1; i++) { - const tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx, noop) - } - var result = txController.getTxList() - assert.equal(result.length, limit, `limit of ${limit} txs enforced`) - assert.equal(result[0].id, 1, 'early txs truncted') - }) - - it('cuts off early txs beyond a limit whether or not it is confirmed or rejected', function () { - const limit = txController.txHistoryLimit - for (let i = 0; i < limit + 1; i++) { - const tx = { id: i, time: new Date(), status: 'rejected', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx, noop) - } - var result = txController.getTxList() - assert.equal(result.length, limit, `limit of ${limit} txs enforced`) - assert.equal(result[0].id, 1, 'early txs truncted') - }) - - it('cuts off early txs beyond a limit but does not cut unapproved txs', function () { - var unconfirmedTx = { id: 0, time: new Date(), status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(unconfirmedTx, noop) - const limit = txController.txHistoryLimit - for (let i = 1; i < limit + 1; i++) { - const tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx, noop) - } - var result = txController.getTxList() - assert.equal(result.length, limit, `limit of ${limit} txs enforced`) - assert.equal(result[0].id, 0, 'first tx should still be there') - assert.equal(result[0].status, 'unapproved', 'first tx should be unapproved') - assert.equal(result[1].id, 2, 'early txs truncted') - }) - }) - - describe('#setTxStatusSigned', function () { - it('sets the tx status to signed', function () { - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx, noop) - txController.setTxStatusSigned(1) - var result = txController.getTxList() - assert.ok(Array.isArray(result)) - assert.equal(result.length, 1) - assert.equal(result[0].status, 'signed') - }) - - it('should emit a signed event to signal the exciton of callback', (done) => { - this.timeout(10000) - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } - const noop = function () { - assert(true, 'event listener has been triggered and noop executed') - done() - } - txController.addTx(tx) - txController.on('1:signed', noop) - txController.setTxStatusSigned(1) - }) - }) - - describe('#setTxStatusRejected', function () { - it('sets the tx status to rejected', function () { - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx) - txController.setTxStatusRejected(1) - var result = txController.getTxList() - assert.ok(Array.isArray(result)) - assert.equal(result.length, 1) - assert.equal(result[0].status, 'rejected') - }) - - it('should emit a rejected event to signal the exciton of callback', (done) => { - this.timeout(10000) - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } - txController.addTx(tx) - const noop = function (err, txId) { - assert(true, 'event listener has been triggered and noop executed') - done() - } - txController.on('1:rejected', noop) - txController.setTxStatusRejected(1) - }) - }) - - describe('#updateTx', function () { - it('replaces the tx with the same id', function () { - txController.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - txController.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - txController.updateTx({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: currentNetworkId, txParams: {} }) - var result = txController.getTx('1') - assert.equal(result.hash, 'foo') - }) - - it('updates gas price', function () { - const originalGasPrice = '0x01' - const desiredGasPrice = '0x02' - - const txMeta = { - id: '1', + it('should emit updates', function (done) { + txMeta = { status: 'unapproved', + id: 1, metamaskNetworkId: currentNetworkId, - txParams: { - gasPrice: originalGasPrice, - }, + txParams: {} } - const updatedMeta = clone(txMeta) - + const eventNames = ['update', 'updateBadge', '1:unapproved'] + const listeners = [] + eventNames.forEach((eventName) => { + listeners.push(new Promise((resolve) => { + txController.once(eventName, (arg) => { + resolve(arg) + }) + })) + }) + Promise.all(listeners) + .then((returnValues) => { + assert.deepEqual(returnValues.pop(), txMeta, 'last event 1:unapproved should return txMeta') + done() + }) + .catch(done) txController.addTx(txMeta) - updatedMeta.txParams.gasPrice = desiredGasPrice - txController.updateTx(updatedMeta) - var result = txController.getTx('1') - assert.equal(result.txParams.gasPrice, desiredGasPrice, 'gas price updated') - }) - }) - - describe('#getUnapprovedTxList', function () { - it('returns unapproved txs in a hash', function () { - txController.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - txController.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - const result = txController.getUnapprovedTxList() - assert.equal(typeof result, 'object') - assert.equal(result['1'].status, 'unapproved') - assert.equal(result['2'], undefined) - }) - }) - - describe('#getTx', function () { - it('returns a tx with the requested id', function () { - txController.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - txController.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - assert.equal(txController.getTx('1').status, 'unapproved') - assert.equal(txController.getTx('2').status, 'confirmed') - }) - }) - - describe('#getFilteredTxList', function () { - it('returns a tx with the requested data', function () { - const txMetas = [ - { id: 0, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, - { id: 1, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, - { id: 2, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, - { id: 3, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, - { id: 4, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, - { id: 5, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, - { id: 6, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, - { id: 7, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, - { id: 8, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, - { id: 9, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, - ] - txMetas.forEach((txMeta) => txController.addTx(txMeta, noop)) - let filterParams - - filterParams = { status: 'unapproved', from: '0xaa' } - assert.equal(txController.getFilteredTxList(filterParams).length, 3, `getFilteredTxList - ${JSON.stringify(filterParams)}`) - filterParams = { status: 'unapproved', to: '0xaa' } - assert.equal(txController.getFilteredTxList(filterParams).length, 2, `getFilteredTxList - ${JSON.stringify(filterParams)}`) - filterParams = { status: 'confirmed', from: '0xbb' } - assert.equal(txController.getFilteredTxList(filterParams).length, 3, `getFilteredTxList - ${JSON.stringify(filterParams)}`) - filterParams = { status: 'confirmed' } - assert.equal(txController.getFilteredTxList(filterParams).length, 5, `getFilteredTxList - ${JSON.stringify(filterParams)}`) - filterParams = { from: '0xaa' } - assert.equal(txController.getFilteredTxList(filterParams).length, 5, `getFilteredTxList - ${JSON.stringify(filterParams)}`) - filterParams = { to: '0xaa' } - assert.equal(txController.getFilteredTxList(filterParams).length, 5, `getFilteredTxList - ${JSON.stringify(filterParams)}`) }) }) @@ -389,11 +219,11 @@ describe('Transaction Controller', function () { const pubStub = sinon.stub(txController, 'publishTransaction').callsFake(() => { txController.setTxHash('1', originalValue) - txController.setTxStatusSubmitted('1') + txController.txStateManager.setTxStatusSubmitted('1') }) txController.approveTransaction(txMeta.id).then(() => { - const result = txController.getTx(txMeta.id) + const result = txController.txStateManager.getTx(txMeta.id) const params = result.txParams assert.equal(params.gas, originalValue, 'gas unmodified') diff --git a/test/unit/tx-state-manager-test.js b/test/unit/tx-state-manager-test.js new file mode 100644 index 000000000..0b35465d6 --- /dev/null +++ b/test/unit/tx-state-manager-test.js @@ -0,0 +1,227 @@ +const assert = require('assert') +const clone = require('clone') +const ObservableStore = require('obs-store') +const TxStateManager = require('../../app/scripts/lib/tx-state-manager') +const noop = () => true + +describe('TransactionStateManger', function () { + let txStateManager + const currentNetworkId = 42 + const otherNetworkId = 2 + + beforeEach(function () { + txStateManager = new TxStateManager({ + initState: { + transactions: [], + }, + txHistoryLimit: 10, + getNetwork: () => currentNetworkId + }) + }) + + describe('#setTxStatusSigned', function () { + it('sets the tx status to signed', function () { + let tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx, noop) + txStateManager.setTxStatusSigned(1) + let result = txStateManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 1) + assert.equal(result[0].status, 'signed') + }) + + it('should emit a signed event to signal the exciton of callback', (done) => { + let tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } + const noop = function () { + assert(true, 'event listener has been triggered and noop executed') + done() + } + txStateManager.addTx(tx) + txStateManager.on('1:signed', noop) + txStateManager.setTxStatusSigned(1) + + }) + }) + + describe('#setTxStatusRejected', function () { + it('sets the tx status to rejected', function () { + let tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx) + txStateManager.setTxStatusRejected(1) + let result = txStateManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 1) + assert.equal(result[0].status, 'rejected') + }) + + it('should emit a rejected event to signal the exciton of callback', (done) => { + let tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx) + const noop = function (err, txId) { + assert(true, 'event listener has been triggered and noop executed') + done() + } + txStateManager.on('1:rejected', noop) + txStateManager.setTxStatusRejected(1) + }) + }) + + describe('#getFullTxList', function () { + it('when new should return empty array', function () { + let result = txStateManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 0) + }) + }) + + describe('#getTxList', function () { + it('when new should return empty array', function () { + let result = txStateManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 0) + }) + }) + + describe('#addTx', function () { + it('adds a tx returned in getTxList', function () { + let tx = { id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx, noop) + let result = txStateManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 1) + assert.equal(result[0].id, 1) + }) + + it('does not override txs from other networks', function () { + let tx = { id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } + let tx2 = { id: 2, status: 'confirmed', metamaskNetworkId: otherNetworkId, txParams: {} } + txStateManager.addTx(tx, noop) + txStateManager.addTx(tx2, noop) + let result = txStateManager.getFullTxList() + let result2 = txStateManager.getTxList() + assert.equal(result.length, 2, 'txs were deleted') + assert.equal(result2.length, 1, 'incorrect number of txs on network.') + }) + + it('cuts off early txs beyond a limit', function () { + const limit = txStateManager.txHistoryLimit + for (let i = 0; i < limit + 1; i++) { + const tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx, noop) + } + let result = txStateManager.getTxList() + assert.equal(result.length, limit, `limit of ${limit} txs enforced`) + assert.equal(result[0].id, 1, 'early txs truncted') + }) + + it('cuts off early txs beyond a limit whether or not it is confirmed or rejected', function () { + const limit = txStateManager.txHistoryLimit + for (let i = 0; i < limit + 1; i++) { + const tx = { id: i, time: new Date(), status: 'rejected', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx, noop) + } + let result = txStateManager.getTxList() + assert.equal(result.length, limit, `limit of ${limit} txs enforced`) + assert.equal(result[0].id, 1, 'early txs truncted') + }) + + it('cuts off early txs beyond a limit but does not cut unapproved txs', function () { + let unconfirmedTx = { id: 0, time: new Date(), status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(unconfirmedTx, noop) + const limit = txStateManager.txHistoryLimit + for (let i = 1; i < limit + 1; i++) { + const tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx, noop) + } + let result = txStateManager.getTxList() + assert.equal(result.length, limit, `limit of ${limit} txs enforced`) + assert.equal(result[0].id, 0, 'first tx should still be there') + assert.equal(result[0].status, 'unapproved', 'first tx should be unapproved') + assert.equal(result[1].id, 2, 'early txs truncted') + }) + }) + + describe('#updateTx', function () { + it('replaces the tx with the same id', function () { + txStateManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txStateManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txStateManager.updateTx({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: currentNetworkId, txParams: {} }) + let result = txStateManager.getTx('1') + assert.equal(result.hash, 'foo') + }) + + it('updates gas price', function () { + const originalGasPrice = '0x01' + const desiredGasPrice = '0x02' + + const txMeta = { + id: '1', + status: 'unapproved', + metamaskNetworkId: currentNetworkId, + txParams: { + gasPrice: originalGasPrice, + }, + } + + const updatedMeta = clone(txMeta) + + txStateManager.addTx(txMeta) + updatedMeta.txParams.gasPrice = desiredGasPrice + txStateManager.updateTx(updatedMeta) + let result = txStateManager.getTx('1') + assert.equal(result.txParams.gasPrice, desiredGasPrice, 'gas price updated') + }) + }) + + describe('#getUnapprovedTxList', function () { + it('returns unapproved txs in a hash', function () { + txStateManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txStateManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + const result = txStateManager.getUnapprovedTxList() + assert.equal(typeof result, 'object') + assert.equal(result['1'].status, 'unapproved') + assert.equal(result['2'], undefined) + }) + }) + + describe('#getTx', function () { + it('returns a tx with the requested id', function () { + txStateManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txStateManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + assert.equal(txStateManager.getTx('1').status, 'unapproved') + assert.equal(txStateManager.getTx('2').status, 'confirmed') + }) + }) + + describe('#getFilteredTxList', function () { + it('returns a tx with the requested data', function () { + const txMetas = [ + { id: 0, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 1, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 2, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 3, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 4, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 5, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 6, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 7, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 8, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 9, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + ] + txMetas.forEach((txMeta) => txStateManager.addTx(txMeta, noop)) + let filterParams + + filterParams = { status: 'unapproved', from: '0xaa' } + assert.equal(txStateManager.getFilteredTxList(filterParams).length, 3, `getFilteredTxList - ${JSON.stringify(filterParams)}`) + filterParams = { status: 'unapproved', to: '0xaa' } + assert.equal(txStateManager.getFilteredTxList(filterParams).length, 2, `getFilteredTxList - ${JSON.stringify(filterParams)}`) + filterParams = { status: 'confirmed', from: '0xbb' } + assert.equal(txStateManager.getFilteredTxList(filterParams).length, 3, `getFilteredTxList - ${JSON.stringify(filterParams)}`) + filterParams = { status: 'confirmed' } + assert.equal(txStateManager.getFilteredTxList(filterParams).length, 5, `getFilteredTxList - ${JSON.stringify(filterParams)}`) + filterParams = { from: '0xaa' } + assert.equal(txStateManager.getFilteredTxList(filterParams).length, 5, `getFilteredTxList - ${JSON.stringify(filterParams)}`) + filterParams = { to: '0xaa' } + assert.equal(txStateManager.getFilteredTxList(filterParams).length, 5, `getFilteredTxList - ${JSON.stringify(filterParams)}`) + }) + }) +}) \ No newline at end of file From 7ea83b6bae34dcf652d85474fe1d82893d592d55 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 18 Aug 2017 12:23:35 -0700 Subject: [PATCH 02/88] Create TxStateManager --- app/scripts/controllers/transactions.js | 73 ++++++++++--------------- app/scripts/lib/tx-state-manager.js | 19 ++++++- app/scripts/metamask-controller.js | 2 +- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 48a882a71..b044948d7 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,6 +1,5 @@ const EventEmitter = require('events') const extend = require('xtend') -const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') @@ -24,16 +23,18 @@ module.exports = class TransactionController extends EventEmitter { this.query = new EthQuery(this.provider) this.txProviderUtil = new TxProviderUtil(this.provider) - this.txStateManager = new TransactionStateManger(extend({ - transactions: [], + this.txStateManager = new TransactionStateManger({ + initState: extend({ + transactions: [], + }, opts.initState), txHistoryLimit: opts.txHistoryLimit, getNetwork: this.getNetwork.bind(this), - }, opts.initState)) + }) this.nonceTracker = new NonceTracker({ provider: this.provider, getPendingTransactions: (address) => { - return this.getFilteredTxList({ + return this.txStateManager.getFilteredTxList({ from: address, status: 'submitted', err: undefined, @@ -52,16 +53,16 @@ module.exports = class TransactionController extends EventEmitter { publishTransaction: this.txProviderUtil.publishTransaction.bind(this.txProviderUtil), getPendingTransactions: () => { const network = this.getNetwork() - return this.getFilteredTxList({ + return this.txStateManager.getFilteredTxList({ status: 'submitted', metamaskNetworkId: network, }) }, }) - this.pendingTxTracker.on('txWarning', this.updateTx.bind(this)) - this.pendingTxTracker.on('txFailed', this.setTxStatusFailed.bind(this)) - this.pendingTxTracker.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) + this.pendingTxTracker.on('txWarning', this.txStateManager.updateTx.bind(this.txStateManager)) + this.pendingTxTracker.on('txFailed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) + this.pendingTxTracker.on('txConfirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) // this is a little messy but until ethstore has been either @@ -91,35 +92,17 @@ module.exports = class TransactionController extends EventEmitter { } getUnapprovedTxCount () { - return Object.keys(this.getUnapprovedTxList()).length + return Object.keys(this.txStateManager.getUnapprovedTxList()).length } getPendingTxCount () { return this.txStateManager.getTxsByMetaData('status', 'signed').length } - // Returns the tx list - - getUnapprovedTxList () { - const txList = this.txStateManager.getTxsByMetaData('status', 'unapproved') - return txList.reduce((result, tx) => { - result[tx.id] = tx - return result - }, {}) - } - // Adds a tx to the txlist addTx (txMeta) { this.txStateManager.addTx(txMeta) this.emit('update') - - this.once(`${txMeta.id}:signed`, function (txId) { - this.removeAllListeners(`${txMeta.id}:rejected`) - }) - this.once(`${txMeta.id}:rejected`, function (txId) { - this.removeAllListeners(`${txMeta.id}:signed`) - }) - this.emit('updateBadge') this.emit(`${txMeta.id}:unapproved`, txMeta) } @@ -130,7 +113,7 @@ module.exports = class TransactionController extends EventEmitter { this.emit('newUnaprovedTx', txMeta) // listen for tx completion (success, fail) return new Promise((resolve, reject) => { - this.once(`${txMeta.id}:finished`, (completedTx) => { + this.txStateManager.once(`${txMeta.id}:finished`, (completedTx) => { switch (completedTx.status) { case 'submitted': return resolve(completedTx.hash) @@ -158,7 +141,7 @@ module.exports = class TransactionController extends EventEmitter { // add default tx params await this.addTxDefaults(txMeta) // save txMeta - this.txStateManager.addTx(txMeta) + this.addTx(txMeta) return txMeta } @@ -175,7 +158,7 @@ module.exports = class TransactionController extends EventEmitter { } async updateAndApproveTransaction (txMeta) { - this.updateTx(txMeta) + this.txStateManager.updateTx(txMeta) await this.approveTransaction(txMeta.id) } @@ -183,9 +166,9 @@ module.exports = class TransactionController extends EventEmitter { let nonceLock try { // approve - this.setTxStatusApproved(txId) + this.txStateManager.setTxStatusApproved(txId) // get next nonce - const txMeta = this.getTx(txId) + const txMeta = this.txStateManager.getTx(txId) const fromAddress = txMeta.txParams.from // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) @@ -193,14 +176,14 @@ module.exports = class TransactionController extends EventEmitter { txMeta.txParams.nonce = nonceLock.nextNonce // add nonce debugging information to txMeta txMeta.nonceDetails = nonceLock.nonceDetails - this.updateTx(txMeta) + this.txStateManager.updateTx(txMeta) // sign transaction const rawTx = await this.signTransaction(txId) await this.publishTransaction(txId, rawTx) // must set transaction to submitted/failed before releasing lock nonceLock.releaseLock() } catch (err) { - this.setTxStatusFailed(txId, err) + this.txStateManager.setTxStatusFailed(txId, err) // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() // continue with error chain @@ -209,29 +192,29 @@ module.exports = class TransactionController extends EventEmitter { } async signTransaction (txId) { - const txMeta = this.getTx(txId) + const txMeta = this.txStateManager.getTx(txId) const txParams = txMeta.txParams const fromAddress = txParams.from // add network/chain id txParams.chainId = this.getChainId() const ethTx = this.txProviderUtil.buildEthTxFromParams(txParams) await this.signEthTx(ethTx, fromAddress) - this.setTxStatusSigned(txMeta.id) + this.txStateManager.setTxStatusSigned(txMeta.id) const rawTx = ethUtil.bufferToHex(ethTx.serialize()) return rawTx } async publishTransaction (txId, rawTx) { - const txMeta = this.getTx(txId) + const txMeta = this.txStateManager.getTx(txId) txMeta.rawTx = rawTx - this.updateTx(txMeta) + this.txStateManager.updateTx(txMeta) const txHash = await this.txProviderUtil.publishTransaction(rawTx) this.setTxHash(txId, txHash) - this.setTxStatusSubmitted(txId) + this.txStateManager.setTxStatusSubmitted(txId) } async cancelTransaction (txId) { - this.setTxStatusRejected(txId) + this.txStateManager.setTxStatusRejected(txId) } @@ -248,9 +231,9 @@ module.exports = class TransactionController extends EventEmitter { // receives a txHash records the tx as signed setTxHash (txId, txHash) { // Add the tx hash to the persisted meta-tx object - const txMeta = this.getTx(txId) + const txMeta = this.txStateManager.getTx(txId) txMeta.hash = txHash - this.updateTx(txMeta) + this.txStateManager.updateTx(txMeta) } /* _____________________________________ @@ -259,8 +242,8 @@ module.exports = class TransactionController extends EventEmitter { |______________________________________*/ _updateMemstore () { - const unapprovedTxs = this.getUnapprovedTxList() - const selectedAddressTxList = this.getFilteredTxList({ + const unapprovedTxs = this.txStateManager.getUnapprovedTxList() + const selectedAddressTxList = this.txStateManager.getFilteredTxList({ from: this.getSelectedAddress(), metamaskNetworkId: this.getNetwork(), }) diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index 91271cc4a..378ea38ab 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -23,7 +23,25 @@ module.exports = class TransactionStateManger extends ObservableStore { return this.getState().transactions } + // Returns the tx list + + getUnapprovedTxList () { + const txList = this.getTxsByMetaData('status', 'unapproved') + return txList.reduce((result, tx) => { + result[tx.id] = tx + return result + }, {}) + } + + addTx (txMeta) { + this.once(`${txMeta.id}:signed`, function (txId) { + this.removeAllListeners(`${txMeta.id}:rejected`) + }) + this.once(`${txMeta.id}:rejected`, function (txId) { + this.removeAllListeners(`${txMeta.id}:signed`) + }) + const transactions = this.getFullTxList() const txCount = this.getTxCount() const txHistoryLimit = this.txHistoryLimit @@ -38,7 +56,6 @@ module.exports = class TransactionStateManger extends ObservableStore { transactions.splice(index, 1) } transactions.push(txMeta) - transactions.push(txMeta) this._saveTxList(transactions) } // gets tx by Id and returns it diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a007d6fc5..7137190ac 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -133,7 +133,7 @@ module.exports = class MetamaskController extends EventEmitter { this.publicConfigStore = this.initPublicConfigStore() // manual disk state subscriptions - this.txController.store.subscribe((state) => { + this.txController.txStateManager.subscribe((state) => { this.store.updateState({ TransactionController: state }) }) this.keyringController.store.subscribe((state) => { From 474a4e941f7bb5c19ac50d61c6c681a19278b52f Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 21 Aug 2017 11:51:34 -0700 Subject: [PATCH 03/88] fix tests --- test/unit/tx-state-manager-test.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/unit/tx-state-manager-test.js b/test/unit/tx-state-manager-test.js index 998bbe152..464e50ee4 100644 --- a/test/unit/tx-state-manager-test.js +++ b/test/unit/tx-state-manager-test.js @@ -2,6 +2,7 @@ const assert = require('assert') const clone = require('clone') const ObservableStore = require('obs-store') const TxStateManager = require('../../app/scripts/lib/tx-state-manager') +const txStateHistoryHelper = require('../../app/scripts/lib/tx-state-history-helper') const noop = () => true describe('TransactionStateManger', function () { @@ -145,7 +146,9 @@ describe('TransactionStateManger', function () { it('replaces the tx with the same id', function () { txStateManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) txStateManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - txStateManager.updateTx({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: currentNetworkId, txParams: {} }) + const txMeta = txStateManager.getTx('1') + txMeta.hash = 'foo' + txStateManager.updateTx(txMeta) let result = txStateManager.getTx('1') assert.equal(result.hash, 'foo') }) @@ -166,16 +169,16 @@ describe('TransactionStateManger', function () { const updatedMeta = clone(txMeta) txStateManager.addTx(txMeta) - const updatedTx = txController.getTx('1') + const updatedTx = txStateManager.getTx('1') // verify tx was initialized correctly assert.equal(updatedTx.history.length, 1, 'one history item (initial)') assert.equal(Array.isArray(updatedTx.history[0]), false, 'first history item is initial state') assert.deepEqual(updatedTx.history[0], txStateHistoryHelper.snapshotFromTxMeta(updatedTx), 'first history item is initial state') // modify value and updateTx updatedTx.txParams.gasPrice = desiredGasPrice - txController.updateTx(updatedTx) + txStateManager.updateTx(updatedTx) // check updated value - const result = txController.getTx('1') + const result = txStateManager.getTx('1') assert.equal(result.txParams.gasPrice, desiredGasPrice, 'gas price updated') // validate history was updated assert.equal(result.history.length, 2, 'two history items (initial + diff)') From 4c554f32eca5ceda2d525b967075ec2b6ff41809 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 5 Sep 2017 20:13:43 -0700 Subject: [PATCH 04/88] remove #buildEthTxFromParams --- app/scripts/lib/tx-utils.js | 17 ----------------- test/unit/tx-utils-test.js | 4 +++- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 5af078dc4..618f69e1c 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -1,5 +1,4 @@ const EthQuery = require('ethjs-query') -const Transaction = require('ethereumjs-tx') const normalize = require('eth-sig-util').normalize const { hexToBn, @@ -78,22 +77,6 @@ module.exports = class txProvideUtil { return bnToHex(upperGasLimitBn) } - // builds ethTx from txParams object - buildEthTxFromParams (txParams) { - // normalize values - txParams.to = normalize(txParams.to) - txParams.from = normalize(txParams.from) - txParams.value = normalize(txParams.value) - txParams.data = normalize(txParams.data) - txParams.gas = normalize(txParams.gas || txParams.gasLimit) - txParams.gasPrice = normalize(txParams.gasPrice) - txParams.nonce = normalize(txParams.nonce) - // build ethTx - log.info(`Prepared tx for signing: ${JSON.stringify(txParams)}`) - const ethTx = new Transaction(txParams) - return ethTx - } - async publishTransaction (rawTx) { return await this.query.sendRawTransaction(rawTx) } diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index 43128b977..dcfb9fb3d 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -1,6 +1,8 @@ const assert = require('assert') +const Transaction = require('ethereumjs-tx') const BN = require('bn.js') + const { hexToBn, bnToHex } = require('../../app/scripts/lib/util') const TxUtils = require('../../app/scripts/lib/tx-utils') @@ -28,7 +30,7 @@ describe('txUtils', function () { nonce: '0x3', chainId: 42, } - const ethTx = txUtils.buildEthTxFromParams(txParams) + const ethTx = new Transaction(txParams) assert.equal(ethTx.getChainId(), 42, 'chainId is set from tx params') }) }) From 00bd5b143ffc13ac0a48f2049f61f26082af12c8 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 5 Sep 2017 20:33:50 -0700 Subject: [PATCH 05/88] rename tx-utils.js -> tx-gas-utils.js --- app/scripts/lib/{tx-utils.js => tx-gas-utils.js} | 4 ---- test/unit/tx-utils-test.js | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) rename app/scripts/lib/{tx-utils.js => tx-gas-utils.js} (96%) diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-gas-utils.js similarity index 96% rename from app/scripts/lib/tx-utils.js rename to app/scripts/lib/tx-gas-utils.js index 618f69e1c..246c94ccd 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-gas-utils.js @@ -77,10 +77,6 @@ module.exports = class txProvideUtil { return bnToHex(upperGasLimitBn) } - async publishTransaction (rawTx) { - return await this.query.sendRawTransaction(rawTx) - } - async validateTxParams (txParams) { if (('value' in txParams) && txParams.value.indexOf('-') === 0) { throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index dcfb9fb3d..8ca13412e 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -4,7 +4,7 @@ const BN = require('bn.js') const { hexToBn, bnToHex } = require('../../app/scripts/lib/util') -const TxUtils = require('../../app/scripts/lib/tx-utils') +const TxUtils = require('../../app/scripts/lib/tx-gas-utils') describe('txUtils', function () { From 15c12ca4bb6996f43518630ded12c0d7be2d571b Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 5 Sep 2017 21:50:36 -0700 Subject: [PATCH 06/88] add better comments --- app/scripts/controllers/transactions.js | 72 ++++++++++++++----------- app/scripts/lib/tx-state-manager.js | 1 - test/unit/tx-controller-test.js | 4 +- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 2349be1ad..6c4701283 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -2,13 +2,29 @@ const EventEmitter = require('events') const extend = require('xtend') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') +const Transaction = require('ethereumjs-tx') const EthQuery = require('ethjs-query') const TransactionStateManger = require('../lib/tx-state-manager') -const TxProviderUtil = require('../lib/tx-utils') +const TxGasUtil = require('../lib/tx-gas-utils') const PendingTransactionTracker = require('../lib/pending-tx-tracker') const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') +/* + Transaction Controller is an aggregate of sub-controllers and trackers + composing them in a way to be exposed to the metamask controller + - txStateManager + responsible for the state of a transaction and + storing the transaction + - pendingTxTracker + watching blocks for transactions to be include + and emitting confirmed events + - txGasUtil + gas calculations and safety buffering + - nonceTracker + calculating nonces +*/ + module.exports = class TransactionController extends EventEmitter { constructor (opts) { super() @@ -21,7 +37,7 @@ module.exports = class TransactionController extends EventEmitter { this.memStore = new ObservableStore({}) this.query = new EthQuery(this.provider) - this.txProviderUtil = new TxProviderUtil(this.provider) + this.txGasUtil = new TxGasUtil(this.provider) this.txStateManager = new TransactionStateManger({ initState: extend({ @@ -37,7 +53,6 @@ module.exports = class TransactionController extends EventEmitter { return this.txStateManager.getFilteredTxList({ from: address, status: 'submitted', - err: undefined, }) }, }) @@ -50,16 +65,17 @@ module.exports = class TransactionController extends EventEmitter { if (!account) return return account.balance }, - publishTransaction: this.txProviderUtil.publishTransaction.bind(this.txProviderUtil), + publishTransaction: this.query.sendRawTransaction, getPendingTransactions: () => { - const network = this.getNetwork() - return this.txStateManager.getFilteredTxList({ - status: 'submitted', - metamaskNetworkId: network, - }) + return this.txStateManager.getFilteredTxList({ status: 'submitted' }) }, }) + this.txStateManager.subscribe(() => { + this.emit('update') + this.emit('updateBadge') + }) + this.pendingTxTracker.on('txWarning', this.txStateManager.updateTx.bind(this.txStateManager)) this.pendingTxTracker.on('txFailed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) this.pendingTxTracker.on('txConfirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) @@ -99,11 +115,19 @@ module.exports = class TransactionController extends EventEmitter { return this.txStateManager.getTxsByMetaData('status', 'signed').length } + getChainId () { + const networkState = this.networkStore.getState() + const getChainId = parseInt(networkState) + if (Number.isNaN(getChainId)) { + return 0 + } else { + return getChainId + } + } + // Adds a tx to the txlist addTx (txMeta) { this.txStateManager.addTx(txMeta) - this.emit('update') - this.emit('updateBadge') this.emit(`${txMeta.id}:unapproved`, txMeta) } @@ -114,7 +138,6 @@ module.exports = class TransactionController extends EventEmitter { // listen for tx completion (success, fail) return new Promise((resolve, reject) => { this.txStateManager.once(`${txMeta.id}:finished`, (completedTx) => { - this.emit('updateBadge') switch (completedTx.status) { case 'submitted': return resolve(completedTx.hash) @@ -129,7 +152,7 @@ module.exports = class TransactionController extends EventEmitter { async addUnapprovedTransaction (txParams) { // validate - await this.txProviderUtil.validateTxParams(txParams) + await this.txGasUtil.validateTxParams(txParams) // construct txMeta const txMeta = { id: createId(), @@ -148,13 +171,11 @@ module.exports = class TransactionController extends EventEmitter { async addTxDefaults (txMeta) { const txParams = txMeta.txParams // ensure value + const gasPrice = txParams.gasPrice || await this.query.gasPrice() txParams.value = txParams.value || '0x0' - if (!txParams.gasPrice) { - const gasPrice = await this.query.gasPrice() - txParams.gasPrice = gasPrice - } + txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16)) // set gasLimit - return await this.txProviderUtil.analyzeGasUsage(txMeta) + return await this.txGasUtil.analyzeGasUsage(txMeta) } async updateAndApproveTransaction (txMeta) { @@ -197,7 +218,7 @@ module.exports = class TransactionController extends EventEmitter { const fromAddress = txParams.from // add network/chain id txParams.chainId = this.getChainId() - const ethTx = this.txProviderUtil.buildEthTxFromParams(txParams) + const ethTx = new Transaction(txParams) await this.signEthTx(ethTx, fromAddress) this.txStateManager.setTxStatusSigned(txMeta.id) const rawTx = ethUtil.bufferToHex(ethTx.serialize()) @@ -208,7 +229,7 @@ module.exports = class TransactionController extends EventEmitter { const txMeta = this.txStateManager.getTx(txId) txMeta.rawTx = rawTx this.txStateManager.updateTx(txMeta) - const txHash = await this.txProviderUtil.publishTransaction(rawTx) + const txHash = await this.query.sendRawTransaction(rawTx) this.setTxHash(txId, txHash) this.txStateManager.setTxStatusSubmitted(txId) } @@ -217,17 +238,6 @@ module.exports = class TransactionController extends EventEmitter { this.txStateManager.setTxStatusRejected(txId) } - - getChainId () { - const networkState = this.networkStore.getState() - const getChainId = parseInt(networkState) - if (Number.isNaN(getChainId)) { - return 0 - } else { - return getChainId - } - } - // receives a txHash records the tx as signed setTxHash (txId, txHash) { // Add the tx hash to the persisted meta-tx object diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index d3314c286..64925c528 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -33,7 +33,6 @@ module.exports = class TransactionStateManger extends ObservableStore { }, {}) } - addTx (txMeta) { this.once(`${txMeta.id}:signed`, function (txId) { this.removeAllListeners(`${txMeta.id}:rejected`) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index fdbddac4b..f969752ec 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -5,7 +5,7 @@ const ObservableStore = require('obs-store') const clone = require('clone') const sinon = require('sinon') const TransactionController = require('../../app/scripts/controllers/transactions') -const TxProvideUtils = require('../../app/scripts/lib/tx-utils') +const TxGasUtils = require('../../app/scripts/lib/tx-gas-utils') const txStateHistoryHelper = require('../../app/scripts/lib/tx-state-history-helper') const noop = () => true @@ -34,7 +34,7 @@ describe('Transaction Controller', function () { }), }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) - txController.txProviderUtils = new TxProvideUtils(txController.provider) + txController.txProviderUtils = new TxGasUtils(txController.provider) }) describe('#newUnapprovedTransaction', function () { From a73aecc796c2f2416227508db8e96e22915cfa65 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 6 Sep 2017 14:01:07 -0700 Subject: [PATCH 07/88] fix merge and errors disaperaing on update --- app/scripts/controllers/transactions.js | 2 +- app/scripts/lib/tx-state-manager.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 3f2a7ce16..5aaee7be0 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -56,7 +56,7 @@ module.exports = class TransactionController extends EventEmitter { }) }, getConfirmedTransactions: (address) => { - return this.getFilteredTxList({ + return this.txStateManager.getFilteredTxList({ from: address, status: 'confirmed', err: undefined, diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index 64925c528..eb2a187db 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -180,9 +180,12 @@ module.exports = class TransactionStateManger extends ObservableStore { this._setTxStatus(txId, 'confirmed') } - setTxStatusFailed (txId, reason) { + setTxStatusFailed (txId, err) { const txMeta = this.getTx(txId) - txMeta.err = reason + txMeta.err = { + message: err.toString(), + stack: err.stack, + } this.updateTx(txMeta) this._setTxStatus(txId, 'failed') } From 714df393b406c515b0bb3abd4fcd8bde4ad53fde Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 6 Sep 2017 14:27:21 -0700 Subject: [PATCH 08/88] Add test template --- test/unit/pending-balance-test.js | 37 +++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 test/unit/pending-balance-test.js diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js new file mode 100644 index 000000000..845d6d552 --- /dev/null +++ b/test/unit/pending-balance-test.js @@ -0,0 +1,37 @@ +const assert = require('assert') +const PendingBalanceCalculator = require('../../app/scripts/lib/pending-balance-calculator') +const MockTxGen = require('../lib/mock-tx-gen') +const BN = require('ethereumjs-util').BN +let providerResultStub = {} + +describe('PendingBalanceCalculator', function () { + let nonceTracker + + describe('if you have no pending txs and one ether', function () { + const ether = '0x' + (new BN(1e18)).toString(16) + + beforeEach(function () { + nonceTracker = generateNonceTrackerWith([], ether) + }) + + it('returns the network balance', function () { + const result = nonceTracker.getBalance() + assert.equal(result, ether, 'returns one ether') + }) + }) +}) + +function generateBalaneCalcWith (transactions, providerStub = '0x0') { + const getPendingTransactions = () => transactions + providerResultStub.result = providerStub + const provider = { + sendAsync: (_, cb) => { cb(undefined, providerResultStub) }, + _blockTracker: { + getCurrentBlock: () => '0x11b568', + }, + } + return new PendingBalanceCalculator({ + provider, + getPendingTransactions, + }) +} From f9a052deed8749a1c6dd544df561967a568749d9 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 6 Sep 2017 14:36:15 -0700 Subject: [PATCH 09/88] Add first passing balance calc test --- app/scripts/lib/pending-balance-calculator.js | 18 ++++++++++++++++++ test/unit/pending-balance-test.js | 15 ++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 app/scripts/lib/pending-balance-calculator.js diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js new file mode 100644 index 000000000..5b30354a2 --- /dev/null +++ b/app/scripts/lib/pending-balance-calculator.js @@ -0,0 +1,18 @@ +const BN = require('ethereumjs-util').BN +const EthQuery = require('ethjs-query') + +class PendingBalanceCalculator { + + constructor ({ getBalance, getPendingTransactions }) { + this.getPendingTransactions = getPendingTransactions + this.getBalance = getBalance + } + + async getBalance() { + const balance = await this.getBalance + return balance + } + +} + +module.exports = PendingBalanceCalculator diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index 845d6d552..dcf1926f0 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -8,21 +8,22 @@ describe('PendingBalanceCalculator', function () { let nonceTracker describe('if you have no pending txs and one ether', function () { - const ether = '0x' + (new BN(1e18)).toString(16) + const ether = '0x' + (new BN(String(1e18))).toString(16) beforeEach(function () { - nonceTracker = generateNonceTrackerWith([], ether) + nonceTracker = generateBalaneCalcWith([], ether) }) - it('returns the network balance', function () { - const result = nonceTracker.getBalance() - assert.equal(result, ether, 'returns one ether') + it('returns the network balance', async function () { + const result = await nonceTracker.getBalance() + assert.equal(result, ether, `gave ${result} needed ${ether}`) }) }) }) function generateBalaneCalcWith (transactions, providerStub = '0x0') { - const getPendingTransactions = () => transactions + const getPendingTransactions = () => Promise.resolve(transactions) + const getBalance = () => Promise.resolve(providerStub) providerResultStub.result = providerStub const provider = { sendAsync: (_, cb) => { cb(undefined, providerResultStub) }, @@ -31,7 +32,7 @@ function generateBalaneCalcWith (transactions, providerStub = '0x0') { }, } return new PendingBalanceCalculator({ - provider, + getBalance, getPendingTransactions, }) } From 74f7fc4613d136b57a4395d273ce4bf52d6685db Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 6 Sep 2017 14:37:46 -0700 Subject: [PATCH 10/88] Check balances in parallel --- app/scripts/lib/pending-balance-calculator.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 5b30354a2..4f6e03138 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -9,7 +9,14 @@ class PendingBalanceCalculator { } async getBalance() { - const balance = await this.getBalance + const results = await Promise.all([ + this.getBalance(), + this.getPendingTransactions(), + ]) + + const balance = results[0] + const pending = results[1] + return balance } From 00fca4f1f236c915eca8f0b068a7c5998e1092f3 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 6 Sep 2017 14:38:39 -0700 Subject: [PATCH 11/88] remove unused variable --- app/scripts/lib/tx-gas-utils.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/lib/tx-gas-utils.js b/app/scripts/lib/tx-gas-utils.js index 246c94ccd..41f67e230 100644 --- a/app/scripts/lib/tx-gas-utils.js +++ b/app/scripts/lib/tx-gas-utils.js @@ -1,5 +1,4 @@ const EthQuery = require('ethjs-query') -const normalize = require('eth-sig-util').normalize const { hexToBn, BnMultiplyByFraction, From 50075c6df5af4441db78b47f3bc2036a65228224 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Thu, 7 Sep 2017 00:55:21 -0700 Subject: [PATCH 12/88] fix messy merge --- app/scripts/controllers/transactions.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 5aaee7be0..d71be37d8 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -62,10 +62,6 @@ module.exports = class TransactionController extends EventEmitter { err: undefined, }) }, - giveUpOnTransaction: (txId) => { - const msg = `Gave up submitting after 3500 blocks un-mined.` - this.setTxStatusFailed(txId, msg) - }, }) this.pendingTxTracker = new PendingTransactionTracker({ @@ -80,6 +76,10 @@ module.exports = class TransactionController extends EventEmitter { getPendingTransactions: () => { return this.txStateManager.getFilteredTxList({ status: 'submitted' }) }, + giveUpOnTransaction: (txId) => { + const msg = `Gave up submitting after 3500 blocks un-mined.` + this.setTxStatusFailed(txId, msg) + }, }) this.txStateManager.subscribe(() => { From b6e8791bc2bc912d874edcc92fcf3c4ce5a9b72a Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 11:59:15 -0700 Subject: [PATCH 13/88] test not passing --- app/scripts/lib/pending-balance-calculator.js | 18 +++++++++- test/unit/pending-balance-test.js | 35 +++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 4f6e03138..9df87e34b 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -1,5 +1,6 @@ const BN = require('ethereumjs-util').BN const EthQuery = require('ethjs-query') +const normalize = require('eth-sig-util').normalize class PendingBalanceCalculator { @@ -9,15 +10,30 @@ class PendingBalanceCalculator { } async getBalance() { + console.log('getting balance') const results = await Promise.all([ this.getBalance(), this.getPendingTransactions(), ]) + console.dir(results) const balance = results[0] const pending = results[1] - return balance + console.dir({ balance, pending }) + + const pendingValue = pending.reduce(function (total, tx) { + return total.sub(this.valueFor(tx)) + }, new BN(0)) + + const balanceBn = new BN(normalize(balance)) + + return `0x${ balanceBn.sub(pendingValue).toString(16) }` + } + + valueFor (tx) { + const value = new BN(normalize(tx.txParams.value)) + return value } } diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index dcf1926f0..9077e8f14 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -5,20 +5,49 @@ const BN = require('ethereumjs-util').BN let providerResultStub = {} describe('PendingBalanceCalculator', function () { - let nonceTracker + let balanceCalculator describe('if you have no pending txs and one ether', function () { const ether = '0x' + (new BN(String(1e18))).toString(16) beforeEach(function () { - nonceTracker = generateBalaneCalcWith([], ether) + balanceCalculator = generateBalaneCalcWith([], ether) }) it('returns the network balance', async function () { - const result = await nonceTracker.getBalance() + const result = await balanceCalculator.getBalance() assert.equal(result, ether, `gave ${result} needed ${ether}`) }) }) + + describe('if you have a one ether pending tx and one ether', function () { + const ether = '0x' + (new BN(String(1e18))).toString(16) + + beforeEach(function () { + const txGen = new MockTxGen() + pendingTxs = txGen.generate({ + status: 'submitted', + txParams: { + value: ether, + gasPrice: '0x0', + gas: '0x0', + } + }, { count: 1 }) + + balanceCalculator = generateBalaneCalcWith(pendingTxs, ether) + }) + + it('returns the network balance', async function () { + console.log('one') + console.dir(balanceCalculator) + const result = await balanceCalculator.getBalance() + console.log('two') + console.dir(result) + assert.equal(result, '0x0', `gave ${result} needed '0x0'`) + return true + }) + + }) }) function generateBalaneCalcWith (transactions, providerStub = '0x0') { From 40585744365c128d1f64c5bf93ee8cedc9e91dae Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:30:25 -0700 Subject: [PATCH 14/88] Add basic test for valueFor --- app/scripts/lib/pending-balance-calculator.js | 13 +++++++--- test/unit/pending-balance-test.js | 25 ++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 9df87e34b..f2c9ce379 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -6,13 +6,13 @@ class PendingBalanceCalculator { constructor ({ getBalance, getPendingTransactions }) { this.getPendingTransactions = getPendingTransactions - this.getBalance = getBalance + this.getNetworkBalance = getBalance } async getBalance() { console.log('getting balance') const results = await Promise.all([ - this.getBalance(), + this.getNetworkBalance(), this.getPendingTransactions(), ]) console.dir(results) @@ -21,18 +21,23 @@ class PendingBalanceCalculator { const pending = results[1] console.dir({ balance, pending }) + console.dir(pending) const pendingValue = pending.reduce(function (total, tx) { - return total.sub(this.valueFor(tx)) + return total.add(this.valueFor(tx)) }, new BN(0)) const balanceBn = new BN(normalize(balance)) + console.log(`subtracting ${pendingValue.toString()} from ${balanceBn.toString()}`) return `0x${ balanceBn.sub(pendingValue).toString(16) }` } valueFor (tx) { - const value = new BN(normalize(tx.txParams.value)) + const txValue = tx.txParams.value + const normalized = normalize(txValue).substring(2) + console.log({ txValue, normalized }) + const value = new BN(normalize(txValue).substring(2), 16) return value } diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index 9077e8f14..7f20270cb 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -4,11 +4,31 @@ const MockTxGen = require('../lib/mock-tx-gen') const BN = require('ethereumjs-util').BN let providerResultStub = {} +const etherBn = new BN(String(1e18)) +const ether = '0x' + etherBn.toString(16) + describe('PendingBalanceCalculator', function () { let balanceCalculator + describe('#valueFor(tx)', function () { + it('returns a BN for a given tx value', function () { + const txGen = new MockTxGen() + pendingTxs = txGen.generate({ + status: 'submitted', + txParams: { + value: ether, + gasPrice: '0x0', + gas: '0x0', + } + }, { count: 1 }) + + const balanceCalculator = generateBalaneCalcWith([], '0x0') + const result = balanceCalculator.valueFor(pendingTxs[0]) + assert.equal(result.toString(), etherBn.toString(), 'computes one ether') + }) + }) + describe('if you have no pending txs and one ether', function () { - const ether = '0x' + (new BN(String(1e18))).toString(16) beforeEach(function () { balanceCalculator = generateBalaneCalcWith([], ether) @@ -21,8 +41,6 @@ describe('PendingBalanceCalculator', function () { }) describe('if you have a one ether pending tx and one ether', function () { - const ether = '0x' + (new BN(String(1e18))).toString(16) - beforeEach(function () { const txGen = new MockTxGen() pendingTxs = txGen.generate({ @@ -40,6 +58,7 @@ describe('PendingBalanceCalculator', function () { it('returns the network balance', async function () { console.log('one') console.dir(balanceCalculator) + console.dir(balanceCalculator.getBalance.toString()) const result = await balanceCalculator.getBalance() console.log('two') console.dir(result) From 7b92268428cc2de4374bc669c524bb61959801f1 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:43:10 -0700 Subject: [PATCH 15/88] Fix valueFor test --- app/scripts/lib/pending-balance-calculator.js | 15 ++++++++------- test/unit/pending-balance-test.js | 9 +++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index f2c9ce379..e4ff1e050 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -15,32 +15,33 @@ class PendingBalanceCalculator { this.getNetworkBalance(), this.getPendingTransactions(), ]) - console.dir(results) const balance = results[0] const pending = results[1] - console.dir({ balance, pending }) console.dir(pending) - const pendingValue = pending.reduce(function (total, tx) { + const pendingValue = pending.reduce((total, tx) => { return total.add(this.valueFor(tx)) }, new BN(0)) - const balanceBn = new BN(normalize(balance)) - console.log(`subtracting ${pendingValue.toString()} from ${balanceBn.toString()}`) + console.log(`subtracting ${pendingValue.toString()} from ${balance.toString()}`) - return `0x${ balanceBn.sub(pendingValue).toString(16) }` + return `0x${ balance.sub(pendingValue).toString(16) }` } valueFor (tx) { const txValue = tx.txParams.value const normalized = normalize(txValue).substring(2) console.log({ txValue, normalized }) - const value = new BN(normalize(txValue).substring(2), 16) + const value = this.hexToBn(txValue) return value } + hexToBn (hex) { + return new BN(normalize(hex).substring(2), 16) + } + } module.exports = PendingBalanceCalculator diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index 7f20270cb..0937579e2 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -4,6 +4,7 @@ const MockTxGen = require('../lib/mock-tx-gen') const BN = require('ethereumjs-util').BN let providerResultStub = {} +const zeroBn = new BN(0) const etherBn = new BN(String(1e18)) const ether = '0x' + etherBn.toString(16) @@ -22,7 +23,7 @@ describe('PendingBalanceCalculator', function () { } }, { count: 1 }) - const balanceCalculator = generateBalaneCalcWith([], '0x0') + const balanceCalculator = generateBalanceCalcWith([], zeroBn) const result = balanceCalculator.valueFor(pendingTxs[0]) assert.equal(result.toString(), etherBn.toString(), 'computes one ether') }) @@ -31,7 +32,7 @@ describe('PendingBalanceCalculator', function () { describe('if you have no pending txs and one ether', function () { beforeEach(function () { - balanceCalculator = generateBalaneCalcWith([], ether) + balanceCalculator = generateBalanceCalcWith([], zeroBn) }) it('returns the network balance', async function () { @@ -52,7 +53,7 @@ describe('PendingBalanceCalculator', function () { } }, { count: 1 }) - balanceCalculator = generateBalaneCalcWith(pendingTxs, ether) + balanceCalculator = generateBalanceCalcWith(pendingTxs, etherBn) }) it('returns the network balance', async function () { @@ -69,7 +70,7 @@ describe('PendingBalanceCalculator', function () { }) }) -function generateBalaneCalcWith (transactions, providerStub = '0x0') { +function generateBalanceCalcWith (transactions, providerStub = zeroBn) { const getPendingTransactions = () => Promise.resolve(transactions) const getBalance = () => Promise.resolve(providerStub) providerResultStub.result = providerStub From 74c6de7d23c979c091028d8bd599f389e2090bc1 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:45:00 -0700 Subject: [PATCH 16/88] Add constructor comment --- app/scripts/lib/pending-balance-calculator.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index e4ff1e050..d5e2e4c17 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -4,6 +4,11 @@ const normalize = require('eth-sig-util').normalize class PendingBalanceCalculator { + // Must be initialized with two functions: + // getBalance => Returns a promise of a BN of the current balance in Wei + // getPendingTransactions => Returns an array of TxMeta Objects, + // which have txParams properties, which include value, gasPrice, and gas, + // all in a base=16 hex format. constructor ({ getBalance, getPendingTransactions }) { this.getPendingTransactions = getPendingTransactions this.getNetworkBalance = getBalance From a95a3c7e4f4d1331394a7bf92a77678fe8087c04 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:47:27 -0700 Subject: [PATCH 17/88] Fix balance calc test --- app/scripts/lib/pending-balance-calculator.js | 2 ++ test/unit/pending-balance-test.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index d5e2e4c17..4e1189a65 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -25,6 +25,8 @@ class PendingBalanceCalculator { const pending = results[1] console.dir(pending) + console.dir(balance.toString()) + console.trace('but why') const pendingValue = pending.reduce((total, tx) => { return total.add(this.valueFor(tx)) diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index 0937579e2..a9b0f7b66 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -32,7 +32,7 @@ describe('PendingBalanceCalculator', function () { describe('if you have no pending txs and one ether', function () { beforeEach(function () { - balanceCalculator = generateBalanceCalcWith([], zeroBn) + balanceCalculator = generateBalanceCalcWith([], etherBn) }) it('returns the network balance', async function () { From c616581001a7413a289b108b347005d53fb14732 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:47:52 -0700 Subject: [PATCH 18/88] Remove logs --- app/scripts/lib/pending-balance-calculator.js | 8 -------- test/unit/pending-balance-test.js | 5 ----- 2 files changed, 13 deletions(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 4e1189a65..8564f0134 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -15,7 +15,6 @@ class PendingBalanceCalculator { } async getBalance() { - console.log('getting balance') const results = await Promise.all([ this.getNetworkBalance(), this.getPendingTransactions(), @@ -24,23 +23,16 @@ class PendingBalanceCalculator { const balance = results[0] const pending = results[1] - console.dir(pending) - console.dir(balance.toString()) - console.trace('but why') - const pendingValue = pending.reduce((total, tx) => { return total.add(this.valueFor(tx)) }, new BN(0)) - console.log(`subtracting ${pendingValue.toString()} from ${balance.toString()}`) - return `0x${ balance.sub(pendingValue).toString(16) }` } valueFor (tx) { const txValue = tx.txParams.value const normalized = normalize(txValue).substring(2) - console.log({ txValue, normalized }) const value = this.hexToBn(txValue) return value } diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index a9b0f7b66..e1d5f9303 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -57,12 +57,7 @@ describe('PendingBalanceCalculator', function () { }) it('returns the network balance', async function () { - console.log('one') - console.dir(balanceCalculator) - console.dir(balanceCalculator.getBalance.toString()) const result = await balanceCalculator.getBalance() - console.log('two') - console.dir(result) assert.equal(result, '0x0', `gave ${result} needed '0x0'`) return true }) From 532a4040de191d455053aa11432dcb9b407c9bf4 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:52:25 -0700 Subject: [PATCH 19/88] Add test for computing gas price --- test/unit/pending-balance-test.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index e1d5f9303..3029a849c 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -27,6 +27,22 @@ describe('PendingBalanceCalculator', function () { const result = balanceCalculator.valueFor(pendingTxs[0]) assert.equal(result.toString(), etherBn.toString(), 'computes one ether') }) + + it('calculates gas costs as well', function () { + const txGen = new MockTxGen() + pendingTxs = txGen.generate({ + status: 'submitted', + txParams: { + value: '0x0', + gasPrice: '0x2', + gas: '0x3', + } + }, { count: 1 }) + + const balanceCalculator = generateBalanceCalcWith([], zeroBn) + const result = balanceCalculator.valueFor(pendingTxs[0]) + assert.equal(result.toString(), '6', 'computes one ether') + }) }) describe('if you have no pending txs and one ether', function () { From fadc0617df016ad1fa3d1c92e220a8e9ede6379d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:52:49 -0700 Subject: [PATCH 20/88] Make tx calculations account for gas prices --- app/scripts/lib/pending-balance-calculator.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 8564f0134..29f1fd63a 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -32,9 +32,15 @@ class PendingBalanceCalculator { valueFor (tx) { const txValue = tx.txParams.value - const normalized = normalize(txValue).substring(2) const value = this.hexToBn(txValue) - return value + const gasPrice = this.hexToBn(tx.txParams.gasPrice) + + const gas = tx.txParams.gas + const gasLimit = tx.txParams.gasLimit + const gasLimitBn = this.hexToBn(gas || gasLimit) + + const gasCost = gasPrice.mul(gasLimitBn) + return value.add(gasCost) } hexToBn (hex) { From 65c00e9fbbb4496db4adee13c227e47ba85837c6 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:53:30 -0700 Subject: [PATCH 21/88] Improve test name --- test/unit/pending-balance-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index 3029a849c..17be0306b 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -72,7 +72,7 @@ describe('PendingBalanceCalculator', function () { balanceCalculator = generateBalanceCalcWith(pendingTxs, etherBn) }) - it('returns the network balance', async function () { + it('returns the subtracted result', async function () { const result = await balanceCalculator.getBalance() assert.equal(result, '0x0', `gave ${result} needed '0x0'`) return true From d4d7c6d89eeddbe865e32b0f3636cc9de2a17cc1 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Sep 2017 12:54:28 -0700 Subject: [PATCH 22/88] Linted --- app/scripts/lib/pending-balance-calculator.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 29f1fd63a..474ed3261 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -1,5 +1,4 @@ const BN = require('ethereumjs-util').BN -const EthQuery = require('ethjs-query') const normalize = require('eth-sig-util').normalize class PendingBalanceCalculator { @@ -27,7 +26,7 @@ class PendingBalanceCalculator { return total.add(this.valueFor(tx)) }, new BN(0)) - return `0x${ balance.sub(pendingValue).toString(16) }` + return `0x${balance.sub(pendingValue).toString(16)}` } valueFor (tx) { From 9b9df417246dbf332f0a7d8afadb664544ceb484 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 8 Sep 2017 14:24:40 -0700 Subject: [PATCH 23/88] more tests and craete a getPendingTransactions function --- app/scripts/controllers/transactions.js | 15 ++++----------- app/scripts/lib/tx-state-manager.js | 6 ++++++ test/unit/components/pending-tx-test.js | 3 ++- test/unit/tx-controller-test.js | 22 ++++++++++++++++++++++ 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index d71be37d8..636424c64 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -49,12 +49,7 @@ module.exports = class TransactionController extends EventEmitter { this.nonceTracker = new NonceTracker({ provider: this.provider, - getPendingTransactions: (address) => { - return this.txStateManager.getFilteredTxList({ - from: address, - status: 'submitted', - }) - }, + getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), getConfirmedTransactions: (address) => { return this.txStateManager.getFilteredTxList({ from: address, @@ -73,9 +68,7 @@ module.exports = class TransactionController extends EventEmitter { return account.balance }, publishTransaction: this.query.sendRawTransaction, - getPendingTransactions: () => { - return this.txStateManager.getFilteredTxList({ status: 'submitted' }) - }, + getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), giveUpOnTransaction: (txId) => { const msg = `Gave up submitting after 3500 blocks un-mined.` this.setTxStatusFailed(txId, msg) @@ -122,8 +115,8 @@ module.exports = class TransactionController extends EventEmitter { return Object.keys(this.txStateManager.getUnapprovedTxList()).length } - getPendingTxCount () { - return this.txStateManager.getTxsByMetaData('status', 'signed').length + getPendingTxCount (account) { + return this.txStateManager.getPendingTransactions(account).length } getChainId () { diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index eb2a187db..05fbba612 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -33,6 +33,12 @@ module.exports = class TransactionStateManger extends ObservableStore { }, {}) } + getPendingTransactions (address) { + const opts = { status: 'submitted' } + if (address) opts.from = address + return this.txStateManager.getFilteredTxList(opts) + } + addTx (txMeta) { this.once(`${txMeta.id}:signed`, function (txId) { this.removeAllListeners(`${txMeta.id}:rejected`) diff --git a/test/unit/components/pending-tx-test.js b/test/unit/components/pending-tx-test.js index 22a98bc93..906564558 100644 --- a/test/unit/components/pending-tx-test.js +++ b/test/unit/components/pending-tx-test.js @@ -24,7 +24,8 @@ describe('PendingTx', function () { 'to': '0xc5b8dbac4c1d3f152cdeb400e2313f309c410acb', 'value': '0xde0b6b3a7640000', gasPrice, - 'gas': '0x7b0c'}, + 'gas': '0x7b0c', + }, 'gasLimitSpecified': false, 'estimatedGas': '0x5208', } diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index f969752ec..937ac55de 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -37,6 +37,28 @@ describe('Transaction Controller', function () { txController.txProviderUtils = new TxGasUtils(txController.provider) }) + describe('#getState', function () { + it('should return a state object with the right keys and datat types', function (){ + const exposedState = txController.getState() + assert('unapprovedTxs' in exposedState, 'state should have the key unapprovedTxs') + assert('selectedAddressTxList' in exposedState, 'state should have the key selectedAddressTxList') + assert(typeof exposedState.unapprovedTxs === 'object', 'should be an object') + assert(Array.isArray(exposedState.selectedAddressTxList), 'should be an array') + }) + }) + + describe('#getUnapprovedTxCount', function () { + it('should return the number of unapproved txs', function () { + txController.txStateManager._saveTxList([ + { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 2, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 3, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, + ]) + const unapprovedTxCount = txController.getUnapprovedTxCount() + assert.equal(unapprovedTxCount, 3, 'should be 3') + }) + }) + describe('#newUnapprovedTransaction', function () { let stub, txMeta, txParams beforeEach(function () { From 62f26c5ba873280b536aa7ce31cf92733a3e707c Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 8 Sep 2017 15:02:36 -0700 Subject: [PATCH 24/88] fix miss type --- app/scripts/lib/tx-state-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index 05fbba612..661523f10 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -36,7 +36,7 @@ module.exports = class TransactionStateManger extends ObservableStore { getPendingTransactions (address) { const opts = { status: 'submitted' } if (address) opts.from = address - return this.txStateManager.getFilteredTxList(opts) + return this.getFilteredTxList(opts) } addTx (txMeta) { From 3ad67d1b14b5b56002cf34ab6dbb18d602705827 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 12 Sep 2017 09:59:59 -0700 Subject: [PATCH 25/88] match other controller patterns --- app/scripts/controllers/transactions.js | 22 ++++++++-------------- app/scripts/lib/tx-state-manager.js | 21 +++++++++++++-------- app/scripts/metamask-controller.js | 2 +- test/unit/tx-controller-test.js | 2 +- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 636424c64..3cb6a609e 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -40,9 +40,7 @@ module.exports = class TransactionController extends EventEmitter { this.txGasUtil = new TxGasUtil(this.provider) this.txStateManager = new TransactionStateManger({ - initState: extend({ - transactions: [], - }, opts.initState), + initState: opts.initState, txHistoryLimit: opts.txHistoryLimit, getNetwork: this.getNetwork.bind(this), }) @@ -70,15 +68,12 @@ module.exports = class TransactionController extends EventEmitter { publishTransaction: this.query.sendRawTransaction, getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), giveUpOnTransaction: (txId) => { - const msg = `Gave up submitting after 3500 blocks un-mined.` - this.setTxStatusFailed(txId, msg) + const err = new Error(`Gave up submitting after 3500 blocks un-mined.`) + this.setTxStatusFailed(txId, err) }, }) - this.txStateManager.subscribe(() => { - this.emit('update') - this.emit('updateBadge') - }) + this.txStateManager.store.subscribe(() => this.emit('updateBadge')) this.pendingTxTracker.on('txWarning', this.txStateManager.updateTx.bind(this.txStateManager)) this.pendingTxTracker.on('txFailed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) @@ -94,7 +89,7 @@ module.exports = class TransactionController extends EventEmitter { this.blockTracker.on('sync', this.pendingTxTracker.queryPendingTxs.bind(this.pendingTxTracker)) // memstore is computed from a few different stores this._updateMemstore() - this.txStateManager.subscribe(() => this._updateMemstore()) + this.txStateManager.store.subscribe(() => this._updateMemstore()) this.networkStore.subscribe(() => this._updateMemstore()) this.preferencesStore.subscribe(() => this._updateMemstore()) } @@ -250,10 +245,9 @@ module.exports = class TransactionController extends EventEmitter { this.txStateManager.updateTx(txMeta) } -/* _____________________________________ -| | -| PRIVATE METHODS | -|______________________________________*/ +// +// PRIVATE METHODS +// _updateMemstore () { const unapprovedTxs = this.txStateManager.getUnapprovedTxList() diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index 661523f10..843592504 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -1,10 +1,16 @@ const extend = require('xtend') +const EventEmitter = require('events') const ObservableStore = require('obs-store') const txStateHistoryHelper = require('./tx-state-history-helper') -module.exports = class TransactionStateManger extends ObservableStore { +module.exports = class TransactionStateManger extends EventEmitter { constructor ({initState, txHistoryLimit, getNetwork}) { - super(initState) + super() + + this.store = new ObservableStore( + extend({ + transactions: [], + }, initState)) this.txHistoryLimit = txHistoryLimit this.getNetwork = getNetwork } @@ -20,7 +26,7 @@ module.exports = class TransactionStateManger extends ObservableStore { } getFullTxList () { - return this.getState().transactions + return this.store.getState().transactions } // Returns the tx list @@ -196,10 +202,9 @@ module.exports = class TransactionStateManger extends ObservableStore { this._setTxStatus(txId, 'failed') } -/* _____________________________________ -| | -| PRIVATE METHODS | -|______________________________________*/ +// +// PRIVATE METHODS +// // Should find the tx in the tx list and // update it. @@ -225,6 +230,6 @@ module.exports = class TransactionStateManger extends ObservableStore { // Saves the new/updated txList. // Function is intended only for internal use _saveTxList (transactions) { - this.updateState({ transactions }) + this.store.updateState({ transactions }) } } \ No newline at end of file diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 7137190ac..23049c46d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -133,7 +133,7 @@ module.exports = class MetamaskController extends EventEmitter { this.publicConfigStore = this.initPublicConfigStore() // manual disk state subscriptions - this.txController.txStateManager.subscribe((state) => { + this.txController.txStateManager.store.subscribe((state) => { this.store.updateState({ TransactionController: state }) }) this.keyringController.store.subscribe((state) => { diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 937ac55de..479010b87 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -197,7 +197,7 @@ describe('Transaction Controller', function () { txParams: {} } - const eventNames = ['update', 'updateBadge', '1:unapproved'] + const eventNames = ['updateBadge', '1:unapproved'] const listeners = [] eventNames.forEach((eventName) => { listeners.push(new Promise((resolve) => { From 9e0c0745ab0f2aa207b0c062ad11efd8df1bb6c5 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 12 Sep 2017 12:19:26 -0700 Subject: [PATCH 26/88] linting && format fixing --- app/scripts/controllers/transactions.js | 10 +++------- app/scripts/lib/pending-tx-tracker.js | 9 ++++----- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 3cb6a609e..3ff53e72b 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,5 +1,4 @@ const EventEmitter = require('events') -const extend = require('xtend') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const Transaction = require('ethereumjs-tx') @@ -60,17 +59,14 @@ module.exports = class TransactionController extends EventEmitter { this.pendingTxTracker = new PendingTransactionTracker({ provider: this.provider, nonceTracker: this.nonceTracker, + retryLimit: 3500, // Retry 3500 blocks, or about 1 day. getBalance: (address) => { const account = this.ethStore.getState().accounts[address] if (!account) return return account.balance }, - publishTransaction: this.query.sendRawTransaction, + publishTransaction: (rawTx) => this.query.sendRawTransaction(rawTx), getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), - giveUpOnTransaction: (txId) => { - const err = new Error(`Gave up submitting after 3500 blocks un-mined.`) - this.setTxStatusFailed(txId, err) - }, }) this.txStateManager.store.subscribe(() => this.emit('updateBadge')) @@ -193,7 +189,7 @@ module.exports = class TransactionController extends EventEmitter { // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) // add nonce to txParams - txMeta.txParams.nonce = nonceLock.nextNonce + txMeta.txParams.nonce = ethUtil.addHexPrefix(nonceLock.nextNonce.toString(16)) // add nonce debugging information to txMeta txMeta.nonceDetails = nonceLock.nonceDetails this.txStateManager.updateTx(txMeta) diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index b90851b58..cbc3f47e6 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -1,7 +1,6 @@ const EventEmitter = require('events') const EthQuery = require('ethjs-query') const sufficientBalance = require('./util').sufficientBalance -const RETRY_LIMIT = 3500 // Retry 3500 blocks, or about 1 day. /* Utility class for tracking the transactions as they @@ -25,11 +24,10 @@ module.exports = class PendingTransactionTracker extends EventEmitter { super() this.query = new EthQuery(config.provider) this.nonceTracker = config.nonceTracker - + this.retryLimit = config.retryLimit || Infinity this.getBalance = config.getBalance this.getPendingTransactions = config.getPendingTransactions this.publishTransaction = config.publishTransaction - this.giveUpOnTransaction = config.giveUpOnTransaction } // checks if a signed tx is in a block and @@ -102,8 +100,9 @@ module.exports = class PendingTransactionTracker extends EventEmitter { if (balance === undefined) return if (!('retryCount' in txMeta)) txMeta.retryCount = 0 - if (txMeta.retryCount > RETRY_LIMIT) { - return this.giveUpOnTransaction(txMeta.id) + if (txMeta.retryCount > this.retryLimit) { + const err = new Error(`Gave up submitting after ${this.retryLimit} blocks un-mined.`) + return this.emit('txFailed', txMeta.id, err) } // if the value of the transaction is greater then the balance, fail. From 53a467cd1e2ab50168b06d36a98effcfd3db3a49 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 12 Sep 2017 15:06:19 -0700 Subject: [PATCH 27/88] Some progress --- app/scripts/controllers/balance.js | 28 ++++++++ app/scripts/controllers/balances.js | 108 ++++++++++++++++++++++++++++ app/scripts/metamask-controller.js | 8 ++- test/unit/pending-balance-test.js | 1 + 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 app/scripts/controllers/balance.js create mode 100644 app/scripts/controllers/balances.js diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js new file mode 100644 index 000000000..5dfe266e3 --- /dev/null +++ b/app/scripts/controllers/balance.js @@ -0,0 +1,28 @@ +const ObservableStore = require('obs-store') +const normalizeAddress = require('eth-sig-util').normalize +const extend = require('xtend') +const PendingBalanceCalculator = require('../lib/pending-balance-calculator') + +class BalanceController { + + constructor (opts = {}) { + const { address, ethQuery, txController } = opts + this.ethQuery = ethQuery + this.txController = txController + + const initState = extend({ + ethBalance: undefined, + }, opts.initState) + this.store = new ObservableStore(initState) + + const { getBalance, getPendingTransactions } = opts + this.balanceCalc = new PendingBalanceCalculator({ + getBalance, + getPendingTransactions, + }) + this.updateBalance() + } + +} + +module.exports = BalanceController diff --git a/app/scripts/controllers/balances.js b/app/scripts/controllers/balances.js new file mode 100644 index 000000000..b0b366628 --- /dev/null +++ b/app/scripts/controllers/balances.js @@ -0,0 +1,108 @@ +const ObservableStore = require('obs-store') +const normalizeAddress = require('eth-sig-util').normalize +const extend = require('xtend') +const BalanceController = require('./balance') + +class BalancesController { + + constructor (opts = {}) { + const { ethStore, txController } = opts + this.ethStore = ethStore + this.txController = txController + + const initState = extend({ + balances: [], + }, opts.initState) + this.store = new ObservableStore(initState) + + this._initBalanceUpdating() + } + + // PUBLIC METHODS + + setSelectedAddress (_address) { + return new Promise((resolve, reject) => { + const address = normalizeAddress(_address) + this.store.updateState({ selectedAddress: address }) + resolve() + }) + } + + getSelectedAddress (_address) { + return this.store.getState().selectedAddress + } + + addToken (rawAddress, symbol, decimals) { + const address = normalizeAddress(rawAddress) + const newEntry = { address, symbol, decimals } + + const tokens = this.store.getState().tokens + const previousIndex = tokens.find((token, index) => { + return token.address === address + }) + + if (previousIndex) { + tokens[previousIndex] = newEntry + } else { + tokens.push(newEntry) + } + + this.store.updateState({ tokens }) + return Promise.resolve() + } + + getTokens () { + return this.store.getState().tokens + } + + updateFrequentRpcList (_url) { + return this.addToFrequentRpcList(_url) + .then((rpcList) => { + this.store.updateState({ frequentRpcList: rpcList }) + return Promise.resolve() + }) + } + + setCurrentAccountTab (currentAccountTab) { + return new Promise((resolve, reject) => { + this.store.updateState({ currentAccountTab }) + resolve() + }) + } + + addToFrequentRpcList (_url) { + const rpcList = this.getFrequentRpcList() + const index = rpcList.findIndex((element) => { return element === _url }) + if (index !== -1) { + rpcList.splice(index, 1) + } + if (_url !== 'http://localhost:8545') { + rpcList.push(_url) + } + if (rpcList.length > 2) { + rpcList.shift() + } + return Promise.resolve(rpcList) + } + + getFrequentRpcList () { + return this.store.getState().frequentRpcList + } + // + // PRIVATE METHODS + // + _initBalanceUpdating () { + const store = this.ethStore.getState() + const balances = store.accounts + + for (let address in balances) { + let updater = new BalancesController({ + address, + ethQuery: this.ethQuery, + txController: this.txController, + }) + } + } +} + +module.exports = BalancesController diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a007d6fc5..81e31a556 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -115,6 +115,12 @@ module.exports = class MetamaskController extends EventEmitter { }) this.txController.on('newUnaprovedTx', opts.showUnapprovedTx.bind(opts)) + // computed balances (accounting for pending transactions) + this.balancesController = new BalancesController({ + ethStore: this.ethStore, + txController: this.txController, + }) + // notices this.noticeController = new NoticeController({ initState: initState.NoticeController, @@ -647,4 +653,4 @@ module.exports = class MetamaskController extends EventEmitter { return Promise.resolve(rpcTarget) }) } -} \ No newline at end of file +} diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index 17be0306b..dde30fecc 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -96,3 +96,4 @@ function generateBalanceCalcWith (transactions, providerStub = zeroBn) { getPendingTransactions, }) } + From e4d7fb244790d547b03d18763aa1d8e501d88b89 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 13 Sep 2017 11:39:39 -0700 Subject: [PATCH 28/88] Add state-labeled events to allow subscribing to any transaction's state change --- app/scripts/controllers/transactions.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index fb3be6073..59a3f5329 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -434,6 +434,7 @@ module.exports = class TransactionController extends EventEmitter { const txMeta = this.getTx(txId) txMeta.status = status this.emit(`${txMeta.id}:${status}`, txId) + this.emit(`${status}`, txId) if (status === 'submitted' || status === 'rejected') { this.emit(`${txMeta.id}:finished`, txMeta) } From 59909601b887b2ed8473bea5a28b852668b2804e Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 13 Sep 2017 14:07:22 -0700 Subject: [PATCH 29/88] add test for pendingTxCount --- test/unit/tx-controller-test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 479010b87..47bfe66f8 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -59,6 +59,19 @@ describe('Transaction Controller', function () { }) }) + describe('#getPendingTxCount', function () { + it('should return the number of pending txs', function () { + txController.txStateManager._saveTxList([ + { id: 1, status: 'submitted', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 2, status: 'submitted', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 3, status: 'submitted', metamaskNetworkId: currentNetworkId, txParams: {} }, + ]) + const pendingTxCount = txController.getPendingTxCount() + assert.equal(pendingTxCount, 3, 'should be 3') + }) + }) + + describe('#newUnapprovedTransaction', function () { let stub, txMeta, txParams beforeEach(function () { From 77a48fb0b16d7415377b02a3b7d383c9ef86fcb6 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 13 Sep 2017 14:27:27 -0700 Subject: [PATCH 30/88] ensure that values written to txParams are hex strings --- app/scripts/controllers/transactions.js | 2 +- app/scripts/lib/tx-state-manager.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 3ff53e72b..3ee1c22aa 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -212,7 +212,7 @@ module.exports = class TransactionController extends EventEmitter { const txParams = txMeta.txParams const fromAddress = txParams.from // add network/chain id - txParams.chainId = this.getChainId() + txParams.chainId = ethUtil.addHexPrefix(this.getChainId().toString(16)) const ethTx = new Transaction(txParams) await this.signEthTx(ethTx, fromAddress) this.txStateManager.setTxStatusSigned(txMeta.id) diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index 843592504..82c0b6131 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -1,6 +1,7 @@ const extend = require('xtend') const EventEmitter = require('events') const ObservableStore = require('obs-store') +const ethUtil = require('ethereumjs-util') const txStateHistoryHelper = require('./tx-state-history-helper') module.exports = class TransactionStateManger extends EventEmitter { @@ -82,6 +83,14 @@ module.exports = class TransactionStateManger extends EventEmitter { } updateTx (txMeta) { + if (txMeta.txParams) { + Object.keys(txMeta.txParams).forEach((key) => { + let value = txMeta.txParams[key] + if (typeof value !== 'string') console.error(`${key}: ${value} in txParams is not a string`) + if (!ethUtil.isHexPrefixed(value)) console.error('is not hex prefixed, anything on txParams must be hex prefixed') + }) + } + // create txMeta snapshot for history const currentState = txStateHistoryHelper.snapshotFromTxMeta(txMeta) // recover previous tx state obj From 86cd4e4fedbea9639de33827733b4b85ef988bee Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 13 Sep 2017 14:20:19 -0700 Subject: [PATCH 31/88] Got pending balance updating correctly --- app/scripts/controllers/balance.js | 45 +++++++++++-- app/scripts/controllers/balances.js | 101 +++++++--------------------- app/scripts/metamask-controller.js | 4 ++ 3 files changed, 69 insertions(+), 81 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index 5dfe266e3..0d4ab7d4f 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -2,12 +2,14 @@ const ObservableStore = require('obs-store') const normalizeAddress = require('eth-sig-util').normalize const extend = require('xtend') const PendingBalanceCalculator = require('../lib/pending-balance-calculator') +const BN = require('ethereumjs-util').BN class BalanceController { constructor (opts = {}) { - const { address, ethQuery, txController } = opts - this.ethQuery = ethQuery + const { address, ethStore, txController } = opts + this.address = address + this.ethStore = ethStore this.txController = txController const initState = extend({ @@ -17,10 +19,43 @@ class BalanceController { const { getBalance, getPendingTransactions } = opts this.balanceCalc = new PendingBalanceCalculator({ - getBalance, - getPendingTransactions, + getBalance: () => Promise.resolve(this._getBalance()), + getPendingTransactions: this._getPendingTransactions.bind(this), }) - this.updateBalance() + + this.registerUpdates() + } + + async updateBalance () { + const balance = await this.balanceCalc.getBalance() + this.store.updateState({ + ethBalance: balance, + }) + } + + registerUpdates () { + const update = this.updateBalance.bind(this) + this.txController.on('submitted', update) + this.txController.on('confirmed', update) + this.txController.on('failed', update) + this.txController.blockTracker.on('block', update) + } + + _getBalance () { + const store = this.ethStore.getState() + const balances = store.accounts + const entry = balances[this.address] + const balance = entry.balance + return balance ? new BN(balance.substring(2), 16) : new BN(0) + } + + _getPendingTransactions () { + const pending = this.txController.getFilteredTxList({ + from: this.address, + status: 'submitted', + err: undefined, + }) + return Promise.resolve(pending) } } diff --git a/app/scripts/controllers/balances.js b/app/scripts/controllers/balances.js index b0b366628..cf3c8a757 100644 --- a/app/scripts/controllers/balances.js +++ b/app/scripts/controllers/balances.js @@ -11,97 +11,46 @@ class BalancesController { this.txController = txController const initState = extend({ - balances: [], + computedBalances: {}, }, opts.initState) this.store = new ObservableStore(initState) this._initBalanceUpdating() } - // PUBLIC METHODS - - setSelectedAddress (_address) { - return new Promise((resolve, reject) => { - const address = normalizeAddress(_address) - this.store.updateState({ selectedAddress: address }) - resolve() - }) - } - - getSelectedAddress (_address) { - return this.store.getState().selectedAddress + _initBalanceUpdating () { + const store = this.ethStore.getState() + this.addAnyAccountsFromStore(store) + this.ethStore.subscribe(this.addAnyAccountsFromStore.bind(this)) } - addToken (rawAddress, symbol, decimals) { - const address = normalizeAddress(rawAddress) - const newEntry = { address, symbol, decimals } - - const tokens = this.store.getState().tokens - const previousIndex = tokens.find((token, index) => { - return token.address === address - }) + addAnyAccountsFromStore(store) { + const balances = store.accounts - if (previousIndex) { - tokens[previousIndex] = newEntry - } else { - tokens.push(newEntry) + for (let address in balances) { + this.trackAddressIfNotAlready(address) } - - this.store.updateState({ tokens }) - return Promise.resolve() - } - - getTokens () { - return this.store.getState().tokens - } - - updateFrequentRpcList (_url) { - return this.addToFrequentRpcList(_url) - .then((rpcList) => { - this.store.updateState({ frequentRpcList: rpcList }) - return Promise.resolve() - }) } - setCurrentAccountTab (currentAccountTab) { - return new Promise((resolve, reject) => { - this.store.updateState({ currentAccountTab }) - resolve() - }) - } - - addToFrequentRpcList (_url) { - const rpcList = this.getFrequentRpcList() - const index = rpcList.findIndex((element) => { return element === _url }) - if (index !== -1) { - rpcList.splice(index, 1) - } - if (_url !== 'http://localhost:8545') { - rpcList.push(_url) + trackAddressIfNotAlready (address) { + const state = this.store.getState() + if (!(address in state.computedBalances)) { + this.trackAddress(address) } - if (rpcList.length > 2) { - rpcList.shift() - } - return Promise.resolve(rpcList) - } - - getFrequentRpcList () { - return this.store.getState().frequentRpcList } - // - // PRIVATE METHODS - // - _initBalanceUpdating () { - const store = this.ethStore.getState() - const balances = store.accounts - for (let address in balances) { - let updater = new BalancesController({ - address, - ethQuery: this.ethQuery, - txController: this.txController, - }) - } + trackAddress (address) { + let updater = new BalanceController({ + address, + ethStore: this.ethStore, + txController: this.txController, + }) + updater.store.subscribe((accountBalance) => { + let newState = this.store.getState() + newState.computedBalances[address] = accountBalance + this.store.updateState(newState) + }) + updater.updateBalance() } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 81e31a556..f2df45947 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -20,6 +20,7 @@ const BlacklistController = require('./controllers/blacklist') const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') const TransactionController = require('./controllers/transactions') +const BalancesController = require('./controllers/balances') const ConfigManager = require('./lib/config-manager') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') @@ -174,6 +175,7 @@ module.exports = class MetamaskController extends EventEmitter { this.networkController.store.subscribe(this.sendUpdate.bind(this)) this.ethStore.subscribe(this.sendUpdate.bind(this)) this.txController.memStore.subscribe(this.sendUpdate.bind(this)) + this.balancesController.store.subscribe(this.sendUpdate.bind(this)) this.messageManager.memStore.subscribe(this.sendUpdate.bind(this)) this.personalMessageManager.memStore.subscribe(this.sendUpdate.bind(this)) this.keyringController.memStore.subscribe(this.sendUpdate.bind(this)) @@ -248,6 +250,7 @@ module.exports = class MetamaskController extends EventEmitter { const wallet = this.configManager.getWallet() const vault = this.keyringController.store.getState().vault const isInitialized = (!!wallet || !!vault) + return extend( { isInitialized, @@ -258,6 +261,7 @@ module.exports = class MetamaskController extends EventEmitter { this.messageManager.memStore.getState(), this.personalMessageManager.memStore.getState(), this.keyringController.memStore.getState(), + this.balancesController.store.getState(), this.preferencesController.store.getState(), this.addressBookController.store.getState(), this.currencyController.store.getState(), From 0ba649317592efced339980a729213876bc25a81 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 13 Sep 2017 14:31:48 -0700 Subject: [PATCH 32/88] Use computed balance for tx confirmation --- ui/app/components/pending-tx.js | 6 +++--- ui/app/conf-tx.js | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index 3e53d47f9..1cc8daebe 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -33,7 +33,7 @@ function PendingTx () { PendingTx.prototype.render = function () { const props = this.props - const { currentCurrency, blockGasLimit } = props + const { currentCurrency, blockGasLimit, computedBalances } = props const conversionRate = props.conversionRate const txMeta = this.gatherTxMeta() @@ -42,8 +42,8 @@ PendingTx.prototype.render = function () { // Account Details const address = txParams.from || props.selectedAddress const identity = props.identities[address] || { address: address } - const account = props.accounts[address] - const balance = account ? account.balance : '0x0' + const account = computedBalances[address] + const balance = account ? account.ethBalance : '0x0' // recipient check const isValidAddress = !txParams.to || util.isValidAddress(txParams.to) diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index 1ee4166f7..8b93305eb 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -29,6 +29,7 @@ function mapStateToProps (state) { conversionRate: state.metamask.conversionRate, currentCurrency: state.metamask.currentCurrency, blockGasLimit: state.metamask.currentBlockGasLimit, + computedBalances: state.metamask.computedBalances, } } @@ -39,7 +40,7 @@ function ConfirmTxScreen () { ConfirmTxScreen.prototype.render = function () { const props = this.props - const { network, provider, unapprovedTxs, currentCurrency, + const { network, provider, unapprovedTxs, currentCurrency, computedBalances, unapprovedMsgs, unapprovedPersonalMsgs, conversionRate, blockGasLimit } = props var unconfTxList = txHelper(unapprovedTxs, unapprovedMsgs, unapprovedPersonalMsgs, network) @@ -104,6 +105,7 @@ ConfirmTxScreen.prototype.render = function () { currentCurrency, blockGasLimit, unconfTxListLength, + computedBalances, // Actions buyEth: this.buyEth.bind(this, txParams.from || props.selectedAddress), sendTransaction: this.sendTransaction.bind(this), From a01921758b25d151cfb1c47d7235f59291c29945 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 13 Sep 2017 15:02:05 -0700 Subject: [PATCH 33/88] Add computed balance to account detail view --- app/scripts/controllers/balance.js | 9 +++------ app/scripts/controllers/balances.js | 9 ++++++++- app/scripts/lib/pending-balance-calculator.js | 2 ++ app/scripts/metamask-controller.js | 4 ++++ ui/app/account-detail.js | 5 +++-- ui/app/conf-tx.js | 1 - 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index 0d4ab7d4f..b4e72e751 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -1,6 +1,4 @@ const ObservableStore = require('obs-store') -const normalizeAddress = require('eth-sig-util').normalize -const extend = require('xtend') const PendingBalanceCalculator = require('../lib/pending-balance-calculator') const BN = require('ethereumjs-util').BN @@ -12,12 +10,11 @@ class BalanceController { this.ethStore = ethStore this.txController = txController - const initState = extend({ + const initState = { ethBalance: undefined, - }, opts.initState) + } this.store = new ObservableStore(initState) - const { getBalance, getPendingTransactions } = opts this.balanceCalc = new PendingBalanceCalculator({ getBalance: () => Promise.resolve(this._getBalance()), getPendingTransactions: this._getPendingTransactions.bind(this), @@ -46,7 +43,7 @@ class BalanceController { const balances = store.accounts const entry = balances[this.address] const balance = entry.balance - return balance ? new BN(balance.substring(2), 16) : new BN(0) + return balance ? new BN(balance.substring(2), 16) : undefined } _getPendingTransactions () { diff --git a/app/scripts/controllers/balances.js b/app/scripts/controllers/balances.js index cf3c8a757..89c2ca95d 100644 --- a/app/scripts/controllers/balances.js +++ b/app/scripts/controllers/balances.js @@ -1,5 +1,4 @@ const ObservableStore = require('obs-store') -const normalizeAddress = require('eth-sig-util').normalize const extend = require('xtend') const BalanceController = require('./balance') @@ -14,10 +13,17 @@ class BalancesController { computedBalances: {}, }, opts.initState) this.store = new ObservableStore(initState) + this.balances = {} this._initBalanceUpdating() } + updateAllBalances () { + for (let address in this.balances) { + this.balances[address].updateBalance() + } + } + _initBalanceUpdating () { const store = this.ethStore.getState() this.addAnyAccountsFromStore(store) @@ -50,6 +56,7 @@ class BalancesController { newState.computedBalances[address] = accountBalance this.store.updateState(newState) }) + this.balances[address] = updater updater.updateBalance() } } diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index 474ed3261..c66bffbbb 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -22,6 +22,8 @@ class PendingBalanceCalculator { const balance = results[0] const pending = results[1] + if (!balance) return undefined + const pendingValue = pending.reduce((total, tx) => { return total.add(this.valueFor(tx)) }, new BN(0)) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index f2df45947..02c06ead2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -121,6 +121,10 @@ module.exports = class MetamaskController extends EventEmitter { ethStore: this.ethStore, txController: this.txController, }) + this.networkController.on('networkDidChange', () => { + this.balancesController.updateAllBalances() + }) + this.balancesController.updateAllBalances() // notices this.noticeController = new NoticeController({ diff --git a/ui/app/account-detail.js b/ui/app/account-detail.js index 02089ecd0..90724dc3f 100644 --- a/ui/app/account-detail.js +++ b/ui/app/account-detail.js @@ -32,6 +32,7 @@ function mapStateToProps (state) { currentCurrency: state.metamask.currentCurrency, currentAccountTab: state.metamask.currentAccountTab, tokens: state.metamask.tokens, + computedBalances: state.metamask.computedBalances, } } @@ -45,7 +46,7 @@ AccountDetailScreen.prototype.render = function () { var selected = props.address || Object.keys(props.accounts)[0] var checksumAddress = selected && ethUtil.toChecksumAddress(selected) var identity = props.identities[selected] - var account = props.accounts[selected] + var account = props.computedBalances[selected] const { network, conversionRate, currentCurrency } = props return ( @@ -180,7 +181,7 @@ AccountDetailScreen.prototype.render = function () { }, [ h(EthBalance, { - value: account && account.balance, + value: account && account.ethBalance, conversionRate, currentCurrency, style: { diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index 8b93305eb..15fb9a59f 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -49,7 +49,6 @@ ConfirmTxScreen.prototype.render = function () { var txParams = txData.params || {} var isNotification = isPopupOrNotification() === 'notification' - log.info(`rendering a combined ${unconfTxList.length} unconf msg & txs`) if (unconfTxList.length === 0) return h(Loading, { isLoading: true }) From c9585f166f3aebea29e7214026a942eb18e4884a Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 13 Sep 2017 15:21:18 -0700 Subject: [PATCH 34/88] Fix test --- test/unit/components/pending-tx-test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/unit/components/pending-tx-test.js b/test/unit/components/pending-tx-test.js index 22a98bc93..fdade1042 100644 --- a/test/unit/components/pending-tx-test.js +++ b/test/unit/components/pending-tx-test.js @@ -34,10 +34,15 @@ describe('PendingTx', function () { const renderer = ReactTestUtils.createRenderer() const newGasPrice = '0x77359400' + const computedBalances = {} + computedBalances[Object.keys(identities)[0]] = { + ethBalance: '0x00000000000000056bc75e2d63100000', + } const props = { identities, accounts: identities, txData, + computedBalances, sendTransaction: (txMeta, event) => { // Assert changes: const result = ethUtil.addHexPrefix(txMeta.txParams.gasPrice) From 0a5ae395099f9cadef5e446a0d591d00be4685e5 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 21 Sep 2017 17:37:30 -0700 Subject: [PATCH 35/88] bug - fix event emitter mem leak warning --- app/scripts/contentscript.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 90a0f1f22..b4708189e 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -42,16 +42,21 @@ function setupStreams () { name: 'contentscript', target: 'inpage', }) - pageStream.on('error', console.error) const pluginPort = extension.runtime.connect({ name: 'contentscript' }) const pluginStream = new PortStream(pluginPort) - pluginStream.on('error', console.error) // forward communication plugin->inpage - pageStream.pipe(pluginStream).pipe(pageStream) + pump( + pageStream, + pluginStream, + pageStream, + (err) => logStreamDisconnectWarning('MetaMask Contentscript Forwarding', err) + ) // setup local multistream channels const mux = new ObjectMultiplex() + mux.setMaxListeners(25) + pump( mux, pageStream, From 08b36b9b582853b411a19e48b407c70596381f61 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 13:29:13 -0700 Subject: [PATCH 36/88] Allow metamaskController to define keyring types --- app/scripts/keyring-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index adfa4a813..4454f0b63 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -27,7 +27,7 @@ class KeyringController extends EventEmitter { constructor (opts) { super() const initState = opts.initState || {} - this.keyringTypes = keyringTypes + this.keyringTypes = opts.keyringTypes || keyringTypes this.store = new ObservableStore(initState) this.memStore = new ObservableStore({ isUnlocked: false, From 977405fc7d89256a911e73b83a6678235fa1cfb8 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 13:33:53 -0700 Subject: [PATCH 37/88] Remove dead code from eth-store --- app/scripts/lib/eth-store.js | 44 +----------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/app/scripts/lib/eth-store.js b/app/scripts/lib/eth-store.js index ebba98f5c..ff22eca4a 100644 --- a/app/scripts/lib/eth-store.js +++ b/app/scripts/lib/eth-store.js @@ -18,10 +18,6 @@ class EthereumStore extends ObservableStore { constructor (opts = {}) { super({ accounts: {}, - transactions: {}, - currentBlockNumber: '0', - currentBlockHash: '', - currentBlockGasLimit: '', }) this._provider = opts.provider this._query = new EthQuery(this._provider) @@ -50,21 +46,6 @@ class EthereumStore extends ObservableStore { this.updateState({ accounts }) } - addTransaction (txHash) { - const transactions = this.getState().transactions - transactions[txHash] = {} - this.updateState({ transactions }) - if (!this._currentBlockNumber) return - this._updateTransaction(this._currentBlockNumber, txHash, noop) - } - - removeTransaction (txHash) { - const transactions = this.getState().transactions - delete transactions[txHash] - this.updateState({ transactions }) - } - - // // private // @@ -72,12 +53,9 @@ class EthereumStore extends ObservableStore { _updateForBlock (block) { const blockNumber = '0x' + block.number.toString('hex') this._currentBlockNumber = blockNumber - this.updateState({ currentBlockNumber: parseInt(blockNumber) }) - this.updateState({ currentBlockHash: `0x${block.hash.toString('hex')}`}) - this.updateState({ currentBlockGasLimit: `0x${block.gasLimit.toString('hex')}` }) + async.parallel([ this._updateAccounts.bind(this), - this._updateTransactions.bind(this, blockNumber), ], (err) => { if (err) return console.error(err) this.emit('block', this.getState()) @@ -104,26 +82,6 @@ class EthereumStore extends ObservableStore { }) } - _updateTransactions (block, cb = noop) { - const transactions = this.getState().transactions - const txHashes = Object.keys(transactions) - async.each(txHashes, this._updateTransaction.bind(this, block), cb) - } - - _updateTransaction (block, txHash, cb = noop) { - // would use the block here to determine how many confirmations the tx has - const transactions = this.getState().transactions - this._query.getTransaction(txHash, (err, result) => { - if (err) return cb(err) - // only populate if the entry is still present - if (transactions[txHash]) { - transactions[txHash] = result - this.updateState({ transactions }) - } - cb(null, result) - }) - } - _getAccount (address, cb = noop) { const query = this._query async.parallel({ From 2ca2df183265edb048ee1f0e6bd2437cab7c4efe Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 22 Sep 2017 13:58:46 -0700 Subject: [PATCH 38/88] deps - bump eth-block-tracker --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 0d02f1df5..a9268060d 100644 --- a/package.json +++ b/package.json @@ -69,6 +69,7 @@ "end-of-stream": "^1.1.0", "ensnare": "^1.0.0", "eth-bin-to-ops": "^1.0.1", + "eth-block-tracker": "^2.2.0", "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", "eth-json-rpc-filters": "^1.1.0", From 11c8c07bfc6677e347873f02ae8c401f8d6c4dcf Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 13:59:25 -0700 Subject: [PATCH 39/88] Refactor eth-store into account-tracker EthStore was only being used for tracking account balances and nonces now, so I removed its block-tracking duties, renamed it account-tracker, and removed it as a dependency from `KeyringController`, so that KRC can go live on without a hard dep on it. --- .../{balances.js => computed-balances.js} | 4 +-- app/scripts/controllers/transactions.js | 6 ++--- .../lib/{eth-store.js => account-tracker.js} | 2 +- app/scripts/metamask-controller.js | 26 ++++++++++++------- 4 files changed, 22 insertions(+), 16 deletions(-) rename app/scripts/controllers/{balances.js => computed-balances.js} (95%) rename app/scripts/lib/{eth-store.js => account-tracker.js} (99%) diff --git a/app/scripts/controllers/balances.js b/app/scripts/controllers/computed-balances.js similarity index 95% rename from app/scripts/controllers/balances.js rename to app/scripts/controllers/computed-balances.js index 89c2ca95d..a85eb5590 100644 --- a/app/scripts/controllers/balances.js +++ b/app/scripts/controllers/computed-balances.js @@ -2,7 +2,7 @@ const ObservableStore = require('obs-store') const extend = require('xtend') const BalanceController = require('./balance') -class BalancesController { +class ComputedbalancesController { constructor (opts = {}) { const { ethStore, txController } = opts @@ -61,4 +61,4 @@ class BalancesController { } } -module.exports = BalancesController +module.exports = ComputedbalancesController diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 59a3f5329..2aff4e5ff 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -22,7 +22,7 @@ module.exports = class TransactionController extends EventEmitter { this.provider = opts.provider this.blockTracker = opts.blockTracker this.signEthTx = opts.signTransaction - this.ethStore = opts.ethStore + this.accountTracker = opts.accountTracker this.nonceTracker = new NonceTracker({ provider: this.provider, @@ -52,7 +52,7 @@ module.exports = class TransactionController extends EventEmitter { provider: this.provider, nonceTracker: this.nonceTracker, getBalance: (address) => { - const account = this.ethStore.getState().accounts[address] + const account = this.accountTracker.getState().accounts[address] if (!account) return return account.balance }, @@ -73,7 +73,7 @@ module.exports = class TransactionController extends EventEmitter { this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition - // where ethStore hasent been populated by the results yet + // where accountTracker hasent been populated by the results yet this.blockTracker.once('latest', () => { this.blockTracker.on('latest', this.pendingTxTracker.resubmitPendingTxs.bind(this.pendingTxTracker)) }) diff --git a/app/scripts/lib/eth-store.js b/app/scripts/lib/account-tracker.js similarity index 99% rename from app/scripts/lib/eth-store.js rename to app/scripts/lib/account-tracker.js index ff22eca4a..bf949597b 100644 --- a/app/scripts/lib/eth-store.js +++ b/app/scripts/lib/account-tracker.js @@ -1,4 +1,4 @@ -/* Ethereum Store +/* Account Tracker * * This module is responsible for tracking any number of accounts * and caching their current balances & transaction counts. diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 02c06ead2..b1cfe1a2d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -4,7 +4,7 @@ const promiseToCallback = require('promise-to-callback') const pipe = require('pump') const Dnode = require('dnode') const ObservableStore = require('obs-store') -const EthStore = require('./lib/eth-store') +const AccountTracker = require('./lib/account-tracker') const EthQuery = require('eth-query') const streamIntoProvider = require('web3-stream-provider/handler') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex @@ -81,19 +81,25 @@ module.exports = class MetamaskController extends EventEmitter { // eth data query tools this.ethQuery = new EthQuery(this.provider) - this.ethStore = new EthStore({ - provider: this.provider, - blockTracker: this.provider, - }) // key mgmt this.keyringController = new KeyringController({ initState: initState.KeyringController, - ethStore: this.ethStore, + accountTracker: this.accountTracker, getNetwork: this.networkController.getNetworkState.bind(this.networkController), }) + + // account tracker watches balances, nonces, and any code at their address. + this.accountTracker = new AccountTracker({ + provider: this.provider, + blockTracker: this.provider, + }) this.keyringController.on('newAccount', (address) => { this.preferencesController.setSelectedAddress(address) + this.accountTracker.addAccount(address) + }) + this.keyringController.on('removedAccount', (address) => { + this.accountTracker.removeAccount(address) }) // address book controller @@ -112,13 +118,13 @@ module.exports = class MetamaskController extends EventEmitter { provider: this.provider, blockTracker: this.provider, ethQuery: this.ethQuery, - ethStore: this.ethStore, + accountTracker: this.accountTracker, }) this.txController.on('newUnaprovedTx', opts.showUnapprovedTx.bind(opts)) // computed balances (accounting for pending transactions) this.balancesController = new BalancesController({ - ethStore: this.ethStore, + accountTracker: this.accountTracker, txController: this.txController, }) this.networkController.on('networkDidChange', () => { @@ -177,7 +183,7 @@ module.exports = class MetamaskController extends EventEmitter { // manual mem state subscriptions this.networkController.store.subscribe(this.sendUpdate.bind(this)) - this.ethStore.subscribe(this.sendUpdate.bind(this)) + this.accountTracker.subscribe(this.sendUpdate.bind(this)) this.txController.memStore.subscribe(this.sendUpdate.bind(this)) this.balancesController.store.subscribe(this.sendUpdate.bind(this)) this.messageManager.memStore.subscribe(this.sendUpdate.bind(this)) @@ -260,7 +266,7 @@ module.exports = class MetamaskController extends EventEmitter { isInitialized, }, this.networkController.store.getState(), - this.ethStore.getState(), + this.accountTracker.getState(), this.txController.memStore.getState(), this.messageManager.memStore.getState(), this.personalMessageManager.memStore.getState(), From d2a747e57e591af5beb2e7cef7fc73a363d9c742 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 14:06:54 -0700 Subject: [PATCH 40/88] Fix computed-balances controller reference --- app/scripts/metamask-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b1cfe1a2d..34d60d253 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -20,7 +20,7 @@ const BlacklistController = require('./controllers/blacklist') const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') const TransactionController = require('./controllers/transactions') -const BalancesController = require('./controllers/balances') +const BalancesController = require('./controllers/computed-balances') const ConfigManager = require('./lib/config-manager') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') From f01b0a818ba67c2549f14382056534768a255e5b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 14:13:56 -0700 Subject: [PATCH 41/88] Fix account-tracker references --- app/scripts/controllers/balance.js | 6 +++--- app/scripts/controllers/computed-balances.js | 10 +++++----- app/scripts/keyring-controller.js | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index b4e72e751..ddeb06cf9 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -5,9 +5,9 @@ const BN = require('ethereumjs-util').BN class BalanceController { constructor (opts = {}) { - const { address, ethStore, txController } = opts + const { address, accountTracker, txController } = opts this.address = address - this.ethStore = ethStore + this.accountTracker = accountTracker this.txController = txController const initState = { @@ -39,7 +39,7 @@ class BalanceController { } _getBalance () { - const store = this.ethStore.getState() + const store = this.accountTracker.getState() const balances = store.accounts const entry = balances[this.address] const balance = entry.balance diff --git a/app/scripts/controllers/computed-balances.js b/app/scripts/controllers/computed-balances.js index a85eb5590..576746164 100644 --- a/app/scripts/controllers/computed-balances.js +++ b/app/scripts/controllers/computed-balances.js @@ -5,8 +5,8 @@ const BalanceController = require('./balance') class ComputedbalancesController { constructor (opts = {}) { - const { ethStore, txController } = opts - this.ethStore = ethStore + const { accountTracker, txController } = opts + this.accountTracker = accountTracker this.txController = txController const initState = extend({ @@ -25,9 +25,9 @@ class ComputedbalancesController { } _initBalanceUpdating () { - const store = this.ethStore.getState() + const store = this.accountTracker.getState() this.addAnyAccountsFromStore(store) - this.ethStore.subscribe(this.addAnyAccountsFromStore.bind(this)) + this.accountTracker.subscribe(this.addAnyAccountsFromStore.bind(this)) } addAnyAccountsFromStore(store) { @@ -48,7 +48,7 @@ class ComputedbalancesController { trackAddress (address) { let updater = new BalanceController({ address, - ethStore: this.ethStore, + accountTracker: this.accountTracker, txController: this.txController, }) updater.store.subscribe((accountBalance) => { diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index fd57fac70..fa470dd89 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -35,7 +35,7 @@ class KeyringController extends EventEmitter { keyrings: [], identities: {}, }) - this.ethStore = opts.ethStore + this.accountTracker = opts.accountTracker this.encryptor = encryptor this.keyrings = [] this.getNetwork = opts.getNetwork @@ -338,7 +338,7 @@ class KeyringController extends EventEmitter { // // Initializes the provided account array // Gives them numerically incremented nicknames, - // and adds them to the ethStore for regular balance checking. + // and adds them to the accountTracker for regular balance checking. setupAccounts (accounts) { return this.getAccounts() .then((loadedAccounts) => { @@ -361,7 +361,7 @@ class KeyringController extends EventEmitter { throw new Error('Problem loading account.') } const address = normalizeAddress(account) - this.ethStore.addAccount(address) + this.accountTracker.addAccount(address) return this.createNickname(address) } @@ -567,12 +567,12 @@ class KeyringController extends EventEmitter { clearKeyrings () { let accounts try { - accounts = Object.keys(this.ethStore.getState()) + accounts = Object.keys(this.accountTracker.getState()) } catch (e) { accounts = [] } accounts.forEach((address) => { - this.ethStore.removeAccount(address) + this.accountTracker.removeAccount(address) }) // clear keyrings from memory From 128cf40f91df6d78e2d5ca87608fe16e91a510fa Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 14:16:19 -0700 Subject: [PATCH 42/88] Fix accont-tracker merge bug --- app/scripts/metamask-controller.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 2fc5b4204..1dd09d0ad 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -86,6 +86,10 @@ module.exports = class MetamaskController extends EventEmitter { // eth data query tools this.ethQuery = new EthQuery(this.provider) + this.accountTracker = new AccountTracker({ + provider: this.provider, + blockTracker: this.blockTracker, + }) // key mgmt this.keyringController = new KeyringController({ From f128240e7f877280fa59bf22f2ea8285bb467022 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 14:19:14 -0700 Subject: [PATCH 43/88] Fix test references --- test/unit/keyring-controller-test.js | 2 +- test/unit/tx-controller-test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/keyring-controller-test.js b/test/unit/keyring-controller-test.js index 2d9a53723..34c314639 100644 --- a/test/unit/keyring-controller-test.js +++ b/test/unit/keyring-controller-test.js @@ -24,7 +24,7 @@ describe('KeyringController', function () { getTxList: () => [], getUnapprovedTxList: () => [], }, - ethStore: { + accountTracker: { addAccount (acct) { accounts.push(ethUtil.addHexPrefix(acct)) }, }, }) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 7bb193242..7b875db66 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -27,7 +27,7 @@ describe('Transaction Controller', function () { networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, blockTracker: { getCurrentBlock: noop, on: noop, once: noop }, - ethStore: { getState: noop }, + accountTracker: { getState: noop }, signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) resolve() @@ -431,4 +431,4 @@ describe('Transaction Controller', function () { }).catch(done) }) }) -}) \ No newline at end of file +}) From 15195bca75f84859b4d4efce9b8155504750c99d Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 22 Sep 2017 14:20:44 -0700 Subject: [PATCH 44/88] deps - bump provider engine for block tracker --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a9268060d..38ed28eef 100644 --- a/package.json +++ b/package.json @@ -139,7 +139,7 @@ "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "^0.20.1", - "web3-provider-engine": "^13.2.9", + "web3-provider-engine": "^13.2.10", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" }, From dd45592641500504d0d804316e6625e5b89718f9 Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 22 Sep 2017 14:22:07 -0700 Subject: [PATCH 45/88] metamask - use provider-engines block tracker --- app/scripts/metamask-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index fef16c3a9..53fb27476 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -81,7 +81,7 @@ module.exports = class MetamaskController extends EventEmitter { // rpc provider this.provider = this.initializeProvider() - this.blockTracker = this.provider + this.blockTracker = this.provider._blockTracker // eth data query tools this.ethQuery = new EthQuery(this.provider) From 443b1a8eb7883b6799692ddc24d0555d49bd1787 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 22 Sep 2017 14:38:40 -0700 Subject: [PATCH 46/88] Remove keyring controller from project --- app/scripts/keyring-controller.js | 594 ----------------------- app/scripts/metamask-controller.js | 2 +- app/scripts/migrations/_multi-keyring.js | 2 +- package.json | 5 +- test/unit/keyring-controller-test.js | 164 ------- 5 files changed, 3 insertions(+), 764 deletions(-) delete mode 100644 app/scripts/keyring-controller.js delete mode 100644 test/unit/keyring-controller-test.js diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js deleted file mode 100644 index fb60a5b3e..000000000 --- a/app/scripts/keyring-controller.js +++ /dev/null @@ -1,594 +0,0 @@ -const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN -const bip39 = require('bip39') -const EventEmitter = require('events').EventEmitter -const ObservableStore = require('obs-store') -const filter = require('promise-filter') -const encryptor = require('browser-passworder') -const sigUtil = require('eth-sig-util') -const normalizeAddress = sigUtil.normalize -// Keyrings: -const SimpleKeyring = require('eth-simple-keyring') -const HdKeyring = require('eth-hd-keyring') -const keyringTypes = [ - SimpleKeyring, - HdKeyring, -] - -class KeyringController extends EventEmitter { - - // PUBLIC METHODS - // - // THE FIRST SECTION OF METHODS ARE PUBLIC-FACING, - // MEANING THEY ARE USED BY CONSUMERS OF THIS CLASS. - // - // THEIR SURFACE AREA SHOULD BE CHANGED WITH GREAT CARE. - - constructor (opts) { - super() - const initState = opts.initState || {} - this.keyringTypes = opts.keyringTypes || keyringTypes - this.store = new ObservableStore(initState) - this.memStore = new ObservableStore({ - isUnlocked: false, - keyringTypes: this.keyringTypes.map(krt => krt.type), - keyrings: [], - identities: {}, - }) - this.encryptor = opts.encryptor || encryptor - this.keyrings = [] - this.getNetwork = opts.getNetwork - } - - // Full Update - // returns Promise( @object state ) - // - // Emits the `update` event and - // returns a Promise that resolves to the current state. - // - // Frequently used to end asynchronous chains in this class, - // indicating consumers can often either listen for updates, - // or accept a state-resolving promise to consume their results. - // - // Not all methods end with this, that might be a nice refactor. - fullUpdate () { - this.emit('update') - return Promise.resolve(this.memStore.getState()) - } - - // Create New Vault And Keychain - // @string password - The password to encrypt the vault with - // - // returns Promise( @object state ) - // - // Destroys any old encrypted storage, - // creates a new encrypted store with the given password, - // randomly creates a new HD wallet with 1 account, - // faucets that account on the testnet. - createNewVaultAndKeychain (password) { - return this.persistAllKeyrings(password) - .then(this.createFirstKeyTree.bind(this)) - .then(this.fullUpdate.bind(this)) - } - - // CreateNewVaultAndRestore - // @string password - The password to encrypt the vault with - // @string seed - The BIP44-compliant seed phrase. - // - // returns Promise( @object state ) - // - // Destroys any old encrypted storage, - // creates a new encrypted store with the given password, - // creates a new HD wallet from the given seed with 1 account. - createNewVaultAndRestore (password, seed) { - if (typeof password !== 'string') { - return Promise.reject('Password must be text.') - } - - if (!bip39.validateMnemonic(seed)) { - return Promise.reject(new Error('Seed phrase is invalid.')) - } - - this.clearKeyrings() - - return this.persistAllKeyrings(password) - .then(() => { - return this.addNewKeyring('HD Key Tree', { - mnemonic: seed, - numberOfAccounts: 1, - }) - }) - .then((firstKeyring) => { - return firstKeyring.getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] - if (!firstAccount) throw new Error('KeyringController - First Account not found.') - const hexAccount = normalizeAddress(firstAccount) - this.emit('newAccount', hexAccount) - return this.setupAccounts(accounts) - }) - .then(this.persistAllKeyrings.bind(this, password)) - .then(this.fullUpdate.bind(this)) - } - - // Set Locked - // returns Promise( @object state ) - // - // This method deallocates all secrets, and effectively locks metamask. - setLocked () { - // set locked - this.password = null - this.memStore.updateState({ isUnlocked: false }) - // remove keyrings - this.keyrings = [] - this._updateMemStoreKeyrings() - return this.fullUpdate() - } - - // Submit Password - // @string password - // - // returns Promise( @object state ) - // - // Attempts to decrypt the current vault and load its keyrings - // into memory. - // - // Temporarily also migrates any old-style vaults first, as well. - // (Pre MetaMask 3.0.0) - submitPassword (password) { - return this.unlockKeyrings(password) - .then((keyrings) => { - this.keyrings = keyrings - return this.fullUpdate() - }) - } - - // Add New Keyring - // @string type - // @object opts - // - // returns Promise( @Keyring keyring ) - // - // Adds a new Keyring of the given `type` to the vault - // and the current decrypted Keyrings array. - // - // All Keyring classes implement a unique `type` string, - // and this is used to retrieve them from the keyringTypes array. - addNewKeyring (type, opts) { - const Keyring = this.getKeyringClassForType(type) - const keyring = new Keyring(opts) - return keyring.deserialize(opts) - .then(() => { - return keyring.getAccounts() - }) - .then((accounts) => { - return this.checkForDuplicate(type, accounts) - }) - .then((checkedAccounts) => { - this.keyrings.push(keyring) - return this.setupAccounts(checkedAccounts) - }) - .then(() => this.persistAllKeyrings()) - .then(() => this._updateMemStoreKeyrings()) - .then(() => this.fullUpdate()) - .then(() => { - return keyring - }) - } - - // For now just checks for simple key pairs - // but in the future - // should possibly add HD and other types - // - checkForDuplicate (type, newAccount) { - return this.getAccounts() - .then((accounts) => { - switch (type) { - case 'Simple Key Pair': - const isNotIncluded = !accounts.find((key) => key === newAccount[0] || key === ethUtil.stripHexPrefix(newAccount[0])) - return (isNotIncluded) ? Promise.resolve(newAccount) : Promise.reject(new Error('The account you\'re are trying to import is a duplicate')) - default: - return Promise.resolve(newAccount) - } - }) - } - - - // Add New Account - // @number keyRingNum - // - // returns Promise( @object state ) - // - // Calls the `addAccounts` method on the Keyring - // in the kryings array at index `keyringNum`, - // and then saves those changes. - addNewAccount (selectedKeyring) { - return selectedKeyring.addAccounts(1) - .then(this.setupAccounts.bind(this)) - .then(this.persistAllKeyrings.bind(this)) - .then(this._updateMemStoreKeyrings.bind(this)) - .then(this.fullUpdate.bind(this)) - } - - // Save Account Label - // @string account - // @string label - // - // returns Promise( @string label ) - // - // Persists a nickname equal to `label` for the specified account. - saveAccountLabel (account, label) { - try { - const hexAddress = normalizeAddress(account) - // update state on diskStore - const state = this.store.getState() - const walletNicknames = state.walletNicknames || {} - walletNicknames[hexAddress] = label - this.store.updateState({ walletNicknames }) - // update state on memStore - const identities = this.memStore.getState().identities - identities[hexAddress].name = label - this.memStore.updateState({ identities }) - return Promise.resolve(label) - } catch (err) { - return Promise.reject(err) - } - } - - // Export Account - // @string address - // - // returns Promise( @string privateKey ) - // - // Requests the private key from the keyring controlling - // the specified address. - // - // Returns a Promise that may resolve with the private key string. - exportAccount (address) { - try { - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.exportAccount(normalizeAddress(address)) - }) - } catch (e) { - return Promise.reject(e) - } - } - - - // SIGNING METHODS - // - // This method signs tx and returns a promise for - // TX Manager to update the state after signing - - signTransaction (ethTx, _fromAddress) { - const fromAddress = normalizeAddress(_fromAddress) - return this.getKeyringForAccount(fromAddress) - .then((keyring) => { - return keyring.signTransaction(fromAddress, ethTx) - }) - } - - // Sign Message - // @object msgParams - // - // returns Promise(@buffer rawSig) - // - // Attempts to sign the provided @object msgParams. - signMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signMessage(address, msgParams.data) - }) - } - - // Sign Personal Message - // @object msgParams - // - // returns Promise(@buffer rawSig) - // - // Attempts to sign the provided @object msgParams. - // Prefixes the hash before signing as per the new geth behavior. - signPersonalMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signPersonalMessage(address, msgParams.data) - }) - } - - // PRIVATE METHODS - // - // THESE METHODS ARE ONLY USED INTERNALLY TO THE KEYRING-CONTROLLER - // AND SO MAY BE CHANGED MORE LIBERALLY THAN THE ABOVE METHODS. - - // Create First Key Tree - // returns @Promise - // - // Clears the vault, - // creates a new one, - // creates a random new HD Keyring with 1 account, - // makes that account the selected account, - // faucets that account on testnet, - // puts the current seed words into the state tree. - createFirstKeyTree () { - this.clearKeyrings() - return this.addNewKeyring('HD Key Tree', { numberOfAccounts: 1 }) - .then((keyring) => { - return keyring.getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] - if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') - const hexAccount = normalizeAddress(firstAccount) - this.emit('newAccount', hexAccount) - this.emit('newVault', hexAccount) - return this.setupAccounts(accounts) - }) - .then(this.persistAllKeyrings.bind(this)) - } - - // Setup Accounts - // @array accounts - // - // returns @Promise(@object account) - // - // Initializes the provided account array - // Gives them numerically incremented nicknames, - // and adds them to the accountTracker for regular balance checking. - setupAccounts (accounts) { - return this.getAccounts() - .then((loadedAccounts) => { - const arr = accounts || loadedAccounts - return Promise.all(arr.map((account) => { - return this.getBalanceAndNickname(account) - })) - }) - } - - // Get Balance And Nickname - // @string account - // - // returns Promise( @string label ) - // - // 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 = normalizeAddress(account) - this.accountTracker.addAccount(address) - return this.createNickname(address) - } - - // Create Nickname - // @string address - // - // returns Promise( @string label ) - // - // Takes an address, and assigns it an incremented nickname, persisting it. - createNickname (address) { - const hexAddress = normalizeAddress(address) - const identities = this.memStore.getState().identities - const currentIdentityCount = Object.keys(identities).length + 1 - const nicknames = this.store.getState().walletNicknames || {} - const existingNickname = nicknames[hexAddress] - const name = existingNickname || `Account ${currentIdentityCount}` - identities[hexAddress] = { - address: hexAddress, - name, - } - this.memStore.updateState({ identities }) - return this.saveAccountLabel(hexAddress, name) - } - - // Persist All Keyrings - // @password string - // - // returns Promise - // - // Iterates the current `keyrings` array, - // serializes each one into a serialized array, - // encrypts that array with the provided `password`, - // and persists that encrypted string to storage. - persistAllKeyrings (password = this.password) { - if (typeof password === 'string') { - this.password = password - this.memStore.updateState({ isUnlocked: true }) - } - return 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) - }) - .then((encryptedString) => { - this.store.updateState({ vault: encryptedString }) - return true - }) - } - - // Unlock Keyrings - // @string password - // - // returns Promise( @array keyrings ) - // - // Attempts to unlock the persisted encrypted storage, - // initializing the persisted keyrings to RAM. - unlockKeyrings (password) { - const encryptedVault = this.store.getState().vault - if (!encryptedVault) { - throw new Error('Cannot unlock without a previous vault.') - } - - return this.encryptor.decrypt(password, encryptedVault) - .then((vault) => { - this.password = password - this.memStore.updateState({ isUnlocked: true }) - vault.forEach(this.restoreKeyring.bind(this)) - return this.keyrings - }) - } - - // Restore Keyring - // @object serialized - // - // returns Promise( @Keyring deserialized ) - // - // Attempts to initialize a new keyring from the provided - // serialized payload. - // - // On success, returns the resulting @Keyring instance. - restoreKeyring (serialized) { - const { type, data } = serialized - - const Keyring = this.getKeyringClassForType(type) - const keyring = new Keyring() - return keyring.deserialize(data) - .then(() => { - return keyring.getAccounts() - }) - .then((accounts) => { - return this.setupAccounts(accounts) - }) - .then(() => { - this.keyrings.push(keyring) - this._updateMemStoreKeyrings() - return keyring - }) - } - - // Get Keyring Class For Type - // @string type - // - // Returns @class Keyring - // - // Searches the current `keyringTypes` array - // for a Keyring class whose unique `type` property - // matches the provided `type`, - // returning it if it exists. - getKeyringClassForType (type) { - return this.keyringTypes.find(kr => kr.type === type) - } - - getKeyringsByType (type) { - return this.keyrings.filter((keyring) => keyring.type === type) - } - - // Get Accounts - // returns Promise( @Array[ @string accounts ] ) - // - // Returns the public addresses of all current accounts - // managed by all currently unlocked keyrings. - getAccounts () { - const keyrings = this.keyrings || [] - return Promise.all(keyrings.map(kr => kr.getAccounts())) - .then((keyringArrays) => { - return keyringArrays.reduce((res, arr) => { - return res.concat(arr) - }, []) - }) - } - - // Get Keyring For Account - // @string address - // - // returns Promise(@Keyring keyring) - // - // Returns the currently initialized keyring that manages - // the specified `address` if one exists. - getKeyringForAccount (address) { - const hexed = normalizeAddress(address) - log.debug(`KeyringController - getKeyringForAccount: ${hexed}`) - - return Promise.all(this.keyrings.map((keyring) => { - return Promise.all([ - keyring, - keyring.getAccounts(), - ]) - })) - .then(filter((candidate) => { - const accounts = candidate[1].map(normalizeAddress) - return accounts.includes(hexed) - })) - .then((winners) => { - if (winners && winners.length > 0) { - return winners[0][0] - } else { - throw new Error('No keyring found for the requested account.') - } - }) - } - - // Display For Keyring - // @Keyring keyring - // - // returns Promise( @Object { type:String, accounts:Array } ) - // - // Is used for adding the current keyrings to the state object. - displayForKeyring (keyring) { - return keyring.getAccounts() - .then((accounts) => { - return { - type: keyring.type, - accounts: accounts, - } - }) - } - - // Add Gas Buffer - // @string gas (as hexadecimal value) - // - // returns @string bufferedGas (as hexadecimal value) - // - // Adds a healthy buffer of gas to an initial gas estimate. - addGasBuffer (gas) { - const gasBuffer = new BN('100000', 10) - const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const correct = bnGas.add(gasBuffer) - return ethUtil.addHexPrefix(correct.toString(16)) - } - - // Clear Keyrings - // - // Deallocates all currently managed keyrings and accounts. - // Used before initializing a new vault. - clearKeyrings () { - let accounts - try { - accounts = Object.keys(this.accountTracker.getState()) - } catch (e) { - accounts = [] - } - accounts.forEach((address) => { - this.accountTracker.removeAccount(address) - }) - - // clear keyrings from memory - this.keyrings = [] - this.memStore.updateState({ - keyrings: [], - identities: {}, - }) - } - - _updateMemStoreKeyrings () { - Promise.all(this.keyrings.map(this.displayForKeyring)) - .then((keyrings) => { - this.memStore.updateState({ keyrings }) - }) - } - -} - -module.exports = KeyringController diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 30e511e19..ebe6b65a8 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -14,7 +14,7 @@ const createOriginMiddleware = require('./lib/createOriginMiddleware') const createLoggerMiddleware = require('./lib/createLoggerMiddleware') const createProviderMiddleware = require('./lib/createProviderMiddleware') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex -const KeyringController = require('./keyring-controller') +const KeyringController = require('eth-keyring-controller') const NetworkController = require('./controllers/network') const PreferencesController = require('./controllers/preferences') const CurrencyController = require('./controllers/currency') diff --git a/app/scripts/migrations/_multi-keyring.js b/app/scripts/migrations/_multi-keyring.js index 253aa3d9d..7a4578ea7 100644 --- a/app/scripts/migrations/_multi-keyring.js +++ b/app/scripts/migrations/_multi-keyring.js @@ -10,7 +10,7 @@ which we dont have access to at the time of this writing. const ObservableStore = require('obs-store') const ConfigManager = require('../../app/scripts/lib/config-manager') const IdentityStoreMigrator = require('../../app/scripts/lib/idStore-migrator') -const KeyringController = require('../../app/scripts/lib/keyring-controller') +const KeyringController = require('eth-keyring-controller') const password = 'obviously not correct' diff --git a/package.json b/package.json index bcfb6c1ac..8526455e7 100644 --- a/package.json +++ b/package.json @@ -53,10 +53,8 @@ "async": "^2.5.0", "await-semaphore": "^0.1.1", "babel-runtime": "^6.23.0", - "bip39": "^2.2.0", "bluebird": "^3.5.0", "bn.js": "^4.11.7", - "browser-passworder": "^2.0.3", "browserify-derequire": "^0.9.4", "client-sw-ready-event": "^3.3.0", "clone": "^2.1.1", @@ -70,12 +68,11 @@ "ensnare": "^1.0.0", "eth-bin-to-ops": "^1.0.1", "eth-contract-metadata": "^1.1.4", - "eth-hd-keyring": "^1.1.1", "eth-json-rpc-filters": "^1.1.0", + "eth-keyring-controller": "^1.0.1", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", - "eth-simple-keyring": "^1.1.1", "eth-token-tracker": "^1.1.3", "ethereumjs-tx": "^1.3.0", "ethereumjs-util": "github:ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", diff --git a/test/unit/keyring-controller-test.js b/test/unit/keyring-controller-test.js deleted file mode 100644 index 135edf365..000000000 --- a/test/unit/keyring-controller-test.js +++ /dev/null @@ -1,164 +0,0 @@ -const assert = require('assert') -const KeyringController = require('../../app/scripts/keyring-controller') -const configManagerGen = require('../lib/mock-config-manager') -const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN -const mockEncryptor = require('../lib/mock-encryptor') -const sinon = require('sinon') - -describe('KeyringController', function () { - let keyringController - const password = 'password123' - const seedWords = 'puzzle seed penalty soldier say clay field arctic metal hen cage runway' - const addresses = ['eF35cA8EbB9669A35c31b5F6f249A9941a812AC1'.toLowerCase()] - const accounts = [] - // let originalKeystore - - beforeEach(function (done) { - this.sinon = sinon.sandbox.create() - window.localStorage = {} // Hacking localStorage support into JSDom - - keyringController = new KeyringController({ - configManager: configManagerGen(), - txManager: { - getTxList: () => [], - getUnapprovedTxList: () => [], - }, - accountTracker: { - addAccount (acct) { accounts.push(ethUtil.addHexPrefix(acct)) }, - }, - encryptor: mockEncryptor, - }) - - keyringController.createNewVaultAndKeychain(password) - .then(function (newState) { - newState - done() - }) - .catch((err) => { - done(err) - }) - }) - - afterEach(function () { - // Cleanup mocks - this.sinon.restore() - }) - - describe('#createNewVaultAndKeychain', function () { - this.timeout(10000) - - it('should set a vault on the configManager', function (done) { - keyringController.store.updateState({ vault: null }) - assert(!keyringController.store.getState().vault, 'no previous vault') - keyringController.createNewVaultAndKeychain(password) - .then(() => { - const vault = keyringController.store.getState().vault - assert(vault, 'vault created') - done() - }) - .catch((reason) => { - done(reason) - }) - }) - }) - - describe('#restoreKeyring', function () { - it(`should pass a keyring's serialized data back to the correct type.`, function (done) { - const mockSerialized = { - type: 'HD Key Tree', - data: { - mnemonic: seedWords, - numberOfAccounts: 1, - }, - } - const mock = this.sinon.mock(keyringController) - - mock.expects('getBalanceAndNickname') - .exactly(1) - - keyringController.restoreKeyring(mockSerialized) - .then((keyring) => { - assert.equal(keyring.wallets.length, 1, 'one wallet restored') - return keyring.getAccounts() - }) - .then((accounts) => { - assert.equal(accounts[0], addresses[0]) - mock.verify() - done() - }) - .catch((reason) => { - done(reason) - }) - }) - }) - - describe('#createNickname', function () { - it('should add the address to the identities hash', function () { - const fakeAddress = '0x12345678' - keyringController.createNickname(fakeAddress) - const identities = keyringController.memStore.getState().identities - const identity = identities[fakeAddress] - assert.equal(identity.address, fakeAddress) - }) - }) - - describe('#saveAccountLabel', function () { - it('sets the nickname', function (done) { - const account = addresses[0] - var nick = 'Test nickname' - const identities = keyringController.memStore.getState().identities - identities[ethUtil.addHexPrefix(account)] = {} - keyringController.memStore.updateState({ identities }) - keyringController.saveAccountLabel(account, nick) - .then((label) => { - try { - assert.equal(label, nick) - const persisted = keyringController.store.getState().walletNicknames[account] - assert.equal(persisted, nick) - done() - } catch (err) { - done() - } - }) - .catch((reason) => { - done(reason) - }) - }) - }) - - describe('#getAccounts', function () { - it('returns the result of getAccounts for each keyring', function (done) { - keyringController.keyrings = [ - { getAccounts () { return Promise.resolve([1, 2, 3]) } }, - { getAccounts () { return Promise.resolve([4, 5, 6]) } }, - ] - - keyringController.getAccounts() - .then((result) => { - assert.deepEqual(result, [1, 2, 3, 4, 5, 6]) - done() - }) - }) - }) - - describe('#addGasBuffer', function () { - it('adds 100k gas buffer to estimates', function () { - const gas = '0x04ee59' // Actual estimated gas example - const tooBigOutput = '0x80674f9' // Actual bad output - const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const correctBuffer = new BN('100000', 10) - const correct = bnGas.add(correctBuffer) - - // const tooBig = new BN(tooBigOutput, 16) - const result = keyringController.addGasBuffer(gas) - const bnResult = new BN(ethUtil.stripHexPrefix(result), 16) - - assert.equal(result.indexOf('0x'), 0, 'included hex prefix') - assert(bnResult.gt(bnGas), 'Estimate increased in value.') - assert.equal(bnResult.sub(bnGas).toString(10), '100000', 'added 100k gas') - assert.equal(result, '0x' + correct.toString(16), 'Added the right amount') - assert.notEqual(result, tooBigOutput, 'not that bad estimate') - }) - }) -}) From 8ad74cf93a8c0170f260507694f6a05eafa56c4f Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 22 Sep 2017 15:16:42 -0700 Subject: [PATCH 47/88] deps - bump filter deps and add random missing deps --- package.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 38ed28eef..018c10483 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "eth-block-tracker": "^2.2.0", "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", - "eth-json-rpc-filters": "^1.1.0", + "eth-json-rpc-filters": "^1.2.1", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", @@ -81,6 +81,7 @@ "ethereumjs-tx": "^1.3.0", "ethereumjs-util": "github:ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", "ethereumjs-wallet": "^0.6.0", + "ethjs-contract": "^0.1.9", "ethjs-ens": "^2.0.0", "ethjs-query": "^0.2.9", "express": "^4.14.0", @@ -91,6 +92,7 @@ "gulp": "github:gulpjs/gulp#4.0", "gulp-eslint": "^4.0.0", "hat": "0.0.3", + "human-standard-token-abi": "^1.0.2", "idb-global": "^2.1.0", "identicon.js": "^2.3.1", "iframe": "^1.0.0", @@ -139,7 +141,7 @@ "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "^0.20.1", - "web3-provider-engine": "^13.2.10", + "web3-provider-engine": "^13.2.12", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" }, From d4578836c99bfcdb66a6c989a7ecf79a896a2dc8 Mon Sep 17 00:00:00 2001 From: tmashuang Date: Fri, 22 Sep 2017 16:15:18 -0700 Subject: [PATCH 48/88] Most of transaction controller tests --- test/unit/tx-controller-test.js | 192 ++++++++++++++++++++++++++++++-- 1 file changed, 180 insertions(+), 12 deletions(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 47bfe66f8..e1e4ae8fe 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -7,12 +7,13 @@ const sinon = require('sinon') const TransactionController = require('../../app/scripts/controllers/transactions') const TxGasUtils = require('../../app/scripts/lib/tx-gas-utils') const txStateHistoryHelper = require('../../app/scripts/lib/tx-state-history-helper') +const TxStateManager = require('../../app/scripts/lib/tx-state-manager') +const { createStubedProvider } = require('../stub/provider') const noop = () => true const currentNetworkId = 42 const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') -const { createStubedProvider } = require('../stub/provider') describe('Transaction Controller', function () { @@ -34,11 +35,11 @@ describe('Transaction Controller', function () { }), }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) - txController.txProviderUtils = new TxGasUtils(txController.provider) + txController.txProviderUtils = new TxGasUtils(txController.provider) }) describe('#getState', function () { - it('should return a state object with the right keys and datat types', function (){ + it('should return a state object with the right keys and datat types', function () { const exposedState = txController.getState() assert('unapprovedTxs' in exposedState, 'state should have the key unapprovedTxs') assert('selectedAddressTxList' in exposedState, 'state should have the key selectedAddressTxList') @@ -71,13 +72,32 @@ describe('Transaction Controller', function () { }) }) + describe('#getConfirmedTransactions', function () { + let address + beforeEach(function () { + address = '0xc684832530fcbddae4b4230a47e991ddcec2831d' + const txParams = { + 'from': address, + 'to': '0xc684832530fcbddae4b4230a47e991ddcec2831d', + } + txController.txStateManager._saveTxList([ + {id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, + {id: 2, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, + {id: 3, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, + ]) + }) + it('should return the number of confirmed txs', function () { + assert.equal(txController.nonceTracker.getConfirmedTransactions(address).length, 3) + }) + }) + describe('#newUnapprovedTransaction', function () { let stub, txMeta, txParams beforeEach(function () { txParams = { - 'from':'0xc684832530fcbddae4b4230a47e991ddcec2831d', - 'to':'0xc684832530fcbddae4b4230a47e991ddcec2831d', + 'from': '0xc684832530fcbddae4b4230a47e991ddcec2831d', + 'to': '0xc684832530fcbddae4b4230a47e991ddcec2831d', }, txMeta = { status: 'unapproved', @@ -157,10 +177,10 @@ describe('Transaction Controller', function () { describe('#addTxDefaults', function () { it('should add the tx defaults if their are none', function (done) { - let txMeta = { + const txMeta = { 'txParams': { - 'from':'0xc684832530fcbddae4b4230a47e991ddcec2831d', - 'to':'0xc684832530fcbddae4b4230a47e991ddcec2831d', + 'from': '0xc684832530fcbddae4b4230a47e991ddcec2831d', + 'to': '0xc684832530fcbddae4b4230a47e991ddcec2831d', }, } providerResultStub.eth_gasPrice = '4a817c800' @@ -168,7 +188,7 @@ describe('Transaction Controller', function () { providerResultStub.eth_estimateGas = '5209' txController.addTxDefaults(txMeta) .then((txMetaWithDefaults) => { - assert(txMetaWithDefaults.txParams.value, '0x0','should have added 0x0 as the value') + assert(txMetaWithDefaults.txParams.value, '0x0', 'should have added 0x0 as the value') assert(txMetaWithDefaults.txParams.gasPrice, 'should have added the gas price') assert(txMetaWithDefaults.txParams.gas, 'should have added the gas field') done() @@ -177,6 +197,16 @@ describe('Transaction Controller', function () { }) }) + xdescribe('#updateAndApprovedTransaction', function () { + it('should update txMeta and approve status for Tx', async function () { + txController.txStateManager.addTx({ id: 0, status: 'unapproved', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', nonce: '0x1', value: '0xfffff' }, metamaskNetworkId: currentNetworkId }) + const txMeta = txController.txStateManager.getTx(0) + txMeta.value = '0xffffe' + provider.eth_sendRawTransaction = 0x7f9fade1c0d57a7af66ab4ead79fade1c0d57a7af66ab4ead7c2c2eb7b11a91385 + await txController.updateAndApproveTransaction(txMeta) + }) + }) + describe('#validateTxParams', function () { it('does not throw for positive values', function (done) { var sample = { @@ -205,9 +235,8 @@ describe('Transaction Controller', function () { const txMeta = { id: '1', status: 'unapproved', - id: 1, metamaskNetworkId: currentNetworkId, - txParams: {} + txParams: {}, } const eventNames = ['updateBadge', '1:unapproved'] @@ -286,4 +315,143 @@ describe('Transaction Controller', function () { }).catch(done) }) }) -}) \ No newline at end of file + + describe('#getChainId', function () { + it('returns 0 when the chainId is NaN', async function () { + txController.networkStore = new ObservableStore('hello') + assert.equal(txController.getChainId(), 0) + }) + }) + + describe('#publishTransaction', async function () { + beforeEach(function () { + const txMeta = [ + { id: 0, status: 'unapproved', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', nonce: '0x1', value: '0xfffff' }, rawTx: 'f8498080808080801ca00255b75b550cf112e18fd699f27d043d85348c29d7e8fd234799db890f7c272da0238121aed40c3141e63aae5335aaa3c9711d4907f28071f81f8d055b9f8435e0', metamaskNetworkId: currentNetworkId }, + ] + txController.txStateManager.addTx(txMeta) + }) + + it('should update rawTx of a transaction', async function () { + txController.publishTransaction(0, 'f84c01808080830fffff801ca0e23554ce6f82186402dcdcfff377793fdfa8a872a5670c0ffc002c9cd2f6827aa02a83dfce20aef4064a55320bda47394043af3cbfa63c4e029c54ba6281dcc70e') + }) + }) + + describe('#cancelTransaction', function () { + beforeEach(function () { + const txMetas = [ + { id: 0, status: 'unapproved', txParams: { }, metamaskNetworkId: currentNetworkId }, + { id: 1, status: 'rejected', txParams: { }, metamaskNetworkId: currentNetworkId }, + { id: 2, status: 'approved', txParams: { }, metamaskNetworkId: currentNetworkId }, + { id: 3, status: 'signed', txParams: { }, metamaskNetworkId: currentNetworkId }, + { id: 4, status: 'submitted', txParams: { }, metamaskNetworkId: currentNetworkId }, + { id: 5, status: 'confirmed', txParams: { }, metamaskNetworkId: currentNetworkId }, + { id: 6, status: 'failed', txParams: { }, metamaskNetworkId: currentNetworkId }, + ] + txMetas.forEach((txMeta) => txController.txStateManager.addTx(txMeta)) + }) + + it('should set the transaction to rejected from unapproved', async function () { + await txController.cancelTransaction(0) + assert(txController.txStateManager.getTx(0).status, 'rejected') + }) + + it('should set the transaction to rejected from rejected', async function () { + await txController.cancelTransaction(1) + assert(txController.txStateManager.getTx(1).status, 'rejected') + }) + + it('should set the transaction to rejected from approved', async function () { + await txController.cancelTransaction(2) + assert(txController.txStateManager.getTx(2).status, 'rejected') + }) + + it('should set the transaction to rejected from signed', async function () { + await txController.cancelTransaction(3) + assert(txController.txStateManager.getTx(3).status, 'rejected') + }) + + it('should set the transaction to rejected from submitted', async function () { + await txController.cancelTransaction(4) + assert(txController.txStateManager.getTx(4).status, 'rejected') + }) + + it('should set the transaction to rejected from confirmed', async function () { + await txController.cancelTransaction(5) + assert(txController.txStateManager.getTx(5).status, 'rejected') + }) + + it('should set the transaction to rejected from failed', async function () { + await txController.cancelTransaction(6) + assert(txController.txStateManager.getTx(6).status, 'rejected') + }) + + }) + + describe('#publishTransaction', function () { + let replaceRawTx, rawTx, hash, txMeta + beforeEach(function () { + rawTx = 'f86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d' + replaceRawTx = 'f84c01808080830fffff801ca0e23554ce6f82186402dcdcfff377793fdfa8a872a5670c0ffc002c9cd2f6827aa02a83dfce20aef4064a55320bda47394043af3cbfa63c4e029c54ba6281dcc70e' + txMeta = { + id: 1, + status: 'approved', + txParams: {}, + rawTx, + hash, + metamaskNetworkId: currentNetworkId, + } + providerResultStub.eth_sendRawTransaction = '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8' + }) + it('should publish a tx, updates the rawTx when provided a one', async function () { + txController.txStateManager.addTx(txMeta) + await txController.publishTransaction(txMeta.id, replaceRawTx) + txController.setTxHash(1, '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8') + assert.equal(txController.txStateManager.getTx(1).rawTx, replaceRawTx) + assert.notEqual(txController.txStateManager.getTx(1).rawTx, rawTx) + }) + }) + + describe('#getBalance', function () { + it('gets balance', function () { + sinon.stub(txController.ethStore, 'getState').callsFake(() => { + return { + accounts: { + '0x1678a085c290ebd122dc42cba69373b5953b831d': { + address: '0x1678a085c290ebd122dc42cba69373b5953b831d', + balance: '0x00000000000000056bc75e2d63100000', + code: '0x', + nonce: '0x0', + }, + '0xc684832530fcbddae4b4230a47e991ddcec2831d': { + address: '0xc684832530fcbddae4b4230a47e991ddcec2831d', + balance: '0x0', + code: '0x', + nonce: '0x0', + }, + }, + } + }) + assert.equal(txController.pendingTxTracker.getBalance('0x1678a085c290ebd122dc42cba69373b5953b831d'), '0x00000000000000056bc75e2d63100000') + assert.equal(txController.pendingTxTracker.getBalance('0xc684832530fcbddae4b4230a47e991ddcec2831d'), '0x0') + }) + }) + + describe('#getPendingTransactions', function () { + beforeEach(function () { + txController.txStateManager._saveTxList([ + { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 2, status: 'rejected', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 3, status: 'approved', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 4, status: 'signed', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 5, status: 'submitted', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 6, status: 'confimed', metamaskNetworkId: currentNetworkId, txParams: {} }, + { id: 7, status: 'failed', metamaskNetworkId: currentNetworkId, txParams: {} }, + ]) + }) + it('should show only submitted transactions as pending transasction', function () { + assert(txController.pendingTxTracker.getPendingTransactions().length, 1) + assert(txController.pendingTxTracker.getPendingTransactions()[0].status, 'submitted') + }) + }) + +}) From 20fea3f1dee33455842d9c2ac7e620d3321b6011 Mon Sep 17 00:00:00 2001 From: tmashuang Date: Mon, 25 Sep 2017 10:47:36 -0700 Subject: [PATCH 49/88] Remove pending updateAndApprovedTransaction test --- test/unit/tx-controller-test.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index e1e4ae8fe..f6b41f2bb 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -197,16 +197,6 @@ describe('Transaction Controller', function () { }) }) - xdescribe('#updateAndApprovedTransaction', function () { - it('should update txMeta and approve status for Tx', async function () { - txController.txStateManager.addTx({ id: 0, status: 'unapproved', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', nonce: '0x1', value: '0xfffff' }, metamaskNetworkId: currentNetworkId }) - const txMeta = txController.txStateManager.getTx(0) - txMeta.value = '0xffffe' - provider.eth_sendRawTransaction = 0x7f9fade1c0d57a7af66ab4ead79fade1c0d57a7af66ab4ead7c2c2eb7b11a91385 - await txController.updateAndApproveTransaction(txMeta) - }) - }) - describe('#validateTxParams', function () { it('does not throw for positive values', function (done) { var sample = { From 40f1d0868401662c42f6a031549c9b023427ccef Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 25 Sep 2017 11:42:08 -0700 Subject: [PATCH 50/88] Made some requested changes --- app/scripts/controllers/balance.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index ddeb06cf9..840b7abc3 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -16,7 +16,7 @@ class BalanceController { this.store = new ObservableStore(initState) this.balanceCalc = new PendingBalanceCalculator({ - getBalance: () => Promise.resolve(this._getBalance()), + getBalance: () => this._getBalance(), getPendingTransactions: this._getPendingTransactions.bind(this), }) @@ -35,24 +35,24 @@ class BalanceController { this.txController.on('submitted', update) this.txController.on('confirmed', update) this.txController.on('failed', update) + this.accountTracker.subscribe(update) this.txController.blockTracker.on('block', update) } - _getBalance () { - const store = this.accountTracker.getState() - const balances = store.accounts - const entry = balances[this.address] + async _getBalance () { + const { accounts } = this.accountTracker.getState() + const entry = accounts[this.address] const balance = entry.balance return balance ? new BN(balance.substring(2), 16) : undefined } - _getPendingTransactions () { + async _getPendingTransactions () { const pending = this.txController.getFilteredTxList({ from: this.address, status: 'submitted', err: undefined, }) - return Promise.resolve(pending) + return pending } } From 8cd7329c91b047ef15c81b164075ea6c1d15b0df Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 25 Sep 2017 14:36:49 -0700 Subject: [PATCH 51/88] Implemented feedback --- app/scripts/controllers/balance.js | 4 ++-- app/scripts/lib/pending-balance-calculator.js | 8 +++----- test/unit/pending-balance-test.js | 20 +++++++------------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index 840b7abc3..9b2566852 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -20,7 +20,7 @@ class BalanceController { getPendingTransactions: this._getPendingTransactions.bind(this), }) - this.registerUpdates() + this._registerUpdates() } async updateBalance () { @@ -30,7 +30,7 @@ class BalanceController { }) } - registerUpdates () { + _registerUpdates () { const update = this.updateBalance.bind(this) this.txController.on('submitted', update) this.txController.on('confirmed', update) diff --git a/app/scripts/lib/pending-balance-calculator.js b/app/scripts/lib/pending-balance-calculator.js index c66bffbbb..cea642f1a 100644 --- a/app/scripts/lib/pending-balance-calculator.js +++ b/app/scripts/lib/pending-balance-calculator.js @@ -19,19 +19,17 @@ class PendingBalanceCalculator { this.getPendingTransactions(), ]) - const balance = results[0] - const pending = results[1] - + const [ balance, pending ] = results if (!balance) return undefined const pendingValue = pending.reduce((total, tx) => { - return total.add(this.valueFor(tx)) + return total.add(this.calculateMaxCost(tx)) }, new BN(0)) return `0x${balance.sub(pendingValue).toString(16)}` } - valueFor (tx) { + calculateMaxCost (tx) { const txValue = tx.txParams.value const value = this.hexToBn(txValue) const gasPrice = this.hexToBn(tx.txParams.gasPrice) diff --git a/test/unit/pending-balance-test.js b/test/unit/pending-balance-test.js index dde30fecc..5048d487b 100644 --- a/test/unit/pending-balance-test.js +++ b/test/unit/pending-balance-test.js @@ -11,7 +11,7 @@ const ether = '0x' + etherBn.toString(16) describe('PendingBalanceCalculator', function () { let balanceCalculator - describe('#valueFor(tx)', function () { + describe('#calculateMaxCost(tx)', function () { it('returns a BN for a given tx value', function () { const txGen = new MockTxGen() pendingTxs = txGen.generate({ @@ -24,7 +24,7 @@ describe('PendingBalanceCalculator', function () { }, { count: 1 }) const balanceCalculator = generateBalanceCalcWith([], zeroBn) - const result = balanceCalculator.valueFor(pendingTxs[0]) + const result = balanceCalculator.calculateMaxCost(pendingTxs[0]) assert.equal(result.toString(), etherBn.toString(), 'computes one ether') }) @@ -40,8 +40,8 @@ describe('PendingBalanceCalculator', function () { }, { count: 1 }) const balanceCalculator = generateBalanceCalcWith([], zeroBn) - const result = balanceCalculator.valueFor(pendingTxs[0]) - assert.equal(result.toString(), '6', 'computes one ether') + const result = balanceCalculator.calculateMaxCost(pendingTxs[0]) + assert.equal(result.toString(), '6', 'computes 6 wei of gas') }) }) @@ -82,15 +82,9 @@ describe('PendingBalanceCalculator', function () { }) function generateBalanceCalcWith (transactions, providerStub = zeroBn) { - const getPendingTransactions = () => Promise.resolve(transactions) - const getBalance = () => Promise.resolve(providerStub) - providerResultStub.result = providerStub - const provider = { - sendAsync: (_, cb) => { cb(undefined, providerResultStub) }, - _blockTracker: { - getCurrentBlock: () => '0x11b568', - }, - } + const getPendingTransactions = async () => transactions + const getBalance = async () => providerStub + return new PendingBalanceCalculator({ getBalance, getPendingTransactions, From 674aac83ce49d21606be7be7afdf1b6a8ceb386f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 25 Sep 2017 14:39:22 -0700 Subject: [PATCH 52/88] Make blockTracker an independent param --- app/scripts/controllers/balance.js | 5 +++-- app/scripts/controllers/computed-balances.js | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index 9b2566852..ab0cfe907 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -5,10 +5,11 @@ const BN = require('ethereumjs-util').BN class BalanceController { constructor (opts = {}) { - const { address, accountTracker, txController } = opts + const { address, accountTracker, txController, blockTracker } = opts this.address = address this.accountTracker = accountTracker this.txController = txController + this.blockTracker = blockTracker const initState = { ethBalance: undefined, @@ -36,7 +37,7 @@ class BalanceController { this.txController.on('confirmed', update) this.txController.on('failed', update) this.accountTracker.subscribe(update) - this.txController.blockTracker.on('block', update) + this.blockTracker.on('block', update) } async _getBalance () { diff --git a/app/scripts/controllers/computed-balances.js b/app/scripts/controllers/computed-balances.js index 576746164..2b27d128d 100644 --- a/app/scripts/controllers/computed-balances.js +++ b/app/scripts/controllers/computed-balances.js @@ -5,9 +5,10 @@ const BalanceController = require('./balance') class ComputedbalancesController { constructor (opts = {}) { - const { accountTracker, txController } = opts + const { accountTracker, txController, blockTracker } = opts this.accountTracker = accountTracker this.txController = txController + this.blockTracker = blockTracker const initState = extend({ computedBalances: {}, @@ -50,6 +51,7 @@ class ComputedbalancesController { address, accountTracker: this.accountTracker, txController: this.txController, + blockTracker: this.blockTracker, }) updater.store.subscribe((accountBalance) => { let newState = this.store.getState() From feed9a5a17f89ee319dc5558634e8c5c07b2ce65 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 25 Sep 2017 14:45:28 -0700 Subject: [PATCH 53/88] Add mock random value generator --- test/lib/mock-encryptor.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/lib/mock-encryptor.js b/test/lib/mock-encryptor.js index cdf13c507..ef229a82f 100644 --- a/test/lib/mock-encryptor.js +++ b/test/lib/mock-encryptor.js @@ -29,4 +29,8 @@ module.exports = { return 'WHADDASALT!' }, + getRandomValues () { + return 'SOO RANDO!!!1' + } + } From 1968d61431183300298f3ea17b5092865cb915bf Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 25 Sep 2017 15:23:37 -0700 Subject: [PATCH 54/88] Make encryptor configurable for keyring-controller --- app/scripts/keyring-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 2a1af6e29..34e008ec4 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -37,7 +37,7 @@ class KeyringController extends EventEmitter { }) this.accountTracker = opts.accountTracker - this.encryptor = encryptor + this.encryptor = opts.encryptor || encryptor this.keyrings = [] this.getNetwork = opts.getNetwork } From ff6f7b52e4b5397daef74695894e0cdad22fe273 Mon Sep 17 00:00:00 2001 From: tmashuang Date: Mon, 25 Sep 2017 19:47:03 -0700 Subject: [PATCH 55/88] Clean up transactionController tests --- test/unit/tx-controller-test.js | 124 ++++++++++++++------------------ 1 file changed, 53 insertions(+), 71 deletions(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index f6b41f2bb..13056417c 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -2,12 +2,9 @@ const assert = require('assert') const ethUtil = require('ethereumjs-util') const EthTx = require('ethereumjs-tx') const ObservableStore = require('obs-store') -const clone = require('clone') const sinon = require('sinon') const TransactionController = require('../../app/scripts/controllers/transactions') const TxGasUtils = require('../../app/scripts/lib/tx-gas-utils') -const txStateHistoryHelper = require('../../app/scripts/lib/tx-state-history-helper') -const TxStateManager = require('../../app/scripts/lib/tx-state-manager') const { createStubedProvider } = require('../stub/provider') const noop = () => true @@ -17,7 +14,7 @@ const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f3 describe('Transaction Controller', function () { - let txController, engine, provider, providerResultStub + let txController, provider, providerResultStub beforeEach(function () { providerResultStub = {} @@ -81,11 +78,18 @@ describe('Transaction Controller', function () { 'to': '0xc684832530fcbddae4b4230a47e991ddcec2831d', } txController.txStateManager._saveTxList([ + {id: 0, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, {id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, {id: 2, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, - {id: 3, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams}, + {id: 3, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams}, + {id: 4, status: 'rejected', metamaskNetworkId: currentNetworkId, txParams}, + {id: 5, status: 'approved', metamaskNetworkId: currentNetworkId, txParams}, + {id: 6, status: 'signed', metamaskNetworkId: currentNetworkId, txParams}, + {id: 7, status: 'submitted', metamaskNetworkId: currentNetworkId, txParams}, + {id: 8, status: 'failed', metamaskNetworkId: currentNetworkId, txParams}, ]) }) + it('should return the number of confirmed txs', function () { assert.equal(txController.nonceTracker.getConfirmedTransactions(address).length, 3) }) @@ -98,7 +102,7 @@ describe('Transaction Controller', function () { txParams = { 'from': '0xc684832530fcbddae4b4230a47e991ddcec2831d', 'to': '0xc684832530fcbddae4b4230a47e991ddcec2831d', - }, + } txMeta = { status: 'unapproved', id: 1, @@ -306,98 +310,76 @@ describe('Transaction Controller', function () { }) }) - describe('#getChainId', function () { - it('returns 0 when the chainId is NaN', async function () { - txController.networkStore = new ObservableStore('hello') - assert.equal(txController.getChainId(), 0) - }) - }) - - describe('#publishTransaction', async function () { + describe('#updateAndApproveTransaction', function () { + let txMeta beforeEach(function () { - const txMeta = [ - { id: 0, status: 'unapproved', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', nonce: '0x1', value: '0xfffff' }, rawTx: 'f8498080808080801ca00255b75b550cf112e18fd699f27d043d85348c29d7e8fd234799db890f7c272da0238121aed40c3141e63aae5335aaa3c9711d4907f28071f81f8d055b9f8435e0', metamaskNetworkId: currentNetworkId }, - ] + txMeta = { + id: 1, + status: 'unapproved', + txParams: { + from: '0xc684832530fcbddae4b4230a47e991ddcec2831d', + to: '0x1678a085c290ebd122dc42cba69373b5953b831d', + gasPrice: '0x77359400', + gas: '0x7b0d', + nonce: '0x4b', + }, + metamaskNetworkId: currentNetworkId, + } + }) + it('should update and approve transactions', function () { txController.txStateManager.addTx(txMeta) + txController.updateAndApproveTransaction(txMeta) + const tx = txController.txStateManager.getTx(1) + assert.equal(tx.status, 'approved') }) + }) - it('should update rawTx of a transaction', async function () { - txController.publishTransaction(0, 'f84c01808080830fffff801ca0e23554ce6f82186402dcdcfff377793fdfa8a872a5670c0ffc002c9cd2f6827aa02a83dfce20aef4064a55320bda47394043af3cbfa63c4e029c54ba6281dcc70e') + describe('#getChainId', function () { + it('returns 0 when the chainId is NaN', function () { + txController.networkStore = new ObservableStore(NaN) + assert.equal(txController.getChainId(), 0) }) }) describe('#cancelTransaction', function () { beforeEach(function () { - const txMetas = [ - { id: 0, status: 'unapproved', txParams: { }, metamaskNetworkId: currentNetworkId }, - { id: 1, status: 'rejected', txParams: { }, metamaskNetworkId: currentNetworkId }, - { id: 2, status: 'approved', txParams: { }, metamaskNetworkId: currentNetworkId }, - { id: 3, status: 'signed', txParams: { }, metamaskNetworkId: currentNetworkId }, - { id: 4, status: 'submitted', txParams: { }, metamaskNetworkId: currentNetworkId }, - { id: 5, status: 'confirmed', txParams: { }, metamaskNetworkId: currentNetworkId }, - { id: 6, status: 'failed', txParams: { }, metamaskNetworkId: currentNetworkId }, - ] - txMetas.forEach((txMeta) => txController.txStateManager.addTx(txMeta)) + txController.txStateManager._saveTxList([ + { id: 0, status: 'unapproved', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + { id: 1, status: 'rejected', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + { id: 2, status: 'approved', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + { id: 3, status: 'signed', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + { id: 4, status: 'submitted', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + { id: 5, status: 'confirmed', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + { id: 6, status: 'failed', txParams: {}, metamaskNetworkId: currentNetworkId, history: [{}] }, + ]) }) it('should set the transaction to rejected from unapproved', async function () { await txController.cancelTransaction(0) - assert(txController.txStateManager.getTx(0).status, 'rejected') - }) - - it('should set the transaction to rejected from rejected', async function () { - await txController.cancelTransaction(1) - assert(txController.txStateManager.getTx(1).status, 'rejected') - }) - - it('should set the transaction to rejected from approved', async function () { - await txController.cancelTransaction(2) - assert(txController.txStateManager.getTx(2).status, 'rejected') - }) - - it('should set the transaction to rejected from signed', async function () { - await txController.cancelTransaction(3) - assert(txController.txStateManager.getTx(3).status, 'rejected') - }) - - it('should set the transaction to rejected from submitted', async function () { - await txController.cancelTransaction(4) - assert(txController.txStateManager.getTx(4).status, 'rejected') - }) - - it('should set the transaction to rejected from confirmed', async function () { - await txController.cancelTransaction(5) - assert(txController.txStateManager.getTx(5).status, 'rejected') - }) - - it('should set the transaction to rejected from failed', async function () { - await txController.cancelTransaction(6) - assert(txController.txStateManager.getTx(6).status, 'rejected') + assert.equal(txController.txStateManager.getTx(0).status, 'rejected') }) }) describe('#publishTransaction', function () { - let replaceRawTx, rawTx, hash, txMeta + let hash, txMeta beforeEach(function () { - rawTx = 'f86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d' - replaceRawTx = 'f84c01808080830fffff801ca0e23554ce6f82186402dcdcfff377793fdfa8a872a5670c0ffc002c9cd2f6827aa02a83dfce20aef4064a55320bda47394043af3cbfa63c4e029c54ba6281dcc70e' + hash = '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8' txMeta = { id: 1, - status: 'approved', + status: 'unapproved', txParams: {}, - rawTx, - hash, metamaskNetworkId: currentNetworkId, } - providerResultStub.eth_sendRawTransaction = '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8' + providerResultStub.eth_sendRawTransaction = hash }) + it('should publish a tx, updates the rawTx when provided a one', async function () { txController.txStateManager.addTx(txMeta) - await txController.publishTransaction(txMeta.id, replaceRawTx) - txController.setTxHash(1, '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8') - assert.equal(txController.txStateManager.getTx(1).rawTx, replaceRawTx) - assert.notEqual(txController.txStateManager.getTx(1).rawTx, rawTx) + await txController.publishTransaction(txMeta.id) + const publishedTx = txController.txStateManager.getTx(1) + assert.equal(publishedTx.hash, hash) + assert.equal(publishedTx.status, 'submitted') }) }) From b46cb3ecb5a3a1b4a197f69960f932d69287aa62 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 26 Sep 2017 09:23:46 -0700 Subject: [PATCH 56/88] Fix token precision bug Had fixed this before in the dependency, but hadn't merged in that version bump yet :( Fixes #2162 --- CHANGELOG.md | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e692c58dc..f04136d78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Fix bug that could mis-render token balances when very small. (Not actually included in 3.9.9) + ## 3.10.3 2017-9-21 - Fix bug where metamask-dapp connections are lost on rpc error diff --git a/package.json b/package.json index bcfb6c1ac..5599f2c20 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", "eth-simple-keyring": "^1.1.1", - "eth-token-tracker": "^1.1.3", + "eth-token-tracker": "^1.1.4", "ethereumjs-tx": "^1.3.0", "ethereumjs-util": "github:ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", "ethereumjs-wallet": "^0.6.0", From 5d300f146a679ba1a639ee9a568e8452c886c736 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 26 Sep 2017 09:38:43 -0700 Subject: [PATCH 57/88] Add computed balance to mock state --- development/states/first-time.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/development/states/first-time.json b/development/states/first-time.json index 683a61fdf..b2cc8ef8f 100644 --- a/development/states/first-time.json +++ b/development/states/first-time.json @@ -4,6 +4,7 @@ "isUnlocked": false, "rpcTarget": "https://rawtestrpc.metamask.io/", "identities": {}, + "computedBalances": {}, "frequentRpcList": [], "unapprovedTxs": {}, "currentCurrency": "USD", @@ -48,5 +49,6 @@ "isLoading": false, "warning": null }, - "identities": {} + "identities": {}, + "computedBalances": {} } From 9e3648c668aed1f3e632efe1693d6a2e0aa76617 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 26 Sep 2017 11:33:36 -0700 Subject: [PATCH 58/88] Pass blocktracker to balances controller --- app/scripts/metamask-controller.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 30e511e19..cca796678 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -136,6 +136,7 @@ module.exports = class MetamaskController extends EventEmitter { this.balancesController = new BalancesController({ accountTracker: this.accountTracker, txController: this.txController, + blockTracker: this.blockTracker, }) this.networkController.on('networkDidChange', () => { this.balancesController.updateAllBalances() From 3bedcd3582519c7afbb8164b40acca4b96eab4bf Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 26 Sep 2017 13:36:41 -0700 Subject: [PATCH 59/88] Restore blockGasLimit to account-tracker --- app/scripts/lib/account-tracker.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.js index bf949597b..3df5fbc9d 100644 --- a/app/scripts/lib/account-tracker.js +++ b/app/scripts/lib/account-tracker.js @@ -18,6 +18,7 @@ class EthereumStore extends ObservableStore { constructor (opts = {}) { super({ accounts: {}, + currentBlockGasLimit: '', }) this._provider = opts.provider this._query = new EthQuery(this._provider) @@ -54,6 +55,8 @@ class EthereumStore extends ObservableStore { const blockNumber = '0x' + block.number.toString('hex') this._currentBlockNumber = blockNumber + this.updateState({ currentBlockGasLimit: `0x${block.gasLimit.toString('hex')}` }) + async.parallel([ this._updateAccounts.bind(this), ], (err) => { From b654eb9b1f37cd3757e4614bb048884ab89d2986 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 11:52:57 -0700 Subject: [PATCH 60/88] wrap block tracker in events proxy --- app/scripts/controllers/network.js | 22 ++++++++------------ app/scripts/lib/events-proxy.js | 31 +++++++++++++++++++++++++++++ test/unit/network-contoller-test.js | 1 + 3 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 app/scripts/lib/events-proxy.js diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index 0a3e5e26b..6daedbb67 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -4,6 +4,7 @@ const ObservableStore = require('obs-store') const ComposedStore = require('obs-store/lib/composed') const extend = require('xtend') const EthQuery = require('eth-query') +const createEventEmitterProxy = require('../lib/events-proxy.js') const RPC_ADDRESS_LIST = require('../config.js').network const DEFAULT_RPC = RPC_ADDRESS_LIST['rinkeby'] @@ -31,16 +32,8 @@ module.exports = class NetworkController extends EventEmitter { initializeProvider (opts, providerContructor = MetaMaskProvider) { this.providerInit = opts this._provider = providerContructor(opts) - this._proxy = new Proxy(this._provider, { - get: (obj, name) => { - if (name === 'on') return this._on.bind(this) - return this._provider[name] - }, - set: (obj, name, value) => { - this._provider[name] = value - return value - }, - }) + this._proxy = createEventEmitterProxy(this._provider) + this.provider._blockTracker = createEventEmitterProxy(this._provider._blockTracker) this.provider.on('block', this._logBlock.bind(this)) this.provider.on('error', this.verifyNetwork.bind(this)) this.ethQuery = new EthQuery(this.provider) @@ -55,11 +48,12 @@ module.exports = class NetworkController extends EventEmitter { this._provider.removeAllListeners() this._provider.stop() - this.provider = MetaMaskProvider(newInit) + this._provider = MetaMaskProvider(newInit) // apply the listners created by other controllers - Object.keys(this._providerListeners).forEach((key) => { - this._providerListeners[key].forEach((handler) => this._provider.addListener(key, handler)) - }) + const blockTrackerHandlers = this.provider._blockTracker.proxyEventHandlers + this.provider.setTarget(this._provider) + + this.provider._blockTracker = createEventEmitterProxy(this._provider._blockTracker, blockTrackerHandlers) this.emit('networkDidChange') } diff --git a/app/scripts/lib/events-proxy.js b/app/scripts/lib/events-proxy.js new file mode 100644 index 000000000..d1199a278 --- /dev/null +++ b/app/scripts/lib/events-proxy.js @@ -0,0 +1,31 @@ +module.exports = function createEventEmitterProxy(eventEmitter, listeners) { + let target = eventEmitter + const eventHandlers = listeners || {} + const proxy = new Proxy({}, { + get: (obj, name) => { + // intercept listeners + if (name === 'on') return addListener + if (name === 'setTarget') return setTarget + if (name === 'proxyEventHandlers') return eventHandlers + return target[name] + }, + set: (obj, name, value) => { + target[name] = value + return true + }, + }) + function setTarget (eventEmitter) { + target = eventEmitter + // migrate listeners + Object.keys(eventHandlers).forEach((name) => { + eventHandlers[name].forEach((handler) => target.on(name, handler)) + }) + } + function addListener (name, handler) { + if (!eventHandlers[name]) eventHandlers[name] = [] + eventHandlers[name].push(handler) + target.on(name, handler) + } + if (listeners) proxy.setTarget(eventEmitter) + return proxy +} \ No newline at end of file diff --git a/test/unit/network-contoller-test.js b/test/unit/network-contoller-test.js index 87c2ee7a3..853e4e457 100644 --- a/test/unit/network-contoller-test.js +++ b/test/unit/network-contoller-test.js @@ -71,6 +71,7 @@ function dummyProviderConstructor() { // provider sendAsync: noop, // block tracker + _blockTracker: {}, start: noop, stop: noop, on: noop, From 2ed8d579daa4d33c38891ac77d1415fcd237a187 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 13:37:13 -0700 Subject: [PATCH 61/88] listen for the blocke event on the block tracker instead of rawBlock on the provider --- app/scripts/controllers/network.js | 1 - app/scripts/controllers/transactions.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index 6daedbb67..00bdca2c3 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -52,7 +52,6 @@ module.exports = class NetworkController extends EventEmitter { // apply the listners created by other controllers const blockTrackerHandlers = this.provider._blockTracker.proxyEventHandlers this.provider.setTarget(this._provider) - this.provider._blockTracker = createEventEmitterProxy(this._provider._blockTracker, blockTrackerHandlers) this.emit('networkDidChange') } diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index fb3be6073..4e52a3c14 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -70,7 +70,7 @@ module.exports = class TransactionController extends EventEmitter { this.pendingTxTracker.on('txFailed', this.setTxStatusFailed.bind(this)) this.pendingTxTracker.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) - this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) + this.blockTracker.on('block', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition // where ethStore hasent been populated by the results yet From 4f887c6a62ebcecfb578d55fffd3c9f7a1bd088a Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 13:44:45 -0700 Subject: [PATCH 62/88] fix test --- test/unit/network-contoller-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/network-contoller-test.js b/test/unit/network-contoller-test.js index 853e4e457..c1fdaf032 100644 --- a/test/unit/network-contoller-test.js +++ b/test/unit/network-contoller-test.js @@ -21,7 +21,7 @@ describe('# Network Controller', function () { it('provider should be updatable without reassignment', function () { networkController.initializeProvider(networkControllerProviderInit, dummyProviderConstructor) const provider = networkController.provider - networkController._provider = {test: true} + networkController.provider.setTarget({test: true, on: () => {}}) assert.ok(provider.test) }) }) From 9d1cb0f76dce203299200940f21e868f6a5efef3 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 13:56:09 -0700 Subject: [PATCH 63/88] network contoller - clean up unused code --- app/scripts/controllers/network.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index 00bdca2c3..dc9978043 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -114,10 +114,4 @@ module.exports = class NetworkController extends EventEmitter { log.info(`BLOCK CHANGED: #${block.number.toString('hex')} 0x${block.hash.toString('hex')}`) this.verifyNetwork() } - - _on (event, handler) { - if (!this._providerListeners[event]) this._providerListeners[event] = [] - this._providerListeners[event].push(handler) - this._provider.on(event, handler) - } } From 2eca5455c0c80d99b10c7d56858f84e605494fba Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 26 Sep 2017 14:15:16 -0700 Subject: [PATCH 64/88] Move obs store into account-tracker instead of inheriting --- app/scripts/controllers/balance.js | 4 +-- app/scripts/controllers/computed-balances.js | 6 ++-- app/scripts/lib/account-tracker.js | 31 ++++++++++++-------- app/scripts/metamask-controller.js | 4 +-- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index ab0cfe907..964dff0df 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -36,12 +36,12 @@ class BalanceController { this.txController.on('submitted', update) this.txController.on('confirmed', update) this.txController.on('failed', update) - this.accountTracker.subscribe(update) + this.accountTracker.store.subscribe(update) this.blockTracker.on('block', update) } async _getBalance () { - const { accounts } = this.accountTracker.getState() + const { accounts } = this.accountTracker.store.getState() const entry = accounts[this.address] const balance = entry.balance return balance ? new BN(balance.substring(2), 16) : undefined diff --git a/app/scripts/controllers/computed-balances.js b/app/scripts/controllers/computed-balances.js index 2b27d128d..2479e1b3a 100644 --- a/app/scripts/controllers/computed-balances.js +++ b/app/scripts/controllers/computed-balances.js @@ -20,15 +20,15 @@ class ComputedbalancesController { } updateAllBalances () { - for (let address in this.balances) { + for (let address in this.accountTracker.store.getState().accounts) { this.balances[address].updateBalance() } } _initBalanceUpdating () { - const store = this.accountTracker.getState() + const store = this.accountTracker.store.getState() this.addAnyAccountsFromStore(store) - this.accountTracker.subscribe(this.addAnyAccountsFromStore.bind(this)) + this.accountTracker.store.subscribe(this.addAnyAccountsFromStore.bind(this)) } addAnyAccountsFromStore(store) { diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.js index 3df5fbc9d..e2892b1ce 100644 --- a/app/scripts/lib/account-tracker.js +++ b/app/scripts/lib/account-tracker.js @@ -10,16 +10,21 @@ const async = require('async') const EthQuery = require('eth-query') const ObservableStore = require('obs-store') +const EventEmitter = require('events').EventEmitter function noop () {} -class EthereumStore extends ObservableStore { +class AccountTracker extends EventEmitter { constructor (opts = {}) { - super({ + super() + + const initState = { accounts: {}, currentBlockGasLimit: '', - }) + } + this.store = new ObservableStore(initState) + this._provider = opts.provider this._query = new EthQuery(this._provider) this._blockTracker = opts.blockTracker @@ -34,17 +39,17 @@ class EthereumStore extends ObservableStore { // addAccount (address) { - const accounts = this.getState().accounts + const accounts = this.store.getState().accounts accounts[address] = {} - this.updateState({ accounts }) + this.store.updateState({ accounts }) if (!this._currentBlockNumber) return this._updateAccount(address) } removeAccount (address) { - const accounts = this.getState().accounts + const accounts = this.store.getState().accounts delete accounts[address] - this.updateState({ accounts }) + this.store.updateState({ accounts }) } // @@ -55,31 +60,31 @@ class EthereumStore extends ObservableStore { const blockNumber = '0x' + block.number.toString('hex') this._currentBlockNumber = blockNumber - this.updateState({ currentBlockGasLimit: `0x${block.gasLimit.toString('hex')}` }) + this.store.updateState({ currentBlockGasLimit: `0x${block.gasLimit.toString('hex')}` }) async.parallel([ this._updateAccounts.bind(this), ], (err) => { if (err) return console.error(err) - this.emit('block', this.getState()) + this.emit('block', this.store.getState()) }) } _updateAccounts (cb = noop) { - const accounts = this.getState().accounts + const accounts = this.store.getState().accounts const addresses = Object.keys(accounts) async.each(addresses, this._updateAccount.bind(this), cb) } _updateAccount (address, cb = noop) { - const accounts = this.getState().accounts this._getAccount(address, (err, result) => { if (err) return cb(err) result.address = address + const accounts = this.store.getState().accounts // only populate if the entry is still present if (accounts[address]) { accounts[address] = result - this.updateState({ accounts }) + this.store.updateState({ accounts }) } cb(null, result) }) @@ -96,4 +101,4 @@ class EthereumStore extends ObservableStore { } -module.exports = EthereumStore +module.exports = AccountTracker diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index cca796678..a86d8d37b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -194,7 +194,7 @@ module.exports = class MetamaskController extends EventEmitter { // manual mem state subscriptions this.networkController.store.subscribe(this.sendUpdate.bind(this)) - this.accountTracker.subscribe(this.sendUpdate.bind(this)) + this.accountTracker.store.subscribe(this.sendUpdate.bind(this)) this.txController.memStore.subscribe(this.sendUpdate.bind(this)) this.balancesController.store.subscribe(this.sendUpdate.bind(this)) this.messageManager.memStore.subscribe(this.sendUpdate.bind(this)) @@ -277,7 +277,7 @@ module.exports = class MetamaskController extends EventEmitter { isInitialized, }, this.networkController.store.getState(), - this.accountTracker.getState(), + this.accountTracker.store.getState(), this.txController.memStore.getState(), this.messageManager.memStore.getState(), this.personalMessageManager.memStore.getState(), From 651098c70d21edbca98a96ef2a8800d164035638 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 26 Sep 2017 14:30:29 -0700 Subject: [PATCH 65/88] Remove duplicate instantiation of account-tracker --- app/scripts/metamask-controller.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a86d8d37b..0f850b7f5 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -86,6 +86,7 @@ module.exports = class MetamaskController extends EventEmitter { // eth data query tools this.ethQuery = new EthQuery(this.provider) + // account tracker watches balances, nonces, and any code at their address. this.accountTracker = new AccountTracker({ provider: this.provider, blockTracker: this.blockTracker, @@ -99,11 +100,6 @@ module.exports = class MetamaskController extends EventEmitter { encryptor: opts.encryptor || undefined, }) - // account tracker watches balances, nonces, and any code at their address. - this.accountTracker = new AccountTracker({ - provider: this.provider, - blockTracker: this.provider, - }) this.keyringController.on('newAccount', (address) => { this.preferencesController.setSelectedAddress(address) this.accountTracker.addAccount(address) From 9fd545811226c16e11e4f2dc100a348e07f094bf Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 16:52:08 -0700 Subject: [PATCH 66/88] transactions: lint fixes and reveal status-update event for balance controller --- app/scripts/background.js | 2 +- app/scripts/controllers/balance.js | 15 ++++++++++++--- app/scripts/controllers/transactions.js | 5 +++-- app/scripts/lib/tx-state-manager.js | 12 ++++++------ app/scripts/metamask-controller.js | 2 +- test/unit/tx-controller-test.js | 2 +- 6 files changed, 24 insertions(+), 14 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 1b96d68b5..195881e15 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -114,7 +114,7 @@ function setupController (initState) { // updateBadge() - controller.txController.on('updateBadge', updateBadge) + controller.txController.on('update:badge', updateBadge) controller.messageManager.on('updateBadge', updateBadge) controller.personalMessageManager.on('updateBadge', updateBadge) diff --git a/app/scripts/controllers/balance.js b/app/scripts/controllers/balance.js index 964dff0df..4fa4c78fe 100644 --- a/app/scripts/controllers/balance.js +++ b/app/scripts/controllers/balance.js @@ -33,9 +33,18 @@ class BalanceController { _registerUpdates () { const update = this.updateBalance.bind(this) - this.txController.on('submitted', update) - this.txController.on('confirmed', update) - this.txController.on('failed', update) + + this.txController.on('tx:status-update', (txId, status) => { + switch (status) { + case 'submitted': + case 'confirmed': + case 'failed': + update() + return + default: + return + } + }) this.accountTracker.store.subscribe(update) this.blockTracker.on('block', update) } diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index de5fa5b20..1b647a4ed 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -43,7 +43,8 @@ module.exports = class TransactionController extends EventEmitter { txHistoryLimit: opts.txHistoryLimit, getNetwork: this.getNetwork.bind(this), }) - + this.store = this.txStateManager.store + this.txStateManager.on('tx:status-update', this.emit.bind(this, 'tx:status-update')) this.nonceTracker = new NonceTracker({ provider: this.provider, getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), @@ -69,7 +70,7 @@ module.exports = class TransactionController extends EventEmitter { getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), }) - this.txStateManager.store.subscribe(() => this.emit('updateBadge')) + this.txStateManager.store.subscribe(() => this.emit('update:badge')) this.pendingTxTracker.on('txWarning', this.txStateManager.updateTx.bind(this.txStateManager)) this.pendingTxTracker.on('txFailed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index d7b76fe22..abb9d7910 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -5,7 +5,7 @@ const ethUtil = require('ethereumjs-util') const txStateHistoryHelper = require('./tx-state-history-helper') module.exports = class TransactionStateManger extends EventEmitter { - constructor ({initState, txHistoryLimit, getNetwork}) { + constructor ({ initState, txHistoryLimit, getNetwork }) { super() this.store = new ObservableStore( @@ -15,7 +15,8 @@ module.exports = class TransactionStateManger extends EventEmitter { this.txHistoryLimit = txHistoryLimit this.getNetwork = getNetwork } - // Returns the number of txs for the current network. + + // Returns the number of txs for the current network. getTxCount () { return this.getTxList().length } @@ -31,7 +32,6 @@ module.exports = class TransactionStateManger extends EventEmitter { } // Returns the tx list - getUnapprovedTxList () { const txList = this.getTxsByMetaData('status', 'unapproved') return txList.reduce((result, tx) => { @@ -69,7 +69,7 @@ module.exports = class TransactionStateManger extends EventEmitter { // or rejected tx's. // not tx's that are pending or unapproved if (txCount > txHistoryLimit - 1) { - const index = transactions.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected'))) + const index = transactions.findIndex((metaTx) => metaTx.status === 'confirmed' || metaTx.status === 'rejected') transactions.splice(index, 1) } transactions.push(txMeta) @@ -229,12 +229,12 @@ module.exports = class TransactionStateManger extends EventEmitter { const txMeta = this.getTx(txId) txMeta.status = status this.emit(`${txMeta.id}:${status}`, txId) - this.emit(`${status}`, txId) + this.emit(`tx:status-update`, txId, status) if (status === 'submitted' || status === 'rejected') { this.emit(`${txMeta.id}:finished`, txMeta) } this.updateTx(txMeta) - this.emit('updateBadge') + this.emit('update:badge') } // Saves the new/updated txList. diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 96b34507f..0f850b7f5 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -157,7 +157,7 @@ module.exports = class MetamaskController extends EventEmitter { this.publicConfigStore = this.initPublicConfigStore() // manual disk state subscriptions - this.txController.txStateManager.store.subscribe((state) => { + this.txController.store.subscribe((state) => { this.store.updateState({ TransactionController: state }) }) this.keyringController.store.subscribe((state) => { diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 8d45fb9b5..c1612a30c 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -210,7 +210,7 @@ describe('Transaction Controller', function () { txParams: {} } - const eventNames = ['updateBadge', '1:unapproved'] + const eventNames = ['update:badge', '1:unapproved'] const listeners = [] eventNames.forEach((eventName) => { listeners.push(new Promise((resolve) => { From 80c98b16531db4c6a13a52df800967af2bc4a676 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 16:55:11 -0700 Subject: [PATCH 67/88] transactions: make evnt names pretty and eaiser to read --- app/scripts/controllers/transactions.js | 6 +++--- app/scripts/lib/pending-tx-tracker.js | 16 ++++++++-------- test/unit/pending-tx-test.js | 18 +++++++++--------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 1b647a4ed..ec1524968 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -72,9 +72,9 @@ module.exports = class TransactionController extends EventEmitter { this.txStateManager.store.subscribe(() => this.emit('update:badge')) - this.pendingTxTracker.on('txWarning', this.txStateManager.updateTx.bind(this.txStateManager)) - this.pendingTxTracker.on('txFailed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) - this.pendingTxTracker.on('txConfirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) + this.pendingTxTracker.on('tx:warning', this.txStateManager.updateTx.bind(this.txStateManager)) + this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) + this.pendingTxTracker.on('tx:confirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) // this is a little messy but until ethstore has been either diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 4541221d5..31cf9babb 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -42,13 +42,13 @@ module.exports = class PendingTransactionTracker extends EventEmitter { if (!txHash) { const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') noTxHashErr.name = 'NoTxHashError' - this.emit('txFailed', txId, noTxHashErr) + this.emit('tx:failed', txId, noTxHashErr) return } block.transactions.forEach((tx) => { - if (tx.hash === txHash) this.emit('txConfirmed', txId) + if (tx.hash === txHash) this.emit('tx:confirmed', txId) }) }) } @@ -94,7 +94,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { // ignore resubmit warnings, return early if (isKnownTx) return // encountered real error - transition to error state - this.emit('txFailed', txMeta.id, err) + this.emit('tx:failed', txMeta.id, err) })) } @@ -106,13 +106,13 @@ module.exports = class PendingTransactionTracker extends EventEmitter { if (txMeta.retryCount > this.retryLimit) { const err = new Error(`Gave up submitting after ${this.retryLimit} blocks un-mined.`) - return this.emit('txFailed', txMeta.id, err) + return this.emit('tx:failed', txMeta.id, err) } // if the value of the transaction is greater then the balance, fail. if (!sufficientBalance(txMeta.txParams, balance)) { const insufficientFundsError = new Error('Insufficient balance during rebroadcast.') - this.emit('txFailed', txMeta.id, insufficientFundsError) + this.emit('tx:failed', txMeta.id, insufficientFundsError) log.error(insufficientFundsError) return } @@ -136,7 +136,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { if (!txHash) { const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') noTxHashErr.name = 'NoTxHashError' - this.emit('txFailed', txId, noTxHashErr) + this.emit('tx:failed', txId, noTxHashErr) return } // get latest transaction status @@ -145,14 +145,14 @@ module.exports = class PendingTransactionTracker extends EventEmitter { txParams = await this.query.getTransactionByHash(txHash) if (!txParams) return if (txParams.blockNumber) { - this.emit('txConfirmed', txId) + this.emit('tx:confirmed', txId) } } catch (err) { txMeta.warning = { error: err, message: 'There was a problem loading this transaction.', } - this.emit('txWarning', txMeta) + this.emit('tx:warning', txMeta) throw err } } diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 8c6d287f8..2865a30e6 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -62,7 +62,7 @@ describe('PendingTransactionTracker', function () { it('should emit \'txFailed\' if the txMeta does not have a hash', function (done) { const block = Proxy.revocable({}, {}).revoke() pendingTxTracker.getPendingTransactions = () => [txMetaNoHash] - pendingTxTracker.once('txFailed', (txId, err) => { + pendingTxTracker.once('tx:failed', (txId, err) => { assert(txId, txMetaNoHash.id, 'should pass txId') done() }) @@ -71,11 +71,11 @@ describe('PendingTransactionTracker', function () { it('should emit \'txConfirmed\' if the tx is in the block', function (done) { const block = { transactions: [txMeta]} pendingTxTracker.getPendingTransactions = () => [txMeta] - pendingTxTracker.once('txConfirmed', (txId) => { + pendingTxTracker.once('tx:confirmed', (txId) => { assert(txId, txMeta.id, 'should pass txId') done() }) - pendingTxTracker.once('txFailed', (_, err) => { done(err) }) + pendingTxTracker.once('tx:failed', (_, err) => { done(err) }) pendingTxTracker.checkForTxInBlock(block) }) }) @@ -108,7 +108,7 @@ describe('PendingTransactionTracker', function () { describe('#_checkPendingTx', function () { it('should emit \'txFailed\' if the txMeta does not have a hash', function (done) { - pendingTxTracker.once('txFailed', (txId, err) => { + pendingTxTracker.once('tx:failed', (txId, err) => { assert(txId, txMetaNoHash.id, 'should pass txId') done() }) @@ -122,11 +122,11 @@ describe('PendingTransactionTracker', function () { it('should emit \'txConfirmed\'', function (done) { providerResultStub.eth_getTransactionByHash = {blockNumber: '0x01'} - pendingTxTracker.once('txConfirmed', (txId) => { + pendingTxTracker.once('tx:confirmed', (txId) => { assert(txId, txMeta.id, 'should pass txId') done() }) - pendingTxTracker.once('txFailed', (_, err) => { done(err) }) + pendingTxTracker.once('tx:failed', (_, err) => { done(err) }) pendingTxTracker._checkPendingTx(txMeta) }) }) @@ -188,7 +188,7 @@ describe('PendingTransactionTracker', function () { ] const enoughForAllErrors = txList.concat(txList) - pendingTxTracker.on('txFailed', (_, err) => done(err)) + pendingTxTracker.on('tx:failed', (_, err) => done(err)) pendingTxTracker.getPendingTransactions = () => enoughForAllErrors pendingTxTracker._resubmitTx = async (tx) => { @@ -202,7 +202,7 @@ describe('PendingTransactionTracker', function () { pendingTxTracker.resubmitPendingTxs() }) it('should emit \'txFailed\' if it encountered a real error', function (done) { - pendingTxTracker.once('txFailed', (id, err) => err.message === 'im some real error' ? txList[id - 1].resolve() : done(err)) + pendingTxTracker.once('tx:failed', (id, err) => err.message === 'im some real error' ? txList[id - 1].resolve() : done(err)) pendingTxTracker.getPendingTransactions = () => txList pendingTxTracker._resubmitTx = async (tx) => { throw new TypeError('im some real error') } @@ -226,7 +226,7 @@ describe('PendingTransactionTracker', function () { // Stubbing out current account state: // Adding the fake tx: - pendingTxTracker.once('txFailed', (txId, err) => { + pendingTxTracker.once('tx:failed', (txId, err) => { assert(err, 'Should have a error') done() }) From 508696f71d6b5b14c708a3229039f9f915ce48ce Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 18:11:51 -0700 Subject: [PATCH 68/88] transactions: reveal #getFilteredTxList from txStateManage and fix accountTracker.store reference --- app/scripts/controllers/transactions.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index ec1524968..6ea2933dc 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -62,7 +62,7 @@ module.exports = class TransactionController extends EventEmitter { nonceTracker: this.nonceTracker, retryLimit: 3500, // Retry 3500 blocks, or about 1 day. getBalance: (address) => { - const account = this.accountTracker.getState().accounts[address] + const account = this.accountTracker.store.getState().accounts[address] if (!account) return return account.balance }, @@ -111,6 +111,10 @@ module.exports = class TransactionController extends EventEmitter { return this.txStateManager.getPendingTransactions(account).length } + getFilteredTxList (opts) { + return this.txStateManager.getFilteredTxList(opts) + } + getChainId () { const networkState = this.networkStore.getState() const getChainId = parseInt(networkState) From b05a6f89cb2c4de1e53d96f8cac01653a8165555 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 18:19:44 -0700 Subject: [PATCH 69/88] fix tests --- test/unit/tx-controller-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index dd8b48684..9ffba31ee 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -25,7 +25,7 @@ describe('Transaction Controller', function () { networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, blockTracker: { getCurrentBlock: noop, on: noop, once: noop }, - accountTracker: { getState: noop }, + accountTracker: { store: {getState: noop} }, signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) resolve() @@ -385,7 +385,7 @@ describe('Transaction Controller', function () { describe('#getBalance', function () { it('gets balance', function () { - sinon.stub(txController.ethStore, 'getState').callsFake(() => { + sinon.stub(txController.accountTracker.store, 'getState').callsFake(() => { return { accounts: { '0x1678a085c290ebd122dc42cba69373b5953b831d': { From 0a94ec41d3a2877ed7cfd3c8f9e9f9d725659183 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 26 Sep 2017 22:42:59 -0700 Subject: [PATCH 70/88] pending-tx - move incrementing of the retryCount on the txMeta outside pending-tx-tracker --- app/scripts/controllers/transactions.js | 5 +++++ app/scripts/lib/pending-tx-tracker.js | 3 +-- test/unit/tx-controller-test.js | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 6ea2933dc..3cd107031 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -75,6 +75,11 @@ module.exports = class TransactionController extends EventEmitter { this.pendingTxTracker.on('tx:warning', this.txStateManager.updateTx.bind(this.txStateManager)) this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) this.pendingTxTracker.on('tx:confirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) + this.pendingTxTracker.on('tx:retry', (txMeta) => { + if (!('retryCount' in txMeta)) txMeta.retryCount = 0 + txMeta.retryCount++ + this.txStateManager.updateTx(txMeta) + }) this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) // this is a little messy but until ethstore has been either diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 31cf9babb..b07a6bd39 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -102,7 +102,6 @@ module.exports = class PendingTransactionTracker extends EventEmitter { const address = txMeta.txParams.from const balance = this.getBalance(address) if (balance === undefined) return - if (!('retryCount' in txMeta)) txMeta.retryCount = 0 if (txMeta.retryCount > this.retryLimit) { const err = new Error(`Gave up submitting after ${this.retryLimit} blocks un-mined.`) @@ -124,7 +123,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { const txHash = await this.publishTransaction(rawTx) // Increment successful tries: - txMeta.retryCount++ + this.emit('tx:retry', txMeta) return txHash } diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 9ffba31ee..66772ff88 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -25,7 +25,7 @@ describe('Transaction Controller', function () { networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, blockTracker: { getCurrentBlock: noop, on: noop, once: noop }, - accountTracker: { store: {getState: noop} }, + accountTracker: { store: { getState: noop } }, signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) resolve() From d77e0aff4df1f41b30653c761ce2ca19cfc19efb Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 11:15:22 -0700 Subject: [PATCH 71/88] Version 3.10.4 --- CHANGELOG.md | 4 ++++ app/manifest.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f04136d78..eedfa89c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ ## Current Master +## 3.10.4 2017-9-27 + - Fix bug that could mis-render token balances when very small. (Not actually included in 3.9.9) +- Fix memory leak warning. +- Fix bug where new event filters would not include historical events. ## 3.10.3 2017-9-21 diff --git a/app/manifest.json b/app/manifest.json index fd07f15a9..8812f4eea 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.10.3", + "version": "3.10.4", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", From 8d3fec42d0a49c2e0fe7ef3dc504533deb223d95 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 12:09:32 -0700 Subject: [PATCH 72/88] Fix bug where block gas limit was incorrectly parsed. --- CHANGELOG.md | 2 ++ app/scripts/lib/account-tracker.js | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eedfa89c5..4898d27fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Fix block gas limit estimation. + ## 3.10.4 2017-9-27 - Fix bug that could mis-render token balances when very small. (Not actually included in 3.9.9) diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.js index e2892b1ce..07fc32b10 100644 --- a/app/scripts/lib/account-tracker.js +++ b/app/scripts/lib/account-tracker.js @@ -11,6 +11,7 @@ const async = require('async') const EthQuery = require('eth-query') const ObservableStore = require('obs-store') const EventEmitter = require('events').EventEmitter +const ethUtil = require('ethereumjs-util') function noop () {} @@ -59,8 +60,9 @@ class AccountTracker extends EventEmitter { _updateForBlock (block) { const blockNumber = '0x' + block.number.toString('hex') this._currentBlockNumber = blockNumber + const currentBlockGasLimit = ethUtil.addHexPrefix(block.gasLimit.toString()) - this.store.updateState({ currentBlockGasLimit: `0x${block.gasLimit.toString('hex')}` }) + this.store.updateState({ currentBlockGasLimit }) async.parallel([ this._updateAccounts.bind(this), From a453eb132d1aa97923df8900f8290a1b3ca1dee3 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 12:10:25 -0700 Subject: [PATCH 73/88] Version 3.10.5 --- CHANGELOG.md | 2 ++ app/manifest.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4898d27fc..3ad9888fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +## 3.10.5 2017-9-27 + - Fix block gas limit estimation. ## 3.10.4 2017-9-27 diff --git a/app/manifest.json b/app/manifest.json index 8812f4eea..4d02cd334 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.10.4", + "version": "3.10.5", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", From 1983e161c658979872bad66f8dd5e9b2c7a616b5 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 12:29:09 -0700 Subject: [PATCH 74/88] Fix accountTracker store references --- app/scripts/controllers/transactions.js | 2 +- app/scripts/keyring-controller.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 4cd307b07..87521c76b 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -52,7 +52,7 @@ module.exports = class TransactionController extends EventEmitter { provider: this.provider, nonceTracker: this.nonceTracker, getBalance: (address) => { - const account = this.accountTracker.getState().accounts[address] + const account = this.accountTracker.store.getState().accounts[address] if (!account) return return account.balance }, diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 34e008ec4..1a1904621 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -568,7 +568,7 @@ class KeyringController extends EventEmitter { clearKeyrings () { let accounts try { - accounts = Object.keys(this.accountTracker.getState()) + accounts = Object.keys(this.accountTracker.store.getState()) } catch (e) { accounts = [] } From 89e690fc794a7cf0af541dbb0c1fd58d73bac368 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 12:33:00 -0700 Subject: [PATCH 75/88] account-tracker - use new block-tracker block format --- app/scripts/lib/account-tracker.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.js index e2892b1ce..e550c2758 100644 --- a/app/scripts/lib/account-tracker.js +++ b/app/scripts/lib/account-tracker.js @@ -57,10 +57,8 @@ class AccountTracker extends EventEmitter { // _updateForBlock (block) { - const blockNumber = '0x' + block.number.toString('hex') - this._currentBlockNumber = blockNumber - - this.store.updateState({ currentBlockGasLimit: `0x${block.gasLimit.toString('hex')}` }) + this._currentBlockNumber = block.number + this.store.updateState({ currentBlockGasLimit: block.gasLimit }) async.parallel([ this._updateAccounts.bind(this), From b41aad6d1ae894ab89380b1c7159da8545ad935b Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 12:33:46 -0700 Subject: [PATCH 76/88] style - small whitespace nitpick --- app/scripts/lib/pending-tx-tracker.js | 2 +- test/unit/pending-tx-test.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 44e9d50fa..8da1253a2 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -55,7 +55,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { }) } - queryPendingTxs ({oldBlock, newBlock}) { + queryPendingTxs ({ oldBlock, newBlock }) { // check pending transactions on start if (!oldBlock) { this._checkPendingTxs() diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 8c6d287f8..7937afa46 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -84,14 +84,14 @@ describe('PendingTransactionTracker', function () { let newBlock, oldBlock newBlock = { number: '0x01' } pendingTxTracker._checkPendingTxs = done - pendingTxTracker.queryPendingTxs({oldBlock, newBlock}) + pendingTxTracker.queryPendingTxs({ oldBlock, newBlock }) }) it('should call #_checkPendingTxs if oldBlock and the newBlock have a diff of greater then 1', function (done) { let newBlock, oldBlock oldBlock = { number: '0x01' } newBlock = { number: '0x03' } pendingTxTracker._checkPendingTxs = done - pendingTxTracker.queryPendingTxs({oldBlock, newBlock}) + pendingTxTracker.queryPendingTxs({ oldBlock, newBlock }) }) it('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less', function (done) { let newBlock, oldBlock @@ -101,7 +101,7 @@ describe('PendingTransactionTracker', function () { const err = new Error('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less') done(err) } - pendingTxTracker.queryPendingTxs({oldBlock, newBlock}) + pendingTxTracker.queryPendingTxs({ oldBlock, newBlock }) done() }) }) From 7e9c6e96a1c0c7ef864bf9b5b04421369de71023 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 14:10:17 -0700 Subject: [PATCH 77/88] metamask - improve comment --- app/scripts/metamask-controller.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index dc39ad13e..5b3161bc6 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -232,8 +232,7 @@ module.exports = class MetamaskController extends EventEmitter { processTransaction: nodeify(async (txParams) => await this.txController.newUnapprovedTransaction(txParams), this), // old style msg signing processMessage: this.newUnsignedMessage.bind(this), - - // new style msg signing + // personal_sign msg signing processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), }) } From c781e11c7a4b8c6d0c74cc741af4d805d8bac00f Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 14:10:58 -0700 Subject: [PATCH 78/88] network - remove getter/setter --- app/scripts/controllers/network.js | 37 +++++++++++------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index dc9978043..253a365e2 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -11,8 +11,8 @@ const DEFAULT_RPC = RPC_ADDRESS_LIST['rinkeby'] module.exports = class NetworkController extends EventEmitter { constructor (config) { super() - this.networkStore = new ObservableStore('loading') config.provider.rpcTarget = this.getRpcAddressForType(config.provider.type, config.provider) + this.networkStore = new ObservableStore('loading') this.providerStore = new ObservableStore(config.provider) this.store = new ComposedStore({ provider: this.providerStore, network: this.networkStore }) this._providerListeners = {} @@ -21,42 +21,33 @@ module.exports = class NetworkController extends EventEmitter { this.providerStore.subscribe((state) => this.switchNetwork({rpcUrl: state.rpcTarget})) } - get provider () { - return this._proxy - } - - set provider (provider) { - this._provider = provider - } - initializeProvider (opts, providerContructor = MetaMaskProvider) { - this.providerInit = opts + this._providerInit = opts this._provider = providerContructor(opts) this._proxy = createEventEmitterProxy(this._provider) - this.provider._blockTracker = createEventEmitterProxy(this._provider._blockTracker) - this.provider.on('block', this._logBlock.bind(this)) - this.provider.on('error', this.verifyNetwork.bind(this)) - this.ethQuery = new EthQuery(this.provider) + this._proxy._blockTracker = createEventEmitterProxy(this._provider._blockTracker) + this._proxy.on('block', this._logBlock.bind(this)) + this._proxy.on('error', this.verifyNetwork.bind(this)) + this.ethQuery = new EthQuery(this._proxy) this.lookupNetwork() - return this.provider + return this._proxy } switchNetwork (providerInit) { this.setNetworkState('loading') - const newInit = extend(this.providerInit, providerInit) - this.providerInit = newInit + const newInit = extend(this._providerInit, providerInit) + this._providerInit = newInit - this._provider.removeAllListeners() - this._provider.stop() + this._proxy.removeAllListeners() + this._proxy.stop() this._provider = MetaMaskProvider(newInit) // apply the listners created by other controllers - const blockTrackerHandlers = this.provider._blockTracker.proxyEventHandlers - this.provider.setTarget(this._provider) - this.provider._blockTracker = createEventEmitterProxy(this._provider._blockTracker, blockTrackerHandlers) + const blockTrackerHandlers = this._proxy._blockTracker.proxyEventHandlers + this._proxy.setTarget(this._provider) + this._proxy._blockTracker = createEventEmitterProxy(this._provider._blockTracker, blockTrackerHandlers) this.emit('networkDidChange') } - verifyNetwork () { // Check network when restoring connectivity: if (this.isNetworkLoading()) this.lookupNetwork() From 7d499df8e32e85f5e4ed5c51f20496028d1dcdbb Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 14:12:45 -0700 Subject: [PATCH 79/88] account-tracker - remove unused import --- app/scripts/lib/account-tracker.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.js index a108d0d4f..cdc21282d 100644 --- a/app/scripts/lib/account-tracker.js +++ b/app/scripts/lib/account-tracker.js @@ -11,7 +11,6 @@ const async = require('async') const EthQuery = require('eth-query') const ObservableStore = require('obs-store') const EventEmitter = require('events').EventEmitter -const ethUtil = require('ethereumjs-util') function noop () {} From 96ebbde634c5f89793f72a2316a0c9aa2a38bc4b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:43:23 -0700 Subject: [PATCH 80/88] Fix Account Selection Do not select accounts on restore, only on creation and deliberate selection. Fixes #2164 --- app/scripts/controllers/preferences.js | 2 +- app/scripts/keyring-controller.js | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index e45224593..bc4848421 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -22,7 +22,7 @@ class PreferencesController { }) } - getSelectedAddress (_address) { + getSelectedAddress () { return this.store.getState().selectedAddress } diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 1a1904621..b5c51fd03 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -107,7 +107,6 @@ class KeyringController extends EventEmitter { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - First Account not found.') const hexAccount = normalizeAddress(firstAccount) - this.emit('newAccount', hexAccount) return this.setupAccounts(accounts) }) .then(this.persistAllKeyrings.bind(this, password)) @@ -207,6 +206,12 @@ class KeyringController extends EventEmitter { // and then saves those changes. addNewAccount (selectedKeyring) { return selectedKeyring.addAccounts(1) + .then((accounts) => { + accounts.forEach((hexAccount) => { + this.emit('newAccount', hexAccount) + }) + return accounts + }) .then(this.setupAccounts.bind(this)) .then(this.persistAllKeyrings.bind(this)) .then(this._updateMemStoreKeyrings.bind(this)) @@ -325,7 +330,6 @@ class KeyringController extends EventEmitter { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') const hexAccount = normalizeAddress(firstAccount) - this.emit('newAccount', hexAccount) this.emit('newVault', hexAccount) return this.setupAccounts(accounts) }) From f2d9b75e94ba52ce34faff0640f494028d037246 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 14:44:13 -0700 Subject: [PATCH 81/88] network controller - refactor to use _setProvider --- app/scripts/controllers/network.js | 45 ++++++++++++++++++----------- test/unit/network-contoller-test.js | 6 ++-- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index 253a365e2..1ed9f7eca 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -15,17 +15,16 @@ module.exports = class NetworkController extends EventEmitter { this.networkStore = new ObservableStore('loading') this.providerStore = new ObservableStore(config.provider) this.store = new ComposedStore({ provider: this.providerStore, network: this.networkStore }) - this._providerListeners = {} + this._proxy = createEventEmitterProxy() this.on('networkDidChange', this.lookupNetwork) - this.providerStore.subscribe((state) => this.switchNetwork({rpcUrl: state.rpcTarget})) + this.providerStore.subscribe((state) => this.switchNetwork({ rpcUrl: state.rpcTarget })) } initializeProvider (opts, providerContructor = MetaMaskProvider) { - this._providerInit = opts - this._provider = providerContructor(opts) - this._proxy = createEventEmitterProxy(this._provider) - this._proxy._blockTracker = createEventEmitterProxy(this._provider._blockTracker) + this._baseProviderParams = opts + const provider = providerContructor(opts) + this._setProvider(provider) this._proxy.on('block', this._logBlock.bind(this)) this._proxy.on('error', this.verifyNetwork.bind(this)) this.ethQuery = new EthQuery(this._proxy) @@ -33,21 +32,33 @@ module.exports = class NetworkController extends EventEmitter { return this._proxy } - switchNetwork (providerInit) { + switchNetwork (opts) { this.setNetworkState('loading') - const newInit = extend(this._providerInit, providerInit) - this._providerInit = newInit - - this._proxy.removeAllListeners() - this._proxy.stop() - this._provider = MetaMaskProvider(newInit) - // apply the listners created by other controllers - const blockTrackerHandlers = this._proxy._blockTracker.proxyEventHandlers - this._proxy.setTarget(this._provider) - this._proxy._blockTracker = createEventEmitterProxy(this._provider._blockTracker, blockTrackerHandlers) + const providerParams = extend(this._baseProviderParams, opts) + this._baseProviderParams = providerParams + const provider = MetaMaskProvider(providerParams) + this._setProvider(provider) this.emit('networkDidChange') } + _setProvider (provider) { + // collect old block tracker events + const oldProvider = this._provider + let blockTrackerHandlers + if (oldProvider) { + // capture old block handlers + blockTrackerHandlers = oldProvider._blockTracker.proxyEventHandlers + // tear down + oldProvider.removeAllListeners() + oldProvider.stop() + } + // override block tracler + provider._blockTracker = createEventEmitterProxy(provider._blockTracker, blockTrackerHandlers) + // set as new provider + this._provider = provider + this._proxy.setTarget(provider) + } + verifyNetwork () { // Check network when restoring connectivity: if (this.isNetworkLoading()) this.lookupNetwork() diff --git a/test/unit/network-contoller-test.js b/test/unit/network-contoller-test.js index c1fdaf032..0b3b5adeb 100644 --- a/test/unit/network-contoller-test.js +++ b/test/unit/network-contoller-test.js @@ -20,9 +20,9 @@ describe('# Network Controller', function () { describe('#provider', function () { it('provider should be updatable without reassignment', function () { networkController.initializeProvider(networkControllerProviderInit, dummyProviderConstructor) - const provider = networkController.provider - networkController.provider.setTarget({test: true, on: () => {}}) - assert.ok(provider.test) + const proxy = networkController._proxy + proxy.setTarget({ test: true, on: () => {} }) + assert.ok(proxy.test) }) }) describe('#getNetworkState', function () { From ea12be2c1bc488cfa5f85ced5472ec1515ddb00f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:44:44 -0700 Subject: [PATCH 82/88] Bump changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ad9888fd..5b54ed1a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Current Master +- Fix bug where newly created accounts were not selected. +- Fix bug where selected account was not persisted between lockings. + ## 3.10.5 2017-9-27 - Fix block gas limit estimation. From 06b5dd2096b6bfcdf7d9ebf7c9bb1e40c8aed2e0 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 14:44:54 -0700 Subject: [PATCH 83/88] network controller - move _setProvider to bottom --- app/scripts/controllers/network.js | 36 +++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index 1ed9f7eca..2a17cdae8 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -41,24 +41,6 @@ module.exports = class NetworkController extends EventEmitter { this.emit('networkDidChange') } - _setProvider (provider) { - // collect old block tracker events - const oldProvider = this._provider - let blockTrackerHandlers - if (oldProvider) { - // capture old block handlers - blockTrackerHandlers = oldProvider._blockTracker.proxyEventHandlers - // tear down - oldProvider.removeAllListeners() - oldProvider.stop() - } - // override block tracler - provider._blockTracker = createEventEmitterProxy(provider._blockTracker, blockTrackerHandlers) - // set as new provider - this._provider = provider - this._proxy.setTarget(provider) - } - verifyNetwork () { // Check network when restoring connectivity: if (this.isNetworkLoading()) this.lookupNetwork() @@ -112,6 +94,24 @@ module.exports = class NetworkController extends EventEmitter { return provider && provider.rpcTarget ? provider.rpcTarget : DEFAULT_RPC } + _setProvider (provider) { + // collect old block tracker events + const oldProvider = this._provider + let blockTrackerHandlers + if (oldProvider) { + // capture old block handlers + blockTrackerHandlers = oldProvider._blockTracker.proxyEventHandlers + // tear down + oldProvider.removeAllListeners() + oldProvider.stop() + } + // override block tracler + provider._blockTracker = createEventEmitterProxy(provider._blockTracker, blockTrackerHandlers) + // set as new provider + this._provider = provider + this._proxy.setTarget(provider) + } + _logBlock (block) { log.info(`BLOCK CHANGED: #${block.number.toString('hex')} 0x${block.hash.toString('hex')}`) this.verifyNetwork() From aefd17ef9418148c1a35dbf72da7991366649f5c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:45:24 -0700 Subject: [PATCH 84/88] Remove dead reference --- app/scripts/keyring-controller.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index b5c51fd03..4627b850e 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -106,7 +106,6 @@ class KeyringController extends EventEmitter { .then((accounts) => { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - First Account not found.') - const hexAccount = normalizeAddress(firstAccount) return this.setupAccounts(accounts) }) .then(this.persistAllKeyrings.bind(this, password)) From c0d7d447647d779deafb8b09a3f7a1d523c322b6 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Wed, 27 Sep 2017 21:54:46 +0000 Subject: [PATCH 85/88] fix(package): update eth-keyring-controller to version 2.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c160cbfde..052d20a22 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", "eth-json-rpc-filters": "^1.2.1", - "eth-keyring-controller": "^1.0.1", + "eth-keyring-controller": "^2.0.0", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", From a246770866d242404ce275517c9223fc922e4312 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:55:34 -0700 Subject: [PATCH 86/88] Commit to the eth-keyring-controller module --- app/scripts/keyring-controller.js | 599 ------------------------------ package.json | 2 +- 2 files changed, 1 insertion(+), 600 deletions(-) delete mode 100644 app/scripts/keyring-controller.js diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js deleted file mode 100644 index 4627b850e..000000000 --- a/app/scripts/keyring-controller.js +++ /dev/null @@ -1,599 +0,0 @@ -const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN -const bip39 = require('bip39') -const EventEmitter = require('events').EventEmitter -const ObservableStore = require('obs-store') -const filter = require('promise-filter') -const encryptor = require('browser-passworder') -const sigUtil = require('eth-sig-util') -const normalizeAddress = sigUtil.normalize -// Keyrings: -const SimpleKeyring = require('eth-simple-keyring') -const HdKeyring = require('eth-hd-keyring') -const keyringTypes = [ - SimpleKeyring, - HdKeyring, -] - -class KeyringController extends EventEmitter { - - // PUBLIC METHODS - // - // THE FIRST SECTION OF METHODS ARE PUBLIC-FACING, - // MEANING THEY ARE USED BY CONSUMERS OF THIS CLASS. - // - // THEIR SURFACE AREA SHOULD BE CHANGED WITH GREAT CARE. - - constructor (opts) { - super() - const initState = opts.initState || {} - this.keyringTypes = keyringTypes - this.store = new ObservableStore(initState) - this.memStore = new ObservableStore({ - isUnlocked: false, - keyringTypes: this.keyringTypes.map(krt => krt.type), - keyrings: [], - identities: {}, - }) - - this.accountTracker = opts.accountTracker - this.encryptor = opts.encryptor || encryptor - this.keyrings = [] - this.getNetwork = opts.getNetwork - } - - // Full Update - // returns Promise( @object state ) - // - // Emits the `update` event and - // returns a Promise that resolves to the current state. - // - // Frequently used to end asynchronous chains in this class, - // indicating consumers can often either listen for updates, - // or accept a state-resolving promise to consume their results. - // - // Not all methods end with this, that might be a nice refactor. - fullUpdate () { - this.emit('update') - return Promise.resolve(this.memStore.getState()) - } - - // Create New Vault And Keychain - // @string password - The password to encrypt the vault with - // - // returns Promise( @object state ) - // - // Destroys any old encrypted storage, - // creates a new encrypted store with the given password, - // randomly creates a new HD wallet with 1 account, - // faucets that account on the testnet. - createNewVaultAndKeychain (password) { - return this.persistAllKeyrings(password) - .then(this.createFirstKeyTree.bind(this)) - .then(this.fullUpdate.bind(this)) - } - - // CreateNewVaultAndRestore - // @string password - The password to encrypt the vault with - // @string seed - The BIP44-compliant seed phrase. - // - // returns Promise( @object state ) - // - // Destroys any old encrypted storage, - // creates a new encrypted store with the given password, - // creates a new HD wallet from the given seed with 1 account. - createNewVaultAndRestore (password, seed) { - if (typeof password !== 'string') { - return Promise.reject('Password must be text.') - } - - if (!bip39.validateMnemonic(seed)) { - return Promise.reject(new Error('Seed phrase is invalid.')) - } - - this.clearKeyrings() - - return this.persistAllKeyrings(password) - .then(() => { - return this.addNewKeyring('HD Key Tree', { - mnemonic: seed, - numberOfAccounts: 1, - }) - }) - .then((firstKeyring) => { - return firstKeyring.getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] - if (!firstAccount) throw new Error('KeyringController - First Account not found.') - return this.setupAccounts(accounts) - }) - .then(this.persistAllKeyrings.bind(this, password)) - .then(this.fullUpdate.bind(this)) - } - - // Set Locked - // returns Promise( @object state ) - // - // This method deallocates all secrets, and effectively locks metamask. - setLocked () { - // set locked - this.password = null - this.memStore.updateState({ isUnlocked: false }) - // remove keyrings - this.keyrings = [] - this._updateMemStoreKeyrings() - return this.fullUpdate() - } - - // Submit Password - // @string password - // - // returns Promise( @object state ) - // - // Attempts to decrypt the current vault and load its keyrings - // into memory. - // - // Temporarily also migrates any old-style vaults first, as well. - // (Pre MetaMask 3.0.0) - submitPassword (password) { - return this.unlockKeyrings(password) - .then((keyrings) => { - this.keyrings = keyrings - return this.fullUpdate() - }) - } - - // Add New Keyring - // @string type - // @object opts - // - // returns Promise( @Keyring keyring ) - // - // Adds a new Keyring of the given `type` to the vault - // and the current decrypted Keyrings array. - // - // All Keyring classes implement a unique `type` string, - // and this is used to retrieve them from the keyringTypes array. - addNewKeyring (type, opts) { - const Keyring = this.getKeyringClassForType(type) - const keyring = new Keyring(opts) - return keyring.deserialize(opts) - .then(() => { - return keyring.getAccounts() - }) - .then((accounts) => { - return this.checkForDuplicate(type, accounts) - }) - .then((checkedAccounts) => { - this.keyrings.push(keyring) - return this.setupAccounts(checkedAccounts) - }) - .then(() => this.persistAllKeyrings()) - .then(() => this._updateMemStoreKeyrings()) - .then(() => this.fullUpdate()) - .then(() => { - return keyring - }) - } - - // For now just checks for simple key pairs - // but in the future - // should possibly add HD and other types - // - checkForDuplicate (type, newAccount) { - return this.getAccounts() - .then((accounts) => { - switch (type) { - case 'Simple Key Pair': - const isNotIncluded = !accounts.find((key) => key === newAccount[0] || key === ethUtil.stripHexPrefix(newAccount[0])) - return (isNotIncluded) ? Promise.resolve(newAccount) : Promise.reject(new Error('The account you\'re are trying to import is a duplicate')) - default: - return Promise.resolve(newAccount) - } - }) - } - - - // Add New Account - // @number keyRingNum - // - // returns Promise( @object state ) - // - // Calls the `addAccounts` method on the Keyring - // in the kryings array at index `keyringNum`, - // and then saves those changes. - addNewAccount (selectedKeyring) { - return selectedKeyring.addAccounts(1) - .then((accounts) => { - accounts.forEach((hexAccount) => { - this.emit('newAccount', hexAccount) - }) - return accounts - }) - .then(this.setupAccounts.bind(this)) - .then(this.persistAllKeyrings.bind(this)) - .then(this._updateMemStoreKeyrings.bind(this)) - .then(this.fullUpdate.bind(this)) - } - - // Save Account Label - // @string account - // @string label - // - // returns Promise( @string label ) - // - // Persists a nickname equal to `label` for the specified account. - saveAccountLabel (account, label) { - try { - const hexAddress = normalizeAddress(account) - // update state on diskStore - const state = this.store.getState() - const walletNicknames = state.walletNicknames || {} - walletNicknames[hexAddress] = label - this.store.updateState({ walletNicknames }) - // update state on memStore - const identities = this.memStore.getState().identities - identities[hexAddress].name = label - this.memStore.updateState({ identities }) - return Promise.resolve(label) - } catch (err) { - return Promise.reject(err) - } - } - - // Export Account - // @string address - // - // returns Promise( @string privateKey ) - // - // Requests the private key from the keyring controlling - // the specified address. - // - // Returns a Promise that may resolve with the private key string. - exportAccount (address) { - try { - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.exportAccount(normalizeAddress(address)) - }) - } catch (e) { - return Promise.reject(e) - } - } - - - // SIGNING METHODS - // - // This method signs tx and returns a promise for - // TX Manager to update the state after signing - - signTransaction (ethTx, _fromAddress) { - const fromAddress = normalizeAddress(_fromAddress) - return this.getKeyringForAccount(fromAddress) - .then((keyring) => { - return keyring.signTransaction(fromAddress, ethTx) - }) - } - - // Sign Message - // @object msgParams - // - // returns Promise(@buffer rawSig) - // - // Attempts to sign the provided @object msgParams. - signMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signMessage(address, msgParams.data) - }) - } - - // Sign Personal Message - // @object msgParams - // - // returns Promise(@buffer rawSig) - // - // Attempts to sign the provided @object msgParams. - // Prefixes the hash before signing as per the new geth behavior. - signPersonalMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signPersonalMessage(address, msgParams.data) - }) - } - - // PRIVATE METHODS - // - // THESE METHODS ARE ONLY USED INTERNALLY TO THE KEYRING-CONTROLLER - // AND SO MAY BE CHANGED MORE LIBERALLY THAN THE ABOVE METHODS. - - // Create First Key Tree - // returns @Promise - // - // Clears the vault, - // creates a new one, - // creates a random new HD Keyring with 1 account, - // makes that account the selected account, - // faucets that account on testnet, - // puts the current seed words into the state tree. - createFirstKeyTree () { - this.clearKeyrings() - return this.addNewKeyring('HD Key Tree', { numberOfAccounts: 1 }) - .then((keyring) => { - return keyring.getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] - if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') - const hexAccount = normalizeAddress(firstAccount) - this.emit('newVault', hexAccount) - return this.setupAccounts(accounts) - }) - .then(this.persistAllKeyrings.bind(this)) - } - - // Setup Accounts - // @array accounts - // - // returns @Promise(@object account) - // - // Initializes the provided account array - // Gives them numerically incremented nicknames, - // and adds them to the accountTracker for regular balance checking. - setupAccounts (accounts) { - return this.getAccounts() - .then((loadedAccounts) => { - const arr = accounts || loadedAccounts - return Promise.all(arr.map((account) => { - return this.getBalanceAndNickname(account) - })) - }) - } - - // Get Balance And Nickname - // @string account - // - // returns Promise( @string label ) - // - // 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 = normalizeAddress(account) - this.accountTracker.addAccount(address) - return this.createNickname(address) - } - - // Create Nickname - // @string address - // - // returns Promise( @string label ) - // - // Takes an address, and assigns it an incremented nickname, persisting it. - createNickname (address) { - const hexAddress = normalizeAddress(address) - const identities = this.memStore.getState().identities - const currentIdentityCount = Object.keys(identities).length + 1 - const nicknames = this.store.getState().walletNicknames || {} - const existingNickname = nicknames[hexAddress] - const name = existingNickname || `Account ${currentIdentityCount}` - identities[hexAddress] = { - address: hexAddress, - name, - } - this.memStore.updateState({ identities }) - return this.saveAccountLabel(hexAddress, name) - } - - // Persist All Keyrings - // @password string - // - // returns Promise - // - // Iterates the current `keyrings` array, - // serializes each one into a serialized array, - // encrypts that array with the provided `password`, - // and persists that encrypted string to storage. - persistAllKeyrings (password = this.password) { - if (typeof password === 'string') { - this.password = password - this.memStore.updateState({ isUnlocked: true }) - } - return 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) - }) - .then((encryptedString) => { - this.store.updateState({ vault: encryptedString }) - return true - }) - } - - // Unlock Keyrings - // @string password - // - // returns Promise( @array keyrings ) - // - // Attempts to unlock the persisted encrypted storage, - // initializing the persisted keyrings to RAM. - unlockKeyrings (password) { - const encryptedVault = this.store.getState().vault - if (!encryptedVault) { - throw new Error('Cannot unlock without a previous vault.') - } - - return this.encryptor.decrypt(password, encryptedVault) - .then((vault) => { - this.password = password - this.memStore.updateState({ isUnlocked: true }) - vault.forEach(this.restoreKeyring.bind(this)) - return this.keyrings - }) - } - - // Restore Keyring - // @object serialized - // - // returns Promise( @Keyring deserialized ) - // - // Attempts to initialize a new keyring from the provided - // serialized payload. - // - // On success, returns the resulting @Keyring instance. - restoreKeyring (serialized) { - const { type, data } = serialized - - const Keyring = this.getKeyringClassForType(type) - const keyring = new Keyring() - return keyring.deserialize(data) - .then(() => { - return keyring.getAccounts() - }) - .then((accounts) => { - return this.setupAccounts(accounts) - }) - .then(() => { - this.keyrings.push(keyring) - this._updateMemStoreKeyrings() - return keyring - }) - } - - // Get Keyring Class For Type - // @string type - // - // Returns @class Keyring - // - // Searches the current `keyringTypes` array - // for a Keyring class whose unique `type` property - // matches the provided `type`, - // returning it if it exists. - getKeyringClassForType (type) { - return this.keyringTypes.find(kr => kr.type === type) - } - - getKeyringsByType (type) { - return this.keyrings.filter((keyring) => keyring.type === type) - } - - // Get Accounts - // returns Promise( @Array[ @string accounts ] ) - // - // Returns the public addresses of all current accounts - // managed by all currently unlocked keyrings. - getAccounts () { - const keyrings = this.keyrings || [] - return Promise.all(keyrings.map(kr => kr.getAccounts())) - .then((keyringArrays) => { - return keyringArrays.reduce((res, arr) => { - return res.concat(arr) - }, []) - }) - } - - // Get Keyring For Account - // @string address - // - // returns Promise(@Keyring keyring) - // - // Returns the currently initialized keyring that manages - // the specified `address` if one exists. - getKeyringForAccount (address) { - const hexed = normalizeAddress(address) - log.debug(`KeyringController - getKeyringForAccount: ${hexed}`) - - return Promise.all(this.keyrings.map((keyring) => { - return Promise.all([ - keyring, - keyring.getAccounts(), - ]) - })) - .then(filter((candidate) => { - const accounts = candidate[1].map(normalizeAddress) - return accounts.includes(hexed) - })) - .then((winners) => { - if (winners && winners.length > 0) { - return winners[0][0] - } else { - throw new Error('No keyring found for the requested account.') - } - }) - } - - // Display For Keyring - // @Keyring keyring - // - // returns Promise( @Object { type:String, accounts:Array } ) - // - // Is used for adding the current keyrings to the state object. - displayForKeyring (keyring) { - return keyring.getAccounts() - .then((accounts) => { - return { - type: keyring.type, - accounts: accounts, - } - }) - } - - // Add Gas Buffer - // @string gas (as hexadecimal value) - // - // returns @string bufferedGas (as hexadecimal value) - // - // Adds a healthy buffer of gas to an initial gas estimate. - addGasBuffer (gas) { - const gasBuffer = new BN('100000', 10) - const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const correct = bnGas.add(gasBuffer) - return ethUtil.addHexPrefix(correct.toString(16)) - } - - // Clear Keyrings - // - // Deallocates all currently managed keyrings and accounts. - // Used before initializing a new vault. - clearKeyrings () { - let accounts - try { - accounts = Object.keys(this.accountTracker.store.getState()) - } catch (e) { - accounts = [] - } - accounts.forEach((address) => { - this.accountTracker.removeAccount(address) - }) - - // clear keyrings from memory - this.keyrings = [] - this.memStore.updateState({ - keyrings: [], - identities: {}, - }) - } - - _updateMemStoreKeyrings () { - Promise.all(this.keyrings.map(this.displayForKeyring)) - .then((keyrings) => { - this.memStore.updateState({ keyrings }) - }) - } - -} - -module.exports = KeyringController diff --git a/package.json b/package.json index c160cbfde..052d20a22 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", "eth-json-rpc-filters": "^1.2.1", - "eth-keyring-controller": "^1.0.1", + "eth-keyring-controller": "^2.0.0", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", From cd0f44e2d6e1c54dc929ad35df08c04422e55b32 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 27 Sep 2017 14:59:10 -0700 Subject: [PATCH 87/88] deps - bump express for security fix --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c160cbfde..60d1ad1eb 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "ethjs-contract": "^0.1.9", "ethjs-ens": "^2.0.0", "ethjs-query": "^0.2.9", - "express": "^4.14.0", + "express": "^4.15.5", "extension-link-enabler": "^1.0.0", "extensionizer": "^1.0.0", "fast-json-patch": "^2.0.4", From b473d440a36d7715582431ff0024d16e4da3bc36 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 15:14:40 -0700 Subject: [PATCH 88/88] Version 3.10.6 --- CHANGELOG.md | 2 ++ app/manifest.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b54ed1a6..599f340f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +## 3.10.6 2017-9-27 + - Fix bug where newly created accounts were not selected. - Fix bug where selected account was not persisted between lockings. diff --git a/app/manifest.json b/app/manifest.json index 4d02cd334..639f3fb4b 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.10.5", + "version": "3.10.6", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension",