Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #81

Open
code423n4 opened this issue Jun 18, 2022 · 2 comments
Open

Gas Optimizations #81

code423n4 opened this issue Jun 18, 2022 · 2 comments
Assignees
Labels

Comments

@code423n4
Copy link
Contributor

Caching the length in for loops

This reduce gas cost as show here https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966/5

  1. if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929 2) for each iteration except for the first),
  2. if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first),
  3. if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L121-L130

    function addOperator(bytes32 operator) external override onlyOwner {
        require(operator != bytes32(""), "NF: INVALID_OPERATOR_NAME");
        bytes32[] memory operatorsCache = operators;
        for (uint256 i = 0; i < operatorsCache.length; i++) {
            require(operatorsCache[i] != operator, "NF: EXISTENT_OPERATOR");
        }
        operators.push(operator);
        rebuildCache();
        emit OperatorAdded(operator);
    }

Can be optimized to

    function addOperator(bytes32 operator) external override onlyOwner {
        require(operator != bytes32(""), "NF: INVALID_OPERATOR_NAME");
        bytes32[] memory operatorsCache = operators;
        uint256 operatorsCacheLength = operatorsCache.length;
        for (uint256 i = 0; i < operatorsCacheLength; i++) {
            require(operatorsCache[i] != operator, "NF: EXISTENT_OPERATOR");
        }
        operators.push(operator);
        rebuildCache();
        emit OperatorAdded(operator);
    }

The increment in for loop postcondition can be made unchecked

This reduce gas cost as show here https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966/5

Gas savings: roughly speaking this can save 30-40 gas per loop iteration. For lengthy loops, this can be significant!

Apply this to all part in your code with for loops. For example

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L196-L199

        for (uint256 i = 0; i < batchedOrdersLength; i++) {
            (uint256 fees, IERC20 tokenSold) = _submitInOrders(nftId, _batchedOrders[i], false);
            _transferFeeWithRoyalty(fees, tokenSold, nftId);
        }

Can be optimized to

        for (uint256 i = 0; i < batchedOrdersLength; ) {
            (uint256 fees, IERC20 tokenSold) = _submitInOrders(nftId, _batchedOrders[i], false);
            _transferFeeWithRoyalty(fees, tokenSold, nftId);

            unchecked { i++; }
        }

Consider using custom errors instead of revert strings

This reduce gas cost as show here https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966/5

Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deployment cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.

Any require statement in your code can be replaced with custom error for example:

require(batchedOrdersLength != 0, "NF: INVALID_MULTI_ORDERS");

Can be replaced with

// declare error before contract declaration
error InvalidMultiOrders();

if (batchedOrdersLength == 0) revert InvalidMultiOrders();
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 18, 2022
code423n4 added a commit that referenced this issue Jun 18, 2022
@obatirou obatirou self-assigned this Jun 21, 2022
@maximebrugel
Copy link
Collaborator

Consider using custom errors instead of revert strings (Duplicated)

#6 (see comment)

@Yashiru
Copy link
Collaborator

Yashiru commented Jun 24, 2022

Caching the length in for loops (Duplicated)

Duplicated of #2 at For loop optimizaion

The increment in for loop postcondition can be made unchecked (Duplicated)

Duplicated of #2 at For loop optimizaion

@Yashiru Yashiru added sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue duplicate This issue or pull request already exists and removed sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue labels Jun 24, 2022
@JeeberC4 JeeberC4 removed the duplicate This issue or pull request already exists label Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants