Gas Optimizations

[code423n4]

FINDINGS

use shorter revert strings(less than 32 bytes)

You can (and should) attach error reason strings along with require statements to make it easier to understand why a contract call reverted. These strings, however, take space in the deployed bytecode. Every reason string takes at least 32 bytes so make sure your string fits in 32 bytes or it will become more expensive.

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Revert strings > 32 bytes:

File: NotionalTradeModule.sol line 169

require(_setToken.isComponent(address(_sendToken)), "Send token must be an index component");

File: NotionalTradeModule.sol line 199

require(_setToken.isComponent(address(wrappedfCash)), "FCash to redeem must be an index component");

File: NotionalTradeModule.sol line 378

require(wrappedfCashAddress.isContract(), "WrappedfCash not deployed for given parameters");

File: NotionalTradeModule.sol line 573

            require(_paymentToken == assetToken, "Token is neither asset nor underlying token");

Cache the length of arrays in loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack. Caching the array length in the stack saves around 3 gas per iteration. Therefore, it’s possible to save a significant amount of gas especially when the length is significantly big.

Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead:

File: NotionalTradeModule.sol line 237-239

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

The above should be changed to

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

Other instances to modify File: NotionalTradeModule.sol line 253-254

        address[] memory modules = setToken.getModules();
        for(uint256 i = 0; i < modules.length; i++) {

No need to initialize variables with their default values

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting gas. It costs more gas to initialize variables to zero than to let the default of zero be applied

File: NotionalTradeModule.sol line 519

        uint32 minImpliedRate = 0;

File: NotionalTradeModule.sol line 238

for(uint256 i = 0; i < modules.length; i++) {

Since i is a uint256, it has a default value of 0 so we don't need to initialize it here The above should be modified to

for(uint256 i; i < modules.length; i++) {

Other instances to modify File: NotionalTradeModule.sol line 254

        for(uint256 i = 0; i < modules.length; i++) {

File: NotionalTradeModule.sol line 393

for(uint256 i = 0; i < positionsLength; i++) {

File: NotionalTradeModule.sol line 605

        for(uint256 i = 0; i < positionsLength; i++) {

File: NotionalTradeModule.sol line 618

Splitting require() statements that use && saves gas

Instead of using the && operator in a single require statement to check multiple conditions,using multiple require statements with 1 condition per require statement will save 8 GAS per && The gas difference would only be realized if the revert condition is realized(met).

File: wfCashLogic.sol line 116-123

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

The above should be modified to

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

Proof The following tests were carried out in remix with both optimization turned on and off

function multiple (uint a) public pure returns (uint){
    require ( a > 1 && a < 5, "Initialized");
    return  a + 2;
}

Execution cost 21617 with optimization and using && 21976 without optimization and using &&

After splitting the require statement

function multiple(uint a) public pure returns (uint){
    require (a > 1 ,"Initialized");
    require (a < 5 , "Initialized");
    return a + 2;
}

Execution cost 21609 with optimization and split require 21968 without optimization and using split require

++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2

Instances include:

File:NotionalTradeModule.sol line 238

        for(uint256 i = 0; i < modules.length; i++) {

I suggest using ++i instead of i++ to increment the value of an uint variable.

Other instances

File:NotionalTradeModule.sol line 254

        for(uint256 i = 0; i < modules.length; i++) {

File:NotionalTradeModule.sol line 393

        for(uint256 i = 0; i < positionsLength; i++) {

File:NotionalTradeModule.sol line 605

        for(uint256 i = 0; i < positionsLength; i++) {

File:NotionalTradeModule.sol line 618

        for(uint256 i = 0; i < positionsLength; i++) {