Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #126

Open
code423n4 opened this issue Aug 3, 2022 · 2 comments
Open

Gas Optimizations #126

code423n4 opened this issue Aug 3, 2022 · 2 comments
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

[G-01] REVERT CHECK ON INPUT CAN BE PLACED AT START OF FUNCTION BODY

When a revert check on the input is placed at the start of the function body, the subsequent operations that cost more gas are prevented from running if it does revert.

For the following code, if (gatewayImplementation.code.length == 0) revert InvalidImplementation(); can be placed above _setAddress(KEY_IMPLEMENTATION, gatewayImplementation);.

contracts\AxelarGatewayProxy.sol
  16-24:
    constructor(address gatewayImplementation, bytes memory params) {
        _setAddress(KEY_IMPLEMENTATION, gatewayImplementation);

        if (gatewayImplementation.code.length == 0) revert InvalidImplementation();

        (bool success, ) = gatewayImplementation.delegatecall(abi.encodeWithSelector(IAxelarGateway.setup.selector, params));

        if (!success) revert SetupFailed();
    }

[G-02] VARIABLE DOES NOT NEED TO BE INITIALIZED TO ITS DEFAULT VALUE

Explicitly initializing a variable with its default value costs more gas than uninitializing it. For example, uint256 i can be used instead of uint256 i = 0 in the following code.

contracts\AxelarGateway.sol
  207: for (uint256 i = 0; i < symbols.length; i++) {

contracts\auth\AxelarAuthWeighted.sol
  69: for (uint256 i = 0; i < weightsLength; ++i) {
  98: for (uint256 i = 0; i < signatures.length; ++i) {

[G-03] ARRAY LENGTH CAN BE CACHED OUTSIDE OF LOOP

Caching the array length outside of the loop and using the cached length in the loop costs less gas than reading the array length for each iteration. For example, symbols.length in the following code can be cached outside of the loop like uint256 symbolsLength = symbols.length, and i < symbolsLength can be used for each iteration.

contracts\AxelarGateway.sol
  207: for (uint256 i = 0; i < symbols.length; i++) {

contracts\auth\AxelarAuthWeighted.sol
  17: for (uint256 i; i < recentOperators.length; ++i) {
  98: for (uint256 i = 0; i < signatures.length; ++i) {
  116: for (uint256 i; i < accounts.length - 1; ++i) {

contracts\deposit-service\AxelarDepositService.sol
  114: for (uint256 i; i < refundTokens.length; i++) {
  168: for (uint256 i; i < refundTokens.length; i++) {
  204: for (uint256 i; i < refundTokens.length; i++) {

contracts\gas-service\AxelarGasService.sol
  123: for (uint256 i; i < tokens.length; i++) {

[G-04] ++VARIABLE CAN BE USED INSTEAD OF VARIABLE++

++variable costs less gas than variable++. For example, i++ can be changed to ++i in the following code.

contracts\AxelarGateway.sol
  207: for (uint256 i = 0; i < symbols.length; i++) {

contracts\deposit-service\AxelarDepositService.sol
  114: for (uint256 i; i < refundTokens.length; i++) {
  168: for (uint256 i; i < refundTokens.length; i++) {
  204: for (uint256 i; i < refundTokens.length; i++) {

contracts\gas-service\AxelarGasService.sol
  123: for (uint256 i; i < tokens.length; i++) {

[G-05] X = X + Y CAN BE USED INSTEAD OF X += Y

x = x + y costs less gas than x += y. For example, totalWeight += newWeights[i] can be changed to totalWeight = totalWeight + newWeights[i] in the following code.

contracts\auth\AxelarAuthWeighted.sol
  70: totalWeight += newWeights[i];
  105: weight += weights[operatorIndex];

[G-06] ARITHMETIC OPERATIONS THAT DO NOT OVERFLOW CAN BE UNCHECKED

Explicitly unchecking arithmetic operations that do not overflow by wrapping these in unchecked {} costs less gas than implicitly checking these.

For the following loops, if increasing the counter variable is very unlikely to overflow, then unchecked {++i} at the end of the loop block can be used, where i is the counter variable.

contracts\AxelarGateway.sol
  195: for (uint256 i; i < adminCount; ++i) {
  207: for (uint256 i = 0; i < symbols.length; i++) {
  292: for (uint256 i; i < commandsLength; ++i) {

contracts\auth\AxelarAuthWeighted.sol
  17: for (uint256 i; i < recentOperators.length; ++i) {
  69: for (uint256 i = 0; i < weightsLength; ++i) {
  98: for (uint256 i = 0; i < signatures.length; ++i) {
  101: for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}
  116: for (uint256 i; i < accounts.length - 1; ++i) {

contracts\deposit-service\AxelarDepositService.sol
  114: for (uint256 i; i < refundTokens.length; i++) {
  168: for (uint256 i; i < refundTokens.length; i++) {
  204: for (uint256 i; i < refundTokens.length; i++) {

contracts\gas-service\AxelarGasService.sol
  123: for (uint256 i; i < tokens.length; i++) {

[G-07] REVERT WITH CUSTOM ERROR CAN BE USED INSTEAD OF REVERT() WITH REASON STRING

revert with custom error can cost less gas than revert() with reason string. Please consider using revert with custom error to replace the following revert().

xc20\contracts\XC20Wrapper.sol
  55: if (axelarToken == address(0)) revert('NotAxelarToken()');
  56: if (xc20Token.codehash != xc20Codehash) revert('NotXc20Token()');
  57: if (wrapped[axelarToken] != address(0)) revert('AlreadyWrappingAxelarToken()');
  58: if (unwrapped[xc20Token] != address(0)) revert('AlreadyWrappingXC20Token()');
  61: if (!LocalAsset(xc20Token).set_team(address(this), address(this), address(this))) revert('NotOwner()');
  62: if (!LocalAsset(xc20Token).set_metadata(newName, newSymbol, IERC20(axelarToken).decimals())) revert('CannotSetMetadata()');
  68: if (axelarToken == address(0)) revert('NotAxelarToken()');
  70: if (xc20Token == address(0)) revert('NotWrappingToken()');
  78: if (wrappedToken == address(0)) revert('NotAxelarToken()');
  79: if (!LocalAsset(wrappedToken).mint(msg.sender, amount)) revert('CannotMint()');
  84: if (axelarToken == address(0)) revert('NotXc20Token()');
  85: if (IERC20(wrappedToken).balanceOf(msg.sender) < amount) revert('InsufficientBalance()');
  86: if (!LocalAsset(wrappedToken).burn(msg.sender, amount)) revert('CannotBurn()');
  98: if (!transferred || tokenAddress.code.length == 0) revert('TransferFailed()');
  111: if (!transferred || tokenAddress.code.length == 0) revert('TransferFailed()');
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 3, 2022
code423n4 added a commit that referenced this issue Aug 3, 2022
@re1ro
Copy link
Member

re1ro commented Aug 5, 2022

1

Ack

2 - 6

Dup #2

7

Dup #13 (8)

@GalloDaSballo
Copy link
Collaborator

Around 300 gas saved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

3 participants