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 #52

Open
code423n4 opened this issue Mar 9, 2022 · 1 comment
Open

Gas Optimizations #52

code423n4 opened this issue Mar 9, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

[S]: Suggested optimation, save a decent amount of gas without compromising readability;

[M]: Minor optimation, the amount of gas saved is minor, change when you see fit;

[N]: Non-preferred, the amount of gas saved is at cost of readability, only apply when gas saving is a top priority.

[S] parseIncomingTokenTransferInfo can be optimized

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L269-L296

    function parseIncomingTokenTransferInfo(bytes memory encoded)
        public
        pure
        returns (IncomingTokenTransferInfo memory incomingTokenTransferInfo)
    {
        uint256 index = 0;

        incomingTokenTransferInfo.chainId = encoded.toUint16(index);
        index += 2;

        incomingTokenTransferInfo.tokenRecipientAddress = encoded.toBytes32(
            index
        );
        index += 32;

        incomingTokenTransferInfo.tokenTransferSequence = encoded.toUint64(
            index
        );
        index += 8;

        incomingTokenTransferInfo.instructionSequence = encoded.toUint64(index);
        index += 8;

        require(
            encoded.length == index,
            "invalid IncomingTokenTransferInfo encoded length"
        );
    }
  • In the check of require(encoded.length == index), the variable index can be replaced with constant value to avoid mload.
  • Check can be done earlier to save gas for revert txs.
  • Avoiding one arithmetic operation (index += 8 at L290).

Recommandation

Change to:

function parseIncomingTokenTransferInfo(bytes memory encoded)
    public
    pure
    returns (IncomingTokenTransferInfo memory incomingTokenTransferInfo)
{
    require(
        encoded.length == 50,
        "invalid IncomingTokenTransferInfo encoded length"
    );

    uint256 index;

    incomingTokenTransferInfo.chainId = encoded.toUint16(index);
    index += 2;

    incomingTokenTransferInfo.tokenRecipientAddress = encoded.toBytes32(
        index
    );
    index += 32;

    incomingTokenTransferInfo.tokenTransferSequence = encoded.toUint64(
        index
    );
    index += 8;

    incomingTokenTransferInfo.instructionSequence = encoded.toUint64(index);
}

[M] Setting uint variables to 0 is redundant

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L149-L149

uint8 i = 0;

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L274-L274

uint256 index = 0;

Setting uint8, uint256 variables to 0 is redundant as they default to 0.

[S] Cache array length in for loops can save gas

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Instances include:

        for (uint8 i = 0; i < _collateralTokens.length; i++) 

[M] ++i is more efficient than i++

Using ++i is more gas efficient than i++, especially in for loops.

For example:

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L149-L149

        for (uint8 i = 0; i < _collateralTokens.length; i++) 

Change to:

        for (uint8 i = 0; i < _collateralTokens.length; ++i) 

[M] Use short reason strings can save gas

Every reason string takes at least 32 bytes.

Use short reason strings that fits in 32 bytes or it will become more expensive.

Instances include:

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L292-L295

        require(
            encoded.length == index,
            "invalid IncomingTokenTransferInfo encoded length"
        );

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L327-L331

        require(
            incomingTokenTransferInfoVM.emitterAddress ==
                TERRA_ANCHOR_BRIDGE_ADDRESS,
            "message does not come from terra anchor bridge"
        );

[M] Unused constant variable

Unused constant variables in contracts increase contract size and gas usage at deployment.

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L104-L104

    uint8 private constant FLAG_NO_ASSC_TRANSFER = 0x00; // 0000 0000

https://github.com/code-423n4/2022-02-anchor/blob/7af353e3234837979a19ddc8093dc9ad3c63ab6b/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L115-L115

    uint8 private constant OP_CODE_CLAIM_REWARDS = 2 | FLAG_OUTGOING_TRANSFER;

In the contract, the constant variable FLAG_NO_ASSC_TRANSFER``OP_CODE_CLAIM_REWARDS are set once but have never been read, therefore it can be removed.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Mar 9, 2022
code423n4 added a commit that referenced this issue Mar 9, 2022
@GalloDaSballo
Copy link
Collaborator

Would save less than 100 gas
Report formatting is great

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

2 participants