Axelar Network contest - ilan's results

name and symbol should be immutable

A mismatch between doc and code.

Proof of Concept:

in ERC20.sol constructor:

/** * @dev Sets the values for {name}, {symbol}, and {decimals}. All three of these values are immutable: they can only be set once during construction. */

however, in the code they are declared: string public name; string public symbol;

therefore they should be immutuable, and anyway there is a mismatch between the doc and the code.

Recommended Mitigation Steps: change variables to

string public imuutable name; string public immutable symbol;

Missing docs

Most functions and logic are missing documentation and comments

AdminMultisigBase AxelarGateway AxelarGatewayMultisig AxelarGatewaySinglesig AxelarGatewayProxy BurnableMintableCappedERC20 ERC20Permit MintableCappedERC20 Ownable TokenDeployer

Use enum or struct for lock states

In DepositHandler.sol, use enum instead of 2 unit256 for LOCK variables. will make the code nicer and more structured.

Change i++ to ++i

i++ is generally more expensive because it must increment a value and return the old value, so it may require holding two numbers in memory. ++i only uses one number in memory.

Example:

for (uint256 i; i < adminCount; i++) -> for (uint256 i; i < adminCount; ++i)

line 255 in AxelarGateway.sol.

Affected contracts: AdminMultisigBase.sol AxelarGateway.sol AxelarGatewayMultisig.sol AxelarGatewaySinglesig.sol AddressFormat.sol

Use calldata instead of memory

using calldata for function parameters which do not change inside the function, instead of memory, has proven to save gas.

AxelarGatewayMultisig.sol, l41 AdminMultisigBase.sol, l144 AxelarGateway.sol AxelarGatewaySinglesig.sol

Loop cache

save array length locally instead of calling .length in the loop every iteration. Fewer memory calls result in less gas consumption.

POC:

for (uint256 i; i < accounts.length - 1; ++i)

every loop call accounts.length is called

change to length = accounts.length for (uint256 i; i < length - 1; ++i) every loop call accounts.length is called

Affected contracts: AxelarGatewayMultisig._isSortedAscAndContainsNoDuplicate AddressFormat.sol, l14 (library)

make cap variable immutable

immutable variables consume less gas

change uint256 public cap; to uint256 public immutable cap;

in MintableCappedERC20.sol

Unnecessary State Variables

In DepositHandler.sol. using 3 state variables for the lock when actually just 1 is needed. Wasting storage space results in wasting gas, and many storage calls consume a lot of gas.

can resolve by changing lock mechanism to receive unit values; set _lockedStatus = unit256(0) and set _lockedStatus = unit256(1)

Do not explicitly initialize uint256 i = 0

Uninitialized variables are automatically initialized to 0, doing it explicitly will cost more gas.

poc: for (uint256 i = 0; i < length; i++)

change to for (uint256 i; i < length; i++)

in AddressFormat.sol