refactor accrueInterest

[MathisGD]

• fixes the weird position of the event emission in _accrueInterest (before the write of lastUpdate)
• it removes a early return that are dangerous imo
• it simplifies the spec of the event emission in _accrueInterest (always emitted)
• alls irm calls are done through one function
• now blue is irm reentrancy safe

I opened this PR to 1- remove the weird call to the irm, and 2- synchronize the update of “lastUpdate” and the call to the irm
in accrueInterest, elapsed=block.timestamp which is weird but as totalBorrow=0 it’s ok
but now I changed my mind, I think that it’s better to update first “lastUpdate” and have elapsed=0 in accrueInterest

[Rubilmax]

The only benefit of this PR to me is that we call the IRM every time the market is interacted with, which can be useful for stateful IRMs
Otherwise, all other changes in the code are drawbacks:

• change in the order in which events are emitted
• AccrueInterest events are emitted when interest accrued is zero (so it is useless)
• call to _accrueInterest in createMarket is more complex
• the IRM costs gas to call

However, it seems to me that this property:

we call the IRM every time the market is interacted with

is unnecessary, because the stateful IRM can always re-calculate the evolution of the state between blocks, based on the last state that is passed to it every time... So I am currently against this change, because the benefit doesn't cover the cost of drawbacks

[MathisGD]

• change in the order in which events are emitted

if you are only talking about createMarket, this is easily fixable if we agree that it's a problem (I'm still not convinced)

• AccrueInterest events are emitted when interest accrued is zero (so it is useless)

it doesn't cost a lot, and simplifies the spec (accrue interest is always called when the accrueInterest is called).

• call to _accrueInterest in createMarket is more complex

what do you mean?

• the IRM costs gas to call

you mean that we increase the number of calls to the irm, which costs gas right?

we call the IRM every time the market is interacted with

is unnecessary, because the stateful IRM can always re-calculate the evolution of the state between blocks, based on the last state that is passed to it every time... So I am currently against this change, because the benefit doesn't cover the cost of drawbacks

not sure what you mean here. If you want an irm that depends all interactions (including multiple interactions in the same block) how can it recompute this if you don't call it when the state changes.

The base branch was changed.

[adhusson]

now blue is irm reentrancy safe

this is unclear to me?

[MerlinEgalite]

I'm a bit uncomfortable with this refactor, we're changing a lot of lines for no really big upsides

[Rubilmax]

Based on morpho-org/metamorpho#269, for a MetaMorpho vault with 10 markets and the AdaptiveCurveIRM, this PR makes a deposit on MM cost on avg 40k more (+5.5% of 715k, min +7k max +117k) which means on avg on a market it costs +4k gas to supply

The tests may not be relevant because 50% of the time, an interaction happens in the same block of another in the same market

[Rubilmax]

I feel like it will be costly at scale because I am so bullish Blue will become the base layer of lending that I expect multiple interactions in a single block
But at worst it leaves room for improvement for V2...

[QGarchery]

Last argument against: we just skip the call of the IRM for idle markets, whereas we gained much more before. This is too bad, and I still think the changes in this PR are not worth the potential added gas cost and changing that many lines, but I'm not blocking it