Rigor Protocol contest - Dravee's results

Overview

Table of Contents

• 1. Low Risk Issues
  • 1.1. Known vulnerabilities exist in currently used @openzeppelin/contracts version
  • 1.2. Add constructor initializers
  • 1.3. Uninitialized Upgradeable contract
  • 1.4. Fees should be upper-bounded
  • 1.5. _safeMint() should be used rather than _mint() wherever possible
  • 1.6. Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions
  • 1.7. Multiple initialize() functions are front-runnable in the solution
  • 1.8. Use a 2-step ownership transfer pattern
• 2. Non-Critical Issues
  • 2.1. It's better to emit after all processing is done
  • 2.2. The nonReentrant modifier should occur before all other modifiers
  • 2.3. Use bytes.concat()
  • 2.4. Adding a return statement when the function defines a named return variable, is redundant

1. Low Risk Issues

1.1. Known vulnerabilities exist in currently used @openzeppelin/contracts version

As some known vulnerabilities exist in the current @openzeppelin/contracts version, consider updating package.json with at least @openzeppelin/contracts@4.7.2 here:

• https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/package.json#L68-L69 :

File: package.json
68:     "@openzeppelin/contracts": "^4.4.2",
69:     "@openzeppelin/contracts-upgradeable": "^4.4.2"

While vulnerabilities are known, the current scope isn't affected (this might not hold true for the whole solution)

1.2. Add constructor initializers

As per OpenZeppelin’s (OZ) recommendation, “The guidelines are now to make it impossible for anyone to run initialize on an implementation contract, by adding an empty constructor with the initializer modifier. So the implementation contract gets initialized automatically upon deployment.”

Note that this behaviour is also incorporated the OZ Wizard since the UUPS vulnerability discovery: “Additionally, we modified the code generated by the Wizard 19 to include a constructor that automatically initializes the implementation when deployed.”

Furthermore, this thwarts any attempts to frontrun the initialization tx of these contracts:

Community.sol:105:        initializer
DebtToken.sol:48:    ) external override initializer {
Disputes.sol:77:        initializer
HomeFi.sol:102:        initializer
HomeFiProxy.sol:60:        initializer
ProjectFactory.sol:48:        initializer

Note that this is already mitigated here:

Project.sol:88:    constructor() initializer {}
Project.sol:98:    ) external override initializer {

1.3. Uninitialized Upgradeable contract

Similar issue in the past: here

Upgradeable dependencies should be initialized. While not causing any harm at the moment, suppose OZ someday decide to upgrade this contract and the sponsor uses the new version: it is possible that the contract will not work anymore.

Consider calling the missing __ReentrancyGuard_init() here (ERC2771ContextUpgradeable doesn't have an init function):

File: Community.sol
21: contract Community is
22:     ICommunity,
23:     PausableUpgradeable,
24:     ReentrancyGuardUpgradeable,
25:     ERC2771ContextUpgradeable
...
102:     function initialize(address _homeFi)
...
108:         // Initialize pausable. Set pause to false;
109:         __Pausable_init();
+ 110:         __ReentrancyGuard_init(); //@audit ERC2771ContextUpgradeable doesn't have an init function

1.4. Fees should be upper-bounded

There should be an upper limit to reasonable fees

File: HomeFi.sol
185:     function replaceLenderFee(uint256 _newLenderFee)
186:         external
187:         override
188:         onlyAdmin
189:     {
190:         // Revert if no change in lender fee
191:         require(lenderFee != _newLenderFee, "HomeFi::!Change");
192: 
193:         // Reset variables
194:         lenderFee = _newLenderFee;
195: 
196:         emit LenderFeeReplaced(_newLenderFee);
197:     }

1.5. _safeMint() should be used rather than _mint() wherever possible

_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver, see the WARNING L278:

File: ERC721Upgradeable.sol
275:     /**
276:      * @dev Mints `tokenId` and transfers it to `to`.
277:      *
278:      * WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible
...
286:      */
287:     function _mint(address to, uint256 tokenId) internal virtual {

Both open OpenZeppelin and solmate have versions of this function so that NFTs aren't lost if they're minted to contracts that cannot transfer them back out.

Consider replacing with _safeMint() here:

File: HomeFi.sol
284:     function mintNFT(address _to, string memory _tokenURI)
...
291:         // Mints NFT and set token URI
292:         _mint(_to, projectCount);

Be careful however to respect the CEI pattern or add a re-entrancy guard as _safeMint adds a callback-check (_checkOnERC721Received) and a malicious onERC721Received could be exploited if not careful.

Reading material:

• https://blocksecteam.medium.com/when-safemint-becomes-unsafe-lessons-from-the-hypebears-security-incident-2965209bda2a
• https://samczsun.com/the-dangers-of-surprising-code/
• https://github.com/KadenZipfel/smart-contract-attack-vectors/blob/master/vulnerabilities/unprotected-callback.md

1.6. Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

See this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

• contracts/Community.sol:

   21  contract Community is
   22      ICommunity,
   23:     PausableUpgradeable,
   24:     ReentrancyGuardUpgradeable,
   25:     ERC2771ContextUpgradeable

• contracts/DebtToken.sol:

   11: contract DebtToken is IDebtToken, ERC20Upgradeable {

• contracts/Disputes.sol:

   17  contract Disputes is
   18      IDisputes,
   19:     ReentrancyGuardUpgradeable,
   20:     ERC2771ContextUpgradeable

• contracts/HomeFi.sol:

   27  contract HomeFi is
   28      IHomeFi,
   29:     ReentrancyGuardUpgradeable,
   30:     ERC721URIStorageUpgradeable,
   31:     ERC2771ContextUpgradeable

• contracts/HomeFiProxy.sol:

   14: contract HomeFiProxy is OwnableUpgradeable {

• contracts/Project.sol:

   24  contract Project is
   25      IProject,
   26:     ReentrancyGuardUpgradeable,
   27:     ERC2771ContextUpgradeable

• contracts/ProjectFactory.sol:

   16  contract ProjectFactory is
   17      IProjectFactory,
   18      Initializable,
   19:     ERC2771ContextUpgradeable
   20  {

1.7. Multiple initialize() functions are front-runnable in the solution

Consider adding some access control to them or deploying atomically or using constructor initializer:

contracts/Community.sol:
  102:     function initialize(address _homeFi)

contracts/DebtToken.sol:
  43:     function initialize(

contracts/Disputes.sol:
  74:     function initialize(address _homeFi)

contracts/HomeFi.sol:
  92:     function initialize(

contracts/ProjectFactory.sol:
  45:     function initialize(address _underlying, address _homeFi)

contracts/libraries/Tasks.sol:
  75:     function initialize(Task storage _self, uint256 _cost) public {

Only Project.sol is protected.

1.8. Use a 2-step ownership transfer pattern

Contracts inheriting from OpenZeppelin's libraries have the default transferOwnership() function (a one-step process). It's possible that the onlyOwner role mistakenly transfers ownership to a wrong address, resulting in a loss of the onlyOwner role. Consider overriding the default transferOwnership() function to first nominate an address as the pendingOwner and implementing an acceptOwnership() function which is called by the pendingOwner to confirm the transfer.

HomeFiProxy.sol:7:import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
HomeFiProxy.sol:14:contract HomeFiProxy is OwnableUpgradeable {
HomeFiProxy.sol:156:        proxyAdmin.transferOwnership(_newAdmin);

2. Non-Critical Issues

2.1. It's better to emit after all processing is done

contracts/Project.sol:
  212:         emit LendToProject(_cost);
  213  
  214          // Allocate funds to tasks and mark then as allocated
  215          allocateFunds();
  216      }

2.2. The nonReentrant modifier should occur before all other modifiers

This is a best-practice to protect against re-entrancy in other modifiers

Disputes.sol:145:    ) external override onlyAdmin nonReentrant resolvable(_disputeID) {

2.3. Use bytes.concat()

Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))

libraries/SignatureDecoder.sol:3:pragma solidity 0.8.6;
libraries/SignatureDecoder.sol:50:                abi.encodePacked("\x19Ethereum Signed Message:\n32", hash)

2.4. Adding a return statement when the function defines a named return variable, is redundant

While not consuming more gas with the Optimizer enabled: using both named returns and a return statement isn't necessary. Removing one of those can improve code clarity.

Affected code:

File: Tasks.sol
191:     function getState(Task storage _self)
192:         internal
193:         view
194:         returns (uint256 _state)
195:     {
196:         return uint256(_self.state);
197:     }

File: Community.sol
899:     function _msgSender()
900:         internal
901:         view
902:         override(ContextUpgradeable, ERC2771ContextUpgradeable)
903:         returns (address sender)
904:     {
905:         // We want to use the _msgSender() implementation of ERC2771ContextUpgradeable
906:         return super._msgSender();
907:     }

File: HomeFi.sol
303:     function _msgSender()
304:         internal
305:         view
306:         override(ContextUpgradeable, ERC2771ContextUpgradeable)
307:         returns (address sender)
308:     {
309:         // We want to use the _msgSender() implementation of ERC2771ContextUpgradeable
310:         return super._msgSender();
311:     }

File: Project.sol
716:     function getAlerts(uint256 _taskID)
717:         public
718:         view
719:         override
720:         returns (bool[3] memory _alerts)
721:     {
722:         return tasks[_taskID].getAlerts();
723:     }

Overview

Warden's impression

While the use of clones to save gas on deployment and the use of meta-transactions is commendable, keep in mind that this patterns relies on delegatecall() calls for every function, which adds an overhead of ~800 gas, which is multiple orders of magnitude less than the amount saved during deployment but should be kept in mind depending on the number of interactions.

Table of Contents:

• 1. Unnecessary SSTORE operation
• 2. Multiple accesses of a mapping/array should use a local variable cache
• 3. Unchecking arithmetics operations that can't underflow/overflow
• 4. Caching storage values in memory
• 5. The result of a function call should be cached rather than re-calling the function
• 6. Project.sol: Tightly pack storage variables
• 7. Use calldata instead of memory
• 8. A modifier used only once and not being inherited should be inlined to save gas
• 9. Internal/Private functions only called once can be inlined to save gas
• 10. Pre-Solidity 0.8.13: > 0 is less efficient than != 0 for unsigned integers (with proof)
• 11. Splitting require() statements that use && saves gas
• 12. Using bool for storage incurs overhead
• 13. <array>.length should not be looked up in every loop of a for-loop
• 14. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
• 15. Increments/decrements can be unchecked in for-loops
• 16. It costs more gas to initialize variables with their default value than letting the default value be applied
• 17. Upgrade pragma
• 18. Optimizations with assembly
  • 18.1. Use assembly for math (add, sub, mul, div)
  • 18.2. Use assembly to check for address(0)
  • 18.3. Use assembly to write storage values
• 19. Use Custom Errors instead of Revert Strings to save Gas
• 20. (Not recommended, but true) Functions guaranteed to revert when called by normal users can be marked payable

1. Unnecessary SSTORE operation

The code here is an equivalent to writing twice in storage as a pre-increment increments first and then assign the value in a second time:

File: Community.sol
206:     function publishProject(bytes calldata _data, bytes calldata _signature)
...
265:         // Store updated details
266:         _community.publishNonce = ++_community.publishNonce;

The following is enough:

File: Community.sol
206:     function publishProject(bytes calldata _data, bytes calldata _signature)
...
265:         // Store updated details
- 266:         _community.publishNonce = ++_community.publishNonce;
+ 266:         ++_community.publishNonce;

2. Multiple accesses of a mapping/array should use a local variable cache

Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time.

Relevant places (multiple accesses):

• Consider using CommunityStruct storage _community:

Community.sol:402:                _communities[_communityID]
Community.sol:405:                    _communities[_communityID]
Community.sol:412:        IDebtToken _currency = _communities[_communityID].currency;
Community.sol:421:        _communities[_communityID]
Community.sol:427:            _communities[_communityID].projectDetails[_project].lentAmount > 0
Community.sol:433:        _communities[_communityID]
Community.sol:438:        _communities[_communityID]
Community.sol:471:        address _lender = _communities[_communityID].owner;
Community.sol:620:            _communities[_communityID].memberCount
Community.sol:624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
Community.sol:625:            _members[i] = _communities[_communityID].members[i];

• Consider using ProjectDetails storage _communityProject:

Community.sol:692:                _communities[_communityID].projectDetails[_project].apr *
Community.sol:836:        address _lender = _communities[_communityID].owner;

• Consider using Task storage _task:

Project.sol:350:        tasks[_taskID].setComplete();
Project.sol:354:            tasks[_taskID].subcontractor,
Project.sol:355:            tasks[_taskID].cost
Project.sol:409:        uint256 _taskCost = tasks[_taskID].cost;
Project.sol:423:            if (tasks[_taskID].alerts[1]) {
Project.sol:449:                    tasks[_taskID].unApprove();
Project.sol:452:                    tasks[_taskID].unAllocateFunds();
Project.sol:464:            tasks[_taskID].cost = _newCost;
Project.sol:470:        if (_newSC != tasks[_taskID].subcontractor) {
Project.sol:474:                tasks[_taskID].unApprove();
Project.sol:485:                tasks[_taskID].subcontractor = address(0);
Project.sol:524:                    signer == tasks[_task].subcontractor,
Project.sol:528:            if (signer == tasks[_task].subcontractor) {
Project.sol:553:        cost = tasks[id].cost;
Project.sol:554:        subcontractor = tasks[id].subcontractor;
Project.sol:555:        state = tasks[id].state;
Project.sol:605:                uint256 _taskCost = tasks[_changeOrderedTask[i]].cost;
Project.sol:619:                    tasks[_changeOrderedTask[i]].fundTask();
Project.sol:652:                uint256 _taskCost = tasks[j].cost;
Project.sol:666:                    tasks[j].fundTask();
Project.sol:711:            _cost += tasks[_taskID].cost;

This is already applied at several places:

contracts/Community.sol:
  143:         CommunityStruct storage _community = _communities[communityCount];
  184:         CommunityStruct storage _community = _communities[_communityID];
  229:         CommunityStruct storage _community = _communities[_communityID];
  230:         ProjectDetails storage _communityProject = _community.projectDetails[
  306:         CommunityStruct storage _community = _communities[_communityID];
  307:         ProjectDetails storage _communityProject = _community.projectDetails[
  343:         ProjectDetails storage _communityProject = _communities[_communityID]
  602:         CommunityStruct storage _community = _communities[_communityID];
  648:         ProjectDetails storage _communityProject = _communities[_communityID]
  680:         ProjectDetails storage _communityProject = _communities[_communityID]
  731:         ProjectDetails storage _communityProject = _communities[
  767:         CommunityStruct storage _community = _communities[_communityID];
  768:         ProjectDetails storage _communityProject = _community.projectDetails[
  837:         ProjectDetails storage _communityProject = _communities[_communityID]

3. Unchecking arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

Consider wrapping with an unchecked block here (around 25 gas saved per instance):

• Community.sol#L794

File: Community.sol
792:             require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
...
794:             _lentAmount = _lentAndInterest - _repayAmount; //@audit should be unchecked due to L792

• Community.sol#L798

File: Community.sol
785:         if (_repayAmount > _interest) {
...
795:         } else {
....
798:             _interest -= _repayAmount;  //@audit should be unchecked due to L785 + L795
799:         }

• Project.sol#L427

File: Project.sol
425:                 if (_newCost < _taskCost) {
426:                     // Find the difference between old - new.
427:                     uint256 _withdrawDifference = _taskCost - _newCost; //@audit should be unchecked due to L425

• Project.sol#L440

File: Project.sol
438:                 else if (totalLent - _totalAllocated >= _newCost - _taskCost) {
439:                     // Increase the difference of new cost and old cost to total allocated.
440:                     totalAllocated += _newCost - _taskCost; //@audit should be unchecked due to L438

• Project.sol#L616

File: Project.sol
614:                 if (_costToAllocate >= _taskCost) {
615:                     // Reduce task cost from _costToAllocate
616:                     _costToAllocate -= _taskCost; //@audit should be unchecked due to L616

• Project.sol#L663

File: Project.sol
661:                 if (_costToAllocate >= _taskCost) {
662:                     // Reduce task cost from _costToAllocate
663:                     _costToAllocate -= _taskCost; //@audit should be unchecked due to L616

4. Caching storage values in memory

The code can be optimized by minimizing the number of SLOADs.

SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.

• contracts/Community.sol:

  113:         tokenCurrency1 = homeFi.tokenCurrency1(); //@audit gas SLOAD (homeFi)
  114:         tokenCurrency2 = homeFi.tokenCurrency2(); //@audit gas SLOAD (homeFi)
  115:         tokenCurrency3 = homeFi.tokenCurrency3(); //@audit gas SLOAD (homeFi)

  132:             !restrictedToAdmin || _sender == homeFi.admin(), //@audit gas SLOAD (homeFi)
  137:         homeFi.validCurrency(_currency); //@audit gas SLOAD (homeFi)

  143:         CommunityStruct storage _community = _communities[communityCount]; //@audit gas SLOAD (communityCount)
  150:         emit CommunityAdded(communityCount, _sender, _currency, _hash); //@audit gas SLOAD (communityCount)

  279:             _communityProject.publishFeePaid, //@audit gas SLOAD (_communityProject.publishFeePaid) (consider using a memory variable in addition to line L272)

  414:             homeFi.wrappedToken(address(_currency))//@audit gas SLOAD (homeFi)

  443:         _currency.safeTransferFrom(_msgSender(), homeFi.treasury(), _lenderFee);//@audit gas SLOAD (homeFi)

• contracts/Disputes.sol:

  212:         if (dispute.actionType == ActionType.TaskAdd) {//@audit gas SLOAD (dispute.actionType)
  216:         else if (dispute.actionType == ActionType.TaskChange) {//@audit gas SLOAD (dispute.actionType)

• contracts/HomeFi.sol:

  228:         projects[projectCount] = _project;//@audit gas SLOAD (projectCount)
  229:         projectTokenId[_project] = projectCount;//@audit gas SLOAD (projectCount)
  231:         emit ProjectAdded(projectCount, _project, _sender, _currency, _hash);//@audit gas SLOAD (projectCount)

  292:         _mint(_to, projectCount);//@audit gas SLOAD (projectCount)
  293:         _setTokenURI(projectCount, _tokenURI);//@audit gas SLOAD (projectCount)
  295:         emit NftCreated(projectCount, _to);//@audit gas SLOAD (projectCount)
  296:         return projectCount;//@audit gas SLOAD (projectCount)

• contracts/Project.sol:

  101:         disputes = homeFi.disputesContract();//@audit gas SLOAD (homeFi)
  102:         lenderFee = homeFi.lenderFee();//@audit gas SLOAD (homeFi)

  144:         emit ContractorInvited(contractor);//@audit gas SLOAD (contractor), use the available _contractor instead

  648:         if (j < taskCount) {//@audit gas SLOAD (taskCount)
  650:             for (++j; j <= taskCount; j++) {//@audit gas SLOAD (taskCount)
  681:             if (j > taskCount) {//@audit gas SLOAD (taskCount)
  682:                 lastAllocatedTask = taskCount;//@audit gas SLOAD (taskCount)

  798:         if (contractor == address(0)) {//@audit gas SLOAD (contractor)

  807:                 checkSignatureValidity(contractor, _hash, _signature, 0);//@audit gas SLOAD (contractor)
  813:                 checkSignatureValidity(contractor, _hash, _signature, 1);//@audit gas SLOAD (contractor)

  842:         if (contractor == address(0)) {//@audit gas SLOAD (contractor)
  852:                 checkSignatureValidity(contractor, _hash, _signature, 0);//@audit gas SLOAD (contractor)
  859:                 checkSignatureValidity(contractor, _hash, _signature, 1);//@audit gas SLOAD (contractor)

• contracts/ProjectFactory.sol:

  84:         require(_msgSender() == homeFi, "PF::!HomeFiContract");//@audit gas SLOAD (homeFi)
  90:         Project(_clone).initialize(_currency, _sender, homeFi);//@audit gas SLOAD (homeFi)

5. The result of a function call should be cached rather than re-calling the function

External calls are expensive. Consider caching the following:

• _projectInstance.lenderFee()

File: Community.sol
392:         // Calculate lenderFee
393:         uint256 _lenderFee = (_lendingAmount * _projectInstance.lenderFee()) /
394:             (_projectInstance.lenderFee() + 1000);

6. Project.sol: Tightly pack storage variables

Suggested change:

File: Project.sol
62:     /*******************************************************************************
63:      * ---------------------VARIABLE PUBLIC STORED PROPERTIES--------------------- *
64:      *******************************************************************************/
65:     /// @inheritdoc IProject
66:     address public override contractor;
67:     /// @inheritdoc IProject
68:     bool public override contractorConfirmed;
+ 69:     /// @inheritdoc IProject
+ 70:     bool public override contractorDelegated;
69:     /// @inheritdoc IProject
70:     uint256 public override hashChangeNonce;
71:     /// @inheritdoc IProject
72:     uint256 public override totalLent;
73:     /// @inheritdoc IProject
74:     uint256 public override totalAllocated;
75:     /// @inheritdoc IProject
76:     uint256 public override taskCount;
- 77:     /// @inheritdoc IProject
- 78:     bool public override contractorDelegated;
79:     /// @inheritdoc IProject
80:     uint256 public override lastAllocatedTask;
81:     /// @inheritdoc IProject
82:     uint256 public override lastAllocatedChangeOrderTask;

Which would save 1 storage slot.

7. Use calldata instead of memory

Affected code (around 60 gas to save):

File: HomeFi.sol
210:     function createProject(bytes memory _hash, address _currency)
211:         external
212:         override
213:         nonReentrant

8. A modifier used only once and not being inherited should be inlined to save gas

Affected code:

Community.sol:67:    modifier nonZero(address _address) {
Disputes.sol:37:    modifier nonZero(address _address) {
Disputes.sol:43:    modifier onlyAdmin() {
Disputes.sol:50:    modifier onlyProject() {

9. Internal/Private functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

Affected code:

Disputes.sol:207:    function resolveHandler(uint256 _disputeID) internal {
Project.sol:770:    function autoWithdraw(uint256 _amount) internal {

10. Pre-Solidity 0.8.13: > 0 is less efficient than != 0 for unsigned integers (with proof)

Up until Solidity 0.8.13: != 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer AND you're in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706

Consider changing > 0 with != 0 here:

Community.sol:764:        require(_repayAmount > 0, "Community::!repay");
Disputes.sol:107:            _actionType > 0 && _actionType <= uint8(ActionType.TaskPay),
Project.sol:195:        require(_cost > 0, "Project::!value>0");

Also, please enable the Optimizer.

11. Splitting require() statements that use && saves gas

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper

Affected code (saving around 3 gas per instance):

Community.sol:354:            _lendingNeeded >= _communityProject.totalLent &&
Disputes.sol:62:            _disputeID < disputeCount &&
Disputes.sol:107:            _actionType > 0 && _actionType <= uint8(ActionType.TaskPay),

12. Using bool for storage incurs overhead

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.

Consider doing like here: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 . They are using uint256(1) and uint256(2) for true/false to avoid the extra SLOAD (100 gas) and the extra SSTORE (20000 gas) when changing from false to true, after having been true in the past:

• State variables:

Community.sol:55:    bool public override restrictedToAdmin;
HomeFi.sol:50:    bool public override addrSet;
Project.sol:68:    bool public override contractorConfirmed;
Project.sol:78:    bool public override contractorDelegated;

• State mappings using booleans:

contracts/Community.sol:
  61:     mapping(address => mapping(bytes32 => bool)) public override approvedHashes;

contracts/HomeFiProxy.sol:
  30:     mapping(address => bool) internal contractsActive;

contracts/Project.sol:
  84:     mapping(address => mapping(bytes32 => bool)) public override approvedHashes;

contracts/libraries/Tasks.sol:
  16:     mapping(uint256 => bool) alerts; // Alerts of task

13. <array>.length should not be looked up in every loop of a for-loop

Reading array length at each iteration of the loop consumes more gas than necessary.

In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration. In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.

Here, consider storing the array's length in a variable before the for-loop, and use this new variable instead:

Project.sol:603:            for (; i < _changeOrderedTask.length; i++) {

14. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)

Pre-increments and pre-decrements are cheaper.

For a uint256 i variable, the following is true with the Optimizer enabled at 10k:

Increment:

• i += 1 is the most expensive form
• i++ costs 6 gas less than i += 1
• ++i costs 5 gas less than i++ (11 gas less than i += 1)

Decrement:

• i -= 1 is the most expensive form
• i-- costs 11 gas less than i -= 1
• --i costs 5 gas less than i-- (16 gas less than i -= 1)

Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:

uint i = 1;  
uint j = 2;
require(j == i++, "This will be false as i is incremented after the comparison");

However, pre-increments (or pre-decrements) return the new value:

uint i = 1;  
uint j = 2;
require(j == ++i, "This will be true as i is incremented before the comparison");

In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.

Affected code:

libraries/Tasks.sol:181:        for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i];
Community.sol:140:        communityCount++;
Community.sol:624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
HomeFi.sol:289:        projectCount += 1;
HomeFiProxy.sol:87:        for (uint256 i = 0; i < _length; i++) {
HomeFiProxy.sol:136:        for (uint256 i = 0; i < _length; i++) {
Project.sol:179:        hashChangeNonce += 1;
Project.sol:248:        for (uint256 i = 0; i < _length; i++) {
Project.sol:250:            _taskCount += 1;
Project.sol:290:        hashChangeNonce += 1;
Project.sol:311:        for (uint256 i = 0; i < _length; i++) {
Project.sol:322:        for (uint256 i = 0; i < _length; i++) {
Project.sol:368:            for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
Project.sol:603:            for (; i < _changeOrderedTask.length; i++) {
Project.sol:625:                    _loopCount++;
Project.sol:650:            for (++j; j <= taskCount; j++) {
Project.sol:672:                    _loopCount++;
Project.sol:710:        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {

Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).

15. Increments/decrements can be unchecked in for-loops

In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

ethereum/solidity#10695

Consider wrapping with an unchecked block here (around 25 gas saved per instance):

libraries/Tasks.sol:181:        for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i];
Community.sol:624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
HomeFiProxy.sol:87:        for (uint256 i = 0; i < _length; i++) {
HomeFiProxy.sol:136:        for (uint256 i = 0; i < _length; i++) {
Project.sol:248:        for (uint256 i = 0; i < _length; i++) {
Project.sol:311:        for (uint256 i = 0; i < _length; i++) {
Project.sol:322:        for (uint256 i = 0; i < _length; i++) {
Project.sol:368:            for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
Project.sol:603:            for (; i < _changeOrderedTask.length; i++) {
Project.sol:650:            for (++j; j <= taskCount; j++) {
Project.sol:710:        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {

The change would be:

- for (uint256 i; i < numIterations; i++) {
+ for (uint256 i; i < numIterations;) {
 // ...  
+   unchecked { ++i; }
}  

The same can be applied with decrements (which should use break when i == 0).

The risk of overflow is non-existant for uint256 here.

16. It costs more gas to initialize variables with their default value than letting the default value be applied

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas (around 3 gas per instance).

Affected code:

libraries/Tasks.sol:181:        for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i];
Community.sol:624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
HomeFiProxy.sol:87:        for (uint256 i = 0; i < _length; i++) {
HomeFiProxy.sol:136:        for (uint256 i = 0; i < _length; i++) {
Project.sol:248:        for (uint256 i = 0; i < _length; i++) {
Project.sol:311:        for (uint256 i = 0; i < _length; i++) {
Project.sol:322:        for (uint256 i = 0; i < _length; i++) {
Project.sol:412:        bool _unapproved = false;

Consider removing explicit initializations for default values.

17. Upgrade pragma

Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.

The advantages here are:

• Contract existence checks (>= 0.8.10): external calls skip contract existence checks if the external call has a return value

Consider upgrading here :

libraries/SignatureDecoder.sol:3:pragma solidity 0.8.6;
libraries/Tasks.sol:3:pragma solidity 0.8.6;
Community.sol:3:pragma solidity 0.8.6;
DebtToken.sol:3:pragma solidity 0.8.6;
Disputes.sol:3:pragma solidity 0.8.6;
HomeFi.sol:3:pragma solidity 0.8.6;
HomeFiProxy.sol:3:pragma solidity 0.8.6;
Project.sol:3:pragma solidity 0.8.6;
ProjectFactory.sol:3:pragma solidity 0.8.6;

18. Optimizations with assembly

The original warden who proved these type of findings is 0xKitsune. Clone the repo 0xKitsune/gas-lab, copy/paste the contract examples and run forge test --gas-report to replicate the gas reports with the optimizer turned on and set to 10000 runs.

18.1. Use assembly for math (add, sub, mul, div)

Use assembly for math instead of Solidity. You can check for overflow/underflow in assembly to ensure safety. If using Solidity versions < 0.8.0 and you are using Safemath, you can gain significant gas savings by using assembly to calculate values and checking for overflow/underflow.

POC Contract:

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    Contract2 c2;
    Contract3 c3;
    Contract4 c4;
    Contract5 c5;
    Contract6 c6;
    Contract7 c7;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
        c2 = new Contract2();
        c3 = new Contract3();
        c4 = new Contract4();
        c5 = new Contract5();
        c6 = new Contract6();
        c7 = new Contract7();
    }

    function testGas() public {
        c0.addTest(34598345, 100);
        c1.addAssemblyTest(34598345, 100);
        c2.subTest(34598345, 100);
        c3.subAssemblyTest(34598345, 100);
        c4.mulTest(34598345, 100);
        c5.mulAssemblyTest(34598345, 100);
        c6.divTest(34598345, 100);
        c7.divAssemblyTest(34598345, 100);
    }
}

contract Contract0 {
    //addition in Solidity
    function addTest(uint256 a, uint256 b) public pure {
        uint256 c = a + b;
    }
}

contract Contract1 {
    //addition in assembly
    function addAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := add(a, b)

            if lt(c, a) {
                mstore(0x00, "overflow")
                revert(0x00, 0x20)
            }
        }
    }
}

contract Contract2 {
    //subtraction in Solidity
    function subTest(uint256 a, uint256 b) public pure {
        uint256 c = a - b;
    }
}

contract Contract3 {
    //subtraction in assembly
    function subAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := sub(a, b)

            if gt(c, a) {
                mstore(0x00, "underflow")
                revert(0x00, 0x20)
            }
        }
    }
}

contract Contract4 {
    //multiplication in Solidity
    function mulTest(uint256 a, uint256 b) public pure {
        uint256 c = a * b;
    }
}

contract Contract5 {
    //multiplication in assembly
    function mulAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := mul(a, b)

            if lt(c, a) {
                mstore(0x00, "overflow")
                revert(0x00, 0x20)
            }
        }
    }
}

contract Contract6 {
    //division in Solidity
    function divTest(uint256 a, uint256 b) public pure {
        uint256 c = a * b;
    }
}

contract Contract7 {
    //division in assembly
    function divAssemblyTest(uint256 a, uint256 b) public pure {
        assembly {
            let c := div(a, b)

            if gt(c, a) {
                mstore(0x00, "underflow")
                revert(0x00, 0x20)
            }
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 40493              ┆ 233             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ addTest            ┆ 303             ┆ 303 ┆ 303    ┆ 303 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37087              ┆ 216             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ addAssemblyTest    ┆ 263             ┆ 263 ┆ 263    ┆ 263 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract2 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 40293              ┆ 232             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ subTest            ┆ 300             ┆ 300 ┆ 300    ┆ 300 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract3 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37287              ┆ 217             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ subAssemblyTest    ┆ 263             ┆ 263 ┆ 263    ┆ 263 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract4 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 41893              ┆ 240             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ mulTest            ┆ 325             ┆ 325 ┆ 325    ┆ 325 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract5 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37087              ┆ 216             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ mulAssemblyTest    ┆ 265             ┆ 265 ┆ 265    ┆ 265 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract6 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 41893              ┆ 240             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ divTest            ┆ 325             ┆ 325 ┆ 325    ┆ 325 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract7 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 37287              ┆ 217             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ divAssemblyTest    ┆ 265             ┆ 265 ┆ 265    ┆ 265 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Affected code:

See the section Unchecking arithmetics operations that can't underflow/overflow for safe places to rewrite.

18.2. Use assembly to check for address(0)

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testGas() public view {
        c0.ownerNotZero(address(this));
        c1.assemblyOwnerNotZero(address(this));
    }
}

contract Contract0 {
    function ownerNotZero(address _addr) public pure {
        require(_addr != address(0), "zero address)");
    }
}

contract Contract1 {
    function assemblyOwnerNotZero(address _addr) public pure {
        assembly {
            if iszero(_addr) {
                mstore(0x00, "zero address")
                revert(0x00, 0x20)
            }
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 61311              ┆ 338             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ ownerNotZero       ┆ 258             ┆ 258 ┆ 258    ┆ 258 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭──────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract   ┆                 ┆     ┆        ┆     ┆         │
╞══════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost      ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 44893                ┆ 255             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name        ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ assemblyOwnerNotZero ┆ 252             ┆ 252 ┆ 252    ┆ 252 ┆ 1       │
╰──────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Affected code:

Community.sol:69:        require(_address != address(0), "Community::0 address");
DebtToken.sol:50:        require(_communityContract != address(0), "DebtToken::0 address");
Disputes.sol:39:        require(_address != address(0), "Disputes::0 address");
HomeFi.sol:78:        require(_address != address(0), "HomeFi::0 address");
HomeFiProxy.sol:41:        require(_address != address(0), "Proxy::0 address");
HomeFiProxy.sol:106:            contractAddress[_contractName] == address(0),
Project.sol:135:        require(_contractor != address(0), "Project::0 address");
Project.sol:153:        require(contractor != address(0), "Project::0 address");
Project.sol:478:            if (_newSC != address(0)) {
Project.sol:753:        require(_sc != address(0), "Project::0 address");
Project.sol:798:        if (contractor == address(0)) {
Project.sol:842:        if (contractor == address(0)) {
ProjectFactory.sol:36:        require(_address != address(0), "PF::0 address");

18.3. Use assembly to write storage values

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testGas() public {
        c0.updateOwner(0x158B28A1b1CB1BE12C6bD8f5a646a0e3B2024734);
        c1.assemblyUpdateOwner(0x158B28A1b1CB1BE12C6bD8f5a646a0e3B2024734);
    }
}

contract Contract0 {
    address owner = 0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84;

    function updateOwner(address newOwner) public {
        owner = newOwner;
    }
}

contract Contract1 {
    address owner = 0xb4c79daB8f259C7Aee6E5b2Aa729821864227e84;

    function assemblyUpdateOwner(address newOwner) public {
        assembly {
            sstore(owner.slot, newOwner)
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 60623              ┆ 261             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ updateOwner        ┆ 5302            ┆ 5302 ┆ 5302   ┆ 5302 ┆ 1       │
╰────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ Contract1 contract ┆                 ┆      ┆        ┆      ┆         │
╞════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 54823              ┆ 232             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ assemblyUpdateOwner┆ 5236            ┆ 5236 ┆ 5236   ┆ 5236 ┆ 1       │
╰────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯

Affected code:

libraries/Tasks.sol:88:    function setComplete(Task storage _self)
HomeFi.sol:123:    function setAddr(
HomeFi.sol:200:    function setTrustedForwarder(address _newForwarder)
Project.sol:330:    function setComplete(bytes calldata _data, bytes calldata _signature)

19. Use Custom Errors instead of Revert Strings to save Gas

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

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met).

Additionally, custom errors can be used inside and outside of contracts (including interfaces and libraries).

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

POC Contract:

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testFailGas() public {
        c0.stringErrorMessage();
        c1.customErrorMessage();
    }
}

contract Contract0 {
    function stringErrorMessage() public {
        bool check = false;
        require(check, "error message");
    }
}

contract Contract1 {
    error CustomError();

    function customErrorMessage() public {
        bool check = false;
        if (!check) {
            revert CustomError();
        }
    }
}

POC Gas Report:

╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 34087              ┆ 200             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ stringErrorMessage ┆ 218             ┆ 218 ┆ 218    ┆ 218 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost    ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 26881              ┆ 164             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name      ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ customErrorMessage ┆ 161             ┆ 161 ┆ 161    ┆ 161 ┆ 1       │
╰────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

Consider replacing all revert strings with custom errors in the solution.

libraries/Tasks.sol:44:        require(_self.state == TaskStatus.Inactive, "Task::active");
libraries/Tasks.sol:50:        require(_self.state == TaskStatus.Active, "Task::!Active");
libraries/Tasks.sol:56:        require(
libraries/Tasks.sol:124:        require(_self.subcontractor == _sc, "Task::!SC");
Community.sol:69:        require(_address != address(0), "Community::0 address");
Community.sol:75:        require(_msgSender() == homeFi.admin(), "Community::!admin");
Community.sol:81:        require(
Community.sol:90:        require(
Community.sol:131:        require(
Community.sol:159:        require(
Community.sol:191:        require(
Community.sol:235:        require(
Community.sol:241:        require(homeFi.isProjectExist(_project), "Community::Project !Exists");
Community.sol:248:        require(_community.isMember[_builder], "Community::!Member");
Community.sol:251:        require(
Community.sol:312:        require(
Community.sol:347:        require(
Community.sol:353:        require(
Community.sol:384:        require(
Community.sol:400:        require(
Community.sol:491:        require(
Community.sol:536:        require(_builder == _projectInstance.builder(), "Community::!Builder");
Community.sol:539:        require(
Community.sol:557:        require(!restrictedToAdmin, "Community::restricted");
Community.sol:568:        require(restrictedToAdmin, "Community::!restricted");
Community.sol:764:        require(_repayAmount > 0, "Community::!repay");
Community.sol:792:            require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
Community.sol:886:        require(
DebtToken.sol:31:        require(
DebtToken.sol:50:        require(_communityContract != address(0), "DebtToken::0 address");
Disputes.sol:39:        require(_address != address(0), "Disputes::0 address");
Disputes.sol:46:        require(homeFi.admin() == _msgSender(), "Disputes::!Admin");
Disputes.sol:52:        require(homeFi.isProjectExist(_msgSender()), "Disputes::!Project");
Disputes.sol:61:        require(
Disputes.sol:106:        require(
Disputes.sol:183:        require(_result, "Disputes::!Member");
HomeFi.sol:73:        require(admin == _msgSender(), "HomeFi::!Admin");
HomeFi.sol:78:        require(_address != address(0), "HomeFi::0 address");
HomeFi.sol:84:        require(_oldAddress != _newAddress, "HomeFi::!Change");
HomeFi.sol:142:        require(!addrSet, "HomeFi::Set");
HomeFi.sol:191:        require(lenderFee != _newLenderFee, "HomeFi::!Change");
HomeFi.sol:255:        require(
HomeFiProxy.sol:41:        require(_address != address(0), "Proxy::0 address");
HomeFiProxy.sol:81:        require(_length == _implementations.length, "Proxy::Lengths !match");
HomeFiProxy.sol:105:        require(
HomeFiProxy.sol:133:        require(_length == _contractAddresses.length, "Proxy::Lengths !match");
Project.sol:123:        require(!contractorConfirmed, "Project::GC accepted");
Project.sol:132:        require(_projectAddress == address(this), "Project::!projectAddress");
Project.sol:135:        require(_contractor != address(0), "Project::0 address");
Project.sol:150:        require(_msgSender() == builder, "Project::!B");
Project.sol:153:        require(contractor != address(0), "Project::0 address");
Project.sol:176:        require(_nonce == hashChangeNonce, "Project::!Nonce");
Project.sol:189:        require(
Project.sol:195:        require(_cost > 0, "Project::!value>0");
Project.sol:199:        require(
Project.sol:238:        require(_taskCount == taskCount, "Project::!taskCount");
Project.sol:241:        require(_projectAddress == address(this), "Project::!projectAddress");
Project.sol:245:        require(_length == _taskCosts.length, "Project::Lengths !match");
Project.sol:277:        require(_nonce == hashChangeNonce, "Project::!Nonce");
Project.sol:301:        require(
Project.sol:308:        require(_length == _scList.length, "Project::Lengths !match");
Project.sol:341:        require(_projectAddress == address(this), "Project::!Project");
Project.sol:369:                require(tasks[_taskID].getState() == 3, "Project::!Complete");
Project.sol:406:        require(_project == address(this), "Project::!projectAddress");
Project.sol:511:        require(_project == address(this), "Project::!projectAddress");
Project.sol:515:            require(
Project.sol:521:            require(
Project.sol:530:                require(getAlerts(_task)[2], "Project::!SCConfirmed");
Project.sol:753:        require(_sc != address(0), "Project::0 address");
Project.sol:886:        require(
Project.sol:906:        require(
ProjectFactory.sol:36:        require(_address != address(0), "PF::0 address");
ProjectFactory.sol:64:        require(
ProjectFactory.sol:84:        require(_msgSender() == homeFi, "PF::!HomeFiContract");

20. (Not recommended, but true) Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Community.sol:555:    function restrictToAdmin() external override onlyHomeFiAdmin {
Community.sol:566:    function unrestrictToAdmin() external override onlyHomeFiAdmin {
Community.sol:577:    function pause() external override onlyHomeFiAdmin {
Community.sol:582:    function unpause() external override onlyHomeFiAdmin {