Gas Optimizations

[code423n4]

Variable re-arrangement for storage packing

contractorConfirmed and contractorDelegated can be placed next to each other, as a result, 1 slot storage can be saved. According to the currently layout, they both occupy 1 slot, but after re-arrangement, they can be packed into 1 slot.

contracts/Project.sol

68:    bool public override contractorConfirmed;
    /// @inheritdoc IProject
78:    bool public override contractorDelegated;

Reference: Layout of State Variables in Storage.

SPLITTING REQUIRE() STATEMENTS THAT USE &&

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.

The demo of the gas comparison can be seen here.

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

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

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

And in Disputes.sol, the require() statement is in modifier resolvable(uint256 _disputeID) which might be called frequently, a gas saving can benefit.

USE CUSTOM ERRORS INSTEAD OF REVERT STRINGS

Custom errors from Solidity 0.8.4 are more gas efficient than revert strings (lower deployment cost and runtime cost when the revert condition is met) reference

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 are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

require() statements are heavily used across the rigor contracts:

• contracts/Project.sol
• contracts/Community.sol

These 2 sol files use around 50 require, and there are lots in other files.

optimization on the condition checks is worth consideration.

The demo of the gas comparison can be seen here.

Duplicate function of parent contract

Since inside these functions, the function is the same as the parent contract function, there is no need to rewrite it.

contracts/HomeFi.sol
/// @dev This is same as ERC2771ContextUpgradeable._msgSender()
function _msgSender()
internal
view
override(ContextUpgradeable, ERC2771ContextUpgradeable)
returns (address sender)
{
// We want to use the _msgSender() implementation of ERC2771ContextUpgradeable
return super._msgSender();
}

/// @dev This is same as ERC2771ContextUpgradeable._msgData()
function _msgData()
    internal
    view
    override(ContextUpgradeable, ERC2771ContextUpgradeable)
    returns (bytes calldata)
{
    // We want to use the _msgData() implementation of ERC2771ContextUpgradeable
    return super._msgData();
}

FOR-LOOPS LENGTH COULD BE CACHED

The for loop length can be cached to memory before the loop, even for memory variables, the comparison can been seen in the reference link at the end (the length() function for test).

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

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

650:        for (++j; j <= taskCount; j++) {

taskCount is a state variable, which uses sload opcode consumes 100 gas (warm access). While read from memory uses mload only use 3 gas.

Suggestion:

    uint length = names.length;

The demo of the loop gas comparison can be seen here.

FOR-LOOPS unchecked{ ++i } COSTS LESS GAS COMPARED TO i++ OR i += 1

Use ++i instead of i++ to increment the value of an uint variable, and for guaranteed non-overflow/underflow, it can be unchecked.

The demo of the loop gas comparison can be seen here.

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

Suggestion:
For readability, uncheck the whole for loop is the same.

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

FOR-LOOPS NO NEED TO EXPLICITLY INITIALIZE VARIABLES WITH DEFAULT VALUES

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.

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

The demo of the loop gas comparison can be seen here.

X = X + Y/X = X - Y IS CHEAPER THAN X += Y/X -= Y

contracts/Community.sol
421-423:
            _communities[_communityID]
                .projectDetails[_project]
                .totalLent += _amountToProject;
433-435:
            _communities[_communityID]
                .projectDetails[_project]
                .lentAmount += _lendingAmount;
798:        _interest -= _repayAmount;
contracts/HomeFi.sol
289:        projectCount += 1;

contracts/Project.sol
179:        hashChangeNonce += 1;
250:        _taskCount += 1;
290:        hashChangeNonce += 1;
440:        totalAllocated += _newCost - _taskCost;
711:        _cost += tasks[_taskID].cost;

431:        totalAllocated -= _withdrawDifference;
456:        totalAllocated -= _taskCost;
616:        _costToAllocate -= _taskCost;
663:        _costToAllocate -= _taskCost;
772:        totalLent -= _amount;

contracts/libraries/SignatureDecoder.sol
83:         v += 27;

Suggestion:
Consider using X = X + Y/X = X - Y.

The demo of the gas comparison can be seen here.

USE CALLDATA INSTEAD OF MEMORY

When arguments are read-only on external functions, the data location can be calldata.

contracts/Community.sol
    function reduceDebt(
        uint256 _communityID,
        address _project,
        uint256 _repayAmount,
        bytes memory _details
    ) external virtual override whenNotPaused {
        // Revert if sender is not _communityID owner (lender)
        require(
            _msgSender() == _communities[_communityID].owner,
            "Community::!Owner"
        );

        // Internal call to reduce debt
        _reduceDebt(_communityID, _project, _repayAmount, _details);
    }

contracts/HomeFi.sol
    function createProject(bytes memory _hash, address _currency)
        external
        override
        nonReentrant
    {}

    function mintNFT(address _to, string memory _tokenURI)
        internal
        returns (uint256)
    {}

The demo of the gas comparison can be seen here.

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.

The extra opcodes avoided are

CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2)

which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

contracts/Community.sol

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

285-291:
    function unpublishProject(uint256 _communityID, address _project)
        external
        override
        whenNotPaused
        isPublishedToCommunity(_communityID, _project)
        onlyProjectBuilder(_project)
    {}

297-304:
    function payPublishFee(uint256 _communityID, address _project)
        external
        override
        nonReentrant
        whenNotPaused
        isPublishedToCommunity(_communityID, _project)
        onlyProjectBuilder(_project)
    {}

331-341:
    function toggleLendingNeeded(
        uint256 _communityID,
        address _project,
        uint256 _lendingNeeded
    )
        external
        override
        whenNotPaused
        isPublishedToCommunity(_communityID, _project)
        onlyProjectBuilder(_project)
    {}

455-466:
    function repayLender(
        uint256 _communityID,
        address _project,
        uint256 _repayAmount
    )
        external
        virtual
        override
        nonReentrant
        whenNotPaused
        onlyProjectBuilder(_project)
    {}

contracts/DebtToken.sol

    function mint(address _to, uint256 _total)
        external
        override
        onlyCommunityContract
    {
        _mint(_to, _total);
    }

    /// @inheritdoc IDebtToken
    function burn(address _to, uint256 _total)
        external
        override
        onlyCommunityContract
    {
        _burn(_to, _total);
    }

contracts/Disputes.sol

    function resolveDispute(
        uint256 _disputeID,
        bytes calldata _judgement,
        bool _ratify
    ) external override onlyAdmin nonReentrant resolvable(_disputeID) {}

    function raiseDispute(bytes calldata _data, bytes calldata _signature)
        external
        override
        onlyProject
    {}

contracts/HomeFi.sol

    function setAddr(...)
        external
        override
        onlyAdmin {}

    function replaceAdmin(address _newAdmin)
        external
        override
        onlyAdmin
        nonZero(_newAdmin)
        noChange(admin, _newAdmin)
    {}

    function replaceTreasury(address _newTreasury)
        external
        override
        onlyAdmin
        nonZero(_newTreasury)
        noChange(treasury, _newTreasury)
    {}

    function replaceLenderFee(uint256 _newLenderFee)
        external
        override
        onlyAdmin
    {}

    function setTrustedForwarder(address _newForwarder)
        external
        override
        onlyAdmin
        noChange(trustedForwarder, _newForwarder)
    {}

Modifier ONLY CALLED ONCE CAN BE INLINED

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

onlyAdmin() and onlyProject() is only called once.
contracts/Disputes.sol

    function resolveDispute(
        uint256 _disputeID,
        bytes calldata _judgement,
        bool _ratify
    ) external override onlyAdmin nonReentrant resolvable(_disputeID) {}

    function raiseDispute(bytes calldata _data, bytes calldata _signature)
        external
        override
        onlyProject
    {}

approvedHashes check can be performed first

contracts/Project.sol

    function checkSignatureValidity(
        address _address,
        bytes32 _hash,
        bytes memory _signature,
        uint256 _signatureIndex
    ) internal {
        address _recoveredSignature = SignatureDecoder.recoverKey(
            _hash,
            _signature,
            _signatureIndex
        );
        require(
            _recoveredSignature == _address || approvedHashes[_address][_hash],
            "Project::invalid signature"
        );
        // delete from approvedHash
        delete approvedHashes[_address][_hash];
    }

If approvedHashes[_address][_hash] is evaluated to be true, there is no need for recoverKey and signature check, the extra operations can be saved.

Suggestion:

    function checkSignatureValidity(
        address _address,
        bytes32 _hash,
        bytes memory _signature,
        uint256 _signatureIndex
    ) internal {
        if (approvedHashes[_address][_hash]) {
            // delete from approvedHash
            delete approvedHashes[_address][_hash];
        } else {
            address _recoveredSignature = SignatureDecoder.recoverKey(
                _hash,
                _signature,
                _signatureIndex
            );
            require(
                _recoveredSignature == _address，
                "Project::invalid signature"
            );
        }        
    }

NFT usage

NFT is only created when createProject(), and not used elsewhere. If it does not have special usage, for transfer, claim ownership of a project, etc, it can be removed for gas saving and reduce code complexity.

contracts/HomeFi.sol

        mintNFT(_sender, string(_hash));

Multiple nonZero() modifier can be combined

There are many nonZero() modifiers, the only difference is the error message.

They can be combined and provide the error message as parameter. In this way, some duplicate compilation cost could be saved.

Suggestion:

• Use a library.
• Use a base contract, and inherit from it.

contracts/Community.sol

    modifier nonZero(address _address) {
        // Revert if _address zero address (0x00) (invalid)
        require(_address != address(0), "Community::0 address");
        _;
    }

contracts/Disputes.sol

    modifier nonZero(address _address) {
        // Revert if _address zero address (0x00)
        require(_address != address(0), "Disputes::0 address");
        _;
    }

contracts/HomeFi.sol

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

contracts/HomeFiProxy.sol

    modifier nonZero(address _address) {
        require(_address != address(0), "Proxy::0 address");
        _;
    }

contracts/ProjectFactory.sol

    modifier nonZero(address _address) {
        // Ensure an address is not the zero address (0x00)
        require(_address != address(0), "PF::0 address");
        _;
    }