Skip to content
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

Update: Show supplyApr for lending strats #1323

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erenjaegar77
Copy link
Contributor

Return supplyApr instead tradingApr on apy/breakdwon for lending strats

eg :

[
"seamless-usdbc": {
"vaultApr": 0.19658461757583676,
"compoundingsPerYear": 2190,
"beefyPerformanceFee": 0.095,
"vaultApy": 0.21722757705799078,
"lpFee": 0,
"supplyApr": 0.00000543258275377198,
"liquidStakingApr": 0,
"composablePoolApr": 0,
"totalApy": 0.2172341897475334
}, //dev
"seamless-usdbc": {
"vaultApr": 0.19418793482588761,
"compoundingsPerYear": 2190,
"beefyPerformanceFee": 0.095,
"vaultApy": 0.2143140214899324,
"lpFee": 0,
"tradingApr": 0.00000543163620594223,
"liquidStakingApr": 0,
"totalApy": 0.21432061720193696
}//prod
] 

@roman-monk
Copy link
Member

adding more fields is a wrong way, need to always adjust the app and imagine how messy it would be to break down this one
image
here is 1 trading apy, 1 supply apy, 1 liquid staking + 3 rewards.

consider to add 1 additional field "labels" that app will display as is (but can re-word with mapping in app if needed)

"seamless-usdbc": {
  "vaultApr": 0.19658461757583676,
  "compoundingsPerYear": 2190,
  "beefyPerformanceFee": 0.095,
  "vaultApy": 0.21722757705799078,
  "lpFee": 0,
  "totalApy": 0.2172341897475334,
  "labels": [
    {
      "name": "Supply",
      "apy": 0.3123
    },
    {
      "name": "Liquid Staking",
      "apy": 0.4567
    },
    {
      "name": "Future Airdrop",
      "apy": 1111.111
    }
  ]
}

and if no "labels" just display as its now

@ReflectiveChimp
Copy link
Collaborator

@roman-monk we can do something like this but the app needs to know which parts are "compoundable" (APY) and which parts are "non-compoundable" (APR); and ideally for those that are compoundable, the number of compoundings per year that was used to calculate the APY so it can be reversed.

This is so we can calculate/show the daily part from the yearly, and also to calculate the monthly/daily yield for the portfolio component.

Perhaps something like:
Where daily is specified, and type: apy = compounding and type: apr =non-compounding

type BreakdownItem = {
  name: string;
  daily: number;
  yearly: number;
  type: 'apy' | 'apr';
}

or where daily isn't specified, we could calculate this given the number of compoundings per year for apy

type BreakdownItem = {
  name: string;
  yearly: number;
} & (
 { type: 'apr'; } 
 | { type: 'apy'; compoundings: number; } 
)

--

A side note: specifying the labels in the API also means we can't do translations (although we currently don't have translations enabled in the app).

@roman-monk
Copy link
Member

@ReflectiveChimp lets try to simplify things, we already have compoundingsPerYear in breakdown object.
we can then have "apy" and "apr" fields, but perhaps can be even simpler, let every item there be strictly APR, so "name" and "apr" fields.

then daily will be just a sum / 365 as it is now right? same as we have vaultApy and vaultApr currently.
maybe its different for portfolio?
lets try to find solution without overthinking it please, we do complicate things too often.

A side note: specifying the labels in the API also means we can't do translations (although we currently don't have translations enabled in the app).

first of all we can, just make hardcoded mapping from label to translation key in app, some labels like "CRV APR" will not even need it, will work better than it sounds.
but again lets keep it simple, translation in app will never happen.
and if it will happen this is not the only blocker, decent amount of work will be required and translating labels will be small part of it.

pragmatic approach is to not spend time now on something that may not happen in the future at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants