Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MorphoBalancesLib.expectedBorrowAssets may return a value greater than totalBorrowAssets #628

Closed
Rubilmax opened this issue Dec 5, 2023 · 4 comments · Fixed by #632
Closed
Assignees

Comments

@Rubilmax
Copy link
Contributor

Rubilmax commented Dec 5, 2023

This issue has not been raised in a cantina finding.

    function testExpectedBorrowAssetsLeTotalBorrowAssets(uint256 borrowedShares) public {
        borrowedShares = bound(borrowedShares, 1, SharesMathLib.VIRTUAL_SHARES - 1);

        collateralToken.setBalance(BORROWER, 1);

        oracle.setPrice(1e37);

        vm.startPrank(BORROWER);
        morpho.supplyCollateral(marketParams, 1, BORROWER, hex"");
        morpho.borrow(marketParams, 0, borrowedShares, BORROWER, BORROWER);
        vm.stopPrank();

        (,, uint256 totalBorrowAssets,) = morpho.expectedMarketBalances(marketParams);

        assertLe(morpho.expectedBorrowAssets(marketParams, BORROWER), totalBorrowAssets);
    }

Run: yarn test:forge -vvv --mt testExpectedBorrowAssetsLeTotalBorrowAssets

This is due to the toAssetsUp

Here is another test that shows that this happens whatever totalBorrowAssets (not only where it is zero):

    function testAssetsUp(uint256 shares, uint256 totalAssets, uint256 totalShares) public {
        totalAssets = bound(totalAssets, 0, type(uint128).max);
        totalShares = bound(totalShares, 1, type(uint128).max);
        shares = bound(shares, 1, totalShares);

        vm.assume(totalAssets >= 1 ether);

        assertLe(shares.toAssetsUp(totalAssets, totalShares), totalAssets);
    }

The end stakes are:

  1. Underflow upon repay/liquidate because totalBorrowAssets < repaidAssets, when using the return value as input
  2. This rounding seems to only happen when shares ~= totalShares +- 10

(1) is addressed via https://github.com/morpho-labs/morpho-blue-private/blob/f463e40f776acd0f26d0d380b51cfd02949c8c23/src/libraries/periphery/MorphoBalancesLib.sol#L108-L109

(2) is unlikely because it requires the borrow shares to be concentrated to a single address

@QGarchery
Copy link
Contributor

I think this is expected, because we want to over value borrow assets. It is also acknowledged in a comment, so I'm fine with not doing anything for this issue

@Rubilmax
Copy link
Contributor Author

Rubilmax commented Dec 5, 2023

It is also acknowledged in a comment

Where is it acknowledged?

@QGarchery
Copy link
Contributor

In the link you provided it seems like it is actually saying the same thing no ?

(1) is addressed via https://github.com/morpho-labs/morpho-blue-private/blob/f463e40f776acd0f26d0d380b51cfd02949c8c23/src/libraries/periphery/MorphoBalancesLib.sol#L108-L109

@Rubilmax
Copy link
Contributor Author

Rubilmax commented Dec 5, 2023

This comment is only about repaying, whereas it's a broader issue
I would rephrase this comment and be fine with it then

@Rubilmax Rubilmax self-assigned this Dec 5, 2023
@Rubilmax Rubilmax linked a pull request Dec 7, 2023 that will close this issue
@Rubilmax Rubilmax mentioned this issue Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants