Nibbl contest - 0xkatana's results

[L-01] ecrecover does not check for zero address

The solidity ecrecover function is called directly to verify the given signatures. However, the ecrecover EVM opcode allows malleable (non-unique) signatures and thus is susceptible to replay attacks. A replay attack may not be possible here, but ensuring the signatures are not malleable is considered a best practice.

https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L563

Recommended Mitigation Steps

Use the recover function from OpenZeppelin's ECDSA library for signature verification. This is what OpenZeppelin does in their ERC20 permit implementation.

[G-01] Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L506 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L525 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L547 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L43 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L70 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L93

Recommended Mitigation Steps

Remove the redundant zero initialization uint256 i; instead of uint256 i = 0;

[G-02] Split up require statements instead of &&

Combining require statement conditions with && logic uses unnecessary gas. It is better to split up each part of the logical statement into a separate require statements

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L107 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L131 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L149 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L166

Recommended Mitigation Steps

Use separate require statements instead of concatenating with &&

[G-03] Cache array length before loop

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. This saves gas.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L506 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L525 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L547 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L43 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L70 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L93

Recommended Mitigation Steps

Cache the array length before the for loop

[G-04] Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L227 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L243

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-05] Short require strings save gas

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L48 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L49 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L107 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L131 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L141 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L149 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L166

Recommended Mitigation Steps

Shorten all require strings to less than 32 characters

[G-06] Use prefix not postfix in loops

Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L506 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L525 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L547 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L562 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L43 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L70 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L93

Recommended Mitigation Steps

Use prefix not postfix to increment in a loop

[G-07] For loop incrementing can be unsafe

For loops that use i++ do not need to use safemath for this operation because the loop would run out of gas long before this point. Making this addition operation unsafe using unchecked saves gas.

Sample code to make the for loop increment unsafe

for (uint i = 0; i < length; i = unchecked_inc(i)) {
    // do something that doesn't change the value of i
}

function unchecked_inc(uint i) returns (uint) {
    unchecked {
        return i + 1;
    }
}

Idea borrowed from https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#the-increment-in-for-loop-post-condition-can-be-made-unchecked

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L506 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L525 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L547 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L562 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L43 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L70 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L93

Recommended Mitigation Steps

Make the increment in for loops unsafe to save gas

[G-08] Use iszero assembly for zero checks

Comparing a value to zero can be done using the iszero EVM opcode. This can save gas

Source from t11s https://twitter.com/transmissions11/status/1474465495243898885

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L139

Recommended Mitigation Steps

Use the assembly iszero evm opcode to compare values to zero

[G-09] Save gas with unchecked

Use unchecked math when there is no overflow risk to save gas. Before index is decreased in remove it is checked for zero condition. This means index will not underflow and can be unchecked.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L319

Recommended Mitigation Steps

Add unchecked around math that can't overflow for gas savings. In Solidity before 0.8.0, use the normal math operators instead of safe math functions.

[G-10] Add payable to constructors that won't receive ETH

Identifying a constructor as payable saves gas. Constructors should only be called by the admin or deployer and should not mistakenly receive ETH. Constructors can be payable to save gas.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Proxy/ProxyBasket.sol#L19 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Proxy/ProxyVault.sol#L19

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-11] Use internal function in place of modifier

An internal function can save gas vs. a modifier. A modifier inlines the code of the original function but an internal function does not.

Source https://blog.polymath.network/solidity-tips-and-tricks-to-save-gas-and-reduce-bytecode-size-c44580b218e6#dde7

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L128 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L138 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L145 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L153

Recommended Mitigation Steps

Use internal functions in place of modifiers to save gas.

[G-12] Unnecessary modulus

Passing a value to uint32() will convert the value to uint32. Using modulus math on this value first uses unnecessary gas.

https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L303

Recommended Mitigation Steps

Remove unnecessary modulus math

[G-13] Use uint not bool

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L113 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L569

Recommended Mitigation Steps

Replace bool variables with uints

[G-14] Use uint256 not smaller ints

From the solidity docs

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L71 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L73 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L90 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L92 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L28 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L57 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L183 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L226 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L303 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L365 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L413 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L445 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L557 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L6 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L11 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L12 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L13 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L21 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L22 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Twav/Twav.sol#L37

