-
Notifications
You must be signed in to change notification settings - Fork 44
refactor: lend markets query refactoring part 2 #1434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ccc539d
4b2b827
c17a98a
8fdc19b
4c71830
c97a829
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,50 +1,48 @@ | ||
import { useMemo } from 'react' | ||
import useStore from '@/lend/store/useStore' | ||
import { ChainId } from '@/lend/types/lend.types' | ||
import { useConnection } from '@ui-kit/features/connect-wallet' | ||
import { useTokenUsdRates } from '@ui-kit/lib/model/entities/token-usd-rate' | ||
import { FETCHING, PartialQueryResult, READY } from '@ui-kit/lib/queries' | ||
import { useOneWayMarketMapping } from './chain-hooks' | ||
import { useLendMarketMapping } from '../lend-markets' | ||
import { calculateChainTvl } from './tvl' | ||
|
||
/** Todo: we should replace this entire hook with prices API some day, there should be an endpoint available */ | ||
0xAlunara marked this conversation as resolved.
Show resolved
Hide resolved
|
||
export const useTvl = (chainId: ChainId | undefined): PartialQueryResult<number> => { | ||
const marketMapping = useOneWayMarketMapping({ chainId }).data | ||
const { llamaApi: api } = useConnection() | ||
|
||
const marketMapping = useLendMarketMapping({ chainId }) | ||
const markets = useMemo( | ||
() => (api && marketMapping ? Object.values(marketMapping).map(api.getLendMarket) : []), | ||
0xAlunara marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[api, marketMapping], | ||
) | ||
const marketsCollateralMapper = useStore((state) => chainId && state.markets.statsAmmBalancesMapper[chainId]) | ||
const marketsTotalSupplyMapper = useStore((state) => chainId && state.markets.totalLiquidityMapper[chainId]) | ||
const marketsTotalDebtMapper = useStore((state) => chainId && state.markets.statsTotalsMapper[chainId]) | ||
const tokenAddresses = useMemo( | ||
() => | ||
Object.values(marketMapping ?? {}) | ||
// note: include the borrowed tokens here, to be used in `filterSmallMarkets` | ||
.flatMap((market) => [market.borrowed_token, market.collateral_token]) | ||
.map((t) => t.address), | ||
[marketMapping], | ||
() => markets.flatMap((market) => [market.borrowed_token, market.collateral_token]).map((t) => t.address), | ||
[markets], | ||
) | ||
const { data: tokenUsdRates, isError: isUsdRatesError } = useTokenUsdRates({ chainId, tokenAddresses }) | ||
|
||
return useMemo(() => { | ||
if ( | ||
!marketMapping || | ||
!marketsCollateralMapper || | ||
!marketsTotalSupplyMapper || | ||
!marketsTotalDebtMapper || | ||
!tokenUsdRates | ||
) { | ||
if (!marketsCollateralMapper || !marketsTotalSupplyMapper || !marketsTotalDebtMapper || !tokenUsdRates) { | ||
return { ...FETCHING, isError: isUsdRatesError } | ||
} | ||
const data = calculateChainTvl( | ||
marketMapping, | ||
markets, | ||
marketsCollateralMapper, | ||
tokenUsdRates, | ||
marketsTotalDebtMapper, | ||
marketsTotalSupplyMapper, | ||
) | ||
return { ...READY, data } | ||
}, [ | ||
isUsdRatesError, | ||
marketMapping, | ||
marketsCollateralMapper, | ||
marketsTotalSupplyMapper, | ||
marketsTotalDebtMapper, | ||
tokenUsdRates, | ||
markets, | ||
isUsdRatesError, | ||
]) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
export { useChainId } from './chain-info' | ||
export { useTvl } from './chain-tvl' | ||
export { useOneWayMarket, useOneWayMarketMapping } from './chain-hooks' | ||
export { calculateChainTvl } from './tvl' |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,63 @@ | ||||||||
import { useMemo } from 'react' | ||||||||
import { isAddress } from 'viem' | ||||||||
import networks from '@/lend/networks' | ||||||||
import { ChainId } from '@/lend/types/lend.types' | ||||||||
import { fromEntries } from '@curvefi/prices-api/objects.util' | ||||||||
import { useConnection } from '@ui-kit/features/connect-wallet' | ||||||||
import type { Address } from '@ui-kit/utils' | ||||||||
|
||||||||
/** | ||||||||
* Hook to get a mapping of lending market controller address to market name for all lending markets on a specific chain. | ||||||||
* Primarily useful fetching lending markets via URL. | ||||||||
*/ | ||||||||
export const useLendMarketMapping = ({ chainId }: { chainId: ChainId | undefined }) => { | ||||||||
const { llamaApi: api } = useConnection() | ||||||||
const mapping = useMemo( | ||||||||
() => | ||||||||
api?.hydrated && | ||||||||
chainId && | ||||||||
fromEntries( | ||||||||
api.lendMarkets | ||||||||
.getMarketList() | ||||||||
.filter((marketName) => !networks[chainId].hideMarketsInUI[marketName]) | ||||||||
.map((name) => [api.getLendMarket(name).addresses.controller as Address, name]), | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only reason we created the mapping like this was because we wanted to use a query. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not a query, though? We only need the mapping for redirecting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could not reliably add the hydration validation in the query, reactivity for api.hydrated is wonky. But a hook with useMemo and api?.hydrated seemed to work, although you said it's still wonky in some way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we need to properly use the state machine from React. Currently it works only when it re-renders for some other reasons. Changing a field of an object is not supported |
||||||||
), | ||||||||
// Need to specifically watch to api?.hydrated, as simply watching api as a whole won't trigger when hydrated is set to true by `useHydration` | ||||||||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||||||||
[api?.hydrated, chainId], | ||||||||
Comment on lines
+26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please never disable this rule, it's always a bug waiting to happen
Suggested change
|
||||||||
) | ||||||||
|
||||||||
return mapping || undefined | ||||||||
} | ||||||||
|
||||||||
/** | ||||||||
* Hook to get a specific lending market by its id or controller address. | ||||||||
* @param chainId The chain for which to get the lending market of | ||||||||
* @param marketId Lend market id or controller address | ||||||||
* @returns The market instance, if found. | ||||||||
*/ | ||||||||
export const useLendMarket = ({ chainId, marketId }: { chainId: ChainId; marketId: string | Address }) => { | ||||||||
const marketMapping = useLendMarketMapping({ chainId }) | ||||||||
const { llamaApi: api } = useConnection() | ||||||||
|
||||||||
return useMemo(() => { | ||||||||
if (!api) return undefined | ||||||||
|
||||||||
// If markets aren't found they throw an error, but we want to return undefined instead | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't do this, ignoring errors is bad. Why are we trying to retrieve a market if it's not found? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because somebody might enter gibberish into the URL? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd argue that should crash the page with an error (or give a proper 404), instead of being ignored which could lead to the page being left in loading state. |
||||||||
const safeGetMarket = (id: string) => { | ||||||||
try { | ||||||||
return api.getLendMarket(id) | ||||||||
} catch { | ||||||||
return undefined | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
// Try to get the market by name first | ||||||||
if (!isAddress(marketId)) return safeGetMarket(marketId) | ||||||||
|
||||||||
// Try to get by controller address mapping | ||||||||
return marketMapping && marketId in marketMapping && api.hydrated && api.chainId === chainId | ||||||||
? safeGetMarket(marketMapping[marketId]) | ||||||||
: undefined | ||||||||
}, [api, chainId, marketId, marketMapping]) | ||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this effect isn't triggered properly 😿
if you visit a market that doesn't exist, the hydration finishes but the redirect takes an unspecified amount of time to happen (when the page is re-rendered for some other reason)