Rigor Protocol contest - rbserver's results

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L685-L694

Vulnerability details

Impact

When returnToLender is called, the unclaimed interest is calculated as below.

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L685-L694

uint256 _noOfDays = (block.timestamp -
	_communityProject.lastTimestamp) / 86400; // 24*60*60

/// Interest formula = (principal * APR * days) / (365 * 1000)
// prettier-ignore
uint256 _unclaimedInterest = 
		_lentAmount *
		_communities[_communityID].projectDetails[_project].apr *
		_noOfDays /
		365000;

Based on this code, if this function is called within 24 hours since the last time that a loan is lent or interest is earned for a project, the unclaimed interest is 0 because block.timestamp - _communityProject.lastTimestamp is less than 86400. However, especially when the lent amount is large, the community owner's loan is in use for the time within the 24 hours and should receive interest accordingly. The community owner will find this very unfair.

Proof of Concept

The following example compares the current code to the suggested code.

For example, the variables are defined as follows.

• block.timestamp - _communityProject.lastTimestamp = 86399
• _lentAmount = 1000000000000
• _communities[_communityID].projectDetails[_project].apr = 50

Using the current code, the unclaimed interest is 0 because (block.timestamp - _communityProject.lastTimestamp) / 86400 = 86399 / 86400 = 0 per Solidity's behavior.

Using the following suggested code that combines the current code, the unclaimed interest is 136984715, per Solidity's behavior, which is larger than 0 and close to the unrounded calculation result.

uint256 _unclaimedInterest = 
		(_lentAmount *
		 _communities[_communityID].projectDetails[_project].apr *
		 (block.timestamp - _communityProject.lastTimestamp))
		/ (86400 * 365000);

Tools Used

VSCode

Recommended Mitigation Steps

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L685-L694 can be changed to the suggested code used in the POC above.

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L267 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L690-L694

Vulnerability details

Impact

When the Community.publishProject function is called, the following code is execute to assign apr of the project that is published to a community.

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L267

_communityProject.apr = _apr;

Later, whenever the Community.claimInterest function, which executes the following Community.returnToLender code, is called, apr of the project is used to calculate interest on the lent amount.

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L690-L694

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

After the project is published to a community, when the project is unpublished or published to another community and then is published again to the previous community, the project's apr will be replaced with the latest new value. For the same community-project combination, after changing the value, calling Community.claimInterest will inflate the interest on the previous lent amount if the new value is larger than the previous value and deflate the interest on the previous lent amount if the new value is smaller than the previous value. Because of the inaccurate calculations, either the builder will lose money by repaying more interest than or the community owner will lose money by receiving less interest than the correct amount.

Proof of Concept

Please add the following test in the Other tests describe block in test\utils\communityTests.ts.

This test will pass to demonstrate the described scenario.

it('apr is replaced with new value when project is published again to the same community', async () => {
  // create project
  let newProject: Project;
  let newCommunityID = 0;

  ({ projectContractInstance: newProject } = await createProject(
	homeFiContract,
	tasksLibrary.address,
	'0x',
	await homeFiContract.connect(signers[2]).tokenCurrency1(),
  ));

  // create community
  const randomHash = '0x0a19';
  let count = (await communityContract.communityCount()).toNumber();
  await communityContract
	.connect(signers[2])
	.createCommunity(randomHash, tokenCurrency1);
  newCommunityID = count + 1;

  // add member
  const data1 = {
	types: ['uint256', 'address', 'bytes'],
	values: [newCommunityID, signers[0].address, sampleHash],
  };
  const [encodedData1, signature1] = await multisig(data1, [
	signers[2],
	signers[0],
  ]);
  await communityContract.addMember(encodedData1, signature1);

  // publish project
  apr = 10;
  const data2 = {
	types: ['uint256', 'address', 'uint256', 'uint256', 'uint256', 'bytes'],
	values: [
	  newCommunityID,
	  newProject.address,
	  apr,
	  0,
	  (await communityContract.communities(newCommunityID)).publishNonce,
	  sampleHash,
	],
  };
  const [encodedData2, signature2] = await multisig(data2, [
	signers[2],
	signers[0],
  ]);
  await communityContract.publishProject(encodedData2, signature2);

  // toggle lendingNeeded
  await communityContract.toggleLendingNeeded(
	newCommunityID,
	newProject.address,
	(await newProject.projectCost()).toNumber(),
  );

  const lendingPrincipal = 1020000000;
  const prjctLenderFee = (await newProject.lenderFee()).toNumber();
  const fee = Number(
	((lendingPrincipal * prjctLenderFee) / (prjctLenderFee + 1000)).toFixed(
	  0,
	),
  );
  const amountToProject = lendingPrincipal - fee;

  // setup dai mocks for lending
  await mockDAIContract.mock.transferFrom
	.withArgs(signers[2].address, treasury, fee)
	.returns(true);
  await mockDAIContract.mock.transferFrom
	.withArgs(signers[2].address, newProject.address, amountToProject)
	.returns(true);

  // lend to project
  await communityContract
	.connect(signers[2])
	.lendToProject(newCommunityID, newProject.address, lendingPrincipal, sampleHash);

  // advance 7 days
  await ethers.provider.send('evm_increaseTime', [7 * 24 * 60 * 60]);

  // apr is not changed when 7 days past after project is published for first time
  const [apr_, , , , , , ,] = await communityContract.connect(signers[0]).projectDetails(newCommunityID, newProject.address);
  expect(apr_).to.be.equals('10');

  // unpublish project
  await communityContract.connect(signers[0]).unpublishProject(newCommunityID, newProject.address);

  // advance 7 days again
  await ethers.provider.send('evm_increaseTime', [7 * 24 * 60 * 60]);

  // publish project again to same community
  apr = 5;
  const data3 = {
	types: ['uint256', 'address', 'uint256', 'uint256', 'uint256', 'bytes'],
	values: [
	  newCommunityID,
	  newProject.address,
	  apr,
	  500,
	  (await communityContract.communities(newCommunityID)).publishNonce,
	  sampleHash,
	],
  };
  const [encodedData3, signature3] = await multisig(data3, [
	signers[2],
	signers[0],
  ]);
  await communityContract.publishProject(encodedData3, signature3);

  // apr is replaced with new value after project is published again to same community
  const [apr2_, , , , , , ,] = await communityContract.connect(signers[0]).projectDetails(newCommunityID, newProject.address);
  expect(apr2_).to.be.equals('5');
});

Tools Used

VSCode

Recommended Mitigation Steps

Instead of recording at the community-project level, the apr and lent amount can be recorded at the community-project-publishNonce level. When Community.claimInterest and Community.returnToLender are called, the apr value corresponding to the community-project-publishNonce level would be used to calculate the interest on the lent amount for the same level. These interests can be aggregated to get the total interest at the community-project level afterwards.

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L577-L579 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L582-L584

Vulnerability details

Impact

The admin is able to pause many functionalities of the Community contract by calling pause. However, calling this function will take effect immediately and can block planned user interactions unexpectedly. For example, the builder calls the repayLender function but the admin calls pause just before that without any announcements. Because the admin's transaction finishes before the builder's starts, the builder's transaction reverts as the Community contract has already been paused. As a result, the builder fails to pay the community owner and is forced to at least own more interest unexpectedly.

Proof of Concept

Please add the following test in the Other tests describe block in test\utils\communityTests.ts. This test will pass to demonstrate the described scenario.