Recommended Mitigation Steps

Replace smaller int or uint variables with uint256 variables

[G-15] Use newer solidity version

Solidity version 0.8.10 is used. The latest release of solidity includes changes that can provide gas savings. The improvements include:

• Solidity version 0.8.13 can save more gas with Yul IR pipeline

Source https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#upgrade-to-at-least-084

Recommended Mitigation Steps

Use solidity release 0.8.13 with Yul IR pipeline and other improvements for gas savings

[G-16] Use Solidity errors instead of require

Solidity errors introduced in version 0.8.4 can save gas on revert conditions https://blog.soliditylang.org/2021/04/21/custom-errors/ https://twitter.com/PatrickAlphaC/status/1505197417884528640

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L48 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L49 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L107 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L114 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L131 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L141 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L149 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVaultFactory.sol#L166 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L129 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L139 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L146 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L147 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L154 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L184 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L185 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L325 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L351 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L387 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L399 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L400 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L404 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L444 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L475 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L486 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L496 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L505 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L516 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L524 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L536 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L546 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L561 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L564 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L570 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L36 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L42 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L53 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L62 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L69 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L79 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L86 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L92

Recommended Mitigation Steps

Replace require blocks with new solidity errors described in https://blog.soliditylang.org/2021/04/21/custom-errors/

[G-17] Use simple comparison in if statement

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

The existing code is https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L315

if (_lowerCurveDiff >= msg.value) {
    _purchaseReturn = _buySecondaryCurve(msg.value, _totalSupply);
} else {
    //Gas Optimization
    _purchaseReturn = _initialTokenSupply - _totalSupply;
    secondaryReserveBalance += _lowerCurveDiff;
    // _purchaseReturn = _buySecondaryCurve(_to, _lowerCurveDiff);
    _purchaseReturn += _buyPrimaryCurve(msg.value - _lowerCurveDiff, _totalSupply + _purchaseReturn);
}

A simple comparison can be used for gas savings by reversing the logic

if (_lowerCurveDiff < msg.value) {
    //Gas Optimization
    _purchaseReturn = _initialTokenSupply - _totalSupply;
    secondaryReserveBalance += _lowerCurveDiff;
    // _purchaseReturn = _buySecondaryCurve(_to, _lowerCurveDiff);
    _purchaseReturn += _buyPrimaryCurve(msg.value - _lowerCurveDiff, _totalSupply + _purchaseReturn);
} else {
    _purchaseReturn = _buySecondaryCurve(msg.value, _totalSupply);
}

Recommended Mitigation Steps

Replace the comparison operator and reverse the logic to save gas using the suggestions above

[G-18] Use calldata instead of memory for function arguments

Using calldata instead of memory for function arguments saves gas sometimes. This can happen when a function is called externally and the memory array values are kept in calldata and copied to memory during ABI decoding (using the opcode calldataload and mstore). If the array is used in a for loop, arr[i] accesses the value in memory using a mload. If calldata is used instead, then instead of going via memory, the value is directly read from calldata using calldataload. That is, there are no intermediate memory operations that carries this value.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L504 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L523 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L545 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L581 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L41 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L68 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L91 https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Basket.sol#L109

Source https://gist.github.com/hrkrshnn/ee8fabd532058307229d65dcd5836ddc#use-calldata-instead-of-memory-for-function-parameters

Recommended Mitigation Steps

Change function arguments from memory to calldata

[G-19] Write contracts in vyper

The contracts are all written entirely in solidity. Writing contracts with vyper instead of solidity can save gas.

Source https://twitter.com/eiber_david/status/1515737811881807876 doggo demonstrates https://twitter.com/fubuloubu/status/1528179581974417414?t=-hcq_26JFDaHdAQZ-wYxCA&s=19

Recommended Mitigation Steps

Write some or all of the contracts in vyper to save gas

[G-20] Use abi.encodePacked() not abi.encode()

Changing abi.encode to abi.encodePacked can save gas. abi.encode pads extra null bytes at the end of the call data which is normally unnecessary. In general, abi.encodePacked is more gas-efficient.

Locations where this was found include https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L562

Recommended Mitigation Steps

Change abi.encode to abi.encodePacked