code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

Gas Optimizations #213

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Optimizations

Table of Contents

[G-01] Public functions that could be declared external to save gas

Description

Following functions should be declared external, as functions that are never called by the contract internally should be declared external to save gas.

Findings

NotionalTradeModule.sol#L210 - function redeemMaturedPositions(ISetToken _setToken)

notional-wrapped-fcash/contracts/wfCashERC4626.sol

L75 - function maxDeposit(address)\ L80 - function maxMint(address)\ L85 - function maxWithdraw(address owner)\ L90 - function maxRedeem(address owner)\ L152 - function previewRedeem(uint256 shares)\ L169 - function deposit(uint256 assets, address receiver)\ L178 - function mint(uint256 shares, address receiver)\ L191 - function withdraw(uint256 assets, address receiver, address owner)\ L209 - function redeem(uint256 shares, address receiver, address owner)

Recommended mitigation steps

Use the external attribute for functions never called from the contract.

[G-02] Remove unused function to save gas

Description

Having any unused functions in the code cost unnecessary gas usage.

Findings

wfCashERC4626.sol#L243-L247

function _safeNegInt88(uint256 x) private pure returns (int88) {
    int256 y = -int256(x);
    require(int256(type(int88).min) <= y);
    return int88(y);
}

Recommended mitigation steps

Evaluate if the function call should be used anywhere otherwise remove the function.

Attention: This _safeNegInt88 is not safe as the name suggests.

[G-03] Prevent zero transfers

Description

Checking non-zero transfer values can avoid an external call to save gas. Checking if tokensTransferred > 0 before making the external call to safeTransfer can save gas by avoiding the external call in such situations

Findings

wfCashERC4626.sol#L243-L247

function _sendTokensToReceiver(
        IERC20 token,
        address receiver,
        bool isETH,
        uint256 balanceBefore
    ) private returns (uint256 tokensTransferred) {
        uint256 balanceAfter = isETH ? address(this).balance : token.balanceOf(address(this));
        tokensTransferred = balanceAfter - balanceBefore;

        if (isETH) {
            WETH.deposit{value: tokensTransferred}();
            IERC20(address(WETH)).safeTransfer(receiver, tokensTransferred);
        } else {
            token.safeTransfer(receiver, tokensTransferred); // @audit-info check `tokensTransferred` for zero transfers
        }
    }

Recommended mitigation steps

Check for zero transfer and avoid calling safeTransfer.

[G-04] Revert early when depositing assets which leads to 0 shares due to rounding down to 0

Description

The function wfCashERC4626.deposit calculates the amount of shares to mint based on the given amount of assets.

To save gas and prevent unnecessary minting of 0 shares, validate the calculated shares value and revert early.

Findings

wfCashERC4626.sol#L170

function deposit(uint256 assets, address receiver) public override returns (uint256) {
    uint256 shares = previewDeposit(assets); // @audit-info missing protection against rounding down to 0 within Notional
    // Will revert if matured
    _mintInternal(assets, _safeUint88(shares), receiver, 0, true);
    emit Deposit(msg.sender, receiver, assets, shares);
    return shares;
}

Recommended mitigation steps

function deposit(uint256 assets, address receiver) public override returns (uint256) {
    uint256 shares = previewDeposit(assets);
    require(shares != 0, "ZERO_SHARES"); // @audit-info add check to revert early and to save gas
    // Will revert if matured
    _mintInternal(assets, _safeUint88(shares), receiver, 0, true);
    emit Deposit(msg.sender, receiver, assets, shares);
    return shares;
}