aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorkumavis <kumavis@users.noreply.github.com>2018-06-19 02:54:26 +0800
committerGitHub <noreply@github.com>2018-06-19 02:54:26 +0800
commit62df4463859bc6836608ecb0478da67456f15e73 (patch)
treeff2aa786ff4b9afa96bea11d0d90382d75312821
parent9a7f363c2a8dcec041569c59e1b1a7c485515a33 (diff)
parent0c8318b02a5ccb11f5de43a8c9894873e29c2401 (diff)
downloadtangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.tar
tangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.tar.gz
tangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.tar.bz2
tangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.tar.lz
tangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.tar.xz
tangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.tar.zst
tangerine-wallet-browser-62df4463859bc6836608ecb0478da67456f15e73.zip
Merge pull request #4584 from MetaMask/fix-send-token
Add hex-prefix to gas estimate result
-rw-r--r--test/integration/lib/send-new-ui.js4
-rw-r--r--ui/app/components/send/currency-display.js7
-rw-r--r--ui/app/components/send_/send-content/send-amount-row/send-amount-row.component.js16
-rw-r--r--ui/app/components/send_/send-content/send-amount-row/tests/send-amount-row-component.test.js9
-rw-r--r--ui/app/components/send_/send-content/send-content.component.js2
-rw-r--r--ui/app/components/send_/send.component.js8
-rw-r--r--ui/app/components/send_/send.container.js2
-rw-r--r--ui/app/components/send_/send.utils.js46
-rw-r--r--ui/app/components/send_/tests/send-component.test.js14
-rw-r--r--ui/app/components/send_/tests/send-container.test.js2
-rw-r--r--ui/app/components/send_/tests/send-utils.test.js42
-rw-r--r--ui/app/conversion-util.js11
12 files changed, 142 insertions, 21 deletions
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 {
<CurrencyDisplay
conversionRate={amountConversionRate}
convertedCurrency={convertedCurrency}
- onBlur={newAmount => 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}
/>
</SendRowWrapper>
)
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 {
<div className="send-v2__form">
<SendFromRow />
<SendToRow updateGas={(updateData) => this.props.updateGas(updateData)} />
- <SendAmountRow />
+ <SendAmountRow updateGas={(updateData) => this.props.updateGas(updateData)} />
<SendGasRow />
</div>
</PageContainerContent>
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,