Spearbit #54, 55, 56, 58, 62, 63, 64, 65, 66

[pakim249CAL]

This discussion is created to help decide how these issues should be divided up and worked on within our repositories, as each of these issues contain many different suggestions, and many are duplicates. Here are the issues separated out as I understand them:

1

• Because _unsafeRepayLogic internally call aave.supply the whole tx could fail in case the supplying has been disabled on Aave (isFrozen == true) for the _poolTokenBorrowed
• Because _unsafeWithdrawLogic internally call aave.borrow the whole tx could fail in case the borrowing has been disabled on Aave (isFrozen == true or borrowingEnabled == false) for the _poolTokenCollateral

2

• Morpho is not checking that the Aave collateralAsset has isActive == true
• Morpho is not checking that the Aave borrowAsset has isActive == true in _unsafeWithdraw
• Morpho do not check if market is active (would borrowingEnabled == false if market is not active?)
• Morpho do not check if market is frozen (would borrowingEnabled == false if market is not active?)

3

• Morpho do not check that remainingToRepay.rayDiv(poolIndexes[_poolToken].poolBorrowIndex) > 0. Trying to repay that amount to Aave would make the whole tx revert
• Morpho do not check that remainingToSupply.rayDiv(poolIndexes[_poolToken].poolSupplyIndex) > 0. Trying to borrow that amount to Aave would make the whole tx revert
• Morpho do not check that remainingToWithdraw.rayDiv(poolIndexes[_poolToken].poolSupplyIndex) > 0. Trying to withdraw that amount from Aave would make the whole tx revert
  Morpho do not check that remainingToBorrow.rayDiv(poolIndexes[_poolToken].poolBorrowIndex) > 0. Trying to borrow that amount from Aave would make the whole tx revert

4

• Both Aave and Morpho are not handling ERC20 token with fees on transfer

5

• Morpho is not checking that the Aave market has isFrozen == false (check done by Aave on the borrow operation), users could be able to borrow in P2P even if the borrow is paused on Aave (isFrozen == true) because Morpho would only call the aave.withdraw (where the frozen flag is not checked)

6

• Morpho do not check that healthFactor > GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD

7

• Morpho does not check if _amount > 0 for liquidate

8

• Compound revert if amountToSeize > userCollateralBalance, Morpho does not revert and instead use min(amountToSeize, userCollateralBalance)

9

• Compound has a mechanism to prevent borrows if the new borrowed amount would go above the current borrowCaps[cToken] threshold. Morpho does not check this threshold and could allow users to borrow on the P2P side (avoiding the revert because it would not trigger the underlying compound borrow action). Morpho should anyway monitor the borrowCaps of the market because it could make increaseP2PDeltasLogic and _unsafeWithdrawLogic reverts.

10

• Both Morpho and Compound DO NOT check if a market is in "deprecated" state. This mean that as soon as a user borrow some tokens, he/she can be instantly liquidated by another user.
  • If the flag is true on Compound, the Morpho User can be liquidated directly on compound
  • If the flag is true on Morpho, the borrower can be liquidated on Morpho

11

• Morpho does not check if borrowGuardianPaused[cToken] on Compound, a user could be able to borrow in P2P while the cToken market has borrow paused.

12

• Morpho is not checking if the underlying Compound market has been paused for the supply action (see mintGuardianPaused[token]). This mean that even if the Compound supply is paused, Morpho could allow users to supply in the P2P.

13

• Morpho is not checking if the market on both Morpho and Compound has been deprecated. If the deprecation flag is intended to be true for a market that will be removed in the next future, probably Morpho should not allow users to provide collateral for such a market.

14

• Morpho should create an in-depth documentation that explains all these flags and how they interact with each other and which was the scenario for which Morpho has decided not to follow the protocol's behavior (if this scenario exists).
• The documentation will be very useful to track where and when those flags interact with each other and what could be the possible outcome of a change decision.

15

• Morpho-Compound does not revert if the borrower is trying to liquidate himself

16

• Morpho-Compound does still allow liquidations even if they are prevented on Compound with seizeGuardianPaused. Note that this flag applies to all markets

[pakim249CAL]

My thoughts on each:

1. I don't think this is viable to fix, as this step is crucial to proper accounting in Morpho. So I think acknowledge and skip.
2. I believe this is covered in Morpho's pause powers and procedure to make sure to follow governance and react accordingly. So acknowledge and skip.
3. Good issue.
4. Acknowledge and skip.
5. Morpho should pause accordingly. So acknowledge and skip.
6. Acknowledge and skip.
7. Good issue.
8. Worth a discussion. I think the safest method is to only use the min value if the amount input is type(uint256).max. Otherwise it should revert.
9. Related to borrow cap discussion?
10. I think this needs to be managed in pause methodology, so acknowledge and skip.
11. Pause methodology should cover, so acknowledge and skip.
12. We should disable p2p matching in this case, so acknowledge and skip.
13. Pause methodology should cover, so acknowledge and skip.
14. Agreed at some point, but probably something for notion?
15. Acknowledge.
16. Acknowledge.

