From 444d8dd51af23c9c7a12c62ec2affdc7968ffc8a Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 9 Mar 2021 13:49:27 -0600 Subject: [PATCH] Fix #10081 - Use fetchWithCache to retrieve and store basic gas estimates (#10384) --- ui/app/ducks/gas/gas-duck.test.js | 167 +++--------------------------- ui/app/ducks/gas/gas.duck.js | 79 ++++---------- 2 files changed, 35 insertions(+), 211 deletions(-) diff --git a/ui/app/ducks/gas/gas-duck.test.js b/ui/app/ducks/gas/gas-duck.test.js index b1c8447d9..b66b1dadd 100644 --- a/ui/app/ducks/gas/gas-duck.test.js +++ b/ui/app/ducks/gas/gas-duck.test.js @@ -1,48 +1,26 @@ import assert from 'assert'; import nock from 'nock'; import sinon from 'sinon'; -import proxyquire from 'proxyquire'; import BN from 'bn.js'; -const fakeStorage = {}; - -const GasDuck = proxyquire('./gas.duck.js', { - '../../../lib/storage-helpers': fakeStorage, -}); - -const { +import GasDuck, { basicGasEstimatesLoadingStarted, basicGasEstimatesLoadingFinished, setBasicGasEstimateData, setCustomGasPrice, setCustomGasLimit, - resetCustomGasState, fetchBasicGasEstimates, -} = GasDuck; -const GasReducer = GasDuck.default; +} from './gas.duck'; + +const mockGasPriceApiResponse = { + SafeGasPrice: 10, + ProposeGasPrice: 20, + FastGasPrice: 30, +}; + +const GasReducer = GasDuck; describe('Gas Duck', function () { - let tempDateNow; - const mockGasPriceApiResponse = { - SafeGasPrice: 10, - ProposeGasPrice: 20, - FastGasPrice: 30, - }; - - beforeEach(function () { - tempDateNow = global.Date.now; - - fakeStorage.getStorageItem = sinon.stub(); - fakeStorage.setStorageItem = sinon.spy(); - global.Date.now = () => 2000000; - }); - - afterEach(function () { - sinon.restore(); - - global.Date.now = tempDateNow; - }); - const mockState = { mockProp: 123, }; @@ -57,7 +35,6 @@ describe('Gas Duck', function () { safeLow: null, }, basicEstimateIsLoading: true, - basicPriceEstimatesLastRetrieved: 0, }; const providerState = { @@ -73,13 +50,10 @@ describe('Gas Duck', function () { 'metamask/gas/BASIC_GAS_ESTIMATE_LOADING_FINISHED'; const BASIC_GAS_ESTIMATE_LOADING_STARTED = 'metamask/gas/BASIC_GAS_ESTIMATE_LOADING_STARTED'; - const RESET_CUSTOM_GAS_STATE = 'metamask/gas/RESET_CUSTOM_GAS_STATE'; const SET_BASIC_GAS_ESTIMATE_DATA = 'metamask/gas/SET_BASIC_GAS_ESTIMATE_DATA'; const SET_CUSTOM_GAS_LIMIT = 'metamask/gas/SET_CUSTOM_GAS_LIMIT'; const SET_CUSTOM_GAS_PRICE = 'metamask/gas/SET_CUSTOM_GAS_PRICE'; - const SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED = - 'metamask/gas/SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED'; describe('GasReducer()', function () { it('should initialize state', function () { @@ -139,13 +113,6 @@ describe('Gas Duck', function () { { customData: { limit: 9876 }, ...mockState }, ); }); - - it('should return the initial state in response to a RESET_CUSTOM_GAS_STATE action', function () { - assert.deepStrictEqual( - GasReducer(mockState, { type: RESET_CUSTOM_GAS_STATE }), - initState, - ); - }); }); describe('basicGasEstimatesLoadingStarted', function () { @@ -167,7 +134,6 @@ describe('Gas Duck', function () { describe('fetchBasicGasEstimates', function () { it('should call fetch with the expected params', async function () { const mockDistpatch = sinon.spy(); - const windowFetchSpy = sinon.spy(window, 'fetch'); nock('https://api.metaswap.codefi.network') @@ -175,32 +141,21 @@ describe('Gas Duck', function () { .reply(200, mockGasPriceApiResponse); await fetchBasicGasEstimates()(mockDistpatch, () => ({ - gas: { ...initState, basicPriceAEstimatesLastRetrieved: 1000000 }, + gas: { ...initState }, metamask: { provider: { ...providerState } }, })); assert.deepStrictEqual(mockDistpatch.getCall(0).args, [ { type: BASIC_GAS_ESTIMATE_LOADING_STARTED }, ]); + assert.ok( windowFetchSpy .getCall(0) .args[0].startsWith('https://api.metaswap.codefi.network/gasPrices'), 'should fetch metaswap /gasPrices', ); - assert.deepStrictEqual(mockDistpatch.getCall(1).args, [ - { type: SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED, value: 2000000 }, - ]); + assert.deepStrictEqual(mockDistpatch.getCall(2).args, [ - { - type: SET_BASIC_GAS_ESTIMATE_DATA, - value: { - average: 20, - fast: 30, - safeLow: 10, - }, - }, - ]); - assert.deepStrictEqual(mockDistpatch.getCall(3).args, [ { type: BASIC_GAS_ESTIMATE_LOADING_FINISHED }, ]); }); @@ -209,7 +164,7 @@ describe('Gas Duck', function () { global.eth = { gasPrice: sinon.fake.returns(new BN(48199313, 10)) }; const mockDistpatch = sinon.spy(); - const providerStateForTestNetwrok = { + const providerStateForTestNetwork = { chainId: '0x5', nickname: '', rpcPrefs: {}, @@ -219,8 +174,8 @@ describe('Gas Duck', function () { }; await fetchBasicGasEstimates()(mockDistpatch, () => ({ - gas: { ...initState, basicPriceAEstimatesLastRetrieved: 1000000 }, - metamask: { provider: { ...providerStateForTestNetwrok } }, + gas: { ...initState }, + metamask: { provider: { ...providerStateForTestNetwork } }, })); assert.deepStrictEqual(mockDistpatch.getCall(0).args, [ { type: BASIC_GAS_ESTIMATE_LOADING_STARTED }, @@ -237,88 +192,6 @@ describe('Gas Duck', function () { { type: BASIC_GAS_ESTIMATE_LOADING_FINISHED }, ]); }); - - it('should fetch recently retrieved estimates from storage', async function () { - const mockDistpatch = sinon.spy(); - - const windowFetchSpy = sinon.spy(window, 'fetch'); - - fakeStorage.getStorageItem - .withArgs('BASIC_PRICE_ESTIMATES_LAST_RETRIEVED') - .returns(2000000 - 1); // one second ago from "now" - fakeStorage.getStorageItem.withArgs('BASIC_PRICE_ESTIMATES').returns({ - average: 25, - fast: 35, - safeLow: 15, - }); - - await fetchBasicGasEstimates()(mockDistpatch, () => ({ - gas: { ...initState }, - metamask: { provider: { ...providerState } }, - })); - assert.deepStrictEqual(mockDistpatch.getCall(0).args, [ - { type: BASIC_GAS_ESTIMATE_LOADING_STARTED }, - ]); - assert.ok(windowFetchSpy.notCalled); - assert.deepStrictEqual(mockDistpatch.getCall(1).args, [ - { - type: SET_BASIC_GAS_ESTIMATE_DATA, - value: { - average: 25, - fast: 35, - safeLow: 15, - }, - }, - ]); - assert.deepStrictEqual(mockDistpatch.getCall(2).args, [ - { type: BASIC_GAS_ESTIMATE_LOADING_FINISHED }, - ]); - }); - - it('should fallback to network if retrieving estimates from storage fails', async function () { - const mockDistpatch = sinon.spy(); - - const windowFetchSpy = sinon.spy(window, 'fetch'); - - nock('https://api.metaswap.codefi.network') - .get('/gasPrices') - .reply(200, mockGasPriceApiResponse); - - fakeStorage.getStorageItem - .withArgs('BASIC_PRICE_ESTIMATES_LAST_RETRIEVED') - .returns(2000000 - 1); // one second ago from "now" - - await fetchBasicGasEstimates()(mockDistpatch, () => ({ - gas: { ...initState }, - metamask: { provider: { ...providerState } }, - })); - assert.deepStrictEqual(mockDistpatch.getCall(0).args, [ - { type: BASIC_GAS_ESTIMATE_LOADING_STARTED }, - ]); - assert.ok( - windowFetchSpy - .getCall(0) - .args[0].startsWith('https://api.metaswap.codefi.network/gasPrices'), - 'should fetch metaswap /gasPrices', - ); - - assert.deepStrictEqual(mockDistpatch.getCall(1).args, [ - { type: SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED, value: 2000000 }, - ]); - assert.deepStrictEqual(mockDistpatch.getCall(2).args, [ - { - type: SET_BASIC_GAS_ESTIMATE_DATA, - value: { - safeLow: 10, - average: 20, - fast: 30, - }, - }, - ]); - assert.deepStrictEqual(mockDistpatch.getCall(3).args, [ - { type: BASIC_GAS_ESTIMATE_LOADING_FINISHED }, - ]); - }); }); describe('setBasicGasEstimateData', function () { @@ -347,12 +220,4 @@ describe('Gas Duck', function () { }); }); }); - - describe('resetCustomGasState', function () { - it('should create the correct action', function () { - assert.deepStrictEqual(resetCustomGasState(), { - type: RESET_CUSTOM_GAS_STATE, - }); - }); - }); }); diff --git a/ui/app/ducks/gas/gas.duck.js b/ui/app/ducks/gas/gas.duck.js index d97dd70c4..5785b2cf3 100644 --- a/ui/app/ducks/gas/gas.duck.js +++ b/ui/app/ducks/gas/gas.duck.js @@ -5,23 +5,18 @@ import { decGWEIToHexWEI, getValueFromWeiHex, } from '../../helpers/utils/conversions.util'; -import getFetchWithTimeout from '../../../../shared/modules/fetch-with-timeout'; import { getIsMainnet, getCurrentChainId } from '../../selectors'; - -const fetchWithTimeout = getFetchWithTimeout(30000); +import fetchWithCache from '../../helpers/utils/fetch-with-cache'; // Actions 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'; -const RESET_CUSTOM_GAS_STATE = 'metamask/gas/RESET_CUSTOM_GAS_STATE'; const RESET_CUSTOM_DATA = 'metamask/gas/RESET_CUSTOM_DATA'; const SET_BASIC_GAS_ESTIMATE_DATA = 'metamask/gas/SET_BASIC_GAS_ESTIMATE_DATA'; const SET_CUSTOM_GAS_LIMIT = 'metamask/gas/SET_CUSTOM_GAS_LIMIT'; const SET_CUSTOM_GAS_PRICE = 'metamask/gas/SET_CUSTOM_GAS_PRICE'; -const SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED = - 'metamask/gas/SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED'; const initState = { customData: { @@ -34,7 +29,6 @@ const initState = { fast: null, }, basicEstimateIsLoading: true, - basicPriceEstimatesLastRetrieved: 0, }; // Reducer @@ -71,18 +65,11 @@ export default function reducer(state = initState, action) { limit: action.value, }, }; - case SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED: - return { - ...state, - basicPriceEstimatesLastRetrieved: action.value, - }; case RESET_CUSTOM_DATA: return { ...state, customData: cloneDeep(initState.customData), }; - case RESET_CUSTOM_GAS_STATE: - return cloneDeep(initState); default: return state; } @@ -103,40 +90,27 @@ export function basicGasEstimatesLoadingFinished() { async function basicGasPriceQuery() { const url = `https://api.metaswap.codefi.network/gasPrices`; - return await fetchWithTimeout(url, { - headers: {}, - referrer: 'https://api.metaswap.codefi.network/gasPrices', - referrerPolicy: 'no-referrer-when-downgrade', - body: null, - method: 'GET', - mode: 'cors', - }); + return await fetchWithCache( + url, + { + referrer: url, + referrerPolicy: 'no-referrer-when-downgrade', + method: 'GET', + mode: 'cors', + }, + { cacheRefreshTime: 75000 }, + ); } export function fetchBasicGasEstimates() { return async (dispatch, getState) => { const isMainnet = getIsMainnet(getState()); - const { basicPriceEstimatesLastRetrieved } = getState().gas; - - const timeLastRetrieved = - basicPriceEstimatesLastRetrieved || - (await getStorageItem('BASIC_PRICE_ESTIMATES_LAST_RETRIEVED')) || - 0; dispatch(basicGasEstimatesLoadingStarted()); let basicEstimates; if (isMainnet || process.env.IN_TEST) { - if (Date.now() - timeLastRetrieved > 75000) { - basicEstimates = await fetchExternalBasicGasEstimates(dispatch); - } else { - const cachedBasicEstimates = await getStorageItem( - 'BASIC_PRICE_ESTIMATES', - ); - basicEstimates = - cachedBasicEstimates || - (await fetchExternalBasicGasEstimates(dispatch)); - } + basicEstimates = await fetchExternalBasicGasEstimates(); } else { basicEstimates = await fetchEthGasPriceEstimates(getState()); } @@ -148,10 +122,12 @@ export function fetchBasicGasEstimates() { }; } -async function fetchExternalBasicGasEstimates(dispatch) { - const response = await basicGasPriceQuery(); - - const { SafeGasPrice, ProposeGasPrice, FastGasPrice } = await response.json(); +async function fetchExternalBasicGasEstimates() { + const { + SafeGasPrice, + ProposeGasPrice, + FastGasPrice, + } = await basicGasPriceQuery(); const [safeLow, average, fast] = [ SafeGasPrice, @@ -165,12 +141,6 @@ async function fetchExternalBasicGasEstimates(dispatch) { fast, }; - const timeRetrieved = Date.now(); - await Promise.all([ - setStorageItem('BASIC_PRICE_ESTIMATES', basicEstimates), - setStorageItem('BASIC_PRICE_ESTIMATES_LAST_RETRIEVED', timeRetrieved), - ]); - dispatch(setBasicPriceEstimatesLastRetrieved(timeRetrieved)); return basicEstimates; } @@ -209,7 +179,7 @@ async function fetchEthGasPriceEstimates(state) { export function setCustomGasPriceForRetry(newPrice) { return async (dispatch) => { if (newPrice === '0x0') { - const { fast } = await getStorageItem('BASIC_PRICE_ESTIMATES'); + const { fast } = await fetchExternalBasicGasEstimates(); dispatch(setCustomGasPrice(decGWEIToHexWEI(fast))); } else { dispatch(setCustomGasPrice(newPrice)); @@ -238,17 +208,6 @@ export function setCustomGasLimit(newLimit) { }; } -export function setBasicPriceEstimatesLastRetrieved(retrievalTime) { - return { - type: SET_BASIC_PRICE_ESTIMATES_LAST_RETRIEVED, - value: retrievalTime, - }; -} - -export function resetCustomGasState() { - return { type: RESET_CUSTOM_GAS_STATE }; -} - export function resetCustomData() { return { type: RESET_CUSTOM_DATA }; }