Platform: Code4rena
Start Date: 29/07/2022
Pot Size: $50,000 USDC
Total HM: 6
Participants: 75
Period: 5 days
Judge: GalloDaSballo
Total Solo HM: 3
Id: 149
League: ETH
Rank: 19/75
Findings: 2
Award: $115.44
π Selected for report: 0
π Solo Findings: 0
π Selected for report: oyc_109
Also found by: 0x1f8b, 0x52, 0xNazgul, 0xSmartContract, 0xf15ers, 8olidity, Aymen0909, Bnke0x0, CertoraInc, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, IllIllI, JC, Lambda, Noah3o6, NoamYakov, RedOneN, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, TomJ, Twpony, Waze, Yiko, __141345__, ajtra, apostle0x01, ashiq0x01, asutorufos, bardamu, benbaessler, berndartmueller, bharg4v, bulej93, c3phas, cccz, ch13fd357r0y3r, codexploder, cryptonue, cryptphi, defsec, djxploit, durianSausage, fatherOfBlocks, gogo, hansfriese, horsefacts, ignacio, kyteg, lucacez, mics, rbserver, robee, sashik_eth, simon135, sseefried, tofunmi, xiaoming90
56.196 USDC - $56.20
Detect missing zero address validation
Include check that the address is not zero
AxelarDepositService.sol#L18 AxelarDepositService.sol#L19
ReceiverImplementation.sol#L17 ReceiverImplementation.sol#L23
ReceiverImplementation.sol#L45 ReceiverImplementation.sol#L51
ReceiverImplementation.sol#L70 ReceiverImplementation.sol#L71 ReceiverImplementation.sol#L86
Detection of shadowing using local variables.
Rename the local variables that shadow another component.
ReceiverImplementation.sol#12 DepositBase.sol#13 IDepositBase.sol#14
ReceiverImplementation.sol#12 DepositBase.sol#13 IDepositBase.sol#14
There is return that are unusefull.
Remove unused return and include the rest code in else condition
ReceiverImplementation.sol#L30 ReceiverImplementation.sol#L54
Actually the code use version 0.8.9
Update to use at least 0.8.10 to add more functionality like external calls skip contract existence checks if the external call has a return value
The use of assembly is error-prone and should be avoided.
Do not use evm assembly.
DepositBase.sol#L54 DepositReceiver.sol#L17
Solidity defines a naming convention that should be followed.
Follow the Solidity naming convention. Link
AxelarGateway.sol#L447 AxelarGateway.sol#L45 AxelarGateway.sol#L46
#0 - GalloDaSballo
2022-08-28T20:35:28Z
L
R
Disagree, they return early in a specific case
NC
Disagree, you have to prove your statements here
Please tell the sponsor what they can do better instead of linking the Style Guide without context
1L 1R 1NC
π Selected for report: IllIllI
Also found by: 0x1f8b, 0xNazgul, 0xsam, 8olidity, Aymen0909, Bnke0x0, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, Fitraldys, JC, Lambda, MiloTruck, Noah3o6, NoamYakov, RedOneN, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, TomJ, Tomio, Waze, __141345__, a12jmx, ajtra, ak1, apostle0x01, asutorufos, benbaessler, bharg4v, bulej93, c3phas, defsec, djxploit, durianSausage, erictee, fatherOfBlocks, gerdusx, gogo, kyteg, lucacez, medikko, mics, owenthurm, oyc_109, rbserver, robee, sashik_eth, simon135, tofunmi
59.2384 USDC - $59.24
++var (--var) cost less gas than var++ (var--)
AxelarGateway.sol#L207 AxelarGasService.sol#L123 AxelarDepositService.sol#L114 AxelarDepositService.sol#L168 AxelarDepositService.sol#L204
Expressions for constant values such as a call to KECCAK256 should use IMMUTABLE rather than constant
AxelarGateway.sol#L30 AxelarGateway.sol#L31 AxelarGateway.sol#L32 AxelarGateway.sol#L33 AxelarGateway.sol#L34 AxelarGateway.sol#L35 AxelarGateway.sol#L36 AxelarGateway.sol#L38 AxelarGateway.sol#L39 AxelarGateway.sol#L40 AxelarGateway.sol#L41 AxelarGateway.sol#L42 AxelarGateway.sol#L43
Change all <= / >= operators for < / > and remember to increse / decrese in consecuence to maintain the logic (example, a <= b for a < b + 1)
AxelarAuthWeighted.sol#L36 AxelarAuthWeighted.sol#L107 AxelarAuthWeighted.sol#L117
Replace all > 0 for != 0
AxelarGateway.sol#L255 AxelarGateway.sol#L613 AxelarGasService.sol#L128 AxelarGasService.sol#L131 AxelarDepositService.sol#L165 ReceiverImplementation.sol#L23 ReceiverImplementation.sol#L51 ReceiverImplementation.sol#L71 AxelarAuthWeighted.sol#L76 AxelarAuthWeighted.sol#L76
AxelarAuthWeighted.sol#L70 AxelarAuthWeighted.sol#L105
XC20Wrapper.sol#L55 XC20Wrapper.sol#L56 XC20Wrapper.sol#L57 XC20Wrapper.sol#L58 XC20Wrapper.sol#L61 XC20Wrapper.sol#L62 XC20Wrapper.sol#L68 XC20Wrapper.sol#L70 XC20Wrapper.sol#L78 XC20Wrapper.sol#L79 XC20Wrapper.sol#L84 XC20Wrapper.sol#L85 XC20Wrapper.sol#L86 XC20Wrapper.sol#L98 XC20Wrapper.sol#L111
When using elements that are smaller than 32 bytes, your contractβs gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
AxelarGateway.sol#L332 AxelarGateway.sol#L334 AxelarDepositService.sol#L225 AxelarDepositService.sol#L226 AxelarAuthWeighted.sol#L14
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
AxelarGateway.sol#L3 IDepositBase.sol#L3 AxelarGasServiceProxy.sol#L3 AxelarGasService.sol#L3 IAxelarGasService.sol#L3 IAxelarAuth.sol#L3 IAxelarAuthWeighted.sol#L3 IAxelarDepositService.sol#L3 DepositReceiver.sol#L3 AxelarDepositService.sol#L3 AxelarDepositServiceProxy.sol#L3 DepositBase.sol#L3 ReceiverImplementation.sol#L3 AxelarAuthWeighted.sol#L3 XC20Wrapper.sol#L3
If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address,...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.
AxelarGateway.sol#L207 AxelarAuthWeighted.sol#L69 AxelarAuthWeighted.sol#L98
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past
AxelarGateway.sol#L101 AxelarGateway.sol#L113 AxelarGateway.sol#L125 AxelarGateway.sol#L138 AxelarGateway.sol#L160 AxelarGateway.sol#L172 AxelarGateway.sol#L176 AxelarGateway.sol#L229 AxelarGateway.sol#L268 AxelarGateway.sol#L320 AxelarGateway.sol#L344 AxelarGateway.sol#L383 AxelarGateway.sol#L388 AxelarGateway.sol#L460 AxelarGateway.sol#L461 AxelarGateway.sol#L462 AxelarGateway.sol#L477 AxelarGateway.sol#L496 AxelarGateway.sol#L626 AxelarGasService.sol#L158 AxelarGasService.sol#L159 AxelarGasService.sol#L172 AxelarGasService.sol#L175 IAxelarAuth.sol#L8 DepositReceiver.sol#L12 DepositBase.sol#L71 DepositBase.sol#L72 AxelarAuthWeighted.sol#L26 AxelarAuthWeighted.sol#L115 XC20Wrapper.sol#L95 XC20Wrapper.sol#L96 XC20Wrapper.sol#L106 XC20Wrapper.sol#L109
Try to use ABI.ENCODEPACKED() to save gas.
AxelarGateway.sol#L566 AxelarGateway.sol#L580 AxelarDepositService.sol#L233 AxelarAuthWeighted.sol#L32
public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted. Link
Saves 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. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
XC20Wrapper.sol#L20 XC20Wrapper.sol#L21
Use calldata instead of memory in a function parameter when you are only to read the data can save gas by storing it in calldata when the function is external
public functions that are never called by the contract should be declared external to save gas
AxelarDepositService.sol#L241 DepositBase.sol#L41
The default βcheckedβ behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected. if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas.
For all for-loops in the code it is possible to change as the following example.
for (uint256 i;i < X;){ -- code -- unchecked { ++i; } }
AxelarGateway.sol#L195 AxelarGateway.sol#L207 AxelarGateway.sol#L292 AxelarGasService.sol#L123 AxelarDepositService.sol#L114 AxelarDepositService.sol#L168 AxelarDepositService.sol#L204 AxelarAuthWeighted.sol#L17 AxelarAuthWeighted.sol#L69 AxelarAuthWeighted.sol#L98 AxelarAuthWeighted.sol#L101 AxelarAuthWeighted.sol#L116
The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})
Constant state variables should be declared constant to save gas. Add the constant attributes to state variables that never change.
#0 - re1ro
2022-08-05T09:58:10Z
Dup #2 #3 #7 #122
#1 - GalloDaSballo
2022-08-20T18:57:02Z
Less than 500 gas saved.
The advice on upgrading solidity is technically correct, however the submission doesn't list the external calls so I can't quantify it