Back to 0xAnah's contests

Axelar Network - 0xAnah's results

Decentralized interoperability network.

General Information

Platform: Code4rena

Start Date: 12/07/2023

Pot Size: $80,000 USDC

Total HM: 11

Participants: 47

Period: 9 days

Judge: berndartmueller

Total Solo HM: 1

Id: 260

League: ETH

Axelar Network

Findings Distribution

Researcher Performance

Rank: 43/47

Findings: 1

Award: $19.28

Gas:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryAxelar Network

Findings Information

🌟 Selected for report: Sathish9098

Also found by: 0x11singh99, 0xAnah, 0xn006e7, Arz, DavidGiladi, K42, Raihan, ReyAdmirado, Rolezn, SAQ, SM3_SS, SY_S, Walter, dharma09, flutter_developer, hunter_w3b, matrix_0wl, naman1778, petrichor, ybansal2403

Awards

19.2767 USDC - $19.28

Labels

bug
G (Gas Optimization)
grade-b
G-14

External Links

Link to GitHub issue

Axelar Gas Optimizations

[G-01] Unused Imports

Some files were imported and were not used these files costs gas during deployment and generally this is bad coding practice

7 Instances

In the file below IProxy was imported with the statement import { IProxy } from '../interfaces/IProxy.sol'but was not used. consider removing this import.

In the file below IAxelarGateway was imported with the statement import { IAxelarGateway } from '../../gmp-sdk/interfaces/IAxelarGateway.sol'but was not used, consider removing this import.

In the file below ITokenManagerDeployer was imported with the statement import { ITokenManagerDeployer } from './ITokenManagerDeployer.sol'but was not used, consider removing this import.

In the file below IERC20BurnableMintable was imported with the statement import { IERC20BurnableMintable } from '../interfaces/IERC20BurnableMintable.sol'but was not used, consider removing this import.

In the file below IAxelarGateway was imported with the statement import { IAxelarGateway } from '../../gmp-sdk/interfaces/IAxelarGateway.sol'but was not used, consider removing this import.

In the file below IERC20 was imported with the statement import { IERC20 } from '../../../gmp-sdk/interfaces/IERC20.sol'but was not used, consider removing this import.

In the file below IAxelarGateway was imported with the statement import { IAxelarGateway } from '../../../gmp-sdk/interfaces/IAxelarGateway.sol';but was not used, consider removing this import.

[G-02] Making constant and Immutable variables private will save gas during deployment.

When constants and immutable are marked public, extra getter functions are created, increasing the deployment cost. Marking these functions private will decrease gas cost. One can still read these variables through the source code. If they need to be accessed by an external contract, a separate single getter function can be used to return all constants as a tuple.

5 Instances

23:    bytes32 public immutable governanceChainHash;
24:    bytes32 public immutable governanceAddressHash;

From the contract above during compilation getter functions would be created for the public immutable variables governanceChainHash and governanceAddressHash this leads to increased deployment cost. The deployment cost could be reduced if the immutable variables are made internal or private thereby eliminating the need for the compiler to create getter functions for them, if these variable are needed externally we could have a single getter function that returns the values of these immutable variables as a tuple. 3213 gas units could be save if the code is refactored to:

    bytes32 internal immutable governanceChainHash;
    bytes32 internal immutable governanceAddressHash;

    function getConstants() public view returns(bytes32, bytes32) {
        return (governanceChainHash, governanceAddressHash);
    }
54:    IAxelarGasService public immutable gasService;
55:    IRemoteAddressValidator public immutable remoteAddressValidator;
56:    address public immutable tokenManagerDeployer;
57:    address public immutable standardizedTokenDeployer;

From the contract above during compilation getter functions would be created for the public immutable variables gasService, remoteAddressValidator, tokenManagerDeployer and standardizedTokenDeployer this leads to increased deployment cost. The deployment cost could be reduced if the immutable variables are made internal or private thereby eliminating the need for the compiler to create getter functions for them, if these variable are needed externally we could have a single getter function that returns the values of these immutable variables as a tuple. 8515 gas units could be save if the code is refactored to:

    IAxelarGasService internal immutable gasService;
    IRemoteAddressValidator internal immutable remoteAddressValidator;
    address internal immutable tokenManagerDeployer;
    address internal immutable standardizedTokenDeployer;

    function getConstants() public view returns(
        IAxelarGasService,
        IRemoteAddressValidator,
        address,
        address
    ) {
        return (gasService, remoteAddressValidator, tokenManagerDeployer, standardizedTokenDeployer);
    }
14:    IInterchainTokenService public immutable interchainTokenServiceAddress;
15:    uint256 public immutable implementationType;
16:    bytes32 public immutable tokenId;

From the contract above during compilation getter functions would be created for the public immutable variables interchainTokenServiceAddress, implementationType and tokenId this leads to increased deployment cost. The deployment cost could be reduced if the immutable variables are made internal or private thereby eliminating the need for the compiler to create getter functions for them, if these variable are needed externally we could have a single getter function that returns the values of these immutable variables as a tuple. 8893 gas units could be save if the code is refactored to:

    IInterchainTokenService internal immutable interchainTokenServiceAddress;
    uint256 internal immutable implementationType;
    bytes32 internal immutable tokenId;


    function getConstants() public view returns(
        IInterchainTokenService,
        uint256,
        bytes32
    ){
        return (interchainTokenServiceAddress, implementationType, tokenId);
    }
17:    address public immutable interchainTokenServiceAddress;
18:    bytes32 public immutable interchainTokenServiceAddressHash;

From the contract above during compilation getter functions would be created for the public immutable variables interchainTokenServiceAddress and interchainTokenServiceAddressHash this leads to increased deployment cost. The deployment cost could be reduced if the immutable variables are made internal or private thereby eliminating the need for the compiler to create getter functions for them, if these variable are needed externally we could have a single getter function that returns the values of these immutable variables as a tuple. 7304 gas units could be save if the code is refactored to:

    address internal immutable interchainTokenServiceAddress;
    bytes32 internal immutable interchainTokenServiceAddressHash;

    function getConstants() public view returns(address, bytes32) {
        return (interchainTokenServiceAddress, interchainTokenServiceAddressHash);
    }
16:    Create3Deployer public immutable deployer;
17:    address public immutable implementationMintBurnAddress;
18:    address public immutable implementationLockUnlockAddress;

From the contract above during compilation getter functions would be created for the public immutable variables deployer, implementationMintBurnAddress and implementationLockUnlockAddress this leads to increased deployment cost. The deployment cost could be reduced if the immutable variables are made internal or private thereby eliminating the need for the compiler to create getter functions for them, if these variable are needed externally we could have a single getter function that returns the values of these immutable variables as a tuple. 5005 gas units could be save if the code is refactored to:

    Create3Deployer internal immutable deployer;
    address internal immutable implementationMintBurnAddress;
    address internal immutable implementationLockUnlockAddress;

    function getConstants() public view returns(
        Create3Deployer,
        address,
        address
    ) {
        return (deployer, implementationMintBurnAddress, implementationLockUnlockAddress);
    }
Total Gas saved: 32930

[G-03] Declaring Unneccessary Variable

Some varibles were defined even though they are used once. Not defining these variables can reduce gas cost and contract size.

3 Instances

    function deployedAddress(
        bytes calldata bytecode,
        address sender,
        bytes32 salt
    ) external view returns (address deployedAddress_) {
        bytes32 newSalt = keccak256(abi.encode(sender, salt));
        deployedAddress_ = address(
            uint160(
                uint256(
                    keccak256(
                        abi.encodePacked(
                            hex'ff',
                            address(this),
                            newSalt,
                            keccak256(bytecode) // init code hash
                        )
                    )
                )
            )
        );
    }

In the function above the newSalt variable was declared and used once. 1080 gas units can be saved during deployment and 13 gas units when the function is called by another contract by not declaring the newSalt variable thus the code could be refactored to:

    function deployedAddress(
        bytes calldata bytecode,
        address sender,
        bytes32 salt
    ) external view returns (address deployedAddress_) {
        deployedAddress_ = address(
            uint160(
                uint256(
                    keccak256(
                        abi.encodePacked(
                            hex'ff',
                            address(this),
                            keccak256(abi.encode(sender, salt)),
                            keccak256(bytecode) // init code hash
                        )
                    )
                )
            )
        );
    }

    function getFlowOutAmount() external view returns (uint256 flowOutAmount) {
        uint256 epoch = block.timestamp / EPOCH_TIME;
        uint256 slot = _getFlowOutSlot(epoch);
        assembly {
            flowOutAmount := sload(slot)
        }
    }

In the function above the epoch variable was declared and used once. 1080 gas units can be saved during deployment and 13 gas units when the function is called by another contract by not declaring the epoch variable thus the code could be refactored to:

    function getFlowOutAmount() external view returns (uint256 flowOutAmount) {
        uint256 slot = _getFlowOutSlot(block.timestamp / EPOCH_TIME);
        assembly {
            flowOutAmount := sload(slot)
        }
    }
function getFlowInAmount() external view returns (uint256 flowInAmount) {
    uint256 epoch = block.timestamp / EPOCH_TIME;
    uint256 slot = _getFlowInSlot(epoch);
    assembly {
        flowInAmount := sload(slot)
    }    
}

In the function above the epoch variable was declared and used once. 1080 gas units can be saved during deployment and 13 gas units when the function is called by another contract by not declaring the epoch variable thus the code could be refactored to:

function getFlowInAmount() external view returns (uint256 flowInAmount) {
        uint256 slot = _getFlowInSlot(block.timestamp / EPOCH_TIME);
        assembly {
            flowInAmount := sload(slot)
    }
}
Total gas saved: 3279

[G-04] Caching of global variables

Caching global variables is more expensive than using the actual variable. The code should be refactored to use msg.sender directly instead of caching it.

6 Instances

In the contract above msg.sender was cached in the variable deployer_ this operation cost 12 gas units and it could be avoided. The code should be refactored, the address deployer_ = msg.sender; statement be removed and msg.sender be used directly in all occurence of the deployer_ variable.

In the contract above msg.sender was cached in the variable caller this operation cost 12 gas units and it could be avoided. The code should be refactored, the address caller = msg.sender; statement be removed and msg.sender be used directly in all occurence of the caller variable.

In the contract above msg.sender was cached in the variable sender this operation cost 12 gas units and it could be avoided. The code should be refactored, the address caller = msg.sender; statement be removed and msg.sender be used directly in all occurence of the sender variable.

In the contract above msg.sender was cached in the variable deployer_ this operation cost 12 gas units and it could be avoided. The code should be refactored, the address deployer_ = msg.sender; statement be removed and msg.sender be used directly in all occurence of the deployer_ variable.

In the contract above msg.sender was cached in the variable caller this operation cost 12 gas units and it could be avoided. The code should be refactored, the address caller = msg.sender; statement be removed and msg.sender be used directly in all occurence of the caller variable.

In the contract above msg.sender was cached in the variable sender this operation cost 12 gas units and it could be avoided. The code should be refactored, the address caller = msg.sender; statement be removed and msg.sender be used directly in all occurence of the sender variable.

Total gas saved: 12 * 6 = 72 gas

[G-05] Superflous Events fields

Refactor event to avoid emitting data that is already present in transaction data, block.timestammp does not have to be emitted since block.timestamp are added to the event information by default, so adding them manually will waste additional gas.

1 Instance

The event ProposalExecuted() event declared in the IInterchainGovernance.sol included a timestamp parameter that represented the time the event was emitted. The event declaration should be refactored and the timestamp parameter removed. Then block.timestamp should be removed from this emit statement in the InterchainGovernance contract

Total gas saved:

[G-06] keccak256() should only need to be called on a specific string literal once It should be saved to an immutable variable, and the variable used instead.

1 Instance

In the contract above the pure function contractId() calculates and returns the keccack256 hash of the string "axelar-gateway" this operation cost 193 gas units. The contractId() function is invoked in the external function upgrade(). This means whenever the upgrade() function is invoked the hash of the string "axelar-gateway" would be calculated doing it this way is very gas inefficient. The keccak256 hash should only need to be called on the string "axelar-gateway" once and it should be saved to an immutable variable, and the variable used instead. The code should be refactored and the contractId() function be removed and replaced with bytes32 public immutable CONTRACT_ID = keccak256('axelar-gateway') . The contractId() function invocation in the upgrade() function be replaced with CONTRACT_ID immutable variable

287:   if (AxelarGateway(newImplementation).contractId() != CONTRACT_ID) revert InvalidImplementation();
Total gas saved:

[G-07] Can transfer 0

In Solidity, performing unnecessary operations can consume more gas than needed, leading to cost inefficiencies. For instance, if a transfer function doesn't have a zero amount check and someone calls it with a zero amount, unnecessary gas will be consumed in executing the function, even though the state of the contract remains the same. By implementing a zero amount check, such unnecessary function calls can be avoided, thereby saving gas and making the contract more efficient.

6 Instances

    function expressReceiveToken(
        bytes32 tokenId,
        address destinationAddress,
        uint256 amount,
        bytes32 commandId
    ) external {
        if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);

        address caller = msg.sender;
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
        IERC20 token = IERC20(tokenManager.tokenAddress());

        SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

        _setExpressReceiveToken(tokenId, destinationAddress, amount, commandId, caller);
    }

In the function above you should factor in a check for zero amount before the SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount) statement to eliminate the possibility of performing unnecessary transfer operation that would not change state. The function should be refactored to:

    function expressReceiveToken(
        bytes32 tokenId,
        address destinationAddress,
        uint256 amount,
        bytes32 commandId
    ) external {
        if (amount == 0) revert zeroAmount()
        if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);

        address caller = msg.sender;
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
        IERC20 token = IERC20(tokenManager.tokenAddress());

        SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

        _setExpressReceiveToken(tokenId, destinationAddress, amount, commandId, caller);
    }
    function expressReceiveTokenWithData(
        bytes32 tokenId,
        string memory sourceChain,
        bytes memory sourceAddress,
        address destinationAddress,
        uint256 amount,
        bytes calldata data,
        bytes32 commandId
    ) external {
        if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);

        address caller = msg.sender;
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
        IERC20 token = IERC20(tokenManager.tokenAddress());

        SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

        _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);

        _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
    }

In the function above you should factor in a check for zero amount before the SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount) statement to eliminate the possibility of performing unnecessary transfer operation that would not change state. The function should be refactored to:

    function expressReceiveTokenWithData(
        bytes32 tokenId,
        string memory sourceChain,
        bytes memory sourceAddress,
        address destinationAddress,
        uint256 amount,
        bytes calldata data,
        bytes32 commandId
    ) external {
        if (amount == 0) revert zeroAmount();
        if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);

        address caller = msg.sender;
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
        IERC20 token = IERC20(tokenManager.tokenAddress());

        SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

        _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);

        _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
    }
    function _mintToken(
        string memory symbol,
        address account,
        uint256 amount
    ) internal {
        address tokenAddress = tokenAddresses(symbol);

        if (tokenAddress == address(0)) revert TokenDoesNotExist(symbol);

        _setTokenMintAmount(symbol, tokenMintAmount(symbol) + amount);

        if (_getTokenType(symbol) == TokenType.External) {
            IERC20(tokenAddress).safeTransfer(account, amount);
        } else {
            IBurnableMintableCappedERC20(tokenAddress).mint(account, amount);
        }
    }

In the function above you should factor in a check for zero amount before the IERC20(tokenAddress).safeTransfer(account, amount) statement to eliminate the possibility of performing unnecessary transfer operation that would not change state. The function should be refactored to:

    function _mintToken(
        string memory symbol,
        address account,
        uint256 amount
    ) internal {
        address tokenAddress = tokenAddresses(symbol);

        if (tokenAddress == address(0)) revert TokenDoesNotExist(symbol);

        _setTokenMintAmount(symbol, tokenMintAmount(symbol) + amount);

        if (_getTokenType(symbol) == TokenType.External) {
            if (amount == 0) revert zeroAmount();
            IERC20(tokenAddress).safeTransfer(account, amount);
        } else {
            IBurnableMintableCappedERC20(tokenAddress).mint(account, amount);
        }
    }
    function _takeToken(address from, uint256 amount) internal override returns (uint256) {
        IERC20 token = IERC20(tokenAddress());
        uint256 balance = token.balanceOf(address(this));

        SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), amount);

        // Note: This allows support for fee-on-transfer tokens
        return IERC20(token).balanceOf(address(this)) - balance;
    }

In the function above you should factor in a check for zero amount before the SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), amount) statement to eliminate the possibility of performing unnecessary transfer operation that would not change state. The function should be refactored to:

    function _takeToken(address from, uint256 amount) internal override returns (uint256) {
        if (amount == 0) revert zeroAmount();
        IERC20 token = IERC20(tokenAddress());
        uint256 balance = token.balanceOf(address(this));

        SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), amount);

        // Note: This allows support for fee-on-transfer tokens
        return IERC20(token).balanceOf(address(this)) - balance;
    }
    function _giveToken(address to, uint256 amount) internal override returns (uint256) {
        IERC20 token = IERC20(tokenAddress());
        uint256 balance = IERC20(token).balanceOf(to);

        SafeTokenTransfer.safeTransfer(token, to, amount);

        return IERC20(token).balanceOf(to) - balance;
    }

In the function above you should factor in a check for zero amount before the SafeTokenTransfer.safeTransfer(token, to, amount) statement to eliminate the possibility of performing unnecessary transfer operation that would not change state. The function should be refactored to:

    function _giveToken(address to, uint256 amount) internal override returns (uint256) {
        if (amount == 0) revert zeroAmount();
        IERC20 token = IERC20(tokenAddress());
        uint256 balance = IERC20(token).balanceOf(to);

        SafeTokenTransfer.safeTransfer(token, to, amount);

        return IERC20(token).balanceOf(to) - balance;
    }
    function _takeToken(address from, uint256 amount) internal override returns (uint256) {
        IERC20 token = IERC20(tokenAddress());
        address liquidityPool_ = liquidityPool();
        uint256 balance = token.balanceOf(liquidityPool_);

        SafeTokenTransferFrom.safeTransferFrom(token, from, liquidityPool_, amount);

        // Note: This allows support for fee-on-transfer tokens
        return IERC20(token).balanceOf(liquidityPool_) - balance;
    }

In the function above you should factor in a check for zero amount before the SafeTokenTransferFrom.safeTransferFrom(token, from, liquidityPool_, amount) statement to eliminate the possibility of performing unnecessary transfer operation that would not change state. The function should be refactored to:

    function _takeToken(address from, uint256 amount) internal override returns (uint256) {
        if (amount == 0) revert zeroAmount();
        IERC20 token = IERC20(tokenAddress());
        address liquidityPool_ = liquidityPool();
        uint256 balance = token.balanceOf(liquidityPool_);

        SafeTokenTransferFrom.safeTransferFrom(token, from, liquidityPool_, amount);

        // Note: This allows support for fee-on-transfer tokens
        return IERC20(token).balanceOf(liquidityPool_) - balance;
    }
Total gas saved:

[G-08] Move lesser gas costing reverts checks to the top

Revert() statements that check input arguments or cost lesser gas should be at the top of the function. Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting alot of gas in a function that may ultimately revert in the unhappy case.

4 Instances

    function _rotateSigners(address[] memory newAccounts, uint256 newThreshold) internal {
        uint256 length = signers.accounts.length;

        // Clean up old signers.
        for (uint256 i; i < length; ++i) {
            signers.isSigner[signers.accounts[i]] = false;
        }

        length = newAccounts.length;

        if (newThreshold > length) revert InvalidSigners();

        if (newThreshold == 0) revert InvalidSignerThreshold();

        ++signerEpoch;

        signers.accounts = newAccounts;
        signers.threshold = newThreshold;

        for (uint256 i; i < length; ++i) {
            address account = newAccounts[i];

            // Check that the account wasn't already set as a signer for this epoch.
            if (signers.isSigner[account]) revert DuplicateSigner(account);
            if (account == address(0)) revert InvalidSigners();

            signers.isSigner[account] = true;
        }

        emit SignersRotated(newAccounts, newThreshold);
    }

In the function above the if (newThreshold == 0) revert InvalidSignerThreshold() statement should be moved to the top of the function since this statement checks if the newThreshold argument is equal to 0, so that in scenarios that newThreshold equals to 0 the function could revert without performing gas consuming operations. The code should be refactored to:

    function _rotateSigners(address[] memory newAccounts, uint256 newThreshold) internal {
        if (newThreshold == 0) revert InvalidSignerThreshold();
        uint256 length = signers.accounts.length;

        // Clean up old signers.
        for (uint256 i; i < length; ++i) {
            signers.isSigner[signers.accounts[i]] = false;
        }

        length = newAccounts.length;

        if (newThreshold > length) revert InvalidSigners();

        ++signerEpoch;

        signers.accounts = newAccounts;
        signers.threshold = newThreshold;

        for (uint256 i; i < length; ++i) {
            address account = newAccounts[i];

            // Check that the account wasn't already set as a signer for this epoch.
            if (signers.isSigner[account]) revert DuplicateSigner(account);
            if (account == address(0)) revert InvalidSigners();

            signers.isSigner[account] = true;
        }

        emit SignersRotated(newAccounts, newThreshold);
    }
    function _burnTokenFrom(
        address sender,
        string memory symbol,
        uint256 amount
    ) internal {
        address tokenAddress = tokenAddresses(symbol);

        if (tokenAddress == address(0)) revert TokenDoesNotExist(symbol);
        if (amount == 0) revert InvalidAmount();

        TokenType tokenType = _getTokenType(symbol);

        if (tokenType == TokenType.External) {
            IERC20(tokenAddress).safeTransferFrom(sender, address(this), amount);
        } else if (tokenType == TokenType.InternalBurnableFrom) {
            IERC20(tokenAddress).safeCall(abi.encodeWithSelector(IBurnableMintableCappedERC20.burnFrom.selector, sender, amount));
        } else {
            IERC20(tokenAddress).safeTransferFrom(sender, IBurnableMintableCappedERC20(tokenAddress).depositAddress(bytes32(0)), amount);
            IBurnableMintableCappedERC20(tokenAddress).burn(bytes32(0));
        }
    }

