Platform: Code4rena
Start Date: 14/07/2022
Pot Size: $25,000 USDC
Total HM: 2
Participants: 63
Period: 3 days
Judge: PierrickGT
Total Solo HM: 1
Id: 147
League: ETH
Rank: 12/63
Findings: 2
Award: $83.86
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: hickuphh3
Also found by: 0x29A, 0x52, 0xNazgul, Chom, Deivitto, ElKu, Funen, IllIllI, Meera, ReyAdmirado, SooYa, TomJ, Trumpero, Waze, __141345__, ak1, asutorufos, c3phas, cRat1st0s, csanuragjain, delfin454000, exd0tpy, fatherOfBlocks, hake, hansfriese, horsefacts, hyh, karanctf, kenzo, kyteg, ladboy233, pashov, peritoflores, rajatbeladiya, rbserver, reassor, rokinot, simon135, wastewa
66.9667 USDC - $66.97
require(condition, "msg"Severity: Informational
Context: Witch.sol
Description: It's better to stick to one or the other to stay consistent.
Recommendation: I would recommend the use of just custom errors to save gas on deployment.
otherWitches Possible Name SuggestionSeverity: Informational
Context: Witch.sol#L68
Description: "The name is a bit misleading, as any address can be entered. Better naming suggestions are welcome."
Recommendation:
Maybe Witched, VaultTaken, bewitched or hexed
Severity: Informational
Context: Witch.sol
Description: Contract code size is 28515 bytes and exceeds 24576 bytes (a limit introduced in Spurious Dragon). This contract may not be deployable on mainnet.
Recommendation: Consider enabling the optimizer (with a low "runs" value!), turning off revert strings, or using libraries.
Severity: Informational
Context: Witch.sol#L577
Description: There should never be any TODOs in the code when deploying.
Recommendation: Add this TODO to the docs instead.
Severity: Informational
Context: Witch.sol#L213 (overriden => overridden), Witch.sol#L220 (repayed => repaid), Witch.sol#L267 (overriden => overridden), Witch.sol#L267 (differente => different), Witch.sol#L462 (overriden => overridden), Witch.sol#L512 (Delete the extra space), Witch.sol#L520 (quoutes => quotes), Witch.sol#L520 (hoy => how)
Description: Spelling errors in comments can cause confusion to both users and developers.
Recommendation: Check all misspellings to ensure they are corrected.
Severity: Informational
Context: Witch.sol
Description: Some functions are missing @notice/@dev NatSpec comments for the function, @param for all/some of their parameters and @return for return values. Given that NatSpec is an important part of code documentation, this affects code comprehension, auditability and usability.
Recommendation: Add in full NatSpec comments for all functions to have complete code documentation for future use.
#0 - alcueca
2022-07-22T14:02:59Z
Ok
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, Aymen0909, Chom, Deivitto, ElKu, JC, JohnSmith, Kaiziron, Limbooo, MadWookie, Meera, ReyAdmirado, Rohan16, Sm4rty, SooYa, TomJ, Trumpero, Waze, __141345__, ajtra, ak1, antonttc, bulej93, c3phas, cRat1st0s, csanuragjain, defsec, durianSausage, fatherOfBlocks, gogo, hake, hickuphh3, ignacio, joestakey, karanctf, kyteg, m_Rassska, pashov, rajatbeladiya, rbserver, robee, rokinot, samruna, sashik_eth, simon135, tofunmi
16.8934 USDC - $16.89
Context: Witch.sol#L176-L210 (cauldron), Witch.sol#L286-L333 (cauldron), Witch.sol#L528-L559 (cauldron)
Description:
Functions that read state variables more than once can cache it into a local variable for repeated reads saving gas by converting expensive SLOADs into much cheaper MLOADs.
Recommendation: It's best to cache the state variables into memory when read more than once.
memory keyword when storage should be usedContext: Witch.sol#L184, Witch.sol#L299, Witch.sol#L357, Witch.sol#L419, Witch.sol#L433
Description:
When copying a state struct in memory, there are as many SLOADs and MSTOREs as there are fields. When reading the whole struct multiple times is not needed, it's better to actually only read the relevant field(s). When only some of the fields are read several times, these particular values should be cached instead of the whole state struct.
Recommendation:
cach these values instead of the memory keyword in these situations.
require(), Use != 0 Instead of > 0 With Uint ValuesContext: Witch.sol#L255, Witch.sol#L300, Witch.sol#L358, Witch.sol#L416
Description:
In a require, when checking a uint, using != 0 instead of > 0 saves 6 gas. This will jump over or avoid an extra ISZERO opcode.
Recommendation:
Use != 0 instead of > 0 with uint values but only in require() statements.
Context: Witch.sol
Description:
You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 21 gas on deployment with no security risks.
Recommendation: Set the constructor to payable.
Context: Witch.sol
Description: To save some gas the use of custom errors leads to cheaper deploy time cost and run time cost. The run time cost is only relevant when the revert condition is met.
Recommendation: Use Custom Errors instead of strings.
Context: Witch.sol
Description:
Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions. One could use This tool to help find alternative function names with lower Method IDs while keeping the original name intact.
Recommendation:
Find a lower method ID name for the most called functions for example mostCalled() vs. mostCalled_41q() is cheaper by 44 gas.