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

various fixes #630

Merged
merged 20 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/Morpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -397,11 +397,11 @@ contract Morpho is IMorphoStaticTyping {
position[id][borrower].borrowShares = 0;
}

IERC20(marketParams.collateralToken).safeTransfer(msg.sender, seizedAssets);

// `repaidAssets` may be greater than `totalBorrowAssets` by 1.
emit EventsLib.Liquidate(id, msg.sender, borrower, repaidAssets, repaidShares, seizedAssets, badDebtShares);

IERC20(marketParams.collateralToken).safeTransfer(msg.sender, seizedAssets);

if (data.length > 0) IMorphoLiquidateCallback(msg.sender).onMorphoLiquidate(repaidAssets, data);

IERC20(marketParams.loanToken).safeTransferFrom(msg.sender, address(this), repaidAssets);
Expand All @@ -413,10 +413,12 @@ contract Morpho is IMorphoStaticTyping {

/// @inheritdoc IMorphoBase
function flashLoan(address token, uint256 assets, bytes calldata data) external {
IERC20(token).safeTransfer(msg.sender, assets);
require(assets != 0, ErrorsLib.ZERO_ASSETS);

emit EventsLib.FlashLoan(msg.sender, token, assets);
Rubilmax marked this conversation as resolved.
Show resolved Hide resolved

IERC20(token).safeTransfer(msg.sender, assets);

IMorphoFlashLoanCallback(msg.sender).onMorphoFlashLoan(assets, data);

IERC20(token).safeTransferFrom(msg.sender, address(this), assets);
Expand All @@ -426,18 +428,24 @@ contract Morpho is IMorphoStaticTyping {

/// @inheritdoc IMorphoBase
function setAuthorization(address authorized, bool newIsAuthorized) external {
require(newIsAuthorized != isAuthorized[msg.sender][authorized], ErrorsLib.ALREADY_SET);
MathisGD marked this conversation as resolved.
Show resolved Hide resolved

isAuthorized[msg.sender][authorized] = newIsAuthorized;

emit EventsLib.SetAuthorization(msg.sender, msg.sender, authorized, newIsAuthorized);
}

/// @inheritdoc IMorphoBase
function setAuthorizationWithSig(Authorization memory authorization, Signature calldata signature) external {
require(
authorization.isAuthorized != isAuthorized[authorization.authorizer][authorization.authorized],
ErrorsLib.ALREADY_SET
);
MerlinEgalite marked this conversation as resolved.
Show resolved Hide resolved
require(block.timestamp <= authorization.deadline, ErrorsLib.SIGNATURE_EXPIRED);
require(authorization.nonce == nonce[authorization.authorizer]++, ErrorsLib.INVALID_NONCE);

bytes32 hashStruct = keccak256(abi.encode(AUTHORIZATION_TYPEHASH, authorization));
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, hashStruct));
bytes32 digest = keccak256(bytes.concat("\x19\x01", DOMAIN_SEPARATOR, hashStruct));
address signatory = ecrecover(digest, signature.v, signature.r, signature.s);

require(signatory != address(0) && authorization.authorizer == signatory, ErrorsLib.INVALID_SIGNATURE);
Expand Down
5 changes: 3 additions & 2 deletions src/interfaces/IIrm.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import {MarketParams, Market} from "./IMorpho.sol";
/// @custom:contact security@morpho.org
/// @notice Interface that Interest Rate Models (IRMs) used by Morpho must implement.
interface IIrm {
/// @notice Returns the borrow rate of the market `marketParams`.
/// @notice Returns the borrow rate per second (scaled by wad) of the market `marketParams`.
MathisGD marked this conversation as resolved.
Show resolved Hide resolved
/// @dev Assumes that `market` corresponds to `marketParams`.
function borrowRate(MarketParams memory marketParams, Market memory market) external returns (uint256);

/// @notice Returns the borrow rate of the market `marketParams` without modifying any storage.
/// @notice Returns the borrow rate per second (scaled by wad) of the market `marketParams` without modifying any
MathisGD marked this conversation as resolved.
Show resolved Hide resolved
/// storage.
/// @dev Assumes that `market` corresponds to `marketParams`.
function borrowRateView(MarketParams memory marketParams, Market memory market) external view returns (uint256);
}
11 changes: 8 additions & 3 deletions src/interfaces/IMorpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ interface IMorphoBase {
/// @notice Whether the `lltv` is enabled.
function isLltvEnabled(uint256 lltv) external view returns (bool);

/// @notice Whether `authorized` is authorized to modify `authorizer`'s positions.
/// @notice Whether `authorized` is authorized to modify `authorizer`'s position on all markets.
/// @dev Anyone is authorized to modify their own positions, regardless of this variable.
function isAuthorized(address authorizer, address authorized) external view returns (bool);

Expand All @@ -91,6 +91,7 @@ interface IMorphoBase {
function enableLltv(uint256 lltv) external;

/// @notice Sets the `newFee` for the given market `marketParams`.
/// @param newFee The new fee, scaled by wad.
/// @dev Warning: The recipient can be the zero address.
function setFee(MarketParams memory marketParams, uint256 newFee) external;

Expand Down Expand Up @@ -129,7 +130,8 @@ interface IMorphoBase {

/// @notice Supplies `assets` or `shares` on behalf of `onBehalf`, optionally calling back the caller's
/// `onMorphoSupply` function with the given `data`.
/// @dev Either `assets` or `shares` should be zero. Most usecases should rely on `assets` as an input so the caller
/// @dev Either `assets` or `shares` should be zero. Most use cases should rely on `assets` as an input so the
/// caller
MathisGD marked this conversation as resolved.
Show resolved Hide resolved
/// is guaranteed to have `assets` tokens pulled from their balance, but the possibility to mint a specific amount
/// of shares is given for full compatibility and precision.
/// @dev If the supply of a market gets depleted, the supply share price instantly resets to
Expand Down Expand Up @@ -172,7 +174,8 @@ interface IMorphoBase {
) external returns (uint256 assetsWithdrawn, uint256 sharesWithdrawn);

/// @notice Borrows `assets` or `shares` on behalf of `onBehalf` to `receiver`.
/// @dev Either `assets` or `shares` should be zero. Most usecases should rely on `assets` as an input so the caller
/// @dev Either `assets` or `shares` should be zero. Most use cases should rely on `assets` as an input so the
/// caller
MathisGD marked this conversation as resolved.
Show resolved Hide resolved
/// is guaranteed to borrow `assets` of tokens, but the possibility to mint a specific amount of shares is given for
/// full compatibility and precision.
/// @dev If the borrow of a market gets depleted, the borrow share price instantly resets to
Expand Down Expand Up @@ -200,6 +203,7 @@ interface IMorphoBase {
/// @dev Repaying an amount corresponding to more shares than borrowed will revert for underflow.
/// @dev It is advised to use the `shares` input when repaying the full position to avoid reverts due to conversion
/// roundings between shares and assets.
/// @dev An attacker can front-run a repay with a small repay making the transaction revert for underflow.
/// @param marketParams The market to repay assets to.
/// @param assets The amount of assets to repay.
/// @param shares The amount of shares to burn.
Expand Down Expand Up @@ -242,6 +246,7 @@ interface IMorphoBase {
/// @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.
/// @dev An attacker can front-run a liquidation with a small repay making the transaction revert for underflow.
Rubilmax marked this conversation as resolved.
Show resolved Hide resolved
/// @param marketParams The market of the position.
/// @param borrower The owner of the position.
/// @param seizedAssets The amount of collateral to seize.
Expand Down
13 changes: 7 additions & 6 deletions src/libraries/EventsLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,18 @@ library EventsLib {
event CreateMarket(Id indexed id, MarketParams marketParams);

/// @notice Emitted on supply of assets.
/// @dev Warning: `feeRecipient` receives some shares during interest accrual without any supply event emitted.
/// @param id The market id.
/// @param caller The caller.
/// @param onBehalf The address that received the supply.
/// @param onBehalf The owner of the modified position.
/// @param assets The amount of assets supplied.
/// @param shares The amount of shares minted.
event Supply(Id indexed id, address indexed caller, address indexed onBehalf, uint256 assets, uint256 shares);

/// @notice Emitted on withdrawal of assets.
/// @param id The market id.
/// @param caller The caller.
/// @param onBehalf The address from which the assets were withdrawn.
/// @param onBehalf The owner of the modified position.
/// @param receiver The address that received the withdrawn assets.
/// @param assets The amount of assets withdrawn.
/// @param shares The amount of shares burned.
Expand All @@ -61,7 +62,7 @@ library EventsLib {
/// @notice Emitted on borrow of assets.
/// @param id The market id.
/// @param caller The caller.
/// @param onBehalf The address from which the assets were borrowed.
/// @param onBehalf The owner of the modified position.
/// @param receiver The address that received the borrowed assets.
/// @param assets The amount of assets borrowed.
/// @param shares The amount of shares minted.
Expand All @@ -77,22 +78,22 @@ library EventsLib {
/// @notice Emitted on repayment of assets.
/// @param id The market id.
/// @param caller The caller.
/// @param onBehalf The address for which the assets were repaid.
/// @param onBehalf The owner of the modified position.
/// @param assets The amount of assets repaid. May be 1 over the corresponding market's `totalBorrowAssets`.
/// @param shares The amount of shares burned.
event Repay(Id indexed id, address indexed caller, address indexed onBehalf, uint256 assets, uint256 shares);

/// @notice Emitted on supply of collateral.
/// @param id The market id.
/// @param caller The caller.
/// @param onBehalf The address that received the collateral.
/// @param onBehalf The owner of the modified position.
/// @param assets The amount of collateral supplied.
event SupplyCollateral(Id indexed id, address indexed caller, address indexed onBehalf, uint256 assets);

/// @notice Emitted on withdrawal of collateral.
/// @param id The market id.
/// @param caller The caller.
/// @param onBehalf The address from which the collateral was withdrawn.
/// @param onBehalf The owner of the modified position.
/// @param receiver The address that received the withdrawn collateral.
/// @param assets The amount of collateral withdrawn.
event WithdrawCollateral(
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/UtilsLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ library UtilsLib {
return uint128(x);
}

/// @dev Returns max(x - y, 0).
/// @dev Returns max(0, x - y).
function zeroFloorSub(uint256 x, uint256 y) internal pure returns (uint256 z) {
assembly {
z := mul(gt(x, y), sub(x, y))
Expand Down
2 changes: 1 addition & 1 deletion test/forge/helpers/SigUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ library SigUtils {
pure
returns (bytes32)
{
return keccak256(abi.encodePacked("\x19\x01", domainSeparator, hashStruct(authorization)));
return keccak256(bytes.concat("\x19\x01", domainSeparator, hashStruct(authorization)));
MerlinEgalite marked this conversation as resolved.
Show resolved Hide resolved
}

function hashStruct(Authorization memory authorization) internal pure returns (bytes32) {
Expand Down
34 changes: 33 additions & 1 deletion test/forge/integration/AuthorizationIntegrationTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,38 @@ contract AuthorizationIntegrationTest is BaseTest {
assertFalse(morpho.isAuthorized(address(this), addressFuzz));
}

function testAlreadySet(address addressFuzz) public {
vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
morpho.setAuthorization(addressFuzz, false);

morpho.setAuthorization(addressFuzz, true);

vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
morpho.setAuthorization(addressFuzz, true);
}

function testAlreadySetWithSig(Authorization memory authorization, Signature memory sig) public {
authorization.isAuthorized = false;
authorization.authorizer = address(this);
authorization.deadline = block.timestamp;
authorization.nonce = 0;

vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
morpho.setAuthorizationWithSig(authorization, sig);

morpho.setAuthorization(authorization.authorized, true);

authorization.isAuthorized = true;
vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
morpho.setAuthorizationWithSig(authorization, sig);
}

function testSetAuthorizationWithSignatureDeadlineOutdated(
Authorization memory authorization,
uint256 privateKey,
uint256 blocks
) public {
authorization.isAuthorized = true;
blocks = _boundBlocks(blocks);
authorization.deadline = block.timestamp - 1;

Expand All @@ -40,6 +67,7 @@ contract AuthorizationIntegrationTest is BaseTest {
}

function testAuthorizationWithSigWrongPK(Authorization memory authorization, uint256 privateKey) public {
authorization.isAuthorized = true;
authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);

// Private key must be less than the secp256k1 curve order.
Expand All @@ -55,6 +83,7 @@ contract AuthorizationIntegrationTest is BaseTest {
}

function testAuthorizationWithSigWrongNonce(Authorization memory authorization, uint256 privateKey) public {
authorization.isAuthorized = true;
authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);
authorization.nonce = bound(authorization.nonce, 1, type(uint256).max);

Expand All @@ -71,6 +100,7 @@ contract AuthorizationIntegrationTest is BaseTest {
}

function testAuthorizationWithSig(Authorization memory authorization, uint256 privateKey) public {
authorization.isAuthorized = true;
authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);

// Private key must be less than the secp256k1 curve order.
Expand All @@ -84,11 +114,12 @@ contract AuthorizationIntegrationTest is BaseTest {

morpho.setAuthorizationWithSig(authorization, sig);

assertEq(morpho.isAuthorized(authorization.authorizer, authorization.authorized), authorization.isAuthorized);
assertEq(morpho.isAuthorized(authorization.authorizer, authorization.authorized), true);
assertEq(morpho.nonce(authorization.authorizer), 1);
}

function testAuthorizationFailsWithReusedSig(Authorization memory authorization, uint256 privateKey) public {
authorization.isAuthorized = true;
authorization.deadline = bound(authorization.deadline, block.timestamp, type(uint256).max);

// Private key must be less than the secp256k1 curve order.
Expand All @@ -102,6 +133,7 @@ contract AuthorizationIntegrationTest is BaseTest {

morpho.setAuthorizationWithSig(authorization, sig);

authorization.isAuthorized = false;
vm.expectRevert(bytes(ErrorsLib.INVALID_NONCE));
morpho.setAuthorizationWithSig(authorization, sig);
}
Expand Down
4 changes: 2 additions & 2 deletions test/forge/integration/BorrowIntegrationTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ contract BorrowIntegrationTest is BaseTest {
vm.startPrank(ONBEHALF);
collateralToken.approve(address(morpho), amountCollateral);
morpho.supplyCollateral(marketParams, amountCollateral, ONBEHALF, hex"");
morpho.setAuthorization(BORROWER, true);
// BORROWER is already authorized.
vm.stopPrank();

uint256 expectedBorrowShares = amountBorrowed.toSharesUp(0, 0);
Expand Down Expand Up @@ -248,7 +248,7 @@ contract BorrowIntegrationTest is BaseTest {
vm.startPrank(ONBEHALF);
collateralToken.approve(address(morpho), amountCollateral);
morpho.supplyCollateral(marketParams, amountCollateral, ONBEHALF, hex"");
morpho.setAuthorization(BORROWER, true);
// BORROWER is already authorized.
vm.stopPrank();

vm.prank(BORROWER);
Expand Down
5 changes: 5 additions & 0 deletions test/forge/integration/CallbacksIntegrationTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ contract CallbacksIntegrationTest is
assertEq(loanToken.balanceOf(address(morpho)), amount, "balanceOf");
}

function testFlashLoanZero() public {
vm.expectRevert(bytes(ErrorsLib.ZERO_ASSETS));
morpho.flashLoan(address(loanToken), 0, abi.encode(this.testFlashLoan.selector, hex""));
}

function testFlashLoanShouldRevertIfNotReimbursed(uint256 amount) public {
amount = bound(amount, 1, MAX_TEST_AMOUNT);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ contract WithdrawCollateralIntegrationTest is BaseTest {

vm.startPrank(ONBEHALF);
morpho.supplyCollateral(marketParams, amountCollateral + amountCollateralExcess, ONBEHALF, hex"");
morpho.setAuthorization(BORROWER, true);
// BORROWER is already authorized.
morpho.borrow(marketParams, amountBorrowed, 0, ONBEHALF, ONBEHALF);
vm.stopPrank();

Expand Down
8 changes: 5 additions & 3 deletions test/forge/invariant/MorphoInvariantTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,17 @@ contract MorphoInvariantTest is InvariantTest {
}

modifier authorized(address onBehalf) {
if (onBehalf != msg.sender) {
if (onBehalf != msg.sender && !morpho.isAuthorized(onBehalf, msg.sender)) {
vm.prank(onBehalf);
morpho.setAuthorization(msg.sender, true);
}

_;

vm.prank(onBehalf);
morpho.setAuthorization(msg.sender, false);
if (morpho.isAuthorized(onBehalf, msg.sender)) {
MerlinEgalite marked this conversation as resolved.
Show resolved Hide resolved
vm.prank(onBehalf);
morpho.setAuthorization(msg.sender, false);
}
}

function _randomMarket(uint256 marketSeed) internal view returns (MarketParams memory _marketParams) {
Expand Down
Loading