re1ro
commented
2 years ago L1
Receive is used when unwrapping WETH and that ether is processed. No need to emit anything. Dup #3
L2
Good spot.
Mitigated
https://github.com/axelarnetwork/axelar-cgp-solidity/pull/138
N1
Ack
N2
Ack
N3
Ack
N4
It's intentional. We want to avoid reusing storage slots in future. So we keep deprecated prefixed commented
N5
Floating Pragma is only for interfaces, so they can be imported by other projects
N6
Ack
GalloDaSballo
[NAZ-L1]
receive()Function Should Emit An EventSeverity: Low Context:
DepositReceiver.sol#L29,AxelarDepositServiceProxy.sol#L13Description: Consider emitting an event inside this function with
msg.senderandmsg.valueas the parameters. This would make it easier to track incoming ether transfers.Recommendation: Add events to the
receive()functions.[NAZ-L2] Local Variable Shadowing
Severity: Low Context:
AxelarDepositService.sol#L19 (both gateway && wrappedSymbol)Description: These Variables shadow state variables. As a result, the use of them locally might be incorrect.
Recommendation: Rename the local variables that shadow another component.
[NAZ-N1] Code Contains Empty Blocks
Severity Informational Context:
AxelarDepositServiceProxy.sol#L13,AxelarAuthWeighted.sol#L101Description: It's best practice that when there is an empty block, to add a comment in the block exmplaining why it's empty.
Recommendation: Consider adding
/* Comment on why */to the empty block.[NAZ-N2] Variable Naming Convention
Severity Informational Context:
AxelarGateway.sol#L45-L46Description: The linked variables do not conform to the standard naming convention of Solidity whereby functions and variable names(local, state and immutable) utilize the
mixedCaseformat unless variables are declared asconstantin which case they utilize theUPPER_CASE_WITH_UNDERSCORESformat.Recommendation: Naming conventions utilized by the linked statements are adjusted to reflect the correcttype of declaration according to the Solidity style guide.
[NAZ-N3] Missing Use of
solhint-disable-next-lineSeverity: Informational Context:
AxelarGateway.sol#L157,AxelarGateway.sol#L229,AxelarGateway.sol#L320,AxelarGateway.sol#L344,AxelarGateway.sol#L461,AxelarGateway.sol#L615,AxelarAuthWeighted.sol#L101Description:
solhint-disable-next-lineis used elsewhere for small linter errors and can be used here to disable these errors.Recommendation: Consider adding
solhint-disable-next-line.[NAZ-N4] Commented Out Code
Severity: Informational Context:
AxelarGateway.sol#L22-L24Description: There is commented code that makes the code messy and unneeded.
Recommendation: Remove the commented out code.
[NAZ-N5] Floating Pragma
Severity: Informational Context:
IAxelarGasService.sol,IAxelarDepositService.sol,IAxelarAuthWeighted.sol,IDepositBase.solDescription: Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.
Recommendation: Lock the pragma version.
[NAZ-N6] Missing or Incomplete NatSpec
Severity: Informational Context:
All ContractsDescription: 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.