Biconomy Hyphen 2.0 contest - wuwe1's results

In LiquidityFarming.sol:_sendRewardsForNft, _to’s existence should be checked before call

Low-level calls call/delegatecall/staticcall return true even if the account called is non-existent (per EVM design). Account existence must be checked prior to calling if needed.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L122

In LiquidityProviders.sol:addNativeLiquidity, liquidityPool’s existence should be checked before call

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L251

In LiquidityProviders.sol:increaseNativeLiquidity, liquidityPool’s existence should be checked before call

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L336

Cache array length in for loops can save gas

Caching the array length first saves an SLOAD  on each iteration of the loop.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L31

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L47

Caching the array length first saves an  MLOAD  on each iteration of the loop.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L179-L180

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L228

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

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

Gas: No need to initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value. Explicitly initializing it with its default value is an anti-pattern and wastes gas

contracts/hyphen/LiquidityPool.sol
355:            fee = 0;

contracts/hyphen/LiquidityFarming.sol
233:        for (index = 0; index < nftsStakedLength; ++index) {
266:        uint256 accumulator = 0;

contracts/hyphen/WhitelistPeriodManager.sol
180:        for (uint256 i = 0; i < _addresses.length; ++i) {
228:        for (uint256 i = 0; i < _tokens.length; ++i) {
247:        uint256 maxLp = 0;

contracts/hyphen/LiquidityProviders.sol
215:            lpFeeAccumulated = 0;
367:            lpFeeAccumulated = 0;

contracts/hyphen/ExecutorManager.sol
31:        for (uint256 i = 0; i < executorArray.length; ++i) {
47:        for (uint256 i = 0; i < executorArray.length; ++i) {

contracts/hyphen/token/LPToken.sol
77:        for (uint256 i = 0; i < nftIds.length; ++i) {

contracts/hyphen/token/TokenManager.sol
78:        for (uint256 index = 0; index < tokenConfig.length; ++index) {

contracts/hyphen/token/svg-helpers/SvgHelperBase.sol
29:        uint256 count = 0;
45:        for (uint256 i = 0; i < _length; ++i) {
68:        uint256 leadingZeroesToAddBeforeDecimal = 0;

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

contracts/hyphen/LiquidityFarming.sol
132:        if (amount > 0) {
318:            if (totalSharesStaked[_baseToken] > 0) {

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

contracts/hyphen/token/svg-helpers/SvgHelperBase.sol
30:        while (_number > 0) {
76:                if (powerRemaining > 0) {

LiquidityProvider.sol:getTokenPriceInLPShares: supply only used once and shouldn’t get cached

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L181

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

Use calldata instead of memory for external functions where the function argument is read-only. This can save some MLOAD and stack operations.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/token/TokenManager.sol#L70-L72

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L178

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L220-L222

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

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L245

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L267

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L365

Unused named returns

Removing unused named return variables can reduce gas usage (MSTOREs/MLOADs) and improve code clarity. To save gas and improve code quality: consider using only one of those.

contracts/hyphen/token/TokenManager.sol
139:    function _msgSender() internal view virtual override(Context, ERC2771Context) returns (address sender) {

contracts/hyphen/LiquidityFarming.sol
351:        returns (address sender)

contracts/hyphen/LiquidityFarming.sol
351:        returns (address sender)

contracts/hyphen/metatx/ERC2771ContextUpgradeable.sol
29:    function _msgSender() internal view virtual override returns (address sender) {

Missing checks for non-zero transfer value calls

Checking non-zero transfer values can avoid an external call to save gas.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L251

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L336

consider add require(msg.value>0)

SLOAD minimizing by using calldata

contracts/hyphen/token/TokenManager.sol:53

		function changeFee(
        address tokenAddress,
        uint256 _equilibriumFee,
        uint256 _maxFee
    ) external override onlyOwner whenNotPaused {
        require(_equilibriumFee != 0, "Equilibrium Fee cannot be 0");
        require(_maxFee != 0, "Max Fee cannot be 0");
        tokensInfo[tokenAddress].equilibriumFee = _equilibriumFee;
        tokensInfo[tokenAddress].maxFee = _maxFee;
        emit FeeChanged(tokenAddress, tokensInfo[tokenAddress].equilibriumFee, tokensInfo[tokenAddress].maxFee);
    }

can be changed to

emit FeeChanged(tokenAddress, _equilibriumFee, _maxFee);

MLOAD minimizing

contracts/hyphen/token/TokenManager.sol:79

depositConfig[toChainId[index]][tokenAddresses[index]].min = tokenConfig[index].min;
depositConfig[toChainId[index]][tokenAddresses[index]].max = tokenConfig[index].max;

can be changed to

address token = tokenAddress[index];
uint256 chainId = toChainId[index];
depositConfig[chainId][token].min = tokenConfig[index].min;
depositConfig[chainId][token].max = tokenConfig[index].max;

Use cached variable to save SLOAD

contracts/hyphen/LiquidityProviders.sol
183:            return totalSharesMinted[_baseToken] / totalReserve[_baseToken];

consider change to

return supply / totalReserve[_baseToken];

Redundant field in TokenInfo: tokenConfig

The only place this field is used is in contracts/hyphen/token/TokenManager.sol:96

tokensInfo[tokenAddress].tokenConfig = transferConfig[tokenAddress];

Consider remove this field to save a SSTORE

Conditional flow optimization in LiquidityProviders

				// Handle edge cases where eligibleLiquidity is less than what was supplied by very small amount 
        if(nftSuppliedLiquidity > eligibleLiquidity) {
            lpFeeAccumulated = 0;
        } else {
            unchecked {
                lpFeeAccumulated = eligibleLiquidity - nftSuppliedLiquidity;
            }
        }

Consider change to

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

to avoid calculation when eligibleLiquidity==nftSuppliedLiquidity

Cache result of getDepositConfig

contracts/hyphen/LiquidityPool.sol
157:            tokenManager.getDepositConfig(toChainId, tokenAddress).min <= amount &&
158:                tokenManager.getDepositConfig(toChainId, tokenAddress).max >= amount,
248:            tokenManager.getDepositConfig(toChainId, NATIVE).min <= msg.value &&
249:                tokenManager.getDepositConfig(toChainId, NATIVE).max >= msg.value,

Consider cache the result using stack variable to save SLOAD

Save SLOAD by caching result of getTokenPriceInLPShares

removeLiquidity of LiquidityProviders invokes getTokenPriceInLPShares twice. Consider cache the result on a stack variable.

Save SLOAD by caching equilibriumFee

function getAmountToTransfer(
        uint256 initialGas,
        address tokenAddress,
        uint256 amount,
        uint256 tokenGasPrice
    ) internal returns (uint256 amountToTransfer) {
        uint256 transferFeePerc = getTransferFee(tokenAddress, amount);
        uint256 lpFee;
        if (transferFeePerc > tokenManager.getTokensInfo(tokenAddress).equilibriumFee) { //@<- 1
            // Here add some fee to incentive pool also
            lpFee = (amount * tokenManager.getTokensInfo(tokenAddress).equilibriumFee) / BASE_DIVISOR; //@<- 2
            incentivePool[tokenAddress] =
                (incentivePool[tokenAddress] +  // @vvvvvvvvvv 3
                    (amount * (transferFeePerc - tokenManager.getTokensInfo(tokenAddress).equilibriumFee))) /
                BASE_DIVISOR;
        } else {
            lpFee = (amount * transferFeePerc) / BASE_DIVISOR;
        }
        uint256 transferFeeAmount = (amount * transferFeePerc) / BASE_DIVISOR;

        liquidityProviders.addLPFee(tokenAddress, lpFee);

        uint256 totalGasUsed = initialGas - gasleft(); // @vvvvv 4
        totalGasUsed = totalGasUsed + tokenManager.getTokensInfo(tokenAddress).transferOverhead;
        totalGasUsed = totalGasUsed + baseGas;

        uint256 gasFee = totalGasUsed * tokenGasPrice;
        gasFeeAccumulatedByToken[tokenAddress] = gasFeeAccumulatedByToken[tokenAddress] + gasFee;
        gasFeeAccumulated[tokenAddress][_msgSender()] = gasFeeAccumulated[tokenAddress][_msgSender()] + gasFee;
        amountToTransfer = amount - (transferFeeAmount + gasFee);

        emit FeeDetails(lpFee, transferFeeAmount, gasFee);
    }

getAmountToTransfer calls getTokensInfo 4 times.

Consider cache the used field into stack variable.

Cache _msgSender() to stack variable

contracts/hyphen/LiquidityPool.sol
336:        gasFeeAccumulated[tokenAddress][_msgSender()] = gasFeeAccumulated[tokenAddress][_msgSender()] + gasFee;

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L372-L392

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L262-L310

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L317-L327

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L345-L395

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L401-L421

Consider cache _msgSender using stack variable when invoke more than once.