From 0f20fce9b761fc0aa16d61b2b739fa7f9b9f6a7d Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 23 May 2018 14:13:25 -0230 Subject: [PATCH] Auto update gas estimate when to changes. --- app/scripts/metamask-controller.js | 14 +++++ ui/app/actions.js | 7 ++- .../send-content/send-content.component.js | 7 ++- .../send-to-row/send-to-row.component.js | 10 +++- .../send-to-row/send-to-row.container.js | 5 +- .../send-to-row/send-to-row.utils.js | 4 +- .../tests/send-to-row-component.test.js | 25 ++++++++- .../tests/send-to-row-container.test.js | 5 +- .../send_/send-footer/send-footer.utils.js | 1 - ui/app/components/send_/send.component.js | 7 ++- ui/app/components/send_/send.container.js | 4 +- ui/app/components/send_/send.utils.js | 30 ++++++----- .../send_/tests/send-component.test.js | 8 +++ .../send_/tests/send-container.test.js | 6 ++- .../components/send_/tests/send-utils.test.js | 54 ++++++++----------- 15 files changed, 123 insertions(+), 64 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 1b1d26886..d3d15e737 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -383,6 +383,8 @@ module.exports = class MetamaskController extends EventEmitter { updateAndApproveTransaction: nodeify(txController.updateAndApproveTransaction, txController), retryTransaction: nodeify(this.retryTransaction, this), getFilteredTxList: nodeify(txController.getFilteredTxList, txController), + isNonceTaken: nodeify(txController.isNonceTaken, txController), + estimateGas: nodeify(this.estimateGas, this), // messageManager signMessage: nodeify(this.signMessage, this), @@ -921,6 +923,18 @@ module.exports = class MetamaskController extends EventEmitter { return state } + estimateGas (estimateGasParams) { + return new Promise((resolve, reject) => { + return this.txController.txGasUtil.query.estimateGas(estimateGasParams, (err, res) => { + if (err) { + return reject(err) + } + + return resolve(res) + }) + }) + } + //============================================================================= // PASSWORD MANAGEMENT //============================================================================= diff --git a/ui/app/actions.js b/ui/app/actions.js index 7a18b1c00..5e92583e0 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -731,16 +731,21 @@ function updateGasData ({ selectedAddress, selectedToken, to, + value, }) { + const estimatedGasPrice = estimateGasPriceFromRecentBlocks(recentBlocks) return (dispatch) => { return Promise.all([ - Promise.resolve(estimateGasPriceFromRecentBlocks(recentBlocks)), + Promise.resolve(estimatedGasPrice), estimateGas({ + estimateGasMethod: background.estimateGas, blockGasLimit, data, selectedAddress, selectedToken, to, + value, + gasPrice: estimatedGasPrice, }), ]) .then(([gasPrice, gas]) => { 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 d610c2a3f..3a14054eb 100644 --- a/ui/app/components/send_/send-content/send-content.component.js +++ b/ui/app/components/send_/send-content/send-content.component.js @@ -1,4 +1,5 @@ import React, { Component } from 'react' +import PropTypes from 'prop-types' import PageContainerContent from '../../page-container/page-container-content.component' import SendAmountRow from './send-amount-row/' import SendFromRow from './send-from-row/' @@ -7,12 +8,16 @@ import SendToRow from './send-to-row/' export default class SendContent extends Component { + static propTypes = { + updateGas: PropTypes.func, + }; + render () { return (
- + this.props.updateGas(updateData)} />
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 901ae97e9..0a83186a5 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 @@ -2,6 +2,7 @@ import React, { Component } from 'react' import PropTypes from 'prop-types' import SendRowWrapper from '../send-row-wrapper/' import EnsInput from '../../../ens-input' +import { getToErrorObject } from './send-to-row.utils.js' export default class SendToRow extends Component { @@ -13,14 +14,19 @@ export default class SendToRow extends Component { to: PropTypes.string, toAccounts: PropTypes.array, toDropdownOpen: PropTypes.bool, + updateGas: PropTypes.func, updateSendTo: PropTypes.func, updateSendToError: PropTypes.func, }; handleToChange (to, nickname = '') { - const { updateSendTo, updateSendToError } = this.props + const { updateSendTo, updateSendToError, updateGas } = this.props + const toErrorObject = getToErrorObject(to) updateSendTo(to, nickname) - updateSendToError(to) + updateSendToError(toErrorObject) + if (toErrorObject.to === null) { + updateGas({ to }) + } } render () { 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 a10da505a..1c9c9d518 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 @@ -8,7 +8,6 @@ import { getToDropdownOpen, sendToIsInError, } from './send-to-row.selectors.js' -import { getToErrorObject } from './send-to-row.utils.js' import { updateSendTo, } from '../../../../actions' @@ -36,8 +35,8 @@ function mapDispatchToProps (dispatch) { closeToDropdown: () => dispatch(closeToDropdown()), openToDropdown: () => dispatch(openToDropdown()), updateSendTo: (to, nickname) => dispatch(updateSendTo(to, nickname)), - updateSendToError: (to) => { - dispatch(updateSendErrors(getToErrorObject(to))) + updateSendToError: (toErrorObject) => { + dispatch(updateSendErrors(toErrorObject)) }, } } 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 22e2e1f34..cea51ee20 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 @@ -8,9 +8,9 @@ function getToErrorObject (to) { let toError = null if (!to) { - toError = REQUIRED_ERROR + toError = REQUIRED_ERROR } else if (!isValidAddress(to)) { - toError = INVALID_RECIPIENT_ADDRESS_ERROR + toError = INVALID_RECIPIENT_ADDRESS_ERROR } return { to: toError } diff --git a/ui/app/components/send_/send-content/send-to-row/tests/send-to-row-component.test.js b/ui/app/components/send_/send-content/send-to-row/tests/send-to-row-component.test.js index e58695210..58fe51dcf 100644 --- a/ui/app/components/send_/send-content/send-to-row/tests/send-to-row-component.test.js +++ b/ui/app/components/send_/send-content/send-to-row/tests/send-to-row-component.test.js @@ -2,7 +2,15 @@ import React from 'react' import assert from 'assert' import { shallow } from 'enzyme' import sinon from 'sinon' -import SendToRow from '../send-to-row.component.js' +import proxyquire from 'proxyquire' + +const SendToRow = proxyquire('../send-to-row.component.js', { + './send-to-row.utils.js': { + getToErrorObject: (to) => ({ + to: to === false ? null : `mockToErrorObject:${to}`, + }), + }, +}).default import SendRowWrapper from '../../send-row-wrapper/send-row-wrapper.component' import EnsInput from '../../../../ens-input' @@ -10,6 +18,7 @@ import EnsInput from '../../../../ens-input' const propsMethodSpies = { closeToDropdown: sinon.spy(), openToDropdown: sinon.spy(), + updateGas: sinon.spy(), updateSendTo: sinon.spy(), updateSendToError: sinon.spy(), } @@ -29,6 +38,7 @@ describe('SendToRow Component', function () { to={'mockTo'} toAccounts={['mockAccount']} toDropdownOpen={false} + updateGas={propsMethodSpies.updateGas} updateSendTo={propsMethodSpies.updateSendTo} updateSendToError={propsMethodSpies.updateSendToError} />, { context: { t: str => str + '_t' } }) @@ -61,10 +71,21 @@ describe('SendToRow Component', function () { assert.equal(propsMethodSpies.updateSendToError.callCount, 1) assert.deepEqual( propsMethodSpies.updateSendToError.getCall(0).args, - ['mockTo2'] + [{ to: 'mockToErrorObject:mockTo2' }] ) }) + it('should not call updateGas if there is a to error', () => { + assert.equal(propsMethodSpies.updateGas.callCount, 0) + instance.handleToChange('mockTo2') + assert.equal(propsMethodSpies.updateGas.callCount, 0) + }) + + it('should call updateGas if there is no to error', () => { + assert.equal(propsMethodSpies.updateGas.callCount, 0) + instance.handleToChange(false) + assert.equal(propsMethodSpies.updateGas.callCount, 1) + }) }) describe('render', () => { 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 433b242b2..92355c00a 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 @@ -31,7 +31,6 @@ proxyquire('../send-to-row.container.js', { getToDropdownOpen: (s) => `mockToDropdownOpen:${s}`, sendToIsInError: (s) => `mockInError:${s}`, }, - './send-to-row.utils.js': { getToErrorObject: (t) => `mockError:${t}` }, '../../../../actions': actionSpies, '../../../../ducks/send.duck': duckActionSpies, }) @@ -99,12 +98,12 @@ describe('send-to-row container', () => { describe('updateSendToError()', () => { it('should dispatch an action', () => { - mapDispatchToPropsObject.updateSendToError('mockTo') + mapDispatchToPropsObject.updateSendToError('mockToErrorObject') assert(dispatchSpy.calledOnce) assert(duckActionSpies.updateSendErrors.calledOnce) assert.equal( duckActionSpies.updateSendErrors.getCall(0).args[0], - 'mockError:mockTo' + 'mockToErrorObject' ) }) }) diff --git a/ui/app/components/send_/send-footer/send-footer.utils.js b/ui/app/components/send_/send-footer/send-footer.utils.js index d5639629d..875e7d948 100644 --- a/ui/app/components/send_/send-footer/send-footer.utils.js +++ b/ui/app/components/send_/send-footer/send-footer.utils.js @@ -42,7 +42,6 @@ function constructUpdatedTx ({ } if (selectedToken) { - console.log(`ethAbi.rawEncode`, ethAbi.rawEncode) 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) diff --git a/ui/app/components/send_/send.component.js b/ui/app/components/send_/send.component.js index 0f82d3f19..97c6d1294 100644 --- a/ui/app/components/send_/send.component.js +++ b/ui/app/components/send_/send.component.js @@ -39,8 +39,9 @@ export default class SendTransactionScreen extends PersistentForm { updateSendTokenBalance: PropTypes.func, }; - updateGas () { + updateGas ({ to } = {}) { const { + amount, blockGasLimit, data, editingTransactionId, @@ -61,6 +62,8 @@ export default class SendTransactionScreen extends PersistentForm { recentBlocks, selectedAddress, selectedToken, + to: to && to.toLowerCase(), + value: amount, }) } @@ -147,7 +150,7 @@ export default class SendTransactionScreen extends PersistentForm { return (
- + this.updateGas(updateData)}/>
) diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js index 3b72a3a5a..7e241aa2d 100644 --- a/ui/app/components/send_/send.container.js +++ b/ui/app/components/send_/send.container.js @@ -76,9 +76,11 @@ function mapDispatchToProps (dispatch) { recentBlocks, selectedAddress, selectedToken, + to, + value, }) => { !editingTransactionId - ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data, blockGasLimit })) + ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data, blockGasLimit, to, value })) : 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 4c731c91b..750411908 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -15,8 +15,8 @@ const { ONE_GWEI_IN_WEI_HEX, SIMPLE_GAS_COST, } = require('./send.constants') -const EthQuery = require('ethjs-query') const abi = require('ethereumjs-abi') +const ethUtil = require('ethereumjs-util') module.exports = { calcGasTotal, @@ -165,40 +165,44 @@ function doesAmountErrorRequireUpdate ({ return amountErrorRequiresUpdate } -async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimit, to }) { - const ethQuery = new EthQuery(global.ethereumProvider) +async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimit, to, value, gasPrice, estimateGasMethod }) { const { symbol } = selectedToken || {} - const estimatedGasParams = { from: selectedAddress } + const paramsForGasEstimate = { from: selectedAddress, value, gasPrice } if (symbol) { - Object.assign(estimatedGasParams, { value: '0x0' }) + Object.assign(paramsForGasEstimate, { value: '0x0' }) } if (data) { - Object.assign(estimatedGasParams, { data }) + Object.assign(paramsForGasEstimate, { data }) } // if recipient has no code, gas is 21k max: const hasRecipient = Boolean(to) let code - if (hasRecipient) code = await ethQuery.getCode(to) - + if (hasRecipient) code = await global.eth.getCode(to) if (hasRecipient && (!code || code === '0x')) { return SIMPLE_GAS_COST } - estimatedGasParams.to = to + paramsForGasEstimate.to = to // if not, fall back to block gasLimit - estimatedGasParams.gas = multiplyCurrencies(blockGasLimit, 0.95, { + paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, { multiplicandBase: 16, multiplierBase: 10, roundDown: '0', toNumericBase: 'hex', - }) + })) // run tx - const estimatedGas = await ethQuery.estimateGas(estimatedGasParams) - return estimatedGas.toString(16) + return new Promise((resolve, reject) => { + estimateGasMethod(paramsForGasEstimate, (err, estimatedGas) => { + if (err) { + reject(err) + } + resolve(estimatedGas.toString(16)) + }) + }) } function generateTokenTransferData (selectedAddress, selectedToken) { diff --git a/ui/app/components/send_/tests/send-component.test.js b/ui/app/components/send_/tests/send-component.test.js index 3abff0d23..ec624b48c 100644 --- a/ui/app/components/send_/tests/send-component.test.js +++ b/ui/app/components/send_/tests/send-component.test.js @@ -217,9 +217,17 @@ describe.only('Send Component', function () { recentBlocks: ['mockBlock'], selectedAddress: 'mockSelectedAddress', selectedToken: 'mockSelectedToken', + to: undefined, + value: 'mockAmount', } ) }) + + it('should call updateAndSetGasTotal with to set to lowercase if passed', () => { + propsMethodSpies.updateAndSetGasTotal.resetHistory() + wrapper.instance().updateGas({ to: '0xABC' }) + assert.equal(propsMethodSpies.updateAndSetGasTotal.getCall(0).args[0].to, '0xabc') + }) }) describe('render', () => { diff --git a/ui/app/components/send_/tests/send-container.test.js b/ui/app/components/send_/tests/send-container.test.js index dca274c9e..d077ab4ee 100644 --- a/ui/app/components/send_/tests/send-container.test.js +++ b/ui/app/components/send_/tests/send-container.test.js @@ -99,6 +99,8 @@ describe('send container', () => { recentBlocks: ['mockBlock'], selectedAddress: '0x4', selectedToken: { address: '0x1' }, + to: 'mockTo', + value: 'mockValue', } it('should dispatch a setGasTotal action when editingTransactionId is truthy', () => { @@ -111,14 +113,14 @@ describe('send container', () => { }) it('should dispatch an updateGasData action when editingTransactionId is falsy', () => { - const { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit } = mockProps + const { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit, to, value } = mockProps mapDispatchToPropsObject.updateAndSetGasTotal( Object.assign({}, mockProps, {editingTransactionId: false}) ) assert(dispatchSpy.calledOnce) assert.deepEqual( actionSpies.updateGasData.getCall(0).args[0], - { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit } + { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit, to, value } ) }) }) diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index a01ab4eba..3c772ed47 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -24,14 +24,6 @@ const stubs = { rawEncode: sinon.stub().returns([16, 1100]), } -const EthQuery = function () {} -EthQuery.prototype.estimateGas = sinon.stub().callsFake( - (data) => Promise.resolve({ toString: (n) => `mockToString:${n}` }) -) -EthQuery.prototype.getCode = sinon.stub().callsFake( - (address) => Promise.resolve(address.match(/isContract/) ? 'not-0x' : '0x') -) - const sendUtils = proxyquire('../send.utils.js', { '../../conversion-util': { addCurrencies: stubs.addCurrencies, @@ -43,7 +35,6 @@ const sendUtils = proxyquire('../send.utils.js', { 'ethereumjs-abi': { rawEncode: stubs.rawEncode, }, - 'ethjs-query': EthQuery, }) const { @@ -249,6 +240,9 @@ describe('send utils', () => { blockGasLimit: '0x64', selectedAddress: 'mockAddress', to: '0xisContract', + estimateGasMethod: sinon.stub().callsFake( + (data, cb) => cb(null, { toString: (n) => `mockToString:${n}` }) + ), } const baseExpectedCall = { from: 'mockAddress', @@ -256,53 +250,51 @@ describe('send utils', () => { to: '0xisContract', } + beforeEach(() => { + global.eth = { + getCode: sinon.stub().callsFake( + (address) => Promise.resolve(address.match(/isContract/) ? 'not-0x' : '0x') + ), + } + }) + afterEach(() => { - EthQuery.prototype.estimateGas.resetHistory() - EthQuery.prototype.getCode.resetHistory() + baseMockParams.estimateGasMethod.resetHistory() + global.eth.getCode.resetHistory() }) it('should call ethQuery.estimateGas with the expected params', async () => { const result = await estimateGas(baseMockParams) - assert.equal(EthQuery.prototype.estimateGas.callCount, 1) + assert.equal(baseMockParams.estimateGasMethod.callCount, 1) assert.deepEqual( - EthQuery.prototype.estimateGas.getCall(0).args[0], - baseExpectedCall + baseMockParams.estimateGasMethod.getCall(0).args[0], + Object.assign({ gasPrice: undefined, value: undefined }, baseExpectedCall) ) assert.equal(result, 'mockToString:16') }) it('should call ethQuery.estimateGas with a value of 0x0 if the passed selectedToken has a symbol', async () => { const result = await estimateGas(Object.assign({ selectedToken: { symbol: true } }, baseMockParams)) - assert.equal(EthQuery.prototype.estimateGas.callCount, 1) + assert.equal(baseMockParams.estimateGasMethod.callCount, 1) assert.deepEqual( - EthQuery.prototype.estimateGas.getCall(0).args[0], - Object.assign({ value: '0x0' }, baseExpectedCall) + baseMockParams.estimateGasMethod.getCall(0).args[0], + Object.assign({ gasPrice: undefined, value: '0x0' }, baseExpectedCall) ) assert.equal(result, 'mockToString:16') }) it('should call ethQuery.estimateGas with data if data is passed', async () => { const result = await estimateGas(Object.assign({ data: 'mockData' }, baseMockParams)) - assert.equal(EthQuery.prototype.estimateGas.callCount, 1) + assert.equal(baseMockParams.estimateGasMethod.callCount, 1) assert.deepEqual( - EthQuery.prototype.estimateGas.getCall(0).args[0], - Object.assign({ data: 'mockData' }, baseExpectedCall) - ) - assert.equal(result, 'mockToString:16') - }) - - it('should call ethQuery.estimateGas with data if data is passed', async () => { - const result = await estimateGas(Object.assign({ data: 'mockData' }, baseMockParams)) - assert.equal(EthQuery.prototype.estimateGas.callCount, 1) - assert.deepEqual( - EthQuery.prototype.estimateGas.getCall(0).args[0], - Object.assign({ data: 'mockData' }, baseExpectedCall) + baseMockParams.estimateGasMethod.getCall(0).args[0], + Object.assign({ gasPrice: undefined, value: undefined, data: 'mockData' }, baseExpectedCall) ) assert.equal(result, 'mockToString:16') }) it(`should return ${SIMPLE_GAS_COST} if ethQuery.getCode does not return '0x'`, async () => { - assert.equal(EthQuery.prototype.estimateGas.callCount, 0) + assert.equal(baseMockParams.estimateGasMethod.callCount, 0) const result = await estimateGas(Object.assign({}, baseMockParams, { to: '0x123' })) assert.equal(result, SIMPLE_GAS_COST) })