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

Reserve and UserReserve balances are wrong #82

Open
ronlv10 opened this issue Aug 29, 2022 · 5 comments
Open

Reserve and UserReserve balances are wrong #82

ronlv10 opened this issue Aug 29, 2022 · 5 comments

Comments

@ronlv10
Copy link
Contributor

ronlv10 commented Aug 29, 2022

TLDR

In v3, there are a bunch of wallets in all v3 chains that have wrong balances in the subgraph compared to the on-chain data.

Example

For example, 0xbc1631afcb916bda28af42955fc970bf004596f8 on Polygon has 0 on-chain balance:
image but, querying this user from the subgraph and using aave-utilities formatter, the user has balances.
Same goes for wallets that still have dust balance on chain: 0xee2826453a4fd5afeb7ceffeef3ffa2320081268 on Avalanche : 0.00000018 WETH.e on chain- comparing to 1+ WETH.e using the subgraph.

Root cause - account balance computation updated in v3

It seems like the Burn and Mint events were changed in AAVE v3. In v3, The event value reports the amount that should be minted/burned while taking into account the interest which was accumulated (which impact the amount of mint/burned) since the last update.
In v2, the events reported the amount which was supplied/withdrawn, and the interest could be calculated using other fields in the subgraph like liquidityIndex, liquidityRate, etc.

Scope of Issue

  • Any wallet that executed two actions with a delta such that interest was accrued stores the wrong balance.
    • For example, a wallet that has performed the supply method twice or a combination of withdraw and supply.
  • This bug also affects any field that is affected by interest accumulated by the wallet balances. Namely, this effects Reserve metrics such as totalSupply and totalBorrow per asset.

Impact

  • This is a large-scale bug as it essentially affects all wallets (barring wallets that have only ever supplied).
  • This means that anyone consuming data on wallets/health/risk from the officially supported subgraphs was reading the wrong data.
  • Any teams, traders, or institutions attempting to make data-driven decisions based on subgraph data are affected.

Proposed Fix

In order to keep consistency and backward compatibility to the structure of v2 (and to aave-utilities functions), we need to sum up the Burn.value and Burn.balanceIncrease and use this value for all the calculations. The opposite goes for Mint - we need to subtract Mint.balanceIncrease from Mint.value.

Proposed PR

We have created a PR that fixes the issue, but we couldn't submit it because of repository permissions. If you can provide me access to create PRs, we would be happy to contribute to the repo with the fixed PR.

@Marykassman
Copy link

That’s my contract address been taken over in the case of not paying for the address approval and I could like anyone get back to me on what to do or what you need .

@defispartan
Copy link
Contributor

That’s my contract address been taken over in the case of not paying for the address approval and I could like anyone get back to me on what to do or what you need .

I have no idea what this means or how this relates to this issue

@defispartan
Copy link
Contributor

@ronlv10 Thank you so much for this contribution and the detailed write up. We are testing this change in a staging deployment and will have it out in prod shortly after some testing

I believe that a similar issue exists for the V2 subgraphs as well. I'm not sure if you would have any capacity to research this as well? If not it's totally fine, we can pick it up after finishing this change.

@ronlv10
Copy link
Contributor Author

ronlv10 commented Sep 28, 2022

Hey @defispartan. I am really happy I could help.
I attached below the subgraphs I deployed using this fix, in case you want to try them out.

I believe that a similar issue exists for the V2 subgraphs as well. I'm not sure if you would have any capacity to research this as well? If not it's totally fine, we can pick it up after finishing this change.

I would be happy to try to help. I think I need some more context regarding the issue in V2: IIUC, the change in the event types was only on v3, no? Any examples of the issue in V2 would be great.

Subgraph links:
Arbitrum Subgraph
Harmony Subgraph
Polygon Subgraph
Fantom Subgraph
Avalanche Subgraph
Optimism Subgraph

@defispartan
Copy link
Contributor

Thank you so much for your help! We just deployed and merged an update including your change to all production and testnet subgraphs 🚀

And yeah we can keep this thread open and share more details about the bugs that we're seeing in V2 as well

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

No branches or pull requests

3 participants