code-423n4 / 2022-08-rigor-findings

1 stars 0 forks source link

Gas Optimizations #401

Open code423n4 opened 2 years ago

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.

Locations where this was found include https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L87 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L136 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L624 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L793 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L248 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L311 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L322

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(_disputeID < disputeCount && disputes[_disputeID].status == Status.Active, "Disputes::!Resolvable");

This can be improved to

require(_disputeID < disputeCount);
require(disputes[_disputeID].status == Status.Active);

Several places had require statements with many logical "and"s. Instead, split into two to save gas https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L62 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L107 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L354

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-08-rigor/tree/main/contracts/Project.sol#L603

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-08-rigor/tree/main/contracts/Disputes.sol#L107 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L261 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L425 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L427 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L764 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L840 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L195 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L380 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L601 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L691 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L245

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-05] 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-08-rigor/tree/main/contracts/HomeFiProxy.sol#L87 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L136 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L140 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L624 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L248 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L311 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L322 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L368 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L603 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L625 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L650 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L672 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L710 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L181

Recommended Mitigation Steps

Use prefix not postfix to increment in a loop

[G-06] 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-08-rigor/tree/main/contracts/HomeFiProxy.sol#L87 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L136 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L140 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L624 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L248 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L311 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L322 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L368 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L603 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L625 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L650 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L672 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L710 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L181

Recommended Mitigation Steps

Make the increment in for loops unsafe to save gas

[G-07] 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-08-rigor/tree/main/contracts/Community.sol#L272 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L513

Recommended Mitigation Steps

Use the assembly iszero evm opcode to compare values to zero

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

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

There are many functions that have the auth modifier in the contracts. Some examples are https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L100 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L125 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L150

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-09] 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-08-rigor/tree/main/contracts/Project.sol#L88

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-10] 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-08-rigor/tree/main/contracts/HomeFiProxy.sol#L40 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L37 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L43 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L50 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L60 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L29 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L34 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L67 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L73 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L79 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L88 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L72 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L77 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L82 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L43 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L49 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L55

Recommended Mitigation Steps

Use internal functions in place of modifiers to save gas.

[G-11] 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-08-rigor/tree/main/contracts/Disputes.sol#L144 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L180 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L55 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L68 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L78 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L148 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L412 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L582 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L750 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L50

Recommended Mitigation Steps

Replace bool variables with uints

[G-12] 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-08-rigor/tree/main/contracts/Disputes.sol#L100 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L103 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L107 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L16 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L47 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L82 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L507

Recommended Mitigation Steps

Replace smaller int or uint variables with uint256 variables

[G-13] Use newer solidity version

A solidity version before 0.8.15 is used in this contract. The latest release of solidity includes changes that can provide gas savings. The improvements include: The advantages of versions 0.8.* over <0.8.0 are:

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

Errors are not used anywhere in this project. Require can be replaced in every place it is used. Locations where this was found include https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L41 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L81 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L105 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L133 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L39 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L46 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L52 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L61 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L106 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L183 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L31 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L50 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L36 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L64 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L84 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L69 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L75 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L81 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L90 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L131 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L159 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L191 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L235 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L241 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L248 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L251 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L312 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L347 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L353 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L384 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L400 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L491 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L536 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L539 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L557 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L568 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L764 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L792 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L886 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L123 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L132 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L135 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L150 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L153 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L176 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L189 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L195 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L199 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L238 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L241 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L245 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L277 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L301 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L308 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L341 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L369 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L406 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L511 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L515 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L521 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L530 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L753 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L886 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L906 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L73 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L78 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L84 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L142 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L191 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L255 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L44 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L50 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L56 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/libraries/Tasks.sol#L124

Recommended Mitigation Steps

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

[G-15] 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-08-rigor/tree/main/contracts/Project.sol#L437-L460

                // If new cost is more than task cost but total lent is enough to cover for it.
                else if (totalLent - _totalAllocated >= _newCost - _taskCost) {
                    // Increase the difference of new cost and old cost to total allocated.
                    totalAllocated += _newCost - _taskCost;
                }
                // If new cost is more than task cost and totalLent is not enough.
                else {
                    // Un-confirm SC, mark task as inactive, mark allocated as false, mark lifecycle as None

                    // Mark task as inactive by unapproving subcontractor.
                    // As subcontractor can only be approved if task is allocated
                    _unapproved = true;
                    tasks[_taskID].unApprove();

                    // Mark task as not allocated.
                    tasks[_taskID].unAllocateFunds();

                    // Reduce total allocation by old task cost.
                    // As as needs to go though funding process again.
                    totalAllocated -= _taskCost;

                    // Add this task to _changeOrderedTask array. These tasks will be allocated first.
                    _changeOrderedTask.push(_taskID);
                }

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

                // If new cost is more than task cost and totalLent is not enough.
                else if (totalLent - _totalAllocated < _newCost - _taskCost){
                    // Un-confirm SC, mark task as inactive, mark allocated as false, mark lifecycle as None

                    // Mark task as inactive by unapproving subcontractor.
                    // As subcontractor can only be approved if task is allocated
                    _unapproved = true;
                    tasks[_taskID].unApprove();

                    // Mark task as not allocated.
                    tasks[_taskID].unAllocateFunds();

                    // Reduce total allocation by old task cost.
                    // As as needs to go though funding process again.
                    totalAllocated -= _taskCost;

                    // Add this task to _changeOrderedTask array. These tasks will be allocated first.
                    _changeOrderedTask.push(_taskID);
                }
                // If new cost is more than task cost but total lent is enough to cover for it.
                else {
                    // Increase the difference of new cost and old cost to total allocated.
                    totalAllocated += _newCost - _taskCost;
                }

Recommended Mitigation Steps

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

[G-16] Non-public variables save gas

A constant variable VERSION is public, but changing the visibility of these variables to private or internal can save gas.

Locations where this was found include https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L60

Recommended Mitigation Steps

Declare public variables as private or internal to save gas

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

parv3213 commented 2 years ago

Awesome suggestions!