Illuminate contest - z3s's results

Non Critical

[N01] For critical changes it's better to use two-step procedure:

lender/Lender.sol
  129,14:     function setAdmin(address a) external authorized(admin) returns (bool) {

marketplace/MarketPlace.sol
  109,14:     function setAdmin(address a) external authorized(admin) returns (bool) {

redeemer/Redeemer.sol
  62,14:     function setAdmin(address a) external authorized(admin) returns (bool) {

Recommended Mitigation Steps

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

Gas Optimizations

[G01] state variables should be cached:

Caching will replace each Gwarmaccess (100 gas) with a much cheaper stack read.

File: redeemer/Redeemer.sol#L107

cache lender

function redeem(
    uint8 p,
    address u,
    uint256 m,
    address o
) public returns (bool) {
    // Get the address of the principal token being redeemed
    address principal = IMarketPlace(marketPlace).markets(u, m, p);

    if (p == uint8(MarketPlace.Principals.Illuminate)) {
        // Get Illuminate's principal token
        IERC5095 token = IERC5095(principal);
        // Get the amount of tokens to be redeemed from the sender
        uint256 amount = token.balanceOf(msg.sender);
        // Make sure the market has matured
        if (block.timestamp < token.maturity()) {
            revert Invalid('not matured');
        }
        // Burn the prinicipal token from Illuminate
        token.burn(o, amount);
        // Transfer the original underlying token back to the user
        Safe.transferFrom(IERC20(u), lender, address(this), amount); // @audit cache lender

        emit Redeem(0, u, m, amount);
    }
    else {
        // Get the amount of tokens to be redeemed from the principal token
        uint256 amount = IERC20(principal).balanceOf(lender); // @audit cache lender
        // Transfer the principal token from the lender contract to here
        Safe.transferFrom(IERC20(u), lender, address(this), amount); // @audit cache lender

        if (p == uint8(MarketPlace.Principals.Apwine)) {
            // Redeem the underlying token from APWine to Illuminate
            IAPWine(apwineAddr).withdraw(o, amount);
        } else if (p == uint8(MarketPlace.Principals.Tempus)) {
            // Redeem the tokens from the Tempus contract to Illuminate
            ITempus(tempusAddr).redeemToBacking(o, amount, 0, address(this));
        } else {
            revert Invalid('principal');
        }
        emit Redeem(0, u, m, amount);
    }

    return true;
}

File: redeemer/Redeemer.sol#L158

cache lender and marketPlace

function redeem(
    uint8 p,
    address u,
    uint256 m
) public returns (bool) {
    // Get the principal token that is being redeemed by the user
    address principal = IMarketPlace(marketPlace).markets(u, m, p); // @audit cache marketPlace

    // Make sure we have the correct principal
    if (
        p != uint8(MarketPlace.Principals.Swivel) &&
        p != uint8(MarketPlace.Principals.Element) &&
        p != uint8(MarketPlace.Principals.Yield) &&
        p != uint8(MarketPlace.Principals.Notional)
    ) {
        revert Invalid('principal');
    }

    // The amount redeemed should be the balance of the principal token held by the Illuminate contract
    uint256 amount = IERC20(principal).balanceOf(lender); // @audit cache lender

    // Transfer the principal token from the lender contract to here
    Safe.transferFrom(IERC20(principal), lender, address(this), amount); // @audit cache lender

    if (p == uint8(MarketPlace.Principals.Swivel)) {
        // Redeems zc tokens to the sender's address
        ISwivel(swivelAddr).redeemZcToken(u, m, amount);
    } else if (p == uint8(MarketPlace.Principals.Element)) {
        // Redeems principal tokens from element
        IElementToken(principal).withdrawPrincipal(amount, marketPlace); // @audit cache marketPlace
    } else if (p == uint8(MarketPlace.Principals.Yield)) {
        // Redeems prinicipal tokens from yield
        IYieldToken(principal).redeem(address(this), address(this), amount);
    } else if (p == uint8(MarketPlace.Principals.Notional)) {
        // Redeems the principal token from notional
        amount = INotional(principal).maxRedeem(address(this));
    }

    emit Redeem(p, u, m, amount);
    return true;
}

File: redeemer/Redeemer.sol#L206

cache lender

function redeem(
    uint8 p,
    address u,
    uint256 m,
    bytes32 i
) public returns (bool) {
    // Check the principal is Pendle
    if (p != uint8(MarketPlace.Principals.Pendle)) {
        revert Invalid('principal');
    }

    // Get the principal token that is being redeemed by the user
    IERC20 token = IERC20(IMarketPlace(marketPlace).markets(u, m, p));

    // Get the balance of tokens to be redeemed by the user
    uint256 amount = token.balanceOf(lender); // @audit cache lender

    // Transfer the user's tokens to the redeem contract
    Safe.transferFrom(token, lender, address(this), amount); // @audit cache lender

    // Redeem the tokens from the Pendle contract
    IPendle(pendleAddr).redeemAfterExpiry(i, u, m);
    
    emit Redeem(p, u, m, amount);
    return true;
}

File: redeemer/Redeemer.sol#L240

cache lender

function redeem(
    uint8 p,
    address u,
    uint256 m,
    address d,
    address o
) public returns (bool) {
    // Check the principal is Sense
    if (p != uint8(MarketPlace.Principals.Sense)) {
        revert Invalid('principal');
    }

    // Get the principal token for the given market
    IERC20 token = IERC20(IMarketPlace(marketPlace).markets(u, m, p));

    // Get the balance of tokens to be redeemed by the user
    uint256 amount = token.balanceOf(lender); // @audit cache lender

    // Transfer the user's tokens to the redeem contract
    Safe.transferFrom(token, lender, address(this), amount); // @audit cache lender

    // Redeem the tokens from the Sense contract
    ISense(d).redeem(o, m, amount);

    emit Redeem(p, u, m, amount);
    return true;
}

File: lender/Lender.sol#L145

cache marketPlace

function setMarketPlace(address m) external authorized(admin) returns (bool) {
    if (marketPlace != address(0)) { // @audit cache marketPlace
        revert Exists(marketPlace); // @audit cache marketPlace
    }
    marketPlace = m;
    return true;
}

File: lender/Lender.sol#L680

cache feenominator

function calculateFee(uint256 a) internal view returns (uint256) {
    return feenominator > 0 ? a / feenominator : 0; // @audit cache feenominator
}

[G02] a = a + b is cheaper than a += b:

lender/Lender.sol
  279,30:                     totalFee += fee;
  283,26:                     lent += amountLent;
  285,30:                     returned += amountLent * order.premium / order.principal;
  294,21:             fees[u] += totalFee;
  340,17:         fees[u] += calculateFee(a);
  404,21:             fees[u] += fee;
  461,17:         fees[u] += fee;
  514,21:             fees[u] += fee;
  572,21:             fees[u] += fee;
  621,17:         fees[u] += fee;

[G03] cache <array>.length:

Even memory arrays incur the overhead of bit tests and bit shifts to calculate the array length.

lender/Lender.sol
  265,13:             for (uint256 i = 0; i < o.length; ) {

[G04] ++i is cheaper than i++:

actually it's important in iterations.

lender/Lender.sol
  96,17:                 i++;
  120,17:                 i++;
  289,21:                     i++;

[G05] uint256 default value is 0 so we can remove = 0 for saving some gas:

lender/Lender.sol
  265,28:             for (uint256 i = 0; i < o.length; ) {

[G06] Use Custom Errors to save Gas:

Custom errors from Solidity 0.8.4 are cheaper than require messages. https://blog.soliditylang.org/2021/04/21/custom-errors/

lender/Lender.sol
  710,9:         require (when != 0, 'no withdrawal scheduled');
  712,9:         require (block.timestamp >= when, 'withdrawal still on hold');

[G07] functions guaranteed to revert when called by normal users can be marked payable:

If a function modifier such as authorized(admin) is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

lender/Lender.sol
  82,16:     ) external authorized(admin) returns (bool) {
  107,75:     function approve(address[] calldata u, address[] calldata a) external authorized(admin) returns (bool) {
  129,43:     function setAdmin(address a) external authorized(admin) returns (bool) {
  137,41:     function setFee(uint256 f) external authorized(admin) returns (bool) {
  145,49:     function setMarketPlace(address m) external authorized(admin) returns (bool) {
  156,44:     function setSwivel(address s) external authorized(admin) returns (bool) {
  662,46:     function withdrawFee(address e) external authorized(admin) returns (bool) {
  687,53:     function scheduleWithdrawal(address e) external authorized(admin) returns (bool) {
  698,50:     function blockWithdrawal(address e) external authorized(admin) returns (bool) {
  708,43:     function withdraw(address e) external authorized(admin) returns (bool) {
  734,46:     function pause(uint8 p, bool b) external authorized(admin) returns (bool) {

marketplace/MarketPlace.sol
  74,16:     ) external authorized(admin) returns (bool) {
  98,78:     function setPrincipal(uint8 p, address u, uint256 m, address a) external authorized(admin) returns (bool) {
  109,43:     function setAdmin(address a) external authorized(admin) returns (bool) {
  123,16:     ) external authorized(admin) returns (bool) {

redeemer/Redeemer.sol
  62,43:     function setAdmin(address a) external authorized(admin) returns (bool) {
  70,49:     function setMarketPlace(address m) external authorized(admin) returns (bool) {
  81,44:     function setLender(address l) external authorized(admin) returns (bool) {
  92,44:     function setSwivel(address s) external authorized(admin) returns (bool) {