In the function above the if (amount == 0) revert InvalidAmount() statement should be moved to the top of the function since this statement checks if the amount argument is equal to 0, so that in scenarios that amount equals to 0 the function could revert without performing gas consuming operations. The code should be refactored to:

    function _burnTokenFrom(
        address sender,
        string memory symbol,
        uint256 amount
    ) internal {
        if (amount == 0) revert InvalidAmount();
        address tokenAddress = tokenAddresses(symbol);

        if (tokenAddress == address(0)) revert TokenDoesNotExist(symbol);

        TokenType tokenType = _getTokenType(symbol);

        if (tokenType == TokenType.External) {
            IERC20(tokenAddress).safeTransferFrom(sender, address(this), amount);
        } else if (tokenType == TokenType.InternalBurnableFrom) {
            IERC20(tokenAddress).safeCall(abi.encodeWithSelector(IBurnableMintableCappedERC20.burnFrom.selector, sender, amount));
        } else {
            IERC20(tokenAddress).safeTransferFrom(sender, IBurnableMintableCappedERC20(tokenAddress).depositAddress(bytes32(0)), amount);
            IBurnableMintableCappedERC20(tokenAddress).burn(bytes32(0));
        }
    }
    function deploy(bytes32 salt, bytes memory bytecode) internal returns (address deployed) {
        deployed = deployedAddress(address(this), salt);

        if (deployed.isContract()) revert AlreadyDeployed();
        if (bytecode.length == 0) revert EmptyBytecode();

        // Deploy using create2
        CreateDeployer deployer = new CreateDeployer{ salt: salt }();

        if (address(deployer) == address(0)) revert DeployFailed();

        deployer.deploy(bytecode);
    }

In the function above the if (bytecode.length == 0) revert EmptyBytecode() statement should be moved to the top of the function since this statement checks if the length of the bytecode argument is equal to 0, so that in scenarios that bytecode length equals 0 the function could revert without performing gas consuming operations. The function should be refactored to:

    function deploy(bytes32 salt, bytes memory bytecode) internal returns (address deployed) {
        if (bytecode.length == 0) revert EmptyBytecode();
        deployed = deployedAddress(address(this), salt);

        if (deployed.isContract()) revert AlreadyDeployed();

        // Deploy using create2
        CreateDeployer deployer = new CreateDeployer{ salt: salt }();

        if (address(deployer) == address(0)) revert DeployFailed();

        deployer.deploy(bytecode);
    }
    function upgrade(
        address newImplementation,
        bytes32 newImplementationCodeHash,
        bytes calldata params
    ) external override onlyOwner {
        if (IUpgradable(newImplementation).contractId() != IUpgradable(this).contractId()) revert InvalidImplementation();
        if (newImplementationCodeHash != newImplementation.codehash) revert InvalidCodeHash();

        if (params.length > 0) {
            (bool success, ) = newImplementation.delegatecall(abi.encodeWithSelector(this.setup.selector, params));

            if (!success) revert SetupFailed();
        }

        emit Upgraded(newImplementation);

        assembly {
            sstore(_IMPLEMENTATION_SLOT, newImplementation)
        }
    }

In the function above the if (newImplementationCodeHash != newImplementation.codehash) revert InvalidCodeHash() statement should be moved to the top of the function. The execution of this statement would consume lesser gas than the if (IUpgradable(newImplementation).contractId() != IUpgradable(this).contractId()) revert InvalidImplementation() statement. In scenarios that if (newImplementationCodeHash != newImplementation.codehash) results to true the function would revert without the EVM executing the if (IUpgradable(newImplementation).contractId() != IUpgradable(this).contractId()) revert InvalidImplementation() statement which would consume a lot of gas. The function should be refactored to:

    function upgrade(
        address newImplementation,
        bytes32 newImplementationCodeHash,
        bytes calldata params
    ) external override onlyOwner {
        if (IUpgradable(newImplementation).contractId() != IUpgradable(this).contractId()) revert InvalidImplementation();
        if (newImplementationCodeHash != newImplementation.codehash) revert InvalidCodeHash();

        if (params.length > 0) {
            (bool success, ) = newImplementation.delegatecall(abi.encodeWithSelector(this.setup.selector, params));

            if (!success) revert SetupFailed();
        }

        emit Upgraded(newImplementation);

        assembly {
            sstore(_IMPLEMENTATION_SLOT, newImplementation)
        }
    }
Total Gas saved:

[G-09] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier or require such as onlyOwner/onlyX 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. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost. the recommended Mitigation Steps is that Functions guaranteed to revert when called by normal users can be marked payable.

17 Instances

    function rotateSigners(address[] memory newAccounts, uint256 newThreshold) external virtual onlySigners {
        
    }

21 gas units can be saved if the function definition is refactored to:

    function rotateSigners(address[] memory newAccounts, uint256 newThreshold) external virtual payable onlySigners {
        
    }
    function deployToken(bytes calldata params, bytes32) external onlySelf {

    }

21 gas units can be saved if the function definition is refactored to:

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

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

21 gas units can be saved if the function definition is refactored to:

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

    }

21 gas units can be saved if the function definition is refactored to:

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

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

    }

21 gas units can be saved if the function definition is refactored to:

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

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

    }

21 gas units can be saved if the function definition is refactored to:

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

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

    }

21 gas units can be saved if the function definition is refactored to:

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

    }
    function setPaused(bool paused) external onlyOwner {

    }

21 gas units can be saved if the function definition is refactored to:

    function setPaused(bool paused) external payable onlyOwner {

    }
    function addTrustedAddress(string memory chain, string memory addr) public onlyOwner {

    }

21 gas units can be saved if the function definition is refactored to:

    function addTrustedAddress(string memory chain, string memory addr) public onlyOwner {

    }
    function removeTrustedAddress(string calldata chain) external onlyOwner {

    }

21 gas units can be saved if the function definition is refactored to:

    function removeTrustedAddress(string calldata chain) external payable onlyOwner {

    }
    function addGatewaySupportedChains(string[] calldata chainNames) external onlyOwner {

    }

21 gas units can be saved if the function definition is refactored to:

    function addGatewaySupportedChains(string[] calldata chainNames) external payable onlyOwner {

    }
    function removeGatewaySupportedChains(string[] calldata chainNames) external onlyOwner {

    }

21 gas units can be saved if the function definition is refactored to:

    function removeGatewaySupportedChains(string[] calldata chainNames) external payable onlyOwner {

    }
    function addTrustedAddress(string memory chain, string memory addr) public onlyOwner {

    }

21 gas units can be saved if the function definition is refactored to:

    function addTrustedAddress(string memory chain, string memory addr) public payable onlyOwner {

    }
    function setFlowLimit(uint256 flowLimit) external onlyOperator {

    }

21 gas units can be saved if the function definition is refactored to:

    function setFlowLimit(uint256 flowLimit) external payable onlyOperator {

    }
    function setLiquidityPool(address newLiquidityPool) external onlyOperator {

    }

21 gas units can be saved if the function definition is refactored to:

    function setLiquidityPool(address newLiquidityPool) external payable onlyOperator {

    }
    function setDistributor(address distr) external onlyDistributor {

    }

21 gas units can be saved if the function definition is refactored to:

    function setDistributor(address distr) external payable onlyDistributor {

    }
    function setOperator(address operator_) external onlyOperator {

    }

21 gas units can be saved if the function definition is refactored to:

    function setOperator(address operator_) external payable onlyOperator {

    }
Total gas saved: 17 * 21 = 357 gas units

[G-10] Using calldata for read-only arguments in external functions

Using calldata instead of memory for read-only arguments in external functions saves gas.

18 Instances

#0 - c4-judge

2023-09-04T11:11:42Z

berndartmueller marked the issue as grade-b

#1 - deanamiel

2023-09-06T23:22:27Z

Implemented suggestions from issue G-01. See PR here

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter