From 753a13f9f37b047f44a886cb1caa4ac031d18241 Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 24 Aug 2023 14:59:08 +0200 Subject: [PATCH 1/5] refactor: general review --- src/Morpho.sol | 35 ++++++++++---------- src/interfaces/IMorpho.sol | 50 ++++++++++++++++++----------- src/interfaces/IMorphoCallbacks.sol | 4 +-- src/libraries/EventsLib.sol | 12 +++---- src/libraries/MathLib.sol | 16 ++++----- src/libraries/UtilsLib.sol | 4 +-- 6 files changed, 66 insertions(+), 55 deletions(-) diff --git a/src/Morpho.sol b/src/Morpho.sol index d76b2dcfa..cb9fda19f 100644 --- a/src/Morpho.sol +++ b/src/Morpho.sol @@ -450,25 +450,24 @@ contract Morpho is IMorpho { // Safe "unchecked" cast. market[id].lastUpdate = uint128(block.timestamp); - if (market[id].totalBorrowAssets != 0) { - uint256 borrowRate = IIrm(marketParams.irm).borrowRate(marketParams, market[id]); - uint256 interest = market[id].totalBorrowAssets.wMulDown(borrowRate.wTaylorCompounded(elapsed)); - market[id].totalBorrowAssets += interest.toUint128(); - market[id].totalSupplyAssets += interest.toUint128(); - - uint256 feeShares; - if (market[id].fee != 0) { - uint256 feeAmount = interest.wMulDown(market[id].fee); - // The fee amount is subtracted from the total supply in this calculation to compensate for the fact - // that total supply is already updated. - feeShares = - feeAmount.toSharesDown(market[id].totalSupplyAssets - feeAmount, market[id].totalSupplyShares); - position[id][feeRecipient].supplyShares += feeShares; - market[id].totalSupplyShares += feeShares.toUint128(); - } - - emit EventsLib.AccrueInterest(id, borrowRate, interest, feeShares); + if (market[id].totalBorrowAssets == 0) return; + + uint256 borrowRate = IIrm(marketParams.irm).borrowRate(marketParams, market[id]); + uint256 interest = market[id].totalBorrowAssets.wMulDown(borrowRate.wTaylorCompounded(elapsed)); + market[id].totalBorrowAssets += interest.toUint128(); + market[id].totalSupplyAssets += interest.toUint128(); + + uint256 feeShares; + if (market[id].fee != 0) { + uint256 feeAmount = interest.wMulDown(market[id].fee); + // The fee amount is subtracted from the total supply in this calculation to compensate for the fact + // that total supply is already increased by the full interest (including the fee amount). + feeShares = feeAmount.toSharesDown(market[id].totalSupplyAssets - feeAmount, market[id].totalSupplyShares); + position[id][feeRecipient].supplyShares += feeShares; + market[id].totalSupplyShares += feeShares.toUint128(); } + + emit EventsLib.AccrueInterest(id, borrowRate, interest, feeShares); } /* HEALTH CHECK */ diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index 769012e13..9e7b4bc4c 100644 --- a/src/interfaces/IMorpho.sol +++ b/src/interfaces/IMorpho.sol @@ -65,10 +65,23 @@ interface IMorpho { function feeRecipient() external view returns (address); /// @notice The state of the position of `user` on the market corresponding to `id`. - function position(Id id, address user) external view returns (uint256, uint128, uint128); + function user(Id id, address user) + external + view + returns (uint256 supplyShares, uint128 borrowShares, uint128 collateral); /// @notice The state of the market corresponding to `id`. - function market(Id id) external view returns (uint128, uint128, uint128, uint128, uint128, uint128); + function market(Id id) + external + view + returns ( + uint128 totalSupplyAssets, + uint128 totalSupplyShares, + uint128 totalBorrowAssets, + uint128 totalBorrowShares, + uint128 lastUpdate, + uint128 fee + ); /// @notice Whether the `irm` is enabled. function isIrmEnabled(address irm) external view returns (bool); @@ -93,14 +106,13 @@ interface IMorpho { /// @notice Sets `newOwner` as owner of the contract. /// @dev Warning: No two-step transfer ownership. - /// @dev Warning: The owner can be set to the zero address. function setOwner(address newOwner) external; - /// @notice Enables `irm` as possible IRM for market creation. + /// @notice Enables `irm` as a possible IRM for market creation. /// @dev Warning: It is not possible to disable an IRM. function enableIrm(address irm) external; - /// @notice Enables `lltv` as possible LLTV for market creation. + /// @notice Enables `lltv` as a possible LLTV for market creation. /// @dev Warning: It is not possible to disable a LLTV. function enableLltv(uint256 lltv) external; @@ -126,7 +138,7 @@ interface IMorpho { /// @param marketParams The market to supply assets to. /// @param assets The amount of assets to supply. /// @param shares The amount of shares to mint. - /// @param onBehalf The address that will receive the position. + /// @param onBehalf The address that will own the increased supply position. /// @param data Arbitrary data to pass to the `onMorphoSupply` callback. Pass empty data if not needed. /// @return assetsSupplied The amount of assets supplied. /// @return sharesSupplied The amount of shares minted. @@ -145,7 +157,7 @@ interface IMorpho { /// @param marketParams The market to withdraw assets from. /// @param assets The amount of assets to withdraw. /// @param shares The amount of shares to burn. - /// @param onBehalf The address of the owner of the withdrawn assets. + /// @param onBehalf The address of the owner of the supply position. /// @param receiver The address that will receive the withdrawn assets. /// @return assetsWithdrawn The amount of assets withdrawn. /// @return sharesWithdrawn The amount of shares burned. @@ -166,8 +178,8 @@ interface IMorpho { /// @param marketParams The market to borrow assets from. /// @param assets The amount of assets to borrow. /// @param shares The amount of shares to mint. - /// @param onBehalf The address of the owner of the debt. - /// @param receiver The address that will receive the debt. + /// @param onBehalf The address that will own the increased borrow position. + /// @param receiver The address that will receive the borrowed assets. /// @return assetsBorrowed The amount of assets borrowed. /// @return sharesBorrowed The amount of shares minted. function borrow( @@ -185,7 +197,7 @@ interface IMorpho { /// @param marketParams The market to repay assets to. /// @param assets The amount of assets to repay. /// @param shares The amount of shares to burn. - /// @param onBehalf The address of the owner of the debt. + /// @param onBehalf The address of the owner of the debt position. /// @param data Arbitrary data to pass to the `onMorphoRepay` callback. Pass empty data if not needed. /// @return assetsRepaid The amount of assets repaid. /// @return sharesRepaid The amount of shares burned. @@ -203,7 +215,7 @@ interface IMorpho { /// @dev Supplying a large amount can revert for overflow. /// @param marketParams The market to supply collateral to. /// @param assets The amount of collateral to supply. - /// @param onBehalf The address that will receive the collateral. + /// @param onBehalf The address that will own the increased collateral position. /// @param data Arbitrary data to pass to the `onMorphoSupplyCollateral` callback. Pass empty data if not needed. function supplyCollateral(MarketParams memory marketParams, uint256 assets, address onBehalf, bytes memory data) external; @@ -213,15 +225,15 @@ interface IMorpho { /// @dev Withdrawing an amount corresponding to more collateral than supplied will revert for underflow. /// @param marketParams The market to withdraw collateral from. /// @param assets The amount of collateral to withdraw. - /// @param onBehalf The address of the owner of the collateral. - /// @param receiver The address that will receive the withdrawn collateral. + /// @param onBehalf The address of the owner of the collateral position. + /// @param receiver The address that will receive the collateral assets. function withdrawCollateral(MarketParams memory marketParams, uint256 assets, address onBehalf, address receiver) external; - /// @notice Liquidates the given `repaidShares` of debt asset or seize the given `seized` of collateral on the given - /// `market` of the given `borrower`'s position, optionally calling back the caller's `onMorphoLiquidate` function - /// with the given `data`. - /// @dev Either `seized` or `repaidShares` should be zero. + /// @notice Liquidates the given `repaidShares` of debt asset or seize the given `seizedAssets` of collateral on the + /// given market `marketParams` of the given `borrower`'s position, optionally calling back the caller's + /// `onMorphoLiquidate` function with the given `data`. + /// @dev Either `seizedAssets` or `repaidShares` should be zero. /// @dev Seizing more than the collateral balance will underflow and revert without any error message. /// @dev Repaying more than the borrow balance will underflow and revert without any error message. /// @param marketParams The market of the position. @@ -237,7 +249,7 @@ interface IMorpho { uint256 seizedAssets, uint256 repaidShares, bytes memory data - ) external returns (uint256, uint256); + ) external returns (uint256 seizedAssets, uint256 repaidAssets); /// @notice Executes a flash loan. /// @param token The token to flash loan. @@ -259,5 +271,5 @@ interface IMorpho { function setAuthorizationWithSig(Authorization calldata authorization, Signature calldata signature) external; /// @notice Returns the data stored on the different `slots`. - function extSloads(bytes32[] memory slots) external view returns (bytes32[] memory res); + function extSloads(bytes32[] memory slots) external view returns (bytes32[] memory); } diff --git a/src/interfaces/IMorphoCallbacks.sol b/src/interfaces/IMorphoCallbacks.sol index c568c9830..5b53475d8 100644 --- a/src/interfaces/IMorphoCallbacks.sol +++ b/src/interfaces/IMorphoCallbacks.sol @@ -6,9 +6,9 @@ pragma solidity >=0.6.2; interface IMorphoLiquidateCallback { /// @notice Callback called when a liquidation occurs. /// @dev The callback is called only if data is not empty. - /// @param assets The amount of repaid assets. + /// @param repaidAssets The amount of repaid assets. /// @param data Arbitrary data passed to the `liquidate` function. - function onMorphoLiquidate(uint256 assets, bytes calldata data) external; + function onMorphoLiquidate(uint256 repaidAssets, bytes calldata data) external; } /// @title IMorphoRepayCallback diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index aae011bfa..8163ab18a 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -14,12 +14,12 @@ library EventsLib { /// @notice Emitted when setting a new fee. /// @param id The market id. - /// @param fee The new fee. - event SetFee(Id indexed id, uint256 fee); + /// @param newFee The new fee. + event SetFee(Id indexed id, uint256 newFee); /// @notice Emitted when setting a new fee recipient. - /// @param feeRecipient The new fee recipient. - event SetFeeRecipient(address indexed feeRecipient); + /// @param newFeeRecipient The new fee recipient. + event SetFeeRecipient(address indexed newFeeRecipient); /// @notice Emitted when enabling an IRM. /// @param irm The IRM that was enabled. @@ -111,9 +111,9 @@ library EventsLib { Id indexed id, address indexed caller, address indexed borrower, - uint256 repaid, + uint256 repaidAssets, uint256 repaidShares, - uint256 seized, + uint256 seizedAssets, uint256 badDebtShares ); diff --git a/src/libraries/MathLib.sol b/src/libraries/MathLib.sol index d78cce272..b04f2db0b 100644 --- a/src/libraries/MathLib.sol +++ b/src/libraries/MathLib.sol @@ -23,18 +23,18 @@ library MathLib { return mulDivUp(x, WAD, y); } - /// @dev (x * y) / denominator rounded down. - function mulDivDown(uint256 x, uint256 y, uint256 denominator) internal pure returns (uint256) { - return (x * y) / denominator; + /// @dev (x * y) / d rounded down. + function mulDivDown(uint256 x, uint256 y, uint256 d) internal pure returns (uint256) { + return (x * y) / d; } - /// @dev (x * y) / denominator rounded up. - function mulDivUp(uint256 x, uint256 y, uint256 denominator) internal pure returns (uint256) { - return (x * y + (denominator - 1)) / denominator; + /// @dev (x * y) / d rounded up. + function mulDivUp(uint256 x, uint256 y, uint256 d) internal pure returns (uint256) { + return (x * y + (d - 1)) / d; } - /// @dev The sum of the last three terms in a four term taylor series expansion - /// to approximate a continuous compound interest rate: e^(nx) - 1. + /// @dev The first three non-zero terms of a Taylor expansion of e^(nx) - 1, + /// to approximate a continuous compound interest rate. function wTaylorCompounded(uint256 x, uint256 n) internal pure returns (uint256) { uint256 firstTerm = x * n; uint256 secondTerm = mulDivDown(firstTerm, firstTerm, 2 * WAD); diff --git a/src/libraries/UtilsLib.sol b/src/libraries/UtilsLib.sol index 616fd8ab4..18e39bd5f 100644 --- a/src/libraries/UtilsLib.sol +++ b/src/libraries/UtilsLib.sol @@ -9,14 +9,14 @@ import {ErrorsLib} from "../libraries/ErrorsLib.sol"; /// @notice Library exposing helpers. /// @dev Inspired by https://github.com/morpho-org/morpho-utils. library UtilsLib { - /// @dev Returns true if there is exactly one zero. + /// @dev Returns true if there is exactly one zero among `x` and `y`. function exactlyOneZero(uint256 x, uint256 y) internal pure returns (bool z) { assembly { z := xor(iszero(x), iszero(y)) } } - /// @dev Returns the min of x and y. + /// @dev Returns the min of `x` and `y`. function min(uint256 x, uint256 y) internal pure returns (uint256 z) { assembly { z := xor(x, mul(xor(x, y), lt(y, x))) From 8fab412d16727ab3cf161e892b7360c20aad60ee Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 24 Aug 2023 15:09:20 +0200 Subject: [PATCH 2/5] fix: user to position --- src/interfaces/IMorpho.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index 9e7b4bc4c..3403f74e6 100644 --- a/src/interfaces/IMorpho.sol +++ b/src/interfaces/IMorpho.sol @@ -65,7 +65,7 @@ interface IMorpho { function feeRecipient() external view returns (address); /// @notice The state of the position of `user` on the market corresponding to `id`. - function user(Id id, address user) + function position(Id id, address user) external view returns (uint256 supplyShares, uint128 borrowShares, uint128 collateral); From 40c232795c5e24ed8562b500b27ca2599efb60bb Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 24 Aug 2023 15:12:22 +0200 Subject: [PATCH 3/5] fix: morpho interfaces name conflicts --- src/interfaces/IMorpho.sol | 2 +- src/libraries/EventsLib.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index 3403f74e6..e1731fe4f 100644 --- a/src/interfaces/IMorpho.sol +++ b/src/interfaces/IMorpho.sol @@ -249,7 +249,7 @@ interface IMorpho { uint256 seizedAssets, uint256 repaidShares, bytes memory data - ) external returns (uint256 seizedAssets, uint256 repaidAssets); + ) external returns (uint256, uint256); /// @notice Executes a flash loan. /// @param token The token to flash loan. diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 8163ab18a..a0fa92494 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -103,9 +103,9 @@ library EventsLib { /// @param id The market id. /// @param caller The caller. /// @param borrower The borrower of the position. - /// @param repaid The amount of assets repaid. + /// @param repaidAssets The amount of assets repaid. /// @param repaidShares The amount of shares burned. - /// @param seized The amount of collateral seized. + /// @param seizedAssets The amount of collateral seized. /// @param badDebtShares The amount of shares minted as bad debt. event Liquidate( Id indexed id, From 5d85676e7c6ca42ed3f71258bab726d9bc081043 Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 24 Aug 2023 18:43:10 +0200 Subject: [PATCH 4/5] docs: taylor comment --- src/libraries/MathLib.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/MathLib.sol b/src/libraries/MathLib.sol index b04f2db0b..02b507c8a 100644 --- a/src/libraries/MathLib.sol +++ b/src/libraries/MathLib.sol @@ -33,7 +33,7 @@ library MathLib { return (x * y + (d - 1)) / d; } - /// @dev The first three non-zero terms of a Taylor expansion of e^(nx) - 1, + /// @dev The sum of the first three non-zero terms of a Taylor expansion of e^(nx) - 1, /// to approximate a continuous compound interest rate. function wTaylorCompounded(uint256 x, uint256 n) internal pure returns (uint256) { uint256 firstTerm = x * n; From 036c8ae1ccceb1e303786d957c94928121f4249e Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Fri, 25 Aug 2023 17:20:35 +0200 Subject: [PATCH 5/5] fix: revert removal of warning on set owner as the 0 address --- src/interfaces/IMorpho.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/interfaces/IMorpho.sol b/src/interfaces/IMorpho.sol index e1731fe4f..5129a6e65 100644 --- a/src/interfaces/IMorpho.sol +++ b/src/interfaces/IMorpho.sol @@ -106,6 +106,7 @@ interface IMorpho { /// @notice Sets `newOwner` as owner of the contract. /// @dev Warning: No two-step transfer ownership. + /// @dev Warning: The owner can be set to the zero address. function setOwner(address newOwner) external; /// @notice Enables `irm` as a possible IRM for market creation.