From 6f633a97e15e8277a065f987880cd666cc2fbf89 Mon Sep 17 00:00:00 2001 From: Dan Date: Sat, 19 May 2018 22:07:44 -0230 Subject: Rename gas change actions. --- ui/app/components/send_/send.container.js | 4 ++-- ui/app/components/send_/tests/send-container.test.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js index 8efaf5aaf..df28caca8 100644 --- a/ui/app/components/send_/send.container.js +++ b/ui/app/components/send_/send.container.js @@ -21,7 +21,7 @@ import { } from './send.selectors' import { updateSendTokenBalance, - updateGasTotal, + updateGasData, setGasTotal, } from '../../actions' import { @@ -73,7 +73,7 @@ function mapDispatchToProps (dispatch) { }) => { console.log(`editingTransactionId`, editingTransactionId) !editingTransactionId - ? dispatch(updateGasTotal({ selectedAddress, selectedToken, data })) + ? dispatch(updateGasData({ selectedAddress, selectedToken, data })) : dispatch(setGasTotal(calcGasTotal(gasLimit, gasPrice))) }, updateSendTokenBalance: ({ selectedToken, tokenContract, address }) => { diff --git a/ui/app/components/send_/tests/send-container.test.js b/ui/app/components/send_/tests/send-container.test.js index 7b6ca1f7b..e589cca05 100644 --- a/ui/app/components/send_/tests/send-container.test.js +++ b/ui/app/components/send_/tests/send-container.test.js @@ -7,7 +7,7 @@ let mapDispatchToProps const actionSpies = { updateSendTokenBalance: sinon.spy(), - updateGasTotal: sinon.spy(), + updateGasData: sinon.spy(), setGasTotal: sinon.spy(), } const duckActionSpies = { @@ -104,14 +104,14 @@ describe('send container', () => { ) }) - it('should dispatch an updateGasTotal action when editingTransactionId is falsy', () => { + it('should dispatch an updateGasData action when editingTransactionId is falsy', () => { const { selectedAddress, selectedToken, data } = mockProps mapDispatchToPropsObject.updateAndSetGasTotal( Object.assign(mockProps, {editingTransactionId: false}) ) assert(dispatchSpy.calledOnce) assert.deepEqual( - actionSpies.updateGasTotal.getCall(0).args[0], + actionSpies.updateGasData.getCall(0).args[0], { selectedAddress, selectedToken, data } ) }) -- cgit From 17909465f283179aad39166b1191dbaba3770bf6 Mon Sep 17 00:00:00 2001 From: Dan Date: Sat, 19 May 2018 22:08:25 -0230 Subject: getParamsForGasEstimate extracts symbol from token instead of just accepting symbol. --- ui/app/components/send_/send.utils.js | 3 ++- ui/app/components/send_/tests/send-utils.test.js | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index a35a55bf8..1c7fd2b42 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -139,7 +139,8 @@ function getAmountErrorObject ({ return { amount: amountError } } -function getParamsForGasEstimate (selectedAddress, symbol, data) { +function getParamsForGasEstimate (selectedAddress, selectedToken, data) { + const { symbol } = selectedToken || {} const estimatedGasParams = { from: selectedAddress, gas: '746a528800', diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index 4d471bcc1..903b531e6 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -147,9 +147,9 @@ describe('send utils', () => { ) }) - it('should return value property if symbol provided', () => { + it('should return value property if selected token provided', () => { assert.deepEqual( - getParamsForGasEstimate('mockAddress', 'ABC'), + getParamsForGasEstimate('mockAddress', { symbol: 'ABC' }), { from: 'mockAddress', gas: '746a528800', @@ -160,7 +160,7 @@ describe('send utils', () => { it('should return data property if data provided', () => { assert.deepEqual( - getParamsForGasEstimate('mockAddress', 'ABC', 'somedata'), + getParamsForGasEstimate('mockAddress', { symbol: 'ABC' }, 'somedata'), { from: 'mockAddress', gas: '746a528800', -- cgit From 166fda58777748141859c0a674a5fce454cfc3d3 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 22 May 2018 05:40:06 -0230 Subject: Simplify gas estimate actions and add local estimateGasPriceFromRecentBlocks method. --- ui/app/components/send_/send.component.js | 3 + ui/app/components/send_/send.constants.js | 8 ++ ui/app/components/send_/send.container.js | 6 +- ui/app/components/send_/send.selectors.js | 13 +++ ui/app/components/send_/send.utils.js | 37 +++++++ .../components/send_/tests/send-component.test.js | 2 + .../components/send_/tests/send-container.test.js | 9 +- .../send_/tests/send-selectors-test-data.js | 1 + .../components/send_/tests/send-selectors.test.js | 10 ++ ui/app/components/send_/tests/send-utils.test.js | 106 +++++++++++++++++++++ 10 files changed, 190 insertions(+), 5 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.component.js b/ui/app/components/send_/send.component.js index 21e1de09b..438923ab3 100644 --- a/ui/app/components/send_/send.component.js +++ b/ui/app/components/send_/send.component.js @@ -28,6 +28,7 @@ export default class SendTransactionScreen extends PersistentForm { history: PropTypes.object, network: PropTypes.string, primaryCurrency: PropTypes.string, + recentBlocks: PropTypes.array, selectedAddress: PropTypes.string, selectedToken: PropTypes.object, tokenBalance: PropTypes.string, @@ -43,6 +44,7 @@ export default class SendTransactionScreen extends PersistentForm { editingTransactionId, gasLimit, gasPrice, + recentBlocks, selectedAddress, selectedToken = {}, updateAndSetGasTotal, @@ -53,6 +55,7 @@ export default class SendTransactionScreen extends PersistentForm { editingTransactionId, gasLimit, gasPrice, + recentBlocks, selectedAddress, selectedToken, }) diff --git a/ui/app/components/send_/send.constants.js b/ui/app/components/send_/send.constants.js index b59fcaaf0..e911a5510 100644 --- a/ui/app/components/send_/send.constants.js +++ b/ui/app/components/send_/send.constants.js @@ -28,6 +28,13 @@ const NEGATIVE_ETH_ERROR = 'negativeETH' const INVALID_RECIPIENT_ADDRESS_ERROR = 'invalidAddressRecipient' const REQUIRED_ERROR = 'required' +const ONE_GWEI_IN_WEI_HEX = ethUtil.addHexPrefix(conversionUtil('0x1', { + fromDenomination: 'GWEI', + toDenomination: 'WEI', + fromNumericBase: 'hex', + toNumericBase: 'hex', +})) + module.exports = { INSUFFICIENT_FUNDS_ERROR, INSUFFICIENT_TOKENS_ERROR, @@ -39,6 +46,7 @@ module.exports = { MIN_GAS_PRICE_HEX, MIN_GAS_TOTAL, NEGATIVE_ETH_ERROR, + ONE_GWEI_IN_WEI_HEX, REQUIRED_ERROR, TOKEN_TRANSFER_FUNCTION_SIGNATURE, } diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js index df28caca8..879aef87c 100644 --- a/ui/app/components/send_/send.container.js +++ b/ui/app/components/send_/send.container.js @@ -10,6 +10,7 @@ import { getGasPrice, getGasTotal, getPrimaryCurrency, + getRecentBlocks, getSelectedAddress, getSelectedToken, getSelectedTokenContract, @@ -53,6 +54,7 @@ function mapStateToProps (state) { gasTotal: getGasTotal(state), network: getCurrentNetwork(state), primaryCurrency: getPrimaryCurrency(state), + recentBlocks: getRecentBlocks(state), selectedAddress: getSelectedAddress(state), selectedToken: getSelectedToken(state), tokenBalance: getTokenBalance(state), @@ -68,12 +70,12 @@ function mapDispatchToProps (dispatch) { editingTransactionId, gasLimit, gasPrice, + recentBlocks, selectedAddress, selectedToken, }) => { - console.log(`editingTransactionId`, editingTransactionId) !editingTransactionId - ? dispatch(updateGasData({ selectedAddress, selectedToken, data })) + ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data })) : dispatch(setGasTotal(calcGasTotal(gasLimit, gasPrice))) }, updateSendTokenBalance: ({ selectedToken, tokenContract, address }) => { diff --git a/ui/app/components/send_/send.selectors.js b/ui/app/components/send_/send.selectors.js index c5ae1ab7f..850328e10 100644 --- a/ui/app/components/send_/send.selectors.js +++ b/ui/app/components/send_/send.selectors.js @@ -3,6 +3,9 @@ const abi = require('human-standard-token-abi') const { multiplyCurrencies, } = require('../../conversion-util') +const { + estimateGasPriceFromRecentBlocks, +} = require('./send.utils') const selectors = { accountsWithSendEtherInfoSelector, @@ -18,8 +21,10 @@ const selectors = { getForceGasMin, getGasLimit, getGasPrice, + getGasPriceFromRecentBlocks, getGasTotal, getPrimaryCurrency, + getRecentBlocks, getSelectedAccount, getSelectedAddress, getSelectedIdentity, @@ -124,6 +129,10 @@ function getGasPrice (state) { return state.metamask.send.gasPrice } +function getGasPriceFromRecentBlocks (state) { + return estimateGasPriceFromRecentBlocks(state.metamask.recentBlocks) +} + function getGasTotal (state) { return state.metamask.send.gasTotal } @@ -133,6 +142,10 @@ function getPrimaryCurrency (state) { return selectedToken && selectedToken.symbol } +function getRecentBlocks (state) { + return state.metamask.recentBlocks +} + function getSelectedAccount (state) { const accounts = state.metamask.accounts const selectedAddress = getSelectedAddress(state) diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index 1c7fd2b42..6055c98b1 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -12,12 +12,15 @@ const { INSUFFICIENT_FUNDS_ERROR, INSUFFICIENT_TOKENS_ERROR, NEGATIVE_ETH_ERROR, + ONE_GWEI_IN_WEI_HEX, } = require('./send.constants') const abi = require('ethereumjs-abi') module.exports = { calcGasTotal, doesAmountErrorRequireUpdate, + estimateGas, + estimateGasPriceFromRecentBlocks, generateTokenTransferData, getAmountErrorObject, getParamsForGasEstimate, @@ -179,6 +182,17 @@ function doesAmountErrorRequireUpdate ({ return amountErrorRequiresUpdate } +function estimateGas (params = {}) { + return new Promise((resolve, reject) => { + global.ethQuery.estimateGas(params, (err, data) => { + if (err) { + return reject(err) + } + return resolve(data) + }) + }) +} + function generateTokenTransferData (selectedAddress, selectedToken) { if (!selectedToken) return console.log(`abi.rawEncode`, abi.rawEncode) @@ -187,3 +201,26 @@ function generateTokenTransferData (selectedAddress, selectedToken) { x => ('00' + x.toString(16)).slice(-2) ).join('') } + +function hexComparator (a, b) { + return conversionGreaterThan( + { value: a, fromNumericBase: 'hex' }, + { value: b, fromNumericBase: 'hex' }, + ) ? 1 : -1 +} + +function estimateGasPriceFromRecentBlocks (recentBlocks) { + // Return 1 gwei if no blocks have been observed: + if (!recentBlocks || recentBlocks.length === 0) { + return ONE_GWEI_IN_WEI_HEX + } + const lowestPrices = recentBlocks.map((block) => { + if (!block.gasPrices || block.gasPrices.length < 1) { + return ONE_GWEI_IN_WEI_HEX + } + return block.gasPrices + .sort(hexComparator)[0] + }) + .sort(hexComparator) + return lowestPrices[Math.floor(lowestPrices.length / 2)] +} diff --git a/ui/app/components/send_/tests/send-component.test.js b/ui/app/components/send_/tests/send-component.test.js index 4aa1978e4..780ee1046 100644 --- a/ui/app/components/send_/tests/send-component.test.js +++ b/ui/app/components/send_/tests/send-component.test.js @@ -42,6 +42,7 @@ describe.only('Send Component', function () { history={{ mockProp: 'history-abc'}} network={'3'} primaryCurrency={'mockPrimaryCurrency'} + recentBlocks={['mockBlock']} selectedAddress={'mockSelectedAddress'} selectedToken={'mockSelectedToken'} tokenBalance={'mockTokenBalance'} @@ -211,6 +212,7 @@ describe.only('Send Component', function () { editingTransactionId: 'mockEditingTransactionId', gasLimit: 'mockGasLimit', gasPrice: 'mockGasPrice', + recentBlocks: ['mockBlock'], selectedAddress: 'mockSelectedAddress', selectedToken: 'mockSelectedToken', } diff --git a/ui/app/components/send_/tests/send-container.test.js b/ui/app/components/send_/tests/send-container.test.js index e589cca05..2129709c1 100644 --- a/ui/app/components/send_/tests/send-container.test.js +++ b/ui/app/components/send_/tests/send-container.test.js @@ -32,6 +32,7 @@ proxyquire('../send.container.js', { getGasPrice: (s) => `mockGasPrice:${s}`, getGasTotal: (s) => `mockGasTotal:${s}`, getPrimaryCurrency: (s) => `mockPrimaryCurrency:${s}`, + getRecentBlocks: (s) => `mockRecentBlocks:${s}`, getSelectedAddress: (s) => `mockSelectedAddress:${s}`, getSelectedToken: (s) => `mockSelectedToken:${s}`, getSelectedTokenContract: (s) => `mockTokenContract:${s}`, @@ -66,6 +67,7 @@ describe('send container', () => { gasTotal: 'mockGasTotal:mockState', network: 'mockNetwork:mockState', primaryCurrency: 'mockPrimaryCurrency:mockState', + recentBlocks: 'mockRecentBlocks:mockState', selectedAddress: 'mockSelectedAddress:mockState', selectedToken: 'mockSelectedToken:mockState', tokenBalance: 'mockTokenBalance:mockState', @@ -91,6 +93,7 @@ describe('send container', () => { editingTransactionId: '0x2', gasLimit: '0x3', gasPrice: '0x4', + recentBlocks: ['mockBlock'], selectedAddress: '0x4', selectedToken: { address: '0x1' }, } @@ -105,14 +108,14 @@ describe('send container', () => { }) it('should dispatch an updateGasData action when editingTransactionId is falsy', () => { - const { selectedAddress, selectedToken, data } = mockProps + const { selectedAddress, selectedToken, data, recentBlocks } = mockProps mapDispatchToPropsObject.updateAndSetGasTotal( - Object.assign(mockProps, {editingTransactionId: false}) + Object.assign({}, mockProps, {editingTransactionId: false}) ) assert(dispatchSpy.calledOnce) assert.deepEqual( actionSpies.updateGasData.getCall(0).args[0], - { selectedAddress, selectedToken, data } + { selectedAddress, selectedToken, data, recentBlocks } ) }) }) diff --git a/ui/app/components/send_/tests/send-selectors-test-data.js b/ui/app/components/send_/tests/send-selectors-test-data.js index ecfe9022f..a1423675d 100644 --- a/ui/app/components/send_/tests/send-selectors-test-data.js +++ b/ui/app/components/send_/tests/send-selectors-test-data.js @@ -198,6 +198,7 @@ module.exports = { }, }, 'currentLocale': 'en', + recentBlocks: ['mockBlock1', 'mockBlock2', 'mockBlock3'], }, 'appState': { 'menuOpen': false, diff --git a/ui/app/components/send_/tests/send-selectors.test.js b/ui/app/components/send_/tests/send-selectors.test.js index 977fe2a47..22e45afdb 100644 --- a/ui/app/components/send_/tests/send-selectors.test.js +++ b/ui/app/components/send_/tests/send-selectors.test.js @@ -17,6 +17,7 @@ const { getGasPrice, getGasTotal, getPrimaryCurrency, + getRecentBlocks, getSelectedAccount, getSelectedAddress, getSelectedIdentity, @@ -239,6 +240,15 @@ describe('send selectors', () => { }) }) + describe('getRecentBlocks()', () => { + it('should return the recent blocks', () => { + assert.deepEqual( + getRecentBlocks(mockState), + ['mockBlock1', 'mockBlock2', 'mockBlock3'] + ) + }) + }) + describe('getSelectedAccount()', () => { it('should return the currently selected account', () => { assert.deepEqual( diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index 903b531e6..b5211a63d 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -1,6 +1,13 @@ import assert from 'assert' import sinon from 'sinon' import proxyquire from 'proxyquire' +import { + ONE_GWEI_IN_WEI_HEX, +} from '../send.constants' +const { + addCurrencies, + subtractCurrencies, +} = require('../../../conversion-util') const { INSUFFICIENT_FUNDS_ERROR, @@ -31,7 +38,9 @@ const sendUtils = proxyquire('../send.utils.js', { const { calcGasTotal, + estimateGas, doesAmountErrorRequireUpdate, + estimateGasPriceFromRecentBlocks, generateTokenTransferData, getAmountErrorObject, getParamsForGasEstimate, @@ -261,4 +270,101 @@ describe('send utils', () => { }) }) + describe('estimateGas', () => { + let tempEthQuery + beforeEach(() => { + tempEthQuery = global.ethQuery + global.ethQuery = { + estimateGas: sinon.stub().callsFake((data, cb) => { + return cb( + data.isMockErr ? 'mockErr' : null, + Object.assign(data, { estimateGasCalled: true }) + ) + }) + } + }) + + afterEach(() => { + global.ethQuery = tempEthQuery + }) + + it('should call ethQuery.estimateGas and resolve that call\'s data', async () => { + const result = await estimateGas({ mockParam: 'someData' }) + assert.equal(global.ethQuery.estimateGas.callCount, 1) + assert.deepEqual( + result, + { mockParam: 'someData', estimateGasCalled: true } + ) + }) + + it('should reject with ethQuery.estimateGas error', async () => { + try { + await estimateGas({ mockParam: 'someData', isMockErr: true }) + } catch (err) { + assert.equal(err, 'mockErr') + } + }) + }) + + describe('estimateGasPriceFromRecentBlocks', () => { + const ONE_GWEI_IN_WEI_HEX_PLUS_ONE = addCurrencies(ONE_GWEI_IN_WEI_HEX, '0x1', { + aBase: 16, + bBase: 16, + toNumericBase: 'hex', + }) + const ONE_GWEI_IN_WEI_HEX_PLUS_TWO = addCurrencies(ONE_GWEI_IN_WEI_HEX, '0x2', { + aBase: 16, + bBase: 16, + toNumericBase: 'hex', + }) + const ONE_GWEI_IN_WEI_HEX_MINUS_ONE = subtractCurrencies(ONE_GWEI_IN_WEI_HEX, '0x1', { + aBase: 16, + bBase: 16, + toNumericBase: 'hex', + }) + + it(`should return ${ONE_GWEI_IN_WEI_HEX} if recentBlocks is falsy`, () => { + assert.equal(estimateGasPriceFromRecentBlocks(), ONE_GWEI_IN_WEI_HEX) + }) + + it(`should return ${ONE_GWEI_IN_WEI_HEX} if recentBlocks is empty`, () => { + assert.equal(estimateGasPriceFromRecentBlocks([]), ONE_GWEI_IN_WEI_HEX) + }) + + it(`should estimate a block's gasPrice as ${ONE_GWEI_IN_WEI_HEX} if it has no gas prices`, () => { + const mockRecentBlocks = [ + { gasPrices: null }, + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_PLUS_ONE ] }, + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_MINUS_ONE ] }, + ] + assert.equal(estimateGasPriceFromRecentBlocks(mockRecentBlocks), ONE_GWEI_IN_WEI_HEX) + }) + + it(`should estimate a block's gasPrice as ${ONE_GWEI_IN_WEI_HEX} if it has empty gas prices`, () => { + const mockRecentBlocks = [ + { gasPrices: [] }, + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_PLUS_ONE ] }, + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_MINUS_ONE ] }, + ] + assert.equal(estimateGasPriceFromRecentBlocks(mockRecentBlocks), ONE_GWEI_IN_WEI_HEX) + }) + + it(`should return the middle value of all blocks lowest prices`, () => { + const mockRecentBlocks = [ + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_PLUS_TWO ] }, + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_MINUS_ONE ] }, + { gasPrices: [ ONE_GWEI_IN_WEI_HEX_PLUS_ONE ] }, + ] + assert.equal(estimateGasPriceFromRecentBlocks(mockRecentBlocks), ONE_GWEI_IN_WEI_HEX_PLUS_ONE) + }) + + it(`should work if a block has multiple gas prices`, () => { + const mockRecentBlocks = [ + { gasPrices: [ '0x1', '0x2', '0x3', '0x4', '0x5' ] }, + { gasPrices: [ '0x101', '0x100', '0x103', '0x104', '0x102' ] }, + { gasPrices: [ '0x150', '0x50', '0x100', '0x200', '0x5' ] }, + ] + assert.equal(estimateGasPriceFromRecentBlocks(mockRecentBlocks), '0x5') + }) + }) }) -- cgit From 4f0b4eef5030575e8ebdf35ca19fbc77376c70b9 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 22 May 2018 12:46:53 -0230 Subject: Estimate gas using same algorithm as backend. --- ui/app/components/send_/send.component.js | 3 + ui/app/components/send_/send.constants.js | 3 + ui/app/components/send_/send.container.js | 5 +- ui/app/components/send_/send.selectors.js | 5 + ui/app/components/send_/send.utils.js | 65 ++++++----- .../components/send_/tests/send-component.test.js | 2 + .../components/send_/tests/send-container.test.js | 7 +- .../send_/tests/send-selectors-test-data.js | 1 + .../components/send_/tests/send-selectors.test.js | 10 ++ ui/app/components/send_/tests/send-utils.test.js | 126 +++++++++++---------- 10 files changed, 134 insertions(+), 93 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.component.js b/ui/app/components/send_/send.component.js index 438923ab3..0f82d3f19 100644 --- a/ui/app/components/send_/send.component.js +++ b/ui/app/components/send_/send.component.js @@ -18,6 +18,7 @@ export default class SendTransactionScreen extends PersistentForm { PropTypes.string, PropTypes.number, ]), + blockGasLimit: PropTypes.string, conversionRate: PropTypes.number, data: PropTypes.string, editingTransactionId: PropTypes.string, @@ -40,6 +41,7 @@ export default class SendTransactionScreen extends PersistentForm { updateGas () { const { + blockGasLimit, data, editingTransactionId, gasLimit, @@ -51,6 +53,7 @@ export default class SendTransactionScreen extends PersistentForm { } = this.props updateAndSetGasTotal({ + blockGasLimit, data, editingTransactionId, gasLimit, diff --git a/ui/app/components/send_/send.constants.js b/ui/app/components/send_/send.constants.js index e911a5510..df5dee371 100644 --- a/ui/app/components/send_/send.constants.js +++ b/ui/app/components/send_/send.constants.js @@ -35,6 +35,8 @@ const ONE_GWEI_IN_WEI_HEX = ethUtil.addHexPrefix(conversionUtil('0x1', { toNumericBase: 'hex', })) +const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send. + module.exports = { INSUFFICIENT_FUNDS_ERROR, INSUFFICIENT_TOKENS_ERROR, @@ -48,5 +50,6 @@ module.exports = { NEGATIVE_ETH_ERROR, ONE_GWEI_IN_WEI_HEX, REQUIRED_ERROR, + SIMPLE_GAS_COST, TOKEN_TRANSFER_FUNCTION_SIGNATURE, } diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js index 879aef87c..3b72a3a5a 100644 --- a/ui/app/components/send_/send.container.js +++ b/ui/app/components/send_/send.container.js @@ -4,6 +4,7 @@ import { withRouter } from 'react-router-dom' import { compose } from 'recompose' import { getAmountConversionRate, + getBlockGasLimit, getConversionRate, getCurrentNetwork, getGasLimit, @@ -45,6 +46,7 @@ function mapStateToProps (state) { return { amount: getSendAmount(state), amountConversionRate: getAmountConversionRate(state), + blockGasLimit: getBlockGasLimit(state), conversionRate: getConversionRate(state), data: generateTokenTransferData(selectedAddress, selectedToken), editingTransactionId: getSendEditingTransactionId(state), @@ -66,6 +68,7 @@ function mapStateToProps (state) { function mapDispatchToProps (dispatch) { return { updateAndSetGasTotal: ({ + blockGasLimit, data, editingTransactionId, gasLimit, @@ -75,7 +78,7 @@ function mapDispatchToProps (dispatch) { selectedToken, }) => { !editingTransactionId - ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data })) + ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data, blockGasLimit })) : dispatch(setGasTotal(calcGasTotal(gasLimit, gasPrice))) }, updateSendTokenBalance: ({ selectedToken, tokenContract, address }) => { diff --git a/ui/app/components/send_/send.selectors.js b/ui/app/components/send_/send.selectors.js index 850328e10..7e7cfe2e9 100644 --- a/ui/app/components/send_/send.selectors.js +++ b/ui/app/components/send_/send.selectors.js @@ -12,6 +12,7 @@ const selectors = { // autoAddToBetaUI, getAddressBook, getAmountConversionRate, + getBlockGasLimit, getConversionRate, getConvertedCurrency, getCurrentAccountWithSendEtherInfo, @@ -89,6 +90,10 @@ function getAmountConversionRate (state) { : getConversionRate(state) } +function getBlockGasLimit (state) { + return state.metamask.currentBlockGasLimit +} + function getConversionRate (state) { return state.metamask.conversionRate } diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index 6055c98b1..4c731c91b 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -13,18 +13,19 @@ const { INSUFFICIENT_TOKENS_ERROR, NEGATIVE_ETH_ERROR, ONE_GWEI_IN_WEI_HEX, + SIMPLE_GAS_COST, } = require('./send.constants') +const EthQuery = require('ethjs-query') const abi = require('ethereumjs-abi') module.exports = { calcGasTotal, + calcTokenBalance, doesAmountErrorRequireUpdate, estimateGas, estimateGasPriceFromRecentBlocks, generateTokenTransferData, getAmountErrorObject, - getParamsForGasEstimate, - calcTokenBalance, isBalanceSufficient, isTokenBalanceSufficient, } @@ -142,24 +143,6 @@ function getAmountErrorObject ({ return { amount: amountError } } -function getParamsForGasEstimate (selectedAddress, selectedToken, data) { - const { symbol } = selectedToken || {} - const estimatedGasParams = { - from: selectedAddress, - gas: '746a528800', - } - - if (symbol) { - Object.assign(estimatedGasParams, { value: '0x0' }) - } - - if (data) { - Object.assign(estimatedGasParams, { data }) - } - - return estimatedGasParams -} - function calcTokenBalance ({ selectedToken, usersToken }) { const { decimals } = selectedToken || {} return calcTokenAmount(usersToken.balance.toString(), decimals) + '' @@ -182,15 +165,40 @@ function doesAmountErrorRequireUpdate ({ return amountErrorRequiresUpdate } -function estimateGas (params = {}) { - return new Promise((resolve, reject) => { - global.ethQuery.estimateGas(params, (err, data) => { - if (err) { - return reject(err) - } - return resolve(data) - }) +async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimit, to }) { + const ethQuery = new EthQuery(global.ethereumProvider) + const { symbol } = selectedToken || {} + const estimatedGasParams = { from: selectedAddress } + + if (symbol) { + Object.assign(estimatedGasParams, { value: '0x0' }) + } + + if (data) { + Object.assign(estimatedGasParams, { 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 || code === '0x')) { + return SIMPLE_GAS_COST + } + + estimatedGasParams.to = to + + // if not, fall back to block gasLimit + estimatedGasParams.gas = multiplyCurrencies(blockGasLimit, 0.95, { + multiplicandBase: 16, + multiplierBase: 10, + roundDown: '0', + toNumericBase: 'hex', }) + + // run tx + const estimatedGas = await ethQuery.estimateGas(estimatedGasParams) + return estimatedGas.toString(16) } function generateTokenTransferData (selectedAddress, selectedToken) { @@ -222,5 +230,6 @@ function estimateGasPriceFromRecentBlocks (recentBlocks) { .sort(hexComparator)[0] }) .sort(hexComparator) + return lowestPrices[Math.floor(lowestPrices.length / 2)] } diff --git a/ui/app/components/send_/tests/send-component.test.js b/ui/app/components/send_/tests/send-component.test.js index 780ee1046..3abff0d23 100644 --- a/ui/app/components/send_/tests/send-component.test.js +++ b/ui/app/components/send_/tests/send-component.test.js @@ -32,6 +32,7 @@ describe.only('Send Component', function () { wrapper = shallow( () => arg2() }, './send.selectors': { getAmountConversionRate: (s) => `mockAmountConversionRate:${s}`, + getBlockGasLimit: (s) => `mockBlockGasLimit:${s}`, getConversionRate: (s) => `mockConversionRate:${s}`, getCurrentNetwork: (s) => `mockNetwork:${s}`, getGasLimit: (s) => `mockGasLimit:${s}`, @@ -58,6 +59,7 @@ describe('send container', () => { assert.deepEqual(mapStateToProps('mockState'), { amount: 'mockAmount:mockState', amountConversionRate: 'mockAmountConversionRate:mockState', + blockGasLimit: 'mockBlockGasLimit:mockState', conversionRate: 'mockConversionRate:mockState', data: 'mockData:mockSelectedAddress:mockStatemockSelectedToken:mockState', editingTransactionId: 'mockEditingTransactionId:mockState', @@ -89,6 +91,7 @@ describe('send container', () => { describe('updateAndSetGasTotal()', () => { const mockProps = { + blockGasLimit: 'mockBlockGasLimit', data: '0x1', editingTransactionId: '0x2', gasLimit: '0x3', @@ -108,14 +111,14 @@ describe('send container', () => { }) it('should dispatch an updateGasData action when editingTransactionId is falsy', () => { - const { selectedAddress, selectedToken, data, recentBlocks } = mockProps + const { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit } = mockProps mapDispatchToPropsObject.updateAndSetGasTotal( Object.assign({}, mockProps, {editingTransactionId: false}) ) assert(dispatchSpy.calledOnce) assert.deepEqual( actionSpies.updateGasData.getCall(0).args[0], - { selectedAddress, selectedToken, data, recentBlocks } + { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit } ) }) }) diff --git a/ui/app/components/send_/tests/send-selectors-test-data.js b/ui/app/components/send_/tests/send-selectors-test-data.js index a1423675d..8f9c19314 100644 --- a/ui/app/components/send_/tests/send-selectors-test-data.js +++ b/ui/app/components/send_/tests/send-selectors-test-data.js @@ -22,6 +22,7 @@ module.exports = { 'name': 'Send Account 4', }, }, + 'currentBlockGasLimit': '0x4c1878', 'currentCurrency': 'USD', 'conversionRate': 1200.88200327, 'conversionDate': 1489013762, diff --git a/ui/app/components/send_/tests/send-selectors.test.js b/ui/app/components/send_/tests/send-selectors.test.js index 22e45afdb..152af8059 100644 --- a/ui/app/components/send_/tests/send-selectors.test.js +++ b/ui/app/components/send_/tests/send-selectors.test.js @@ -5,6 +5,7 @@ const { accountsWithSendEtherInfoSelector, // autoAddToBetaUI, getAddressBook, + getBlockGasLimit, getAmountConversionRate, getConversionRate, getConvertedCurrency, @@ -135,6 +136,15 @@ describe('send selectors', () => { }) }) + describe('getBlockGasLimit', () => { + it('should return the current block gas limit', () => { + assert.deepEqual( + getBlockGasLimit(mockState), + '0x4c1878' + ) + }) + }) + describe('getConversionRate()', () => { it('should return the eth conversion rate', () => { assert.deepEqual( diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index b5211a63d..a01ab4eba 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -3,6 +3,7 @@ import sinon from 'sinon' import proxyquire from 'proxyquire' import { ONE_GWEI_IN_WEI_HEX, + SIMPLE_GAS_COST, } from '../send.constants' const { addCurrencies, @@ -18,11 +19,19 @@ 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), - multiplyCurrencies: sinon.stub().callsFake((a, b) => a * b), + multiplyCurrencies: sinon.stub().callsFake((a, b) => `${a}x${b}`), calcTokenAmount: sinon.stub().callsFake((a, d) => 'calc:' + a + d), 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, @@ -34,6 +43,7 @@ const sendUtils = proxyquire('../send.utils.js', { 'ethereumjs-abi': { rawEncode: stubs.rawEncode, }, + 'ethjs-query': EthQuery, }) const { @@ -43,7 +53,6 @@ const { estimateGasPriceFromRecentBlocks, generateTokenTransferData, getAmountErrorObject, - getParamsForGasEstimate, calcTokenBalance, isBalanceSufficient, isTokenBalanceSufficient, @@ -54,7 +63,7 @@ describe('send utils', () => { describe('calcGasTotal()', () => { it('should call multiplyCurrencies with the correct params and return the multiplyCurrencies return', () => { const result = calcGasTotal(12, 15) - assert.equal(result, 180) + assert.equal(result, '12x15') const call_ = stubs.multiplyCurrencies.getCall(0).args assert.deepEqual( call_, @@ -145,41 +154,6 @@ describe('send utils', () => { }) }) - describe('getParamsForGasEstimate()', () => { - it('should return from and gas properties if no symbol or data', () => { - assert.deepEqual( - getParamsForGasEstimate('mockAddress'), - { - from: 'mockAddress', - gas: '746a528800', - } - ) - }) - - it('should return value property if selected token provided', () => { - assert.deepEqual( - getParamsForGasEstimate('mockAddress', { symbol: 'ABC' }), - { - from: 'mockAddress', - gas: '746a528800', - value: '0x0', - } - ) - }) - - it('should return data property if data provided', () => { - assert.deepEqual( - getParamsForGasEstimate('mockAddress', { symbol: 'ABC' }, 'somedata'), - { - from: 'mockAddress', - gas: '746a528800', - value: '0x0', - data: 'somedata', - } - ) - }) - }) - describe('calcTokenBalance()', () => { it('should return the calculated token blance', () => { assert.equal(calcTokenBalance({ @@ -271,38 +245,66 @@ describe('send utils', () => { }) describe('estimateGas', () => { - let tempEthQuery - beforeEach(() => { - tempEthQuery = global.ethQuery - global.ethQuery = { - estimateGas: sinon.stub().callsFake((data, cb) => { - return cb( - data.isMockErr ? 'mockErr' : null, - Object.assign(data, { estimateGasCalled: true }) - ) - }) - } - }) + const baseMockParams = { + blockGasLimit: '0x64', + selectedAddress: 'mockAddress', + to: '0xisContract', + } + const baseExpectedCall = { + from: 'mockAddress', + gas: '0x64x0.95', + to: '0xisContract', + } afterEach(() => { - global.ethQuery = tempEthQuery + EthQuery.prototype.estimateGas.resetHistory() + EthQuery.prototype.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.deepEqual( + EthQuery.prototype.estimateGas.getCall(0).args[0], + 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.deepEqual( + EthQuery.prototype.estimateGas.getCall(0).args[0], + Object.assign({ 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.deepEqual( + EthQuery.prototype.estimateGas.getCall(0).args[0], + Object.assign({ data: 'mockData' }, baseExpectedCall) + ) + assert.equal(result, 'mockToString:16') }) - it('should call ethQuery.estimateGas and resolve that call\'s data', async () => { - const result = await estimateGas({ mockParam: 'someData' }) - assert.equal(global.ethQuery.estimateGas.callCount, 1) + 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( - result, - { mockParam: 'someData', estimateGasCalled: true } + EthQuery.prototype.estimateGas.getCall(0).args[0], + Object.assign({ data: 'mockData' }, baseExpectedCall) ) + assert.equal(result, 'mockToString:16') }) - it('should reject with ethQuery.estimateGas error', async () => { - try { - await estimateGas({ mockParam: 'someData', isMockErr: true }) - } catch (err) { - assert.equal(err, 'mockErr') - } + it(`should return ${SIMPLE_GAS_COST} if ethQuery.getCode does not return '0x'`, async () => { + assert.equal(EthQuery.prototype.estimateGas.callCount, 0) + const result = await estimateGas(Object.assign({}, baseMockParams, { to: '0x123' })) + assert.equal(result, SIMPLE_GAS_COST) }) }) -- cgit From 0f20fce9b761fc0aa16d61b2b739fa7f9b9f6a7d Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 23 May 2018 14:13:25 -0230 Subject: Auto update gas estimate when to changes. --- .../send_/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-content/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 ++++++------ .../components/send_/tests/send-component.test.js | 8 ++++ .../components/send_/tests/send-container.test.js | 6 ++- ui/app/components/send_/tests/send-utils.test.js | 54 +++++++++------------- 13 files changed, 103 insertions(+), 63 deletions(-) (limited to 'ui/app/components/send_') 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.deepEqual( - EthQuery.prototype.estimateGas.getCall(0).args[0], - Object.assign({ 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) + 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) + 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) }) -- cgit From 5a842e440f0ec565eba38aec4c1c953a775557ea Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 25 May 2018 11:07:16 -0230 Subject: Gas estimation uses block gas limit as fallback if query.estimateGas returns an expected error. --- ui/app/components/send_/send.utils.js | 15 ++++++++---- ui/app/components/send_/tests/send-utils.test.js | 29 +++++++++++++++++++++++- 2 files changed, 39 insertions(+), 5 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index 750411908..9b8f1d118 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -193,14 +193,21 @@ async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimi roundDown: '0', toNumericBase: 'hex', })) - // run tx return new Promise((resolve, reject) => { - estimateGasMethod(paramsForGasEstimate, (err, estimatedGas) => { + return estimateGasMethod(paramsForGasEstimate, (err, estimatedGas) => { if (err) { - reject(err) + const simulationFailed = ( + err.message.includes('Transaction execution error.') || + err.message.includes('gas required exceeds allowance or always failing transaction') + ) + if (simulationFailed) { + return resolve(paramsForGasEstimate.gas) + } else { + return reject(err) + } } - resolve(estimatedGas.toString(16)) + return resolve(estimatedGas.toString(16)) }) }) } diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index 3c772ed47..4801d4a09 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -241,7 +241,10 @@ describe('send utils', () => { selectedAddress: 'mockAddress', to: '0xisContract', estimateGasMethod: sinon.stub().callsFake( - (data, cb) => cb(null, { toString: (n) => `mockToString:${n}` }) + (data, cb) => cb( + data.to.match(/willFailBecauseOf:/) ? { message: data.to.match(/\:(.+)$/)[1] } : null, + { toString: (n) => `mockToString:${n}` } + ) ), } const baseExpectedCall = { @@ -298,6 +301,30 @@ describe('send utils', () => { const result = await estimateGas(Object.assign({}, baseMockParams, { to: '0x123' })) assert.equal(result, SIMPLE_GAS_COST) }) + + it(`should return the adjusted blockGasLimit if it fails with a 'Transaction execution error.'`, async () => { + const result = await estimateGas(Object.assign({}, baseMockParams, { + to: 'isContract willFailBecauseOf:Transaction execution error.', + })) + assert.equal(result, '0x64x0.95') + }) + + it(`should return the adjusted blockGasLimit if it fails with a 'gas required exceeds allowance or always failing transaction.'`, async () => { + const result = await estimateGas(Object.assign({}, baseMockParams, { + to: 'isContract willFailBecauseOf:gas required exceeds allowance or always failing transaction.', + })) + assert.equal(result, '0x64x0.95') + }) + + it(`should reject other errors`, async () => { + try { + await estimateGas(Object.assign({}, baseMockParams, { + to: 'isContract willFailBecauseOf:some other error', + })) + } catch (err) { + assert.deepEqual(err, { message: 'some other error' }) + } + }) }) describe('estimateGasPriceFromRecentBlocks', () => { -- cgit From 64aa56b5a6f753d198ceadc9cf2c79929046fc19 Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 28 May 2018 12:54:19 -0700 Subject: test - send-utils.test - lint fix --- ui/app/components/send_/tests/send-utils.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js index 4801d4a09..14125d7a6 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -242,7 +242,7 @@ describe('send utils', () => { to: '0xisContract', estimateGasMethod: sinon.stub().callsFake( (data, cb) => cb( - data.to.match(/willFailBecauseOf:/) ? { message: data.to.match(/\:(.+)$/)[1] } : null, + data.to.match(/willFailBecauseOf:/) ? { message: data.to.match(/:(.+)$/)[1] } : null, { toString: (n) => `mockToString:${n}` } ) ), -- cgit From 1bde2892ec222cec1ba3265d50ed59f665afbf83 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 30 May 2018 12:56:42 -0230 Subject: Improve efficiency of estimateGasPriceFromRecentBlocks --- ui/app/components/send_/send.utils.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index 9b8f1d118..e685cc274 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -221,26 +221,21 @@ function generateTokenTransferData (selectedAddress, selectedToken) { ).join('') } -function hexComparator (a, b) { - return conversionGreaterThan( - { value: a, fromNumericBase: 'hex' }, - { value: b, fromNumericBase: 'hex' }, - ) ? 1 : -1 -} - function estimateGasPriceFromRecentBlocks (recentBlocks) { // Return 1 gwei if no blocks have been observed: if (!recentBlocks || recentBlocks.length === 0) { return ONE_GWEI_IN_WEI_HEX } + const lowestPrices = recentBlocks.map((block) => { if (!block.gasPrices || block.gasPrices.length < 1) { return ONE_GWEI_IN_WEI_HEX } - return block.gasPrices - .sort(hexComparator)[0] + return block.gasPrices.reduce((currentLowest, next) => { + return parseInt(next, 16) < parseInt(currentLowest, 16) ? next : currentLowest + }) }) - .sort(hexComparator) + .sort((a, b) => parseInt(a, 16) > parseInt(b, 16) ? 1 : -1) return lowestPrices[Math.floor(lowestPrices.length / 2)] } -- cgit From 1b879f45bc0d53e8c0ffa9513b525e0055ed8f81 Mon Sep 17 00:00:00 2001 From: Dan Date: Sat, 2 Jun 2018 01:53:01 -0230 Subject: Fix calculation of data property for gas estimation on token transfers. --- ui/app/components/send_/send.component.js | 3 -- ui/app/components/send_/send.container.js | 8 +--- ui/app/components/send_/send.utils.js | 23 +++++------- .../components/send_/tests/send-component.test.js | 2 - .../components/send_/tests/send-container.test.js | 7 +--- ui/app/components/send_/tests/send-utils.test.js | 43 ++++++++++++++-------- 6 files changed, 41 insertions(+), 45 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.component.js b/ui/app/components/send_/send.component.js index 97c6d1294..516251e22 100644 --- a/ui/app/components/send_/send.component.js +++ b/ui/app/components/send_/send.component.js @@ -20,7 +20,6 @@ export default class SendTransactionScreen extends PersistentForm { ]), blockGasLimit: PropTypes.string, conversionRate: PropTypes.number, - data: PropTypes.string, editingTransactionId: PropTypes.string, from: PropTypes.object, gasLimit: PropTypes.string, @@ -43,7 +42,6 @@ export default class SendTransactionScreen extends PersistentForm { const { amount, blockGasLimit, - data, editingTransactionId, gasLimit, gasPrice, @@ -55,7 +53,6 @@ export default class SendTransactionScreen extends PersistentForm { updateAndSetGasTotal({ blockGasLimit, - data, editingTransactionId, gasLimit, gasPrice, diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js index 7e241aa2d..1fd96d61f 100644 --- a/ui/app/components/send_/send.container.js +++ b/ui/app/components/send_/send.container.js @@ -31,7 +31,6 @@ import { } from '../../ducks/send.duck' import { calcGasTotal, - generateTokenTransferData, } from './send.utils.js' module.exports = compose( @@ -40,15 +39,11 @@ module.exports = compose( )(SendEther) function mapStateToProps (state) { - const selectedAddress = getSelectedAddress(state) - const selectedToken = getSelectedToken(state) - return { amount: getSendAmount(state), amountConversionRate: getAmountConversionRate(state), blockGasLimit: getBlockGasLimit(state), conversionRate: getConversionRate(state), - data: generateTokenTransferData(selectedAddress, selectedToken), editingTransactionId: getSendEditingTransactionId(state), from: getSendFromObject(state), gasLimit: getGasLimit(state), @@ -69,7 +64,6 @@ function mapDispatchToProps (dispatch) { return { updateAndSetGasTotal: ({ blockGasLimit, - data, editingTransactionId, gasLimit, gasPrice, @@ -80,7 +74,7 @@ function mapDispatchToProps (dispatch) { value, }) => { !editingTransactionId - ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data, blockGasLimit, to, value })) + ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, 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 e685cc274..855d12303 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -14,6 +14,7 @@ const { NEGATIVE_ETH_ERROR, ONE_GWEI_IN_WEI_HEX, SIMPLE_GAS_COST, + TOKEN_TRANSFER_FUNCTION_SIGNATURE, } = require('./send.constants') const abi = require('ethereumjs-abi') const ethUtil = require('ethereumjs-util') @@ -165,26 +166,23 @@ function doesAmountErrorRequireUpdate ({ return amountErrorRequiresUpdate } -async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimit, to, value, gasPrice, estimateGasMethod }) { - const { symbol } = selectedToken || {} +async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, value, gasPrice, estimateGasMethod }) { const paramsForGasEstimate = { from: selectedAddress, value, gasPrice } - if (symbol) { - Object.assign(paramsForGasEstimate, { value: '0x0' }) + if (selectedToken) { + paramsForGasEstimate.value = '0x0' + paramsForGasEstimate.data = generateTokenTransferData({ toAddress: to, amount: value, selectedToken }) } - if (data) { - Object.assign(paramsForGasEstimate, { data }) - } // if recipient has no code, gas is 21k max: const hasRecipient = Boolean(to) let code if (hasRecipient) code = await global.eth.getCode(to) - if (hasRecipient && (!code || code === '0x')) { + if (hasRecipient && (!code || code === '0x') && !selectedToken) { return SIMPLE_GAS_COST } - paramsForGasEstimate.to = to + paramsForGasEstimate.to = selectedToken ? selectedToken.address : to // if not, fall back to block gasLimit paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, { @@ -212,11 +210,10 @@ async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimi }) } -function generateTokenTransferData (selectedAddress, selectedToken) { +function generateTokenTransferData ({ toAddress = '0x0', amount = '0x0', selectedToken }) { if (!selectedToken) return - console.log(`abi.rawEncode`, abi.rawEncode) - return Array.prototype.map.call( - abi.rawEncode(['address', 'uint256'], [selectedAddress, '0x0']), + return TOKEN_TRANSFER_FUNCTION_SIGNATURE + Array.prototype.map.call( + abi.rawEncode(['address', 'uint256'], [toAddress, ethUtil.addHexPrefix(amount)]), x => ('00' + x.toString(16)).slice(-2) ).join('') } diff --git a/ui/app/components/send_/tests/send-component.test.js b/ui/app/components/send_/tests/send-component.test.js index 2529d6e5f..4e33d8f63 100644 --- a/ui/app/components/send_/tests/send-component.test.js +++ b/ui/app/components/send_/tests/send-component.test.js @@ -34,7 +34,6 @@ describe('Send Component', function () { amountConversionRate={'mockAmountConversionRate'} blockGasLimit={'mockBlockGasLimit'} conversionRate={10} - data={'mockData'} editingTransactionId={'mockEditingTransactionId'} from={ { address: 'mockAddress', balance: 'mockBalance' } } gasLimit={'mockGasLimit'} @@ -210,7 +209,6 @@ describe('Send Component', function () { propsMethodSpies.updateAndSetGasTotal.getCall(0).args[0], { blockGasLimit: 'mockBlockGasLimit', - data: 'mockData', editingTransactionId: 'mockEditingTransactionId', gasLimit: 'mockGasLimit', gasPrice: 'mockGasPrice', diff --git a/ui/app/components/send_/tests/send-container.test.js b/ui/app/components/send_/tests/send-container.test.js index d077ab4ee..056aad148 100644 --- a/ui/app/components/send_/tests/send-container.test.js +++ b/ui/app/components/send_/tests/send-container.test.js @@ -47,7 +47,6 @@ proxyquire('../send.container.js', { '../../ducks/send.duck': duckActionSpies, './send.utils.js': { calcGasTotal: (gasLimit, gasPrice) => gasLimit + gasPrice, - generateTokenTransferData: (a, b) => `mockData:${a + b}`, }, }) @@ -61,7 +60,6 @@ describe('send container', () => { amountConversionRate: 'mockAmountConversionRate:mockState', blockGasLimit: 'mockBlockGasLimit:mockState', conversionRate: 'mockConversionRate:mockState', - data: 'mockData:mockSelectedAddress:mockStatemockSelectedToken:mockState', editingTransactionId: 'mockEditingTransactionId:mockState', from: 'mockFrom:mockState', gasLimit: 'mockGasLimit:mockState', @@ -92,7 +90,6 @@ describe('send container', () => { describe('updateAndSetGasTotal()', () => { const mockProps = { blockGasLimit: 'mockBlockGasLimit', - data: '0x1', editingTransactionId: '0x2', gasLimit: '0x3', gasPrice: '0x4', @@ -113,14 +110,14 @@ describe('send container', () => { }) it('should dispatch an updateGasData action when editingTransactionId is falsy', () => { - const { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit, to, value } = mockProps + const { selectedAddress, selectedToken, 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, to, value } + { selectedAddress, selectedToken, 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 14125d7a6..b3f6372ef 100644 --- a/ui/app/components/send_/tests/send-utils.test.js +++ b/ui/app/components/send_/tests/send-utils.test.js @@ -106,11 +106,23 @@ describe('send utils', () => { describe('generateTokenTransferData()', () => { it('should return undefined if not passed a selected token', () => { - assert.equal(generateTokenTransferData('mockAddress', false), undefined) + assert.equal(generateTokenTransferData({ toAddress: 'mockAddress', amount: '0xa', selectedToken: false}), undefined) + }) + + it('should call abi.rawEncode with the correct params', () => { + stubs.rawEncode.resetHistory() + generateTokenTransferData({ toAddress: 'mockAddress', amount: 'ab', selectedToken: true}) + assert.deepEqual( + stubs.rawEncode.getCall(0).args, + [['address', 'uint256'], ['mockAddress', '0xab']] + ) }) it('should return encoded token transfer data', () => { - assert.equal(generateTokenTransferData('mockAddress', true), '104c') + assert.equal( + generateTokenTransferData({ toAddress: 'mockAddress', amount: '0xa', selectedToken: true}), + '0xa9059cbb104c' + ) }) }) @@ -276,22 +288,17 @@ describe('send utils', () => { 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)) + it('should call ethQuery.estimateGas with a value of 0x0 and the expected data and to if passed a selectedToken', async () => { + const result = await estimateGas(Object.assign({ data: 'mockData', selectedToken: { address: 'mockAddress' } }, baseMockParams)) assert.equal(baseMockParams.estimateGasMethod.callCount, 1) assert.deepEqual( 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(baseMockParams.estimateGasMethod.callCount, 1) - assert.deepEqual( - baseMockParams.estimateGasMethod.getCall(0).args[0], - Object.assign({ gasPrice: undefined, value: undefined, data: 'mockData' }, baseExpectedCall) + Object.assign({}, baseExpectedCall, { + gasPrice: undefined, + value: '0x0', + data: '0xa9059cbb104c', + to: 'mockAddress', + }) ) assert.equal(result, 'mockToString:16') }) @@ -302,6 +309,12 @@ describe('send utils', () => { 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: '' } })) + assert.notEqual(result, SIMPLE_GAS_COST) + }) + it(`should return the adjusted blockGasLimit if it fails with a 'Transaction execution error.'`, async () => { const result = await estimateGas(Object.assign({}, baseMockParams, { to: 'isContract willFailBecauseOf:Transaction execution error.', -- cgit From ada59054c9d102cc99b950f1377256cac5545649 Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 4 Jun 2018 15:50:52 -0230 Subject: Simplify recipient code check in send.utils estimateGas --- ui/app/components/send_/send.utils.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'ui/app/components/send_') diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js index 855d12303..67699be77 100644 --- a/ui/app/components/send_/send.utils.js +++ b/ui/app/components/send_/send.utils.js @@ -176,10 +176,11 @@ async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, // if recipient has no code, gas is 21k max: const hasRecipient = Boolean(to) - let code - if (hasRecipient) code = await global.eth.getCode(to) - if (hasRecipient && (!code || code === '0x') && !selectedToken) { - return SIMPLE_GAS_COST + if (hasRecipient && !selectedToken) { + const code = await global.eth.getCode(to) + if (!code || code === '0x') { + return SIMPLE_GAS_COST + } } paramsForGasEstimate.to = selectedToken ? selectedToken.address : to -- cgit