-
Notifications
You must be signed in to change notification settings - Fork 359
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
feat: implement underlying token APYs #2092
base: main
Are you sure you want to change the base?
Conversation
|
}, | ||
]; | ||
|
||
const contract = new Contract('0x9b37180d847B27ADC13C2277299045C1237Ae281', abi); // cbETH Oracle |
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.
all contract addresses should be pulled from the aave-address-book. If they are not they should be added https://github.com/bgd-labs/aave-address-book
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.
I've just push a PR on the address book repo
I'll tag you once the modifications will be done on this PR
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.
Chiming in here as a maintainer of address-book.
We don't really want to list these addresses on address-book. The reason is that address book first and foremost is a library for usage in solidity and these addresses (for now) have no value for solidity devs
This repository contains an up-to-date registry of all addresses of the Aave ecosystem's smart contracts, for its usage in Solidity codebases.
The js export is a low effort feature we added at a later point to also expose these addresses in js(why not expose them when we already have them). This by no means implies though, that we'll add any address that might be useful in js on the address-book as this always comes at a cost for the solidity users of the library.
This out of the way, i had a quick look at what is done here and want to share my 2ct:
- I think what @MartinGbz did here is quite valuable and could be useful for other ecosystem projects. I'd like to encourage you to develop this as an independent & modular package so you can rely on viem, multicall etc. We'd be happy to help & contribute.
- probably it would be better to use
getLogs
with a topics filter overqueryFilter
. The reason is that queryFilter is not as widely supported and buggy on some node implementations. - For use in an ui (like this one) I think it's quite wasteful to actually do what is done here. Every user will need to do multiple api calls, fetch events over time and do calculations etc. Offloading unnecessary work to the client comes as a big cost and might cause rate-limits being reached on the 3th party endpoints. I would highly encourage the maintainers of this app to put 1) behind some cached api so users have to do a single request and not calculate anything clientside.
❌ CI run has failed! |
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
Page | Size (compressed) |
---|---|
global |
539.29 KB (🟡 +1.5 KB) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script>
tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Nine Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
Page | Size (compressed) | First Load |
---|---|---|
/ |
76.97 KB (🟢 -127 B) |
616.26 KB |
/faucet |
28.32 KB (🟡 +1 B) |
567.61 KB |
/governance |
88.11 KB (🟢 -487 B) |
627.4 KB |
/governance/v3/proposal |
128.38 KB (🟢 -479 B) |
667.67 KB |
/history |
36.5 KB (🟢 -422 B) |
575.79 KB |
/markets |
30.93 KB (🟢 -228 B) |
570.22 KB |
/reserve-overview |
24.19 KB (🟢 -8 B) |
563.48 KB |
/staking |
25.75 KB (🟢 -828 B) |
565.04 KB |
/v3-migration |
41.7 KB (🟢 -485 B) |
580.99 KB |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link
is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script>
tag are not accounted for in this analysis
Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.
c3e8516
to
cac4374
Compare
…MartinGbz/aave-interface into feat/implement-lst-native-yield
❌ CI run has failed! |
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
Page | Size (compressed) |
---|---|
global |
552.44 KB (🟡 +7.71 KB) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script>
tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Ten Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
Page | Size (compressed) | First Load |
---|---|---|
/ |
73.59 KB (🟢 -212 B) |
626.03 KB |
/bridge |
44.04 KB (🟢 -553 B) |
596.48 KB |
/faucet |
23.75 KB (🟡 +2 B) |
576.18 KB |
/governance |
83.37 KB (🟢 -836 B) |
635.81 KB |
/governance/v3/proposal |
123.65 KB (🟢 -522 B) |
676.09 KB |
/history |
31.94 KB (🟢 -426 B) |
584.37 KB |
/markets |
27.03 KB (🟢 -387 B) |
579.47 KB |
/reserve-overview |
19.79 KB (🟡 +4 B) |
572.23 KB |
/staking |
21.17 KB (🟢 -833 B) |
573.61 KB |
/v3-migration |
37.5 KB (🟢 -536 B) |
589.94 KB |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link
is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script>
tag are not accounted for in this analysis
Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.
General Changes
Adds underlying tokens APY to:
Desktop & Mobile are managed
Only the sum of both APYs is displayed, a tooltip button has been added to see the APY details (Supply/Borrow APY & Underlying APY)
APY are fetched/compute using onchain data. But if the onchain fetch failed (eg: no events during the last week), there is for each token a backup function that use an API (except for the
cbETH
that returns 0 because I didn't found any official API, so no underlying APY will be display in this case).Q: what are the chances (no events during the last week) ?
A:
But event if one day is 1 day without event, I take the last and latest event from a period of 1 week, so the front will use backup function only if there are no events during the last 7 days. But if you want we can increase it for 14 days to be full safe, I'm just afraid about RPC range block error / rate limit.
APYs Implemented:
Developer Notes
I didn't implement the APY in the migration page because I think it's clearer to keep only Aave Supply APYs here.
For the fork mode, I set the max block range (for event logs fetching) to 1k. Because on some cases, tenderly provide RPC with only 1k max block range capacity per request. This make the fetch slower, but because it's only on fork env it's alright. For the prod mode we use 100k block as max range.
General Notes:
SupplyAssetsListMobileItem.tsx
file wasn't used because there's already aSupplyAssetsListItemMobile
component in theSupplyAssetsListItem.tsx
file.I proposed to either remove the
SupplyAssetsListMobileItem.tsx
file or remove theSupplyAssetsListItemMobile
include in theSupplyAssetsListItem.tsx
file and use the one inSupplyAssetsListMobileItem.tsx
Let me know :))
Author Checklist
Please ensure you, the author, have gone through this checklist to ensure there is an efficient workflow for the reviewers.
main
If the PR is ready for review:
Open
state and not inDraft
modeReady for Dev Review
label has been addedReviewer Checklist
Please ensure you, as the reviewer(s), have gone through this checklist to ensure that the code changes are ready to ship safely and to help mitigate any downstream issues that may occur.
.env.example
file as well as the pertinant.github/actions/*
files