fix(#patch); aave v3; handle change to BalanceTransfer event

[defispartan]

The value parameter of the BalanceTransfer event was updated in the Aave 3.0.1 contract update to be the scaled balance in place of current balance.

This PR interprets value based on current block compared to V3 -> V3.0.1 contract upgrade for that market.

Once Aave V3 markets are upgraded to V3.0.1 via Governance, the update blocks will need to be added to this helper function and subgraphs re-deployed.

[melotik]

Hey @defispartan thanks for this! Do you know when we can expect this to be deployed to mainnet so we can update the blocknumber accordingly?

[defispartan]

Mainnet is already live, the block number is just set to 0 since this market will only have the updated events and the old logic will never be used. This subgraph should good to deploy with no other changes.

All of the other V3 markets will have a block number entry once they're upgraded.

[melotik]

Can you use the global constant from constants.ts for the network name

[melotik]

We had a change to aave v3 that we made recently so you will need to rebase.

One of the changes is that we do not actually calculate the account balances from transfer.balance event param anymore. We make a contract call to get the new balances of the sender/receiver. My question is: does the balanceOf() function in the AToken contract use this logic? From that PR aave/aave-v3-core#682 it looks like no

[melotik]

This will be necessary to do once we upgrade aave-forks to the 3.0.0 schema and we are keeping track of Transfers. #1519

[tnkrxyz]

@defispartan , can you help clarify about the aToken upgrade?

It seems to me that the aToken contract has identical signature for BalanceTransfer since January, this is the first aToken created on Jan 27 according to ReserveInitialized events emitted by the PoolConfigurator contract, while this one the last one on Apr 10th. I see that the value argument is divided by index before BalanceTransfer event emitting on both contracts.

Are you saying that we should start to multiplying event.params.index with ``event.params.value` on later aToken deployment, but not on earlier ones? Which block did the switch/upgrade happen? and how can we tell? thx!

Follow up

Looked into this a bit more and follow up on above. have-forks subgraphs (v2, v3) have always listened on AToken.Transfer instead of BalanceTransfer, and the argument to both Transfer() and BalanceTransfer event have changed from v2 to v3.

From what I can see, in v2, the amount passed to _transfer() is discounted by index and index is not available in the Transfer event. In this case, switch to BalanceTransfer and directly use event.params.value (no multiplying with event.params.index is needed) since amount in BalanceTransfer event in v2 is not discounted by index.

In v3, I see at least two difference versions of the _tranfer function. In the lastest version, the amount arg is flipped in event Transfer and event BalanceTransfer (amount arg in Transfer is not discounted; amount in BalanceTransfer is). We can continue to listen the Transfer event and index is not needed.

In version at/before this commit on Nov 15th, event Transfer and BalanceTransfer is similar to that in v2. I suppose this was the v3.0.1 change you were talking about in this issue. Since the first AToken were not deployed on ethereum mainnet untill Jan 27th and from my check on the early AToken contract, AToken contracts are all using the latest version. That is we can continue to listen on Transfer event and don't worry about multiplying with index. Does this sounds right to you?

But it is trickier for other chains, the aave arbitrum deployed the pre v3.0.1 version AToken contract to this day and I don't know whether/when they will upgrade. Also I haven't found a reliable way to detect whether the AToken deployed is pre 3.0.1 or after using on-chain data. The hack I come up at this point is to obtain the amount in both Transfer and BalanceTransfer event (using transaction receipt) and always use the higher amount as the transfer amount. Do you see a problem with this hack? thx again!

[tnkrxyz]

@dmelotik , appreciate your input too!

[melotik]

@tnkrxyz to me it seems like this change only affected the value from BalanceTransfer event. And the current aave v3 code doesn't need that event.

I don't think we even need to account for this then. Do you agree? And is there a reason we would need to add BalanceTransfer back in??

[tnkrxyz]

@tnkrxyz to me it seems like this change only affected the value from BalanceTransfer event. And the current aave v3 code doesn't need that event.

I don't think we even need to account for this then. Do you agree? And is there a reason we would need to add BalanceTransfer back in??

@dmelotik From what I found so far, we should switch to use BalanceTransfer for aave-v2 because the Transfer event gives transfer amount that has been discounted by liquidity index. It is trickier for v3, because they flipped the amount argument between Transfer event and BalanceTransfer in middle of aave-v3 deployment (see my description above), so my current implementation is to get amount from both BalanceTransfer and Transfer event, and use the large number of the two. Let me know if this makes sense.

[tnkrxyz]

#1993 implements the BalanceTransfer/Transfer logic for aave-v3. We can close this issue once #1993 is merged.

[melotik]

Closing, this was tackled in #1993 Thanks all!