FactoryDAO contest - rfa's results

##LOW

Title: No check that rewardsWeiPerSecondPerToken != 0

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L105

In case it allowed to input 0 as rewardsWeiPerSecondPerToken, There are no reward user can claim as L169, and it doesn't incentivise the holders as it was expected in the docs. I recommend to check that there is no 0 value on rewardsWeiPerSecondPerToken array

RECOMMENDED MITIGATION STEP

    function addPool (
        uint startTime,
        uint maxDeposit,
        uint[] memory rewardsWeiPerSecondPerToken,
        uint programLengthDays,
        address depositTokenAddress,
        address excessBeneficiary,
        address[] memory rewardTokenAddresses,
        bytes32 ipfsHash,
        bytes32 name
    ) external {

	for(uint i; i < rewardsWeiPerSecondPerToken.length; ++i){
	   require(rewardsWeiPerSecondPerToken[i] != 0);
	}
        Pool storage pool = pools[++numPools];
        pool.id = numPools;
        pool.rewardsWeiPerSecondPerToken = rewardsWeiPerSecondPerToken;
        pool.startTime = startTime > block.timestamp ? startTime : block.timestamp;
        pool.endTime = pool.startTime + (programLengthDays * 1 days);
        pool.depositToken = depositTokenAddress;
        pool.excessBeneficiary = excessBeneficiary;
        pool.taxPerCapita = globalTaxPerCapita;

Title: Missing check that programLengthDays != 0

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L107

If programLengthDays == 0, the pool is just gonna be useless because no one can deposit

Title: Using SafeERC20 library for IERC20

Occurrences: https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L144 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L198 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L230-L233 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L252 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L269

There are some token which are not implementing current ERC20 standard (example: USDT, OmiseGo and BNB). Using SafeERC20 library will be nice to prevent them stuck when the transaction failed.

##GAS

Title: Initializing var with default value

Occurrences: https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L115 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L168

By declaring var by not set its default value (0 for uint) can save deployment gas cost Change to:

(uint i; i < rewardTokenAddresses.length; i++)

Title: Using prefix increment and unchecked for i in a for() loop

Occurrences: https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L115 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L168

Using prefix increment and unchecked for i can save execution gas fee:

for (uint i = 0; i < rewardTokenAddresses.length;) {
            pool.rewardTokens.push(rewardTokenAddresses[i]);
            pool.rewardsWeiClaimed.push(0);
            pool.rewardFunding.push(0);
            taxes[numPools].push(0);
	unchecked{++i;}
        }

Title: Putting require() earlier possible

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L112

By putting the line that validating the parameters before doing any SSTORE can save a lot of gas in case it's reverted

RECOMMENDED MITIGATION STEP

        require(rewardsWeiPerSecondPerToken.length == rewardTokenAddresses.length, 'Rewards and reward token arrays must be same length');

        Pool storage pool = pools[++numPools];
        pool.id = numPools;
        pool.rewardsWeiPerSecondPerToken = rewardsWeiPerSecondPerToken;
        pool.startTime = startTime > block.timestamp ? startTime : block.timestamp;
        pool.endTime = pool.startTime + (programLengthDays * 1 days);
        pool.depositToken = depositTokenAddress;
        pool.excessBeneficiary = excessBeneficiary;
        pool.taxPerCapita = globalTaxPerCapita;

Title: Effective way to make receipt in deposit()

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L190-L192

By using prefix increment for pool.numReceipt and put it directly as a key at pool.receipts[] can save gas (save 1 SLOAD). As L103 was implemented

RECOMMENDED MITIGATION STEP

        Receipt storage receipt = pool.receipts[++pool.numReceipts]; // @audit-info: remove L190 then do increment here

Title: Using calldata to store array as a parameter

Occurrences: https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L95 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L99

Storing read only array with calldata is more effective for gas opt than using memory