Skip to content

Commit

Permalink
Fix finance USD conversion mechanism (#1177)
Browse files Browse the repository at this point in the history
* Fix finance USD conversion mechanism

* Add Jest

* Simplify conversion function

* Add some tests for the conversion utility

Co-authored-by: Pierre Bertet <[email protected]>
  • Loading branch information
Evalir and bpierre authored Jun 25, 2020
1 parent b9a4de1 commit 41ac706
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 79 deletions.
21 changes: 16 additions & 5 deletions apps/finance/app/.babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,24 @@
{
"modules": false,
"useBuiltIns": "entry",
"core-js": 3,
"shippedProposals": true,
"corejs": 3,
"shippedProposals": true
}
],
"@babel/preset-react"
],
"plugins": [
["styled-components", { "displayName": true }],
]
"plugins": [["styled-components", { "displayName": true }]],
"env": {
"test": {
"presets": [
[
"@babel/preset-env",
{
"modules": "commonjs",
"targets": { "node": "current" }
}
]
]
}
}
}
6 changes: 5 additions & 1 deletion apps/finance/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,20 @@
"@babel/preset-env": "^7.10.2",
"@babel/preset-react": "^7.10.1",
"babel-eslint": "^10.0.1",
"babel-jest": "^26.1.0",
"babel-plugin-styled-components": "^1.10.7",
"eslint": "^5.6.0",
"eslint-config-prettier": "^3.1.0",
"eslint-config-standard": "^12.0.0",
"eslint-config-standard-react": "^7.0.2",
"eslint-plugin-import": "^2.8.0",
"eslint-plugin-jest": "^23.17.1",
"eslint-plugin-node": "^7.0.1",
"eslint-plugin-prettier": "^2.7.0",
"eslint-plugin-promise": "^4.0.1",
"eslint-plugin-react": "^7.5.1",
"eslint-plugin-standard": "^4.0.0",
"jest": "^26.1.0",
"parcel-bundler": "^1.12.4",
"parcel-plugin-bundle-visualiser": "^1.2.0",
"prettier": "^1.11.1"
Expand All @@ -49,7 +52,8 @@
"build:script": "parcel build src/script.js --out-dir build/",
"watch:script": "parcel watch src/script.js --out-dir build/ --no-hmr",
"sync-assets": "copy-aragon-ui-assets -n aragon-ui ./build && rsync -rtu ./public/ ./build",
"now-build": "npm run build"
"now-build": "npm run build",
"test": "jest"
},
"browserslist": [
">2%",
Expand Down
92 changes: 19 additions & 73 deletions apps/finance/app/src/components/Balances.js
Original file line number Diff line number Diff line change
@@ -1,67 +1,9 @@
import React, { useEffect, useMemo, useRef, useState } from 'react'
import React, { useMemo } from 'react'
import BN from 'bn.js'
import { Box, GU, textStyle, useTheme, useLayout } from '@aragon/ui'
import BalanceToken from './BalanceToken'

const CONVERT_API_RETRY_DELAY = 2 * 1000
const CONVERT_API_RETRY_DELAY_MAX = 60 * 1000

function convertRatesUrl(symbolsQuery) {
return `https://min-api.cryptocompare.com/data/price?fsym=USD&tsyms=${symbolsQuery}`
}

function useConvertRates(symbols) {
const [rates, setRates] = useState({})
const retryDelay = useRef(CONVERT_API_RETRY_DELAY)

const symbolsQuery = symbols.join(',')

useEffect(() => {
let cancelled = false
let retryTimer = null

const update = async () => {
if (!symbolsQuery) {
setRates({})
return
}

try {
const response = await fetch(convertRatesUrl(symbolsQuery))
const rates = await response.json()
if (!cancelled) {
setRates(rates)
retryDelay.current = CONVERT_API_RETRY_DELAY
}
} catch (err) {
// The !cancelled check is needed in case:
// 1. The fetch() request is ongoing.
// 2. The component gets unmounted.
// 3. An error gets thrown.
//
// Assuming the fetch() request keeps throwing, it would create new
// requests even though the useEffect() got cancelled.
if (!cancelled) {
// Add more delay after every failed attempt
retryDelay.current = Math.min(
CONVERT_API_RETRY_DELAY_MAX,
retryDelay.current * 1.2
)
retryTimer = setTimeout(update, retryDelay.current)
}
}
}
update()

return () => {
cancelled = true
clearTimeout(retryTimer)
retryDelay.current = CONVERT_API_RETRY_DELAY
}
}, [symbolsQuery])

return rates
}
import { getConvertedAmount } from '../lib/conversion-utils'
import { useConvertRates } from './useConvertRates'

// Prepare the balances for the BalanceToken component
function useBalanceItems(balances) {
Expand All @@ -72,18 +14,22 @@ function useBalanceItems(balances) {
const convertRates = useConvertRates(verifiedSymbols)

const balanceItems = useMemo(() => {
return balances.map(({ address, amount, decimals, symbol, verified }) => ({
address,
amount,
convertedAmount: convertRates[symbol]
? amount.divn(convertRates[symbol])
: new BN(-1),
decimals,
symbol,
verified,
}))
}, [balances, convertRates])

return balances.map(
({ address, amount, decimals, symbol, verified }) => {
return {
address,
amount,
convertedAmount: convertRates[symbol]
? getConvertedAmount(amount, convertRates[symbol], decimals)
: new BN('-1'),
decimals,
symbol,
verified,
}
},
[balances, convertRates]
)
})
return balanceItems
}

