hyh - Redeemer autoRedeem will not have meaningful incentives in the case of high decimal underlyings

[sherlock-admin]

hyh

medium

Redeemer autoRedeem will not have meaningful incentives in the case of high decimal underlyings

Summary

Redeemer doesn't have the functionality to change the fee set on construction. For high decimal underlyings, for example DAI, initially set fee is just 4e-11 basis points, which provides no incentives for third parties to run autoRedeem().

Vulnerability Detail

feeChange is missing in Redeemer, which hard codes the feenominator to one set on construction, that is basically only feasible for 6 decimals underlying (40 bp in this case).

For all others, especially for 18 decimal ones, like DAI or LUSD, it is very close to zero and provides no incentives.

Impact

The funds that are normally retrieved via autoRedeem will remain on the balance. Say for accounts that are unable to run redeem directly for any reason.

Setting the severity to be medium as that's an unavailability of functionality leading to temporal funds freeze.

Code Snippet

setFee() will always revert if feeChange is zero:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L165-L187

    /// @notice sets the feenominator to the given value
    /// @param f the new value of the feenominator, fees are not collected when the feenominator is 0
    /// @return bool true if successful
    function setFee(uint256 f) external authorized(admin) returns (bool) {
        uint256 feeTime = feeChange;
        if (feeTime == 0) {
            revert Exception(23, 0, 0, address(0), address(0));
        } else if (feeTime < block.timestamp) {
            revert Exception(
                24,
                block.timestamp,
                feeTime,
                address(0),
                address(0)
            );
        } else if (f < MIN_FEENOMINATOR) {
            revert Exception(25, 0, 0, address(0), address(0));
        }
        feenominator = f;
        delete feeChange;
        emit SetFee(f);
        return true;
    }

But there is no functionality to set feeChange in Redeemer, it's always zero:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L58-L59

    /// @notice represents a point in time where the feenominator may change
    uint256 public feeChange;

autoRedeem() uses the fee for incentivizing the third-party redeem:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L479-L548

    /// @notice implements a redeem method to enable third-party redemptions
    /// @dev expects approvals from owners to redeemer
    /// @param u address of the underlying asset
    /// @param m maturity of the market
    /// @param f address from where the principal token will be burned
    /// @return uint256 amount of underlying yielded as a fee
    function autoRedeem(
        address u,
        uint256 m,
        address[] calldata f
    ) external returns (uint256) {
        // Get the principal token for the given market
        IERC5095 pt = IERC5095(IMarketPlace(marketPlace).token(u, m, 0));

        // Make sure the market has matured
        uint256 maturity = pt.maturity();
        if (block.timestamp < maturity) {
            revert Exception(7, maturity, 0, address(0), address(0));
        }

        // Retrieve the underlying
        IERC20 uToken = IERC20(u);

        // Sum up the fees received by the caller
        uint256 incentiveFee;

        // Get the number of owners to loop through
        uint256 length = f.length;

        // Loop through the provided arrays and mature each individual position
        for (uint256 i; i != length; ) {
            // Fetch the allowance set by the holder of the principal tokens
            uint256 allowance = uToken.allowance(f[i], address(this));

            // Get the amount of tokens held by the owner
            uint256 amount = pt.balanceOf(f[i]);

            // Calculate how many tokens the user should receive
            uint256 redeemed = (amount * holdings[u][m]) / pt.totalSupply();

            // Calculate the fees to be received (currently .025%)
            uint256 fee = redeemed / feenominator;

            // Verify allowance
            if (allowance < amount) {
                revert Exception(20, allowance, amount, address(0), address(0));
            }

            // Burn the tokens from the user
            pt.authBurn(f[i], amount);

            // Update the holdings for this market
            holdings[u][m] = holdings[u][m] - redeemed;

            // Transfer the underlying to the user
            Safe.transfer(uToken, f[i], redeemed - fee);

            unchecked {
                // Track the fees gained by the caller
                incentiveFee += fee;

                ++i;
            }
        }

        // Transfer the fee to the caller
        Safe.transfer(uToken, msg.sender, incentiveFee);

        return incentiveFee;
    }

This way the feenominator being set on construction cannot change:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L102-L119

    /// @notice Initializes the Redeemer contract
    /// @param l the lender contract
    /// @param s the Swivel contract
    /// @param p the Pendle contract
    /// @param t the Tempus contract
    constructor(
        address l,
        address s,
        address p,
        address t
    ) {
        admin = msg.sender;
        lender = l;
        swivelAddr = s;
        pendleAddr = p;
        tempusAddr = t;
        feenominator = 4000;
    }

But, taking DAI as an example, 4000 / 1e18, i.e. 4e-15, is a meaningless fee, providing no incentives for autoRedeem().

Tool used

Manual Review

Recommendation

Consider adding the feeChange management functionality to Redeemer as it is done in Lender:

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L812-L829

    /// @notice allows the admin to schedule a change to the fee denominators
    function scheduleFeeChange() external authorized(admin) returns (bool) {
        uint256 when = block.timestamp + HOLD;
        feeChange = when;

        emit ScheduleFeeChange(when);

        return true;
    }

    /// @notice Emergency function to block unplanned changes to fee structure
    function blockFeeChange() external authorized(admin) returns (bool) {
        delete feeChange;

        emit BlockFeeChange();

        return true;
    }

[sourabhmarathe]

We do not consider lack of incentives for autoRedeem to be a major problem. Users will be able to conduct redemptions on their own.

[JTraversa]

As sourabh is mentioning, due to the multitude of redemption methods no funds would really be at risk and this would be an incentivization discussion.

However I also dont think this is a valid issue either?

When you use a feenominator, you are always doing a % of the deposits, meaning regardless of the # of decimals, you are always taking the same proportional fee.