morpho-org / morpho-blue-irm

Morpho Blue Interest Rate Models
https://morpho.org
MIT License
26 stars 17 forks source link

IRM refactor #54

Closed MathisGD closed 11 months ago

MathisGD commented 12 months ago

Implements a new abstraction of the IRM, with a moving rate curve.

Fixes notably #42, #55, https://github.com/cantinasec/review-morpho-blue-1/issues/37, https://github.com/cantinasec/review-morpho-blue-1/issues/36, https://github.com/cantinasec/review-morpho-blue-1/issues/35

MerlinEgalite commented 11 months ago

I think this PR does not fix:

But fixes:

MerlinEgalite commented 11 months ago

shit I was not supposed to do it on this PR

EDIT: fixed

Jonah246 commented 11 months ago

This factor looks great! While I still need some time on this one, I don't think there would be many issues. The code is so much cleaner and resolves many issues at the same time. The biggest concern on my side would be the parameters listed here. https://github.com/morpho-labs/morpho-blue-irm/blob/222b04e20f3f52478ceed3e81d213aad8ef8e81f/test/SpeedJumpIrmTest.sol#L19-L22

At the current parameters, the borrow rates would easily reach the bounds (MAX_RATE_AT_TARGET or MIN_RATE_AT_TARGET) within minutes making most pools vulnerable to attackers.

Jonah246 commented 11 months ago

Another relatively minor concern is that the range of borrowRate is now dependent on CURVE_STEEPNESS and is not bounded by MAX_RATE_AT_TARGET or MIN_RATE_AT_TARGET. There's a chance that borrowRate goes too high and DOS the morpho-blue market. We can consider whether to check the CURVE_STEEPNESS in the constructor.

MathisGD commented 11 months ago

At the current parameters, the borrow rates would easily reach the bounds (MAX_RATE_AT_TARGET or MIN_RATE_AT_TARGET) within minutes making most pools vulnerable to attackers.

How would it reach the bounds within minutes?

Jonah246 commented 11 months ago

@MathisGD Sorry. I made a stupid mistake. It's not the case.

MerlinEgalite commented 11 months ago

merge conflicts to fix