diff --git a/CHANGELOG.md b/CHANGELOG.md index b23fc701a..420296bbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Add a check for when a tx is included in a block. - Minor modifications to network display. - Network now displays properly for pending transactions. +- Implement replay attack protections allowed by EIP 155. +- Fix bug where sometimes loading account data would fail by querying a future block. ## 2.14.1 2016-12-20 diff --git a/app/scripts/background.js b/app/scripts/background.js index ca2efc114..6b7926526 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -22,7 +22,6 @@ const controller = new MetamaskController({ setData, loadData, }) -const keyringController = controller.keyringController const txManager = controller.txManager function triggerUi () { if (!popupIsOpen) notification.show() @@ -81,13 +80,11 @@ function setupControllerConnection (stream) { stream.pipe(dnode).pipe(stream) dnode.on('remote', (remote) => { // push updates to popup - controller.ethStore.on('update', controller.sendUpdate.bind(controller)) - controller.listeners.push(remote) - keyringController.on('update', controller.sendUpdate.bind(controller)) - + var sendUpdate = remote.sendUpdate.bind(remote) + controller.on('update', sendUpdate) // teardown on disconnect eos(stream, () => { - controller.ethStore.removeListener('update', controller.sendUpdate.bind(controller)) + controller.removeListener('update', sendUpdate) popupIsOpen = false }) }) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 92429f7f5..c58be0aae 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -316,20 +316,16 @@ module.exports = class KeyringController extends EventEmitter { // // This method signs tx and returns a promise for // TX Manager to update the state after signing - signTransaction (ethTx, selectedAddress, txId, cb) { - try { - const address = normalize(selectedAddress) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signTransaction(address, ethTx) - }).then((tx) => { - this.emit(`${txId}:signed`, {tx, txId, cb}) - }) - } catch (e) { - cb(e) - } - } + signTransaction (ethTx, selectedAddress, txId) { + const address = normalize(selectedAddress) + return this.getKeyringForAccount(address) + .then((keyring) => { + return keyring.signTransaction(address, ethTx) + }).then((tx) => { + return {tx, txId} + }) + } // Add Unconfirmed Message // @object msgParams // @function cb diff --git a/app/scripts/lib/eth-store.js b/app/scripts/lib/eth-store.js new file mode 100644 index 000000000..a42b2417f --- /dev/null +++ b/app/scripts/lib/eth-store.js @@ -0,0 +1,144 @@ +/* Ethereum Store + * + * This module is responsible for tracking any number of accounts + * and caching their current balances & transaction counts. + * + * It also tracks transaction hashes, and checks their inclusion status + * on each new block. + */ + +const EventEmitter = require('events').EventEmitter +const inherits = require('util').inherits +const async = require('async') +const clone = require('clone') +const EthQuery = require('eth-query') + +module.exports = EthereumStore + + +inherits(EthereumStore, EventEmitter) +function EthereumStore(engine) { + const self = this + EventEmitter.call(self) + self._currentState = { + accounts: {}, + transactions: {}, + } + self._query = new EthQuery(engine) + + engine.on('block', self._updateForBlock.bind(self)) +} + +// +// public +// + +EthereumStore.prototype.getState = function () { + const self = this + return clone(self._currentState) +} + +EthereumStore.prototype.addAccount = function (address) { + const self = this + self._currentState.accounts[address] = {} + self._didUpdate() + if (!self.currentBlockNumber) return + self._updateAccount(address, noop) +} + +EthereumStore.prototype.removeAccount = function (address) { + const self = this + delete self._currentState.accounts[address] + self._didUpdate() +} + +EthereumStore.prototype.addTransaction = function (txHash) { + const self = this + self._currentState.transactions[txHash] = {} + self._didUpdate() + if (!self.currentBlockNumber) return + self._updateTransaction(self.currentBlockNumber, txHash, noop) +} + +EthereumStore.prototype.removeTransaction = function (address) { + const self = this + delete self._currentState.transactions[address] + self._didUpdate() +} + + +// +// private +// + +EthereumStore.prototype._didUpdate = function () { + const self = this + var state = self.getState() + self.emit('update', state) +} + +EthereumStore.prototype._updateForBlock = function (block) { + const self = this + var blockNumber = '0x' + block.number.toString('hex') + self.currentBlockNumber = blockNumber + async.parallel([ + self._updateAccounts.bind(self), + self._updateTransactions.bind(self, blockNumber), + ], function (err) { + if (err) return console.error(err) + self.emit('block', self.getState()) + self._didUpdate() + }) +} + +EthereumStore.prototype._updateAccounts = function (cb) { + var accountsState = this._currentState.accounts + var addresses = Object.keys(accountsState) + async.each(addresses, this._updateAccount.bind(this), cb) +} + +EthereumStore.prototype._updateAccount = function (address, cb) { + var accountsState = this._currentState.accounts + this.getAccount(address, function (err, result) { + if (err) return cb(err) + result.address = address + // only populate if the entry is still present + if (accountsState[address]) { + accountsState[address] = result + } + cb(null, result) + }) +} + +EthereumStore.prototype.getAccount = function (address, cb) { + const query = this._query + async.parallel({ + balance: query.getBalance.bind(query, address), + nonce: query.getTransactionCount.bind(query, address), + code: query.getCode.bind(query, address), + }, cb) +} + +EthereumStore.prototype._updateTransactions = function (block, cb) { + const self = this + var transactionsState = self._currentState.transactions + var txHashes = Object.keys(transactionsState) + async.each(txHashes, self._updateTransaction.bind(self, block), cb) +} + +EthereumStore.prototype._updateTransaction = function (block, txHash, cb) { + const self = this + // would use the block here to determine how many confirmations the tx has + var transactionsState = self._currentState.transactions + self._query.getTransaction(txHash, function (err, result) { + if (err) return cb(err) + // only populate if the entry is still present + if (transactionsState[txHash]) { + transactionsState[txHash] = result + self._didUpdate() + } + cb(null, result) + }) +} + +function noop() {} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index c0d2f3b4c..555460f3d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1,5 +1,6 @@ +const EventEmitter = require('events') const extend = require('xtend') -const EthStore = require('eth-store') +const EthStore = require('./lib/eth-store') const MetaMaskProvider = require('web3-provider-engine/zero.js') const KeyringController = require('./keyring-controller') const NoticeController = require('./notice-controller') @@ -13,12 +14,12 @@ const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const IdStoreMigrator = require('./lib/idStore-migrator') -module.exports = class MetamaskController { +module.exports = class MetamaskController extends EventEmitter { constructor (opts) { + super() this.state = { network: 'loading' } this.opts = opts - this.listeners = [] this.configManager = new ConfigManager(opts) this.keyringController = new KeyringController({ configManager: this.configManager, @@ -62,6 +63,7 @@ module.exports = class MetamaskController { }) this.ethStore.on('update', this.sendUpdate.bind(this)) + this.keyringController.on('update', this.sendUpdate.bind(this)) } getState () { @@ -165,10 +167,7 @@ module.exports = class MetamaskController { sendUpdate () { this.getState() .then((state) => { - - this.listeners.forEach((remote) => { - remote.sendUpdate(state) - }) + this.emit('update', state) }) } @@ -185,10 +184,23 @@ module.exports = class MetamaskController { }, // tx signing approveTransaction: this.newUnsignedTransaction.bind(this), - signTransaction: (...args) => { - this.setupSigningListners(...args) - this.txManager.formatTxForSigining(...args) - this.sendUpdate() + signTransaction: (txParams, cb) => { + this.txManager.formatTxForSigining(txParams) + .then(({ethTx, address, txId}) => { + return this.keyringController.signTransaction(ethTx, address, txId) + }) + .then(({tx, txId}) => { + return this.txManager.resolveSignedTransaction({tx, txId}) + }) + .then((rawTx) => { + cb(null, rawTx) + this.sendUpdate() + this.txManager.emit(`${txParams.metamaskId}:signingComplete`) + }) + .catch((err) => { + console.error(err) + cb(err) + }) }, // msg signing @@ -266,13 +278,6 @@ module.exports = class MetamaskController { }) } - setupSigningListners (txParams) { - var txId = txParams.metamaskId - // apply event listeners for signing and formating events - 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.` @@ -453,7 +458,7 @@ module.exports = class MetamaskController { return this.state.network } - markAccountsFound(cb) { + markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() cb(null, this.getState()) diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index fd136a51b..6becfa6d1 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -128,7 +128,7 @@ module.exports = class TransactionManager extends EventEmitter { approveTransaction (txId, cb = warn) { this.setTxStatusSigned(txId) - cb() + this.once(`${txId}:signingComplete`, cb) } cancelTransaction (txId, cb = warn) { @@ -137,7 +137,7 @@ module.exports = class TransactionManager extends EventEmitter { } // formats txParams so the keyringController can sign it - formatTxForSigining (txParams, cb) { + formatTxForSigining (txParams) { var address = txParams.from var metaTx = this.getTx(txParams.metamaskId) var gasMultiplier = metaTx.gasMultiplier @@ -153,9 +153,8 @@ module.exports = class TransactionManager extends EventEmitter { txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) txParams.nonce = normalize(txParams.nonce) const ethTx = new Transaction(txParams) - - // listener is assigned in metamaskController - this.emit(`${txParams.metamaskId}:formatted`, ethTx, address, txParams.metamaskId, cb) + var txId = txParams.metamaskId + return Promise.resolve({ethTx, address, txId}) } // receives a signed tx object and updates the tx hash @@ -167,7 +166,8 @@ module.exports = class TransactionManager extends EventEmitter { metaTx.hash = txHash this.updateTx(metaTx) var rawTx = ethUtil.bufferToHex(tx.serialize()) - cb(null, rawTx) + return Promise.resolve(rawTx) + } /* diff --git a/package.json b/package.json index 4c33ad9ab..6f0fe00c7 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "eth-bin-to-ops": "^1.0.1", "eth-lightwallet": "^2.3.3", "eth-query": "^1.0.3", - "eth-store": "^1.1.0", "ethereumjs-tx": "^1.0.0", "ethereumjs-util": "^4.4.0", "ethereumjs-wallet": "^0.6.0",