Gas Optimizations

[code423n4]

Require message is too long

The revert strings below can be shortened to 32 characters or fewer (as shown) to save gas

contracts/NibblVaultFactory.sol: L48

        require(msg.value >= MIN_INITIAL_RESERVE_BALANCE, "NibblVaultFactory: Initial reserve balance too low");

Change message to NibblVaultFactory: Low res bal

contracts/NibblVaultFactory.sol: L49

        require(IERC721(_assetAddress).ownerOf(_assetTokenID) == msg.sender, "NibblVaultFactory: Invalid sender");

Change message to NibblVaultFactory: Inval sender

Identical long require revert strings occur in all four lines referenced below. It's not clear how to shorten the message without abbreviating 'NibblVaultFactory'

contracts/NibblVaultFactory.sol: L107 contracts/NibblVaultFactory.sol: L131 contracts/NibblVaultFactory.sol: L149 contracts/NibblVaultFactory.sol: L166

Example:

        require(basketUpdateTime != 0 && block.timestamp >= basketUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");

Use of '&&' within a require function

Splitting such require() statements into separate requires instead of using '&&' saves gas

contracts/NibblVaultFactory.sol: L107

        require(basketUpdateTime != 0 && block.timestamp >= basketUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");

Recommendation:

        require(basketUpdateTime != 0, "NibblVaultFactory: UPDATE_TIME has not passed");
        require(block.timestamp >= basketUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");

Similarly for the three additional instances of '&&' within require functions referenced below:

contracts/NibblVaultFactory.sol: L131 contracts/NibblVaultFactory.sol: L149 contracts/NibblVaultFactory.sol: L166

Variables should not be initialized to their default values

For example, initializing uint variables to their default value of 0 is unnecessary and costs gas

The for loop counter i is initialized unnecessarily to zero in the six loops referenced below:

contracts/NibblVault.sol: L506-508 contracts/NibblVault.sol: L525-527 contracts/NibblVault.sol: L547-550 contracts/Basket.sol: L43-46 contracts/Basket.sol: L70-74 contracts/Basket.sol: L93-96

Example:

        for (uint256 i = 0; i < _assetAddresses.length; i++) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
        }

Change uint256 i = 0; to uint256 i; in all cases

Array length should not be looked up in every iteration of for loop

Since calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop

contracts/NibblVault.sol: L506-508

        for (uint256 i = 0; i < _assetAddresses.length; i++) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
        }

Recommendation:

        uint256 totalAssetAddresses = _assetAddresses.length; 
        for (uint256 i = 0; i < totalAssetAddresses; ++i) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
        }

Similarly for the five for loops referenced below:

contracts/NibblVault.sol: L525-527 contracts/NibblVault.sol: L547-550 contracts/Basket.sol: L43-46 contracts/Basket.sol: L70-74 contracts/Basket.sol: L93-96

Use of i++ to increase count in a for loop

Since use of i++ costs more gas, it would be better to use ++i in the six for loops referenced below:

contracts/NibblVault.sol: L506-508 contracts/NibblVault.sol: L525-527 contracts/NibblVault.sol: L547-550 contracts/Basket.sol: L43-46 contracts/Basket.sol: L70-74 contracts/Basket.sol: L93-96

Use of default "checked" behavior in for loop

Under/overflow checks are made every time i++ or (++i) is called. Such checks are unnecessary since i is already limited. Therefore, use unchecked{++i}/unchecked{i++} instead

contracts/NibblVault.sol: L506-508

        for (uint256 i = 0; i < _assetAddresses.length; i++) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
        }

Recommendation:

        for (uint256 i = 0; i < _assetAddresses.length;) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
            unchecked{ i++; }
        }

Similarly for the five additional for loops that could save gas by using unchecked:

contracts/NibblVault.sol: L525-527 contracts/NibblVault.sol: L547-550 contracts/Basket.sol: L43-46 contracts/Basket.sol: L70-74 contracts/Basket.sol: L93-96

For loop gas optimization example

While, for presentation purposes, I have separated the for loop-related gas savings methods, they should be combined. Below I show how all four of the gas saving methods outlined in this submission can be implemented together.

1. Don't initialize the loop counter (i) to zero
2. Read the array length before executing the loop
3. Use ++i rather than i++
4. Use unchecked ++i

contracts/NibblVault.sol: L506-508

        for (uint256 i = 0; i < _assetAddresses.length; i++) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
        }

Recommendation:

        uint256 totalAssetAddresses = _assetAddresses.length; 
        for (uint256 i; i < totalAssetAddresses;) {
            IERC721(_assetAddresses[i]).safeTransferFrom(address(this), _to, _assetIDs[i]);
            unchecked{ ++i; }
        }

Storage of uints or ints smaller than 32 bytes

Storage of uints or ints smaller than 32 bytes incurs overhead. Instead, use size of at least 32, then downcast where needed

Recommendation: Use a larger size than uint8 in the four lines referenced below:

contracts/NibblVault.sol: L557 contracts/Twav/Twav.sol: L11 contracts/Twav/Twav.sol: L12 contracts/Twav/Twav.sol: L37