FactoryDAO contest - Funen's results

Low

1. Correct code for ooner on VoterId

In this particular code on constructor was intended to be the owner of this contract and has declared as being ooner instead. This informational word can be changed synonyms owner as actual code should be and can be declared as holder . It can be found in this contract too cause ooner and holder it has to be the same as owners[tokenId]. So it can be changed for ooner to holder instead.

##Tool Used Manual Review, Remix

2. Different compiler version

Since This contract : PermissionlessBasicPoolFactory can be found error as was

ParserError: Source file requires different compiler version

so it would be error to compile since interface was using 0.8.9, so it can be changed into the same pragma version to avoid this error as dev intend to be.

##Tool Used Manual Review, Remix

Non Critical

3. Unnecessary comment

notice was not declaring much and unnecessary, it can be deleted instead or it can be changed for more information. Since it was done at /// @notice This contract is .

##Tool Used Manual Review

1. Title : Set Value as immutable for saving more gas

This value can be set as immutable since gateMaster which causes to be read-only, but assignable in the constructor

##Tool Used Remix, Manual Review

##Recommended Mitigation add immutable

2. Title : Saving gas by removing = 0

If a variable was not set/initialized, it is assumed to have default value to 0 this implementation was used for saving more gas by removing = 0

##TOOLS USED Remix, Manual Review

##Mitigation Step Remove = 0

##Occurances PermissionlessBasicPoolFactory.sol#L251 PermissionlessBasicPoolFactory.sol#L268 MerkleVesting.sol#L16 VoterID.sol#L69 VoterID.sol#L319 VoterID.sol#L323 VoterID.sol#L326

3. Title : Code can be shorter for saving more gas

This numIdentities and balances[thisOwner Implementation can be used for saving more gas, instead of doube caching, it can be changed by using += instead.

##POC https://www.tutorialspoint.com/solidity/solidity_operators.htm

##Tool Used Manual Review, Remix

##Recommended Mitigation

        numIdentities += 1;

and

        balances[thisOwner] += 1;

4. Title : using ++i than i++ for saving more gas

Using i++ instead ++i for all the loops, the variable i is incremented using i++. It is known that implementation by using ++i costs less gas per iteration than i++.

Tools Used

Manual Review, Remix

Occurances

PermissionlessBasicPoolFactory.sol#L115 PermissionlessBasicPoolFactory.sol#L141 PermissionlessBasicPoolFactory.sol#L168 PermissionlessBasicPoolFactory.sol#L224 PermissionlessBasicPoolFactory.sol#L249 PermissionlessBasicPoolFactory.sol#L266

5. Title : change uint i = 0 into uint i for saving more gas

using this implementation can saving more gas for each loops.

##Tool Used Manual Review & Remix

##Recommended Mitigation Change it

##Occurances

PermissionlessBasicPoolFactory.sol#L115 PermissionlessBasicPoolFactory.sol#L141 PermissionlessBasicPoolFactory.sol#L168 PermissionlessBasicPoolFactory.sol#L224 PermissionlessBasicPoolFactory.sol#L249 PermissionlessBasicPoolFactory.sol#L266

6. Caching array length can saving more gas

this implementation can be saving more gas, since if caching the array length is more gas efficient. This is because access to a local variable in solidity is more efficient.

##Tool Used Manual Review

##Occurances

PermissionlessBasicPoolFactory.sol#L115 PermissionlessBasicPoolFactory.sol#L141 PermissionlessBasicPoolFactory.sol#L168 PermissionlessBasicPoolFactory.sol#L224 PermissionlessBasicPoolFactory.sol#L249 PermissionlessBasicPoolFactory.sol#L266

7. Instead cathing in memory used calldata for saving more gas

Since params proof in MerkleEligibility.sol#L70 and MerkleEligibility.sol#L85 was used memory instead calldata Since was calldata is the cheapest location to use, but it has a limited size. In particular, that means that functions may be limited in their number of arguments. Notable implementation details about calldata are as follows:

It can only be used for function declaration parameters (and not function logic)
It is immutable (it can't be overwritten and changed)
It must be used for dynamic parameters of an external function
It is non-persistent (the value does not persist after the transaction has completed)

##POC https://ethereum.stackexchange.com/questions/74442/when-should-i-use-calldata-and-when-should-i-use-memory

##Tool Used Manual Review, Remix

##Recomended Mitigation change memory into calldata

8. Reorder Struct for cost less gas

This struct can be reorder for saving gas, implementation below can described how much gas saving for doing it(+-20 gas).

##Tool used Remix

##Recomended Mitigation

    struct Receipt {
        address owner;  // the owner of the deposit
        uint id;   // primary key
        uint amountDepositedWei;  // amount of tokens originally deposited
        uint timeDeposited;  // the time the deposit was made
        uint timeWithdrawn;  // the time the deposit was withdrawn, or 0 if not withdrawn yet
    }

    // this represents a single staking pool with >= 1 reward tokens
    struct Pool {
        uint id; // primary key
        uint[] rewardsWeiPerSecondPerToken; // array of reward rates, this number gets multiplied by time and tokens (not wei) to determine rewards
        uint[] rewardsWeiClaimed;  // bookkeeping of how many rewards have been paid out for each token
        uint[] rewardFunding;  // bookkeeping of how many rewards have been supplied for each token
        uint startTime;  // the time that the pool begins
        uint endTime;    // time that the pool ends
        uint maximumDepositWei;  // the size of the pool, maximum sum of all deposits
        uint totalDepositsWei;  // current sum of all deposits
        uint numReceipts;  // number of receipts issued
        uint taxPerCapita;  // portion of rewards that go to the contract creator
        address depositToken;  // token that user deposits (stakes)
        address excessBeneficiary;  // address that is able to reclaim unused rewards
        address[] rewardTokens;  // array of token contract addresses that stakers will receive as rewards
        mapping (uint => Receipt) receipts;  // mapping of receipt ids to receipt structs
    }

// 3142978 gas deploy -> Before
// 2733024 tx cost -> before
// 2733024 ex cost -> before

// 3142950 gas deploy -> after (-20)
// 2733000 tx cost -> after(-24)
// 2733000 tx cost -> after(-24)

9. Set Value as constants

This perCapita can be set as constant and saving more gas

##Tool Used Manual Review

10. Used ++pool.numReceipts can saving more gas

This implementation using ++pool.numReceipts can be saving more gas instead. since prefix was more cheaper than postfix.

##Tool Used Remix