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
Rank: 63/133
Findings: 2
Award: $62.72
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Lambda
Also found by: 0x1f8b, 0x52, 0xA5DF, 0xNazgul, 0xNineDec, 0xSmartContract, 0xSolus, 0xf15ers, 0xkatana, 0xsolstars, 8olidity, Aymen0909, Bahurum, Bnke0x0, CertoraInc, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, Extropy, Funen, GalloDaSballo, Guardian, IllIllI, JC, Jujic, MEP, Noah3o6, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, SooYa, Soosh, Throne6g, TomJ, Tomio, TrungOre, Waze, Yiko, _Adam, __141345__, a12jmx, ajtra, ak1, arcoun, asutorufos, ayeslick, benbaessler, berndartmueller, bin2chen, bobirichman, brgltd, bulej93, byndooa, c3phas, codexploder, cryptonue, cryptphi, defsec, delfin454000, dipp, djxploit, erictee, exd0tpy, fatherOfBlocks, gogo, hake, hansfriese, horsefacts, hyh, ignacio, indijanc, joestakey, kaden, mics, minhquanym, neumo, obront, oyc_109, p_crypt0, pfapostol, poirots, rbserver, robee, rokinot, rotcivegaf, sach1r0, saian, samruna, saneryee, scaraven, sikorico, simon135, sseefried, supernova
40.692 USDC - $40.69
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
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
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
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
🌟 Selected for report: c3phas
Also found by: 0x040, 0x1f8b, 0xA5DF, 0xNazgul, 0xSmartContract, 0xSolus, 0xc0ffEE, 0xkatana, 0xsam, 8olidity, Aymen0909, Bnke0x0, CertoraInc, Chinmay, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, Extropy, Fitraldys, Funen, GalloDaSballo, Guardian, IllIllI, JC, Lambda, MEP, Metatron, MiloTruck, Noah3o6, NoamYakov, PaludoX0, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, SooYa, TomJ, Tomio, Waze, _Adam, __141345__, a12jmx, ajtra, ak1, apostle0x01, asutorufos, ballx, benbaessler, bharg4v, bobirichman, brgltd, cryptonue, defsec, delfin454000, dharma09, djxploit, durianSausage, eierina, erictee, fatherOfBlocks, gerdusx, gogo, hake, hyh, ignacio, jag, kaden, kyteg, lucacez, mics, minhquanym, oyc_109, pfapostol, rbserver, ret2basic, robee, rokinot, sach1r0, saian, samruna, scaraven, sikorico, simon135, supernova, teddav, tofunmi, zeesaw
22.0296 USDC - $22.03
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
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
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
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
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
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
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");
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 - zgorizzo69
2022-08-07T12:20:11Z
Good job 👍