Rigor Protocol contest - delfin454000's results

Typos

---

HomeFiProxy.sol: L22

    /// @notice bytes2 array of upgradable contracts initials

Change contracts to contract

---

HomeFiProxy.sol: L29

    /// @dev mapping that tell if a particular address is active(latest version of contract)

Change tell to tells or shows

---

HomeFiProxy.sol: L32

    /// @dev mapping that maps contract initials with there implementation address

Change there to their

---

Disputes.sol: L51

        // Revert if project not originated of HomeFi

Change of to by

---

Disputes.sol: L179

        // Revert is signer is not builder, contractor or subcontractor.

Change first instance of is to if

---

HomeFi.sol: L115

        lenderFee = _lenderFee; // the percentage must be multiplied with 10

Change with to by

---

Project.sol: L214

        // Allocate funds to tasks and mark then as allocated

Change then to them or else remove then

---

Project.sol: L455

                    // As as needs to go though funding process again.

Replace repeated word as

---

The same typo (Revet) occurs in both lines referenced below:

Project.sol: L514

Project.sol: L520

Example (Project.sol: L514):

            // Revet if sender is not builder or contractor

Change Revet to Revert in both cases

---

Project.sol: L574

        // Max amount out times this loop will run

Change out to of

---

Project.sol: L575

        // This is to ensure the transaction do not run out of gas (max gas limit)

Change do to does

---

The same typo (is) occurs in both lines referenced below:

Project.sol: L613

Project.sol: L660

                // If there is enough funds to allocate this task

Change is to are in both cases

---

Community.sol: L164

        // Emit event if _hash. This way this hash needs not be stored in memory.

Change needs to need

---

Community.sol: L271

        // If _publishFee is zero than mark publish fee as paid

Change than to then

---

The same typo (address - address) occurs in both lines referenced below:

Project.sol: L866

Community.sol: L821

Example (Project.sol: L866):

     * @param _address address - address checked for validity

Suggestion:

     * @param _address address, which is checked for validity

---

Community.sol: L618

        // Initiate empty equal equal to member count length

Remove repeated word equal

---

---

Unclear comments

Comments should communicate clearly, immediately and without ambiguity. The comments below could be improved, as shown:

---

Project.sol: L379

        // If balance is present then it to the builder.

Suggestion: Change then it to then transfer it

---

Community.sol: L752

     * @param _communityID uint256 - the the uuid of the community

Suggestion:

     * @param _communityID uint256 - the uuid of community the project is held in

---

---

Long single line comments

In theory, comments over 79 characters should wrap using multi-line comment syntax. Even if somewhat longer comments are acceptable, there are cases where very long comments interfere with readability.

---

Below are five instances of long comments whose readability could be improved, as shown:

HomeFiProxy.sol: L50

     * @notice Initialize all the homeFi contract in the correct sequential order and generate upgradable proxy for them.

Suggestion:

     * @notice Initialize all the homeFi contract in the correct sequential order
     *  and generate upgradable proxy for them.

---

HomeFiProxy.sol: L123

     * @param _contractAddresses address array of contract implementation address that needs to be upgraded

Suggestion:

     * @param _contractAddresses address array of contract implementation —  
     *  address that needs to be upgraded.

---

Project.sol: L785

     * If contractor is assigned but not delegated then only checks for builder and contractor signature.

Suggestion:

     * If contractor is assigned but not delegated, then only checks 
     *  for builder and contractor signature.

---

Project.sol: L819

     * @dev Check if recovered signatures match with builder, contractor and subcontractor address for a task.

Suggestion:

     * @dev Check if recovered signatures match with builder, contractor 
     *  and subcontractor address for a task.

---

SignatureDecoder.sol: L58

    * @param pos which signature to read. A prior bounds check of this parameter should be performed, to avoid out of bounds access

Suggestion:

    * @param pos which signature to read — a prior bounds check of this parameter should be performed 
    *  to avoid out of bounds access.

---

---

Update sensitive terms

Terms incorporating "white," "black," "master" or "slave" are potentially problematic. Substituting more neutral terminology is becoming common practice. It is apparent that use of whitelist has been avoided in Rigor Protocol. There is one instance of master, however, as shown below:

---

Project.sol: L86

    /// @dev Added to make sure master implementation cannot be initialized

---

---

Avoid use of '&&' within a require function

Splitting into separate require() statements saves gas

Disputes.sol: L61-65

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

Recommendation:

        require(_disputeID < disputeCount, "Disputes::!Resolvable");
        require(disputes[_disputeID].status == Status.Active, "Disputes::!Resolvable");

---

Disputes.sol: L106-109

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

Recommendation:

        require(_actionType != 0, "Disputes::!ActionType");
        require(_actionType <= uint8(ActionType.TaskPay), "Disputes::!ActionType");

---

Community: L353-357

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

Recommendation:

        require(_lendingNeeded >= _communityProject.totalLent, "Community::invalid lending");
        require(_lendingNeeded <= IProject(_project).projectCost()), "Community::invalid lending");

---

---

Use != 0 instead of > 0 in a require statement if variable is an unsigned integer

!= 0 should be used where possible since > 0 costs more gas

Disputes.sol: L106-109

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

Recommendation: Change _actionType > 0 to _actionType != 0

---

Project.sol: L195

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

Recommendation: Change _cost > 0 to _cost != 0

---

Community.sol: L764

        require(_repayAmount > 0, "Community::!repay");

Recommendation: Change _repayAmount > 0 to _repayAmount != 0

---

---

Variables should not be initialized to their default values

For example, initializing bool variables to their default value of false is unnecessary and costs gas

Project.sol: L412

        bool _unapproved = false;

Change to bool _unapproved;

---

---

Array length should not be looked up in every iteration of a for loop

Since calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop

Community.sol: L624-626

        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
            _members[i] = _communities[_communityID].members[i];
        }

Suggestion:

        uint256 totalCommunitiesMembers = _communities[_communityID].memberCount; 
        for (uint256 i = 0; i < totalCommunitiesMembers; i++) {
            _members[i] = _communities[_communityID].members[i];
        }

---

---

Use ++i instead of i++ to increase count in a for loop

Since use of i++ (or equivalent counter) costs more gas, it is better to use ++i

HomeFiProxy.sol: L87-89

        for (uint256 i = 0; i < _length; i++) {
            _generateProxy(allContractNames[i], _implementations[i]);
        }

Suggestion:

        for (uint256 i = 0; i < _length; ++i) {
            _generateProxy(allContractNames[i], _implementations[i]);
        }

---

Similarly for the nine for loops referenced below:

HomeFiProxy.sol: L136-138

Project.sol: L248-257

Project.sol: L311-313

Project.sol: L322-324

Project.sol: L368-370

Project.sol: L603-611

Project.sol: L650-658

Project.sol: L710-712

Community.sol: L624-626

---

---

Avoid use of default "checked" behavior in a for loop

Underflow/overflow checks are made every time i++ (or ++i or equivalent counter) is called. Such checks are unnecessary since i is already limited. Therefore, use unchecked{i++}/unchecked{++i} instead

HomeFiProxy.sol: L87-89

        for (uint256 i = 0; i < _length; i++) {
            _generateProxy(allContractNames[i], _implementations[i]);
        }

Suggestion:

        for (uint256 i = 0; i < _length;) {
            _generateProxy(allContractNames[i], _implementations[i]);

            unchecked{
              ++i;
          }
        }

---

Similarly for the nine for loops referenced below:

HomeFiProxy.sol: L136-138

Project.sol: L248-257

Project.sol: L311-313

Project.sol: L322-324

Project.sol: L368-370

Project.sol: L603-611

Project.sol: L650-658

Project.sol: L710-712

Community.sol: L624-626

---

---

Note that while, for presentation purposes, I have separated the for loop-related gas savings opportunities above, they should be combined.

---

---