[MerlinEgalite]

1. I'm not sure why it's crucial for Morpho accounting.
2. Yep, but here it would be automated (less DAO management, remember that over time we'll need to create a vote for everything).
3. I think we never faced this in prod so I don't think it's worth implementing it.
4. Acknowledge.
5. Might be automated.
6. This is wrong IMO.
7. Ok.
8. Not a problem to me, this is the behavior for all entry points.
9. Might do it.
10. Same as for Aave.
11. Same as for Aave.
12. Same as for Aave.
13. Same as for Aave.
14. I already started writing something about it.

[Rubilmax]

Agreed on all except 5): why wouldn't we include these checks in the protocol? So that Morpho is following the market decisions of the underlying protocol (otherwise unexpected behaviors may appear)

[pakim249CAL]

1: I think it's crucial that the supply step to the pool to not be skipped. This SHOULD revert, since otherwise we are allowing users to skip this step in the unsafeRepay and unsafeWithdraw steps. Am I wrong about this?
3: I don't think not facing the issue in prod is a good reason to skip this, especially since the issue is very simple to tackle. Is there another good reason to skip it?
5: I think including these checks is redundant. If the borrow is frozen somehow without Morpho noticing beforehand in the underlying pool, then the borrow call would revert, but just later in the call. Isn't this already the correct behavior?

@MerlinEgalite @Rubilmax

[Rubilmax]

What is necessary to me is that Morpho should follow the market decisions of the underlying protocol (pause supply when supply is paused on the underlying protocol, etc)

1: It's suggested here to skip repaying if supply is paused on the pool, because it would revert. As this is a decision at the level of Morpho (repaying is not necessarily paused on the pool, this issue is a edge case implied by Morpho's behavior), I don't think it's necessary (so I agree to acknowledge the issue)

5: The check is indeed redundant in the case where Morpho supplies (resp. borrows, repays, etc). But Morpho does not necessarily always supply (resp. borrow, repay, etc) on the pool (e.g. when matching p2p), and it would thus cause Morpho to not replicate the market decisions of the underlying protocol (which may have frozen supply (resp. borrow, repay, etc) for example). So I think it's necessary to include these checks

[MerlinEgalite]

Calls do not necessarily revert because of the P2P... So I would be for the maximum of automation.

3. I don't have a strong opinion on this, we can add it. It's just adding things that are unnecessary just complexity the code for no real upside (eg handling fees for ERC20).

[MerlinEgalite]

Agree we should add them thx!

[pakim249CAL]

Issues to tackle (based on the numbering in this issue)

• 5 Check isFrozen == false (Spearbit Audit Issues) #1600
  Done.

• 7 Morpho does not check if _amount > 0 for liquidate (Spearbit Audit Issues) #1593
  Done.

• 14 In progress. Not relevant to this repo IIRC since it will be done off repo.

• 15 Morpho-Compound does not revert if the borrower is trying to liquidate himself (Spearbit Audit Issues) #1594
  Done.

[MerlinEgalite]

I think @pakim249CAL agrees with you on this point. But I leave him to give his thoughts on it as well as @Rubilmax and @MathisGD

[Rubilmax]

I'm not sure I get why it wouldn't be enough to pause supply (resp. borrow) when it's paused on the pool?
The thing we're trying to avoid here is to have users supplying and matched P2P in this case, which to me is solved by following the underlying pools behavior

[QGarchery]

I'm not sure I get why it wouldn't be enough to pause supply (resp. borrow) when it's paused on the pool?
The thing we're trying to avoid here is to have users supplying and matched P2P in this case, which to me is solved by following the underlying pools behavior

It's because the situation might require more urgent actions to be taken. We are not only trying to prevent new p2p credit lines to be created, but we are also trying to make existing credit lines liquid. See my reply above for the actions we might want to take.

For example, if borrow is disabled on the pool, p2p credit lines migth be stuck. Notice that in this case, it's possible for users to supply, get matched, and not be able to withdraw. So in some scenarios it make sense to also pause supply

[Rubilmax]

Ok I'm sorry I didn't exactly say what I wanted to: I get that actions taken this way would not be enough, but I don't agree with:

The checks that we would add would never be enough so I don't see the point of adding them

It's still better to reflect the underlying pool's behavior rather than doing nothing?

I'm sorry if we already talked about this but I still disagree with not doing anything

[pakim249CAL]

Ultimately, I think this issue is not so easy to resolve because of the costs associated. Retrieving all the underlying pool parameters to make sure that Morpho follows them incurs a non-trivial gas cost and code complexity. This is ultimately why I think Morpho should rely on its granular pausing to resolve issues rather than having it solved in the contract level. But it's an opinionated tradeoff admittedly.

[MerlinEgalite]

Do we agree that the 6. is wrong?

[Rubilmax]

We do not explicitly check that healthFactor >= liquidationThreshold, but we implicitly do via debtEth <= borrowableEth
I think the issue 63 was more here to gather explicit differences between Morpho & AaveV2

So I'd acknowledge this

[MerlinEgalite]

Ok yes thank you, I'd acknowledge it too