diff options
Simplify gas estimate actions and add local estimateGasPriceFromRecentBlocks method.
-rw-r--r-- | ui/app/actions.js | 51 | ||||
-rw-r--r-- | ui/app/components/customize-gas-modal/index.js | 6 | ||||
-rw-r--r-- | ui/app/components/send/currency-display.js | 6 | ||||
-rw-r--r-- | ui/app/components/send_/send.component.js | 3 | ||||
-rw-r--r-- | ui/app/components/send_/send.constants.js | 8 | ||||
-rw-r--r-- | ui/app/components/send_/send.container.js | 6 | ||||
-rw-r--r-- | ui/app/components/send_/send.selectors.js | 13 | ||||
-rw-r--r-- | ui/app/components/send_/send.utils.js | 37 | ||||
-rw-r--r-- | ui/app/components/send_/tests/send-component.test.js | 2 | ||||
-rw-r--r-- | ui/app/components/send_/tests/send-container.test.js | 9 | ||||
-rw-r--r-- | ui/app/components/send_/tests/send-selectors-test-data.js | 1 | ||||
-rw-r--r-- | ui/app/components/send_/tests/send-selectors.test.js | 10 | ||||
-rw-r--r-- | ui/app/components/send_/tests/send-utils.test.js | 106 |
13 files changed, 203 insertions, 55 deletions
diff --git a/ui/app/actions.js b/ui/app/actions.js index bec9a8cfb..70ec3aed8 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -6,6 +6,8 @@ const { calcGasTotal, getParamsForGasEstimate, calcTokenBalance, + estimateGas, + estimateGasPriceFromRecentBlocks, } = require('./components/send_/send.utils') const ethUtil = require('ethereumjs-util') const { fetchLocale } = require('../i18n-helper') @@ -160,9 +162,6 @@ var actions = { updateTransactionParams, UPDATE_TRANSACTION_PARAMS: 'UPDATE_TRANSACTION_PARAMS', // send screen - estimateGas, - getGasEstimate, - getGasPrice, UPDATE_GAS_LIMIT: 'UPDATE_GAS_LIMIT', UPDATE_GAS_PRICE: 'UPDATE_GAS_PRICE', UPDATE_GAS_TOTAL: 'UPDATE_GAS_TOTAL', @@ -705,22 +704,6 @@ function signTx (txData) { } } -function estimateGas (params = {}) { - return (dispatch) => { - return new Promise((resolve, reject) => { - global.ethQuery.estimateGas(params, (err, data) => { - if (err) { - dispatch(actions.displayWarning(err.message)) - return reject(err) - } - dispatch(actions.hideWarning()) - dispatch(actions.setGasLimit(data)) - return resolve(data) - }) - }) - } -} - function setGasLimit (gasLimit) { return { type: actions.UPDATE_GAS_LIMIT, @@ -728,22 +711,6 @@ function setGasLimit (gasLimit) { } } -function getGasPrice () { - return (dispatch) => { - return new Promise((resolve, reject) => { - global.ethQuery.gasPrice((err, data) => { - if (err) { - dispatch(actions.displayWarning(err.message)) - return reject(err) - } - dispatch(actions.hideWarning()) - dispatch(actions.setGasPrice(data)) - return resolve(data) - }) - }) - } -} - function setGasPrice (gasPrice) { return { type: actions.UPDATE_GAS_PRICE, @@ -758,22 +725,18 @@ function setGasTotal (gasTotal) { } } -function getGasEstimate ({ selectedAddress, selectedToken, data }) { +function updateGasData ({ recentBlocks, selectedAddress, selectedToken, data }) { return (dispatch) => { const estimateGasParams = getParamsForGasEstimate(selectedAddress, selectedToken, data) return Promise.all([ - dispatch(actions.getGasPrice()), - dispatch(actions.estimateGas(estimateGasParams)), + Promise.resolve(estimateGasPriceFromRecentBlocks(recentBlocks)), + estimateGas(estimateGasParams), ]) .then(([gasPrice, gas]) => { + dispatch(actions.setGasPrice(gasPrice)) + dispatch(actions.setGasLimit(gas)) return calcGasTotal(gas, gasPrice) }) - } -} - -function updateGasData ({ selectedAddress, selectedToken, data }) { - return (dispatch) => { - return dispatch(actions.getGasEstimate({ selectedAddress, selectedToken, data })) .then((gasEstimate) => { dispatch(actions.setGasTotal(gasEstimate)) dispatch(updateSendErrors({ gasLoadingError: null })) diff --git a/ui/app/components/customize-gas-modal/index.js b/ui/app/components/customize-gas-modal/index.js index 9ab785760..6637d412a 100644 --- a/ui/app/components/customize-gas-modal/index.js +++ b/ui/app/components/customize-gas-modal/index.js @@ -67,7 +67,7 @@ function mapDispatchToProps (dispatch) { hideModal: () => dispatch(actions.hideModal()), setGasPrice: newGasPrice => dispatch(actions.setGasPrice(newGasPrice)), setGasLimit: newGasLimit => dispatch(actions.setGasLimit(newGasLimit)), - updateGasData: newGasTotal => dispatch(actions.setGasTotal(newGasTotal)), + setGasTotal: newGasTotal => dispatch(actions.setGasTotal(newGasTotal)), updateSendAmount: newAmount => dispatch(actions.updateSendAmount(newAmount)), updateSendErrors: error => dispatch(updateSendErrors(error)), } @@ -112,7 +112,7 @@ CustomizeGasModal.prototype.save = function (gasPrice, gasLimit, gasTotal) { setGasPrice, setGasLimit, hideModal, - updateGasData, + setGasTotal, maxModeOn, selectedToken, balance, @@ -131,7 +131,7 @@ CustomizeGasModal.prototype.save = function (gasPrice, gasLimit, gasTotal) { setGasPrice(ethUtil.addHexPrefix(gasPrice)) setGasLimit(ethUtil.addHexPrefix(gasLimit)) - updateGasData(ethUtil.addHexPrefix(gasTotal)) + setGasTotal(ethUtil.addHexPrefix(gasTotal)) updateSendErrors({ insufficientFunds: false }) hideModal() } diff --git a/ui/app/components/send/currency-display.js b/ui/app/components/send/currency-display.js index 90fb2b66c..52dc56cb0 100644 --- a/ui/app/components/send/currency-display.js +++ b/ui/app/components/send/currency-display.js @@ -5,6 +5,7 @@ const CurrencyInput = require('../currency-input') const { conversionUtil, multiplyCurrencies } = require('../../conversion-util') const currencyFormatter = require('currency-formatter') const currencies = require('currency-formatter/currencies') +const ethUtil = require('ethereumjs-util') module.exports = CurrencyDisplay @@ -35,18 +36,17 @@ CurrencyDisplay.prototype.getAmount = function (value) { CurrencyDisplay.prototype.getValueToRender = function () { const { selectedToken, conversionRate, value } = this.props - const { decimals, symbol } = selectedToken || {} const multiplier = Math.pow(10, Number(decimals || 0)) return selectedToken - ? conversionUtil(value, { + ? conversionUtil(ethUtil.addHexPrefix(value), { fromNumericBase: 'hex', toCurrency: symbol, conversionRate: multiplier, invertConversionRate: true, }) - : conversionUtil(value, { + : conversionUtil(ethUtil.addHexPrefix(value), { fromNumericBase: 'hex', toNumericBase: 'dec', fromDenomination: 'WEI', 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') + }) + }) }) |