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

Gas Optimizations #213

Open
code423n4 opened this issue Jun 14, 2022 · 0 comments
Open

Gas Optimizations #213

code423n4 opened this issue Jun 14, 2022 · 0 comments
Labels

Comments

@code423n4
Copy link
Contributor

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;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants