Rigor Protocol contest - samruna's results

Community lending and instant payments for new home construction.

General Information

Platform: Code4rena

Start Date: 01/08/2022

Pot Size: $50,000 USDC

Total HM: 26

Participants: 133

Period: 5 days

Judge: Jack the Pug

Total Solo HM: 6

Id: 151

League: ETH

Rigor Protocol

Findings Distribution

Researcher Performance

Rank: 63/133

Findings: 2

Award: $62.72

🌟 Selected for report: 0

🚀 Solo Findings: 0

1. Use of addrSet variable in HomeFi.sol

The addrSet variable in HomeFi.sol, does not look to have any significant use other than checking if the address is already set. This can be also achieved by checking for already set addresses such as projectFactoryInstance or communityContract or even checking the length of wrappedToken[]. The addrSet variable can be safely removed with small code modification. Code: https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/HomeFi.sol#L50

2. tokenCurrent1-3 can an array

In HomeFi.sol, three separate variables are defined for tokenCurrent(1-3). These variables are primarily used as indexes and it seems the number of currencies is always 3. In this case, instead of defining 3 separate variables, one array of length 3 can be defined to store all three currencies. This will save some storage gas. Additionally, these 3 variables can be completely removed and directly referenced when populating the wrappedToken map at line 148 This will make the code a little smaller and much more readable.

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

3. Remove unused code:

The below code is never used and can be safely removed to save some gas

Code: https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L910-918 https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/HomeFi.sol#L314-322

4 bool parameter for delegateContractor can be removed

Code: https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/delegateContractor.sol#L148

The function delegateContractor(bool _bool) take bool parameter. However, this parameter can be removed as I am assuming when the function is called, the contractDelegated will either be true or false and that can be determined by evaluating the existing value of contractDelegated. The code can be changed to contractDelegated = !contractDelegated;

#0 - zgorizzo69

2022-08-07T12:41:40Z

Thx for the findings 3. It is used for meta transaction and we need to override as it is present in both ContextUpgradeable and ERC2771ContextUpgradeable 4. then maybe a more suitable name would be toggleContractorDelegation

1. Use of custom error and revert() instead of require()

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.

In the below code references, require can be replaced with if (a != b) revert ERROR()

Code: https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L69-90 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L131 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L159 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L191 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L235-251 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L312 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L347 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L353 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L384 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L401 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L491 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L536-539 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L557-568 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L764 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L792 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L886 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L31 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L50 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L36 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L64 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/ProjectFactory.sol#L84 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L41 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L81 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L105 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFiProxy.sol#L133 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L39 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L46 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L52 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L61 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L106 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Disputes.sol#L183 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L73 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L78 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L84 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L142 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L191 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L255 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L123-135 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L150-153 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L176 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L189-199 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L238-245 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L277 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L301-308 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L341 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L369 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L406 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L511-530 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L753 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L886 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L906

2. Use of != 0 instead of > 0 for integer comparison

Use of != 0 instead of > 0 is more cheaper. Below code references can be updated to use !=0

Code: https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L261 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L427 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L840 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L245 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L380 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L691

3. Use of unchecked block in loops

Although solidity 0.8.0 onwards, all arithmetic operations rever on over and underflow by defaults, it's best to use an unchecked block for any arithmetic operations to save gas fees

Code references where this can be safely implemented, https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L140 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomFi.sol#L289 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L179 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L250 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L290 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L625 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L672

4. Use of bytes32 than string

A string is a dynamic data structure and therefore is more gas-consuming than bytes32. For below code references, including function parameters, bytes32 can be used instead of string

Code: https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L45-46 https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/HomeFi.sol#L284

5. Use external vs public if not called within

For all the public functions, the input parameters are copied to memory automatically, and it costs gas. If your function is only called externally, then you should explicitly mark it as external. External function parameters are not copied into memory but are read from calldata directly. This small optimization in your solidity code can save you a lot of gas when the function input parameters are huge.

For below code references, function visibility can be changed to external https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/DebtToken.sol#L91

6. mintNFT return value is not used

function mintNFT returns a value that is never used. No need to return value, this can save some gas.

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

7. Rearrange code to save some extra gas

Code; https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Community.sol#L235

L228 // Local instance of community and community project details. For saving gas. L229 CommunityStruct storage _community = _communities[_communityID]; L230 ProjectDetails storage _communityProject = _community.projectDetails[_project]; L235 require(_publishNonce == _community.publishNonce,"Community::invalid publishNonce");

The above require statement at line 235 can be moved after line 229. If the decoded nonce is incorrect, no need to create _communityProject storage variable at line 230. The above block can be rearranged to save some gas by not creating an extra storage variable as below.

L228 // Local instance of community and community project details. For saving gas. L229 CommunityStruct storage _community = _communities[_communityID]; L235 require(_publishNonce == _community.publishNonce,"Community::invalid publishNonce"); L230 ProjectDetails storage _communityProject = _community.projectDetails[_project];

Similarly, line 241 can be moved to line 227.

Reverts if _project not originated from HomeFi require(homeFi.isProjectExist(_project), "Community::Project !Exists");

8 Duplicate variable casting is not needed

Code: https://github.com/code-423n4/2022-08-rigor/tree/main/contracts/Project.sol#L198-200

In the above function, variable _newTotalLEnt is being type casted again at line 200. This is not necessary and can consume more gas. Remove the double casting.

#0 - zgorizzo69

2022-08-07T12:12:48Z

  1. We need the actual name and symbol for the token

#1 - zgorizzo69

2022-08-07T12:20:11Z

Good job 👍

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter