Biconomy Hyphen 2.0 contest - gzeon's results

Upgrade Solidity Version

Recommended to upgrade to latest Solidity 0.8.12

Upgrade ERC2771Context to latest OZ implementation

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/metatx/ERC2771Context.sol to https://github.com/OpenZeppelin/openzeppelin-contracts/blob/52eeebecda140ebaf4ec8752ed119d8288287fac/contracts/metatx/ERC2771Context.sol

Upgrade ERC2771ContextUpgradeable to latest OZ implementation

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/metatx/ERC2771ContextUpgradeable.sol to https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/3dec82093ea4a490d63aab3e925fed4f692909e8/contracts/metatx/ERC2771ContextUpgradeable.sol

No cap on fee parameters

Consider adding caps to fee parameters to reduce rug risk https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/token/TokenManager.sol#L44

Extra space

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

            " ERR_ARRAY_LENGTH_MISMATCH"

Make constants public

Consider make them public for easier external consumption https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L68 https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L19-20

removeExecutor lack check to see if the address is registered

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

Use BASE_DIVISOR constant instead of 10000000000

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

increaseNativeLiquidity lack msg.value != 0 check

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

Consider remove setLpToken function

Owner can call setLpToken to change the value of lpToken in WhitelistPeriodManager, which will make all onlyLpNft function revert https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/WhitelistPeriodManager#L170-176

    function _setLpToken(address _lpToken) internal {
        lpToken = ILPToken(_lpToken);
    }

    function setLpToken(address _lpToken) external onlyOwner {
        _setLpToken(_lpToken);
    }

Consider remove setLiquidityProviders function

Owner can call setLpToken to change the value of lpToken in WhitelistPeriodManager, which will make all onlyLiquidityPool function revert https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/WhitelistPeriodManager#L162-168

    function _setLiquidityProviders(address _liquidityProviders) internal {
        liquidityProviders = ILiquidityProviders(_liquidityProviders);
    }

    function setLiquidityProviders(address _liquidityProviders) external onlyOwner {
        _setLiquidityProviders(_liquidityProviders);
    }

Use custom errors

Solidity ^0.8.4 allow the use of custom errors to optimize gas usage. https://blog.soliditylang.org/2021/04/21/custom-errors/

> 0 is less efficient than != 0 for uint in require condition

Ref: https://twitter.com/GalloDaSballo/status/1485430908165443590

./contracts/hyphen/LiquidityProviders.sol:239:        require(_amount > 0, "ERR__AMOUNT_IS_0");
./contracts/hyphen/LiquidityProviders.sol:283:        require(_amount > 0, "ERR__AMOUNT_IS_0");
./contracts/hyphen/LiquidityProviders.sol:410:        require(lpFeeAccumulated > 0, "ERR__NO_REWARDS_TO_CLAIM");

Variables can be set immutable

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/metatx/ERC2771Context.sol#L11 (contract out of scope but inherited by other contract in scope)

    address internal immutable _trustedForwarder;

Optimize struct layout

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

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

Use uint(1) instead of bool(true)

Use uint(1) instead of bool(true) to save gas since evm work with 256-bits word

contracts/hyphen/ExecutorManager.sol:10:    mapping(address => bool) internal executorStatus;
contracts/hyphen/WhitelistPeriodManager.sol:23:    mapping(address => bool) public isExcludedAddress;
contracts/hyphen/LiquidityPool.sol:37:    mapping(bytes32 => bool) public processedHash;

Float multiplication optimization

We can use the following function to save gas on float multiplications

// out = x * y unchecked{/} z
function fmul(uint256 x, uint256 y, uint256 z) internal pure returns(uint256 out){
assembly{
   if iszero(eq(div(mul(x,y),x),y)) {revert(0,0)}
   out := div(mul(x,y),z)
}
}

https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L163 https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L318 https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L324 https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityPool.sol#L326

For loop optimization

Instead of

for (uint256 i = 1; i <= something; ++i){...}

we can do

for (uint256 i = 1; i <= something; ){...unchecked{++i;}}

contracts/hyphen/token/TokenManager.sol:78:        for (uint256 index = 0; index < tokenConfig.length; ++index) {
contracts/hyphen/token/LPToken.sol:77:        for (uint256 i = 0; i < nftIds.length; ++i) {
contracts/hyphen/token/svg-helpers/SvgHelperBase.sol:31:            ++count;
contracts/hyphen/token/svg-helpers/SvgHelperBase.sol:45:        for (uint256 i = 0; i < _length; ++i) {
contracts/hyphen/ExecutorManager.sol:31:        for (uint256 i = 0; i < executorArray.length; ++i) {
contracts/hyphen/ExecutorManager.sol:47:        for (uint256 i = 0; i < executorArray.length; ++i) {
contracts/hyphen/LiquidityFarming.sol:233:        for (index = 0; index < nftsStakedLength; ++index) {
contracts/hyphen/WhitelistPeriodManager.sol:180:        for (uint256 i = 0; i < _addresses.length; ++i) {
contracts/hyphen/WhitelistPeriodManager.sol:228:        for (uint256 i = 0; i < _tokens.length; ++i) {
contracts/hyphen/WhitelistPeriodManager.sol:248:        for (uint256 i = 1; i <= totalSupply; ++i) {

Use cached value

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

        nftIdsStaked[msgSender][index] = nftIdsStaked[msgSender][nftsStakedLength - 1];

Unchecked safe math

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

        unchecked{
            totalSharesStaked[baseToken] -= amount;
        }

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

    function addLPFee(address _token, uint256 _amount) external onlyLiquidityPool tokenChecks(_token) whenNotPaused {
        totalReserve[_token] += _amount;
        unchecked{ // fee should always be less than reserve
            totalLPFees[_token] += _amount;
        }
        emit FeeAdded(_token, _amount);
    }

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

        totalLPFees[_tokenAddress] -= lpFeeAccumulated;
        unchecked{ // fee should always be less than reserve
            totalReserve[_tokenAddress] -= lpFeeAccumulated;
        }
        totalSharesMinted[_tokenAddress] -= lpSharesRepresentingFee;

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

        totalLiquidity[_token] += _amount;
        unchecked{
            totalLiquidityByLp[_token][_lp] += _amount;
        }

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

        totalLiquidityByLp[_token][_lp] -= _amount;
        unchecked{
            totalLiquidity[_token] -= _amount;
        }

Use >= instead of >

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

        if(nftSuppliedLiquidity >= eligibleLiquidity) {
            lpFeeAccumulated = 0;
        } else {
            unchecked {
                lpFeeAccumulated = eligibleLiquidity - nftSuppliedLiquidity;
            }
        }