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

1 stars 1 forks source link

Gas Optimizations #100

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

wfCashERC4626.sol

convertToShares: unitfCashValue should be inlined

In the convertToShares function, we store unitfCashValue in the unitfCashValue variable on line 56.

This value being only used once, we can inline it and save one mstore.

Constants.INTERNAL_TOKEN_PRECISION being used twice, we can store it in a variable to avoid sloading and also casting to uint256 twice.

Recommendation:

uint256 internalTokenPrecision = uint256(Constants.INTERNAL_TOKEN_PRECISION);
return (assets * internalTokenPrecision) / _getPresentValue(internalTokenPrecision);

convertToShares: the variable totalSupply should be re-used

In the convertToShares function, totalSupply() is stored in the totalSupply variable on line 53.

This variable should also be used to compute the amount of shares instead of calling totalSupply() again on line 60

This change allows us to save 94 gas units.

Recommendation:

return (assets * supply) / totalAssets();

redeem: asset() should be stored in a variable and balanceAfter inlined

In the redeem function, asset() is being called twice, on line 212 and line 219;

We can store it in a variable and save one sload.

Recommendation:

IERC20 assetRedeemed = IERC20(asset());
uint256 balanceBefore = assetRedeemed.balanceOf(receiver);

balanceAfter being only used once, we can inline it and save one mstore.

Recommendation:

uint256 assets = assetRedeemed.balanceOf(receiver) - balanceBefore;

Use internal functions instead of public ones

This is a broad recommendation but it would save gas to declare internal functions, that would be called in external functions, instead of declaring all external functions as public to be able to reuse them in internal functions.

For example, we could have an internal _asset function:

function asset() external view override returns (address) {
    return _asset();
}

function _asset() internal view returns (address) {
    (IERC20 underlyingToken, bool isETH) = getToken(true);
    return isETH ? address(WETH) : address(underlyingToken);
}

We can then call this _asset function in the redeem function:

uint256 balanceBefore = IERC20(_asset()).balanceOf(receiver);

wfCashLogic.sol

_sendTokensToReceiver: balanceAfter can be inlined

In the _sendTokensToReceiver function, we store balanceAfter in the balanceAfter variable on line 304.

This variable being only used once, we can inline it and save one mstore.

Recommendation:

tokensTransferred = (isETH ? address(this).balance : token.balanceOf(address(this))) - balanceBefore;

NotionalTradeModule.sol

_mintFCashPosition: preTradeSendTokenBalance and preTradeReceiveTokenBalance can be inlined

In the _mintFCashPosition function, we store preTradeSendTokenBalance and preTradeReceiveTokenBalance variables on lines 435 and 436.

These variables being only used once, we can inline them in the _updateSetTokenPositions call and save two mstore.

Recommendation:

(sentAmount,) = _updateSetTokenPositions(
    _setToken,
    address(_sendToken),
    _sendToken.balanceOf(address(_setToken)),
    address(_fCashPosition),
    _fCashPosition.balanceOf(address(_setToken))
);

_mint: minImpliedRate can be inlined

In the _mint function, we store minImpliedRate in a variable on line 519.

This variable being only used once, we can inline it in the encodeWithSelector call and save one mstore.

Recommendation:

bytes memory mintCallData = abi.encodeWithSelector(
    functionSelector,
    _maxAssetAmount,
    uint88(_fCashAmount),
    address(_setToken),
    0,
    _fromUnderlying
);

_redeemFCashPosition: toUnderlying, preTradeReceiveTokenBalance and preTradeSendTokenBalance can be inlined

In the _redeemFCashPosition function, we store toUnderlying, preTradeReceiveTokenBalance and preTradeSendTokenBalance variables from lines 469 to 471.

These variables being only used once, we can inline them in the _updateSetTokenPositions call and save three mstore.

Recommendation:

_redeem(_setToken, _fCashPosition, _fCashAmount, _isUnderlying(_fCashPosition, _receiveToken));

(, receivedAmount) = _updateSetTokenPositions(
    _setToken,
    address(_fCashPosition),
    _fCashPosition.balanceOf(address(_setToken)),
    address(_receiveToken),
    _receiveToken.balanceOf(address(_setToken))
);

_redeemMaturedPositions: fCashBalance can be inlined

In the _redeemMaturedPositions function, we store fCashBalance in a variable on line 404.

This variable being only used once, we can inline it in the _redeemFCashPosition call and save one mstore.

Recommendation:

_redeemFCashPosition(_setToken, fCashPosition, receiveToken, fCashPosition.balanceOf(address(_setToken)), 0);

initialize: modules.length should be stored in a variable

In the initialize function, we loop through modules in the for loop from line 238 to 240.

modules.length should be stored in a variable to avoid doing a mload every time the for loop is ran.

Recommendation:

  address[] memory modules = _setToken.getModules();
  uint256 modulesLength = modules.length;

  for(uint256 i = 0; i < modulesLength; i++) {
      try IDebtIssuanceModule(modules[i]).registerToIssuanceModule(_setToken) {} catch {}
  }

removeModule: modules.length should be stored in a variable

In the initialize function, we loop through modules in the for loop from line 254 to 258.

modules.length should be stored in a variable to avoid doing a mload every time the for loop is ran.

Recommendation:

address[] memory modules = setToken.getModules();
uint256 modulesLength = modules.length;

for(uint256 i = 0; i < modulesLength; i++) {
    if(modules[i].isContract()){
        try IDebtIssuanceModule(modules[i]).unregisterFromIssuanceModule(setToken) {} catch {}
    }
}