code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

Gas Optimizations #13

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[G-01] State variables only set in the constructor should be declared immutable:-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L23 :

    bytes32 public xc20Codehash;

[G-02] x = x + y is cheaper than x += y :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L70 :

    totalWeight += newWeights[i];

  2. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L105:

    weight += weights[operatorIndex];

[G-03] .length should not be looked up in every loop of a for-loop :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L207 :

    for (uint256 i = 0; i < symbols.length; i++) {

  2. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L123 :

    for (uint256 i; i < tokens.length; i++) {

  3. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L114 :

    for (uint256 i; i < refundTokens.length; i++) {

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L168 :

    for (uint256 i; i < refundTokens.length; i++) {

  5. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L204 :

    for (uint256 i; i < refundTokens.length; i++) {

  6. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L17 :

    for (uint256 i; i < recentOperators.length; ++i) {

  7. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L98 :

    for (uint256 i = 0; i < signatures.length; ++i) {

  8. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L116 :

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

[G-04] Use prefix not postfix in loops (Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.) :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L207 :

    for (uint256 i = 0; i < symbols.length; i++) {

  2. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L123 :

    for (uint256 i; i < tokens.length; i++) {

  3. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L114 :

    for (uint256 i; i < refundTokens.length; i++) {

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L168 :

    for (uint256 i; i < refundTokens.length; i++) {

  5. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L204 :

    for (uint256 i; i < refundTokens.length; i++) {

  6. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L17 :

    for (uint256 i; i < recentOperators.length; ++i) {

  7. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L98 :

    for (uint256 i = 0; i < signatures.length; ++i) {

  8. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L116 :

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

  9. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L195 :

    for (uint256 i; i < adminCount; ++i) {

  10. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L292 :

    ffor (uint256 i; i < commandsLength; ++i) {

  11. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L69 :

    for (uint256 i = 0; i < weightsLength; ++i) {

[G-05] Not using the named return variables when a function returns, wastes deployment gas :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L182 :

    return _adminEpoch();

[G-06] It costs more gas to initialize variables to zero than to let the default of zero be applied :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L207 :

    for (uint256 i = 0; i < symbols.length; i++) {

  2. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L98 :

    for (uint256 i = 0; i < signatures.length; ++i) {

  3. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L116 :

  4. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L69 :

    for (uint256 i = 0; i < weightsLength; ++i) {

[G-07] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L14 :

    uint8 internal constant OLD_KEY_RETENTION = 16;

[G-08] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L30#L43 :

    `bytes32 internal constant PREFIX_COMMAND_EXECUTED = keccak256('command-executed'); bytes32 internal constant PREFIX_TOKEN_ADDRESS = keccak256('token-address'); bytes32 internal constant PREFIX_TOKEN_TYPE = keccak256('token-type'); bytes32 internal constant PREFIX_CONTRACT_CALL_APPROVED = keccak256('contract-call-approved'); bytes32 internal constant PREFIX_CONTRACT_CALL_APPROVED_WITH_MINT = keccak256('contract-call-approved-with-mint'); bytes32 internal constant PREFIX_TOKEN_DAILY_MINT_LIMIT = keccak256('token-daily-mint-limit'); bytes32 internal constant PREFIX_TOKEN_DAILY_MINT_AMOUNT = keccak256('token-daily-mint-amount');

    bytes32 internal constant SELECTOR_BURN_TOKEN = keccak256('burnToken'); bytes32 internal constant SELECTOR_DEPLOY_TOKEN = keccak256('deployToken'); bytes32 internal constant SELECTOR_MINT_TOKEN = keccak256('mintToken'); bytes32 internal constant SELECTOR_APPROVE_CONTRACT_CALL = keccak256('approveContractCall'); bytes32 internal constant SELECTOR_APPROVE_CONTRACT_CALL_WITH_MINT = keccak256('approveContractCallWithMint'); bytes32 internal constant SELECTOR_TRANSFER_OPERATORSHIP = keccak256('transferOperatorship');`

[G-09] Use custom errors rather than revert()/require() strings to save deployment gas :-

  1. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L55#L58 :

    revert('NotAxelarToken()'); revert('NotXc20Token()'); revert('AlreadyWrappingAxelarToken()'); revert('AlreadyWrappingXC20Token()');

  2. https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L60#L61 :

    revert('NotOwner()'); revert('CannotSetMetadata()');

  3. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L68 :

    'revert('NotAxelarToken()');'

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L70 :

    revert('NotWrappingToken()');

  5. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L78#L79 :

    revert('NotAxelarToken()'); revert('CannotMint()');

  6. https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L84#L86 :

    revert('NotXc20Token()'); revert('InsufficientBalance()'); revert('CannotBurn()');

  7. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L98 :

    'revert('TransferFailed()');'

  8. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L111 :

    revert('TransferFailed()');

[G-09] Functions guaranteed to revert when called by normal users can be marked payable (If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.) :-

  1. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L204 :

    function setTokenDailyMintLimits(string[] calldata symbols, uint256[] calldata limits) external override onlyAdmin {

  2. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L217#L221 :

    function upgrade( address newImplementation, bytes32 newImplementationCodeHash, bytes calldata setupParams ) external override onlyAdmin {

  3. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L331 :

    'function deployToken(bytes calldata params, bytes32) external onlySelf {'

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L367 :

    function mintToken(bytes calldata params, bytes32) external onlySelf {

  5. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L373 :

    function burnToken(bytes calldata params, bytes32) external onlySelf {

  6. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L397 :

    function approveContractCall(bytes calldata params, bytes32 commandId) external onlySelf {

  7. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L411 :

    'function approveContractCallWithMint(bytes calldata params, bytes32 commandId) external onlySelf {'

  8. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L437 :

    function transferOperatorship(bytes calldata newOperatorsData, bytes32) external onlySelf {

  9. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L120 :

    function collectFees(address payable receiver, address[] calldata tokens) external onlyOwner {

  10. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L136#L140 :

    function refund( address payable receiver, address token, uint256 amount ) external onlyOwner {

  11. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L47 :

    'function transferOperatorship(bytes calldata params) external onlyOwner {'

  12. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L44 :

    function setXc20Codehash(bytes32 newCodehash) external onlyOwner {

  13. https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L48#L53 :

    function addWrapping( string calldata symbol, address xc20Token, string memory newName, string memory newSymbol ) external payable onlyOwner {

  14. https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L66 :

    function removeWrapping(string calldata symbol) external onlyOwner {

  15. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L411 :

    'function approveContractCallWithMint(bytes calldata params, bytes32 commandId) external onlySelf {'

  16. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L437 :

    function transferOperatorship(bytes calldata newOperatorsData, bytes32) external onlySelf {

[G-10] Use a more recent version of solidity :-

  1. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L3 :

    pragma solidity 0.8.9;

  2. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IDepositBase.sol#L3 :

    pragma solidity 0.8.9;

  3. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L3 :

    'pragma solidity 0.8.9;'

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasServiceProxy.sol#L3 :

    pragma solidity 0.8.9;

  5. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarAuth.sol#L3 :

    pragma solidity 0.8.9;

  6. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarGasService.sol#L3 :

    pragma solidity 0.8.9;

  7. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarDepositService.sol#L3 :

    'pragma solidity 0.8.9;'

  8. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/interfaces/IAxelarAuthWeighted.sol#L3 :

    pragma solidity 0.8.9;

  9. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/DepositReceiver.sol#L3 :

    pragma solidity 0.8.9;

  10. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositService.sol#L3 :

    pragma solidity 0.8.9;

  11. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/DepositBase.sol#L3 :

    'pragma solidity 0.8.9;'

  12. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/ReceiverImplementation.sol#L3 :

    pragma solidity 0.8.9;

  13. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositServiceProxy.sol#L3 :

    pragma solidity 0.8.9;

  14. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L3 :

    pragma solidity 0.8.9;

  15. File: https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L3 :

    'pragma solidity 0.8.9;'

[G-11] internal functions only called once can be inlined to save gas :-

  1. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L460 :

    function _callERC20Token(address tokenAddress, bytes memory callData) internal returns (bool) {

  2. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L465#L469 :

    function _mintToken( string memory symbol, address account, uint256 amount ) internal {

  3. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L485#L489 :

    'function _burnTokenFrom( address sender, string memory symbol, uint256 amount ) internal {'

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L539#L557 :

    `function _getTokenDailyMintLimitKey(string memory symbol) internal pure returns (bytes32) { return keccak256(abi.encodePacked(PREFIX_TOKEN_DAILY_MINT_LIMIT, symbol)); }

    function _getTokenDailyMintAmountKey(string memory symbol, uint256 day) internal pure returns (bytes32) { return keccak256(abi.encodePacked(PREFIX_TOKEN_DAILY_MINT_AMOUNT, symbol, day)); }

    function _getTokenTypeKey(string memory symbol) internal pure returns (bytes32) { return keccak256(abi.encodePacked(PREFIX_TOKEN_TYPE, symbol)); }

    function _getTokenAddressKey(string memory symbol) internal pure returns (bytes32) { return keccak256(abi.encodePacked(PREFIX_TOKEN_ADDRESS, symbol)); }

    function _getIsCommandExecutedKey(bytes32 commandId) internal pure returns (bytes32) { return keccak256(abi.encodePacked(PREFIX_COMMAND_EXECUTED, commandId)); }`

  5. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/AxelarGateway.sol#L559#L658 :

    ` function _getIsContractCallApprovedKey( bytes32 commandId, string memory sourceChain, string memory sourceAddress, address contractAddress, bytes32 payloadHash ) internal pure returns (bytes32) { return keccak256(abi.encode(PREFIX_CONTRACT_CALL_APPROVED, commandId, sourceChain, sourceAddress, contractAddress, payloadHash)); }

    function _getIsContractCallApprovedWithMintKey( bytes32 commandId, string memory sourceChain, string memory sourceAddress, address contractAddress, bytes32 payloadHash, string memory symbol, uint256 amount ) internal pure returns (bytes32) { return keccak256( abi.encode( PREFIX_CONTRACT_CALL_APPROVED_WITH_MINT, commandId, sourceChain, sourceAddress, contractAddress, payloadHash, symbol, amount ) ); }

    /****\ | Internal Getters | ****/

    function _getTokenType(string memory symbol) internal view returns (TokenType) { return TokenType(getUint(_getTokenTypeKey(symbol))); }

    /****\ | Internal Setters | ****/

    function _setTokenDailyMintLimit(string memory symbol, uint256 limit) internal { _setUint(_getTokenDailyMintLimitKey(symbol), limit);

    emit TokenDailyMintLimitUpdated(symbol, limit);

    }

    function _setTokenDailyMintAmount(string memory symbol, uint256 amount) internal { uint256 limit = tokenDailyMintLimit(symbol); if (limit > 0 && amount > limit) revert ExceedDailyMintLimit(symbol);

    _setUint(_getTokenDailyMintAmountKey(symbol, block.timestamp / 1 days), amount);

    }

    function _setTokenType(string memory symbol, TokenType tokenType) internal { _setUint(_getTokenTypeKey(symbol), uint256(tokenType)); }

    function _setTokenAddress(string memory symbol, address tokenAddress) internal { _setAddress(_getTokenAddressKey(symbol), tokenAddress); }

    function _setCommandExecuted(bytes32 commandId, bool executed) internal { _setBool(_getIsCommandExecutedKey(commandId), executed); }

    function _setContractCallApproved( bytes32 commandId, string memory sourceChain, string memory sourceAddress, address contractAddress, bytes32 payloadHash ) internal { _setBool(_getIsContractCallApprovedKey(commandId, sourceChain, sourceAddress, contractAddress, payloadHash), true); }

    function _setContractCallApprovedWithMint( bytes32 commandId, string memory sourceChain, string memory sourceAddress, address contractAddress, bytes32 payloadHash, string memory symbol, uint256 amount ) internal { _setBool( _getIsContractCallApprovedWithMintKey(commandId, sourceChain, sourceAddress, contractAddress, payloadHash, symbol, amount), true ); }

    function _setImplementation(address newImplementation) internal { _setAddress(KEY_IMPLEMENTATION, newImplementation); } }`

  6. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L150#L154 :

    function _safeTransfer( address tokenAddress, address receiver, uint256 amount ) internal {

  7. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/gas-service/AxelarGasService.sol#L164#L168 :

    'function _safeTransferFrom( address tokenAddress, address from, uint256 amount ) internal {'

[G-12] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate :-

  1. https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L20#L21 :

    mapping(address => address) public wrapped; mapping(address => address) public unwrapped;

[G-13] Empty blocks should be removed or emit something :-

  1. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/DepositReceiver.sol#L29 :

    receive() external payable {}

  2. https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/AxelarDepositServiceProxy.sol#L13 :

    receive() external payable override {}

  3. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/deposit-service/ReceiverImplementation.sol#L12 :

    'constructor(address gateway, string memory wrappedSymbol) DepositBase(gateway, wrappedSymbol) {}'

  4. File: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L101 :

    for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}

re1ro commented 2 years ago

1

Not applicable. There is setXc20Codehash() that changes the storage.

2

Yup. Dup #2

3

Yup. Dup #2

4

Yup. Dup #2

5

Correct. Good spot

6

Yup. Dup #2

7

Yup. Dup #7

8

Yup. Dup #12

9

Good spot

Mitigated

https://github.com/axelarnetwork/axelar-xc20-wrapper/pull/4

9.5

Yup. Dup #7

10

Dup #3

11

We prefer cleaner code.

12

Not applicable. Dup #2

13

Not applicable. Dup #2

GalloDaSballo commented 2 years ago

[G-01] State variables only set in the constructor should be declared immutable:-

Contracts are upgradeable, cannot be done

[G-08] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant :-

Not true https://twitter.com/GalloDaSballo/status/1543729080926871557

Less than 500 gas saved in total