From 1f1fc2c49ecbb5c6a0a1d925d5c02cf48f795b2f Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 19 Dec 2017 11:16:11 -0330 Subject: [PATCH 1/5] Canceled, edited transactions show edited amount. --- app/scripts/controllers/transactions.js | 5 +++++ app/scripts/metamask-controller.js | 1 + ui/app/actions.js | 19 +++++++++++++++++++ .../pending-tx/confirm-send-ether.js | 9 ++++++++- .../pending-tx/confirm-send-token.js | 9 ++++++++- 5 files changed, 41 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index ce709bd28..0d6b97d51 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -189,6 +189,11 @@ module.exports = class TransactionController extends EventEmitter { await this.approveTransaction(txMeta.id) } + async updateAndCancelTransaction (txMeta) { + this.txStateManager.updateTx(txMeta, 'confTx: user rejected transaction') + await this.cancelTransaction(txMeta.id) + } + async approveTransaction (txId) { let nonceLock try { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 018eb2c76..935a3e76e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -366,6 +366,7 @@ module.exports = class MetamaskController extends EventEmitter { // txController cancelTransaction: nodeify(txController.cancelTransaction, txController), updateAndApproveTransaction: nodeify(txController.updateAndApproveTransaction, txController), + updateAndCancelTransaction: nodeify(txController.updateAndCancelTransaction, txController), // messageManager signMessage: nodeify(this.signMessage, this), diff --git a/ui/app/actions.js b/ui/app/actions.js index e694152a2..9d3d7184e 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -126,6 +126,7 @@ var actions = { signTx: signTx, signTokenTx: signTokenTx, updateAndApproveTx, + updateAndCancelTx, cancelTx: cancelTx, completedTx: completedTx, txError: txError, @@ -710,6 +711,24 @@ function updateAndApproveTx (txData) { } } +function updateAndCancelTx (txData) { + log.info('actions: updateAndCancelTx: ' + JSON.stringify(txData)) + return (dispatch) => { + log.debug(`actions calling background.updateAndCancelTx`) + background.updateAndCancelTransaction(txData, (err) => { + dispatch(actions.hideLoadingIndication()) + dispatch(actions.updateTransactionParams(txData.id, txData.txParams)) + dispatch(actions.clearSend()) + if (err) { + dispatch(actions.txError(err)) + dispatch(actions.goHome()) + return log.error(err.message) + } + dispatch(actions.completedTx(txData.id)) + }) + } +} + function completedTx (id) { return { type: actions.COMPLETED_TX, diff --git a/ui/app/components/pending-tx/confirm-send-ether.js b/ui/app/components/pending-tx/confirm-send-ether.js index 1264da153..01195502e 100644 --- a/ui/app/components/pending-tx/confirm-send-ether.js +++ b/ui/app/components/pending-tx/confirm-send-ether.js @@ -55,6 +55,7 @@ function mapDispatchToProps (dispatch) { dispatch(actions.showSendPage()) }, cancelTransaction: ({ id }) => dispatch(actions.cancelTx({ id })), + updateAndCancelTx: txMeta => dispatch(actions.updateAndCancelTx(txMeta)), } } @@ -421,7 +422,13 @@ ConfirmSendEther.prototype.onSubmit = function (event) { ConfirmSendEther.prototype.cancel = function (event, txMeta) { event.preventDefault() - this.props.cancelTransaction(txMeta) + const { send, updateAndCancelTx, cancelTransaction } = this.props + + if (send.editingTransactionId) { + updateAndCancelTx(txMeta) + } else { + cancelTransaction(txMeta) + } } ConfirmSendEther.prototype.checkValidity = function () { diff --git a/ui/app/components/pending-tx/confirm-send-token.js b/ui/app/components/pending-tx/confirm-send-token.js index 727cd260b..e6ce3f6e6 100644 --- a/ui/app/components/pending-tx/confirm-send-token.js +++ b/ui/app/components/pending-tx/confirm-send-token.js @@ -89,6 +89,7 @@ function mapDispatchToProps (dispatch, ownProps) { })) dispatch(actions.showSendTokenPage()) }, + updateAndCancelTx: txMeta => dispatch(actions.updateAndCancelTx(txMeta)), } } @@ -415,7 +416,13 @@ ConfirmSendToken.prototype.onSubmit = function (event) { ConfirmSendToken.prototype.cancel = function (event, txMeta) { event.preventDefault() - this.props.cancelTransaction(txMeta) + const { send, updateAndCancelTx, cancelTransaction } = this.props + + if (send.editingTransactionId) { + updateAndCancelTx(txMeta) + } else { + cancelTransaction(txMeta) + } } ConfirmSendToken.prototype.checkValidity = function () { From bf4043c59bb67ea93599207d91cb7a4f4426e75f Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 20 Dec 2017 13:47:16 -0330 Subject: [PATCH 2/5] Adds updateTransaction to background and used it to update after editing in send-v2. --- app/scripts/controllers/transactions.js | 9 +-- app/scripts/metamask-controller.js | 2 +- ui/app/actions.js | 22 +++--- .../pending-tx/confirm-send-ether.js | 29 +------ .../pending-tx/confirm-send-token.js | 42 +--------- ui/app/components/send/send-v2-container.js | 2 + ui/app/conf-tx.js | 2 +- ui/app/send-v2.js | 78 +++++++++++++++---- 8 files changed, 85 insertions(+), 101 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 0d6b97d51..5b687f67a 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -184,16 +184,15 @@ module.exports = class TransactionController extends EventEmitter { return await this.txGasUtil.analyzeGasUsage(txMeta) } + async updateTransaction (txMeta) { + this.txStateManager.updateTx(txMeta, 'confTx: user updated transaction') + } + async updateAndApproveTransaction (txMeta) { this.txStateManager.updateTx(txMeta, 'confTx: user approved transaction') await this.approveTransaction(txMeta.id) } - async updateAndCancelTransaction (txMeta) { - this.txStateManager.updateTx(txMeta, 'confTx: user rejected transaction') - await this.cancelTransaction(txMeta.id) - } - async approveTransaction (txId) { let nonceLock try { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 935a3e76e..a2d584cf9 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -365,8 +365,8 @@ module.exports = class MetamaskController extends EventEmitter { // txController cancelTransaction: nodeify(txController.cancelTransaction, txController), + updateTransaction: nodeify(txController.updateTransaction, txController), updateAndApproveTransaction: nodeify(txController.updateAndApproveTransaction, txController), - updateAndCancelTransaction: nodeify(txController.updateAndCancelTransaction, txController), // messageManager signMessage: nodeify(this.signMessage, this), diff --git a/ui/app/actions.js b/ui/app/actions.js index 9d3d7184e..ef8a5ec46 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -125,8 +125,8 @@ var actions = { sendTx: sendTx, signTx: signTx, signTokenTx: signTokenTx, + updateTransaction, updateAndApproveTx, - updateAndCancelTx, cancelTx: cancelTx, completedTx: completedTx, txError: txError, @@ -693,29 +693,28 @@ function signTokenTx (tokenAddress, toAddress, amount, txData) { } } -function updateAndApproveTx (txData) { - log.info('actions: updateAndApproveTx: ' + JSON.stringify(txData)) +function updateTransaction (txData) { + log.info('actions: updateTx: ' + JSON.stringify(txData)) return (dispatch) => { - log.debug(`actions calling background.updateAndApproveTx`) - background.updateAndApproveTransaction(txData, (err) => { + log.debug(`actions calling background.updateTx`) + background.updateTransaction(txData, (err) => { dispatch(actions.hideLoadingIndication()) dispatch(actions.updateTransactionParams(txData.id, txData.txParams)) - dispatch(actions.clearSend()) if (err) { dispatch(actions.txError(err)) dispatch(actions.goHome()) return log.error(err.message) } - dispatch(actions.completedTx(txData.id)) + dispatch(actions.showConfTxPage({ id: txData.id })) }) } } -function updateAndCancelTx (txData) { - log.info('actions: updateAndCancelTx: ' + JSON.stringify(txData)) +function updateAndApproveTx (txData) { + log.info('actions: updateAndApproveTx: ' + JSON.stringify(txData)) return (dispatch) => { - log.debug(`actions calling background.updateAndCancelTx`) - background.updateAndCancelTransaction(txData, (err) => { + log.debug(`actions calling background.updateAndApproveTx`) + background.updateAndApproveTransaction(txData, (err) => { dispatch(actions.hideLoadingIndication()) dispatch(actions.updateTransactionParams(txData.id, txData.txParams)) dispatch(actions.clearSend()) @@ -773,6 +772,7 @@ function cancelTx (txData) { return (dispatch) => { log.debug(`background.cancelTransaction`) background.cancelTransaction(txData.id, () => { + dispatch(actions.clearSend()) dispatch(actions.completedTx(txData.id)) }) } diff --git a/ui/app/components/pending-tx/confirm-send-ether.js b/ui/app/components/pending-tx/confirm-send-ether.js index 01195502e..566224864 100644 --- a/ui/app/components/pending-tx/confirm-send-ether.js +++ b/ui/app/components/pending-tx/confirm-send-ether.js @@ -55,7 +55,6 @@ function mapDispatchToProps (dispatch) { dispatch(actions.showSendPage()) }, cancelTransaction: ({ id }) => dispatch(actions.cancelTx({ id })), - updateAndCancelTx: txMeta => dispatch(actions.updateAndCancelTx(txMeta)), } } @@ -422,13 +421,9 @@ ConfirmSendEther.prototype.onSubmit = function (event) { ConfirmSendEther.prototype.cancel = function (event, txMeta) { event.preventDefault() - const { send, updateAndCancelTx, cancelTransaction } = this.props + const { cancelTransaction } = this.props - if (send.editingTransactionId) { - updateAndCancelTx(txMeta) - } else { - cancelTransaction(txMeta) - } + cancelTransaction(txMeta) } ConfirmSendEther.prototype.checkValidity = function () { @@ -452,26 +447,6 @@ ConfirmSendEther.prototype.gatherTxMeta = function () { const state = this.state const txData = clone(state.txData) || clone(props.txData) - if (props.send.editingTransactionId) { - const { - send: { - memo, - amount: value, - gasLimit: gas, - gasPrice, - }, - } = props - const { txParams: { from, to } } = txData - txData.txParams = { - from: ethUtil.addHexPrefix(from), - to: ethUtil.addHexPrefix(to), - memo: memo && ethUtil.addHexPrefix(memo), - value: ethUtil.addHexPrefix(value), - gas: ethUtil.addHexPrefix(gas), - gasPrice: ethUtil.addHexPrefix(gasPrice), - } - } - // log.debug(`UI has defaulted to tx meta ${JSON.stringify(txData)}`) return txData } diff --git a/ui/app/components/pending-tx/confirm-send-token.js b/ui/app/components/pending-tx/confirm-send-token.js index e6ce3f6e6..a07835911 100644 --- a/ui/app/components/pending-tx/confirm-send-token.js +++ b/ui/app/components/pending-tx/confirm-send-token.js @@ -89,7 +89,6 @@ function mapDispatchToProps (dispatch, ownProps) { })) dispatch(actions.showSendTokenPage()) }, - updateAndCancelTx: txMeta => dispatch(actions.updateAndCancelTx(txMeta)), } } @@ -416,13 +415,9 @@ ConfirmSendToken.prototype.onSubmit = function (event) { ConfirmSendToken.prototype.cancel = function (event, txMeta) { event.preventDefault() - const { send, updateAndCancelTx, cancelTransaction } = this.props + const { send, cancelTransaction } = this.props - if (send.editingTransactionId) { - updateAndCancelTx(txMeta) - } else { - cancelTransaction(txMeta) - } + cancelTransaction(txMeta) } ConfirmSendToken.prototype.checkValidity = function () { @@ -446,39 +441,6 @@ ConfirmSendToken.prototype.gatherTxMeta = function () { const state = this.state const txData = clone(state.txData) || clone(props.txData) - if (props.send.editingTransactionId) { - const { - send: { - memo, - amount, - gasLimit: gas, - gasPrice, - to, - }, - } = props - - const { txParams: { from, to: tokenAddress } } = txData - - const tokenParams = { - from: ethUtil.addHexPrefix(from), - value: '0', - gas: ethUtil.addHexPrefix(gas), - gasPrice: ethUtil.addHexPrefix(gasPrice), - } - - const data = '0xa9059cbb' + Array.prototype.map.call( - ethAbi.rawEncode(['address', 'uint256'], [to, ethUtil.addHexPrefix(amount)]), - x => ('00' + x.toString(16)).slice(-2) - ).join('') - - txData.txParams = { - ...tokenParams, - to: ethUtil.addHexPrefix(tokenAddress), - memo: memo && ethUtil.addHexPrefix(memo), - data, - } - } - // log.debug(`UI has defaulted to tx meta ${JSON.stringify(txData)}`) return txData } diff --git a/ui/app/components/send/send-v2-container.js b/ui/app/components/send/send-v2-container.js index 655de8897..ff7714e82 100644 --- a/ui/app/components/send/send-v2-container.js +++ b/ui/app/components/send/send-v2-container.js @@ -50,6 +50,7 @@ function mapStateToProps (state) { data, amountConversionRate: selectedToken ? tokenToFiatRate : conversionRate, tokenContract: getSelectedTokenContract(state), + unapprovedTxs: state.metamask.unapprovedTxs, } } @@ -64,6 +65,7 @@ function mapDispatchToProps (dispatch) { ), signTx: txParams => dispatch(actions.signTx(txParams)), updateAndApproveTx: txParams => dispatch(actions.updateAndApproveTx(txParams)), + updateTx: txData => dispatch(actions.updateTransaction(txData)), setSelectedAddress: address => dispatch(actions.setSelectedAddress(address)), addToAddressBook: address => dispatch(actions.addToAddressBook(address)), updateGasTotal: newTotal => dispatch(actions.updateGasTotal(newTotal)), diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index 9f273aaec..d71e4b35f 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -115,7 +115,7 @@ function currentTxView (opts) { log.info('rendering current tx view') const { txData } = opts const { txParams, msgParams } = txData - + console.log(`22222 currentTxView txData`, txData); if (txParams) { log.debug('txParams detected, rendering pending tx') return h(PendingTx, opts) diff --git a/ui/app/send-v2.js b/ui/app/send-v2.js index e1b88f0db..617211b20 100644 --- a/ui/app/send-v2.js +++ b/ui/app/send-v2.js @@ -2,6 +2,7 @@ const { inherits } = require('util') const PersistentForm = require('../lib/persistent-form') const h = require('react-hyperscript') +const ethAbi = require('ethereumjs-abi') const ethUtil = require('ethereumjs-util') const Identicon = require('./components/identicon') @@ -552,6 +553,47 @@ SendTransactionScreen.prototype.addToAddressBookIfNew = function (newAddress) { } } +SendTransactionScreen.prototype.getEditedTx = function () { + const { + from: {address: from}, + to, + amount, + gasLimit: gas, + gasPrice, + selectedToken, + editingTransactionId, + unapprovedTxs, + } = this.props + + const editingTx = unapprovedTxs[editingTransactionId] + + editingTx.txParams = { + from: ethUtil.addHexPrefix(from), + gas: ethUtil.addHexPrefix(gas), + gasPrice: ethUtil.addHexPrefix(gasPrice), + } + + if (selectedToken) { + const data = '0xa9059cbb' + Array.prototype.map.call( + ethAbi.rawEncode(['address', 'uint256'], [to, ethUtil.addHexPrefix(amount)]), + x => ('00' + x.toString(16)).slice(-2) + ).join('') + + Object.assign(editingTx.txParams, { + value: ethUtil.addHexPrefix('0'), + to: ethUtil.addHexPrefix(selectedToken.address), + data, + }) + } else { + Object.assign(editingTx.txParams, { + value: ethUtil.addHexPrefix(amount), + to: ethUtil.addHexPrefix(to), + }) + } + + return editingTx +} + SendTransactionScreen.prototype.onSubmit = function (event) { event.preventDefault() const { @@ -562,10 +604,12 @@ SendTransactionScreen.prototype.onSubmit = function (event) { gasPrice, signTokenTx, signTx, + updateTx, selectedToken, editingTransactionId, errors: { amount: amountError, to: toError }, backToConfirmScreen, + unapprovedTxs, } = this.props const noErrors = !amountError && toError === null @@ -577,23 +621,25 @@ SendTransactionScreen.prototype.onSubmit = function (event) { this.addToAddressBookIfNew(to) if (editingTransactionId) { - backToConfirmScreen(editingTransactionId) - return - } + const editedTx = this.getEditedTx() - const txParams = { - from, - value: '0', - gas, - gasPrice, - } + updateTx(editedTx) + } else { - if (!selectedToken) { - txParams.value = amount - txParams.to = to - } + const txParams = { + from, + value: '0', + gas, + gasPrice, + } - selectedToken - ? signTokenTx(selectedToken.address, to, amount, txParams) - : signTx(txParams) + if (!selectedToken) { + txParams.value = amount + txParams.to = to + } + + selectedToken + ? signTokenTx(selectedToken.address, to, amount, txParams) + : signTx(txParams) + } } From 5fe3c5aae6756f225edd0f8646ac0a23c264a81c Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 20 Dec 2017 15:05:41 -0330 Subject: [PATCH 3/5] Lint fixes. --- ui/app/components/pending-tx/confirm-send-token.js | 3 +-- ui/app/components/send/send-v2-container.js | 1 - ui/app/conf-tx.js | 2 +- ui/app/send-v2.js | 2 -- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/ui/app/components/pending-tx/confirm-send-token.js b/ui/app/components/pending-tx/confirm-send-token.js index a07835911..aa4f29fb0 100644 --- a/ui/app/components/pending-tx/confirm-send-token.js +++ b/ui/app/components/pending-tx/confirm-send-token.js @@ -2,7 +2,6 @@ const Component = require('react').Component const { connect } = require('react-redux') const h = require('react-hyperscript') const inherits = require('util').inherits -const ethAbi = require('ethereumjs-abi') const tokenAbi = require('human-standard-token-abi') const abiDecoder = require('abi-decoder') abiDecoder.addABI(tokenAbi) @@ -415,7 +414,7 @@ ConfirmSendToken.prototype.onSubmit = function (event) { ConfirmSendToken.prototype.cancel = function (event, txMeta) { event.preventDefault() - const { send, cancelTransaction } = this.props + const { cancelTransaction } = this.props cancelTransaction(txMeta) } diff --git a/ui/app/components/send/send-v2-container.js b/ui/app/components/send/send-v2-container.js index ff7714e82..2d2ed4546 100644 --- a/ui/app/components/send/send-v2-container.js +++ b/ui/app/components/send/send-v2-container.js @@ -79,7 +79,6 @@ function mapDispatchToProps (dispatch) { updateSendErrors: newError => dispatch(actions.updateSendErrors(newError)), goHome: () => dispatch(actions.goHome()), clearSend: () => dispatch(actions.clearSend()), - backToConfirmScreen: editingTransactionId => dispatch(actions.showConfTxPage({ id: editingTransactionId })), setMaxModeTo: bool => dispatch(actions.setMaxModeTo(bool)), } } diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index d71e4b35f..9f273aaec 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -115,7 +115,7 @@ function currentTxView (opts) { log.info('rendering current tx view') const { txData } = opts const { txParams, msgParams } = txData - console.log(`22222 currentTxView txData`, txData); + if (txParams) { log.debug('txParams detected, rendering pending tx') return h(PendingTx, opts) diff --git a/ui/app/send-v2.js b/ui/app/send-v2.js index 617211b20..1c0ff3aea 100644 --- a/ui/app/send-v2.js +++ b/ui/app/send-v2.js @@ -608,8 +608,6 @@ SendTransactionScreen.prototype.onSubmit = function (event) { selectedToken, editingTransactionId, errors: { amount: amountError, to: toError }, - backToConfirmScreen, - unapprovedTxs, } = this.props const noErrors = !amountError && toError === null From 9ced63584bc93cf6ac82786dec0984b5022346ae Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 20 Dec 2017 19:06:58 -0330 Subject: [PATCH 4/5] Add constanst for token transfer function signature. --- ui/app/components/send/send-constants.js | 3 +++ ui/app/send-v2.js | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ui/app/components/send/send-constants.js b/ui/app/components/send/send-constants.js index 9c240972f..b3ee0899a 100644 --- a/ui/app/components/send/send-constants.js +++ b/ui/app/components/send/send-constants.js @@ -20,6 +20,8 @@ const MIN_GAS_TOTAL = multiplyCurrencies(MIN_GAS_LIMIT_HEX, MIN_GAS_PRICE_HEX, { multiplierBase: 16, }) +const TOKEN_TRANSFER_FUNCTION_SIGNATURE = '0xa9059cbb' + module.exports = { MIN_GAS_PRICE_GWEI, MIN_GAS_PRICE_HEX, @@ -27,4 +29,5 @@ module.exports = { MIN_GAS_LIMIT_HEX, MIN_GAS_LIMIT_DEC, MIN_GAS_TOTAL, + TOKEN_TRANSFER_FUNCTION_SIGNATURE, } diff --git a/ui/app/send-v2.js b/ui/app/send-v2.js index 1c0ff3aea..cf9e709d4 100644 --- a/ui/app/send-v2.js +++ b/ui/app/send-v2.js @@ -14,6 +14,7 @@ const GasFeeDisplay = require('./components/send/gas-fee-display-v2') const { MIN_GAS_TOTAL, + TOKEN_TRANSFER_FUNCTION_SIGNATURE, } = require('./components/send/send-constants') const { @@ -574,7 +575,7 @@ SendTransactionScreen.prototype.getEditedTx = function () { } if (selectedToken) { - const data = '0xa9059cbb' + Array.prototype.map.call( + const data = TOKEN_TRANSFER_FUNCTION_SIGNATURE + Array.prototype.map.call( ethAbi.rawEncode(['address', 'uint256'], [to, ethUtil.addHexPrefix(amount)]), x => ('00' + x.toString(16)).slice(-2) ).join('') From e7e1b7a95180597308bd167bd4a152bbbf53ff21 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 20 Dec 2017 19:11:18 -0330 Subject: [PATCH 5/5] Clone transaction while editing instead of mutating object from state. --- ui/app/send-v2.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ui/app/send-v2.js b/ui/app/send-v2.js index cf9e709d4..b7f2e7277 100644 --- a/ui/app/send-v2.js +++ b/ui/app/send-v2.js @@ -566,12 +566,13 @@ SendTransactionScreen.prototype.getEditedTx = function () { unapprovedTxs, } = this.props - const editingTx = unapprovedTxs[editingTransactionId] - - editingTx.txParams = { - from: ethUtil.addHexPrefix(from), - gas: ethUtil.addHexPrefix(gas), - gasPrice: ethUtil.addHexPrefix(gasPrice), + const editingTx = { + ...unapprovedTxs[editingTransactionId], + txParams: { + from: ethUtil.addHexPrefix(from), + gas: ethUtil.addHexPrefix(gas), + gasPrice: ethUtil.addHexPrefix(gasPrice), + } } if (selectedToken) {