diff --git a/CHANGELOG.md b/CHANGELOG.md index a3b97d477..8383e104d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Remove certain non-essential permissions from certain builds. - Add a check for when a tx is included in a block. +- 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/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/metamask-controller.js b/app/scripts/metamask-controller.js index 76ca99b25..555460f3d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -184,10 +184,23 @@ module.exports = class MetamaskController extends EventEmitter { }, // 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 @@ -265,13 +278,6 @@ module.exports = class MetamaskController extends EventEmitter { }) } - 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.` @@ -452,7 +458,7 @@ module.exports = class MetamaskController extends EventEmitter { 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) + } /*