1. Variable should be set as constant

Impact

Variable UPDATE_TIME in NibblVaultFactoryData cannot be changed and should be set as constant to save gas.

Proof of Concept

NibblVaultFactoryData.sol:

UPDATE_TIME - https://github.com/NibblNFT/nibbl-smartcontracts/blob/49bf364d9e81a554cfdf47ae5cfc3daf52a54ad6/contracts/Utilities/NibblVaultFactoryData.sol#L6

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to set UPDATE_TIME as constant.

2. Cache array length outside of loop

Impact

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.

Proof of Concept

Basket.sol:43:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:70:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:93:        for (uint256 i = 0; i < _tokens.length; i++) {
NibblVault.sol:506:        for (uint256 i = 0; i < _assetAddresses.length; i++) {
NibblVault.sol:525:        for (uint256 i = 0; i < _assets.length; i++) {
NibblVault.sol:547:        for (uint256 i = 0; i < _assets.length; i++) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to cache the array length outside of for loop.

3. Long revert error messages

Impact

Shortening revert error messages to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.

Proof of Concept

NibblVaultFactory.sol:48:        require(msg.value >= MIN_INITIAL_RESERVE_BALANCE, "NibblVaultFactory: Initial reserve balance too low");
NibblVaultFactory.sol:49:        require(IERC721(_assetAddress).ownerOf(_assetTokenID) == msg.sender, "NibblVaultFactory: Invalid sender");
NibblVaultFactory.sol:107:        require(basketUpdateTime != 0 && block.timestamp >= basketUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
NibblVaultFactory.sol:131:        require(feeToUpdateTime != 0 && block.timestamp >= feeToUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
NibblVaultFactory.sol:141:        require(_newFee <= MAX_ADMIN_FEE, "NibblVaultFactory: Fee value greater than MAX_ADMIN_FEE");
NibblVaultFactory.sol:149:        require(feeAdminUpdateTime != 0 && block.timestamp >= feeAdminUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");
NibblVaultFactory.sol:166:        require(vaultUpdateTime != 0 && block.timestamp >= vaultUpdateTime, "NibblVaultFactory: UPDATE_TIME has not passed");

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to decrease revert messages to maximum 32 bytes.

4. Use custom errors instead of revert strings to save gas

Impact

Usage of custom errors reduces the gas cost.

Proof of Concept

Contracts that should be using custom errors:

Basket.sol
NibblVaultFactory.sol
NibblVault.sol
Utilities/AccessControlMechanism.sol

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add custom errors to listed contracts.

5. Remove obsolete math operations

Impact

Contract NibblVault packs timestamps to uint32 variables. To set blockTimestamp value it first runs math operation modulo on block.timestamp and then casts the result to uint32. Executing modulo operation in this case is obsolete, uint32 will successfully cast the value to 32bits bits.

Proof of Concept

NibblVault.sol:303:            uint32 _blockTimestamp = uint32(block.timestamp % 2**32);
NibblVault.sol:365:            uint32 _blockTimestamp = uint32(block.timestamp % 2**32);
NibblVault.sol:413:        _updateTWAV(_currentValuation, uint32(block.timestamp % 2**32));
NibblVault.sol:445:        uint32 _blockTimestamp = uint32(block.timestamp % 2**32);

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to remove % 2**32 math modulo operation.

6. ++i/--i costs less gas compared to i++, i += 1, i-- or i -= 1

Impact

++i or --i costs less gas compared to i++, i += 1, i-- or i -= 1 for unsigned integer as pre-increment/pre-decrement is cheaper (about 5 gas per iteration).

Proof of Concept

Basket.sol:43:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:70:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:93:        for (uint256 i = 0; i < _tokens.length; i++) {
NibblVault.sol:506:        for (uint256 i = 0; i < _assetAddresses.length; i++) {
NibblVault.sol:525:        for (uint256 i = 0; i < _assets.length; i++) {
NibblVault.sol:547:        for (uint256 i = 0; i < _assets.length; i++) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use ++i or --i instead of i++, i += 1, i-- or i -= 1 to increment value of an unsigned integer variable.

7. Obsolete overflow/underflow check

Impact

Starting from solidity 0.8.0 there is built-in check for overflows/underflows. This mechanism automatically checks if the variable overflows or underflows and throws an error. Multiple contracts use increments that cannot overflow but consume additional gas for checks.

Proof of Concept

Basket.sol:43:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:70:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:93:        for (uint256 i = 0; i < _tokens.length; i++) {
NibblVault.sol:506:        for (uint256 i = 0; i < _assetAddresses.length; i++) {
NibblVault.sol:525:        for (uint256 i = 0; i < _assets.length; i++) {
NibblVault.sol:547:        for (uint256 i = 0; i < _assets.length; i++) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended wrap incrementing with unchecked block, for example: unchecked { ++i } or unchecked { --i }

8. No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for addresses). Explicitly initializing it with its default value is an anti-pattern and waste of gas.

Proof of Concept

Basket.sol:43:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:70:        for (uint256 i = 0; i < _tokens.length; i++) {
Basket.sol:93:        for (uint256 i = 0; i < _tokens.length; i++) {
NibblVault.sol:506:        for (uint256 i = 0; i < _assetAddresses.length; i++) {
NibblVault.sol:525:        for (uint256 i = 0; i < _assets.length; i++) {
NibblVault.sol:547:        for (uint256 i = 0; i < _assets.length; i++) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to remove explicit initializations with default values.

9. Use != 0 instead of > 0 for unsigned integer comparison

Impact

When dealing with unsigned integer types, comparisons with != 0 are cheaper than with > 0.

Proof of Concept

NibblVault.sol:227:        if(_adminFeeAmt > 0) {
NibblVault.sol:243:        if(_adminFeeAmt > 0) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use != 0 instead of > 0.

code-423n4 / 2022-06-nibbl-findings

Gas Optimizations #294

1. Variable should be set as constant

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

2. Cache array length outside of loop

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

3. Long revert error messages

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

4. Use custom errors instead of revert strings to save gas

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

5. Remove obsolete math operations

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

6. ++i/--i costs less gas compared to i++, i += 1, i-- or i -= 1

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

7. Obsolete overflow/underflow check

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

8. No need to explicitly initialize variables with default values

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

9. Use != 0 instead of > 0 for unsigned integer comparison

Impact

Proof of Concept

Tools Used

Recommended Mitigation Steps

2, #3, #6, #7, #8, #9, #15