Expand Down
61 changes: 61 additions & 0 deletions apps/finance/app/src/components/useConvertRates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { useEffect, useState, useRef } from 'react'

const CONVERT_API_RETRY_DELAY = 2 * 1000
const CONVERT_API_RETRY_DELAY_MAX = 60 * 1000

function convertRatesUrl(symbolsQuery) {
return `https://min-api.cryptocompare.com/data/price?fsym=USD&tsyms=${symbolsQuery}`
}

export function useConvertRates(symbols) {
const [rates, setRates] = useState({})
const retryDelay = useRef(CONVERT_API_RETRY_DELAY)

const symbolsQuery = symbols.join(',')

useEffect(() => {
let cancelled = false
let retryTimer = null

const update = async () => {
if (!symbolsQuery) {
setRates({})
return
}

try {
const response = await fetch(convertRatesUrl(symbolsQuery))
const rates = await response.json()
if (!cancelled) {
setRates(rates)
retryDelay.current = CONVERT_API_RETRY_DELAY
}
} catch (err) {
// The !cancelled check is needed in case:
// 1. The fetch() request is ongoing.
// 2. The component gets unmounted.
// 3. An error gets thrown.
//
// Assuming the fetch() request keeps throwing, it would create new
// requests even though the useEffect() got cancelled.
if (!cancelled) {
// Add more delay after every failed attempt
retryDelay.current = Math.min(
CONVERT_API_RETRY_DELAY_MAX,
retryDelay.current * 1.2
)
retryTimer = setTimeout(update, retryDelay.current)
}
}
}
update()

return () => {
cancelled = true
clearTimeout(retryTimer)
retryDelay.current = CONVERT_API_RETRY_DELAY
}
}, [symbolsQuery])

return rates
}
16 changes: 16 additions & 0 deletions apps/finance/app/src/lib/conversion-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import BN from 'bn.js'

export function getConvertedAmount(amount, convertRate) {
const [whole = '', dec = ''] = convertRate.toString().split('.')
// Remove any trailing zeros from the decimal part
const parsedDec = dec.replace(/0*$/, '')
// Construct the final rate, and remove any leading zeros
const rate = `${whole}${parsedDec}`.replace(/^0*/, '')

// Number of decimals to shift the amount of the token passed in,
// resulting from converting the rate to a number without any decimal
// places
const carryAmount = new BN(parsedDec.length.toString())

return amount.mul(new BN('10').pow(carryAmount)).div(new BN(rate))
}
29 changes: 29 additions & 0 deletions apps/finance/app/src/lib/conversion-utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import BN from 'bn.js'
import { getConvertedAmount } from './conversion-utils'

const ONE_ETH = new BN('10').pow(new BN('18'))

describe('getConvertedAmount tests', () => {
test('Converts amounts correctly', () => {
expect(getConvertedAmount(new BN('1'), 1).toString()).toEqual('1')
expect(getConvertedAmount(new BN(ONE_ETH), 1).toString()).toEqual(
ONE_ETH.toString()
)
expect(getConvertedAmount(new BN('1'), 0.5).toString()).toEqual('2')
expect(getConvertedAmount(new BN('1'), 0.25).toString()).toEqual('4')
expect(getConvertedAmount(new BN('1'), 0.125).toString()).toEqual('8')

expect(getConvertedAmount(new BN('100'), 50).toString()).toEqual('2')
// This is the exact case that broke the previous implementation,
// which is AAVE's amount of WBTC + the exchange rate at a certain
// hour on 2020-06-24
expect(
getConvertedAmount(new BN('1145054'), 0.00009248).toString()
).toEqual('12381639273')
})

test('Throws on invalid inputs', () => {
expect(() => getConvertedAmount(new BN('1'), 0)).toThrow()
expect(() => getConvertedAmount('1000', 0)).toThrow()
})
})

0 comments on commit 41ac706

Please sign in to comment.