Fix event emission of AToken and DebtToken events

[miguelmtzinf]

• Fix emission of scaled balance in BalanceTransfer event
• Fix emission of whole amounts in Transfer event
• Add emission of Mint events in the transfer function to reflect interest accruals
• Clarify natspec docs of events and functions
• Add multiple test cases for AToken and DebtToken events

[miguelmtzinf]

Closes #681

[defispartan]

Changing this parameter name will likely break the subgraph

[miguelmtzinf]

The subgraph will need to differentiate pre and post patch versions

[eboadom]

This doesn't seem reasonable only due to a name change. Unless the subgraph breaks anyway or there is some major reason

[miguelmtzinf]

The subgraph will need to differentiate either way. However, this change will affect the subgraph logic so It may be interesting to revert it back. Thoughts @defispartan ?

[defispartan]

Last month we implemented a fix for the current V3 subgraphs, so now these subgraphs correctly handle mint and burn events. With this change we would need to add versioning throughout each V3 subgraph to say:

if(blockNumber > ...){
	// use updated mint/burn logic
	// use updated naming: event.params.value
} else {
	// use current mint/burn logic
	// use current naming: event.params.amount
}

Since we have a working version already and handling this would be quite ugly, I would be in favor of reverting the event change

[miguelmtzinf]

Makes sense to me. Reverting the change in order to make the subgraph's logic less painful

[eboadom]

@miguelmtzinf Given that this code operates on _userState[sender] (or recipient) after and being 1 slot packed with additionalData, is not more gas-efficient to do a

UserState memory senderState = _userState[sender];

and then just operate with the fields in memory?
Unless the compiler takes this into account already

[miguelmtzinf]

Not worth to do what you suggest because we need to update storage sender/recipient 's state later on

[eboadom]

Probably better to simplify the operation to only do 1 rayMul, no?

[miguelmtzinf]

Agree, but we are using this approach on other parts throughout the code

[eboadom]

How is the gas impact of this @miguelmtzinf ?

[miguelmtzinf]

How is the gas impact of this @miguelmtzinf ?

There is a slight increase up to 5k gas units. Here you can see the diff:
https://editor.mergely.com/i0TdBTPn/

[defispartan]

Would it be possible to add amount as another field to this event? It won't be used for anything, but this way the subgraph can differentiate which logic to use based on a different event signature.

The alternative is that we'd need to store the exact block that each market was updated to 3.0.1, and decide which logic to use based on the market and blockNumber.

[miguelmtzinf]

It does not make much sense to add another param if its not needed tbh.

Are not we using blockNumber already to differentiate? I think there is no other way

[defispartan]

No, there's nowhere else in the subgraphs where events need to be differentiated between contract versions using block number. It's extremely ugly, and prone to errors if tokens have events in this same block as the update, or if not all tokens are updated in the exact same block