Fix logging in gas price service (#583)

2021-07-05 13:50:18 +03:00 · 2021-07-05 13:50:18 +03:00 · c8eb0f1ed8
commit c8eb0f1ed8
parent 3e5e50c06e
6 changed files with 59 additions and 89 deletions
--- a/commons/package.json
+++ b/commons/package.json
@ -9,7 +9,8 @@
  },
  "dependencies": {
    "gas-price-oracle": "^0.1.5",
-    "web3-utils": "^1.3.0"
+    "web3-utils": "^1.3.0",
+    "node-fetch": "^2.1.2"
  },
  "devDependencies": {
    "bn-chai": "^1.0.1",
--- a/commons/utils.js
+++ b/commons/utils.js
@ -1,5 +1,6 @@
 const { toWei, toBN, BN } = require('web3-utils')
 const { GasPriceOracle } = require('gas-price-oracle')
+const fetch = require('node-fetch')
 const { BRIDGE_MODES } = require('./constants')
 const { REWARDABLE_VALIDATORS_ABI } = require('./abis')

@ -178,17 +179,16 @@ const normalizeGasPrice = (oracleGasPrice, factor, limits = null) => {
  return toBN(toWei(gasPrice.toFixed(2).toString(), 'gwei'))
 }

-// fetchFn has to be supplied (instead of just url to oracle),
-// because this utility function is shared between Browser and Node,
-// we use built-in 'fetch' on browser side, and `node-fetch` package in Node.
-const gasPriceFromSupplier = async (fetchFn, options = {}) => {
+const gasPriceFromSupplier = async (url, options = {}) => {
  try {
    let json
-    if (fetchFn) {
-      const response = await fetchFn()
+    if (url === 'gas-price-oracle') {
+      json = await gasPriceOracle.fetchGasPricesOffChain()
+    } else if (url) {
+      const response = await fetch(url, { timeout: 2000 })
      json = await response.json()
    } else {
-      json = await gasPriceOracle.fetchGasPricesOffChain()
+      return null
    }
    const oracleGasPrice = json[options.speedType]

--- a/e2e-commons/components-envs/oracle-amb.env
+++ b/e2e-commons/components-envs/oracle-amb.env
@ -8,12 +8,12 @@ COMMON_HOME_BRIDGE_ADDRESS=0x8397be90BCF57b0B71219f555Fe121b22e5a994C
 COMMON_FOREIGN_BRIDGE_ADDRESS=0x1feB40aD9420b186F019A717c37f5546165d411E
 ORACLE_VALIDATOR_ADDRESS=0xaaB52d66283F7A1D5978bcFcB55721ACB467384b
 ORACLE_VALIDATOR_ADDRESS_PRIVATE_KEY=8e829f695aed89a154550f30262f1529582cc49dc30eff74a6b491359e0230f9
-COMMON_HOME_GAS_PRICE_SUPPLIER_URL=https://gasprice.poa.network/
+COMMON_HOME_GAS_PRICE_SUPPLIER_URL=
 COMMON_HOME_GAS_PRICE_SPEED_TYPE=standard
 COMMON_HOME_GAS_PRICE_FALLBACK=1000000000
 ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL=600000
 COMMON_HOME_GAS_PRICE_FACTOR=1
-COMMON_FOREIGN_GAS_PRICE_SUPPLIER_URL=https://gasprice.poa.network/
+COMMON_FOREIGN_GAS_PRICE_SUPPLIER_URL=
 COMMON_FOREIGN_GAS_PRICE_SPEED_TYPE=standard
 COMMON_FOREIGN_GAS_PRICE_FALLBACK=10000000000
 ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL=600000
--- a/monitor/validators.js
+++ b/monitor/validators.js
@ -1,6 +1,5 @@
 require('dotenv').config()
 const Web3Utils = require('web3').utils
-const fetch = require('node-fetch')
 const logger = require('./logger')('validators')
 const { getBridgeABIs, BRIDGE_VALIDATORS_ABI, gasPriceFromSupplier } = require('../commons')
 const { web3Home, web3Foreign, getHomeBlockNumber, getForeignBlockNumber } = require('./utils/web3')
@ -81,7 +80,7 @@ async function main(bridgeMode) {
  if (MONITOR_VALIDATOR_HOME_TX_LIMIT) {
    logger.debug('calling home getGasPrices')
    homeGasPrice =
-      (await gasPriceFromSupplier(() => fetch(COMMON_HOME_GAS_PRICE_SUPPLIER_URL), homeGasPriceSupplierOpts)) ||
+      (await gasPriceFromSupplier(COMMON_HOME_GAS_PRICE_SUPPLIER_URL, homeGasPriceSupplierOpts)) ||
      Web3Utils.toBN(COMMON_HOME_GAS_PRICE_FALLBACK)
    homeGasPriceGwei = Web3Utils.fromWei(homeGasPrice.toString(), 'gwei')
    homeTxCost = homeGasPrice.mul(Web3Utils.toBN(MONITOR_VALIDATOR_HOME_TX_LIMIT))
@ -93,13 +92,9 @@ async function main(bridgeMode) {

  if (MONITOR_VALIDATOR_FOREIGN_TX_LIMIT) {
    logger.debug('calling foreign getGasPrices')
-    const fetchFn =
-      COMMON_FOREIGN_GAS_PRICE_SUPPLIER_URL === 'gas-price-oracle'
-        ? null
-        : () => fetch(COMMON_FOREIGN_GAS_PRICE_SUPPLIER_URL)

    foreignGasPrice =
-      (await gasPriceFromSupplier(fetchFn, foreignGasPriceSupplierOpts)) ||
+      (await gasPriceFromSupplier(COMMON_FOREIGN_GAS_PRICE_SUPPLIER_URL, foreignGasPriceSupplierOpts)) ||
      Web3Utils.toBN(COMMON_FOREIGN_GAS_PRICE_FALLBACK)
    foreignGasPriceGwei = Web3Utils.fromWei(foreignGasPrice.toString(), 'gwei')
    foreignTxCost = foreignGasPrice.mul(Web3Utils.toBN(MONITOR_VALIDATOR_FOREIGN_TX_LIMIT))
--- a/oracle/src/services/gasPrice.js
+++ b/oracle/src/services/gasPrice.js
@ -1,5 +1,4 @@
 require('../../env')
-const fetch = require('node-fetch')
 const { home, foreign } = require('../../config/base.config')
 const logger = require('../services/logger').child({
  module: 'gasPrice'
@ -25,11 +24,11 @@ let cachedGasPrice = null

 let fetchGasPriceInterval = null

-const fetchGasPrice = async (speedType, factor, bridgeContract, gasPriceSupplierFetchFn) => {
+const fetchGasPrice = async (speedType, factor, bridgeContract, gasPriceSupplierUrl) => {
  const contractOptions = { logger }
  const supplierOptions = { speedType, factor, limits: GAS_PRICE_BOUNDARIES, logger }
  cachedGasPrice =
-    (await gasPriceFromSupplier(gasPriceSupplierFetchFn, supplierOptions)) ||
+    (await gasPriceFromSupplier(gasPriceSupplierUrl, supplierOptions)) ||
    (await gasPriceFromContract(bridgeContract, contractOptions)) ||
    cachedGasPrice
  return cachedGasPrice
@ -63,16 +62,15 @@ async function start(chainId, fetchOnce) {
    throw new Error(`Unrecognized chainId '${chainId}'`)
  }

-  let fetchFn = null
-  if (gasPriceSupplierUrl !== 'gas-price-oracle') {
-    fetchFn = () => fetch(gasPriceSupplierUrl, { timeout: 2000 })
+  if (!gasPriceSupplierUrl) {
+    logger.warn({ chainId }, 'Gas price API is not configured, will fallback to the contract-supplied gas price')
  }

  if (fetchOnce) {
-    await fetchGasPrice(speedType, factor, contract, fetchFn)
+    await fetchGasPrice(speedType, factor, contract, gasPriceSupplierUrl)
  } else {
    fetchGasPriceInterval = await setIntervalAndRun(
-      () => fetchGasPrice(speedType, factor, contract, fetchFn),
+      () => fetchGasPrice(speedType, factor, contract, gasPriceSupplierUrl),
      updateInterval
    )
  }
--- a/oracle/test/gasPrice.test.js
+++ b/oracle/test/gasPrice.test.js
@ -3,77 +3,75 @@ const { expect } = require('chai')
 const proxyquire = require('proxyquire').noPreserveCache()
 const { DEFAULT_UPDATE_INTERVAL } = require('../src/utils/constants')

+const utils = { setIntervalAndRun: sinon.spy() }
+const fetchStub = () => ({
+  json: () => ({
+    standard: '103'
+  })
+})
+const fakeLogger = { error: sinon.spy(), warn: sinon.spy(), child: () => fakeLogger }
+fetchStub['@global'] = true
+const gasPriceDefault = proxyquire('../src/services/gasPrice', {
+  '../utils/utils': utils,
+  'node-fetch': fetchStub,
+  '../services/logger': { child: () => fakeLogger }
+})
+process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL = 15000
+process.env.ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL = 30000
+process.env.COMMON_HOME_GAS_PRICE_FALLBACK = '101000000000'
+const gasPrice = proxyquire('../src/services/gasPrice', {
+  '../utils/utils': utils,
+  'node-fetch': fetchStub,
+  '../services/logger': { child: () => fakeLogger }
+})
+
 describe('gasPrice', () => {
-  describe('start', () => {
-    const utils = { setIntervalAndRun: sinon.spy() }
  beforeEach(() => {
    utils.setIntervalAndRun.resetHistory()
+    fakeLogger.error.resetHistory()
+    fakeLogger.warn.resetHistory()
  })
-    it('should call setIntervalAndRun with ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL interval value on Home', async () => {
-      // given
-      process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL = 15000
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })

+  describe('start', () => {
+    it('should call setIntervalAndRun with ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL interval value on Home', async () => {
      // when
      await gasPrice.start('home')

      // then
-      expect(process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL).to.equal('15000')
-      expect(process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL).to.not.equal(DEFAULT_UPDATE_INTERVAL.toString())
      expect(utils.setIntervalAndRun.args[0][1]).to.equal(process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL.toString())
    })
    it('should call setIntervalAndRun with ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL interval value on Foreign', async () => {
-      // given
-      process.env.ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL = 15000
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
-
      // when
      await gasPrice.start('foreign')

      // then
-      expect(process.env.ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL).to.equal('15000')
-      expect(process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL).to.not.equal(DEFAULT_UPDATE_INTERVAL.toString())
      expect(utils.setIntervalAndRun.args[0][1]).to.equal(
        process.env.ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL.toString()
      )
    })
    it('should call setIntervalAndRun with default interval value on Home', async () => {
-      // given
-      delete process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
-
      // when
-      await gasPrice.start('home')
+      await gasPriceDefault.start('home')

      // then
-      expect(process.env.ORACLE_HOME_GAS_PRICE_UPDATE_INTERVAL).to.equal(undefined)
      expect(utils.setIntervalAndRun.args[0][1]).to.equal(DEFAULT_UPDATE_INTERVAL)
    })
    it('should call setIntervalAndRun with default interval value on Foreign', async () => {
-      // given
-      delete process.env.ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
-
      // when
-      await gasPrice.start('foreign')
+      await gasPriceDefault.start('foreign')

      // then
-      expect(process.env.ORACLE_FOREIGN_GAS_PRICE_UPDATE_INTERVAL).to.equal(undefined)
      expect(utils.setIntervalAndRun.args[0][1]).to.equal(DEFAULT_UPDATE_INTERVAL)
    })
  })

  describe('fetching gas price', () => {
-    const utils = { setIntervalAndRun: () => {} }
-
    it('should fall back to default if contract and supplier are not working', async () => {
      // given
-      process.env.COMMON_HOME_GAS_PRICE_FALLBACK = '101000000000'
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
      await gasPrice.start('home')

      // when
-      await gasPrice.fetchGasPrice('standard', 1, null, () => null)
+      await gasPrice.fetchGasPrice('standard', 1, null, null)

      // then
      expect(gasPrice.getPrice()).to.equal('101000000000')
@ -81,18 +79,10 @@ describe('gasPrice', () => {

    it('should fetch gas from supplier', async () => {
      // given
-      process.env.COMMON_HOME_GAS_PRICE_FALLBACK = '101000000000'
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
      await gasPrice.start('home')

-      const gasPriceSupplierFetchFn = () => ({
-        json: () => ({
-          standard: '103'
-        })
-      })
-
      // when
-      await gasPrice.fetchGasPrice('standard', 1, null, gasPriceSupplierFetchFn)
+      await gasPrice.fetchGasPrice('standard', 1, null, 'url')

      // then
      expect(gasPrice.getPrice().toString()).to.equal('103000000000')
@ -100,8 +90,6 @@ describe('gasPrice', () => {

    it('should fetch gas from contract', async () => {
      // given
-      process.env.COMMON_HOME_GAS_PRICE_FALLBACK = '101000000000'
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
      await gasPrice.start('home')

      const bridgeContractMock = {
@ -113,7 +101,7 @@ describe('gasPrice', () => {
      }

      // when
-      await gasPrice.fetchGasPrice('standard', 1, bridgeContractMock, () => {})
+      await gasPrice.fetchGasPrice('standard', 1, bridgeContractMock, null)

      // then
      expect(gasPrice.getPrice().toString()).to.equal('102000000000')
@ -121,8 +109,6 @@ describe('gasPrice', () => {

    it('should fetch the gas price from the oracle first', async () => {
      // given
-      process.env.COMMON_HOME_GAS_PRICE_FALLBACK = '101000000000'
-      const gasPrice = proxyquire('../src/services/gasPrice', { '../utils/utils': utils })
      await gasPrice.start('home')

      const bridgeContractMock = {
@ -133,33 +119,23 @@ describe('gasPrice', () => {
        }
      }

-      const gasPriceSupplierFetchFn = () => ({
-        json: () => ({
-          standard: '103'
-        })
-      })
-
      // when
-      await gasPrice.fetchGasPrice('standard', 1, bridgeContractMock, gasPriceSupplierFetchFn)
+      await gasPrice.fetchGasPrice('standard', 1, bridgeContractMock, 'url')

      // then
      expect(gasPrice.getPrice().toString()).to.equal('103000000000')
    })

-    it('log errors using the logger', async () => {
+    it('log error using the logger', async () => {
      // given
-      const fakeLogger = { error: sinon.spy() }
-      const gasPrice = proxyquire('../src/services/gasPrice', {
-        '../utils/utils': utils,
-        '../services/logger': { child: () => fakeLogger }
-      })
      await gasPrice.start('home')

      // when
-      await gasPrice.fetchGasPrice('standard', 1, null, () => {})
+      await gasPrice.fetchGasPrice('standard', 1, null, null)

      // then
-      expect(fakeLogger.error.calledTwice).to.equal(true) // two errors
+      expect(fakeLogger.warn.calledOnce).to.equal(true) // one warning
+      expect(fakeLogger.error.calledOnce).to.equal(true) // one error
    })
  })
 })