Joyn contest - 0x1f8b's results

1. There are a lack of input checks around the contracts:

• CoreCollection.sol#L88-L96
• CoreCollection.sol#L125-L126
• CoreCollection.sol#L193 and CoreCollection.sol#L209 could be critical because only can be assigned once.
• CoreFactory.sol#L28-L29

2. It was found some transfer, approve or transferFrom without checking the boolean result, ERC20 standard specify that the token can return false if this call was not made, so it's mandatory to check the result of approve methods.

• CoreCollection.sol#L175
• ERC721Payable.sol#L54

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

• Collection.sol#L5

struct Collection {
    bool isForSale;
    uint256 maxSupply;
    uint256 mintFee;
    string baseURI;
    string name;
    string symbol;
    string id;
    bytes32 claimsMerkleRoot;
    address payableToken; // <- Move close to isForSale
}

• MultiSigWallet.sol#L32-L36

struct Transaction {
    address destination;
    uint256 value;
    bytes data;
    bool executed; // <- move close to address
}

2. Change the incremental logic from i++ to ++i in order to save some opcodes:

• CoreCollection.sol#L279
• CoreFactory.sol#L79
• MultiSigWallet.sol#L98
• MultiSigWallet.sol#L127
• MultiSigWallet.sol#L147
• MultiSigWallet.sol#L268-L269
• MultiSigWallet.sol#L314
• MultiSigWallet.sol#L330
• MultiSigWallet.sol#L357
• MultiSigWallet.sol#L363
• MultiSigWallet.sol#L384
• MultiSigWallet.sol#L393
• Splitter.sol#L50
• Splitter.sol#L274

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

• CoreFactory.sol#L28-L29
• RoyaltyVaultFactory.sol#L28
• SplitFactory.sol#L63

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

• MultiSigWallet.sol#L126
• MultiSigWallet.sol#L152
• MultiSigWallet.sol#L206

5. It's compared a boolean value using == true or == false, instead of using the boolean value, or NOT opcode, it's cheaper to use NOT when the value it's false, or just the value without == true, when it's true, because it will use less opcode inside the VM.

• RoyaltyVault.sol#L44
• RoyaltyVault.sol#L48
• RoyaltyVault.sol#L55