From 090935f90aa3c2589fee7bc038c8f4fcf77da03c Mon Sep 17 00:00:00 2001 From: Frances Pangilinan Date: Wed, 14 Dec 2016 12:55:41 -0800 Subject: Create a TxManager --- app/scripts/background.js | 26 +++++- app/scripts/keyring-controller.js | 118 ++++-------------------- app/scripts/lib/config-manager.js | 51 +---------- app/scripts/lib/idStore.js | 13 ++- app/scripts/lib/provider-utils.js | 106 ++++++++++++++++++++++ app/scripts/metamask-controller.js | 16 +++- app/scripts/transaction-manager.js | 179 +++++++++++++++++++++++++++++++++++++ 7 files changed, 350 insertions(+), 159 deletions(-) create mode 100644 app/scripts/lib/provider-utils.js create mode 100644 app/scripts/transaction-manager.js (limited to 'app') diff --git a/app/scripts/background.js b/app/scripts/background.js index 7cb25d8bf..854b679da 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -23,7 +23,7 @@ const controller = new MetamaskController({ loadData, }) const keyringController = controller.keyringController - +const txManager = controller.txManager function triggerUi () { if (!popupIsOpen) notification.show() } @@ -97,12 +97,11 @@ function setupControllerConnection (stream) { // plugin badge text // -keyringController.on('update', updateBadge) +txManager.on('update', updateBadge) function updateBadge () { var label = '' - var unconfTxs = controller.configManager.unconfirmedTxs() - var unconfTxLen = Object.keys(unconfTxs).length + var unconfTxLen = controller.txManager.unConftxCount var unconfMsgs = messageManager.unconfirmedMsgs() var unconfMsgLen = Object.keys(unconfMsgs).length var count = unconfTxLen + unconfMsgLen @@ -113,6 +112,25 @@ function updateBadge () { extension.browserAction.setBadgeBackgroundColor({ color: '#506F8B' }) } +// txManger :: tx approvals and rejection cb's + +txManager.on('signed', function (txId) { + var approvalCb = this._unconfTxCbs[txId] + + approvalCb(null, true) + // clean up + delete this._unconfTxCbs[txId] +}) + +txManager.on('rejected', function (txId) { + var approvalCb = this._unconfTxCbs[txId] + approvalCb(null, false) + // clean up + delete this._unconfTxCbs[txId] +}) + +// data :: setters/getters + function loadData () { var oldData = getOldStyleData() var newData diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 40c9695dd..37b3a947f 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -1,6 +1,4 @@ -const async = require('async') const ethUtil = require('ethereumjs-util') -const EthQuery = require('eth-query') const bip39 = require('bip39') const Transaction = require('ethereumjs-tx') const EventEmitter = require('events').EventEmitter @@ -36,7 +34,7 @@ module.exports = class KeyringController extends EventEmitter { this.ethStore = opts.ethStore this.encryptor = encryptor this.keyringTypes = keyringTypes - + this.txManager = opts.txManager this.keyrings = [] this.identities = {} // Essentially a name hash @@ -73,7 +71,7 @@ module.exports = class KeyringController extends EventEmitter { // or accept a state-resolving promise to consume their results. // // Not all methods end with this, that might be a nice refactor. - fullUpdate() { + fullUpdate () { this.emit('update') return Promise.resolve(this.getState()) } @@ -102,8 +100,8 @@ module.exports = class KeyringController extends EventEmitter { isInitialized: (!!wallet || !!vault), isUnlocked: Boolean(this.password), isDisclaimerConfirmed: this.configManager.getConfirmedDisclaimer(), // AUDIT this.configManager.getConfirmedDisclaimer(), - unconfTxs: this.configManager.unconfirmedTxs(), - transactions: this.configManager.getTxList(), + transactions: this.txManager.getTxList(), + unconfTxs: this.txManager.getUnapprovedTxList(), unconfMsgs: messageManager.unconfirmedMsgs(), messages: messageManager.getMsgList(), selectedAccount: address, @@ -341,7 +339,7 @@ module.exports = class KeyringController extends EventEmitter { // Caches the requesting Dapp's callback, `onTxDoneCb`, for resolution later. addUnconfirmedTransaction (txParams, onTxDoneCb, cb) { const configManager = this.configManager - + const txManager = this.txManager // create txData obj with parameters and meta data var time = (new Date()).getTime() var txId = createId() @@ -351,95 +349,26 @@ module.exports = class KeyringController extends EventEmitter { id: txId, txParams: txParams, time: time, - status: 'unconfirmed', + status: 'unapproved', gasMultiplier: configManager.getGasMultiplier() || 1, metamaskNetworkId: this.getNetwork(), } - // keep the onTxDoneCb around for after approval/denial (requires user interaction) // This onTxDoneCb fires completion to the Dapp's write operation. - this._unconfTxCbs[txId] = onTxDoneCb - - var provider = this.ethStore._query.currentProvider - var query = new EthQuery(provider) - + txManager.txProviderUtils.analyzeGasUsage(txData, this.txDidComplete.bind(this, txData, onTxDoneCb, cb)) // calculate metadata for tx - this.analyzeTxGasUsage(query, txData, this.txDidComplete.bind(this, txData, cb)) - } - - estimateTxGas (query, txData, blockGasLimitHex, cb) { - const txParams = txData.txParams - // check if gasLimit is already specified - txData.gasLimitSpecified = Boolean(txParams.gas) - // if not, fallback to block gasLimit - if (!txData.gasLimitSpecified) { - txParams.gas = blockGasLimitHex - } - // run tx, see if it will OOG - query.estimateGas(txParams, cb) - } - - checkForTxGasError (txData, estimatedGasHex, cb) { - txData.estimatedGas = estimatedGasHex - // all gas used - must be an error - if (estimatedGasHex === txData.txParams.gas) { - txData.simulationFails = true - } - cb() - } - - setTxGas (txData, blockGasLimitHex, cb) { - const txParams = txData.txParams - // if OOG, nothing more to do - if (txData.simulationFails) { - cb() - return - } - // if gasLimit was specified and doesnt OOG, - // use original specified amount - if (txData.gasLimitSpecified) { - txData.estimatedGas = txParams.gas - cb() - return - } - // if gasLimit not originally specified, - // try adding an additional gas buffer to our estimation for safety - const estimatedGasBn = new BN(ethUtil.stripHexPrefix(txData.estimatedGas), 16) - const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) - const estimationWithBuffer = new BN(this.addGasBuffer(estimatedGasBn), 16) - // added gas buffer is too high - if (estimationWithBuffer.gt(blockGasLimitBn)) { - txParams.gas = txData.estimatedGas - // added gas buffer is safe - } else { - const gasWithBufferHex = ethUtil.intToHex(estimationWithBuffer) - txParams.gas = gasWithBufferHex - } - cb() - return } - txDidComplete (txData, cb, err) { + txDidComplete (txData, onTxDoneCb, cb, err) { if (err) return cb(err) - const configManager = this.configManager - configManager.addTx(txData) + const txManager = this.txManager + txManager.addTx(txData, onTxDoneCb) // signal update this.emit('update') // signal completion of add tx cb(null, txData) } - analyzeTxGasUsage (query, txData, cb) { - query.getBlockByNumber('latest', true, (err, block) => { - if (err) return cb(err) - async.waterfall([ - this.estimateTxGas.bind(this, query, txData, block.gasLimit), - this.checkForTxGasError.bind(this, txData), - this.setTxGas.bind(this, txData, block.gasLimit), - ], cb) - }) - } - // Cancel Transaction // @string txId // @function cb @@ -448,14 +377,8 @@ module.exports = class KeyringController extends EventEmitter { // // Forgets any tx matching `txId`. cancelTransaction (txId, cb) { - const configManager = this.configManager - var approvalCb = this._unconfTxCbs[txId] || noop - - // reject tx - approvalCb(null, false) - // clean up - configManager.rejectTx(txId) - delete this._unconfTxCbs[txId] + const txManager = this.txManager + txManager.setTxStatusRejected(txId) if (cb && typeof cb === 'function') { cb() @@ -473,16 +396,10 @@ module.exports = class KeyringController extends EventEmitter { // // Calls back the cached Dapp's confirmation callback, also. approveTransaction (txId, cb) { - const configManager = this.configManager - var approvalCb = this._unconfTxCbs[txId] || noop - - // accept tx - cb() - approvalCb(null, true) - // clean up - configManager.confirmTx(txId) - delete this._unconfTxCbs[txId] + const txManager = this.txManager + txManager.setTxStatusSigned(txId) this.emit('update') + cb() } signTransaction (txParams, cb) { @@ -510,9 +427,9 @@ module.exports = class KeyringController extends EventEmitter { .then((tx) => { // Add the tx hash to the persisted meta-tx object var txHash = ethUtil.bufferToHex(tx.hash()) - var metaTx = this.configManager.getTx(txParams.metamaskId) + var metaTx = this.txManager.getTx(txParams.metamaskId) metaTx.hash = txHash - this.configManager.updateTx(metaTx) + this.txManager.updateTx(metaTx) // return raw serialized tx var rawTx = ethUtil.bufferToHex(tx.serialize()) @@ -586,7 +503,6 @@ module.exports = class KeyringController extends EventEmitter { // Attempts to sign the provided @object msgParams. signMessage (msgParams, cb) { try { - const msgId = msgParams.metamaskId delete msgParams.metamaskId const approvalCb = this._unconfMsgCbs[msgId] || noop diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js index 59cc2b63c..913a76a6e 100644 --- a/app/scripts/lib/config-manager.js +++ b/app/scripts/lib/config-manager.js @@ -209,61 +209,12 @@ ConfigManager.prototype.getTxList = function () { } } -ConfigManager.prototype.unconfirmedTxs = function () { - var transactions = this.getTxList() - return transactions.filter(tx => tx.status === 'unconfirmed') - .reduce((result, tx) => { result[tx.id] = tx; return result }, {}) -} - -ConfigManager.prototype._saveTxList = function (txList) { +ConfigManager.prototype.setTxList = function (txList) { var data = this.migrator.getData() data.transactions = txList this.setData(data) } -ConfigManager.prototype.addTx = function (tx) { - var transactions = this.getTxList() - while (transactions.length > this.txLimit - 1) { - transactions.shift() - } - transactions.push(tx) - this._saveTxList(transactions) -} - -ConfigManager.prototype.getTx = function (txId) { - var transactions = this.getTxList() - var matching = transactions.filter(tx => tx.id === txId) - return matching.length > 0 ? matching[0] : null -} - -ConfigManager.prototype.confirmTx = function (txId) { - this._setTxStatus(txId, 'confirmed') -} - -ConfigManager.prototype.rejectTx = function (txId) { - this._setTxStatus(txId, 'rejected') -} - -ConfigManager.prototype._setTxStatus = function (txId, status) { - var tx = this.getTx(txId) - tx.status = status - this.updateTx(tx) -} - -ConfigManager.prototype.updateTx = function (tx) { - var transactions = this.getTxList() - var found, index - transactions.forEach((otherTx, i) => { - if (otherTx.id === tx.id) { - found = true - index = i - } - }) - if (found) { - transactions[index] = tx - } - this._saveTxList(transactions) -} // wallet nickname methods diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index d36504f13..71bee8026 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -13,6 +13,8 @@ const autoFaucet = require('./auto-faucet') const messageManager = require('./message-manager') const DEFAULT_RPC = 'https://testrpc.metamask.io/' const IdManagement = require('./id-management') +const TxManager = require('../transaction-manager') + module.exports = IdentityStore @@ -36,6 +38,11 @@ function IdentityStore (opts = {}) { } // not part of serilized metamask state - only kept in memory + this.txManager = new TxManager({ + TxListFromStore: opts.configManager.getTxList(), + setTxList: opts.configManager.setTxList.bind(opts.configManager), + txLimit: 40, + }) this._unconfTxCbs = {} this._unconfMsgCbs = {} } @@ -87,6 +94,7 @@ IdentityStore.prototype.recoverFromSeed = function (password, seed, cb) { IdentityStore.prototype.setStore = function (store) { this._ethStore = store + this.txManager.setProvider(this._ethStore._query.currentProvider) } IdentityStore.prototype.clearSeedWordCache = function (cb) { @@ -97,14 +105,15 @@ IdentityStore.prototype.clearSeedWordCache = function (cb) { IdentityStore.prototype.getState = function () { const configManager = this.configManager + const TxManager = this.txManager var seedWords = this.getSeedIfUnlocked() return clone(extend(this._currentState, { isInitialized: !!configManager.getWallet() && !seedWords, isUnlocked: this._isUnlocked(), seedWords: seedWords, isDisclaimerConfirmed: configManager.getConfirmedDisclaimer(), - unconfTxs: configManager.unconfirmedTxs(), - transactions: configManager.getTxList(), + unconfTxs: TxManager.getUnapprovedTxList(), + transactions: TxManager.getTxList(), unconfMsgs: messageManager.unconfirmedMsgs(), messages: messageManager.getMsgList(), selectedAddress: configManager.getSelectedAccount(), diff --git a/app/scripts/lib/provider-utils.js b/app/scripts/lib/provider-utils.js new file mode 100644 index 000000000..d1678c964 --- /dev/null +++ b/app/scripts/lib/provider-utils.js @@ -0,0 +1,106 @@ +const async = require('async') +const EthQuery = require('eth-query') +const ethUtil = require('ethereumjs-util') +const BN = ethUtil.BN +const ethBinToOps = require('eth-bin-to-ops') + +module.exports = class txProviderUtils { + constructor (provider) { + this.provider = provider + this.query = new EthQuery(provider) + } + analyzeGasUsage (txData, cb) { + var self = this + this.query.getBlockByNumber('latest', true, (err, block) => { + if (err) return cb(err) + async.waterfall([ + self.estimateTxGas.bind(self, txData, block.gasLimit), + self.checkForTxGasError.bind(self, txData), + self.setTxGas.bind(self, txData, block.gasLimit), + ], cb) + }) + } + + // perform static analyis on the target contract code + analyzeForDelegateCall (txParams, cb) { + if (txParams.to) { + this.query.getCode(txParams.to, function (err, result) { + if (err) return cb(err) + + var code = ethUtil.toBuffer(result) + if (code !== '0x') { + var ops = ethBinToOps(code) + var containsDelegateCall = ops.some((op) => op.name === 'DELEGATECALL') + cb(containsDelegateCall) + } else { + cb() + } + }) + } else { + cb() + } + } + + estimateTxGas (txData, blockGasLimitHex, cb) { + const txParams = txData.txParams + // check if gasLimit is already specified + txData.gasLimitSpecified = Boolean(txParams.gas) + // if not, fallback to block gasLimit + if (!txData.gasLimitSpecified) { + txParams.gas = blockGasLimitHex + } + // run tx, see if it will OOG + this.query.estimateGas(txParams, cb) + } + + checkForTxGasError (txData, estimatedGasHex, cb) { + txData.estimatedGas = estimatedGasHex + // all gas used - must be an error + if (estimatedGasHex === txData.txParams.gas) { + txData.simulationFails = true + } + cb() + } + + handleFork (block) { + + } + + setTxGas (txData, blockGasLimitHex, cb) { + const txParams = txData.txParams + // if OOG, nothing more to do + if (txData.simulationFails) { + cb() + return + } + // if gasLimit was specified and doesnt OOG, + // use original specified amount + if (txData.gasLimitSpecified) { + txData.estimatedGas = txParams.gas + cb() + return + } + // if gasLimit not originally specified, + // try adding an additional gas buffer to our estimation for safety + const estimatedGasBn = new BN(ethUtil.stripHexPrefix(txData.estimatedGas), 16) + const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) + const estimationWithBuffer = new BN(this.addGasBuffer(estimatedGasBn), 16) + // added gas buffer is too high + if (estimationWithBuffer.gt(blockGasLimitBn)) { + txParams.gas = txData.estimatedGas + // added gas buffer is safe + } else { + const gasWithBufferHex = ethUtil.intToHex(estimationWithBuffer) + txParams.gas = gasWithBufferHex + } + cb() + return + } + + 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)) + } +} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ae761c753..3b70f63db 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -3,6 +3,7 @@ const EthStore = require('eth-store') const MetaMaskProvider = require('web3-provider-engine/zero.js') const KeyringController = require('./keyring-controller') const messageManager = require('./lib/message-manager') +const TxManager = require('./transaction-manager') const HostStore = require('./lib/remote-store.js').HostStore const Web3 = require('web3') const ConfigManager = require('./lib/config-manager') @@ -18,13 +19,21 @@ module.exports = class MetamaskController { this.opts = opts this.listeners = [] this.configManager = new ConfigManager(opts) + this.txManager = new TxManager({ + TxListFromStore: this.configManager.getTxList(), + txLimit: this.configManager.txLimit, + setTxList: this.configManager.setTxList.bind(this.configManager), + }) + this.keyringController = new KeyringController({ configManager: this.configManager, + txManager: this.txManager, getNetwork: this.getStateNetwork.bind(this), }) this.provider = this.initializeProvider(opts) this.ethStore = new EthStore(this.provider) this.keyringController.setStore(this.ethStore) + this.txManager.setProvider(this.provider) this.getNetwork() this.messageManager = messageManager this.publicConfigStore = this.initPublicConfigStore() @@ -49,7 +58,7 @@ module.exports = class MetamaskController { getApi () { const keyringController = this.keyringController - + const txManager = this.txManager return { getState: (cb) => { cb(null, this.getState()) }, setRpcTarget: this.setRpcTarget.bind(this), @@ -81,6 +90,9 @@ module.exports = class MetamaskController { signMessage: keyringController.signMessage.bind(keyringController), cancelMessage: keyringController.cancelMessage.bind(keyringController), + // forward directly to txManager + getUnapprovedTxList: txManager.getTxList.bind(txManager), + getFilterdTxList: txManager.getFilterdTxList.bind(txManager), // coinbase buyEth: this.buyEth.bind(this), // shapeshift @@ -154,7 +166,7 @@ module.exports = class MetamaskController { var web3 = new Web3(provider) this.web3 = web3 keyringController.web3 = web3 - + this.txManager.web3 = web3 provider.on('block', this.processBlock.bind(this)) provider.on('error', this.getNetwork.bind(this)) diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js new file mode 100644 index 000000000..07e588679 --- /dev/null +++ b/app/scripts/transaction-manager.js @@ -0,0 +1,179 @@ +const EventEmitter = require('events') +const extend = require('xtend') +const TxProviderUtil = require('./lib/provider-utils') + +module.exports = class TransactionManager extends EventEmitter { + constructor (opts) { + super() + this.txList = opts.TxListFromStore || [] + this._persistTxList = opts.setTxList + this._unconfTxCbs = {} + this.txLimit = opts.txLimit + this.provider = opts.provider + } + +// Returns the tx list + getTxList () { + return this.txList + } + + // Saves the new/updated txList. + // Function is intended only for internal use + _saveTxList (txList) { + this.txList = txList + this._persistTxList(txList) + } + + // Adds a tx to the txlist + addTx (txData, onTxDoneCb) { + var txList = this.getTxList() + var txLimit = this.txLimit + if (txList.length > txLimit - 1) { + txList.shift() + } + txList.push(txData) + this._saveTxList(txList) + this.addOnTxDoneCb(txData.id, onTxDoneCb) + this.emit('unapproved', txData) + this.emit('update') + } + + getTx (txId, cb) { + var txList = this.getTxList() + var tx = txList.find((tx) => tx.id === txId) + return cb ? cb(tx) : tx + } + + updateTx (txData) { + var txId = txData.id + var txList = this.getTxList() + + var updatedTxList = txList.map((tx) => { + if (tx.id === txId) { + tx = txData + } + return tx + }) + this._saveTxList(updatedTxList) + } + + get unConftxCount () { + return Object.keys(this.getUnapprovedTxList()).length + } + + get pendingTxCount () { + return this.getTxsByMetaData('status', 'signed').length + } + + getUnapprovedTxList () { + var txList = this.getTxList() + return txList.filter((tx) => { + return tx.status === 'unapproved' + }).reduce((result, tx) => { + result[tx.id] = tx + return result + }, {}) + } + + getFilterdTxList (opts) { + var filteredTxList + Object.keys(opts).forEach((key) => { + filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList) + }) + return filteredTxList + } + + getTxsByMetaData (key, value, txList = this.getTxList()) { + return txList.filter((tx) => { + if (key in tx.txParams) { + return tx.txParams[key] === value + } else { + return tx[key] === value + } + }) + } + + addOnTxDoneCb (txId, cb) { + this._unconfTxCbs[txId] = cb || noop + } + + // should return the tx + + // 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! + // // - `'signed'` the tx is signed + // // - `'submitted'` the tx is sent to a server + // // - `'confirmed'` the tx has been included in a block. + setTxStatus (txId, status) { + var txData = this.getTx(txId) + txData.status = status + this.emit(status, txId) + this.updateTx(txData, status) + } + + + // should return the status of the tx. + getTxStatus (txId, cb) { + const txData = this.getTx(txId) + return cb ? cb(txData.staus) : txData.status + } + + + // should update the status of the tx to 'signed'. + setTxStatusSigned (txId) { + this.setTxStatus(txId, 'signed') + this.emit('update') + } + + // should update the status of the tx to 'rejected'. + setTxStatusRejected (txId) { + this.setTxStatus(txId, 'rejected') + this.emit('update') + } + + setTxStatusConfirmed (txId) { + this.setTxStatus(txId, 'confirmed') + // this.removeListener(`check${txId}`, this.checkForTxInBlock) + } + + // merges txParams obj onto txData.txParams + // use extend to ensure that all fields are filled + updateTxParams (txId, txParams) { + var txData = this.getTx(txId) + txData.txParams = extend(txData, txParams) + this.updateTx(txData) + } + + setProvider (provider) { + this.provider = provider + this.txProviderUtils = new TxProviderUtil(provider) + this.provider.on('block', this.checkForTxInBlock.bind(this)) + } + + checkForTxInBlock () { + var signedTxList = this.getFilterdTxList({status: 'signed'}) + if (!signedTxList.length) return + var self = this + signedTxList.forEach((tx) => { + var txHash = tx.hash + var txId = tx.id + if (!txHash) return + // var d + this.txProviderUtils.query.getTransactionByHash(txHash, (err, txData) => { + if (err) { + tx + + return console.error(err) + } + if (txData.blockNumber !== null) { + self.setTxStatusConfirmed(txId) + } + }) + }) + } +} + +function noop () {} -- cgit v1.2.3 From da9349fe63de70d29a13b6c7d94eb40dc5fcb127 Mon Sep 17 00:00:00 2001 From: Frances Pangilinan Date: Wed, 14 Dec 2016 15:04:33 -0800 Subject: Clean up and comment functions --- app/scripts/background.js | 11 ++--------- app/scripts/transaction-manager.js | 30 +++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 10 deletions(-) (limited to 'app') diff --git a/app/scripts/background.js b/app/scripts/background.js index 854b679da..f476e89e4 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -115,18 +115,11 @@ function updateBadge () { // txManger :: tx approvals and rejection cb's txManager.on('signed', function (txId) { - var approvalCb = this._unconfTxCbs[txId] - - approvalCb(null, true) - // clean up - delete this._unconfTxCbs[txId] + this.execOnTxDoneCb(txId, true) }) txManager.on('rejected', function (txId) { - var approvalCb = this._unconfTxCbs[txId] - approvalCb(null, false) - // clean up - delete this._unconfTxCbs[txId] + this.execOnTxDoneCb(txId, false) }) // data :: setters/getters diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index 07e588679..d0226a600 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -38,12 +38,14 @@ module.exports = class TransactionManager extends EventEmitter { this.emit('update') } + // gets tx by Id and returns it getTx (txId, cb) { var txList = this.getTxList() var tx = txList.find((tx) => tx.id === txId) return cb ? cb(tx) : tx } + // updateTx (txData) { var txId = txData.id var txList = this.getTxList() @@ -75,6 +77,21 @@ module.exports = class TransactionManager extends EventEmitter { }, {}) } + /* + Takes an object of fields to search for eg: + var thingsToLookFor = { + to: '0x0..', + from: '0x0..', + status: 'signed', + } + and returns a list of tx with all + options matching + + 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' + */ getFilterdTxList (opts) { var filteredTxList Object.keys(opts).forEach((key) => { @@ -93,10 +110,19 @@ module.exports = class TransactionManager extends EventEmitter { }) } +// keeps around the txCbs for later addOnTxDoneCb (txId, cb) { this._unconfTxCbs[txId] = cb || noop } + execOnTxDoneCb (txId, conf) { + var approvalCb = this._unconfTxCbs[txId] + + approvalCb(null, conf) + // clean up + delete this._unconfTxCbs[txId] + } + // should return the tx // Should find the tx in the tx list and @@ -136,7 +162,6 @@ module.exports = class TransactionManager extends EventEmitter { setTxStatusConfirmed (txId) { this.setTxStatus(txId, 'confirmed') - // this.removeListener(`check${txId}`, this.checkForTxInBlock) } // merges txParams obj onto txData.txParams @@ -147,12 +172,15 @@ module.exports = class TransactionManager extends EventEmitter { this.updateTx(txData) } + // sets provider for provider utils and event listener setProvider (provider) { this.provider = provider this.txProviderUtils = new TxProviderUtil(provider) this.provider.on('block', this.checkForTxInBlock.bind(this)) } + // checks if a signed tx is in a block and + // if included sets the tx status as 'confirmed' checkForTxInBlock () { var signedTxList = this.getFilterdTxList({status: 'signed'}) if (!signedTxList.length) return -- cgit v1.2.3 From 6e78494846c9032fbf1264a0225c0df4df0867cb Mon Sep 17 00:00:00 2001 From: Frances Pangilinan Date: Fri, 16 Dec 2016 10:33:36 -0800 Subject: First pass at revision requests --- app/scripts/background.js | 14 +- app/scripts/keyring-controller.js | 101 +------------- app/scripts/lib/config-manager.js | 3 - app/scripts/lib/idStore.js | 261 ------------------------------------- app/scripts/lib/provider-utils.js | 106 --------------- app/scripts/lib/tx-utils.js | 87 +++++++++++++ app/scripts/metamask-controller.js | 38 +++--- app/scripts/transaction-manager.js | 247 +++++++++++++++++++++-------------- 8 files changed, 265 insertions(+), 592 deletions(-) delete mode 100644 app/scripts/lib/provider-utils.js create mode 100644 app/scripts/lib/tx-utils.js (limited to 'app') diff --git a/app/scripts/background.js b/app/scripts/background.js index f476e89e4..a7e525999 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -17,7 +17,7 @@ const controller = new MetamaskController({ // User confirmation callbacks: showUnconfirmedMessage: triggerUi, unlockAccountMessage: triggerUi, - showUnconfirmedTx: triggerUi, + showUnapprovedTx: triggerUi, // Persistence Methods: setData, loadData, @@ -101,7 +101,7 @@ txManager.on('update', updateBadge) function updateBadge () { var label = '' - var unconfTxLen = controller.txManager.unConftxCount + var unconfTxLen = controller.txManager.unconfTxCount var unconfMsgs = messageManager.unconfirmedMsgs() var unconfMsgLen = Object.keys(unconfMsgs).length var count = unconfTxLen + unconfMsgLen @@ -112,16 +112,6 @@ function updateBadge () { extension.browserAction.setBadgeBackgroundColor({ color: '#506F8B' }) } -// txManger :: tx approvals and rejection cb's - -txManager.on('signed', function (txId) { - this.execOnTxDoneCb(txId, true) -}) - -txManager.on('rejected', function (txId) { - this.execOnTxDoneCb(txId, false) -}) - // data :: setters/getters function loadData () { diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 37b3a947f..64c2ac933 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -100,8 +100,6 @@ module.exports = class KeyringController extends EventEmitter { isInitialized: (!!wallet || !!vault), isUnlocked: Boolean(this.password), isDisclaimerConfirmed: this.configManager.getConfirmedDisclaimer(), // AUDIT this.configManager.getConfirmedDisclaimer(), - transactions: this.txManager.getTxList(), - unconfTxs: this.txManager.getUnapprovedTxList(), unconfMsgs: messageManager.unconfirmedMsgs(), messages: messageManager.getMsgList(), selectedAccount: address, @@ -319,89 +317,10 @@ module.exports = class KeyringController extends EventEmitter { } - // SIGNING RELATED METHODS + // SIGNING METHODS // - // SIGN, SUBMIT TX, CANCEL, AND APPROVE. - // THIS SECTION INVOLVES THE REQUEST, STORING, AND SIGNING OF DATA - // WITH THE KEYS STORED IN THIS CONTROLLER. - - - // Add Unconfirmed Transaction - // @object txParams - // @function onTxDoneCb - // @function cb - // - // Calls back `cb` with @object txData = { txParams } - // Calls back `onTxDoneCb` with `true` or an `error` depending on result. - // - // Prepares the given `txParams` for final confirmation and approval. - // Estimates gas and other preparatory steps. - // Caches the requesting Dapp's callback, `onTxDoneCb`, for resolution later. - addUnconfirmedTransaction (txParams, onTxDoneCb, cb) { - const configManager = this.configManager - const txManager = this.txManager - // create txData obj with parameters and meta data - var time = (new Date()).getTime() - var txId = createId() - txParams.metamaskId = txId - txParams.metamaskNetworkId = this.getNetwork() - var txData = { - id: txId, - txParams: txParams, - time: time, - status: 'unapproved', - gasMultiplier: configManager.getGasMultiplier() || 1, - metamaskNetworkId: this.getNetwork(), - } - // keep the onTxDoneCb around for after approval/denial (requires user interaction) - // This onTxDoneCb fires completion to the Dapp's write operation. - txManager.txProviderUtils.analyzeGasUsage(txData, this.txDidComplete.bind(this, txData, onTxDoneCb, cb)) - // calculate metadata for tx - } - - txDidComplete (txData, onTxDoneCb, cb, err) { - if (err) return cb(err) - const txManager = this.txManager - txManager.addTx(txData, onTxDoneCb) - // signal update - this.emit('update') - // signal completion of add tx - cb(null, txData) - } - - // Cancel Transaction - // @string txId - // @function cb - // - // Calls back `cb` with no error if provided. - // - // Forgets any tx matching `txId`. - cancelTransaction (txId, cb) { - const txManager = this.txManager - txManager.setTxStatusRejected(txId) - - if (cb && typeof cb === 'function') { - cb() - } - } - - // Approve Transaction - // @string txId - // @function cb - // - // Calls back `cb` with no error always. - // - // Attempts to sign a Transaction with `txId` - // and submit it to the blockchain. - // - // Calls back the cached Dapp's confirmation callback, also. - approveTransaction (txId, cb) { - const txManager = this.txManager - txManager.setTxStatusSigned(txId) - this.emit('update') - cb() - } - + // This method signs tx and returns a promise for + // TX Manager to update the state after signing signTransaction (txParams, cb) { try { const address = normalize(txParams.from) @@ -420,20 +339,10 @@ module.exports = class KeyringController extends EventEmitter { txParams.data = normalize(txParams.data) txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) txParams.nonce = normalize(txParams.nonce) - const tx = new Transaction(txParams) return keyring.signTransaction(address, tx) - }) - .then((tx) => { - // Add the tx hash to the persisted meta-tx object - var txHash = ethUtil.bufferToHex(tx.hash()) - var metaTx = this.txManager.getTx(txParams.metamaskId) - metaTx.hash = txHash - this.txManager.updateTx(metaTx) - - // return raw serialized tx - var rawTx = ethUtil.bufferToHex(tx.serialize()) - cb(null, rawTx) + }).then((tx) => { + return {tx, txParams, cb} }) } catch (e) { cb(e) diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js index 913a76a6e..a32842cc7 100644 --- a/app/scripts/lib/config-manager.js +++ b/app/scripts/lib/config-manager.js @@ -8,7 +8,6 @@ const normalize = require('./sig-util').normalize const TESTNET_RPC = MetamaskConfig.network.testnet const MAINNET_RPC = MetamaskConfig.network.mainnet const MORDEN_RPC = MetamaskConfig.network.morden -const txLimit = 40 /* The config-manager is a convenience object * wrapping a pojo-migrator. @@ -19,8 +18,6 @@ const txLimit = 40 */ module.exports = ConfigManager function ConfigManager (opts) { - this.txLimit = txLimit - // ConfigManager is observable and will emit updates this._subs = [] diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 71bee8026..5c0f8d7f7 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -1,19 +1,12 @@ const EventEmitter = require('events').EventEmitter const inherits = require('util').inherits -const async = require('async') const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN -const EthQuery = require('eth-query') const KeyStore = require('eth-lightwallet').keystore const clone = require('clone') const extend = require('xtend') -const createId = require('./random-id') -const ethBinToOps = require('eth-bin-to-ops') const autoFaucet = require('./auto-faucet') -const messageManager = require('./message-manager') const DEFAULT_RPC = 'https://testrpc.metamask.io/' const IdManagement = require('./id-management') -const TxManager = require('../transaction-manager') module.exports = IdentityStore @@ -36,15 +29,7 @@ function IdentityStore (opts = {}) { selectedAddress: null, identities: {}, } - // not part of serilized metamask state - only kept in memory - this.txManager = new TxManager({ - TxListFromStore: opts.configManager.getTxList(), - setTxList: opts.configManager.setTxList.bind(opts.configManager), - txLimit: 40, - }) - this._unconfTxCbs = {} - this._unconfMsgCbs = {} } // @@ -94,7 +79,6 @@ IdentityStore.prototype.recoverFromSeed = function (password, seed, cb) { IdentityStore.prototype.setStore = function (store) { this._ethStore = store - this.txManager.setProvider(this._ethStore._query.currentProvider) } IdentityStore.prototype.clearSeedWordCache = function (cb) { @@ -105,17 +89,12 @@ IdentityStore.prototype.clearSeedWordCache = function (cb) { IdentityStore.prototype.getState = function () { const configManager = this.configManager - const TxManager = this.txManager var seedWords = this.getSeedIfUnlocked() return clone(extend(this._currentState, { isInitialized: !!configManager.getWallet() && !seedWords, isUnlocked: this._isUnlocked(), seedWords: seedWords, isDisclaimerConfirmed: configManager.getConfirmedDisclaimer(), - unconfTxs: TxManager.getUnapprovedTxList(), - transactions: TxManager.getTxList(), - unconfMsgs: messageManager.unconfirmedMsgs(), - messages: messageManager.getMsgList(), selectedAddress: configManager.getSelectedAccount(), shapeShiftTxList: configManager.getShapeShiftTxList(), currentFiat: configManager.getCurrentFiat(), @@ -214,245 +193,6 @@ IdentityStore.prototype.exportAccount = function (address, cb) { cb(null, privateKey) } -// -// Transactions -// - -// comes from dapp via zero-client hooked-wallet provider -IdentityStore.prototype.addUnconfirmedTransaction = function (txParams, onTxDoneCb, cb) { - const configManager = this.configManager - - var self = this - // create txData obj with parameters and meta data - var time = (new Date()).getTime() - var txId = createId() - txParams.metamaskId = txId - txParams.metamaskNetworkId = self._currentState.network - var txData = { - id: txId, - txParams: txParams, - time: time, - status: 'unconfirmed', - gasMultiplier: configManager.getGasMultiplier() || 1, - } - - console.log('addUnconfirmedTransaction:', txData) - - // keep the onTxDoneCb around for after approval/denial (requires user interaction) - // This onTxDoneCb fires completion to the Dapp's write operation. - self._unconfTxCbs[txId] = onTxDoneCb - - var provider = self._ethStore._query.currentProvider - var query = new EthQuery(provider) - - // calculate metadata for tx - async.parallel([ - analyzeForDelegateCall, - estimateGas, - ], didComplete) - - // perform static analyis on the target contract code - function analyzeForDelegateCall (cb) { - if (txParams.to) { - query.getCode(txParams.to, (err, result) => { - if (err) return cb(err.message || err) - var containsDelegateCall = self.checkForDelegateCall(result) - txData.containsDelegateCall = containsDelegateCall - cb() - }) - } else { - cb() - } - } - - function estimateGas (cb) { - var estimationParams = extend(txParams) - query.getBlockByNumber('latest', true, function(err, block){ - if (err) return cb(err) - // check if gasLimit is already specified - const gasLimitSpecified = Boolean(txParams.gas) - // if not, fallback to block gasLimit - if (!gasLimitSpecified) { - estimationParams.gas = block.gasLimit - } - // run tx, see if it will OOG - query.estimateGas(estimationParams, function(err, estimatedGasHex){ - if (err) return cb(err.message || err) - // all gas used - must be an error - if (estimatedGasHex === estimationParams.gas) { - txData.simulationFails = true - txData.estimatedGas = estimatedGasHex - txData.txParams.gas = estimatedGasHex - cb() - return - } - // otherwise, did not use all gas, must be ok - - // if specified gasLimit and no error, we're done - if (gasLimitSpecified) { - txData.estimatedGas = txParams.gas - cb() - return - } - - // try adding an additional gas buffer to our estimation for safety - const estimatedGasBn = new BN(ethUtil.stripHexPrefix(estimatedGasHex), 16) - const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(block.gasLimit), 16) - const estimationWithBuffer = self.addGasBuffer(estimatedGasBn) - // added gas buffer is too high - if (estimationWithBuffer.gt(blockGasLimitBn)) { - txData.estimatedGas = estimatedGasHex - txData.txParams.gas = estimatedGasHex - // added gas buffer is safe - } else { - const gasWithBufferHex = ethUtil.intToHex(estimationWithBuffer) - txData.estimatedGas = gasWithBufferHex - txData.txParams.gas = gasWithBufferHex - } - cb() - return - }) - }) - } - - function didComplete (err) { - if (err) return cb(err.message || err) - configManager.addTx(txData) - // signal update - self._didUpdate() - // signal completion of add tx - cb(null, txData) - } -} - -IdentityStore.prototype.checkForDelegateCall = function (codeHex) { - const code = ethUtil.toBuffer(codeHex) - if (code !== '0x') { - const ops = ethBinToOps(code) - const containsDelegateCall = ops.some((op) => op.name === 'DELEGATECALL') - return containsDelegateCall - } else { - return false - } -} - -IdentityStore.prototype.addGasBuffer = function (gasBn) { - // add 20% to specified gas - const gasBuffer = gasBn.div(new BN('5', 10)) - const gasWithBuffer = gasBn.add(gasBuffer) - return gasWithBuffer -} - -// comes from metamask ui -IdentityStore.prototype.approveTransaction = function (txId, cb) { - const configManager = this.configManager - var approvalCb = this._unconfTxCbs[txId] || noop - - // accept tx - cb() - approvalCb(null, true) - // clean up - configManager.confirmTx(txId) - delete this._unconfTxCbs[txId] - this._didUpdate() -} - -// comes from metamask ui -IdentityStore.prototype.cancelTransaction = function (txId) { - const configManager = this.configManager - var approvalCb = this._unconfTxCbs[txId] || noop - - // reject tx - approvalCb(null, false) - // clean up - configManager.rejectTx(txId) - delete this._unconfTxCbs[txId] - this._didUpdate() -} - -// performs the actual signing, no autofill of params -IdentityStore.prototype.signTransaction = function (txParams, cb) { - try { - console.log('signing tx...', txParams) - var rawTx = this._idmgmt.signTx(txParams) - cb(null, rawTx) - } catch (err) { - cb(err) - } -} - -// -// Messages -// - -// comes from dapp via zero-client hooked-wallet provider -IdentityStore.prototype.addUnconfirmedMessage = function (msgParams, cb) { - // create txData obj with parameters and meta data - var time = (new Date()).getTime() - var msgId = createId() - var msgData = { - id: msgId, - msgParams: msgParams, - time: time, - status: 'unconfirmed', - } - messageManager.addMsg(msgData) - console.log('addUnconfirmedMessage:', msgData) - - // keep the cb around for after approval (requires user interaction) - // This cb fires completion to the Dapp's write operation. - this._unconfMsgCbs[msgId] = cb - - // signal update - this._didUpdate() - - return msgId -} - -// comes from metamask ui -IdentityStore.prototype.approveMessage = function (msgId, cb) { - var approvalCb = this._unconfMsgCbs[msgId] || noop - - // accept msg - cb() - approvalCb(null, true) - // clean up - messageManager.confirmMsg(msgId) - delete this._unconfMsgCbs[msgId] - this._didUpdate() -} - -// comes from metamask ui -IdentityStore.prototype.cancelMessage = function (msgId) { - var approvalCb = this._unconfMsgCbs[msgId] || noop - - // reject tx - approvalCb(null, false) - // clean up - messageManager.rejectMsg(msgId) - delete this._unconfTxCbs[msgId] - this._didUpdate() -} - -// performs the actual signing, no autofill of params -IdentityStore.prototype.signMessage = function (msgParams, cb) { - try { - console.log('signing msg...', msgParams.data) - var rawMsg = this._idmgmt.signMsg(msgParams.from, msgParams.data) - if ('metamaskId' in msgParams) { - var id = msgParams.metamaskId - delete msgParams.metamaskId - - this.approveMessage(id, cb) - } else { - cb(null, rawMsg) - } - } catch (err) { - cb(err) - } -} - -// // private // @@ -607,4 +347,3 @@ IdentityStore.prototype._autoFaucet = function () { // util -function noop () {} diff --git a/app/scripts/lib/provider-utils.js b/app/scripts/lib/provider-utils.js deleted file mode 100644 index d1678c964..000000000 --- a/app/scripts/lib/provider-utils.js +++ /dev/null @@ -1,106 +0,0 @@ -const async = require('async') -const EthQuery = require('eth-query') -const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN -const ethBinToOps = require('eth-bin-to-ops') - -module.exports = class txProviderUtils { - constructor (provider) { - this.provider = provider - this.query = new EthQuery(provider) - } - analyzeGasUsage (txData, cb) { - var self = this - this.query.getBlockByNumber('latest', true, (err, block) => { - if (err) return cb(err) - async.waterfall([ - self.estimateTxGas.bind(self, txData, block.gasLimit), - self.checkForTxGasError.bind(self, txData), - self.setTxGas.bind(self, txData, block.gasLimit), - ], cb) - }) - } - - // perform static analyis on the target contract code - analyzeForDelegateCall (txParams, cb) { - if (txParams.to) { - this.query.getCode(txParams.to, function (err, result) { - if (err) return cb(err) - - var code = ethUtil.toBuffer(result) - if (code !== '0x') { - var ops = ethBinToOps(code) - var containsDelegateCall = ops.some((op) => op.name === 'DELEGATECALL') - cb(containsDelegateCall) - } else { - cb() - } - }) - } else { - cb() - } - } - - estimateTxGas (txData, blockGasLimitHex, cb) { - const txParams = txData.txParams - // check if gasLimit is already specified - txData.gasLimitSpecified = Boolean(txParams.gas) - // if not, fallback to block gasLimit - if (!txData.gasLimitSpecified) { - txParams.gas = blockGasLimitHex - } - // run tx, see if it will OOG - this.query.estimateGas(txParams, cb) - } - - checkForTxGasError (txData, estimatedGasHex, cb) { - txData.estimatedGas = estimatedGasHex - // all gas used - must be an error - if (estimatedGasHex === txData.txParams.gas) { - txData.simulationFails = true - } - cb() - } - - handleFork (block) { - - } - - setTxGas (txData, blockGasLimitHex, cb) { - const txParams = txData.txParams - // if OOG, nothing more to do - if (txData.simulationFails) { - cb() - return - } - // if gasLimit was specified and doesnt OOG, - // use original specified amount - if (txData.gasLimitSpecified) { - txData.estimatedGas = txParams.gas - cb() - return - } - // if gasLimit not originally specified, - // try adding an additional gas buffer to our estimation for safety - const estimatedGasBn = new BN(ethUtil.stripHexPrefix(txData.estimatedGas), 16) - const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) - const estimationWithBuffer = new BN(this.addGasBuffer(estimatedGasBn), 16) - // added gas buffer is too high - if (estimationWithBuffer.gt(blockGasLimitBn)) { - txParams.gas = txData.estimatedGas - // added gas buffer is safe - } else { - const gasWithBufferHex = ethUtil.intToHex(estimationWithBuffer) - txParams.gas = gasWithBufferHex - } - cb() - return - } - - 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)) - } -} diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js new file mode 100644 index 000000000..a976173f5 --- /dev/null +++ b/app/scripts/lib/tx-utils.js @@ -0,0 +1,87 @@ +const async = require('async') +const EthQuery = require('eth-query') +const ethUtil = require('ethereumjs-util') +const BN = ethUtil.BN + +/* +tx-utils are utility methods for Transaction manager +its passed a provider and that is passed to ethquery +and used to do things like calculate gas of a tx. +*/ + +module.exports = class txProviderUtils { + constructor (provider) { + this.provider = provider + this.query = new EthQuery(provider) + } + analyzeGasUsage (txData, cb) { + var self = this + this.query.getBlockByNumber('latest', true, (err, block) => { + if (err) return cb(err) + async.waterfall([ + self.estimateTxGas.bind(self, txData, block.gasLimit), + self.checkForTxGasError.bind(self, txData), + self.setTxGas.bind(self, txData, block.gasLimit), + ], cb) + }) + } + + estimateTxGas (txData, blockGasLimitHex, cb) { + const txParams = txData.txParams + // check if gasLimit is already specified + txData.gasLimitSpecified = Boolean(txParams.gas) + // if not, fallback to block gasLimit + if (!txData.gasLimitSpecified) { + txParams.gas = blockGasLimitHex + } + // run tx, see if it will OOG + this.query.estimateGas(txParams, cb) + } + + checkForTxGasError (txData, estimatedGasHex, cb) { + txData.estimatedGas = estimatedGasHex + // all gas used - must be an error + if (estimatedGasHex === txData.txParams.gas) { + txData.simulationFails = true + } + cb() + } + + setTxGas (txData, blockGasLimitHex, cb) { + const txParams = txData.txParams + // if OOG, nothing more to do + if (txData.simulationFails) { + cb() + return + } + // if gasLimit was specified and doesnt OOG, + // use original specified amount + if (txData.gasLimitSpecified) { + txData.estimatedGas = txParams.gas + cb() + return + } + // if gasLimit not originally specified, + // try adding an additional gas buffer to our estimation for safety + const estimatedGasBn = new BN(ethUtil.stripHexPrefix(txData.estimatedGas), 16) + const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) + const estimationWithBuffer = new BN(this.addGasBuffer(estimatedGasBn), 16) + // added gas buffer is too high + if (estimationWithBuffer.gt(blockGasLimitBn)) { + txParams.gas = txData.estimatedGas + // added gas buffer is safe + } else { + const gasWithBufferHex = ethUtil.intToHex(estimationWithBuffer) + txParams.gas = gasWithBufferHex + } + cb() + return + } + + 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)) + } +} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 3b70f63db..d5b70c647 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -11,7 +11,6 @@ const extension = require('./lib/extension') const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') - module.exports = class MetamaskController { constructor (opts) { @@ -19,12 +18,6 @@ module.exports = class MetamaskController { this.opts = opts this.listeners = [] this.configManager = new ConfigManager(opts) - this.txManager = new TxManager({ - TxListFromStore: this.configManager.getTxList(), - txLimit: this.configManager.txLimit, - setTxList: this.configManager.setTxList.bind(this.configManager), - }) - this.keyringController = new KeyringController({ configManager: this.configManager, txManager: this.txManager, @@ -33,9 +26,17 @@ module.exports = class MetamaskController { this.provider = this.initializeProvider(opts) this.ethStore = new EthStore(this.provider) this.keyringController.setStore(this.ethStore) - this.txManager.setProvider(this.provider) this.getNetwork() this.messageManager = messageManager + this.txManager = new TxManager({ + txList: this.configManager.getTxList(), + txHistoryLimit: 40, + setTxList: this.configManager.setTxList.bind(this.configManager), + getGasMultiplier: this.configManager.getGasMultiplier.bind(this.configManager), + getNetwork: this.getStateNetwork.bind(this), + provider: this.provider, + blockTracker: this.provider, + }) this.publicConfigStore = this.initPublicConfigStore() var currentFiat = this.configManager.getCurrentFiat() || 'USD' @@ -52,7 +53,8 @@ module.exports = class MetamaskController { this.state, this.ethStore.getState(), this.configManager.getConfig(), - this.keyringController.getState() + this.keyringController.getState(), + this.txManager.getState() ) } @@ -85,14 +87,14 @@ module.exports = class MetamaskController { exportAccount: nodeify(keyringController.exportAccount).bind(keyringController), // signing methods - approveTransaction: keyringController.approveTransaction.bind(keyringController), - cancelTransaction: keyringController.cancelTransaction.bind(keyringController), + approveTransaction: txManager.approveTransaction.bind(txManager), + cancelTransaction: txManager.cancelTransaction.bind(txManager), signMessage: keyringController.signMessage.bind(keyringController), cancelMessage: keyringController.cancelMessage.bind(keyringController), // forward directly to txManager - getUnapprovedTxList: txManager.getTxList.bind(txManager), - getFilterdTxList: txManager.getFilterdTxList.bind(txManager), + getUnapprovedTxList: txManager.getUnapprovedTxList.bind(txManager), + getFilteredTxList: txManager.getFilteredTxList.bind(txManager), // coinbase buyEth: this.buyEth.bind(this), // shapeshift @@ -150,7 +152,8 @@ module.exports = class MetamaskController { // tx signing approveTransaction: this.newUnsignedTransaction.bind(this), signTransaction: (...args) => { - keyringController.signTransaction(...args) + var signedTxPromise = keyringController.signTransaction(...args) + this.txManager.resolveSignedTransaction(signedTxPromise) this.sendUpdate() }, @@ -166,7 +169,6 @@ module.exports = class MetamaskController { var web3 = new Web3(provider) this.web3 = web3 keyringController.web3 = web3 - this.txManager.web3 = web3 provider.on('block', this.processBlock.bind(this)) provider.on('error', this.getNetwork.bind(this)) @@ -220,13 +222,13 @@ module.exports = class MetamaskController { } newUnsignedTransaction (txParams, onTxDoneCb) { - const keyringController = this.keyringController + const txManager = this.txManager const err = this.enforceTxValidations(txParams) if (err) return onTxDoneCb(err) - keyringController.addUnconfirmedTransaction(txParams, onTxDoneCb, (err, txData) => { + txManager.addUnapprovedTransaction(txParams, onTxDoneCb, (err, txData) => { if (err) return onTxDoneCb(err) this.sendUpdate() - this.opts.showUnconfirmedTx(txParams, txData, onTxDoneCb) + this.opts.showUnapprovedTx(txParams, txData, onTxDoneCb) }) } diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index d0226a600..6b3d1806f 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -1,15 +1,31 @@ const EventEmitter = require('events') const extend = require('xtend') -const TxProviderUtil = require('./lib/provider-utils') +const ethUtil = require('ethereumjs-util') +const TxProviderUtil = require('./lib/tx-utils') +const createId = require('./lib/random-id') module.exports = class TransactionManager extends EventEmitter { constructor (opts) { super() - this.txList = opts.TxListFromStore || [] - this._persistTxList = opts.setTxList + this.txList = opts.txList || [] + this._setTxList = opts.setTxList this._unconfTxCbs = {} - this.txLimit = opts.txLimit + this.txHistoryLimit = opts.txHistoryLimit + // txManager :: tx approvals and rejection cb's + this.provider = opts.provider + this.blockTracker = opts.blockTracker + this.txProviderUtils = new TxProviderUtil(this.provider) + this.blockTracker.on('block', this.checkForTxInBlock.bind(this)) + this.getGasMultiplier = opts.getGasMultiplier + this.getNetwork = opts.getNetwork + } + + getState () { + return { + transactions: this.getTxList(), + unconfTxs: this.getUnapprovedTxList(), + } } // Returns the tx list @@ -17,49 +33,49 @@ module.exports = class TransactionManager extends EventEmitter { return this.txList } - // Saves the new/updated txList. - // Function is intended only for internal use - _saveTxList (txList) { - this.txList = txList - this._persistTxList(txList) - } - // Adds a tx to the txlist - addTx (txData, onTxDoneCb) { + addTx (txMeta, onTxDoneCb = noop) { var txList = this.getTxList() - var txLimit = this.txLimit - if (txList.length > txLimit - 1) { - txList.shift() + var txHistoryLimit = this.txHistoryLimit + if (txList.length > txHistoryLimit - 1) { + var index = txList.findIndex((metaTx) => metaTx.status === 'confirmed' || metaTx.status === 'rejected') + index ? txList.splice(index, index) : txList.shift() } - txList.push(txData) + txList.push(txMeta) this._saveTxList(txList) - this.addOnTxDoneCb(txData.id, onTxDoneCb) - this.emit('unapproved', txData) + // keep the onTxDoneCb around in a listener + // for after approval/denial (requires user interaction) + // This onTxDoneCb fires completion to the Dapp's write operation. + this.once(`${txMeta.id}:signed`, function (txId) { + this.removeAllListeners(`${txMeta.id}:rejected`) + onTxDoneCb(null, true) + }) + this.once(`${txMeta.id}:rejected`, function (txId) { + this.removeAllListeners(`${txMeta.id}:signed`) + onTxDoneCb(null, false) + }) + this.emit('update') + this.emit(`${txMeta.id}:unapproved`, txMeta) } // gets tx by Id and returns it getTx (txId, cb) { var txList = this.getTxList() - var tx = txList.find((tx) => tx.id === txId) - return cb ? cb(tx) : tx + var txMeta = txList.find((txData) => txData.id === txId) + return cb ? cb(txMeta) : txMeta } // - updateTx (txData) { - var txId = txData.id + updateTx (txMeta) { + var txId = txMeta.id var txList = this.getTxList() - - var updatedTxList = txList.map((tx) => { - if (tx.id === txId) { - tx = txData - } - return tx - }) - this._saveTxList(updatedTxList) + var index = txList.findIndex((txData) => txData.id === txId) + txList[index] = txMeta + this._saveTxList(txList) } - get unConftxCount () { + get unconfTxCount () { return Object.keys(this.getUnapprovedTxList()).length } @@ -67,16 +83,66 @@ module.exports = class TransactionManager extends EventEmitter { return this.getTxsByMetaData('status', 'signed').length } + addUnapprovedTransaction (txParams, onTxDoneCb, cb) { + // create txData obj with parameters and meta data + var time = (new Date()).getTime() + var txId = createId() + txParams.metamaskId = txId + txParams.metamaskNetworkId = this.getNetwork() + var txData = { + id: txId, + txParams: txParams, + time: time, + status: 'unapproved', + gasMultiplier: this.getGasMultiplier() || 1, + metamaskNetworkId: this.getNetwork(), + } + this.txProviderUtils.analyzeGasUsage(txData, this.txDidComplete.bind(this, txData, onTxDoneCb, cb)) + // calculate metadata for tx + } + + txDidComplete (txMeta, onTxDoneCb, cb, err) { + if (err) return cb(err) + this.addTx(txMeta, onTxDoneCb) + cb(null, txMeta) + } + getUnapprovedTxList () { var txList = this.getTxList() - return txList.filter((tx) => { - return tx.status === 'unapproved' - }).reduce((result, tx) => { + return txList.filter((txMeta) => txMeta.status === 'unapproved') + .reduce((result, tx) => { result[tx.id] = tx return result }, {}) } + approveTransaction (txId, cb) { + this.setTxStatusSigned(txId) + cb() + } + + cancelTransaction (txId, cb) { + this.setTxStatusRejected(txId) + if (cb && typeof cb === 'function') { + cb() + } + } + + resolveSignedTransaction (txPromise) { + const self = this + + txPromise.then(({tx, txParams, cb}) => { + // Add the tx hash to the persisted meta-tx object + var txHash = ethUtil.bufferToHex(tx.hash()) + + var metaTx = self.getTx(txParams.metamaskId) + metaTx.hash = txHash + // return raw serialized tx + var rawTx = ethUtil.bufferToHex(tx.serialize()) + cb(null, rawTx) + }) + } + /* Takes an object of fields to search for eg: var thingsToLookFor = { @@ -92,7 +158,7 @@ module.exports = class TransactionManager extends EventEmitter { or for filltering for all txs from one account and that have been 'confirmed' */ - getFilterdTxList (opts) { + getFilteredTxList (opts) { var filteredTxList Object.keys(opts).forEach((key) => { filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList) @@ -101,107 +167,96 @@ module.exports = class TransactionManager extends EventEmitter { } getTxsByMetaData (key, value, txList = this.getTxList()) { - return txList.filter((tx) => { - if (key in tx.txParams) { - return tx.txParams[key] === value + return txList.filter((txMeta) => { + if (key in txMeta.txParams) { + return txMeta.txParams[key] === value } else { - return tx[key] === value + return txMeta[key] === value } }) } -// keeps around the txCbs for later - addOnTxDoneCb (txId, cb) { - this._unconfTxCbs[txId] = cb || noop - } - - execOnTxDoneCb (txId, conf) { - var approvalCb = this._unconfTxCbs[txId] - - approvalCb(null, conf) - // clean up - delete this._unconfTxCbs[txId] - } - - // should return the tx - - // 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! - // // - `'signed'` the tx is signed - // // - `'submitted'` the tx is sent to a server - // // - `'confirmed'` the tx has been included in a block. - setTxStatus (txId, status) { - var txData = this.getTx(txId) - txData.status = status - this.emit(status, txId) - this.updateTx(txData, status) - } - - // should return the status of the tx. getTxStatus (txId, cb) { - const txData = this.getTx(txId) - return cb ? cb(txData.staus) : txData.status + const txMeta = this.getTx(txId) + return cb ? cb(txMeta.staus) : txMeta.status } // should update the status of the tx to 'signed'. setTxStatusSigned (txId) { - this.setTxStatus(txId, 'signed') + this._setTxStatus(txId, 'signed') this.emit('update') } // should update the status of the tx to 'rejected'. setTxStatusRejected (txId) { - this.setTxStatus(txId, 'rejected') + this._setTxStatus(txId, 'rejected') this.emit('update') } setTxStatusConfirmed (txId) { - this.setTxStatus(txId, 'confirmed') + this._setTxStatus(txId, 'confirmed') } // merges txParams obj onto txData.txParams // use extend to ensure that all fields are filled updateTxParams (txId, txParams) { - var txData = this.getTx(txId) - txData.txParams = extend(txData, txParams) - this.updateTx(txData) - } - - // sets provider for provider utils and event listener - setProvider (provider) { - this.provider = provider - this.txProviderUtils = new TxProviderUtil(provider) - this.provider.on('block', this.checkForTxInBlock.bind(this)) + var txMeta = this.getTx(txId) + txMeta.txParams = extend(txMeta, txParams) + this.updateTx(txMeta) } // checks if a signed tx is in a block and // if included sets the tx status as 'confirmed' checkForTxInBlock () { - var signedTxList = this.getFilterdTxList({status: 'signed'}) + var signedTxList = this.getFilteredTxList({status: 'signed', err: undefined}) if (!signedTxList.length) return - var self = this signedTxList.forEach((tx) => { var txHash = tx.hash var txId = tx.id if (!txHash) return - // var d - this.txProviderUtils.query.getTransactionByHash(txHash, (err, txData) => { - if (err) { - tx - - return console.error(err) + this.txProviderUtils.query.getTransactionByHash(txHash, (err, txMeta) => { + if (err || !txMeta) { + tx.err = err || 'Tx could possibly have not submitted' + this.updateTx(tx) + return txMeta ? console.error(err) : console.debug(`txMeta is ${txMeta} for:`, tx) } - if (txData.blockNumber !== null) { - self.setTxStatusConfirmed(txId) + if (txMeta.blockNumber) { + this.setTxStatusConfirmed(txId) } }) }) } + + // Private functions + + // Saves the new/updated txList. + // Function is intended only for internal use + _saveTxList (txList) { + this.txList = txList + this._setTxList(txList) + } + + // should return the tx + + // 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! + // - `'signed'` the tx is signed + // - `'submitted'` the tx is sent to a server + // - `'confirmed'` the tx has been included in a block. + _setTxStatus (txId, status) { + var txMeta = this.getTx(txId) + txMeta.status = status + this.emit(`${txMeta.id}:${status}`, txId) + this.updateTx(txMeta) + } + + } -function noop () {} + +const noop = () => console.warn('noop was used no cb provided') -- cgit v1.2.3 From 1ebcbe296b060c9cf431d485d7bb84f696edbdf5 Mon Sep 17 00:00:00 2001 From: Frances Pangilinan Date: Tue, 20 Dec 2016 13:12:14 -0800 Subject: Migrate all tx mutation code out of keyring controller and Fix up txManager to reflect code review requests --- app/scripts/background.js | 4 +- app/scripts/keyring-controller.js | 23 ++--------- app/scripts/metamask-controller.js | 14 ++++++- app/scripts/transaction-manager.js | 84 ++++++++++++++++++++++++-------------- 4 files changed, 71 insertions(+), 54 deletions(-) (limited to 'app') diff --git a/app/scripts/background.js b/app/scripts/background.js index a7e525999..d05ec989f 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -101,10 +101,10 @@ txManager.on('update', updateBadge) function updateBadge () { var label = '' - var unconfTxLen = controller.txManager.unconfTxCount + var unapprovedTxCount = controller.txManager.unapprovedTxCount var unconfMsgs = messageManager.unconfirmedMsgs() var unconfMsgLen = Object.keys(unconfMsgs).length - var count = unconfTxLen + unconfMsgLen + var count = unapprovedTxCount + unconfMsgLen if (count) { label = String(count) } diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 64c2ac933..0a06e4a5d 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -1,6 +1,5 @@ const ethUtil = require('ethereumjs-util') const bip39 = require('bip39') -const Transaction = require('ethereumjs-tx') const EventEmitter = require('events').EventEmitter const filter = require('promise-filter') const normalize = require('./lib/sig-util').normalize @@ -321,28 +320,14 @@ module.exports = class KeyringController extends EventEmitter { // // This method signs tx and returns a promise for // TX Manager to update the state after signing - signTransaction (txParams, cb) { + signTransaction (ethTx, selectedAddress, txId, cb) { try { - const address = normalize(txParams.from) + const address = normalize(selectedAddress) return this.getKeyringForAccount(address) .then((keyring) => { - // Handle gas pricing - var gasMultiplier = this.configManager.getGasMultiplier() || 1 - var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16) - gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10)) - txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber()) - - // normalize values - txParams.to = normalize(txParams.to) - txParams.from = normalize(txParams.from) - txParams.value = normalize(txParams.value) - txParams.data = normalize(txParams.data) - txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) - txParams.nonce = normalize(txParams.nonce) - const tx = new Transaction(txParams) - return keyring.signTransaction(address, tx) + return keyring.signTransaction(address, ethTx) }).then((tx) => { - return {tx, txParams, cb} + this.emit(`${txId}:signed`, {tx, txId, cb}) }) } catch (e) { cb(e) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d5b70c647..7798a6a60 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -39,6 +39,8 @@ module.exports = class MetamaskController { }) this.publicConfigStore = this.initPublicConfigStore() + + var currentFiat = this.configManager.getCurrentFiat() || 'USD' this.configManager.setCurrentFiat(currentFiat) this.configManager.updateConversionRate() @@ -152,8 +154,8 @@ module.exports = class MetamaskController { // tx signing approveTransaction: this.newUnsignedTransaction.bind(this), signTransaction: (...args) => { - var signedTxPromise = keyringController.signTransaction(...args) - this.txManager.resolveSignedTransaction(signedTxPromise) + this.setupSigningListners(...args) + this.txManager.formatTxForSigining(...args) this.sendUpdate() }, @@ -232,6 +234,14 @@ module.exports = class MetamaskController { }) } + setupSigningListners (txParams) { + var txId = txParams.metamaskId + // apply event listeners for signing and formating events + this.txManager.once(`${txId}:formated`, this.keyringController.signTransaction.bind(this.keyringController)) + this.keyringController.once(`${txId}:signed`, this.txManager.resolveSignedTransaction.bind(this.txManager)) + } + + enforceTxValidations (txParams) { if (('value' in txParams) && txParams.value.indexOf('-') === 0) { const msg = `Invalid transaction value of ${txParams.value} not a positive number.` diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index 6b3d1806f..031993424 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -1,8 +1,11 @@ const EventEmitter = require('events') const extend = require('xtend') const ethUtil = require('ethereumjs-util') +const Transaction = require('ethereumjs-tx') +const BN = ethUtil.BN const TxProviderUtil = require('./lib/tx-utils') const createId = require('./lib/random-id') +const normalize = require('./lib/sig-util').normalize module.exports = class TransactionManager extends EventEmitter { constructor (opts) { @@ -75,7 +78,7 @@ module.exports = class TransactionManager extends EventEmitter { this._saveTxList(txList) } - get unconfTxCount () { + get unapprovedTxCount () { return Object.keys(this.getUnapprovedTxList()).length } @@ -128,19 +131,39 @@ module.exports = class TransactionManager extends EventEmitter { } } - resolveSignedTransaction (txPromise) { - const self = this - - txPromise.then(({tx, txParams, cb}) => { - // Add the tx hash to the persisted meta-tx object - var txHash = ethUtil.bufferToHex(tx.hash()) + // formats txParams so the keyringController can sign it + formatTxForSigining (txParams, cb) { + var address = txParams.from + var metaTx = this.getTx(txParams.metamaskId) + var gasMultiplier = metaTx.gasMultiplier + var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16) + gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10)) + txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber()) + + // normalize values + txParams.to = normalize(txParams.to) + txParams.from = normalize(txParams.from) + txParams.value = normalize(txParams.value) + txParams.data = normalize(txParams.data) + txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) + txParams.nonce = normalize(txParams.nonce) + const ethTx = new Transaction(txParams) + // this.updateTxParams(txParams.metamaskId, ethTx) + + // listener is assigned in metamaskController + this.emit(`${txParams.metamaskId}:formated`, ethTx, address, txParams.metamaskId, cb) + } - var metaTx = self.getTx(txParams.metamaskId) - metaTx.hash = txHash - // return raw serialized tx - var rawTx = ethUtil.bufferToHex(tx.serialize()) - cb(null, rawTx) - }) + // receives a signed tx object and updates the tx hash + // and pass it to the cb to be sent off + resolveSignedTransaction ({tx, txId, cb}) { + // Add the tx hash to the persisted meta-tx object + var txHash = ethUtil.bufferToHex(tx.hash()) + var metaTx = this.getTx(txId) + metaTx.hash = txHash + this.updateTx(metaTx) + var rawTx = ethUtil.bufferToHex(tx.serialize()) + cb(null, rawTx) } /* @@ -176,20 +199,23 @@ module.exports = class TransactionManager extends EventEmitter { }) } - // should return the status of the tx. - getTxStatus (txId, cb) { + // STATUS METHODS + // get::set status + + // should return the status of the tx. + getTxStatus (txId) { const txMeta = this.getTx(txId) - return cb ? cb(txMeta.staus) : txMeta.status + return txMeta.status } - // should update the status of the tx to 'signed'. + // should update the status of the tx to 'signed'. setTxStatusSigned (txId) { this._setTxStatus(txId, 'signed') this.emit('update') } - // should update the status of the tx to 'rejected'. + // should update the status of the tx to 'rejected'. setTxStatusRejected (txId) { this._setTxStatus(txId, 'rejected') this.emit('update') @@ -203,7 +229,7 @@ module.exports = class TransactionManager extends EventEmitter { // use extend to ensure that all fields are filled updateTxParams (txId, txParams) { var txMeta = this.getTx(txId) - txMeta.txParams = extend(txMeta, txParams) + txMeta.txParams = extend(txMeta.txParams, txParams) this.updateTx(txMeta) } @@ -218,7 +244,7 @@ module.exports = class TransactionManager extends EventEmitter { if (!txHash) return this.txProviderUtils.query.getTransactionByHash(txHash, (err, txMeta) => { if (err || !txMeta) { - tx.err = err || 'Tx could possibly have not submitted' + tx.err = err || 'Tx could possibly have not been submitted' this.updateTx(tx) return txMeta ? console.error(err) : console.debug(`txMeta is ${txMeta} for:`, tx) } @@ -229,16 +255,7 @@ module.exports = class TransactionManager extends EventEmitter { }) } - // Private functions - - // Saves the new/updated txList. - // Function is intended only for internal use - _saveTxList (txList) { - this.txList = txList - this._setTxList(txList) - } - - // should return the tx + // PRIVATE METHODS // Should find the tx in the tx list and // update it. @@ -255,7 +272,12 @@ module.exports = class TransactionManager extends EventEmitter { this.updateTx(txMeta) } - + // Saves the new/updated txList. + // Function is intended only for internal use + _saveTxList (txList) { + this.txList = txList + this._setTxList(txList) + } } -- cgit v1.2.3 From a85c691b71d5142d2412000930328fbe9161760a Mon Sep 17 00:00:00 2001 From: Frances Pangilinan Date: Wed, 21 Dec 2016 14:06:15 -0800 Subject: Remove txManager in keyring controller --- app/scripts/keyring-controller.js | 2 -- 1 file changed, 2 deletions(-) (limited to 'app') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 0a06e4a5d..a58742228 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -33,11 +33,9 @@ module.exports = class KeyringController extends EventEmitter { this.ethStore = opts.ethStore this.encryptor = encryptor this.keyringTypes = keyringTypes - this.txManager = opts.txManager this.keyrings = [] this.identities = {} // Essentially a name hash - this._unconfTxCbs = {} this._unconfMsgCbs = {} this.getNetwork = opts.getNetwork -- cgit v1.2.3 From 9e1c90eafcace4ac67100ae0a567faba13f9e618 Mon Sep 17 00:00:00 2001 From: Frances Pangilinan Date: Wed, 21 Dec 2016 14:46:10 -0800 Subject: fix merge --- app/scripts/metamask-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 125753063..6aec825fe 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -64,7 +64,7 @@ module.exports = class MetamaskController { this.ethStore.getState(), this.configManager.getConfig(), this.keyringController.getState(), - this.txManager.getState() + this.txManager.getState(), this.noticeController.getState() ) } -- cgit v1.2.3 From fde69ea0baf32b5d2a6932b73f4772e983aef552 Mon Sep 17 00:00:00 2001 From: Frankie Date: Fri, 23 Dec 2016 12:34:12 -0800 Subject: fix some minor spelling mistakes and clean up code --- app/scripts/background.js | 2 +- app/scripts/metamask-controller.js | 10 ++------- app/scripts/transaction-manager.js | 42 +++++++++++++++++++++----------------- 3 files changed, 26 insertions(+), 28 deletions(-) (limited to 'app') diff --git a/app/scripts/background.js b/app/scripts/background.js index d05ec989f..ca2efc114 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -97,7 +97,7 @@ function setupControllerConnection (stream) { // plugin badge text // -txManager.on('update', updateBadge) +txManager.on('updateBadge', updateBadge) function updateBadge () { var label = '' diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 6aec825fe..4af5cd78d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -21,7 +21,6 @@ module.exports = class MetamaskController { this.configManager = new ConfigManager(opts) this.keyringController = new KeyringController({ configManager: this.configManager, - txManager: this.txManager, getNetwork: this.getStateNetwork.bind(this), }) // notices @@ -40,6 +39,7 @@ module.exports = class MetamaskController { txList: this.configManager.getTxList(), txHistoryLimit: 40, setTxList: this.configManager.setTxList.bind(this.configManager), + getSelectedAccount: this.configManager.getSelectedAccount.bind(this.configManager), getGasMultiplier: this.configManager.getGasMultiplier.bind(this.configManager), getNetwork: this.getStateNetwork.bind(this), provider: this.provider, @@ -47,8 +47,6 @@ module.exports = class MetamaskController { }) this.publicConfigStore = this.initPublicConfigStore() - - var currentFiat = this.configManager.getCurrentFiat() || 'USD' this.configManager.setCurrentFiat(currentFiat) this.configManager.updateConversionRate() @@ -105,9 +103,6 @@ module.exports = class MetamaskController { signMessage: keyringController.signMessage.bind(keyringController), cancelMessage: keyringController.cancelMessage.bind(keyringController), - // forward directly to txManager - getUnapprovedTxList: txManager.getUnapprovedTxList.bind(txManager), - getFilteredTxList: txManager.getFilteredTxList.bind(txManager), // coinbase buyEth: this.buyEth.bind(this), // shapeshift @@ -251,11 +246,10 @@ module.exports = class MetamaskController { setupSigningListners (txParams) { var txId = txParams.metamaskId // apply event listeners for signing and formating events - this.txManager.once(`${txId}:formated`, this.keyringController.signTransaction.bind(this.keyringController)) + this.txManager.once(`${txId}:formatted`, this.keyringController.signTransaction.bind(this.keyringController)) this.keyringController.once(`${txId}:signed`, this.txManager.resolveSignedTransaction.bind(this.txManager)) } - enforceTxValidations (txParams) { if (('value' in txParams) && txParams.value.indexOf('-') === 0) { const msg = `Invalid transaction value of ${txParams.value} not a positive number.` diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index 031993424..fd136a51b 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -12,10 +12,8 @@ module.exports = class TransactionManager extends EventEmitter { super() this.txList = opts.txList || [] this._setTxList = opts.setTxList - this._unconfTxCbs = {} this.txHistoryLimit = opts.txHistoryLimit - // txManager :: tx approvals and rejection cb's - + this.getSelectedAccount = opts.getSelectedAccount this.provider = opts.provider this.blockTracker = opts.blockTracker this.txProviderUtils = new TxProviderUtil(this.provider) @@ -25,9 +23,11 @@ module.exports = class TransactionManager extends EventEmitter { } getState () { + var selectedAccount = this.getSelectedAccount() return { transactions: this.getTxList(), unconfTxs: this.getUnapprovedTxList(), + selectedAccountTxList: this.getFilteredTxList({metamaskNetworkId: this.getNetwork(), from: selectedAccount}), } } @@ -37,14 +37,21 @@ module.exports = class TransactionManager extends EventEmitter { } // Adds a tx to the txlist - addTx (txMeta, onTxDoneCb = noop) { + addTx (txMeta, onTxDoneCb = warn) { var txList = this.getTxList() var txHistoryLimit = this.txHistoryLimit + + // checks if the length of th 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 (txList.length > txHistoryLimit - 1) { var index = txList.findIndex((metaTx) => metaTx.status === 'confirmed' || metaTx.status === 'rejected') - index ? txList.splice(index, index) : txList.shift() + txList.splice(index, 1) } txList.push(txMeta) + this._saveTxList(txList) // keep the onTxDoneCb around in a listener // for after approval/denial (requires user interaction) @@ -58,14 +65,14 @@ module.exports = class TransactionManager extends EventEmitter { onTxDoneCb(null, false) }) - this.emit('update') + this.emit('updateBadge') this.emit(`${txMeta.id}:unapproved`, txMeta) } // gets tx by Id and returns it getTx (txId, cb) { var txList = this.getTxList() - var txMeta = txList.find((txData) => txData.id === txId) + var txMeta = txList.find(txData => txData.id === txId) return cb ? cb(txMeta) : txMeta } @@ -73,7 +80,7 @@ module.exports = class TransactionManager extends EventEmitter { updateTx (txMeta) { var txId = txMeta.id var txList = this.getTxList() - var index = txList.findIndex((txData) => txData.id === txId) + var index = txList.findIndex(txData => txData.id === txId) txList[index] = txMeta this._saveTxList(txList) } @@ -119,16 +126,14 @@ module.exports = class TransactionManager extends EventEmitter { }, {}) } - approveTransaction (txId, cb) { + approveTransaction (txId, cb = warn) { this.setTxStatusSigned(txId) cb() } - cancelTransaction (txId, cb) { + cancelTransaction (txId, cb = warn) { this.setTxStatusRejected(txId) - if (cb && typeof cb === 'function') { - cb() - } + cb() } // formats txParams so the keyringController can sign it @@ -148,15 +153,14 @@ module.exports = class TransactionManager extends EventEmitter { txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) txParams.nonce = normalize(txParams.nonce) const ethTx = new Transaction(txParams) - // this.updateTxParams(txParams.metamaskId, ethTx) // listener is assigned in metamaskController - this.emit(`${txParams.metamaskId}:formated`, ethTx, address, txParams.metamaskId, cb) + this.emit(`${txParams.metamaskId}:formatted`, ethTx, address, txParams.metamaskId, cb) } // receives a signed tx object and updates the tx hash // and pass it to the cb to be sent off - resolveSignedTransaction ({tx, txId, cb}) { + resolveSignedTransaction ({tx, txId, cb = warn}) { // Add the tx hash to the persisted meta-tx object var txHash = ethUtil.bufferToHex(tx.hash()) var metaTx = this.getTx(txId) @@ -212,13 +216,13 @@ module.exports = class TransactionManager extends EventEmitter { // should update the status of the tx to 'signed'. setTxStatusSigned (txId) { this._setTxStatus(txId, 'signed') - this.emit('update') + this.emit('updateBadge') } // should update the status of the tx to 'rejected'. setTxStatusRejected (txId) { this._setTxStatus(txId, 'rejected') - this.emit('update') + this.emit('updateBadge') } setTxStatusConfirmed (txId) { @@ -281,4 +285,4 @@ module.exports = class TransactionManager extends EventEmitter { } -const noop = () => console.warn('noop was used no cb provided') +const warn = () => console.warn('warn was used no cb provided') -- cgit v1.2.3 From 616721f47d5ead283378abf63aedcc953506376a Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 2 Jan 2017 12:20:09 -0800 Subject: inpage-provider - add isMetaMask adds `metamaskInpageProvider.isMetaMask === true` Fixes #727 --- app/scripts/lib/inpage-provider.js | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app') diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index a64c745ce..11bd5cc3a 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -111,6 +111,8 @@ MetamaskInpageProvider.prototype.isConnected = function () { return true } +MetamaskInpageProvider.prototype.isMetaMask = true + // util function remoteStoreWithLocalStorageCache (storageKey) { -- cgit v1.2.3