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

for (uint256 index = 0; index < tokenConfig.length; ++index) {

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/token/TokenManager.sol#L78

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. 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-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/token/LPToken.sol#L70

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.

3. > 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-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L132
https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityProviders.sol#L182

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

4. Change string to byteX if possible

Vulnerability details

Impact

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

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L154

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

5. Struct layout in LiquidityFarming.sol

Impact

NFTInfo struct i can be optimized to reduce 1 storage slot

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L29-L34

struct NFTInfo {
        address payable staker;
        uint256 rewardDebt;
        uint256 unpaidRewards;
        bool isStaked;
    }

Moving the bool isStaked close to address payable staker it's possible to save one slot.

struct NFTInfo {
        address payable staker;
        bool isStaked;
        uint256 rewardDebt;
        uint256 unpaidRewards;
    }

Tools

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

6. 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-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L178-L179

if (currentLiquidity < providedLiquidity) {
            uint256 liquidityDifference = providedLiquidity - currentLiquidity;

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/token/LPToken.sol#L84

uint256 tokenId = totalSupply() + 1;

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L269

uint256 i = rewardRateLog[_baseToken].length - 1;

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/WhitelistPeriodManager.sol#L92-L98

 // Per Token Total Cap or PTTC
        require(ifEnabled(totalLiquidity[_token] + _amount <= perTokenTotalCap[_token]), "ERR__LIQUIDITY_EXCEEDS_PTTC");
        require(
            ifEnabled(totalLiquidityByLp[_token][_lp] + _amount <= perTokenWalletCap[_token]),
            "ERR__LIQUIDITY_EXCEEDS_PTWC"
        );
        totalLiquidity[_token] += _amount;
        totalLiquidityByLp[_token][_lp] += _amount;

Recommended Mitigation Steps

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

7. Caching variables

Impact

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

Proof of Concept

lpToken can be cached.
https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L196-L213

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L246-L250

tokenManager can be cached.
https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L308-L340

Tools

Remix

Recommended Mitigation Steps

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

8. count 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-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/token/svg-helpers/SvgHelperBase.sol#L29

uint256 count = 0;

Recommended Mitigation Steps

Remove the assignment.

9. Use Custom Errors to save Gas

Impact

Custom errors 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.

10. && operator can use more gas

Impact

More expensive gas usage

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L156-L160

 require(
            tokenManager.getDepositConfig(toChainId, tokenAddress).min <= amount &&
                tokenManager.getDepositConfig(toChainId, tokenAddress).max >= amount,
            "Deposit amount not in Cap limit"
        );

Recommended Mitigation Steps

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

11. Use 10 ** 18 for constant

Impact

More expensive gas usage

Proof of Concept

uint256 public constant BASE_DIVISOR = 10**18;

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityProviders.sol#L27

Recommended Mitigation Steps

Change to:

uint256 public constant BASE_DIVISOR = 1e18;

12. Upgrade pragma to at least 0.8.4 если ^0.8.0

Impact

Using newer compiler versions and the optimizer gives gas optimizations
and additional safety checks are available for free.
The advantages of versions 0.8.* over <0.8.0 are:
• Safemath by default from 0.8.0 (can be more gas efficient than
library based safemath.)
• Low level inliner : from 0.8.2, leads to cheaper runtime gas. Especially relevant when the contract has small functions. For example, OpenZeppelin libraries typically have a lot of small helper functions and if they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.
• Optimizer improvements in packed structs: Before 0.8.3, storing packed structs, in some cases used an
additional storage read operation. After EIP-2929, if the slot was already cold, this means unnecessary stack operations and extra deploy time costs. However, if the slot was already warm, this means
additional cost of 100 gas alongside the same unnecessary stack operations and extra deploy time costs.
• Custom errors from 0.8.4, leads to cheaper deploy time cost and run time cost. Note: the run time cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.
code-423n4/2021-10-mochi-findings#34

Recommended Mitigation Steps

Consider to upgrade pragma to at least 0.8.4.

13.Less than 256 uints are not gas efficient

Impact

Lower than uint256 size storage instance variables are actually less gas efficient. E.g. using uint128 does not give any efficiency, actually, it is the opposite as EVM operates on default of 256-bit values so uint128 is more expensive in this case as it needs a conversion. It only gives improvements in cases where you can pack variables together, e.g. structs.

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L119

function setBaseGas(uint128 gas) external onlyOwner {
        baseGas = gas;
    }

Recommended Mitigation Steps

Consider to review all uint types. Change them with uint256 If the integer is not necessary to present with uint128.

14.Changing bool to uint256 can save gas

Vulnerability details

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.
See: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/ReentrancyGuard.sol#L23-L27

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L37

mapping(bytes32 => bool) public processedHash;

15. Mappings are expensive. All PID related variables could be extracted to a struct

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityProviders.sol#L42-L46

// LP Fee Distribution
    mapping(address => uint256) public totalReserve; // Include Liquidity + Fee accumulated
    mapping(address => uint256) public totalLiquidity; // Include Liquidity only
    mapping(address => uint256) public currentLiquidity; // Include current liquidity, updated on every in and out transfer
    mapping(address => uint256) public totalLPFees;
    mapping(address => uint256) public totalSharesMinted;

16. Placement of require statements

Impact

The require statements in LiquidityPool.sol can be placed earlier to reduce gas usage.
https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L271

...
 uint256 initialGas = gasleft();
        require(
            tokenManager.getTransferConfig(tokenAddress).min <= amount &&
                tokenManager.getTransferConfig(tokenAddress).max >= amount,
            "Withdraw amnt not in Cap limits"
        );
        require(receiver != address(0), "Bad receiver address");
...

Recommended Mitigation Steps

Relocate the said require statement

17. Useless imports

Impact

Contract SvgHelperBase.sol not use the import base64.sol library.

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/token/svg-helpers/SvgHelperBase.sol#L6

Recommended Mitigation Steps

Consider reviewing all the unused imports and removing them to reduce the size of the contract and thus save some deployment gas.

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

Impact

Checking if _amont > 0 before making the external call can save gas.
https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L109-L117

function _sendErc20AndGetSentAmount(
        IERC20Upgradeable _token,
        uint256 _amount,
        address _to
    ) private returns (uint256) {
        uint256 recepientBalance = _token.balanceOf(_to);
        _token.safeTransfer(_to, _amount);
        return _token.balanceOf(_to) - recepientBalance;
    }

19. 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

The LogUpdatePool event in the updatePool() uses state variable totalSharesStaked[_baseToken] instead of using the caching variable.

 function updatePool(address _baseToken) public whenNotPaused returns (PoolInfo memory pool) {
        pool = poolInfo[_baseToken];
        if (block.timestamp > pool.lastRewardTime) {
            if (totalSharesStaked[_baseToken] > 0) {
                pool.accTokenPerShare = getUpdatedAccTokenPerShare(_baseToken);
            }
            pool.lastRewardTime = block.timestamp;
            poolInfo[_baseToken] = pool;
            emit LogUpdatePool(_baseToken, pool.lastRewardTime, totalSharesStaked[_baseToken], pool.accTokenPerShare);
        }
    }

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L323

20. Gas: removeToken iteration over all tokens can be avoided

Vulnerability details

The WhitelistPeriodManager.setIsExcludedAddressStatus function iterates over all addresses to check for status of an element in the _addresses array.

Recommended Mitigation Steps

A more efficient solution would be to use (OpenZeppelin's EnumerableSet) https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/structs/EnumerableSet.sol

21. Use calldata instead of memory for external functions where the function argument is read-only

Impact

On external functions, when using the memory keyword with a function argument, what's happening is that a memory acts as an intermediate.
Reading directly from calldata using calldataload instead of going via memory saves the gas from the intermediate memory operations that carry the values.
As an extract from https://ethereum.stackexchange.com/questions/74442/when-should-i-use-calldata-and-when-should-i-use-memory :
«memory and calldata (as well as storage) are keywords that define the data area where a variable is stored. To answer your question directly, memory should be used when declaring variables (both function parameters as well as inside the logic of a function) that you want stored in memory (temporary), and calldata must be used when declaring an external function's dynamic parameters. The easiest way to think about the difference is that calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.»

Proof of Concept

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L199

Recommended Mitigation Steps

Use calldata instead of memory for external functions where the function argument is read-only.

[pauliax]

Duplicate issue by the same warden: #89