mirror of
https://github.com/kremalicious/metamask-extension.git
synced 2024-12-23 09:52:26 +01:00
Merge pull request #4584 from MetaMask/fix-send-token
Add hex-prefix to gas estimate result
This commit is contained in:
commit
62df446385
@ -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'
|
||||
)
|
||||
|
||||
|
@ -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))
|
||||
}
|
||||
|
||||
|
@ -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>
|
||||
)
|
||||
|
@ -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,
|
||||
|
@ -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>
|
||||
|
@ -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,
|
||||
})
|
||||
}
|
||||
|
||||
|
@ -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),
|
||||
|
@ -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()
|
||||
}
|
||||
|
@ -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' })
|
||||
|
@ -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',
|
||||
|
@ -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')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
@ -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,
|
||||
|
Loading…
Reference in New Issue
Block a user