-
Notifications
You must be signed in to change notification settings - Fork 1
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
Wire up liquidity stat on MarketStats #240
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
}), | ||
); | ||
|
||
return { |
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.
Can we just return the useQuery itself? Then we have access to all the query helper methods from the hook if we need them.
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 is a design decision @DannyDelott has made to reduce the return signature. If we change it here, we'd probably want to change it everywhere we're not returning the useQuery
result.
Given the merge conflicts, I think there may have been some duplication of efforts here. I implemented this in the following PR I believe: However, this PR has better conventions in some places so probably best to fix up the merge conflicts and keep the improvements where you see they fit. |
My bad, I didn't see the second PR, I will slim this one down 👍 |
6a8fbfb
to
967d995
Compare
967d995
to
1916fbe
Compare
|
||
const marketLiquidity = sharePrice * shareReserves - longsOutstanding; | ||
const liquidity = | ||
(sharePrice * shareReserves) / 10n ** 18n - longsOutstanding; |
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.
Can you add a comment here explaining the / 10n ** 18n
bit? I'm wondering if there's a way to capture this in a function somehow, like multiplyBigInt(a, b, decimals)
if that makes sense 🤔
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.
Yeah, I had a function for that in another PR, but in the UI. I'll move it to the package and use it here.
queryKey: ["liquidity", { hyperdriveAddress, publicClient }], | ||
queryFn: () => getLiquidity(hyperdriveAddress as Address, publicClient), | ||
enabled: queryEnabled, | ||
queryKey: ["@hyperdrive/core", "getLiquidity", { hyperdriveAddress }], |
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.
nit: switch to makeQueryKey
, I'll land #248 real quick so you can rebase!
1916fbe
to
5deb4de
Compare
Commits tell the story