it('admin can pause Community functionalities unexpectedly to block planned user interactions, including loan repayments', async () => {
  // create project
  let newProject: Project;
  let newCommunityID = 0;

  ({ projectContractInstance: newProject } = await createProject(
	homeFiContract,
	tasksLibrary.address,
	'0x',
	await homeFiContract.connect(signers[2]).tokenCurrency1(),
  ));

  // create community
  const randomHash = '0x0a19';
  let count = (await communityContract.communityCount()).toNumber();
  await communityContract
	.connect(signers[2])
	.createCommunity(randomHash, tokenCurrency1);
  newCommunityID = count + 1;

  // add member
  const data1 = {
	types: ['uint256', 'address', 'bytes'],
	values: [newCommunityID, signers[0].address, sampleHash],
  };
  const [encodedData1, signature1] = await multisig(data1, [
	signers[2],
	signers[0],
  ]);
  await communityContract.addMember(encodedData1, signature1);

  // publish project
  apr = 10;
  const data2 = {
	types: ['uint256', 'address', 'uint256', 'uint256', 'uint256', 'bytes'],
	values: [
	  newCommunityID,
	  newProject.address,
	  apr,
	  0,
	  (await communityContract.communities(newCommunityID)).publishNonce,
	  sampleHash,
	],
  };
  const [encodedData2, signature2] = await multisig(data2, [
	signers[2],
	signers[0],
  ]);
  await communityContract.publishProject(encodedData2, signature2);

  // toggle lendingNeeded
  await communityContract.toggleLendingNeeded(
	newCommunityID,
	newProject.address,
	(await newProject.projectCost()).toNumber(),
  );

  const lendingPrincipal = 1020000000;
  const prjctLenderFee = (await newProject.lenderFee()).toNumber();
  const fee = Number(
	((lendingPrincipal * prjctLenderFee) / (prjctLenderFee + 1000)).toFixed(
	  0,
	),
  );
  const amountToProject = lendingPrincipal - fee;

  // setup dai mocks for lending
  await mockDAIContract.mock.transferFrom
	.withArgs(signers[2].address, treasury, fee)
	.returns(true);
  await mockDAIContract.mock.transferFrom
	.withArgs(signers[2].address, newProject.address, amountToProject)
	.returns(true);

  // lend to project
  await communityContract
	.connect(signers[2])
	.lendToProject(newCommunityID, newProject.address, lendingPrincipal, sampleHash);

  // admin pause Community functionalities unexpectedly
  await communityContract.connect(signers[0]).pause();

  // planned functions like repayLender cannot be called
  await expect(
	communityContract.repayLender(newCommunityID, newProject.address, 1000),
  ).to.be.revertedWith('Pausable: paused');

  // restore pause status for other tests
  await communityContract.connect(signers[0]).unpause();
});

Tools Used

VSCode

Recommended Mitigation Steps

pause and unpause can be changed to be time-locked functions so users would have more time to react and unexpectedness can be avoided.

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L206 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L331 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L366 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/HomeFi.sol#L210 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Project.sol#L219

Vulnerability details

Impact

For publishing a project, the documentation, in Step 6, states "that you cannot submit a project with no total budget. Therefore it requires at least one task with a budget > 0."

However, it is possible to create a project without adding any tasks or cost and publish it to a community by using the publishProject function. For this project, calling the toggleLendingNeeded function with a positive _lendingNeeded input and the lendToProject function with a positive _lendingAmount input will revert. Users, who trust the documentation, might create and publish this kind of projects and expect to add tasks later. They will waste gas and have bad user experience after encountering the unexpected reversions.

Proof of Concept

Please add the following helper code in test\utils\projectHelpers.ts first.

export async function createProjectWithoutTasks(
  homeFiContract: HomeFi,
  tasksLibraryAddress: string,
  hash: string,
  currency: string,
): Promise<any> {
  const signers = await ethers.getSigners();
  let tx = await homeFiContract
    .connect(signers[0])
    .createProject(hash, currency);
  const receipt = await tx.wait();
  const event = receipt.events?.find(x => x.event === 'ProjectAdded');
  const project = event?.args?._project;
  const builder = signers[0].address;
  const contractor = signers[1].address;

  const ProjectContractFactory = await ethers.getContractFactory('Project', {
    libraries: {
      Tasks: tasksLibraryAddress,
    },
  });
  const data = {
    types: ['address', 'address'],
    values: [contractor, project],
  };
  const [encodedData, signature] = await multisig(data, [
    signers[0],
    signers[1],
  ]);
  const projectContractInstance = ProjectContractFactory.attach(
    project,
  ) as Project;
  await projectContractInstance.inviteContractor(encodedData, signature);
  return { projectContractInstance, project, builder, contractor };
}

Please add the following test in the lendToProject() describe block in test\utils\communityTests.ts. This test will pass to demonstrate the described scenario.

it('project can be published with no tasks or cost but positive lendingNeeded cannot be toggled and positive amount cannot be lent to it', async () => {
  // create project
  let newProject: Project;
  let newCommunityID = 0;

  ({ projectContractInstance: newProject } = await createProjectWithoutTasks(
	homeFiContract,
	tasksLibrary.address,
	'0x',
	await homeFiContract.connect(signers[2]).tokenCurrency1(),
  ));

  // project cost is 0 because there are no tasks
  expect(await newProject.projectCost()).to.be.equals("0");

  // create community
  const randomHash = '0x0a19';
  let count = (await communityContract.communityCount()).toNumber();
  await communityContract
	.connect(signers[2])
	.createCommunity(randomHash, tokenCurrency1);
  newCommunityID = count + 1;
  expect(await communityContract.communityCount()).to.equal(
	newCommunityID,
  );

  // add member
  const data1 = {
	types: ['uint256', 'address', 'bytes'],
	values: [newCommunityID, signers[0].address, sampleHash],
  };
  const [encodedData1, signature1] = await multisig(data1, [
	signers[2],
	signers[0],
  ]);
  const tx1 = await communityContract.addMember(encodedData1, signature1);
  await expect(tx1)
	.to.emit(communityContract, 'MemberAdded(uint256,address,bytes)')
	.withArgs(newCommunityID, signers[0].address, sampleHash);

  // publish project
  apr = 10;
  let data2 = {
	types: ['uint256', 'address', 'uint256', 'uint256', 'uint256', 'bytes'],
	values: [
	  newCommunityID,
	  newProject.address,
	  apr,
	  0,
	  (await communityContract.communities(newCommunityID)).publishNonce,
	  sampleHash,
	],
  };

  let [encodedData2, signature2] = await multisig(data2, [
	signers[2],
	signers[0],
  ]);
  const tx2 = await communityContract.publishProject(encodedData2, signature2);

  // project can be published successfully
  await expect(tx2)
	.to.emit(communityContract, 'ProjectPublished')
	.withArgs(
	  newCommunityID,
	  newProject.address,
	  apr,
	  0,
	  true,
	  sampleHash,
	);

  // cannot toggle lendingNeeded with positive _lendingNeeded for this project
  const tx3 = communityContract.toggleLendingNeeded(
	newCommunityID,
	newProject.address,
	(await newProject.projectCost()).add(1),
  );
  await expect(tx3).to.be.revertedWith('Community::invalid lending');

  // cannot lend with positive _lendingAmount to this project
  const tx4 = communityContract
	.connect(signers[2])
	.lendToProject(newCommunityID, newProject.address, (await newProject.projectCost()).add(1), sampleHash);
  await expect(tx4).to.be.revertedWith('Community::lending>needed');
});

Tools Used

VSCode

Recommended Mitigation Steps

In publishProject, the corresponding project needs to be checked to ensure that it has at least one task and some cost. The documentation needs to be updated accordingly as well.

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/HomeFi.sol#L284-L297

Vulnerability details

Impact

After calling the mintNFT function, an NFT is minted for the project to the builder. After the community owner lends to the project, the builder can sell this NFT in other markets. Someone, who believes in the value of the project, can purchase this NFT and think that she or he will take over the builder role. However, owning the NFT does not change who the builder is. As the original builder keeps the builder role, this NFT buyer loses money and does not have any builder control over the project.

Proof of Concept

Please add the following test in the Other tests describe block in test\utils\communityTests.ts. This test will pass to demonstrate the described scenario.

it('Someone, who owns project NFT from original builder, does not become project builder', async () => {
  // create project
  let newProject: Project;
  let newCommunityID = 0;

  ({ projectContractInstance: newProject } = await createProject(
	homeFiContract,
	tasksLibrary.address,
	'0x',
	await homeFiContract.connect(signers[2]).tokenCurrency1(),
  ));

  // create community
  const randomHash = '0x0a19';
  let count = (await communityContract.communityCount()).toNumber();
  await communityContract
	.connect(signers[2])
	.createCommunity(randomHash, tokenCurrency1);
  newCommunityID = count + 1;

  // add member
  const data1 = {
	types: ['uint256', 'address', 'bytes'],
	values: [newCommunityID, signers[0].address, sampleHash],
  };
  const [encodedData1, signature1] = await multisig(data1, [
	signers[2],
	signers[0],
  ]);
  await communityContract.addMember(encodedData1, signature1);

  // publish project
  apr = 10;
  const data2 = {
	types: ['uint256', 'address', 'uint256', 'uint256', 'uint256', 'bytes'],
	values: [
	  newCommunityID,
	  newProject.address,
	  apr,
	  0,
	  (await communityContract.communities(newCommunityID)).publishNonce,
	  sampleHash,
	],
  };
  const [encodedData2, signature2] = await multisig(data2, [
	signers[2],
	signers[0],
  ]);
  await communityContract.publishProject(encodedData2, signature2);

  // toggle lendingNeeded
  await communityContract.toggleLendingNeeded(
	newCommunityID,
	newProject.address,
	(await newProject.projectCost()).toNumber(),
  );

  const lendingPrincipal = 1020000000;
  const prjctLenderFee = (await newProject.lenderFee()).toNumber();
  const fee = Number(
	((lendingPrincipal * prjctLenderFee) / (prjctLenderFee + 1000)).toFixed(
	  0,
	),
  );
  const amountToProject = lendingPrincipal - fee;

  // setup dai mocks for lending
  await mockDAIContract.mock.transferFrom
	.withArgs(signers[2].address, treasury, fee)
	.returns(true);
  await mockDAIContract.mock.transferFrom
	.withArgs(signers[2].address, newProject.address, amountToProject)
	.returns(true);

  // lend to project
  await communityContract
	.connect(signers[2])
	.lendToProject(newCommunityID, newProject.address, lendingPrincipal, sampleHash);

  const projectTokenId_ = await homeFiContract.connect(signers[0]).projectTokenId(newProject.address);

  // both project builder and nft owner are the same at this moment
  expect(await newProject.builder()).to.be.equals(signers[0].address);
  expect(await homeFiContract.connect(signers[0]).ownerOf(projectTokenId_)).to.be.equals(signers[0].address);

  // after nft transfer, project nft owner changes
  await homeFiContract.connect(signers[0]).transferFrom(signers[0].address, signers[3].address, projectTokenId_);
  expect(await newProject.builder()).to.be.equals(signers[0].address);
  expect(await homeFiContract.connect(signers[0]).ownerOf(projectTokenId_)).to.be.equals(signers[3].address);

  // new project nft owner cannot call functions, such as unpublishProject, that can only be called by project builder
  const tx1 = communityContract.connect(signers[3]).unpublishProject(newCommunityID, newProject.address);
  await expect(tx1).to.be.revertedWith('Community::!Builder');

  // original project builder can still call functions, such as unpublishProject, that can only be called by project builder
  const tx2 = await communityContract.connect(signers[0]).unpublishProject(newCommunityID, newProject.address);
  await expect(tx2)
	.to.emit(communityContract, 'ProjectUnpublished')
	.withArgs(newCommunityID, newProject.address);
});

Tools Used

VSCode

Recommended Mitigation Steps

Similar to the DebtToken contract, the HomeFi contract can also be updated to make the project NFT non-transferrable.

[L-01] PROTOCOL CANNOT BE EXTENDED TO SUPPORT NEW TOKENS

The initialize function of the HomeFi contract is called to set the supported tokens for the whole protocol. Because initialize has the initializer modifier and cannot be re-initialized, the currently supported tokens cannot be replaced with any new tokens, and new tokens cannot be added to the existing ones, even that the HomeFiProxy contract includes the addNewContract function for adding new proxy contracts. In the future, if there is a need for new tokens, the HomeFi contract needs to be refactored so it can be re-initialized with possibly more than three supported tokens.

[L-02] ADMIN FUNCTIONS CAN BE TIME-LOCKED

The replaceAdmin, replaceTreasury, replaceLenderFee, and setTrustedForwarder functions in the HomeFi contract can be time-locked to give users enough time to react and prevent unexpectedness. It would also be helpful to have communication channels for announcing any upcoming admin actions.

[L-03] MISSING ZERO-ADDRESS CHECK FOR CRITICAL ADDRESS

To prevent unintended actions, unexpected loss of assets, etc., the critical address input should be checked against address(0). Please consider checking the addresses in the following initialize function.

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Project.sol#L94-L105

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

[N-01] CONSTANTS CAN BE USED INSTEAD OF MAGIC NUMBERS

To improve readability and maintainability, constants can be used instead of magic numbers. Please consider replacing the magic number in the following code with a constant.

contracts\Community.sol
  394: (_projectInstance.lenderFee() + 1000);
  686: _communityProject.lastTimestamp) / 86400; // 24*60*60
  694: 365000;

ontracts\Project.sol
  576: uint256 _maxLoop = 50;

[N-02] REDUNDANT CAST

_newTotalLent is already an uint256 type so it does not need to be cast to uint256 again in the following function.

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Project.sol#L198-L202

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

[N-03] NON-VIEW FUNCTION IS IN VIEW FUNCTION CODE SECTION

The following createProject function is not an view function but is in the view function section. It can be moved to other section for consistency and maintainability.

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/ProjectFactory.sol#L73-L91

    /*******************************************************************************
     * ------------------------------EXTERNAL VIEWS------------------------------- *
     *******************************************************************************/

    /// @inheritdoc IProjectFactory
    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);
    }

[G-01] memory KEYWORD CAN BE USED INSTEAD OF storage KEYWORD

When there is no need to make a reference to a state value, the memory keyword can be used instead of the storage keyword to save gas. Accessing storage using sload is more expensive than accessing memory using mload. The memory keyword can be used for _communityProject below.

contracts\Community.sol
    679-698:
        // Local instance of variables. For saving gas.
        ProjectDetails storage _communityProject = _communities[_communityID]
            .projectDetails[_project];
        uint256 _lentAmount = _communityProject.lentAmount;

        // Calculate number of days difference current and last timestamp
        uint256 _noOfDays = (block.timestamp -
            _communityProject.lastTimestamp) / 86400; // 24*60*60

        /// Interest formula = (principal * APR * days) / (365 * 1000)
        // prettier-ignore
        uint256 _unclaimedInterest =
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                _noOfDays /
                365000;

        // Old (already rTokens claimed) + new interest
        uint256 _totalInterest = _unclaimedInterest +
            _communityProject.interest;

[G-02] VARIABLE DOES NOT NEED TO BE INITIALIZED TO ITS DEFAULT VALUE

Explicitly initializing a variable with its default value costs more gas than uninitializing it. For example, uint256 i can be used instead of uint256 i = 0 in the following code.

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

[G-03] ARRAY LENGTH CAN BE CACHED OUTSIDE OF LOOP

Caching the array length outside of the loop and using the cached length in the loop costs less gas than reading the array length for each iteration. For example, _changeOrderedTask.length in the following code can be cached outside of the loop like uint256 _changeOrderedTaskLength = _changeOrderedTask.length, and i < _changeOrderedTaskLength can be used for each iteration.

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

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

[G-04] ++VARIABLE CAN BE USED INSTEAD OF VARIABLE++

++variable costs less gas than variable++. For example, i++ can be changed to ++i in the following code.

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

[G-05] X = X + Y OR X = X - Y CAN BE USED INSTEAD OF X += Y OR X -= Y

x = x + y or x = x - y costs less gas than x += y or x -= y. For example, _interest -= _repayAmount can be changed to _interest = _interest - _repayAmount in the following code.

contracts\Community.sol
  423: .totalLent += _amountToProject;
  435: .lentAmount += _lendingAmount;
  798: _interest -= _repayAmount;

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

contracts\Project.sol
  179: hashChangeNonce += 1;
  250: _taskCount += 1;
  290: hashChangeNonce += 1;
  431: totalAllocated -= _withdrawDifference;
  440: totalAllocated += _newCost - _taskCost;
  456: totalAllocated -= _taskCost;
  616: _costToAllocate -= _taskCost;
  663: _costToAllocate -= _taskCost;
  711: _cost += tasks[_taskID].cost;
  772: totalLent -= _amount;

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

[G-06] ARITHMETIC OPERATIONS THAT DO NOT OVERFLOW CAN BE UNCHECKED

Explicitly unchecking arithmetic operations that do not overflow by wrapping these in unchecked {} costs less gas than implicitly checking these.

For the following loops, if increasing the counter variable is very unlikely to overflow, then unchecked {++i} at the end of the loop block can be used, where i is the counter variable.

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

[G-07] REVERT WITH CUSTOM ERROR CAN BE USED INSTEAD OF REQUIRE() OR REVERT() WITH REASON STRING

revert with custom error can cost less gas than require() or revert() with reason string. Please consider using revert with custom error to replace the following require() and revert().

contracts\Community.sol
  69: require(_address != address(0), "Community::0 address");
  75: require(_msgSender() == homeFi.admin(), "Community::!admin");
  81: require(
  90: require(
  131: require(
  159: require(
  191: require(
  235: require(
  241: require(homeFi.isProjectExist(_project), "Community::Project !Exists");
  248: require(_community.isMember[_builder], "Community::!Member");
  251: require(
  312: require(
  347: require(
  353: require(
  384: require(
  400: require(
  491: require(
  536: require(_builder == _projectInstance.builder(), "Community::!Builder");
  539: require(
  557: require(!restrictedToAdmin, "Community::restricted");
  568: require(restrictedToAdmin, "Community::!restricted");
  764: require(_repayAmount > 0, "Community::!repay");
  792: require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
  886: require(

contracts\DebtToken.sol
  31: require(
  50: require(_communityContract != address(0), "DebtToken::0 address");
  96: revert("DebtToken::blocked");
  104: revert("DebtToken::blocked");

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(
  106: require(
  183: require(_result, "Disputes::!Member");

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(

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

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(
  195: require(_cost > 0, "Project::!value>0");
  199: require(
  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(
  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(
  521: require(
  530: require(getAlerts(_task)[2], "Project::!SCConfirmed");
  753: require(_sc != address(0), "Project::0 address");
  886: require(
  906: require(

contracts\ProjectFactory.sol
  36: require(_address != address(0), "PF::0 address");
  64: require(
  84: require(_msgSender() == homeFi, "PF::!HomeFiContract");

contracts\libraries\Tasks.sol
  44: require(_self.state == TaskStatus.Inactive, "Task::active");
  50: require(_self.state == TaskStatus.Active, "Task::!Active");
  56: require(
  124: require(_self.subcontractor == _sc, "Task::!SC");

[G-08] NEWER VERSION OF SOLIDITY CAN BE USED

The protocol can benefit from more gas-efficient features and fixes by using a newer version of Solidity. Changes for newer Solidity versions can be viewed here.

contracts\Community.sol
  3: pragma solidity 0.8.6;

contracts\DebtToken.sol
  3: pragma solidity 0.8.6;

contracts\Disputes.sol
  3: pragma solidity 0.8.6;

contracts\HomeFi.sol
  3: pragma solidity 0.8.6;

contracts\HomeFiProxy.sol
  3: pragma solidity 0.8.6;

contracts\Project.sol
  3: pragma solidity 0.8.6;

contracts\ProjectFactory.sol
  3: pragma solidity 0.8.6;

contracts\interfaces\ICommunity.sol
  3: pragma solidity 0.8.6;

contracts\interfaces\IDebtToken.sol
  3: pragma solidity 0.8.6;

contracts\interfaces\IDisputes.sol
  3: pragma solidity 0.8.6;

contracts\interfaces\IHomeFi.sol
  3: pragma solidity 0.8.6;

contracts\interfaces\IProject.sol
  3: pragma solidity 0.8.6;

contracts\interfaces\IProjectFactory.sol
  3: pragma solidity 0.8.6;

contracts\libraries\SignatureDecoder.sol
  3: pragma solidity 0.8.6;

contracts\libraries\Tasks.sol
  3: pragma solidity 0.8.6;