Gas Optimizations

[code423n4]

[G-01] Unnecessary variable initialization of default value

When variable is not initialized, it will have its default values. Example: 0 for uint, false for bool and address(0) for address

I suggest removing default value initialization for following variables.

NotionalTradeModule.sol:48:    address internal constant ETH_ADDRESS = address(0);
NotionalTradeModule.sol:238:        for(uint256 i = 0; i < modules.length; i++) {
NotionalTradeModule.sol:254:        for(uint256 i = 0; i < modules.length; i++) {
NotionalTradeModule.sol:393:        for(uint256 i = 0; i < positionsLength; i++) {
NotionalTradeModule.sol:519:        uint32 minImpliedRate = 0;
NotionalTradeModule.sol:605:        for(uint256 i = 0; i < positionsLength; i++) {
NotionalTradeModule.sol:618:        for(uint256 i = 0; i < positionsLength; i++) {

For example these can change to:

• address internal constant ETH_ADDRESS;
• for (uint256 i; i < modules.length; i++) {
• uint32 minImpliedRate;

[G-02] Save Gas in For-Loops by storing array's length as a variable

3 gas per iteration can be saved by storing an array's length as a variable before the for-loop.

Issue found at:

NotionalTradeModule.sol:238:        for(uint256 i = 0; i < modules.length; i++) {
NotionalTradeModule.sol:254:        for(uint256 i = 0; i < modules.length; i++) {

For example, I suggest changing it to:

modulesLength = modules.length
for (uint i; i < modulesLength; i++) {

[G-03] ++i costs less gas than i++

It is better to use ++i than i++ when possible since it costs less gas.

Issue found at:

NotionalTradeModule.sol:238:        for(uint256 i = 0; i < modules.length; i++) {
NotionalTradeModule.sol:254:        for(uint256 i = 0; i < modules.length; i++) {
NotionalTradeModule.sol:393:        for(uint256 i = 0; i < positionsLength; i++) {
NotionalTradeModule.sol:605:        for(uint256 i = 0; i < positionsLength; i++) {
NotionalTradeModule.sol:610:                    numFCashPositions++;
NotionalTradeModule.sol:618:        for(uint256 i = 0; i < positionsLength; i++) {
NotionalTradeModule.sol:623:                    j++;

[G-04] Not Defining Variables to Reduce Gas

Certain variables is defined even though they are used only once. Remove these unnecessary variables to save gas. For cases where it will reduce the readability, one can use comments to help describe what the code is doing.

1. Remove wrappedfCashAddress of _deployWrappedfCash function at NotionalTradeModule.sol

Originally

368:    function _deployWrappedfCash(uint16 _currencyId, uint40 _maturity) internal returns(IWrappedfCashComplete) {
369:        address wrappedfCashAddress = wrappedfCashFactory.deployWrapper(_currencyId, _maturity);
370:        return IWrappedfCashComplete(wrappedfCashAddress);

Can be changed to

    function _deployWrappedfCash(uint16 _currencyId, uint40 _maturity) internal returns(IWrappedfCashComplete) {
        return IWrappedfCashComplete(wrappedfCashFactory.deployWrapper(_currencyId, _maturity));

2. Remove initialTotalSupply of _burn function at wfCashLogic.sol

Originally

209:        uint256 initialTotalSupply = totalSupply();
228:            uint256 assetInternalCashClaim = (uint256(cashBalance) * amount) / initialTotalSupply;

Can be changed to

228:            uint256 assetInternalCashClaim = (uint256(cashBalance) * amount) / totalSupply();

3. Remove unitfCashValue of convertToShares function at wfCashERC4626.sol

Originally

56:            uint256 unitfCashValue = _getPresentValue(uint256(Constants.INTERNAL_TOKEN_PRECISION));
57:            return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / unitfCashValue;

Can be changed to

57:            return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / _getPresentValue(uint256(Constants.INTERNAL_TOKEN_PRECISION));

[G-05] Use defined variable to save gas at wfCashERC4626.sol

Since totalSupply() is already defined as supply at line 53, use supply variable at line 60 as well.

can be changed to

53:        uint256 supply = totalSupply();
60:        return (assets * supply / totalAssets();

[G-06] != 0 costs less gass then > 0

!= 0 costs less gas when optimizer is enabled and is used for unsigned integers in "require" statement. I suggest changing > 0 to != 0

Issue found at:

wfCashBase.sol:37:        require(cashGroup.maxMarketIndex > 0, "Invalid currency");

[G-07] Reduce the long revert strings of error messages

By keeping the revert strings within 32 bytes will save you gas since each slot is 32 bytes.

Following are revert strings that are more than 32 bytes.

NotionalTradeModule.sol:169:        require(_setToken.isComponent(address(_sendToken)), "Send token must be an index component");
NotionalTradeModule.sol:199:        require(_setToken.isComponent(address(wrappedfCash)), "FCash to redeem must be an index component");
NotionalTradeModule.sol:378:        require(wrappedfCashAddress.isContract(), "WrappedfCash not deployed for given parameters");
NotionalTradeModule.sol:573:            require(_paymentToken == assetToken, "Token is neither asset nor underlying token");

[G-08] Remove unused parameter

There are unused parameters in the contract. It is possible to save gas by removing these parameters or comment out the uint256 as well.

Issue found at

NotionalTradeModule.sol:309:   function moduleIssueHook(ISetToken _setToken, uint256 /* _setTokenAmount */) external override onlyModule(_setToken) {
NotionalTradeModule.sol:317   function moduleRedeemHook(ISetToken _setToken, uint256 /* _setTokenAmount */) external override onlyModule(_setToken) {

Change it to

function moduleIssueHook(ISetToken _setToken) external override onlyModule(_setToken) {
function moduleRedeemHook(ISetToken _setToken) external override onlyModule(_setToken) {

[G-09] Both named returns and return statement are used

Removing unused named returns variable in below code can save gas and improve code readability.

Issue found at

1. NotionalTradeModule.sol (remove returns variable positions)

   355:    function getFCashPositions(ISetToken _setToken)
   356:    external
   357:    view
   358:    returns(address[] memory positions)
   359:    {
   360:        return _getFCashPositions(_setToken);
   361:    }

2. wfCashERC4626.sol (remove returns variable shares)

   52:    function convertToShares(uint256 assets) public view override returns (uint256 shares) {
   57:            return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / unitfCashValue;
   60:        return (assets * totalSupply()) / totalAssets();

3. wfCashERC4626.sol (remove returns variable assets)

   64:    function convertToAssets(uint256 shares) public view override returns (uint256 assets) {
   68:            return _getPresentValue(shares);
   71:        return (shares * totalAssets()) / supply;

4. wfCashBase.sol (remove returns variable "underlyingToken" and "underlyingPrecision")

   124:    function getUnderlyingToken() public view override returns (IERC20 underlyingToken, int256 underlyingPrecision) {
   129:            return (IERC20(asset.tokenAddress), asset.decimals);
   130:        } else {
   131:            return (IERC20(underlying.tokenAddress), underlying.decimals);

5. wfCashBase.sol (remove returns variable "assetToken", "underlyingPrecision" and "tokenType")

   137:    function getAssetToken() public view override returns (IERC20 assetToken, int256 underlyingPrecision, TokenType tokenType) {
   139:        return (IERC20(asset.tokenAddress), asset.decimals, asset.tokenType);

[G-10] Use multiple require instead of &&

When there are multiple conditions in require statement, break down the require statement into multiple require statements instead of using && can save gas.

Issue found at

1. wfCashLogic.sol

   116:        require(
   117:           msg.sender == address(NotionalV2) &&
   118:           // Only accept the fcash id that corresponds to the listed currency and maturity
   119:           _id == fCashID &&
   120:           // Protect against signed value underflows
   121:           int256(_value) > 0,
   122:           "Invalid"
   123:       );

2. wfCashLogic.sol

   129:        require(
   130:            ac.hasDebt == 0x00 &&
   131:            assets.length == 1 &&
   132:            EncodeDecode.encodeERC1155Id(

For example these can be changed to

        require(msg.sender == address(NotionalV2)); 
        require(_id == fCashID);
        require(int256(_value) > 0, "Invalid");