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

refactor(errors): use custom errors #141

Merged
merged 1 commit into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .github/workflows/foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
- uses: ./.github/actions/install

- name: Build contracts via IR & check sizes
run: yarn build:forge --force # don't use compilation cache
run: yarn build:forge --force --sizes # don't use compilation cache

build-no-ir:
name: Compilation (without IR)
Expand Down
5 changes: 2 additions & 3 deletions foundry.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
[profile.default]
names = true
sizes = true
via-ir = false
src = "src"
test = "test/forge"
evm_version = "paris"
fs_permissions = [
Expand All @@ -18,6 +16,7 @@ wrap_comments = true


[profile.build]
via-ir = true
test = "/dev/null"
script = "/dev/null"

Expand Down
73 changes: 38 additions & 35 deletions src/MetaMorpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
string memory _name,
string memory _symbol
) ERC4626(IERC20(_asset)) ERC20Permit(_name) ERC20(_name, _symbol) {
require(initialTimelock <= MAX_TIMELOCK, ErrorsLib.MAX_TIMELOCK_EXCEEDED);
if (initialTimelock > MAX_TIMELOCK) revert ErrorsLib.MaxTimelockExceeded();

_transferOwnership(owner);

Expand All @@ -93,60 +93,62 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
/* MODIFIERS */

modifier onlyRiskManager() {
require(_msgSender() == riskManager || _msgSender() == owner(), ErrorsLib.NOT_RISK_MANAGER);
if (_msgSender() != riskManager && _msgSender() != owner()) revert ErrorsLib.NotRiskManager();

_;
}

modifier onlyGuardian() {
require(_msgSender() == guardian, ErrorsLib.NOT_GUARDIAN);
if (_msgSender() != guardian) revert ErrorsLib.NotGuardian();

_;
}

modifier onlyAllocator() {
require(isAllocator(_msgSender()), ErrorsLib.NOT_ALLOCATOR);
if (!isAllocator(_msgSender())) revert ErrorsLib.NotAllocator();

_;
}

modifier timelockElapsed(uint256 submittedAt) {
require(submittedAt != 0, ErrorsLib.NO_PENDING_VALUE);
require(block.timestamp >= submittedAt + timelock, ErrorsLib.TIMELOCK_NOT_ELAPSED);
require(block.timestamp <= submittedAt + timelock + TIMELOCK_EXPIRATION, ErrorsLib.TIMELOCK_EXPIRATION_EXCEEDED);
if (submittedAt == 0) revert ErrorsLib.NoPendingValue();
if (block.timestamp < submittedAt + timelock) revert ErrorsLib.TimelockNotElapsed();
if (block.timestamp > submittedAt + timelock + TIMELOCK_EXPIRATION) {
revert ErrorsLib.TimelockExpirationExceeded();
}

_;
}

/* ONLY OWNER FUNCTIONS */

function setRiskManager(address newRiskManager) external onlyOwner {
require(newRiskManager != riskManager, ErrorsLib.ALREADY_SET);
if (newRiskManager == riskManager) revert ErrorsLib.AlreadySet();

riskManager = newRiskManager;

emit EventsLib.SetRiskManager(newRiskManager);
}

function setIsAllocator(address newAllocator, bool newIsAllocator) external onlyOwner {
require(_isAllocator[newAllocator] != newIsAllocator, ErrorsLib.ALREADY_SET);
if (_isAllocator[newAllocator] == newIsAllocator) revert ErrorsLib.AlreadySet();

_isAllocator[newAllocator] = newIsAllocator;

emit EventsLib.SetIsAllocator(newAllocator, newIsAllocator);
}

function setRewardsDistributor(address newRewardsDistributor) external onlyOwner {
require(newRewardsDistributor != rewardsDistributor, ErrorsLib.ALREADY_SET);
if (newRewardsDistributor == rewardsDistributor) revert ErrorsLib.AlreadySet();

rewardsDistributor = newRewardsDistributor;

emit EventsLib.SetRewardsDistributor(newRewardsDistributor);
}

function submitTimelock(uint256 newTimelock) external onlyOwner {
require(newTimelock <= MAX_TIMELOCK, ErrorsLib.MAX_TIMELOCK_EXCEEDED);
require(newTimelock != timelock, ErrorsLib.ALREADY_SET);
if (newTimelock > MAX_TIMELOCK) revert ErrorsLib.MaxTimelockExceeded();
if (newTimelock == timelock) revert ErrorsLib.AlreadySet();

if (newTimelock > timelock || timelock == 0) {
_setTimelock(newTimelock);
Expand All @@ -163,8 +165,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
}

function submitFee(uint256 newFee) external onlyOwner {
require(newFee <= MAX_FEE, ErrorsLib.MAX_FEE_EXCEEDED);
require(newFee != fee, ErrorsLib.ALREADY_SET);
if (newFee > MAX_FEE) revert ErrorsLib.MaxFeeExceeded();
if (newFee == fee) revert ErrorsLib.AlreadySet();

if (newFee < fee || timelock == 0) {
_setFee(newFee);
Expand All @@ -181,8 +183,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
}

function setFeeRecipient(address newFeeRecipient) external onlyOwner {
require(newFeeRecipient != feeRecipient, ErrorsLib.ALREADY_SET);
require(newFeeRecipient != address(0) || fee == 0, ErrorsLib.ZERO_FEE_RECIPIENT);
if (newFeeRecipient == feeRecipient) revert ErrorsLib.AlreadySet();
if (newFeeRecipient == address(0) && fee != 0) revert ErrorsLib.ZeroFeeRecipient();

// Accrue interest to the previous fee recipient set before changing it.
_updateLastTotalAssets(_accrueFee());
Expand All @@ -193,8 +195,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
}

function submitGuardian(address newGuardian) external onlyOwner {
require(timelock != 0, ErrorsLib.NO_TIMELOCK);
require(newGuardian != guardian, ErrorsLib.ALREADY_SET);
if (timelock == 0) revert ErrorsLib.NoTimelock();
if (newGuardian == guardian) revert ErrorsLib.AlreadySet();

if (guardian == address(0)) {
_setGuardian(newGuardian);
Expand All @@ -212,13 +214,13 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
/* ONLY RISK MANAGER FUNCTIONS */

function submitCap(MarketParams memory marketParams, uint256 newMarketCap) external onlyRiskManager {
require(marketParams.loanToken == asset(), ErrorsLib.INCONSISTENT_ASSET);
if (marketParams.loanToken != asset()) revert ErrorsLib.InconsistentAsset();

Id id = marketParams.id();
require(MORPHO.lastUpdate(id) != 0, ErrorsLib.MARKET_NOT_CREATED);
if (MORPHO.lastUpdate(id) == 0) revert ErrorsLib.MarketNotCreated();

uint256 marketCap = config[id].cap;
require(newMarketCap != marketCap, ErrorsLib.ALREADY_SET);
if (newMarketCap == marketCap) revert ErrorsLib.AlreadySet();

if (newMarketCap < marketCap || timelock == 0) {
_setCap(id, newMarketCap.toUint192());
Expand All @@ -241,7 +243,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
uint256 length = newSupplyQueue.length;

for (uint256 i; i < length; ++i) {
require(config[newSupplyQueue[i]].cap > 0, ErrorsLib.UNAUTHORIZED_MARKET);
if (config[newSupplyQueue[i]].cap == 0) revert ErrorsLib.UnauthorizedMarket();
}

supplyQueue = newSupplyQueue;
Expand All @@ -260,14 +262,13 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph

for (uint256 i; i < newLength; ++i) {
uint256 prevIndex = indexes[i];
Id id = withdrawQueue[prevIndex];

// If prevIndex >= currLength, reverts with native "Index out of bounds".
require(!seen[prevIndex], ErrorsLib.DUPLICATE_MARKET);
if (seen[prevIndex]) revert ErrorsLib.DuplicateMarket(id);

seen[prevIndex] = true;

Id id = withdrawQueue[prevIndex];

newWithdrawQueue[i] = id;

// Safe "unchecked" cast because i < currLength.
Expand All @@ -278,7 +279,9 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
if (!seen[i]) {
Id id = withdrawQueue[i];

require(MORPHO.supplyShares(id, address(this)) == 0 && config[id].cap == 0, ErrorsLib.MISSING_MARKET);
if (MORPHO.supplyShares(id, address(this)) != 0 || config[id].cap != 0) {
revert ErrorsLib.MissingMarket(id);
}

delete config[id].withdrawRank;
}
Expand Down Expand Up @@ -317,17 +320,17 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph

totalSupplied += suppliedAssets;

require(
_supplyBalance(allocation.marketParams) <= config[allocation.marketParams.id()].cap,
ErrorsLib.SUPPLY_CAP_EXCEEDED
);
Id id = allocation.marketParams.id();
if (_supplyBalance(allocation.marketParams) > config[id].cap) {
revert ErrorsLib.SupplyCapExceeded(id);
}
}

if (totalWithdrawn > totalSupplied) {
idle += totalWithdrawn - totalSupplied;
} else {
uint256 idleSupplied = totalSupplied - totalWithdrawn;
require(idle >= idleSupplied, ErrorsLib.INSUFFICIENT_IDLE);
if (idle < idleSupplied) revert ErrorsLib.InsufficientIdle();

idle -= idleSupplied;
}
Expand All @@ -336,7 +339,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
/* EXTERNAL */

function transferRewards(address token) external {
require(rewardsDistributor != address(0), ErrorsLib.ZERO_ADDRESS);
if (rewardsDistributor == address(0)) revert ErrorsLib.ZeroAddress();

uint256 amount = IERC20(token).balanceOf(address(this));
if (token == asset()) amount -= idle;
Expand Down Expand Up @@ -513,7 +516,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
internal
override
{
require(_withdrawMorpho(assets) == 0, ErrorsLib.WITHDRAW_FAILED_MORPHO);
if (_withdrawMorpho(assets) != 0) revert ErrorsLib.WithdrawMorphoFailed();

super._withdraw(caller, receiver, owner, assets, shares);
}
Expand Down Expand Up @@ -551,7 +554,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
supplyQueue.push(id);
withdrawQueue.push(id);

require(withdrawQueue.length <= MAX_QUEUE_SIZE, ErrorsLib.MAX_QUEUE_SIZE_EXCEEDED);
if (withdrawQueue.length > MAX_QUEUE_SIZE) revert ErrorsLib.MaxQueueSizeExceeded();

// Safe "unchecked" cast because withdrawQueue.length <= MAX_QUEUE_SIZE.
marketConfig.withdrawRank = uint64(withdrawQueue.length);
Expand All @@ -565,7 +568,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
}

function _setFee(uint256 newFee) internal {
require(newFee == 0 || feeRecipient != address(0), ErrorsLib.ZERO_FEE_RECIPIENT);
if (newFee != 0 && feeRecipient == address(0)) revert ErrorsLib.ZeroFeeRecipient();

// Accrue interest using the previous fee set before changing it.
_updateLastTotalAssets(_accrueFee());
Expand Down
2 changes: 1 addition & 1 deletion src/MetaMorphoFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract MetaMorphoFactory {
/* CONSTRCUTOR */

constructor(address morpho) {
require(morpho != address(0), ErrorsLib.ZERO_ADDRESS);
if (morpho == address(0)) revert ErrorsLib.ZeroAddress();

MORPHO = morpho;
}
Expand Down
44 changes: 23 additions & 21 deletions src/libraries/ErrorsLib.sol
Original file line number Diff line number Diff line change
@@ -1,48 +1,50 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {Id} from "@morpho-blue/interfaces/IMorpho.sol";

library ErrorsLib {
string internal constant ZERO_ADDRESS = "zero address";
error ZeroAddress();

string internal constant NOT_RISK_MANAGER = "not risk manager";
error NotRiskManager();

string internal constant NOT_ALLOCATOR = "not allocator";
error NotAllocator();

string internal constant NOT_GUARDIAN = "not guardian";
error NotGuardian();

string internal constant UNAUTHORIZED_MARKET = "unauthorized market";
error UnauthorizedMarket();

string internal constant INCONSISTENT_ASSET = "inconsistent asset";
error InconsistentAsset();

string internal constant SUPPLY_CAP_EXCEEDED = "supply cap exceeded";
error SupplyCapExceeded(Id id);

/// @notice Thrown when the fee to set exceeds the maximum fee.
string internal constant MAX_FEE_EXCEEDED = "max fee exceeded";
error MaxFeeExceeded();

/// @notice Thrown when the value is already set.
string internal constant ALREADY_SET = "already set";
error AlreadySet();

string internal constant NO_TIMELOCK = "no timelock";
error NoTimelock();

string internal constant DUPLICATE_MARKET = "duplicate market";
error DuplicateMarket(Id id);

string internal constant MISSING_MARKET = "missing market";
error MissingMarket(Id id);

string internal constant NO_PENDING_VALUE = "no pending value";
error NoPendingValue();

string internal constant WITHDRAW_FAILED_MORPHO = "withdraw failed on Morpho";
error WithdrawMorphoFailed();

string internal constant MARKET_NOT_CREATED = "market not created";
error MarketNotCreated();

string internal constant MAX_TIMELOCK_EXCEEDED = "max timelock exceeded";
error MaxTimelockExceeded();

string internal constant TIMELOCK_NOT_ELAPSED = "timelock not elapsed";
error TimelockNotElapsed();

string internal constant TIMELOCK_EXPIRATION_EXCEEDED = "timelock expiration exceeded";
error TimelockExpirationExceeded();

string internal constant MAX_QUEUE_SIZE_EXCEEDED = "max queue size exceeded";
error MaxQueueSizeExceeded();

string internal constant ZERO_FEE_RECIPIENT = "fee recipient is zero";
error ZeroFeeRecipient();

string internal constant INSUFFICIENT_IDLE = "insufficient idle liquidity";
error InsufficientIdle();
}
4 changes: 2 additions & 2 deletions test/forge/ERC4626Test.sol
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ contract ERC4626Test is BaseTest {
assets = bound(assets, deposited + 1, type(uint256).max / (deposited + 10 ** DECIMALS_OFFSET));

vm.prank(ONBEHALF);
vm.expectRevert(bytes(ErrorsLib.WITHDRAW_FAILED_MORPHO));
vm.expectRevert(ErrorsLib.WithdrawMorphoFailed.selector);
vault.withdraw(assets, RECEIVER, ONBEHALF);
}

Expand All @@ -222,7 +222,7 @@ contract ERC4626Test is BaseTest {
vm.stopPrank();

vm.prank(ONBEHALF);
vm.expectRevert(bytes(ErrorsLib.WITHDRAW_FAILED_MORPHO));
vm.expectRevert(ErrorsLib.WithdrawMorphoFailed.selector);
vault.withdraw(assets, RECEIVER, ONBEHALF);
}

Expand Down
4 changes: 2 additions & 2 deletions test/forge/FeeTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,13 @@ contract FeeTest is BaseTest {
fee = bound(fee, MAX_FEE + 1, type(uint256).max);

vm.prank(OWNER);
vm.expectRevert(bytes(ErrorsLib.MAX_FEE_EXCEEDED));
vm.expectRevert(ErrorsLib.MaxFeeExceeded.selector);
vault.submitFee(fee);
}

function testSubmitFeeAlreadySet() public {
vm.prank(OWNER);
vm.expectRevert(bytes(ErrorsLib.ALREADY_SET));
vm.expectRevert(ErrorsLib.AlreadySet.selector);
vault.submitFee(FEE);
}

Expand Down
Loading