Rigor Protocol contest - MiloTruck's results

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L392-L394 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/HomeFi.sol#L184-L197

Vulnerability details

Missing upper limit definition in replaceLenderFee() of HomeFi.sol

Impact

The admin of the HomeFi contract can set lenderFee to greater than 100%, forcing calls to lendToProject() to all projects created in the future to revert.

Proof of Concept

Using the function replaceLenderFee(), admins of the HomeFi contract can set lenderFee to any arbitrary uint256 value:

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

New projects that are created will then get its lenderFee from the HomeFi contract. When communities wish to lend to these projects, it calls lendToProject(), which has the following calculation:

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

If lenderFee a large value, such as type(uint256).max, the calculation shown above to overflow. This prevents any community from lending to any new projects.

Recommended Mitigation Steps

Consider adding a reasonable fee rate bounds checks in the replaceLenderFee() function. This would prevent potential griefing and increase the trust of users in the contract.

• Gas Report
  • For-loops: Index initialized with default value
  • For-Loops: Cache array length outside of loops
  • For-Loops: Index increments can be left unchecked
  • Arithmetics: ++i costs less gas compared to i++ or i += 1
  • Arithmetics: Use != 0 instead of > 0 for unsigned integers in require statements
  • Errors: Use multiple require statements instead of &&
  • Duplicated require() checks should be refactored to a modifier or function
  • Errors: Use custom errors instead of revert strings
  • Unnecessary initialization of variables with default values
  • State variables should be cached in stack variables rather than re-reading them from storage
  • <x> += <y> costs more gas than <x> = <x> + <y> for state variables
  • Using bool for storage incurs overhead
  • Stack variable used to cache a state variable is only accessed once
  • Use calldata instead of memory for read-only arguments in external functions
  • Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead
  • Not using the named return variables when a function returns, wastes deployment gas
  • internal functions only called once can be inlined to save gas
  • Multiple accesses of a mapping/array should use a local variable cache

Gas Report

For-loops: Index initialized with default value

Uninitialized uint variables are assigned with a default value of 0.

Thus, in for-loops, explicitly initializing an index with 0 costs unnecesary gas. For example, the following code:

