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 #308

Open
code423n4 opened this issue Aug 6, 2022 · 0 comments
Open

Gas Optimizations #308

code423n4 opened this issue Aug 6, 2022 · 0 comments
Labels

Comments

@code423n4
Copy link
Contributor

Caching the length in for loops and 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

Caching the length in for loops:

  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)

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

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L603

for (; i < _changeOrderedTask.length; i++) {

Can be optimized to

uint256 changeOrderedTaskLength = _changeOrderedTask.length;
for (uint256 i; i < changeOrderedTaskLength;) {
...
unchecked { ++i; }
}

Consider using ++i instead of i++

Currently every loops such as

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L603

are using i++ which is more expensive than ++i

for (; i < _changeOrderedTask.length; ++i) {

is better than

for (; i < _changeOrderedTask.length; 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(_projectAddress == address(this), "Project::!projectAddress");

Can be replaced with

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

if (_projectAddress != address(this)) revert InvalidProject();

Consider adding unchecked

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L672

_loopCount++;

to

unchecked {
++_loopCount;
}

https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L613-L630

                // If there is enough funds to allocate this task
                if (_costToAllocate >= _taskCost) {
                    // Reduce task cost from _costToAllocate
                    _costToAllocate -= _taskCost;

                    // Mark the task as allocated
                    tasks[_changeOrderedTask[i]].fundTask();

                    // Add task to _tasksAllocated array
                    _tasksAllocated[_loopCount] = _changeOrderedTask[i];

                    // Increment loop counter
                    _loopCount++;
                }
                // If there are not enough funds to allocate this task then stop looping
                else {
                    break;
                }

to

            unchecked {
                // If there is enough funds to allocate this task
                if (_costToAllocate >= _taskCost) {
                    // Reduce task cost from _costToAllocate
                    _costToAllocate -= _taskCost;

                    // Mark the task as allocated
                    tasks[_changeOrderedTask[i]].fundTask();

                    // Add task to _tasksAllocated array
                    _tasksAllocated[_loopCount] = _changeOrderedTask[i];

                    // Increment loop counter
                    ++_loopCount;
                }
                // If there are not enough funds to allocate this task then stop looping
                else {
                    break;
                }
            }
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 6, 2022
code423n4 added a commit that referenced this issue Aug 6, 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

2 participants