FactoryDAO contest - hansfriese's results

Non-critical Issues

1. NatSpec is incomplete @param is missing MerkleIdentity.sol#L140 VoterID.sol#L411 @return is missing MerkleIdentity.sol#L163 VoterID.sol#L397 VoterID.sol#L402

2. Spelling issues PermissionlessBasicPoolFactory.sol#L183 -> "Cannot deposit before pool starts"

3. Wrong comment PermissionlessBasicPoolFactory.sol#L205 Only receipt owner may call this function while pool is alive but anyone can do after pool ends

4. Wrong error message PermissionlessBasicPoolFactory.sol#L214 -> "Can only withdraw your own deposit while pool is alive"

1. use != 0 instead of > 0 FixedPricePassThruGate.sol#L51 MerkleVesting.sol#L147 VoterID.sol#L348

2. use ++i instead of i++, i+=1 MerkleEligibility.sol#L47 MerkleLib.sol#L22 PermissionlessBasicPoolFactory.sol#L115 PermissionlessBasicPoolFactory.sol#L141 PermissionlessBasicPoolFactory.sol#L168 PermissionlessBasicPoolFactory.sol#L190 PermissionlessBasicPoolFactory.sol#L224 PermissionlessBasicPoolFactory.sol#L249 PermissionlessBasicPoolFactory.sol#L266 VoterID.sol#L130 VoterID.sol#L140 VoterID.sol#L333

3. check require() at the start time of function You can move this line to the first line of function PermissionlessBasicPoolFactory.sol#L112

4. no need to initialize variables with default values MerkleDropFactory.sol#L17 MerkleEligibility.sol#L31 MerkleLib.sol#L22 MerkleResistor.sol#L24 MerkleResistor.sol#L176 MerkleVesting.sol#L16 MerkleVesting.sol#L150 PermissionlessBasicPoolFactory.sol#L115 PermissionlessBasicPoolFactory.sol#L141 PermissionlessBasicPoolFactory.sol#L168 PermissionlessBasicPoolFactory.sol#L224 PermissionlessBasicPoolFactory.sol#L249 PermissionlessBasicPoolFactory.sol#L266 VoterID.sol#L69

5. change storage to memory if possible PermissionlessBasicPoolFactory.sol#L157 PermissionlessBasicPoolFactory.sol#L262 PermissionlessBasicPoolFactory.sol#L280 PermissionlessBasicPoolFactory.sol#L293 PermissionlessBasicPoolFactory.sol#L306 PermissionlessBasicPoolFactory.sol#L307

6. you can add require() to avoid unnecessary calculations MerkleDropFactory.sol#L68 you can add this require at #L69 require(treeIndex <= numTrees, "Provided merkle index doesn't exist");

7. you can check success inside "for" loop to avoid unnecessary calculations PermissionlessBasicPoolFactory.sol#L141-L147 PermissionlessBasicPoolFactory.sol#L224-L231 PermissionlessBasicPoolFactory.sol#L249-L253 PermissionlessBasicPoolFactory.sol#L266-L270

Currently it iterates all of the loop even if success = false in the middle. It would be good to check require() for success inside the "for" loop after transaction so that it can revoke without doing unnecessary calculations anymore. require(success, 'Token transfer failed');