From 508a712479d847ba90428e65989455c6f67b727d Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Thu, 27 Sep 2018 10:07:41 -0230 Subject: [PATCH 1/5] Allow sending transactions with hex data and no recipient --- ui/app/components/send/send-footer/send-footer.component.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/components/send/send-footer/send-footer.component.js b/ui/app/components/send/send-footer/send-footer.component.js index 518cff06e..230bf450f 100644 --- a/ui/app/components/send/send-footer/send-footer.component.js +++ b/ui/app/components/send/send-footer/send-footer.component.js @@ -86,9 +86,9 @@ export default class SendFooter extends Component { } formShouldBeDisabled () { - const { inError, selectedToken, tokenBalance, gasTotal, to } = this.props + const { data, inError, selectedToken, tokenBalance, gasTotal, to } = this.props const missingTokenBalance = selectedToken && !tokenBalance - return inError || !gasTotal || missingTokenBalance || !to + return inError || !gasTotal || missingTokenBalance || !(data || to) } render () { From 3741927d8d63bbee1eb9a628d2aac4c8e2e67f99 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Thu, 27 Sep 2018 11:49:21 -0230 Subject: [PATCH 2/5] Update send.utils.test to with better mocks --- ui/app/components/send/tests/send-utils.test.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ui/app/components/send/tests/send-utils.test.js b/ui/app/components/send/tests/send-utils.test.js index 18dde495a..3d983a5dc 100644 --- a/ui/app/components/send/tests/send-utils.test.js +++ b/ui/app/components/send/tests/send-utils.test.js @@ -304,10 +304,13 @@ describe('send utils', () => { selectedAddress: 'mockAddress', to: '0xisContract', estimateGasMethod: sinon.stub().callsFake( - (data, cb) => cb( - data.to.match(/willFailBecauseOf:/) ? { message: data.to.match(/:(.+)$/)[1] } : null, - { toString: (n) => `0xabc${n}` } - ) + ({to}, cb) => { + const err = typeof to === 'string' && to.match(/willFailBecauseOf:/) + ? new Error(to.match(/:(.+)$/)[1]) + : null + const result = { toString: (n) => `0xabc${n}` } + return cb(err, result) + } ), } const baseExpectedCall = { @@ -407,7 +410,7 @@ describe('send utils', () => { to: 'isContract willFailBecauseOf:some other error', })) } catch (err) { - assert.deepEqual(err, { message: 'some other error' }) + assert.equal(err.message, 'some other error') } }) }) From 918fb71df3079a80102fb77893af69ef3372e75f Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Thu, 27 Sep 2018 11:16:24 -0230 Subject: [PATCH 3/5] Update gas when hex data changes on send screen --- ui/app/actions.js | 2 ++ .../send-content/send-content.component.js | 12 +++++++--- .../send-hex-data-row.component.js | 8 ++++--- ui/app/components/send/send.component.js | 3 ++- ui/app/components/send/send.container.js | 3 ++- ui/app/components/send/send.utils.js | 24 +++++++++++++++---- .../send/tests/send-component.test.js | 1 + .../send/tests/send-container.test.js | 5 ++-- .../components/send/tests/send-utils.test.js | 12 ++++++++++ 9 files changed, 56 insertions(+), 14 deletions(-) diff --git a/ui/app/actions.js b/ui/app/actions.js index 3afdfaadc..66dc80509 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -917,6 +917,7 @@ function updateGasData ({ selectedToken, to, value, + data, }) { return (dispatch) => { dispatch(actions.gasLoadingStarted()) @@ -937,6 +938,7 @@ function updateGasData ({ to, value, estimateGasPrice, + data, }), ]) }) diff --git a/ui/app/components/send/send-content/send-content.component.js b/ui/app/components/send/send-content/send-content.component.js index 9e0ce9c23..1b03ffd2b 100644 --- a/ui/app/components/send/send-content/send-content.component.js +++ b/ui/app/components/send/send-content/send-content.component.js @@ -15,18 +15,24 @@ export default class SendContent extends Component { showHexData: PropTypes.bool, }; + updateGas = (updateData) => this.props.updateGas(updateData) + render () { return (
this.props.updateGas(updateData)} + updateGas={this.updateGas} scanQrCode={ _ => this.props.scanQrCode()} /> - this.props.updateGas(updateData)} /> + - { this.props.showHexData ? : null } + {(this.props.showHexData && ( + + ))}
) diff --git a/ui/app/components/send/send-content/send-hex-data-row/send-hex-data-row.component.js b/ui/app/components/send/send-content/send-hex-data-row/send-hex-data-row.component.js index 063930db3..62a74a77b 100644 --- a/ui/app/components/send/send-content/send-hex-data-row/send-hex-data-row.component.js +++ b/ui/app/components/send/send-content/send-hex-data-row/send-hex-data-row.component.js @@ -7,6 +7,7 @@ export default class SendHexDataRow extends Component { data: PropTypes.string, inError: PropTypes.bool, updateSendHexData: PropTypes.func.isRequired, + updateGas: PropTypes.func.isRequired, }; static contextTypes = { @@ -14,9 +15,10 @@ export default class SendHexDataRow extends Component { }; onInput = (event) => { - const {updateSendHexData} = this.props - event.target.value = event.target.value.replace(/\n/g, '') - updateSendHexData(event.target.value || null) + const {updateSendHexData, updateGas} = this.props + const data = event.target.value.replace(/\n/g, '') || null + updateSendHexData(data) + updateGas({ data }) } render () { diff --git a/ui/app/components/send/send.component.js b/ui/app/components/send/send.component.js index 0dc973632..fb7beca16 100644 --- a/ui/app/components/send/send.component.js +++ b/ui/app/components/send/send.component.js @@ -62,7 +62,7 @@ export default class SendTransactionScreen extends PersistentForm { } } - updateGas ({ to: updatedToAddress, amount: value } = {}) { + updateGas ({ to: updatedToAddress, amount: value, data } = {}) { const { amount, blockGasLimit, @@ -86,6 +86,7 @@ export default class SendTransactionScreen extends PersistentForm { selectedToken, to: getToAddressForGasUpdate(updatedToAddress, currentToAddress), value: value || amount, + data, }) } diff --git a/ui/app/components/send/send.container.js b/ui/app/components/send/send.container.js index 6ee8de9aa..87056499f 100644 --- a/ui/app/components/send/send.container.js +++ b/ui/app/components/send/send.container.js @@ -86,9 +86,10 @@ function mapDispatchToProps (dispatch) { selectedToken, to, value, + data, }) => { !editingTransactionId - ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, blockGasLimit, to, value })) + ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, blockGasLimit, to, value, data })) : dispatch(setGasTotal(calcGasTotal(gasLimit, gasPrice))) }, updateSendTokenBalance: ({ selectedToken, tokenContract, address }) => { diff --git a/ui/app/components/send/send.utils.js b/ui/app/components/send/send.utils.js index aa255c3d4..ccfdd67c0 100644 --- a/ui/app/components/send/send.utils.js +++ b/ui/app/components/send/send.utils.js @@ -200,16 +200,34 @@ function doesAmountErrorRequireUpdate ({ return amountErrorRequiresUpdate } -async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, value, gasPrice, estimateGasMethod }) { +async function estimateGas ({ + selectedAddress, + selectedToken, + blockGasLimit, + to, + value, + data, + gasPrice, + estimateGasMethod, +}) { const paramsForGasEstimate = { from: selectedAddress, value, gasPrice } if (selectedToken) { paramsForGasEstimate.value = '0x0' paramsForGasEstimate.data = generateTokenTransferData({ toAddress: to, amount: value, selectedToken }) + paramsForGasEstimate.to = selectedToken.address + } else { + if (data) { + paramsForGasEstimate.data = data + } + + if (to) { + paramsForGasEstimate.to = to + } } // if recipient has no code, gas is 21k max: - if (!selectedToken) { + if (!selectedToken && !data) { const code = Boolean(to) && await global.eth.getCode(to) if (!code || code === '0x') { return SIMPLE_GAS_COST @@ -218,8 +236,6 @@ async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, return BASE_TOKEN_GAS_COST } - paramsForGasEstimate.to = selectedToken ? selectedToken.address : to - // if not, fall back to block gasLimit paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, { multiplicandBase: 16, diff --git a/ui/app/components/send/tests/send-component.test.js b/ui/app/components/send/tests/send-component.test.js index d2c2ee926..f4943e707 100644 --- a/ui/app/components/send/tests/send-component.test.js +++ b/ui/app/components/send/tests/send-component.test.js @@ -289,6 +289,7 @@ describe('Send Component', function () { selectedToken: 'mockSelectedToken', to: '', value: 'mockAmount', + data: undefined, } ) }) diff --git a/ui/app/components/send/tests/send-container.test.js b/ui/app/components/send/tests/send-container.test.js index 85eec6a53..6aa4bf826 100644 --- a/ui/app/components/send/tests/send-container.test.js +++ b/ui/app/components/send/tests/send-container.test.js @@ -105,6 +105,7 @@ describe('send container', () => { selectedToken: { address: '0x1' }, to: 'mockTo', value: 'mockValue', + data: undefined, } it('should dispatch a setGasTotal action when editingTransactionId is truthy', () => { @@ -117,14 +118,14 @@ describe('send container', () => { }) it('should dispatch an updateGasData action when editingTransactionId is falsy', () => { - const { selectedAddress, selectedToken, recentBlocks, blockGasLimit, to, value } = mockProps + const { selectedAddress, selectedToken, recentBlocks, blockGasLimit, to, value, data } = mockProps mapDispatchToPropsObject.updateAndSetGasTotal( Object.assign({}, mockProps, {editingTransactionId: false}) ) assert(dispatchSpy.calledOnce) assert.deepEqual( actionSpies.updateGasData.getCall(0).args[0], - { selectedAddress, selectedToken, recentBlocks, blockGasLimit, to, value } + { selectedAddress, selectedToken, recentBlocks, blockGasLimit, to, value, data } ) }) }) diff --git a/ui/app/components/send/tests/send-utils.test.js b/ui/app/components/send/tests/send-utils.test.js index 3d983a5dc..b72d87eee 100644 --- a/ui/app/components/send/tests/send-utils.test.js +++ b/ui/app/components/send/tests/send-utils.test.js @@ -367,6 +367,18 @@ describe('send utils', () => { assert.equal(result, '0xabc16') }) + it('should call ethQuery.estimateGas without a recipient if the recipient is empty and data passed', async () => { + const data = 'mockData' + const to = '' + const result = await estimateGas({...baseMockParams, data, to}) + assert.equal(baseMockParams.estimateGasMethod.callCount, 1) + assert.deepEqual( + baseMockParams.estimateGasMethod.getCall(0).args[0], + { gasPrice: undefined, value: undefined, data, from: baseExpectedCall.from, gas: baseExpectedCall.gas}, + ) + assert.equal(result, '0xabc16') + }) + it(`should return ${SIMPLE_GAS_COST} if ethQuery.getCode does not return '0x'`, async () => { assert.equal(baseMockParams.estimateGasMethod.callCount, 0) const result = await estimateGas(Object.assign({}, baseMockParams, { to: '0x123' })) From c9f22916dd026445b2eb0ba343b54cc672fdf6f0 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Thu, 27 Sep 2018 12:22:12 -0230 Subject: [PATCH 4/5] Rework estimateGas logic The tests still pass! --- ui/app/components/send/send.utils.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ui/app/components/send/send.utils.js b/ui/app/components/send/send.utils.js index ccfdd67c0..a18a9e4b3 100644 --- a/ui/app/components/send/send.utils.js +++ b/ui/app/components/send/send.utils.js @@ -212,6 +212,16 @@ async function estimateGas ({ }) { const paramsForGasEstimate = { from: selectedAddress, value, gasPrice } + // if recipient has no code, gas is 21k max: + if (!selectedToken && !data) { + const code = Boolean(to) && await global.eth.getCode(to) + if (!code || code === '0x') { + return SIMPLE_GAS_COST + } + } else if (selectedToken && !to) { + return BASE_TOKEN_GAS_COST + } + if (selectedToken) { paramsForGasEstimate.value = '0x0' paramsForGasEstimate.data = generateTokenTransferData({ toAddress: to, amount: value, selectedToken }) @@ -226,16 +236,6 @@ async function estimateGas ({ } } - // if recipient has no code, gas is 21k max: - if (!selectedToken && !data) { - const code = Boolean(to) && await global.eth.getCode(to) - if (!code || code === '0x') { - return SIMPLE_GAS_COST - } - } else if (selectedToken && !to) { - return BASE_TOKEN_GAS_COST - } - // if not, fall back to block gasLimit paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, { multiplicandBase: 16, From ce2e068b436f5e04c89bbef5c43afb23bd05b919 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Mon, 1 Oct 2018 10:20:33 -0230 Subject: [PATCH 5/5] Recipient not required on send screen when hex data present --- .../send/send-content/send-to-row/send-to-row.component.js | 5 +++-- .../send/send-content/send-to-row/send-to-row.container.js | 2 ++ .../send/send-content/send-to-row/send-to-row.utils.js | 6 ++++-- .../send-to-row/tests/send-to-row-container.test.js | 2 ++ .../send-to-row/tests/send-to-row-utils.test.js | 6 ++++++ 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/ui/app/components/send/send-content/send-to-row/send-to-row.component.js b/ui/app/components/send/send-content/send-to-row/send-to-row.component.js index 434db81e5..17c75c817 100644 --- a/ui/app/components/send/send-content/send-to-row/send-to-row.component.js +++ b/ui/app/components/send/send-content/send-to-row/send-to-row.component.js @@ -8,6 +8,7 @@ export default class SendToRow extends Component { static propTypes = { closeToDropdown: PropTypes.func, + hasHexData: PropTypes.bool.isRequired, inError: PropTypes.bool, network: PropTypes.string, openToDropdown: PropTypes.func, @@ -25,8 +26,8 @@ export default class SendToRow extends Component { }; handleToChange (to, nickname = '', toError) { - const { updateSendTo, updateSendToError, updateGas } = this.props - const toErrorObject = getToErrorObject(to, toError) + const { hasHexData, updateSendTo, updateSendToError, updateGas } = this.props + const toErrorObject = getToErrorObject(to, toError, hasHexData) updateSendTo(to, nickname) updateSendToError(toErrorObject) if (toErrorObject.to === null) { diff --git a/ui/app/components/send/send-content/send-to-row/send-to-row.container.js b/ui/app/components/send/send-content/send-to-row/send-to-row.container.js index 1c9c9d518..3ee188bad 100644 --- a/ui/app/components/send/send-content/send-to-row/send-to-row.container.js +++ b/ui/app/components/send/send-content/send-to-row/send-to-row.container.js @@ -3,6 +3,7 @@ import { getCurrentNetwork, getSendTo, getSendToAccounts, + getSendHexData, } from '../../send.selectors.js' import { getToDropdownOpen, @@ -22,6 +23,7 @@ export default connect(mapStateToProps, mapDispatchToProps)(SendToRow) function mapStateToProps (state) { return { + hasHexData: Boolean(getSendHexData(state)), inError: sendToIsInError(state), network: getCurrentNetwork(state), to: getSendTo(state), diff --git a/ui/app/components/send/send-content/send-to-row/send-to-row.utils.js b/ui/app/components/send/send-content/send-to-row/send-to-row.utils.js index 6b90a9f09..0eeaa3a11 100644 --- a/ui/app/components/send/send-content/send-to-row/send-to-row.utils.js +++ b/ui/app/components/send/send-content/send-to-row/send-to-row.utils.js @@ -4,9 +4,11 @@ const { } = require('../../send.constants') const { isValidAddress } = require('../../../../util') -function getToErrorObject (to, toError = null) { +function getToErrorObject (to, toError = null, hasHexData = false) { if (!to) { - toError = REQUIRED_ERROR + if (!hasHexData) { + toError = REQUIRED_ERROR + } } else if (!isValidAddress(to) && !toError) { toError = INVALID_RECIPIENT_ADDRESS_ERROR } diff --git a/ui/app/components/send/send-content/send-to-row/tests/send-to-row-container.test.js b/ui/app/components/send/send-content/send-to-row/tests/send-to-row-container.test.js index 92355c00a..dfce7652f 100644 --- a/ui/app/components/send/send-content/send-to-row/tests/send-to-row-container.test.js +++ b/ui/app/components/send/send-content/send-to-row/tests/send-to-row-container.test.js @@ -24,6 +24,7 @@ proxyquire('../send-to-row.container.js', { }, '../../send.selectors.js': { getCurrentNetwork: (s) => `mockNetwork:${s}`, + getSendHexData: (s) => s, getSendTo: (s) => `mockTo:${s}`, getSendToAccounts: (s) => `mockToAccounts:${s}`, }, @@ -41,6 +42,7 @@ describe('send-to-row container', () => { it('should map the correct properties to props', () => { assert.deepEqual(mapStateToProps('mockState'), { + hasHexData: true, inError: 'mockInError:mockState', network: 'mockNetwork:mockState', to: 'mockTo:mockState', diff --git a/ui/app/components/send/send-content/send-to-row/tests/send-to-row-utils.test.js b/ui/app/components/send/send-content/send-to-row/tests/send-to-row-utils.test.js index 4d2447c32..c779aeb76 100644 --- a/ui/app/components/send/send-content/send-to-row/tests/send-to-row-utils.test.js +++ b/ui/app/components/send/send-content/send-to-row/tests/send-to-row-utils.test.js @@ -29,6 +29,12 @@ describe('send-to-row utils', () => { }) }) + it('should return null if to is falsy and hexData is truthy', () => { + assert.deepEqual(getToErrorObject(null, undefined, true), { + to: null, + }) + }) + it('should return an invalid recipient error if to is truthy but invalid', () => { assert.deepEqual(getToErrorObject('mockInvalidTo'), { to: INVALID_RECIPIENT_ADDRESS_ERROR,