Reserve and UserReserve balances are wrong

ronlv10 commented 2 years ago

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: 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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

aave / protocol-subgraphs