Platform: Code4rena
Start Date: 18/10/2022
Pot Size: $75,000 USDC
Total HM: 27
Participants: 144
Period: 7 days
Judge: gzeon
Total Solo HM: 13
Id: 170
League: ETH
Rank: 46/144
Findings: 2
Award: $82.02
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Rolezn
Also found by: 0x1f8b, 0x52, 0x5rings, 0xNazgul, 0xSmartContract, 0xZaharina, 0xhunter, 0xzh, 8olidity, Amithuddar, Aymen0909, B2, Bnke0x0, Chom, Deivitto, Diana, Diraco, Dravee, Franfran, JC, Jeiwan, Josiah, JrNet, Jujic, KingNFT, KoKo, Lambda, Margaret, Migue, Ocean_Sky, PaludoX0, Picodes, Rahoz, RaoulSchaffranek, RaymondFam, RedOneN, ReyAdmirado, Shinchan, Tagir2003, Trust, Waze, Yiko, __141345__, a12jmx, adriro, ajtra, arcoun, aysha, ballx, bin2chen, bobirichman, brgltd, bulej93, catchup, catwhiskeys, caventa, cccz, cdahlheimer, ch0bu, chaduke, chrisdior4, cloudjunky, cryptostellar5, cryptphi, csanuragjain, cylzxje, d3e4, delfin454000, djxploit, durianSausage, erictee, fatherOfBlocks, francoHacker, gianganhnguyen, gogo, hansfriese, i_got_hacked, ignacio, imare, karanctf, kv, leosathya, louhk, lukris02, lyncurion, m_Rassska, malinariy, martin, mcwildy, mics, minhtrng, nicobevi, oyc_109, pashov, peanuts, pedr02b2, peiw, rbserver, ret2basic, rotcivegaf, rvierdiiev, ryshaw, sakman, sakshamguruji, saneryee, securerodd, seyni, sikorico, svskaushik, teawaterwire, tnevler, w0Lfrum
55.6726 USDC - $55.67
UPGRADEABLE CONTRACT IS MISSING A __GAP[50] STORAGE VARIABLE TO ALLOW FOR NEW STORAGE VARIABLES IN LATER VERSIONSWhile some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
require()/revert() STATEMENTS SHOULD HAVE DESCRIPTIVE REASON STRINGSTODOSCode architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.
block.timestamp and block.number are not be used a source or reference of time. Instead use oracle or verifiable random numbersEVENT AND OR TIMELOCK FOR CRITICAL PARAMETER CHANGEEvents help non-contract tools to track changes, and events prevent users from being surprised by changes.
ecrecover() is deprecated. Consider adding checks for signature malleability.Use OpenZeppelin’s ECDSA contract rather than calling ecrecover() directly.
Event is missing indexed fieldsIndex event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it’s not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
garbage value in mapping for deleting thatIf there is a mapping data structure present inside struct, then deleting the struct doesn't delete the mapping. Instead one should use lock to lock that data structure from further use.
modifier on constructorOpenZeppelin recommends that the initializer modifier be applied to constructors in order to avoid potential griefs, social engineering, or exploits. Ensure that the modifier is applied to the implementation contract. If the default constructor is currently being used, it should be changed to be an explicit one with the modifier applied.
All the contracts present inside scope
arg1 or arg2 is null, or is user-controllable(returning null).When using abi.decode(arg1,arg2) , one should keep in mind that if arg1 or arg2 is null, or is user-controllable(returning null), then the abi.decode() will revert. If such cases are not handled, then it may lead to revert the whole function where abi.decode was, and may lead to DOS-kind of situation, based on the logic of the function
🌟 Selected for report: oyc_109
Also found by: 0x040, 0x1f8b, 0x5rings, 0xNazgul, 0xSmartContract, 0xZaharina, 0xsam, 0xzh, 2997ms, Amithuddar, Aymen0909, B2, Bnke0x0, Deivitto, Diana, Dinesh11G, Franfran, JC, JrNet, Jujic, KingNFT, KoKo, Mathieu, Metatron, Mukund, Olivierdem, PaludoX0, Pheonix, Picodes, RaymondFam, RedOneN, ReyAdmirado, Rolezn, Saintcode_, Satyam_Sharma, Shinchan, Tagir2003, Tomio, Waze, Yiko, __141345__, adriro, ajtra, aysha, ballx, beardofginger, bobirichman, brgltd, bulej93, catchup, catwhiskeys, cdahlheimer, ch0bu, chaduke, chrisdior4, cryptostellar5, cylzxje, d3e4, delfin454000, dharma09, djxploit, durianSausage, emrekocak, erictee, exolorkistis, fatherOfBlocks, gianganhnguyen, gogo, halden, hxzy, i_got_hacked, iepathos, karanctf, leosathya, lucacez, lukris02, lyncurion, m_Rassska, martin, mcwildy, mics, nicobevi, peanuts, peiw, rbserver, ret2basic, rotcivegaf, ryshaw, sakman, sakshamguruji, saneryee, sikorico, skyle, svskaushik, tnevler, vv7, w0Lfrum, zishansami
26.3525 USDC - $26.35
Multiple mappings of same key can be combined into a single mapping to a struct, where appropriateSaves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot
Using both named returns and a return statement isn’t necessary. Removing one of those can improve code clarity:
calldata instead of memory for external functions where the function argument is read-onlyThe memory keyword in a function argument causes the underlying code to copy the argument into memory. Calldata causes the code to read the transaction without copying it directly, ultimately saves gases.
require() statements that use && saves gasInstead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement (saving 3 gas per &):
internal functions not called by the contract should be removed to save deployment gasIf the functions are required by an interface, the contract should inherit from that interface and use the override
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
Inline a modifier that's only used onceAs onlyOperator() modifier is only used once in this contract (in function bridgeInRequest()), it should get inlined to save gas without losing readability:
require() functions in loop .It is better to break than to revert