Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #410

Open
code423n4 opened this issue Aug 6, 2022 · 0 comments
Open

Gas Optimizations #410

code423n4 opened this issue Aug 6, 2022 · 0 comments
Labels

Comments

@code423n4
Copy link
Contributor

Gas Optimizations

HomeFi

  • Prefix incrementation in the mintNFT function (change projectCount += 1 to ++projectCount)
  • Use the calldata modifier on struct and array arguments instead of memory in the createProject and mintNFT functions
  • Use return value of mintNFT instead of accessing the projectCount storage variable 3 times
    code before:
    function createProject(bytes memory _hash, address _currency)
        external
        override
        nonReentrant
    {
        // Revert if currency not supported by HomeFi
        validCurrency(_currency);
    
        address _sender = _msgSender();
    
        // Create a new project Clone and mint a new NFT for it
        address _project = projectFactoryInstance.createProject(
            _currency,
            _sender
        );
        mintNFT(_sender, string(_hash));
    
        // Update project related mappings
        projects[projectCount] = _project;
        projectTokenId[_project] = projectCount;
    
        emit ProjectAdded(projectCount, _project, _sender, _currency, _hash);
    }
    code after:
    function createProject(bytes memory _hash, address _currency)
        external
        override
        nonReentrant
    {
        // Revert if currency not supported by HomeFi
        validCurrency(_currency);
    
        address _sender = _msgSender();
    
        // Create a new project Clone and mint a new NFT for it
        address _project = projectFactoryInstance.createProject(
            _currency,
            _sender
        );
        uint tokenId = mintNFT(_sender, string(_hash));
    
        // Update project related mappings
        projects[tokenId] = _project;
        projectTokenId[_project] = tokenId;
    
        emit ProjectAdded(tokenId, _project, _sender, _currency, _hash);
    }

ProjectFactory

  • Cache homeFi in the createProject function instead of accessing the same storage slot twice
    code before:
    function createProject(address _currency, address _sender)
        external
        override
        returns (address _clone)
    {
        // Revert if sender is not HomeFi
        require(_msgSender() == homeFi, "PF::!HomeFiContract");
    
        // Create clone of Project implementation
        _clone = ClonesUpgradeable.clone(underlying);
    
        // Initialize project
        Project(_clone).initialize(_currency, _sender, homeFi);
    }
    code after:
    function createProject(address _currency, address _sender)
        external
        override
        returns (address _clone)
    {
        address _homeFi = homeFi;
        // Revert if sender is not HomeFi
        require(_msgSender() == _homeFi, "PF::!HomeFiContract");
    
        // Create clone of Project implementation
        _clone = ClonesUpgradeable.clone(underlying);
    
        // Initialize project
        Project(_clone).initialize(_currency, _sender, _homeFi);
    }

Project

  • Use _homeFiAddress in the initialize instead of accessing the storage twice to read homeFi (which contains the same value)
    code before:

    function initialize(
        address _currency,
        address _sender,
        address _homeFiAddress
    ) external override initializer {
        // Initialize variables
        homeFi = IHomeFi(_homeFiAddress);
        disputes = homeFi.disputesContract();
        lenderFee = homeFi.lenderFee();
        builder = _sender;
        currency = IDebtToken(_currency);
    }

    code after:

    function initialize(
        address _currency,
        address _sender,
        address _homeFiAddress
    ) external override initializer {
        // Initialize variables
        homeFi = IHomeFi(_homeFiAddress);
        disputes = IHomeFi(_homeFiAddress).disputesContract();
        lenderFee = IHomeFi(_homeFiAddress).lenderFee();
        builder = _sender;
        currency = IDebtToken(_currency);
    }

    another way is to change the _homeFiAddress argument to IHomeFi type

  • Cache contractor in the checkSignature function
    code before:

    function checkSignature(bytes calldata _data, bytes calldata _signature)
        internal
    {
        // Calculate hash from bytes
        bytes32 _hash = keccak256(_data);
    
        // When there is no contractor
        if (contractor == address(0)) {
            // Check for builder's signature
            checkSignatureValidity(builder, _hash, _signature, 0);
        }
        // When there is a contractor
        else {
            // When builder has delegated his rights to contractor
            if (contractorDelegated) {
                //  Check contractor's signature
                checkSignatureValidity(contractor, _hash, _signature, 0);
            }
            // When builder has not delegated rights to contractor
            else {
                // Check for both B and GC signatures
                checkSignatureValidity(builder, _hash, _signature, 0);
                checkSignatureValidity(contractor, _hash, _signature, 1);
            }
        }
    }

    code after:

    function checkSignature(bytes calldata _data, bytes calldata _signature)
        internal
    {
        // Calculate hash from bytes
        bytes32 _hash = keccak256(_data);
    
        address _contractor = contractor;
    
        // When there is no contractor
        if (_contractor == address(0)) {
            // Check for builder's signature
            checkSignatureValidity(builder, _hash, _signature, 0);
        }
        // When there is a contractor
        else {
            // When builder has delegated his rights to contractor
            if (contractorDelegated) {
                //  Check contractor's signature
                checkSignatureValidity(_contractor, _hash, _signature, 0);
            }
            // When builder has not delegated rights to contractor
            else {
                // Check for both B and GC signatures
                checkSignatureValidity(builder, _hash, _signature, 0);
                checkSignatureValidity(_contractor, _hash, _signature, 1);
            }
        }
    }
  • Cache hashChangeNonce in updateProjectHash
    code before:

    // Revert if decoded nonce is incorrect. This indicates wrong _data.
    require(_nonce == hashChangeNonce, "Project::!Nonce");
    
    // Increment to ensure a set of data and signature cannot be re-used.
    hashChangeNonce += 1;

    code after:

    // Revert if decoded nonce is incorrect. This indicates wrong _data.
    // Increment to ensure a set of data and signature cannot be re-used.
    require(_nonce == hashChangeNonce++, "Project::!Nonce");
  • Cache the condition in the lendToProject function instead of calculating it twice

    address _sender = _msgSender();
    bool senderIsBuilder == _sender == builder;
    
    // Revert if sender is not builder or Community Contract (lender)
    require(
        senderIsBuilder || _sender == homeFi.communityContract(),
        "Project::!Builder&&!Community"
    );
    
    // Revert if try to lend 0
    require(_cost > 0, "Project::!value>0");
    
    // Revert if try to lend more than project cost
    uint256 _newTotalLent = totalLent + _cost;
    require(
        projectCost() >= uint256(_newTotalLent),
        "Project::value>required"
    );
    
    if (senderIsBuilder) {
        // Transfer assets from builder to this contract
        currency.safeTransferFrom(_sender, address(this), _cost);
    }
    
    // ...
    
  • Optimizations to the allocateFunds function:

    1. cache totalLent and taskCount
    2. cache _changeOrderedTask[i]
    3. cache tasks[j]
    4. save the array's length instead of accessing it in every iteration (_changeOrderedTask.length)
    5. prefix incrementation of the loop indices
      The code will look like this after the changes:
    function allocateFunds() public override {
        // Max amount out times this loop will run
        // This is to ensure the transaction do not run out of gas (max gas limit)
        uint256 _maxLoop = 50;
    
        uint _taskCount = taskCount;
        uint _totalLent = totalLent;
    
        // Difference of totalLent and totalAllocated is what can be used to allocate new tasks
        uint256 _costToAllocate = _totalLent - totalAllocated;
    
        // Bool if max loop limit is exceeded
        bool _exceedLimit;
    
        // Local instance of lastAllocatedChangeOrderTask. To save gas.
        uint256 i = lastAllocatedChangeOrderTask;
    
        // Local instance of lastAllocatedTask. To save gas.
        uint256 j = lastAllocatedTask;
    
        uint length = _changeOrderedTask.length;
    
        // Initialize empty array in which allocated tasks will be added.
        uint256[] memory _tasksAllocated = new uint256[](
            _taskCount - j + length - i
        );
    
        // Number of times a loop has run.
        uint256 _loopCount;
    
        Task storage _task;
    
        /// CHANGE ORDERED TASK FUNDING ///
    
        // Any tasks added to _changeOrderedTask will be allocated first
        if (length > 0) {
            // Loop from lastAllocatedChangeOrderTask to _changeOrderedTask length (until _maxLoop)
            for (; i < length; ++i) {
                _changeOrderedTask_i = _changeOrderedTask[i];
                _task = tasks[_changeOrderedTask_i];
    
                // Local instance of task cost. To save gas.
                uint256 _taskCost = _task.cost;
    
                // If _maxLoop limit is reached then stop looping
                if (_loopCount >= _maxLoop) {
                    _exceedLimit = true;
                    break;
                }
    
                // If there is enough funds to allocate this task
                if (_costToAllocate >= _taskCost) {
                    // Reduce task cost from _costToAllocate
                    _costToAllocate -= _taskCost;
    
                    // Mark the task as allocated
                    _task.fundTask();
    
                    // Add task to _tasksAllocated array
                    _tasksAllocated[_loopCount] = _changeOrderedTask_i;
    
                    // Increment loop counter
                    ++_loopCount;
                }
                // If there are not enough funds to allocate this task then stop looping
                else {
                    break;
                }
            }
    
            // If all the change ordered tasks are allocated, then delete
            // the changeOrderedTask array and reset lastAllocatedChangeOrderTask.
            if (i == length) {
                lastAllocatedChangeOrderTask = 0;
                delete _changeOrderedTask;
            }
            // Else store the last allocated change order task index.
            else {
                lastAllocatedChangeOrderTask = i;
            }
        }
    
        /// TASK FUNDING ///
    
        // If lastAllocatedTask is lesser than taskCount, that means there are un-allocated tasks
        if (j < _taskCount) {
            // Loop from lastAllocatedTask + 1 to taskCount (until _maxLoop)
            for (++j; j <= _taskCount; j++) {
    
                _task = tasks[j];
    
                // Local instance of task cost. To save gas.
                uint256 _taskCost = _task.cost;
    
                // If _maxLoop limit is reached then stop looping
                if (_loopCount >= _maxLoop) {
                    _exceedLimit = true;
                    break;
                }
    
                // If there is enough funds to allocate this task
                if (_costToAllocate >= _taskCost) {
                    // Reduce task cost from _costToAllocate
                    _costToAllocate -= _taskCost;
    
                    // Mark the task as allocated
                    _task.fundTask();
    
                    // Add task to _tasksAllocated array
                    _tasksAllocated[_loopCount] = j;
    
                    // Increment loop counter
                    _loopCount++;
                }
                // If there are not enough funds to allocate this task then stop looping
                else {
                    break;
                }
            }
    
            // If all pending tasks are allocated store lastAllocatedTask equal to taskCount
            if (j > _taskCount) {
                lastAllocatedTask = _taskCount;
            }
            // If not all tasks are allocated store updated lastAllocatedTask
            else {
                lastAllocatedTask = --j;
            }
        }
    
        // If any tasks is allocated, then emit event
        if (_loopCount > 0) emit TaskAllocated(_tasksAllocated);
    
        // If allocation was incomplete, then emit event
        if (_exceedLimit) emit IncompleteAllocation();
    
        // Update totalAllocated with all allocations
        totalAllocated = _totalLent - _costToAllocate;
    }
  • Emit the event in the calling function instead of sending it to _inviteSC, that will save the gas spent on the check of the sent boolean.

    The function will look like this now:

    function _inviteSC(
        uint256 _taskID,
        address _sc
    ) internal {
        // Revert if sc to invite is address 0
        require(_sc != address(0), "Project::0 address");
    
        // Internal call to tasks invite contractor
        tasks[_taskID].inviteSubcontractor(_sc);
    }

    The call from inviteSC will look like this:

    function inviteSC(uint256[] calldata _taskList, address[] calldata _scList)
        external
        override
    {
        // Revert if sender is neither builder nor contractor.
        require(
            _msgSender() == builder || _msgSender() == contractor,
            "Project::!Builder||!GC"
        );
    
        // Revert if taskList array length not equal to scList array length.
        uint256 _length = _taskList.length;
        require(_length == _scList.length, "Project::Lengths !match");
    
        // Invite subcontractor for each task.
        for (uint256 i = 0; i < _length; i++) {
            _inviteSC(_taskList[i], _scList[i]);
        }
    
        emit MultipleSCInvited(_taskList, _scList);
    }

    And the call from changeOrder will look like this:

    // If new subcontractor is not zero address.
    if (_newSC != address(0)) {
        // Invite the new subcontractor for the task.
        _inviteSC(_taskID, _newSC);
        emit SingleSCInvited(_taskID, _newSC);
    }

Community

  • Prefix incrementation on line 140
  • Cache communityCount
    function createCommunity(bytes calldata _hash, address _currency)
        external
        override
        whenNotPaused
    {
        // Local variable for sender. For gas saving.
        address _sender = _msgSender();
    
        // Revert if community creation is paused or sender is not HomeFi admin
        require(
            !restrictedToAdmin || _sender == homeFi.admin(),
            "Community::!admin"
        );
    
        // Revert if currency is not supported by HomeFi
        homeFi.validCurrency(_currency);
    
        uint _communityCount = communityCount + 1;
    
        // Increment community counter
        communityCount = _communityCount;
    
        // Store community details
        CommunityStruct storage _community = _communities[_communityCount];
        _community.owner = _sender;
        _community.currency = IDebtToken(_currency);
        _community.memberCount = 1;
        _community.members[0] = _sender;
        _community.isMember[_sender] = true;
    
        emit CommunityAdded(_communityCount, _sender, _currency, _hash);
    }
  • Redundant assignment on line 266
  • Cache _projectInstance.lenderFee() in lines 393-394 instead of making 2 external calls with the same result
  • Cache _communities[_communityID].projectDetails[_project] in lines 402-407

Tasks

  • Use a uint8 field member instead of uint => bool mapping (use the first 3 bits of a uint instead of creating a whole mapping)
// Task metadata
struct Task {
    // Metadata //
    uint256 cost; // Cost of task
    address subcontractor; // Subcontractor of task
    // Lifecycle //
    TaskStatus state; // Status of task
    uint8 alerts; // Alerts of task
}

function getAlert(Task storage task, TaskStatus state) returns (bool) {
    return task.alerts & (1 << uint8(state)) != 0;
}

function setAlert(Task storage task, TaskStatus state, bool value) {
    if (value) {
        task.alerts = task.alerts | (1 << uint8(state));
    } else {
        task.alerts = task.alerts & ~(1 << uint8(state));
    }
}
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 6, 2022
code423n4 added a commit that referenced this issue Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants