-
Notifications
You must be signed in to change notification settings - Fork 2
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
Luke/dpe 1861 jito api changes #33
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
loaded: mSolMetrics.loaded, | ||
}; | ||
case JITO_SOL.symbol: | ||
return { | ||
lstPriceApy30d: jitoSolMetrics.past30DaysApyAvg ?? 0, | ||
lstPriceApr30d: mSolMetrics?.past30DaysAprAvg, |
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.
should be jitoSolMetrics... right??
aprFromApy(lstMetrics.lstPriceApy30d, Math.floor(365 / 2)) / 100; | ||
const lstApr = lstMetrics?.lstPriceApr30d | ||
? lstMetrics?.lstPriceApr30d / 100 | ||
: aprFromApy(lstMetrics.lstPriceApy30d, Math.floor(365 / 2)) / 100; |
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.
Not really related to your changes but I often ask myself why we show this in APR not APY? The LSTs are auto-compounding and so is the deposit yield, so the APY number is more truthful to what you're actually getting, right? The only thing that would not really be factored in is the BLZE rewards for bSOL because those are not compounded automatically.
Not blocking I just wonder what everyone's thoughts are on it. I feel like the only reason we APR in the first place is because the designs said APR not APY, but I keep wondering if we should change that which would also simplify the code.
This was my original attempt at removing the soon-to-be deprecated Jito API ... I think these changes are better but I got blocked by not being able to emulate superstake accounts to confirm that it's all working OK
For now, we should be able to just merge these smaller changes .. but I think long term this PR is better for the codebase if someone can help me figure out how to test it all