Axelar Network v2 contest - ElKu's results

Non-critical Issues

Summary Of Findings:

Details on Findings:

1. <ins>Custom errors could be used with parameters for better user experience</ins>

Custom errors can take in parameters and are useful in certain situations for knowing what actually went wrong. This will help the user to make the next transaction smartly.

The following custom errors could have had a parameter:

File:   contracts/AxelarGateway.sol

Line 286:   if (chainId != block.chainid) revert InvalidChainId();


File:   contracts/auth/AxelarAuthWeighted.sol

Line 066:   if (weightsLength != operatorsLength) revert InvalidWeights();
Line 072:   if (newThreshold == 0 || totalWeight < newThreshold) revert InvalidThreshold();  

2. <ins>Due to syntax error, custom errors are compiled as revert strings</ins>

In XC20Wrapper.sol, there are custom errors declared. But due to a mistake in syntax, they are not used. But the compiler uses regular revert strings instead.

Note that making this correction will also reduce the deployment cost by a huge amount. As per this example test I implemented, the gas cost will be reduced by 15*11,000 = 165,000.

See the below 15 instances of this issue:

File:   xc20/contracts/XC20Wrapper.sol

Line 055:   if (axelarToken == address(0)) revert('NotAxelarToken()');
Line 056:   if (xc20Token.codehash != xc20Codehash) revert('NotXc20Token()');
Line 057:   if (wrapped[axelarToken] != address(0)) revert('AlreadyWrappingAxelarToken()');   
Line 058:   if (unwrapped[xc20Token] != address(0)) revert('AlreadyWrappingXC20Token()'); 
Line 061:   if (!LocalAsset(xc20Token).set_team(address(this), address(this), address(this))) revert('NotOwner()');
Line 062:   if (!LocalAsset(xc20Token).set_metadata(newName, newSymbol, IERC20(axelarToken).decimals())) revert('CannotSetMetadata()');
Line 068:   if (axelarToken == address(0)) revert('NotAxelarToken()');
Line 070:   if (xc20Token == address(0)) revert('NotWrappingToken()');
Line 078:   if (wrappedToken == address(0)) revert('NotAxelarToken()');
Line 079:   if (!LocalAsset(wrappedToken).mint(msg.sender, amount)) revert('CannotMint()');
Line 084:   if (axelarToken == address(0)) revert('NotXc20Token()');
Line 085:   if (IERC20(wrappedToken).balanceOf(msg.sender) < amount) revert('InsufficientBalance()');
Line 086:   if (!LocalAsset(wrappedToken).burn(msg.sender, amount)) revert('CannotBurn()');
Line 098:   if (!transferred || tokenAddress.code.length == 0) revert('TransferFailed()');
Line 111:   if (!transferred || tokenAddress.code.length == 0) revert('TransferFailed()');

The <ins>mitigation</ins> would be to correct the syntax. Lets take the first instance in the above list:

Line 055:   if (axelarToken == address(0)) revert('NotAxelarToken()');
//should be
if (axelarToken == address(0)) revert NotAxelarToken();

3. <ins>Natspec comments are not used in the source code</ins>

The following files doesn't use natspec comments:

1. AxelarGateway
2. IDepositBase
3. AxelarGasService
4. AxelarGasServiceProxy
5. IAxelarAuth
6. IAxelarGasService
7. IAxelarDepositService
8. IAxelarAuthWeighted
9. DepositReceiver
10. AxelarDepositService
11. DepositBase
12. ReceiverImplementation
13. AxelarDepositServiceProxy
14. AxelarAuthWeighted
15. XC20Wrapper

Gas Optimizations

Summary Of Findings:

Detailed Report on Gas Optimization Findings:

1. <ins>Emit events at the end of the function</ins>

Emitting events at the end of the function will save any gas cost of emitting incase the transaction reverts after that.

Instances listed below:

```solidity
File:  contracts/AxelarGateway.sol

Line 224:  emit Upgraded(newImplementation);

2. <ins>Use custom errors instead of revert strings</ins>

Note that, since it looked like a syntax mistake, I have made a submission in the QA report as well. Please remove one of them as deemed necessary.

File : XC20Wrapper.sol

As per this example test I implemented, custom errors can cut down around 11,000 gas in deployment cost and around 55 gas (upon a revert) per external/public function.

See the below 15 instances of this issue:

File:   xc20/contracts/XC20Wrapper.sol

Line 055:   if (axelarToken == address(0)) revert('NotAxelarToken()');
Line 056:   if (xc20Token.codehash != xc20Codehash) revert('NotXc20Token()');
Line 057:   if (wrapped[axelarToken] != address(0)) revert('AlreadyWrappingAxelarToken()');   
Line 058:   if (unwrapped[xc20Token] != address(0)) revert('AlreadyWrappingXC20Token()');
Line 061:   if (!LocalAsset(xc20Token).set_team(address(this), address(this), address(this))) revert('NotOwner()');
Line 062:   if (!LocalAsset(xc20Token).set_metadata(newName, newSymbol, IERC20(axelarToken).decimals())) revert('CannotSetMetadata()');
Line 068:   if (axelarToken == address(0)) revert('NotAxelarToken()');
Line 070:   if (xc20Token == address(0)) revert('NotWrappingToken()');
Line 078:   if (wrappedToken == address(0)) revert('NotAxelarToken()');
Line 079:   if (!LocalAsset(wrappedToken).mint(msg.sender, amount)) revert('CannotMint()');
Line 084:   if (axelarToken == address(0)) revert('NotXc20Token()');
Line 085:   if (IERC20(wrappedToken).balanceOf(msg.sender) < amount) revert('InsufficientBalance()');
Line 086:   if (!LocalAsset(wrappedToken).burn(msg.sender, amount)) revert('CannotBurn()');
Line 098:   if (!transferred || tokenAddress.code.length == 0) revert('TransferFailed()');
Line 111:   if (!transferred || tokenAddress.code.length == 0) revert('TransferFailed()');

Number of instances = 15. Deployment gas cost saved = 15 * 11,000 = 165,000. Number of affected external functions = 4. Dynamic gas cost saved(on a revert) = 4 * 55 = 220.