for (uint256 i = 0; i < length; ++i) {

can be changed to:

for (uint256 i; i < length; ++i) {

Consider declaring the following lines without explicitly setting the index to 0:

contracts/Community.sol:
 624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {

contracts/HomeFiProxy.sol:
  87:        for (uint256 i = 0; i < _length; i++) {
 136:        for (uint256 i = 0; i < _length; i++) {

contracts/Project.sol:
 248:        for (uint256 i = 0; i < _length; i++) {
 311:        for (uint256 i = 0; i < _length; i++) {
 322:        for (uint256 i = 0; i < _length; i++) {

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

For-Loops: Cache array length outside of loops

Reading an array's length at each iteration has the following gas overheads:

• storage arrays incur a Gwarmaccess (100 gas)
• memory arrays use mload (3 gas)
• calldata arrays use calldataload (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset. This would save around 3 gas per iteration.

For example:

for (uint256 i; i < arr.length; ++i) {}

can be changed to:

uint256 len = arr.length;
for (uint256 i; i < len; ++i) {}

Consider making the following change to these lines:

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

For-Loops: Index increments can be left unchecked

From Solidity v0.8 onwards, all arithmetic operations come with implicit overflow and underflow checks.

In for-loops, as it is impossible for the index to overflow, index increments can be left unchecked to save 30-40 gas per loop iteration.

For example, the code below:

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

can be changed to:

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

Consider making the following change to these lines:

contracts/Community.sol:
 624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {

contracts/HomeFiProxy.sol:
  87:        for (uint256 i = 0; i < _length; i++) {
 136:        for (uint256 i = 0; i < _length; i++) {

contracts/Project.sol:
 248:        for (uint256 i = 0; i < _length; i++) {
 311:        for (uint256 i = 0; i < _length; i++) {
 322:        for (uint256 i = 0; i < _length; i++) {
 368:        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
 603:        for (; i < _changeOrderedTask.length; i++) {
 650:        for (++j; j <= taskCount; j++) {
 710:        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {

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

Arithmetics: ++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2, thus it costs more gas.

The same logic applies for --i and i--.

Consider using ++i instead of i++ or i += 1 in the following instances:

contracts/Community.sol:
 140:        communityCount++;
 624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {

contracts/HomeFiProxy.sol:
  87:        for (uint256 i = 0; i < _length; i++) {
 136:        for (uint256 i = 0; i < _length; i++) {

contracts/Project.sol:
 179:        hashChangeNonce += 1;
 248:        for (uint256 i = 0; i < _length; i++) {
 250:        _taskCount += 1;
 290:        hashChangeNonce += 1;
 311:        for (uint256 i = 0; i < _length; i++) {
 322:        for (uint256 i = 0; i < _length; i++) {
 368:        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {
 603:        for (; i < _changeOrderedTask.length; i++) {
 625:        _loopCount++;
 650:        for (++j; j <= taskCount; j++) {
 672:        _loopCount++;
 710:        for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {

contracts/HomeFi.sol:
 289:        projectCount += 1;

contracts/Disputes.sol:
 121:        emit DisputeRaised(disputeCount++, _reason);

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

Arithmetics: Use != 0 instead of > 0 for unsigned integers in require statements

uint will never go below 0. Thus, checking if != 0 rather than > 0 is sufficient, which would save 6 gas per instance.

Consider changing > 0 to != 0 in these lines:

contracts/Community.sol:
 764:        require(_repayAmount > 0, "Community::!repay");

contracts/Project.sol:
 195:        require(_cost > 0, "Project::!value>0");

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

Errors: Use multiple require statements instead of &&

Instead of using a single require statement with the && operator, use multiple require statements. With reference to this issue, this helps to reduce runtime gas cost at the expense of a larger deployment gas cost, which becomes cheaper with enough runtime calls.

A require statement can be split as such:

// Original code:
require(a && b, 'error');

// Changed to:
require(a, 'error: a');
require(b, 'error: b');

Instances where multiple require statements should be used:

contracts/Community.sol:
 353:        require(
 354:            _lendingNeeded >= _communityProject.totalLent &&
 355:                _lendingNeeded <= IProject(_project).projectCost(),
 356:            "Community::invalid lending"
 357:        );

contracts/Disputes.sol:
  61:        require(
  62:            _disputeID < disputeCount &&
  63:                disputes[_disputeID].status == Status.Active,
  64:            "Disputes::!Resolvable"
  65:        );


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

Duplicated require() checks should be refactored to a modifier or function

If a require() statement that is used to validate function parameters or global variables is present across multiple functions in a contract, it should be rewritten into modifier if possible. This would help to reduce code deployment size, which saves gas, and improve code readability.

The following require() statements are repeated multiple times:

contracts/Project.sol:
 132:        require(_projectAddress == address(this), "Project::!projectAddress");
 241:        require(_projectAddress == address(this), "Project::!projectAddress");
 406:        require(_project == address(this), "Project::!projectAddress");
 511:        require(_project == address(this), "Project::!projectAddress");

contracts/Project.sol:
 176:        require(_nonce == hashChangeNonce, "Project::!Nonce");
 277:        require(_nonce == hashChangeNonce, "Project::!Nonce");

Errors: Use custom errors instead of revert strings

Since Solidity v0.8.4, custom errors can be used rather than require statements.

Taken from Custom Errors in Solidity:

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.

Custom errors reduce runtime gas costs as they save about 50 gas each time they are hit by avoiding having to allocate and store the revert string. Furthermore, not definiting error strings also help to reduce deployment gas costs.

Instances where custom errors can be used instead:

contracts/Community.sol:
  69:        require(_address != address(0), "Community::0 address");
  75:        require(_msgSender() == homeFi.admin(), "Community::!admin");

  81:        require(
  82:            projectPublished[_project] == _communityID,
  83:            "Community::!published"
  84:        );


  90:        require(
  91:            _msgSender() == IProject(_project).builder(),
  92:            "Community::!Builder"
  93:        );


 131:        require(
 132:            !restrictedToAdmin || _sender == homeFi.admin(),
 133:            "Community::!admin"
 134:        );


 159:        require(
 160:            _communities[_communityID].owner == _msgSender(),
 161:            "Community::!owner"
 162:        );


 191:        require(
 192:            !_community.isMember[_newMemberAddr],
 193:            "Community::Member Exists"
 194:        );


 235:        require(
 236:            _publishNonce == _community.publishNonce,
 237:            "Community::invalid publishNonce"
 238:        );

 241:        require(homeFi.isProjectExist(_project), "Community::Project !Exists");
 248:        require(_community.isMember[_builder], "Community::!Member");

 251:        require(
 252:            _projectInstance.currency() == _community.currency,
 253:            "Community::!Currency"
 254:        );


 312:        require(
 313:            !_communityProject.publishFeePaid,
 314:            "Community::publish fee paid"
 315:        );


 347:        require(
 348:            _communityProject.publishFeePaid,
 349:            "Community::publish fee !paid"
 350:        );


 353:        require(
 354:            _lendingNeeded >= _communityProject.totalLent &&
 355:                _lendingNeeded <= IProject(_project).projectCost(),
 356:            "Community::invalid lending"
 357:        );


 384:        require(
 385:            _sender == _communities[_communityID].owner,
 386:            "Community::!owner"
 387:        );


 400:        require(
 401:            _amountToProject <=
 402:                _communities[_communityID]
 403:                    .projectDetails[_project]
 404:                    .lendingNeeded -
 405:                    _communities[_communityID]
 406:                        .projectDetails[_project]
 407:                        .totalLent,
 408:            "Community::lending>needed"
 409:        );


 491:        require(
 492:            _msgSender() == _communities[_communityID].owner,
 493:            "Community::!Owner"
 494:        );

 536:        require(_builder == _projectInstance.builder(), "Community::!Builder");

 539:        require(
 540:            _lender == _communities[_communityID].owner,
 541:            "Community::!Owner"
 542:        );

 557:        require(!restrictedToAdmin, "Community::restricted");
 568:        require(restrictedToAdmin, "Community::!restricted");
 764:        require(_repayAmount > 0, "Community::!repay");
 792:        require(_lentAndInterest >= _repayAmount, "Community::!Liquid");

 886:        require(
 887:            _recoveredSignature == _address || approvedHashes[_address][_hash],
 888:            "Community::invalid signature"
 889:        );

contracts/HomeFiProxy.sol:
  41:        require(_address != address(0), "Proxy::0 address");
  81:        require(_length == _implementations.length, "Proxy::Lengths !match");

 105:        require(
 106:            contractAddress[_contractName] == address(0),
 107:            "Proxy::Name !OK"
 108:        );

 133:        require(_length == _contractAddresses.length, "Proxy::Lengths !match");

contracts/DebtToken.sol:
  31:        require(
  32:            communityContract == _msgSender(),
  33:            "DebtToken::!CommunityContract"
  34:        );

  50:        require(_communityContract != address(0), "DebtToken::0 address");

contracts/Project.sol:
 123:        require(!contractorConfirmed, "Project::GC accepted");
 132:        require(_projectAddress == address(this), "Project::!projectAddress");
 135:        require(_contractor != address(0), "Project::0 address");
 150:        require(_msgSender() == builder, "Project::!B");
 153:        require(contractor != address(0), "Project::0 address");
 176:        require(_nonce == hashChangeNonce, "Project::!Nonce");

 189:        require(
 190:            _sender == builder || _sender == homeFi.communityContract(),
 191:            "Project::!Builder&&!Community"
 192:        );

 195:        require(_cost > 0, "Project::!value>0");

 199:        require(
 200:            projectCost() >= uint256(_newTotalLent),
 201:            "Project::value>required"
 202:        );

 238:        require(_taskCount == taskCount, "Project::!taskCount");
 241:        require(_projectAddress == address(this), "Project::!projectAddress");
 245:        require(_length == _taskCosts.length, "Project::Lengths !match");
 277:        require(_nonce == hashChangeNonce, "Project::!Nonce");

 301:        require(
 302:            _msgSender() == builder || _msgSender() == contractor,
 303:            "Project::!Builder||!GC"
 304:        );

 308:        require(_length == _scList.length, "Project::Lengths !match");
 341:        require(_projectAddress == address(this), "Project::!Project");
 369:        require(tasks[_taskID].getState() == 3, "Project::!Complete");
 406:        require(_project == address(this), "Project::!projectAddress");
 511:        require(_project == address(this), "Project::!projectAddress");

 515:        require(
 516:            signer == builder || signer == contractor,
 517:            "Project::!(GC||Builder)"
 518:        );


 521:        require(
 522:            signer == builder ||
 523:                signer == contractor ||
 524:                signer == tasks[_task].subcontractor,
 525:            "Project::!(GC||Builder||SC)"
 526:        );

 530:        require(getAlerts(_task)[2], "Project::!SCConfirmed");
 753:        require(_sc != address(0), "Project::0 address");

 886:        require(
 887:            _recoveredSignature == _address || approvedHashes[_address][_hash],
 888:            "Project::invalid signature"
 889:        );


 906:        require(
 907:            ((_amount / 1000) * 1000) == _amount,
 908:            "Project::Precision>=1000"
 909:        );

contracts/HomeFi.sol:
  73:        require(admin == _msgSender(), "HomeFi::!Admin");
  78:        require(_address != address(0), "HomeFi::0 address");
  84:        require(_oldAddress != _newAddress, "HomeFi::!Change");
 142:        require(!addrSet, "HomeFi::Set");
 191:        require(lenderFee != _newLenderFee, "HomeFi::!Change");

 255:        require(
 256:            _currency == tokenCurrency1 ||
 257:                _currency == tokenCurrency2 ||
 258:                _currency == tokenCurrency3,
 259:            "HomeFi::!Currency"
 260:        );

contracts/ProjectFactory.sol:
  36:        require(_address != address(0), "PF::0 address");

  64:        require(
  65:            _msgSender() == IHomeFi(homeFi).admin(),
  66:            "ProjectFactory::!Owner"
  67:        );

  84:        require(_msgSender() == homeFi, "PF::!HomeFiContract");

contracts/Disputes.sol:
  39:        require(_address != address(0), "Disputes::0 address");
  46:        require(homeFi.admin() == _msgSender(), "Disputes::!Admin");
  52:        require(homeFi.isProjectExist(_msgSender()), "Disputes::!Project");

  61:        require(
  62:            _disputeID < disputeCount &&
  63:                disputes[_disputeID].status == Status.Active,
  64:            "Disputes::!Resolvable"
  65:        );


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

 183:        require(_result, "Disputes::!Member");

contracts/libraries/Tasks.sol:
  44:        require(_self.state == TaskStatus.Inactive, "Task::active");
  50:        require(_self.state == TaskStatus.Active, "Task::!Active");

  56:        require(
  57:            _self.alerts[uint256(Lifecycle.TaskAllocated)],
  58:            "Task::!funded"
  59:        );

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

Unnecessary initialization of variables with default values

Uninitialized variables are assigned with a default value depending on its type:

• uint: 0
• bool: false
• address: address(0)

Thus, explicitly initializing a variable with its default value costs unnecesary gas. For example, the following code:

bool b = false;
address c = address(0);
uint256 a = 0;

can be changed to:

uint256 a;
bool b;
address c;

Consider declaring the following lines without explicitly setting a value:

contracts/Project.sol:
 412:        bool _unapproved = false;

State variables should be cached in stack variables rather than re-reading them from storage

If a state variable is read from storage multiple times in a function, it should be cached in a stack variable.

Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

communityCount in createCommunity():

contracts/Community.sol:
 143:        CommunityStruct storage _community = _communities[communityCount];
 150:        emit CommunityAdded(communityCount, _sender, _currency, _hash);

builder in lendToProject():

contracts/Project.sol:
 190:        _sender == builder || _sender == homeFi.communityContract(),
 204:        if (_sender == builder) {

totalLent in allocateFunds():

contracts/Project.sol:
 579:        uint256 _costToAllocate = totalLent - totalAllocated;
 697:        totalAllocated = totalLent - _costToAllocate;

taskCount in allocateFunds():

contracts/Project.sol:
 592:        taskCount - j + _changeOrderedTask.length - i
 648:        if (j < taskCount) {
 650:        for (++j; j <= taskCount; j++) {  // @audit This reads taskCount per iteration
 681:        if (j > taskCount) {
 682:        lastAllocatedTask = taskCount;

contractor in checkSignature():

contracts/Project.sol:
 798:        if (contractor == address(0)) {
 807:        checkSignatureValidity(contractor, _hash, _signature, 0);
 813:        checkSignatureValidity(contractor, _hash, _signature, 1);

contractor in checkSignatureTask():

contracts/Project.sol:
 842:        if (contractor == address(0)) {
 852:        checkSignatureValidity(contractor, _hash, _signature, 0);
 859:        checkSignatureValidity(contractor, _hash, _signature, 1);

projectCount in createProject():

contracts/HomeFi.sol:
 228:        projects[projectCount] = _project;
 229:        projectTokenId[_project] = projectCount;
 231:        emit ProjectAdded(projectCount, _project, _sender, _currency, _hash);

projectCount in mintNFT():

contracts/HomeFi.sol:
 292:        _mint(_to, projectCount);
 293:        _setTokenURI(projectCount, _tokenURI);
 295:        emit NftCreated(projectCount, _to);
 296:        return projectCount;

homeFi in createProject():

contracts/ProjectFactory.sol:
  84:        require(_msgSender() == homeFi, "PF::!HomeFiContract");
  90:        Project(_clone).initialize(_currency, _sender, homeFi);

disputeCount in raiseDispute():

contracts/Disputes.sol:
 112:        Dispute storage _dispute = disputes[disputeCount];
 121:        emit DisputeRaised(disputeCount++, _reason);

<x> += <y> costs more gas than <x> = <x> + <y> for state variables

The above also applies to state variables that use -=.

contracts/Project.sol:
 431:        totalAllocated -= _withdrawDifference;
 440:        totalAllocated += _newCost - _taskCost;
 456:        totalAllocated -= _taskCost;
 772:        totalLent -= _amount;

Using bool for storage incurs overhead

Declaring storage variables as bool is more expensive compared to uint256, as explained here:

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.

Use uint256(1) and uint256(2) rather than true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past.

Consider redefining the following state variables to use uint256 instead of bool:

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

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

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

contracts/HomeFi.sol:
  50:        bool public override addrSet;

Stack variable used to cache a state variable is only accessed once

If the variable is only accessed once, it is cheaper to use the state variable directly.

contracts/Project.sol:
 420:        uint256 _totalAllocated = totalAllocated;

Use calldata instead of memory for read-only arguments in external functions

When an external function with a memory array is called, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length).

Using calldata directly helps to save gas as values are read directly from calldata using calldataload, thus removing the need for such a loop in the contract code during runtime execution.

Also, structs have the same overhead as an array of length one.

Consider changing the following from memory to calldata:

contracts/Community.sol:
 488:        bytes memory _details

contracts/DebtToken.sol:
  45:        string memory name_,
  46:        string memory symbol_,

contracts/HomeFi.sol:
 210:        function createProject(bytes memory _hash, address _currency)

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

As seen from here:

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.

However, this does not apply to storage values as using reduced-size types might be beneficial to pack multiple elements into a single storage slot. Thus, where appropriate, use uint256/int256 and downcast when needed.

Consider using uint256/int256 instead of bool for the following:

contracts/DebtToken.sol:
  47:        uint8 decimals_
  82:        function decimals() public view virtual override returns (uint8) {

contracts/Disputes.sol:
 100:        uint8 _actionType,

contracts/libraries/SignatureDecoder.sol:
  29:        uint8 v;
  65:        uint8 v,

Not using the named return variables when a function returns, wastes deployment gas

contracts/Community.sol:
 906:        return super._msgSender();

contracts/Project.sol:
 722:        return tasks[_taskID].getAlerts();

contracts/HomeFi.sol:
 310:        return super._msgSender();

contracts/libraries/Tasks.sol:
 196:        return uint256(_self.state);

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

Consider inlining the following internal functions:

contracts/HomeFiProxy.sol:
 197:        function _replaceImplementation(
 198:            bytes2 _contractName,
 199:            address _contractAddress
 200:        ) internal nonZero(_contractAddress) {

contracts/Project.sol:
 770:        function autoWithdraw(uint256 _amount) internal {

contracts/HomeFi.sol:
 284:        function mintNFT(address _to, string memory _tokenURI)
 285:            internal
 286:            returns (uint256)
 287:        {

contracts/Disputes.sol:
 207:        function resolveHandler(uint256 _disputeID) internal {

 236:        function executeTaskAdd(address _project, bytes memory _actionData)
 237:            internal
 238:        {


 254:        function executeTaskChange(address _project, bytes memory _actionData)
 255:            internal
 256:        {


 268:        function executeTaskPay(address _project, bytes memory _actionData)
 269:            internal
 270:        {

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

If a mapping/array is read with the same key/index multiple times in a function, it should be cached in a stack variable.

Caching a mapping's value in a local storage variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory.

_communities[_communityID] in lendToProject():

contracts/Community.sol:
 385:        _sender == _communities[_communityID].owner,

 401:        _amountToProject <=
 402:            _communities[_communityID]


 401:        _amountToProject <=
 402:            _communities[_communityID]
 403:                .projectDetails[_project]
 404:                .lendingNeeded -
 405:                _communities[_communityID]

 412:        IDebtToken _currency = _communities[_communityID].currency;
 427:        _communities[_communityID].projectDetails[_project].lentAmount > 0

_communities[_communityID] in repayLender():

contracts/Community.sol:
 471:        address _lender = _communities[_communityID].owner;
 474:        _communities[_communityID].currency.safeTransferFrom(

_communities[_communityID] in members():

contracts/Community.sol:
 620:        _communities[_communityID].memberCount
 624:        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
 625:        _members[i] = _communities[_communityID].members[i];

_communities[_communityID] in returnToLender():

contracts/Community.sol:
 680:        ProjectDetails storage _communityProject = _communities[_communityID]

 691:        _lentAmount *
 692:        _communities[_communityID].projectDetails[_project].apr *

_communities[_communityID] in claimInterest():

contracts/Community.sol:
 836:        address _lender = _communities[_communityID].owner;
 837:        ProjectDetails storage _communityProject = _communities[_communityID]

tasks[_taskID] in setComplete():

contracts/Project.sol:
 350:        tasks[_taskID].setComplete();
 354:        tasks[_taskID].subcontractor,
 355:        tasks[_taskID].cost

tasks[_taskID] in changeOrder():

contracts/Project.sol:
 409:        uint256 _taskCost = tasks[_taskID].cost;
 423:        if (tasks[_taskID].alerts[1]) {
 449:        tasks[_taskID].unApprove();
 452:        tasks[_taskID].unAllocateFunds();
 470:        if (_newSC != tasks[_taskID].subcontractor) {
 474:        tasks[_taskID].unApprove();

tasks[_task] in raiseDispute():

contracts/Project.sol:
 522:        signer == builder ||
 523:            signer == contractor ||
 524:            signer == tasks[_task].subcontractor,

 528:        if (signer == tasks[_task].subcontractor) {

tasks[id] in getTask():

contracts/Project.sol:
 553:        cost = tasks[id].cost;
 554:        subcontractor = tasks[id].subcontractor;
 555:        state = tasks[id].state;

_changeOrderedTask[i] in allocateFunds():

contracts/Project.sol:
 605:        uint256 _taskCost = tasks[_changeOrderedTask[i]].cost;
 619:        tasks[_changeOrderedTask[i]].fundTask();
 622:        _tasksAllocated[_loopCount] = _changeOrderedTask[i];

tasks[_changeOrderedTask[i]] in allocateFunds():

contracts/Project.sol:
 605:        uint256 _taskCost = tasks[_changeOrderedTask[i]].cost;
 619:        tasks[_changeOrderedTask[i]].fundTask();

tasks[j] in allocateFunds():

contracts/Project.sol:
 652:        uint256 _taskCost = tasks[j].cost;
 666:        tasks[j].fundTask();