From 1fbdce8916151df2b31eebc5de29a1365e5dadff Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 10 Dec 2018 18:21:00 -0330 Subject: Improve ux for low gas price set (#5862) * Show user warning if they set gas price below safelow minimum, error if 0. * Properly cache basic price estimate data. * Default retry price to recommended price if original price was 0x0 * Use mock fetch in send-new-ui integration tests. --- ui/app/ducks/gas.duck.js | 43 ++++++++++++++++++++++--- ui/app/ducks/tests/gas-duck.test.js | 62 +++++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 7 deletions(-) (limited to 'ui/app/ducks') diff --git a/ui/app/ducks/gas.duck.js b/ui/app/ducks/gas.duck.js index 83c236d81..ee235a757 100644 --- a/ui/app/ducks/gas.duck.js +++ b/ui/app/ducks/gas.duck.js @@ -23,6 +23,7 @@ const SET_CUSTOM_GAS_TOTAL = 'metamask/gas/SET_CUSTOM_GAS_TOTAL' const SET_PRICE_AND_TIME_ESTIMATES = 'metamask/gas/SET_PRICE_AND_TIME_ESTIMATES' const SET_API_ESTIMATES_LAST_RETRIEVED = 'metamask/gas/SET_API_ESTIMATES_LAST_RETRIEVED' const SET_BASIC_API_ESTIMATES_LAST_RETRIEVED = 'metamask/gas/SET_BASIC_API_ESTIMATES_LAST_RETRIEVED' +const SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED = 'metamask/gas/SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED' // TODO: determine if this approach to initState is consistent with conventional ducks pattern const initState = { @@ -49,6 +50,7 @@ const initState = { basicPriceAndTimeEstimates: [], priceAndTimeEstimatesLastRetrieved: 0, basicPriceAndTimeEstimatesLastRetrieved: 0, + basicPriceEstimatesLastRetrieved: 0, errors: {}, } @@ -129,6 +131,11 @@ export default function reducer ({ gas: gasState = initState }, action = {}) { ...newState, basicPriceAndTimeEstimatesLastRetrieved: action.value, } + case SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED: + return { + ...newState, + basicPriceEstimatesLastRetrieved: action.value, + } case RESET_CUSTOM_DATA: return { ...newState, @@ -167,10 +174,17 @@ export function gasEstimatesLoadingFinished () { } export function fetchBasicGasEstimates () { - return (dispatch) => { + return (dispatch, getState) => { + const { + basicPriceEstimatesLastRetrieved, + basicPriceAndTimeEstimates, + } = getState().gas + const timeLastRetrieved = basicPriceEstimatesLastRetrieved || loadLocalStorageData('BASIC_PRICE_ESTIMATES_LAST_RETRIEVED') || 0 + dispatch(basicGasEstimatesLoadingStarted()) - return fetch('https://dev.blockscale.net/api/gasexpress.json', { + const promiseToFetch = Date.now() - timeLastRetrieved > 75000 + ? fetch('https://dev.blockscale.net/api/gasexpress.json', { 'headers': {}, 'referrer': 'https://dev.blockscale.net/api/', 'referrerPolicy': 'no-referrer-when-downgrade', @@ -195,10 +209,24 @@ export function fetchBasicGasEstimates () { blockTime, blockNum, } - dispatch(setBasicGasEstimateData(basicEstimates)) - dispatch(basicGasEstimatesLoadingFinished()) + + const timeRetrieved = Date.now() + dispatch(setBasicPriceEstimatesLastRetrieved(timeRetrieved)) + saveLocalStorageData(timeRetrieved, 'BASIC_PRICE_ESTIMATES_LAST_RETRIEVED') + saveLocalStorageData(basicEstimates, 'BASIC_PRICE_ESTIMATES') + return basicEstimates }) + : Promise.resolve(basicPriceAndTimeEstimates.length + ? basicPriceAndTimeEstimates + : loadLocalStorageData('BASIC_PRICE_ESTIMATES') + ) + + return promiseToFetch.then(basicEstimates => { + dispatch(setBasicGasEstimateData(basicEstimates)) + dispatch(basicGasEstimatesLoadingFinished()) + return basicEstimates + }) } } @@ -473,6 +501,13 @@ export function setBasicApiEstimatesLastRetrieved (retrievalTime) { } } +export function setBasicPriceEstimatesLastRetrieved (retrievalTime) { + return { + type: SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED, + value: retrievalTime, + } +} + export function resetCustomGasState () { return { type: RESET_CUSTOM_GAS_STATE } } diff --git a/ui/app/ducks/tests/gas-duck.test.js b/ui/app/ducks/tests/gas-duck.test.js index bf374c7ec..3637d8f29 100644 --- a/ui/app/ducks/tests/gas-duck.test.js +++ b/ui/app/ducks/tests/gas-duck.test.js @@ -20,6 +20,7 @@ const { setCustomGasErrors, resetCustomGasState, fetchBasicGasAndTimeEstimates, + fetchBasicGasEstimates, gasEstimatesLoadingStarted, gasEstimatesLoadingFinished, setPricesAndTimeEstimates, @@ -43,6 +44,7 @@ describe('Gas Duck', () => { safeLow: 10, safeLowWait: 'mockSafeLowWait', speed: 'mockSpeed', + standard: 20, } const mockPredictTableResponse = [ { expectedTime: 400, expectedWait: 40, gasprice: 0.25, somethingElse: 'foobar' }, @@ -67,7 +69,7 @@ describe('Gas Duck', () => { { expectedTime: 1, expectedWait: 0.5, gasprice: 20, somethingElse: 'foobar' }, ] const fetchStub = sinon.stub().callsFake((url) => new Promise(resolve => { - const dataToResolve = url.match(/ethgasAPI/) + const dataToResolve = url.match(/ethgasAPI|gasexpress/) ? mockEthGasApiResponse : mockPredictTableResponse resolve({ @@ -83,6 +85,7 @@ describe('Gas Duck', () => { }) afterEach(() => { + fetchStub.resetHistory() global.fetch = tempFetch global.Date.now = tempDateNow }) @@ -117,8 +120,7 @@ describe('Gas Duck', () => { priceAndTimeEstimatesLastRetrieved: 0, basicPriceAndTimeEstimates: [], basicPriceAndTimeEstimatesLastRetrieved: 0, - - + basicPriceEstimatesLastRetrieved: 0, } const BASIC_GAS_ESTIMATE_LOADING_FINISHED = 'metamask/gas/BASIC_GAS_ESTIMATE_LOADING_FINISHED' const BASIC_GAS_ESTIMATE_LOADING_STARTED = 'metamask/gas/BASIC_GAS_ESTIMATE_LOADING_STARTED' @@ -133,6 +135,7 @@ describe('Gas Duck', () => { const SET_PRICE_AND_TIME_ESTIMATES = 'metamask/gas/SET_PRICE_AND_TIME_ESTIMATES' const SET_API_ESTIMATES_LAST_RETRIEVED = 'metamask/gas/SET_API_ESTIMATES_LAST_RETRIEVED' const SET_BASIC_API_ESTIMATES_LAST_RETRIEVED = 'metamask/gas/SET_BASIC_API_ESTIMATES_LAST_RETRIEVED' + const SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED = 'metamask/gas/SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED' describe('GasReducer()', () => { it('should initialize state', () => { @@ -301,6 +304,59 @@ describe('Gas Duck', () => { }) }) + describe('fetchBasicGasEstimates', () => { + const mockDistpatch = sinon.spy() + it('should call fetch with the expected params', async () => { + await fetchBasicGasEstimates()(mockDistpatch, () => ({ gas: Object.assign( + {}, + initState, + { basicPriceAEstimatesLastRetrieved: 1000000 } + ) })) + assert.deepEqual( + mockDistpatch.getCall(0).args, + [{ type: BASIC_GAS_ESTIMATE_LOADING_STARTED} ] + ) + assert.deepEqual( + global.fetch.getCall(0).args, + [ + 'https://dev.blockscale.net/api/gasexpress.json', + { + 'headers': {}, + 'referrer': 'https://dev.blockscale.net/api/', + 'referrerPolicy': 'no-referrer-when-downgrade', + 'body': null, + 'method': 'GET', + 'mode': 'cors', + }, + ] + ) + + assert.deepEqual( + mockDistpatch.getCall(1).args, + [{ type: SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED, value: 2000000 } ] + ) + + assert.deepEqual( + mockDistpatch.getCall(2).args, + [{ + type: SET_BASIC_GAS_ESTIMATE_DATA, + value: { + average: 20, + blockTime: 'mockBlock_time', + blockNum: 'mockBlockNum', + fast: 30, + fastest: 40, + safeLow: 10, + }, + }] + ) + assert.deepEqual( + mockDistpatch.getCall(3).args, + [{ type: BASIC_GAS_ESTIMATE_LOADING_FINISHED }] + ) + }) + }) + describe('fetchBasicGasAndTimeEstimates', () => { const mockDistpatch = sinon.spy() it('should call fetch with the expected params', async () => { -- cgit