Biconomy Hyphen 2.0 contest - samruna's results

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L10 The executorstatus map is not really helpful in any business logic. If the executor is present in executors array then the status is true or else false. Once the executor is added, it cannot be added again even though the status is set to FALSE.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L53 The function removeExecutor is just setting the status to false.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L55 Check if the executor is present and executorStatus is not already set to false.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/token/svg-helpers/SvgHelperBase.sol#L126 Both inpurt parameters are not used in the function. Remove these.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L169 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L330 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L333 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L107 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L113 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L123 If function is not called from inside the contract, better to make it external

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L211 NFTInfo storage nft = nftInfo[_nftId]; require(!nft.isStaked, "ERR__NFT_ALREADY_STAKED"); Move above code immidiately after the require(lpToken.isApprovedForAll...). If NFT is not approved, then no need to get token metadata

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L30 Consider adding require(!executorStatus[executorArray[i]], "Executor already registered"); in the for loop before calling addExecutor. This will make the transaction faster and will save some gas fees.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/ExecutorManager.sol#L55 Check if the executor is present and executorStatus is not already set to false. I can call the function in an infinity loop and it can block other transactions to executed while(i == 1){ executorManager.removeExecutor(address) }

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L99 Add zero check for rewardPerSecond. EVen though onlyOwner, it can create zero records and can exhust gas fees

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L170 Check if the baseToken is already added to rewardRateLog. Unless the business logic is to allow override, the emit will exhuast gas fees.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L182 Add zero check for amount. No point in transfering 0 amount. This will cost extra gas fees.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L113 Add zero check before setting baseGas.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L201 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L225 Add zero check. Although zero check if done in functions called further, having zero check at the begainning will definitly help save some gas