diff --git a/test/integration/lib/send-new-ui.js b/test/integration/lib/send-new-ui.js index 4d2ea2ea4..72e4a8cb1 100644 --- a/test/integration/lib/send-new-ui.js +++ b/test/integration/lib/send-new-ui.js @@ -117,12 +117,12 @@ async function runSendFlowTest(assert, done) { const sendGasField = await queryAsync($, '.send-v2__gas-fee-display') assert.equal( sendGasField.find('.currency-display__input-wrapper > input').val(), - '0.000198264', + '0.000021', 'send gas field should show estimated gas total' ) assert.equal( sendGasField.find('.currency-display__converted-value')[0].textContent, - '$0.24 USD', + '$0.03 USD', 'send gas field should show estimated gas total converted to USD' ) diff --git a/ui/app/components/send/currency-display.js b/ui/app/components/send/currency-display.js index 3bc9ad226..e410bc070 100644 --- a/ui/app/components/send/currency-display.js +++ b/ui/app/components/send/currency-display.js @@ -57,6 +57,7 @@ CurrencyDisplay.prototype.getValueToRender = function ({ selectedToken, conversi return selectedToken ? conversionUtil(ethUtil.addHexPrefix(value), { fromNumericBase: 'hex', + toNumericBase: 'dec', toCurrency: symbol, conversionRate: multiplier, invertConversionRate: true, @@ -91,8 +92,12 @@ CurrencyDisplay.prototype.getConvertedValueToRender = function (nonFormattedValu : convertedValue } +function removeLeadingZeroes (str) { + return str.replace(/^0*(?=\d)/, '') +} + CurrencyDisplay.prototype.handleChange = function (newVal) { - this.setState({ valueToRender: newVal }) + this.setState({ valueToRender: removeLeadingZeroes(newVal) }) this.props.onChange(this.getAmount(newVal)) } diff --git a/ui/app/components/send_/send-content/send-amount-row/send-amount-row.component.js b/ui/app/components/send_/send-content/send-amount-row/send-amount-row.component.js index 8aefeed4a..8da36d3b7 100644 --- a/ui/app/components/send_/send-content/send-amount-row/send-amount-row.component.js +++ b/ui/app/components/send_/send-content/send-amount-row/send-amount-row.component.js @@ -23,6 +23,7 @@ export default class SendAmountRow extends Component { tokenBalance: PropTypes.string, updateSendAmount: PropTypes.func, updateSendAmountError: PropTypes.func, + updateGas: PropTypes.func, } validateAmount (amount) { @@ -56,6 +57,14 @@ export default class SendAmountRow extends Component { updateSendAmount(amount) } + updateGas (amount) { + const { selectedToken, updateGas } = this.props + + if (selectedToken) { + updateGas({ amount }) + } + } + render () { const { amount, @@ -77,12 +86,15 @@ export default class SendAmountRow extends Component { this.updateAmount(newAmount)} + onBlur={newAmount => { + this.updateGas(newAmount) + this.updateAmount(newAmount) + }} onChange={newAmount => this.validateAmount(newAmount)} inError={inError} primaryCurrency={primaryCurrency || 'ETH'} selectedToken={selectedToken} - value={amount || '0x0'} + value={amount} /> ) diff --git a/ui/app/components/send_/send-content/send-amount-row/tests/send-amount-row-component.test.js b/ui/app/components/send_/send-content/send-amount-row/tests/send-amount-row-component.test.js index 2205579ca..579e18585 100644 --- a/ui/app/components/send_/send-content/send-amount-row/tests/send-amount-row-component.test.js +++ b/ui/app/components/send_/send-content/send-amount-row/tests/send-amount-row-component.test.js @@ -12,10 +12,12 @@ const propsMethodSpies = { setMaxModeTo: sinon.spy(), updateSendAmount: sinon.spy(), updateSendAmountError: sinon.spy(), + updateGas: sinon.spy(), } sinon.spy(SendAmountRow.prototype, 'updateAmount') sinon.spy(SendAmountRow.prototype, 'validateAmount') +sinon.spy(SendAmountRow.prototype, 'updateGas') describe('SendAmountRow Component', function () { let wrapper @@ -36,6 +38,7 @@ describe('SendAmountRow Component', function () { tokenBalance={'mockTokenBalance'} updateSendAmount={propsMethodSpies.updateSendAmount} updateSendAmountError={propsMethodSpies.updateSendAmountError} + updateGas={propsMethodSpies.updateGas} />, { context: { t: str => str + '_t' } }) instance = wrapper.instance() }) @@ -139,8 +142,14 @@ describe('SendAmountRow Component', function () { assert.equal(primaryCurrency, 'mockPrimaryCurrency') assert.deepEqual(selectedToken, { address: 'mockTokenAddress' }) assert.equal(value, 'mockAmount') + assert.equal(SendAmountRow.prototype.updateGas.callCount, 0) assert.equal(SendAmountRow.prototype.updateAmount.callCount, 0) onBlur('mockNewAmount') + assert.equal(SendAmountRow.prototype.updateGas.callCount, 1) + assert.deepEqual( + SendAmountRow.prototype.updateGas.getCall(0).args, + ['mockNewAmount'] + ) assert.equal(SendAmountRow.prototype.updateAmount.callCount, 1) assert.deepEqual( SendAmountRow.prototype.updateAmount.getCall(0).args, 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 3a14054eb..adc114c0e 100644 --- a/ui/app/components/send_/send-content/send-content.component.js +++ b/ui/app/components/send_/send-content/send-content.component.js @@ -18,7 +18,7 @@ export default class SendContent extends Component {
this.props.updateGas(updateData)} /> - + this.props.updateGas(updateData)} />
diff --git a/ui/app/components/send_/send.component.js b/ui/app/components/send_/send.component.js index 516251e22..219b362f2 100644 --- a/ui/app/components/send_/send.component.js +++ b/ui/app/components/send_/send.component.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types' import PersistentForm from '../../../lib/persistent-form' import { getAmountErrorObject, + getToAddressForGasUpdate, doesAmountErrorRequireUpdate, } from './send.utils' @@ -38,7 +39,7 @@ export default class SendTransactionScreen extends PersistentForm { updateSendTokenBalance: PropTypes.func, }; - updateGas ({ to } = {}) { + updateGas ({ to: updatedToAddress, amount: value } = {}) { const { amount, blockGasLimit, @@ -48,6 +49,7 @@ export default class SendTransactionScreen extends PersistentForm { recentBlocks, selectedAddress, selectedToken = {}, + to: currentToAddress, updateAndSetGasTotal, } = this.props @@ -59,8 +61,8 @@ export default class SendTransactionScreen extends PersistentForm { recentBlocks, selectedAddress, selectedToken, - to: to && to.toLowerCase(), - value: amount, + to: getToAddressForGasUpdate(updatedToAddress, currentToAddress), + value: value || amount, }) } diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js index 1fd96d61f..185653c5f 100644 --- a/ui/app/components/send_/send.container.js +++ b/ui/app/components/send_/send.container.js @@ -19,6 +19,7 @@ import { getSendAmount, getSendEditingTransactionId, getSendFromObject, + getSendTo, getTokenBalance, } from './send.selectors' import { @@ -54,6 +55,7 @@ function mapStateToProps (state) { recentBlocks: getRecentBlocks(state), selectedAddress: getSelectedAddress(state), selectedToken: getSelectedToken(state), + to: getSendTo(state), tokenBalance: getTokenBalance(state), tokenContract: getSelectedTokenContract(state), tokenToFiatRate: getSelectedTokenToFiatRate(state), diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index 67699be77..dfd459731 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -4,6 +4,7 @@ const { conversionGTE, multiplyCurrencies, conversionGreaterThan, + conversionLessThan, } = require('../../conversion-util') const { calcTokenAmount, @@ -20,6 +21,7 @@ const abi = require('ethereumjs-abi') const ethUtil = require('ethereumjs-util') module.exports = { + addGasBuffer, calcGasTotal, calcTokenBalance, doesAmountErrorRequireUpdate, @@ -27,6 +29,7 @@ module.exports = { estimateGasPriceFromRecentBlocks, generateTokenTransferData, getAmountErrorObject, + getToAddressForGasUpdate, isBalanceSufficient, isTokenBalanceSufficient, } @@ -175,9 +178,8 @@ async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, } // if recipient has no code, gas is 21k max: - const hasRecipient = Boolean(to) - if (hasRecipient && !selectedToken) { - const code = await global.eth.getCode(to) + if (!selectedToken) { + const code = Boolean(to) && await global.eth.getCode(to) if (!code || code === '0x') { return SIMPLE_GAS_COST } @@ -201,16 +203,46 @@ async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, err.message.includes('gas required exceeds allowance or always failing transaction') ) if (simulationFailed) { - return resolve(paramsForGasEstimate.gas) + const estimateWithBuffer = addGasBuffer(paramsForGasEstimate.gas, blockGasLimit, 1.5) + return resolve(ethUtil.addHexPrefix(estimateWithBuffer)) } else { return reject(err) } } - return resolve(estimatedGas.toString(16)) + const estimateWithBuffer = addGasBuffer(estimatedGas.toString(16), blockGasLimit, 1.5) + return resolve(ethUtil.addHexPrefix(estimateWithBuffer)) }) }) } +function addGasBuffer (initialGasLimitHex, blockGasLimitHex, bufferMultiplier = 1.5) { + const upperGasLimit = multiplyCurrencies(blockGasLimitHex, 0.9, { + toNumericBase: 'hex', + multiplicandBase: 16, + multiplierBase: 10, + numberOfDecimals: '0', + }) + const bufferedGasLimit = multiplyCurrencies(initialGasLimitHex, bufferMultiplier, { + toNumericBase: 'hex', + multiplicandBase: 16, + multiplierBase: 10, + numberOfDecimals: '0', + }) + + // if initialGasLimit is above blockGasLimit, dont modify it + if (conversionGreaterThan( + { value: initialGasLimitHex, fromNumericBase: 'hex' }, + { value: upperGasLimit, fromNumericBase: 'hex' }, + )) return initialGasLimitHex + // if bufferedGasLimit is below blockGasLimit, use bufferedGasLimit + if (conversionLessThan( + { value: bufferedGasLimit, fromNumericBase: 'hex' }, + { value: upperGasLimit, fromNumericBase: 'hex' }, + )) return bufferedGasLimit + // otherwise use blockGasLimit + return upperGasLimit +} + function generateTokenTransferData ({ toAddress = '0x0', amount = '0x0', selectedToken }) { if (!selectedToken) return return TOKEN_TRANSFER_FUNCTION_SIGNATURE + Array.prototype.map.call( @@ -237,3 +269,7 @@ function estimateGasPriceFromRecentBlocks (recentBlocks) { return lowestPrices[Math.floor(lowestPrices.length / 2)] } + +function getToAddressForGasUpdate (...addresses) { + return [...addresses, ''].find(str => str !== undefined && str !== null).toLowerCase() +} diff --git a/ui/app/components/send_/tests/send-component.test.js b/ui/app/components/send_/tests/send-component.test.js index 4e33d8f63..4ba9b226d 100644 --- a/ui/app/components/send_/tests/send-component.test.js +++ b/ui/app/components/send_/tests/send-component.test.js @@ -201,7 +201,7 @@ describe('Send Component', function () { }) describe('updateGas', () => { - it('should call updateAndSetGasTotal with the correct params', () => { + it('should call updateAndSetGasTotal with the correct params if no to prop is passed', () => { propsMethodSpies.updateAndSetGasTotal.resetHistory() wrapper.instance().updateGas() assert.equal(propsMethodSpies.updateAndSetGasTotal.callCount, 1) @@ -215,12 +215,22 @@ describe('Send Component', function () { recentBlocks: ['mockBlock'], selectedAddress: 'mockSelectedAddress', selectedToken: 'mockSelectedToken', - to: undefined, + to: '', value: 'mockAmount', } ) }) + it('should call updateAndSetGasTotal with the correct params if a to prop is passed', () => { + propsMethodSpies.updateAndSetGasTotal.resetHistory() + wrapper.setProps({ to: 'someAddress' }) + wrapper.instance().updateGas() + assert.equal( + propsMethodSpies.updateAndSetGasTotal.getCall(0).args[0].to, + 'someaddress', + ) + }) + it('should call updateAndSetGasTotal with to set to lowercase if passed', () => { propsMethodSpies.updateAndSetGasTotal.resetHistory() wrapper.instance().updateGas({ to: '0xABC' }) diff --git a/ui/app/components/send_/tests/send-container.test.js b/ui/app/components/send_/tests/send-container.test.js index 056aad148..91484f4d8 100644 --- a/ui/app/components/send_/tests/send-container.test.js +++ b/ui/app/components/send_/tests/send-container.test.js @@ -39,6 +39,7 @@ proxyquire('../send.container.js', { getSelectedTokenContract: (s) => `mockTokenContract:${s}`, getSelectedTokenToFiatRate: (s) => `mockTokenToFiatRate:${s}`, getSendAmount: (s) => `mockAmount:${s}`, + getSendTo: (s) => `mockTo:${s}`, getSendEditingTransactionId: (s) => `mockEditingTransactionId:${s}`, getSendFromObject: (s) => `mockFrom:${s}`, getTokenBalance: (s) => `mockTokenBalance:${s}`, @@ -70,6 +71,7 @@ describe('send container', () => { recentBlocks: 'mockRecentBlocks:mockState', selectedAddress: 'mockSelectedAddress:mockState', selectedToken: 'mockSelectedToken:mockState', + to: 'mockTo:mockState', tokenBalance: 'mockTokenBalance:mockState', tokenContract: 'mockTokenContract:mockState', tokenToFiatRate: 'mockTokenToFiatRate:mockState', diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index b3f6372ef..f3d5674b7 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -18,10 +18,12 @@ const { const stubs = { addCurrencies: sinon.stub().callsFake((a, b, obj) => a + b), conversionUtil: sinon.stub().callsFake((val, obj) => parseInt(val, 16)), - conversionGTE: sinon.stub().callsFake((obj1, obj2) => obj1.value > obj2.value), + conversionGTE: sinon.stub().callsFake((obj1, obj2) => obj1.value >= obj2.value), multiplyCurrencies: sinon.stub().callsFake((a, b) => `${a}x${b}`), calcTokenAmount: sinon.stub().callsFake((a, d) => 'calc:' + a + d), rawEncode: sinon.stub().returns([16, 1100]), + conversionGreaterThan: sinon.stub().callsFake((obj1, obj2) => obj1.value > obj2.value), + conversionLessThan: sinon.stub().callsFake((obj1, obj2) => obj1.value < obj2.value), } const sendUtils = proxyquire('../send.utils.js', { @@ -30,6 +32,8 @@ const sendUtils = proxyquire('../send.utils.js', { conversionUtil: stubs.conversionUtil, conversionGTE: stubs.conversionGTE, multiplyCurrencies: stubs.multiplyCurrencies, + conversionGreaterThan: stubs.conversionGreaterThan, + conversionLessThan: stubs.conversionLessThan, }, '../../token-util': { calcTokenAmount: stubs.calcTokenAmount }, 'ethereumjs-abi': { @@ -44,6 +48,7 @@ const { estimateGasPriceFromRecentBlocks, generateTokenTransferData, getAmountErrorObject, + getToAddressForGasUpdate, calcTokenBalance, isBalanceSufficient, isTokenBalanceSufficient, @@ -255,7 +260,7 @@ describe('send utils', () => { estimateGasMethod: sinon.stub().callsFake( (data, cb) => cb( data.to.match(/willFailBecauseOf:/) ? { message: data.to.match(/:(.+)$/)[1] } : null, - { toString: (n) => `mockToString:${n}` } + { toString: (n) => `0xabc${n}` } ) ), } @@ -279,13 +284,23 @@ describe('send utils', () => { }) it('should call ethQuery.estimateGas with the expected params', async () => { - const result = await estimateGas(baseMockParams) + const result = await sendUtils.estimateGas(baseMockParams) assert.equal(baseMockParams.estimateGasMethod.callCount, 1) assert.deepEqual( baseMockParams.estimateGasMethod.getCall(0).args[0], Object.assign({ gasPrice: undefined, value: undefined }, baseExpectedCall) ) - assert.equal(result, 'mockToString:16') + assert.equal(result, '0xabc16') + }) + + it('should call ethQuery.estimateGas with the expected params when initialGasLimitHex is lower than the upperGasLimit', async () => { + const result = await estimateGas(Object.assign({}, baseMockParams, { blockGasLimit: '0xbcd' })) + assert.equal(baseMockParams.estimateGasMethod.callCount, 1) + assert.deepEqual( + baseMockParams.estimateGasMethod.getCall(0).args[0], + Object.assign({ gasPrice: undefined, value: undefined }, baseExpectedCall, { gas: '0xbcdx0.95' }) + ) + assert.equal(result, '0xabc16x1.5') }) it('should call ethQuery.estimateGas with a value of 0x0 and the expected data and to if passed a selectedToken', async () => { @@ -300,7 +315,7 @@ describe('send utils', () => { to: 'mockAddress', }) ) - assert.equal(result, 'mockToString:16') + assert.equal(result, '0xabc16') }) it(`should return ${SIMPLE_GAS_COST} if ethQuery.getCode does not return '0x'`, async () => { @@ -309,6 +324,12 @@ describe('send utils', () => { assert.equal(result, SIMPLE_GAS_COST) }) + it(`should return ${SIMPLE_GAS_COST} if not passed a selectedToken or truthy to address`, async () => { + assert.equal(baseMockParams.estimateGasMethod.callCount, 0) + const result = await estimateGas(Object.assign({}, baseMockParams, { to: null })) + assert.equal(result, SIMPLE_GAS_COST) + }) + it(`should not return ${SIMPLE_GAS_COST} if passed a selectedToken`, async () => { assert.equal(baseMockParams.estimateGasMethod.callCount, 0) const result = await estimateGas(Object.assign({}, baseMockParams, { to: '0x123', selectedToken: { address: '' } })) @@ -401,4 +422,15 @@ describe('send utils', () => { assert.equal(estimateGasPriceFromRecentBlocks(mockRecentBlocks), '0x5') }) }) + + describe('getToAddressForGasUpdate()', () => { + it('should return empty string if all params are undefined or null', () => { + assert.equal(getToAddressForGasUpdate(undefined, null), '') + }) + + it('should return the first string that is not defined or null in lower case', () => { + assert.equal(getToAddressForGasUpdate('A', null), 'a') + assert.equal(getToAddressForGasUpdate(undefined, 'B'), 'b') + }) + }) }) diff --git a/ui/app/conversion-util.js b/ui/app/conversion-util.js index 100402d95..337763067 100644 --- a/ui/app/conversion-util.js +++ b/ui/app/conversion-util.js @@ -190,6 +190,16 @@ const conversionGreaterThan = ( return firstValue.gt(secondValue) } +const conversionLessThan = ( + { ...firstProps }, + { ...secondProps }, +) => { + const firstValue = converter({ ...firstProps }) + const secondValue = converter({ ...secondProps }) + + return firstValue.lt(secondValue) +} + const conversionMax = ( { ...firstProps }, { ...secondProps }, @@ -229,6 +239,7 @@ module.exports = { addCurrencies, multiplyCurrencies, conversionGreaterThan, + conversionLessThan, conversionGTE, conversionLTE, conversionMax,