Gas Optimizations

[code423n4]

Contracts are using 0.8.x Solidity version but do not use Custom Errors

Proof of concept

Using Solidity custom errors is gas efficient and can be helpful for external integrations’ error handling. Having a require statement with an error string is costly both for deployment cost for contract deployer and failed transaction cost for the user

Impact

This can save users a good amount of gas on failed transactions.

Recommendation

Replace all require statements in the code with a revert CustomError statements

Contract functions that are only called externally should be declared external

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L64-L93

Proof of concept

All contract functions in NibblVaultFactory.sol that are declared as public can be declared as external.

Impact

Functions that are declared as external have their arguments in calldata instead of memory which saves gas for users

Recommendation

Change all public methods in NibblVaultFactory.sol to external

Inefficient usage of for loops

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L43

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L70

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/Basket.sol#L43

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L506

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L525

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L547

Proof of concept

Basket.sol and NibblVault.sol contain for loops that are implemented inefficiently in terms of gas for the EVM. For example omitting assigning a default-zero type variable to zero, caching array's length, using ++i instead of i++ can save a good chunk of gas, especially if the loop is long running.

Impact

Gas savings for protocol users

Recommendation

1. Don't initialise index counter variable to zero - zero is its default value and reinitialising to zero costs extra gas
2. When the for loop end check expression is checking for some array's length, always cache the length of the array in a separate stack variable.
3. Use ++i instead of i++ in for loops (small gas saving)
4. You can wrap the ++i in an unchecked block since you have a for loop end check expression anyway

Example for loop implemented using the recommendations:

for (uint256 i = 0; i < _assets.length; i++) {
            uint256 balance = IERC1155(_assets[i]).balanceOf(address(this),  _assetIDs[i]);
            IERC1155(_assets[i]).safeTransferFrom(address(this), _to, _assetIDs[i], balance, "0");
 }

With recommended gas optimisations:

uint256 assetsLength = _assets.length;
for (uint256 i; i < assetsLength;) {
         uint256 balance = IERC1155(_assets[i]).balanceOf(address(this),  _assetIDs[i]);
        IERC1155(_assets[i]).safeTransferFrom(address(this), _to, _assetIDs[i], balance, "0");

    unchecked {
        ++i;
    }
}