Gas Optimizations

[code423n4]

1. Loops can be more efficient

Impact

The local variable used as for loop index need not be initialized to 0 because the default value is 0. Avoiding this anti-pattern can save a few opcodes and therefore a tiny bit of gas.

Proof of Concept

function settleFunding() override external whenNotPaused {
        for (uint i = 0; i < amms.length; i++) {
            amms[i].settleFunding();
        }
    }

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L130

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L170

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L251

The same situation are in other scope contracts where loops use.

Remix

Recommended Mitigation Steps

Remove explicit 0 initialization of for loop index variable.

2. Cache array length in for loops can save gas

Vulnerability details

Impact

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.

Proof of Concept

function settleFunding() override external whenNotPaused {
        for (uint i = 0; i < amms.length; i++) {
            amms[i].settleFunding();
        }
    }

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L130

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L170

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L251

The same situation are in other scope contracts where loops use.

Tools

Manual

Recommended Mitigation Steps

Caching len = amms.length and using the len instead will save gas.

3. Prefix increments are cheaper than postfix increments (i++)

Impact

There is no risk of overflow caused by increamenting the iteration index in for loops.
Increments perform overflow checks that are not necessary in this case.

Proof of Concept

function settleFunding() override external whenNotPaused {
        for (uint i = 0; i < amms.length; i++) {
            amms[i].settleFunding();
        }
    }

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L130

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L170

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L251

The same situation are in other scope contracts where loops use.

Tools

Remix

Recommended Mitigation Steps

Surround the increment expressions with an unchecked { ... } block to avoid the default overflow checks

4. Long Revert Strings

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L101

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L354

There are several other places throughout the codebase where the same optimization can be used.

Tools

https://planetcalc.com/9029/

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

5. > 0 can be replaced with != 0 for gas optimisation

Vulnerability details

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L574
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L211

There are several other places throughout the codebase where the same optimization can be used.

Remix

Recommended Mitigation Steps

6. Change string to byteX if possible

Vulnerability details

Impact

In the AMM.sol, declaring the type bytes32 can save gas.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L28

https://medium.com/layerx/how-to-reduce-gas-cost-in-solidity-f2e5321e0395#2a78

Recommended Mitigation Steps

7. Struct layout in AMM.sol

Impact

ReserveSnapshot struct in AMM.sol can be optimized to reduce 2 storage slot

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L61-L62

struct ReserveSnapshot {
        uint256 lastPrice;
        uint256 timestamp;
        uint256 blockNumber;
    }

timestamp and blockNumber store block numbers, and 2^128 is be enough for a very long time.

Tools

https://docs.soliditylang.org/en/v0.8.0/internals/layout_in_storage.html?highlight=Structs#layout-of-state-variables-in-storage

Recommended Mitigation Steps

Change the struct as suggested above

struct ReserveSnapshot {
        uint256 lastPrice;
        uint128 timestamp;
        uint128 blockNumber;
    }

8. Adding unchecked directive can save gas

Impact

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L726
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L212
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L175-L177

require(margin[idx][trader] >= amount.toInt256(), "Insufficient balance");
margin[idx][trader] -= amount.toInt256();

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/Oracle.sol#L73
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/Oracle.sol#L81

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L370

uint256 minNextValidFundingTime = _blockTimestamp() + fundingBufferPeriod;

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so.

9. Caching variables

Impact

Some of the variable can be cached to slightly reduce gas usage.

Proof of Concept

vamm can be cached.
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L188-L195

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L486-L501

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L510-L525

_blockTimestamp() can be cahed

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L656-L704

vusd can be cashed

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L572-L579

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L581-L593

Tools

Remix

Recommended Mitigation Steps

Consider caching those variable for read and make sure write back to storage

10. Placement of require statements

Impact

The require statement in the function initialize() can be placed earlier to reduce gas usage on revert.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/ClearingHouse.sol#L51

function initialize(
        address _governance,
        address _insuranceFund,
        address _marginAccount,
        address _vusd,
        int256 _maintenanceMargin,
        int256 _minAllowableMargin,
        uint _tradeFee,
        uint _liquidationPenalty
    ) external initializer {
        _setGovernace(_governance);

        insuranceFund = IInsuranceFund(_insuranceFund);
        marginAccount = IMarginAccount(_marginAccount);
        vusd = VUSD(_vusd);

        require(_maintenanceMargin > 0, "_maintenanceMargin < 0");
        maintenanceMargin = _maintenanceMargin;
        minAllowableMargin = _minAllowableMargin;
        tradeFee = _tradeFee;
        liquidationPenalty = _liquidationPenalty;
    }

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/Oracle.sol#L48

 function getUnderlyingTwapPrice(address underlying, uint256 intervalInSeconds)
        virtual
        public
        view
        returns (int256)
    {
        if (stablePrice[underlying] != 0) {
            return stablePrice[underlying];
        }
        AggregatorV3Interface aggregator = AggregatorV3Interface(chainLinkAggregatorMap[underlying]);
        requireNonEmptyAddress(address(aggregator));
        require(intervalInSeconds != 0, "interval can't be 0");

Tools

Remix

Recommended Mitigation Steps

Relocate the said require statement

11. Constant is being assigned its default value.

Impact

The constant variable is being assigned its default value which is unnecessary.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L31

uint constant VUSD_IDX = 0;

Recommended Mitigation Steps

Remove the assignment.

12. Checking non-zero value can avoid an external call to save gas

Checking if _amount > 0 before making the external call to vusd.safeTransferFrom() can save 2600 gas by avoiding the external call in such situations.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/InsuranceFund.sol#L51

Recommended Mitigation Steps

Add check

13. function decimals() can be refactored to a constant variable

Impact

function decimals() just returns a constant of uint8(6). To save some gas and improve the readability this can be extracted to a constant variable and used where necessary.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/VUSD.sol#L69

function decimals() public pure override returns (uint8) {
        return 6;
    }

Recommended Mitigation Steps

14. Avoid use of state variables in event emissions to save gas

Impact

Where possible, use equivalent function parameters or local variables in event emits instead of state variables to prevent expensive SLOADs. Post-Berlin, SLOADs on state variables accessed first-time in a transaction increased from 800 gas to 2100, which is a 2.5x increase.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L205
https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L381

function realizePnL(address trader, int256 realizedPnl)
        override    external       onlyClearingHouse    {
           if (realizedPnl != 0) {
            margin[VUSD_IDX][trader] += realizedPnl;
            emit PnLRealized(trader, realizedPnl, _blockTimestamp());
        }

Recommended Mitigation Steps

Use equivalent function parameters or local variables in event emits instead of state variables.

15. Unnecessary indirection to access block.timestamp value

Impact

_blockTimestamp() returns the block.timestamp value in AMM contract. This internal call only to get value of block.timestamp seems unnecessary because there isn’t any other way of getting current time on the blockchain which justifies moving this to a separate function for modularity.
Adds an additional jump and other supporting bytecode of making the internal call which increase gas usage unnecessarily.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L652

function _blockTimestamp() internal view virtual returns (uint256) {
        return block.timestamp;
    }

Recommended Mitigation Steps
Use block.timestamp directly to save a little gas by avoiding this unnecessary indirection.

16. Consider making some constants as non-public to save gas

Impact

Each function part of contract's external interface is part of the function dispatch, i.e., every time a contract is called, it goes through a switch statement (a set of eq ... JUMPI blocks in EVM)
matching the selector of each externally available functions with the chosen function selector (the first 4 bytes of calldata).
This means that any unnecessary function that is part of contract's external interface will lead to more gas for (almost) every single function calls to the contract. There are several cases where constants were made public. This is unnecessary; the constants can simply be readfrom the verified contract, i.e., it is unnecessary to expose it with a public function.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L15-L17

17. Gas Optimization on the Public Function

Impact

This does not directly impact the smart contract in anyway besides cost. This is a gas optimization to reduce cost of smart contract.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/VUSD.sol#L69

Recommended Mitigation Steps

The function decimals() public could be set external instead of public.

18. Use Custom Errors to save Gas

Impact

Custom errors from Solidity 0.8.4 are cheaper than revert strings.

Proof of Concept

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:
Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.
Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Recommended Mitigation Steps

Replace revert strings with custom errors.

19. && operator can use more gas

Impact

More expensive gas usage

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L461

require(idx > VUSD_IDX && idx < supportedCollateral.length, "collateral not seizable");

Tools Used

Recommended Mitigation Steps

Instead of using operator && on single require check using double require check can save more gas

20. Use 10 ** DECIMALS for constant

Impact

More expensive gas usage

Proof of Concept

uint8 constant DECIMALS = 6;
uint constant PRECISION = 10 ** DECIMALS;

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/InsuranceFund.sol#L17

Recommended Mitigation Steps

Change to:

uint constant PRECISION = 1e6;

[atvanguard]

Good report.

[moose-code]

Like the attention to struct packing. A uint32 timestamp lasts till 2106 and allows for some nice packing