Rigor Protocol contest - Metatron's results

[G-01] ++i costs less gas compared to i++

++i costs about 5 gas less per iteration compared to i++ for unsigned integer. This statement is true even with the optimizer enabled. Summerized my results where i is used in a loop, is unsigned integer, and you safly can be changed to ++i without changing any behavior,

I've found 14 locations in 4 files:

contracts/Community.sol:
  139          // Increment community counter
  140:         communityCount++;
  141  

  623          // Append member addresses in _members array
  624:         for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
  625              _members[i] = _communities[_communityID].members[i];

contracts/HomeFiProxy.sol:
   86          // Generate proxy for all implementation
   87:         for (uint256 i = 0; i < _length; i++) {
   88              _generateProxy(allContractNames[i], _implementations[i]);

  135          // Replace implementations
  136:         for (uint256 i = 0; i < _length; i++) {
  137              _replaceImplementation(_contractNames[i], _contractAddresses[i]);

contracts/Project.sol:
  247          // Loop over all the new tasks.
  248:         for (uint256 i = 0; i < _length; i++) {
  249              // Increment local task counter.

  310          // Invite subcontractor for each task.
  311:         for (uint256 i = 0; i < _length; i++) {
  312              _inviteSC(_taskList[i], _scList[i], false);

  321          uint256 _length = _taskList.length;
  322:         for (uint256 i = 0; i < _length; i++) {
  323              tasks[_taskList[i]].acceptInvitation(_msgSender());

  367              uint256 _length = taskCount;
  368:             for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
  369                  require(tasks[_taskID].getState() == 3, "Project::!Complete");

  602              // Loop from lastAllocatedChangeOrderTask to _changeOrderedTask length (until _maxLoop)
  603:             for (; i < _changeOrderedTask.length; i++) {
  604                  // Local instance of task cost. To save gas.

  624                      // Increment loop counter
  625:                     _loopCount++;
  626                  }

  649              // Loop from lastAllocatedTask + 1 to taskCount (until _maxLoop)
  650:             for (++j; j <= taskCount; j++) {
  651                  // Local instance of task cost. To save gas.

  671                      // Increment loop counter
  672:                     _loopCount++;
  673                  }

  709          // Iterate over all tasks to sum their cost
  710:         for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
  711              _cost += tasks[_taskID].cost;

contracts/libraries/Tasks.sol:
  180          uint256 _length = _alerts.length;
  181:         for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i];
  182      }

---

[G-02] An arrays length should be cached to save gas in for-loops

An array’s length should be cached to save gas in for-loops Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset). Caching the array length in the stack saves around 3 gas per iteration.

I've found only one spot in the files in scope: (nice work!)

contracts/Project.sol:
  602              // Loop from lastAllocatedChangeOrderTask to _changeOrderedTask length (until _maxLoop)
  603:             for (; i < _changeOrderedTask.length; i++) {
  604                  // Local instance of task cost. To save gas.

---

[G-03] Using default values is cheaper than assignment

If a variable is not set/initialized, it is assumed to have the default value 0 for uint, and false for boolean. Explicitly initializing it with its default value is an anti-pattern and wastes gas. For example: uint8 i = 0; should be replaced with uint8 i;

I've found 8 locations in 4 different files:

contracts/Community.sol:
  623          // Append member addresses in _members array
  624:         for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
  625              _members[i] = _communities[_communityID].members[i];

contracts/HomeFiProxy.sol:
   86          // Generate proxy for all implementation
   87:         for (uint256 i = 0; i < _length; i++) {
   88              _generateProxy(allContractNames[i], _implementations[i]);

  135          // Replace implementations
  136:         for (uint256 i = 0; i < _length; i++) {
  137              _replaceImplementation(_contractNames[i], _contractAddresses[i]);

contracts/Project.sol:
  247          // Loop over all the new tasks.
  248:         for (uint256 i = 0; i < _length; i++) {
  249              // Increment local task counter.

  310          // Invite subcontractor for each task.
  311:         for (uint256 i = 0; i < _length; i++) {
  312              _inviteSC(_taskList[i], _scList[i], false);

  321          uint256 _length = _taskList.length;
  322:         for (uint256 i = 0; i < _length; i++) {
  323              tasks[_taskList[i]].acceptInvitation(_msgSender());

  411          // Local variable indicating if subcontractor is already unapproved.
  412:         bool _unapproved = false;
  413  

contracts/libraries/Tasks.sol:
  180          uint256 _length = _alerts.length;
  181:         for (uint256 i = 0; i < _length; i++) _alerts[i] = _self.alerts[i];
  182      }

---

[G-04] != 0 is cheaper than > 0

!= 0 costs less gas compared to > 0 for unsigned integers even when optimizer enabled All of the following findings are uint (E&OE) so >0 and != have exectly the same effect. ** saves 6 gas ** each

I've found 12 locations in 4 different files:

contracts/Community.sol:
  260          // If already published then unpublish first
  261:         if (projectPublished[_project] > 0) {
  262              _unpublishProject(_project);

  424  
  425:         // First claim interest if principal lent > 0
  426          if (
  427:             _communities[_communityID].projectDetails[_project].lentAmount > 0
  428          ) {

  763          // Revert if repayment amount is zero.
  764:         require(_repayAmount > 0, "Community::!repay");
  765  

  839  
  840:         if (_interestEarned > 0) {
  841              // If any new interest is to be claimed.

contracts/Disputes.sol:
  106          require(
  107:             _actionType > 0 && _actionType <= uint8(ActionType.TaskPay),
  108              "Disputes::!ActionType"

contracts/HomeFi.sol:
  244      {
  245:         return projectTokenId[_project] > 0;
  246      }

contracts/Project.sol:
  194          // Revert if try to lend 0
  195:         require(_cost > 0, "Project::!value>0");
  196  

  379          // If balance is present then it to the builder.
  380:         if (_leftOutTokens > 0) {
  381              _token.safeTransfer(builder, _leftOutTokens);

  600          // Any tasks added to _changeOrderedTask will be allocated first
  601:         if (_changeOrderedTask.length > 0) {
  602              // Loop from lastAllocatedChangeOrderTask to _changeOrderedTask length (until _maxLoop)

  690          // If any tasks is allocated, then emit event
  691:         if (_loopCount > 0) emit TaskAllocated(_tasksAllocated);
  692  

---

[G-05] Custom errors save gas

From solidity 0.8.4 and above, Custom errors from are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) 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.

I've found 45 locations in 8 different files:

contracts/Community.sol:
   68          // Revert if _address zero address (0x00) (invalid)
   69:         require(_address != address(0), "Community::0 address");
   70          _;

   74          // Revert if sender is not homeFi admin
   75:         require(_msgSender() == homeFi.admin(), "Community::!admin");
   76          _;

  240          // Reverts if _project not originated from HomeFi
  241:         require(homeFi.isProjectExist(_project), "Community::Project !Exists");
  242  

  247          // Revert if project builder is not community member
  248:         require(_community.isMember[_builder], "Community::!Member");
  249  

  535          // Revert if decoded builder is not decoded project's builder
  536:         require(_builder == _projectInstance.builder(), "Community::!Builder");
  537  

  556          // Revert if already restricted to admin
  557:         require(!restrictedToAdmin, "Community::restricted");
  558  

  567          // Revert if already unrestricted to admin
  568:         require(restrictedToAdmin, "Community::!restricted");
  569  

  763          // Revert if repayment amount is zero.
  764:         require(_repayAmount > 0, "Community::!repay");
  765  

  791              // Revert if repayment amount is greater than sum of lent and interest.
  792:             require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
  793              _interest = 0;

contracts/DebtToken.sol:
  49          // Revert if _communityContract is zero address. Invalid address.
  50:         require(_communityContract != address(0), "DebtToken::0 address");
  51  

contracts/Disputes.sol:
   38          // Revert if _address zero address (0x00)
   39:         require(_address != address(0), "Disputes::0 address");
   40          _;

   45          // Only HomeFi admin can resolve dispute
   46:         require(homeFi.admin() == _msgSender(), "Disputes::!Admin");
   47          _;

   51          // Revert if project not originated of HomeFi
   52:         require(homeFi.isProjectExist(_msgSender()), "Disputes::!Project");
   53          _;

  182              _sc == _address;
  183:         require(_result, "Disputes::!Member");
  184      }

contracts/HomeFi.sol:
   72      modifier onlyAdmin() {
   73:         require(admin == _msgSender(), "HomeFi::!Admin");
   74          _;

   77      modifier nonZero(address _address) {
   78:         require(_address != address(0), "HomeFi::0 address");
   79          _;

   83          // Revert if new address is same as old
   84:         require(_oldAddress != _newAddress, "HomeFi::!Change");
   85          _;

  141          // Revert if addrSet is true
  142:         require(!addrSet, "HomeFi::Set");
  143  

  190          // Revert if no change in lender fee
  191:         require(lenderFee != _newLenderFee, "HomeFi::!Change");
  192  

contracts/HomeFiProxy.sol:
   40      modifier nonZero(address _address) {
   41:         require(_address != address(0), "Proxy::0 address");
   42          _;

   80          // Revert if _implementations length is wrong. Indicating wrong set of _implementations.
   81:         require(_length == _implementations.length, "Proxy::Lengths !match");
   82  

  132          // Revert if _contractNames and _contractAddresses length mismatch
  133:         require(_length == _contractAddresses.length, "Proxy::Lengths !match");
  134  

contracts/Project.sol:
  122          // Revert if contractor has already confirmed his invitation
  123:         require(!contractorConfirmed, "Project::GC accepted");
  124  

  131          // Revert if decoded project address does not match this contract. Indicating incorrect _data.
  132:         require(_projectAddress == address(this), "Project::!projectAddress");
  133  
  134          // Revert if contractor address is invalid.
  135:         require(_contractor != address(0), "Project::0 address");
  136  

  149          // Revert if sender is not builder
  150:         require(_msgSender() == builder, "Project::!B");
  151  
  152          // Revert if contract not assigned
  153:         require(contractor != address(0), "Project::0 address");
  154  

  175          // Revert if decoded nonce is incorrect. This indicates wrong _data.
  176:         require(_nonce == hashChangeNonce, "Project::!Nonce");
  177  

  194          // Revert if try to lend 0
  195:         require(_cost > 0, "Project::!value>0");
  196  

  237          // Revert if decoded taskCount is incorrect. This indicates wrong data.
  238:         require(_taskCount == taskCount, "Project::!taskCount");
  239  
  240          // Revert if decoded project address does not match this contract. Indicating incorrect _data.
  241:         require(_projectAddress == address(this), "Project::!projectAddress");
  242  

  244          uint256 _length = _hash.length;
  245:         require(_length == _taskCosts.length, "Project::Lengths !match");
  246  

  276          // Revert if decoded nonce is incorrect. This indicates wrong data.
  277:         require(_nonce == hashChangeNonce, "Project::!Nonce");
  278  

  307          uint256 _length = _taskList.length;
  308:         require(_length == _scList.length, "Project::Lengths !match");
  309  

  340          // Revert if decoded project address does not match this contract. Indicating incorrect _data.
  341:         require(_projectAddress == address(this), "Project::!Project");
  342  

  368              for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
  369:                 require(tasks[_taskID].getState() == 3, "Project::!Complete");
  370              }

  405          // Revert if decoded project address does not match this contract. Indicating incorrect _data.
  406:         require(_project == address(this), "Project::!projectAddress");
  407  

  510          // Revert if decoded project address does not match this contract. Indicating incorrect _data.
  511:         require(_project == address(this), "Project::!projectAddress");
  512  

  529                  // If sender is task's subcontractor, revert if invitation is not accepted.
  530:                 require(getAlerts(_task)[2], "Project::!SCConfirmed");
  531              }

  752          // Revert if sc to invite is address 0
  753:         require(_sc != address(0), "Project::0 address");
  754  

contracts/ProjectFactory.sol:
  35          // Ensure an address is not the zero address (0x00)
  36:         require(_address != address(0), "PF::0 address");
  37          _;

  83          // Revert if sender is not HomeFi
  84:         require(_msgSender() == homeFi, "PF::!HomeFiContract");
  85  

contracts/libraries/Tasks.sol:
   43      modifier onlyInactive(Task storage _self) {
   44:         require(_self.state == TaskStatus.Inactive, "Task::active");
   45          _;

   49      modifier onlyActive(Task storage _self) {
   50:         require(_self.state == TaskStatus.Active, "Task::!Active");
   51          _;

  123          // Prerequisites //
  124:         require(_self.subcontractor == _sc, "Task::!SC");
  125  

---

[G-06] Upgrade pragma to 0.8.15 to save gas

Across the whole solution, the declared pragma is 0.8.6 Upgrading to 0.8.15 has many benefits, cheaper on gas is one of them. The following information is regarding 0.8.15 over 0.8.14. Assume that over 0.8.6 the save is higher!

According to the release note of 0.8.15: https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/
The benchmark shows saving of 2.5-10% Bytecode size,
Saving 2-8% Deployment gas,
And saving up to 6.2% Runtime gas.