code423n4 commented 2 years ago

[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.

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

One example is

require(_tokenTransferProxy != address(0) && _augustusSwapper != address(0), "PSO: INVALID_ADDRESS");

This can be improved to

require(_tokenTransferProxy != address(0));
require(_augustusSwapper != address(0), "PSO: INVALID_ADDRESS");

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.

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-nested/tree/main/contracts/governance/TimelockControllerEmergency.sol#L120

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.

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.

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

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

There are many places where a value is compared to zero https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedAsset.sol#L92 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/FeeSplitter.sol#L219 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedRecords.sol#L70

Recommended Mitigation Steps

Use the assembly iszero evm opcode to compare values to zero

[G-09] Add payable to functions that won't receive ETH

Identifying a function as payable saves gas. Functions that have a modifier like onlyOwner cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas.

Recommended Mitigation Steps

Add payable to these functions for gas savings

[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.

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

Many modifiers can use this change https://github.com/code-423n4/2022-06-nested/tree/main/contracts/governance/TimelockControllerEmergency.sol#L103 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/abstracts/OwnableFactoryHandler.sol#L20 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/abstracts/OwnableProxyDelegation.sol#L40 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedAsset.sol#L42 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedFactory.sol#L98 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedFactory.sol#L106

Recommended Mitigation Steps

Use internal functions in place of modifiers to save gas.

[G-12] 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.

Recommended Mitigation Steps

Replace bool variables with uints

[G-13] 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.

Recommended Mitigation Steps

Replace bool variables with uints

[G-14] 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

Recommended Mitigation Steps

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

[G-15] Bitshift for divide by 2

When multiply or dividing by a power of two, it is cheaper to bitshift than to use standard math operations.

There is a divide by 2 operation on these lines https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol#L273 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol#L275

Recommended Mitigation Steps

Bitshift right by one bit instead of dividing by 2 to save gas

[G-16] Non-public variables save gas

Recommended Mitigation Steps

Declare some public variables as private or internal to save gas

[G-17] 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.

One case of function arguments using memory instead of calldata can use this improvement to save gas https://github.com/code-423n4/2022-06-nested/tree/main/contracts/governance/scripts/OperatorScripts.sol#L52

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-18] 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-19] 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.

There are some places where this change can be made: https://github.com/code-423n4/2022-06-nested/tree/main/contracts/governance/TimelockControllerEmergency.sol#L173 https://github.com/code-423n4/2022-06-nested/tree/main/contracts/governance/TimelockControllerEmergency.sol#L187

Recommended Mitigation Steps

Change abi.encode to abi.encodePacked

[G-20] 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.

This subtraction do not need to be checked for underflows because it is after a require statements that confirms the underflow will not happen https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedFactory.sol#L525

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.

obatirou commented 2 years ago

[G-18] Write contracts in vyper (disputed)

Audit is in solidity

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

First example: encodePacked can’t be used with dynamic length array Second example: acknowledge

Yashiru commented 2 years ago

[G-12] Use uint not bool (Disputed)

This is only relevant if you want to pack more than one boolean, but none of the specified booleans can be paked.

Yashiru commented 2 years ago

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

This optimizes the deployment costs but reduces the readability of the code. We prefer to keep good readability even if we have to pay more for the deployment.

Yashiru commented 2 years ago

[G-15] Bitshift for divide by 2 (Duplicated)

Duplicated of #89 at Use Shift Right/Left instead of Division/Multiplication

obatirou commented 2 years ago

[G-16] Non-public variables save gas (duplicate)

Duplicate from point 14 of gas opti report #75

Yashiru commented 2 years ago

[G-17] Use calldata instead of memory for function arguments (Duplicated)

Duplicated of #75 at Using calldata instead of memory for read-only arguments in external functions saves gas

maximebrugel commented 2 years ago

[G-20] Save gas with unchecked (Disputed)

We can’t uncheck this math operation https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedFactory.sol#L525

In fact, we can’t guarantee that the balance after is more/equal than before (with an exotic token)

maximebrugel commented 2 years ago

[G-09] Add payable to functions that won't receive ETH (Duplicated)

62 (see comment)

maximebrugel commented 2 years ago

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

For the constructor, will help saving gas, but no need to save 10 unit gas for the deployment by impacting the readability.

maximebrugel commented 2 years ago

[G-14] Use Solidity errors instead of require (Duplicated)

6 (see comment)

obatirou commented 2 years ago

[G-05] Short require strings save gas (duplicate)

https://github.com/code-423n4/2022-06-nested-findings/issues/62#issuecomment-1165547704

Yashiru commented 2 years ago

[G-03] Cache array length before loop (Duplicated)

Duplicated of #2 at For loop optimizaion

[G-06] Use prefix not postfix in loops (Duplicated)

Duplicated of #2 at For loop optimizaion

[G-07] For loop incrementing can be unsafe (Duplicated)

Duplicated of #2 at For loop optimizaion

obatirou commented 2 years ago

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

Confirmed except for https://github.com/code-423n4/2022-06-nested/tree/main/contracts/NestedBuybacker.sol#L52 which is out of scope

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

Gas Optimizations #29

[G-01] Redundant zero initialization

Recommended Mitigation Steps

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

Recommended Mitigation Steps

[G-03] Cache array length before loop

Recommended Mitigation Steps

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

Recommended Mitigation Steps

[G-05] Short require strings save gas

Recommended Mitigation Steps

[G-06] Use prefix not postfix in loops

Recommended Mitigation Steps

[G-07] For loop incrementing can be unsafe

Recommended Mitigation Steps

[G-08] Use iszero assembly for zero checks

Recommended Mitigation Steps

[G-09] Add payable to functions that won't receive ETH

Recommended Mitigation Steps

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

Recommended Mitigation Steps

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

Recommended Mitigation Steps

[G-12] Use uint not bool

Recommended Mitigation Steps

[G-13] Use uint256 not smaller ints

Recommended Mitigation Steps

[G-14] Use Solidity errors instead of require

Recommended Mitigation Steps

[G-15] Bitshift for divide by 2

Recommended Mitigation Steps

[G-16] Non-public variables save gas

Recommended Mitigation Steps

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

Recommended Mitigation Steps

[G-18] Write contracts in vyper

Recommended Mitigation Steps

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

Recommended Mitigation Steps

[G-20] Save gas with unchecked

Recommended Mitigation Steps

[G-18] Write contracts in vyper (disputed)

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

[G-12] Use uint not bool (Disputed)

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

[G-15] Bitshift for divide by 2 (Duplicated)

[G-16] Non-public variables save gas (duplicate)

[G-17] Use calldata instead of memory for function arguments (Duplicated)

[G-20] Save gas with unchecked (Disputed)

[G-09] Add payable to functions that won't receive ETH (Duplicated)

62 (see comment)

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

[G-14] Use Solidity errors instead of require (Duplicated)

6 (see comment)

[G-05] Short require strings save gas (duplicate)

[G-03] Cache array length before loop (Duplicated)

[G-06] Use prefix not postfix in loops (Duplicated)

[G-07] For loop incrementing can be unsafe (Duplicated)

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

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