code-423n4 / 2022-02-anchor-findings

0 stars 0 forks source link

Gas Optimizations #52

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[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"
        );
    }

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.

GalloDaSballo commented 2 years ago

Would save less than 100 gas Report formatting is great