QA Report

[code423n4]

Low Risk Findings Overview

	Finding	Instances
[L-01]	Admin transfer should be a two-step process	1
[L-02]	Strict equality might lead to unexpected revert	1
[L-03]	Fee-on-transfer token not supported	8

Non-critical Findings Overview

	Finding	Instances
[N-01]	The use of magic numbers is not recommended	4
[N-02]	Critical functions should return value	2

QA overview per contract

Contract	Total Instances	Total Findings	Low Findings	Low Instances	NC Findings	NC Instances
Community.sol	7	2	1	4	1	3
Project.sol	7	4	2	5	2	2
HomeFi.sol	2	2	1	1	1	1

Low Risk Findings

[L-01] Admin transfer should be a two-step process

If ownership is accidentally transferred to an inactive account all functionalities depending on it will be lost.
1 instance of this issue has been found:

[L-01] HomeFi.sol#L157-L168

    function replaceAdmin(address _newAdmin)
        external
        override
        onlyAdmin
        nonZero(_newAdmin)
        noChange(admin, _newAdmin)
    {
        // Replace admin
        admin = _newAdmin;

        emit AdminReplaced(_newAdmin);
    }

[L-02] Strict equality might lead to unexpected revert

Strict equallity might lead to eventual revert and loss of gas for users.
Consider accepting a range above required and reimburse the difference.
1 instance of this issue has been found:

[L-02] Project.sol#L199-L202

        require(
            projectCost() >= uint256(_newTotalLent),
            "Project::value>required"
        );

[L-03] Fee-on-transfer token not supported

Transfer amount is currently based on input rather than difference in balance before and after the transfer.
If protocol ever wishes to accept/support fee-on-transfer tokens it should follow the latter.
8 instances of this issue have been found:

[L-03] Community.sol#L474-L475

        _communities[_communityID].currency.safeTransferFrom(

[L-03b] Community.sol#L446-L447

        _currency.safeTransferFrom(_msgSender(), _project, _amountToProject);

[L-03c] Community.sol#L443-L444

        _currency.safeTransferFrom(_msgSender(), homeFi.treasury(), _lenderFee);

[L-03d] Community.sol#L321-L322

        _community.currency.safeTransferFrom(

[L-03e] Project.sol#L775-L776

        currency.safeTransfer(builder, _amount);

[L-03f] Project.sol#L381-L382

            _token.safeTransfer(builder, _leftOutTokens);

[L-03g] Project.sol#L353-L354

        currency.safeTransfer(

[L-03h] Project.sol#L206-L207

            currency.safeTransferFrom(_sender, address(this), _cost);

Non-critical Findings

[N-01] The use of magic numbers is not recommended

Consider setting constant numbers as a constant variable for better readability and clarity.
4 instances of this issue have been found:

[N-01] Community.sol#L690-L694

        uint256 _unclaimedInterest = 
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                _noOfDays /
                365000;

[N-01b] Community.sol#L686-L687

            _communityProject.lastTimestamp) / 86400; // 24*60*60

[N-01c] Community.sol#L394-L395

            (_projectInstance.lenderFee() + 1000);

[N-01d] Project.sol#L906-L909

        require(
            ((_amount / 1000) * 1000) == _amount,
            "Project::Precision>=1000"
        );

[N-02] Critical functions should return a value

Providing a return value improves code clarity and readability.
2 instances of this issue have been found:

[N-02] Project.sol#L219-L264

 function addTasks(bytes calldata _data, bytes calldata _signature)
        external
        override
    {
        // If the sender is disputes contract, then do not check for signatures.
        if (_msgSender() != disputes) {
            // Check for required signatures
            checkSignature(_data, _signature);
        }

        // Decode params from _data
        (
            bytes[] memory _hash,
            uint256[] memory _taskCosts,
            uint256 _taskCount,
            address _projectAddress
        ) = abi.decode(_data, (bytes[], uint256[], uint256, address));

        // Revert if decoded taskCount is incorrect. This indicates wrong data.
        require(_taskCount == taskCount, "Project::!taskCount");

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

        // Revert if IPFS hash array length is not equal to task cost array length.
        uint256 _length = _hash.length;
        require(_length == _taskCosts.length, "Project::Lengths !match");

        // Loop over all the new tasks.
        for (uint256 i = 0; i < _length; i++) {
            // Increment local task counter.
            _taskCount += 1;

            // Check task cost precision. Revert if too precise.
            checkPrecision(_taskCosts[i]);

            // Initialize task.
            tasks[_taskCount].initialize(_taskCosts[i]);
        }

        // Update task counter equal to local task counter.
        taskCount = _taskCount;

        emit TasksAdded(_taskCosts, _hash);
    }

[N-02b] HomeFi.sol#L210-L232

    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);
    }