Biconomy Hyphen 2.0 contest - 0x1f8b's results

1. It seems that use an outdated version of openzeppelin, the current version is 4.5.0

"@openzeppelin/contracts": "4.3.0"

2. Lack of ACK during pauser change Because an human error it's possible to set a new invalid pauser. When you want to change the pauser's address it's better to propose a new pauser, and then accept this role with the new wallet.

• Pausable.sol#L52

3. Possible Denial of service by gas exhaustion

• LPToken.sol#L75
• WhitelistPeriodManager.sol#L246

4. Lack of input checks. If the owner set high values, the contract can be unusable, and the service could be denied.

• TokenManager.sol#L51-L52

5. Logic mismatch. The logic around removeExecutor doesn't work as expected. It require to remove the address from the executors array, otherwise, the executor will be returned at getAllExecutors

• ExecutorManager.sol#L55

6. Lack of input checks. Is not checked that the address is a contract distinct to address(0).

• WhitelistPeriodManager.sol#L61-L65
• WhitelistPeriodManager.sol#L159
• WhitelistPeriodManager.sol#L167
• WhitelistPeriodManager.sol#L175
• LiquidityFarming.sol#L79-L82
• LiquidityFarming.sol#L169
• LiquidityProviders.sol#L79-L82
• LiquidityProviders.sol#L116
• LiquidityProviders.sol#L127
• LiquidityProviders.sol#L131
• LiquidityProviders.sol#L148
• LiquidityProviders.sol#L163
• LiquidityProviders.sol#L171

7. Integer underflow, if _baseToken doesn't exists the methods getUpdatedAccTokenPerShare and getRewardRatePerSecond are denied.

• LiquidityFarming.sol#L269
• LiquidityFarming.sol#L334

8. The method setBaseGas should emit an event in order to be able to detect this call by dapps. It also lacks any input verification and can incur disproportionate costs to the user if it is misconfigured.

1. It's possible to avoid storage access a save gas using immutable keyword for the following variables:

• ERC2771Context.sol#L11

2. Use delete instead of set to default value (false or 0)

• TokenManager.sol#L102
• ExecutorManager.sol#L55
• When _status it's false in WhitelistPeriodManager.sol#L181
• LiquidityPool.sol#L387
• LiquidityPool.sol#L378

3. It's possible to optimize the following structs in order to save storage slots:

• LiquidityFarming.sol#L33

struct NFTInfo {
        address payable staker;
        uint256 rewardDebt;
        uint256 unpaidRewards;
        bool isStaked; // <- Move close to the address
    }

4. Cache the constant value keccak256(xxxxxxxxxxxx) as a constant

• LiquidityFarming.sol#L343