Axelar Network contest - Funen's results

1. Unmatch comment with actual code

https://github.com/code-423n4/2022-04-axelar/blob/main/src/ERC20.sol#L48

its said that : ``` * All three of these values are immutable``

but in actual codename and symbol arent immutable value. so it would be missleading information on comment sectioin.

##Tool Used Manual Review, Remix

##Recommended Mitigation Change it or remove it

1. Better way to use nonReenter for saving more gas

https://github.com/code-423n4/2022-04-axelar/blob/main/src/DepositHandler.sol#L6-L7

This implementation for nonReenter, can be used 1 and 2, instead of 0 and 1 for saving more gas.

##Tool Used Manual Review

##POC

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/ReentrancyGuard.sol

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.


    // The values being non-zero value makes deployment a bit more expensive,
    // but in exchange the refund on every call to nonReentrant will be lower in
    // amount. Since refunds are capped to a percentage of the total
    // transaction's gas, it is best to keep them low in cases like this one, to
    // increase the likelihood of the full refund coming into effect.

2. Variable can be set as immutable for saving gas

https://github.com/code-423n4/2022-04-axelar/blob/main/src/MintableCappedERC20.sol

set cap as immutable for saving more gas.

Tool Used

Manual Review

3. Used msg.sender than _msg.sender() for saving more gas

https://github.com/code-423n4/2022-04-axelar/blob/main/src/ERC20.sol

Using msg.sender was less cost gas than _msg.Sender(). This _msg.Sender() was a replacement for msg.sender.For regular transactions it returns msg.sender and for meta transactions it can be used to return the end user (rather than the relayer). but the implementation for msg.sender can saving more gas more.

##Tool Used Manual Review

4. Used || operator for saving more gas

https://github.com/code-423n4/2022-04-axelar/blob/dee2f2d352e8f20f20027977d511b19bfcca23a3/src/AxelarGatewayMultisig.sol#L119

This implementation below can be saving more gas, has the same output either.

##Tool Used Remix & test

##Recommended Mitigation

            if (_isOwner(epoch, accounts[i]) && ++validSignerCount >= threshold) return true;
           // 6960521 before

change to :

           if (_isOwner(epoch, accounts[i]) || ++validSignerCount >= threshold) return true;
          // 6960341 after

5. using ++i than i++ for saving more gas

Using i++ instead ++i for all the loops, the variable i is incremented using i++. It is known that implementation by using ++i costs less gas per iteration than i++.

Tools Used

Remix

Occurances

main/src/AxelarGatewayMultisig.sol#L118
main/src/AxelarGatewayMultisig.sol#L140
main/src/AxelarGatewayMultisig.sol#L182
main/src/AxelarGatewayMultisig.sol#L271
main/src/AxelarGatewayMultisig.sol#L293
main/src/AxelarGatewayMultisig.sol#L332
main/src/AxelarGatewayMultisig.sol#L495
main/src/AxelarGatewayMultisig.sol#L526

6. Unnecessary check

https://github.com/code-423n4/2022-04-axelar/blob/dee2f2d352e8f20f20027977d511b19bfcca23a3/src/AxelarGatewayMultisig.sol#L571-L572

this can be shorten by not checking if (success) just passing into emit and can be saving more gas instead.

##Tool Used Test, Remix

##Recommended Mitigation Remove if (success) and pass into emit.

7. Unnecessary Duplicate Check

https://github.com/code-423n4/2022-04-axelar/blob/dee2f2d352e8f20f20027977d511b19bfcca23a3/src/AxelarGatewayMultisig.sol#L529

since if was ignore duplicate commandId received, no function inside contract was called so it would be effective that remove it and saving more gas.

##Tool Used Manual Review, test